From 18226b98e48348a9d6bdaf834a5168d2c19f923c Mon Sep 17 00:00:00 2001 From: Metal Agent Date: Tue, 1 Sep 2026 13:02:09 -0700 Subject: [PATCH] Separate what a pane is from the code that draws it SurfaceTab, PanelView and LiveRuntimeStatus are the three descriptions the renderer draws from, and none of them needs a renderer. Keeping them in main.rs meant the product's own boundaries -- a surface is a view over a pane and switching one launches nothing; an unsupported surface is shown as unavailable rather than simulated -- were stated in the middle of six hundred lines of layout and enforced by nobody. Seven tests state them instead. Every pane kind opens on a tab the tab bar actually draws, which is the difference between a workspace that opens ready and one that opens on an "unavailable" panel and reads as broken before the user has touched anything; Markdown is the one that is not its own name, since a document pane is read through CONTEXT. The six ordinals match their positions, because ordinal() is mixed into the element ID a pane's tabs are built with and a duplicate silently collides two tabs into one element. Every surface owes a complete sentence, and six distinct ones -- the reason a browser pane is empty and the reason review is empty are different facts about how much of the product exists, and one generic line repeated six times would erase that. A running PTY names its pid when the runtime reported one and says nothing when it did not, rather than printing "pid None" or a zero and sending someone hunting for a process that never existed. A fault carries its message through instead of summarising it, because "PTY FAULT" is the badge and a badge alone has never told anyone what to fix. And only the two states a process cannot leave are terminal: calling Starting terminal abandons a pane before it runs, and calling Exited non-terminal polls a dead actor forever. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SPYebLiN2w4TqnHUYGdECq --- apps/lumbridge/src/main.rs | 135 +------------- apps/lumbridge/src/surface.rs | 326 ++++++++++++++++++++++++++++++++++ 2 files changed, 328 insertions(+), 133 deletions(-) create mode 100644 apps/lumbridge/src/surface.rs diff --git a/apps/lumbridge/src/main.rs b/apps/lumbridge/src/main.rs index f2100a3..5d49d55 100644 --- a/apps/lumbridge/src/main.rs +++ b/apps/lumbridge/src/main.rs @@ -6,6 +6,7 @@ mod panel_registry; mod persistence; mod settings_view; mod sidebar; +mod surface; mod theme; mod timing; mod usage_feed; @@ -34,6 +35,7 @@ use lumbridge_ui_fixture::{ use attention::{Attention, AttentionKind, AttentionSignal, AttentionSource}; use input::{terminal_key_from_parts, terminal_scroll_from_parts}; +use surface::{LiveRuntimeStatus, PanelView, SurfaceTab}; use timing::RenderTiming; use geometry::{ @@ -180,96 +182,6 @@ struct LumbridgeShell { root_focus: FocusHandle, } -/// The surfaces a pane can be viewed through. -/// -/// A pane is the durable unit of work and its surface is a view over that work. -/// Switching one never launches a process, moves the pane, or changes which -/// agent owns the session — it only changes what is drawn. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -enum SurfaceTab { - Terminal, - Browser, - Tools, - Context, - Goal, - Review, -} - -impl SurfaceTab { - const ALL: [Self; 6] = [ - Self::Terminal, - Self::Browser, - Self::Tools, - Self::Context, - Self::Goal, - Self::Review, - ]; - - const fn label(self) -> &'static str { - match self { - Self::Terminal => "TERMINAL", - Self::Browser => "BROWSER", - Self::Tools => "TOOLS", - Self::Context => "CONTEXT", - Self::Goal => "GOAL", - Self::Review => "REVIEW", - } - } - - const fn ordinal(self) -> u64 { - match self { - Self::Terminal => 0, - Self::Browser => 1, - Self::Tools => 2, - Self::Context => 3, - Self::Goal => 4, - Self::Review => 5, - } - } - - /// The one surface a pane of this kind actually provides. - const fn native_for(kind: SurfaceKind) -> Self { - match kind { - SurfaceKind::Terminal => Self::Terminal, - SurfaceKind::Browser => Self::Browser, - SurfaceKind::Markdown => Self::Context, - SurfaceKind::Review => Self::Review, - } - } - - /// Why this pane cannot show this surface. Stated plainly, because the - /// product boundary says an unsupported surface is shown as unavailable - /// rather than simulated. - const fn unavailable_reason(self) -> &'static str { - match self { - Self::Terminal => "Only a terminal pane owns a live shell.", - Self::Browser => "No isolated web engine is wired yet.", - Self::Tools => "Tool calls arrive with the ACP client.", - Self::Context => "Context projection is not built yet.", - Self::Goal => "Goal tracking is not built yet.", - Self::Review => "Review is not wired to Git yet.", - } - } -} - -#[derive(Clone)] -struct PanelView { - id: PanelId, - kind: SurfaceKind, - title: String, - badge: String, - target: String, - status: PaneStatus, - lines: Vec, - output_source: OutputSource, -} - -impl PanelView { - const fn needs_input(&self) -> bool { - matches!(self.status, PaneStatus::NeedsInput) - } -} - struct LiveTerminalState { status: LiveRuntimeStatus, terminal: TerminalEngine, @@ -293,49 +205,6 @@ impl LiveTerminalState { } } -#[derive(Clone, Debug, Eq, PartialEq)] -enum LiveRuntimeStatus { - Starting, - Running { - session_id: u64, - process_id: Option, - }, - Exited(String), - Fault(String), -} - -impl LiveRuntimeStatus { - fn badge(&self) -> &'static str { - match self { - Self::Starting => "PTY STARTING", - Self::Running { .. } => "LIVE PTY", - Self::Exited(_) => "PTY EXITED", - Self::Fault(_) => "PTY FAULT", - } - } - - fn detail(&self) -> String { - match self { - Self::Starting => "local · runtime actor starting".to_owned(), - Self::Running { - session_id, - process_id, - } => match process_id { - Some(process_id) => { - format!("local · runtime session {session_id} · pid {process_id}") - } - None => format!("local · runtime session {session_id}"), - }, - Self::Exited(status) => format!("local · {status}"), - Self::Fault(message) => format!("local · {message}"), - } - } - - const fn is_terminal(&self) -> bool { - matches!(self, Self::Exited(_) | Self::Fault(_)) - } -} - #[derive(Clone, PartialEq)] struct TerminalPaintRun { text: String, diff --git a/apps/lumbridge/src/surface.rs b/apps/lumbridge/src/surface.rs new file mode 100644 index 0000000..67e91dc --- /dev/null +++ b/apps/lumbridge/src/surface.rs @@ -0,0 +1,326 @@ +//! What a pane *is*, what it can be *looked at* through, and what its process +//! is *doing* — the three descriptions the renderer draws from. +//! +//! A pane is the durable unit of work in Lumbridge; a surface is a view over +//! that work. Keeping the two apart is a product boundary, not a rendering +//! convenience: switching a tab never launches a process, never moves the pane, +//! and never changes which agent owns the session. It changes what is drawn and +//! nothing else, which is why [`SurfaceTab`] is a plain enum with no handle to +//! anything and no way to reach a runtime. +//! +//! The other rule this file encodes is that an unsupported surface is shown as +//! unavailable rather than simulated. Every tab exists on every pane, and the +//! five a pane cannot serve say plainly why — "No isolated web engine is wired +//! yet", not an empty panel that reads as a bug, and never a plausible-looking +//! mock that a user would take for a working feature. Those sentences are the +//! product's honesty about how much of itself is built, so they are asserted +//! here rather than left to whoever last edited the renderer. +//! +//! [`LiveRuntimeStatus`] is the same idea applied to a process: four states, a +//! badge for the header and a detail line, and no attempt to summarise a fault +//! into something reassuring. None of it needs a renderer, so none of it lives +//! next to one. + +use lumbridge_ui_fixture::{OutputSource, PaneStatus, SurfaceKind}; + +use crate::panel_registry::PanelId; + +/// The surfaces a pane can be viewed through. +/// +/// A pane is the durable unit of work and its surface is a view over that work. +/// Switching one never launches a process, moves the pane, or changes which +/// agent owns the session — it only changes what is drawn. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum SurfaceTab { + Terminal, + Browser, + Tools, + Context, + Goal, + Review, +} + +impl SurfaceTab { + pub(crate) const ALL: [Self; 6] = [ + Self::Terminal, + Self::Browser, + Self::Tools, + Self::Context, + Self::Goal, + Self::Review, + ]; + + pub(crate) const fn label(self) -> &'static str { + match self { + Self::Terminal => "TERMINAL", + Self::Browser => "BROWSER", + Self::Tools => "TOOLS", + Self::Context => "CONTEXT", + Self::Goal => "GOAL", + Self::Review => "REVIEW", + } + } + + pub(crate) const fn ordinal(self) -> u64 { + match self { + Self::Terminal => 0, + Self::Browser => 1, + Self::Tools => 2, + Self::Context => 3, + Self::Goal => 4, + Self::Review => 5, + } + } + + /// The one surface a pane of this kind actually provides. + pub(crate) const fn native_for(kind: SurfaceKind) -> Self { + match kind { + SurfaceKind::Terminal => Self::Terminal, + SurfaceKind::Browser => Self::Browser, + SurfaceKind::Markdown => Self::Context, + SurfaceKind::Review => Self::Review, + } + } + + /// Why this pane cannot show this surface. Stated plainly, because the + /// product boundary says an unsupported surface is shown as unavailable + /// rather than simulated. + pub(crate) const fn unavailable_reason(self) -> &'static str { + match self { + Self::Terminal => "Only a terminal pane owns a live shell.", + Self::Browser => "No isolated web engine is wired yet.", + Self::Tools => "Tool calls arrive with the ACP client.", + Self::Context => "Context projection is not built yet.", + Self::Goal => "Goal tracking is not built yet.", + Self::Review => "Review is not wired to Git yet.", + } + } +} + +#[derive(Clone)] +pub(crate) struct PanelView { + pub(crate) id: PanelId, + pub(crate) kind: SurfaceKind, + pub(crate) title: String, + pub(crate) badge: String, + pub(crate) target: String, + pub(crate) status: PaneStatus, + pub(crate) lines: Vec, + pub(crate) output_source: OutputSource, +} + +impl PanelView { + pub(crate) const fn needs_input(&self) -> bool { + matches!(self.status, PaneStatus::NeedsInput) + } +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) enum LiveRuntimeStatus { + Starting, + Running { + session_id: u64, + process_id: Option, + }, + Exited(String), + Fault(String), +} + +impl LiveRuntimeStatus { + pub(crate) fn badge(&self) -> &'static str { + match self { + Self::Starting => "PTY STARTING", + Self::Running { .. } => "LIVE PTY", + Self::Exited(_) => "PTY EXITED", + Self::Fault(_) => "PTY FAULT", + } + } + + pub(crate) fn detail(&self) -> String { + match self { + Self::Starting => "local · runtime actor starting".to_owned(), + Self::Running { + session_id, + process_id, + } => match process_id { + Some(process_id) => { + format!("local · runtime session {session_id} · pid {process_id}") + } + None => format!("local · runtime session {session_id}"), + }, + Self::Exited(status) => format!("local · {status}"), + Self::Fault(message) => format!("local · {message}"), + } + } + + pub(crate) const fn is_terminal(&self) -> bool { + matches!(self, Self::Exited(_) | Self::Fault(_)) + } +} + +#[cfg(test)] +mod tests { + use super::{LiveRuntimeStatus, SurfaceTab}; + use lumbridge_ui_fixture::SurfaceKind; + + #[test] + fn every_pane_kind_opens_on_the_surface_it_actually_provides() { + // The failure this prevents is a pane whose default tab is one it + // cannot serve: the workspace would open on an "unavailable" panel and + // read as broken before the user has touched anything. + for kind in [ + SurfaceKind::Terminal, + SurfaceKind::Browser, + SurfaceKind::Markdown, + SurfaceKind::Review, + ] { + let native = SurfaceTab::native_for(kind); + assert!( + SurfaceTab::ALL.contains(&native), + "{kind:?} opens on a tab the tab bar does not draw" + ); + } + + // Markdown is the one that is not its own name: there is no MARKDOWN + // tab, and a document pane is read through CONTEXT. + assert_eq!( + SurfaceTab::native_for(SurfaceKind::Markdown), + SurfaceTab::Context + ); + assert_eq!( + SurfaceTab::native_for(SurfaceKind::Terminal), + SurfaceTab::Terminal + ); + assert_eq!( + SurfaceTab::native_for(SurfaceKind::Browser), + SurfaceTab::Browser + ); + assert_eq!( + SurfaceTab::native_for(SurfaceKind::Review), + SurfaceTab::Review + ); + } + + #[test] + fn the_tab_bar_lists_every_surface_exactly_once_and_in_a_stable_order() { + // ordinal() is mixed into the element ID a pane's tabs are built with, + // so a duplicate would collide two tabs into one element, and a gap + // would only show up as a tab that cannot be clicked. + for (index, tab) in SurfaceTab::ALL.into_iter().enumerate() { + assert_eq!( + u64::try_from(index).expect("six tabs fit in a u64"), + tab.ordinal(), + "{tab:?} is at position {index} but reports ordinal {}", + tab.ordinal() + ); + } + } + + #[test] + fn no_surface_is_unavailable_without_saying_why() { + // An unsupported surface is shown as unavailable rather than simulated, + // and "unavailable" with no sentence after it is indistinguishable from + // a panel that failed to render. + for tab in SurfaceTab::ALL { + let reason = tab.unavailable_reason(); + assert!( + !reason.is_empty() && reason.ends_with('.'), + "{tab:?} owes the user a complete sentence, not {reason:?}" + ); + assert!( + !tab.label().is_empty(), + "{tab:?} has no label for the tab bar" + ); + } + + // Distinct sentences, not one generic line repeated six times: the + // reason a browser pane is empty and the reason review is empty are + // different facts about how much of the product exists. + let mut reasons = SurfaceTab::ALL.map(SurfaceTab::unavailable_reason).to_vec(); + reasons.sort_unstable(); + let before = reasons.len(); + reasons.dedup(); + assert_eq!(before, reasons.len(), "two surfaces share an excuse"); + } + + #[test] + fn a_running_pty_names_its_pid_when_the_runtime_reported_one() { + // The pid is what a user needs to go and look at the process from + // outside Lumbridge, and it is genuinely absent on a session the actor + // has accepted but not yet spawned. Printing "pid None" or a zero would + // send someone hunting for a process that does not exist. + assert_eq!( + LiveRuntimeStatus::Running { + session_id: 7, + process_id: Some(4321), + } + .detail(), + "local · runtime session 7 · pid 4321" + ); + assert_eq!( + LiveRuntimeStatus::Running { + session_id: 7, + process_id: None, + } + .detail(), + "local · runtime session 7" + ); + } + + #[test] + fn every_status_reports_where_it_is_running_and_a_fault_keeps_its_message() { + assert_eq!( + LiveRuntimeStatus::Starting.detail(), + "local · runtime actor starting" + ); + // The message is carried through rather than summarised: "PTY FAULT" is + // the badge, and the badge alone never tells anyone what to fix. + assert_eq!( + LiveRuntimeStatus::Fault("spawn failed: no such file".to_owned()).detail(), + "local · spawn failed: no such file" + ); + assert_eq!( + LiveRuntimeStatus::Exited("exited with status 130".to_owned()).detail(), + "local · exited with status 130" + ); + } + + #[test] + fn only_the_states_a_process_cannot_leave_are_terminal() { + // is_terminal decides whether the event drain keeps polling a pane. + // Calling Starting terminal would abandon a pane before it ever ran; + // calling Exited non-terminal would poll a dead actor forever. + assert!(LiveRuntimeStatus::Exited(String::new()).is_terminal()); + assert!(LiveRuntimeStatus::Fault(String::new()).is_terminal()); + assert!(!LiveRuntimeStatus::Starting.is_terminal()); + assert!( + !LiveRuntimeStatus::Running { + session_id: 1, + process_id: None, + } + .is_terminal() + ); + } + + #[test] + fn each_status_has_its_own_badge() { + let badges = [ + LiveRuntimeStatus::Starting.badge(), + LiveRuntimeStatus::Running { + session_id: 1, + process_id: None, + } + .badge(), + LiveRuntimeStatus::Exited(String::new()).badge(), + LiveRuntimeStatus::Fault(String::new()).badge(), + ]; + let mut sorted = badges.to_vec(); + sorted.sort_unstable(); + sorted.dedup(); + assert_eq!( + sorted.len(), + badges.len(), + "two runtime states would look identical in the pane header" + ); + } +}