Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 17 additions & 32 deletions src/tui/src/ui/app/changes/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
//! Session-scoped Git inspection and review comments for the Changes tab.
//! Session-scoped Git inspection and review comments for the harness diff pane.
//!
//! A Git diff is a property of one session — what it changed since it launched
//! — so it is shown in the Sessions tab's harness pane (`d` on a row swaps the
//! terminal for the diff) rather than as a top-level tab over the whole fleet.
//!
//! Git subprocesses live in [`repository`], the cursor and cache in [`types`],
//! and the reusable review model in [`medulla::ui::git_review`]. This module is
Expand All @@ -23,44 +27,25 @@ use std::path::Path;
use medulla::ui::git_review::CommentAnchor;
pub(crate) use types::GitChangesState;

use super::types::{App, Cmd, PaneView, PromptKind, TABS};
use super::types::{App, PaneView, PromptKind};
use crate::ui::composer::{Draft, TextPrompt};
use baseline::select_harness_baseline;

impl App {
/// Open the Changes tab for the harness currently shown in the Sessions pane.
///
/// The draw path records the exact session resolved from the selected rail
/// row. Keeping that id before changing tabs lets the forced refresh pick
/// the matching launch snapshot instead of falling back to another, newer
/// harness in a different checkout. Unlike ordinary tab entry, this also
/// replaces an operator-selected commit or manual baseline: `d` means the
/// immutable launch diff for the harness under the cursor.
pub(super) fn open_selected_harness_changes(&mut self) -> Option<Cmd> {
let session = self.pane_session.clone()?;
self.rail_session = Some(session);
self.tab_index = TABS
.iter()
.position(|tab| *tab == "Changes")
.expect("Changes is a top-level tab");
self.selected = 0;
self.refresh_changes_from_selected_harness();
None
}

/// Swap the harness pane between its terminal and its diff.
///
/// The diff is drawn *in the pane* rather than on the Changes tab, because
/// the operator asked a question about the row they are sitting on: sending
/// them to another tab makes the rail cursor, the pane title and the tab bar
/// all move at once to answer "what has this one changed". Same state and
/// same bindings as the tab — only the real estate differs — so `d` again
/// puts the terminal back.
/// The diff is drawn *in the pane* rather than on its own tab, because the
/// operator asked a question about the row they are sitting on: a Git diff
/// is a property of one session — what it changed since it launched — not a
/// view over the whole fleet, so `d` answers "what has this one changed"
/// without moving the rail cursor, the pane title or the tab bar. Same state
/// and same bindings as a full diff would have — only the real estate differs
/// — so `d` again puts the terminal back.
///
/// Each opening re-points Git at this session's launch snapshot, for the
/// reason [`open_selected_harness_changes`](Self::open_selected_harness_changes)
/// does: the collected diff is shared with the Changes tab, so it may be
/// describing a different harness entirely.
/// Each opening re-points Git at this session's launch snapshot: the
/// collected diff is shared state that may have been left describing a
/// different harness entirely, so it is re-rooted on the row under the cursor
/// before anything is drawn.
pub(super) fn toggle_harness_diff_pane(&mut self) {
if self.pane_view == PaneView::Diff {
self.pane_view = PaneView::Harness;
Expand Down
3 changes: 0 additions & 3 deletions src/tui/src/ui/app/keys/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,6 @@ impl App {
return cmd;
}
}
if tab == "Changes" && self.on_changes_key(k.code) {
return None;
}
// Workflows owns three panes, one of which is a composer, so it gets
// first refusal on every key that is not a global chord — exactly as
// Settings and Routing do for their subpages.
Expand Down
5 changes: 0 additions & 5 deletions src/tui/src/ui/app/keys/sessions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,6 @@ impl App {
self.toggle_harness_diff_pane();
SessionsKey::Handled(None)
}
// `D` is the same diff on the Changes tab: the pane is half a
// screen, and a review with comments on it wants the whole one.
KeyCode::Char('D') if !ctrl && !alt && self.pane_session.is_some() => {
SessionsKey::Handled(self.open_selected_harness_changes())
}
// `k` closes the harness the pane is showing — the other half of the
// two things an operator wants from a session they are looking at
// but not typing into. It asks first; see `close_pane_session_prompt`.
Expand Down
5 changes: 0 additions & 5 deletions src/tui/src/ui/app/render/changes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ use medulla::ui::git_review::CommentAnchor;
use super::super::types::App;

impl App {
/// Draw the session-start summary/file rail and selected unified patch.
pub(super) fn draw_changes(&mut self, frame: &mut Frame, area: Rect) {
self.draw_changes_into(frame, area, " Git changes · b baseline ");
}

/// The same two panes, drawn over the Sessions harness pane.
///
/// Same state, same bindings, different real estate: `d` on a harness row
Expand Down
4 changes: 0 additions & 4 deletions src/tui/src/ui/app/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,6 @@ impl App {
// Its own line, and a short one: the placeholder binds nothing, and
// the default hint below advertises the session steering chords.
"Tab views · Subconscious coming soon"
} else if self.tab() == "Changes" {
"Tab views · ↑↓ files · j/k line · [ ] hunk · b baseline · c comment · C file · e edit · r refresh"
} else if workflows {
"Tab views · ⏎ open · Esc back · ←→ follow edges · ↑↓ lanes · i inspect · c copilot · x run · d dry-run · r refresh"
} else if self.tab() == "Sessions" && self.pane_view == PaneView::Diff {
Expand Down Expand Up @@ -310,7 +308,6 @@ impl App {
match self.tab() {
"Overview" => self.draw_overview(f, area),
"Sessions" => self.draw_sessions_tab(f, area),
"Changes" => self.draw_changes(f, area),
#[cfg(feature = "workflows")]
"Workflows" => self.draw_workflows_tab(f, area),
// Not feature-gated: the tab exists in the slim build too, and a
Expand Down Expand Up @@ -355,7 +352,6 @@ fn compact_tab_label(name: &str, compact: bool) -> &str {
"Sessions" => "Sess",
"Workflows" => "Flows",
"Subconscious" => "Sub",
"Changes" => "Diff",
"Feedback" => "Feed",
"Settings" => "Set",
_ => name,
Expand Down
3 changes: 2 additions & 1 deletion src/tui/src/ui/app/render/settings/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,12 @@ impl App {
TLine::from("Account: Enter twice to log out · Usage: r refresh"),
TLine::from(" "),
TLine::from(Span::styled("Changes", bold)),
TLine::from("Tab / Shift-Tab to the Changes view to inspect the Git diff since session start"),
TLine::from("On a session row, press d to inspect the Git diff since that session launched"),
TLine::from("↑↓ select files · j/k move by line · [/] jump hunks · PageUp/PageDown move faster"),
TLine::from(
"c comments on a line or hunk · e edits it · C comments on or edits the file · r refreshes",
),
TLine::from("d or Esc puts the harness terminal back"),
TLine::from(" "),
TLine::from(Span::styled("Mouse", bold)),
TLine::from("Click a tab to switch views · click a rail row to select it · wheel scrolls"),
Expand Down
34 changes: 19 additions & 15 deletions src/tui/src/ui/app/render/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use medulla::config::LoadedConfig;
use medulla::runtime::mock::MockRuntime;
use medulla::runtime::Runtime;

use crate::ui::app::changes::types::ChangedFile;
use crate::ui::app::types::{tab_pos, PaneView};
use crate::ui::app::App;

fn app() -> App {
Expand All @@ -23,11 +25,9 @@ fn compact_tab_labels_shorten_the_current_wide_destinations() {
assert_eq!(super::compact_tab_label("Sessions", true), "Sess");
assert_eq!(super::compact_tab_label("Workflows", true), "Flows");
assert_eq!(super::compact_tab_label("Subconscious", true), "Sub");
assert_eq!(super::compact_tab_label("Changes", true), "Diff");
assert_eq!(super::compact_tab_label("Feedback", true), "Feed");
assert_eq!(super::compact_tab_label("Settings", true), "Set");
assert_eq!(super::compact_tab_label("Hosts", true), "Hosts");
assert_eq!(super::compact_tab_label("Changes", false), "Changes");
}

#[test]
Expand Down Expand Up @@ -110,8 +110,6 @@ fn leaving_the_agents_tab_takes_the_keyboard_back_from_an_attached_harness() {

#[test]
fn a_stale_harness_diff_does_not_advertise_agents_shortcuts_on_another_tab() {
use crate::ui::app::types::{tab_pos, PaneView};

let mut app = app();
app.tab_index = tab_pos("Overview");
app.pane_view = PaneView::Diff;
Expand All @@ -130,16 +128,16 @@ fn a_stale_harness_diff_does_not_advertise_agents_shortcuts_on_another_tab() {
assert!(!output.contains("d/Esc harness"), "{output}");
}

/// Put the Changes tab in front of a patch whose selected line is far wider
/// than the diff pane, so one rendered row wraps past the whole viewport.
/// Put the harness diff pane in front of a patch whose selected line is far
/// wider than the diff pane, so one rendered row wraps past the whole viewport.
///
/// The diff is reached only as a session pane (`d` on a row) now that the
/// top-level Changes tab is gone, so this renders through `draw_harness_diff`
/// — the same path the Sessions tab takes — rather than a removed tab arm.
fn app_on_an_oversized_diff_line() -> App {
use crate::ui::app::changes::types::ChangedFile;

let mut app = app();
app.tab_index = crate::ui::app::types::TABS
.iter()
.position(|t| *t == "Changes")
.expect("Changes tab");
app.tab_index = tab_pos("Sessions");
app.pane_view = PaneView::Diff;
app.changes.root = Some(std::path::PathBuf::from("/repo"));
app.changes.baseline = Some("baseline".to_owned());
app.changes.files = vec![ChangedFile {
Expand All @@ -162,7 +160,9 @@ fn a_wrapped_diff_bounds_the_scroll_by_the_rows_it_actually_occupies() {
let mut terminal =
ratatui::Terminal::new(ratatui::backend::TestBackend::new(120, 40)).expect("terminal");

terminal.draw(|f| app.draw(f)).expect("draw");
terminal
.draw(|f| app.draw_harness_diff(f, f.area()))
.expect("draw");

// The oversized line wraps to many rows, so the bound has to exceed the
// three logical patch lines rather than counting them one row each.
Expand All @@ -179,9 +179,13 @@ fn a_cursor_on_an_oversized_line_holds_a_stable_scroll_offset() {
let mut terminal =
ratatui::Terminal::new(ratatui::backend::TestBackend::new(120, 40)).expect("terminal");

terminal.draw(|f| app.draw(f)).expect("draw");
terminal
.draw(|f| app.draw_harness_diff(f, f.area()))
.expect("draw");
let first = app.changes.scroll;
terminal.draw(|f| app.draw(f)).expect("draw again");
terminal
.draw(|f| app.draw_harness_diff(f, f.area()))
.expect("draw again");

// Showing the top of the row keeps consecutive frames identical instead of
// oscillating between the row's top and bottom edges.
Expand Down
37 changes: 21 additions & 16 deletions src/tui/src/ui/app/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use medulla::config::LoadedConfig;
use medulla::runtime::{ContextItem, Runtime};

use super::types::{
App, Cmd, HandbackPolicy, ResumePicker, ROUTING_SUBPAGES, SETTINGS_SUBPAGES, SP_CONTEXT,
SP_FEEDBACK, SP_USAGE, TABS,
App, Cmd, HandbackPolicy, PaneView, ResumePicker, ROUTING_SUBPAGES, SETTINGS_SUBPAGES,
SP_CONTEXT, SP_FEEDBACK, SP_USAGE, TABS,
};

impl App {
Expand Down Expand Up @@ -529,23 +529,28 @@ impl App {
/// active subpage.
pub(super) fn tab_enter_cmd(&mut self) -> Option<Cmd> {
// Arriving at a tab should put the keyboard on the thing the tab is
// *about*. Both of these used to land it somewhere else — Hosts on its
// two-item menu — so the first arrow press did
// nothing visible and the list had to be clicked before it would move.
match self.tab() {
"Changes" => {
self.refresh_changes();
}
// The list is the page; the menu is two rows and reachable with `1`
// and `2`, or with Esc.
"Hosts" => self.routing_focused = true,
// Safe because the rail forwards typing: a printable key moves focus
// to the composer and lands the character there, so nothing is lost
// by not starting in it.
_ => {}
// *about*. Hosts used to land on its two-item menu, so the first arrow
Comment thread
senamakel marked this conversation as resolved.
// press did nothing visible and the list had to be clicked before it
// would move — the list is the page, and the menu is two rows reachable
// with `1` and `2` or Esc.
if self.tab() == "Hosts" {
self.routing_focused = true;
}
// No other tab needs a nudge: the rail forwards typing, so a printable
// key moves focus to the composer and lands the character there, and
// nothing is lost by not starting in it.
match self.tab() {
"Feedback" => Some(Cmd::LoadFeedback(self.feedback.query.clone())),
// The diff pane is the Changes surface now, so entering the tab
// re-loads its git data the way the removed Changes tab did on
// entry: a repo that changed while the operator was on another tab
// would otherwise stay stale beneath the open diff.
"Sessions" => {
if self.pane_view == PaneView::Diff {
self.refresh_changes();
}
None
}
// The workflow store is files on this machine, so entering the tab
// reads them rather than asking the runtime for anything — which is
// why this arm returns no command and does the work here.
Expand Down
16 changes: 1 addition & 15 deletions src/tui/src/ui/app/tests/harness_pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use medulla::config::LoadedConfig;
use medulla::runtime::mock::MockRuntime;
use std::sync::Arc;

use super::super::types::{tab_pos, App, PaneView};
use super::super::types::{tab_pos, App};

/// Build the standard app fixture with the harness pane available.
fn app() -> App {
Expand All @@ -20,20 +20,6 @@ fn tab(name: &str) -> usize {
tab_pos(name)
}

#[test]
fn shift_d_on_a_selected_harness_opens_its_changes_tab() {
let mut app = app();
app.tab_index = tab("Sessions");
app.pane_session = Some("selected-harness".to_owned());

let cmd = app.on_key(KeyEvent::new(KeyCode::Char('D'), KeyModifiers::SHIFT));

assert!(cmd.is_none());
assert_eq!(app.tab(), "Changes");
assert_eq!(app.pane_view, PaneView::Harness);
assert_eq!(app.rail_session.as_deref(), Some("selected-harness"));
}

#[test]
fn k_on_a_selected_harness_asks_before_closing_it() {
let mut app = app();
Expand Down
18 changes: 14 additions & 4 deletions src/tui/src/ui/app/types/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,26 +52,36 @@ use super::rail_hit::RailHit;
/// both still builds and their render paths are intact, so restoring either is
/// putting one line back. Memory is out of the build entirely (its tab said
/// "coming soon"); Tasks duplicates what the Sessions tab already shows per lane.
///
/// `Changes` is gone rather than commented out: a Git diff is a property of one
/// session — what it changed since it launched — not a view over the whole fleet.
/// It lives on the Sessions tab as the `d` pane (`PaneView::Diff`), drawn over
/// the harness terminal for the row under the cursor. The shared diff state and
/// bindings stay in `app::changes`; only the top-level tab and its `D` shortcut
/// were removed.
#[cfg(feature = "workflows")]
pub const TABS: [&str; 8] = [
pub const TABS: [&str; 7] = [
"Overview",
"Sessions",
"Workflows",
"Subconscious",
"Changes",
"Hosts",
"Feedback",
"Settings",
];

/// Without the workflow engine. A slim build must not offer a tab that cannot
/// draw anything.
///
/// This is the workflow-enabled list minus `Workflows`: every remaining tab —
/// the fleet Overview, the Sessions rail with its `d` diff pane, Subconscious
/// (still drawing its placeholder), Hosts, Feedback, and Settings — renders
/// without the workflow engine.
#[cfg(not(feature = "workflows"))]
pub const TABS: [&str; 7] = [
pub const TABS: [&str; 6] = [
"Overview",
"Sessions",
"Subconscious",
"Changes",
"Hosts",
"Feedback",
"Settings",
Expand Down
Loading