-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix(tui): Tighten message and tool spacing #4595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
848d6a1
6d0531b
2193118
c2242b6
5930070
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| # TUI Spacing And Density PR1 | ||
|
|
||
| ## Why | ||
|
|
||
| The current TUI often spends extra rows on spacing before assistant output, | ||
| between status/tool blocks, and inside expanded tool groups. In common | ||
| sessions this makes simple answers, file lists, tool output, error states, | ||
| diffs, and long streaming output harder to scan because users need to scroll | ||
| through blank space rather than content. | ||
|
|
||
| This PR is the first focused pass for QwenLM/qwen-code#4588. It addresses only | ||
| spacing and density so the review can compare row usage before and after | ||
| without also reviewing thinking visibility, tool borders, SubAgent layout, | ||
| branding, or theme color changes. | ||
|
|
||
| ## How | ||
|
|
||
| The implementation keeps the existing information structure and rendering | ||
| surfaces intact: | ||
|
|
||
| - History item spacing is centralized near `HistoryItemDisplay`. User prompts | ||
| and standalone command views still start with a turn separator, while | ||
| assistant continuations, tool groups, status messages, tool summaries, and | ||
| related in-turn output no longer add an extra leading spacer row. | ||
| - Expanded tool groups keep their current border and status/title structure, | ||
| but no longer insert blank rows between adjacent tool entries. | ||
| - Tool results render directly below the tool title/status row. This removes | ||
| the extra blank line between the tool header and its output without changing | ||
| output content, truncation, shell focus, confirmation prompts, or compact | ||
| mode behavior. | ||
|
|
||
| Markdown blank-line behavior is intentionally left unchanged. The renderer | ||
| already collapses consecutive blank lines to one spacer and preserves complex | ||
| blocks such as tables, code blocks, and math blocks. | ||
|
|
||
| ## Spacing Standard | ||
|
|
||
| - Independent user turns keep one visual separator. | ||
| - Assistant output and in-turn follow-up blocks do not add a second separator. | ||
| - Tool header and tool result content are adjacent. | ||
| - Expanded multi-tool groups do not insert blank rows between each tool entry. | ||
| - Complex Markdown blocks keep their existing internal layout. | ||
|
|
||
| ## Expected Effect | ||
|
|
||
| Under the same terminal width and same rendered content, target scenarios should | ||
| use fewer visible rows: | ||
|
|
||
| - Simple Q&A should drop at least one visible row. | ||
| - Expanded tool output should drop at least one row for each rendered tool | ||
| result that previously had a blank header/result spacer. | ||
| - Multi-tool groups should drop one row between each adjacent tool entry. | ||
| - Project inspection, diff, file-list, error, and long-stream scenarios should | ||
| not gain rows unless terminal wrapping changes make that unavoidable. | ||
|
|
||
| ## Measurement | ||
|
|
||
| The automated spacing assertions and terminal evidence use 100-column fixtures | ||
| for the changed rules: | ||
|
|
||
| | Scenario | Width | Baseline rows | PR1 rows | Delta | Evidence | | ||
| | --- | ---: | ---: | ---: | ---: | --- | | ||
| | Simple assistant reply | 100 | 2 | 1 | -1 | leading history spacer removed | | ||
| | Tool header with one-line result | 100 | 3 | 2 | -1 | header and result are adjacent | | ||
| | Three-tool expanded group with rendered results | 100 | 16 | 11 | -5 | one header/result spacer removed per tool result and one inter-tool separator removed between adjacent tools | | ||
| | Full representative fixture | 100 | 26 | 19 | -7 | same rendered content captured in tmux | | ||
|
|
||
| The snapshot diffs also cover the existing 80-column fixtures to confirm the | ||
| same row-count deltas in the current component test harness. | ||
|
|
||
| ## Out Of Scope | ||
|
|
||
| - Hiding thinking traces. | ||
| - Removing tool borders. | ||
| - Redesigning SubAgent output. | ||
| - Changing startup branding or the banner. | ||
| - Changing theme colors. | ||
| - Adding per-turn assistant elapsed time. | ||
| - Changing table inline-code highlighting. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| # TUI 间距优化 PR2 — 半行色带与紧凑间距 | ||
|
|
||
| ## 背景 | ||
|
|
||
| PR1 通过去除工具组内部多余空行,初步收紧了 TUI 垂直间距。但在实际使用中仍有两个体验问题: | ||
|
|
||
| 1. **用户消息与助手回复之间缺少视觉分界** — 长对话中难以快速定位"我的提问从哪里开始" | ||
| 2. **块间距仍然偏大** — 输入区域上方、问答交替处各有一整行空白,浪费屏幕空间 | ||
|
|
||
| ## 本次改动 | ||
|
|
||
| ### 1. 用户消息半行色带 | ||
|
|
||
| 在用户消息上下各添加一条半高的淡色线条,形成类似"色带"的视觉效果: | ||
|
|
||
| ``` | ||
| ▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄ ← 淡色半行线(仅占半个字符高度) | ||
| > 用户的提问内容 | ||
| ▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀ ← 淡色半行线 | ||
| ``` | ||
|
|
||
| - 颜色自动适配深色/浅色主题,在背景色基础上混入 15% 主题强调色,不会过于显眼 | ||
| - 不支持 24 位色的终端自动降级为普通显示 | ||
|
|
||
| ### 2. 收紧问答间距 | ||
|
|
||
| | 位置 | 改动前 | 改动后 | | ||
| |------|--------|--------| | ||
| | 用户消息上方 | 1 行空白 | 0(由色带提供视觉分隔) | | ||
| | 助手回复与工具之间 | 1 行空白 | 0 | | ||
| | 工具与下一段回复之间 | 1 行空白 | 0 | | ||
|
|
||
| 同一轮对话内的"回复 → 工具调用 → 回复"序列不再有多余空行,信息更紧凑连贯。 | ||
|
|
||
| ### 3. 输入区域分隔线 | ||
|
|
||
| 底部输入框上方的 1 行空白替换为一条半高淡色分隔线,视觉更轻,占用空间减半。 | ||
|
|
||
| ## 效果对比 | ||
|
|
||
| **改动前:** | ||
| ``` | ||
| (1 行空白) | ||
| > 帮我读取 package.json | ||
| (1 行空白) | ||
| ✦ 好的,我来读取文件。 | ||
| (1 行空白) | ||
| ┌ Read package.json ─────────┐ | ||
| │ ✓ Read package.json │ | ||
| └────────────────────────────┘ | ||
| (1 行空白) | ||
| ✦ 文件内容如下:... | ||
|
|
||
| (1 行空白) | ||
| ┌─ 输入框 ──────────────────┐ | ||
| ``` | ||
|
|
||
| **改动后:** | ||
| ``` | ||
| ▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄ | ||
| > 帮我读取 package.json | ||
| ▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀ | ||
| ✦ 好的,我来读取文件。 | ||
| ┌ Read package.json ─────────┐ | ||
| │ ✓ Read package.json │ | ||
| └────────────────────────────┘ | ||
| ✦ 文件内容如下:... | ||
| ▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄ | ||
| ┌─ 输入框 ──────────────────┐ | ||
| ``` | ||
|
|
||
| ## 未改动 | ||
|
|
||
| - 工具调用边框样式保持不变 | ||
| - Markdown 正文段落间距保持不变(1 行已是终端最小单位) | ||
| - 深色/浅色主题色值不变 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| # TUI Spacing And Density PR1 Evidence | ||
|
|
||
| ## Goal | ||
|
|
||
| Provide before/after evidence that PR1 reduces visible row usage without | ||
| removing content or changing rendering scope. | ||
|
|
||
| ## Fixed Conditions | ||
|
|
||
| - Terminal width: 100 columns. | ||
| - Compare the same prompt/output fixture before and after this PR. | ||
| - Strip ANSI control sequences before counting visible rows. | ||
| - Count rendered rows from the first non-empty fixture row through the last | ||
| non-empty fixture row. This keeps internal blank spacer rows in the metric | ||
| because those are the rows this PR removes. | ||
| - The fixture renders the real Ink TUI components directly, so it does not | ||
| require a model call or network access. | ||
|
|
||
| ## Scenarios | ||
|
|
||
| - Simple Q&A. | ||
| - File list output. | ||
| - Long shell output. | ||
| - File-read error output. | ||
| - Multi-block project inspection output. | ||
| - Diff output. | ||
| - Long streaming output. | ||
|
|
||
| ## Commands | ||
|
|
||
| Terminal capture: | ||
|
|
||
| ```bash | ||
| git checkout origin/main | ||
| REPO_ROOT="$PWD" | ||
| /tmp/qwen-pr1-spacing-evidence/run-tmux-capture.sh "$REPO_ROOT" 'base origin/main 34b7d472e' base | ||
| git switch feat/tui-spacing-density-pr1 | ||
| /tmp/qwen-pr1-spacing-evidence/run-tmux-capture.sh "$REPO_ROOT" 'PR1 fixed 848d6a166' fixed | ||
| ``` | ||
|
|
||
| VHS visual capture: | ||
|
|
||
| ```bash | ||
| git checkout origin/main | ||
| PATH=/Users/gawain/.nvm/versions/node/v24.15.0/bin:$PATH vhs /tmp/qwen-pr1-spacing-evidence/base.tape | ||
| git switch feat/tui-spacing-density-pr1 | ||
| PATH=/Users/gawain/.nvm/versions/node/v24.15.0/bin:$PATH vhs /tmp/qwen-pr1-spacing-evidence/fixed.tape | ||
| ffmpeg -y -i /tmp/qwen-pr1-spacing-evidence/base.gif -i /tmp/qwen-pr1-spacing-evidence/fixed.gif -filter_complex "[0:v]fps=5,scale=780:-1:flags=lanczos[left];[1:v]fps=5,scale=780:-1:flags=lanczos[right];[left][right]hstack=inputs=2,split[s0][s1];[s0]palettegen[p];[s1][p]paletteuse" /tmp/qwen-pr1-spacing-evidence/base-vs-fixed-optimized.gif | ||
| ``` | ||
|
|
||
| ## Evidence Artifacts | ||
|
|
||
| - Release: <https://github.com/QwenLM/qwen-code/releases/tag/tui-spacing-density-pr1-evidence> | ||
| - Side-by-side GIF: <https://github.com/QwenLM/qwen-code/releases/download/tui-spacing-density-pr1-evidence/base-vs-fixed-optimized.gif> | ||
| - Final screenshot: <https://github.com/QwenLM/qwen-code/releases/download/tui-spacing-density-pr1-evidence/base-vs-fixed-final.png> | ||
| - Base tmux capture: <https://github.com/QwenLM/qwen-code/releases/download/tui-spacing-density-pr1-evidence/base.tmux.txt> | ||
| - Fixed tmux capture: <https://github.com/QwenLM/qwen-code/releases/download/tui-spacing-density-pr1-evidence/fixed.tmux.txt> | ||
| - Base summary JSON: <https://github.com/QwenLM/qwen-code/releases/download/tui-spacing-density-pr1-evidence/base.summary.json> | ||
| - Fixed summary JSON: <https://github.com/QwenLM/qwen-code/releases/download/tui-spacing-density-pr1-evidence/fixed.summary.json> | ||
|
|
||
| ## Expected Results | ||
|
|
||
| - Simple Q&A: at least 1 fewer visible row. | ||
| - Expanded tool output: at least 1 fewer visible row per rendered tool result | ||
| that previously had a blank header/result spacer. | ||
| - Multi-tool expanded groups: 1 fewer visible row between each adjacent tool | ||
| entry. | ||
| - No scenario should lose user-visible content. | ||
|
|
||
| ## Results | ||
|
|
||
| | Scenario | Width | Baseline rows | PR1 rows | Delta | Notes | | ||
| | --- | ---: | ---: | ---: | ---: | --- | | ||
| | Simple Q&A | 100 | 2 | 1 | -1 | Assistant history item no longer starts with a spacer row | | ||
| | File list or shell output | 100 | 3 | 2 | -1 | Tool header and first result row are adjacent | | ||
| | File-read error | 100 | 3 | 2 | -1 | Error result uses the same tool header/result spacing | | ||
| | Project inspection | 100 | 16 | 11 | -5 | Three expanded tools no longer have header/result spacer rows or blank inter-tool rows | | ||
| | Diff output | 100 | 3 | 2 | -1 | Diff renderer remains unchanged; only tool header/result spacing changes | | ||
| | Long streaming output | 100 | N + 2 | N + 1 | -1 | Content rows are unchanged; the extra header/result spacer is removed | | ||
| | Full representative fixture | 100 | 26 | 19 | -7 | Same content rendered through real Ink components and captured in tmux | | ||
|
|
||
| ## What This Proves | ||
|
|
||
| - The base branch reproduces the extra spacer rows in a real terminal capture. | ||
| - PR1 removes the targeted spacer rows while preserving the same fixture content. | ||
| - The row-count improvement is measurable under fixed 100-column conditions. | ||
|
|
||
| ## What This Does Not Prove | ||
|
|
||
| - It does not cover later PR scopes such as thinking trace visibility, tool | ||
| border removal, SubAgent layout, branding, or theme colors. | ||
| - It does not replace manual review for extremely narrow terminal wrapping. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -58,6 +58,37 @@ describe('<HistoryItemDisplay />', () => { | |||||||||||||||||||||||||||||||||||||
| expect(lastFrame()).toContain('/theme'); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it('renders assistant replies without a leading spacer row', () => { | ||||||||||||||||||||||||||||||||||||||
| const item: HistoryItem = { | ||||||||||||||||||||||||||||||||||||||
| id: 1, | ||||||||||||||||||||||||||||||||||||||
| type: 'gemini', | ||||||||||||||||||||||||||||||||||||||
| text: 'Hello', | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
| const { lastFrame } = renderWithProviders( | ||||||||||||||||||||||||||||||||||||||
| <HistoryItemDisplay item={item} terminalWidth={100} isPending={false} />, | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const output = lastFrame() ?? ''; | ||||||||||||||||||||||||||||||||||||||
| expect(output.startsWith('\n')).toBe(false); | ||||||||||||||||||||||||||||||||||||||
| expect(output).toContain('✦ Hello'); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it('renders tool summaries without a leading spacer row', () => { | ||||||||||||||||||||||||||||||||||||||
| const item: HistoryItem = { | ||||||||||||||||||||||||||||||||||||||
| id: 1, | ||||||||||||||||||||||||||||||||||||||
| type: 'tool_use_summary', | ||||||||||||||||||||||||||||||||||||||
| summary: 'Read txt files', | ||||||||||||||||||||||||||||||||||||||
| precedingToolUseIds: ['c1'], | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
| const { lastFrame } = renderWithProviders( | ||||||||||||||||||||||||||||||||||||||
| <HistoryItemDisplay item={item} terminalWidth={100} isPending={false} />, | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const output = lastFrame() ?? ''; | ||||||||||||||||||||||||||||||||||||||
| expect(output.startsWith('\n')).toBe(false); | ||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The two new tests cover the no-leading-spacer contract for Worth adding:
— qwen3.7-max via Qwen Code /review
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The two new tests cover the no-leading-spacer contract for Worth adding:
— qwen3.7-max via Qwen Code /review |
||||||||||||||||||||||||||||||||||||||
| expect(output).toContain('Read txt files'); | ||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The two new tests confirm
Suggested change
— qwen3.7-max via Qwen Code /review |
||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it('renders StatsDisplay for "stats" type', () => { | ||||||||||||||||||||||||||||||||||||||
| const item: HistoryItem = { | ||||||||||||||||||||||||||||||||||||||
| ...baseItem, | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Critical] The decorative
▄separator renders unconditionally when truecolor is supported, without checkingisScreenReaderEnabled. Screen reader users will hear a row of meaningless half-block characters announced as noise.The hook is already imported at line 7 and used elsewhere in this file (e.g.,
<Footer />gate).HalfLinePaddedBoxin the same PR also correctly bails out for screen readers.— qwen3.7-max via Qwen Code /review