Skip to content

perf(history): move integrity scans off the serving thread - #7972

Merged
lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:fix/history-background-integrity
Oct 30, 2026
Merged

lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:fix/history-background-integrity

Conversation

@luvs01

@luvs01 luvs01 commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Closes #7966.

Healthy history refreshes currently scan the complete SQLite index synchronously, even when a selective query needs one row. Move full integrity checks into the existing history worker while keeping schema/source/offset validation and automatic reconstruction from canonical usage.jsonl.

  • Validate an existing index on first use in a read-only worker. Async readers share that check; synchronous routing reads report busy/unknown evidence instead of blocking the serving loop.
  • After verification, trigger one background check on the first read at least five minutes later. Verified reads continue while it runs. Idle indexes have no timer. A successful fresh inline rebuild is usable immediately; a private worker rebuild is checked before publication.
  • Preserve automatic repair for a non-OK check and SQLite CORRUPT/NOTADB errors. History pages, row lookup and analytics retry a synchronous read once after repair; routing health marks the live generation for repair, and incremental-ingest corruption also rebuilds.
  • Keep BUSY, LOCKED, I/O, disk-full and worker-start failures separate from corruption. Check failures back off for 30 seconds without deleting a healthy index.
  • Bind verdicts to handle generations, make corruption sticky, share manual rebuilds, and drain the scanner through its actual worker-exit callback before file replacement, including the Windows settling path.
  • Add 20 native regression cases, update the architecture contract and English/Korean routing documentation.

Based on current upstream dev@3ea89653f5727aea41948ddbd5e833f74afe3e36. The reported indexer is identical in main@d844ff4af67c98271be1c1451e7583d36c323527. The partial-tail optimization in #7680 is preserved.

Strategy and tradeoff

An initialization-only check removes repeated work but leaves untouched damaged pages undetected in a long-running process. A timer executing synchronous SQLite on the serving thread still stalls that thread. This change instead uses first-use and demand-triggered periodic worker validation, plus immediate repair when a statement encounters corruption. Initial validation still takes time proportional to index size and may cost worker startup latency; periodic detection is not instantaneous. Existing source identity, truncation, worker-rebuild cooldown/fallback, pagination and canonical-ledger rules stay in force.

Verification

Native Linux x64 Bun 1.4.4 (38afee6c8), real Bun SQLite and real workers, frozen dependencies. The published tree is the exact locally validated tree 8ba2b1ac13b78cfbe33639bc85f57e95e3c66474.

139 passed / 0 failed / 2,708 assertions across these files, each executed in a separate native bun test process:

bun test tests/usage/request-history-integrity.test.ts
bun test tests/usage/request-history-index.test.ts
bun test tests/usage/request-history-rebuild-worker.test.ts
bun test tests/server/health-scoring.test.ts
bun test tests/routing/routing-analytics.test.ts
bun test tests/usage/usage-ledger-retention.test.ts
bun test tests/lib/worker-embed.test.ts
bun test tests/lib/worker-embed-coverage.test.ts
bun test tests/test-layout.test.ts
bun test tests/test-layout-tooling.test.ts
bun test tests/ci-workflows/file-size-ratchet.test.ts

The 20 new cases prove zero serving-thread full-check SQL during repeated page/id/status/routing reads; initial and periodic single-flight behavior; cooldowns; actual corrupted B-tree detection/rebuilding; error-driven history, analytics, health and tail-ingest repair; bounded retries; and close/manual-rebuild/source-replacement races. The damaged secondary-index fixture leaves schema metadata and ordinary timestamp queries readable, so it cannot pass merely through corrupt-header handling. Recovery verifies canonical ledger bytes are unchanged.

Also passed:

  • bun run typecheck — 2,245 files, zero type errors.
  • bun run structure:check.
  • bun run privacy:scan.
  • git diff --cached --check.
  • cd docs-site && bun run build with telemetry disabled and a writable scratch XDG_CONFIG_HOME — 625 pages, 90,515 internal links checked.

