Opening a thread reads its bounded snapshot inside a writer transaction, so a long snapshot blocks thread creation, message dispatch, and other database writes. A file-backed WAL reproduction fails on main when another connection holds the writer.
Route the entire bounded snapshot transaction through the existing SQLite reader helper. Anchor lookup, fork-parent projections, and snapshot sequence stay in one consistent transaction. Preserve the contributor’s implementation and regression test from this PR; the updated branch also includes current main.
Fixes #21840 GitCafe: https://git.cafe/pingdotgg/t3code/issues/21840 Triaged bug via the automated issue flow. Accepted bug. Scope is bounded snapshot reader routing; query indexing and other projection reads are separate work.
windowed snapshots return committed data while the writer is held fails with “snapshot must return while another connection holds the writer”; the cause is SQLite writer lock contention.vp test run src/orchestration-v2/ProjectionStore.test.ts src/orchestration-v2/ThreadSearch.test.ts passes 37 tests; shared vp test run src/nodeSqliteClient.test.ts passes 16 tests.writer on main to reader with the fix.Runtime excerpt (both enclosing bounded requests succeeded):
| 1 | before 8d3b061121: sql.transaction db.connection=writer durationMs=13.297433 exit=Success |
| 2 | after b16843cb8d: sql.transaction db.connection=reader durationMs=32.628450 exit=Success |
This server-only change has an empty visual fixture matrix. No render paths or wire contracts changed. The reporter’s 10 GB cold-cache latency was not reproduced; no latency improvement is claimed.
Model: GPT-6.1-Sol and Claude Opus 5.5. Harness: Codex and Claude Code.
Review completed
Verdict: Approved at b0493ba
Macroscope's review found this PR approvable — This is a small, self-contained SQLite concurrency bug fix that reuses the existing reader transaction mechanism and adds a regression test for writer contention. Its runtime impact is limited to routing existing windowed snapshots to the reader when available, with the established writer fallback preserved.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review. 🎉
abc3c733-8f87-4463-96be-d5734ccd51bfReviewing files that changed from the base of the PR and between b0493ba0411c0f8600be4dbb352dab49b639d812 and 135b85c12c9a782d7bc2cdf577b708e0c128c775.
apps/server/src/orchestration-v2/ProjectionStore.test.tsapps/server/src/orchestration-v2/ProjectionStore.tsIncluded review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
getThreadSnapshotWindow now uses Node SQLite’s read-only operator. A live SQLite test verifies that the snapshot succeeds during a writer transaction and uses the reader connection.
Changes: Snapshot reader connection
| Layer / File(s) | Summary |
|---|---|
Read snapshots through a reader connection apps/server/src/orchestration-v2/ProjectionStore.ts, apps/server/src/orchestration-v2/ProjectionStore.test.ts | getThreadSnapshotWindow applies NodeSqliteClient.readOnly. A live test holds a writer lock, reads the 76-row snapshot, checks the thread identity and title, and verifies the "reader" connection trace. |
Priority: ⬆️ High
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Suggested reviewers: t3dotgg
135b8The snapshot change is mergeable after normal checks; no actionable issue remains in the supplied evidence.
b0493The change preserves the snapshot transaction and response contract while avoiding writer contention when a reader is available. No introduced security weakness was identified. Slow snapshots can still delay other work sharing the reader.
Retained concerns No architecture-level concerns identified.
Security Blast Radius
Resilience and Maintainability Implications
| Check name | Status | Explanation |
|---|---|---|
| Title check | Passed | The title follows conventional commit form and names the main change: windowed thread snapshot reads now run on the SQLite reader connection instead of the writer. It matches the raw summary, which sh… |
| Linked Issues check | Passed | Directly linked issue #21840 requires snapshot reads to avoid holding the SQLite writer. The supplied PR evidence states that getThreadSnapshotWindow runs anchor lookup, projection reads, fork-paren… |
| Out of Scope Changes check | Passed | The whole-PR summary identifies one production change in ProjectionStore.ts and one focused regression test in ProjectionStore.test.ts. The production change implements the reader routing for #218… |
| Description check | Passed | The description includes all required sections: Problem, Change, Scope and approval, and Verification. It links the triaged issue and approval, explains the implementation, documents focused test resu… |
Explanation
The title follows conventional commit form and names the main change: windowed thread snapshot reads now run on the SQLite reader connection instead of the writer. It matches the raw summary, which shows the ProjectionStore.ts read path switched to NodeSqliteClient.readOnly, and the PR objectives, which describe the same change.
Explanation
Directly linked issue #21840 requires snapshot reads to avoid holding the SQLite writer. The supplied PR evidence states that getThreadSnapshotWindow runs anchor lookup, projection reads, fork-parent reads, and MAX(sequence) through NodeSqliteClient.readOnly. The reviewed test holds BEGIN IMMEDIATE on another connection and verifies committed snapshot data and db.connection: reader. Calls inside a write transaction and reader fallback remain supported. The separate index-only cold-open suggestion is assigned to #21193 and is not required by #21840 for this PR.
Explanation
The whole-PR summary identifies one production change in ProjectionStore.ts and one focused regression test in ProjectionStore.test.ts. The production change implements the reader routing for #21840. The test verifies WAL concurrency and reader selection for that behavior. No unrelated production behavior or unrelated test scope is shown.
Explanation
The description includes all required sections: Problem, Change, Scope and approval, and Verification. It links the triaged issue and approval, explains the implementation, documents focused test results and limitations, and identifies the models and harness used.
Comment @coderabbitai help to get the list of available commands.
Added 50 commits