fix(consumer-prices): recover partial retailer extraction - #6185
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
… rewrite Commit 58d05cc replaced noon_sa's `urlPathContains: /saudi-en/` (and its "noon.com also serves UAE (/uae-en/) and Egypt" warning) with the route filters `/p/` + `/now-product/`. That correctly stopped search and category pages reaching extraction, but `matchesAnyPathFilter` is an OR over substrings, so the same field cannot also pin the storefront — and on www.noon.com the locale is the first path segment while `/p/` is the last, so no single substring spans both. The market scope was silently dropped. Effect: `https://www.noon.com/uae-en/<slug>/<sku>/p/` and `https://minutes.noon.com/uae-en/now-product/<id>` both passed admission for the `sa` market. Nothing downstream catches it — the currency check is skipped when the extractor omits `currency`, and validate.ts only quarantines outside 0.35x-2.5x of the median while AED:SAR is ~1:1.02, so a UAE price published as SAR is numerically invisible. noon_grocery_ae had the mirror exposure via the newly shared minutes.noon.com alias. Adds `urlPathMustContain`: segments that must ALL appear, AND-ed on top of `urlPathContains`, matched against `pathname` only so a locale echoed in a query string cannot satisfy it, failing closed on an unparseable URL. Applied at both admission sites — discovered URLs and stored pins, since pins outlive config changes and bypass discovery entirely. The pin rejection log now distinguishes "market scope" from "host mismatch". Keeps the route filters commit 58d05cc added; this is the missing conjunct, not a revert. Tests: 4 adapter cases (cross-storefront rejected on both hosts, same-storefront still admitted, query-string spoof rejected, stored foreign pin dropped), 6 unit cases for the predicate, and a YAML assertion pinning the shipped scope. Each was confirmed red before the fix, and five mutations — dropping either admission check, `every`->`some`, `pathname`->full URL, and removing the YAML scope — each turn at least one red. Package suite 167 passed (was 156), `tsc` clean. Claude-Session: https://claude.ai/code/session_015pEwgoK5a3jBf3EpcHUAMR
… run Second review pass on this PR, covering every remaining finding. Cooldowns were treating non-outages as outages, reproducing the COVERAGE_PARTIAL this PR exists to fix: - An Exa *extraction* cooldown aborted the whole target before discovery or Firecrawl ran. Exa is only the fallback there and Firecrawl's own streak resets on every success, so a fallback outage cost the rest of the basket while the primary extractor was healthy. Only the DISCOVERY cooldown aborts now; extraction cooldown is already honoured per-candidate. The test that asserted the old behaviour never checked whether Firecrawl was called, so it locked the bug in — rewritten to assert Firecrawl still runs. - FirecrawlProvider.extract threw when a *successful* response carried no `extract` object, feeding that page-level outcome into the outage streak. Two ordinary no-product pages could disable Firecrawl for a whole scrape, on all 20 search-adapter retailers without a fallback — none of them in this PR's scope. Now throws only on `success: false`. The PR's own recovery tests already mocked this shape as a non-throwing no-price result. - The extract client abort equalled the render budget sent in the same request, so a page using its full allowance was aborted before it could answer. The deadline is now a pure `extractAbortMs()` (budget + 5s, matching fetch()), unit-tested rather than asserted by waiting. Extraction correctness: - `looksLikeQuantityAsPrice` reached its canonical-name fallback only when sizeText was nullish, so `''` or an unparseable `'400 gm'` / `'24 pack'` (UNIT_MAP has neither) skipped the check entirely and admitted the quantity as the price — the exact Cold Storage bug this PR targets. - A missing `currency` passed the market gate silently. Under strict mode that is now a rejection, matching the sibling gates. - Rejections that mean the PAGE is wrong (currency mismatch, validator verdict, missing constraints) no longer escalate to the second provider — a different extractor reading the same page cannot fix them. Title mismatch and missing-price still escalate, since an extractor can misread a correct page. Accounting and observability: - `rejected_count` was structurally always 0 for non-pin search targets before this PR; only retailers that opted into the strict validator contribute it now, so ~20 untouched retailers keep their existing semantics. - The Exa discovery error detail was computed and then dropped, leaving logs unable to separate auth failure from rate limit from timeout. It now reaches the thrown message and the cooldown log. - `[search:*]` lines carry the retailer slug; 5 workers interleave 25 retailers with overlapping item names, so item name alone was not attributable. - Firecrawl requests send a User-Agent, per AGENTS.md:236. Run bounding: - The target loop had no wall-clock ceiling while this PR doubled worst-case per-candidate latency. An external kill skips updateScrapeRun entirely and strands the row at status='running', which the active-run query serves with no age bound. Adds a configurable budget (CONSUMER_PRICES_RUN_BUDGET_MS, default 20m) that stops cleanly and reports 'partial'. An invalid value disables the ceiling rather than truncating every run at target zero. Tests: 184 passed (was 156). New coverage for the opt-in fallback boundary, missing-size, currency-mismatch, missing-currency, the JioMart non-grocery path, cross-storefront pins, and the budget helpers. Eight mutations — the cooldown disjunction, the empty-extract throw, the abort headroom, both escalation breaks, the rejectedCount scope, the sizeText fallback, and the budget fallback/status — each turn at least one test red. `tsc` and biome clean. Claude-Session: https://claude.ai/code/session_015pEwgoK5a3jBf3EpcHUAMR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enabled consumer-price retailers could publish complete 12/12 category snapshots while losing the underlying retailer pages, leaving markets
COVERAGE_PARTIAL. This change restores product-page discovery and bounded extraction recovery for the diagnosed host/path, provider, and quantity-as-price failures without weakening the validators or last-known-good publication contract.Evidence
Railway production logs identified four recoverable classes behind the current gaps: Noon results on the retailer-owned
minutes.noon.comhost, JioMart results in a broad non-grocery/p/namespace, Carrefour UAE pins using the bare hostname, and Cold Storage extraction echoing weights such as400gas prices. The adapter now emits provider, page, HTTP/result, and final rejection reasons for those paths so the next scheduled run can distinguish recovery from legitimate validator rejection.What changed
sizeTextand rejects unknown size in strict mode when no expected size exists.Related: #6182
Verification
npm run buildinconsumer-prices-core— passed.npm test— 18 test files, 156 tests passed.Post-Deploy Monitoring & Validation
seed-consumer-pricesservice; search for[search:discovery],[search:extract],[search:provider-cooldown],provider-error,missing-price,quantity-as-price,validator-rejected, andRun .* finished./api/health?compact=1plus the per-market coverage and retailer diagnostics in seed metadata after each scheduled scrape → aggregate → publish cycle.COVERAGE_PARTIALand last-known-good snapshot intact; revert this PR if the failure persists.Known Residuals
scrapeRetailerrejection-counter integration test (consumer-prices-core/src/jobs/scrape.ts:298): theSearchTargetError.rejectedCounthandoff is covered by adapter and publication tests, but no DB-freescrapeRetailerharness directly exercises this catch branch. Follow-up: extract the loop/error accounting behind an injectable runner.consumer-prices-core/src/adapters/search.ts:89): exact host checking occurs before Exa/Firecrawl fetches, but final redirect destinations are controlled by those providers and are not observable through the current interfaces. Follow-up: add resolved-URL telemetry or a provider contract that returns the final URL.New concepts
Per-scrape provider cooldowns are a small circuit-breaker boundary around paid or rate-limited extraction services. After repeated transport failures, the adapter stops multiplying calls across the remaining basket targets while still allowing a configured recovery provider to run once per candidate.
Why here: a normal retry loop would turn one provider outage into a long, expensive scrape and obscure the source of missing pages. This PR keeps discovery and extraction streaks separate, so successful URL search cannot hide repeated extraction failures.
Use this boundary for repeated provider/transport failures; do not cooldown on a single page-level missing price or validator rejection, which may be a legitimate product-page result.