mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-02 13:39:41 +02:00
fix(codex): smaller first-frame write budget + scroll-up grace for codex
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>
This commit is contained in:
co-authored by
Saqeb Akhter
parent
055f18fb66
commit
fe821fb679
@@ -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
|
||||
if (this._wasAtBottomBeforeWrite) {
|
||||
// Sticky scroll: if user was at bottom, keep them there after new output.
|
||||
// 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();
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user