fix(ui): display model name instead of id in statusline and startup banner - #4741
Conversation
|
@pomelo-nwu Hi, I've implemented the fix for #4722 Issue 1 (display bug). The PR adds |
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Local Verification Report — PR #4741Tested on: macOS Darwin 25.4.0 (Apple Silicon) Test Results Summary
Unit tests: 78 passed, 0 failures. But build and lint fail. Build-Breaking IssueCommit Current (broken): {model.id !== model.label && (
<Text color={theme.text.secondary} italic>
{' '}
({model.id})
</Text>
)}
{' '} // ← orphaned lines, should be deleted
({model.id}) // ← orphaned lines, should be deleted
</Text> // ← orphaned lines, should be deletedFix — remove lines 299-301 in {model.id !== model.label && (
<Text color={theme.text.secondary} italic>
{' '}
({model.id})
</Text>
)}
{isRuntime && (This is the only issue found. The core logic ( ConclusionNot merge-ready — please remove the 3 orphaned lines in Verified locally by wenshao |
| * 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 { |
There was a problem hiding this comment.
[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
| */ | ||
| getModelDisplayName(): string { | ||
| const modelId = this.getModel(); | ||
| return modelId ? this.modelsConfig.getModelDisplayName(modelId) : 'unknown'; |
There was a problem hiding this comment.
[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
Verification ReportReviewer: wenshao Build
Note: Root-level Tests
Broader UI suite: 3615/3632 passed, 17 failures in Code Review
Verdict✅ LGTM — ready to merge. Pure display-layer change with safe fallback semantics. All 341 PR-affected tests pass, TypeScript and ESLint clean, no regressions introduced. |
Local Verification Report (Round 2) — PR #4741Tested on: macOS Darwin 25.4.0 (Apple Silicon) Previous Issue — ResolvedThe syntax error in Test Results Summary
Total: 348 tests passed, 0 failures. All checks green. Changes Since Round 1The author addressed feedback with 3 new commits:
ConclusionPR is now merge-ready. The syntax error from Round 1 is fixed, new unit tests cover the Verified locally by wenshao |
|
@wenshao Thanks for the review! All R2 comments addressed:
|
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No high-confidence issues found. LGTM! ✅ All R2 feedback addressed. 333 tests pass, tsc/eslint clean. — qwen3.7-max via Qwen Code /review
|
@wenshao Appreciate the approval!🙏 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Clean display-layer fix — getModelDisplayName() resolves the model id to its registry name at the UI boundary without touching any business logic. The fallback-to-raw-id design covers runtime and unregistered models gracefully. Test coverage looks solid across all three branches.
One minor nit (non-blocking): the modelId ? ... : 'unknown' ternary in Config.getModelDisplayName() is dead code since getModel() always returns a truthy string via DEFAULT_QWEN_MODEL. Could simplify to a direct call.
yiliang114
added
category/ui
3 participants
What this PR does
Adds
getModelDisplayName()toModelsConfigandConfigthat resolves the current model id to its human-readable display name from the model registry, falling back to the raw id when not found. Replacesui.currentModel || cfg.getModel()withcfg.getModelDisplayName()in the statusline (preset and command mode), the statusline preview dialog, and the startup banner. The model selection dialog now shows the model id in italic parentheses after the model name.Why it's needed
The statusline and startup banner displayed the raw model id (e.g.
qwen3-coder-plus) instead of the model's display name. The data flow was a pure-string pipeline —getModel()returns only a bare string id, and thename/labelfield was never threaded to the UI layer. This PR adds name resolution at the display layer without changing any existing API or business logic.Fixes: #4722
Reviewer Test Plan
Evidence (Before & After)
Before:
After:
How to verify
Build and run
Statusline shows model display name instead of id
Qwen3 Coder Plusinstead ofqwen3-coder-plus)Startup banner shows model display name
API Key | Qwen3 Coder Plus) should show the display name, not the raw idModel selection dialog shows name with id suffix
/modelto open the model selection dialogname (id)with the id in italicRegression
/modelcommand, settings persistence: all work as before (these use model id internally, unchanged)Tested on
Risk & Scope
getModelDisplayName()is a new method; existinggetModel()and all business logic remain unchanged. Falls back to raw id if model not found in registry.Linked Issues
中文说明
新增
getModelDisplayName()方法，将 model id 解析为 model name 显示。statusline 和启动 banner 两处改为显示 model name（原为 model id）。模型选择界面在 name 后追加斜体显示 model id。本次改动仅在显示层增加 name 解析，不修改任何现有 API 或业务逻辑。
getModel()返回值不变，ui.currentModel类型和值不变，模型切换、设置持久化、provider lookup 等所有业务逻辑不受影响。