[#113] Archive: persist coaching rewrites into session files

[realproject7]

Closes #113

EPIC Alignment

• Parent: [EPIC] LiveCap v1.2 — STT quality, coaching polish, release readiness #107 — [EPIC] LiveCap v1.2 — STT quality, coaching polish, release readiness
• Epic goal: close the gap between "feature-complete" and a product the operator trusts in real meetings.
• This PR's role: resolves epic problem Translation engine interface + Claude CLI adapter (stream-json, env-strip, parser) #5 ("Coaching rewrites are not persisted") — the archive kept only raw utterances, so the Dashboard coaching view could only ever show "before" text. This is the data layer: persist coaching into the session file so past sessions retain their coaching value. UI wiring is the separate UI: persist coaching on generate + Dashboard renders saved rewrites #114.
• Contracts consumed/exposed: exposes CaptionEntry.coaching?: CoachingData (types.ts), the rendered ## Coaching section (render.ts renderCoaching), its parse inverse (parse.ts), and SessionArchiveWriter.amendCoaching(updates) (writer.ts). The (timestamp · occurrence) key is the contract UI: persist coaching on generate + Dashboard renders saved rewrites #114 uses to target a me entry. No existing signature changed; the new field is optional and additive.
• Fits with merged siblings: Archive: persist coaching rewrites into session files (render + parse round-trip + amend-after-finalize) #113 has no Depends on:. Consumes the just-merged Engine: batch coaching rewrites into grouped completions (~30s → ~6s for 10 utterances) #112 coach-result shape structurally ({ better, changes[], explanation }) without importing packages/engine (this package stays engine-free). Deliberately does not touch packages/engine (Engine: batch coaching rewrites into grouped completions (~30s → ~6s for 10 utterances) #112) or src/ (UI: persist coaching on generate + Dashboard renders saved rewrites #114) — UI: persist coaching on generate + Dashboard renders saved rewrites #114 builds on amendCoaching + the parsed coaching field cleanly.

Self-Verification

• Lint: pnpm lint → exit 0 (clean).
• Typecheck: pnpm typecheck + pnpm -r --filter './packages/*' typecheck → clean (archive tsc --noEmit passes under strict + noUncheckedIndexedAccess).
• Tests: pnpm --filter './packages/archive' test → 75 passed (11 new in coaching.test.ts, existing 64 preserved — goldens/parse/dashboard all green, proving byte-identity + backward compat). pnpm test:app → 91 passed; engine typecheck clean (untouched).
• Kill-list scan: clean — no stub/mock/fake in runtime paths, no TODO/FIXME, no dead code, no swallowed errors, no new runtime dependencies (no package.json/lockfile change), no Tauri imports (grep clean — headless), no caption content logged (git diff adds zero console./log(). scripts/no-stub-gate.sh → passed.
• Acceptance criteria 1:1: (a) round-trip render→parse deep-equal, EN + KO ✓; (b) finalize-then-amend keeps every non-coaching section byte-identical ✓; (c) backward-compat fixture without the section parses as today ✓; (d) CJK-safe multibyte round-trip (byte-cap/[follow-up] Archive title length cap is chars not bytes — CJK filenames overflow 255-byte limit, finalize() fails #32 lesson — deep-equal proves no truncation/surrogate split) ✓; (e) all existing archive tests green ✓.
• Manual check: headless TS data layer, no UI/port — exercised by driving the real SessionArchiveWriter through the in-memory FakeFs (open → append → finalize → amendCoaching → parseSession), so round-trips run against the exact bytes the writer emits.

What

• types.ts — optional CaptionEntry.coaching + exported CoachingData (only ever populated for speaker: "me" entries).
• render.ts — renderCoaching(entries) emits a trailing ## Coaching section, each coached me entry as ### (timestamp · k) — <source echo> + **Better:** … (+ optional **Changes:**/**Explanation:**, omitted when empty). k is the 1-based occurrence among me entries sharing that timestamp. Returns "" when nothing is coached → coaching-free documents are byte-identical (append-only invariant + goldens intact). Section is placed after the transcript so amendCoaching only ever appends.
• parse.ts — the exact inverse, keyed off render.ts's named separators (not guessed): parses the section into a (timestamp · occurrence) → CoachingData map (multi-line better/explanation supported) and re-attaches to me entries by recomputing the same occurrence. Files without the section parse exactly as before.
• writer.ts — amendCoaching(updates): Promise<void>, callable only after finalize (every other method throws once finalized), atomic via the existing temp-file + rename discipline. Attaches to a copy of each matched entry so the caller's CaptionEntry objects are never mutated; unmatched updates are ignored (never throws).

Deviations

• Coaching renders as a trailing ## Coaching section, not inline in the transcript — this makes amendCoaching a pure append (guaranteeing byte-identity of all prior sections) and keeps the transcript's append-only invariant intact. The schema still lives on CaptionEntry.coaching as specified; the section is keyed back to entries by (timestamp · occurrence).
• amendCoaching returns Promise<void> (per the amendment's suggested signature) though the injected fs is synchronous — it resolves immediately; callers await it.
• Otherwise none — scope confined to packages/archive; packages/engine (Engine: batch coaching rewrites into grouped completions (~30s → ~6s for 10 utterances) #112) and src/ (UI: persist coaching on generate + Dashboard renders saved rewrites #114) untouched.

[project7-interns]

Verdict: REQUEST CHANGES

Epic Alignment: PASS

The archive-only design matches #113/#107: optional coaching data, rendered/parsing ## Coaching, and post-finalize amend are in the right package and preserve #114 boundaries. One source hygiene issue blocks approval.

Checked (evidence)

• Structural gate: gh pr view 122 --json body,headRefOid,mergeStateStatus at e422337df81d2ce90c88f75338532bdc1b4f1397 includes filled ## EPIC Alignment and ## Self-Verification; PR is CLEAN.
• Issue/epic context: gh issue view 113 plus gh api repos/realproject7/livecap/issues/107 problem #5 require archive persistence for coaching rewrites, with #114 UI wiring later.
• Scope: gh pr diff 122 --name-only shows only packages/archive source/tests.
• Render contract: packages/archive/src/render.ts:77 / :78 define named separators; :103 renders trailing ## Coaching only for coached me entries; :130 appends it after transcript.
• Parse contract: packages/archive/src/parse.ts:167 parses coaching blocks; :324 attaches parsed coaching to transcript entries; :365 only attaches to speaker: "me".
• Writer contract: packages/archive/src/writer.ts:145 adds post-finalize amendCoaching; :148/:149 gate opened/finalized state; :165 writes via existing atomic temp+rename path.
• Riskiest part: the persisted markdown keying and amend path; conceptually sound, but the implementation currently introduces a binary byte into a TypeScript source file.
• Kill-list: scanned changed diff for TODO/FIXME/stubs/logging/new deps/Tauri; finding below.
• CI: gh pr checks 122 → app-macos pass, packages-linux pass, no-stub-gate pass.

Findings

• [blocking] TypeScript source contains a literal NUL byte, causing parse.ts to be treated as binary.
  • File: packages/archive/src/parse.ts:157
  • Why it fails: coachingKey() uses an actual null byte between ${timestamp} and ${occurrence}. Live byte check found one 0x00 byte in the file, and gh pr diff 122 --patch shows packages/archive/src/parse.ts as a GIT binary patch instead of a text diff. Source files must remain text-reviewable; otherwise future reviews, blame, grep/diff tooling, and patch review can miss behavior changes in this parser.
  • Do instead: replace the literal NUL with a text-safe key encoding, e.g. JSON.stringify([timestamp, occurrence]), or an escaped string sequence that leaves the source file ASCII/text ("\u0000" as characters in source, not an embedded byte). Add a small regression check if useful that the key remains collision-safe without embedding control bytes in the source.

Decision

REQUEST CHANGES. The feature shape is aligned, but the binary byte in parse.ts must be removed before this can be approved.

[realproject7]

RE2 review — PR #122 (Closes #113, archive coaching persistence) @ e422337 — APPROVE

Reviewed the live diff against issue #113 + its 2026-07-05 amendment, deriving the parser from render (not the description).

Checked (evidence)

• Byte-identity of amend / backward compat. render.ts:112 renderCoaching returns "" when no entry carries coaching, and render.ts:135 appends it AFTER the transcript, so a coaching-free document is byte-identical (existing goldens untouched). writer.ts:127 finalize reassigns this.workingPath = finalPath, so amendCoaching's atomicWrite(this.workingPath, …) (writer.ts:165) targets the finalized file, and since coaching only affects the trailing section the finalized bytes are an exact prefix (test after.startsWith(before) + stripped-coaching deep-equal of the rest).
• Amend post-finalize only + atomic. writer.ts:148-149 throws unless opened AND finalized; write goes through the same temp-sibling + rename atomicWrite (writer.ts:180-183). Tested: throws before finalize, no .tmp left behind.
• Input immutability. writer.ts amend attaches to a COPY (this.model.entries[index] = { ...entry, coaching: data }), so caller CaptionEntry objects aliased in via appendCaption are never mutated. Regression-tested.
• Derived, not guessed / round-trip. Parser inverts render's exported COACHING_ARROW/COACHING_CHANGE_SEP (render.ts:77-78); COACHING_HEADING = /^### \((.+?) · (\d+)\) — / keys off (timestamp · k) and ignores the advisory source echo. The round-trip test drives the REAL SessionArchiveWriter (open→append→finalize→amend→parseSession), EN + KO, deep-equal.
• Occurrence disambiguation is symmetric. render.ts:103-115 counts k over ALL me entries at a timestamp (coached or not); parse.ts parseEntries recomputes the identical k and re-attaches via coachingKey(timestamp, k). Tested with a duplicate-timestamp me pair split by a them line.
• Multi-line & empty-field round-trip. parseCoachingSection collects continuation lines into better/explanation until the next label/heading and .trim()s — which correctly absorbs the inter-block separator blank while preserving internal blanks; empty changes/explanation omit their line and default to []/"". All tested.
• CJK safety ([follow-up] Archive title length cap is chars not bytes — CJK filenames overflow 255-byte limit, finalize() fails #32 lesson). Multibyte KO (+emoji) in every field deep-equal round-trips — no truncation/surrogate split (render/parse are string-level, no byte cap on this section).
• Boundaries / safety. Only packages/archive (5 src + 1 test); no packages/engine / src/ touch; no new runtime deps (all imports ./-relative); no Tauri import; no caption logging (grep clean). CoachingData mirrors the engine shape structurally without importing engine.
• CI: direct gh pr view 122 @ e422337 → app-macos / packages-linux / no-stub-gate all SUCCESS; MERGEABLE/CLEAN. Archive 75 (11 new) / test:app 91 green.

Findings

• None blocking.

Non-blocking notes (optional, not gating)

1. coachingKey (parse.ts:156) and the writer's inline key are commented "Null-delimited so no pair can collide", but both build ${timestamp} ${occurrence} with a SPACE, not a null byte. No real collision risk (timestamps are clocks with no spaces, and render/parse/writer all construct the key identically and consistently) — purely a comment/impl wording mismatch; fix the comment or use \0.
2. Change from/to text containing the literal separators · or =>, or fields with intentional leading/trailing whitespace, would not round-trip exactly (split/indexOf + .trim()). Non-issue for real engine data (short phrase edits, already trimmed) and mirrors the existing engine coach parser's behavior — flagging for awareness only.

Decision

APPROVE at e422337. Amend is append-only/byte-identical and post-finalize+atomic, parser is a real derived inverse (EN/KO/CJK/multi-line/occurrence all round-trip), input immutability fixed, backward-compatible, boundaries respected, no new deps / no caption logging. No merge (notify-and-wait per dispatch).

[realproject7]

RE2 re-review — PR #122 (Closes #113) @ be8c514 — APPROVE

@re1's blocking finding is fully resolved. Reviewed the delta e422337..be8c514.

Checked (evidence)

• NUL byte eliminated / file is text again. coachingKey (parse.ts:157-159) is now JSON.stringify([timestamp, occurrence]); the writer's inline key matches (writer.ts:151-152). git diff origin/main...be8c514 -- parse.ts is a normal text diff (110/4), no longer Binary files differ. Full-tree audit for \x00 across packages/archive is clean. (Correcting my first-pass note: the binary flag was this NUL, which I under-called as a comment mismatch — @re1 diagnosed it correctly.)
• Collision-safe. JSON.stringify([ts, occ]) unambiguously encodes both fields (quoting/escaping any separator), strictly better than the prior delimiter approach; keys built identically in parse.ts and writer.ts.
• Regression guard added. coaching.test.ts:227-234 reads parse.ts/writer.ts/render.ts and asserts no control bytes except \t\n\r, so a NUL can't silently return.
• No behavior change. The key is module-internal (never crosses the markdown boundary), so this only swaps the internal encoding — round-trip/amend byte-identity/occurrence/CJK/backward-compat all unaffected. Archive suite 76 green (75 + the hygiene guard).
• Everything from my first-pass APPROVE still holds: append-only/byte-identical amend, post-finalize+atomic, copy-on-attach immutability, derived-from-render inverse, EN/KO/CJK/multi-line round-trip, boundaries (packages/archive only), no new deps / no Tauri / no caption logging.
• CI: direct gh pr view 122 @ be8c514 → app-macos / packages-linux / no-stub-gate all SUCCESS; MERGEABLE.

Findings

• None.

Decision

APPROVE at be8c514.

[project7-interns]

Verdict: APPROVE

Epic Alignment: PASS

PR #122 satisfies #113 under #107 by adding archive-only persisted coaching data, a rendered/parsed ## Coaching section, and post-finalize amendment while preserving #114 UI boundaries.

Checked (evidence)

• Re-review delta: reviewed fix commit be8c514 after prior RE1 finding at e422337; PR head is be8c5147ab620daf4da1232d3dafa7a7af1d1450.
• Prior finding resolved: current packages/archive/src/parse.ts:157 now uses JSON.stringify([timestamp, occurrence]); live byte check on current parse.ts found 0 NUL bytes.
• Writer symmetry: packages/archive/src/writer.ts:151 defines the same JSON-encoded key and :155/:164 use it for update lookup and entry matching.
• Regression coverage: packages/archive/test/coaching.test.ts:227 adds the source-hygiene guard; :235 rejects control bytes except tab/newline/carriage return in parse/writer/render sources.
• Text-reviewability: gh api repos/realproject7/livecap/pulls/122/files now returns a normal text patch for packages/archive/src/parse.ts (additions: 110, deletions: 4) instead of omitting it as binary.
• Original scope still checked: archive-only file set; render appends ## Coaching, parse reattaches only to me entries, and writer gates amendCoaching to post-finalize atomic rewrite.
• Riskiest part: persisted markdown keying; acceptable now because keying is collision-safe without embedding binary/control bytes in source.
• Kill-list: scanned changed delta; clean.
• CI: gh pr checks 122 → app-macos pass, packages-linux pass, no-stub-gate pass.

Findings

• None.

Decision

APPROVE. The blocking source hygiene issue is fixed and covered by a regression guard; the archive coaching persistence remains scoped and aligned. No merge action taken.