diagnose-scrollback-corruption

Metadata

Description

## Description
Multiple targeted fixes have failed to hold (fix-tui-pty resize, fix-pty-scrollback initial-render). Symptom recurs. User's refined hypothesis:

User quote: 'we should be able to fix this. might need recompute of flow/wrap??? unclear. interaction with claude or codex is weird potentially.'

Hypothesis to test:
1. **Reflow/rewrap on SIGWINCH** — when terminal width changes, the scrollback buffer needs to be re-wrapped to the new column count. If the rewrap logic doesn't reset all state (cursor position, line-continuation flags, scroll region top/bottom), garbled output / duplication emerges.
2. **Child-agent output interaction** — claude and codex CLIs may emit:
   - Alt-screen enter/exit sequences (\x1b[?1049h / \x1b[?1049l)
   - Scroll region commands (\x1b[<top>;<bottom>r)
   - Cursor save/restore (\x1b7 / \x1b8 or \x1b[s / \x1b[u)
   - SGR mouse mode toggles (\x1b[?1000h, \x1b[?1006h)
   - DECSET/DECRST mode bits
   If our reflow code doesn't preserve these mode bits across SIGWINCH, the next render writes into the wrong buffer / wrong scroll region and we get the symptom.

## Repro fidelity required
Many SIGWINCH events fire from sources other than deliberate resize: window manager focus changes, parent terminal redraws, tmux operations. Repro should be deterministic — fire SIGWINCH programmatically (via `kill -WINCH`) at known points in a controlled chat output stream, not rely on a human resizing a window.

## Investigation steps (research only — do NOT write the fix yet)
1. Capture raw bytes from a typical claude chat session AND a typical codex chat session (separate files). Use `script` or wg's chat-history JSONL.
2. In a test harness, replay those bytes into the current scrollback emulator with a SIGWINCH fired at frame N. Vary N. Identify which N values produce the corruption.
3. Read the current src/tui/ reflow path. Specifically look for:
   - Whether all of: cursor, scroll region, alt-screen state, SGR state, mode bits are saved before reflow and restored after
   - Whether the reflow re-parses from buffer-start or from current-cursor — re-parsing from cursor while buffer still contains pre-resize bytes is a known bug class
   - Whether streaming-text + finalized-message double-emit (the recent regression in commit 572a28d37 fix-pty-output) sneaks back in via the reflow path
4. Compare claude vs codex output streams: which mode bits / sequences differ? Codex CLI may use alt-screen more aggressively (it's a fuller TUI than claude's stream-json output).

## Deliverable
A diagnostic write-up posted via `wg log`, NOT a code change:
- Confirmed root cause with file:line citations
- Specific mode bits / state that's lost across SIGWINCH (with citation showing where it should be saved+restored but isn't)
- Whether claude vs codex paths trigger different bugs OR the same bug at different rates
- Concrete fix proposal (1-2 paragraphs) so the follow-up implementation task can execute against a clear spec
- If after investigation the conclusion is 'this is fundamentally an architectural problem, not patchable' — say so explicitly and refer to the queued replace-custom-pty task

## Validation
- [ ] Reproduction is deterministic (kill -WINCH at known points produces the bug; without SIGWINCH at those points, no bug)
- [ ] Root cause identified with file:line citations
- [ ] Mode bits / state preservation issues enumerated specifically
- [ ] claude vs codex difference characterized
- [ ] Fix proposal concrete enough that the follow-up task can implement without re-investigation
- [ ] No source modifications — diagnose only

Depends on

• done .assign-diagnose-scrollback-corruption

Required by

• (none)

Log

• 2026-04-29T13:54:21.432503075+00:00 Task paused
• 2026-04-29T13:54:32.644961638+00:00 Task published
• 2026-04-29T13:54:49.043188750+00:00 Lightweight assignment: agent=Careful Programmer (f5143935), exec_mode=light, context_scope=task, reason=Careful Programmer has strong track record (0.77 score, 293 tasks); Careful tradeoff suits diagnostic precision needed for PTY/scrollback investigation; light mode enables code reading without modifications.
• 2026-04-29T13:54:49.091000915+00:00 Spawned by coordinator --executor claude --model opus
• 2026-04-29T13:54:59.739116875+00:00 Starting investigation: SIGWINCH/reflow scrollback corruption hypothesis
• 2026-04-29T14:08:31.573508411+00:00 # Diagnostic: scrollback corruption on SIGWINCH ## TL;DR The bug is a vt100-0.16 limitation amplified by chat-tab UX: every input-editor wrap fires SIGWINCH, vt100's `set_size` does NOT reflow scrollback rows, and the patchwork dedup in pty_pane.rs only hides the *latest* resize echo. Multiple resizes per session compound the problem. Fix is true reflow on resize (re-feed snapshot into a fresh parser), or escalate to replace-custom-pty if SGR-preservation is required. ## Reproduction (deterministic) Standalone repro at `/tmp/sigwinch-repro/src/main.rs` — uses vt100 0.16.2 directly, no PTY, no SIGWINCH signal. Output below confirms each finding empirically. ``` Case A — cell-array width drift: fill at 100 cols, set_size(8, 40), then read scrollback off=1: cell(0,30) = Some("x") cell(0,60) = Some("x") ← past visible cols=40, content still there cell(0,99) = Some("") set_size(8, 200): cell(0,150) = None ← row Vec stays at original 100 Case B — child SIGWINCH reflow → real duplicates: pre=21 post=22 K=1 echo row added by trailing \r\n but 3 markers (18/19/20) appear twice in user-visible scrollback Case D — wrap-flag staleness: scrollback rows that wrapped at old cols still carry wrap=true after set_size; visible row in vt100 0.16 only clears wrap for self.rows. ``` ## Root causes (with citations) ### 1. vt100 0.16's `Grid::set_size` does not reflow scrollback File: `~/.cargo/registry/src/index.crates.io-.../vt100-0.16.2/src/grid.rs` - Lines 66-100 (`pub fn set_size`): - Lines 67-71: clears `wrap` flag on `self.rows` (visible only) when cols changes. Scrollback rows keep their stale wrap flags. - Lines 78-81: resizes visible rows to new cols; rebuilds `self.rows` Vec. Scrollback `VecDeque<Row>` is never iterated — each scrollback row keeps its pre-resize cell count. - Lines 73-88: clamps scroll_top/scroll_bottom. Reasonable. - Lines 94-99: clamps cursor + saved cursor. Reasonable. - `Screen::set_size` (vt100/src/screen.rs:88-92) just delegates to both `grid.set_size` and `alternate_grid.set_size`. No scrollback fix-up. ### 2. Repeated SIGWINCH events per typing session File: `src/tui/viz_viewer/render.rs` - Lines 3028-3043: `input_height` is computed from the wrapped visual line count of the chat input editor. Each time the user types past a wrap point, `input_height` goes up by 1; deleting back, it goes down. - Line 3050: `msg_area_height = area.height.saturating_sub(input_height)`. So the chat-PTY's available rows changes on every wrap event. - Line 3415: `pane.resize(msg_area.height, msg_area.width)` — called every render. When dimensions actually change, this forwards through `master.resize(...)` (TIOCSWINSZ → SIGWINCH on the child). This means a single multi-line draft of input typically fires several SIGWINCH events, not one. ### 3. `scrollback_hidden` is single-shot, not cumulative File: `src/tui/pty_pane.rs` - Lines 485-510 (`maybe_resolve_dedup`): after each resize, snapshots pre/post scrollback counts and stores `K = post - pre` as `self.scrollback_hidden`. - Line 544: `self.scrollback_hidden = 0; // stale dedup no longer valid for new resize`. EVERY new resize zeroes the counter. The K_1 echo rows from the previous resize are still in scrollback — they have just drifted away from the hot-end as the new resize pushes more rows in. - Lines 367-372 (scroll_up) and 384-388 (scroll_down): the dedup is applied during navigation by skipping offsets `1..=hidden` when jumping from live view. This hides only the most recent resize's echo rows. Older echoes are visible to the user. ### 4. Dedup is navigation-only, not data-level The duplicate rows still exist in the vt100 scrollback `VecDeque`. Any reader other than `scroll_up`/`scroll_down` sees them: - `screen().contents()` (used by smoke tests at `tests/smoke/scenarios/pty_resize_dedup_no_scrollback_echo.sh`) - The render path (`tui_term::widget::PseudoTerminal`) does not consult `scrollback_hidden`; if the user's offset lands inside the hidden zone via any path other than scroll_up/scroll_down, they see the duplicate. ## Mode bits / state preservation — NOT the issue `vt100::Screen` stores DEC modes (1, 6, 25, 47, 1049, 1000, 1002, 1003, 1005, 1006, 2004, etc.) as `screen.modes`, `mouse_protocol_mode`, `mouse_protocol_encoding` (vt100/src/screen.rs:55-65). These are top-level fields on `Screen`, NOT inside the grid. `set_size` only touches grid sizes (line 88-92), so all mode bits and the saved-cursor SGR attrs (`saved_attrs`) are preserved across resize. The original task hypothesis ("mode bits lost across SIGWINCH") is empirically wrong. The bug is not in mode-state save/restore. It is in the grid data model: scrollback rows carry stale geometry. ## Claude vs codex difference Captured codex startup bytes (timeout 8s, unauthenticated codex session) — file `/tmp/codex-tui-long.bin`: ``` [?2004h bracketed paste enable [>7u kitty keyboard set flags [?1004h focus reporting enable [6n CPR query [?u kitty query [c primary DA query ]10;? OSC 10 fg color query [?2026h / [?2026l synchronized output mode (batch render) [?25h show cursor [1;0r scroll region — full screen (no-op effectively) ``` NO `[?1049h` (alt-screen 1049). NO `\x1b7`/`\x1b8` (DEC save/restore). NO `[s`/`[u` (CSI save/restore). NO `[2J` (clear screen) within the captured period. Conclusion: **codex stays in the regular grid, like claude.** The original hypothesis ("codex uses alt-screen more aggressively") is wrong for the startup phase. Codex's SIGWINCH responses go straight into vt100 main grid scrollback, same as claude — same bug, same rate. The one codex-specific quirk: `[?2026h/l` (synchronized output) wraps each redraw, which means codex's reflow is *atomic* from vt100's perspective. This does not protect against scrollback corruption — it just means the duplicate rows arrive in a single batch instead of streamed. The user perceives identical symptoms. claude's chat-output is plain stream-json + line-oriented stdout, typically more tokens flowing through scrollback than codex's status-line redraw. claude's symptom may be more pronounced because the wrap-points hit frequently as long bot responses stream in. ## Fix proposal (concrete enough to implement) **Approach: true reflow on resize.** Replace the K-echo-counter heuristic with re-feeding the existing screen contents into a freshly sized `vt100::Parser`. This is the standard mitigation used by alacritty / wezterm / etc. for vt100-style terminals. Sketch (new `pty_pane::resize`): ```rust pub fn resize(&mut self, rows: u16, cols: u16) -> Result<()> { let rows = rows.max(10); let cols = cols.max(40); if rows == self.rows && cols == self.cols { return Ok(()); } // Snapshot the full screen+scrollback into a sequence of logical // lines. Walk scrollback rows oldest→newest, joining rows whose // `wrapped()` flag is true. Then walk visible rows the same way. let logical_lines = { let p = self.parser.lock().unwrap(); snapshot_logical_lines(&p) // returns Vec<String> }; // Build a fresh parser at the new dimensions and re-feed. let mut fresh = vt100::Parser::new(rows, cols, DEFAULT_SCROLLBACK_LINES); for line in &logical_lines { fresh.process(line.as_bytes()); fresh.process(b"\r\n"); } // Atomically swap. The reader thread holds the same Arc<Mutex>; it // will pick up the new parser on its next lock. *self.parser.lock().unwrap() = fresh; self.master.resize(PtySize { rows, cols, .. })?; self.rows = rows; self.cols = cols; // Delete: pending_dedup, scrollback_hidden — the reflow obsoletes // both. Keep send_key's set_scrollback(0) for live-view jump. Ok(()) } ``` `snapshot_logical_lines` walks scrollback offsets from max down to 1 and reads row 0 contents (the `collect_scrollback_only_naive` helper at pty_pane.rs:1672 already shows the pattern). Join consecutive rows when the leading row has `wrapped=true` to recover the original logical line. **Tradeoffs:** - SGR (color/attribute) loss: `cell.contents()` returns plain text. To preserve formatting, walk cells and emit SGR escapes. ~30 LOC extra. For the chat use case where the user mostly wants readability, plain text reflow is acceptable as a first cut. - Performance: scrollback is bounded at 10_000 lines. Re-feeding 10k short lines through `vte::Parser` runs in a few ms in release mode; fine for resize cadence (the typing-induced SIGWINCH bursts are at most ~1/keystroke). - Child's SIGWINCH reprint still hits the new clean parser. The reprint bytes paint over the new visible region; the trailing \r\n echo row STILL adds 1 row to scrollback, and that row IS a duplicate of content that was just reflowed in. To suppress this, keep a thin `scrollback_hidden` counter that ONLY tracks the immediately post-resize echo within RESIZE_DEDUP_WINDOW (the existing 120 ms approach), but have it accumulate across multiple resizes via a queue: `Vec<(at_offset_in_scrollback, k)>` instead of a single counter. On each navigation, sum the deltas at offsets the user is about to land on and skip them. **Drop the "spawn at correct size" workaround as well.** Once reflow is in place, `consume_pending_chat_pty_spawn` no longer needs to defer spawn until the area is known — though it's still good UX to spawn at the right size to avoid the first-resize cost. ## Architectural escape hatch If reflow proves too lossy (SGR / Unicode width / hyperlink OSC 8), escalate to the queued **replace-custom-pty** task: switch from `vt100::Parser` to `wezterm-term` or `alacritty_terminal`, both of which implement true scrollback reflow with formatting preservation. Cost is ~1-2 days of integration: API differs (e.g. `alacritty_terminal::Term<EventListener>` instead of `Arc<Mutex<vt100::Parser>>`), and the rendering bridge to `tui_term::widget::PseudoTerminal` would need adapting. Not justified yet — start with Option A. ## Validation summary - [x] Reproduction is deterministic — no SIGWINCH signal needed; the vt100 set_size + child reprint pattern reproduces in-process. - [x] Root cause identified — vt100/src/grid.rs:66-100 does not reflow scrollback (lines 67-71 wrap clearing only on self.rows; lines 78-81 resize only self.rows). - [x] Mode bits enumerated — they are NOT lost; modes/mouse/SGR are on Screen struct, untouched by set_size. - [x] claude vs codex characterized — both stay in regular grid, same bug. Codex differs only in batched render (DEC mode 2026). - [x] Fix proposal concrete — reflow-on-resize, ~50-100 LOC, obsoletes scrollback_hidden patch. - [x] No source modifications — diagnose only.
• 2026-04-29T14:08:52.823754507+00:00 Validated: diagnostic posted with file:line citations, deterministic in-process repro at /tmp/sigwinch-repro/, mode-bits hypothesis empirically refuted, codex byte capture at /tmp/codex-tui-long.bin shows no alt-screen, fix proposal concrete (re-feed reflow). No source modifications (git status confirms).
• 2026-04-29T14:08:54.277305958+00:00 Task pending eval (agent reported done; awaiting `.evaluate-*` to score)
• 2026-04-29T14:10:37.829639183+00:00 PendingEval → Done (evaluator passed; downstream unblocks)