Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
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
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
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.
Resolved

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

return (
<Box
flexDirection="column"
Expand Down
91 changes: 85 additions & 6 deletions packages/cli/src/ui/components/MainContent.test.tsx
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,74 @@ 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.
Resolved
it('skips cross-group merge in Static mode to avoid screen flash (issue #4794)', () => {
Comment thread
zzhenyao marked this conversation as resolved.
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.
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.

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]);
});
});

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 +766,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
62 changes: 26 additions & 36 deletions packages/cli/src/ui/components/MainContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { ShowMoreLines } from './ShowMoreLines.js';
import { Notifications } from './Notifications.js';
import { OverflowProvider } from '../contexts/OverflowContext.js';
import { useUIState } from '../contexts/UIStateContext.js';
import { useUIActions } from '../contexts/UIActionsContext.js';
import { useAppContext } from '../contexts/AppContext.js';
import { AppHeader } from './AppHeader.js';
import { DebugModeNotification } from './DebugModeNotification.js';
Expand Down Expand Up @@ -105,8 +104,7 @@ const virtualIsStaticItem = (item: HistoryItem) => item.id > 0;
export const MainContent = () => {
const { version } = useAppContext();
const uiState = useUIState();
const uiActions = useUIActions();
const { compactMode } = useCompactMode();
const { compactMode, compactInline } = useCompactMode();
const {
pendingHistoryItems,
terminalWidth,
Expand Down Expand Up @@ -136,7 +134,12 @@ export const MainContent = () => {
// `isSummaryAbsorbed` → `renderVirtualItem` → every static item re-renders.
const prevAbsorbedCallIdsRef = useRef<Set<string>>(EMPTY_ABSORBED_CALL_IDS);
const absorbedCallIds = useMemo(() => {
if (!compactMode) return EMPTY_ABSORBED_CALL_IDS;
// When no cross-group merge will happen (Static mode or compactInline),
// don't mark summaries as absorbed so the standalone `● <label>` line
// in HistoryItemDisplay can render — <Static> can't repaint committed
// items, so the label's only path to the screen is the standalone line.
if (!compactMode || !uiState.useTerminalBuffer || compactInline)
Comment thread
zzhenyao marked this conversation as resolved.
Resolved
return EMPTY_ABSORBED_CALL_IDS;
const absorbed = new Set<string>();
for (const item of uiState.history) {
if (item.type !== 'tool_group') continue;
Expand Down Expand Up @@ -166,15 +169,18 @@ export const MainContent = () => {
return absorbed;
}, [
compactMode,
compactInline,
uiState.history,
uiState.embeddedShellFocused,
uiState.activePtyId,
uiState.useTerminalBuffer,
]);

// Merge consecutive tool_groups for compact mode display. Summaries for
// absorbed call IDs are dropped during merge so refreshStatic fires;
// summaries for force-expanded (non-absorbed) groups pass through so
// HistoryItemDisplay can render them as standalone `● <label>` lines.
// absorbed call IDs are dropped during merging so the compact header can
// display the label directly; summaries for force-expanded (non-absorbed)
// groups pass through so HistoryItemDisplay can render them as standalone
// `● <label>` lines.
//
// Compact-mode content-stable: `mergeCompactToolGroups` always allocates a
// fresh array. With `activePtyId` / `embeddedShellFocused` in the deps,
Expand All @@ -188,6 +194,17 @@ export const MainContent = () => {
prevMergedHistoryRef.current = uiState.history;
return uiState.history;
}
// <Static> is append-only: merging reduces item count, which <Static>
Comment thread
zzhenyao marked this conversation as resolved.
Resolved
// can't handle without clearTerminal + remount (the flash we're fixing).
if (!uiState.useTerminalBuffer) {
prevMergedHistoryRef.current = uiState.history;
return uiState.history;
}
// compactInline: user opted into per-group display without cross-group merging.
if (compactInline) {
prevMergedHistoryRef.current = uiState.history;
return uiState.history;
}
const next = mergeCompactToolGroups(
uiState.history,
uiState.embeddedShellFocused,
Expand All @@ -209,7 +226,9 @@ export const MainContent = () => {
return next;
}, [
compactMode,
compactInline,
uiState.history,
uiState.useTerminalBuffer,
uiState.embeddedShellFocused,
uiState.activePtyId,
absorbedCallIds,
Expand Down Expand Up @@ -261,35 +280,6 @@ export const MainContent = () => {
[summaryByCallId],
);

// Ink's <Static> is append-only: once an item is rendered to the terminal
// buffer, it cannot be replaced. In compact mode, when a new tool_group is
// merged into a previous one, the merged result has FEWER items than the
// raw history. Static would not re-render the older items even though their
// content changed, so we explicitly call refreshStatic() to clear the
// terminal and re-render the merged view.
//
// Detection: if history length grew but mergedHistory length did NOT grow
// proportionally (i.e., a merge consolidated items), trigger a refresh.
const prevHistoryLengthRef = useRef(uiState.history.length);
const prevMergedLengthRef = useRef(mergedHistory.length);
useEffect(() => {
if (!compactMode) {
prevHistoryLengthRef.current = uiState.history.length;
prevMergedLengthRef.current = mergedHistory.length;
return;
}
const prevHLen = prevHistoryLengthRef.current;
const currHLen = uiState.history.length;
const prevMLen = prevMergedLengthRef.current;
const currMLen = mergedHistory.length;
// History grew, but merged length stayed same or shrank → a merge happened.
if (currHLen > prevHLen && currMLen <= prevMLen) {
uiActions.refreshStatic();
}
prevHistoryLengthRef.current = currHLen;
prevMergedLengthRef.current = currMLen;
}, [compactMode, uiState.history, mergedHistory, uiActions]);

// Virtual viewport path short-circuits below before any of the
// <Static>-only machinery is needed. The offsets / progressive-replay
// state still computes because it lives at the top of the component, but
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/ui/components/SettingsDialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ vi.mock('../contexts/CompactModeContext.js', async () => {
...actual,
useCompactMode: () => ({
compactMode: false,
compactInline: false,
setCompactMode: mockSetCompactMode,
}),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ describe('<ToolGroupMessage />', () => {
const renderCompact = (component: React.ReactElement, compactMode = true) =>
render(
<ConfigContext.Provider value={mockConfig}>
<CompactModeProvider value={{ compactMode }}>
<CompactModeProvider value={{ compactMode, compactInline: false }}>
{component}
</CompactModeProvider>
</ConfigContext.Provider>,
Expand Down
8 changes: 4 additions & 4 deletions packages/cli/src/ui/components/messages/ToolMessage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ const renderWithContext = (
) => {
const contextValue: StreamingState = streamingState;
return render(
<CompactModeProvider value={{ compactMode }}>
<CompactModeProvider value={{ compactMode, compactInline: false }}>
<SettingsContext.Provider value={mockSettings}>
<StreamingContext.Provider value={contextValue}>
{ui}
Expand Down Expand Up @@ -631,7 +631,7 @@ describe('<ToolMessage />', () => {
merged: { ui: { shellOutputMaxLines: 0 } },
} as unknown as LoadedSettings;
const { lastFrame } = render(
<CompactModeProvider value={{ compactMode: false }}>
<CompactModeProvider value={{ compactMode: false, compactInline: false }}>
<SettingsContext.Provider value={settingsWithDisabledCap}>
<StreamingContext.Provider value={StreamingState.Idle}>
<ToolMessage
Expand Down Expand Up @@ -670,7 +670,7 @@ describe('<ToolMessage />', () => {
merged: { ui: { shellOutputMaxLines: 12 } },
} as unknown as LoadedSettings;
const { lastFrame } = render(
<CompactModeProvider value={{ compactMode: false }}>
<CompactModeProvider value={{ compactMode: false, compactInline: false }}>
<SettingsContext.Provider value={settingsWithCustomCap}>
<StreamingContext.Provider value={StreamingState.Idle}>
<ToolMessage
Expand Down Expand Up @@ -815,7 +815,7 @@ describe('<ToolMessage />', () => {
merged: { ui: { shellOutputMaxLines: badValue } },
} as unknown as LoadedSettings;
const { lastFrame } = render(
<CompactModeProvider value={{ compactMode: false }}>
<CompactModeProvider value={{ compactMode: false, compactInline: false }}>
<SettingsContext.Provider value={settingsWithBadCap}>
<StreamingContext.Provider value={StreamingState.Idle}>
<ToolMessage
Expand Down
Loading
Loading