fix(server): PR sync and run-status checks stop reading whole thread histories#25726

open3

main

fix/issue-24895

View on GitHubTwo-way sync enabledPR last imported 2d ago

Problem

On large databases, two background reads hold the server's single SQLite reader far longer than they need to, so every other read, thread switches included, queues behind them (#24895: thread.visit median 12 s on a 22 GB database).

The PR-sync sweep runs every minute. It builds full thread shells, which means reading every active thread's runs, turn items, provider threads and messages, even though it uses only thread payload fields.
During a run start, the run-status checks (isCurrentAttemptInStatus, shouldFinalizeRun) each load the thread's whole runtime recovery projection to check a single run's status. That happens four to six times per run.

Reproduced on main (becf853def) with the red tests below:

The sweep reads runs, turn_items, provider_threads and messages.
A run start plus its two controls loads the recovery projection 6 times.

Change

ProjectionStore.getPullRequestSyncSnapshot reads only the thread payloads and the snapshot sequence, in one read transaction on the reader:
- It filters archived threads on the maintained archived_at column.
- It keeps the shell read's unsettledOnly condition and ordering.
- It shares the shell's MAX(sequence) statement, so the existing sequence guard on thread.pull-request.sync behaves the same.
ThreadPullRequestServiceV2 uses this read for both the sweep and the single-thread refresh. The single-thread refresh previously made two separate reads, for the sequence and then the shell.
ProjectionStore.getRun(threadId, runId) reads one run by primary key. isCurrentAttemptInStatus and shouldFinalizeRun now use it. A missing thread or a read error still fails the same way as before, instead of being treated as a superseded run.

There is no schema, migration or wire-contract change.

Scope and approval

Fixes pingdotgg/t3code#24895

Verification

Red tests, failing on main and passing here:
- ThreadPullRequestService.test.ts › "sweeps branch pull requests without reading thread history". It records the SQL the real ProjectionStore runs during a sweep. On main it fails with "tables the PR sync sweep read: expected [messages, provider_threads, runs, turn_items] to deeply equal []".
- ProviderTurnStartService.test.ts › "checks the run's status without loading the thread's history". On main it fails with "expected 6 to be 1". The single remaining load is the initial inherited-background-items read, which this change does not touch.
Added ProjectionStore.test.ts › the new sweep read returns the same sequence and fields as getShellSnapshot for active, unsettled-only, archived and deleted threads.
Results:
- ThreadPullRequestService, ProviderTurnStartService, ProjectionStore and ProviderTurnControlService tests pass.
- vp run check:changed passes on the merged head: format, lint, knip, typecheck of 3 packages, 6 test files.

Runtime transcript from the real ProjectionStore and migrations on a synthetic, file-backed database with the reader worker enabled. The data is 500 active branch threads, 2,000 archived threads with about 18 KB of pull-request JSON each, and one thread with 2,000 runs and 16,000 turn items. Times are ms per call over five calls:

Read	Before (becf853def)	After (a843632029)
PR-sync sweep	87–184 (`getShellSnapshot`)	18–25 (`getPullRequestSyncSnapshot`)
Single-thread PR refresh	two separate reads	0.6–1.3 (`getPullRequestSyncSnapshot`)
Run-status check	68–106 (`getRuntimeRecoveryProjection`)	0.6–1.1 (`getRun`)

The driver and transcripts are uploaded, not committed. All data in them is synthetic:

This is a server-only change with no visible surface, so there are no captures. It was not measured against a real multi-GB database.

Work by Claude Opus 5.5 (Claude Code in T3 Code), with review by Codex gpt-6.1-sol.

🤖 Generated with Claude Code

Checks

22 successful · 10 skipped

All checks passed

Added 3 commits4d ago

Merge remote-tracking branch 'origin/main' into fix/issue-248954777669
fix(server): PR sync sweep and run-status checks read only what they usea843632
test(server): reproduce #24895 background reads of whole thread historyfec62ad

MmacroscopeappBotapproved these changes4d ago

MmacroscopeappBot4d ago

