Skip to content

fix(cli): keep long model stats header on one line#4032

Merged
tanzhenxin merged 2 commits into
QwenLM:mainfrom
Jerry2003826:codex/fix-model-stats-long-model-header
May 11, 2026
Merged

fix(cli): keep long model stats header on one line#4032
tanzhenxin merged 2 commits into
QwenLM:mainfrom
Jerry2003826:codex/fix-model-stats-long-model-header

Conversation

@Jerry2003826

Copy link
Copy Markdown
Contributor

Fixes #4031

Summary

  • Expand the model/source column when /stats model renders a single model and the panel has available width.
  • Preserve the existing compact 24-column layout for multi-model or multi-source stats.
  • Add regression coverage for a long model name that previously wrapped despite enough terminal space.

Root Cause

ModelStatsDisplay always used a fixed 24-column model header/value column. Long model IDs such as ggml-org/gemma-4-E4B-it-GGUF therefore wrapped inside the column even when the surrounding stats panel still had unused horizontal space.

Validation

  • npm run test --workspace=packages/cli -- src/ui/components/ModelStatsDisplay.test.tsx -t "long single model name"
  • npm run test --workspace=packages/cli -- src/ui/components/ModelStatsDisplay.test.tsx
  • npm run lint --workspace=packages/cli
  • npm run typecheck --workspace=packages/cli
  • npx prettier --check packages/cli/src/ui/components/ModelStatsDisplay.tsx packages/cli/src/ui/components/ModelStatsDisplay.test.tsx

@Jerry2003826 Jerry2003826 marked this pull request as ready for review May 10, 2026 19:51
@qqqys

qqqys commented May 11, 2026

Copy link
Copy Markdown
Collaborator

LGTM. No blocking issues.

I reviewed the current PR diff against the latest main; the effective scope is limited to:

  • packages/cli/src/ui/components/ModelStatsDisplay.tsx
  • packages/cli/src/ui/components/ModelStatsDisplay.test.tsx

The implementation looks correctly scoped: it only expands the model column when there is a single flattened model/source entry and a container width is available, while preserving the existing fixed-width behavior for multi-model / multi-source layouts.

Non-blocking suggestions

  • Consider adding one more regression test for the multi-model or multi-source case with a wide width, asserting that it still uses the fixed MODEL_COL_WIDTH behavior rather than expanding every column.
  • The PANEL_HORIZONTAL_CHROME_WIDTH = 6 calculation is correct for the current border + padding setup, but it would be useful to mention in the comment that it must stay in sync with the surrounding Box borderStyle and paddingX.

I attempted local checks, but this worktree does not have a complete dependency install, so test/typecheck execution was blocked by missing dependencies rather than this PR's code.

@tanzhenxin tanzhenxin added the type/bug Something isn't working as expected label May 11, 2026

@tanzhenxin tanzhenxin left a comment

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.

Review

Correctly expands the model/source column dynamically when /stats model renders a single entry, while preserving the compact 24-column layout for multi-model and multi-source displays. The Math.max(24, …) floor keeps narrow terminals working, the truthy-width check skips the dynamic branch when width is undefined or 0, and the new regression test would have caught the original wrap at width=100 — it's an actual test of the fix, not a passive snapshot.

Verdict

APPROVE — correct fix, tight scope, regression test that exercises the bug.

@tanzhenxin tanzhenxin merged commit 4bba75f into QwenLM:main May 11, 2026
8 checks passed
wenshao pushed a commit that referenced this pull request May 17, 2026
* fix(cli): keep long model stats header on one line

* test(cli): cover fixed model stats columns
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/stats model: Broken line

3 participants