perf(feed-debug): pace persistence appends by time instead of per runtimes replacement

[Juliusolsson05]

Problem

useFeedDebugPersist runs in a useEffect keyed on the runtimes map, which is replaced dozens of times per second while a turn streams. Every replacement that found new feed-debug entries sent them to main immediately and the success path re-drained at once, so the append cadence followed the streaming cadence. Perf journal evidence (#748): 8,649–10,159 debug:append-feed-log invokes in a 77-minute run (111/min average, 20/s in bursts); 44 calls ≥ 50 ms, max 206 ms; on the Sep 1 run p50 87 ms, p95 352 ms, max 1,159 ms. Each invoke is a writeFile(flag:'a') and a retention-prune schedule on the main thread.

Behaviour

• A pure policy (feedDebugFlushPolicy.ts, decideFeedDebugFlush) decides when a session's pending entries go out: the first batch after a quiet period flushes immediately; anything that arrives within FEED_DEBUG_FLUSH_INTERVAL_MS (1.5 s) of the last attempt waits for one per-session timer; FEED_DEBUG_FLUSH_MAX_PENDING (256 entries) or FEED_DEBUG_FLUSH_MAX_PENDING_BYTES (1 MiB, from the ring's cached per-entry estimate) forces an immediate flush.
• The persisted / in-flight cursors and the one-append-in-flight rule are untouched, so retry-on-failure and idempotence by (sessionId, id) on the main side still hold.
• A rejected append counts as an attempt: entries stay pending (the persisted cursor only advances on success) and the policy re-runs so an idle session retries after the interval instead of waiting for another replacement or following every streamed delta.
• A session that leaves runtimes (replacement, pane close, tab kill, hard reload — soft reload replaces the runtime under the same key and does not fire the removal flush; its pending entries ride the armed timer into the next epoch, tracked in bug(feed-debug): soft reload restarts entry ids below the persisted cursor so persistence silently stops #770) gets one final, unpaced flush of its trailing entries from its last snapshot — parked until an in-flight append resolves if there is one — then its timers/cursors are dropped. Those entries (exit code, kill reason) are what debug bundles read for closed panes.
• Pending-count is computed by scanning from the tail (O(pending), not O(ring)), since the hook runs per replacement.
• Timers live in a ref and are cleared on unmount only; the per-replacement effect must not tear them down or the interval would never elapse under load.

Design decisions

• Time is the primary pacing because the goal is fewer main-thread writes; the count ceiling bounds batch size, and the byte ceiling exists because the ring is byte-capped at 4 MiB and evicts from the head — with the perf(renderer): feed-debug ring is count-capped only and reached 130 MiB in one session #722 shape (hundreds of KB per entry) twenty entries already exceed the ring, so waiting for 256 would let unpersisted entries be evicted. A quarter of the ring forces the flush long before that.
• Immediate first flush keeps "Save Debug Logs" right after a bad paint truthful: the entry that explains it is on disk within one round trip, not up to 1.5 s later.
• No pagehide flush: an invoke cannot be awaited past teardown, so up to 1.5 s of feed-debug can be lost at window close or on a hard renderer crash. Session removal is NOT in that category — it flushes synchronously with the removal (see Behaviour).

Linked issues

Fixes #748. Refs #722, #103.

Verification

• feedDebugFlushPolicy.test.ts (6): immediate on first batch and after the interval; timer for the remainder inside it; forced at the count ceiling; forced at the byte ceiling before the count is reached; nothing while in flight or with nothing pending; tail-scan pending count.
• useFeedDebugPersist.renderer.test.tsx (8, fake timers, stubbed window.api.appendFeedDebugLog): 20 replacements inside the interval produce one immediate append and exactly one paced append carrying all 20 entries in order; entries arriving while an append is in flight are paced on resolve (not drained); the count ceiling forces a flush inside the interval; three ~512 KB entries force a flush via the byte ceiling; a removed session's trailing entries are flushed at once, and once its in-flight append resolves; a rejected append keeps entries pending and retries no sooner than the interval with the full range; unmount clears the timer.
• --project renderer src/renderer/src/workspace/hook/persistence/ — 22 passed; --project unit persistence + feedDebug.test.ts — 13 passed; npx tsc -b — clean.

Limitations / follow-ups

• The runtimes replacement per frame that drives this effect (and many others) is a structural item being audited separately; this PR only stops it from turning into disk writes.
• Main-side feedDebugLog.ts is unchanged; with ~1 append/1.5 s/session its per-batch stat/prune scheduling is no longer hot.
• Two pre-existing durability bugs found in review are tracked separately: bug(feed-debug): soft reload restarts entry ids below the persisted cursor so persistence silently stops #770 (soft reload restarts ids below the persisted cursor) and bug(feed-debug): the fail-closed unknown-file-size path resolves the append so the renderer marks unwritten entries durable #771 (the fail-closed stat path resolves instead of rejecting).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Kk16MNVJtWAnCeGxGRuHqa

[Juliusolsson05]

Review applied in 6a6f84a:

• Blocker: a session leaving runtimes now gets one final, unpaced flush from its last snapshot (parked until an in-flight append resolves), then its timers/cursors are dropped. Tests: removal while the timer is armed → immediate append with the trailing entries and no later timer; removal while an append is in flight → flush on resolve.
• Resolve-path pacing now has a deferred-promise test (entries arriving in flight are paced, not drained).
• .catch re-runs the policy (arms the interval), post-unmount resolutions no longer re-arm, lastAttemptAt/timer entries are cleared for removed sessions.
• Plan updated (policy inputs incl. pendingBytes, byte ceiling, removal flush); stale comments in feedDebug.ts and ipc/debug.ts corrected; PR body counts and the crash-loss framing fixed.
• Pre-existing bugs filed as bug(feed-debug): soft reload restarts entry ids below the persisted cursor so persistence silently stops #770 and bug(feed-debug): the fail-closed unknown-file-size path resolves the append so the renderer marks unwritten entries durable #771.

[Juliusolsson05]

Adversarial review (re-run after the pause; worktree 6a6f84a, all commands run locally).

VERDICT: REQUEST-CHANGES

Core pacing design is sound — pure policy, one timer/session, one-in-flight preserved, cursors untouched — but the rework commit introduced the issues below.

1. BLOCKER — Removed-session final flush has no pacing: persistent rejection spins an unbounded retry loop. useFeedDebugPersist.ts:157-163 — the final path calls send() whenever remaining > 0 without consulting lastAttemptAtRef or decideFeedDebugFlush. On rejection, .catch (line 137) re-invokes consider(sessionId, undefined) → session is gone from latestRuntimesRef → final path again → send() again. No interval, ceiling, or attempt cap — a tight loop for as long as main keeps rejecting (disk full/EACCES/EIO at close time, shape validation, or #771's fail-closed branch after its own fix). This reintroduces exactly the retry storm the PR exists to kill, and contradicts the PR body claim "A rejected append counts as an attempt … retries after the interval" (true only for live sessions). Fix: apply the interval to the final path too. No test covers rejection on a removed session.
2. SHOULD-FIX — unmountedRef never reset on (re)mount; permanently true after StrictMode's dev double-mount. useFeedDebugPersist.ts:73-77 sets the flag in cleanup but the effect body never sets it back to false. App runs under <React.StrictMode> (main.tsx:79); after the simulated unmount/remount every .then/.catch bails — resolve-path pacing and failure retry are dead in dev. One-line fix: unmountedRef.current = false in the mount-effect body. New tests render without StrictMode so they cannot catch this.
3. SHOULD-FIX — Soft reload bypasses the removal flush; up to 1.5 s of trailing entries is a new loss window. The removal flush only fires when a session key leaves the map (useFeedDebugPersist.ts:202-208). softReloadAgentView/softReloadRuntime replaces the runtime under the same key (session.ts:120-192; reset = emptyRuntime() at :142 restarts feedDebugNextId), so no final flush fires: pending entries on the armed timer are silently dropped, and #770 keeps the new epoch off disk. Hard paths (replaceSession, reloadAgentSessions, killSession) do remove the key and are covered — the PR body's blanket claim is wrong for soft reload. Fix: detect the epoch reset (incoming feedDebugNextId below the previous pass's) and final-flush the previous runtime; at minimum correct the body and document the window.
4. NIT — forget() doesn't clear the session's feed-debug cursors in WorkspaceRefs (useFeedDebugPersist.ts:141-149); entries leak for the app lifetime.
5. NIT — per-replacement allocation at useFeedDebugPersist.ts:175: slice(-pendingCount) feeds the byte estimator dozens of times/second/session; an index-based tail sum avoids it.
6. NIT — tests: install param shadows the imported appendFeedDebugLog; missing coverage for findings 1-3.

Verified non-issues: byte ceiling at ¼ of the ring correctly bounds eviction risk while not in flight; countPendingFeedDebug tail scan consistent with selectFeedDebugAppendBatch under monotone ids including the #770 restart shape; armed-timer-at-removal race resolves benignly; #770/#771 behaviors unchanged (correctly separate).

Commands: renderer persistence suite 22/22 PASS; unit persistence+feedDebug 13/13 PASS; npm run typecheck PASS.

[Juliusolsson05]

Round-3 rework pushed (f622997):

• Blocker 1 (final-flush retry storm) — fixed. The final branch now keeps its own per-session attempt stamp (finalAttemptAtRef): the first final flush stays immediate (delaying it would drop exactly the exit-code entries the branch exists to persist; the live cadence's stamp can't be reused because the pre-removal batch is typically milliseconds old), while any retry the final branch itself made inside the interval arms the shared per-session timer instead of re-sending. forget() clears the stamp. Regression test drives two consecutive rejections and asserts exactly one attempt per interval and a clean stop after success.
• Should-fix 2 (StrictMode unmountedRef) — fixed. The unmount-only effect re-arms its flag in the effect body; new test renders under <StrictMode> and asserts the durable cursor still advances after the simulated remount.
• Should-fix 3 (soft-reload loss window) — not in this PR. It is entangled with the epoch-restart behavior tracked separately in bug(feed-debug): soft reload restarts entry ids below the persisted cursor so persistence silently stops #770; the PR body claim about soft reload has been corrected below.

Verification: renderer persistence suite 10/10 (incl. 2 new), unit persistence + feedDebug 13/13, npm run typecheck clean.