feat(ui): remove top-level Changes tab, keep session-level d diff - #257
Conversation
A Git diff is a property of one session — what it changed since it launched — not a view over the whole fleet, so it no longer has a top-level tab. It lives on the Sessions tab as the `d` pane (`PaneView::Diff`), drawn over the harness terminal for the row under the cursor; `d`/`Esc` swaps the terminal back. Removed: - `"Changes"` from both TABS arrays (workflows 8->7, slim 7->6) - the Changes render dispatch arm, hint line, and "Diff" compact label - the Changes key-dispatch arm and its `refresh_changes` on tab-enter - the capital `D` shortcut (it jumped to the now-gone Changes tab) - `open_selected_harness_changes` and the dead `draw_changes` wrapper - the `shift_d_on_a_selected_harness_opens_its_changes_tab` test Untouched: the shared `GitChangesState`, `on_changes_key` bindings (file/hunk nav, `b` baseline, `c`/`C`/`e` comments, `r` refresh), and the SDK-side `git_review` model — both access paths already shared them, so the session `d` pane keeps the full review surface. The oversized-diff wrapping tests now render via `draw_harness_diff` (the remaining path) instead of the removed tab arm. Validation: cargo build, cargo clippy --all-targets -- -D warnings, cargo fmt --check, cargo test (all green, 0 failures). Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe standalone Changes tab was removed from tab lists, key handling, rendering, and state entry logic. Git diff access remains in the Sessions harness pane through ChangesSession diff navigation
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/tui/src/ui/app/render/tests.rs (1)
138-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove these imports to the module import group.
Keep
ChangedFile,tab_pos, andPaneViewwith the file-level imports. Do not declare imports insideapp_on_an_oversized_diff_line.As per coding guidelines, “Keep imports grouped at the top of Rust source files.”
🤖 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 `@src/tui/src/ui/app/render/tests.rs` around lines 138 - 140, Move the ChangedFile, tab_pos, and PaneView imports into the file-level import group at the top of the test module, and remove their local import declarations from app_on_an_oversized_diff_line.Source: Coding guidelines
src/tui/src/ui/app/types/model.rs (1)
76-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the non-workflow
TABSconstant.The documentation at Lines 55-61 applies only to the workflow-enabled constant. Add equivalent
///documentation before Line 76.As per coding guidelines, “Every public Rust item—including functions, types, traits, fields, and variants—must have a
///documentation comment.”🤖 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 `@src/tui/src/ui/app/types/model.rs` around lines 76 - 83, Add a `///` documentation comment immediately before the public non-workflow `TABS` constant, documenting the tabs it exposes and matching the equivalent workflow-enabled `TABS` documentation.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@src/tui/src/ui/app/render/tests.rs`:
- Around line 138-140: Move the ChangedFile, tab_pos, and PaneView imports into
the file-level import group at the top of the test module, and remove their
local import declarations from app_on_an_oversized_diff_line.
In `@src/tui/src/ui/app/types/model.rs`:
- Around line 76-83: Add a `///` documentation comment immediately before the
public non-workflow `TABS` constant, documenting the tabs it exposes and
matching the equivalent workflow-enabled `TABS` documentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c4e0c2a8-a681-4d11-8d7e-f4d9467e5212
📒 Files selected for processing (10)
src/tui/src/ui/app/changes/mod.rssrc/tui/src/ui/app/keys/mod.rssrc/tui/src/ui/app/keys/sessions.rssrc/tui/src/ui/app/render/changes.rssrc/tui/src/ui/app/render/mod.rssrc/tui/src/ui/app/render/settings/help.rssrc/tui/src/ui/app/render/tests.rssrc/tui/src/ui/app/state.rssrc/tui/src/ui/app/tests/harness_pane.rssrc/tui/src/ui/app/types/model.rs
💤 Files with no reviewable changes (4)
- src/tui/src/ui/app/keys/sessions.rs
- src/tui/src/ui/app/keys/mod.rs
- src/tui/src/ui/app/render/mod.rs
- src/tui/src/ui/app/render/changes.rs
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0075 · 148,878 in / 10,997 out · 93,296 cached (63%) · openrouter/openai/text-embedding-3-small, z-ai/glm-5.2 · 560 embedded
critique: $0.0032 · 65,032 in / 8,119 out · 55,697 cached (86%) · z-ai/glm-5.2
security: $0.0036 · 63,038 in / 1,338 out · 19,003 cached (30%) · z-ai/glm-5.2
tests: $0.0005 · 14,298 in / 1,180 out · 12,868 cached (90%) · z-ai/glm-5.2
description: $0.0002 · 6,510 in / 360 out · 5,728 cached (88%) · z-ai/glm-5.2
What this change touches10 files, +74 -100 across 7 components. It reaches 6 untouched components (60 graph nodes walked). 12 further components left out to keep the diagram readable. flowchart LR
n0["src/tui/src/ui/app/changes<br/>1 file +17 -32"]:::changed
n1["src/tui/src/ui/app/render<br/>3 files +19 -24"]:::changed
n2["src/tui/src/ui/app<br/>1 file +21 -16"]:::changed
n3["src/tui/src/ui/app/types<br/>1 file +14 -4"]:::changed
n4["src/tui/src/ui/app/tests<br/>1 file +1 -15"]:::changed
n5["src/tui/src/ui/app/keys<br/>2 files +0 -8"]:::changed
n6["src/tui/src/ui/app/render/settings<br/>1 file +2 -1"]:::changed
n7["src/tui/src/ui/app/changes<br/>2 files reached"]:::impacted
n8["src/link/src<br/>1 file reached"]:::impacted
n9["src/sdk/src/mcp<br/>1 file reached"]:::impacted
n10["src/sdk/src/mcp/tools<br/>1 file reached"]:::impacted
n11["src/sdk/src/ui/git_review<br/>1 file reached"]:::impacted
n12["src/sdk/tests/support<br/>1 file reached"]:::impacted
n1 -->|8 refs| n11
n1 -->|6 refs| n8
n1 -->|6 refs| n9
n1 -->|5 refs| n3
n2 -->|5 refs| n3
n4 -->|5 refs| n3
n5 -->|5 refs| n3
n1 -->|4 refs| n12
n6 -->|4 refs| n3
n2 -->|3 refs| n11
n4 -->|3 refs| n2
n1 -->|2 refs| n10
n2 -->|2 refs| n8
n3 -->|2 refs| n8
n5 -->|2 refs| n8
n6 -->|2 refs| n8
n7 -->|2 refs| n3
n1 -->|1 ref| n6
n2 -->|1 ref| n1
n2 -->|1 ref| n4
n2 -->|1 ref| n5
n2 -->|1 ref| n7
n4 -->|1 ref| n8
n7 -->|1 ref| n8
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed. Grey: untouched, reached through an import or a call. Orange: has findings. Red: has a finding that blocks the merge.
Changed files
|
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Summary
Removes the top-level Changes tab from the TUI. A Git diff is a property of one session — what it changed since it launched — not a view over the whole fleet, so it now lives only at the session level: press
don a session row in the Sessions tab to swap the harness terminal for that session's launch diff;d/Escswaps it back.What changed
Removed (the tab surface):
"Changes"from bothTABSarrays (workflows build 8→7, slim build 7→6) —src/tui/src/ui/app/types/model.rs"Changes" => draw_changesrender dispatch arm, the Changes hint line, and the"Changes" => "Diff"compact tab label —render/mod.rsif tab == "Changes"key-dispatch arm and itsrefresh_changeson tab-enter —keys/mod.rs,state.rsDshortcut from the Sessions rail (it jumped to the now-gone Changes tab) —keys/sessions.rsopen_selected_harness_changes(dead with the tab gone) and the deaddraw_changestab wrapper —changes/mod.rs,render/changes.rsKept (the session
dpath — unchanged behavior):toggle_harness_diff_paneanddraw_harness_diff→draw_changes_into, the render path the Sessions tab already usedGitChangesState,on_changes_keybindings (↑↓ files,j/kline,[/]hunk,bbaseline,c/C/ecomments,rrefresh)git_reviewmodelBoth access paths already shared this state, so the session
dpane retains the full review surface — only the top-level tab and itsDshortcut were removed.Tests:
shift_d_on_a_selected_harness_opens_its_changes_tab(tested the removedD).compact_tab_label("Changes", …)assertions.draw_harness_diff(the remaining path) instead of the removed tab arm.don a session row.Configuration / API changes
None. No public API, config, or persistence change — purely a TUI surface removal.
Validation
All green, zero test failures.
🤖 Generated with Medulla
Summary by CodeRabbit
d.dorEscto return to the harness terminal.