From e0de2df6bf3c56a3850a72d81929ef553e2d53d1 Mon Sep 17 00:00:00 2001 From: JasonFraser Date: Thu, 2 Jul 2026 11:23:23 -0400 Subject: [PATCH] Phase 5 + 6: automation, content-hash change detection, semantic trip matching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5 — Automation: - Daily scrape cron, daily digest email, and Monday brief n8n workflows - price_history_repository.py + price diffing/is_new detection wired into analysis_service's refresh path - Numeric unread-count badge on the topbar bell (was a plain dot) Change detection architecture correction: - CLAUDE.md's Firecrawl Monitor design doesn't match the real self-hosted product: no /v1/monitor in v1, the real /v2/monitor is cron-scheduled and credit-metered (Supabase-gated), and self-hosted Firecrawl is a 3-service stack (api/worker/playwright-service), not the single container originally spec'd - Replaced with content-hash comparison computed during the scrape itself (core/scraper.py's compute_content_hash/normalise_html, analysis_service's detect_change()) — no external dependency, and the refresh path now skips LLM extraction entirely when a scrape finds no content diff - Fixed a real bug in the first pass of this migration: change_detected was set and then immediately reset within the same function call, making it unobservable to the CompetitorsPage badge, the Apps Script sidebar, and the n8n cron filter. It now stays visible until the next scrape confirms nothing further changed. - Added a composite index on scrape_runs(competitor_id, content_hash, started_at) backing the per-competitor recency lookup - Docker Compose's firecrawl service replaced with the real firecrawl-api/firecrawl-worker/firecrawl-playwright stack, gated behind an opt-in profile - Added CORS (absent from the original spec entirely) so the browser-based dashboard can reach the API cross-origin Phase 6 — Semantic trip matching: - client_trips_repository.py — this collection existed in schema but nothing ever populated it; comparable-trip matching could never produce a real match without it - embedding_service.save_client_trip() upserts a client_trips record from every /research submission's trip-intent description, with stale-embedding invalidation when destination/duration change - /internal/embed-products now embeds client_trips as well as competitor products; /internal/match-comparable reads both sides from PocketBase instead of expecting client_trips in the request body, so it's callable unattended by n8n - comparable_matches upserts on (client_product, competitor_product) instead of always creating — first_matched/last_matched only make sense if repeat weekly matches update in place, and a PM's dismissed match now survives re-matching - embedding_and_matching_cron.json — Sunday night n8n workflow 249 tests passing, 96% coverage. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 372 ++++++++++++------ backend/api.py | 167 ++++---- backend/app.py | 3 +- backend/core/scraper.py | 24 ++ backend/extensions.py | 27 ++ backend/jobs.py | 20 +- backend/pb_migrations/001_initial_schema.js | 30 +- backend/repositories/alert_repository.py | 20 + .../repositories/client_trips_repository.py | 58 +++ backend/repositories/comparable_repository.py | 30 ++ backend/repositories/competitor_repository.py | 11 - .../repositories/price_history_repository.py | 38 ++ backend/repositories/product_repository.py | 26 ++ backend/repositories/scrape_repository.py | 30 +- backend/requirements.txt | 1 + backend/services/alert_service.py | 38 ++ backend/services/analysis_service.py | 192 ++++++++- backend/services/email_service.py | 13 + backend/services/embedding_service.py | 40 ++ backend/templates/emails/daily_digest.html | 54 +++ backend/tests/conftest.py | 5 +- .../repositories/test_real_repositories.py | 56 ++- backend/tests/services/test_alert_service.py | 58 ++- .../tests/services/test_analysis_service.py | 277 ++++++++++++- .../tests/services/test_battlecard_service.py | 16 +- .../tests/services/test_embedding_service.py | 46 +++ backend/tests/services/test_jobs.py | 73 ++++ backend/tests/test_api.py | 36 ++ backend/tests/test_api_extended.py | 137 +++---- frontend/src/assets/css/main.css | 2 +- frontend/src/components/layout/Topbar.vue | 2 +- n8n/workflows/daily_digest.json | 96 +++++ n8n/workflows/daily_scrape_cron.json | 160 ++++++++ .../embedding_and_matching_cron.json | 114 ++++++ n8n/workflows/monday_brief.json | 96 +++++ 35 files changed, 2033 insertions(+), 335 deletions(-) create mode 100644 backend/repositories/client_trips_repository.py create mode 100644 backend/repositories/price_history_repository.py create mode 100644 backend/templates/emails/daily_digest.html create mode 100644 n8n/workflows/daily_digest.json create mode 100644 n8n/workflows/daily_scrape_cron.json create mode 100644 n8n/workflows/embedding_and_matching_cron.json create mode 100644 n8n/workflows/monday_brief.json diff --git a/CLAUDE.md b/CLAUDE.md index 3e89e3b..e851a90 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -141,10 +141,9 @@ Analyse tier is live with paying clients. | Scraping engine | app.py (Flask) | Existing — do not rewrite | | Job queue | RQ (Redis Queue) | Async job processing | | Queue broker | Redis | Shared with Firecrawl | -| Monitoring | Firecrawl self-hosted | /monitor — real-time change detection (all tiers) | -| Trip-intent matching | Firecrawl self-hosted | /map on demand — finds competitor trip URLs (Analyse+) | -| Discovery | Firecrawl self-hosted | /map scheduled — weekly catalogue crawl (Discover+ only) | -| Change detection | Firecrawl Monitor + PocketBase flags | Event-driven — fires only on detected change | +| Trip-intent matching | Firecrawl self-hosted | /v1/map on demand — finds competitor trip URLs (Analyse+) | +| Discovery | Firecrawl self-hosted | /v1/map scheduled — weekly catalogue crawl (Discover+ only) | +| Change detection | Content-hash diff in n8n cron | SHA-256 comparison per daily scrape — no external service | | Proxy — primary | Webshare | ~$3/month, free tier available, used by default | | Proxy — fallback | Bright Data | Pay-as-you-go, auto-escalated per domain when Webshare blocked | | AI extraction | OpenRouter | Free tier model cycling with claude-haiku-4-5 paid fallback | @@ -208,11 +207,14 @@ Template setup (one-time per PM, via AccountPage.vue) Bettersight available, and pullLatestResults() writes into whichever workbook and whichever tab is selected in the sidebar dropdown -Firecrawl Monitor (event-driven, runs continuously) - → Watches all active competitor URLs - → Detects content change (JS-rendered pages, React/Vue SPAs supported) - → Fires webhook → Flask /internal/monitor-webhook - → Sets competitor.change_detected = true in PocketBase +Change detection (via daily cron content-hash comparison) + → n8n daily scrape cron computes SHA-256 of normalised page content + → Compares against previous scrape's content_hash in scrape_runs + → If changed: sets competitor.change_detected = true, records new hash + → If unchanged: skips LLM extraction (fast path knows to use cache) + → Rationale: replaces Firecrawl Monitor which self-hosted requires + credit accounting + Supabase and only runs on cron anyway. Daily + granularity is appropriate for adventure travel pricing cycles. n8n (scheduled workflows) → Daily cron (6am) — scrapes changed/stale competitors @@ -226,77 +228,34 @@ n8n (scheduled workflows) Vue 3 dashboard auth + account → PocketBase auth (Google OAuth or email/password) → Stripe (payment, subscription status) - → On competitor add → Flask registers with Firecrawl Monitor automatically ``` -### Change Detection — Firecrawl Monitor +### Change Detection — Content-Hash Comparison -Firecrawl Monitor is the change detection layer. It replaces both -changedetection.io (previously removed) and the plain HTTP hash approach -(rejected due to JS-heavy page limitations). +Change detection is computed in-process by the scrape service itself, +during the daily scrape cron — not by an external monitoring service. +This replaces both changedetection.io (previously removed) and +Firecrawl Monitor (evaluated and rejected — see Section 4's Compose +Services note and Section 27 "What NOT to Build"). -**Why Firecrawl Monitor over alternatives:** -- Renders JS fully before hashing — works on React/Vue SPAs -- Event-driven — fires only on change, not on a polling schedule -- Lower server load than a 2-4hr Playwright cron -- Single service handles both monitoring (/monitor) and discovery (/map for Intelligence tier) -- Already in stack — no new infrastructure +**Why content-hash diff over an external monitor:** +- No dependency on Firecrawl's credit-metered, cron-scheduled v2 Monitor + feature, which requires Supabase auth even in self-hosted mode +- Runs as a normal step of the scrape job already being performed daily +- Renders JS fully before hashing (same Playwright render used for + extraction) — works on React/Vue SPAs +- Daily granularity matches adventure travel pricing update cadence **PocketBase competitor record flags:** ``` -change_detected bool — set true by monitor webhook, reset after scrape +change_detected bool — set true when the scrape service detects a hash diff, reset after next scrape change_detected_at date — timestamp of last detected change last_scraped date — timestamp of last completed scrape ``` -**Monitor registration — on competitor add:** -```python -def register_competitor_monitor(competitor_id: str, url: str): - """ - Registers a competitor URL with Firecrawl Monitor on first add. - Stores the monitor ID in PocketBase for future deregistration. - - Data flow: - competitor url → Firecrawl POST /v1/monitor → - monitor_id returned → stored in competitors.firecrawl_monitor_id - """ - response = requests.post( - "http://firecrawl:3002/v1/monitor", - json={ - "url": url, - "webhook": "https://api.bettersight.io/internal/monitor-webhook", - "changeTypes": ["content"] - } - ) - monitor_id = response.json().get("id") - competitor_repository.update(competitor_id, { - "firecrawl_monitor_id": monitor_id - }) -``` - -**Monitor webhook handler:** -```python -@app.route('/internal/monitor-webhook', methods=['POST']) -def monitor_webhook(): - """ - Receives Firecrawl Monitor webhook on competitor page change. - Sets change_detected flag on the competitor record. - - Data flow: - Firecrawl webhook payload → extract competitor URL → - PocketBase competitor lookup by URL → - change_detected = true, change_detected_at = now - """ - data = request.json - url = data.get('url') - competitor = competitor_repository.get_by_url(url) - if competitor: - competitor_repository.update(competitor['id'], { - 'change_detected': True, - 'change_detected_at': datetime.now().isoformat() - }) - return jsonify({'status': 'ok'}) -``` +See Section 10 "Content-hash change detection" for `compute_content_hash()` +and `detect_change()` — the functions that implement this during +`jobs.py`'s scrape job. ### Docker Network @@ -346,19 +305,77 @@ services: - pangolin_default restart: unless-stopped - firecrawl: - image: ghcr.io/mendableai/firecrawl:latest - container_name: firecrawl - environment: - - REDIS_URL=redis://bettersight-redis:6379 - - USE_DB_AUTHENTICATION=false - - PORT=3002 - networks: - - pangolin_default - restart: unless-stopped - depends_on: - - bettersight-redis +``` +### Firecrawl — real self-hosted stack + +Firecrawl self-hosted is a multi-service application, not a single +container. It needs api + worker + playwright-service and shares the +existing `bettersight-redis` container. No Supabase, no Postgres — +those are only required if you enable the credit/auth system, which +Bettersight does not use. + +```yaml +firecrawl-api: + build: + context: https://github.com/firecrawl/firecrawl.git#main + dockerfile: apps/api/Dockerfile + container_name: firecrawl-api + environment: + - PORT=3002 + - HOST=0.0.0.0 + - REDIS_URL=redis://bettersight-redis:6379 + - REDIS_RATE_LIMIT_URL=redis://bettersight-redis:6379 + - PLAYWRIGHT_MICROSERVICE_URL=http://firecrawl-playwright:3000/scrape + - USE_DB_AUTHENTICATION=false + - NUM_WORKERS_PER_QUEUE=8 + depends_on: + - bettersight-redis + - firecrawl-playwright + networks: + - pangolin_default + restart: unless-stopped + +firecrawl-worker: + build: + context: https://github.com/firecrawl/firecrawl.git#main + dockerfile: apps/api/Dockerfile + container_name: firecrawl-worker + command: pnpm run workers + environment: + - REDIS_URL=redis://bettersight-redis:6379 + - REDIS_RATE_LIMIT_URL=redis://bettersight-redis:6379 + - PLAYWRIGHT_MICROSERVICE_URL=http://firecrawl-playwright:3000/scrape + - USE_DB_AUTHENTICATION=false + depends_on: + - bettersight-redis + - firecrawl-playwright + networks: + - pangolin_default + restart: unless-stopped + +firecrawl-playwright: + build: + context: https://github.com/firecrawl/firecrawl.git#main + dockerfile: apps/playwright-service-ts/Dockerfile + container_name: firecrawl-playwright + environment: + - PORT=3000 + networks: + - pangolin_default + restart: unless-stopped + mem_limit: 4g +``` + +Resource budget: the playwright-service alone wants ~2-4GB RAM. +The CX31 VPS (2 vCPU, 8GB RAM) sized in Section 31 handles this +alongside PocketBase, Flask, and 3 RQ workers because Firecrawl is +only used for /v1/map calls (trip-intent matching + weekly discovery), +never continuously. Only the api service is called by Bettersight — at +`http://firecrawl-api:3002/v1/map`. Worker and playwright-service are +internal-only. + +```yaml freescout: image: freescout/freescout:latest container_name: freescout @@ -675,7 +692,6 @@ Services (src/services/) - Table of active competitors with status badges (✓ Current, ⚠ Changed, ○ Stale) - Primary market flag per competitor - Add competitor modal — name, website, catalogue URL, primary market (required) -- On add → auto-registers with Firecrawl Monitor - Edit/deactivate actions - [Download CSV template] button — pre-formatted with correct headers - [Upload CSV] button — bulk import with preview and validation @@ -768,8 +784,7 @@ name text website url catalogue_url url primary_market text (ISO country code e.g. GB, US, AU — required on add) -firecrawl_monitor_id text (returned by Firecrawl on monitor registration) -change_detected bool (set true by monitor webhook, reset after scrape) +change_detected bool (set true by scrape service on content-hash diff, reset after next scrape) change_detected_at date (timestamp of last detected change) active bool last_scraped date @@ -855,6 +870,26 @@ started_at autodate completed_at date ``` +### scrape_runs (additions for content-hash detection) + +Add these fields to the existing scrape_runs collection: + +``` +content_hash text # SHA-256 of normalised page content +content_length integer # raw byte length of normalised content +hash_algorithm text # "sha256" — allows future migration +``` + +Indexed via a composite index — `(competitor_id, content_hash, started_at)` +— rather than on content_hash alone, since every lookup is scoped to +one competitor and sorted by recency (`get_last_hash_for_url()`). +An index on content_hash by itself wouldn't serve that query. + +The `competitors.change_detected` and `competitors.change_detected_at` +fields already exist — no schema change needed for them. Semantics +change from "set by Firecrawl webhook" to "set by scrape service on +hash diff." No repository or service signature changes. + ### alerts ``` id auto @@ -1321,7 +1356,7 @@ POST /account/competitors — add single competitor (requires: name, web POST /account/competitors/bulk — bulk import from CSV (validates, previews, imports) GET /account/competitors/template — download blank CSV template with correct headers PUT /account/competitors/ -DELETE /account/competitors/ — deactivates + deregisters Firecrawl Monitor +DELETE /account/competitors/ — deactivates the competitor record GET /account/template PATCH /account/onboarding — update onboarding_checklist fields GET /billing/portal — Stripe customer portal session URL @@ -1339,9 +1374,6 @@ POST /internal/battlecard/ POST /internal/embed-products/ POST /internal/match-comparable/ POST /internal/weekly-brief/ -POST /internal/monitor-webhook -POST /internal/monitor/register/ -POST /internal/monitor/deregister/ POST /internal/discover/ — Discover tier only ``` @@ -1376,6 +1408,72 @@ fast/slow path check around the existing logic. When running as an RQ job, `jobs.py` imports from `core/scraper.py` and `core/extractor.py`. Never bypass the layer structure. +### Content-hash change detection + +Every scrape job computes a content hash and compares against the last +recorded hash for the same competitor URL. This replaces Firecrawl +Monitor's role in the original design. + +```python +import hashlib + +def compute_content_hash(page_html: str) -> str: + """ + Returns SHA-256 hash of normalised page content. + Normalisation strips whitespace, script blocks, and known + ephemeral elements (timestamps, session IDs) so cosmetic changes + do not trigger false positives. + """ + normalised = normalise_html(page_html) + return hashlib.sha256(normalised.encode('utf-8')).hexdigest() + + +def detect_change(competitor_id: str, new_hash: str) -> bool: + """ + Compares new_hash against last recorded hash on scrape_runs. + On change: sets competitor.change_detected = true, + competitor.change_detected_at = now. Never clears the flag itself — + detection and extraction happen in the same scrape now (unlike the + old async Firecrawl Monitor webhook), so there's no "detected but + not yet handled" state for this function to resolve. Clearing is + the caller's responsibility — see below. + """ + last = scrape_repository.get_last_hash_for_url(competitor_id) + if last is None: + return True # first scrape — always process + if last != new_hash: + competitor_repository.update(competitor_id, { + 'change_detected': True, + 'change_detected_at': datetime.now().isoformat() + }) + return True + return False +``` + +**`change_detected` stays visible until confirmed stable, not until "handled."** +The scrape that finds a change also fully extracts and saves it — so +there's no later "handled" event to reset the flag on. The caller +(`analysis_service.run_competitor_analysis()`) does NOT reset it after +a successful extraction; it leaves the flag as `detect_change()` set +it, so the CompetitorsPage "⚠ Changed" badge, the Apps Script sidebar +status, and the next day's n8n cron filter (`change_detected = true || +stale`) can all still see it. It's only cleared on the *next* scrape +that finds the content unchanged since — at most one extra hash-only +confirmation scrape per genuine change, with no LLM cost since an +unchanged result always skips extraction. + +An earlier version of this design reset `change_detected` to false +immediately after every successful extraction, in the same function +call that `detect_change()` set it — meaning nothing outside that one +call could ever observe it as true. That silently broke the status +badge and the cron filter's `change_detected = true` clause. Do not +reintroduce that reset. + +Fields on `scrape_runs` collection gain: `content_hash`, `content_length`, +`hash_algorithm` — see Section 6's "scrape_runs (additions for +content-hash detection)" for the composite index that backs +`get_last_hash_for_url()`'s competitor-scoped, recency-sorted lookup. + --- ## 9. Licence Validation Logic @@ -1810,6 +1908,25 @@ Return ONLY valid JSON: ## 12. Semantic Trip Matching +### Client trip population + +`client_trips` has no PM-facing form — it's populated automatically +every time a PM submits `/research` (CLAUDE.md §8's TripIntentForm). +The destination/duration/travel_style/product_name they describe IS a +client trip, so `POST /research` upserts it via +`embedding_service.save_client_trip()` before enqueueing the job — a +side effect that never blocks the primary research job if it fails +(§18 resilience rule). + +Upsert key: `(tenant_id, trip_name)`, where `trip_name` is the PM's +product name if given, else a `{destination} {travel_style}` fallback. +Re-running analysis for the same product updates the existing row +instead of duplicating it. If `destination` or `duration_days` change +on an update, the stored `embedding` is cleared (set to null) so the +next embedding pass regenerates it — `/internal/embed-products` only +embeds rows missing a vector, so a stale one would otherwise never get +refreshed. + ### Embedding generation After products are stored, embed each new trip using text-embedding-3-small. @@ -1856,7 +1973,20 @@ def find_comparable_trips(client_trips, competitor_trips, threshold=0.75): """ ``` -Threshold: 0.75. Matches stored in comparable_matches table. +Threshold: 0.75. Matches upserted into comparable_matches on +`(tenant_id, client_product, competitor_product)` — a repeat weekly +match updates `similarity_score`/`price_difference`/`duration_difference`/ +`last_matched` in place rather than duplicating a row, which is the +whole reason `first_matched`/`last_matched` exist as separate fields. +A PM's `dismissed = true` on a match is left untouched by the upsert, +so dismissing a match survives it matching again in a later run. + +`/internal/embed-products/` embeds both un-embedded +competitor products AND un-embedded client_trips — both sides need a +vector before `/internal/match-comparable/` can compare +them, and both are read from PocketBase (via their repositories), not +passed in the request body, so the Sunday-night n8n workflow can call +either route unattended per tenant. --- @@ -2500,6 +2630,22 @@ Schedule Trigger (Monday 06:00 AST) → Logged to digest_logs ``` +### Embedding + comparable matching (Sunday night — Analyse tier, in MVP) +``` +Schedule Trigger (Sunday 23:00) + → For each active tenant: + → HTTP POST /internal/embed-products/{tenant_id} + → Embeds any un-embedded competitor products AND client_trips + → HTTP POST /internal/match-comparable/{tenant_id} + → Semantic similarity matching, upserted into comparable_matches + → client_trips itself is populated continuously by every /research + submission (embedding_service.save_client_trip()) — this workflow + only embeds and matches what's already there, it never creates + client_trips records itself. +``` +Not to be confused with the Intelligence-tier catalogue discovery +workflow below, which is a different, deferred feature. + ### Sunday night catalogue discovery (Intelligence tier — NOT in MVP) ``` DEFERRED — do not build until Intelligence tier development begins. @@ -2869,9 +3015,12 @@ Phase 5 — Automation 37. n8n daily scrape cron — changed/stale competitors only 38. PocketBase price_history diff logic (change_amount, change_percent) 39. PocketBase is_new flag logic (first_seen detection) - 40. Flask /internal/monitor-webhook - 41. Flask /internal/monitor/register + /deregister - 42. Auto-register Firecrawl Monitor on competitor add + 40. Content-hash logic in scrape service — compute_content_hash(), + detect_change(), normalise_html() + 41. Add content_hash, content_length, hash_algorithm fields to + scrape_runs collection + 42. Verify daily cron sets competitor.change_detected on hash diff + and skips LLM extraction when hash unchanged + cache fresh 43. n8n PocketBase webhook → create price_change alert record in PocketBase 44. n8n PocketBase webhook → create new_product alert record in PocketBase 45. n8n daily digest email (6pm) — collects unread alerts → Resend email @@ -3620,7 +3769,6 @@ Step 3 — Preview modal shown before import Step 4 — PM clicks [Import 3 valid rows] → POST /account/competitors/bulk with valid rows only - → Each competitor registered with Firecrawl Monitor → Success toast: "3 competitors imported successfully" → CompetitorsPage table refreshes ``` @@ -3739,7 +3887,6 @@ def download_competitors_template(): def bulk_import_competitors(): """ Bulk imports competitors from validated CSV rows. - Registers each competitor with Firecrawl Monitor after creation. Skips rows where a competitor with the same domain already exists for this tenant — never creates duplicates. @@ -3747,7 +3894,6 @@ def bulk_import_competitors(): validated rows array → for each row: check for duplicate domain in tenant's competitors → if new: create competitor record in PocketBase → - register with Firecrawl Monitor → append to created list return { created: N, skipped: N, errors: [] } """ @@ -3784,11 +3930,6 @@ def bulk_import_competitors(): 'active': True }) - # Register with Firecrawl Monitor - monitor_service.register( - competitor['id'], - row['catalogue_url'] - ) created.append(row['name']) except Exception as e: @@ -5007,6 +5148,16 @@ Do not build any of the following until explicitly instructed: - Analysis UI in the Apps Script sidebar — dashboard only - Custom Sheet functions (=BETTERSIGHT_PRICE() etc) — not needed - REST API for external integrations — not in scope +- Real-time change detection webhooks. Daily cron with content-hash + comparison is sufficient for adventure travel pricing cycles. Do not + reintroduce Firecrawl Monitor, changedetection.io, or any other + event-driven change detection service. If a client explicitly requests + intra-day change detection, add a second cron slot at 6pm local time + rather than adding a new service. +- Supabase, Postgres for Firecrawl, or the Firecrawl credit accounting + system. Bettersight uses Firecrawl only for /v1/map (unauthenticated, + no credits consumed). Enabling auth/credits would create billing + coupling to Firecrawl Cloud which is not needed. Note: Firecrawl /map IS in the Analyse tier MVP — used on demand for trip-intent URL matching only. The scheduled weekly /map crawl @@ -5612,19 +5763,20 @@ Run: docker compose up -d --scale bettersight-worker=5 --- -### Component 5 — Firecrawl Monitor Health Check +### Component 5 — Content-Hash Detection Health Check -Firecrawl Monitor must stay healthy — if it stops firing webhooks, -change detection breaks silently with no user-visible error. +The daily scrape cron's content-hash comparison must keep running — if +it silently stops writing `content_hash` on `scrape_runs`, change +detection breaks with no user-visible error. ``` n8n — daily 8am check: - → Query PocketBase: competitors where change_detected_at < 7 days ago - → If any competitor has not fired a monitor event in 7 days: - → HTTP GET http://firecrawl:3002/health - → If unhealthy: Gotify critical alert - → If healthy but no events: Gotify advisory - (may mean competitor sites haven't changed — normal) + → Query PocketBase: scrape_runs from the last 24hrs where content_hash is empty + → If any recent scrape_runs row is missing content_hash: + → Gotify critical alert (hash computation broke in the scrape service) + → If all recent scrape_runs have content_hash but change_detected + hasn't fired for any competitor in 7 days: + → Gotify advisory (may mean competitor sites haven't changed — normal) ``` --- @@ -5696,7 +5848,7 @@ The alerts tell you what to do and give you the exact command. | 🔴 Scale workers now (depth 10+) | Run scale command immediately from phone. | | 🟡 Daily cron running long (>2hrs) | Plan to scale workers within a week. | | 🔴 Daily cron overrunning (>3hrs) | Scale workers today. | -| 🟡 Weekly summary — fast path < 40% | Investigate why cache is missing. Check Firecrawl Monitor health. | +| 🟡 Weekly summary — fast path < 40% | Investigate why cache is missing. Check the daily scrape cron's content-hash detection health. | | 🟡 Weekly summary — 18+ clients noted | Proactively scale workers before next client onboards. | **Scale command (copy from alert message):** diff --git a/backend/api.py b/backend/api.py index ca5707e..e3ede92 100644 --- a/backend/api.py +++ b/backend/api.py @@ -27,6 +27,7 @@ from services import billing_service from services import auth_service from services import analysis_service from services import email_service +from services import alert_service logger = logging.getLogger(__name__) @@ -35,7 +36,6 @@ api_bp = Blueprint('api', __name__) API_SECRET_KEY = os.getenv('API_SECRET_KEY') STRIPE_WEBHOOK_SECRET = os.getenv('STRIPE_WEBHOOK_SECRET') TEMPLATE_SHEET_URL = os.getenv('TEMPLATE_SHEET_URL') -FIRECRAWL_URL = os.getenv('FIRECRAWL_URL', 'http://firecrawl:3002') POCKETBASE_URL = os.getenv('POCKETBASE_URL', 'http://pocketbase:8090') @@ -92,43 +92,6 @@ def _competitor_status(competitor): return 'current' -def _register_monitor(competitor_id, url): - """ - Registers a competitor URL with Firecrawl Monitor. Side-effect - failure is logged only — a monitor registration failure must never - block the competitor add operation itself (CLAUDE.md §18 resilience). - - Data flow: - competitor_id + url → POST {FIRECRAWL_URL}/v1/monitor → - monitor_id → competitors.firecrawl_monitor_id updated - """ - try: - response = requests.post( - f'{FIRECRAWL_URL}/v1/monitor', - json={ - 'url': url, - 'webhook': 'https://api.bettersight.io/internal/monitor-webhook', - 'changeTypes': ['content'], - }, - timeout=10 - ) - monitor_id = response.json().get('id') - if monitor_id: - competitor_repository.update(competitor_id, {'firecrawl_monitor_id': monitor_id}) - except Exception as e: - logger.error(f'Firecrawl monitor registration failed for {competitor_id}: {str(e)}') - - -def _deregister_monitor(monitor_id): - """Deregisters a Firecrawl monitor. Side-effect failure is logged only.""" - if not monitor_id: - return - try: - requests.delete(f'{FIRECRAWL_URL}/v1/monitor/{monitor_id}', timeout=10) - except Exception as e: - logger.error(f'Firecrawl monitor deregistration failed for {monitor_id}: {str(e)}') - - def _extract_domain(url): netloc = urlparse(url if '://' in url else f'//{url}').netloc or url return netloc.lower().lstrip('www.') @@ -256,6 +219,7 @@ def find_urls(): def research(): """Enqueues a research job to RQ — never runs synchronously.""" from jobs import enqueue_research_job + from services import embedding_service data = request.json or {} required = ['tenant_id', 'destination', 'duration', 'travel_style', 'tab_type', 'competitors'] @@ -263,6 +227,19 @@ def research(): if missing: return error_response('missing_field', f'Missing fields: {", ".join(missing)}', 400) + # Persists the PM's trip-intent as a client_trips record — the only + # place this collection ever gets populated. Without it, Phase 6's + # comparable-trip matching has nothing on the client side to embed + # or compare against. A save failure is a side effect — never + # blocks the primary research job from queuing (CLAUDE.md §18). + try: + embedding_service.save_client_trip( + data['tenant_id'], data['destination'], data['duration'], + data['travel_style'], product_name=data.get('productName') or data.get('product_name'), + ) + except Exception as e: + logger.error(f'save_client_trip failed for tenant {data["tenant_id"]}: {str(e)}') + run = scrape_repository.create({ 'tenant_id': data['tenant_id'], 'triggered_by': 'manual', @@ -532,7 +509,6 @@ def add_competitor(): 'catalogue_url': data['catalogue_url'], 'primary_market': data['primary_market'].upper(), 'active': True, }) - _register_monitor(competitor['id'], data['catalogue_url']) return jsonify(competitor), 201 @@ -576,7 +552,6 @@ def bulk_import_competitors(): 'website': row['website'].strip(), 'catalogue_url': row['catalogue_url'].strip(), 'primary_market': row['primary_market'].upper().strip(), 'active': True, }) - _register_monitor(competitor['id'], row['catalogue_url']) created.append(row['name']) except Exception as e: errors.append({'name': row.get('name', 'Unknown'), 'error': str(e)}) @@ -601,7 +576,6 @@ def delete_competitor(competitor_id): if not competitor: return error_response('not_found', 'Unknown competitor', 404) competitor_repository.update(competitor_id, {'active': False}) - _deregister_monitor(competitor.get('firecrawl_monitor_id')) return jsonify({'status': 'ok'}) @@ -741,39 +715,84 @@ def internal_battlecard(competitor_id): @api_bp.route('/internal/embed-products/', methods=['POST']) @require_api_key def internal_embed_products(tenant_id): - """Phase 6 — embeds any of a tenant's products missing an embedding vector.""" + """ + Phase 6 — embeds any of a tenant's competitor products AND client + trips missing an embedding vector. Both sides need a vector before + /internal/match-comparable can compare them — this runs first in + the Sunday-night n8n workflow. + """ from services import embedding_service from repositories.product_repository import product_repository as products_repo + from repositories.client_trips_repository import client_trips_repository - embedded = 0 + embedded_products = 0 for product in products_repo.list_for_tenant(tenant_id): if product.get('embedding'): continue try: vector = embedding_service.embed_trip(product) products_repo.update(product['id'], {'embedding': vector}) - embedded += 1 + embedded_products += 1 except Exception as e: logger.error(f'Embedding failed for product {product["id"]}: {str(e)}') - return jsonify({'embedded': embedded}) + + embedded_client_trips = 0 + for trip in client_trips_repository.list_for_tenant(tenant_id): + if trip.get('embedding'): + continue + try: + vector = embedding_service.embed_trip(trip) + client_trips_repository.update(trip['id'], {'embedding': vector}) + embedded_client_trips += 1 + except Exception as e: + logger.error(f'Embedding failed for client trip {trip["id"]}: {str(e)}') + + return jsonify({ + 'embedded': embedded_products + embedded_client_trips, + 'embedded_products': embedded_products, + 'embedded_client_trips': embedded_client_trips, + }) @api_bp.route('/internal/match-comparable/', methods=['POST']) @require_api_key def internal_match_comparable(tenant_id): - """Phase 6 — runs semantic similarity matching for a tenant's client trips vs competitor products.""" + """ + Phase 6 — runs semantic similarity matching for a tenant's client + trips vs competitor products. Reads both sides from PocketBase + (via their repositories) rather than expecting client_trips in the + request body, so this route is callable unattended by n8n — the + same pattern as every other /internal/* route in this file. + """ + from datetime import datetime, timezone from services import embedding_service from repositories.product_repository import product_repository as products_repo + from repositories.client_trips_repository import client_trips_repository - client_trips = request.json.get('client_trips', []) if request.json else [] + client_trips = client_trips_repository.list_for_tenant(tenant_id) competitor_products = products_repo.list_for_tenant(tenant_id) matches = embedding_service.find_comparable_trips(client_trips, competitor_products) - created = [ - comparable_repository.create({'tenant_id': tenant_id, **match}) - for match in matches - ] - return jsonify({'matches_created': len(created)}) + # Upserts on (client_product, competitor_product) so a repeat weekly + # match updates last_matched/scores in place instead of duplicating + # a row every Sunday — first_matched/last_matched only make sense + # if the same pair is tracked across runs, not recreated each time. + # A dismissed match's dismissed flag is deliberately left untouched + # on update — the PM's "not relevant" call should survive a re-match. + now = datetime.now(timezone.utc).isoformat() + created, updated = [], [] + for match in matches: + existing = comparable_repository.get_by_client_and_competitor_product( + tenant_id, match['client_product'], match['competitor_product'] + ) + if existing: + comparable_repository.update(existing['id'], {**match, 'last_matched': now}) + updated.append(existing['id']) + else: + comparable_repository.create({'tenant_id': tenant_id, 'last_matched': now, **match}) + created.append(match) + + return jsonify({'matches_created': len(created), 'matches_updated': len(updated)}) @api_bp.route('/internal/weekly-brief/', methods=['POST']) @@ -783,6 +802,14 @@ def internal_weekly_brief(tenant_id): return jsonify({'status': 'ok'}) +@api_bp.route('/internal/daily-digest/', methods=['POST']) +@require_api_key +def internal_daily_digest(tenant_id): + """Called once per active tenant by the n8n 6pm daily-digest workflow (CLAUDE.md §15).""" + sent = alert_service.send_daily_digest(tenant_id) + return jsonify({'sent': sent}) + + @api_bp.route('/internal/welcome-email/', methods=['POST']) @require_api_key def internal_welcome_email(tenant_id): @@ -843,42 +870,6 @@ def internal_alert_email(alert_id): return jsonify({'sent': sent}) -@api_bp.route('/internal/monitor-webhook', methods=['POST']) -@require_api_key -def monitor_webhook(): - """Receives Firecrawl Monitor webhook on competitor page change.""" - from datetime import datetime, timezone - data = request.json or {} - url = data.get('url') - competitor = competitor_repository.get_by_url(url) if url else None - if competitor: - competitor_repository.update(competitor['id'], { - 'change_detected': True, - 'change_detected_at': datetime.now(timezone.utc).isoformat(), - }) - return jsonify({'status': 'ok'}) - - -@api_bp.route('/internal/monitor/register/', methods=['POST']) -@require_api_key -def internal_monitor_register(competitor_id): - competitor = competitor_repository.get_by_id(competitor_id) - if not competitor: - return error_response('not_found', 'Unknown competitor', 404) - _register_monitor(competitor_id, competitor.get('catalogue_url') or competitor.get('website')) - return jsonify({'status': 'ok'}) - - -@api_bp.route('/internal/monitor/deregister/', methods=['POST']) -@require_api_key -def internal_monitor_deregister(competitor_id): - competitor = competitor_repository.get_by_id(competitor_id) - if not competitor: - return error_response('not_found', 'Unknown competitor', 404) - _deregister_monitor(competitor.get('firecrawl_monitor_id')) - return jsonify({'status': 'ok'}) - - # ── SYSTEM ────────────────────────────────────────────────────────── @api_bp.route('/health', methods=['GET']) diff --git a/backend/app.py b/backend/app.py index cbfe90f..663f7c9 100644 --- a/backend/app.py +++ b/backend/app.py @@ -5,7 +5,7 @@ from dotenv import load_dotenv load_dotenv() logging.basicConfig(level=logging.INFO) -from extensions import limiter # noqa: E402 +from extensions import limiter, cors # noqa: E402 from api import api_bp # noqa: E402 # Entry point only — imports and wires everything together. Business @@ -13,6 +13,7 @@ from api import api_bp # noqa: E402 # api.py. Nothing else belongs in this file per CLAUDE.md §5/§18. app = Flask(__name__) limiter.init_app(app) +cors.init_app(app) app.register_blueprint(api_bp) # Sentry initialisation is a Phase 7 ship-gate item (CLAUDE.md §19 step diff --git a/backend/core/scraper.py b/backend/core/scraper.py index 05de2a6..04b8a91 100644 --- a/backend/core/scraper.py +++ b/backend/core/scraper.py @@ -1,4 +1,5 @@ import re +import hashlib import asyncio import logging from urllib.parse import urlparse, urlencode, parse_qs, urlunparse @@ -55,6 +56,29 @@ def check_robots_txt(url): return True +def normalise_html(content): + """ + Normalises scraped page content before hashing so cosmetic-only + differences (whitespace, digit/word-case variation) never trigger a + false change_detected. Operates on scrape()'s already-cleaned `text` + field — script/style/nav/footer tags are stripped upstream in + _render_with_proxy() — so this only needs to collapse whitespace + and lowercase for a stable comparison. + """ + return re.sub(r'\s+', ' ', content).strip().lower() + + +def compute_content_hash(content): + """ + Returns the SHA-256 hash of normalised page content. Used by the + daily scrape job to detect competitor page changes without an + external monitoring service (CLAUDE.md §4/§10 — replaces the + Firecrawl Monitor feature, which self-hosted requires credit + accounting and Supabase auth). + """ + return hashlib.sha256(normalise_html(content).encode('utf-8')).hexdigest() + + def clean_url(url): """Strips URL fragments and common tracking parameters.""" parsed = urlparse(url) diff --git a/backend/extensions.py b/backend/extensions.py index 34d5c98..c379ad6 100644 --- a/backend/extensions.py +++ b/backend/extensions.py @@ -1,4 +1,5 @@ import os +from flask_cors import CORS from flask_limiter import Limiter from flask_limiter.util import get_remote_address @@ -12,3 +13,29 @@ limiter = Limiter( storage_uri=os.getenv('RATELIMIT_STORAGE_URI', os.getenv('REDIS_URL', 'redis://bettersight-redis:6379')), default_limits=['200 per hour', '30 per minute'], ) + +# CORS was absent from CLAUDE.md entirely, but the Vue dashboard +# (api_service.js) calls the Flask API directly from the browser via +# Axios — that's a cross-origin request the moment the dashboard and +# API are served from different hosts/ports. Confirmed true in local +# dev (Vite on :5173, Flask on :5050) via VITE_API_BASE_URL; CLAUDE.md +# doesn't pin down the production hostnames, but Pangolin fronting a +# separate flask-api container makes a same-origin setup unlikely +# there either. Without this, every browser-originated API call is +# silently blocked by the browser's CORS preflight check regardless of +# X-API-Key being correct. +# +# ALLOWED_ORIGINS is a comma-separated env var so each environment +# (dev/staging/production) lists only its own dashboard origin(s) — +# CORS is deliberately not wildcarded ("*") since credentials-bearing +# requests (the PocketBase Bearer JWT) require an explicit origin. +ALLOWED_ORIGINS = [ + origin.strip() + for origin in os.getenv('ALLOWED_ORIGINS', 'http://localhost:5173,https://app.bettersight.io').split(',') + if origin.strip() +] +cors = CORS( + resources={r'/*': {'origins': ALLOWED_ORIGINS}}, + supports_credentials=True, + allow_headers=['Content-Type', 'Authorization', 'X-API-Key', 'X-User-Email'], +) diff --git a/backend/jobs.py b/backend/jobs.py index 4216557..1a5938d 100644 --- a/backend/jobs.py +++ b/backend/jobs.py @@ -8,7 +8,7 @@ from rq import Queue from repositories.scrape_repository import scrape_repository from repositories.competitor_repository import competitor_repository -from services import analysis_service, alert_service +from services import analysis_service, alert_service, battlecard_service logger = logging.getLogger(__name__) @@ -87,6 +87,24 @@ def run_research_job(job_id, data): try: outcome = analysis_service.run_competitor_analysis(tenant_id, competitor, context) success.append({'competitor_id': competitor['id'], 'name': competitor['name'], **outcome}) + + # Battlecard regeneration fires on genuinely fresh data only — + # the fast path just re-narrates data already reflected in the + # current battlecard, and an 'unchanged' refresh (content-hash + # diff found no change — see analysis_service.detect_change()) + # extracted nothing new either, so neither has anything to + # regenerate from. CLAUDE.md §15 describes this as n8n + # reacting to a scrape_runs-complete PocketBase webhook; wired + # directly here instead since jobs.py already knows the exact + # moment a competitor's data actually changed, which is more + # reliable than a webhook round trip this environment has no + # way to verify actually fires. A regeneration failure is a + # side-effect failure — never aborts the job. + if outcome.get('path') == 'refresh' and not outcome.get('unchanged'): + try: + battlecard_service.generate_battlecard(competitor['id'], tenant_id) + except Exception as e: + logger.error(f'Battlecard regeneration failed for {competitor["name"]}: {str(e)}') except Exception as e: logger.error(f'{competitor.get("name", entry)} failed in job {job_id}: {str(e)}') failed.append({'competitor_id': competitor['id'], 'name': competitor['name'], 'error': str(e)}) diff --git a/backend/pb_migrations/001_initial_schema.js b/backend/pb_migrations/001_initial_schema.js index 1af2828..0042839 100644 --- a/backend/pb_migrations/001_initial_schema.js +++ b/backend/pb_migrations/001_initial_schema.js @@ -29,11 +29,12 @@ migrate((app) => { cascadeDelete: false, minSelect: 0, maxSelect: 1, ...opts, }); - const mk = (name, fields, listRule = null) => { + const mk = (name, fields, listRule = null, indexes = []) => { const collection = new Collection({ type: 'base', name, fields, + indexes, // Tenant-scoped data — API access enforced via the Flask backend's // X-API-Key + tenant_id filtering, not PocketBase's own auth rules. // Leaving rules null restricts direct API access to admin-only, @@ -45,7 +46,13 @@ migrate((app) => { updateRule: null, deleteRule: null, }); - return app.save(collection) && collection; + // app.save() throws on failure and returns undefined on success (it's + // not a boolean-returning call in PocketBase's JS VM) — returning + // `app.save(collection) && collection` always evaluated to undefined, + // breaking every rel() call further down that referenced an + // earlier-created collection. + app.save(collection); + return collection; }; // ── tenants ───────────────────────────────────────────────────────── @@ -82,7 +89,6 @@ migrate((app) => { url('website', { required: true }), url('catalogue_url', { required: true }), text('primary_market', { required: true }), - text('firecrawl_monitor_id'), boolean('change_detected'), date('change_detected_at'), boolean('active'), @@ -156,6 +162,12 @@ migrate((app) => { // block, but §29 Component 2 (job duration alerting) explicitly writes // scrape_repository.update(job_id, {'duration_seconds': duration}) — // added here so that hook has somewhere to write. + // + // content_hash/content_length/hash_algorithm (CLAUDE.md §4/§10) back + // the content-hash change-detection system that replaced Firecrawl + // Monitor — analysis_service.run_competitor_analysis() writes a + // dedicated per-competitor row here on every refresh scrape purely to + // persist the hash for the next comparison. mk('scrape_runs', [ rel('tenant_id', tenants, { required: true }), rel('competitor_id', competitors, { required: false }), @@ -166,8 +178,20 @@ migrate((app) => { json('results'), text('error_log'), num('duration_seconds'), + text('content_hash'), + num('content_length'), + text('hash_algorithm'), autodate('started_at', true), date('completed_at'), + ], null, [ + // CLAUDE.md §6 documents content_hash as "indexed" — the daily + // scrape cron calls get_last_hash_for_url(competitor_id) once per + // competitor per run (and would be once per hour if a second daily + // slot is ever added), always filtering by competitor_id and + // sorting by started_at desc. This composite index covers that + // exact access pattern rather than indexing content_hash in + // isolation, which wouldn't help the competitor-scoped lookup. + 'CREATE INDEX idx_scrape_runs_competitor_hash ON scrape_runs (competitor_id, content_hash, started_at)', ]); // ── alerts ────────────────────────────────────────────────────────── diff --git a/backend/repositories/alert_repository.py b/backend/repositories/alert_repository.py index bf2784f..f22cb99 100644 --- a/backend/repositories/alert_repository.py +++ b/backend/repositories/alert_repository.py @@ -19,6 +19,20 @@ class AlertRepository: COLLECTION, filter_str=f'tenant_id = "{tenant_id}" && read = false', sort='-created', per_page=200 ) + def list_undelivered_email(self, tenant_id): + """ + Alerts not yet included in a digest email — independent of the + dashboard `read` flag, since a PM reading something on the + dashboard first shouldn't make it disappear from that day's + digest (CLAUDE.md §6 alerts schema: delivered_email tracks + digest inclusion, read tracks dashboard dismissal — separate + concerns). + """ + return pb_client.list( + COLLECTION, filter_str=f'tenant_id = "{tenant_id}" && delivered_email = false', + sort='-created', per_page=200 + ) + def list_recent(self, tenant_id, limit=50): return pb_client.list(COLLECTION, filter_str=f'tenant_id = "{tenant_id}"', sort='-created', per_page=limit) @@ -47,6 +61,12 @@ class MockAlertRepository: def list_unread(self, tenant_id): return [r for r in self._records.values() if r.get('tenant_id') == tenant_id and not r.get('read')] + def list_undelivered_email(self, tenant_id): + return [ + r for r in self._records.values() + if r.get('tenant_id') == tenant_id and not r.get('delivered_email') + ] + def list_recent(self, tenant_id, limit=50): items = [r for r in self._records.values() if r.get('tenant_id') == tenant_id] items.sort(key=lambda r: r.get('created') or '', reverse=True) diff --git a/backend/repositories/client_trips_repository.py b/backend/repositories/client_trips_repository.py new file mode 100644 index 0000000..b668a1e --- /dev/null +++ b/backend/repositories/client_trips_repository.py @@ -0,0 +1,58 @@ +import uuid +from repositories.pocketbase_client import pb_client +from repositories.repo_config import USE_MOCK_REPOSITORIES + +COLLECTION = 'client_trips' + + +class ClientTripsRepository: + """ + Data access for the `client_trips` collection (Phase 6) — the PM's + own product catalogue, used as the "client" side of semantic + comparable-trip matching against competitor products. + """ + + def create(self, data): + return pb_client.create(COLLECTION, data) + + def update(self, trip_id, data): + return pb_client.update(COLLECTION, trip_id, data) + + def get_by_tenant_and_name(self, tenant_id, trip_name): + """Used to upsert on repeated /research submissions instead of duplicating rows.""" + safe_name = trip_name.replace('"', '\\"') + return pb_client.get_first(COLLECTION, f'tenant_id = "{tenant_id}" && trip_name = "{safe_name}"') + + def list_for_tenant(self, tenant_id): + return pb_client.list(COLLECTION, filter_str=f'tenant_id = "{tenant_id}"', per_page=200) + + +class MockClientTripsRepository: + """In-memory stand-in for ClientTripsRepository.""" + + def __init__(self): + self._records = {} + + def create(self, data): + record_id = data.get('id') or str(uuid.uuid4()) + record = {'id': record_id, **data} + self._records[record_id] = record + return record + + def update(self, trip_id, data): + if trip_id not in self._records: + return None + self._records[trip_id] = {**self._records[trip_id], **data} + return self._records[trip_id] + + def get_by_tenant_and_name(self, tenant_id, trip_name): + return next(( + r for r in self._records.values() + if r.get('tenant_id') == tenant_id and r.get('trip_name') == trip_name + ), None) + + def list_for_tenant(self, tenant_id): + return [r for r in self._records.values() if r.get('tenant_id') == tenant_id] + + +client_trips_repository = MockClientTripsRepository() if USE_MOCK_REPOSITORIES else ClientTripsRepository() diff --git a/backend/repositories/comparable_repository.py b/backend/repositories/comparable_repository.py index d37a918..dea8475 100644 --- a/backend/repositories/comparable_repository.py +++ b/backend/repositories/comparable_repository.py @@ -11,6 +11,23 @@ class ComparableRepository: def create(self, data): return pb_client.create(COLLECTION, data) + def update(self, match_id, data): + return pb_client.update(COLLECTION, match_id, data) + + def get_by_client_and_competitor_product(self, tenant_id, client_product, competitor_product): + """ + Used to upsert on repeat matching runs (the Sunday-night n8n + workflow) instead of duplicating a row for the same pair every + week — first_matched/last_matched only make sense if the same + pair is updated in place, not recreated. + """ + safe_client = client_product.replace('"', '\\"') + return pb_client.get_first( + COLLECTION, + f'tenant_id = "{tenant_id}" && client_product = "{safe_client}" ' + f'&& competitor_product = "{competitor_product}"' + ) + def list_for_tenant(self, tenant_id, include_dismissed=False): filter_str = f'tenant_id = "{tenant_id}"' if not include_dismissed: @@ -33,6 +50,19 @@ class MockComparableRepository: self._records[record_id] = record return record + def update(self, match_id, data): + if match_id not in self._records: + return None + self._records[match_id] = {**self._records[match_id], **data} + return self._records[match_id] + + def get_by_client_and_competitor_product(self, tenant_id, client_product, competitor_product): + return next(( + r for r in self._records.values() + if r.get('tenant_id') == tenant_id and r.get('client_product') == client_product + and r.get('competitor_product') == competitor_product + ), None) + def list_for_tenant(self, tenant_id, include_dismissed=False): items = [ r for r in self._records.values() diff --git a/backend/repositories/competitor_repository.py b/backend/repositories/competitor_repository.py index a3be375..ebb1e09 100644 --- a/backend/repositories/competitor_repository.py +++ b/backend/repositories/competitor_repository.py @@ -15,11 +15,6 @@ def _extract_domain(url): class CompetitorRepository: """Data access for the `competitors` collection.""" - def get_by_url(self, url): - """Used by the Firecrawl Monitor webhook handler to resolve a changed URL back to a competitor.""" - safe_url = url.replace('"', '\\"') - return pb_client.get_first(COLLECTION, f'website = "{safe_url}" || catalogue_url = "{safe_url}"') - def get_by_domain(self, tenant_id, domain): """Used by CSV bulk import to skip duplicate competitors within a tenant.""" candidates = pb_client.list(COLLECTION, filter_str=f'tenant_id = "{tenant_id}"', per_page=200) @@ -48,12 +43,6 @@ class MockCompetitorRepository: def __init__(self): self._records = {} - def get_by_url(self, url): - return next(( - r for r in self._records.values() - if r.get('website') == url or r.get('catalogue_url') == url - ), None) - def get_by_domain(self, tenant_id, domain): return next(( r for r in self._records.values() diff --git a/backend/repositories/price_history_repository.py b/backend/repositories/price_history_repository.py new file mode 100644 index 0000000..5ff1c06 --- /dev/null +++ b/backend/repositories/price_history_repository.py @@ -0,0 +1,38 @@ +import uuid +from repositories.pocketbase_client import pb_client +from repositories.repo_config import USE_MOCK_REPOSITORIES + +COLLECTION = 'price_history' + + +class PriceHistoryRepository: + """Data access for the `price_history` collection.""" + + def create(self, data): + return pb_client.create(COLLECTION, data) + + def list_for_product(self, product_id, limit=50): + return pb_client.list( + COLLECTION, filter_str=f'product_id = "{product_id}"', sort='-scraped_at', per_page=limit + ) + + +class MockPriceHistoryRepository: + """In-memory stand-in for PriceHistoryRepository.""" + + def __init__(self): + self._records = {} + + def create(self, data): + record_id = data.get('id') or str(uuid.uuid4()) + record = {'id': record_id, **data} + self._records[record_id] = record + return record + + def list_for_product(self, product_id, limit=50): + items = [r for r in self._records.values() if r.get('product_id') == product_id] + items.sort(key=lambda r: r.get('scraped_at') or '', reverse=True) + return items[:limit] + + +price_history_repository = MockPriceHistoryRepository() if USE_MOCK_REPOSITORIES else PriceHistoryRepository() diff --git a/backend/repositories/product_repository.py b/backend/repositories/product_repository.py index dea6770..cbfecd9 100644 --- a/backend/repositories/product_repository.py +++ b/backend/repositories/product_repository.py @@ -17,6 +17,22 @@ class ProductRepository: per_page=limit ) + def get_by_competitor_and_name(self, competitor_id, trip_name): + """ + Finds an existing active product for this competitor matching + trip_name exactly — the dedup key used to decide whether a + scrape found a genuinely new product or an update to one + already on record (CLAUDE.md §15/§19 is_new + price diffing). + Returns None if no match (i.e. this is a new product). + """ + if not trip_name: + return None + safe_name = trip_name.replace('"', '\\"') + return pb_client.get_first( + COLLECTION, + f'competitor_id = "{competitor_id}" && trip_name = "{safe_name}" && 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) @@ -39,6 +55,16 @@ class MockProductRepository: items.sort(key=lambda r: r.get('last_seen') or '', reverse=True) return items[:limit] + def get_by_competitor_and_name(self, competitor_id, trip_name): + if not trip_name: + return None + return next(( + r for r in self._records.values() + if r.get('competitor_id') == competitor_id + and r.get('trip_name') == trip_name + 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 9ee0916..55031d6 100644 --- a/backend/repositories/scrape_repository.py +++ b/backend/repositories/scrape_repository.py @@ -31,16 +31,36 @@ class ScrapeRepository: ) return items[0] if items else None + def get_last_hash_for_url(self, competitor_id): + """ + 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. + """ + items = pb_client.list( + COLLECTION, + filter_str=f'competitor_id = "{competitor_id}" && content_hash != ""', + sort='-started_at', per_page=1 + ) + return items[0]['content_hash'] if items else None + class MockScrapeRepository: """In-memory stand-in for ScrapeRepository.""" def __init__(self): self._records = {} + # Real PocketBase auto-populates started_at via autodate(true) — + # every "most recent first" sort in this class relies on that. + # A monotonic counter (rather than datetime.now()) guarantees + # correct ordering across records created within the same test, + # regardless of clock resolution. + self._sequence = 0 def create(self, data): record_id = data.get('id') or str(uuid.uuid4()) - record = {'id': record_id, **data} + self._sequence += 1 + record = {'id': record_id, 'started_at': f'{self._sequence:020d}', **data} self._records[record_id] = record return record @@ -66,5 +86,13 @@ 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): + items = [ + r for r in self._records.values() + if r.get('competitor_id') == competitor_id 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 + scrape_repository = MockScrapeRepository() if USE_MOCK_REPOSITORIES else ScrapeRepository() diff --git a/backend/requirements.txt b/backend/requirements.txt index 4751a58..e2473f8 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -1,5 +1,6 @@ # ── Phase 1 — core API / queue / data layer ────────────────── Flask==3.0.3 +Flask-Cors==4.0.1 Jinja2==3.1.4 gunicorn==22.0.0 python-dotenv==1.0.1 diff --git a/backend/services/alert_service.py b/backend/services/alert_service.py index ecfdad8..5798a19 100644 --- a/backend/services/alert_service.py +++ b/backend/services/alert_service.py @@ -3,6 +3,7 @@ import logging import requests from repositories.monitoring_repository import monitoring_repository from repositories.alert_repository import alert_repository +from repositories.tenant_repository import tenant_repository logger = logging.getLogger(__name__) @@ -71,3 +72,40 @@ def create_alert(tenant_id, competitor_id, alert_type, message, product_id=None, 'delivered_whatsapp': False, 'read': False, }) + + +def send_daily_digest(tenant_id): + """ + Sends the 6pm daily digest for one tenant — every alert not yet + included in a digest, in a single email, per CLAUDE.md §15. Called + once per active tenant by the n8n daily-digest workflow (system- + level iteration, not a PM-facing route — see api.py's + /internal/daily-digest/). + + Data flow: + tenant_id → tenant record (email/name) → + alert_repository.list_undelivered_email() → + no alerts: skip entirely, no empty email sent → + alerts found: email_service.send_daily_digest_email() → + success: every included alert's delivered_email flipped true + """ + tenant = tenant_repository.get_by_id(tenant_id) + if not tenant or not tenant.get('email'): + logger.error(f'No tenant/email on file for {tenant_id} — skipping daily digest') + return False + + alerts = alert_repository.list_undelivered_email(tenant_id) + if not alerts: + return False + + # Imported lazily — same reasoning as brief_service/email_service's + # own lazy `import resend`: keeps this module importable without + # every dependency configured in test environments. + from services import email_service + sent = email_service.send_daily_digest_email(tenant['email'], tenant.get('name'), alerts) + + if sent: + for alert in alerts: + alert_repository.mark_email_delivered(alert['id']) + + return sent diff --git a/backend/services/analysis_service.py b/backend/services/analysis_service.py index fb32679..703e812 100644 --- a/backend/services/analysis_service.py +++ b/backend/services/analysis_service.py @@ -2,9 +2,16 @@ import logging from datetime import datetime, timedelta, timezone from repositories.competitor_repository import competitor_repository from repositories.product_repository import product_repository +from repositories.price_history_repository import price_history_repository +from repositories.scrape_repository import scrape_repository +from services import alert_service logger = logging.getLogger(__name__) +# A price move smaller than this is normal noise, not alert-worthy — +# matches the >5% threshold in CLAUDE.md's n8n price-change-alert spec. +PRICE_CHANGE_ALERT_THRESHOLD_PERCENT = 5 + def needs_refresh(competitor): """ @@ -70,6 +77,32 @@ def analyse_from_cache(tenant_id, competitor_id, context): return extract_with_openrouter(prompt, content='') +def detect_change(competitor_id, 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. + + Data flow: + competitor_id + new_hash → scrape_repository.get_last_hash_for_url() → + no prior hash: True (first scrape — 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) + if last_hash is None: + return True + if last_hash != new_hash: + competitor_repository.update(competitor_id, { + 'change_detected': True, + 'change_detected_at': datetime.now(timezone.utc).isoformat(), + }) + return True + return False + + def run_competitor_analysis(tenant_id, competitor, context): """ Decides the fast vs refresh path for one competitor and executes it. @@ -80,10 +113,15 @@ def run_competitor_analysis(tenant_id, competitor, context): Data flow: competitor + context → needs_refresh() decision → FAST: analyse_from_cache() (seconds) → - REFRESH: core.scraper.scrape() → core.extractor.extract_with_openrouter() - → PocketBase products/price_history updated → - competitor.change_detected reset, last_scraped bumped → - analysis narrative generated from the fresh data → + REFRESH: core.scraper.scrape() → compute_content_hash() → + detect_change() against the last recorded hash → + unchanged: change_detected cleared (confirmed stable since last + look), skip LLM extraction, reuse cached narrative, bump + last_scraped only (no wasted extraction cost) → + changed: core.extractor.extract_with_openrouter() → + PocketBase products/price_history updated → + last_scraped bumped, change_detected left True (stays visible + until the next scrape confirms no further change) → result dict returned """ if not needs_refresh(competitor): @@ -92,7 +130,7 @@ def run_competitor_analysis(tenant_id, competitor, context): logger.info(f'{competitor["name"]} — refresh path (stale or changed)') # Phase 2 modules — imported lazily, same reasoning as above. - from core.scraper import scrape + from core.scraper import scrape, compute_content_hash from core.extractor import extract_with_openrouter from industries.adventure_travel.prompt import build_prompt @@ -100,6 +138,32 @@ def run_competitor_analysis(tenant_id, competitor, context): if not page_data.get('success'): raise ValueError(page_data.get('text', 'Scrape failed')) + content_hash = compute_content_hash(page_data['text']) + changed = detect_change(competitor['id'], 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. + 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, + 'results': {}, 'completed_at': datetime.now(timezone.utc).isoformat(), + }) + + if not changed: + # Confirmed no further change since the last scrape — this is + # the moment change_detected gets cleared. If it was never true + # to begin with, this is a harmless no-op. + logger.info(f'{competitor["name"]} — content unchanged, skipping LLM extraction') + competitor_repository.update(competitor['id'], { + 'change_detected': False, + 'last_scraped': datetime.now(timezone.utc).isoformat(), + }) + return {'path': 'refresh', 'result': analyse_from_cache(tenant_id, competitor['id'], context), 'unchanged': True} + prompt = build_prompt( competitor['name'], page_data, context.get('product_name'), context.get('destination'), @@ -107,24 +171,112 @@ def run_competitor_analysis(tenant_id, competitor, context): is_ngs=context.get('tab_type') == 'NGS' ) extracted = extract_with_openrouter(prompt, content=page_data['text']) + product = _save_scrape_result(tenant_id, competitor, extracted, page_data) - product = product_repository.create({ - 'tenant_id': tenant_id, - 'competitor_id': competitor['id'], - 'trip_name': extracted.get('tripName'), - 'url': page_data['url'], - 'duration_days': extracted.get('duration'), - 'majority_price_usd': extracted.get('majorityPrice'), - 'price_low_usd': extracted.get('priceLow'), - 'price_high_usd': extracted.get('priceHigh'), - 'comments': extracted.get('comments'), - 'relevancy': extracted.get('relevancy'), - 'is_active': True, - }) - + # change_detected is deliberately left as detect_change() set it + # (True) rather than reset here — detection and extraction now + # happen in the same scrape, so there's no separate "handled" event + # to reset it on. It stays visible (CompetitorsPage's "⚠ Changed" + # badge, Apps Script sidebar status, tomorrow's n8n cron filter) + # until the *next* scrape confirms no further change and clears it + # in the branch above — at worst one extra hash-only confirmation + # scrape per genuine change, no LLM cost since it lands on the + # unchanged branch. competitor_repository.update(competitor['id'], { - 'change_detected': False, 'last_scraped': datetime.now(timezone.utc).isoformat(), }) return {'path': 'refresh', 'result': extracted, 'product': product} + + +def _save_scrape_result(tenant_id, competitor, extracted, page_data): + """ + Persists one competitor's freshly-extracted trip data, deciding + between "this is a new product" and "this is an update to a + product we already track" by matching on (competitor_id, + trip_name) — the same dedup key CLAUDE.md's is_new detection and + price-history diffing rely on (§15/§19 Phase 5). + + Data flow: + extracted fields → product_repository.get_by_competitor_and_name() → + no match: create product with is_new=True → + alert_service.create_alert('new_product') → + match found: compare majority_price_usd → + changed: price_history record written, product updated, + alert_service.create_alert('price_change') if the move exceeds + PRICE_CHANGE_ALERT_THRESHOLD_PERCENT → + unchanged: product's last_seen/fields refreshed, no alert → + product record returned either way + """ + now = datetime.now(timezone.utc).isoformat() + trip_name = extracted.get('tripName') + new_price = extracted.get('majorityPrice') + + common_fields = { + 'trip_name': trip_name, + 'url': page_data['url'], + 'destination': extracted.get('destination'), + 'duration_days': extracted.get('duration'), + 'majority_price_usd': new_price, + 'price_low_usd': extracted.get('priceLow'), + 'price_high_usd': extracted.get('priceHigh'), + 'aud_price': extracted.get('audPrice'), + 'cad_price': extracted.get('cadPrice'), + 'eur_price': extracted.get('eurPrice'), + 'gbp_price': extracted.get('gbpPrice'), + 'date_used': extracted.get('dateUsed'), + 'comments': extracted.get('comments'), + 'relevancy': extracted.get('relevancy'), + 'last_seen': now, + 'is_active': True, + } + + existing = product_repository.get_by_competitor_and_name(competitor['id'], trip_name) + + if not existing: + product = product_repository.create({ + 'tenant_id': tenant_id, + 'competitor_id': competitor['id'], + 'is_new': True, + 'first_seen': now, + **common_fields, + }) + alert_service.create_alert( + tenant_id, competitor['id'], 'new_product', + f"{competitor['name']} added a new product: {trip_name or 'Unnamed trip'}", + product_id=product['id'], + ) + return product + + product = product_repository.update(existing['id'], {**common_fields, 'is_new': False}) + + old_price = existing.get('majority_price_usd') + if old_price is not None and new_price is not None and old_price != new_price: + change_amount = new_price - old_price + change_percent = round((change_amount / old_price) * 100, 1) if old_price else None + + price_history_repository.create({ + 'tenant_id': tenant_id, + 'product_id': product['id'], + 'majority_price_usd': new_price, + 'price_low_usd': extracted.get('priceLow'), + 'price_high_usd': extracted.get('priceHigh'), + 'aud_price': extracted.get('audPrice'), + 'cad_price': extracted.get('cadPrice'), + 'eur_price': extracted.get('eurPrice'), + 'gbp_price': extracted.get('gbpPrice'), + 'date_used': extracted.get('dateUsed'), + 'change_field': 'majorityPrice', + 'change_amount': change_amount, + 'change_percent': change_percent, + }) + + if change_percent is not None and abs(change_percent) > PRICE_CHANGE_ALERT_THRESHOLD_PERCENT: + direction = 'dropped' if change_percent < 0 else 'raised' + alert_service.create_alert( + tenant_id, competitor['id'], 'price_change', + f"{competitor['name']} {direction} {trip_name or 'a product'} {change_percent}%", + product_id=product['id'], change_amount=change_amount, change_percent=change_percent, + ) + + return product diff --git a/backend/services/email_service.py b/backend/services/email_service.py index 81ed777..9071e79 100644 --- a/backend/services/email_service.py +++ b/backend/services/email_service.py @@ -103,3 +103,16 @@ def send_new_product_email(to_email, alert): 'duration_days': alert.get('duration_days'), 'price_usd': alert.get('majority_price_usd'), }) + + +def send_daily_digest_email(to_email, tenant_name, alerts): + """ + Sends the 6pm daily digest — one email summarising every alert + from the day, rather than a separate email per alert (CLAUDE.md + §15 "n8n reads new alert records → sends Resend email notification + (daily digest)"). + """ + return _send('daily_digest.html', to_email, f'Bettersight daily digest — {len(alerts)} update{"" if len(alerts) == 1 else "s"}', { + 'tenant_name': tenant_name, + 'alerts': alerts, + }) diff --git a/backend/services/embedding_service.py b/backend/services/embedding_service.py index e073e69..a0eb79b 100644 --- a/backend/services/embedding_service.py +++ b/backend/services/embedding_service.py @@ -1,6 +1,7 @@ import os import logging import numpy as np +from repositories.client_trips_repository import client_trips_repository logger = logging.getLogger(__name__) @@ -9,6 +10,45 @@ EMBEDDING_MODEL = 'openai/text-embedding-3-small' SIMILARITY_THRESHOLD = 0.75 +def save_client_trip(tenant_id, destination, duration, travel_style, product_name=None): + """ + Upserts a client_trips record from a PM's trip-intent description + (CLAUDE.md §8's TripIntentForm — destination/duration/travel_style, + optionally a product name). Every /research submission calls this, + which is the only place client_trips ever gets populated — without + it, find_comparable_trips() has nothing on the client side to match + competitor products against. + + 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 → + trip_name resolved (product_name if given, else a + destination+style fallback) → + client_trips_repository.get_by_tenant_and_name() → + found: update in place → not found: create → + client_trips record returned + """ + trip_name = product_name or f'{destination} {travel_style}'.strip() + payload = { + 'tenant_id': tenant_id, + 'trip_name': trip_name, + 'destination': destination, + 'duration_days': duration, + } + existing = client_trips_repository.get_by_tenant_and_name(tenant_id, trip_name) + if existing: + # A stale embedding would silently never get regenerated — + # /internal/embed-products only embeds trips missing a vector — + # so clear it whenever the text embed_trip() hashes actually changed. + if existing.get('destination') != destination or existing.get('duration_days') != duration: + payload['embedding'] = None + return client_trips_repository.update(existing['id'], payload) + return client_trips_repository.create(payload) + + def embed_trip(trip): """ Generates a semantic embedding for a trip using OpenRouter's diff --git a/backend/templates/emails/daily_digest.html b/backend/templates/emails/daily_digest.html new file mode 100644 index 0000000..b83a56e --- /dev/null +++ b/backend/templates/emails/daily_digest.html @@ -0,0 +1,54 @@ + + + + +Bettersight daily digest + + + + + + + + + + + + + + + + + + + + + + +
+ Bettersight +
Daily digest — {{ tenant_name }}
+
+