Synthetic performance check

Same synthetic 100,000-row ledger with three attempts per record and unique API-key selectors; 122,866,680 canonical bytes and a 234,721,280-byte index. Eight sequential selective limit=1 indexer queries, after reopening/validating, compared against the exact base commit:

Metric Base This change
Mean warm query 150.860 ms 0.237 ms
Warm-query range 146.174–167.571 ms 0.179–0.335 ms
Serving-thread full scans across eight warm reads 8 0
First lookup after reopening 154.877 ms 530.905 ms
1 ms heartbeat ticks during that first lookup 0 485

The first patched lookup awaits one worker check; its serving thread remains responsive. Neither implementation changes the canonical bytes. These are synthetic indexer-level measurements, not a reproduction of the reporter's 4.8 GB production database or HTTP latency.

Validation scope

The repository wrapper stalled in bun test --isolate --parallel=2 startup before any test cases on the unmodified base. The final bun run test:changed --parallel=2 likewise did not start cases and was terminated after a 45-second bound (exit 124). Under the documented focused-validation resource exception, the relevant suites above were executed directly and passed; the full local suite is not claimed. Native Windows 11 x64/Bun 1.4.4 follow-up now passed the three unchanged history suites (62 tests / 1,854 assertions), a real-worker component fixture (27 assertions), and three production-indexer integration cases in two focused runs (60 + 40 assertions). Those cases cover manual rebuild, close/reopen before drain completion, and ordinary-query source-identity replacement while a periodic scan is actually blocked in SQLite. The production filesystem calls occur after the old worker's actual exit barrier; expected replacement rows and metadata were checked. See the native validation report for instrumentation and execution limits. Native macOS, a real-time five-minute soak, installed-app/live-database behavior and maintainer acceptance remain separate.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • New Features
    • Request-history indexes are now checked for integrity on first use and periodically in the background.
    • Corrupted indexes are rebuilt automatically from the usage ledger, with affected history and analytics reads retried once.
    • Integrity checks preserve the original usage records and avoid blocking proxy serving.
  • Documentation
    • Added guidance on index validation, recovery, and how reads behave during checks and rebuilds.

Validate persisted indexes once per handle and every five minutes in a
read-only worker while preserving automatic corruption repair. Bound read
retries, keep operational failures nondestructive, and drain old scanners
before replacing files. Cover warm reads, real B-tree corruption, analytics,
routing evidence, ingestion and worker lifetime races with native Bun tests.

Refs lidge-jun#7966
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the enhancement New feature or request label Oct 30, 2026
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

The request-history index now uses worker-based integrity checks during opening and periodic use. The indexer coordinates checks with reads and rebuilds, reconstructs indexes after detected corruption, and retries affected reads once. Tests and documentation describe these behaviors.

Changes: Request-history integrity

Layer / File(s) Summary
Worker checks and integrity monitor
src/routing/history/integrity.ts, src/routing/history/rebuild-protocol.ts, src/routing/history/rebuild-job.ts, src/routing/history/rebuild-worker.ts, src/routing/history/sqlite-errors.ts
Adds worker-based quick_check requests and replies, monitor states for pending, ready, corrupt, and unavailable checks, cooldown handling, and SQLite corruption classification.
Index validation and replacement
src/routing/history/indexer.ts, src/routing/history/build-file.ts
Uses monitor results instead of synchronous full integrity checks. Coordinates scans with index replacement and validates built indexes before publication.
Read recovery and validation coverage
src/routing/history/indexer.ts, src/routing/analytics.ts, src/routing/health.ts, src/routing/history/ingest.ts, tests/usage/request-history-integrity.test.ts, tests/fixtures/test-layout-expected.json, scripts/test-layout/layout.json, docs-site/src/content/docs/reference/configuration/routing.md, docs-site/src/content/docs/ko/reference/configuration/routing.md, structure/dashboard-and-usage.md
Routes history and analytics reads through shared recovery logic. Reports corruption from health-history reads. Adds tests for lifecycle coordination, corruption recovery, and bounded retries, plus documentation of check and recovery behavior.

