Skip to content

fix(ui): add SlicingMaxSizedBox to prevent terminal flickering on large tool outputs#3013

Closed
chiga0 wants to merge 12 commits into
QwenLM:mainfrom
chiga0:feat/fix-terminal-flickering
Closed

fix(ui): add SlicingMaxSizedBox to prevent terminal flickering on large tool outputs#3013
chiga0 wants to merge 12 commits into
QwenLM:mainfrom
chiga0:feat/fix-terminal-flickering

Conversation

@chiga0

@chiga0 chiga0 commented Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Background

When verboseMode is 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.

Current flow:  500 lines → Ink layouts 500 lines → visual crop → flicker
Desired flow:  500 lines → slice to 15 lines → Ink layouts 15 lines → no flicker

This is a known divergence from upstream Gemini CLI, which added SlicingMaxSizedBox in March 2026 (after the Qwen Code fork point of October 2025).

Solution

Introduce SlicingMaxSizedBox — a wrapper around MaxSizedBox that uses useMemo() to truncate data BEFORE React rendering:

  1. Layer 1 — Character truncation: Caps text at 20KB (down from 1MB), preventing Ink from parsing massive strings.
  2. Layer 2 — Line-level slicing: .slice(-maxLines) retains only the last N lines before the React render tree. Ink only receives ~15 lines → layout is instant → no flicker.
  3. Layer 3 — Visual cropping (fallback): Inner MaxSizedBox still provides overflow hidden as a safety net.

The additionalHiddenLinesCount prop is passed to MaxSizedBox so the "... first N lines hidden ..." indicator correctly reflects the total hidden lines from both pre-render slicing and visual cropping.

Changes

File Change
packages/cli/src/ui/components/shared/SlicingMaxSizedBox.tsx New — Pre-render slicing component (~97 lines)
packages/cli/src/ui/components/messages/ToolMessage.tsx Replace MaxSizedBox with SlicingMaxSizedBox in StringResultRenderer; add 20KB truncation guard for markdown path
docs/design/fix-terminal-flickering/slicing-max-sized-box-design.md Design document

Before vs After

Scenario Before After
npm install (500 lines) Ink layouts 500 lines per update → flicker Sliced to 15 lines → stable
git log (200 lines) Ink layouts 200 lines → flicker Sliced to 15 lines → stable
cat large-file (5000 lines) Ink layouts 5000 lines → freeze Sliced to 15 lines → stable
Layout cost O(output lines) O(15) = constant

Compatibility

  • compact mode: Unaffected — tool outputs are fully hidden (not rendered).
  • ANSI output: Unaffected — AnsiOutputText already has its own independent .slice() logic.
  • Diff output: Unaffected — DiffRenderer uses its own height management.
  • Ctrl+S expand: Still works — ShowMoreLines and constrainHeight infrastructure unchanged.
  • Short outputs (<15 lines): Displayed in full, no slicing applied.

Test plan

  • Build passes (tsc --noEmit)
  • Verbose mode: run npm install — no flickering, output capped at terminal height
  • Verbose mode: run git log --oneline — no flickering
  • Verbose mode: cat a large file — no flickering or freeze
  • Compact mode: tool outputs still hidden correctly
  • Short outputs (<15 lines): displayed in full
  • "... first N lines hidden ..." indicator shows correct count
  • Ctrl+S expand/collapse still works

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

📋 Review Summary

This PR addresses terminal flickering issues when displaying large tool outputs in verbose mode by introducing a new SlicingMaxSizedBox component that truncates data before React rendering, rather than relying solely on visual overflow cropping. The implementation is well-structured, follows existing patterns from Gemini CLI, and includes comprehensive documentation. The changes are focused and low-risk, affecting only the UI rendering layer.

