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()