Skip to content
1 change: 1 addition & 0 deletions packages/cli/src/ui/components/DialogManager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ export const DialogManager = ({
isFastModelMode={uiState.isFastModelMode}
isVoiceModelMode={uiState.isVoiceModelMode}
isVisionModelMode={uiState.isVisionModelMode}
availableTerminalHeight={listDialogHeight}
/>
);
}
Expand Down
125 changes: 124 additions & 1 deletion packages/cli/src/ui/components/ModelDialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { render, cleanup } from '@testing-library/react';
import { render, cleanup, act } from '@testing-library/react';
import process from 'node:process';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { ModelDialog, encodeAuxModelSelector } from './ModelDialog.js';
Expand Down Expand Up @@ -151,6 +151,129 @@ describe('<ModelDialog />', () => {
expect(props.showNumbers).toBe(true);
});

it('caps visible model options to the available dialog height', () => {
Comment thread
tanzhenxin marked this conversation as resolved.
renderComponent(
{ availableTerminalHeight: 20 },
{
getModel: vi.fn(() => 'model-1'),
getAuthType: vi.fn(() => AuthType.USE_OPENAI),
getAllConfiguredModels: vi.fn(() =>
Array.from({ length: 12 }, (_, i) => ({
id: `model-${i + 1}`,
label: `Model ${i + 1}`,
description: '',
authType: AuthType.USE_OPENAI,
})),
),
},
);

const props = mockedSelect.mock.calls[0][0];
expect(props.items).toHaveLength(12);
expect(props.maxItemsToShow).toBe(6);
// The picker deliberately leaves the ▲/▼ scroll indicators off: they are
// two always-rendered chrome rows better spent on two more entries.
expect(props.showScrollArrows).toBeUndefined();
});

it('floors visible model options to 1 when the terminal is very short', () => {
renderComponent(
{ availableTerminalHeight: 5 },
{
getModel: vi.fn(() => 'model-1'),
getAuthType: vi.fn(() => AuthType.USE_OPENAI),
getAllConfiguredModels: vi.fn(() =>
Array.from({ length: 12 }, (_, i) => ({
id: `model-${i + 1}`,
label: `Model ${i + 1}`,
description: '',
authType: AuthType.USE_OPENAI,
})),
),
},
);

const props = mockedSelect.mock.calls[0][0];
expect(props.maxItemsToShow).toBe(1);
});

it('accounts for the taller two-row option height when descriptions are present', () => {
renderComponent(
{ availableTerminalHeight: 20 },
{
getModel: vi.fn(() => 'model-1'),
getAuthType: vi.fn(() => AuthType.USE_OPENAI),
getAllConfiguredModels: vi.fn(() =>
Array.from({ length: 12 }, (_, i) => ({
id: `model-${i + 1}`,
label: `Model ${i + 1}`,
description: `Description ${i + 1}`,
authType: AuthType.USE_OPENAI,
})),
),
},
);

const props = mockedSelect.mock.calls[0][0];
expect(props.maxItemsToShow).toBe(3);
});

it('falls back to the default max item count when no terminal height is given', () => {
renderComponent(
{},
{
getModel: vi.fn(() => 'model-1'),
getAuthType: vi.fn(() => AuthType.USE_OPENAI),
getAllConfiguredModels: vi.fn(() =>
Array.from({ length: 12 }, (_, i) => ({
id: `model-${i + 1}`,
label: `Model ${i + 1}`,
description: '',
authType: AuthType.USE_OPENAI,
})),
),
},
);

const props = mockedSelect.mock.calls[0][0];
expect(props.maxItemsToShow).toBe(10);
});

it('shrinks visible model options to leave room for a displayed error message', async () => {
const switchModel = vi.fn().mockRejectedValue(new Error('network down'));

renderComponent(
{ availableTerminalHeight: 20 },
{
getModel: vi.fn(() => 'model-1'),
getAuthType: vi.fn(() => AuthType.USE_OPENAI),
switchModel,
getAllConfiguredModels: vi.fn(() =>
Array.from({ length: 12 }, (_, i) => ({
id: `model-${i + 1}`,
label: `Model ${i + 1}`,
description: '',
authType: AuthType.USE_OPENAI,
})),
),
},
);

const initialProps = mockedSelect.mock.calls[0][0];
expect(initialProps.maxItemsToShow).toBe(6);

await act(async () => {
await initialProps.onSelect(initialProps.items[0].value);
});

const propsAfterError =
mockedSelect.mock.calls[mockedSelect.mock.calls.length - 1][0];
// errorMessage = "Failed to switch model to 'model-1'.\n\nnetwork down"
// (3 lines) -> errorMessageRows = 2 + 3 = 5 ->
// max(1, floor((20 - 14 - 5) / 1)) = 1.
expect(propsAfterError.maxItemsToShow).toBe(1);
});
Comment thread
tanzhenxin marked this conversation as resolved.

