mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-15 19:54:54 +00:00
fix(cli): stop resize repaint from causing scroll storm (#8009)
* fix(cli): stop resize repaint from causing scroll storm (#8004) Remove the useResizeSettleRepaint -> refreshStatic wiring that wrote clearTerminal (destroying scrollback) and remounted <Static> on every settled resize, re-emitting all conversation history in 50-item chunks. Ghostty's panel-toggle animation exceeds the 200ms debounce, triggering multiple settle-repaint cycles per toggle -- visible as continuous scrolling/flickering. Ink's dynamic region already re-renders on width changes via useTerminalSize; modern terminals handle scrollback reflow natively. The full remount is no longer necessary. The now-unused hook and its test are removed (no remaining callers). * test(cli): guard resize no-repaint contract against settle-time regression (#8004) * test(cli): make resize settle regression test non-vacuous (#8004) The previous test used rerender() which remounts the tree via ink's ErrorBoundary (measureElement returns undefined → layout effect throws → tree unmounted), so the settle debounce never fired and the test passed regardless. Rewrite to keep the tree alive (measureElement mock returns a real value) and deliver width changes to the same mounted instance via a listener pattern. Mutation-verified: fails when the removed useResizeSettleRepaint hook is restored. * chore(cli): remove dead useResizeSettleRepaint from eslint legacy filenames (#8004) --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
This commit is contained in:
ChiGao
GitHub
parent
eb5798b7f5
commit
566d2c1279
5 changed files with 69 additions and 164 deletions
|
|
@ -563,7 +563,6 @@ export const legacyFilenames = [
|
|||
'toolResultDisplayCompaction',
|
||||
'usageHistoryService',
|
||||
'useMcpApproval',
|
||||
'useResizeSettleRepaint',
|
||||
'useStatsDialog',
|
||||
'useTeamInProcess',
|
||||
'userMemory',
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ import {
|
|||
type Mock,
|
||||
} from 'vitest';
|
||||
import { render, cleanup } from 'ink-testing-library';
|
||||
import { useContext, useState, act } from 'react';
|
||||
import { useContext, useState, useReducer, useEffect, act } from 'react';
|
||||
import {
|
||||
AppContainer,
|
||||
dedupeNewestFirst,
|
||||
|
|
@ -1092,13 +1092,10 @@ describe('AppContainer State Management', () => {
|
|||
expect(capturedUIState.useTerminalBuffer).toBe(true);
|
||||
});
|
||||
|
||||
// #4891 changed the resize contract: width changes now trigger ONE full
|
||||
// clearTerminal after RESIZE_REPAINT_SETTLE_MS (trailing-edge debounce),
|
||||
// instead of never (#3967) or per-event (pre-#3967). This test pins the
|
||||
// synchronous half: no immediate clear during the burst. The settle-time
|
||||
// half is not observable here — ink-testing-library's rerender does not
|
||||
// flush update-time passive effects — and is covered by
|
||||
// useResizeSettleRepaint.test.ts.
|
||||
// Resize no longer triggers a clearTerminal or history remount (#8004).
|
||||
// The old settle → refreshStatic path caused a scroll storm; the dynamic
|
||||
// region now re-renders via useTerminalSize alone. This test pins that
|
||||
// no synchronous clear fires during a width change.
|
||||
it('does not clear the terminal synchronously on width change', () => {
|
||||
vi.spyOn(mockConfig, 'initialize').mockResolvedValue(undefined);
|
||||
mockedUseTerminalSize.mockReturnValue({ columns: 80, rows: 24 });
|
||||
|
|
@ -1127,6 +1124,64 @@ describe('AppContainer State Management', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('does not repaint static history after a resize settles (#8004)', () => {
|
||||
vi.useFakeTimers();
|
||||
vi.spyOn(mockConfig, 'initialize').mockResolvedValue(undefined);
|
||||
// measureElement must return a real measurement; a bare vi.fn() returns
|
||||
// undefined, the controlsHeight layout effect throws on .height, and
|
||||
// ink's ErrorBoundary silently unmounts the tree — making every
|
||||
// post-mount assertion vacuous.
|
||||
(measureElement as Mock).mockReturnValue({ width: 80, height: 2 });
|
||||
|
||||
// Deliver width changes to the SAME mounted instance. rerender() from
|
||||
// ink-testing-library remounts the tree (ErrorBoundary issue above),
|
||||
// re-seeding useRef(terminalWidth) so a settle debounce never fires.
|
||||
let columns = 80;
|
||||
const resizeListeners = new Set<() => void>();
|
||||
mockedUseTerminalSize.mockImplementation(() => {
|
||||
const [, force] = useReducer((x: number) => x + 1, 0);
|
||||
useEffect(() => {
|
||||
resizeListeners.add(force);
|
||||
return () => {
|
||||
resizeListeners.delete(force);
|
||||
};
|
||||
}, []);
|
||||
return { columns, rows: 24 };
|
||||
});
|
||||
|
||||
render(
|
||||
<AppContainer
|
||||
config={mockConfig}
|
||||
settings={mockSettings}
|
||||
version="1.0.0"
|
||||
initializationResult={mockInitResult}
|
||||
/>,
|
||||
);
|
||||
|
||||
// Liveness control: fails if the ErrorBoundary unmounted the tree.
|
||||
expect(resizeListeners.size).toBeGreaterThan(0);
|
||||
const remountKeyBefore = capturedUIState.historyRemountKey;
|
||||
mockStdout.write.mockClear();
|
||||
|
||||
act(() => {
|
||||
columns = 100;
|
||||
for (const notify of resizeListeners) notify();
|
||||
});
|
||||
|
||||
// Advance well past the old RESIZE_REPAINT_SETTLE_MS (200ms) debounce.
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(500);
|
||||
});
|
||||
|
||||
expect(mockStdout.write).not.toHaveBeenCalledWith(
|
||||
ansiEscapes.clearTerminal,
|
||||
);
|
||||
expect(capturedUIState.historyRemountKey).toBe(remountKeyBefore);
|
||||
|
||||
vi.useRealTimers();
|
||||
(measureElement as Mock).mockReturnValue(undefined);
|
||||
});
|
||||
|
||||
it('handleClearScreen avoids a second clearTerminal write', () => {
|
||||
const clearSpy = vi.spyOn(console, 'clear').mockImplementation(() => {});
|
||||
|
||||
|
|
|
|||
|
|
@ -119,7 +119,6 @@ const MCP_BATCH_FLUSH_MS = 16;
|
|||
const STARTUP_PROFILE_FINALIZE_CAP_MS = 35_000;
|
||||
import { useHistory } from './hooks/useHistoryManager.js';
|
||||
import { useMemoryMonitor } from './hooks/useMemoryMonitor.js';
|
||||
import { useResizeSettleRepaint } from './hooks/useResizeSettleRepaint.js';
|
||||
import { useWakeRepaint } from './hooks/use-wake-repaint.js';
|
||||
import { useThemeCommand } from './hooks/useThemeCommand.js';
|
||||
import { useFeedbackDialog } from './hooks/useFeedbackDialog.js';
|
||||
|
|
@ -3245,8 +3244,12 @@ export const AppContainer = (props: AppContainerProps) => {
|
|||
}
|
||||
}, [terminalWidth, availableTerminalHeight, activePtyId]);
|
||||
|
||||
// Repaint static history on the trailing edge of a resize burst (#4891).
|
||||
useResizeSettleRepaint(terminalWidth, refreshStatic);
|
||||
// Resize no longer repaints static history (#8004). The old settle →
|
||||
// refreshStatic path wrote clearTerminal (destroying scrollback) and remounted
|
||||
// <Static>, re-emitting all history in 50-item chunks — a scroll storm when
|
||||
// the terminal's resize animation exceeded the debounce window (e.g. Ghostty
|
||||
// panel toggle). Ink's dynamic region already re-renders on width changes via
|
||||
// useTerminalSize; modern terminals reflow scrollback natively.
|
||||
|
||||
// Repaint after the process resumes from OS sleep / suspend (lid close,
|
||||
// display sleep, Ctrl+Z → fg). The terminal's screen buffer is stale but
|
||||
|
|
|
|||
|
|
@ -1,102 +0,0 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2025 Qwen
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { renderHook, act } from '@testing-library/react';
|
||||
import {
|
||||
RESIZE_REPAINT_SETTLE_MS,
|
||||
useResizeSettleRepaint,
|
||||
} from './useResizeSettleRepaint.js';
|
||||
|
||||
describe('useResizeSettleRepaint (#4891)', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
const setup = (initialWidth: number) => {
|
||||
const refreshStatic = vi.fn();
|
||||
const view = renderHook(
|
||||
({ width }: { width: number }) =>
|
||||
useResizeSettleRepaint(width, refreshStatic),
|
||||
{ initialProps: { width: initialWidth } },
|
||||
);
|
||||
return { refreshStatic, view };
|
||||
};
|
||||
|
||||
it('does not repaint on first mount', () => {
|
||||
const { refreshStatic } = setup(80);
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(RESIZE_REPAINT_SETTLE_MS + 50);
|
||||
});
|
||||
|
||||
expect(refreshStatic).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('coalesces a burst of width changes into one repaint after settle', () => {
|
||||
const { refreshStatic, view } = setup(80);
|
||||
|
||||
// Three rapid width changes, each well within the settle window.
|
||||
act(() => view.rerender({ width: 90 }));
|
||||
act(() => vi.advanceTimersByTime(100));
|
||||
act(() => view.rerender({ width: 100 }));
|
||||
act(() => vi.advanceTimersByTime(100));
|
||||
act(() => view.rerender({ width: 110 }));
|
||||
|
||||
// Burst not yet settled (measured from the last change): nothing fired.
|
||||
act(() => vi.advanceTimersByTime(RESIZE_REPAINT_SETTLE_MS - 1));
|
||||
expect(refreshStatic).not.toHaveBeenCalled();
|
||||
|
||||
// Settle window elapses → exactly one repaint for the whole burst.
|
||||
act(() => vi.advanceTimersByTime(1));
|
||||
expect(refreshStatic).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does not repaint when a drag returns to the original width', () => {
|
||||
const { refreshStatic, view } = setup(80);
|
||||
|
||||
act(() => view.rerender({ width: 100 }));
|
||||
act(() => vi.advanceTimersByTime(RESIZE_REPAINT_SETTLE_MS - 1)); // pending
|
||||
act(() => view.rerender({ width: 80 })); // back to start before it fires
|
||||
|
||||
act(() => vi.advanceTimersByTime(RESIZE_REPAINT_SETTLE_MS + 50));
|
||||
expect(refreshStatic).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('repaints exactly once after a single settled width change', () => {
|
||||
const { refreshStatic, view } = setup(80);
|
||||
|
||||
act(() => view.rerender({ width: 100 }));
|
||||
act(() => vi.advanceTimersByTime(RESIZE_REPAINT_SETTLE_MS));
|
||||
|
||||
expect(refreshStatic).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('schedules nothing when a re-render leaves the width unchanged', () => {
|
||||
// A height-only resize re-renders with the same width: no repaint.
|
||||
const { refreshStatic, view } = setup(80);
|
||||
|
||||
act(() => view.rerender({ width: 80 }));
|
||||
act(() => vi.advanceTimersByTime(RESIZE_REPAINT_SETTLE_MS + 50));
|
||||
|
||||
expect(refreshStatic).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('cancels a pending repaint when unmounted mid-settle', () => {
|
||||
const { refreshStatic, view } = setup(80);
|
||||
|
||||
act(() => view.rerender({ width: 100 }));
|
||||
act(() => vi.advanceTimersByTime(RESIZE_REPAINT_SETTLE_MS - 1)); // pending
|
||||
view.unmount();
|
||||
|
||||
act(() => vi.advanceTimersByTime(RESIZE_REPAINT_SETTLE_MS + 50));
|
||||
expect(refreshStatic).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -1,50 +0,0 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2025 Qwen
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { useEffect, useRef } from 'react';
|
||||
|
||||
// Trailing-edge debounce for resize-triggered static repaints (#4891).
|
||||
export const RESIZE_REPAINT_SETTLE_MS = 200;
|
||||
|
||||
/**
|
||||
* Repaint the static history once the terminal width settles (#4891).
|
||||
*
|
||||
* A window drag fires dozens of `resize` events. Per-event repainting restarted
|
||||
* the progressive <Static> replay (#3899) mid-flight, and the viewport-only
|
||||
* cursorTo+eraseDown erase introduced by #3967 (4bab7a1a) cannot reach output
|
||||
* that has already scrolled into the scrollback, so each event stranded a
|
||||
* fragment at that instant's width. Debouncing to the trailing edge and issuing
|
||||
* one full `refreshStatic` (clearTerminal incl. ESC[3J + remount) wipes the
|
||||
* fragments and re-emits the history once at the final width. The cleanup
|
||||
* cancels the pending timer on the next width change or unmount, so a drag
|
||||
* returning to the start width is a no-op.
|
||||
*
|
||||
* Trade-off: the full-screen flash #3967 removed returns, but at most once per
|
||||
* resize gesture; the settle-time ESC[3J clears pre-session scrollback — the
|
||||
* same as the pre-#3967 per-event behavior and today's /clear.
|
||||
*
|
||||
* `refreshStatic` must be referentially stable (e.g. `useCallback`) so an
|
||||
* unrelated re-render does not cancel an in-flight settle.
|
||||
*/
|
||||
export function useResizeSettleRepaint(
|
||||
terminalWidth: number,
|
||||
refreshStatic: () => void,
|
||||
): void {
|
||||
// Width at the last settled repaint; starts at mount width (first mount is a
|
||||
// no-op) and only advances when a repaint actually fires.
|
||||
const settledTerminalWidthRef = useRef(terminalWidth);
|
||||
|
||||
useEffect(() => {
|
||||
if (settledTerminalWidthRef.current === terminalWidth) {
|
||||
return;
|
||||
}
|
||||
const timer = setTimeout(() => {
|
||||
settledTerminalWidthRef.current = terminalWidth;
|
||||
refreshStatic();
|
||||
}, RESIZE_REPAINT_SETTLE_MS);
|
||||
return () => clearTimeout(timer);
|
||||
}, [terminalWidth, refreshStatic]);
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue