Skip to content

fix(tui): skip cross-group tool merge in <Static> mode to eliminate screen flash #4795

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tanzhenxin merged 9 commits into from
Jun 8, 2026
Merged

fix(tui): skip cross-group tool merge in <Static> mode to eliminate screen flash #4795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
437f358
fix(cli): skip cross-group tool merge in <Static> mode to eliminate s…
zzhenyao Jun 5, 2026
03b545a
Merge branch 'main' into fix/compact-mode-merge-flash
zzhenyao Jun 5, 2026
f82f622
fix(cli): update stale refreshStatic comments and add Static-mode reg…
zzhenyao Jun 7, 2026
a74d3e8
fix(cli): set compactInline requiresRestart to true
zzhenyao Jun 7, 2026
5865d26
fix(cli): address review feedback on compactInline and Static-mode skip
zzhenyao Jun 7, 2026
cda2c94
fix(cli): address review feedback on compactInline and Static-mode paths
zzhenyao Jun 8, 2026
b8e3726
Merge branch 'main' into fix/compact-mode-merge-flash
zzhenyao Jun 8, 2026
4b79738
fix(test): use ToolCallStatus.Success instead of invalid 'completed' …
zzhenyao Jun 8, 2026
f23ca79
fix(cli): address review feedback on compact mode double-display and …
zzhenyao Jun 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions packages/cli/src/config/settingsSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,16 @@ const SETTINGS_SCHEMA = {
'Hide tool output and thinking for a cleaner view (toggle with Ctrl+O).',
showInDialog: true,
},
compactInline: {
type: 'boolean',
label: 'Compact Inline',
category: 'UI',
requiresRestart: true,
default: false,
description:
'Compact tool display within each group instead of merging across groups. Requires compactMode to be enabled.',
showInDialog: true,
},
useTerminalBuffer: {
type: 'boolean',
label: 'Virtualized History (reduces flicker on long sessions)',
Expand Down
7 changes: 5 additions & 2 deletions packages/cli/src/ui/AppContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2236,6 +2236,9 @@ export const AppContainer = (props: AppContainerProps) => {
const [compactMode, setCompactMode] = useState<boolean>(
settings.merged.ui?.compactMode ?? false,
);
const [compactInline] = useState<boolean>(
settings.merged.ui?.compactInline ?? false,
);
const configuredRenderMode = settings.merged.ui?.renderMode;
const [renderMode, setRenderMode] = useState<RenderMode>(
configuredRenderMode === 'raw' ? 'raw' : 'render',
Expand Down Expand Up @@ -3649,8 +3652,8 @@ export const AppContainer = (props: AppContainerProps) => {
);

const compactModeValue = useMemo(
() => ({ compactMode, setCompactMode }),
[compactMode, setCompactMode],
() => ({ compactMode, compactInline, setCompactMode }),
[compactMode, compactInline, setCompactMode],
);
const renderModeValue = useMemo(
() => ({ renderMode, setRenderMode }),
Expand Down
19 changes: 14 additions & 5 deletions packages/cli/src/ui/components/HistoryItemDisplay.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,25 @@ const HistoryItemDisplayComponent: React.FC<HistoryItemDisplayProps> = ({
summaryAbsorbed = false,
sourceCopyIndexOffsets,
}) => {
const marginTop =
item.type === 'gemini_content' || item.type === 'gemini_thought_content'
? 0
: 1;

const { compactMode } = useCompactMode();

const isHiddenInCompact =
compactMode &&
(item.type === 'gemini_thought' ||
item.type === 'gemini_thought_content' ||
(item.type === 'tool_use_summary' && summaryAbsorbed));

const itemForDisplay = useMemo(() => escapeAnsiCtrlCodes(item), [item]);
const contentWidth = terminalWidth - 4;
const boxWidth = mainAreaWidth || contentWidth;

if (isHiddenInCompact) return null;
Comment thread
zzhenyao marked this conversation as resolved.

const marginTop =
item.type === 'gemini_content' || item.type === 'gemini_thought_content'
? 0
: 1;

return (
<Box
flexDirection="column"
Expand Down
162 changes: 156 additions & 6 deletions packages/cli/src/ui/components/MainContent.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import { AppContext } from '../contexts/AppContext.js';
import { CompactModeProvider } from '../contexts/CompactModeContext.js';
import { OverflowProvider } from '../contexts/OverflowContext.js';
import { ToolCallStatus } from '../types.js';

const staticPropsSpy = vi.fn();
const staticItemsSpy = vi.fn();
Expand Down Expand Up @@ -238,7 +239,7 @@ const createUIActions = (): UIActions =>
const renderMainContent = (uiState: UIState) =>
render(
<AppContext.Provider value={{ version: '1.2.3', startupWarnings: [] }}>
<CompactModeProvider value={{ compactMode: false }}>
<CompactModeProvider value={{ compactMode: false, compactInline: false }}>
<UIActionsContext.Provider value={createUIActions()}>
<UIStateContext.Provider value={uiState}>
<OverflowProvider>
Expand Down Expand Up @@ -277,7 +278,9 @@ describe('<MainContent />', () => {

rerender(
<AppContext.Provider value={{ version: '1.2.3', startupWarnings: [] }}>
<CompactModeProvider value={{ compactMode: false }}>
<CompactModeProvider
value={{ compactMode: false, compactInline: false }}
>
<UIActionsContext.Provider value={createUIActions()}>
<UIStateContext.Provider
value={createUIState({
Expand Down Expand Up @@ -427,7 +430,9 @@ describe('<MainContent />', () => {
staticItemsSpy.mockClear();
rerender(
<AppContext.Provider value={{ version: '1.2.3', startupWarnings: [] }}>
<CompactModeProvider value={{ compactMode: false }}>
<CompactModeProvider
value={{ compactMode: false, compactInline: false }}
>
<UIActionsContext.Provider value={createUIActions()}>
<UIStateContext.Provider
value={createUIState({
Expand Down Expand Up @@ -485,7 +490,9 @@ describe('<MainContent />', () => {
// meant to avoid.
rerender(
<AppContext.Provider value={{ version: '1.2.3', startupWarnings: [] }}>
<CompactModeProvider value={{ compactMode: false }}>
<CompactModeProvider
value={{ compactMode: false, compactInline: false }}
>
<UIActionsContext.Provider value={createUIActions()}>
<UIStateContext.Provider
value={createUIState({ history, historyRemountKey: 2 })}
Expand Down Expand Up @@ -551,7 +558,9 @@ describe('<MainContent />', () => {
// someone correctly drives the reset off the model dimension instead.
rerender(
<AppContext.Provider value={{ version: '1.2.3', startupWarnings: [] }}>
<CompactModeProvider value={{ compactMode: false }}>
<CompactModeProvider
value={{ compactMode: false, compactInline: false }}
>
<UIActionsContext.Provider value={createUIActions()}>
<UIStateContext.Provider
value={createUIState({
Expand All @@ -573,6 +582,145 @@ describe('<MainContent />', () => {
expect(staticItemsSpy.mock.calls.at(-1)?.[0]).toHaveLength(TOTAL);
});

describe('compact mode + Static path (useTerminalBuffer=false)', () => {
Comment thread
zzhenyao marked this conversation as resolved.
it('skips cross-group merge in Static mode to avoid screen flash (issue #4794)', () => {
Comment thread
zzhenyao marked this conversation as resolved.
staticItemsSpy.mockClear();
historyItemDisplayPropsSpy.mockClear();

// Two consecutive tool_groups that mergeCompactToolGroups would normally
// consolidate into a single item. In Static mode this merge MUST be
// skipped because Ink's <Static> is append-only and cannot handle
// item-count changes without a full clearTerminal + remount (flash).
const history = [
{
id: 1,
type: 'tool_group' as const,
tools: [
{
callId: 'a1',
name: 'bash',
description: 'run ls',
status: ToolCallStatus.Success,
resultDisplay: undefined,
confirmationDetails: undefined,
},
],
},
{
id: 2,
type: 'tool_group' as const,
tools: [
{
callId: 'b1',
name: 'bash',
description: 'run wc',
status: ToolCallStatus.Success,
resultDisplay: undefined,
confirmationDetails: undefined,
},
],
},
];

// Render with compactMode=true and useTerminalBuffer=false (default Static path).
render(
<AppContext.Provider value={{ version: '1.2.3', startupWarnings: [] }}>
<CompactModeProvider
value={{ compactMode: true, compactInline: false }}
>
<UIActionsContext.Provider value={createUIActions()}>
<UIStateContext.Provider value={createUIState({ history })}>
Comment thread
zzhenyao marked this conversation as resolved.
<OverflowProvider>
<MainContent />
</OverflowProvider>
</UIStateContext.Provider>
</UIActionsContext.Provider>
</CompactModeProvider>
</AppContext.Provider>,
);

// 3 prefix items (header / debug / notifications) + 2 raw history items
// The 2 tool_groups should NOT be merged into 1.
expect(staticItemsSpy.mock.calls.at(-1)?.[0]).toHaveLength(5);
// Verify both tool_group ids are present via historyItemDisplayPropsSpy.

@wenshao wenshao Jun 8, 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.

[Suggestion] This test (and the preceding one at line 585) asserts item count and IDs via historyItemDisplayPropsSpy, but never asserts the compactLabel prop value. The new getCompactLabel early-return at MainContent.tsx:273 (if (!uiState.useTerminalBuffer || compactInline) return undefined) is therefore untested. If a future change accidentally makes getCompactLabel return a value in Static mode, the label would double-display — but these tests would still pass.

Consider adding:

// Verify compactLabel is undefined for all rendered tool_groups in Static mode.
const compactLabels = historyItemDisplayPropsSpy.mock.calls
  .map((call) => call[0].compactLabel)
  .filter((label) => label !== undefined);
expect(compactLabels).toHaveLength(0);

— qwen3.7-max via Qwen Code /review

const renderedIds = historyItemDisplayPropsSpy.mock.calls
.map((call) => call[0].item.id)
.filter((id) => id === 1 || id === 2);
expect(renderedIds).toEqual([1, 2]);
});

it('preserves tool_use_summary as standalone line when merge is skipped (Static mode)', () => {
staticItemsSpy.mockClear();
historyItemDisplayPropsSpy.mockClear();

// History with a tool_group followed by its tool_use_summary, then another tool_group.
// When merge is skipped (Static mode), absorbedCallIds returns EMPTY_ABSORBED_CALL_IDS
// so isSummaryAbsorbed returns false — the summary MUST pass through as a standalone
// item and render as `● <label>` line in HistoryItemDisplay.
const history = [
{
id: 1,
type: 'tool_group' as const,
tools: [
{
callId: 'a1',
name: 'bash',
description: 'run ls',
status: ToolCallStatus.Success,
resultDisplay: undefined,
confirmationDetails: undefined,
},
],
},
{
id: 2,
type: 'tool_use_summary' as const,
precedingToolUseIds: ['a1'],
summary: 'Searched in auth/',
},
{
id: 3,
type: 'tool_group' as const,
tools: [
{
callId: 'b1',
name: 'bash',
description: 'run wc',
status: ToolCallStatus.Success,
resultDisplay: undefined,
confirmationDetails: undefined,
},
],
},
];

render(
<AppContext.Provider value={{ version: '1.2.3', startupWarnings: [] }}>
<CompactModeProvider
value={{ compactMode: true, compactInline: false }}
>
<UIActionsContext.Provider value={createUIActions()}>
<UIStateContext.Provider value={createUIState({ history })}>
<OverflowProvider>
<MainContent />
</OverflowProvider>
</UIStateContext.Provider>
</UIActionsContext.Provider>
</CompactModeProvider>
</AppContext.Provider>,
);

// 3 prefix items (header / debug / notifications) + 3 raw history items
// (tool_group + tool_use_summary + tool_group). The summary must NOT be dropped.
expect(staticItemsSpy.mock.calls.at(-1)?.[0]).toHaveLength(6);
// Verify all three history item ids are present.
const renderedIds = historyItemDisplayPropsSpy.mock.calls
.map((call) => call[0].item.id)
.filter((id) => id === 1 || id === 2 || id === 3);
expect(renderedIds).toEqual([1, 2, 3]);
});
});

describe('virtual viewport path (ui.useTerminalBuffer)', () => {
it('renders ScrollableList and skips <Static> entirely when useTerminalBuffer is true', () => {
staticPropsSpy.mockClear();
Expand Down Expand Up @@ -689,7 +837,9 @@ describe('<MainContent />', () => {
// Flip activePtyId; identical re-render except this one streaming-state field.
rerender(
<AppContext.Provider value={{ version: '1.2.3', startupWarnings: [] }}>
<CompactModeProvider value={{ compactMode: false }}>
<CompactModeProvider
value={{ compactMode: false, compactInline: false }}
>
<UIActionsContext.Provider value={createUIActions()}>
<UIStateContext.Provider
value={createUIState({
Expand Down
Loading
Loading