fix(tui): Tighten message and tool spacing

[chiga0]

This PR is closed and superseded by QwenLM/qwen-code#4595.

The active PR uses the upstream branch feat/tui-spacing-density-pr1 from QwenLM/qwen-code, and its evidence artifacts are hosted under the upstream repository:

• 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

[github-actions]

📋 Review Summary

This PR tightens TUI spacing and density by centralizing margin logic in HistoryItemDisplay, reducing ToolGroupMessage gap from 1 to 0, and removing marginTop={1} from tool result rendering. The changes are well-scoped, focused on spacing only (as promised in the design doc), and backed by comprehensive test coverage with snapshot evidence showing 1-2 row reductions across common scenarios.

🔍 General Feedback

• Focused scope: The PR stays tightly scoped to spacing/density as outlined in the design doc, avoiding scope creep into thinking traces, tool borders, or theme changes.
• Good test coverage: New tests specifically assert the spacing behavior (no leading newlines, adjacent tool entries), and snapshot updates provide visual regression protection.
• Clear documentation: Design doc (.qwen/design/tui-spacing-density-pr1.md) and E2E test plan (.qwen/e2e-tests/tui-spacing-density-pr1.md) provide excellent context for the "why" and expected impact.
• Centralized logic: Moving spacing logic into getHistoryItemMarginTop() is a clean refactor that makes the behavior explicit and maintainable.
• Positive impact: Row reductions (1-2 rows per scenario) will meaningfully improve terminal density for common workflows without removing content.

🎯 Specific Feedback

🟢 Medium

• File: packages/cli/src/ui/components/HistoryItemDisplay.tsx:89-118 - The getHistoryItemMarginTop() function lists 23 history item types explicitly. Consider adding a comment explaining why certain types (e.g., user, user_shell) still get marginTop: 1 while others don't, to help future maintainers understand the "independent user turns keep one visual separator" rule.

• File: packages/cli/src/ui/components/messages/ToolGroupMessage.tsx:429 - Changing gap={1} to gap={0} is a simple but impactful change. The snapshot diffs show this removes blank rows between adjacent tools, but consider adding a brief inline comment like {/* Adjacent tools render without separator rows for density */} to explain the intent.

🔵 Low

• File: packages/cli/src/ui/components/HistoryItemDisplay.test.tsx:61-91 - The two new tests (renders assistant replies without a leading spacer row and renders tool summaries without a leading spacer row) are excellent. Consider adding one more test case for a user type message to verify it does still get a leading spacer (preserving the "independent user turns keep separator" rule).

• File: packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx:158-178 - The test renders expanded tool entries without blank separator rows asserts secondLine === firstLine + 1. Consider adding a comment explaining this is testing the removal of inter-tool blank rows, not just adjacency.

• File: .qwen/design/tui-spacing-density-pr1.md:47-53 - The "Expected Effect" section mentions "Multi-tool expanded groups should drop one row between each adjacent tool entry." Consider clarifying whether this scales linearly (e.g., 4 tools = 3 removed rows) or has any upper bound.

✅ Highlights

• Excellent scoping: Keeping this PR limited to spacing changes (no thinking trace, border, or theme changes) makes it independently reviewable and low-risk.
• Data-driven approach: The PR body includes a clear table with before/after row counts for each scenario, making the impact immediately visible.
• Test-driven validation: New tests specifically target the spacing behavior changes, and snapshot updates provide visual confirmation of the denser layout.
• Clean refactoring: Extracting getHistoryItemMarginTop() is a maintainable solution that makes the spacing rules explicit rather than scattered.
• Backward compatible: The changes reduce whitespace without removing any content or changing functionality—users will see denser output but no broken features.

[chiga0]

Superseded by #4595, which uses the upstream feat/tui-spacing-density-pr1 branch.

[wenshao]

⚠️ Downgraded from Approve to Comment: CI still running. Two suggestions below (exhaustiveness guard + RESERVED_LINE_COUNT alignment). — qwen3.7-max via Qwen Code /review

[wenshao]

[Suggestion] The default: return 1 branch silently assigns marginTop=1 to any future HistoryItem variant. The codebase's established pattern for HistoryItem switches — see historyUtils.ts:94 (const _exhaustive: never = item) and utils/checks.ts:8 (assumeExhaustive()) — uses a never guard so adding a new variant without classifying it here triggers a TS2322 compile error. Without it, the next new variant will silently get a spacer row, undermining this PR's density goal.

Suggested change

	default:
	default: {
	// Compile-time exhaustiveness — adding a new HistoryItem variant
	// without classifying it here triggers a TS2322 on this line.
	const _exhaustive: never = item;
	void _exhaustive;
	return 1;
	}

— qwen3.7-max via Qwen Code /review

[wenshao]

[Suggestion] The marginTop={1} removal here also needs the matching update to RESERVED_LINE_COUNT at line 49: it should drop from 5 to 4, since the removed margin was one of the rows that constant reserved for. Otherwise availableHeight = availableTerminalHeight - STATIC_HEIGHT(1) - RESERVED_LINE_COUNT(5) (line 614) undercounts content space by 1 row per tool, and 5+ test assertions (e.g. expect(output).toContain('height=94') at lines 591, 624, 663, 857, 877) encode the stale budget.

At line 49:

Suggested change

	<Box paddingLeft={STATUS_INDICATOR_WIDTH} width="100%">
	const RESERVED_LINE_COUNT = 4; // for tool name, status etc.

Also update the corresponding height=94 test assertions to height=95 and the inline // 100 - STATIC_HEIGHT(1) - RESERVED_LINE_COUNT(5) = 94 comments to ... - 4 = 95.

— qwen3.7-max via Qwen Code /review