🔍 General Feedback

  • Well-documented solution: The design document (slicing-max-sized-box-design.md) provides excellent context, including problem definition, root cause analysis, and before/after comparisons. The bilingual documentation (Chinese/English) is thorough.
  • Clean architectural approach: The three-layer defense strategy (character truncation → line slicing → visual cropping) is sound and follows the Gemini CLI reference implementation.
  • Minimal code changes: Only 3 files affected with ~100 lines of new code and ~20 lines of modifications—appropriately scoped for the problem.
  • Good separation of concerns: SlicingMaxSizedBox is a reusable shared component that properly wraps MaxSizedBox without duplicating its functionality.
  • TypeScript compliance: Build and type checks pass without errors.

🎯 Specific Feedback

🟡 High

  • File: packages/cli/src/ui/components/shared/SlicingMaxSizedBox.tsx:67-72 - The line slicing logic reserves 1 line for the "hidden" indicator with Math.max(1, maxLines - 1), but this assumes MaxSizedBox will always show the indicator. If additionalHiddenLinesCount is 0 and no visual overflow occurs, the indicator won't display, potentially causing confusion about why 1 line was reserved. Consider documenting this assumption or making the reservation conditional based on whether the indicator will actually render.

  • File: packages/cli/src/ui/components/messages/ToolMessage.tsx:197-202 - The markdown path now has character truncation protection, but unlike the plain text path which uses SlicingMaxSizedBox for line-level slicing, the markdown path relies solely on character truncation. For large markdown outputs (e.g., 500 lines of markdown), this could still cause performance issues since MarkdownDisplay will receive all lines up to 20KB. Consider whether MarkdownDisplay should also support a similar pre-render slicing mechanism.

🟢 Medium

  • File: packages/cli/src/ui/components/shared/SlicingMaxSizedBox.tsx:51 - The useMemo dependency array includes overflowDirection but not maxHeight. While maxHeight is only passed to MaxSizedBox and doesn't affect the truncation logic, adding a comment explaining why it's excluded would improve code clarity for future maintainers.

  • File: packages/cli/src/ui/components/messages/ToolMessage.tsx:44-46 - The comment explains the character limit was moved but doesn't mention that the old MAXIMUM_RESULT_DISPLAY_CHARACTERS constant (1,000,000 chars) is now unused in this file. Consider removing the stale comment about "Large threshold to ensure we don't cause performance issues" since that logic has been relocated.

  • File: packages/cli/src/ui/components/shared/SlicingMaxSizedBox.tsx:1 - The copyright header states "Google LLC" but this is a new file specific to Qwen Code's fork. While it originated from Gemini CLI, consider whether the copyright should reflect Qwen Code's contribution or include both Google and Qwen.

🔵 Low

  • File: packages/cli/src/ui/components/shared/SlicingMaxSizedBox.tsx:27-28 - The children prop uses a render callback pattern (truncatedData: string) => React.ReactNode. While this is correct for the use case, adding a JSDoc comment explaining this pattern (similar to the one for data) would improve API discoverability for other developers who might use this component.

  • File: docs/design/fix-terminal-flickering/slicing-max-sized-box-design.md:335 - The design document is in Chinese. While this is valuable for the team, consider adding an English summary or parallel English version for broader accessibility, especially since Qwen Code is an open-source project with international contributors.

  • File: packages/cli/src/ui/components/messages/ToolMessage.tsx:218-227 - The SlicingMaxSizedBox usage passes both maxLines and maxHeight with the same value (availableHeight). While this is intentional (pre-render slicing + visual fallback), a brief inline comment explaining this redundancy would help future reviewers understand the two-layer defense strategy.

✅ Highlights

  • Excellent problem analysis: The PR description clearly explains the root cause (Ink layout computation for all lines vs. visible lines) with a helpful diagram showing the before/after flow.
  • Comprehensive test plan: The PR includes a detailed test matrix covering verbose mode, compact mode, short outputs, ANSI output, diff output, and Ctrl+S expand functionality.
  • Proper fallback mechanism: The inner MaxSizedBox still provides visual cropping as a safety net, ensuring robustness even if the pre-render slicing has edge cases.
  • Correct integration with existing infrastructure: The additionalHiddenLinesCount prop correctly communicates pre-render truncation to MaxSizedBox so the "... first N lines hidden ..." indicator displays accurate counts.
  • Minimal blast radius: The changes are isolated to string result rendering—ANSI output, diff output, and compact mode remain unaffected as intended.

