Fix dashboard overflow on shorter terminal heights#18
Conversation
effectiveViewMode() picked Hero/Compact using fixed height thresholds (e.g. height >= 22 for Hero) that no longer matched how many lines renderDashboard actually produces (28-37 lines depending on mode and terminal height). When the real content was taller than the terminal, the terminal itself scrolled the output, clipping the top of the TUI (title/graphs) while the footer stayed visible. Replace the static thresholds with a measurement-based pick: render each candidate mode and choose the largest one whose actual line count fits the current terminal height. Also clamp centerFrame's output to the terminal height as a safety net, trimming the tail (footer/hints) instead of relying on terminal scroll to hide overflow from the top. Adds a regression test that renders the dashboard across a grid of widths/heights and fails if any combination exceeds the terminal height; it reproduces the original bug when run against the old code.
Reviewer's GuideThis PR replaces hardcoded height thresholds with measurement-based mode selection for the dashboard, ensures the centered frame never exceeds terminal height, and adds a regression test to guarantee the UI adapts correctly to smaller terminals without clipping the top content. Sequence diagram for measurement-based dashboard view mode selectionsequenceDiagram
actor User
participant Terminal
participant Model
participant dashboardLineCount
participant renderDashboard
User->>Terminal: resize
Terminal->>Model: updateSize(width, height)
Terminal->>Model: effectiveViewMode
Model->>Model: build candidates
alt height > 0
loop for each mode in candidates
Model->>dashboardLineCount: dashboardLineCount(model, mode)
dashboardLineCount->>renderDashboard: renderDashboard(model, mode)
renderDashboard-->>dashboardLineCount: dashboard string
dashboardLineCount-->>Model: line count
Model->>Model: [line count <= height]
end
Model-->>Terminal: selected mode or ViewTiny
else height <= 0
Model-->>Terminal: first candidate mode
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
effectiveViewModenow renders the dashboard up to three times per view to measure height; consider extracting a lightweight layout-height calculation or caching the rendered height to avoid repeated full renders on every refresh. - Using
strings.Count(renderDashboard(...), "\n") + 1indashboardLineCountassumes there is always at least one line and no trailing newline; ifrenderDashboardever changes to include a trailing newline or return an empty string, the line count logic may become inaccurate, so it may be worth enforcing or documenting this invariant.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `effectiveViewMode` now renders the dashboard up to three times per view to measure height; consider extracting a lightweight layout-height calculation or caching the rendered height to avoid repeated full renders on every refresh.
- Using `strings.Count(renderDashboard(...), "\n") + 1` in `dashboardLineCount` assumes there is always at least one line and no trailing newline; if `renderDashboard` ever changes to include a trailing newline or return an empty string, the line count logic may become inaccurate, so it may be worth enforcing or documenting this invariant.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces hardcoded height thresholds with a dynamic view-mode selection based on the actual rendered height of the dashboard, and adds a regression test to ensure the layout fits the terminal. However, the review identifies a critical bug: because the rendering pipeline forcefully truncates the output to the terminal height, the line-count check will always evaluate to true, rendering the dynamic mode selection ineffective and causing the regression test to pass falsely. Additionally, rendering the dashboard multiple times per frame introduces a significant performance overhead. It is recommended to implement a lightweight, mathematical line-count calculation and update the test to assert the expected view mode directly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Gemini Code Assist correctly flagged that dashboardLineCount measured renderDashboard's already-clamped output, so every candidate mode looked like it fit (centerFrame's safety net truncates to terminal height regardless of mode) and effectiveViewMode always picked Hero, silently relying on truncation instead of actually adapting. Fix it by measuring the pre-clamp content height directly. A second, self-inflicted bug surfaced while fixing the first: an earlier version of this measurement used len(contentLines) as the line count, but panel elements are single strings with embedded "\n"s (a bordered panel is one slice element, not one per row), which undercounted badly (12 vs. the real ~28). Fixed by counting real newlines in the joined content instead. Added TestEffectiveViewModeFitsBeforeClamping, which measures the pre-clamp content directly (not via dashboardLineCount, so it still catches a regression in that helper itself) and would have failed against both of the above. Verified it fails on the old code and passes now. Also addresses Sourcery's performance note: introduced pickViewModeAndContent, which builds each candidate's content once during mode selection and returns the winning mode's lines directly, so View() no longer re-renders the chosen mode from scratch a second time. Removed the now-dead renderHero/renderCompact/renderMini/ renderDashboard wrappers.
|
Thanks both — @gemini-code-assist's catch was correct and important: Pushed a fix that measures the pre-clamp content height directly instead. While fixing it I introduced (and caught, via a test written specifically to make this class of bug fail loudly) a second miscount: an intermediate version used Added Also took @sourcery-ai's performance note: added |
add gemini suggestion Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
tysm @CMS-20 ! |
|
Tip For best results, initiate chat on the files or code changes.
Just follow up with the details and I'll take it from there! (=^・ω・^=) |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)❌ Error creating Unit Test PR.
Comment |
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
❌ Failed to create PR with unit tests: AGENT_CHAT: Failed to open pull request |
2 participants
Summary
effectiveViewMode()chose Hero/Compact/Mini using fixed height thresholds (e.g.height >= 22for Hero) that didn't match how many linesrenderDashboardactually produces (28-37 lines depending on mode/height). When the real terminal was shorter than that, the terminal itself scrolled the output, clipping the top of the TUI (title/graphs) while the footer stayed visible — the dashboard never adapted below a certain size.centerFrameso output can never exceed the terminal height even in edge cases, trimming the tail (footer/hints) instead of relying on terminal scroll to hide overflow from the top.Test plan
go build ./...go vet ./...go test ./...internal/ui/resize_test.go, a regression test that renders the dashboard across a grid of widths/heights and fails if any combination exceeds the terminal height. Verified it fails against the pre-fix code (reproducing the exact reported bug) and passes with the fix.Summary by Sourcery
Adjust dashboard view mode selection and frame centering to prevent terminal overflow and clipping on short terminal heights.
Bug Fixes:
Tests:
Summary by CodeRabbit