R1 wave-driver — post-landing review (bugs + concurrency + functional)

Session: backend, 2026-07-19, on top of the landed R1 driver-switch (89d5bf9). Ask (owner): a clean functional + concurrency-bug look at the wave feature — "what happens when a book is loaded into the backend" — plus three owner findings to fold in (R1-FL-A/B/C). Method: own source trace + 4 independent adversarial reviewers (author≠reviewer) on distinct lenses (money · store/SQLite · wave-logic · end-to-end functional), then a verification workflow (3 lenses + synthesis) over the fix I wrote. Every reviewer finding was re-verified against source before acting. Status: two LIVE fixes applied to the working tree (UNCOMMITTED — orchestrator lands); R1-FL-A/B/C deferred to the pere-run mining batch per owner. go build/vet, full go test -race ./..., golden byte-stable.

1. Concurrency verdict — SOUND (3 reviewers + trace agree)

No money-correctness, budget, resume, or data-race defect on the concurrency axis. Confirmed at file:line:

Money atomic under the single writer. Reserve/SettleWithCheckpoint are single BEGIN IMMEDIATE transactions on the 1-connection write pool (store.go:71, ledger.go:50) → committed == Σ settled; two parallel reserves fully serialize (no ceiling skip). Settle uses a fresh opContext (not the wave ctx), so a cancelling wave still persists a paid 2xx; a kill-9 loses ≤ N in-flight, each re-paid once (recoverReservations).
Escalation soft-cap exact. escMu is held across the fresh hop's settle (escalation.go:119-128); the next worker's read-pool EscalationSpentUSD sees the committed hop (WAL read-after-commit across the mutex) → overshoot ≤ 1 hop.
No reservation leak: every post-Reserve exit settles or releases (stagerun.go:377/382/393/422/443/483).
Shared chapter×stage job row is advisory-only (EnsureJob keyed (book,chapter,stage)): chunk_status/ checkpoints are the resume authority; jobs.status thrash is telemetry, nothing gates on it.
retrieval_state RMW race-free: each unit owns a distinct leader (chapter,FirstChunkIdx); waves are sequential (barrier at wg.Wait()).
No shared mutable Runner field written during a wave; MemoryBank.Select/postcheck are pure reads over immutable state; roleInjectionRenderers read-only; ReqInfo value-typed (no ctx aliasing).
«переоплата ОДНА» airtight at BOTH levels: the draft injection selects over r.baseMemory (Source≠mined) and the draft snapshot folds r.memory.BaseVersion() (same mined-exclusion) — a mining sign moves only the edit snapshot.

2. Bug FIXED — `runWave` swallowed parent cancellation (the one real defect, live)

The single concurrency bug all reviewers-but-one missed on the first pass — the wave-logic lens caught it.

runWave (waverun.go) set firstErr only from a worker error and returned it bare after wg.Wait() — no parent.Err() check. The feeder drops items on ctx.Done(), and a worker need not error to finish (a $0 resume worker resolves entirely through opContext() store reads, never touching the cancellable ctx). So a Ctrl-C / SIGTERM (main.go:55 signal.NotifyContext) whose in-flight workers don't error → runWave returns nil with items undone →

edit pipeline (prod C1 default): unitOutcomes[i]==nil → *oc nil-deref panic;
draft-only: zero-value → fake DispOK "" → exit 0 on an incomplete book.

Reliably hits on a Ctrl-C'd resume. The durable store was always correct (undone items never checkpointed → a re-run resumes), so no data/money loss — the damage is a crash / a false success signal.

Fix: return parent.Err() after wg.Wait() when firstErr==nil. Proven by execution: TestRunWaveSurfacesParentCancellation fails red on the old code, green on the fix.

3. Owner-escalated DECISION → implemented — edit-unit flag blast radius (c-lite)

The live behavioural consequence of the chunk→unit shift. Pre-fix, in runEditUnit one flagged draft member skipped the whole unit's edit and shipped exportNormalize(memberRecovered) — always "" for a draft stage (the sanitizer only fills recovered on isFinal). So a single bad draft chunk blanked a whole chapter-scale unit, discarding the good, already-paid sibling drafts — a regression from the retired per-chunk model.

Chosen (owner deferred the call; owner leaned toward option c): edit-clean-remainder ("c-lite"). A flagged member is DROPPED; the editor runs over the CLEAN members' source+draft, addressed at the manifest leader; the unit SHIPS the edited clean remainder but is FLAGGED (a member dropped). Only an all-members-flagged unit ships "".