Priority: Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant HistoryReader
  participant RequestHistoryIndex
  participant IntegrityMonitor
  participant RebuildWorker
  participant UsageLedger
  HistoryReader->>RequestHistoryIndex: request history read
  RequestHistoryIndex->>IntegrityMonitor: inspect index state
  IntegrityMonitor->>RebuildWorker: check index in read-only worker
  RebuildWorker-->>IntegrityMonitor: return integrity verdict
  IntegrityMonitor-->>RequestHistoryIndex: provide check state
  RequestHistoryIndex->>UsageLedger: rebuild index after detected corruption
  UsageLedger-->>RequestHistoryIndex: provide ledger records
  RequestHistoryIndex-->>HistoryReader: return read result or retry once
Loading

Suggested labels: bug

Suggested reviewers: lidge-jun

Merge Risk: Minimal · up to 87db0

The change moves full integrity scans off the serving thread and keeps corruption recovery and bounded read retries. No actionable merge-blocking risk was found in the review.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 42.31% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "perf(history): move integrity scans off the serving thread" matches the main change. The raw summary shows full SQLite integrity checks moved to a read-only worker through RequestHistoryInt…
Linked Issues check Passed The PR meets the coding requirements in [#7966]. src/routing/history/integrity.ts moves initial and periodic quick_check work to a disposable worker, keeps verified reads available during later ch…
Out of Scope Changes check Passed The changes stay within [#7966]. The edits in src/routing/analytics.ts and src/routing/health.ts route existing history consumers through the required corruption-reporting and retry behavior. The …
Full details: Title check

Explanation

The title "perf(history): move integrity scans off the serving thread" matches the main change. The raw summary shows full SQLite integrity checks moved to a read-only worker through RequestHistoryIntegrityMonitor and rebuild-worker check requests, and the PR objectives state the same goal. The title is concise, specific, and follows a conventional commit style. It does not name the corruption recovery or the synchronous routing busy-state changes, but these are secondary to the main change.

Full details: Linked Issues check

Explanation

The PR meets the coding requirements in [#7966]. src/routing/history/integrity.ts moves initial and periodic quick_check work to a disposable worker, keeps verified reads available during later checks, and applies a failure cooldown. src/routing/history/indexer.ts still gates reads with schema, source identity, and offset handling, reports synchronous routing reads as busy during initial validation, and rebuilds corrupt indexes from the canonical ledger. src/routing/history/rebuild-worker.ts and src/routing/history/build-file.ts validate worker-built files before publication. tests/usage/request-history-integrity.test.ts covers repeated healthy reads without serving-thread full scans, corruption in an unqueried B-tree, recovery, retries, cooldowns, and lifecycle races. The reported focused tests and type checks provide supporting automated validation.

Full details: Out of Scope Changes check

Explanation

The changes stay within [#7966]. The edits in src/routing/analytics.ts and src/routing/health.ts route existing history consumers through the required corruption-reporting and retry behavior. The worker protocol, SQLite error classification, rebuild publication checks, lifecycle documentation, Korean and English routing documentation, and test-layout updates directly support the requested integrity strategy and regression coverage. No unrelated product behavior or canonical-ledger truncation/capping appears in the supplied change summary.

Full details: Docstring Coverage

Explanation

Docstring coverage is 42.31% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 26 functions across 11 files. (5 skipped: 5 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

luvs01 commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator Author

Coordination snapshot for 87db01f, checked on 2026-10-30. This PR stays independent. Latest upstream dev is still 3ea89653f5727aea41948ddbd5e833f74afe3e36, already its base; no branch update is needed for that snapshot.

Current CI and review

Cross-platform CI 40991885848 and Catalog async contracts 40991885850 succeeded. The CI test checkout was ed10086b8a8cfd3d4f9ca0ddc76e997db751f749, containing this exact head and the dev commit above.

The relevant cases actually ran on Ubuntu / Bun 1.4.4:

File Actual result
request-history-integrity.test.ts 20 pass / 0 fail / 0 skip
request-history-index.test.ts 24 pass / 0 fail / 0 skip
request-history-rebuild-worker.test.ts 18 pass / 0 fail / 0 skip

The Windows matrix job 123856074107 is skipped with zero steps. The successful runner-selector job ran on Ubuntu. Neither is native Windows validation. All-event Actions searches of the head and inspection of PR discussion found no separate native Windows result.

At inspection, there were no submitted PR reviews or review threads. The CodeRabbit report found no actionable merge-blocking issue; its docstring-coverage warning remains. Requested human review has not been substituted with CI success.

Native Windows verification remains pending

The existing Windows verification owner needs to run the three files above at this exact head with native Bun 1.4.4, preserving the test preload, isolated home, queue guards, and existing storage limits. This is a coordination request, not an acknowledgement or completed execution.

In addition, actual cancellation/file replacement needs native evidence:

  1. Start a real read-only SQLite integrity scan on a disposable index and establish that its connection is open.
  2. Trigger rebuild/source replacement while it is active; separately exercise close/reopen.
  3. Record actual worker termination and settling before index/WAL/SHM deletion, rename, or publication. Retain sharing-violation, EPERM/EBUSY, timeout, and first-run failure evidence.
  4. Verify replacement rows, rejection of stale verdicts, unchanged canonical ledger bytes, and released handles/temporary files.

The deterministic afterExit barrier test substitutes worker lifecycle functions. It verifies ordering, but even a Windows pass of that case alone would not prove real OS handle release. No native Windows trace of the full sequence is currently available.

No existing workflow input selects only these Windows tests. The current CI workflow offers all, release-gates, and macos-control; the Windows suite uses eleven shards. No per-PR lane=all dispatch or workflow/runner/permission change was made.

Independent #7971 combination

Pinned #7971 head: 719b4ffcdd7334bcf587fa7161d74ea5b04a3ebd, existing author NoRain211. A detached scratch application of its two-file diff produced tree a47c4e83d61abf726b12cfd0607cf7c61d7434f5; no merge commit or PR combination was created.

Native Linux results: five new integration cases passed (153 assertions), plus 43 unchanged slug-codec cases (119 assertions). Cold model-cache decoding, initial history validation, provider/model/accountRef separation, missing-index reconstruction, and empty-history exclusion behaved as expected. Pending health obeyed allow (0.5), penalize (0.3), and exclude (no eligible route, no fallback).

After a real worker verdict, cached unknown health remained until the existing 1,500 ms TTL expired. Recovery was verified after the real wait, without clearing the cache.

A separate exact-target diagnostic failed two cases both on #7971 alone and on the combination: policy/combo candidate other/a-b became emitted model a/b, while evidence/eligibility retained a-b. The reproduction is recorded on #7971; that fix stays with its author and PR. This is not a claim that all combined routing cases pass.

Shared registration audit

Pinned #7376 207c3a5e, #7931 52799f3f, this head, and latest dev were checked through temporary per-file three-way applications only.

  • Expanded registry/golden: dev 2,785, this PR 2,786, dev + feat: add opt-in plaintext V2 subagent mode #7376 2,791, dev + fix(codex): await recovery ACL checks in async parent admission #7931 2,786.
  • All six application orders of the three deltas produce 2,793 identical mappings and retain all 19 current explicit shards.
  • All 13 snapshots passed real loadLayout()/resolver checks, duplicate-preserving JSON parsing, independent golden comparison, registered-file existence/location checks, and textual conflict checks.
  • No new missing entries, duplicate keys, cross-shard duplicates, or registry/golden mismatches. The new integrity file appears exactly once as usage.
  • The same 23 pre-existing seed-resolved files lack explicit/golden entries, including the rebuild-worker test; they remain resolvable and unchanged. This is not a claim that every existing test has an explicit entry.

#7972 overlaps #7376/#7931 only in the registry and golden, not runtime source or test bodies. Their shared source paths remain with their existing owners. Fork #796/#798 remain separate with their existing task owners; no fixes were taken over.

No merge, force push, installed-app replacement, security/permission change, workflow dispatch, or production-source edit was performed for this coordination pass.

luvs01 commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Windows follow-up for 87db01f2bc2de98e0db122fd460d89a7e9662af6 (Windows 11 x64, Bun 1.4.4):

  • The unchanged integrity, index and rebuild-worker suites passed: 62 tests / 1,854 assertions, retaining the preload, isolated home and queue guard.
  • The earlier real-worker/monitor fixture passed 27 assertions, including SQLite close-before-response and cancellation while a connection was actually blocked.
  • Production-indexer integration now passed two cases / 60 assertions: manual rebuild during an active locked scan, and close followed immediately by a new query before the old scan had drained. The old reader rejected with RequestHistoryIndexClosedError; the new reader waited for the old barrier and a fresh check, then returned the expected rows.
  • A separate ordinary-query case passed 40 assertions (the other two cases were filtered out). A verified generation served a read while its due periodic scan was blocked in real SQLite. Replacing the fixture ledger changed its recorded identity; the next ordinary query triggered production destroyAndRecreate and worker rebuild without calling the manual rebuild API. The old worker closed, its actual afterExit barrier ran, and only then did production unlink/rename the database. The query returned exactly the three replacement rows and expected metadata.

The disposable harness uses synchronous pass-through SQLite/worker logging and Bun module instrumentation around the existing guarded filesystem functions. It observes actual production calls rather than substituting verdicts, file operations or afterExit. Production source and the 5-second disposal/1,500-ms Windows settling settings were unchanged. All accepted runs ended with native exit 0, both output streams at EOF, and no remaining owned child processes; actual cleanup callbacks for every recorded job were drained.

Earlier wrapper-startup, instrumentation and test-selection failures are retained separately and are not counted as product failures or passes. Periodic scheduling used an injected clock; this does not claim a real-time five-minute soak, native macOS, installed-app/live-database validation, or merge acceptance. No production configuration, account data or live database was changed.

@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer integration into dev under the MAINTAINERS.md exception, recorded by @lidge-jun's L5 maintenance lane. This is maintainer integration, not an independent approval.

  • Exact head: 87db01f2bc2de98e0db122fd460d89a7e9662af6
  • Exact-head CI (ci run 40991885848): ci, gates, test 1/4–test 4/4, api usage and storage policy succeeded. The one cancelled enforce-target run was superseded by successful reruns on the same head.
  • Independent review (fresh reviewer, two rounds). Round 1 raised one major issue: the initial worker check() has no time limit, so a worker that stays alive but never answers would leave the scan pending. Round 2 verified that this is not a regression and downgraded it to PASS-WITH-NITS. A crashed or exited worker already rejects through fail(), and dev's worker rebuild wait has the same unbounded contract. Before this PR, the same quick_check blocked the serving thread synchronously. Follow-up: Bound the request-history integrity check so a silent worker cannot stall history readers #7984.
  • Local check on the PR head: bun test tests/usage/request-history-integrity.test.ts passed 20/20 (one process; the host is shared).
  • Union hazards against current dev: git merge-tree is clean, none of the touched files has a file-size cap, the test-layout registration is consistent, and no new import reaches src/lab from the core request path.

Closes #7966; the issue will be closed manually after merge, because PRs into dev do not auto-close issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants