Merge pull request #117 from aakhter/pr/cod-86-codex-frontend

fix(codex): smaller first-frame write budget + scroll-up grace for codex

src/web/public/terminal-ui.js

@@ -15,6 +15,10 @@
 (function (global) {
   const TERMINAL_QUERY_RESPONSE_PATTERN = /^\x1b\[[\?>=]?[\d;]*[cnR]$/;
   const TERMINAL_OSC_RESPONSE_PATTERN = /^\x1b\][\d;]*[^\x07\x1b]*(?:\x07|\x1b\\)$/;
+  // Grace window after a manual scroll-up gesture during which sticky-scroll is
+  // suppressed, so high-frequency Codex status redraws don't snap the viewport
+  // back to the bottom while the user is inspecting earlier output.
+  const USER_SCROLL_STICKY_SUPPRESS_MS = 1500;
 
   function isTerminalQueryResponse(data) {
     return TERMINAL_QUERY_RESPONSE_PATTERN.test(data) || TERMINAL_OSC_RESPONSE_PATTERN.test(data);
@@ -27,6 +31,7 @@
   global.CodemanTerminalInput = {
     isTerminalQueryResponse,
     shouldSuppressTerminalQueryResponse,
+    USER_SCROLL_STICKY_SUPPRESS_MS,
   };
 })(window);
 
@@ -302,6 +307,7 @@ Object.assign(CodemanApp.prototype, {
       (ev) => {
         ev.preventDefault();
         const lines = Math.round(ev.deltaY / 25) || (ev.deltaY > 0 ? 1 : -1);
+        this._noteTerminalUserScroll(lines);
         this.terminal.scrollLines(lines);
       },
       { passive: false }
@@ -376,6 +382,7 @@ Object.assign(CodemanApp.prototype, {
             const ch = cellHeight();
             const lines = Math.trunc(pixelAccum / ch);
             if (lines !== 0) {
+              this._noteTerminalUserScroll(lines);
               this.terminal.scrollLines(lines);
               pixelAccum -= lines * ch;
             }
@@ -428,6 +435,7 @@ Object.assign(CodemanApp.prototype, {
     this._chunkedWriteGen = 0;
     this._bufferLoadSeq = 0;
     this._bufferLoadOwner = null;
+    this._lastUserScrollUpAt = null;
 
     // Handle resize with throttling for performance
     this._resizeTimeout = null;
@@ -1364,6 +1372,22 @@ Object.assign(CodemanApp.prototype, {
     return buffer.viewportY >= buffer.baseY - 2;
   },
 
+  // Record manual scroll gestures so sticky-scroll can give an upward scroll a
+  // short grace window (see _hasRecentUserScrollUp). A downward scroll that
+  // lands back at the bottom clears the suppression immediately.
+  _noteTerminalUserScroll(lines) {
+    if (lines < 0) {
+      this._lastUserScrollUpAt = performance.now();
+    } else if (this.isTerminalAtBottom()) {
+      this._lastUserScrollUpAt = null;
+    }
+  },
+
+  _hasRecentUserScrollUp() {
+    if (typeof this._lastUserScrollUpAt !== 'number') return false;
+    return performance.now() - this._lastUserScrollUpAt < window.CodemanTerminalInput.USER_SCROLL_STICKY_SUPPRESS_MS;
+  },
+
   batchTerminalWrite(data) {
     // If a buffer load (chunkedTerminalWrite) is in progress, queue live events
     // to prevent interleaving historical buffer data with live SSE data.
@@ -1615,8 +1639,16 @@ Object.assign(CodemanApp.prototype, {
 
     // Per-frame byte budget to prevent main thread blocking.
     // Large writes (141KB+) can freeze Chrome for 2+ minutes.
-    const MAX_FRAME_BYTES = 65536; // 64KB budget per frame
+    // Codex's TUI emits dense synchronized redraws during thinking/high-effort
+    // phases, so it gets a smaller first frame to keep per-frame xterm/WebGL
+    // stalls short; other modes keep the larger 64KB budget.
+    const activeSession = this.activeSessionId && this.sessions ? this.sessions.get(this.activeSessionId) : null;
+    const MAX_FRAME_BYTES = activeSession?.mode === 'codex' ? 32768 : 65536;
     let deferred = false;
+    // If the user recently scrolled up, remember the viewport so we can restore
+    // it after the write — Codex status redraws would otherwise jump it.
+    const preserveViewportY =
+      this._hasRecentUserScrollUp() && this.terminal.buffer?.active ? this.terminal.buffer.active.viewportY : null;
 
     if (_joinedLen <= MAX_FRAME_BYTES) {
       this.terminal.write(joined);
@@ -1633,6 +1665,13 @@ Object.assign(CodemanApp.prototype, {
         });
       }
     }
+    if (
+      preserveViewportY !== null &&
+      this.terminal.buffer?.active?.viewportY !== preserveViewportY &&
+      typeof this.terminal.scrollToLine === 'function'
+    ) {
+      this.terminal.scrollToLine(preserveViewportY);
+    }
     const bytesThisFrame = deferred ? MAX_FRAME_BYTES : _joinedLen;
     const _dt = performance.now() - _t0;
     if (_dt > 100 || deferred)
@@ -1640,8 +1679,11 @@ Object.assign(CodemanApp.prototype, {
         `[CRASH-DIAG] flushPendingWrites: ${_dt.toFixed(0)}ms, ${(bytesThisFrame / 1024).toFixed(0)}KB written${deferred ? ', rest deferred' : ''} (total ${(_joinedLen / 1024).toFixed(0)}KB)`
       );
 
-    // Sticky scroll: if user was at bottom, keep them there after new output
+    // Sticky scroll: if user was at bottom, keep them there after new output.
-    if (this._wasAtBottomBeforeWrite) {
+    // Give manual scroll-up gestures a short grace window so high-frequency
+    // Codex status ticks do not snap the viewport back while the user is
+    // trying to inspect earlier output.
+    if (this._wasAtBottomBeforeWrite && !this._hasRecentUserScrollUp()) {
       this.terminal.scrollToBottom();
     }
 

test/terminal-flush-budget.test.ts

@@ -0,0 +1,147 @@
+import { readFileSync } from 'node:fs';
+import { resolve } from 'node:path';
+import vm from 'node:vm';
+import { describe, expect, it, vi } from 'vitest';
+
+function loadTerminalUiHarness(mode: string) {
+  const CodemanApp = function CodemanApp(this: any) {};
+  const context = vm.createContext({
+    window: {},
+    CodemanApp,
+    console: { warn: vi.fn(), log: vi.fn() },
+    _crashDiag: { log: vi.fn() },
+    performance: { now: () => 0 },
+    requestAnimationFrame: (_fn: () => void) => 1,
+    setTimeout: (_fn: () => void) => 1,
+    Blob: function Blob() {},
+    URL: {
+      createObjectURL: () => 'blob:yield',
+      revokeObjectURL: () => {},
+    },
+    Worker: function Worker(this: any) {
+      this.postMessage = () => {};
+    },
+    DEC_SYNC_STRIP_RE: /\x1b\[\?2026[hl]/g,
+    TERMINAL_CHUNK_SIZE: 32 * 1024,
+  });
+
+  const code = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8');
+  vm.runInContext(code, context, { filename: 'terminal-ui.js' });
+
+  const app = new (CodemanApp as any)();
+  const writes: string[] = [];
+  app.activeSessionId = 'session-1';
+  app.sessions = new Map([['session-1', { mode }]]);
+  app.pendingWrites = [];
+  app.writeFrameScheduled = false;
+  app._wasAtBottomBeforeWrite = false;
+  app._workerYield = () => {};
+  app._chunkedWriteGen = 0;
+  app.terminal = {
+    write: (data: string) => writes.push(data),
+    scrollToBottom: () => {},
+    scrollToLine: () => {},
+  };
+
+  return { app, writes };
+}
+
+describe('terminal flush budget', () => {
+  it('uses a smaller first-frame write budget for Codex output to reduce renderer stalls', () => {
+    const { app, writes } = loadTerminalUiHarness('codex');
+    app.pendingWrites.push('x'.repeat(96 * 1024));
+
+    app.flushPendingWrites();
+
+    expect(writes).toHaveLength(1);
+    expect(writes[0]).toHaveLength(32 * 1024);
+    expect(app.pendingWrites.join('')).toHaveLength(64 * 1024);
+  });
+
+  it('keeps the larger first-frame write budget for non-Codex terminal output', () => {
+    const { app, writes } = loadTerminalUiHarness('claude');
+    app.pendingWrites.push('x'.repeat(96 * 1024));
+
+    app.flushPendingWrites();
+
+    expect(writes).toHaveLength(1);
+    expect(writes[0]).toHaveLength(64 * 1024);
+    expect(app.pendingWrites.join('')).toHaveLength(32 * 1024);
+  });
+
+  it('waits for xterm to process small buffer replays before completing buffer load', async () => {
+    const { app, writes } = loadTerminalUiHarness('codex');
+    let writeDone: (() => void) | undefined;
+    let resolved = false;
+    const finishBufferLoad = vi.fn();
+    app._finishBufferLoad = finishBufferLoad;
+    app.terminal.write = (data: string, callback?: () => void) => {
+      writes.push(data);
+      writeDone = callback;
+    };
+
+    const promise = app.chunkedTerminalWrite('fresh tmux pane frame').then(() => {
+      resolved = true;
+    });
+
+    await Promise.resolve();
+
+    expect(writes).toEqual(['fresh tmux pane frame']);
+    expect(writeDone).toBeTypeOf('function');
+    expect(resolved).toBe(false);
+    expect(finishBufferLoad).not.toHaveBeenCalled();
+
+    writeDone?.();
+    await promise;
+
+    expect(resolved).toBe(true);
+    expect(finishBufferLoad).toHaveBeenCalledOnce();
+  });
+
+  it('keeps stale buffer load owners from finishing a newer load', () => {
+    const { app } = loadTerminalUiHarness('codex');
+
+    app._beginBufferLoad('select-1');
+    app._beginBufferLoad('select-2');
+
+    expect(app._finishBufferLoad('select-1')).toBe(false);
+    expect(app._isLoadingBuffer).toBe(true);
+    expect(app._bufferLoadOwner).toBe('select-2');
+
+    expect(app._finishBufferLoad('select-2')).toBe(true);
+    expect(app._isLoadingBuffer).toBe(false);
+    expect(app._bufferLoadOwner).toBe(null);
+  });
+
+  it('does not snap back to bottom during Codex Working redraws right after the user scrolls up', () => {
+    const { app } = loadTerminalUiHarness('codex');
+    const scrollToBottom = vi.fn();
+    app.terminal.scrollToBottom = scrollToBottom;
+    app._wasAtBottomBeforeWrite = true;
+    app._lastUserScrollUpAt = 0;
+    app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
+
+    app.flushPendingWrites();
+
+    expect(scrollToBottom).not.toHaveBeenCalled();
+  });
+
+  it('restores the user scroll position when Codex Working redraws move the viewport', () => {
+    const { app } = loadTerminalUiHarness('codex');
+    const buffer = { viewportY: 40, baseY: 100 };
+    app.terminal.buffer = { active: buffer };
+    app.terminal.write = vi.fn(() => {
+      buffer.viewportY = buffer.baseY;
+    });
+    app.terminal.scrollToLine = vi.fn((line: number) => {
+      buffer.viewportY = line;
+    });
+    app._wasAtBottomBeforeWrite = true;
+    app._lastUserScrollUpAt = 0;
+    app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
+
+    app.flushPendingWrites();
+
+    expect(buffer.viewportY).toBe(40);
+  });
+});