Why c-lite over a full positional split: any "ship edited content" option forces export to become member-aware (today's contract is "flagged unit → edit row non-OK → ships nothing"). A full split (N sub-edits with in-place gaps) additionally forces every read-model to reconstruct the sub-run partition from stored draft dispositions and makes status's expected-stage-count flag-dependent — the fragile surface where read-model bugs breed. c-lite keeps ONE edit row per unit (edit row stays DispOK → resume unchanged), so status and the golden are untouched. Byte-identical to a full split whenever the flagged member is at a unit edge (the common case); differs only for a mid-unit flag (flanking clean pieces edited as one joined block, not positioned sub-edits) — a negligible artifact on a rare, human-reviewed flagged unit. One cost: redriving a dropped member later re-edits over the now-complete unit (one rare wasted editor call).

Files: runEditUnit (drop flagged members, edit clean remainder, ship flagged); export.go re-derives "dropped member ⇒ flagged unit" from member draft rows so export/status/translate agree while the text ships; status/golden untouched (the no-flag path is byte-identical → cleanSources join == unit.sourceText()). Tests: dropped-member → edits the clean remainder + ships it flagged (export agrees); all-members-flagged → ships "".

3.1 Verification-workflow findings closed (all MINOR/NIT telemetry — no bytes/money/resume impact)

The 3-lens verification of c-lite found the core sound (no CRITICAL/MAJOR) and four consistency items, all fixed:

quality.go echo/KPI (MINOR — the one un-aligned read-model, an acceptance metric): a recovered unit's edit row is now DispOK, so EchoRate/CosmeticStripRate stopped counting its (real, dropped) draft echo. Fixed: quality re-derives the drop like export/status and counts the dropped member's reason toward the rates. The structural KPI intentionally measures the shipped remainder (accurate). Owner nuance: this preserves the pre-c-lite semantics ("echo anywhere, incl. dropped members"); if you'd rather EchoRate mean "echo in shipped text only", say so and it's a one-line revert of this preservation.
status.go flag_reason first-vs-last (NIT): resolveChunkState overwrote reason on every flagged row (last-wins), contradicting its own "first (only)" comment and diverging from translate/export on a multi-flag unit. Fixed: first-wins guard (aligns all read-models; also closes the pre-existing plain-multi-member case).
export --pairs source (NIT): --pairs source was the full unit source while the target is the clean-members edit, mis-pairing the DC1/DC2 FP-measure. Fixed: for a dropped-member unit the --pairs source is reconstructed from the clean members (byte-for-byte as runEditUnit's cleanSources).
export Detail (NIT): the member-drop override flipped FlagReason but kept the OK edit's blank Detail. Fixed: propagate the dropped member's Detail alongside its reason.

4. Owner findings R1-FL-A/B/C — confirmed, DEFERRED to the pere-run mining batch

All three reproduce exactly and are gated behind LIVE mining (a langpack + a contrast artifact — stand-only). Per owner: apply in the pere-run-prep batch together, NOT as a micro-commit.

R1-FL-A (main.go:34-44): exitCode maps only *CompletedWithFlags→2; a *WaveSignatureStop→1 (== a crash). The translate() path returns it raw with no errors.As arm; the waverun.go:72-77 comment over-claims CLI wiring. Human sees the stderr text; automation can't distinguish a sign-boundary stop from an infra crash. Fix shape: an errors.As(&sigStop) arm → a distinct exit code.
R1-FL-B (mining.go:56, miner_emit.go): the mining STOP clears only on an EMPTY delta and there is no reject/ignore list — a term the owner declines to sign is re-mined → STOP forever (livelock). The only mechanical escape (undocumented, semantically wrong) is a status:auto no-dst seed row. Ties to the deferred W1.5-UX design (a reject/suppression set).
R1-FL-C: x/text/norm carries its own Unicode revision, not folded into Ш-2 (only unicode.Version is) — a real tripwire before an x/text bump.

5. Reviewed — not bugs (NITs, for awareness)

jobs.status thrash under the shared chapter×stage job row — advisory, nothing gates on it.
Read-model fields say chunk/total_chunks but now carry UNIT counts — naming/contract debt for JSON consumers (counts are internally correct).
escMu held across the escalation LLM call — throughput smell (rare, opt-in escalations), no correctness impact.
minedToCandidates is dead in the live path (superseded by file-based loadMinedDelta) — cleanliness.
The real HTTP adapter's concurrency is unexercised by the fake-client tests — worth one live parallel smoke (standard http.Client is safe by design).

6. Change set (working tree, UNCOMMITTED — orchestrator lands)

LIVE wave-driver correctness (NOT mining-gated) — land these:

waverun.go: runWave parent-cancellation → error; runEditUnit c-lite (edit the clean remainder).
export.go: member-drop re-derivation + Detail + --pairs clean-source alignment.
status.go: resolveChunkState first-wins guard.
quality.go: echo/strip preservation for a recovered unit.
waverun_test.go: TestRunWaveSurfacesParentCancellation, updated TestWaveEditUnitFlaggedMemberDraft (c-lite + export + quality assertions), TestWaveEditUnitAllMembersFlagged.

Evidence: go build/vet clean; full go test -race ./... clean (~31s); golden byte-stable (no-flag path identical); gofmt clean. Nothing committed (orchestrator lands per the standing workflow).

7. Open for the owner

Echo-metric semantics (finding 3.1-1): kept as "echo anywhere incl. dropped members" (non-regression). Switch to "shipped-text only" if preferred.
R1-FL-A/B/C land in the pere-run mining batch; R1-FL-B needs the W1.5-UX reject-set design.
Landing order: the two LIVE fixes (§6) can land before the mining batch.

11 KiB Raw Blame History Unescape Escape