Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
3 changes: 2 additions & 1 deletion packages/cli/src/ui/components/AppHeader.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const createSettings = (options?: {
const createMockConfig = (overrides = {}) => ({
getContentGeneratorConfig: vi.fn(() => ({ authType: undefined })),
getModel: vi.fn(() => 'gemini-pro'),
getModelDisplayName: vi.fn(() => 'Gemini Pro'),
getTargetDir: vi.fn(() => '/projects/qwen-code'),
getMcpServers: vi.fn(() => ({})),
getBlockedMcpServers: vi.fn(() => []),
Expand Down Expand Up @@ -106,7 +107,7 @@ describe('<AppHeader />', () => {
it('shows the header with all info when banner is visible', () => {
const { lastFrame } = renderWithProviders(createMockUIState());
expect(lastFrame()).toContain('>_ Qwen Code');
expect(lastFrame()).toContain('gemini-pro');
expect(lastFrame()).toContain('Gemini Pro');
expect(lastFrame()).toContain('/projects/qwen-code');
});

Expand Down
5 changes: 1 addition & 4 deletions packages/cli/src/ui/components/AppHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { Header, AuthDisplayType } from './Header.js';
import { Tips } from './Tips.js';
import { useSettings } from '../contexts/SettingsContext.js';
import { useConfig } from '../contexts/ConfigContext.js';
import { useUIState } from '../contexts/UIStateContext.js';
import { resolveCustomBanner } from '../utils/customBanner.js';

interface AppHeaderProps {
Expand Down Expand Up @@ -50,11 +49,9 @@ function getAuthDisplayType(
export const AppHeader = ({ version }: AppHeaderProps) => {
const settings = useSettings();
const config = useConfig();
const uiState = useUIState();

const contentGeneratorConfig = config.getContentGeneratorConfig();
const authType = contentGeneratorConfig?.authType;
const model = uiState.currentModel;
const model = config.getModelDisplayName();
const targetDir = config.getTargetDir();
const showBanner =
!config.getScreenReader() && !settings.merged.ui?.hideBanner;
Expand Down
9 changes: 9 additions & 0 deletions packages/cli/src/ui/components/ModelDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,15 @@ export function ModelDialog({
[{t2}]
</Text>
<Text>{` ${model.label}`}</Text>
{model.id !== model.label && (
<Text color={theme.text.secondary} italic>
{' '}
({model.id})
</Text>
)}
{' '}
({model.id})
Comment thread
zzhenyao marked this conversation as resolved.
Outdated
</Text>
{isRuntime && (
<Text color={theme.status.warning}> (Runtime)</Text>
)}
Expand Down
5 changes: 3 additions & 2 deletions packages/cli/src/ui/components/StatusLineDialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ function createSettings(): LoadedSettings {
const config = {
getCliVersion: () => '1.2.3',
getModel: () => 'qwen3-code-plus',
getModelDisplayName: () => 'Qwen3 Code Plus',
getTargetDir: () => '/repo/project',
getContentGeneratorConfig: () => ({
contextWindowSize: 1000,
Expand Down Expand Up @@ -106,7 +107,7 @@ describe('StatusLineDialog', () => {
frame.indexOf('current-dir'),
);
expect(lastFrame()).toContain('Preview');
expect(lastFrame()).toContain('qwen3-code-plus high');
expect(lastFrame()).toContain('Qwen3 Code Plus high');
});

it('persists selected presets on enter', async () => {
Expand Down Expand Up @@ -187,7 +188,7 @@ describe('StatusLineDialog', () => {
await press(' ');

expect(lastFrame()).toContain(
'qwen3-code-plus high | feature/pr-4087-statusline | Context 75% left',
'Qwen3 Code Plus high | feature/pr-4087-statusline | Context 75% left',
);

await press('\r');
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/ui/components/StatusLineDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ function getPreviewData(config: Config, uiState: UIState) {
return buildStatusLinePresetData({
sessionId: stats.sessionId,
version: config.getCliVersion(),
modelDisplayName: uiState.currentModel || config.getModel(),
modelDisplayName: config.getModelDisplayName(),
reasoning: contentGeneratorConfig?.reasoning,
currentDir: config.getTargetDir(),
branch: uiState.branchName,
Expand Down
19 changes: 10 additions & 9 deletions packages/cli/src/ui/hooks/useStatusLine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ const getMockContentGeneratorConfig = (): MockContentGeneratorConfig => ({
const mockConfig = {
getTargetDir: vi.fn(() => '/test/dir'),
getModel: vi.fn(() => 'test-model'),
getModelDisplayName: vi.fn(() => 'Test Model'),
getCliVersion: vi.fn(() => '1.0.0'),
getContentGeneratorConfig: vi.fn(getMockContentGeneratorConfig),
};
Expand Down Expand Up @@ -282,7 +283,7 @@ describe('useStatusLine', () => {
const { result } = renderHook(() => useStatusLine());

expect(result.current.useThemeColors).toBe(true);
expect(result.current.lines).toEqual(['test-model']);
expect(result.current.lines).toEqual(['Test Model']);
});

it('looks up the current branch pull request number with gh', async () => {
Expand Down Expand Up @@ -315,7 +316,7 @@ describe('useStatusLine', () => {
const { result } = renderHook(() => useStatusLine());

expect(child_process.exec).not.toHaveBeenCalled();
expect(result.current.lines).toEqual(['test-model']);
expect(result.current.lines).toEqual(['Test Model']);
});

it('renders model-with-reasoning and model-only together', () => {
Expand All @@ -330,7 +331,7 @@ describe('useStatusLine', () => {
const { result } = renderHook(() => useStatusLine());

expect(child_process.exec).not.toHaveBeenCalled();
expect(result.current.lines).toEqual(['test-model high | test-model']);
expect(result.current.lines).toEqual(['Test Model high | Test Model']);
});

it('refreshes when status line settings are saved in the same process', async () => {
Expand All @@ -342,7 +343,7 @@ describe('useStatusLine', () => {
const { result, rerender } = renderHook(() => useStatusLine());

expect(child_process.exec).not.toHaveBeenCalled();
expect(result.current.lines).toEqual(['test-model']);
expect(result.current.lines).toEqual(['Test Model']);

setStatusLineConfig({
type: 'preset',
Expand All @@ -365,7 +366,7 @@ describe('useStatusLine', () => {
vi.advanceTimersByTime(300);
});

expect(result.current.lines).toEqual(['test-model | #4118']);
expect(result.current.lines).toEqual(['Test Model | #4118']);
});

it('reloads status line settings from disk when streaming becomes idle', async () => {
Expand All @@ -375,7 +376,7 @@ describe('useStatusLine', () => {
});
const { result, rerender } = renderHook(() => useStatusLine());

expect(result.current.lines).toEqual(['test-model']);
expect(result.current.lines).toEqual(['Test Model']);

mockSettings.reloadScopeFromDisk.mockImplementationOnce(() => {
setStatusLineConfig({
Expand All @@ -397,7 +398,7 @@ describe('useStatusLine', () => {
});

expect(mockSettings.reloadScopeFromDisk).toHaveBeenCalledOnce();
expect(result.current.lines).toEqual(['test-model high']);
expect(result.current.lines).toEqual(['Test Model high']);
});

it('uses command settings when a stale preset override no longer matches the settings type', () => {
Expand Down Expand Up @@ -582,7 +583,7 @@ describe('useStatusLine', () => {
const input = JSON.parse(stdinWrittenData);
expect(input.session_id).toBe('test-session');
expect(input.version).toBe('1.0.0');
expect(input.model.display_name).toBe('test-model');
expect(input.model.display_name).toBe('Test Model');
expect(input.workspace.current_dir).toBe('/test/dir');
});

Expand Down Expand Up @@ -687,7 +688,7 @@ describe('useStatusLine', () => {
renderHook(() => useStatusLine());

const input = JSON.parse(stdinWrittenData);
expect(input.model.display_name).toBe('test-model');
expect(input.model.display_name).toBe('Test Model');
});
});

Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/ui/hooks/useStatusLine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ export function useStatusLine(): {
const data = buildStatusLinePresetData({
sessionId: stats.sessionId,
version: cfg.getCliVersion(),
modelDisplayName: ui.currentModel || cfg.getModel(),
modelDisplayName: cfg.getModelDisplayName(),
reasoning: contentGeneratorConfig?.reasoning,
currentDir,
branch: ui.branchName,
Expand Down Expand Up @@ -444,7 +444,7 @@ export function useStatusLine(): {
session_id: stats.sessionId,
version: cfg.getCliVersion() || 'unknown',
model: {
display_name: ui.currentModel || cfg.getModel() || 'unknown',
display_name: cfg.getModelDisplayName(),
},
context_window: {
context_window_size: contextWindowSize,
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2199,6 +2199,16 @@ export class Config {
);
}

/**
* Get the human-readable display name for the currently selected model.
* Resolves the model id to its name from the model registry.
* Falls back to the raw model id when the model is not found.
*/
getModelDisplayName(): string {
const modelId = this.getModel();
return modelId ? this.modelsConfig.getModelDisplayName(modelId) : 'unknown';

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 modelId ? ... : 'unknown' guard is unreachable dead code — this.getModel() always returns a truthy string because ModelsConfig.getModel() falls back to DEFAULT_QWEN_MODEL ('coder-model'). The 'unknown' branch can never execute.

This creates a misleading impression that a "no model" scenario is handled when it cannot actually occur. Consider either simplifying to return this.modelsConfig.getModelDisplayName(this.getModel()); or adding a unit test that documents the intended contract.

— qwen3.7-max via Qwen Code /review

}

onModelChange(listener: (model: string) => void): () => void {
this.modelChangeListeners.add(listener);
return () => {
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/models/modelsConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,18 @@ export class ModelsConfig {
return this.modelRegistry.getModel(authType, modelId);
}

/**
* Get the display name for a model by its id.
* Looks up the model in the registry using the current authType and returns
* its resolved name. Falls back to the raw model id when the model is not
* found in the registry (e.g. runtime models or unknown models).
*/
getModelDisplayName(modelId: string): string {

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] ModelsConfig.getModelDisplayName() and Config.getModelDisplayName() have zero unit tests. The consumer tests (AppHeader, StatusLineDialog, useStatusLine) mock the method entirely with vi.fn(() => '...'), so the actual resolution logic is untested.

Three branches in this method are uncovered: (1) model found in registry → returns resolved.name, (2) currentAuthType falsy → returns raw modelId, (3) model not found → returns raw modelId. Same applies to the Config wrapper's 'unknown' fallback.

Consider adding describe('getModelDisplayName') blocks in both modelsConfig.test.ts and config.test.ts.

— qwen3.7-max via Qwen Code /review

if (!this.currentAuthType) return modelId;
const resolved = this.modelRegistry.getModel(this.currentAuthType, modelId);
return resolved?.name ?? modelId;
}

/**
* Set model programmatically (e.g., VLM auto-switch, fallback).
* Supports both registry models and raw model IDs.
Expand Down
Loading