Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions docs/impls/0039-resize-storm-coalescing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Resize storm coalescing: one settled purge per drag, one writer per session

## Status

Implemented. Tracking issue [#373](https://github.com/yicheng47/runner/issues/373). Corrects the resize-frequency assumption in [0024](archive/0024-resume-scrollback-preservation.md) decision 5. The purge semantics themselves (impl 0020 / #306) and the launch fork-width machinery ([0038](0038-launch-resume-fork-width.md), verified correct by the #373 production log) are unchanged.

## Problem

For full-repaint TUI runtimes, every cols change purges the entire output ring (`SessionManager::resize`, `src-tauri/src/session/manager/output.rs`) — correct for genuinely stale-width bytes, catastrophic when cols changes arrive in storms. The v0.4.7 `[launch-dims]` instrumentation caught production doing exactly that: ~50 cols-gate purges in 90 seconds across three claude-code sessions, wiping the resume transcripts that 0038's correctly-sized forks had just repainted. Three distinct defects were visible in the log:

1. **No coalescing.** A live drag (window edge, sidebar, split gutter) emits one width per frame; every intermediate width purged the full ring and forced a claude repaint, and the final width often equaled the starting width — the whole storm was for nothing.
2. **A transient garbage fit got pushed.** A ~209-col session received a 78×23 push from a container measured mid-hydration. The source: a terminal wrapper in `ChatPaneGroup`'s flat stack that geometry sync had not yet positioned. An unplaced absolute wrapper shrink-wraps xterm's default 80×24 canvas, which minus padding measures as a plausible-looking ~78×23 box.
3. **Two writers fought over one session.** A `76 ↔ 209 ↔ 76 ↔ 210` oscillation: the visible owning pane and a background keep-alive mount (hidden pool / background tab under `visibility:hidden`) alternately pushing different sizes for the same session — the #313 desync class inside a single window.

## Key Decisions

1. **The debounce lives in the backend (`SessionManager::resize`), not in the frontend push path.** Considered both; backend wins on four counts. (a) Every push funnels there — `pushSize`, the activation refresh-push, the resize dance, the went-live reassert, stopped-pane geometry persistence, and any future caller — so one chokepoint covers them all, where a frontend debounce would need to wrap N sites and still miss the next one. (b) The round-trip skip needs the cols the ring's bytes were emitted at (`last_pty_cols`); that is backend ground truth, while the frontend's `lastPushed` mirror is deliberately reset by the disabled-change and rejected-push bookkeeping. (c) A backend debounce also coalesces storms produced by *multiple* writers — a per-mount frontend debounce cannot see the other mount. (d) The behavior under test is "how many purges", and the purge lives in Rust; the tests land next to it.
2. **Rows-only pushes stay synchronous.** The activation dance forces a SIGWINCH repaint by nudging rows−1 → rows with cols held constant; a debounce would collapse those two ioctls into one same-size kernel no-op and kill the dance. Rows-only changes never purge (reflow depends on cols alone, #306), so there is nothing to coalesce away. Same-cols pushes therefore apply immediately, exactly as before — including the failed-ioctl error propagation.
3. **Only clears-on-resize runtimes debounce.** Shells never purge, so a debounce would buy nothing and cost 175ms of drag latency for a TUI running inside the shell. Their cols changes keep the synchronous ioctl.
4. **Trailing debounce with a storm-scoped settle thread, 175ms quiescence.** A cols change parks in `SessionState::pending_resize`; every further push (any cols/rows) folds in, extends the deadline, and bumps a suppressed counter. One thread per storm sleeps out the deadline and applies: the child **always lands on the settled size**, and the purge fires only when the settled cols differ from `last_pty_cols` — a drag that ends where it began costs zero purges. 175ms sits in the issue's 150–200ms band: above a drag frame, small enough that the settle repaint feels attached to the drag ending.
5. **Every settle guarantees the repaint the owner's viewport clear is waiting for.** The owning pane hard-clears its visible region per (real) push to prevent the stacking artifact, and with the debounce no repaint arrives until settle — so the settle must produce one. A purge settle does via the genuine width-change SIGWINCH. A round-trip settle would be a same-size `TIOCSWINSZ` — a kernel no-op with no SIGWINCH — leaving the pane blank until the next output; it therefore applies the rows nudge (rows−1 → rows, width constant, the same trick the frontend dance documents) to force the repaint. On the frontend, `shouldClearViewportBeforePush` pins the other half of the pairing: only a `verdict === "push"` may clear, so a suppressed or deduped push can never blank a viewport that no repaint will restore.
6. **The existing resize contracts survive the debounce.** `update_last_size` still runs at request time, so pane geometry persistence stays prompt while the ioctl waits (the row already carries the next fork's desired size). A failed settled ioctl logs a warning, drops the storm, and leaves `last_pty_cols` untouched — a failed live resize still cannot trigger a ring purge; the caller-visible error is gone (there is no caller left at settle time), which is acceptable because the retry path is the next real resize.
7. **The settle is linearized against kill/respawn under the state lock.** Two windows make anything weaker unsound. First, `kill` sets `killed = true` and only then enters `runtime.stop`, which can spend hundreds of ms reaping the child — the handle/pending teardown runs after stop returns, so a settle firing inside that window sees a live-looking state. Second, the runtime resolves `RuntimeSession` by reusable session id and a respawn *overwrites* that mapping, so an ioctl issued outside the lock could physically resize a child spawned after the storm — a generation check on the commit alone cannot prevent the ioctl itself. So the whole settle — take `pending_resize`, validate, ioctl, commit — runs as one critical section under the session's state lock, aborting if `killed` or `resuming` is set or the handle is gone. The ordering argument: `kill` sets `killed` under this lock *before* stop begins, and `resume` sets `resuming` under this lock *before* `runtime.spawn` can overwrite the map — so a settle that finds neither flag holds the lock while the id still resolves to the child it measured, and no teardown or replacement can start until it releases. An earlier draft used a `resize_epoch` generation validated at commit; it was dropped for the linearization because it could not stop the physical ioctl (review of #373). Holding the lock across a `TIOCSWINSZ` stalls output ingestion only for the ioctl's duration.
8. **Settle-gate via wrapper placement, not a size heuristic.** `ChatPaneGroup`'s geometry closure now tracks which sessions have been positioned from a real pane-body rect; until then the wrapper is held at 0×0, and a zero-rect pane body refuses to place. That makes an untrustworthy box *unmeasurable* — the same state as a `display:none` pane — so `RunnerTerminal`'s existing rect guards hold every fit and push with no new props, no re-render plumbing, and no magic minimum size. The signal is the layout-hydration state ChatPaneGroup itself owns; when placement lands, the pane's own ResizeObserver drives the first real fit.
9. **Single-writer via `active`.** `sizePushVerdict` (`src/lib/terminalResize.ts`) is the one gate for backend size pushes: dedupe first (`unchanged`), then transitional suppression (`resizeDisabled`), then ownership — only the visible owning pane (`active`, i.e. surface visible ∧ in-layout ∧ not transitional) may write. Hidden-pool wrappers and background `visibility:hidden` surfaces keep refitting their local xterm but observe without writing; the activation refresh pushes the new owner's size when a surface comes to the front, so ownership hands over exactly there. This deliberately retires the old "invisible persistent surfaces keep their PTY geometry current" behavior — that standing write was defect 3. The single-window case only; the #313 multi-window ownership protocol is out of scope.
10. **Instrumentation extends #372 in the same log file.** Backend: `cols-gate purge: … (N coalesced)` on a settled width change, `cols-gate settle: … round-trip, purge skipped (N coalesced)` on a free storm. Frontend, mirrored into `runner.log` via `frontend_log`: `[resize-gate] suppressed-transitional|suppressed-nonowner session=… WxH` (throttled to one line per second per mount) and `[resize-gate] pane-placed session=… WxH` once per wrapper placement. A launch now reads as: fork at real width → pane-placed → first-fit → at most one settle line per storm.

## Goals

- A continuous drag across a live claude-code pane costs at most one purge and one repaint, at the final width; a drag that returns to its starting width costs zero purges and exactly one restoring repaint (the settle's rows nudge — see decision 5).
- A measurement from an unsettled or transitional container is never pushed to the PTY.
- Exactly one surface pushes sizes for a session; background mounts never fight it.
- After a launch with auto-resume, a claude-code pane retains the transcript its resume repaint produced, and the log file proves the storms were coalesced.

## Non-Goals

- Changing the purge semantics. Genuinely stale-width bytes still purge (impl 0020 / #306); this impl only removes purges whose "stale" width was never applied.
- The full multi-window ownership protocol (#313). `active` resolves the writer within one window; cross-window ownership stays as-is.
- Debouncing the local xterm fit. Panes keep tracking their container live; only the backend push coalesces.
- Restoring scrollback already lost to earlier storms.

## Implementation Notes

- `src-tauri/src/session/manager/mod.rs` — `PendingResize`, `SessionState::pending_resize`, `RESIZE_SETTLE_MS` (175), `resize_settle_ms` field (test-tunable), `install_handle` storm invalidation.
- `src-tauri/src/session/manager/output.rs` — `resize()` split into synchronous same-cols / non-clearing paths and the debounced cols-change path; `settle_pending_resize` (apply + round-trip skip + logs); test-only `settle_pending_resize_now`. The now-unused `purge_output_buffer_keep_modes` helper is folded into the settle (the buffer-only clear happens under the same lock as the `last_pty_cols` update).
- `src-tauri/src/session/manager/lifecycle.rs` — `kill` drops `pending_resize` with the handle.
- `src/lib/terminalResize.ts` — `sizePushVerdict`.
- `src/lib/frontendLog.ts` — `logResizeGate` (`[resize-gate]` lines into `runner.log`).
- `src/components/RunnerTerminal.tsx` — `pushSize` / `pushBackendResize` routed through the verdict; suppression logging; comments updated for the retired hidden-surface write.
- `src/lib/paneGeometry.ts` — `createPaneGeometry` moved out of `ChatPaneGroup.tsx` (the export for tests tripped `react-refresh/only-export-components` there), gaining placement tracking, the 0×0 hold for unplaced wrappers, zero-rect body refusal, and the `pane-placed` log.

## Validation

- Rust (`session::manager::tests`): a resize storm collapses to one settled ioctl and one purge; a round-trip storm produces zero purges and forces the repaint via the rows-nudge ioctl pair; a failed settled ioctl purges nothing and leaves the gate unmoved for the retry; rows-only resizes stay synchronous and keep the ring; the settle thread applies without a manual nudge; a settle firing inside kill's stop window (blocking-stop gate) aborts with no ioctl and no purge; a stale settle after kill + respawn touches neither the fresh PTY nor its ring, and the respawn's seeded cols still gate the next push.
- Frontend (vitest): `sizePushVerdict` — transitional and non-owning mounts push nothing, the visible owner pushes, unchanged sizes dedupe silently; `shouldClearViewportBeforePush` — the viewport clear pairs only with real owner pushes, never with suppressed or deduped ones; `createPaneGeometry` — unplaced wrappers hold at 0×0, zero-rect bodies refuse to place, detach re-gates.
- Manual smoke: drag the sidebar, a split gutter, and the window edge across a live claude-code pane — scrollback survives with at most one repaint at the end of the drag, and a round-trip drag keeps its full ring and ends with the single nudge-forced repaint; quit and relaunch with auto-resume — `runner.log` shows `pane-placed` before `first-fit`, coalesced settle lines instead of purge storms, and no transient-width purge; scroll history is intact afterwards.
2 changes: 1 addition & 1 deletion docs/impls/archive/0024-resume-scrollback-preservation.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Contract 3 is real for codex but wrong for claude-code: codex repaints the whole
2. **A seq watermark replaces "clean buffer" as the pill contract.** At the same top-of-`resume()` point, record `resume_watermark_seq = output_seq` in `SessionState` (all runtimes, uniformly — for codex it's equal to the post-purge floor, so filtering is a no-op). The pill effects' snapshot fast-path only honors TUI-ready escapes in events with `seq > watermark`. Old chunks stay replayable for the terminal but can no longer clear the overlay early.
3. **Expose the watermark via a dedicated read command,** `session_replay_watermark(session_id) -> u64`, rather than changing `session_output_snapshot`'s return shape (which `RunnerTerminal` replay consumes as a bare array) or `session_resume`'s (a resume can be triggered from another window — impl 0018 — so the pill can't rely on the resume RPC's response reaching it). Fresh spawns report 0, so the filter is inert outside resume flows.
4. **Do not reset the terminal-mode flags on the keep path.** `purge_output_buffer` resets `alt_screen_on` / `bracketed_paste_on` because after a purge no escape bytes remain to justify a synthetic snapshot prefix. When the buffer is kept, the old chunks carry their own mode escapes and `update_terminal_mode_state` keeps deriving state from the live stream; the seq=0 synthetic prefix stays correct for the evicted-escape case it was built for. Worst case is a redundant `\x1b[?2004h` replayed from both the prefix and a surviving chunk — harmless.
5. **Accept the double-tail and stale-width artifacts.** The last ~4 turns will appear twice after a resume (once in kept scrollback, once in claude's repaint) — identical to resuming in Ghostty; not a bug. Kept bytes were emitted at the old grid width; a later remount replays them into the current grid, so lines wrap at the recorded width. For claude-code's inline text this reads like ordinary terminal reflow; it is bounded anyway because live resizes still purge the ring for claude-code (`resize` → `purge_output_buffer_keep_modes`, unchanged by this impl), so the kept segment never spans a width change that happened while the session was running.
5. **Accept the double-tail and stale-width artifacts.** The last ~4 turns will appear twice after a resume (once in kept scrollback, once in claude's repaint) — identical to resuming in Ghostty; not a bug. Kept bytes were emitted at the old grid width; a later remount replays them into the current grid, so lines wrap at the recorded width. For claude-code's inline text this reads like ordinary terminal reflow; it is bounded anyway because live resizes still purge the ring for claude-code (`resize` → `purge_output_buffer_keep_modes`, unchanged by this impl), so the kept segment never spans a width change that happened while the session was running. **Correction ([#373](https://github.com/yicheng47/runner/issues/373)):** this bullet leaned on resizes being rare and user-driven. Production launch logs falsified that — post-launch layout churn fired ~50 cols-gate purges in 90 seconds with nobody touching anything, each one dropping the very transcript this impl preserved. The purge semantics stand, but the push path now coalesces a resize storm into one settled resize and skips the purge entirely when a storm round-trips to the ring's own width — see impl [0039](../0039-resize-storm-coalescing.md).
6. **No frontend rendering changes.** In-place resume already preserves the mounted xterm buffer under the `opacity-0` overlay; this impl makes the backend ring agree with it so remounts stop losing what the screen already showed. `ResumeSettleTracker` (`src/pages/RunnerChat.tsx`) listens to live events only — live events during a resume window can only come from the new PTY (resume is refused while the row is running) — so it needs no watermark. The stale `clearVersion` comment block at `src/pages/RunnerChat.tsx:1120` gets rewritten to describe the real mechanism.

## Goals
Expand Down
6 changes: 6 additions & 0 deletions src-tauri/src/session/manager/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ impl SessionManager {
state.last_local_input_at = None;
state.mission_status_sink = None;
state.completion_armed = false;
// Drop any coalescing resize storm with the child; a
// settle that fires later sees None and exits without
// an ioctl. One that fired between `killed = true` and
// here aborted on the killed flag (settles run under
// the state lock, so none is mid-flight right now).
state.pending_resize = None;
(h.stop.clone(), h.forwarder.take())
}
None => return Ok(()), // raced with another caller; no-op
Expand Down
Loading
Loading