@chiga0

chiga0 commented Apr 8, 2026

Copy link
Copy Markdown
Collaborator Author

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: availableTerminalHeight fluctuates during streaming due to footer remeasurement, tool count changes, and tab bar toggles, causing the displayed line count to jump.

New Changes

1. useStableHeight hook (packages/cli/src/ui/hooks/useStableHeight.ts)

Caches availableTerminalHeight during streaming with state-aware rules:

  • Height increases: always accepted (more space = no content jump)
  • Small decreases (<5 lines) during streaming: absorbed, MaxSizedBox clips overflow visually
  • Large decreases (≥5 lines) or stale cache (>2s): accepted as real layout changes
  • Idle state: sync immediately for accuracy

2. MIN_TOOL_OUTPUT_HEIGHT (ToolGroupMessage.tsx)

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 (AppContainer.tsx)

Both setShellExecutionConfig and resizePty use the raw (unstabilized) height so the underlying shell process always sees real terminal dimensions.

Design Doc

Full analysis at docs/design/fix-terminal-flickering/height-stabilization-design.md, including:

  • Root cause analysis of all height fluctuation sources
  • Industry technique survey (height locking, monotonic increase, debounce, threshold filtering)
  • Before/after comparison with specific scenarios
  • Edge case and risk assessment

Summary of Both Phases

Phase Problem Solution Layout cost
Phase 1 500-line output → Ink layouts all 500 lines SlicingMaxSizedBox pre-render slicing O(n) → O(15)
Phase 2 Height fluctuates → line count jumps useStableHeight absorbs small fluctuations Stable during streaming

@chiga0

chiga0 commented Apr 8, 2026

Copy link
Copy Markdown
Collaborator Author

Phase 3: Hard Cap for All Tool Outputs (commit e1000a6)

Completed tool outputs in the static (scrollback) area had no practical height limit — staticAreaMaxItemHeight = terminalHeight * 4 gave them 96-160 lines. A git diff --stat with 40+ lines displayed entirely without truncation.

Changes

MAX_TOOL_OUTPUT_LINES = 15 in ToolMessage.tsx — Hard cap matching Gemini CLI's ACTIVE_SHELL_MAX_LINES / COMPLETED_SHELL_MAX_LINES:

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 availableHeight is now always defined (number, never undefined), the markdown rendering path for tool outputs became unreachable. Removed:

  • MarkdownDisplay import
  • markdownData truncation guard
  • renderAsMarkdown prop from StringResultRenderer
  • renderOutputAsMarkdown override guard

All Three Phases Summary

Phase Commit Problem Solution
1 f5ed185 500-line output → Ink layouts all 500 lines SlicingMaxSizedBox pre-render slicing to 15 lines
2 845f197 Height fluctuates → line count jumps useStableHeight absorbs small fluctuations during streaming
3 e1000a6 Static area has no practical height limit MAX_TOOL_OUTPUT_LINES = 15 hard cap for all tool outputs

Together, these three phases provide a comprehensive anti-flicker solution:

  • Phase 1: Reduces layout cost from O(n) to O(15)
  • Phase 2: Stabilizes height during streaming
  • Phase 3: Ensures consistent 15-line cap across all areas (pending + static)

@tanzhenxin tanzhenxin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@tanzhenxin

tanzhenxin commented Apr 9, 2026

Copy link
Copy Markdown
Collaborator

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:

  • For bug fixes: show the before/after behavior.
  • For features: show the new functionality in use.

Specifically it'd be great to see:

  • A web_fetch result (or similar markdown-heavy tool output) before and after
  • The hidden lines indicator in action on a large tool output
  • The flickering fix in a short recording

This will help reviewers assess the UX tradeoffs. Thanks!

