Mark dedupe-cap trap pages post-hoc; default-enable --dedupe-cap - #264
Merged
Conversation
…p marking --dedupe-cap's crawl-time judgment is bounded by actual fetches, so a same-cluster trap can admit a burst of pages before the shape becomes sticky. Add a self-healing nullable FK column so those already-admitted pages can be marked after the fact once the shape is confirmed, instead of only stopping future enqueues. Fix scripts/migrate-to-0.13.mjs: the new FK made pre-0.13 archive migration fail (entity data population ran before the referenced adjunct table existed under foreign_keys=ON). Move adjunct-table creation earlier in the migration sequence, and update every isolated entity-table test helper that enables FK enforcement to provision adjunct tables alongside it.
Recompute content_items.dedupe_cap_event_id against dedupe_cap_events on every viewer-build/crawl-completion run, matching every internal page's URL shape regardless of when it was crawled relative to the cap firing. Skip entirely when dedupe_cap_events is empty — the deferred FK on the column guarantees no row can be marked in that case, not just a performance assumption. Expose the mark through isDedupeCapped (ListPagesOptions, ListViewerPagesOptions) and PageDetail.isDedupeCapped/dedupeCapShapeKey. An archive predating this feature (no dedupe_cap_event_id column) degrades to "nothing is marked" instead of throwing, since --dedupe-cap is opt-in and a missing column is semantically identical to zero marks.
Add a checklist filter (viewer_pages.is_dedupe_capped, schema v24->v25) to the Pages view and a shape-key display on page-detail, mirroring the isSkipped/skipReason presentation. No list column: matches the existing asymmetry where PageListItem omits fields PageDetail-only exposes.
… query pages An unbounded --dedupe-cap trap can make a crawl never finish (observed: 6,400+ self-generating /search/ URLs from one double-encoding pagination bug on a real site). Default the crawl/pipeline --dedupe-cap flag to 10 so a fresh crawl stops itself against this class of trap without requiring users to know the flag exists. False positives on legitimate large sections are structurally prevented by the underlying majority-vote tracker (each such page differs in title/og tags, so its counter never accumulates) — see DedupeCapTracker. Use --no-dedupe-cap (or --dedupeCap 0) to disable. yargs-parser's boolean-negation coercion turns --no-dedupe-cap into dedupeCap: 0, not undefined, so map-flags-to-crawl-config.ts converts 0 to null explicitly before it reaches DedupeCapTracker (which floors any positive threshold at 1 — passing 0 through unchanged would cap on the very first observation, the opposite of disabling). pipeline.ts hand-writes its own flag definitions rather than reusing crawl.ts's (pre-existing, tracked separately) — synced its default there too so pipeline doesn't silently stay opt-in while crawl becomes on by default. Add query pages --isDedupeCapped / MCP list_pages isDedupeCapped, already wired through in the query/mcp-server commits, to the CLI flag surface.
…hange Add content_items.dedupe_cap_event_id to the schema index alongside dedupe_cap_events, generalize the throw-vs-degrade decision rule for future self-healing FK columns, and record --dedupe-cap's new default-on CLI behavior (crawl.ts default: 10, pipeline.ts kept in sync, the migrate-to-0.13.mjs adjunct-table ordering fix) in the dedupe-cap Reading path section.
…ems.spec.ts Rebasing onto dev's newly-merged inventory-exclusion feature surfaced the same gap the earlier crawler commit fixed elsewhere: this test's isolated DB setup calls createEntityTables without createAdjunctTables, so its foreign_keys=ON inserts into content_items fail against the new dedupe_cap_event_id column's unresolvable REFERENCES target.
The viewer's Pages-view checklist filter requires a real table column to attach its button to (paged-table.tsx renders filter controls strictly per-column, no column-less filter exists in this framework), but the isDedupeCapped filter was added list-option-only with no matching PageListItem field, so the filter button never rendered. Add isDedupeCappedSelectColumn (degrades to a 0 literal on archives predating the column, mirroring templateKeySelectColumn) and wire it into listPages, listPagesByTag, listPagesByJsonLdType, and joinViewerPageIdsToListItems so every page-list path exposes the flag consistently.
…nders paged-table.tsx renders every checklist-filter button strictly attached to a matching TanStack column (header.column.id) — there is no column-less filter in this table implementation. The dedupe-cap trap filter had no corresponding column, so its button never appeared, silently failing the new e2e test on CI. Add the isDedupeCapped column (boolCell, same pattern as hasCSP/hasHSTS) now that PageListItem carries the field. Update ARCHITECTURE.md's post-hoc-marking entries to reflect PageListItem now exposing isDedupeCapped and to record why list filters need a column.
inventory-snapshot was already the slowest shard (~14.5 min on a 15-minute job timeout per recent scheduled runs); adding dedupe-cap.e2e.ts (a full crawl -> viewer-read-model-build -> query pipeline test) there pushed the job past the timeout three runs in a row on this PR, even though every individual test passed. Move it to append-pipeline, the lightest shard.
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
--dedupe-cap's crawl-time judgment is bounded by actual fetches (the shape's metaSig can only beknown after fetching a few matching pages), so a same-cluster trap can admit a burst of pages before
the shape becomes sticky. Add
content_items.dedupe_cap_event_id, a self-healing nullable FK todedupe_cap_events, and recompute it against every internal page's URL shape on everyviewer-build/crawl-completion run — regardless of when a page was crawled relative to the capfiring — so already-admitted burst pages get marked after the fact.
ListPagesOptions.isDedupeCapped/ListViewerPagesOptions.isDedupeCapped/
PageDetail.isDedupeCapped+dedupeCapShapeKey(query),viewer_pages.is_dedupe_capped(read modelschema v24→v25),
query pages --isDedupeCapped(CLI),list_pages/get_page_detail(MCP), and aPages-view checklist filter + page-detail display (viewer).
--dedupe-cap(was opt-in with no default) to10oncrawl/pipeline, since anunbounded trap can make a crawl never finish. False positives on legitimate large sections are
structurally prevented by the underlying majority-vote tracker (each such page differs in
title/og tags, so its counter never accumulates). Use
--no-dedupe-cap(or--dedupeCap 0) todisable —
map-flags-to-crawl-config.tsexplicitly converts the0yargs-parser produces for--no-dedupe-captonull, since the tracker floors any positive threshold at 1 and wouldotherwise cap on the very first observation instead of disabling.
scripts/migrate-to-0.13.mjs: the new FK broke pre-0.13 archive migration (entity-table datapopulation ran before the referenced adjunct table existed under
foreign_keys = ON). Moveadjunct-table creation immediately after entity-table creation, before any data write.
Test plan
yarn lint— 0 errors (pre-existing warnings only)NX_WORKSPACE_ROOT_PATH=<worktree> yarn build— all 13 projectsyarn test— 608 files / 4428 tests passingmigrate-content-items-dedupe-cap-event-id,backfill-dedupe-cap-event-id,has-dedupe-cap-event-id-column,dedupe-cap-shape-key-select-column,list-pages/get-page-detail/apply-viewer-pages-filtersisDedupeCappedcases,register-pages-routefast/live path, MCPlist_pages/get_page_detail,CLI flag-parsing (
crawl-flag-parsing.spec.ts: default 10,--no-dedupe-cap→ 0, explicitoverride), pipeline/crawl default sync, viewer e2e checklist filter
dedupe-cap.e2e.tsverifies the full crawl →buildViewerReadModel→listPages/getPageDetailpipeline against a real self-generating trap fixture