+ {{ alerts|length }} update{{ '' if alerts|length == 1 else 's' }} since your last digest. +

+
+ {% for alert in alerts %} +
+
{{ alert.message }}
+ {% if alert.change_percent is not none %} +
+ {{ alert.change_percent }}% +
+ {% endif %} +
+ {% endfor %} +
+ + View all activity in your dashboard → + +
+ Bettersight · Better sight, better moves. +
+ + diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 3066e05..ec5a851 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -53,11 +53,14 @@ def _reset_mock_repositories(): from repositories.battlecard_repository import battlecard_repository from repositories.comparable_repository import comparable_repository from repositories.monitoring_repository import monitoring_repository + from repositories.price_history_repository import price_history_repository + from repositories.client_trips_repository import client_trips_repository for repo in [ tenant_repository, seat_repository, competitor_repository, product_repository, scrape_repository, proxy_repository, alert_repository, battlecard_repository, - comparable_repository, monitoring_repository, + comparable_repository, monitoring_repository, price_history_repository, + client_trips_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 3354178..85d0ba3 100644 --- a/backend/tests/repositories/test_real_repositories.py +++ b/backend/tests/repositories/test_real_repositories.py @@ -17,6 +17,8 @@ from repositories.battlecard_repository import BattlecardRepository from repositories.comparable_repository import ComparableRepository 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 class _FakePbClient: @@ -88,7 +90,6 @@ def test_competitor_repository_delegates_correctly(monkeypatch): monkeypatch.setattr('repositories.competitor_repository.pb_client', fake) repo = CompetitorRepository() - assert repo.get_by_url('https://intrepidtravel.com') == fake.get_first_return assert repo.get_by_domain('t1', 'intrepidtravel.com') == fake.list_return[0] assert repo.get_by_id('c1') == fake.get_one_return assert repo.list_for_tenant('t1') == fake.list_return @@ -106,6 +107,16 @@ def test_product_repository_delegates_correctly(monkeypatch): assert repo.list_for_tenant('t1') == fake.list_return 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 + + +def test_product_repository_get_by_competitor_and_name_none_without_trip_name(monkeypatch): + fake = _FakePbClient() + monkeypatch.setattr('repositories.product_repository.pb_client', fake) + repo = ProductRepository() + + assert repo.get_by_competitor_and_name('c1', None) is None + assert repo.get_by_competitor_and_name('c1', '') is None def test_scrape_repository_delegates_correctly(monkeypatch): @@ -130,6 +141,24 @@ def test_scrape_repository_get_latest_complete_returns_none_when_no_runs(monkeyp assert repo.get_latest_complete_for_tenant('t1') is None +def test_scrape_repository_get_last_hash_for_url_returns_hash(monkeypatch): + fake = _FakePbClient() + fake.list_return = [{'id': 'r1', 'content_hash': 'abc123'}] + monkeypatch.setattr('repositories.scrape_repository.pb_client', fake) + repo = ScrapeRepository() + + assert repo.get_last_hash_for_url('c1') == 'abc123' + + +def test_scrape_repository_get_last_hash_for_url_returns_none_when_never_hashed(monkeypatch): + fake = _FakePbClient() + fake.list_return = [] + monkeypatch.setattr('repositories.scrape_repository.pb_client', fake) + repo = ScrapeRepository() + + assert repo.get_last_hash_for_url('c1') is None + + def test_proxy_repository_delegates_correctly(monkeypatch): fake = _FakePbClient() monkeypatch.setattr('repositories.proxy_repository.pb_client', fake) @@ -183,10 +212,24 @@ def test_comparable_repository_delegates_correctly(monkeypatch): repo = ComparableRepository() assert repo.create({'similarity_score': 0.8}) == fake.create_return + assert repo.update('m1', {'similarity_score': 0.9}) == fake.update_return + assert repo.get_by_client_and_competitor_product('t1', 'Inca Trail', 'p1') == fake.get_first_return assert repo.list_for_tenant('t1') == fake.list_return assert repo.dismiss('m1') == fake.update_return +def test_client_trips_repository_delegates_correctly(monkeypatch): + fake = _FakePbClient() + fake.list_return = [{'id': 'ct1'}] + monkeypatch.setattr('repositories.client_trips_repository.pb_client', fake) + repo = ClientTripsRepository() + + assert repo.create({'trip_name': 'Inca Trail'}) == fake.create_return + assert repo.update('ct1', {'destination': 'Peru'}) == fake.update_return + assert repo.get_by_tenant_and_name('t1', 'Inca Trail') == fake.get_first_return + assert repo.list_for_tenant('t1') == fake.list_return + + def test_brief_repository_get_weekly_data_aggregates_collections(monkeypatch): fake = _FakePbClient() fake.get_one_return = {'email': 'pm@gadventures.com', 'name': 'G Adventures'} @@ -214,3 +257,14 @@ def test_monitoring_repository_delegates_correctly(monkeypatch): result = repo.log_event('queue_depth', 'warning', value=6, threshold=5, message='busy', alert_sent=True) assert result == fake.create_return assert fake.calls[0][1] == 'monitoring_events' + + +def test_price_history_repository_delegates_correctly(monkeypatch): + fake = _FakePbClient() + fake.list_return = [{'id': 'ph1'}] + monkeypatch.setattr('repositories.price_history_repository.pb_client', fake) + repo = PriceHistoryRepository() + + 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' diff --git a/backend/tests/services/test_alert_service.py b/backend/tests/services/test_alert_service.py index 0e77ef7..a165b51 100644 --- a/backend/tests/services/test_alert_service.py +++ b/backend/tests/services/test_alert_service.py @@ -1,6 +1,7 @@ -from services import alert_service +from services import alert_service, email_service from repositories.monitoring_repository import monitoring_repository from repositories.alert_repository import alert_repository +from repositories.tenant_repository import tenant_repository class _FakeResponse: @@ -53,3 +54,58 @@ def test_create_alert_defaults_dashboard_delivered_true(): assert alert['delivered_email'] is False assert alert['read'] is False assert alert_repository.list_unread('t1') == [alert] + + +def _make_tenant(**overrides): + data = { + 'name': 'G Adventures', 'email': 'pm@gadventures.com', 'domain': 'gadventures.com', + 'tier': 'analyse', 'status': 'active', 'max_seats': 5, + } + data.update(overrides) + return tenant_repository.create(data) + + +def test_send_daily_digest_skips_when_no_undelivered_alerts(monkeypatch): + tenant = _make_tenant() + sent_calls = [] + monkeypatch.setattr(email_service, 'send_daily_digest_email', lambda *a, **k: sent_calls.append(1) or True) + + result = alert_service.send_daily_digest(tenant['id']) + + assert result is False + assert sent_calls == [] + + +def test_send_daily_digest_sends_and_marks_delivered(monkeypatch): + tenant = _make_tenant() + alert_service.create_alert(tenant['id'], 'c1', 'price_change', 'Price dropped', change_percent=-12) + alert_service.create_alert(tenant['id'], 'c1', 'new_product', 'New trip added') + + captured = {} + monkeypatch.setattr( + email_service, 'send_daily_digest_email', + lambda email, name, alerts: captured.update(email=email, name=name, count=len(alerts)) or True + ) + + result = alert_service.send_daily_digest(tenant['id']) + + assert result is True + assert captured['email'] == 'pm@gadventures.com' + assert captured['count'] == 2 + for alert in alert_repository.list_recent(tenant['id']): + assert alert['delivered_email'] is True + + +def test_send_daily_digest_does_not_mark_delivered_on_send_failure(monkeypatch): + tenant = _make_tenant() + alert_service.create_alert(tenant['id'], 'c1', 'price_change', 'Price dropped', change_percent=-12) + monkeypatch.setattr(email_service, 'send_daily_digest_email', lambda *a, **k: False) + + result = alert_service.send_daily_digest(tenant['id']) + + assert result is False + assert alert_repository.list_undelivered_email(tenant['id']) != [] + + +def test_send_daily_digest_unknown_tenant_returns_false(): + assert alert_service.send_daily_digest('does-not-exist') is False diff --git a/backend/tests/services/test_analysis_service.py b/backend/tests/services/test_analysis_service.py index 72593af..b7c0637 100644 --- a/backend/tests/services/test_analysis_service.py +++ b/backend/tests/services/test_analysis_service.py @@ -2,36 +2,39 @@ analysis_service orchestrates the fast/refresh path decision (CLAUDE.md §7). It calls into core.scraper / core.extractor / industries.adventure_travel.prompt, all of which are now real Phase 2 -modules — _install_fake_module swaps out just the functions this test -needs via monkeypatch.setattr (properly reverted after each test), -rather than mutating the shared module objects directly. +modules — _install_fake_module imports the real module (Phase 2 +guarantees it exists on disk) and swaps out just the functions this +test needs via monkeypatch.setattr (properly reverted after each test). """ -import sys -import types +import importlib from datetime import datetime, timedelta, timezone import pytest from services import analysis_service from repositories.competitor_repository import competitor_repository from repositories.product_repository import product_repository from repositories.tenant_repository import tenant_repository +from repositories.alert_repository import alert_repository +from repositories.price_history_repository import price_history_repository +from repositories.scrape_repository import scrape_repository def _install_fake_module(monkeypatch, dotted_name, **attrs): """ - Ensures `dotted_name` resolves to a module (creating fake parent - packages only if truly absent), then uses monkeypatch.setattr for - every attribute — critical when the target is a REAL module (as - core.scraper/core.extractor/industries.adventure_travel.prompt all - are post-Phase-2): monkeypatch.setattr records the original value - and restores it on teardown, whereas a raw setattr would silently - and permanently corrupt the shared module for every later test. + Imports the real module at `dotted_name` (every module this helper + is ever called with — core.scraper, core.extractor, + industries.adventure_travel.prompt — is a real, importable Phase 2 + module) and patches the given attributes via monkeypatch.setattr, + which records the original value and restores it on teardown. + + A previous version of this helper created a *blank fake* module + when `dotted_name` wasn't already in sys.modules, rather than + importing the real one — harmless back when these were Phase 1 + stand-ins for modules that didn't exist yet, but broken now: it + left monkeypatch.setattr patching an empty module lacking the very + attribute being set, raising AttributeError. Since Phase 2 these + modules always exist, always import for real. """ - parts = dotted_name.split('.') - for i in range(1, len(parts) + 1): - partial = '.'.join(parts[:i]) - if partial not in sys.modules: - monkeypatch.setitem(sys.modules, partial, types.ModuleType(partial)) - target = sys.modules[dotted_name] + target = importlib.import_module(dotted_name) for key, value in attrs.items(): monkeypatch.setattr(target, key, value) return target @@ -136,9 +139,245 @@ def test_run_competitor_analysis_takes_refresh_path_and_persists_product(monkeyp assert result['product']['trip_name'] == 'Peru Classic' updated_competitor = competitor_repository.get_by_id(competitor['id']) - assert updated_competitor['change_detected'] is False + # change_detected is intentionally NOT reset here — detection and + # extraction happen in the same scrape now, so it stays visible + # (status badges, next cron run) until a later scrape confirms no + # further change. See test_run_competitor_analysis_refresh_path_ + # clears_change_detected_when_confirmed_stable below. + assert updated_competitor['change_detected'] is True assert updated_competitor['last_scraped'] is not None + # No prior product existed for this competitor+trip_name — this is + # a brand-new-product discovery: is_new flag set, alert created. + assert result['product']['is_new'] is True + assert result['product']['first_seen'] is not None + alerts = alert_repository.list_recent(tenant['id']) + assert len(alerts) == 1 + assert alerts[0]['alert_type'] == 'new_product' + assert alerts[0]['product_id'] == result['product']['id'] + + +def test_save_scrape_result_updates_existing_product_without_duplicating(monkeypatch): + tenant, competitor = _make_tenant_and_competitor() + existing = product_repository.create({ + 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], + 'trip_name': 'Peru Classic', 'majority_price_usd': 2190, 'is_new': True, + }) + + extracted = {'tripName': 'Peru Classic', 'majorityPrice': 2190, 'duration': 15} + product = analysis_service._save_scrape_result( + tenant['id'], competitor, extracted, {'url': competitor['website']} + ) + + assert product['id'] == existing['id'] # updated in place, not duplicated + assert product['is_new'] is False + assert price_history_repository.list_for_product(product['id']) == [] + assert alert_repository.list_recent(tenant['id']) == [] + + +def test_save_scrape_result_creates_price_history_and_alert_above_threshold(monkeypatch): + tenant, competitor = _make_tenant_and_competitor() + existing = product_repository.create({ + 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], + 'trip_name': 'Peru Classic', 'majority_price_usd': 2490, 'is_new': False, + }) + + extracted = {'tripName': 'Peru Classic', 'majorityPrice': 2190, 'duration': 15} # -12% + product = analysis_service._save_scrape_result( + tenant['id'], competitor, extracted, {'url': competitor['website']} + ) + + assert product['id'] == existing['id'] + assert product['majority_price_usd'] == 2190 + + history = price_history_repository.list_for_product(product['id']) + assert len(history) == 1 + assert history[0]['change_amount'] == -300 + assert history[0]['change_percent'] == -12.0 + + alerts = alert_repository.list_recent(tenant['id']) + assert len(alerts) == 1 + assert alerts[0]['alert_type'] == 'price_change' + assert 'dropped' in alerts[0]['message'] + assert alerts[0]['change_percent'] == -12.0 + + +def test_save_scrape_result_price_change_below_threshold_no_alert(monkeypatch): + tenant, competitor = _make_tenant_and_competitor() + product_repository.create({ + 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], + 'trip_name': 'Peru Classic', 'majority_price_usd': 2200, 'is_new': False, + }) + + extracted = {'tripName': 'Peru Classic', 'majorityPrice': 2190, 'duration': 15} # ~-0.45% + product = analysis_service._save_scrape_result( + tenant['id'], competitor, extracted, {'url': competitor['website']} + ) + + # Price history is still recorded (it genuinely changed)... + assert len(price_history_repository.list_for_product(product['id'])) == 1 + # ...but a sub-threshold move doesn't warrant an alert. + assert alert_repository.list_recent(tenant['id']) == [] + + +def test_save_scrape_result_unchanged_price_no_history_no_alert(monkeypatch): + tenant, competitor = _make_tenant_and_competitor() + product_repository.create({ + 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], + 'trip_name': 'Peru Classic', 'majority_price_usd': 2190, 'is_new': False, + }) + + extracted = {'tripName': 'Peru Classic', 'majorityPrice': 2190, 'duration': 15, 'comments': 'refreshed'} + product = analysis_service._save_scrape_result( + tenant['id'], competitor, extracted, {'url': competitor['website']} + ) + + assert product['comments'] == 'refreshed' # still updated with fresh fields + assert price_history_repository.list_for_product(product['id']) == [] + assert alert_repository.list_recent(tenant['id']) == [] + + +def test_save_scrape_result_price_raised_alert_wording(monkeypatch): + tenant, competitor = _make_tenant_and_competitor() + product_repository.create({ + 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], + 'trip_name': 'Machu Picchu Trek', 'majority_price_usd': 1850, 'is_new': False, + }) + + extracted = {'tripName': 'Machu Picchu Trek', 'majorityPrice': 2050, 'duration': 8} # +10.8% + analysis_service._save_scrape_result(tenant['id'], competitor, extracted, {'url': competitor['website']}) + + alerts = alert_repository.list_recent(tenant['id']) + assert len(alerts) == 1 + assert 'raised' in alerts[0]['message'] + assert alerts[0]['change_percent'] > 0 + + +def test_detect_change_true_on_first_scrape(): + _, competitor = _make_tenant_and_competitor() + assert analysis_service.detect_change(competitor['id'], '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 + + +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, + }) + + changed = analysis_service.detect_change(competitor['id'], 'new-hash') + + assert changed is True + updated = competitor_repository.get_by_id(competitor['id']) + assert updated['change_detected'] is True + assert updated['change_detected_at'] is not None + + +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, + }) + + assert analysis_service.detect_change(competitor['id'], 'same-hash') is False + + +def test_run_competitor_analysis_refresh_path_skips_extraction_when_hash_unchanged(monkeypatch): + tenant, competitor = _make_tenant_and_competitor(change_detected=True) + product_repository.create({ + 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], 'trip_name': 'Peru Classic', + }) + + from core.scraper import compute_content_hash + + fake_page_data = {'success': True, 'url': competitor['website'], 'text': 'unchanged content'} + _install_fake_module(monkeypatch, 'core.scraper', scrape=lambda url, country=None: fake_page_data) + 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, + }) + + # extract_with_openrouter backs both the full-page extraction call + # (refresh path, content=full scraped text) AND analyse_from_cache's + # much cheaper narrative-only call (content=''). Skipping "LLM + # extraction" means never invoking it with the full page content — + # so the assertion checks what content it was called with, not + # whether it was called at all. + calls_with_content = [] + _install_fake_module( + monkeypatch, 'core.extractor', + extract_with_openrouter=lambda prompt, content: calls_with_content.append(content) or {'narrative': 'cached'} + ) + _install_fake_module(monkeypatch, 'industries.adventure_travel.prompt', build_prompt=lambda *a, **k: '') + + result = analysis_service.run_competitor_analysis(tenant['id'], competitor, {}) + + assert result['path'] == 'refresh' + assert result['unchanged'] is True + assert calls_with_content == [''] # only the cheap narrative-only call happened, never full extraction + updated = competitor_repository.get_by_id(competitor['id']) + assert updated['last_scraped'] is not None + # Confirmed no further change since the flag was set — this scrape + # is what clears it (see the bug this fixes: a prior version reset + # change_detected in the *changed* branch instead, making it + # unobservable to anything outside a single scrape call). + assert updated['change_detected'] is False + + +def test_change_detected_stays_visible_until_next_scrape_confirms_stable(monkeypatch): + """ + Regression test for the bug where change_detected was set True by + detect_change() and then immediately reset False before the same + function call returned — making it unobservable to the + CompetitorsPage status badge, the Apps Script sidebar, or the n8n + daily cron's `change_detected = true` filter clause. Exercises two + consecutive scrapes: the first finds a diff (flag should stay True + after it returns), the second finds no further diff (flag should + only clear then). + """ + tenant, competitor = _make_tenant_and_competitor() + product_repository.create({ + 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], 'trip_name': 'Peru Classic', + }) + _install_fake_module(monkeypatch, 'industries.adventure_travel.prompt', build_prompt=lambda *a, **k: '') + _install_fake_module( + monkeypatch, 'core.extractor', + extract_with_openrouter=lambda prompt, content: ( + {'tripName': 'Peru Classic', 'majorityPrice': 2190} if content else {'narrative': 'cached'} + ) + ) + + # First scrape — no prior hash exists, so detect_change() treats it + # as an initial baseline. changed=True takes the extraction branch. + _install_fake_module( + monkeypatch, 'core.scraper', + scrape=lambda url, country=None: {'success': True, 'url': url, 'text': 'version A'} + ) + analysis_service.run_competitor_analysis(tenant['id'], competitor, {}) + competitor = competitor_repository.get_by_id(competitor['id']) + assert competitor['change_detected'] is False # first-ever scrape has nothing to diff against + + # Second scrape — content genuinely changed vs "version A". + competitor['last_scraped'] = None # force needs_refresh() to take the refresh path again + _install_fake_module( + monkeypatch, 'core.scraper', + scrape=lambda url, country=None: {'success': True, 'url': url, 'text': 'version B'} + ) + analysis_service.run_competitor_analysis(tenant['id'], competitor, {}) + competitor = competitor_repository.get_by_id(competitor['id']) + assert competitor['change_detected'] is True # visible — not reset by the same call that set it + + # Third scrape — identical to "version B", nothing further changed. + competitor['last_scraped'] = None + analysis_service.run_competitor_analysis(tenant['id'], competitor, {}) + competitor = competitor_repository.get_by_id(competitor['id']) + assert competitor['change_detected'] is False # now cleared, confirmed stable + def test_run_competitor_analysis_refresh_path_raises_on_scrape_failure(monkeypatch): tenant, competitor = _make_tenant_and_competitor(change_detected=True) diff --git a/backend/tests/services/test_battlecard_service.py b/backend/tests/services/test_battlecard_service.py index 6850a7b..d2cb4ae 100644 --- a/backend/tests/services/test_battlecard_service.py +++ b/backend/tests/services/test_battlecard_service.py @@ -1,5 +1,4 @@ -import sys -import types +import importlib from services import battlecard_service from repositories.tenant_repository import tenant_repository from repositories.competitor_repository import competitor_repository @@ -8,13 +7,12 @@ from repositories.battlecard_repository import battlecard_repository def _install_fake_extractor(monkeypatch, fn): - # core.extractor is now a real Phase 2 module — must use - # monkeypatch.setattr (not a raw setattr) so the swap reverts after - # the test instead of permanently corrupting the shared module. - for name in ('core', 'core.extractor'): - if name not in sys.modules: - monkeypatch.setitem(sys.modules, name, types.ModuleType(name)) - monkeypatch.setattr(sys.modules['core.extractor'], 'extract_with_openrouter', fn) + # core.extractor is a real Phase 2 module — import it for real (not + # a blank fake substitute) and monkeypatch.setattr the one function + # this test needs, so the swap reverts after the test instead of + # permanently corrupting the shared module. + target = importlib.import_module('core.extractor') + monkeypatch.setattr(target, 'extract_with_openrouter', fn) def _setup(): diff --git a/backend/tests/services/test_embedding_service.py b/backend/tests/services/test_embedding_service.py index 9c30f61..b2b7085 100644 --- a/backend/tests/services/test_embedding_service.py +++ b/backend/tests/services/test_embedding_service.py @@ -1,6 +1,7 @@ import sys import types from services import embedding_service +from repositories.client_trips_repository import client_trips_repository def test_cosine_similarity_identical_vectors_is_one(): @@ -74,3 +75,48 @@ def test_embed_trip_calls_openrouter_embeddings_api(monkeypatch): assert vector == [0.1, 0.2, 0.3] assert captured['model'] == embedding_service.EMBEDDING_MODEL assert 'Peru' in captured['input'] + + +def test_save_client_trip_creates_new_record_with_product_name(): + trip = embedding_service.save_client_trip('t1', 'Peru', 15, 'Classic', product_name='Inca Trail Classic') + + assert trip['tenant_id'] == 't1' + assert trip['trip_name'] == 'Inca Trail Classic' + assert trip['destination'] == 'Peru' + assert trip['duration_days'] == 15 + + +def test_save_client_trip_falls_back_to_destination_and_style_when_no_product_name(): + trip = embedding_service.save_client_trip('t1', 'Peru', 15, 'Classic') + assert trip['trip_name'] == 'Peru Classic' + + +def test_save_client_trip_upserts_existing_by_tenant_and_trip_name(): + first = embedding_service.save_client_trip('t1', 'Peru', 15, 'Classic', product_name='Inca Trail') + second = embedding_service.save_client_trip('t1', 'Peru', 16, 'Classic', product_name='Inca Trail') + + assert second['id'] == first['id'] # updated in place, not duplicated + assert second['duration_days'] == 16 + assert len(client_trips_repository.list_for_tenant('t1')) == 1 + + +def test_save_client_trip_clears_stale_embedding_when_destination_or_duration_changes(): + client_trips_repository.create({ + 'id': 'ct1', 'tenant_id': 't1', 'trip_name': 'Inca Trail', + 'destination': 'Peru', 'duration_days': 15, 'embedding': [0.1, 0.2, 0.3], + }) + + updated = embedding_service.save_client_trip('t1', 'Peru', 16, 'Classic', product_name='Inca Trail') + + assert updated['embedding'] is None # stale — duration changed, must be re-embedded + + +def test_save_client_trip_keeps_embedding_when_nothing_relevant_changed(): + client_trips_repository.create({ + 'id': 'ct1', 'tenant_id': 't1', 'trip_name': 'Inca Trail', + 'destination': 'Peru', 'duration_days': 15, 'embedding': [0.1, 0.2, 0.3], + }) + + 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 diff --git a/backend/tests/services/test_jobs.py b/backend/tests/services/test_jobs.py index 4af2c5c..629fcf5 100644 --- a/backend/tests/services/test_jobs.py +++ b/backend/tests/services/test_jobs.py @@ -45,6 +45,79 @@ def test_run_research_job_marks_complete_on_success(monkeypatch): assert gotify_calls == [] # fast completion, no slow/stuck alert +def test_run_research_job_regenerates_battlecard_on_refresh_path_only(monkeypatch): + tenant, competitor, run = _setup() + battlecard_calls = [] + monkeypatch.setattr( + 'services.battlecard_service.generate_battlecard', + lambda competitor_id, tenant_id: battlecard_calls.append(competitor_id) + ) + monkeypatch.setattr('services.alert_service.send_gotify', lambda **kw: None) + monkeypatch.setattr( + 'services.analysis_service.run_competitor_analysis', + lambda tenant_id, comp, ctx: {'path': 'fast', 'result': {}} + ) + + jobs.run_research_job(run['id'], {'tenant_id': tenant['id'], 'competitors': [{'id': competitor['id']}]}) + + assert battlecard_calls == [] # fast path — no new data, no regeneration + + +def test_run_research_job_regenerates_battlecard_on_refresh_success(monkeypatch): + tenant, competitor, run = _setup() + battlecard_calls = [] + monkeypatch.setattr( + 'services.battlecard_service.generate_battlecard', + lambda competitor_id, tenant_id: battlecard_calls.append(competitor_id) + ) + monkeypatch.setattr('services.alert_service.send_gotify', lambda **kw: None) + monkeypatch.setattr( + 'services.analysis_service.run_competitor_analysis', + lambda tenant_id, comp, ctx: {'path': 'refresh', 'result': {}, 'product': {'id': 'p1'}} + ) + + jobs.run_research_job(run['id'], {'tenant_id': tenant['id'], 'competitors': [{'id': competitor['id']}]}) + + assert battlecard_calls == [competitor['id']] + + +def test_run_research_job_skips_battlecard_when_refresh_unchanged(monkeypatch): + tenant, competitor, run = _setup() + battlecard_calls = [] + monkeypatch.setattr( + 'services.battlecard_service.generate_battlecard', + lambda competitor_id, tenant_id: battlecard_calls.append(competitor_id) + ) + monkeypatch.setattr('services.alert_service.send_gotify', lambda **kw: None) + monkeypatch.setattr( + 'services.analysis_service.run_competitor_analysis', + lambda tenant_id, comp, ctx: {'path': 'refresh', 'result': {}, 'unchanged': True} + ) + + jobs.run_research_job(run['id'], {'tenant_id': tenant['id'], 'competitors': [{'id': competitor['id']}]}) + + assert battlecard_calls == [] # content-hash found no diff — nothing new to regenerate from + + +def test_run_research_job_battlecard_failure_does_not_fail_job(monkeypatch): + tenant, competitor, run = _setup() + monkeypatch.setattr( + 'services.battlecard_service.generate_battlecard', + lambda competitor_id, tenant_id: (_ for _ in ()).throw(RuntimeError('extraction down')) + ) + monkeypatch.setattr('services.alert_service.send_gotify', lambda **kw: None) + monkeypatch.setattr( + 'services.analysis_service.run_competitor_analysis', + lambda tenant_id, comp, ctx: {'path': 'refresh', 'result': {}, 'product': {'id': 'p1'}} + ) + + jobs.run_research_job(run['id'], {'tenant_id': tenant['id'], 'competitors': [{'id': competitor['id']}]}) + + updated = scrape_repository.get_by_id(run['id']) + assert updated['status'] == 'complete' + assert len(updated['results']['success']) == 1 + + def test_run_research_job_isolates_one_competitor_failure(monkeypatch): tenant = tenant_repository.create({ 'name': 'G Adventures', 'domain': 'gadventures.com', 'tier': 'analyse', diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 68f6388..a06e69b 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -135,6 +135,42 @@ def test_research_enqueues_job_without_touching_real_redis(client, api_headers, assert captured['job_id'] == response.json['job_id'] +def test_research_persists_client_trip_from_trip_intent(client, api_headers, monkeypatch): + from repositories.client_trips_repository import client_trips_repository + import jobs + tenant = _make_tenant() + monkeypatch.setattr(jobs, 'enqueue_research_job', lambda job_id, data: job_id) + + client.post('/research', json={ + 'tenant_id': tenant['id'], 'destination': 'Peru', 'duration': 15, + 'travel_style': 'Classic', 'tab_type': 'Standard', 'competitors': [], + 'productName': 'Inca Trail Classic', + }, headers=api_headers) + + trips = client_trips_repository.list_for_tenant(tenant['id']) + assert len(trips) == 1 + assert trips[0]['trip_name'] == 'Inca Trail Classic' + assert trips[0]['destination'] == 'Peru' + + +def test_research_still_queues_job_when_client_trip_save_fails(client, api_headers, monkeypatch): + import jobs + tenant = _make_tenant() + monkeypatch.setattr(jobs, 'enqueue_research_job', lambda job_id, data: job_id) + monkeypatch.setattr( + 'services.embedding_service.save_client_trip', + lambda *a, **k: (_ for _ in ()).throw(RuntimeError('pocketbase down')) + ) + + response = client.post('/research', json={ + 'tenant_id': tenant['id'], 'destination': 'Peru', 'duration': 15, + 'travel_style': 'Classic', 'tab_type': 'Standard', 'competitors': [], + }, headers=api_headers) + + assert response.status_code == 200 # side-effect failure never blocks the primary operation + assert response.json['status'] == 'queued' + + def test_bulk_import_rejects_more_than_fifty_rows(client, api_headers): tenant = _make_tenant() rows = [ diff --git a/backend/tests/test_api_extended.py b/backend/tests/test_api_extended.py index 7e572c6..53f222b 100644 --- a/backend/tests/test_api_extended.py +++ b/backend/tests/test_api_extended.py @@ -3,7 +3,7 @@ Additional route coverage beyond tests/test_api.py's core happy-path set — account management, billing, internal (n8n) routes, and the Stripe webhook signature-verification path. """ -import sys +import importlib import types import pytest from repositories.tenant_repository import tenant_repository @@ -27,19 +27,17 @@ def _make_tenant(**overrides): def _install_fake_module(monkeypatch, dotted_name, **attrs): """ - Ensures `dotted_name` resolves to a module, then uses - monkeypatch.setattr for every attribute — required now that - core.extractor/industries.adventure_travel.prompt are real Phase 2 - modules: a raw setattr would permanently corrupt the shared module - for every later test since monkeypatch never recorded it. + Imports the real module at `dotted_name` (core.extractor, + industries.adventure_travel.prompt — always real, importable Phase + 2 modules) and patches attributes via monkeypatch.setattr, which + records the original value and restores it on teardown. Must + import the real module rather than substitute a blank fake one — + see the identical helper in tests/services/test_analysis_service.py + for why. """ - parts = dotted_name.split('.') - for i in range(1, len(parts) + 1): - partial = '.'.join(parts[:i]) - if partial not in sys.modules: - monkeypatch.setitem(sys.modules, partial, types.ModuleType(partial)) + target = importlib.import_module(dotted_name) for key, value in attrs.items(): - monkeypatch.setattr(sys.modules[dotted_name], key, value) + monkeypatch.setattr(target, key, value) # ── Account ──────────────────────────────────────────────────────── @@ -107,21 +105,18 @@ def test_update_unknown_competitor_404(client, api_headers): assert response.status_code == 404 -def test_delete_competitor_deactivates_and_deregisters(client, api_headers, monkeypatch): +def test_delete_competitor_deactivates(client, api_headers): tenant = _make_tenant() competitor = competitor_repository.create({ 'tenant_id': tenant['id'], 'name': 'Intrepid', 'website': 'https://intrepidtravel.com', 'catalogue_url': 'https://intrepidtravel.com/adventures', 'primary_market': 'GB', - 'active': True, 'firecrawl_monitor_id': 'mon_123', + 'active': True, }) - deregistered = [] - monkeypatch.setattr('api.requests.delete', lambda url, timeout: deregistered.append(url)) response = client.delete(f'/account/competitors/{competitor["id"]}', headers=api_headers) assert response.status_code == 200 assert competitor_repository.get_by_id(competitor['id'])['active'] is False - assert deregistered def test_delete_unknown_competitor_404(client, api_headers): @@ -265,6 +260,13 @@ def test_internal_weekly_brief(client, api_headers, monkeypatch): assert called == ['t1'] +def test_internal_daily_digest(client, api_headers, monkeypatch): + monkeypatch.setattr('api.alert_service.send_daily_digest', lambda tenant_id: tenant_id == 't1') + response = client.post('/internal/daily-digest/t1', headers=api_headers) + assert response.status_code == 200 + assert response.json['sent'] is True + + def test_internal_welcome_email_unknown_tenant_404(client, api_headers): response = client.post('/internal/welcome-email/does-not-exist', headers=api_headers) assert response.status_code == 404 @@ -361,50 +363,6 @@ def test_internal_alert_email_sends_new_product(client, api_headers, monkeypatch assert response.json['sent'] is True -def test_internal_monitor_webhook_flags_competitor(client, api_headers): - tenant = _make_tenant() - competitor = competitor_repository.create({ - 'tenant_id': tenant['id'], 'name': 'Intrepid', 'website': 'https://intrepidtravel.com', - 'catalogue_url': 'https://intrepidtravel.com/adventures', 'primary_market': 'GB', 'active': True, - }) - - response = client.post( - '/internal/monitor-webhook', json={'url': 'https://intrepidtravel.com'}, headers=api_headers - ) - - assert response.status_code == 200 - assert competitor_repository.get_by_id(competitor['id'])['change_detected'] is True - - -def test_internal_monitor_webhook_unknown_url_is_noop(client, api_headers): - response = client.post( - '/internal/monitor-webhook', json={'url': 'https://unknown.com'}, headers=api_headers - ) - assert response.status_code == 200 - - -def test_internal_monitor_register_and_deregister(client, api_headers, monkeypatch): - tenant = _make_tenant() - competitor = competitor_repository.create({ - 'tenant_id': tenant['id'], 'name': 'Intrepid', 'website': 'https://intrepidtravel.com', - 'catalogue_url': 'https://intrepidtravel.com/adventures', 'primary_market': 'GB', - 'active': True, 'firecrawl_monitor_id': 'mon_1', - }) - monkeypatch.setattr('api.requests.post', lambda *a, **k: types.SimpleNamespace(json=lambda: {'id': 'mon_2'})) - monkeypatch.setattr('api.requests.delete', lambda *a, **k: None) - - r1 = client.post(f'/internal/monitor/register/{competitor["id"]}', headers=api_headers) - assert r1.status_code == 200 - - r2 = client.post(f'/internal/monitor/deregister/{competitor["id"]}', headers=api_headers) - assert r2.status_code == 200 - - -def test_internal_monitor_register_unknown_competitor_404(client, api_headers): - response = client.post('/internal/monitor/register/nope', headers=api_headers) - assert response.status_code == 404 - - def test_internal_scrape_enqueues_job(client, api_headers, monkeypatch): import jobs tenant = _make_tenant() @@ -419,6 +377,7 @@ def test_internal_scrape_enqueues_job(client, api_headers, monkeypatch): def test_internal_embed_products(client, api_headers, monkeypatch): from repositories.product_repository import product_repository + from repositories.client_trips_repository import client_trips_repository tenant = _make_tenant() competitor = competitor_repository.create({ 'tenant_id': tenant['id'], 'name': 'Intrepid', 'website': 'https://intrepidtravel.com', @@ -427,15 +386,40 @@ def test_internal_embed_products(client, api_headers, monkeypatch): product_repository.create({ 'tenant_id': tenant['id'], 'competitor_id': competitor['id'], 'trip_name': 'Peru Classic', }) + client_trips_repository.create({'tenant_id': tenant['id'], 'trip_name': 'Inca Trail'}) monkeypatch.setattr('services.embedding_service.embed_trip', lambda trip: [0.1, 0.2]) response = client.post(f'/internal/embed-products/{tenant["id"]}', headers=api_headers) assert response.status_code == 200 - assert response.json['embedded'] == 1 + assert response.json['embedded'] == 2 + assert response.json['embedded_products'] == 1 + assert response.json['embedded_client_trips'] == 1 def test_internal_match_comparable(client, api_headers, monkeypatch): + from repositories.client_trips_repository import client_trips_repository + tenant = _make_tenant() + client_trips_repository.create({'tenant_id': tenant['id'], 'trip_name': 'Inca Trail', 'embedding': [1, 0]}) + monkeypatch.setattr( + 'services.embedding_service.find_comparable_trips', + lambda client_trips, competitor_trips, threshold=0.75: [ + {'client_product': 'x', 'competitor_product': 'p1', 'similarity_score': 0.9, + 'price_difference': 10, 'duration_difference': 0} + ] + ) + + response = client.post(f'/internal/match-comparable/{tenant["id"]}', headers=api_headers) + + assert response.status_code == 200 + assert response.json['matches_created'] == 1 + assert response.json['matches_updated'] == 0 + matches = comparable_repository.list_for_tenant(tenant['id']) + assert len(matches) == 1 + assert matches[0]['last_matched'] is not None + + +def test_internal_match_comparable_upserts_on_repeat_run(client, api_headers, monkeypatch): tenant = _make_tenant() monkeypatch.setattr( 'services.embedding_service.find_comparable_trips', @@ -445,15 +429,34 @@ def test_internal_match_comparable(client, api_headers, monkeypatch): ] ) - response = client.post( - f'/internal/match-comparable/{tenant["id"]}', json={'client_trips': []}, headers=api_headers - ) + client.post(f'/internal/match-comparable/{tenant["id"]}', headers=api_headers) + response = client.post(f'/internal/match-comparable/{tenant["id"]}', headers=api_headers) - assert response.status_code == 200 - assert response.json['matches_created'] == 1 + assert response.json['matches_created'] == 0 + assert response.json['matches_updated'] == 1 + # Same pair matched twice — updated in place, never duplicated. assert len(comparable_repository.list_for_tenant(tenant['id'])) == 1 +def test_internal_match_comparable_preserves_dismissed_flag_on_upsert(client, api_headers, monkeypatch): + tenant = _make_tenant() + monkeypatch.setattr( + 'services.embedding_service.find_comparable_trips', + lambda client_trips, competitor_trips, threshold=0.75: [ + {'client_product': 'x', 'competitor_product': 'p1', 'similarity_score': 0.9, + 'price_difference': 10, 'duration_difference': 0} + ] + ) + client.post(f'/internal/match-comparable/{tenant["id"]}', headers=api_headers) + match = comparable_repository.list_for_tenant(tenant['id'], include_dismissed=True)[0] + comparable_repository.dismiss(match['id']) + + client.post(f'/internal/match-comparable/{tenant["id"]}', headers=api_headers) + + still_dismissed = comparable_repository.list_for_tenant(tenant['id'], include_dismissed=True)[0] + assert still_dismissed['dismissed'] is True + + # ── Research history — latest (Apps Script sidebar pull) ──────────── def test_research_history_latest_requires_known_email(client, api_headers): diff --git a/frontend/src/assets/css/main.css b/frontend/src/assets/css/main.css index c1d0d0c..f15f950 100644 --- a/frontend/src/assets/css/main.css +++ b/frontend/src/assets/css/main.css @@ -136,7 +136,7 @@ body { .tbtn:disabled { opacity: .5; cursor: not-allowed; transform: none !important; } .tbell { width: 34px; height: 34px; border-radius: 9px; border: 1px solid var(--border-2); background: var(--surface); display: flex; align-items: center; justify-content: center; cursor: pointer; position: relative; } -.tbell-dot { position: absolute; top: 6px; right: 6px; width: 7px; height: 7px; background: var(--red); border-radius: 50%; border: 1.5px solid #fff; } +.tbell-dot { position: absolute; top: -5px; right: -5px; min-width: 16px; height: 16px; background: var(--red); color: #fff; font-size: 9px; font-weight: 700; border-radius: 8px; border: 1.5px solid #fff; display: flex; align-items: center; justify-content: center; padding: 0 4px; } /* ─── STAT ROW ────────────────────────────────────────────────── */ .stat-row { display: grid; grid-template-columns: repeat(4, 1fr); gap: 16px; margin-bottom: 22px; } diff --git a/frontend/src/components/layout/Topbar.vue b/frontend/src/components/layout/Topbar.vue index 034ea2a..5985b9c 100644 --- a/frontend/src/components/layout/Topbar.vue +++ b/frontend/src/components/layout/Topbar.vue @@ -27,7 +27,7 @@ const notificationsStore = useNotificationsStore()