@tanzhenxin tanzhenxin added the type/bug Something isn't working as expected label Apr 9, 2026

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will restore the markdown rendering path. SlicingMaxSizedBox is designed for plain text (render callback receives string), so for markdown outputs:

  • Apply 20KB character truncation before passing to MarkdownDisplay
  • Use MAX_TOOL_OUTPUT_LINES to set availableTerminalHeight on MarkdownDisplay
  • MarkdownDisplay + MaxSizedBox handles the height limiting for formatted output

This keeps the anti-flicker benefit for plain text while preserving formatted rendering for markdown tools.

text = lines.slice(0, targetLines).join('\n');
}
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed — the count mixes logical (pre-sliced) and visual (wrap-overflow) lines. When pre-sliced text has long wrapping lines, `MaxSizedBox` adds its own `hiddenLinesCount` on top of `additionalHiddenLinesCount`, inflating the indicator.

Fix: increase the `maxHeight` passed to inner `MaxSizedBox` when pre-slicing is active (e.g., `maxHeight = maxLines * 2`) so wrapping doesn't trigger independent visual truncation. The pre-sliced count alone drives the indicator, and `MaxSizedBox` acts as a width limiter + safety net.

@chiga0

chiga0 commented Apr 10, 2026

Copy link
Copy Markdown
Collaborator Author

Addressing Review Feedback

1. Markdown rendering removal (@tanzhenxin, @wenshao)

Acknowledged — removing MarkdownDisplay entirely was too aggressive. Tools like web_fetch, MCP tools, and DeclarativeTool (which defaults isOutputMarkdown: true) will show raw markdown syntax instead of formatted output.

Plan: Restore the markdown rendering path:

  • Re-add MarkdownDisplay with renderAsMarkdown prop in StringResultRenderer
  • Apply the 20KB character truncation guard before passing to MarkdownDisplay
  • Cap height via MAX_TOOL_OUTPUT_LINES to ensure it's bounded
  • Keep SlicingMaxSizedBox for the plain text path only

2. Double-counting hidden lines (@tanzhenxin, @wenshao)

When pre-sliced text contains lines that soft-wrap within maxWidth, MaxSizedBox independently detects visual overflow and adds its own hiddenLinesCount on top of additionalHiddenLinesCount. This mixes logical and visual line counts in the indicator.

Fix: When SlicingMaxSizedBox has already pre-sliced, increase maxHeight passed to inner MaxSizedBox to accommodate potential wrapping overflow, so it acts as a width limiter + safety net without independently truncating height. The pre-sliced count alone drives the indicator.

3. useStableHeight mutates refs during render (@tanzhenxin)

This is intentional for Ink's rendering model. Ink uses synchronous rendering — no StrictMode, no concurrent features. Using useEffect + state would introduce an extra render cycle per height update, which directly counteracts the anti-flicker goal (the whole point is to stabilize height within the same render pass).

The code comment at useStableHeight.ts:32-33 documents this trade-off explicitly. If Ink adopts concurrent features in the future, this hook will need refactoring. For now, this is the correct approach for this environment.

4. Copyright header (@tanzhenxin)

Will update from "Google LLC" to match project conventions.

5. Test failure ToolMessage.test.tsx:147 (@wenshao)

Confirmed — the assertion expects MockMarkdown:Test result but StringResultRenderer no longer routes through MarkdownDisplay. Will fix when restoring the markdown path (point 1).


Will push fixes in a follow-up commit. Before/after comparison recordings will be added shortly.

@chiga0

chiga0 commented Apr 10, 2026

Copy link
Copy Markdown
Collaborator Author

@tanzhenxin Recordings are in progress. Will update this comment with:

  • Flickering fix demo (large `npm install` / `git log` output)
  • Hidden lines indicator on large tool output
  • `web_fetch` markdown rendering before vs after

🎬 Before/After comparison recordings will be uploaded here shortly.

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the earlier feedback. The overall direction looks much better now, but I still see two blocking issues in the current implementation: markdown output is still effectively disabled in the normal constrained-height path, and the hidden-line fix removes the visual height cap for wrapped content. I left the details inline.

// for anti-flicker. Markdown rendering is used only when no terminal height
// constraint is provided (e.g., static area items without height info).
const effectiveRenderAsMarkdown =
renderOutputAsMarkdown && availableTerminalHeight === undefined;

