Repository navigation
fix(server): avoid loading v2 transcripts for checkpoint effects - #12339
juliusmarminge wants to merge 1 commit into
Conversation
Thread transfer impact
This comment will update automatically after the next completed run. |
6974410 to
f1547a7
Compare
This comment has been minimized.
This comment has been minimized.
f1547a7 to
ab03d21
Compare
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — The PR substantially replaces full transcript projection reads with new target-specific SQLite and in-memory selection logic across checkpoint capture, rollback, and finalization. Although the scope and tests are focused, these critical runtime and persistence-path changes merit human review. Notes:
You can add or adjust custom eligibility rules. Learn more. |
ab03d21 to
05ed3bf
Compare
|
Taking over this one for batch server-misc-06. Main already has the targeted capture/finalization reads; I will port the remaining rollback reads to the current architecture in a replacement origin stack, preserving stopped runs and repeated rollback accounting. The original stays open until the replacement is published and compared. No merge in this batch. |
|
Rebuilt as #21804, a focused origin stack layer for targeted rollback reads. Thanks to @juliusmarminge for identifying the broad transcript/projection reads and designing the targeted capture and rollback direction. I compared the complete original diff against current main and the replacement: targeted capture and finalization are already shipped on main; the remaining rollback read is in the replacement. The port preserves newer stopped-run support and repeated rollback accounting across every discarded attempt, which the older completed-only/target-attempt-only selection did not cover. No original behavior is left unaccounted for; the obsolete projection-only test file and old benchmark claims were intentionally omitted. Two service regressions replace three selection cases, with 90 focused tests passing and independent Opus/Sol reviews. Closing this older-base PR in favor of that replacement. Neither PR has been merged. |
V2 checkpoint capture and rollback loaded the full thread projection, including conversation history and fork ancestry. Long conversations unnecessarily allocated and decoded large transcript payloads, and obsolete unrelated history could prevent checkpointing.
Add transactional reads for the exact checkpoint targets and switch capture/rollback to them. Finalization also uses checkpoint metadata for its workspace refresh instead of reloading the conversation after capture. Capture reads only its run, root node, scope, provider thread, and baseline readiness columns. Rollback retains the target attempt/provider turns, completed later runs/root nodes, bound session, and ready later checkpoints needed for existing rollback behavior. The in-memory store follows the same target selection.
Depends on #12338 and targets v2 from #2829.
Verification: two capture cases fail on the old full-read path and pass with these reads, while preserving concurrent delegated completion and at-least-once idempotency. SQLite/memory parity tests cover target selection, stale checkpoint scope/status filtering, and cross-thread ownership. An actual SQLite regression ignores obsolete transcript, unrelated run, and checkpoint file payloads. The seven focused projection/checkpoint/provider-control/finalization suites pass (52 tests). Server typecheck and focused lint pass.
A synthetic SQLite conversation with 200 × 128 KiB assistant items (25 MiB) measured these median read times over ten warmed reads on Node 24:
This measures database projection work only, not Git capture/restore latency. No timing thresholds were added to tests. Backend-only change; no visual evidence applies.
Prepared with Codex.