feat(query): switch phase 6-f readers to new entity tables (#195)#207
Merged
YusukeHirao merged 15 commits intoJul 14, 2026
Merged
Conversation
Update every `@nitpicker/query` reader (list*, find*, get-summary, get-violations, get-page-*, resource/link/image joins, viewer read-model compute-*/build-viewer-read-model, header-presence expression, isolated cluster / graph / URL-sort helpers) to source rows from the Phase 6-C entity tables (`content_items`, `page_meta`, `resource_items`, `anchor_edges`, `resource_ref_edges`, `image_items`) and the Phase 6-A ref tables (`url_refs`, `text_refs`, `content_type_refs`, `json_refs`, `blob_refs`, `header_flags`) instead of the pre-6 write model (`pages`, `anchors`, `images`, `resources`, `resources-referrers`, and the `pages.responseHeaders` LIKE scan). Add a Phase 6-populated guard (`assertPhase6Populated`) into `Database.#init` for both read/write opens so pre-6 archives surface a clear "run scripts/migrate-to-phase6.mjs" error instead of an empty viewer response, and extend the migrator's Phase 6-E verification with `checkReaderParity` — eight totals covering listPages / listLinks / listImages / listResources / checkHeaders / findDuplicates / findMismatches / getViolations compared between the still-present pre-6 tables and the new entity tables inside the migrator's `.bak`-protected transaction. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- get-page-detail: use `node:zlib`'s `zstdDecompressSync` for zstd meta_extras (Phase 6-B compresses with the built-in `zstdCompressSync` from `node:zlib`, not `@mongodb-js/zstd`) - exclude-skipped-pages: drop the `is_skipped` default so any missed legacy caller becomes a TypeScript error instead of a silent `no such column` at query time Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Replace `IncompatibleArchiveError` (which is scoped to the 0.10 format
cut and requires a `requiredVersion` argument) with a dedicated
`Phase6NotMigratedError` so CLI / viewer boundaries can print the
Phase 6-F migration hint without collision.
- Extract a `countRows` helper that null-safe-reads Knex's aggregate
row (its typing is `{ count } | undefined`, so the previous inline
destructure tripped strict-mode TS on CI).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…on cut Replace the phase-scoped `assertPhase6Populated` guard + dedicated `Phase6NotMigratedError` class with a single `info.version` bump — `REQUIRED_FORMAT_VERSION` goes from `0.10.0` to `0.13.0`, the migrator writes `0.13.0` on completion, `assertCompatibleVersion` alone gates every reader path. No per-phase asserts or per-phase error classes to maintain. Also decouple the archive-format version from the npm package version: `setConfig` now writes `REQUIRED_FORMAT_VERSION` (a schema-shape constant), not `pkg.version` (the code release version). The two change on different cadences — a patch release must not silently bump the format version. `IncompatibleArchiveError`'s message picks the right migration script from the archive's version (pre-0.10 needs both scripts, 0.10-through-0.12 needs just migrate-to-phase6). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Rename every "phase6"-scoped identifier / directory / script / comment to the target archive format version 0.13 (matching the existing migrate-to-0.10 naming convention). "phase6" was an epic-internal project-management codename that had leaked into script filenames, error messages, module directories, and JSDoc — nothing operators or downstream consumers should ever see. Directories: - populate-ref-tables/ (was phase6b/) - populate-entity-tables/ (was phase6d/) - verify-migration/ (was phase6e/) Files: - create-ref-tables.ts (was create-phase6a-ref-tables.ts) - create-entity-tables.ts (was create-phase6c-entity-tables.ts) - migrate-ref-tables.ts (was migrate-phase6a-ref-tables.ts) - migrate-entity-tables.ts (was migrate-phase6c-entity-tables.ts) - scripts/migrate-to-0.13.mjs (was migrate-to-phase6.mjs) Identifiers: - MigrationVerificationError / MigrationVerificationSummary - verifyMigration / populateRefTables / populateEntityTables - migrateRefTables / migrateEntityTables - createRefTables / createEntityTables - Phase6BContentTypeCategory → ContentTypeCategory - applyPhase6Migrations → applyMigrations - setupPhase6DDb → setupMigrationDb - completePhase6Prerequisites → completeMigrationPrerequisites JSDoc / comments: every "Phase 6", "Phase 6-A"–"Phase 6-H", and bare "phase6" reference is either dropped or rewritten to reference "0.13". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- suggestMigrationScript: use compareSemver instead of `<` string compare
so pre-0.10 archives (e.g. '0.9.0') are routed through the chained
migration hint. Lexicographic `'0.9.0' < '0.10.0' === false` would
otherwise misroute them into the single-step branch.
- check-reader-parity:
- getViolations parity check now compares the JOIN through
content_items+url_refs against the legacy pages JOIN (previously
both sides queried analysis_violations directly — tautology).
- zero-vs-zero pairs are skipped instead of counted as pass, and
`compared_checks` is exposed on the error context so a silent
all-skip run is auditable.
- IncompatibleArchiveError.suggestMigrationScript now covered by three
new spec cases: pre-0.10, 0.10-0.12, and unknown-version archives all
produce the correct chained/single-step hint.
- verify-migration.spec.ts: add "#9 fires when parity fails after every
other invariant passes" — a mutation that keeps row counts intact but
drifts the content-type predicate.
- migrate-to-0.13-script.spec.ts: after a successful run, assert every
info.version row equals '0.13.0' so a bump-that-fails-silently would
fail this spec instead of surfacing downstream.
- migrate-to-0.10-script.spec.ts: drop the chained Archive.open block —
the end-to-end path is exercised in migrate-to-0.13-script.spec.ts
already, and running jsdom-heavy 6-D twice per CI job proved nothing
new. Kept the raw SQLite inspection.
- check-reader-parity.spec.ts: new file. Pins the four contracts —
matched-totals pass, all-zero silent skip, mismatch throws with the
failing labels in the error context, and the compared_checks count.
- accept-newer-archive test bumped from '0.14.0' to '999.0.0' so the
next real format bump does not accidentally self-reference this test.
Reader specs (Phase 6-F interim, until Phase 6-G moves the crawler
write path onto the new tables):
- New __test-utils__/populate-migration-tables.ts helper drives the
same populateRefTables + populateEntityTables the migrator uses,
in-process against a spec-created archive.
- 53 reader spec files were batch-updated (via
scripts/update-reader-specs.mjs one-off) so their beforeAll blocks
populate the 0.13 tables after seeding the legacy tables. The
remaining four specs that set-and-read inside individual `it()`
blocks were updated by hand.
- populateEntityTables + PageDomPathResolver are now exported from
@nitpicker/crawler so the query test-utils can drive them.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…p labels
- Add a "0.13 format" note next to the existing "0.10 format" note in
CLAUDE.md describing what changed at the write-model cut, which
migrator to run, and why `REQUIRED_FORMAT_VERSION` is decoupled from
`pkg.version` (the format cut and npm release cadence are different
concerns).
- migrate-to-0.13.mjs: rewrite the file-header docstring to say what
the script actually does at 0.13 (previously read "0.10 migration"
after the phase6 → 0.13 rename left labels behind), drop the
epic-internal `[6-A]` … `[6-F]` log prefixes for plain descriptions.
- Purge remaining epic-internal step labels (`6-A`, `6-B`, `6-D-4`,
`Phase-6-D`, …) from JSDoc across `create-*/migrate-*/populate-*/
verify-migration/*` and reader helpers. Every user-visible surface
now speaks in terms of "0.13" and functional step names ("ref
populate", "entity populate", "verify invariants").
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The v1 spec-update pass only touched `@nitpicker/query/src/*.spec.ts` (top-level glob), missing `viewer-read-model/` subdir specs plus the whole `@nitpicker/viewer` and `@nitpicker/mcp-server` reader specs. CI caught it as ~30 spec files failing because their setPage-seeded legacy rows never got populated into the 0.13 entity tables and the readers under test returned empty results. - Move `populateMigrationTables` to `@nitpicker/crawler` (added to `crawler.ts` exports) so it is importable from every package regardless of which archive-facing spec needs it. - Batch-update every spec that calls `archive.setPage` / `archive.setResource` / `archive.setExternalPage` under `@nitpicker/query`, `@nitpicker/viewer`, and `@nitpicker/mcp-server` to import `populateMigrationTables` from `@nitpicker/crawler` and invoke it once after all legacy-side setup. - external-url-sort / url-sort-cache specs have no `beforeAll` block — hand-inject the call inside each `it()` after its setup so the raw archive-setup path is covered. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…t QA
Batch v2 injected `await populateMigrationTables(archive);` at
`beforeAll` scope in viewer-route specs where `archive` is actually a
local const inside `buildFixture()` — a ReferenceError at runtime, not a
correct populate call. Fixed by:
- Moving the populate call INSIDE `buildFixture` (11 viewer-route specs),
right before the read-model builder step. Both `withReadModel` branches
now populate — the legacy-fallback branch was silently skipping populate
in the intermediate v2 fix which would have left half the spec set
running against empty phase6 tables.
- Moving 3 close-then-populate call sites (`mcp-server.spec.ts`,
`archive-manager.spec.ts`, `archive-manager-cache.spec.ts`) so populate
runs BEFORE `archive.write() / archive.close()` — otherwise the DB
handle is destroyed before populate can query it.
- Adding populate to 4 per-`it()` blocks the batch script missed
(`url-sort-cache.spec.ts` x3, `external-url-sort.spec.ts`,
`url-sort-temp-table.spec.ts` x2).
- Repairing the `checkReaderParity` "compared_checks" spec: its seed
data never produced a real mismatch (all extra rows were zero-vs-zero
candidates), so the `throw new Error('Expected throw')` fallback path
fired instead of a MigrationVerificationError. Fixed by inserting an
unpaired `pages` row so listPages legacy=2 vs current=1 fires.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… ordering
Two remaining CI failures after the last spec-batch pass:
1. **25 reader spec files ran `populateMigrationTables(archive)` AFTER
`buildViewerReadModel(archive)`.** The read-model builder now reads
from `content_items` etc., so if populate runs after the build the
`viewer_pages` / `viewer_anchor_facts` / … tables get built from an
empty phase6 source and every downstream reader test fails with
`expected N to be >= K` empty-result assertions. Swap the pair so
populate always runs first.
2. **Real crawl + E2E fixtures had no populate step.** The 3 e2e-shard
crawls (`resource-reuse.e2e.ts`, `inventory.e2e.ts`, viewer E2E) and
the two viewer `generate-*-fixture.mjs` scripts finalise archives
without ever running populate, so every reader-level assertion
(`expect(paths).toContain('/resource-reuse/')`,
`orphan.pdf must be present in unused resources`, viewer route
snapshots) sees empty phase6 tables and fails. Fix by:
- Adding `populateMigrationTables(archive)` inside
`CrawlerOrchestrator.write()` so ANY crawl-driven finalisation
populates the entity tables before the `.nitpicker` file is
written. #196 will move the crawler write path onto the new
tables directly, at which point this call becomes an idempotent
`INSERT OR IGNORE` no-op.
- Adding the same call to `viewer/e2e/generate-fixture.mjs` (before
`archive.write() / archive.close()`) and
`viewer/e2e/generate-stub-fixture.mjs` (before
`archive.releaseHandle()` — stub fixtures cannot use write()).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous commit put `populateMigrationTables` inside `CrawlerOrchestrator.write()`, but the test-server e2e helper (`packages/test-server/src/__tests__/e2e/helpers.ts`) calls `CrawlerOrchestrator.crawling(...)` and then `Archive.connect(tmpDir)` directly — it never calls `orchestrator.write()`, so the bridge did not fire and every e2e reader assertion still saw empty phase6 tables. Move the bridge to run at every crawl-end site (5 total: `crawling`, `append`, `resume`, `retryFailed`, and the `inventory` mid-flow re-tar plus one no-render fast path), immediately after `archive.setUrlOrder()`. Also add the same populate call at the top of `ensureViewerReadModelQuietly` so the CLI's `nitpicker crawl` flow populates BEFORE it invokes `buildViewerReadModel` (which itself reads from the phase6 tables and would otherwise produce empty `viewer_pages` etc.). Remove the now-redundant populate from `CrawlerOrchestrator.write()`: every path that reaches `write()` has already populated during the crawl-end sequence. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`CrawlerOrchestrator.resume(stubPath, ...)` was the only remaining crawl-end site without a `populateMigrationTables` call — `--resume` of an interrupted crawl would leave the newly-scraped pages in `pages`/`anchors`/… but never mirrored into `content_items`/... so any reader opening the resumed archive would see stale (or empty) phase6 tables. Add the call right after `#finalizeCrawlSession`. All six crawl-end factories (`crawling` / `append` / `inventory` (2 new-data return paths) / `retryFailed` / `resume`) now populate before returning. The `inventory` noop-return path (all URLs already exist, line 753) deliberately skips populate: no new legacy rows were written, and the archive was 0.13-populated already or `Database.#init` would have rejected it via `assertCompatibleVersion`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…dlock, and migrator edge cases Fixes surfaced by strict QA review of the phase 6-F reader migration: - populate-entities now TRUNCATEs the six entity/edge tables (child-first) at the start of every run so `crawl --append` / `--retry-failed` / `--inventory` re-populates refresh mutable columns (source-priority upgrades, refreshed status, deleted-and-reinserted anchors/images). INSERT OR IGNORE alone would leave first-populate values stale. - populate-image-items reads HTML BLOBs via the same writer `trx` (new helper readPageHtmlInTrx). The old `getPageHtml` callback routed through Database.getHtmlOfPageById which uses the non-transactional Knex instance and deadlocks on libsql's single writer connection. decodeStoredBlob is extracted to `decode-html-blob.ts` so both Database.getHtmlOfPageById and the trx-scoped reader share one implementation. - migrate-to-0.13.mjs aborts before repacking when `info` has no rows — a bare UPDATE would silently produce an archive whose `info.version` is null and blow up in assertCompatibleVersion at next open. - crawler-orchestrator restores populateMigrationTables at every crawl-end site (crawling / append / two inventory branches / retryFailed / resume) so archives created via the SDK path (not just the CLI wrapper) reach every reader with populated entity tables. - Regression tests for all four behaviours + captureRejection helper shared across verify-migration specs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…eader specs with 0.13 API changes - find-duplicates.ts URL_DELIMITER lost the control-character byte in an earlier phase 6-F refactor. The delimiter passed to GROUP_CONCAT and the JS `.split()` argument both became the empty string, so every result row's `urls` array was split character-by-character. Restored the raw 0x1F byte + regression test. - Spec rewrites for the 0.13 reader-signature changes: content-type filter now joins content_type_refs (ctr) alias; exclude-skipped runs against content_items.is_skipped; header-presence expression takes a required flagsAlias; get-page-detail expects lower-cased header names (`decompose-header-set` lowercases per RFC). - Populate-migration injections across every reader spec that seeds legacy pages / anchors / images / resources through the writer API — needed until phase 6-G moves the crawler write path onto the new entity tables. Injection is placed after all setPage/setRedirect/ addPageError/setSkippedPage calls and before any Archive.close / Archive.connect / buildViewerReadModel / reader invocation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…13 entity-table readers Every reader spec that seeds the archive via `archive.setPage(...)` / `setRedirect` / etc. now calls `populateMigrationTables` after all writer calls and before the reader/reopen. The CLI `ensureViewerReadModelQuietly` mock exercises the same code path from a `vi.mock` boundary. E2E crawl helpers (`test-server/e2e/helpers.ts`, `retry-failed`, `append`, `inventory`, `output-path`, `archive-pipeline`, `analyze-pipeline`) run populate right after `orchestrator.crawling(...)` returns and before `orchestrator.write()` / `Archive.connect(...)`. Needed until phase 6-G lands the crawler write path onto the new entity tables directly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
Part of #103 epic. Closes #195 (Phase 6-F: update reader code to use new tables).
@nitpicker/queryreader (list*, find*, get-summary, get-violations, get-page-*, resource/link/image joins, allviewer-read-modelcompute-*/build-viewer-read-model, header-presence expression, isolated cluster / graph / URL-sort helpers) to source rows from the Phase 6-C entity tables (content_items,page_meta,resource_items,anchor_edges,resource_ref_edges,image_items) and Phase 6-A ref tables (url_refs,text_refs,content_type_refs,json_refs,blob_refs,header_flags) instead of the pre-6 write model (pages,anchors,images,resources,resources-referrers, and thepages.responseHeadersLIKE scan). No legacy branches left in code (v0.x clean cut).assertPhase6Populatedguard intoDatabase.#initfor both read/write opens so pre-6 archives surface a clear "run scripts/migrate-to-phase6.mjs" error instead of an empty viewer response.checkReaderParity— eight totals (listPages / listLinks / listImages / listResources / checkHeaders / findDuplicates / findMismatches / getViolations) compared between the still-present pre-6 tables and the new entity tables inside the migrator's.bak-protected transaction.Non-goals (scope out — future PRs)
Databaseinsert methods) — Phase 6-G / Phase 6-G: update crawler write path to target new tables #196pages/anchors/images/resources/resources-referrerstables — Phase 6-H / Phase 6-H: drop old tables and migrate FK declarations #197Test plan
yarn lintpasses (localyarn lint:cspellverified green)yarn buildpasses (sandbox has no tsc so local build was a no-op; CI is the authoritative check)yarn test— expect existing spec files that still referencepages/anchors/etc columns to fail; those will be addressed in follow-upsnode scripts/migrate-to-phase6.mjs <fixture>on a pre-6 archive, verify[6-E]reports success with the newcheckReaderParitystepnitpicker viewer, verify/api/pages,/api/links,/api/images,/api/headers,/api/summary,/api/duplicates,/api/mismatchesreturn non-empty results with the expected shapenitpicker viewer, verifyassertPhase6Populatedthrows with the migration-script hint🤖 Generated with Claude Code