@yiliang114 yiliang114 Apr 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I noticed here: this still effectively disables markdown rendering for the default UI path. The effectiveRenderAsMarkdown flag only becomes true when availableTerminalHeight is undefined, but both the static history path and the constrained live tool path pass a height into ToolMessage. In practice, markdown-producing tools like web_fetch will still render raw markdown in normal constrained mode. Could we preserve markdown rendering when height is known and solve the flicker separately for that path?

// additionalHiddenLinesCount, mixing logical and visual line counts.
// With maxHeight=undefined, MaxSizedBox handles width only and renders the
// indicator from additionalHiddenLinesCount alone.
const effectiveMaxHeight = hiddenLineCount > 0 ? undefined : maxHeight;

@yiliang114 yiliang114 Apr 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This avoids double-counting hidden lines, but it also removes the visual height cap entirely once pre-slicing is active. If the retained logical lines contain long wrapped content, MaxSizedBox will render all wrapped rows, so the 15-line cap no longer holds and layout cost can grow again. Could we keep a bounded fallback here instead of fully disabling height limiting?

chiga0 and others added 9 commits April 10, 2026 20:34
…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>
…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>
…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>
…ne counting

- Restore MarkdownDisplay rendering path in StringResultRenderer for tools
  with renderOutputAsMarkdown=true (e.g., web_fetch, MCP tools). Markdown
  is used when availableTerminalHeight is undefined; plain text with
  SlicingMaxSizedBox is used when terminal height is known (anti-flicker).
- Fix hidden line double-counting in SlicingMaxSizedBox: when pre-slicing
  is active, pass maxHeight=undefined to inner MaxSizedBox so it doesn't
  independently truncate wrapped lines and inflate the indicator count.
- Update copyright headers from "Google LLC" to "Qwen" for new files
  (SlicingMaxSizedBox.tsx, useStableHeight.ts).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AgentExecutionDisplay rendered content without height constraints, causing
Ink to layout 60-90+ lines when multiple sub-agents ran in parallel — far
exceeding the visible terminal area and producing severe flickering.

Changes:
- Add computeContentBudget() to dynamically limit task prompt lines and
  tool calls based on availableHeight, preventing Ink from laying out
  content that overflows the terminal
- Add effectiveDisplayMode that auto-downgrades to compact when height is
  critically low (<5 lines), preventing expansion into flickering states
- Fix Ctrl+E/F keyboard guard: restrict to running sub-agents only, so
  completed agents don't re-render on toggle (reduces churn in long
  sessions) and confirmation prompts don't conflict
- Update TaskPromptSection, ToolCallsList, and ResultsSection to accept
  dynamic max limits instead of hardcoded constants

Fixes QwenLM#2928, helps QwenLM#2950, QwenLM#2972, QwenLM#2912
…ht constraint

- Unit tests for computeContentBudget: NaN, negative, zero, small, medium,
  large terminal heights; verifies min/max bounds always hold
- Render tests: compact mode default, auto-downgrade at small heights,
  completed agent display
…act mode

Two fixes:
- Remove auto-downgrade to compact for default mode. The content budget
  already limits rendered content, and forcing compact overrode explicit
  Ctrl+E expansion (height per sub-agent is often <5 with parallel agents).
  Only verbose→default downgrade is kept for very small heights.
- Force-expand tool group from compact mode when a subagent has a pending
  confirmation (pendingConfirmation). Previously only direct tool-level
  ToolCallStatus.Confirming triggered expansion, but subagent approvals
  use a different mechanism (AgentResultDisplay.pendingConfirmation) that
  was not checked, hiding the approval prompt in compact mode.
@chiga0 chiga0 force-pushed the feat/fix-terminal-flickering branch from a249947 to f9923d5 Compare April 10, 2026 12:38
…ize output

