fix(ui): add SlicingMaxSizedBox to prevent terminal flickering on large tool outputs#3013
fix(ui): add SlicingMaxSizedBox to prevent terminal flickering on large tool outputs#3013chiga0 wants to merge 3 commits intoQwenLM:mainfrom
Conversation
…ge tool outputs When verboseMode is enabled, large tool outputs (npm install ~500 lines, git log ~200 lines) cause terminal flickering because MaxSizedBox lets Ink layout ALL content before visually cropping. SlicingMaxSizedBox uses useMemo() to slice data to maxLines BEFORE React rendering, reducing layout cost from O(output lines) to O(15) constant time. - New SlicingMaxSizedBox component with two-layer pre-render truncation (20KB char limit + line slicing) - ToolMessage StringResultRenderer now uses SlicingMaxSizedBox for plain text path - Markdown path also protected with 20KB character truncation guard Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📋 Review SummaryThis PR addresses terminal flickering issues when displaying large tool outputs in verbose mode by introducing a new 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
…t flickering During tool execution, availableTerminalHeight fluctuates due to controlsHeight remeasurement, tool count changes, and tabBar toggles. These fluctuations cause the displayed line count to jump, creating flicker even with pre-render slicing. Add useStableHeight hook that caches height during streaming: - Height increases: always accepted (more space, no content jump) - Small decreases (<5 lines): absorbed during streaming, MaxSizedBox clips overflow - Large decreases (≥5 lines) or stale cache (>2s): accepted as real layout changes - Idle state: sync immediately for accuracy Also add MIN_TOOL_OUTPUT_HEIGHT=8 in ToolGroupMessage to prevent dramatic height jumps when tool count changes. Shell PTY uses raw (unstabilized) height to ensure the underlying process always sees real terminal dimensions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Phase 2: Height Stabilization (commit 845f197)Phase 1 (SlicingMaxSizedBox) solved flickering caused by large data volume — Ink no longer layouts 500 lines. However, a second flickering source remained: New Changes1. Caches
2. MIN_TOOL_OUTPUT_HEIGHT ( Added floor of 8 lines per tool (guarded for tiny terminals) to prevent dramatic height jumps when tool count changes (e.g., 2→3 tools would shrink each from 15→9 lines). 3. Shell PTY uses raw height ( Both Design DocFull analysis at
Summary of Both Phases
|
…utputs Completed tool outputs in the static (scrollback) area had no practical height limit because staticAreaMaxItemHeight = terminalHeight * 4 (~96-160 lines). This meant git diff --stat with 40+ lines displayed entirely without truncation. Add MAX_TOOL_OUTPUT_LINES = 15 hard cap (matching Gemini CLI's ACTIVE_SHELL_MAX_LINES / COMPLETED_SHELL_MAX_LINES) that applies to all tool outputs regardless of whether they are pending or in scrollback history. Also clean up dead code: since availableHeight is now always defined, the markdown rendering path for tool outputs is unreachable. Remove MarkdownDisplay import, markdownData truncation, renderAsMarkdown prop from StringResultRenderer, and the renderOutputAsMarkdown override guard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Phase 3: Hard Cap for All Tool Outputs (commit e1000a6)Completed tool outputs in the static (scrollback) area had no practical height limit — Changes
const availableHeight = availableTerminalHeight
? Math.min(MAX_TOOL_OUTPUT_LINES, Math.max(computed, MIN_LINES_SHOWN + 1))
: MAX_TOOL_OUTPUT_LINES; // Also caps when undefined (was unlimited before)Dead code cleanup — Since
All Three Phases Summary
Together, these three phases provide a comprehensive anti-flicker solution:
|
tanzhenxin
left a comment
tanzhenxin
left a comment
There was a problem hiding this comment.
Review — fix(ui): add SlicingMaxSizedBox to prevent terminal flickering on large tool outputs
Files changed: 7 (+825 / -57)
The core slicing concept is sound — pre-render truncation is the right approach for this performance problem, and useStableHeight is a clever idea for streaming stabilization. A few issues to address:
1. Markdown rendering silently removed for all tool outputs
The PR removes MarkdownDisplay from StringResultRenderer entirely, with a comment saying it "does not respect availableTerminalHeight properly." This is a significant UX regression for tools like web_fetch that return formatted markdown — users will see raw syntax (#, **, [links](...)) instead of formatted output. The renderOutputAsMarkdown field is still populated elsewhere but is now dead code.
Suggestion: If MarkdownDisplay doesn't respect height limits, wrap it in SlicingMaxSizedBox. Don't remove markdown rendering wholesale. If this must ship without markdown support, gate it behind a config option and document prominently.
2. Double-counting of hidden lines when text wraps
SlicingMaxSizedBox reserves 1 line from maxLines for the hidden indicator and passes additionalHiddenLinesCount to MaxSizedBox. But when Text wrap="wrap" causes logical lines to soft-wrap into multiple visual rows, MaxSizedBox will independently detect overflow and add its own hiddenLinesCount. The total displayed count mixes data-level and visual-level hidden lines, producing an inaccurate number.
Suggestion: Either make MaxSizedBox purely a width limiter when pre-slicing is active (set maxHeight to undefined), or let MaxSizedBox be the sole source of the hidden indicator.
3. useStableHeight mutates refs during render
The hook reads Date.now() and mutates refs in the render body. The comment says this is safe under Ink's synchronous model, but it's a React anti-pattern — StrictMode double-invokes render, and each call sees a different timestamp. Should use useMemo or useEffect + state instead.
4. Copyright header says "Google LLC"
Both new files have @license Copyright 2025 Google LLC, likely copied from Gemini CLI reference. Should match this project's conventions.
|
Hey @chiga0 — since this PR significantly changes how tool outputs render (removing markdown formatting, adding truncation with hidden line counts), could you attach before/after screenshots or a short video showing the new behavior? Per the PR template:
Specifically it'd be great to see:
This will help reviewers assess the UX tradeoffs. Thanks! |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Critical] ToolMessage.test.tsx:147 — Test failure. The test asserts expect(output).toContain('MockMarkdown:Test result') but StringResultRenderer no longer uses MarkdownDisplay, so output renders as plain text (Test result). Fix: change assertion to expect(output).toContain('Test result').
— qwen3.6-plus via Qwen Code /review
| </Text> | ||
| </Box> | ||
| )} | ||
| </SlicingMaxSizedBox> |
There was a problem hiding this comment.
[Suggestion] StringResultRenderer unconditionally removes the markdown rendering path. Several tools set isOutputMarkdown: true (default in DeclarativeTool, explicitly in agent.ts, mcp-tool.ts, web_fetch). Their markdown-formatted output will now render as raw markdown syntax (**bold**, # headers) instead of formatted text.
The comment says this is intentional because "MarkdownDisplay does not respect availableTerminalHeight properly." If acceptable, consider making this explicit in the PR description as a behavioral change. Alternatively, wrap MarkdownDisplay with SlicingMaxSizedBox to retain markdown rendering with height limits.
— qwen3.6-plus via Qwen Code /review
| text = lines.slice(0, targetLines).join('\n'); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
[Suggestion] hiddenLineCount is computed from pre-sliced logical lines vs targetLines, then passed to MaxSizedBox as additionalHiddenLinesCount. MaxSizedBox also independently computes hiddenLinesCount from its layout (laidOutStyledText.length - visibleContentHeight). When long lines wrap within maxWidth, a single logical line becomes multiple rendered lines, causing over-counting in the hidden line indicator.
The "... first N lines hidden ..." indicator may show an incorrect count when tool output contains long wrapping lines.
— qwen3.6-plus via Qwen Code /review
3 participants
Background
When
verboseModeis enabled (the default), executing commands that produce large outputs (e.g.,npm install~500 lines,git log~200 lines,cat large-file.json~5000 lines) causes visible terminal screen flickering and stuttering, severely degrading user experience.Root Cause
The current rendering pipeline passes all output data to
MaxSizedBox, which relies on Ink's visual overflow cropping. However, Ink still needs to layout the entire content to determine what overflows — even though only ~15 lines are visible to the user. For a 500-line output, Ink computes layout for all 500 lines, and every new line triggers a full re-layout, causing the flicker.This is a known divergence from upstream Gemini CLI, which added
SlicingMaxSizedBoxin March 2026 (after the Qwen Code fork point of October 2025).Solution
Introduce
SlicingMaxSizedBox— a wrapper aroundMaxSizedBoxthat usesuseMemo()to truncate data BEFORE React rendering:.slice(-maxLines)retains only the last N lines before the React render tree. Ink only receives ~15 lines → layout is instant → no flicker.MaxSizedBoxstill provides overflow hidden as a safety net.The
additionalHiddenLinesCountprop is passed toMaxSizedBoxso the "... first N lines hidden ..." indicator correctly reflects the total hidden lines from both pre-render slicing and visual cropping.Changes
packages/cli/src/ui/components/shared/SlicingMaxSizedBox.tsxpackages/cli/src/ui/components/messages/ToolMessage.tsxMaxSizedBoxwithSlicingMaxSizedBoxinStringResultRenderer; add 20KB truncation guard for markdown pathdocs/design/fix-terminal-flickering/slicing-max-sized-box-design.mdBefore vs After
npm install(500 lines)git log(200 lines)cat large-file(5000 lines)Compatibility
AnsiOutputTextalready has its own independent.slice()logic.DiffRendereruses its own height management.ShowMoreLinesandconstrainHeightinfrastructure unchanged.Test plan
tsc --noEmit)npm install— no flickering, output capped at terminal heightgit log --oneline— no flickeringcata large file — no flickering or freeze🤖 Generated with Claude Code