Approvability

Verdict: Approved at 4777669

Macroscope's review found this PR approvable — This is a contained server performance fix that replaces whole-thread history reads with targeted projection/run reads while preserving existing synchronization and status semantics. It adds focused regression coverage and makes no schema, product-default, security, or deployment changes.

^{You can add or adjust custom eligibility rules. Learn more.}

Ggithub-actionsBot4d ago

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	+7 B (+0.1%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	+7 B (+0.6%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	20.9 KiB	20.9 KiB	0 B (0.0%)	29.3 KiB	✅
Codex	Live turn messages	2	2	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.2 KiB	21.2 KiB	0 B (0.0%)	29.3 KiB	✅
Claude	Live turn messages	2	2	0 (0.0%)	8	✅

Baseline: 71ba6b3 · PR result: 4777669 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

Codex decoded thread snapshot: 108.5 KiB
Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

CcoderabbitaiBot4d ago

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info

External image Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: fix/issue-24895
File: .coderabbit.config.ts
Commit: 4777669
Layers: Repository TS config
Review profile: CHILL
Plan: Team
Run ID: f27aefc1-c27a-41be-91db-196ad050a03c

External image Commits

Reviewing files that changed from the base of the PR and between 71ba6b374c0b0181130c0cc3b3d5aaa3387ada67 and 4777669c151e579f6e5c81a736b1105066e397ec.

External image Files selected for processing: 8

apps/server/src/orchestration-v2/Orchestrator.ts
apps/server/src/orchestration-v2/ProjectionStore.test.ts
apps/server/src/orchestration-v2/ProjectionStore.ts
apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts
apps/server/src/orchestration-v2/ProviderTurnStartService.test.ts
apps/server/src/orchestration-v2/ProviderTurnStartService.ts
apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts
apps/server/src/orchestration-v2/ThreadPullRequestService.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Walkthrough

The projection store now provides pull-request synchronization snapshots and targeted run lookup. The orchestrator exposes the snapshot API. Pull-request synchronization and run-status checks use these targeted reads instead of the previous shell-snapshot and recovery-projection reads.

Changes: Orchestration projection reads

Layer / File(s)	Summary
Projection snapshot and run lookup `apps/server/src/orchestration-v2/ProjectionStore.ts`, `apps/server/src/orchestration-v2/ProjectionStore.test.ts`	The SQL-backed store reads selected thread metadata and the snapshot sequence in one transaction. The in-memory store derives the snapshot from the shell snapshot. Both stores add targeted run lookup. Tests compare snapshot results across filter configurations.
Pull-request synchronization snapshot `apps/server/src/orchestration-v2/Orchestrator.ts`, `apps/server/src/orchestration-v2/ThreadPullRequestService.ts`, `apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts`	The orchestrator delegates snapshot requests to the projection store. Pull-request synchronization uses the snapshot API for full scans and specific-thread reads. Tests check filters, restoration behavior, and queried projection tables.
Targeted run-status checks `apps/server/src/orchestration-v2/ProviderTurnStartService.ts`, `apps/server/src/orchestration-v2/ProviderTurnStartService.test.ts`, `apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts`	Run-start and finalization checks use `getRun` to inspect the current run and attempt. Tests update projection-store mocks and verify recovery-projection reads.

Priority: External image Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

pingdotgg/t3code#24660 — This change replaces two history-wide read patterns that contribute to SQLite reader contention.
pingdotgg/t3code#21578 — This change replaces selected recovery-projection status reads with targeted run lookup.

Possibly related PRs

pingdotgg/t3code#25443 — It replaces full shell and history reads in pull-request discovery; this PR adds a related snapshot API for synchronization.

Suggested labels: bugfix, size:M

Suggested reviewers: maria-rcks, voltcrash, yordis

Merge Risk: External image Minimal · up to 47776

No identified issue blocks merging after normal checks.

Pre-merge checks: ^Passed 3 · ^Failed 1

^Failed Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Linked Issues check	Warning	Issue [`#24895`] requires several coding optimizations. This PR implements the PR-sync snapshot, `archived_at` filtering, and single-run reads for run-status checks. The issue also requires recovery-pro…	Implement and test the remaining [`#24895`] recovery-projection, canonical-projection, and usage-limit worker query optimizations, or provide separate linked changes that establish those requirements are complete.

^Passed Passed checks: 3 passed

Check name	Status	Explanation
Title check	Passed	The title "fix(server): PR sync and run-status checks stop reading whole thread histories" matches the main change. The PR-sync sweep and single-thread refresh now read thread payloads and the snapsho…
Description check	Passed	The description follows the template. It has Problem, Change, Scope and approval, and Verification sections. The Problem section explains the SQLite reader contention and the two background reads. The…
Out of Scope Changes check	Passed	The changed source files support the addressed [`#24895`] subset. The new projection-store and orchestrator APIs, PR-sync service integration, run-status integration, and related tests directly support …

Full details: Title check

Explanation

The title "fix(server): PR sync and run-status checks stop reading whole thread histories" matches the main change. The PR-sync sweep and single-thread refresh now read thread payloads and the snapshot sequence through getPullRequestSyncSnapshot. The run-status checks now read a single run through getRun. The title is conventional-commit formatted, specific, and concise.

Full details: Description check

Explanation

The description follows the template. It has Problem, Change, Scope and approval, and Verification sections. The Problem section explains the SQLite reader contention and the two background reads. The Change section lists the ProjectionStore and service changes and states there is no schema or wire-contract change. Scope links issue #24895. Verification lists red tests that fail on main and pass on the branch, the passing test suites, a check run, and synthetic benchmark timings. It also states the change was not measured against a real multi-GB database and that there are no visible UI captures. The Scope and approval section is brief. It names the fixed issue but does not include an explicit maintainer approval comment. The template asks for that when the change is not a small obvious fix.

Full details: Out of Scope Changes check

Explanation

The changed source files support the addressed [#24895] subset. The new projection-store and orchestrator APIs, PR-sync service integration, run-status integration, and related tests directly support the reduced reads. No unrelated product behavior, schema change, or wire-contract change is shown.

Full details: Linked Issues check

Explanation

Issue [#24895] requires several coding optimizations. This PR implements the PR-sync snapshot, archived_at filtering, and single-run reads for run-status checks. The issue also requires recovery-projection query/index changes, canonical-projection query changes, and the UsageLimitRecoveryWorker latest-run lookup optimization. The reviewed changes do not include those remaining optimizations or their tests.

Resolution

Implement and test the remaining [#24895] recovery-projection, canonical-projection, and usage-limit worker query optimizations, or provide separate linked changes that establish those requirements are complete.

Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

LLuneDevelops3d ago

Field report from a fork, since the description says this was not measured against a real multi-GB database.

Setup: nightly v0.1.4-nightly.20261027.3796 plus #24762 (four-worker reader pool), with the two code commits of this PR cherry-picked on top. Server over SSH on a Linux desktop against an 8.6 GB statev2.sqlite with about 30 agent processes attached. Running in production since 2026-10-28 16:49 UTC.

What we can say after the first 7 minutes:

No regressions in the server log. PR sync and run starts proceed normally, no new warnings.
Reader lock wait on the pool is p99 0.34 s, max 0.62 s, down from p99 11.5 s and max 42 s on the single reader. That number is mostly #24762's doing; this PR's contribution is that the per-minute sweep no longer occupies a worker for a full-shell read, which matters once the pool is near its four-worker ceiling.
Individual reader spans still reach p90 424 ms and max 1.85 s. Those are thread-history reads, not the sweep, so the remaining cost is in the direction #26240 is going.

Two landing notes for whoever merges:

The fix commit depends on the test commit; cherry-picking in PR order works, the reverse order conflicts.
The branch's third commit is a merge of main and should be skipped when cherry-picking.

Clean cherry-pick onto the nightly otherwise, no conflicts, and the three test files named in the description pass on the merged head.