Each sub-agent tool call update triggers a re-render of the entire
ToolGroupMessage. Without a fixed height container, the rendered height
fluctuates between re-renders, causing Ink's bordered-box rendering to
produce flickering and tearing (known Ink issue noted in ToolGroupMessage
comments). Wrapping in Box with height={availableHeight} and
overflow="hidden" keeps the terminal output height stable.
taskPrompt:
'Do something\nLine 2\nLine 3\nLine 4\nLine 5\nLine 6\nLine 7\nLine 8\nLine 9\nLine 10',
status: 'running',
toolCalls: Array.from({ length: 8 }, (_, i) => ({

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] toolCalls mock data is missing the required callId: string property. Each entry only has { name, status, description }, causing TS2322 error and breaking npm run build.

Suggested change
toolCalls: Array.from({ length: 8 }, (_, i) => ({
toolCalls: Array.from({ length: 8 }, (_, i) => ({
callId: `call-${i + 1}`,
name: `tool-${i + 1}`,
status: 'success' as const,
description: `Description ${i + 1}`,
})),

— qwen3.6-plus via Qwen Code /review

/>,
);
const output = lastFrame();
const lineCount = output.split('\n').length;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] lastFrame() returns string | undefined, but .split('\n') is called without a null check (TS18048). Same issue at line 160.

Suggested change
const lineCount = output.split('\n').length;
expect(output).toBeDefined();
const lineCount = output!.split('\n').length;

— qwen3.6-plus via Qwen Code /review

it('shows completed agent summary in compact mode', () => {
const data = makeAgentData({
status: 'completed',
executionSummary: {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] executionSummary is missing 5 required AgentStatsSummary properties: inputTokens, outputTokens, thoughtTokens, cachedTokens, toolUsage (TS2739).

Suggested change
executionSummary: {
executionSummary: {
totalToolCalls: 5,
successfulToolCalls: 5,
failedToolCalls: 0,
successRate: 100,
totalTokens: 1000,
totalDurationMs: 5000,
rounds: 3,
inputTokens: 0,
outputTokens: 0,
thoughtTokens: 0,
cachedTokens: 0,
toolUsage: [],
},

— qwen3.6-plus via Qwen Code /review

// also truncates, its hiddenLinesCount would be added to our
// additionalHiddenLinesCount, mixing logical and visual line counts.
// With maxHeight=undefined, MaxSizedBox handles width only and renders the
// indicator from additionalHiddenLinesCount alone.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] When hiddenLineCount > 0, effectiveMaxHeight is set to undefined, disabling MaxSizedBox's visual cropping entirely. In narrow terminals where pre-sliced lines soft-wrap into multiple visual rows, content can overflow the allocated space with no safety net.

Consider maxHeight = maxLines * 2 as a bounded fallback — MaxSizedBox still catches soft-wrap overflow while the pre-sliced count alone drives the indicator.

— qwen3.6-plus via Qwen Code /review

const targetLines = Math.max(1, maxLines - 1);
hidden = lines.length - targetLines;
if (overflowDirection === 'top') {
text = lines.slice(-targetLines).join('\n');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] When overflowDirection === 'bottom', line slicing keeps the first N lines, but character truncation (line 68) always keeps the last 20KB. If data exceeds 20KB, the user sees the first N lines of the last 20KB — not the first N lines of the original content.

For overflowDirection === 'bottom', use text.slice(0, MAXIMUM_RESULT_DISPLAY_CHARACTERS) instead.

— qwen3.6-plus via Qwen Code /review

if (text.length > MAXIMUM_RESULT_DISPLAY_CHARACTERS) {
text = '...' + text.slice(-MAXIMUM_RESULT_DISPLAY_CHARACTERS);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] When character truncation splits mid-line (the 20KB boundary falls within a line), the first "line" of the truncated text is a partial line. hiddenLineCount counts full lines only, so the indicator underreports by 1.

Consider incrementing hidden by 1 when character truncation is active.

— qwen3.6-plus via Qwen Code /review

@@ -145,11 +151,17 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
const countOneLineToolCalls = toolCalls.length - countToolCallsWithResults;
const availableTerminalHeightPerToolMessage = availableTerminalHeight

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The height allocation formula can over-allocate per tool when there are many one-line tools. Example: availableTerminalHeight=30, countToolCallsWithResults=1, countOneLineToolCalls=25 → returns 8, but real available space after overhead is only 30-3-25=2 lines.

Apply MIN_TOOL_OUTPUT_HEIGHT as a cap on the final result, not as a competing term in Math.max:
Math.max(1, Math.min(MIN_TOOL_OUTPUT_HEIGHT, floor((total - staticHeight - oneLineTools) / countWithResults)))

— qwen3.6-plus via Qwen Code /review

const timeSinceUpdate = Date.now() - lastUpdateRef.current;

if (delta > 0) {
// More space available — always safe to expand, no content jump

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The STALE_TIMEOUT_MS check only fires when the component re-renders for another reason. If nothing else triggers a re-render, the cached height could remain stale indefinitely.

Consider adding a useEffect with setTimeout that forces a re-check when isStreaming is true and the timeout has elapsed.

— qwen3.6-plus via Qwen Code /review

@tanzhenxin tanzhenxin dismissed their stale review April 17, 2026 07:53

Dismissing — the author has addressed the markdown regression across multiple rounds and is still actively iterating. Re-reviewing is better handled fresh rather than keeping a stale changes-requested state in the way.

…le rendering

Root cause: when streaming a long markdown table (e.g. 12 columns × 10 rows
in vertical format = 130+ lines), the pending dynamic area exceeds the terminal
viewport height. Open-source Ink's log-update clears the entire visible area
on every chunk and rewrites it — the terminal briefly renders blank between
clear and rewrite, producing high-frequency black-frame flicker.

This commit applies a multi-layered fix inspired by Claude Code's architecture:

**Layer 1 — Table layout stability (TableRenderer.tsx)**
- Hysteresis thresholds (enter vertical on >5, exit on ≤3) with streaming
  monotonic lock (once vertical, stays vertical until stream ends) to prevent
  layout flip-flop between horizontal and vertical formats.
- Headers-based React key for stable component identity across streaming chunks.

**Layer 2 — Streaming row cap (TableRenderer.tsx, P2)**
- During isPending, cap rendered table rows to fit within availableTerminalHeight.
- Show last N rows + "... N earlier rows hidden during streaming ..." hint.
- Format-specific overhead calculation (vertical: marginY; horizontal: borders +
  header + marginY) ensures the cap never overflows the viewport.
- Final committed render (isPending=false) shows all rows.

**Layer 3 — Streaming partial-row guard (MarkdownDisplay.tsx)**
- Skip the last line during streaming if it starts with `|` but has no closing
  `|` — prevents per-chunk flicker as each character of a partial row toggles
  between "table row" and "raw text" rendering.
- Detect bullet-wrapped tables (`+ | ID | Name |`) and emit as table blocks.
- Emit tables with 0 rows as header-only skeletons during streaming to prevent
  height collapse.

**Layer 4 — DEC 2026 synchronized output (synchronizedOutput.ts)**
- Wrap each Ink frame's stdout burst in BSU/ESU sequences so supporting
  terminals (iTerm2, WezTerm, VSCode, ghostty, Warp, kitty, alacritty, etc.)
  commit the clear→redraw atomically — no intermediate blank state visible.
- Terminal detection via env-var allowlist; no-op on unsupported terminals.
- Exit handler ensures ESU is sent on process termination.

**Layer 5 — Application-level render throttle (useRenderThrottledStateAndRef.ts)**
- Drop-in replacement for useStateAndRef that caps React re-render frequency
  to ~60fps (16ms). Ref updates synchronously for finalize paths; only the
  React setState (which drives Ink frame writes) is throttled.
- Null/undefined bypass throttle so finalize clears pending instantly.
- Explicit flush() at commit boundaries (split, type transition) prevents
  duplicate-render artifacts where history shows committed content while
  pending still shows stale pre-split text.

**Layer 6 — Width oscillation guard (AppContainer.tsx)**
- Debounce terminalWidth changes with MIN_WIDTH_DELTA_FOR_REFRESH = 4 to
  prevent scrollbar show/hide (97↔95 columns) from triggering refreshStatic.
- Diagnostic logging for refreshStatic calls (temporary, for verification).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
taskPrompt:
'Do something\nLine 2\nLine 3\nLine 4\nLine 5\nLine 6\nLine 7\nLine 8\nLine 9\nLine 10',
status: 'running',
toolCalls: Array.from({ length: 8 }, (_, i) => ({

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] This new test fixture no longer matches the current AgentResultDisplay / AgentStatsSummary types: toolCalls entries are missing required callId, later assertions call lastFrame().split(...) without guarding against undefined, and the completed executionSummary fixture is missing required fields. As written, this file breaks the CLI package build.

Suggested change
toolCalls: Array.from({ length: 8 }, (_, i) => ({
toolCalls: Array.from({ length: 8 }, (_, i) => ({
callId: `call-${i + 1}`,
name: `tool-${i + 1}`,
status: 'success' as const,
description: `Description ${i + 1}`,
})),

— gpt-5.4 via Qwen Code /review


// Capture what the patched write forwards to the underlying stream.
const writes: string[] = [];
const fakeWrite = vi.fn<Parameters<typeof process.stdout.write>, boolean>(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] vi.fn<Parameters<typeof process.stdout.write>, boolean>(...) is not compatible with the Vitest typings currently used in this repo. That generic signature makes the new test fail type-checking, which in turn breaks npm run build for packages/cli.

Suggested change
const fakeWrite = vi.fn<Parameters<typeof process.stdout.write>, boolean>(
const fakeWrite = vi.fn((chunk: unknown) => {
writes.push(typeof chunk === 'string' ? chunk : String(chunk));
return true;
});

— gpt-5.4 via Qwen Code /review

const [modelSwitchedFromQuotaError, setModelSwitchedFromQuotaError] =
useState<boolean>(false);
const [historyRemountKey, setHistoryRemountKey] = useState(0);
// TEMP: prove our diagnostic code path is actually loaded in this run.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] These TEMP diagnostics are still enabled in the normal interactive path and write directly to stderr on every mount. Combined with the stack-trace dump added in refreshStatic(), this makes the PR change user-visible CLI output outside the flicker fix itself and can interfere with stderr-based tooling/tests. Please remove these unconditional diagnostics before merge, or gate them behind an explicit debug flag that is off by default.

— gpt-5.4 via Qwen Code /review

@lgzzzz

lgzzzz commented Apr 22, 2026

Copy link
Copy Markdown
Contributor
  • I tested and compared the output of the current PR against the original Qwen Code. In the video below, the one with obvious flickering is the pre-PR version.
  • The original code flickers (intensely) when output goes beyond 15 lines, but the PR version is much smoother.
    This bug is a major UX issue. I'm not sure about the technical specifics, so this is all the info I can offer. Hope to see this merged soon!
450b11cfc5c551f5a6155971daa36329.mp4
04c7e6790ab11f28a554c8af8e93a1c6.mp4

@chiga0

chiga0 commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator Author
  • I tested and compared the output of the current PR against the original Qwen Code. In the video below, the one with obvious flickering is the pre-PR version.
  • The original code flickers (intensely) when output goes beyond 15 lines, but the PR version is much smoother.
    This bug is a major UX issue. I'm not sure about the technical specifics, so this is all the info I can offer. Hope to see this merged soon!

450b11cfc5c551f5a6155971daa36329.mp4

04c7e6790ab11f28a554c8af8e93a1c6.mp4

thank you for your test report. I'm working on it now. But I find this PR contains many submit and issue fixing. I would cancel this PR and split it into more PR to fix all flicker issue. thank you again.

@chiga0

chiga0 commented Apr 24, 2026

Copy link
Copy Markdown
Collaborator Author

I will close this PR, the new PR refer #3591

@chiga0 chiga0 closed this Apr 24, 2026
xaelistic pushed a commit to xaelistic/qwen-code that referenced this pull request Jun 7, 2026
Co-authored-by: N. Taylor Mullen <ntaylormullen@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants