fix(storage): a re-keyed raw replaces its stale verdict-cache row - #4693
Merged
Conversation
The cache writer deleted the cohort's rows by logical key and inserted by raw_id; a raw cached under a pending key and written again under its resolved key collided on the unique raw_id, and the converger's verdict-cache stage failed on every batch (84 times in rehearsal-10). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DNGJ3awfNrsLaMdHgQZvid
Sinity
enabled auto-merge (squash)
September 5, 2026 15:45
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Sinity
added a commit
that referenced
this pull request
Sep 5, 2026
…ut (#4694) ## Summary During a chunked catch-up every chunk paid archive-wide convergence costs, so per-chunk convergence grew with the archive. Whole-archive stages now run once per catch-up (on the last chunk), the archive-wide FTS readiness audit is its own deferred stage, hook-paste enrichment reads only the batch's sessions' sidecars, FTS partition inspection uses the `block_id` index instead of a `substr` scan, the raw-authority warmer finds its work in two statements (its `raw_id` collision is fixed by #4693, on which this branch is rebased), the periodic whole-archive FTS sweep waits while catch-up is active, and the convergence ledger now attributes stage `check` time. A separate defect found on the way: a catch-up with no source file modified in the last hour ingested nothing. ## Problem Rehearsal (fresh daemon, real sources, `/realm/tmp/work/rehearsal-4`): chunk 1 parse 1.1 s / convergence 5.6 s; chunk 150 parse 23.6 s / convergence 62.9 s for 1.9 MB; average over 150 chunks parse 10.6 s, convergence 20.2 s, both growing with archive size (305 sessions, 45,657 messages). Chunk 149 held the writer 103.9 s while its stage ledger summed to 2.5 s. Profile (in-process sampler on a scratch daemon over 400 real files; SQL timed against a copy of the rehearsal's 359-session index): - `enrich_paste_from_hooks` after every chunk scanned every sidecar journal in `hooks/` (371 files, 1.65 GB): 4.5 s warm, 72.9 s once the page cache was evicted. Untimed (outside the stage ledger); this is the size of chunk 149's unattributed time. - `fts_invariant_snapshot_sync` (five whole-table aggregates over `blocks`/`messages_fts_docsize`/identity) ran twice per chunk, from `FtsConvergenceOwner._publish_readiness_projection` and `_record_fts_freshness_after_insights`: 0.6 s per call at 90k blocks, 49 % of scratch convergence time (98 s of 200 s). - FTS partition `inspect`/`publish` selected a session's identity rows with `substr(block_id, 1, length(?)+1) = ? || ':'`: `SCAN messages_fts_identity` per key, 10.5 ms at 85k rows vs 0.1 ms for the index range. - `raw_authority_verdict_cache.check_many` probed every cohort with two statements (532 cohorts at chunk 150), and its warmup failed 54 times with `UNIQUE constraint failed: raw_authority_verdicts.raw_id`, re-running the whole-archive check on every following chunk. - `claude_workflow` and `delegation_work_evidence` re-read every raw artifact / every delegation row per chunk. - `maintenance.fts_convergence` (whole-archive `inspect_all`) held the writer 1061 s across 40 runs (max 132 s); chunks waited 1977 s on the writer in total. - Cold-start defect: `_catch_up` broke out of its (hot, cold) group loop on the first empty group, so a backlog with no file modified in the last hour was never planned. The after-run reproduced it (0 attempts, 0 cursors, `catch_up_complete` set after 40 s). ## Solution - `ConvergenceStage.whole_archive`; `DaemonConverger.converge_batch(whole_archive=False)` records such stages `SKIPPED` (converged, no debt: their staleness is re-derived from content). The watcher runs every catch-up chunk but the last in that scope. Stages flagged: `raw_authority_verdict_cache`, `claude_workflow`, `delegation_work_evidence`, and the new `fts_readiness`. - `make_fts_readiness_stage` publishes the exact archive-wide FTS audit once per pass, after `fts` and `derived`; partition-scoped FTS passes and insight rebuilds no longer publish it. `_periodic_convergence_check` skips the whole-archive FTS sweep while `watcher.catch_up_active`. - `enrich_paste_from_hooks(db, session_ids=...)` reads only `*-<native_id>.jsonl` for the batch's sessions (`session_ids_by_path` on append, `changed_session_ids` on full ingest). - FTS derivation: `[key || ':', key || ';')` range on the `block_id` UNIQUE index in `inspect` (excess, duplicates) and `publish`. - `find_raw_authority_verdict_cache_work`: one pass over `raw_sessions` and one over `raw_authority_verdicts` (master's #4693 writer, which removes a re-keyed raw's stale row, is kept as is). - Converger ledger: `<stage>.check` and `hook_paste_enrichment` timings. - Watcher: an empty priority group is skipped; only a stop request ends the loop. ## Verification - `.venv/bin/python -m devtools verify --quick` -> all 13 gates `ok` (format, lint, mypy, generated-surfaces, layering, patterns, doc-commands, schema-manifest, oracle-integrity, testmon-selection, consumer-reachability, timestamp-doctrine, schema-privacy). - `.venv/bin/python -m devtools test tests/unit/daemon/test_catch_up_chunk_cost.py tests/unit/daemon/test_daemon_convergence.py tests/unit/sources/test_hook_paste_enrichment.py tests/unit/storage/test_fts_derivation.py tests/unit/storage/test_raw_authority_verdict_cache.py tests/unit/daemon/test_convergence_stages.py tests/unit/daemon/test_fts_readiness_fallback.py tests/unit/daemon/test_fts_identity_convergence.py tests/unit/sources/test_live_batch_convergence.py tests/unit/pipeline/test_ingest_batch_fts_repair.py tests/unit/sources/test_live_catchup_planning.py tests/unit/sources/test_live_watcher.py tests/unit/sources/test_live_hot_session_convergence.py tests/unit/daemon/test_daemon_cli.py` -> pending (host pytest slot, pueue task 1646); this line is updated with the receipt - New tests: `test_chunk_convergence_cost_does_not_grow_with_archive_size` (real live-ingest route, archives of 2 and 14 sessions, one 2-file chunk: product SQL statements inside `_converge_paths` equal, 2 sidecars read, 0 snapshot calls, no deferred stage in the ledger; reverting any deferral adds two statements per cohort / one per artifact row), `test_final_catch_up_chunk_runs_the_whole_archive_stages`, converger chunk-scope and check-ledger, sidecar scoping, FTS `EXPLAIN QUERY PLAN` has no `SCAN` of the identity ledger, raw-authority rebinding and two-statement work finder, cold-backlog catch-up, last-chunk `whole_archive_convergence` flag. Scratch run (same host, same 400 files / 325 MB from `~/.claude/projects` and `~/.codex/sessions`, 100 catch-up chunks, `polylogued run` with an in-process sampler): | | before (dfb1322) | after | | --- | --- | --- | | convergence total | 200.4 s | 64.1 s | | convergence per chunk, by 20-chunk window | 1.61 / 1.89 / 2.14 / 2.60 / 1.78 s | 0.71 / 0.56 / 0.70 / 0.69 / 0.54 s | | `fts_invariant_snapshot_sync` inside the chunk path | 62 s | 0 s (once, on the last chunk) | | raw-authority warm inside the chunk path | 40 s | 0 s | | periodic whole-archive FTS sweeps during catch-up | 16 | 0 | | parse total | 406.0 s | 534.0 s (three chunks at 79 / 57 / 39 s in plain `messages`/`blocks` inserts; a full-corpus pytest ran on the host during the after run and was OOM-killed, pueue 1636 exit 137) | | end to end | 0.54 MB/s | 0.55 MB/s (0.77 MB/s without the three contended chunks) | Per-chunk convergence no longer grows with the archive. End-to-end throughput on this scratch is parse-bound (0.61–0.80 MB/s parse-only), so the 2x-of-parse criterion from polylogue-t4iy5.11.1 still needs the rehearsal driver on a quiet host for its operational proof. ## Residuals - The medium health tier (`_check_fts_readiness_medium`) recomputes the exact archive-wide FTS audit on every status payload: 51 s of the after-run's profile, all from `/api/status` and the status collector. The rehearsal driver probes status every 60 s; at 5M blocks each probe would take tens of seconds. It should read the freshness ledger the `fts_readiness` stage publishes. - Index write cost is per-session CPU (parser-source fingerprinting is 22 % of `index_parsed_write`; `messages`/`blocks` inserts with 18 secondary indexes) plus page-cache-miss stalls under host memory pressure; no whole-archive statement was found on the write path. - `claude_workflow` and `delegation_work_evidence` still rebuild from the whole archive whenever they run (once per catch-up now, then per live batch). - If a catch-up stops before its last chunk, the deferred stages run on the next live batch or catch-up rather than immediately. Beads: polylogue-t4iy5.11.1, polylogue-623q 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01DNGJ3awfNrsLaMdHgQZvid --------- Co-authored-by: Claude Fable 5.1 <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
write_raw_authority_verdict_cachedeleted a cohort's rows by logical key and inserted by raw_id. A raw cached under a pending key and written again under its resolved key collided on the uniqueraw_id; the converger'sraw_authority_verdict_cachestage then failed on every batch (84UNIQUE constraint failed: raw_authority_verdicts.raw_idin rehearsal-10, 2026-09-05, /realm/tmp/work/rehearsal-4/daemon.log). The writer now also removes the incoming raw ids' rows before inserting.Verification
devtools test tests/unit/storage/test_raw_authority_verdict_cache.py(newtest_rekeyed_raw_replaces_its_stale_cache_row; red without the raw_id delete). Quick gate via the pre-push hook.🤖 Generated with Claude Code
https://claude.ai/code/session_01DNGJ3awfNrsLaMdHgQZvid