mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
fix(cli): bound the live streaming-table pending height (fix scroll-to-top lock, stall-then-dump, header flash) (#6421)
* fix(cli): charge a streaming table's wrapped height so it doesn't jump The rendered-height estimator charged a table `2 * dataRows + 5`, assuming one line per data row. When cells wrap (a wide table — many columns, long content — on a bounded content width), each row renders taller, so the live frame briefly exceeds the viewport and ink repaints from the top (a jump to the top; #6170's clip/commit recover it, so it does not lock, but the jump is visible). Charge each row (header + data) by its wrapped height instead: approximate the column width as an equal share of the content area (TableRenderer shrinks columns proportionally to fit; an equal share never gives a wide cell more room than TableRenderer would, so it is a safe upper bound) and sum the tallest wrapped cell per row plus the inter-row separators and chrome. For a table that fits, every row is one line and the formula reduces exactly to the previous `2 * dataRows + 5`. Only the height estimate changes (shared by the render-side clip and the incremental scrollback commit); no rendering behaviour changes. 28 estimator tests pass; MarkdownDisplay clip tests unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): charge a wide-terminal table's vertical fallback height The pending-height estimator mirrored only TableRenderer's WIDTH-based vertical-fallback trigger, not the maxRowLines one. On a wide terminal a multi-column table whose cells wrap past MAX_ROW_LINES is laid out vertically (label: value — much taller), but the estimator charged the shorter horizontal height, so the live frame overflowed and Ink fell into its from-top full-redraw path (the scroll-to-top lock). Model both triggers: compute the tallest wrapped cell (maxRowLines) in the same pass as the horizontal height, and when it exceeds MAX_ROW_LINES charge the vertical height instead. The vertical estimate now also wraps each label:value at contentWidth so a long value that wraps is not under-counted. maxRowLines uses an equal column share (never wider than TableRenderer's column), so it is an upper bound and a real vertical table is never missed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): cap the live pending region at the viewport height (non-VP) The estimator's source-line slice is the primary bound on the live (non-<Static>) pending frame, but it is disabled whenever availableTerminalHeight is undefined — which is exactly what happens when constrainHeight is off (ctrl-s "show more lines"). A tall pending item, e.g. a long vertical-fallback table, then renders past the viewport; Ink cannot update incrementally, clears the terminal and redraws from the top on every repaint — the scroll-to-top lock. Wrap the non-VP pending region in an Ink maxHeight={availableTerminalHeight} overflow="hidden" box as a hard backstop. availableTerminalHeight already excludes the footer/controls, so the live frame can never exceed the viewport and Ink never trips clearTerminal. While constrained the estimator keeps content well under this, so the clamp is inert and only engages on residual overflow. ShowMoreLines stays outside the clamp (it renders only while constrained, so the clamp is inert then, and must not be clipped). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): anchor the estimator's vertical trigger to the first row Mirror TableRenderer's change to decide the horizontal-vs-vertical format from the header + first data row only (not every row), so the estimator and the renderer still agree on which format a streaming table uses. Row heights above continue to sum every row; only the maxRowLines trigger is anchored. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): commit a completed table that alone fills the pending budget The rendered-height-aware incremental commit stalled when a single block (a long-text table modelled tall / vertical) charged more than the commit budget on its own. fitPendingSlice returns kept = the block's trailing blank line, so the safe split boundary sits exactly at keptLines, but the boundary search started at keptLines - 1 and missed it. The table has no internal blank line and the blank before it was already committed, so the search found nothing and broke the loop. Every later block then appended past keptLines, so the search window never again contained a blank line — nothing committed until the stream finalized and dumped all remaining tables at once (the "stream a few tables, pause, then dump the rest" bug). Start the boundary search at keptLines so a completed over-tall block's trailing blank is found and the block commits to <Static>. Committing an over-tall completed block is fine — only the live pending frame must stay within the viewport. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): don't flash the header while its separator streams in The forming-table hold-back released the header the moment its separator line ended with `|` at a column count different from the header — meant to let a genuinely mismatched separator render as plain text. But a separator is typed one group at a time and momentarily ends with `|` at every intermediate count (`| --- | --- |` on the way to seven columns), so the header flashed as raw `| … |` text on every closed-group frame while the separator streamed in — a visible strobe for wide (7-column) tables. A streaming separator only ever gains columns, so treat a mismatch as final only when it can no longer become valid: it overshot the header's column count, or a further line has already committed it (it is not the trailing line). Also hold the header while the separator is still a bare-pipe prefix (`|`, `| `) before its first dash. The only remaining flash is the unavoidable one-cell header window (`| Foo |`), indistinguishable from a single-pipe line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: add a terminal-capture ratchet for the streaming-table scroll-to-top lock Drives ten wide 7-column tables with ~200-char wrapping cells through the real TUI via a chunked fake OpenAI server and counts the full-screen clears (`\x1b[2J\x1b[3J\x1b[H`) the app emits while they stream. Each such clear resets the terminal scroll position, so it is exactly the "jump to top" a user hits when scrolling up mid-stream. With the pending-height estimator fix the count is 0; without it the under-charged frame overflows and the count is ~300. Ratchet fails if it exceeds 20. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): tighten the streaming-table vertical height estimate (review) Two accuracy fixes to fitPendingSlice from PR review, both reducing residual under-charge of the vertical fallback: - Charge each vertical data cell as its rendered `label: value` line (parsing the header labels once), not the value alone — TableRenderer's renderVerticalFormat prefixes the header label, so a long label pushed the wrapped line count higher than the estimator accounted for. - Only model the vertical layout once a data row exists (`dataRows > 0`). With just a header + separator, TableRenderer keeps the horizontal header box, so the estimator no longer charges the shorter 2-row vertical stub for that transient state on a narrow terminal. Adds a test where the `label: value` line itself wraps (covering the wrapped, label-inclusive formula) and a zero-data-row narrow-terminal test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): correct the clamp margin and share MAX_ROW_LINES (review round 2) - Charge tableClampRows + 2 when the clamp engages: TableRenderer wraps the height-clamped <Text> in <Box marginY={1}>, so a clamped table renders two margin rows beyond maxHeight that the estimator was dropping. - Make TABLE_MAX_ROW_LINES the single source of truth (pending-rendered-height) and import it into TableRenderer as MAX_ROW_LINES, so the renderer and the estimator can never disagree on the wrap-to-vertical threshold. Direction is util→renderer so the pure height module stays free of the React/ink graph. - Correct the perColWidth comment: an equal column share is exact for uniform columns but can under-count a heterogeneous table (the renderer shrinks a narrow column below the share); the MainContent maxHeight backstop is the hard cap for that residual case. - Add a horizontal-layout test whose cells wrap within MAX_ROW_LINES, covering the per-row wrapped contentRows sum (not just the old flat 2*dataRows). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cli): estimator cleanups and stronger tests (review round 3) - Charge the clamped-table margin: covered by a new test asserting a clamped table costs tableClampRows + 2 (the marginY the earlier fix added). - Strengthen the "no stall-then-dump" regression: require all three tables to have committed (>= 3 items, each table marker present), so a partial stall no longer passes. - Reuse headerCells instead of re-parsing the header row inside the loop, drop the redundant .trim() (splitMarkdownTableRow already trims), and start the data-row loop at i + 2. - Refresh the stale TABLE_CHROME_ROWS JSDoc (no longer 2*dataRows+5) and document the estimator's known under-charge gaps (proportional column widths, word-aware wrapping, the renderer's post-layout width fallback) that the MainContent maxHeight backstop is the hard cap for. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
MikeWang0316tw
GitHub
parent
d8dc8043d6
commit
5b43edcaf6
9 changed files with 765 additions and 54 deletions
|
|
@ -0,0 +1,310 @@
|
|||
#!/usr/bin/env npx tsx
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Qwen Team
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
/**
|
||||
* Deterministic TUI ratchet for the streaming-table pending-height fix.
|
||||
*
|
||||
* Streams ten horizontal 7-column tables whose cells hold ~200 chars of
|
||||
* wrapping text (the user's repro) into the real TUI at a fixed terminal size,
|
||||
* progressively, via a chunked OpenAI-compatible fake server. On a wide
|
||||
* terminal these wrap tall, so TableRenderer falls back to the vertical
|
||||
* layout. Without the pending-height estimator fix the live frame is
|
||||
* under-charged, overflows the viewport, and Ink repaints from scratch by
|
||||
* emitting a full-screen clear (`\x1b[2J\x1b[3J\x1b[H`). Each such clear
|
||||
* resets the terminal's scroll position — so a user who scrolled up to read
|
||||
* gets yanked back to the top ("跳頂"). With the fix the estimator charges the
|
||||
* table's real (wrapped / vertical) height, the frame stays inside the
|
||||
* viewport budget, and the app updates in place instead of clearing.
|
||||
*
|
||||
* This script counts the full-screen clears the app writes to the PTY while
|
||||
* the tables stream — the same escape-sequence-ratchet methodology as
|
||||
* `subagent-flicker-regression.ts` and `table-inline-code-wrap-regression.ts`.
|
||||
*
|
||||
* Reference numbers (100×28 terminal, 10 tables, ~200-char cells):
|
||||
*
|
||||
* With the pending-height fix (current branch):
|
||||
* clearTerminalTriples=0, clear2J=0, eraseLine≈3270, ptyBytes≈0.32 MB
|
||||
* Without it (estimator under-charges the wrapped/vertical height):
|
||||
* clearTerminalTriples≈300 (≈172 mid-stream), eraseLine≈315, ptyBytes≈4.8 MB
|
||||
*
|
||||
* The signal is `clearTerminalTriples`: it collapses from ~300 to 0 with the
|
||||
* fix. The default threshold (20) sits far below the no-fix count so a full
|
||||
* regression trips the ratchet, and far above 0 so incidental start-up/exit
|
||||
* clears never flake it.
|
||||
*
|
||||
* Usage:
|
||||
* npm run build && npm run bundle
|
||||
* cd integration-tests/terminal-capture
|
||||
* npx tsx table-pending-height-scroll-lock-regression.ts
|
||||
*
|
||||
* Env:
|
||||
* QWEN_TUI_E2E_MAX_FULL_CLEARS pass threshold on clearTerminalTriples (default 20)
|
||||
* QWEN_TUI_E2E_EXPECT_PASS set to "false" to assert the ratchet FAILS
|
||||
* (use when running against pre-fix code)
|
||||
* QWEN_TUI_E2E_OUT output dir (default under os.tmpdir())
|
||||
* QWEN_TUI_E2E_REPO repo root whose dist/cli.js is launched
|
||||
*/
|
||||
import { createServer, type AddressInfo } from 'node:http';
|
||||
import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { basename, dirname, join, resolve } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { TerminalCapture } from './terminal-capture.js';
|
||||
|
||||
const TERMINAL_COLS = 100;
|
||||
const TERMINAL_ROWS = 28;
|
||||
const NUM_TABLES = 10;
|
||||
const DONE_MARKER = 'ALL_TABLES_DONE';
|
||||
const MID_STREAM_MARKER = 'Table 5';
|
||||
const PROMPT_TEXT = 'Print ten wide tables with long wrapping cells.';
|
||||
|
||||
const WORDS =
|
||||
'lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua enim ad minim veniam quis nostrud exercitation ullamco laboris nisi aliquip ex ea commodo consequat duis aute irure reprehenderit voluptate velit esse cillum'.split(
|
||||
' ',
|
||||
);
|
||||
|
||||
/** A deterministic ~200-char cell that wraps across several lines. */
|
||||
function longCell(seed: number): string {
|
||||
let s = '';
|
||||
let i = seed;
|
||||
while (s.length < 190) {
|
||||
s += (s ? ' ' : '') + WORDS[i % WORDS.length];
|
||||
i += 1;
|
||||
}
|
||||
return s;
|
||||
}
|
||||
|
||||
function buildMarkdown(): string {
|
||||
const lines: string[] = ['Here are the requested tables:', ''];
|
||||
for (let t = 1; t <= NUM_TABLES; t++) {
|
||||
lines.push(`### Table ${t}`, '');
|
||||
lines.push('| C1 | C2 | C3 | C4 | C5 | C6 | C7 |');
|
||||
lines.push('| --- | --- | --- | --- | --- | --- | --- |');
|
||||
for (let r = 0; r < 2; r++) {
|
||||
const cells: string[] = [];
|
||||
for (let c = 0; c < 7; c++) cells.push(longCell(t * 100 + r * 7 + c));
|
||||
lines.push(`| ${cells.join(' | ')} |`);
|
||||
}
|
||||
lines.push('');
|
||||
}
|
||||
lines.push(DONE_MARKER);
|
||||
return lines.join('\n');
|
||||
}
|
||||
|
||||
/** Minimal OpenAI-compatible server that streams `content` in small delayed
|
||||
* chunks, so the TUI grows its pending frame progressively (like a live
|
||||
* model) rather than receiving the whole reply in one delta. */
|
||||
function startChunkedServer(content: string): Promise<{
|
||||
baseUrl: string;
|
||||
requestCount: () => number;
|
||||
close: () => void;
|
||||
}> {
|
||||
let requests = 0;
|
||||
const server = createServer(async (req, res) => {
|
||||
if (req.method !== 'POST' || !req.url?.endsWith('/chat/completions')) {
|
||||
res.writeHead(404);
|
||||
res.end('not found');
|
||||
return;
|
||||
}
|
||||
await new Promise<void>((r) => {
|
||||
req.on('data', () => {});
|
||||
req.on('end', () => r());
|
||||
});
|
||||
const index = requests++;
|
||||
res.writeHead(200, {
|
||||
'cache-control': 'no-cache',
|
||||
connection: 'keep-alive',
|
||||
'content-type': 'text/event-stream',
|
||||
});
|
||||
const id = `chatcmpl-${index}`;
|
||||
const send = (delta: object, finish: string | null = null) =>
|
||||
res.write(
|
||||
`data: ${JSON.stringify({
|
||||
id,
|
||||
object: 'chat.completion.chunk',
|
||||
created: Math.floor(Date.now() / 1000),
|
||||
model: 'dummy',
|
||||
choices: [{ index: 0, delta, finish_reason: finish }],
|
||||
})}\n\n`,
|
||||
);
|
||||
const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms));
|
||||
|
||||
send({ role: 'assistant' });
|
||||
// Only the first turn streams the tables; any later turn ends fast.
|
||||
const body = index === 0 ? content : 'OK.';
|
||||
const PIECE = 55;
|
||||
for (let i = 0; i < body.length; i += PIECE) {
|
||||
send({ content: body.slice(i, i + PIECE) });
|
||||
if (index === 0) await sleep(18);
|
||||
}
|
||||
send({}, 'stop');
|
||||
res.write('data: [DONE]\n\n');
|
||||
res.end();
|
||||
});
|
||||
|
||||
return new Promise((resolveServer) => {
|
||||
server.listen(0, '127.0.0.1', () => {
|
||||
const port = (server.address() as AddressInfo).port;
|
||||
resolveServer({
|
||||
baseUrl: `http://127.0.0.1:${port}/v1`,
|
||||
requestCount: () => requests,
|
||||
close: () => server.close(),
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
function countAll(hay: string, needle: string): number {
|
||||
let n = 0;
|
||||
let i = hay.indexOf(needle);
|
||||
while (i !== -1) {
|
||||
n++;
|
||||
i = hay.indexOf(needle, i + needle.length);
|
||||
}
|
||||
return n;
|
||||
}
|
||||
|
||||
function envNumber(name: string, fallback: number): number {
|
||||
const value = process.env[name];
|
||||
if (value === undefined || value === '') return fallback;
|
||||
const parsed = Number(value);
|
||||
return Number.isFinite(parsed) ? parsed : fallback;
|
||||
}
|
||||
|
||||
function qwenArgs(baseUrl: string): string[] {
|
||||
return [
|
||||
'dist/cli.js',
|
||||
'--no-chat-recording',
|
||||
'--approval-mode',
|
||||
'yolo',
|
||||
'--auth-type',
|
||||
'openai',
|
||||
'--openai-api-key',
|
||||
'dummy',
|
||||
'--openai-base-url',
|
||||
baseUrl,
|
||||
'--model',
|
||||
'dummy',
|
||||
];
|
||||
}
|
||||
|
||||
async function main(): Promise<void> {
|
||||
const scriptDir = dirname(fileURLToPath(import.meta.url));
|
||||
const repoRoot = resolve(
|
||||
process.env['QWEN_TUI_E2E_REPO'] ?? resolve(scriptDir, '../..'),
|
||||
);
|
||||
const outputDir = resolve(
|
||||
process.env['QWEN_TUI_E2E_OUT'] ??
|
||||
join(tmpdir(), 'qwen-table-scroll-lock', basename(repoRoot)),
|
||||
);
|
||||
const maxFullClears = envNumber('QWEN_TUI_E2E_MAX_FULL_CLEARS', 20);
|
||||
const expectedPass = process.env['QWEN_TUI_E2E_EXPECT_PASS'] !== 'false';
|
||||
|
||||
if (existsSync(outputDir)) rmSync(outputDir, { recursive: true });
|
||||
mkdirSync(outputDir, { recursive: true });
|
||||
|
||||
const server = await startChunkedServer(buildMarkdown());
|
||||
const homeDir = join(outputDir, 'home');
|
||||
mkdirSync(homeDir, { recursive: true });
|
||||
|
||||
const env: NodeJS.ProcessEnv = {
|
||||
...process.env,
|
||||
FORCE_COLOR: '1',
|
||||
HOME: homeDir,
|
||||
USERPROFILE: homeDir,
|
||||
NODE_NO_WARNINGS: '1',
|
||||
QWEN_CODE_DISABLE_SYNCHRONIZED_OUTPUT: '1',
|
||||
QWEN_CODE_NO_RELAUNCH: '1',
|
||||
QWEN_SANDBOX: 'false',
|
||||
TERM: 'xterm-256color',
|
||||
};
|
||||
delete env['NO_COLOR'];
|
||||
delete env['QWEN_CODE_SIMPLE'];
|
||||
for (const key of [
|
||||
'HTTP_PROXY',
|
||||
'http_proxy',
|
||||
'HTTPS_PROXY',
|
||||
'https_proxy',
|
||||
'ALL_PROXY',
|
||||
'all_proxy',
|
||||
]) {
|
||||
delete env[key];
|
||||
}
|
||||
|
||||
const terminal = await TerminalCapture.create({
|
||||
chrome: false,
|
||||
cols: TERMINAL_COLS,
|
||||
rows: TERMINAL_ROWS,
|
||||
cwd: repoRoot,
|
||||
env,
|
||||
fontSize: 14,
|
||||
outputDir,
|
||||
theme: 'github-dark',
|
||||
title: 'table pending-height scroll lock regression',
|
||||
});
|
||||
|
||||
try {
|
||||
await terminal.spawn('node', qwenArgs(server.baseUrl));
|
||||
// The TUI's input hint is localized; match on a stable ASCII marker of the
|
||||
// ready prompt instead. The '❯' prompt glyph shows once input is ready.
|
||||
await terminal.waitFor('YOLO', { timeout: 30000 });
|
||||
await terminal.type(PROMPT_TEXT, { delay: 8, slow: true });
|
||||
await terminal.idle(300, 4000);
|
||||
await terminal.type('\n');
|
||||
|
||||
// Sample the clear count mid-stream (while tables are still arriving — the
|
||||
// window where a scrolled-up viewport gets yanked).
|
||||
await terminal.waitFor(MID_STREAM_MARKER, { timeout: 30000 });
|
||||
const midStreamClears = countAll(terminal.getRawOutput(), '\x1b[2J');
|
||||
|
||||
await terminal.waitForAndIdle(DONE_MARKER, {
|
||||
stableMs: 1500,
|
||||
timeout: 45000,
|
||||
});
|
||||
|
||||
const raw = terminal.getRawOutput();
|
||||
const clearTerminalTriples = countAll(raw, '\x1b[2J\x1b[3J\x1b[H');
|
||||
const pass = clearTerminalTriples <= maxFullClears;
|
||||
|
||||
const summary = {
|
||||
repoRoot,
|
||||
outputDir,
|
||||
cols: TERMINAL_COLS,
|
||||
rows: TERMINAL_ROWS,
|
||||
requestCount: server.requestCount(),
|
||||
ptyBytes: raw.length,
|
||||
clearTerminalTriples,
|
||||
clear2J: countAll(raw, '\x1b[2J'),
|
||||
midStreamClear2J: midStreamClears,
|
||||
eraseLine: countAll(raw, '\x1b[2K'),
|
||||
maxFullClears,
|
||||
pass,
|
||||
expectedPass,
|
||||
};
|
||||
writeFileSync(join(outputDir, 'raw.ansi.log'), raw);
|
||||
writeFileSync(
|
||||
join(outputDir, 'summary.json'),
|
||||
`${JSON.stringify(summary, null, 2)}\n`,
|
||||
);
|
||||
console.log(JSON.stringify(summary, null, 2));
|
||||
|
||||
if (pass !== expectedPass) {
|
||||
throw new Error(
|
||||
`Expected pass=${expectedPass} but observed pass=${pass} ` +
|
||||
`(clearTerminalTriples=${clearTerminalTriples}, threshold=${maxFullClears}). ` +
|
||||
`See ${join(outputDir, 'summary.json')}`,
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
await terminal.close();
|
||||
server.close();
|
||||
}
|
||||
}
|
||||
|
||||
main().catch((error) => {
|
||||
console.error(error);
|
||||
process.exit(1);
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue