fix(cli): Keep model picker entries contiguous in short terminals - #6359
Merged
Conversation
Collaborator
Author
E2E test reportVerified against the local bundle on macOS using direct PTY runs of
Result: VERIFIED_FIXED. |
tanzhenxin
enabled auto-merge
July 6, 2026 03:45
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: this is an observed, reproduced bug. The PR includes before/after screenshots at Direction: aligned. The model picker is a core interactive CLI feature, and entries disappearing in constrained terminals is a real usability issue. No CHANGELOG reference needed — this is a straightforward bug fix. Approach: scope feels right and minimal. Two small, complementary changes: (1) calculate Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个已观测到的、有复现的 bug。PR 提供了 方向：对齐。模型选择器是 CLI 的核心交互功能，在受限终端中条目消失是真实的可用性问题。CHANGELOG 无需参考——这是一个直接的 bug 修复。 方案：范围合理且最小化。两个小而互补的改动：(1) 渲染前根据可用终端高度计算 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewClean implementation. Two observations: Height calculation is conservative (not a blocker).
No correctness bugs, no security concerns, no AGENTS.md violations. TestingUnit tests: PASS — 31/31 tests pass across Tmux real-scenario testing: partial. Launched the bundled CLI ( The multi-model clipping scenario (the actual bug) could not be reproduced — the CI environment has only 1 configured model, so there are no entries to clip. The author's E2E report provides sweep data across heights 14–30 with 10+ models showing contiguous entries, which covers the fix adequately. Build + bundle: PASS. 中文说明代码审查实现干净。两个观察： 高度计算偏保守（非阻断性）。
无正确性 bug、无安全问题、无 AGENTS.md 违规。 测试单元测试：通过 — Tmux 真实场景测试：部分完成。 在 多模型裁剪场景（实际的 bug）无法复现——CI 环境只有 1 个已配置模型，没有可被裁剪的条目。作者的 E2E 报告提供了 14-30 行高度下 10+ 个模型连续显示的扫描数据，充分覆盖了修复效果。 构建 + 打包：通过。 — Qwen Code · qwen3.7-max |
Collaborator
|
This is a clean, well-scoped bug fix. The problem is real — non-contiguous picker entries in short terminals — and the before/after evidence is convincing. The implementation does exactly what's needed: size the visible window from available height, skip blank description rows. No more, no less. My independent proposal before reading the diff would have been the same two changes. The conservative height calculation (uniform row height based on whether any model has a description) is the right tradeoff — a per-item calculation would be more complex for negligible user-visible benefit with ≤10 items. Unit tests pass, build passes, author's E2E sweep covers the fix across terminal heights. The only gap in my own testing is that the CI environment has a single configured model, so I couldn't independently reproduce the multi-model clipping — but the author's evidence is thorough. Approved. ✅ 中文说明这是一个干净、范围合理的 bug 修复。问题是真实存在的——短终端中选择器条目不连续——before/after 证据有说服力。实现做了恰好需要的事：根据可用高度计算可见窗口大小，跳过空白描述行。不多不少。 我在阅读 diff 之前的独立方案也是同样的两个改动。保守的高度计算（根据是否有任一模型具有描述来统一行高）是正确的权衡——对 ≤10 个条目的选择器来说，逐条目计算的复杂度收益比不合理。 单元测试通过，构建通过，作者的 E2E 扫描覆盖了多个终端高度的修复效果。我自己测试的唯一缺口是 CI 环境只有 1 个已配置模型，无法独立复现多模型裁剪——但作者的证据是充分的。 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously approved these changes
Jul 6, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
The model list's row budget didn't reserve space for the inline error message shown after a failed switch, so it could still overflow a short terminal in that state. Also cover the capping formula's untested branches (floor at very small heights, the two-row description path, the undefined-height fallback) and the DescriptiveRadioButtonSelect ReactNode description path introduced by the same change.
errorMessageRows only counted explicit newlines, undercounting rows when the error Text wraps on narrow terminals. Add a small buffer and tighten the regression test's assertion to the exact expected value.
Short terminals can now cap the model list well below its old worst case of 10, hiding most entries with no indicator that the list scrolls (unlike ThemeDialog, ApprovalModeDialog, and ArenaStartDialog, which already show scroll arrows). Enable them here too, and reserve the 2 extra chrome rows they add. Also document the fixed-rows budget so future layout changes know to keep it in sync.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
…ntries The scroll arrows are two always-rendered chrome rows, so on dialogs too short to fit them plus a single option row they pushed the option rows past the dialog's clipped height — the picker showed arrows, title, and footer but no entries. Hide the arrows in that case and spend their rows on the list instead. Verified with an E2E height sweep (rows 14-34): at least one entry is now visible at every height and windows stay contiguous, with arrows still shown wherever they fit.
The ▲/▼ indicators are two always-rendered chrome rows, and in a height-capped dialog those rows are the scarcest resource — enabling them cost two visible entries at every constrained height and required extra logic to avoid crowding out the list entirely on very short dialogs. Remove them and restore the 14-row chrome budget: the entry numbering already shows where the visible window sits in the list, and the footer hint covers navigation. Supersedes the earlier change that enabled the arrows.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
@qwen-code /review |
Contributor
| _Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/28791358694)._ |
Collaborator
Author
|
@qwen-code /review |
qwen-code-ci-bot
approved these changes
Jul 6, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
Clean, well-tested fix for the model picker contiguous-entries bug. The height-calculation formula is correct across all boundary conditions, tests cover the key branches (short terminal floor, tall terminal cap, description row height, error-state shrinking), and the DescriptiveRadioButtonSelect change correctly suppresses empty description rows. All 37 tests pass, typecheck and lint clean.
— qwen3.7-max via Qwen Code /review
wenshao
approved these changes
Jul 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR makes the model picker size its visible option window from the available dialog height before rendering, and it avoids rendering blank description rows for options that do not have descriptions. In short terminals, the picker now shows a smaller contiguous window instead of letting the outer terminal-height clamp hide every other row. The row budget also reserves space for the inline error message shown after a failed switch, so the dialog does not overflow again in that state. The picker deliberately 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.
Why it's needed
When
/modelwas opened in a constrained terminal, the picker could display non-contiguous entries such as2, 4, 6, 8, 10, making it look like some models were missing from the selectable list. The issue happened because the list rendered more physical rows than the dialog could display, so overflow clipping removed rows after the list had already chosen which logical items to show.Reviewer Test Plan
How to verify
Open
/modelin a short terminal and confirm that the visible model numbers are contiguous. At very small heights, only a small subset may be visible, but it should be ordered without missing entries; at taller heights, the picker should expand up to the normal ten-entry window. There are no scroll indicators; the entry numbers show where the window sits and the footer hint covers navigation.Evidence (Before & After)
Evidence re-captured after the review-round changes, with eight configured models.
Before: with a 24-row terminal, the visible picker entries were
1, 2, 4, 5, 6, 7, 8— entry3was missing — and two rows of the details panel were overlapped into one corrupted line.After: with the same 24-row terminal, the picker shows a contiguous
1, 2, 3, 4, 5window with an intact details panel. A height sweep with the bundled CLI at terminal heights14, 20, 21, 22, 23, 24, 25, 26, 29, 30, 34showed a contiguous window at every height, growing from one visible entry at14to all eight from29up.Tested on
Environment (optional)
Local verification used
node dist/cli.jsin a direct PTY script with constrained terminal row counts.Risk & Scope
Linked Issues
User-reported issue; no GitHub issue number is attached.
中文说明
What this PR does
这个 PR 会在渲染前根据可用的对话框高度计算模型选择器可显示的选项数量，并且不再为没有描述内容的选项渲染空白描述行。在较矮的终端里，模型选择器现在会显示一个更小但连续的窗口，而不是让外层高度裁剪隐藏掉部分行。行数预算同时为切换失败后显示的内联错误信息预留了空间，避免该状态下对话框再次溢出。选择器有意不启用其他列表对话框使用的 ▲/▼ 滚动指示符：它们是两行始终渲染的装饰行，在高度受限的对话框里这两行更值得用来多显示两个条目——条目编号本身已经能标示可见窗口在列表中的位置。
Why it's needed
当在受限高度的终端中打开
/model时，模型选择器可能显示不连续的条目，例如2, 4, 6, 8, 10，看起来像是有些模型从可选列表中丢失了。问题的原因是列表渲染出的实际行数超过了对话框可以显示的高度，导致外层溢出裁剪在列表已经选定逻辑条目之后移除了部分物理行。Reviewer Test Plan
How to verify
在较矮的终端中打开
/model，确认可见的模型编号是连续的。在非常小的高度下，可能只显示少量条目，但它们应该保持有序且不会缺号；在更高的终端中，选择器应扩展到正常的十个条目窗口。没有滚动指示符；条目编号标示窗口位置，页脚提示覆盖了导航操作。Evidence (Before & After)
以下证据是在评审轮次的修改之后重新采集的，配置了八个模型。
修复前：在 24 行终端中，可见的模型条目为
1, 2, 4, 5, 6, 7, 8，缺少条目3，且详情面板有两行被挤压重叠成一行乱码。修复后：在相同的 24 行终端中，选择器显示连续的
1, 2, 3, 4, 5窗口，详情面板完整。使用打包后的 CLI 在终端高度14, 20, 21, 22, 23, 24, 25, 26, 29, 30, 34下做高度扫描，每个高度都显示连续的可见窗口，从14行时的一个条目逐步扩展到29行及以上的全部八个条目。截图见上方英文部分的前后对比表。Tested on
Environment (optional)
本地验证使用
node dist/cli.js，通过直接 PTY 脚本设置受限终端行数。Risk & Scope
Linked Issues
用户报告的问题；目前没有关联的 GitHub issue 编号。