Agent Tabs Missing Footer System - #262
Open
jbrinkman wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThe change documents the unified three-line footer contract and updates ChangesUnified footer viewport integration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/tui/output_view.go`:
- Around line 125-139: The resize contract subtracts footer space twice,
reducing agent viewports unnecessarily. In internal/tui/output_view.go:125-139,
remove the local footer subtraction so Resize consumes the content height
supplied by TabManager; in internal/tui/output_view.go:78-86, remove the child
resize path or make it follow that same content-height contract; update
documentation at internal/tui/output_view.go:13-28 and
internal/tui/tui.go:753-770 to state that ResizeForFooter owns footer and
tab-header sizing.
In `@internal/tui/tui.go`:
- Around line 753-770: Update the resize and footer-rendering documentation
around renderTabContentWithFooter and ResizeForFooter to state the actual
contract: ResizeForFooter passes each tab the already-adjusted content height,
so tabs must not subtract the footer height again. Align the sizing
implementation with this contract and ensure the documented footer height
remains consistent with FooterManager.GetFooterHeight().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d4785b8-804e-4033-91a8-0aadd502f57f
📒 Files selected for processing (3)
.kiro-krew/specs/issue-211-agent-tabs-missing-footer-system.mdinternal/tui/output_view.gointernal/tui/tui.go
Comment on lines
+125
to
+139
| // Note: height should be the total available height including footer space. | ||
| // The viewport height will be adjusted to leave room for the footer (3 lines). | ||
| func (ov *OutputView) Resize(width, height int) { | ||
| ov.width = width | ||
| ov.height = height | ||
| ov.viewport = viewport.New(viewport.WithWidth(width), viewport.WithHeight(height)) | ||
|
|
||
| // Reserve space for the footer system (separator + input row + status row = 3 lines) | ||
| // This ensures the footer rendered by renderTabContentWithFooter() doesn't overflow | ||
| footerHeight := 3 | ||
| viewportHeight := height - footerHeight | ||
| if viewportHeight < 1 { | ||
| viewportHeight = 1 // Minimum viewport height | ||
| } | ||
|
|
||
| ov.viewport = viewport.New(viewport.WithWidth(width), viewport.WithHeight(viewportHeight)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use a single owner for footer subtraction.
ResizeForFooter already removes the footer and tab-header space before invoking tab resizing, so the new local subtraction shortens every agent viewport by three rows. The resize paths and their documentation should share one explicit height contract. (raw.githubusercontent.com)
internal/tui/output_view.go#L125-L139: remove the second footer subtraction and consume the content height supplied byTabManager.internal/tui/output_view.go#L78-L86: remove the child resize path or make it use the same content-height contract.internal/tui/output_view.go#L13-L28: document the actual ownership of footer sizing.internal/tui/tui.go#L753-L770: align the unified-footer documentation with that contract.
📍 Affects 2 files
internal/tui/output_view.go#L125-L139(this comment)internal/tui/output_view.go#L78-L86internal/tui/output_view.go#L13-L28internal/tui/tui.go#L753-L770
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/tui/output_view.go` around lines 125 - 139, The resize contract
subtracts footer space twice, reducing agent viewports unnecessarily. In
internal/tui/output_view.go:125-139, remove the local footer subtraction so
Resize consumes the content height supplied by TabManager; in
internal/tui/output_view.go:78-86, remove the child resize path or make it
follow that same content-height contract; update documentation at
internal/tui/output_view.go:13-28 and internal/tui/tui.go:753-770 to state that
ResizeForFooter owns footer and tab-header sizing.
Comment on lines
+753
to
+770
| // renderTabContentWithFooter creates a unified rendering method that combines tab content with footer. | ||
| // | ||
| // Unified Footer Rendering Flow (Issue #195, completed by Issue #211): | ||
| // This method is the central point for all tab rendering, ensuring consistent footer | ||
| // display across all tab types (main, planning, agent, log). | ||
| // | ||
| // For agent tabs specifically (Issue #211): | ||
| // 1. AgentTab.View() returns OutputView content (viewport with agent output) | ||
| // 2. This method wraps that content with the footer from FooterManager | ||
| // 3. OutputView must pre-reserve footer space (3 lines) in its viewport height | ||
| // to prevent the footer from overflowing the available screen space | ||
| // | ||
| // Footer Structure (3 lines total): | ||
| // - Line 1: Separator line (─────) | ||
| // - Line 2: Input row (kiro-krew> prompt with autocomplete) | ||
| // - Line 3: Status row (base: theme info, enhanced for planning: context/model/directory) | ||
| // | ||
| // The footer height calculation must match FooterManager.GetFooterHeight() = 3. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Document the actual resize contract.
These comments say OutputView receives total height and must subtract the footer, but ResizeForFooter passes already-adjusted content height to each tab. Update this wording together with the sizing fix so future changes do not reintroduce the double subtraction. (raw.githubusercontent.com)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/tui/tui.go` around lines 753 - 770, Update the resize and
footer-rendering documentation around renderTabContentWithFooter and
ResizeForFooter to state the actual contract: ResizeForFooter passes each tab
the already-adjusted content height, so tabs must not subtract the footer height
again. Align the sizing implementation with this contract and ensure the
documented footer height remains consistent with
FooterManager.GetFooterHeight().
There was a problem hiding this comment.
Pull request overview
This PR aims to restore UI consistency by ensuring agent tabs display the same two-row footer system (input row + status row) established for other tabs, primarily by adjusting how agent output view height is calculated and by documenting the unified footer rendering flow.
Changes:
- Updates
OutputViewsizing logic to account for footer height when creating the viewport. - Expands inline documentation in
tui.goandoutput_view.godescribing the footer rendering pipeline. - Adds a detailed design/spec document for Issue #211.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/tui/tui.go | Adds detailed documentation for the unified footer rendering flow and agent-tab integration. |
| internal/tui/output_view.go | Adjusts viewport height calculations in Update/Resize to try to reserve footer space for agent tabs. |
| .kiro-krew/specs/issue-211-agent-tabs-missing-footer-system.md | Adds a full design specification and validation notes for the footer consistency fix. |
Comments suppressed due to low confidence (2)
internal/tui/output_view.go:83
- OutputView.Update(tea.WindowSizeMsg) subtracts a 3-line footer from msg.Height, but tabs are already resized using TabManager.ResizeForFooter (tab_manager.go calculates contentHeight = height - footerHeight - tabHeaderHeight). Subtracting again will make the viewport 3 lines too short and can introduce empty space / inconsistent scrolling.
// Reserve space for the footer system (separator + input row + status row = 3 lines)
footerHeight := 3
viewportHeight := msg.Height - footerHeight
if viewportHeight < 1 {
viewportHeight = 1 // Minimum viewport height
internal/tui/output_view.go:135
- OutputView.Resize subtracts a hard-coded footer height (3) from the provided height. In the tabbed UI, TabManager.ResizeForFooter already removes the footer height before calling Tab.Resize(), so this double-subtracts and shrinks the viewport unnecessarily (and can reintroduce truncation in small terminals).
// Reserve space for the footer system (separator + input row + status row = 3 lines)
// This ensures the footer rendered by renderTabContentWithFooter() doesn't overflow
footerHeight := 3
viewportHeight := height - footerHeight
if viewportHeight < 1 {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+759
to
+763
| // For agent tabs specifically (Issue #211): | ||
| // 1. AgentTab.View() returns OutputView content (viewport with agent output) | ||
| // 2. This method wraps that content with the footer from FooterManager | ||
| // 3. OutputView must pre-reserve footer space (3 lines) in its viewport height | ||
| // to prevent the footer from overflowing the available screen space |
Comment on lines
+25
to
+28
| // To prevent the footer from being pushed off-screen, OutputView must reserve | ||
| // space by subtracting the footer height (3 lines) from the viewport height. | ||
| // This ensures the total rendered content (viewport + footer) fits within the | ||
| // allocated screen space without overflow or layout issues. |
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
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.
Summary
This PR fixes the missing footer system in agent tabs to ensure consistency with the two-row footer system established in Issue #195.
Problem
Agent tabs were not displaying the footer system (input row + status row) that appears on main and planning tabs, creating an inconsistent user interface.
Solution
Key Files Modified
Testing
✅ All QA checks passed:
✅ Acceptance criteria validated:
Note: Docker-dependent sandbox tests failed due to unavailable Docker daemon (infrastructure issue, not code issue).
Impact
Closes #211
Summary by CodeRabbit
Bug Fixes
Documentation