test(chat-routes): prove the autonomous lane's final write is authoritative#255
Conversation
…tative Mutation testing over the incremental-persistence suite found one invariant no test could see: replacing runDetachedTurn's `draft.finalize(values)` with `draft.close()` — dropping the completion-time settlement entirely — left all 11 tests green. The existing autonomous-lane test hides it by coincidence. At `intervalMs: 0` over a turn whose tools all complete, the last draft happens to equal the final projection, so a lane that never settled authoritative values still produced a correct-looking row. This test removes both coincidences: the SHIPPED 2 s cadence (only the first draft escapes the floor, so the last draft is stale) and a tool that never completes (only `finalizeAssistantParts` terminalizes it; `draftParts` deliberately leaves it `running`). With the mutation applied it fails with `expected 'Looking' to be 'Looking, then gave up.'`; the other 7 mutations covering coalescing, deterministic row ids, the authoritative final write, empty-turn retraction, draft redaction, the fail-loud opt-in, and the retry discriminator were already killed by the suite.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — c3b6714b
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-07-25T02:43:57Z
✅ No Blockers —
|
tangletools
left a comment
There was a problem hiding this comment.
✅ Clean — c3b6714b
Full multi-shot audit completed 1/1 planned shots over 1 changed files. Global verifier still owns final merge decision.
Full immutable report for this review: trace
Summary comment for this run: full summary
tangletools · 2026-07-25T02:49:03Z · immutable trace
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 0 (none) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 184.5s (2 bridge agents) |
| Total | 184.5s |
💰 Value — sound
Adds one test that closes a verified mutation gap in runDetachedTurn's completion-time settlement — reproducible, targeted, no duplication.
- What it does: Adds a single regression test to tests/chat-routes/incremental-persistence.test.ts:474 asserting that runDetachedTurn's final write is authoritative over the last throttled draft — using the shipped 2s cadence (so only the first draft escapes the floor) and a tool part left permanently 'running' (only finalizeAssistantParts terminalizes it).
- Goals it achieves: Kill the one surviving mutation (M6) from the #250 mutation-testing pass: replacing draft.finalize(values) with draft.close() in detached-turn.ts left all 11 existing tests green because the existing autonomous-lane test hid the invariant behind two coincidences (intervalMs:0 + completed tools). The test makes the completion-time settlement observable.
- Assessment: High-quality, minimal, in-grain. It reuses the suite's existing real-SQLite harness (freshStore/rows/waitUntil/partUpdated) and sits in the same describe block. I reproduced the mutation: finalize->close fails ONLY the new test (expected 'Looking' to be 'Looking, then gave up.'), other 11 stay green — exactly the PR's claim. Reverted; 12/12 green; clean tree. The two coincidence-removers are load-
- Better / existing approach: none — this is the right approach. Searched for an existing equivalent: incremental-persistence.live.test.ts:76 covers the interactive lane (createChatTurnRoutes), is CI-skipped under LIVE_SANDBOX, and asserts the opposite (mid-stream snapshots must NOT terminalize) — so it neither covers the autonomous lane nor duplicates this invariant. No mutation-testing harness (stryker/mutate) exists in pack
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content
🎯 Usefulness — sound
A mutation-found test that closes the one gap (#250's M6) where dropping the autonomous lane's authoritative final write left all prior tests green; verified to kill the mutation.
- Integration: The test exercises the real, shipped, exported
runDetachedTurn(src/chat-routes/detached-turn.ts:261) through its realpersistseam against the real SQLite-backedcreateChatStore— the same harness and fixtures (freshStore/rows/waitUntil/partUpdated) as the other 11 tests in this file. It runs in the existingpnpm testsuite (confirmed: 12 passed). No new surface, no new wiring — i - Fit with existing patterns: Follows the exact pattern of the neighboring crash-convergence test (tests/chat-routes/incremental-persistence.test.ts:419): same
runDetachedTurnshape, samepersistseam, same assertion style, samewaitUntilmid-run probe. It sits naturally in thecrash-safe convergencedescribe block. The distinction it draws (draftAssistantPartsleaves a toolrunningvsfinalizeAssistantPartsterm - Real-world viability: The scenario is realistic and non-coincidental: default 2 s cadence (the shipped default, so later drafts are throttled to staleness) plus a tool that never completes (the dangling-tool case
finalizeAssistantPartsexists for, src/stream/stream-normalizer.ts:495-501). The mid-runwaitUntilprobe proves the row is created by a DRAFT — so a lane that skipped finalize would leave a stale row, not - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
No concerns — sound change, no better or existing approach found. ✅
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
What
Mutation testing over the incremental-persistence suite (#250) found one invariant no test could see: replacing
runDetachedTurn'sdraft.finalize(values)withdraft.close()— dropping the completion-time settlement entirely — left all 11 tests green.The existing autonomous-lane test hides it by coincidence. At
intervalMs: 0over a turn whose tools all complete, the last draft happens to equal the final projection, so a lane that never settled authoritative values still produced a correct-looking row.This test removes both coincidences:
finalizeAssistantPartsterminalizes it;draftPartsdeliberately leaves itrunning).Proof the check can fail
With
await draft.finalize(values)->await draft.close()indetached-turn.ts:Restored: 12 passed (12).
Full mutation table
finalize()skips the authoritative writediscard()made a no-optransformFinalTextVerification
pnpm typecheck— cleanpnpm test— 3171 passed / 1 skipped (195 files)agent-docs --check— docs are freshgemini-2.5-prosandbox turn (76 raw events): durable row grew 28 -> 244 -> 783 -> 2136 chars mid-run at the shipped 2 s cadence, final row 3134 chars byte-identical to the single-write path, and a crash at 1009 chars re-entered onto the same row id with no duplicated parts.