From a25909a459c87a5b94d16659060f218b8dc2e11e Mon Sep 17 00:00:00 2001 From: JasonFraser Date: Sat, 4 Jul 2026 22:03:54 -0400 Subject: [PATCH] Add per-seat trip watchlist, fix cross-URL caching bug, and improve results table UX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The biggest fix: change-detection and the fast-path cache lookup were scoped only by competitor_id, not by which specific trip page was scraped. Since trip-intent matching lets a PM scrape a different URL per run, get_last_hash_for_url() compared each new scrape against whatever page happened to be hashed most recently for that competitor — almost always a different page — so change_detected never settled back to False and the true no-scrape fast path was unreachable in practice. Scoped scrape_runs/products lookups by (competitor_id, url) instead, and threaded a cleaned confirmed_url consistently through needs_refresh(), detect_change(), and analyse_from_cache(). Verified end-to-end with a regression test reproducing the exact reported scenario. New: a per-seat, tier-capped trip watchlist (1 trip/seat at Analyse). The nightly cron now re-scrapes only the specific trips a PM chose to track instead of every competitor's homepage (a page already flagged low-signal by is_generic_homepage()) — bounded, predictable volume, and every resulting alert is about a trip someone actually cares about. Reuses the existing confirmed_url/per-url caching machinery with zero changes needed to jobs.py's job loop. Also fixes a real gap: auth_service.py resolved the caller's active seat only to validate the JWT and then discarded it — every route before this only ever needed tenant_id. get_seat_from_request() now returns it. Also fixed the Firecrawl /map timeout (15s → 30s; a large real site's sitemap fetch took ~21s and was silently read as "no match found"), a job left stuck at "Stopping…" forever after a worker process was killed mid-run (status endpoint now self-heals via the underlying RQ job state), and several results-table issues: hardcoded "G Adventures" in the extraction prompt and a literal "G" in the comments column label (replaced with the real tenant name), non-clickable trip links, a cache-source badge that was silently clipped on most columns, a missing view into the client's own scraped trip data, and bolding for Strengths/Weaknesses/Differences/Pricing in the comments text. 370+ new/updated backend tests; full suite green. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 149 +++++++++- backend/api.py | 195 ++++++++++++- backend/industries/adventure_travel/prompt.py | 143 ++++++++- backend/jobs.py | 24 +- .../pb_migrations/006_add_own_trip_fields.js | 39 +++ .../007_add_scrape_runs_scraped_url.js | 40 +++ .../pb_migrations/008_add_trip_watchlist.js | 62 ++++ backend/repositories/product_repository.py | 27 ++ backend/repositories/scrape_repository.py | 27 +- backend/repositories/seat_repository.py | 8 + backend/repositories/watchlist_repository.py | 74 +++++ backend/services/analysis_service.py | 196 ++++++++++--- backend/services/auth_service.py | 42 ++- backend/services/embedding_service.py | 111 ++++++- backend/services/trip_finder_service.py | 29 +- backend/services/watchlist_service.py | 88 ++++++ backend/tests/conftest.py | 3 +- .../repositories/test_real_repositories.py | 30 +- .../tests/services/test_analysis_service.py | 225 +++++++++++++- backend/tests/services/test_auth_service.py | 54 ++++ .../tests/services/test_embedding_service.py | 113 ++++++++ backend/tests/services/test_jobs.py | 98 +++++++ .../services/test_trip_finder_service.py | 40 +++ .../tests/services/test_watchlist_service.py | 139 +++++++++ backend/tests/test_api.py | 47 +++ backend/tests/test_api_extended.py | 274 +++++++++++++++++- .../tests/test_industries_adventure_travel.py | 46 +++ .../src/components/activity/WatchlistCard.vue | 55 ++++ .../src/components/analyse/ResultsTable.vue | 165 ++++++++++- .../src/components/analyse/SyncToSheet.vue | 7 +- .../components/analyse/UrlConfirmation.vue | 55 +++- frontend/src/config/resultFields.js | 16 +- frontend/src/pages/AccountPage.vue | 35 +++ frontend/src/pages/ActivityPage.vue | 20 +- frontend/src/pages/AnalysePage.vue | 64 +++- .../src/repositories/analysis_repository.js | 8 +- .../src/repositories/tenant_repository.js | 5 + .../src/repositories/watchlist_repository.js | 21 ++ n8n/workflows/daily_scrape_cron.json | 28 +- 39 files changed, 2669 insertions(+), 133 deletions(-) create mode 100644 backend/pb_migrations/006_add_own_trip_fields.js create mode 100644 backend/pb_migrations/007_add_scrape_runs_scraped_url.js create mode 100644 backend/pb_migrations/008_add_trip_watchlist.js create mode 100644 backend/repositories/watchlist_repository.py create mode 100644 backend/services/watchlist_service.py create mode 100644 backend/tests/services/test_watchlist_service.py create mode 100644 frontend/src/components/activity/WatchlistCard.vue create mode 100644 frontend/src/repositories/watchlist_repository.js diff --git a/CLAUDE.md b/CLAUDE.md index 5e9a34e..22f7916 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1250,6 +1250,71 @@ competitor with no real match from the batch entirely, instead of being pressured into typing a low-value guess (like the competitor's own homepage) just to unblock "Confirm & Run." +### ResultsTable.vue — clickable links, cache-source pill, own-trip column + +- The `link` row (every column, including the own-trip column below) now + renders as a real `` instead of + plain text, wherever a value is present — opens the actual scraped + page in a new tab. +- Every column header shows a subtle blue database SVG icon next to the + name whenever this run's data came from PocketBase (`path === 'fast'` + or `unchanged: true`) — a fresh live scrape gets no icon at all, since + that's the default expectation and doesn't need flagging. Gives the PM + direct, per-competitor visibility into which path §7's fast/refresh + decision actually took, rather than having to infer it. Went through + two iterations: first a text badge ("From PocketBase"), which was too + noisy and also got silently clipped on any column whose name+badge + combination didn't fit the shared `nowrap` + `text-overflow: ellipsis` + rule (only ever appeared on whichever competitor had the shortest + name) — replaced with an icon-only indicator, and header cells now + wrap (name renders in its own `div`) instead of truncating. +- A dedicated "🏠 {tenant name}" column renders BEFORE the first + competitor column whenever `context['own_trip']` was populated this + run (client self-scrape feature, §8's trip-intent matching applied to + the tenant's own site) — same row layout, same cache pill (based on + `own_trip.cached`, set by `embedding_service.scrape_own_trip()`), same + clickable link. Omitted entirely when no own-trip URL was confirmed + this run (no website configured, or explicitly skipped) — this was + previously invisible in the dashboard even when the feature was + working, since `own_trip` was only ever consumed internally by + `build_prompt()` and never returned to the frontend at all. + `jobs.py`'s `run_research_job()` now includes it as + `results['own_trip']` on the persisted `scrape_runs` record, so it + also survives into RunHistory restores. +- The `comments` row label was hardcoded to `"Suggested changes for + G/ Competitive Comments"` — a leftover literal "G" from "G Adventures" + (the hackathon reference client), showing every tenant's own table as + if it belonged to a company literally named "G". `resultFields.js`'s + label is now `"Suggested changes for {company}/ Competitive + Comments"`, substituted at render time via `formatLabel(label, + companyName)` — used by both `ResultsTable.vue` (display) and + `SyncToSheet.vue` (clipboard copy) via a new `companyName` prop each, + wired from `AnalysePage.vue`'s `authStore.tenant?.name`. +- `analyse_from_cache()`'s (§7 fast path) narrative prompt used to dump + the raw `context` dict (Python repr) into the prompt with no priority + framing — `own_trip`'s presence was easy for the model to under-weight + against the typed destination/duration/style fields sitting right next + to it in the same blob. It now explicitly builds a `client_context_desc` + string that's grounded in `context['own_trip']` (the tenant's own REAL + extracted trip) FIRST when present, falling back to the PM's typed + form fields only when no own-trip data was returned for this run — + matching `build_prompt()`'s existing own_trip-first priority on the + refresh path, so both paths ground `comments` the same way. +- The `comments` cell bolds every comparison-section label + `build_prompt()`'s own JSON schema instruction asks the LLM for — + "(Key) Differences:", "Strength(s):", "Weakness(es):", "Pricing + (Position):" (case-insensitive) — wherever they occur in every + column's comments text, so a PM can skim multiple competitors' analysis + at a glance. Includes the own-trip column, though that one is always + blank since `build_own_trip_prompt()` never asks for a `comments` + field (nothing to compare against at extraction time). `ResultsTable.vue`'s + `boldKeywords()` HTML-escapes the text first, then re-inserts only its + own `` tags around an exact keyword match, so `v-html` can't be + used to inject arbitrary markup even though the underlying text + originates from an LLM extraction. Purely a display change — + `SyncToSheet.vue`'s clipboard copy (plain text, pasted into the real + Sheet template) is untouched. + --- ## 8. Trip-Intent Matching @@ -3015,21 +3080,89 @@ GET /research/history/ — full results for a specific run by job_id ## 15. n8n Workflows -### Daily scrape (6am every day) +### Daily scrape (6am every day) — targets the trip watchlist, not competitor homepages + ``` Schedule Trigger (06:00 AST) → For each active tenant: - → Query PocketBase for competitors where: - change_detected = true OR last_scraped > 24hrs ago - → HTTP POST /internal/scrape/{tenant_id} with filtered competitor list - → RQ job queued → Playwright scrape → AI extraction - → PocketBase updated (products, price_history) - → change_detected reset to false - → last_scraped updated to now + → Query PocketBase for that tenant's trip_watchlist items (every + seat's watched trips — no staleness pre-filter; every watched + trip is sent every night) + → HTTP POST /internal/scrape/{tenant_id} with + competitors: [{ id: competitor_id, url: watched_url }, ...] — + one entry per watched trip (a competitor can appear more than + once if two seats watch two different trips on it) + → RQ job queued → jobs.py's confirmed_url threading scrapes the + SPECIFIC watched url per entry (analysis_service.needs_refresh()'s + per-url freshness check decides fast-vs-refresh internally, + exactly as a manual dashboard run would) + → PocketBase updated (products, price_history) for that specific trip → price_history diff calculated automatically → is_new flag set on new products automatically ``` +This replaced an earlier design that queried `competitors` directly +(`change_detected = true || last_scraped > 24hrs`) and re-scraped each +competitor's root `website` every night — a page +`services/url_utils.py`'s `is_generic_homepage()` already flags as +low-signal and non-trip-specific, with no natural bound on nightly +volume. The trip watchlist (below) replaces it with a small, predictable +ceiling and alerts that are always about a trip a PM explicitly chose to +track. + +### Trip watchlist — per-seat, tier-capped nightly monitoring + +Each **active seat** (not the tenant as a whole) can pin a small number +of specific already-analyzed competitor trips to a personal watchlist — +`trip_watchlist` PocketBase collection (migration 008), one row per +watched trip, `seat_id` + `competitor_id` + `url` + `trip_name`. The cap +is per-seat and tier-based +(`services/watchlist_service.py`'s `WATCHLIST_CAP_PER_SEAT`): + +```python +WATCHLIST_CAP_PER_SEAT = { + 'analyse': 1, # a fully-seated ($349, 5 seats) tenant tops out + # at 5 real trip scrapes/night — same ceiling as + # scraping every competitor once, but every + # alert is now about a trip a PM chose to track + 'discover': 2, # placeholder — Discover isn't built yet (§27) + 'intelligence': 3, # placeholder — layered on top of that tier's + 'enterprise': 3, # full catalogue auto-discovery, not instead of it +} +``` + +**Routes** (`api.py`): `GET /watchlist` returns every watched item for +the session's tenant (all seats', with the watching seat's email +denormalized for display — visibility is tenant-wide even though the +cap is per-seat); `POST /watchlist` (`{competitor_id, url, trip_name}`) +resolves the calling seat via `auth_service.get_seat_from_request()` and +enforces the cap, 400 `watchlist_full` with a clear message on breach; +`DELETE /watchlist/` removes an item — any active seat on the tenant +may remove any item, no per-seat ownership lock (matching this app's +existing no-fine-grained-permissions convention). + +`auth_service.get_tenant_id_from_request()` used to look up the caller's +active seat purely to validate the JWT, then discard it — every +authenticated route before this feature only ever needed `tenant_id`. +`get_seat_from_request()` now returns `(tenant_id, seat)` instead, with +`get_tenant_id_from_request()` kept as a thin backward-compatible +wrapper around it. + +**Frontend**: `ResultsTable.vue` gets a "☆ Watch" / "★ Watching" toggle +per competitor column (not shown on the own-trip column — watching your +own trip isn't meaningful), using the row-derived `link`/`tripName` +values so it works identically for fast-path and refresh-path results. +`ActivityPage.vue`'s new `WatchlistCard.vue` lists every tenant item +(trip name, watched-by email, remove button) plus an "X of Y slots +used" indicator for the current seat (mirrors +`WATCHLIST_CAP_PER_SEAT` client-side for display only — the server +enforces the real cap). + +Not built in this pass: no ad-hoc "watch any URL" entry (v1 only lets a +PM watch a trip that already appears in a completed analysis result), +and Discover/Intelligence's actual cap numbers are placeholders, not +load-bearing until those tiers are built. + ### Battlecard regeneration (after scrape) ``` PocketBase webhook on scrape_runs status=complete diff --git a/backend/api.py b/backend/api.py index 95271e2..b1b102c 100644 --- a/backend/api.py +++ b/backend/api.py @@ -1,8 +1,10 @@ import os import logging import time +from datetime import datetime, timezone from functools import wraps from urllib.parse import urlparse +from concurrent.futures import ThreadPoolExecutor import requests from flask import Blueprint, request, jsonify, Response @@ -17,6 +19,7 @@ from repositories.scrape_repository import scrape_repository from repositories.battlecard_repository import battlecard_repository from repositories.comparable_repository import comparable_repository from repositories.alert_repository import alert_repository +from repositories.watchlist_repository import watchlist_repository from services import licence_service from services import trip_finder_service @@ -28,6 +31,7 @@ from services import auth_service from services import analysis_service from services import email_service from services import alert_service +from services import watchlist_service logger = logging.getLogger(__name__) @@ -197,7 +201,20 @@ def confirm_password_reset(): @limiter.limit('20 per hour') @require_api_key def find_urls(): - """Runs Firecrawl /map concurrently per selected competitor to find matching trip URLs.""" + """ + Runs Firecrawl /map concurrently per selected competitor to find + matching trip URLs, and — when the caller supplies tenant_id and that + tenant has a website on file — also finds the tenant's own matching + trip page the same way (client self-scrape feature). tenant_id is + optional and not in `required`; omitting it (or a tenant with no + website configured) simply omits `own_trip` from the response, + which the frontend treats as "not available for this tenant." + + The own-trip lookup is kicked off in its own thread BEFORE the + competitor batch, then joined after — so it overlaps with the + competitor ThreadPoolExecutor instead of adding its own /map call's + latency on top of the whole competitor batch sequentially. + """ data = request.json or {} required = ['destination', 'duration', 'travel_style', 'competitor_ids'] missing = [f for f in required if f not in data] @@ -207,10 +224,30 @@ def find_urls(): competitors = [ c for c in (competitor_repository.get_by_id(cid) for cid in data['competitor_ids']) if c ] + + own_trip_future = None + own_trip_executor = None + tenant_id = data.get('tenant_id') + if tenant_id: + tenant = tenant_repository.get_by_id(tenant_id) + website = (tenant or {}).get('website') + if website: + own_trip_executor = ThreadPoolExecutor(max_workers=1) + own_trip_future = own_trip_executor.submit( + trip_finder_service.find_own_trip_url, + website, data['destination'], data['duration'], data['travel_style'] + ) + matches = trip_finder_service.find_all_competitor_urls( competitors, data['destination'], data['duration'], data['travel_style'] ) - return jsonify({'matches': matches}) + + own_trip = None + if own_trip_future: + own_trip = own_trip_future.result() + own_trip_executor.shutdown() + + return jsonify({'matches': matches, 'own_trip': own_trip}) @api_bp.route('/research', methods=['POST']) @@ -281,6 +318,10 @@ def research_status(job_id): run = scrape_repository.get_by_id(job_id) if not run: return error_response('not_found', 'Unknown job_id', 404) + + if run.get('status') in ('running', 'cancelling'): + run = _reconcile_orphaned_job(job_id, run) + return jsonify({ 'status': run.get('status'), 'tab_type': run.get('tab_type'), @@ -293,6 +334,65 @@ def research_status(job_id): }) +def _reconcile_orphaned_job(job_id, run): + """ + Self-heals a scrape_runs row stuck at status='running' (or the PM + having requested cancellation, cancel_requested=True, while it's + still nominally 'running') because the RQ worker process actually + handling it died outright — e.g. `docker compose up -d --build + bettersight-worker` recreating the container mid-job — before + run_research_job()'s own try/except/finally ever got to write a + terminal status. Cancellation is cooperative (jobs.cancel_research_job()'s + docstring): a running job only notices cancel_requested at its next + between-competitor checkpoint, which assumes the SAME process stays + alive to reach it. A hard-killed worker never reaches that checkpoint + again, so without this check the dashboard polls a job that will + never change state, showing "Stopping…" (or a stuck progress bar) + forever. Checked lazily here, on the exact endpoint the dashboard + already polls every 3s (analysis_store.js), rather than a separate + background sweep. + + Data flow: + job_id → fetch the underlying RQ Job from Redis → + RQ status is 'started'/'queued'/'deferred'/'scheduled' → still + genuinely in flight → run returned unchanged → + RQ status is 'failed', 'finished', or the job record is missing + entirely (NoSuchJobError — expired from Redis, or the worker + was killed before RQ even wrote a heartbeat) → this job will + never update scrape_runs itself anymore → mark it 'cancelled' + (cancel_requested was set) or 'failed' (otherwise) now → + updated run record returned + """ + import redis + from jobs import redis_conn + from rq.job import Job + from rq.exceptions import NoSuchJobError + + try: + rq_status = Job.fetch(job_id, connection=redis_conn).get_status() + except NoSuchJobError: + rq_status = None + except redis.exceptions.RedisError as e: + # Can't reach Redis to check — don't guess. Leave the row alone + # and let a later poll (once Redis is reachable again) decide, + # rather than risk marking a genuinely still-running job failed. + logger.warning(f'Could not reach Redis to reconcile job {job_id}: {str(e)}') + return run + + if rq_status in ('started', 'queued', 'deferred', 'scheduled'): + return run # genuinely still in flight — nothing to reconcile + + new_status = 'cancelled' if run.get('cancel_requested') else 'failed' + logger.warning( + f'Job {job_id} orphaned (RQ status: {rq_status}) — reconciling scrape_runs to "{new_status}"' + ) + return scrape_repository.update(job_id, { + 'status': new_status, + 'error_log': run.get('error_log') or 'Job was interrupted before it could finish (worker restarted or crashed).', + 'completed_at': datetime.now(timezone.utc).isoformat(), + }) + + @api_bp.route('/research/cancel/', methods=['POST']) @limiter.limit('20 per hour') @require_api_key @@ -454,6 +554,75 @@ def mark_alert_read(alert_id): return jsonify(alert_repository.mark_read(alert_id)) +# ── TRIP WATCHLIST ─────────────────────────────────────────────────── +# Per-seat, tier-capped list of specific trips the nightly n8n cron +# re-scrapes instead of each competitor's root website — see +# services/watchlist_service.py's WATCHLIST_CAP_PER_SEAT and +# n8n/workflows/daily_scrape_cron.json. + +@api_bp.route('/watchlist', methods=['GET']) +@require_api_key +def get_watchlist(): + """ + Returns every watched item for the session's tenant (every seat's, + not just the caller's) — the cap is per-seat, but visibility is + tenant-wide so teammates can see what each other is tracking. + """ + tenant_id = auth_service.get_tenant_id_from_request(request) + if not tenant_id: + return error_response('unauthorized', 'Could not resolve tenant from session', 401) + return jsonify(watchlist_service.list_watchlist_for_tenant(tenant_id)) + + +@api_bp.route('/watchlist', methods=['POST']) +@limiter.limit('30 per hour') +@require_api_key +def add_watchlist_item(): + """ + Pins a specific competitor trip (from a completed analysis result) + to the calling seat's personal watchlist. 400 with a clear message + if that seat is already at WATCHLIST_CAP_PER_SEAT for its tier. + """ + tenant_id, seat = auth_service.get_seat_from_request(request) + if not tenant_id or not seat: + return error_response('unauthorized', 'Could not resolve seat from session', 401) + + data = request.json or {} + required = ['competitor_id', 'url'] + missing = [f for f in required if not data.get(f)] + if missing: + return error_response('missing_field', f'Missing fields: {", ".join(missing)}', 400) + + competitor = competitor_repository.get_by_id(data['competitor_id']) + if not competitor or competitor.get('tenant_id') != tenant_id: + return error_response('not_found', 'Unknown competitor', 404) + + tenant = tenant_repository.get_by_id(tenant_id) + result = watchlist_service.add_to_watchlist( + tenant, seat, data['competitor_id'], data['url'], data.get('trip_name') + ) + if not result['success']: + return error_response('watchlist_full', result['reason'], 400) + return jsonify(result['item']), 201 + + +@api_bp.route('/watchlist/', methods=['DELETE']) +@limiter.limit('30 per hour') +@require_api_key +def remove_watchlist_item(item_id): + """Removes a watched trip — any active seat on the tenant may remove any item.""" + tenant_id = auth_service.get_tenant_id_from_request(request) + if not tenant_id: + return error_response('unauthorized', 'Could not resolve tenant from session', 401) + + item = watchlist_repository.get_by_id(item_id) + if not item or item.get('tenant_id') != tenant_id: + return error_response('not_found', 'Unknown watchlist item', 404) + + watchlist_service.remove_from_watchlist(item_id) + return jsonify({'status': 'ok'}) + + # ── APPS SCRIPT SHEET SYNC ────────────────────────────────────────── # No dedicated routes here per CLAUDE.md §15 — the Sheet-initiated pull # uses GET /research/history/latest (above) and GET @@ -656,6 +825,28 @@ def update_timezone(): return jsonify({'timezone': updated['timezone']}) +@api_bp.route('/account/website', methods=['PATCH']) +@require_api_key +def update_website(): + """ + Sets or changes the tenant's own storefront URL — used by the + client self-scrape feature to find/scrape the tenant's own matching + trip page the same way competitor pages are matched/scraped. Unlike + /account/timezone, this is freely editable at any time (a PM may + reasonably correct or change their site later), not first-write-wins. + """ + tenant_id = auth_service.get_tenant_id_from_request(request) + if not tenant_id: + return error_response('unauthorized', 'Could not resolve tenant from session', 401) + + website = (request.json or {}).get('website') + if not website: + return error_response('missing_field', 'website is required', 400) + + updated = tenant_repository.update(tenant_id, {'website': website}) + return jsonify({'website': updated['website']}) + + @api_bp.route('/account/onboarding', methods=['PATCH']) @require_api_key def update_onboarding(): diff --git a/backend/industries/adventure_travel/prompt.py b/backend/industries/adventure_travel/prompt.py index e3d3dfa..165e260 100644 --- a/backend/industries/adventure_travel/prompt.py +++ b/backend/industries/adventure_travel/prompt.py @@ -5,7 +5,7 @@ def build_prompt(competitor_name, page_data, product_name, - destination, duration, travel_style, company_name=None, is_ngs=False): + destination, duration, travel_style, company_name=None, is_ngs=False, own_trip=None): """ Builds the full extraction prompt for one competitor page, embedding the scraped page content directly into the returned template. @@ -18,11 +18,23 @@ def build_prompt(competitor_name, page_data, product_name, to a generic placeholder only for the dry-run /preview-prompt route, which has no tenant context to draw a real name from. + own_trip is the tenant's own trip, ACTUALLY extracted from their own + site by embedding_service.scrape_own_trip() (see jobs.py's + context['own_trip']) — not just the PM's typed destination/duration/ + style strings. When present, the comparison block and the `comments` + field instruction reference these real facts instead of only the + typed intent, so the LLM compares against ground truth rather than a + guess. None (the default) preserves the exact prior behavior for + every existing call site and for tenants who haven't confirmed an + own-trip URL for this run. + Data flow: competitor_name + page_data (scraped text/tables/url) + client product context (name/destination/duration/style/company) + is_ngs - flag → NGS-specific field block appended if is_ngs → - full prompt string returned, ready to send to core.extractor + flag + optional own_trip (real extracted client data) → + NGS-specific field block appended if is_ngs → own_trip block + appended if present → full prompt string returned, ready to send + to core.extractor """ company_name = company_name or 'the client' ngs_fields = ''' @@ -36,6 +48,24 @@ def build_prompt(competitor_name, page_data, product_name, url = page_data['url'] if page_data else '[COMPETITOR URL]' + own_trip_block = '' + comments_target = f'{company_name}' + if own_trip: + own_trip_name = own_trip.get('tripName') or product_name + own_trip_price = own_trip.get('majorityPrice') + own_trip_duration = own_trip.get('duration') or duration + own_trip_activities = own_trip.get('activities') + own_trip_block = f''' + +{company_name}'s OWN trip, actually extracted from their own site (real +data, not a typed description) — use this as the ground truth for comparison: +- Trip name: {own_trip_name} +- Price: {own_trip_price if own_trip_price is not None else 'not extracted'} USD (majority price) +- Duration: {own_trip_duration} days +- Service level: {own_trip.get('serviceLevel') or 'not extracted'} +- Activities: {own_trip_activities or 'not extracted'}''' + comments_target = f"{company_name}'s own {own_trip_name} (${own_trip_price}, {own_trip_duration} days: {own_trip_activities})" + return f'''You are a travel industry analyst helping {company_name} research competitors. Be concise — use short phrases not full sentences for all fields except comments. Read the ENTIRE page content carefully before extracting any fields. @@ -48,7 +78,7 @@ This is the specific page selected as most comparable to: - {company_name} Product: {product_name} - Destination: {destination} - Duration: ~{duration} days -- {company_name} Travel Style: {travel_style} +- {company_name} Travel Style: {travel_style}{own_trip_block} PAGE CONTENT: --- @@ -98,7 +128,110 @@ Return ONLY a valid JSON object. No markdown, no code fences, no explanation. "majorityPrice": standard adult USD price as number or null, "priceLow": lowest standard USD price as number or null, "priceHigh": highest standard USD price as number or null, - "comments": "analysis: key differences from {company_name}, strengths, weaknesses, pricing position", + "comments": "analysis: key differences from {comments_target}, strengths, weaknesses, pricing position", + "dateUsed": "specific departure date used for majority price", + "audPrice": standard AUD price as number or null, + "cadPrice": standard CAD price as number or null, + "eurPrice": standard EUR price as number or null, + "gbpPrice": standard GBP price as number or null +}}''' + + +def build_own_trip_prompt(page_data, product_name, destination, duration, + travel_style, company_name, is_ngs=False): + """ + Builds the extraction prompt for the TENANT'S OWN trip page — + embedding_service.scrape_own_trip() calls this, not build_prompt(), + because the framing is different: this is not "researching a + competitor," it's extracting structured data from the client's own + product so later competitor extractions (build_prompt()'s own_trip + param) have real ground truth to compare against instead of typed + strings. + + Shares the same pricing/meals extraction rules as build_prompt() so + the numbers are directly comparable, but the returned JSON schema + drops `relevancy` (relevance to itself is meaningless) and `comments` + (nothing to compare against yet at this stage). + + Data flow: + page_data (scraped from the tenant's own confirmed/matched URL) + + typed intent (product_name/destination/duration/travel_style) + + company_name + is_ngs → full prompt string, ready to send to + core.extractor.extract_with_openrouter() + """ + ngs_fields = ''' + "exclusiveAccess": "exclusive access or special unique inclusions", + "groupLeader": "group leader and/or local expert description", + "sustainability": "sustainability inclusions or null",''' if is_ngs else '' + + content = page_data['text'] if page_data else '[PAGE CONTENT WILL BE INSERTED HERE]' + if page_data and page_data.get('tables'): + content += f'\n\nTABLE DATA:\n{page_data["tables"]}' + + url = page_data['url'] if page_data else '[OWN TRIP URL]' + + return f'''You are a travel industry analyst extracting structured trip data +from {company_name}'s own tour page — this is {company_name}'s own product, +not a competitor's. Be concise — use short phrases, not full sentences. +Read the ENTIRE page content carefully before extracting any fields. +Pay particular attention to itinerary sections, departure tables, and pricing grids +which may appear later in the content. + +The content below was scraped from {company_name}'s own tour page at: {url} + +This is the page selected as matching: +- Product: {product_name} +- Destination: {destination} +- Duration: ~{duration} days +- Travel Style: {travel_style} + +PAGE CONTENT: +--- +{content} +--- + +CRITICAL PRICING RULES: +- majorityPrice: Standard/regular adult price for the most common departure. + IGNORE sale, promotional, early bird, or "was/now" pricing. Use non-discounted rack rate only. +- Prices may appear as "from $X", "per person from $X", or inside a departures table or pricing grid. + Extract the most commonly listed standard adult price regardless of how it is labelled on the page. +- priceLow: Lowest standard price across all departures shown (not sale price). +- priceHigh: Highest standard price across all departures shown (not sale price). +- If a site requires clicking "see more departures" to show all dates, note this in seasonality + and use only prices visible on the main listing page. +- Multi-currency: only fill if the price is explicitly listed in that currency on the page. + Do not convert. If the page shows GBP only, fill gbpPrice and leave USD fields null. + +MEALS RULE: +- meals: Return the TOTAL number of included meals as a single integer only. No text description. +- Meals may be listed individually per day in the itinerary section + (e.g. "Day 1: Breakfast", "Day 3: Breakfast and Dinner"). + Count every individual meal mention across all itinerary days. + Do not rely on a summary box — read through the full itinerary to count. +- Count each meal type separately: 1 breakfast + 1 lunch + 1 dinner = 3 meals. + +Return ONLY a valid JSON object. No markdown, no code fences, no explanation. + +{{ + "company": "{company_name}", + "link": "{url}", + "tripName": "exact trip name from the page", + "tripCode": "trip code if shown, otherwise null", + "serviceLevel": "accommodation service level", + "groupSize": max group size as integer, + "duration": number of days as integer, + "meals": total included meals as integer,{ngs_fields} + "startLocation": "departure city", + "endLocation": "end city", + "departures": approximate departures per year as integer or null, + "seasonality": "price bands by season. Note if more departures are hidden behind a button.", + "startDays": "departure days of week", + "targetAudience": "target traveller", + "hotels": "hotel types or properties", + "activities": "key included activities", + "majorityPrice": standard adult USD price as number or null, + "priceLow": lowest standard USD price as number or null, + "priceHigh": highest standard USD price as number or null, "dateUsed": "specific departure date used for majority price", "audPrice": standard AUD price as number or null, "cadPrice": standard CAD price as number or null, diff --git a/backend/jobs.py b/backend/jobs.py index 65fdd94..7e90634 100644 --- a/backend/jobs.py +++ b/backend/jobs.py @@ -11,7 +11,7 @@ from rq.exceptions import NoSuchJobError from repositories.scrape_repository import scrape_repository from repositories.competitor_repository import competitor_repository from repositories.tenant_repository import tenant_repository -from services import analysis_service, alert_service, battlecard_service +from services import analysis_service, alert_service, battlecard_service, embedding_service logger = logging.getLogger(__name__) @@ -137,6 +137,22 @@ def run_research_job(job_id, data): 'force_refresh': data.get('force_refresh', False), } + # Client self-scrape — if the PM confirmed/swapped a URL for the + # tenant's own trip page (UrlConfirmation.vue's "Your trip page" row), + # scrape and extract it once here so every competitor's build_prompt() + # call below gets real ground-truth data instead of only typed + # destination/duration/style strings. A side-effect failure (site + # blocked, extraction error) must never fail the whole batch — logged + # and left as None, which build_prompt() treats identically to a + # tenant who never confirmed an own-trip URL for this run. + own_trip_url = data.get('own_trip_url') + context['own_trip'] = None + if own_trip_url: + try: + context['own_trip'] = embedding_service.scrape_own_trip(tenant_id, own_trip_url, context) + except Exception as e: + logger.error(f'scrape_own_trip failed for tenant {tenant_id}: {str(e)}') + success, failed = [], [] done = 0 cancelled = False @@ -199,7 +215,11 @@ def run_research_job(job_id, data): status = 'cancelled' if cancelled else ('complete' if success or not competitors_input else 'failed') scrape_repository.update(job_id, { 'status': status, - 'results': {'success': success, 'failed': failed}, + # own_trip surfaces the tenant's own extracted trip data (or None + # if no website was confirmed this run) to the dashboard — before + # this it was only ever consumed internally by build_prompt(), + # with no way for the PM to see it was actually scraped and used. + 'results': {'success': success, 'failed': failed, 'own_trip': context.get('own_trip')}, 'error_log': '; '.join(f['error'] for f in failed) if failed else '', 'completed_at': datetime.now(timezone.utc).isoformat(), }) diff --git a/backend/pb_migrations/006_add_own_trip_fields.js b/backend/pb_migrations/006_add_own_trip_fields.js new file mode 100644 index 0000000..8e53e2e --- /dev/null +++ b/backend/pb_migrations/006_add_own_trip_fields.js @@ -0,0 +1,39 @@ +/// +// +// Adds the fields needed to scrape the TENANT's OWN trip page, mirroring +// how competitors are scraped, so build_prompt()'s comparison and the +// `comments` field are grounded in a real extracted trip instead of only +// the PM's typed destination/duration/style strings. +// +// tenants.website url the tenant's own storefront root — distinct +// from `domain` (bare text, used only for +// email-domain seat matching, never a URL) +// client_trips.url text confirmed/matched own-trip page for a run +// client_trips.extracted json full structured extraction blob (mirrors +// products' shape — tripName/price/duration/ +// activities/serviceLevel/etc.) +// client_trips.last_scraped date drives the same 24hr freshness reuse as +// competitors (core/scraper.py's +// CACHE_STALENESS_HOURS) + +migrate((app) => { + const tenants = app.findCollectionByNameOrId('tenants'); + tenants.fields.add(new Field({ type: 'url', name: 'website' })); + app.save(tenants); + + const clientTrips = app.findCollectionByNameOrId('client_trips'); + clientTrips.fields.add(new Field({ type: 'text', name: 'url' })); + clientTrips.fields.add(new Field({ type: 'json', name: 'extracted' })); + clientTrips.fields.add(new Field({ type: 'date', name: 'last_scraped' })); + return app.save(clientTrips); +}, (app) => { + const tenants = app.findCollectionByNameOrId('tenants'); + tenants.fields.removeByName('website'); + app.save(tenants); + + const clientTrips = app.findCollectionByNameOrId('client_trips'); + clientTrips.fields.removeByName('url'); + clientTrips.fields.removeByName('extracted'); + clientTrips.fields.removeByName('last_scraped'); + return app.save(clientTrips); +}); diff --git a/backend/pb_migrations/007_add_scrape_runs_scraped_url.js b/backend/pb_migrations/007_add_scrape_runs_scraped_url.js new file mode 100644 index 0000000..dff214e --- /dev/null +++ b/backend/pb_migrations/007_add_scrape_runs_scraped_url.js @@ -0,0 +1,40 @@ +/// +// +// Adds `scraped_url` to `scrape_runs` — fixes a real bug where content-hash +// change detection was scoped only by competitor_id, not by which specific +// page was actually scraped. Once trip-intent matching (CLAUDE.md §8) let a +// PM scrape different trip pages for the same competitor across different +// analysis runs, get_last_hash_for_url(competitor_id) compared each new +// scrape's hash against whatever page happened to be scraped MOST +// RECENTLY for that competitor — almost always a different page — so +// change_detected kept getting set back to True on nearly every run, and +// needs_refresh() (which gates the fast path on change_detected being +// False) never let the true cache-only fast path trigger in practice. +// Confirmed directly against real scrape_runs history: the same +// competitor's recorded content_hash changed on 5 of 6 consecutive scrapes +// over one test session, even though re-scraping the exact same URL twice +// in immediate succession produced an identical hash both times. +// +// See services/analysis_service.py's detect_change()/needs_refresh()/ +// run_competitor_analysis() and repositories/scrape_repository.py's +// get_last_hash_for_url() for the corresponding code fix. + +migrate((app) => { + const collection = app.findCollectionByNameOrId('scrape_runs'); + + collection.fields.add(new Field({ type: 'text', name: 'scraped_url' })); + + collection.indexes = [ + ...collection.indexes, + 'CREATE INDEX idx_scrape_runs_url_hash ON scrape_runs (competitor_id, scraped_url, content_hash, started_at)', + ]; + + return app.save(collection); +}, (app) => { + const collection = app.findCollectionByNameOrId('scrape_runs'); + + collection.fields.removeByName('scraped_url'); + collection.indexes = collection.indexes.filter((idx) => !idx.includes('idx_scrape_runs_url_hash')); + + return app.save(collection); +}); diff --git a/backend/pb_migrations/008_add_trip_watchlist.js b/backend/pb_migrations/008_add_trip_watchlist.js new file mode 100644 index 0000000..35d0f99 --- /dev/null +++ b/backend/pb_migrations/008_add_trip_watchlist.js @@ -0,0 +1,62 @@ +/// +// +// Adds `trip_watchlist` — the per-seat trip-monitoring feature. Each +// active seat can pin a small, tier-capped number of specific already- +// analyzed competitor trips (WATCHLIST_CAP_PER_SEAT in +// services/watchlist_service.py — 1 per seat at Analyse). The nightly +// n8n cron re-scrapes exactly these URLs instead of each competitor's +// root website, replacing an unbounded, low-signal daily scrape (a +// homepage `is_generic_homepage()` already flags as non-trip-specific) +// with a small, predictable, and meaningful one — every resulting +// price/product alert is about a trip a PM explicitly chose to track. +// +// Reuses the exact same confirmed_url / per-url content-hash caching +// already built for manual trip-intent analysis this session — no +// schema change needed on products/scrape_runs/competitors, and no +// jobs.py/analysis_service.py code change either. See +// n8n/workflows/daily_scrape_cron.json for the cron-side wiring. + +migrate((app) => { + const tenants = app.findCollectionByNameOrId('tenants'); + const tenantSeats = app.findCollectionByNameOrId('tenant_seats'); + const competitors = app.findCollectionByNameOrId('competitors'); + + const rel = (name, collection, opts = {}) => ({ + name, type: 'relation', collectionId: collection.id, + cascadeDelete: false, minSelect: 0, maxSelect: 1, ...opts, + }); + const text = (name, opts = {}) => ({ name, type: 'text', ...opts }); + const autodate = (name, onCreate, onUpdate = false) => ({ + name, type: 'autodate', onCreate, onUpdate, + }); + + const collection = new Collection({ + type: 'base', + name: 'trip_watchlist', + fields: [ + rel('tenant_id', tenants, { required: true }), + rel('seat_id', tenantSeats, { required: true }), + rel('competitor_id', competitors, { required: true }), + text('url', { required: true }), + text('trip_name'), + autodate('added_at', true), + ], + indexes: [ + 'CREATE INDEX idx_trip_watchlist_seat ON trip_watchlist (seat_id)', + 'CREATE INDEX idx_trip_watchlist_tenant ON trip_watchlist (tenant_id)', + ], + // Same access model as every other tenant-scoped collection — Flask + // is the only direct PocketBase client, enforcing tenant_id/seat_id + // scoping itself. + listRule: null, + viewRule: null, + createRule: null, + updateRule: null, + deleteRule: null, + }); + + return app.save(collection); +}, (app) => { + const collection = app.findCollectionByNameOrId('trip_watchlist'); + if (collection) app.delete(collection); +}); diff --git a/backend/repositories/product_repository.py b/backend/repositories/product_repository.py index 5e28abc..73facc5 100644 --- a/backend/repositories/product_repository.py +++ b/backend/repositories/product_repository.py @@ -33,6 +33,23 @@ class ProductRepository: f'competitor_id = "{competitor_id}" && trip_name = "{safe_name}" && is_active = true' ) + def get_by_competitor_and_url(self, competitor_id, url): + """ + Finds the active product matching this exact competitor+URL — + used by needs_refresh()/analyse_from_cache() to check freshness + for and serve the SPECIFIC trip page confirmed for this run, not + just whichever product happens to be most recently scraped for + this competitor overall (which could be a completely different + trip if the PM has researched more than one product here). + """ + if not url: + return None + safe_url = url.replace('"', '\\"') + return pb_client.get_first( + COLLECTION, + f'competitor_id = "{competitor_id}" && url = "{safe_url}" && is_active = true' + ) + def list_for_tenant(self, tenant_id): """Returns all active products for a tenant — used for embedding/comparable matching.""" return pb_client.list(COLLECTION, filter_str=f'tenant_id = "{tenant_id}" && is_active = true', per_page=500) @@ -72,6 +89,16 @@ class MockProductRepository: and r.get('is_active', True) ), None) + def get_by_competitor_and_url(self, competitor_id, url): + if not url: + return None + return next(( + r for r in self._records.values() + if r.get('competitor_id') == competitor_id + and r.get('url') == url + and r.get('is_active', True) + ), None) + def list_for_tenant(self, tenant_id): return [r for r in self._records.values() if r.get('tenant_id') == tenant_id and r.get('is_active', True)] diff --git a/backend/repositories/scrape_repository.py b/backend/repositories/scrape_repository.py index 9a1ab73..f2526e4 100644 --- a/backend/repositories/scrape_repository.py +++ b/backend/repositories/scrape_repository.py @@ -56,15 +56,28 @@ class ScrapeRepository: ) return items[0] if items else None - def get_last_hash_for_url(self, competitor_id): + def get_last_hash_for_url(self, competitor_id, url): """ - Returns the most recently recorded content_hash for this - competitor's prior scrape, or None if it has never been hashed - before. Backs detect_change()'s content-hash comparison. + Returns the most recently recorded content_hash for this exact + competitor+url combination, or None if this specific page has + never been hashed before. Backs detect_change()'s content-hash + comparison. + + Scoped by url as well as competitor_id — CLAUDE.md §8's + trip-intent matching means one competitor can have many + different confirmed trip URLs scraped over time. Comparing + against whichever page happened to be scraped MOST RECENTLY for + this competitor (the old, url-blind lookup) meant a fresh scrape + of URL A got compared against URL B's hash whenever B was + scraped more recently — almost always a mismatch, which kept + change_detected stuck True and made the true cache-only fast + path unreachable in practice. Confirmed directly against real + scrape_runs history before this fix. """ + safe_url = url.replace('"', '\\"') items = pb_client.list( COLLECTION, - filter_str=f'competitor_id = "{competitor_id}" && content_hash != ""', + filter_str=f'competitor_id = "{competitor_id}" && scraped_url = "{safe_url}" && content_hash != ""', sort='-started_at', per_page=1 ) return items[0]['content_hash'] if items else None @@ -125,10 +138,10 @@ class MockScrapeRepository: items.sort(key=lambda r: r.get('started_at') or '', reverse=True) return items[0] if items else None - def get_last_hash_for_url(self, competitor_id): + def get_last_hash_for_url(self, competitor_id, url): items = [ r for r in self._records.values() - if r.get('competitor_id') == competitor_id and r.get('content_hash') + if r.get('competitor_id') == competitor_id and r.get('scraped_url') == url and r.get('content_hash') ] items.sort(key=lambda r: r.get('started_at') or '', reverse=True) return items[0]['content_hash'] if items else None diff --git a/backend/repositories/seat_repository.py b/backend/repositories/seat_repository.py index f5e8958..ad90b0b 100644 --- a/backend/repositories/seat_repository.py +++ b/backend/repositories/seat_repository.py @@ -21,6 +21,11 @@ class SeatRepository: f'tenant_id = "{tenant_id}" && email = "{safe_email}" && active = true' ) + def get_by_id(self, seat_id): + """Returns a seat record by id, or None — used to denormalize the + watching seat's email onto a watchlist item for display.""" + return pb_client.get_one(COLLECTION, seat_id) + def count_active(self, tenant_id): """Returns the count of active seats for a tenant.""" return len(pb_client.list( @@ -58,6 +63,9 @@ class MockSeatRepository: if r.get('tenant_id') == tenant_id and r.get('email') == email and r.get('active') ), None) + def get_by_id(self, seat_id): + return self._records.get(seat_id) + def count_active(self, tenant_id): return len([ r for r in self._records.values() diff --git a/backend/repositories/watchlist_repository.py b/backend/repositories/watchlist_repository.py new file mode 100644 index 0000000..22376d2 --- /dev/null +++ b/backend/repositories/watchlist_repository.py @@ -0,0 +1,74 @@ +import uuid +from repositories.pocketbase_client import pb_client +from repositories.repo_config import USE_MOCK_REPOSITORIES + +COLLECTION = 'trip_watchlist' + + +class WatchlistRepository: + """ + Data access for the `trip_watchlist` collection — pure CRUD only, per + the no-business-logic-in-repositories rule. Cap enforcement and + ownership decisions live in services/watchlist_service.py. + """ + + def create(self, data): + return pb_client.create(COLLECTION, data) + + def delete(self, item_id): + return pb_client.delete(COLLECTION, item_id) + + def get_by_id(self, item_id): + return pb_client.get_one(COLLECTION, item_id) + + def list_for_tenant(self, tenant_id): + """All watched items for a tenant (every seat's), newest first.""" + return pb_client.list( + COLLECTION, filter_str=f'tenant_id = "{tenant_id}"', + sort='-added_at', per_page=200 + ) + + def count_for_seat(self, seat_id): + """Returns how many watchlist slots this seat currently occupies.""" + return len(pb_client.list( + COLLECTION, filter_str=f'seat_id = "{seat_id}"', per_page=200 + )) + + +class MockWatchlistRepository: + """In-memory stand-in for WatchlistRepository.""" + + def __init__(self): + self._records = {} + # Mirrors MockScrapeRepository's monotonic counter — real + # PocketBase auto-populates added_at via autodate(true), and + # every "newest first" sort here relies on strict ordering. + self._sequence = 0 + + def create(self, data): + record_id = data.get('id') or str(uuid.uuid4()) + self._sequence += 1 + record = { + 'id': record_id, + 'added_at': data.get('added_at') or f'{self._sequence:020d}', + **data, + } + self._records[record_id] = record + return record + + def delete(self, item_id): + return self._records.pop(item_id, None) is not None + + def get_by_id(self, item_id): + return self._records.get(item_id) + + def list_for_tenant(self, tenant_id): + items = [r for r in self._records.values() if r.get('tenant_id') == tenant_id] + items.sort(key=lambda r: r.get('added_at') or '', reverse=True) + return items + + def count_for_seat(self, seat_id): + return len([r for r in self._records.values() if r.get('seat_id') == seat_id]) + + +watchlist_repository = MockWatchlistRepository() if USE_MOCK_REPOSITORIES else WatchlistRepository() diff --git a/backend/services/analysis_service.py b/backend/services/analysis_service.py index 2abb177..6b84cc6 100644 --- a/backend/services/analysis_service.py +++ b/backend/services/analysis_service.py @@ -15,17 +15,60 @@ logger = logging.getLogger(__name__) PRICE_CHANGE_ALERT_THRESHOLD_PERCENT = 5 -def needs_refresh(competitor): +def needs_refresh(competitor, confirmed_url=None): """ - Decides whether a competitor needs a live re-scrape (REFRESH path) - or can be served from PocketBase (FAST path). + Decides whether a competitor's specific confirmed page needs a live + re-scrape (REFRESH path) or can be served from PocketBase (FAST path). - Data flow: + When confirmed_url is given (trip-intent matching, CLAUDE.md §8), + freshness is judged against the cached PRODUCT for that exact URL, + not the competitor's global change_detected/last_scraped flags. + Those flags reflect whichever page was scraped MOST RECENTLY for + this competitor — which may be a completely different trip than the + one being asked about now. Gating on them directly made the fast + path unreachable (or worse, risked serving the wrong cached trip) + whenever a PM researched more than one product for the same + competitor across different runs — confirmed directly against real + scrape_runs history, where the same competitor's recorded hash + changed on nearly every run despite re-scraping one specific URL + twice in a row producing an identical hash both times. + + Falls back to the original competitor-wide check when no + confirmed_url is available — the daily cron path (§15), which + scrapes one canonical page per competitor and has no trip-intent + context at all, is unaffected by this fix. + + Data flow (confirmed_url given): + confirmed_url → product_repository.get_by_competitor_and_url() → + no cached product for this exact page → True (refresh) → + cached product's last_seen missing/stale (> CACHE_STALENESS_HOURS) + → True (refresh) → otherwise → False (fast path is safe) + + Data flow (no confirmed_url — cron path, unchanged from before): competitor record → change_detected flag set? → True (refresh) → last_scraped missing or older than CACHE_STALENESS_HOURS? → True (refresh) → otherwise → False (fast path is safe) """ + # CACHE_STALENESS_HOURS' canonical, adjustable home is core/scraper.py + # per CLAUDE.md §7 ("in core/scraper.py — adjust here only") — imported + # lazily here for the same reason every other core.* reference in this + # file is lazy: this module must stay importable independent of Phase 2. + from core.scraper import CACHE_STALENESS_HOURS + + if confirmed_url: + product = product_repository.get_by_competitor_and_url(competitor['id'], confirmed_url) + if not product: + return True + last_seen = product.get('last_seen') + if not last_seen: + return True + try: + seen_at = datetime.fromisoformat(last_seen.replace('Z', '+00:00')) + except (ValueError, AttributeError): + return True + return (datetime.now(timezone.utc) - seen_at) > timedelta(hours=CACHE_STALENESS_HOURS) + if competitor.get('change_detected'): return True @@ -38,12 +81,6 @@ def needs_refresh(competitor): except (ValueError, AttributeError): return True - # CACHE_STALENESS_HOURS' canonical, adjustable home is core/scraper.py - # per CLAUDE.md §7 ("in core/scraper.py — adjust here only") — imported - # lazily here for the same reason every other core.* reference in this - # file is lazy: this module must stay importable independent of Phase 2. - from core.scraper import CACHE_STALENESS_HOURS - age = datetime.now(timezone.utc) - scraped_at return age > timedelta(hours=CACHE_STALENESS_HOURS) @@ -76,21 +113,59 @@ def analyse_from_cache(tenant_id, competitor_id, context): constrained to {comments, relevancy} only (via core.extractor) → cached structured fields + fresh comments/relevancy merged and returned - """ - products = product_repository.get_latest_for_competitor(competitor_id, limit=1) - if not products: - raise ValueError('No cached product data available for fast path') - latest = products[0] + context['confirmed_url'], when present, selects the cached product + for that EXACT trip page (product_repository.get_by_competitor_and_url()) + rather than whichever product happens to be most recently scraped for + this competitor overall — the same competitor can have cached data + for several different trips, and "most recent" is not necessarily the + one this run is actually about. Falls back to "latest for competitor" + when no confirmed_url is given (cron path) or no product matches it yet. + """ + confirmed_url = context.get('confirmed_url') + latest = product_repository.get_by_competitor_and_url(competitor_id, confirmed_url) if confirmed_url else None + if not latest: + products = product_repository.get_latest_for_competitor(competitor_id, limit=1) + if not products: + raise ValueError('No cached product data available for fast path') + latest = products[0] # core.extractor is a Phase 2 module — imported lazily so this # service is importable before the scraping engine refactor lands. from core.extractor import extract_with_openrouter + # Ground the comparison in the tenant's own REAL extracted trip + # (context['own_trip'], from embedding_service.scrape_own_trip()) + # whenever it's available — only falling back to the PM's typed + # destination/duration/style/product name when no own-trip data was + # returned for this run (no website confirmed, or the scrape/ + # extraction failed). Previously this dumped the raw `context` dict + # (Python repr) into the prompt with no priority framing at all, so + # own_trip's presence was easy for the model to under-weight against + # the typed fields sitting right next to it in the same blob. + own_trip = context.get('own_trip') + company_name = context.get('company_name') or 'the client' + if own_trip: + own_trip_name = own_trip.get('tripName') or context.get('product_name') + client_context_desc = ( + f"{company_name}'s own {own_trip_name} — ACTUAL extracted data from " + f"their own site, not a typed description: " + f"{own_trip.get('majorityPrice', 'price not extracted')} USD, " + f"{own_trip.get('duration') or context.get('duration')} days, " + f"activities: {own_trip.get('activities') or 'not extracted'}" + ) + else: + client_context_desc = ( + f"{company_name}'s typed product intent (no own-site data confirmed " + f"this run): product={context.get('product_name')}, " + f"destination={context.get('destination')}, " + f"duration={context.get('duration')} days, style={context.get('travel_style')}" + ) + prompt = ( 'You are a travel industry analyst. Given this already-extracted ' 'competitor trip data, assess it against ' - f"the client's product context: {context}.\n\nCompetitor data:\n{latest}\n\n" + f"{client_context_desc}.\n\nCompetitor data:\n{latest}\n\n" 'Return ONLY a valid JSON object:\n' '{\n' ' "comments": "short comparative analysis: key differences, strengths, weaknesses, pricing position",\n' @@ -141,21 +216,28 @@ def analyse_from_cache(tenant_id, competitor_id, context): return result -def detect_change(competitor_id, new_hash): +def detect_change(competitor_id, url, new_hash): """ Compares new_hash against the last content_hash recorded for this - competitor. On a diff: sets competitor.change_detected = true and - change_detected_at = now. Replaces Firecrawl Monitor's webhook role - (CLAUDE.md §4/§10) — the daily/on-demand scrape itself is now the - only place a change is ever detected. + exact competitor+url. On a diff: sets competitor.change_detected = + true and change_detected_at = now. Replaces Firecrawl Monitor's + webhook role (CLAUDE.md §4/§10) — the daily/on-demand scrape itself + is now the only place a change is ever detected. + + Scoped by url, not just competitor_id — see + scrape_repository.get_last_hash_for_url()'s docstring for why + comparing against whatever page was scraped most recently for this + competitor (regardless of which page that was) made change_detected + effectively never settle back to False once a competitor had more + than one trip page scraped over time. Data flow: - competitor_id + new_hash → scrape_repository.get_last_hash_for_url() → - no prior hash: True (first scrape — always process) → + competitor_id + url + new_hash → scrape_repository.get_last_hash_for_url() → + no prior hash for this url: True (first scrape of this page — always process) → hash differs: competitor_repository.update() sets change flags → True → hash matches: no update → False """ - last_hash = scrape_repository.get_last_hash_for_url(competitor_id) + last_hash = scrape_repository.get_last_hash_for_url(competitor_id, url) if last_hash is None: return True if last_hash != new_hash: @@ -184,10 +266,11 @@ def run_competitor_analysis(tenant_id, competitor, context): when forced, so future non-forced runs keep an accurate hash history. Data flow: - competitor + context → needs_refresh() decision (or force_refresh) → + competitor + context → target_url resolved and cleaned → + needs_refresh() decision, scoped to target_url (or force_refresh) → FAST: analyse_from_cache() (seconds) → REFRESH: core.scraper.scrape() → compute_content_hash() → - detect_change() against the last recorded hash → + detect_change() against the last recorded hash FOR THIS URL → unchanged (and not force_refresh): change_detected cleared (confirmed stable since last look), skip LLM extraction, reuse cached narrative, bump last_scraped only (no wasted extraction @@ -200,9 +283,40 @@ def run_competitor_analysis(tenant_id, competitor, context): """ force_refresh = context.get('force_refresh', False) - if not needs_refresh(competitor) and not force_refresh: + # Phase 2 module — imported lazily, same reasoning as the other + # core.* imports further down in this function. + from core.scraper import clean_url + + # context['confirmed_url'] is the page confirmed at UrlConfirmation.vue + # (a Firecrawl /map match or a manual swap) — falls back to the + # competitor's stored root domain only when nothing was confirmed for + # this run (e.g. the daily cron path, which has no trip-intent context + # at all). Regression fix: this used to always scrape + # competitor['website'] unconditionally, silently discarding whatever + # trip-specific URL was actually matched/confirmed for this run. + # + # Cleaned up front (matching what scrape() does internally) so every + # url-scoped lookup below (needs_refresh, detect_change, + # analyse_from_cache) compares against the same normalised form + # _save_scrape_result() actually persists as products.url — a + # trailing-slash or tracking-param difference would otherwise + # silently defeat the url match and force an unnecessary refresh. + # Cleaned regardless of source (confirmed_url or the website + # fallback) so scrape_runs.scraped_url/products.url are always + # consistent with each other either way. + raw_confirmed_url = context.get('confirmed_url') + target_url = clean_url(raw_confirmed_url or competitor['website']) + # Only url-scope the fast-path/cache lookups below when this run + # actually had an explicit confirmed_url (trip-intent matching) — + # the cron/website-fallback path keeps using the competitor-wide + # change_detected/last_scraped flags, unchanged from before this fix. + confirmed_url = target_url if raw_confirmed_url else None + + if not needs_refresh(competitor, confirmed_url=confirmed_url) and not force_refresh: logger.info(f'{competitor["name"]} — fast path (cache fresh)') - fast_result = analyse_from_cache(tenant_id, competitor['id'], context) + fast_result = analyse_from_cache( + tenant_id, competitor['id'], {**context, 'confirmed_url': confirmed_url} + ) return { 'path': 'fast', 'result': fast_result, 'is_generic_page': is_generic_homepage(fast_result.get('link')), @@ -214,14 +328,6 @@ def run_competitor_analysis(tenant_id, competitor, context): from core.extractor import extract_with_openrouter from industries.adventure_travel.prompt import build_prompt - # context['confirmed_url'] is the page confirmed at UrlConfirmation.vue - # (a Firecrawl /map match or a manual swap) — falls back to the - # competitor's stored root domain only when nothing was confirmed for - # this run (e.g. the daily cron path, which has no trip-intent context - # at all). Regression fix: this used to always scrape - # competitor['website'] unconditionally, silently discarding whatever - # trip-specific URL was actually matched/confirmed for this run. - target_url = context.get('confirmed_url') or competitor['website'] # Computed once here and reused by both possible refresh-path returns # below — a meta-fact about which page was actually scraped this run, # not an extracted trip field, so it's a top-level sibling of `result`/ @@ -232,17 +338,19 @@ def run_competitor_analysis(tenant_id, competitor, context): raise ValueError(page_data.get('text', 'Scrape failed')) content_hash = compute_content_hash(page_data['text']) - changed = detect_change(competitor['id'], content_hash) + changed = detect_change(competitor['id'], target_url, content_hash) # Every refresh scrape gets its own scrape_runs record purely to - # persist content_hash for the next comparison — the outer batch - # job's scrape_runs row (created before run_research_job) tracks - # overall job status, not per-competitor hash history. + # persist content_hash (and the url it was hashed from) for the next + # comparison — the outer batch job's scrape_runs row (created before + # run_research_job) tracks overall job status, not per-competitor + # hash history. scrape_repository.create({ 'tenant_id': tenant_id, 'competitor_id': competitor['id'], 'triggered_by': 'cron', 'status': 'complete', 'content_hash': content_hash, 'content_length': len(page_data['text']), - 'hash_algorithm': 'sha256', 'competitors_total': 1, 'competitors_done': 1, + 'hash_algorithm': 'sha256', 'scraped_url': target_url, + 'competitors_total': 1, 'competitors_done': 1, 'results': {}, 'completed_at': datetime.now(timezone.utc).isoformat(), }) @@ -256,7 +364,8 @@ def run_competitor_analysis(tenant_id, competitor, context): 'last_scraped': datetime.now(timezone.utc).isoformat(), }) return { - 'path': 'refresh', 'result': analyse_from_cache(tenant_id, competitor['id'], context), + 'path': 'refresh', + 'result': analyse_from_cache(tenant_id, competitor['id'], {**context, 'confirmed_url': confirmed_url}), 'unchanged': True, 'is_generic_page': is_generic, } @@ -274,7 +383,8 @@ def run_competitor_analysis(tenant_id, competitor, context): context.get('product_name'), context.get('destination'), context.get('duration'), context.get('travel_style'), company_name=context.get('company_name'), - is_ngs=context.get('tab_type') == 'NGS' + is_ngs=context.get('tab_type') == 'NGS', + own_trip=context.get('own_trip'), ) extracted = extract_with_openrouter(prompt, content=page_data['text']) product = _save_scrape_result(tenant_id, competitor, extracted, page_data) diff --git a/backend/services/auth_service.py b/backend/services/auth_service.py index a111e51..d20bb97 100644 --- a/backend/services/auth_service.py +++ b/backend/services/auth_service.py @@ -3,44 +3,58 @@ from repositories.tenant_repository import tenant_repository from repositories.seat_repository import seat_repository -def get_tenant_id_from_request(request): +def get_seat_from_request(request): """ - Resolves the tenant_id for a dashboard request from its - `Authorization: Bearer {jwt}` header. The JWT itself is issued and - verified by PocketBase (the dashboard authenticates directly against - PocketBase) — this backend doesn't hold the JWT signing secret, so - it validates the token by asking PocketBase to refresh it, then - resolves the returned email to a tenant via the existing seat record. + Resolves both the tenant_id AND the specific active seat record for a + dashboard request from its `Authorization: Bearer {jwt}` header. The + JWT itself is issued and verified by PocketBase (the dashboard + authenticates directly against PocketBase) — this backend doesn't + hold the JWT signing secret, so it validates the token by asking + PocketBase to refresh it, then resolves the returned email to a + tenant and seat. + + Needed by seat-scoped features (the trip watchlist's per-seat cap) + where "which tenant" isn't enough — every other authenticated route + only ever needed tenant_id, so the seat record this lookup already + finds was previously discarded (see get_tenant_id_from_request() + below, now a thin wrapper around this). Data flow: request.headers['Authorization'] → strip "Bearer " prefix → PocketBase POST /api/collections/users/auth-refresh → { record: { email } } → seat lookup by email across known tenants - (via domain) → tenant_id returned, or None if unresolvable + (via domain) → (tenant_id, seat) returned, or (None, None) if + unresolvable """ auth_header = request.headers.get('Authorization', '') if not auth_header.startswith('Bearer '): - return None + return None, None token = auth_header[len('Bearer '):] record = pb_client.auth_refresh(token) if not record or not record.get('email'): - return None + return None, None email = record['email'] domain = email.split('@', 1)[1].lower() if '@' in email else None if not domain: - return None + return None, None tenant = tenant_repository.get_by_domain(domain) if not tenant: - return None + return None, None # Confirm this email actually holds an active seat on the tenant — # a verified PocketBase login alone isn't sufficient authorization, # the email must also be a registered, active seat. seat = seat_repository.get_active(tenant['id'], email) if not seat: - return None + return None, None - return tenant['id'] + return tenant['id'], seat + + +def get_tenant_id_from_request(request): + """Back-compat wrapper — most routes only need the tenant_id, not the seat.""" + tenant_id, _ = get_seat_from_request(request) + return tenant_id diff --git a/backend/services/embedding_service.py b/backend/services/embedding_service.py index a0eb79b..ea4ef1a 100644 --- a/backend/services/embedding_service.py +++ b/backend/services/embedding_service.py @@ -10,7 +10,8 @@ EMBEDDING_MODEL = 'openai/text-embedding-3-small' SIMILARITY_THRESHOLD = 0.75 -def save_client_trip(tenant_id, destination, duration, travel_style, product_name=None): +def save_client_trip(tenant_id, destination, duration, travel_style, product_name=None, + url=None, extracted=None, price_usd=None, activities=None): """ Upserts a client_trips record from a PM's trip-intent description (CLAUDE.md §8's TripIntentForm — destination/duration/travel_style, @@ -19,12 +20,19 @@ def save_client_trip(tenant_id, destination, duration, travel_style, product_nam it, find_comparable_trips() has nothing on the client side to match competitor products against. + url/extracted/price_usd/activities are optional and only ever passed + by scrape_own_trip() below, once the tenant's own trip page has + actually been scraped and extracted — the original typed-values-only + call site in api.py's /research route passes none of these, so its + behavior is unchanged. + Dedupes on (tenant_id, trip_name) so re-running analysis for the same product doesn't create duplicate rows — it just keeps the existing one current. Data flow: - destination/duration/travel_style/product_name → + destination/duration/travel_style/product_name (+ optional + url/extracted/price_usd/activities from a real scrape) → trip_name resolved (product_name if given, else a destination+style fallback) → client_trips_repository.get_by_tenant_and_name() → @@ -38,6 +46,15 @@ def save_client_trip(tenant_id, destination, duration, travel_style, product_nam 'destination': destination, 'duration_days': duration, } + if url is not None: + payload['url'] = url + if extracted is not None: + payload['extracted'] = extracted + if price_usd is not None: + payload['price_usd'] = price_usd + if activities is not None: + payload['activities'] = activities + existing = client_trips_repository.get_by_tenant_and_name(tenant_id, trip_name) if existing: # A stale embedding would silently never get regenerated — @@ -49,6 +66,96 @@ def save_client_trip(tenant_id, destination, duration, travel_style, product_nam return client_trips_repository.create(payload) +def scrape_own_trip(tenant_id, website_url, context): + """ + Scrapes and extracts the tenant's OWN confirmed/matched trip page — + the same Firecrawl-matched-or-manually-swapped URL pattern already + used for competitors, pointed at the tenant's own site instead. Gives + build_prompt()'s own_trip param and client_trips real ground-truth + data instead of only the PM's typed destination/duration/style. + + Reuses the same CACHE_STALENESS_HOURS freshness window as competitors + (core/scraper.py) so the tenant's own site isn't rescraped on every + single analysis run — only when the cached client_trips row is stale + or context['force_refresh'] is set, both exactly mirroring + analysis_service.needs_refresh()'s competitor logic. + + Never raises — a side-effect failure here (site blocked, proxy + error, extraction failure) must not fail the whole batch (CLAUDE.md + §18). Logs and returns None on any failure, which build_prompt() + treats identically to a tenant who never confirmed an own-trip URL. + + The returned dict always carries a `cached` bool (True when served + from client_trips.extracted without a live scrape, False when just + freshly extracted) — surfaced to the dashboard via jobs.py's + results['own_trip'] so ResultsTable.vue can show the same "from + PocketBase vs new scrape" pill on the own-trip column as every + competitor column. `link` is always overwritten with the real + website_url actually scraped, rather than trusting the LLM's own + echoed `"link"` field verbatim — same reasoning as + analysis_service._save_scrape_result() using page_data['url'], not + extracted.get('link'), for a competitor's stored product URL. + + Data flow: + website_url + context → trip_name resolved exactly as + save_client_trip() does (product_name, else destination+style) → + existing client_trips row fresh (and not force_refresh)? → return + its cached `extracted` blob (cached=True) → otherwise: + core.scraper.scrape() → build_own_trip_prompt() → + core.extractor.extract_with_openrouter() → link corrected to the + real website_url → save_client_trip() with + url/extracted/price_usd/activities → extracted dict returned + (cached=False) + """ + from datetime import datetime, timedelta, timezone + + trip_name = context.get('product_name') or f"{context.get('destination')} {context.get('travel_style')}".strip() + existing = client_trips_repository.get_by_tenant_and_name(tenant_id, trip_name) + force_refresh = context.get('force_refresh', False) + + if existing and existing.get('extracted') and not force_refresh: + last_scraped = existing.get('last_scraped') + if last_scraped: + try: + from core.scraper import CACHE_STALENESS_HOURS + scraped_at = datetime.fromisoformat(last_scraped.replace('Z', '+00:00')) + if datetime.now(timezone.utc) - scraped_at <= timedelta(hours=CACHE_STALENESS_HOURS): + logger.info(f'Own trip for tenant {tenant_id} — cache fresh, reusing extracted data') + return {**existing['extracted'], 'cached': True} + except (ValueError, AttributeError): + pass # unparseable timestamp — fall through and rescrape + + try: + from core.scraper import scrape + from core.extractor import extract_with_openrouter + from industries.adventure_travel.prompt import build_own_trip_prompt + + page_data = scrape(website_url) + if not page_data.get('success'): + logger.warning(f'Own-trip scrape failed for tenant {tenant_id}: {page_data.get("text")}') + return None + + prompt = build_own_trip_prompt( + page_data, context.get('product_name'), context.get('destination'), + context.get('duration'), context.get('travel_style'), + context.get('company_name'), is_ngs=context.get('tab_type') == 'NGS', + ) + extracted = extract_with_openrouter(prompt, content=page_data['text']) + extracted['link'] = website_url + + saved = save_client_trip( + tenant_id, context.get('destination'), context.get('duration'), + context.get('travel_style'), product_name=context.get('product_name'), + url=website_url, extracted=extracted, + price_usd=extracted.get('majorityPrice'), activities=extracted.get('activities'), + ) + client_trips_repository.update(saved['id'], {'last_scraped': datetime.now(timezone.utc).isoformat()}) + return {**extracted, 'cached': False} + except Exception as e: + logger.error(f'scrape_own_trip failed for tenant {tenant_id}: {str(e)}') + return None + + def embed_trip(trip): """ Generates a semantic embedding for a trip using OpenRouter's diff --git a/backend/services/trip_finder_service.py b/backend/services/trip_finder_service.py index d3f1e3c..f755dcc 100644 --- a/backend/services/trip_finder_service.py +++ b/backend/services/trip_finder_service.py @@ -11,6 +11,16 @@ FIRECRAWL_URL = os.getenv('FIRECRAWL_URL', 'http://firecrawl:3002') CATEGORY_KEYWORDS = ['classic', 'trek', 'trail', 'safari', 'adventure', 'explorer', 'journey'] CONFIDENCE_FOUND_THRESHOLD = 0.3 MAX_CONCURRENT_MAP_CALLS = 5 +# Firecrawl /map fetches the target site's sitemap.xml before scoring +# candidates — a real production site with a large catalogue (thousands +# of URLs, e.g. a tenant's own established storefront rather than a +# smaller competitor test site) can take well over 15s just for that +# fetch. Confirmed directly: a real /map call against a large site's +# sitemap took ~21.6s, which a 15s timeout silently turned into a false +# "no match found" (requests.RequestException swallowed below) even +# though a retry with more time succeeded in ~2s. 30s covers this with +# margin while still failing within a bounded time on a truly dead site. +FIRECRAWL_MAP_TIMEOUT_SECONDS = 30 def score_and_rank_urls(urls, destination, duration): @@ -97,7 +107,7 @@ def find_competitor_trip_url(root_url, destination, duration, travel_style): response = requests.post( f'{FIRECRAWL_URL}/v1/map', json={'url': root_url, 'search': f'{destination} {duration} days {travel_style} tour'}, - timeout=15 + timeout=FIRECRAWL_MAP_TIMEOUT_SECONDS ) response.raise_for_status() urls = response.json().get('links', []) @@ -111,6 +121,23 @@ def find_competitor_trip_url(root_url, destination, duration, travel_style): return score_and_rank_urls(urls, destination, duration) +def find_own_trip_url(website, destination, duration, travel_style): + """ + Finds the tenant's own best-matching trip page on their own site, + using the exact same Firecrawl /map + scoring gate as competitor + matching (score_and_rank_urls()'s destination-keyword hard gate and + root-domain exclusion apply unchanged) — named separately purely for + call-site clarity ("our own site" vs "a competitor's site"), not + because the matching behavior differs. + + Data flow: + website (tenant's own storefront root) + destination/duration/ + travel_style → find_competitor_trip_url() → { url, confidence, + found } + """ + return find_competitor_trip_url(website, destination, duration, travel_style) + + def find_all_competitor_urls(competitors, destination, duration, travel_style): """ Runs trip URL finding for all selected competitors concurrently — diff --git a/backend/services/watchlist_service.py b/backend/services/watchlist_service.py new file mode 100644 index 0000000..e74ea3a --- /dev/null +++ b/backend/services/watchlist_service.py @@ -0,0 +1,88 @@ +import logging +from repositories.watchlist_repository import watchlist_repository +from repositories.seat_repository import seat_repository + +logger = logging.getLogger(__name__) + +# Per-seat cap on watched trips, keyed by tenant tier — bounds the nightly +# cron's scrape volume predictably (seats x cap) instead of the old +# "re-scrape every competitor's homepage" approach. 1/seat at Analyse +# means a fully-seated ($349, 5 seats) tenant tops out at 5 real trip +# scrapes a night, same ceiling as scraping every competitor once, but +# now every alert is about a trip a PM actually chose to track instead of +# homepage noise. Discover/Intelligence values are placeholders — those +# tiers aren't built yet (CLAUDE.md §27) and will also get full catalogue +# auto-discovery on top, so the watchlist there is "pin your priorities," +# not the only detection mechanism. +WATCHLIST_CAP_PER_SEAT = { + 'analyse': 1, + 'discover': 2, + 'intelligence': 3, + 'enterprise': 3, +} + + +def add_to_watchlist(tenant, seat, competitor_id, url, trip_name): + """ + Adds a specific competitor trip to a seat's personal watchlist, + enforcing WATCHLIST_CAP_PER_SEAT[tenant['tier']] against the seat's + current count first — same count-vs-cap shape as + licence_service.validate_email()'s seat check and api.py's + add_seat(). Never silently overwrites or evicts an existing item. + + Data flow: + tenant + seat + competitor_id/url/trip_name → + watchlist_repository.count_for_seat(seat['id']) → + count >= cap for this tier → {'success': False, 'reason': ...} → + otherwise → watchlist_repository.create() → + {'success': True, 'item': created_record} + """ + cap = WATCHLIST_CAP_PER_SEAT.get(tenant.get('tier'), 1) + current_count = watchlist_repository.count_for_seat(seat['id']) + if current_count >= cap: + return { + 'success': False, + 'reason': f'Your watchlist is full ({current_count}/{cap}). Remove a trip to add another.', + } + + item = watchlist_repository.create({ + 'tenant_id': tenant['id'], + 'seat_id': seat['id'], + 'competitor_id': competitor_id, + 'url': url, + 'trip_name': trip_name, + }) + return {'success': True, 'item': item} + + +def remove_from_watchlist(item_id): + """ + Removes a watchlist item outright. Any active seat on the tenant may + remove any item — no per-seat ownership lock, matching this app's + existing no-fine-grained-permissions convention (e.g. CompetitorsPage + edits aren't restricted to whoever added a competitor either). + Returns True if a record was deleted, False if item_id was unknown. + """ + return watchlist_repository.delete(item_id) + + +def list_watchlist_for_tenant(tenant_id): + """ + Returns every watched item for a tenant (all seats'), each enriched + with the watching seat's email for display — a PM should be able to + see what teammates are tracking even though the cap itself is + per-seat. A seat that's since been removed (seat_repository.get_by_id + returns None) still shows the item with watched_by_email=None rather + than raising, so a removed teammate's picks don't break the whole list. + + Data flow: + tenant_id → watchlist_repository.list_for_tenant() → + per item: seat_repository.get_by_id(item['seat_id']) → + item + watched_by_email merged → list returned + """ + items = watchlist_repository.list_for_tenant(tenant_id) + enriched = [] + for item in items: + seat = seat_repository.get_by_id(item.get('seat_id')) + enriched.append({**item, 'watched_by_email': seat.get('email') if seat else None}) + return enriched diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index de6a9f5..179b724 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -96,12 +96,13 @@ def _reset_mock_repositories(): from repositories.price_history_repository import price_history_repository from repositories.client_trips_repository import client_trips_repository from repositories.ngs_meta_repository import ngs_meta_repository + from repositories.watchlist_repository import watchlist_repository for repo in [ tenant_repository, seat_repository, competitor_repository, product_repository, scrape_repository, proxy_repository, alert_repository, battlecard_repository, comparable_repository, monitoring_repository, price_history_repository, - client_trips_repository, ngs_meta_repository, + client_trips_repository, ngs_meta_repository, watchlist_repository, ]: if hasattr(repo, '_records'): repo._records.clear() diff --git a/backend/tests/repositories/test_real_repositories.py b/backend/tests/repositories/test_real_repositories.py index 85d0ba3..2724a16 100644 --- a/backend/tests/repositories/test_real_repositories.py +++ b/backend/tests/repositories/test_real_repositories.py @@ -19,6 +19,7 @@ from repositories.brief_repository import BriefRepository from repositories.monitoring_repository import MonitoringRepository from repositories.price_history_repository import PriceHistoryRepository from repositories.client_trips_repository import ClientTripsRepository +from repositories.watchlist_repository import WatchlistRepository class _FakePbClient: @@ -77,6 +78,7 @@ def test_seat_repository_delegates_correctly(monkeypatch): repo = SeatRepository() assert repo.get_active('t1', 'pm@gadventures.com') == fake.get_first_return + assert repo.get_by_id('s1') == fake.get_one_return assert repo.count_active('t1') == 2 assert repo.list_for_tenant('t1') == fake.list_return assert repo.create({'email': 'x'}) == fake.create_return @@ -108,6 +110,7 @@ def test_product_repository_delegates_correctly(monkeypatch): assert repo.create({'trip_name': 'x'}) == fake.create_return assert repo.update('p1', {'majority_price_usd': 100}) == fake.update_return assert repo.get_by_competitor_and_name('c1', 'Peru Classic') == fake.get_first_return + assert repo.get_by_competitor_and_url('c1', 'https://intrepidtravel.com/peru') == fake.get_first_return def test_product_repository_get_by_competitor_and_name_none_without_trip_name(monkeypatch): @@ -119,6 +122,15 @@ def test_product_repository_get_by_competitor_and_name_none_without_trip_name(mo assert repo.get_by_competitor_and_name('c1', '') is None +def test_product_repository_get_by_competitor_and_url_none_without_url(monkeypatch): + fake = _FakePbClient() + monkeypatch.setattr('repositories.product_repository.pb_client', fake) + repo = ProductRepository() + + assert repo.get_by_competitor_and_url('c1', None) is None + assert repo.get_by_competitor_and_url('c1', '') is None + + def test_scrape_repository_delegates_correctly(monkeypatch): fake = _FakePbClient() fake.list_return = [{'id': 'r1'}] @@ -147,7 +159,7 @@ def test_scrape_repository_get_last_hash_for_url_returns_hash(monkeypatch): monkeypatch.setattr('repositories.scrape_repository.pb_client', fake) repo = ScrapeRepository() - assert repo.get_last_hash_for_url('c1') == 'abc123' + assert repo.get_last_hash_for_url('c1', 'https://intrepidtravel.com/peru') == 'abc123' def test_scrape_repository_get_last_hash_for_url_returns_none_when_never_hashed(monkeypatch): @@ -156,7 +168,7 @@ def test_scrape_repository_get_last_hash_for_url_returns_none_when_never_hashed( monkeypatch.setattr('repositories.scrape_repository.pb_client', fake) repo = ScrapeRepository() - assert repo.get_last_hash_for_url('c1') is None + assert repo.get_last_hash_for_url('c1', 'https://intrepidtravel.com/peru') is None def test_proxy_repository_delegates_correctly(monkeypatch): @@ -268,3 +280,17 @@ def test_price_history_repository_delegates_correctly(monkeypatch): assert repo.create({'majority_price_usd': 2190}) == fake.create_return assert repo.list_for_product('p1') == fake.list_return assert fake.calls[-1][1] == 'price_history' + + +def test_watchlist_repository_delegates_correctly(monkeypatch): + fake = _FakePbClient() + fake.list_return = [{'id': 'w1'}, {'id': 'w2'}] + monkeypatch.setattr('repositories.watchlist_repository.pb_client', fake) + repo = WatchlistRepository() + + assert repo.create({'seat_id': 's1'}) == fake.create_return + assert repo.get_by_id('w1') == fake.get_one_return + assert repo.list_for_tenant('t1') == fake.list_return + assert repo.count_for_seat('s1') == 2 + assert repo.delete('w1') is True + assert fake.calls[-1] == ('delete', 'trip_watchlist', 'w1') diff --git a/backend/tests/services/test_analysis_service.py b/backend/tests/services/test_analysis_service.py index c5b03df..d21c379 100644 --- a/backend/tests/services/test_analysis_service.py +++ b/backend/tests/services/test_analysis_service.py @@ -77,6 +77,136 @@ def test_needs_refresh_true_on_malformed_timestamp(): assert analysis_service.needs_refresh({'change_detected': False, 'last_scraped': 'not-a-date'}) is True +def test_needs_refresh_url_scoped_false_when_product_for_url_is_fresh(): + # A competitor-wide change_detected=True (leftover from a DIFFERENT + # trip page's scrape) must NOT force a refresh for a url we already + # have fresh cached data for — the whole point of url-scoping. + _, competitor = _make_tenant_and_competitor(change_detected=True) + fresh = (datetime.now(timezone.utc) - timedelta(hours=1)).isoformat() + product_repository.create({ + 'tenant_id': 't1', 'competitor_id': competitor['id'], 'trip_name': 'Peru Classic', + 'url': 'https://intrepidtravel.com/peru', 'last_seen': fresh, + }) + + assert analysis_service.needs_refresh(competitor, confirmed_url='https://intrepidtravel.com/peru') is False + + +def test_needs_refresh_url_scoped_true_when_no_product_for_that_url(): + _, competitor = _make_tenant_and_competitor(change_detected=False) + fresh = (datetime.now(timezone.utc) - timedelta(hours=1)).isoformat() + product_repository.create({ + 'tenant_id': 't1', 'competitor_id': competitor['id'], 'trip_name': 'Vietnam Explorer', + 'url': 'https://intrepidtravel.com/vietnam', 'last_seen': fresh, + }) + + # Fresh data exists, but for a different page entirely. + assert analysis_service.needs_refresh(competitor, confirmed_url='https://intrepidtravel.com/peru') is True + + +def test_needs_refresh_url_scoped_true_when_product_for_url_is_stale(): + _, competitor = _make_tenant_and_competitor(change_detected=False) + stale = (datetime.now(timezone.utc) - timedelta(hours=48)).isoformat() + product_repository.create({ + 'tenant_id': 't1', 'competitor_id': competitor['id'], 'trip_name': 'Peru Classic', + 'url': 'https://intrepidtravel.com/peru', 'last_seen': stale, + }) + + assert analysis_service.needs_refresh(competitor, confirmed_url='https://intrepidtravel.com/peru') is True + + +def test_analyse_from_cache_prefers_product_matching_confirmed_url_over_latest(monkeypatch): + # Regression test — analyse_from_cache() used to always grab whichever + # product was scraped most recently for the competitor, which could be + # a completely different trip than the one this run is actually about. + tenant, competitor = _make_tenant_and_competitor() + product_repository.create({ + 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], 'trip_name': 'Peru Classic', + 'url': 'https://intrepidtravel.com/peru', 'last_seen': '2026-01-01', + }) + # Scraped later — "latest for competitor" would wrongly pick this one. + product_repository.create({ + 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], 'trip_name': 'Vietnam Explorer', + 'url': 'https://intrepidtravel.com/vietnam', 'last_seen': '2026-02-01', + }) + _install_fake_module( + monkeypatch, 'core.extractor', + extract_with_openrouter=lambda prompt, content: {'comments': 'ok', 'relevancy': 4} + ) + + result = analysis_service.analyse_from_cache( + tenant['id'], competitor['id'], {'destination': 'Peru', 'confirmed_url': 'https://intrepidtravel.com/peru'} + ) + + assert result['tripName'] == 'Peru Classic' + + +def test_run_competitor_analysis_reuses_cache_for_one_url_even_after_a_different_url_changed(monkeypatch): + # End-to-end regression test for the exact bug reported in + # production: a PM re-runs analysis on a URL they already researched, + # without checking "force fresh scrape", after a DIFFERENT trip page + # for the same competitor genuinely changed in between. Before this + # fix, that other page's change flipped change_detected back to True + # competitor-wide, which made needs_refresh() force a real re-scrape + # on literally every subsequent run for this competitor, regardless + # of which URL was actually being asked about. + tenant, competitor = _make_tenant_and_competitor() + peru_url = 'https://intrepidtravel.com/peru' + vietnam_url = 'https://intrepidtravel.com/vietnam' + + _install_fake_module(monkeypatch, 'industries.adventure_travel.prompt', build_prompt=lambda *a, **k: '') + + scraped_text = {'url': None} + + def fake_scrape(url, country=None): + return {'success': True, 'url': url, 'text': scraped_text['url']} + _install_fake_module(monkeypatch, 'core.scraper', scrape=fake_scrape) + + def fake_extract(prompt, content): + if content == 'peru v1': + return {'tripName': 'Peru Classic', 'majorityPrice': 2190} + if content in ('vietnam v1', 'vietnam v2'): + return {'tripName': 'Vietnam Explorer', 'majorityPrice': 1890} + return {'narrative': 'cached'} # analyse_from_cache's narrative-only call + _install_fake_module(monkeypatch, 'core.extractor', extract_with_openrouter=fake_extract) + + # Run 1 — scrape Peru for the first time (real extraction, no prior + # hash to compare against for this url — establishes its baseline). + scraped_text['url'] = 'peru v1' + result1 = analysis_service.run_competitor_analysis(tenant['id'], competitor, {'confirmed_url': peru_url}) + assert result1['path'] == 'refresh' + assert result1['result']['tripName'] == 'Peru Classic' + + # Run 2 — scrape Vietnam for the first time (its own baseline — + # different url, no prior hash for IT either, so still no diff yet). + competitor = competitor_repository.get_by_id(competitor['id']) + scraped_text['url'] = 'vietnam v1' + result2 = analysis_service.run_competitor_analysis(tenant['id'], competitor, {'confirmed_url': vietnam_url}) + assert result2['path'] == 'refresh' + + # Run 3 — Vietnam's content genuinely changes. force_refresh here + # only to bypass the fast-path gate (Vietnam's own product would + # otherwise still look fresh from run 2) so the hash comparison + # actually runs against the new content — this is what sets the + # competitor-wide change_detected flag True, rooted entirely in + # Vietnam's page and nothing to do with Peru. + competitor = competitor_repository.get_by_id(competitor['id']) + scraped_text['url'] = 'vietnam v2' + analysis_service.run_competitor_analysis( + tenant['id'], competitor, {'confirmed_url': vietnam_url, 'force_refresh': True} + ) + competitor = competitor_repository.get_by_id(competitor['id']) + assert competitor['change_detected'] is True # confirms the "leftover" state this fix must see past + + # Run 4 — re-run Peru again, same URL and same content as run 1, no + # force_refresh. Must hit the FAST path (no re-scrape) since Peru's + # own cached product is fresh — despite the competitor-wide + # change_detected flag still being True from Vietnam's change. + result4 = analysis_service.run_competitor_analysis(tenant['id'], competitor, {'confirmed_url': peru_url}) + + assert result4['path'] == 'fast' + assert result4['result']['tripName'] == 'Peru Classic' + + def test_analyse_from_cache_raises_without_cached_products(): _, competitor = _make_tenant_and_competitor() with pytest.raises(ValueError): @@ -103,6 +233,54 @@ def test_analyse_from_cache_uses_latest_product(monkeypatch): assert 'Peru Classic' in captured['prompt'] +def test_analyse_from_cache_grounds_comments_in_own_trip_when_present(monkeypatch): + # Regression test — the comparison prompt used to dump the raw + # context dict verbatim with no priority framing, so own_trip's + # presence was easy to under-weight against the typed fields sitting + # right next to it. own_trip must be the primary ground truth. + tenant, competitor = _make_tenant_and_competitor() + product_repository.create({ + 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], + 'trip_name': 'Peru Classic', 'last_seen': '2026-01-01', + }) + captured = {} + _install_fake_module( + monkeypatch, 'core.extractor', + extract_with_openrouter=lambda prompt, content: captured.update(prompt=prompt) or {'comments': 'ok', 'relevancy': 4} + ) + + analysis_service.analyse_from_cache(tenant['id'], competitor['id'], { + 'destination': 'Peru', 'product_name': 'Inca Trail', 'company_name': 'Wild Horizons Travel', + 'own_trip': {'tripName': 'Inca Trail Classic', 'majorityPrice': 1999, 'duration': 15, 'activities': 'Trekking'}, + }) + + assert 'ACTUAL extracted data' in captured['prompt'] + assert "Wild Horizons Travel's own Inca Trail Classic" in captured['prompt'] + assert '1999' in captured['prompt'] + + +def test_analyse_from_cache_falls_back_to_typed_form_data_without_own_trip(monkeypatch): + tenant, competitor = _make_tenant_and_competitor() + product_repository.create({ + 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], + 'trip_name': 'Peru Classic', 'last_seen': '2026-01-01', + }) + captured = {} + _install_fake_module( + monkeypatch, 'core.extractor', + extract_with_openrouter=lambda prompt, content: captured.update(prompt=prompt) or {'comments': 'ok', 'relevancy': 4} + ) + + analysis_service.analyse_from_cache(tenant['id'], competitor['id'], { + 'destination': 'Peru', 'duration': 15, 'travel_style': 'Classic', + 'product_name': 'Inca Trail', 'company_name': 'Wild Horizons Travel', + }) + + assert 'typed product intent' in captured['prompt'] + assert 'ACTUAL extracted data' not in captured['prompt'] + assert 'Inca Trail' in captured['prompt'] + + def test_analyse_from_cache_returns_structured_fields_resultstable_expects(monkeypatch): """ Regression test — a real "complete" job showed an empty-looking @@ -565,7 +743,7 @@ def test_analyse_from_cache_skips_ngs_lookup_for_standard_tab(monkeypatch): def test_detect_change_true_on_first_scrape(): _, competitor = _make_tenant_and_competitor() - assert analysis_service.detect_change(competitor['id'], 'somehash') is True + assert analysis_service.detect_change(competitor['id'], 'https://intrepidtravel.com/peru', 'somehash') is True # First scrape never had a prior hash to compare against, so it # shouldn't flag change_detected — there's nothing to diff against. assert competitor_repository.get_by_id(competitor['id'])['change_detected'] is False @@ -575,10 +753,11 @@ def test_detect_change_true_and_flags_competitor_on_diff(): _, competitor = _make_tenant_and_competitor() scrape_repository.create({ 'tenant_id': 't1', 'competitor_id': competitor['id'], 'triggered_by': 'cron', - 'status': 'complete', 'content_hash': 'old-hash', 'competitors_total': 1, 'competitors_done': 1, + 'status': 'complete', 'content_hash': 'old-hash', 'scraped_url': 'https://intrepidtravel.com/peru', + 'competitors_total': 1, 'competitors_done': 1, }) - changed = analysis_service.detect_change(competitor['id'], 'new-hash') + changed = analysis_service.detect_change(competitor['id'], 'https://intrepidtravel.com/peru', 'new-hash') assert changed is True updated = competitor_repository.get_by_id(competitor['id']) @@ -590,10 +769,40 @@ def test_detect_change_false_when_hash_unchanged(): _, competitor = _make_tenant_and_competitor() scrape_repository.create({ 'tenant_id': 't1', 'competitor_id': competitor['id'], 'triggered_by': 'cron', - 'status': 'complete', 'content_hash': 'same-hash', 'competitors_total': 1, 'competitors_done': 1, + 'status': 'complete', 'content_hash': 'same-hash', 'scraped_url': 'https://intrepidtravel.com/peru', + 'competitors_total': 1, 'competitors_done': 1, }) - assert analysis_service.detect_change(competitor['id'], 'same-hash') is False + assert analysis_service.detect_change(competitor['id'], 'https://intrepidtravel.com/peru', 'same-hash') is False + + +def test_detect_change_scoped_by_url_not_just_competitor(): + # Regression test for the exact bug reported in production: two + # different trip pages scraped for the same competitor must keep + # independent hash histories. The old, url-blind lookup compared a + # fresh scrape of URL A against whichever page was hashed MOST + # RECENTLY for this competitor (URL B here) — almost always a + # mismatch, which kept change_detected permanently stuck True. + _, competitor = _make_tenant_and_competitor() + scrape_repository.create({ + 'tenant_id': 't1', 'competitor_id': competitor['id'], 'triggered_by': 'cron', 'status': 'complete', + 'content_hash': 'hash-for-peru', 'scraped_url': 'https://intrepidtravel.com/peru', + 'competitors_total': 1, 'competitors_done': 1, + }) + # Created (and thus "most recently scraped") second — the old, + # url-blind lookup would compare against this row instead. + scrape_repository.create({ + 'tenant_id': 't1', 'competitor_id': competitor['id'], 'triggered_by': 'cron', 'status': 'complete', + 'content_hash': 'hash-for-vietnam', 'scraped_url': 'https://intrepidtravel.com/vietnam', + 'competitors_total': 1, 'competitors_done': 1, + }) + + # Re-scraping the Peru page again with the exact same content + # (unchanged) must report unchanged, even though Vietnam's hash + # (the most recently recorded row overall) differs entirely. + changed = analysis_service.detect_change(competitor['id'], 'https://intrepidtravel.com/peru', 'hash-for-peru') + + assert changed is False def test_run_competitor_analysis_refresh_path_skips_extraction_when_hash_unchanged(monkeypatch): @@ -609,7 +818,8 @@ def test_run_competitor_analysis_refresh_path_skips_extraction_when_hash_unchang prior_hash = compute_content_hash('unchanged content') scrape_repository.create({ 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], 'triggered_by': 'cron', - 'status': 'complete', 'content_hash': prior_hash, 'competitors_total': 1, 'competitors_done': 1, + 'status': 'complete', 'content_hash': prior_hash, 'scraped_url': competitor['website'], + 'competitors_total': 1, 'competitors_done': 1, }) # extract_with_openrouter backs both the full-page extraction call @@ -675,7 +885,8 @@ def test_force_refresh_bypasses_unchanged_shortcut_even_when_hash_matches(monkey prior_hash = compute_content_hash('unchanged content') scrape_repository.create({ 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], 'triggered_by': 'cron', - 'status': 'complete', 'content_hash': prior_hash, 'competitors_total': 1, 'competitors_done': 1, + 'status': 'complete', 'content_hash': prior_hash, 'scraped_url': competitor['website'], + 'competitors_total': 1, 'competitors_done': 1, }) calls_with_content = [] diff --git a/backend/tests/services/test_auth_service.py b/backend/tests/services/test_auth_service.py index 1808e61..a03f69b 100644 --- a/backend/tests/services/test_auth_service.py +++ b/backend/tests/services/test_auth_service.py @@ -54,3 +54,57 @@ def test_returns_tenant_id_for_valid_active_seat(monkeypatch): result = auth_service.get_tenant_id_from_request(_FakeRequest('Bearer validtoken')) assert result == tenant['id'] + + +# ── get_seat_from_request ────────────────────────────────────────── + +def test_get_seat_from_request_returns_none_none_without_bearer_prefix(): + assert auth_service.get_seat_from_request(_FakeRequest('not-bearer-token')) == (None, None) + + +def test_get_seat_from_request_returns_none_none_when_no_active_seat(monkeypatch): + tenant_repository.create({ + 'name': 'G Adventures', 'domain': 'gadventures.com', 'tier': 'analyse', + 'status': 'active', 'max_seats': 5, + }) + monkeypatch.setattr( + 'services.auth_service.pb_client.auth_refresh', + lambda token: {'email': 'pm@gadventures.com'} + ) + assert auth_service.get_seat_from_request(_FakeRequest('Bearer sometoken')) == (None, None) + + +def test_get_seat_from_request_returns_tenant_id_and_seat_record(monkeypatch): + tenant = tenant_repository.create({ + 'name': 'G Adventures', 'domain': 'gadventures.com', 'tier': 'analyse', + 'status': 'active', 'max_seats': 5, + }) + seat = seat_repository.create({'tenant_id': tenant['id'], 'email': 'pm@gadventures.com', 'active': True}) + monkeypatch.setattr( + 'services.auth_service.pb_client.auth_refresh', + lambda token: {'email': 'pm@gadventures.com'} + ) + + tenant_id, resolved_seat = auth_service.get_seat_from_request(_FakeRequest('Bearer validtoken')) + + assert tenant_id == tenant['id'] + assert resolved_seat['id'] == seat['id'] + assert resolved_seat['email'] == 'pm@gadventures.com' + + +def test_get_tenant_id_from_request_delegates_to_get_seat_from_request(monkeypatch): + # Regression test — get_tenant_id_from_request() is now a thin + # wrapper; confirm it still returns just the tenant_id half. + tenant = tenant_repository.create({ + 'name': 'G Adventures', 'domain': 'gadventures.com', 'tier': 'analyse', + 'status': 'active', 'max_seats': 5, + }) + seat_repository.create({'tenant_id': tenant['id'], 'email': 'pm@gadventures.com', 'active': True}) + monkeypatch.setattr( + 'services.auth_service.pb_client.auth_refresh', + lambda token: {'email': 'pm@gadventures.com'} + ) + + result = auth_service.get_tenant_id_from_request(_FakeRequest('Bearer validtoken')) + assert result == tenant['id'] + assert not isinstance(result, tuple) diff --git a/backend/tests/services/test_embedding_service.py b/backend/tests/services/test_embedding_service.py index b2b7085..f220712 100644 --- a/backend/tests/services/test_embedding_service.py +++ b/backend/tests/services/test_embedding_service.py @@ -120,3 +120,116 @@ def test_save_client_trip_keeps_embedding_when_nothing_relevant_changed(): updated = embedding_service.save_client_trip('t1', 'Peru', 15, 'Classic', product_name='Inca Trail') assert updated['embedding'] == [0.1, 0.2, 0.3] # unchanged inputs — no need to invalidate + + +def test_save_client_trip_persists_url_extracted_price_and_activities_when_given(): + trip = embedding_service.save_client_trip( + 't1', 'Peru', 15, 'Classic', product_name='Inca Trail', + url='https://client.com/inca-trail', extracted={'tripName': 'Inca Trail'}, + price_usd=1999, activities='Trekking, Camping', + ) + assert trip['url'] == 'https://client.com/inca-trail' + assert trip['extracted'] == {'tripName': 'Inca Trail'} + assert trip['price_usd'] == 1999 + assert trip['activities'] == 'Trekking, Camping' + + +def test_save_client_trip_omits_own_trip_fields_when_not_given(): + # The original typed-values-only call site (api.py's /research route) + # must be unaffected — no url/extracted/price_usd/activities keys at all. + trip = embedding_service.save_client_trip('t1', 'Peru', 15, 'Classic', product_name='Inca Trail') + assert 'url' not in trip + assert 'extracted' not in trip + assert 'price_usd' not in trip + assert 'activities' not in trip + + +def _fake_scrape(success=True, text='Peru Classic 15 days...'): + def _scrape(url): + return {'success': success, 'text': text, 'tables': '', 'url': url} + return _scrape + + +def test_scrape_own_trip_scrapes_and_extracts_when_no_cached_row(monkeypatch): + monkeypatch.setattr('core.scraper.scrape', _fake_scrape(), raising=False) + monkeypatch.setattr( + 'core.extractor.extract_with_openrouter', + lambda prompt, content='': {'tripName': 'Inca Trail Classic', 'majorityPrice': 1999, 'activities': 'Trekking'}, + raising=False, + ) + + context = { + 'destination': 'Peru', 'duration': 15, 'travel_style': 'Classic', + 'product_name': 'Inca Trail Classic', 'company_name': 'Wild Horizons', 'tab_type': 'Standard', + } + result = embedding_service.scrape_own_trip('t1', 'https://client.com/inca-trail', context) + + assert result == { + 'tripName': 'Inca Trail Classic', 'majorityPrice': 1999, 'activities': 'Trekking', + 'link': 'https://client.com/inca-trail', 'cached': False, + } + saved = client_trips_repository.get_by_tenant_and_name('t1', 'Inca Trail Classic') + assert saved['url'] == 'https://client.com/inca-trail' + assert saved['price_usd'] == 1999 + assert saved['last_scraped'] is not None + + +def test_scrape_own_trip_reuses_fresh_cached_extraction_without_rescraping(monkeypatch): + from datetime import datetime, timezone + client_trips_repository.create({ + 'id': 'ct1', 'tenant_id': 't1', 'trip_name': 'Inca Trail Classic', + 'destination': 'Peru', 'duration_days': 15, + 'extracted': {'tripName': 'Inca Trail Classic', 'majorityPrice': 1999}, + 'last_scraped': datetime.now(timezone.utc).isoformat(), + }) + + calls = [] + monkeypatch.setattr('core.scraper.scrape', lambda url: calls.append(url) or _fake_scrape()(url), raising=False) + + context = {'destination': 'Peru', 'duration': 15, 'travel_style': 'Classic', 'product_name': 'Inca Trail Classic'} + result = embedding_service.scrape_own_trip('t1', 'https://client.com/inca-trail', context) + + assert result == {'tripName': 'Inca Trail Classic', 'majorityPrice': 1999, 'cached': True} + assert calls == [] # cache was fresh — scrape() never called + + +def test_scrape_own_trip_rescrapes_when_force_refresh_even_if_fresh(monkeypatch): + from datetime import datetime, timezone + client_trips_repository.create({ + 'id': 'ct1', 'tenant_id': 't1', 'trip_name': 'Inca Trail Classic', + 'destination': 'Peru', 'duration_days': 15, + 'extracted': {'tripName': 'stale'}, 'last_scraped': datetime.now(timezone.utc).isoformat(), + }) + monkeypatch.setattr('core.scraper.scrape', _fake_scrape(), raising=False) + monkeypatch.setattr( + 'core.extractor.extract_with_openrouter', + lambda prompt, content='': {'tripName': 'fresh'}, raising=False, + ) + + context = { + 'destination': 'Peru', 'duration': 15, 'travel_style': 'Classic', + 'product_name': 'Inca Trail Classic', 'force_refresh': True, + } + result = embedding_service.scrape_own_trip('t1', 'https://client.com/inca-trail', context) + + assert result == {'tripName': 'fresh', 'link': 'https://client.com/inca-trail', 'cached': False} + + +def test_scrape_own_trip_returns_none_on_scrape_failure_without_raising(monkeypatch): + monkeypatch.setattr('core.scraper.scrape', _fake_scrape(success=False, text='Blocked'), raising=False) + + context = {'destination': 'Peru', 'duration': 15, 'travel_style': 'Classic', 'product_name': 'Inca Trail Classic'} + result = embedding_service.scrape_own_trip('t1', 'https://client.com/inca-trail', context) + + assert result is None + + +def test_scrape_own_trip_returns_none_and_never_raises_on_unexpected_error(monkeypatch): + def _raise(url): + raise RuntimeError('boom') + monkeypatch.setattr('core.scraper.scrape', _raise, raising=False) + + context = {'destination': 'Peru', 'duration': 15, 'travel_style': 'Classic', 'product_name': 'Inca Trail Classic'} + result = embedding_service.scrape_own_trip('t1', 'https://client.com/inca-trail', context) + + assert result is None diff --git a/backend/tests/services/test_jobs.py b/backend/tests/services/test_jobs.py index d458202..f4af2cc 100644 --- a/backend/tests/services/test_jobs.py +++ b/backend/tests/services/test_jobs.py @@ -130,6 +130,104 @@ def test_run_research_job_threads_real_tenant_company_name_into_context(monkeypa assert captured_contexts[0]['company_name'] == 'Wild Horizons Travel' +def test_run_research_job_threads_own_trip_result_into_context(monkeypatch): + tenant, competitor, run = _setup() + captured_contexts = [] + monkeypatch.setattr( + 'services.analysis_service.run_competitor_analysis', + lambda tenant_id, comp, ctx: captured_contexts.append(ctx) or {'path': 'refresh', 'result': {}} + ) + monkeypatch.setattr('services.alert_service.send_gotify', lambda **kw: None) + monkeypatch.setattr( + 'services.embedding_service.scrape_own_trip', + lambda tenant_id, website_url, ctx: {'tripName': 'Inca Trail Classic', 'majorityPrice': 1999}, + ) + + jobs.run_research_job(run['id'], { + 'tenant_id': tenant['id'], 'competitors': [{'id': competitor['id']}], + 'own_trip_url': 'https://wildhorizons.com/inca-trail', + }) + + assert captured_contexts[0]['own_trip'] == {'tripName': 'Inca Trail Classic', 'majorityPrice': 1999} + + +def test_run_research_job_surfaces_own_trip_in_persisted_results(monkeypatch): + # Regression test — own_trip was computed and fed into build_prompt() + # but never actually returned to the dashboard, so there was no way + # for a PM to confirm their own site was really scraped and used. + tenant, competitor, run = _setup() + monkeypatch.setattr( + 'services.analysis_service.run_competitor_analysis', + lambda tenant_id, comp, ctx: {'path': 'refresh', 'result': {}} + ) + monkeypatch.setattr('services.alert_service.send_gotify', lambda **kw: None) + monkeypatch.setattr( + 'services.embedding_service.scrape_own_trip', + lambda tenant_id, website_url, ctx: {'tripName': 'Inca Trail Classic', 'majorityPrice': 1999, 'cached': False}, + ) + + jobs.run_research_job(run['id'], { + 'tenant_id': tenant['id'], 'competitors': [{'id': competitor['id']}], + 'own_trip_url': 'https://wildhorizons.com/inca-trail', + }) + + updated = scrape_repository.get_by_id(run['id']) + assert updated['results']['own_trip'] == {'tripName': 'Inca Trail Classic', 'majorityPrice': 1999, 'cached': False} + + +def test_run_research_job_own_trip_is_none_in_results_without_own_trip_url(monkeypatch): + tenant, competitor, run = _setup() + monkeypatch.setattr( + 'services.analysis_service.run_competitor_analysis', + lambda tenant_id, comp, ctx: {'path': 'refresh', 'result': {}} + ) + monkeypatch.setattr('services.alert_service.send_gotify', lambda **kw: None) + + jobs.run_research_job(run['id'], {'tenant_id': tenant['id'], 'competitors': [{'id': competitor['id']}]}) + + updated = scrape_repository.get_by_id(run['id']) + assert updated['results']['own_trip'] is None + + +def test_run_research_job_own_trip_defaults_to_none_without_own_trip_url(monkeypatch): + tenant, competitor, run = _setup() + captured_contexts = [] + monkeypatch.setattr( + 'services.analysis_service.run_competitor_analysis', + lambda tenant_id, comp, ctx: captured_contexts.append(ctx) or {'path': 'refresh', 'result': {}} + ) + monkeypatch.setattr('services.alert_service.send_gotify', lambda **kw: None) + + jobs.run_research_job(run['id'], {'tenant_id': tenant['id'], 'competitors': [{'id': competitor['id']}]}) + + assert captured_contexts[0]['own_trip'] is None + + +def test_run_research_job_own_trip_failure_does_not_fail_the_batch(monkeypatch): + # A side-effect failure (site blocked, extraction error) must never + # fail the whole batch — CLAUDE.md §18 resilience rule. + tenant, competitor, run = _setup() + captured_contexts = [] + monkeypatch.setattr( + 'services.analysis_service.run_competitor_analysis', + lambda tenant_id, comp, ctx: captured_contexts.append(ctx) or {'path': 'refresh', 'result': {}} + ) + monkeypatch.setattr('services.alert_service.send_gotify', lambda **kw: None) + + def _raise(tenant_id, website_url, ctx): + raise RuntimeError('boom') + monkeypatch.setattr('services.embedding_service.scrape_own_trip', _raise) + + jobs.run_research_job(run['id'], { + 'tenant_id': tenant['id'], 'competitors': [{'id': competitor['id']}], + 'own_trip_url': 'https://wildhorizons.com/inca-trail', + }) + + assert captured_contexts[0]['own_trip'] is None + updated = scrape_repository.get_by_id(run['id']) + assert updated['status'] == 'complete' + + def test_run_research_job_defaults_force_refresh_to_false(monkeypatch): tenant, competitor, run = _setup() captured_contexts = [] diff --git a/backend/tests/services/test_trip_finder_service.py b/backend/tests/services/test_trip_finder_service.py index a504610..2d8c24f 100644 --- a/backend/tests/services/test_trip_finder_service.py +++ b/backend/tests/services/test_trip_finder_service.py @@ -77,6 +77,46 @@ def test_find_competitor_trip_url_handles_firecrawl_failure_gracefully(monkeypat assert result == {'url': None, 'confidence': 0, 'found': False} +def test_find_competitor_trip_url_uses_generous_map_timeout(monkeypatch): + # Regression test — a large real production site's sitemap.xml fetch + # can take well over 15s (confirmed directly: ~21.6s for a real site), + # which a too-tight timeout silently turns into a false "no match + # found" instead of a real answer. Guards against re-tightening this. + captured = {} + + class _FakeResponse: + def raise_for_status(self): + pass + + def json(self): + return {'links': []} + + def _fake_post(url, json, timeout): + captured['timeout'] = timeout + return _FakeResponse() + + monkeypatch.setattr(trip_finder_service.requests, 'post', _fake_post) + trip_finder_service.find_competitor_trip_url('https://gadventures.com', 'Peru', 15, 'Classic') + + assert captured['timeout'] == trip_finder_service.FIRECRAWL_MAP_TIMEOUT_SECONDS + assert captured['timeout'] >= 30 + + +def test_find_own_trip_url_delegates_to_find_competitor_trip_url(monkeypatch): + calls = [] + + def _fake_find(root_url, destination, duration, travel_style): + calls.append((root_url, destination, duration, travel_style)) + return {'url': f'{root_url}/peru-classic-15-days', 'confidence': 0.9, 'found': True} + + monkeypatch.setattr(trip_finder_service, 'find_competitor_trip_url', _fake_find) + + result = trip_finder_service.find_own_trip_url('https://gadventures.com', 'Peru', 15, 'Classic') + + assert calls == [('https://gadventures.com', 'Peru', 15, 'Classic')] + assert result == {'url': 'https://gadventures.com/peru-classic-15-days', 'confidence': 0.9, 'found': True} + + def test_find_all_competitor_urls_runs_concurrently_and_preserves_competitor_ids(monkeypatch): def _fake_find(root_url, destination, duration, travel_style): return {'url': f'{root_url}/match', 'confidence': 0.8, 'found': True} diff --git a/backend/tests/services/test_watchlist_service.py b/backend/tests/services/test_watchlist_service.py new file mode 100644 index 0000000..03dbb2f --- /dev/null +++ b/backend/tests/services/test_watchlist_service.py @@ -0,0 +1,139 @@ +from services import watchlist_service +from repositories.tenant_repository import tenant_repository +from repositories.seat_repository import seat_repository +from repositories.competitor_repository import competitor_repository +from repositories.watchlist_repository import watchlist_repository + + +def _make_tenant_and_seat(tier='analyse'): + tenant = tenant_repository.create({ + 'name': 'Wild Horizons Travel', 'domain': 'wildhorizons.com', 'tier': tier, + 'status': 'active', 'max_seats': 5, + }) + seat = seat_repository.create({'tenant_id': tenant['id'], 'email': 'pm@wildhorizons.com', 'active': True}) + return tenant, seat + + +def _make_competitor(tenant_id): + return competitor_repository.create({ + 'tenant_id': tenant_id, 'name': 'Intrepid Travel', 'website': 'https://intrepidtravel.com', + 'catalogue_url': 'https://intrepidtravel.com/adventures', 'primary_market': 'GB', 'active': True, + }) + + +def test_add_to_watchlist_succeeds_under_cap(): + tenant, seat = _make_tenant_and_seat() + competitor = _make_competitor(tenant['id']) + + result = watchlist_service.add_to_watchlist( + tenant, seat, competitor['id'], 'https://intrepidtravel.com/peru-classic-15-days', 'Peru Classic' + ) + + assert result['success'] is True + assert result['item']['competitor_id'] == competitor['id'] + assert result['item']['url'] == 'https://intrepidtravel.com/peru-classic-15-days' + assert watchlist_repository.count_for_seat(seat['id']) == 1 + + +def test_add_to_watchlist_blocked_at_analyse_cap_of_one(): + tenant, seat = _make_tenant_and_seat(tier='analyse') + competitor = _make_competitor(tenant['id']) + watchlist_service.add_to_watchlist(tenant, seat, competitor['id'], 'https://intrepidtravel.com/peru', 'Peru') + + result = watchlist_service.add_to_watchlist( + tenant, seat, competitor['id'], 'https://intrepidtravel.com/vietnam', 'Vietnam' + ) + + assert result['success'] is False + assert '1/1' in result['reason'] + assert watchlist_repository.count_for_seat(seat['id']) == 1 # unchanged — never silently overwrites + + +def test_add_to_watchlist_respects_higher_discover_cap(): + tenant, seat = _make_tenant_and_seat(tier='discover') + competitor = _make_competitor(tenant['id']) + + first = watchlist_service.add_to_watchlist(tenant, seat, competitor['id'], 'https://intrepidtravel.com/peru', 'Peru') + second = watchlist_service.add_to_watchlist(tenant, seat, competitor['id'], 'https://intrepidtravel.com/vietnam', 'Vietnam') + third = watchlist_service.add_to_watchlist(tenant, seat, competitor['id'], 'https://intrepidtravel.com/kenya', 'Kenya') + + assert first['success'] is True + assert second['success'] is True + assert third['success'] is False # Discover cap is 2/seat + + +def test_add_to_watchlist_defaults_to_cap_of_one_for_unknown_tier(): + tenant, seat = _make_tenant_and_seat(tier='unknown-future-tier') + competitor = _make_competitor(tenant['id']) + + first = watchlist_service.add_to_watchlist(tenant, seat, competitor['id'], 'https://intrepidtravel.com/peru', 'Peru') + second = watchlist_service.add_to_watchlist(tenant, seat, competitor['id'], 'https://intrepidtravel.com/vietnam', 'Vietnam') + + assert first['success'] is True + assert second['success'] is False + + +def test_two_seats_each_get_their_own_independent_slot(): + tenant, seat_a = _make_tenant_and_seat() + seat_b = seat_repository.create({'tenant_id': tenant['id'], 'email': 'other@wildhorizons.com', 'active': True}) + competitor = _make_competitor(tenant['id']) + + result_a = watchlist_service.add_to_watchlist(tenant, seat_a, competitor['id'], 'https://intrepidtravel.com/peru', 'Peru') + result_b = watchlist_service.add_to_watchlist(tenant, seat_b, competitor['id'], 'https://intrepidtravel.com/vietnam', 'Vietnam') + + assert result_a['success'] is True + assert result_b['success'] is True # seat_a being at cap doesn't block seat_b + + +def test_remove_from_watchlist_frees_the_slot(): + tenant, seat = _make_tenant_and_seat() + competitor = _make_competitor(tenant['id']) + added = watchlist_service.add_to_watchlist(tenant, seat, competitor['id'], 'https://intrepidtravel.com/peru', 'Peru') + + removed = watchlist_service.remove_from_watchlist(added['item']['id']) + + assert removed is True + assert watchlist_repository.count_for_seat(seat['id']) == 0 + second = watchlist_service.add_to_watchlist(tenant, seat, competitor['id'], 'https://intrepidtravel.com/vietnam', 'Vietnam') + assert second['success'] is True + + +def test_remove_from_watchlist_returns_false_for_unknown_item(): + assert watchlist_service.remove_from_watchlist('does-not-exist') is False + + +def test_list_watchlist_for_tenant_enriches_with_seat_email(): + tenant, seat = _make_tenant_and_seat() + competitor = _make_competitor(tenant['id']) + watchlist_service.add_to_watchlist(tenant, seat, competitor['id'], 'https://intrepidtravel.com/peru', 'Peru Classic') + + items = watchlist_service.list_watchlist_for_tenant(tenant['id']) + + assert len(items) == 1 + assert items[0]['watched_by_email'] == 'pm@wildhorizons.com' + assert items[0]['trip_name'] == 'Peru Classic' + + +def test_list_watchlist_for_tenant_shows_items_from_every_seat(): + tenant, seat_a = _make_tenant_and_seat() + seat_b = seat_repository.create({'tenant_id': tenant['id'], 'email': 'other@wildhorizons.com', 'active': True}) + competitor = _make_competitor(tenant['id']) + watchlist_service.add_to_watchlist(tenant, seat_a, competitor['id'], 'https://intrepidtravel.com/peru', 'Peru') + watchlist_service.add_to_watchlist(tenant, seat_b, competitor['id'], 'https://intrepidtravel.com/vietnam', 'Vietnam') + + items = watchlist_service.list_watchlist_for_tenant(tenant['id']) + + emails = {item['watched_by_email'] for item in items} + assert emails == {'pm@wildhorizons.com', 'other@wildhorizons.com'} + + +def test_list_watchlist_for_tenant_handles_removed_seat_gracefully(): + tenant, seat = _make_tenant_and_seat() + competitor = _make_competitor(tenant['id']) + watchlist_service.add_to_watchlist(tenant, seat, competitor['id'], 'https://intrepidtravel.com/peru', 'Peru') + seat_repository.delete(seat['id']) # seat later removed from the team + + items = watchlist_service.list_watchlist_for_tenant(tenant['id']) + + assert len(items) == 1 + assert items[0]['watched_by_email'] is None diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 10fd35c..680725d 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -113,6 +113,53 @@ def test_research_find_urls_requires_fields(client, api_headers): assert response.json['code'] == 'missing_field' +def test_research_find_urls_omits_own_trip_without_tenant_id(client, api_headers, monkeypatch): + monkeypatch.setattr( + 'api.trip_finder_service.find_all_competitor_urls', + lambda competitors, destination, duration, travel_style: [], + ) + response = client.post('/research/find-urls', json={ + 'destination': 'Peru', 'duration': 15, 'travel_style': 'Classic', 'competitor_ids': [], + }, headers=api_headers) + + assert response.status_code == 200 + assert response.json['own_trip'] is None + + +def test_research_find_urls_omits_own_trip_when_tenant_has_no_website(client, api_headers, monkeypatch): + tenant = _make_tenant() + monkeypatch.setattr( + 'api.trip_finder_service.find_all_competitor_urls', + lambda competitors, destination, duration, travel_style: [], + ) + response = client.post('/research/find-urls', json={ + 'tenant_id': tenant['id'], 'destination': 'Peru', 'duration': 15, + 'travel_style': 'Classic', 'competitor_ids': [], + }, headers=api_headers) + + assert response.status_code == 200 + assert response.json['own_trip'] is None + + +def test_research_find_urls_includes_own_trip_when_tenant_has_website(client, api_headers, monkeypatch): + tenant = _make_tenant(website='https://wildhorizons.com') + monkeypatch.setattr( + 'api.trip_finder_service.find_all_competitor_urls', + lambda competitors, destination, duration, travel_style: [], + ) + monkeypatch.setattr( + 'api.trip_finder_service.find_own_trip_url', + lambda website, destination, duration, travel_style: {'url': f'{website}/peru-classic', 'confidence': 0.9, 'found': True}, + ) + response = client.post('/research/find-urls', json={ + 'tenant_id': tenant['id'], 'destination': 'Peru', 'duration': 15, + 'travel_style': 'Classic', 'competitor_ids': [], + }, headers=api_headers) + + assert response.status_code == 200 + assert response.json['own_trip'] == {'url': 'https://wildhorizons.com/peru-classic', 'confidence': 0.9, 'found': True} + + def test_research_enqueues_job_without_touching_real_redis(client, api_headers, monkeypatch): import jobs tenant = _make_tenant() diff --git a/backend/tests/test_api_extended.py b/backend/tests/test_api_extended.py index abb4fea..56a6b9d 100644 --- a/backend/tests/test_api_extended.py +++ b/backend/tests/test_api_extended.py @@ -124,6 +124,35 @@ def test_update_timezone_never_overwrites_existing(client, api_headers, monkeypa assert response.json['timezone'] == 'America/Toronto' # unchanged — first-write-wins +def test_update_website_requires_session(client, api_headers): + response = client.patch('/account/website', json={'website': 'https://client.com'}, headers=api_headers) + assert response.status_code == 401 + + +def test_update_website_requires_field(client, api_headers, monkeypatch): + tenant = _make_tenant() + monkeypatch.setattr('api.auth_service.get_tenant_id_from_request', lambda req: tenant['id']) + + response = client.patch('/account/website', json={}, headers=api_headers) + + assert response.status_code == 400 + assert response.json['code'] == 'missing_field' + + +def test_update_website_is_freely_editable_unlike_timezone(client, api_headers, monkeypatch): + tenant = tenant_repository.create({ + 'name': 'Wild Horizons', 'domain': 'wildhorizons.com', 'tier': 'analyse', + 'status': 'active', 'max_seats': 5, 'website': 'https://old.wildhorizons.com', + }) + monkeypatch.setattr('api.auth_service.get_tenant_id_from_request', lambda req: tenant['id']) + + response = client.patch('/account/website', json={'website': 'https://wildhorizons.com'}, headers=api_headers) + + assert response.status_code == 200 + assert response.json['website'] == 'https://wildhorizons.com' + assert tenant_repository.get_by_id(tenant['id'])['website'] == 'https://wildhorizons.com' + + def test_update_competitor(client, api_headers): tenant = _make_tenant() competitor = competitor_repository.create({ @@ -610,17 +639,101 @@ def test_research_status_unknown_job_404(client, api_headers): assert response.status_code == 404 -def test_research_status_known_job(client, api_headers): +def test_research_status_known_job(client, api_headers, monkeypatch): + import jobs tenant = _make_tenant() run = scrape_repository.create({ 'tenant_id': tenant['id'], 'triggered_by': 'manual', 'status': 'running', 'competitors_total': 2, 'competitors_done': 1, 'results': {}, }) + # Genuinely still executing — the reconciliation check must leave it alone. + monkeypatch.setattr(jobs.Job, 'fetch', lambda job_id, connection=None: types.SimpleNamespace(get_status=lambda: 'started')) + response = client.get(f'/research/status/{run["id"]}', headers=api_headers) assert response.status_code == 200 + assert response.json['status'] == 'running' assert response.json['progress'] == {'total': 2, 'done': 1} +def test_research_status_reconciles_orphaned_job_to_cancelled(client, api_headers, monkeypatch): + # Regression test — a PM clicks Cancel, then the worker process + # handling the job is killed outright (e.g. a container rebuild) + # before run_research_job() ever reaches its cooperative cancel + # checkpoint. Without reconciliation, scrape_runs stays 'running' + # forever and the dashboard shows "Stopping…" indefinitely. + import jobs + tenant = _make_tenant() + run = scrape_repository.create({ + 'tenant_id': tenant['id'], 'triggered_by': 'manual', 'status': 'running', + 'cancel_requested': True, 'competitors_total': 5, 'competitors_done': 3, 'results': {}, + }) + monkeypatch.setattr(jobs.Job, 'fetch', lambda job_id, connection=None: types.SimpleNamespace(get_status=lambda: 'failed')) + + response = client.get(f'/research/status/{run["id"]}', headers=api_headers) + + assert response.status_code == 200 + assert response.json['status'] == 'cancelled' + updated = scrape_repository.get_by_id(run['id']) + assert updated['status'] == 'cancelled' + assert updated['completed_at'] is not None + + +def test_research_status_reconciles_orphaned_job_to_failed_without_cancel_request(client, api_headers, monkeypatch): + # Same orphaned-worker scenario, but the PM never clicked cancel — + # an uncaught exception or a killed worker should surface as a real + # failure, not silently stay 'running' forever either. + import jobs + tenant = _make_tenant() + run = scrape_repository.create({ + 'tenant_id': tenant['id'], 'triggered_by': 'manual', 'status': 'running', + 'competitors_total': 5, 'competitors_done': 2, 'results': {}, + }) + monkeypatch.setattr(jobs.Job, 'fetch', lambda job_id, connection=None: types.SimpleNamespace(get_status=lambda: 'failed')) + + response = client.get(f'/research/status/{run["id"]}', headers=api_headers) + + assert response.status_code == 200 + assert response.json['status'] == 'failed' + assert response.json['error'] + + +def test_research_status_reconciles_when_rq_has_no_record_at_all(client, api_headers, monkeypatch): + import jobs + tenant = _make_tenant() + run = scrape_repository.create({ + 'tenant_id': tenant['id'], 'triggered_by': 'manual', 'status': 'running', + 'cancel_requested': True, 'competitors_total': 3, 'competitors_done': 1, 'results': {}, + }) + + def _raise_no_such_job(job_id, connection=None): + raise jobs.NoSuchJobError() + monkeypatch.setattr(jobs.Job, 'fetch', _raise_no_such_job) + + response = client.get(f'/research/status/{run["id"]}', headers=api_headers) + + assert response.status_code == 200 + assert response.json['status'] == 'cancelled' + + +def test_research_status_does_not_touch_terminal_jobs(client, api_headers, monkeypatch): + """A job already complete/failed/cancelled must never hit the RQ reconciliation path at all.""" + import jobs + tenant = _make_tenant() + run = scrape_repository.create({ + 'tenant_id': tenant['id'], 'triggered_by': 'manual', 'status': 'complete', + 'competitors_total': 1, 'competitors_done': 1, 'results': {'success': []}, + }) + + def _fail_if_called(job_id, connection=None): + raise AssertionError('must not check RQ for an already-terminal job') + monkeypatch.setattr(jobs.Job, 'fetch', _fail_if_called) + + response = client.get(f'/research/status/{run["id"]}', headers=api_headers) + + assert response.status_code == 200 + assert response.json['status'] == 'complete' + + def test_research_history_detail_unknown_404(client, api_headers): response = client.get('/research/history/nope', headers=api_headers) assert response.status_code == 404 @@ -841,6 +954,165 @@ def test_mark_alert_read_success(client, api_headers, monkeypatch): assert response.json['read'] is True +# ── Trip watchlist ───────────────────────────────────────────────── + +def _make_competitor(tenant_id, **overrides): + data = { + 'tenant_id': tenant_id, 'name': 'Intrepid Travel', 'website': 'https://intrepidtravel.com', + 'catalogue_url': 'https://intrepidtravel.com/adventures', 'primary_market': 'GB', 'active': True, + } + data.update(overrides) + return competitor_repository.create(data) + + +def test_get_watchlist_requires_session(client, api_headers): + response = client.get('/watchlist', headers=api_headers) + assert response.status_code == 401 + + +def test_get_watchlist_returns_tenant_items(client, api_headers, monkeypatch): + from services import watchlist_service + tenant = _make_tenant() + seat = seat_repository.create({'tenant_id': tenant['id'], 'email': 'pm@gadventures.com', 'active': True}) + competitor = _make_competitor(tenant['id']) + watchlist_service.add_to_watchlist(tenant, seat, competitor['id'], 'https://intrepidtravel.com/peru', 'Peru Classic') + monkeypatch.setattr('api.auth_service.get_tenant_id_from_request', lambda req: tenant['id']) + + response = client.get('/watchlist', headers=api_headers) + + assert response.status_code == 200 + assert len(response.json) == 1 + assert response.json[0]['watched_by_email'] == 'pm@gadventures.com' + + +def test_add_watchlist_item_requires_session(client, api_headers): + response = client.post('/watchlist', json={'competitor_id': 'c1', 'url': 'x'}, headers=api_headers) + assert response.status_code == 401 + + +def test_add_watchlist_item_requires_fields(client, api_headers, monkeypatch): + tenant = _make_tenant() + seat = seat_repository.create({'tenant_id': tenant['id'], 'email': 'pm@gadventures.com', 'active': True}) + monkeypatch.setattr('api.auth_service.get_seat_from_request', lambda req: (tenant['id'], seat)) + + response = client.post('/watchlist', json={}, headers=api_headers) + + assert response.status_code == 400 + assert response.json['code'] == 'missing_field' + + +def test_add_watchlist_item_unknown_competitor_404(client, api_headers, monkeypatch): + tenant = _make_tenant() + seat = seat_repository.create({'tenant_id': tenant['id'], 'email': 'pm@gadventures.com', 'active': True}) + monkeypatch.setattr('api.auth_service.get_seat_from_request', lambda req: (tenant['id'], seat)) + + response = client.post( + '/watchlist', json={'competitor_id': 'does-not-exist', 'url': 'https://x.com/y'}, headers=api_headers + ) + + assert response.status_code == 404 + + +def test_add_watchlist_item_rejects_other_tenants_competitor(client, api_headers, monkeypatch): + tenant = _make_tenant() + other_tenant = _make_tenant(domain='otherco.com', email='pm@otherco.com') + competitor = _make_competitor(other_tenant['id']) + seat = seat_repository.create({'tenant_id': tenant['id'], 'email': 'pm@gadventures.com', 'active': True}) + monkeypatch.setattr('api.auth_service.get_seat_from_request', lambda req: (tenant['id'], seat)) + + response = client.post( + '/watchlist', json={'competitor_id': competitor['id'], 'url': 'https://intrepidtravel.com/peru'}, + headers=api_headers + ) + + assert response.status_code == 404 + + +def test_add_watchlist_item_success(client, api_headers, monkeypatch): + tenant = _make_tenant() + seat = seat_repository.create({'tenant_id': tenant['id'], 'email': 'pm@gadventures.com', 'active': True}) + competitor = _make_competitor(tenant['id']) + monkeypatch.setattr('api.auth_service.get_seat_from_request', lambda req: (tenant['id'], seat)) + + response = client.post( + '/watchlist', + json={'competitor_id': competitor['id'], 'url': 'https://intrepidtravel.com/peru', 'trip_name': 'Peru Classic'}, + headers=api_headers + ) + + assert response.status_code == 201 + assert response.json['url'] == 'https://intrepidtravel.com/peru' + assert response.json['seat_id'] == seat['id'] + + +def test_add_watchlist_item_blocked_at_cap(client, api_headers, monkeypatch): + tenant = _make_tenant() # analyse tier — cap is 1/seat + seat = seat_repository.create({'tenant_id': tenant['id'], 'email': 'pm@gadventures.com', 'active': True}) + competitor = _make_competitor(tenant['id']) + monkeypatch.setattr('api.auth_service.get_seat_from_request', lambda req: (tenant['id'], seat)) + + client.post( + '/watchlist', json={'competitor_id': competitor['id'], 'url': 'https://intrepidtravel.com/peru'}, + headers=api_headers + ) + response = client.post( + '/watchlist', json={'competitor_id': competitor['id'], 'url': 'https://intrepidtravel.com/vietnam'}, + headers=api_headers + ) + + assert response.status_code == 400 + assert response.json['code'] == 'watchlist_full' + + +def test_remove_watchlist_item_requires_session(client, api_headers): + response = client.delete('/watchlist/w1', headers=api_headers) + assert response.status_code == 401 + + +def test_remove_watchlist_item_unknown_404(client, api_headers, monkeypatch): + tenant = _make_tenant() + monkeypatch.setattr('api.auth_service.get_tenant_id_from_request', lambda req: tenant['id']) + response = client.delete('/watchlist/does-not-exist', headers=api_headers) + assert response.status_code == 404 + + +def test_remove_watchlist_item_rejects_other_tenants_item(client, api_headers, monkeypatch): + from services import watchlist_service + tenant = _make_tenant() + other_tenant = _make_tenant(domain='otherco.com', email='pm@otherco.com') + other_seat = seat_repository.create({'tenant_id': other_tenant['id'], 'email': 'pm@otherco.com', 'active': True}) + other_competitor = _make_competitor(other_tenant['id']) + added = watchlist_service.add_to_watchlist( + other_tenant, other_seat, other_competitor['id'], 'https://intrepidtravel.com/peru', 'Peru' + ) + monkeypatch.setattr('api.auth_service.get_tenant_id_from_request', lambda req: tenant['id']) + + response = client.delete(f'/watchlist/{added["item"]["id"]}', headers=api_headers) + + assert response.status_code == 404 + + +def test_remove_watchlist_item_success_frees_the_slot(client, api_headers, monkeypatch): + from services import watchlist_service + tenant = _make_tenant() + seat = seat_repository.create({'tenant_id': tenant['id'], 'email': 'pm@gadventures.com', 'active': True}) + competitor = _make_competitor(tenant['id']) + added = watchlist_service.add_to_watchlist( + tenant, seat, competitor['id'], 'https://intrepidtravel.com/peru', 'Peru' + ) + monkeypatch.setattr('api.auth_service.get_tenant_id_from_request', lambda req: tenant['id']) + + response = client.delete(f'/watchlist/{added["item"]["id"]}', headers=api_headers) + + assert response.status_code == 200 + monkeypatch.setattr('api.auth_service.get_seat_from_request', lambda req: (tenant['id'], seat)) + retry = client.post( + '/watchlist', json={'competitor_id': competitor['id'], 'url': 'https://intrepidtravel.com/vietnam'}, + headers=api_headers + ) + assert retry.status_code == 201 + + def test_preview_prompt(client, api_headers, monkeypatch): _install_fake_module( monkeypatch, 'industries.adventure_travel.prompt', diff --git a/backend/tests/test_industries_adventure_travel.py b/backend/tests/test_industries_adventure_travel.py index 579f0c9..d207c7b 100644 --- a/backend/tests/test_industries_adventure_travel.py +++ b/backend/tests/test_industries_adventure_travel.py @@ -42,6 +42,52 @@ def test_build_prompt_falls_back_to_generic_placeholder_without_company_name(): assert 'G Adventures' not in result +def test_build_prompt_enriches_comparison_with_real_own_trip_data_when_given(): + page_data = {'text': 'content', 'tables': '', 'url': 'https://example.com'} + own_trip = { + 'tripName': 'Inca Trail Classic', 'majorityPrice': 1999, + 'duration': 15, 'activities': 'Trekking, Camping', 'serviceLevel': 'Mid-range', + } + result = prompt.build_prompt( + 'Competitor', page_data, 'Inca Trail Classic', 'Peru', 15, 'Classic', + company_name='Wild Horizons Travel', own_trip=own_trip, + ) + assert 'Inca Trail Classic' in result + assert '1999' in result + assert 'Trekking, Camping' in result + assert "Wild Horizons Travel's own Inca Trail Classic" in result + + +def test_build_prompt_falls_back_to_generic_comments_wording_without_own_trip(): + page_data = {'text': 'content', 'tables': '', 'url': 'https://example.com'} + result = prompt.build_prompt( + 'Competitor', page_data, 'Inca Trail Classic', 'Peru', 15, 'Classic', + company_name='Wild Horizons Travel', + ) + assert 'key differences from Wild Horizons Travel,' in result + assert 'own site' not in result + + +def test_build_own_trip_prompt_frames_extraction_as_the_clients_own_page_not_a_competitor(): + page_data = {'text': 'Peru Classic 15 days itinerary...', 'tables': '', 'url': 'https://client.com/peru'} + result = prompt.build_own_trip_prompt( + page_data, 'Inca Trail Classic', 'Peru', 15, 'Classic', 'Wild Horizons Travel', + ) + assert "Wild Horizons Travel's own tour page" in result + assert 'not a competitor' in result + assert '"tripName"' in result + assert '"relevancy"' not in result + assert '"comments"' not in result + + +def test_build_own_trip_prompt_includes_ngs_fields_when_flagged(): + page_data = {'text': 'content', 'tables': '', 'url': 'https://client.com/peru'} + result = prompt.build_own_trip_prompt( + page_data, 'Product', 'Peru', 10, 'NGS', 'Wild Horizons Travel', is_ngs=True, + ) + assert '"exclusiveAccess"' in result + + def test_build_prompt_includes_ngs_fields_when_flagged(): page_data = {'text': 'content', 'tables': '', 'url': 'https://example.com'} result = prompt.build_prompt('Competitor', page_data, 'Product', 'Peru', 10, 'NGS', is_ngs=True) diff --git a/frontend/src/components/activity/WatchlistCard.vue b/frontend/src/components/activity/WatchlistCard.vue new file mode 100644 index 0000000..2a567ea --- /dev/null +++ b/frontend/src/components/activity/WatchlistCard.vue @@ -0,0 +1,55 @@ + + + + + diff --git a/frontend/src/components/analyse/ResultsTable.vue b/frontend/src/components/analyse/ResultsTable.vue index 29ad1cb..68d0e6b 100644 --- a/frontend/src/components/analyse/ResultsTable.vue +++ b/frontend/src/components/analyse/ResultsTable.vue @@ -4,14 +4,84 @@ // frontend/src/config/resultFields.js for the row order/labels, verified // directly against the real completed run in "G Compete App V1.xlsx". import { computed } from 'vue' -import { getRowsForTabType, rawValue, formatForDisplay } from '../../config/resultFields' +import { getRowsForTabType, rawValue, formatForDisplay, formatLabel } from '../../config/resultFields' const props = defineProps({ - results: { type: Array, required: true }, // [{ competitor_id, name, path, result, product, is_generic_page }] - tabType: { type: String, default: 'Standard' } + results: { type: Array, required: true }, // [{ competitor_id, name, path, unchanged, result, product, is_generic_page }] + tabType: { type: String, default: 'Standard' }, + // The tenant's own extracted trip (client self-scrape feature) — null + // when no website was confirmed for this run. Shown as its own + // column before every competitor so it's visible that this data was + // actually fetched and grounding the analysis, not just typed intent. + ownTrip: { type: Object, default: null }, + ownTripLabel: { type: String, default: 'Your trip' }, + companyName: { type: String, default: '' }, + // Tenant-wide watchlist items (every seat's) — used only to determine + // whether the CURRENT seat (currentSeatEmail) already watches a given + // competitor's trip, so the button reads "★ Watching" vs "☆ Watch". + watchlist: { type: Array, default: () => [] }, + currentSeatEmail: { type: String, default: '' } }) +const emit = defineEmits(['watch', 'unwatch']) + +// Not shown on the own-trip column — watching your own trip isn't +// meaningful, only competitor trips are monitored. +function watchedItemFor(item) { + const url = rawValue({ key: 'link' }, item) + if (!url) return null + return props.watchlist.find((w) => + w.competitor_id === item.competitor_id && w.url === url && w.watched_by_email === props.currentSeatEmail + ) || null +} + +function toggleWatch(item) { + const existing = watchedItemFor(item) + if (existing) { + emit('unwatch', { itemId: existing.id }) + return + } + emit('watch', { + competitorId: item.competitor_id, + url: rawValue({ key: 'link' }, item), + tripName: rawValue({ key: 'tripName' }, item), + }) +} + const rows = computed(() => getRowsForTabType(props.tabType)) +// Synthetic "item" shape so rawValue()/the cache pill work identically +// for the own-trip column and every competitor column. +const ownTripItem = computed(() => ({ + name: props.ownTripLabel, result: props.ownTrip, path: props.ownTrip?.cached ? 'fast' : 'refresh', +})) + +// Only the cached case gets an indicator — that's the noteworthy state +// worth flagging (§7's fast path served data instead of a live scrape); +// a fresh scrape is the default expectation and needs no badge. +function isFromCache(item) { + return item.path === 'fast' || !!item.unchanged +} + +function escapeHtml(str) { + return str.replace(/&/g, '&').replace(//g, '>') +} + +// Bolds the comparison-section labels build_prompt()'s own JSON schema +// instruction asks the LLM for — "key differences from {company}, " +// "strengths, weaknesses, pricing position" — wherever they appear in +// the comments row, so a PM can skim multiple competitors' analysis at +// a glance. Only the comments cell ever renders via v-html — text is +// HTML-escaped first, then only our own tags are reinserted +// around an exact keyword match, so this can't be used to inject +// arbitrary markup even though the underlying text originates from an +// LLM extraction. +function boldKeywords(value) { + if (value == null || value === '') return '—' + return escapeHtml(String(value)).replace( + /((?:key )?differences:|strengths?:|weakness(?:es)?:|pricing(?: position)?:)/gi, + '$1' + ) +}