fix(codex): smaller first-frame write budget + scroll-up grace for codex#117
Conversation
Two render-polish fixes for codex sessions in the terminal write pipeline: - flushPendingWrites uses a 32KB first-frame budget for codex (vs 64KB for other modes). Codex's TUI emits dense synchronized redraws during thinking/high-effort phases; a smaller first frame keeps per-frame xterm/WebGL stalls short and avoids multi-second main-thread blocks. - Sticky-scroll now honours a short grace window after a manual scroll-up gesture (USER_SCROLL_STICKY_SUPPRESS_MS = 1500ms). High-frequency codex "Working (Ns)" status ticks were snapping the viewport back to the bottom while the user tried to read earlier output. The wheel/touch scroll handlers record the gesture (_noteTerminalUserScroll); flushPendingWrites suppresses the auto-scroll-to-bottom and restores the preserved viewport via scrollToLine while the grace window is active. Adds test/terminal-flush-budget.test.ts (vm-sandbox harness over terminal-ui.js): codex vs non-codex first-frame budget, buffer-load ownership, and the scroll-up suppression / viewport restore. Co-Authored-By: Saqeb Akhter <saqeb.akhter@gmail.com>
Ark0N
left a comment
There was a problem hiding this comment.
Reviewed — no blockers, merging as-is. Verified locally on a merge with current master (post-#115/#116): tsc, eslint, prettier --check, frontend-syntax clean; targeted tests + full test:ci green (2822 passed).
Two notes for the record, neither gating:
-
The scroll-up grace is wired globally, not codex-only as the description says.
_noteTerminalUserScrollhooks the universal wheel/touch handlers and_hasRecentUserScrollUp()gates sticky-scroll for every mode — only the 32KB first-frame budget is actually codex-gated. I traced the behavior and kept it: the grace closes a real race for all modes (_wasAtBottomBeforeWriteis captured at enqueue time, so a scroll-up between enqueue and the rAF flush used to get yanked back; Ink's high-frequency redraws hit the same window). Outside the 1.5s window behavior is unchanged, since a scrolled-up viewport fails the at-bottom capture anyway. -
The viewport restore can no-op against xterm's async parse.
terminal.write()returns before the bytes are parsed, so theviewportY !== preserveViewportYcheck usually sees the pre-parse state and a post-parse viewport move would be missed. Harmless — the sticky-scroll suppression is the primary guard and the restore is belt-and-braces — just don't be surprised if the restore branch rarely fires in real xterm (it fires reliably in the vm-harness tests because the mock write is synchronous).
Problem
Two render annoyances in Codex sessions:
• Working (Ns)status redraw. Because the user was "at the bottom" when the redraw arrived, sticky-scroll yanked the viewport back to the bottom on every tick — so you couldn't scroll up to read earlier output during a long run.Fix
Both changes live in
terminal-ui.js'sflushPendingWritesand the scroll handlers, and only affect codex (other modes keep their existing behavior):MAX_FRAME_BYTESis32768formode === 'codex',65536otherwise. A smaller first frame keeps per-frame xterm/WebGL work short; the remainder defers to the next frame as before._noteTerminalUserScroll); forUSER_SCROLL_STICKY_SUPPRESS_MS(1500ms) afterward,_hasRecentUserScrollUp()is true, soflushPendingWritesskips the auto-scrollToBottomand restores the user's viewport viascrollToLine. A downward scroll back to the bottom clears the suppression immediately.Tests
Adds
test/terminal-flush-budget.test.ts— avm-sandbox harness overterminal-ui.js(no real DOM), covering:chunkedTerminalWritewaits for xterm's write callback before finishing a buffer load; stale buffer-load owners can't finish a newer loadnpm run test:cipasses locally (2727 passed, 0 failures);tsc --noEmit,eslint,prettier --check, and the frontend-syntax check are all clean.