mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
5b43edcaf6
* 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> |
||
|---|---|---|
| .. | ||
| baselines | ||
| cli | ||
| concurrent-runner | ||
| fixtures | ||
| hook-integration | ||
| interactive | ||
| sdk-typescript | ||
| terminal-bench | ||
| terminal-capture | ||
| channel-plugin.test.ts | ||
| fake-openai-server.test.ts | ||
| fake-openai-server.ts | ||
| globalSetup.ts | ||
| test-helper.ts | ||
| test-mcp-server.ts | ||
| tsconfig.json | ||
| vitest.config.ts | ||
| vitest.loadtest.config.ts | ||
| vitest.terminal-bench.config.ts | ||