it('hides discontinued qwen-oauth models for other auth types', () => {
renderComponent(
{},
Expand Down
51 changes: 49 additions & 2 deletions packages/cli/src/ui/components/ModelDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,23 @@ interface ModelDialogProps {
isFastModelMode?: boolean;
isVoiceModelMode?: boolean;
isVisionModelMode?: boolean;
availableTerminalHeight?: number;
}

const MAX_MODEL_ITEMS_TO_SHOW = 10;
// Non-list dialog chrome to reserve when capping visible model rows: outer
// round border (2) + outer padding (2) + title (1) + gap before the list (1)
// + highlighted-entry detail panel (divider + up to 4 detail rows, ~6) +
// footer gap and hint text (2). The list intentionally omits the ▲/▼ scroll
// indicators other list dialogs enable: they are two always-rendered chrome
// rows, and in a height-capped dialog those rows are better spent on two
// more entries — the entry numbering already shows where the visible window
// sits in the list. Adjust this whenever the surrounding layout changes, and
// re-verify with an E2E height sweep rather than guessing.
const MODEL_DIALOG_FIXED_ROWS = 14;
Comment thread
tanzhenxin marked this conversation as resolved.
Comment thread
tanzhenxin marked this conversation as resolved.
const MODEL_OPTION_ROW_HEIGHT = 1;
const MODEL_OPTION_ROW_HEIGHT_WITH_DESCRIPTION = 2;

function maskApiKey(apiKey: string | undefined): string {
if (!apiKey) return `(${t('not set')})`;
const trimmed = apiKey.trim();
Expand Down Expand Up @@ -239,6 +254,7 @@ export function ModelDialog({
isFastModelMode,
isVoiceModelMode,
isVisionModelMode,
availableTerminalHeight,
}: ModelDialogProps): React.JSX.Element {
const config = useContext(ConfigContext);
const uiState = useContext(UIStateContext);
Expand Down Expand Up @@ -379,12 +395,41 @@ export function ModelDialog({
),
[availableModelEntries],
);
const modelOptionRowHeight = MODEL_OPTIONS.some(
Comment thread
tanzhenxin marked this conversation as resolved.
Comment thread
tanzhenxin marked this conversation as resolved.
({ description }) =>
typeof description !== 'string' || description.trim().length > 0,
)
? MODEL_OPTION_ROW_HEIGHT_WITH_DESCRIPTION
: MODEL_OPTION_ROW_HEIGHT;
// The error box adds its own marginTop plus one row per line (errorPrefix +
// blank line from the "\n\n" join + the underlying error's own lines), plus
// a buffer since the error Text wraps and long lines can span extra rows on
// narrow terminals.
const errorMessageRows = errorMessage
? 2 + errorMessage.split('\n').length
: 0;
const maxModelItemsToShow =
availableTerminalHeight === undefined
? MAX_MODEL_ITEMS_TO_SHOW
: Math.max(
1,
Math.min(
MAX_MODEL_ITEMS_TO_SHOW,
Math.floor(
(availableTerminalHeight -
MODEL_DIALOG_FIXED_ROWS -
errorMessageRows) /
modelOptionRowHeight,
),
),
);

// In fast model mode, default to the currently configured fast model
const fastModelSetting = settings?.merged?.fastModel as string | undefined;
const voiceModelSetting = settings?.merged?.voiceModel as string | undefined;
const visionModelSetting = settings?.merged?.visionModel as
string | undefined;
| string
| undefined;
const parsedVisionModelValue = parseVisionModelSetting(visionModelSetting);
const parsedFastModelSetting = useMemo(() => {
if (!isFastModelMode) return undefined;
Expand Down Expand Up @@ -704,7 +749,8 @@ export function ModelDialog({
}

after = config.getContentGeneratorConfig?.() as
ContentGeneratorConfig | undefined;
| ContentGeneratorConfig
| undefined;
effectiveAuthType = after?.authType ?? selectedAuthType ?? authType;
effectiveModelId = after?.model ?? modelId;
} catch (e) {
Expand Down Expand Up @@ -800,6 +846,7 @@ export function ModelDialog({
onHighlight={handleHighlight}
initialIndex={initialIndex}
showNumbers={true}
maxItemsToShow={maxModelItemsToShow}
Comment thread
tanzhenxin marked this conversation as resolved.
/>
</Box>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { Text } from 'ink';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { renderWithProviders } from '../../../test-utils/render.js';
import {
Expand Down Expand Up @@ -94,4 +95,40 @@ describe('DescriptiveRadioButtonSelect', () => {
});
expect(lastFrame()).toMatchSnapshot();
});

it('does not render empty description rows', () => {
const { lastFrame } = renderComponent({
items: [
{
title: 'Foo Title',
description: '',
value: 'foo',
key: 'foo',
},
{
title: 'Bar Title',
description: ' ',
value: 'bar',
key: 'bar',
},
],
});

expect(lastFrame()).toBe('› Foo Title\n Bar Title');
});

it('renders a non-string (ReactNode) description', () => {
const { lastFrame } = renderComponent({
items: [
{
title: 'Foo Title',
description: <Text>Custom node description</Text>,
value: 'foo',
key: 'foo',
},
],
});

expect(lastFrame()).toBe('› Foo Title\n Custom node description');
});
Comment thread
tanzhenxin marked this conversation as resolved.
});
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,24 @@ export function DescriptiveRadioButtonSelect<T>({
showScrollArrows={showScrollArrows}
maxItemsToShow={maxItemsToShow}
itemGap={itemGap}
renderItem={(item, { titleColor }) => (
<Box flexDirection="column" key={item.key}>
<Text color={titleColor}>{item.title}</Text>
{typeof item.description === 'string' ? (
<Text color={theme.text.secondary}>{item.description}</Text>
) : (
item.description
)}
</Box>
)}
renderItem={(item, { titleColor }) => {
const hasDescription =
typeof item.description === 'string'
? item.description.trim().length > 0
: item.description != null;

return (
<Box flexDirection="column" key={item.key}>
<Text color={titleColor}>{item.title}</Text>
{hasDescription &&
(typeof item.description === 'string' ? (
<Text color={theme.text.secondary}>{item.description}</Text>
) : (
item.description
))}
</Box>
);
}}
/>
);
}
Loading