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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SPYebLiN2w4TqnHUYGdECq
This commit is contained in:
co-authored by
Claude Opus 5
parent
beaffbd0a7
commit
18226b98e4
+2
-133
@@ -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<String>,
|
||||
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<u32>,
|
||||
},
|
||||
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,
|
||||
|
||||
@@ -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<String>,
|
||||
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<u32>,
|
||||
},
|
||||
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"
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user