From 4b5d92249ef037af6efcc3c9f42ff8d0d54926ce Mon Sep 17 00:00:00 2001 From: Metal Agent Date: Tue, 1 Sep 2026 13:14:01 -0700 Subject: [PATCH] Write the accessibility adapter decision 0017 promised Decision 0017 chose published gpui 0.2.2 over a Zed git revision, accepted that the crate has no AccessKit API at all, and said the mitigation was "a narrow `a11y` adapter trait, introduced now while there are few call sites" so that moving off 0.2.2 would be "a swap rather than a rewrite". No such trait, module, or file was ever written. Decision 0023 records what that cost: `apps/lumbridge/src` now builds 147 elements and gives 20 of them a stable identity, and the only trace of the promise is a `describe()` in the sidebar model that produces the screen-reader sentence, is tested, is called by nothing, and carries an `#[allow(dead_code)]` naming a record it outlived. `apps/lumbridge/src/a11y.rs` is that adapter, landed on 0.2.2 as stage 1 of decision 0023 and before the dependency moves, so the dependency change is paid for on its own. The trait carries the five operations `spikes/gpui-accessibility-probe` proves compile at Zed `ce48461e` -- role, label, description, selected state, and a stable accessibility identity -- implemented for `Div` and `Stateful
`, which are the only two builders the shell constructs. Every body drops the value it is given and returns `self`. That is the whole point: 0.2.2 has nothing to hand the value to, so the file is a vocabulary and a set of call sites, not a feature. What it is not is a blanket `#[allow]` -- the arguments are consumed by a `discard` helper so `clippy::pedantic` passes on the code's merits, and the next real mistake in this file is still caught. Two choices exist only to keep stage 5 confined to this file. The methods are prefixed `a11y_` because GPUI's own builders at the target revision are named `role`, `aria_label`, `aria_description`, `aria_selected` and `accessibility_id`, and a trait of ours carrying those names would make every call site ambiguous the moment both are in scope -- which is the rewrite the adapter exists to avoid. `Role` is Lumbridge's own enum rather than an alias, because `gpui::Role` does not exist to alias and because the enum should name what the product claims, not what today's dependency happens to spell. The derivations sit here too, and are the testable part: `PaneSemantics` derives the four facts `UX_VERTICAL_SLICE.md`'s hard gate names -- which pane, its selected state, its execution target, its waiting state -- from strings the pane header already prints, so an assistive technology cannot announce a machine the screen is not showing. The gate is not met and this does not move it. No accessibility tree is produced, nothing reaches AT-SPI or VoiceOver, and no test here is an assistive-technology claim. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SPYebLiN2w4TqnHUYGdECq --- apps/lumbridge/src/a11y.rs | 545 +++++++++++++++++++++++++++++++++++++ 1 file changed, 545 insertions(+) create mode 100644 apps/lumbridge/src/a11y.rs diff --git a/apps/lumbridge/src/a11y.rs b/apps/lumbridge/src/a11y.rs new file mode 100644 index 0000000..9c359bc --- /dev/null +++ b/apps/lumbridge/src/a11y.rs @@ -0,0 +1,545 @@ +//! What the shell's elements *mean*, kept in one file so the framework move is a +//! swap and not a rewrite. +//! +//! Decision 0017 promised "a narrow `a11y` adapter trait, introduced now while +//! there are few call sites" and never wrote one; decision 0023 records what +//! that cost — 147 elements and 20 stable identities later, the call sites are +//! no longer few — and makes writing the adapter the first stage of the move to +//! a pinned Zed revision, *before* the dependency changes. +//! +//! This is that adapter. It exists so the eventual dependency swap changes this +//! file and nothing else. +//! +//! # Why the bodies do nothing +//! +//! The product compiles against published `gpui 0.2.2`. That crate has no +//! AccessKit dependency at all: no `Role`, no `aria_label`, no +//! `accessibility_id`, no semantic tree of any kind. So every method below +//! accepts the semantic value it is given and drops it. **Nothing in this file +//! produces an accessibility tree today, and the hard gate in +//! `UX_VERTICAL_SLICE.md` is still unmet.** What it produces is call sites: the +//! places that know a pane's name, its execution target, whether it is selected +//! and whether it is waiting on a human now *say* so, in code, in the vocabulary +//! the target revision understands. +//! +//! `spikes/gpui-accessibility-probe` pins Zed `ce48461e` and proves the real API +//! compiles — `role`, `aria_label`, `aria_description`, `aria_selected`, +//! `accessibility_id` — so the shape below is not a guess about a future API. It +//! is the shape of an API that exists, deliberately kept one dependency line +//! away. +//! +//! # Why the methods are prefixed `a11y_` +//! +//! At the target revision GPUI's own builders are named `role`, `aria_label`, +//! `aria_description`, `aria_selected` and `accessibility_id`. A trait of ours +//! carrying those names would, the moment both are in scope, make every call +//! site ambiguous — and resolving that ambiguity at 40 call sites is exactly the +//! rewrite this adapter exists to avoid. The prefix is ugly on purpose: it is +//! what keeps stage 5 a change to this file. +//! +//! # Why Lumbridge declares its own `Role` +//! +//! Re-exporting `gpui::Role` is impossible — it does not exist in 0.2.2 — and +//! aliasing it to something local would make the enum's spelling a fact about +//! today's dependency rather than a fact about the product. [`Role`] is +//! therefore Lumbridge's own vocabulary, listing only the roles the shell +//! actually claims, and stage 5 adds the one function that maps it onto +//! `accesskit::Role`. +//! +//! # What stage 5 changes here +//! +//! The default method bodies (drop the value, return `self` unchanged) become +//! calls into GPUI's builders, [`Role`] gains a mapping to `accesskit::Role`, +//! and [`discard`] is deleted. The derivations below — [`PaneSemantics`], +//! [`sidebar_row_semantics`], the identifier helpers — do not change, because +//! they are about Lumbridge's model and not about the framework. + +use gpui::{Div, SharedString, Stateful}; + +use crate::sidebar::model::{RowBody, RowKey, Section, SidebarRow, describe}; +use crate::surface::PanelView; + +/// The semantic roles Lumbridge claims for its own elements. +/// +/// Deliberately not the whole of `accesskit::Role`. A role listed here is one +/// the shell actually applies to an element, so the enum doubles as the list of +/// mappings stage 5 has to write and nothing more. +/// +/// `Application`, `Region`, `Pane`, `Terminal` and `Status` are the five the +/// accessibility probe already renders at Zed `ce48461e`, so their spelling is +/// known-good. `Dialog`, `List` and `ListItem` are standard ARIA roles that +/// AccessKit is modelled on, but this repository has not compiled them; stage 5 +/// confirms each spelling against `accesskit::Role` rather than assuming it. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum Role { + /// The window root. One per process. + Application, + /// A named area of the window: the sidebar, the workspace row, the usage + /// strip in the footer. + Region, + /// One workspace panel — the durable unit of work, in `surface.rs`'s terms. + Pane, + /// A live terminal surface. Its *contents* are not exposed as a tree; a + /// screen reader reaches terminal text through the platform's terminal + /// support, not through 80×24 element nodes per frame. + Terminal, + /// A live region reporting a changing value. + Status, + /// A modal overlay that takes the keyboard: the command palette, the + /// add-panel chooser. + Dialog, + /// The sidebar's flat row list. + List, + /// One sidebar row. + ListItem, +} + +/// Accepts a semantic value and drops it. +/// +/// The honest spelling of a no-op. Writing `self` and ignoring the argument +/// would need either an `_`-prefixed parameter — which loses the name that +/// documents the method — or a module-wide `#[allow]`, which would also +/// suppress the next real mistake made in this file. Moving the value into a +/// function that consumes it says "this value is deliberately going nowhere +/// yet" in a way `clippy::pedantic` accepts on its merits. +/// +/// Deleted by stage 5, when every caller has something real to do with its +/// argument. +fn discard(_value: T) {} + +/// The semantic operations Lumbridge asks of an element builder. +/// +/// Implemented for the two builder types the shell actually constructs: `Div`, +/// and the `Stateful
` a `Div` becomes once it is given a GPUI `ElementId`. +/// Everything the shell draws is one of those two, so there is no third impl to +/// keep in step. +/// +/// Every method takes `self` and returns `Self`, matching GPUI's builder style, +/// so an annotation drops into an existing chain without restructuring it. +pub(crate) trait A11y: Sized { + /// What kind of thing this element is. + /// + /// Becomes `role(...)` at the target revision. + #[must_use] + fn a11y_role(self, role: Role) -> Self { + discard(role); + self + } + + /// The name a screen reader announces for this element. + /// + /// Becomes `aria_label(...)`. The string must be one the model already + /// holds: a label an assistive technology speaks and a label a sighted user + /// reads that disagree are two descriptions of one thing, and one of them is + /// wrong. + #[must_use] + fn a11y_label(self, label: impl Into) -> Self { + discard(label.into()); + self + } + + /// The supporting sentence: what state this element is in, beyond its name. + /// + /// Becomes `aria_description(...)`. This is where a pane's waiting state + /// lives, which is one of the four facts `UX_VERTICAL_SLICE.md` requires the + /// tree to carry. + #[must_use] + fn a11y_description(self, description: impl Into) -> Self { + discard(description.into()); + self + } + + /// Whether this element is the selected one among its siblings. + /// + /// Becomes `aria_selected(...)`. Passed unconditionally rather than only + /// when true: an element that never reports `false` is indistinguishable + /// from one whose selection is unknown. + #[must_use] + fn a11y_selected(self, selected: bool) -> Self { + discard(selected); + self + } + + /// A stable, externally addressable identity for this element. + /// + /// Becomes `accessibility_id(...)`, which AccessKit surfaces as a node's + /// author id. Distinct from GPUI's `ElementId`: an `ElementId` is a + /// within-frame identity used to keep interaction state attached to the + /// right element, while this is a name a test harness or an assistive + /// technology can look up across runs. They are set together at every call + /// site here, because an element worth naming to a screen reader is an + /// element worth keeping state on. + #[must_use] + fn a11y_id(self, id: impl Into) -> Self { + discard(id.into()); + self + } +} + +impl A11y for Div {} +impl A11y for Stateful
{} + +/// The prefix every Lumbridge accessibility identifier carries. +/// +/// A node id is global to the platform's accessibility bus, not local to this +/// window, so `pane.3` is not a name — it is a collision waiting for the next +/// application that also has panes. +const ID_PREFIX: &str = "lumbridge"; + +/// The accessibility identity of a workspace pane. +/// +/// Built from the panel registry's durable id rather than from a position, for +/// the same reason `RowKey` is not an index: a pane keeps its identity when the +/// pane to its left is detached, and an identity that silently moves to a +/// different pane is worse than no identity. +pub(crate) fn pane_id(pane: &PanelView) -> String { + format!("{ID_PREFIX}.pane.{}", pane.id.get()) +} + +/// The accessibility identity of a sidebar row. +/// +/// Derived from the same [`RowKey`] that `sidebar::view::element_id` uses, so a +/// row's element identity and its accessibility identity cannot come apart — +/// which they would if one were keyed on the row's position in the flattened +/// list and the other on its key. +pub(crate) fn sidebar_row_id(key: &RowKey) -> String { + match key { + RowKey::Header(section) => format!("{ID_PREFIX}.sidebar.header.{}", section_slug(*section)), + RowKey::Empty(section) => format!("{ID_PREFIX}.sidebar.empty.{}", section_slug(*section)), + RowKey::Attention(id) => format!("{ID_PREFIX}.sidebar.attention.{id}"), + RowKey::Pane(id) => format!("{ID_PREFIX}.sidebar.pane.{id}"), + RowKey::Detached(id) => format!("{ID_PREFIX}.sidebar.detached.{id}"), + RowKey::Host(index) => format!("{ID_PREFIX}.sidebar.host.{index}"), + RowKey::Quota(id) => format!("{ID_PREFIX}.sidebar.quota.{id}"), + } +} + +/// A section's name, lowercased and hyphenated for an identifier. +/// +/// Taken from the section's own title rather than from a second table, so a +/// renamed section cannot end up with an identifier naming the old one. +fn section_slug(section: Section) -> String { + section.title().to_lowercase().replace(' ', "-") +} + +/// The standing of a pane, in the words its header already prints. +/// +/// The pane header's bottom-right corner shows exactly one of these three, and +/// so does the pane's accessibility description. Written once, here, because the +/// alternative is a screen reader saying "waiting" while the screen says +/// "RUNNING" — the class of untruth `AGENTS.md` exists to prevent. +pub(crate) const fn pane_standing(needs_input: bool, selected: bool) -> &'static str { + if needs_input { + "NEEDS INPUT" + } else if selected { + "KEYBOARD OWNER" + } else { + "RUNNING" + } +} + +/// Everything the accessibility tree says about one workspace pane. +/// +/// `UX_VERTICAL_SLICE.md`'s hard gate names four facts — the pane, its selected +/// state, its execution target, and its waiting state — so they are derived +/// together, in one place, and tested as a unit. Deriving them at each call site +/// is how three of the four end up correct and the fourth ends up stale. +/// +/// None of the strings are invented here. [`PaneSemantics::label`] is the pane +/// title and the detail line the header prints; [`PaneSemantics::description`] +/// is the standing the header prints and the status line beside it. An +/// assistive technology hears what is on the screen. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct PaneSemantics { + /// Stable across rebuilds; see [`pane_id`]. + pub(crate) accessibility_id: String, + /// The pane's name and where it runs. + pub(crate) label: String, + /// The pane's standing and what its process is doing. + pub(crate) description: String, + /// Whether this pane owns the keyboard. + pub(crate) selected: bool, +} + +impl PaneSemantics { + /// Derives the four facts from the pane and the two strings its header + /// prints. + /// + /// `detail` and `status` are passed in rather than recomputed because the + /// header computes them from live runtime state the pane view does not + /// carry: a pane's declared target is "local shell" while its attached PTY + /// reports "local · runtime session 7 · pid 4321", and the tree must say + /// whichever one the header says. + pub(crate) fn derive(pane: &PanelView, selected: bool, detail: &str, status: &str) -> Self { + Self { + accessibility_id: pane_id(pane), + label: format!("{}, {detail}", pane.title), + description: format!("{}, {status}", pane_standing(pane.needs_input(), selected)), + selected, + } + } +} + +/// Everything the accessibility tree says about one sidebar row. +/// +/// The label is [`describe`] and only [`describe`]. Decision 0017 wrote that +/// function, tested it, and left it uncalled behind an `#[allow(dead_code)]`; +/// decision 0023 requires that allow be deleted rather than carried, which means +/// something has to call it. This is the caller. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct RowSemantics { + pub(crate) accessibility_id: String, + /// The screen-reader sentence from `sidebar::model::describe`. + pub(crate) label: String, + /// A row is selected when it is the pane that owns the keyboard. The + /// keyboard *cursor* is a different thing and is not reported here: it moves + /// with the arrow keys and does not change what is running. + pub(crate) selected: bool, +} + +/// Derives one sidebar row's semantics. +pub(crate) fn sidebar_row_semantics(row: &SidebarRow) -> RowSemantics { + RowSemantics { + accessibility_id: sidebar_row_id(&row.key), + label: describe(row), + selected: matches!(&row.body, RowBody::Pane(pane) if pane.selected), + } +} + +#[cfg(test)] +mod tests { + use lumbridge_ui_fixture::{OutputSource, PaneStatus, SurfaceKind}; + + use super::{ + PaneSemantics, Role, pane_id, pane_standing, sidebar_row_id, sidebar_row_semantics, + }; + use crate::panel_registry::PanelId; + use crate::sidebar::model::{ + PaneEntry, RowKey, Section, SidebarInput, SidebarState, describe, flatten, + }; + use crate::surface::PanelView; + + fn panel(id: u64, status: PaneStatus) -> PanelView { + PanelView { + id: PanelId::from_raw(id), + kind: SurfaceKind::Terminal, + title: format!("Terminal {id}"), + badge: "TERMINAL".to_owned(), + target: "local shell".to_owned(), + status, + lines: Vec::new(), + output_source: OutputSource::External, + } + } + + #[test] + fn a_selected_pane_and_an_unselected_one_differ_in_more_than_a_flag() { + // The gate is that the tree names the selected state, not that the + // renderer knows it. Two panes drawn from the same title must still be + // distinguishable to an assistive technology, and their standing — the + // words the header prints — has to move with the selection too. + let pane = panel(3, PaneStatus::Ready); + let owner = PaneSemantics::derive(&pane, true, "local shell", "LIVE PTY"); + let bystander = PaneSemantics::derive(&pane, false, "local shell", "LIVE PTY"); + + assert!(owner.selected); + assert!(!bystander.selected); + assert_ne!(owner, bystander); + assert!( + owner.description.starts_with("KEYBOARD OWNER"), + "the selected pane owes its standing, not just a boolean: {}", + owner.description + ); + assert!( + bystander.description.starts_with("RUNNING"), + "an unselected pane is running, not idle: {}", + bystander.description + ); + assert_eq!( + owner.accessibility_id, bystander.accessibility_id, + "selection is a state of a pane, never a different pane" + ); + } + + #[test] + fn a_pane_waiting_on_a_human_says_so_before_anything_else() { + // "The accessibility tree names each pane, selected state, execution + // target, and waiting state." The waiting state leads the description, + // because a screen reader is read from the front and a person scanning + // for the pane that needs them should not have to hear the PTY geometry + // first. + let waiting = PaneSemantics::derive( + &panel(4, PaneStatus::NeedsInput), + false, + "local shell", + "LIVE PTY · 80×24", + ); + assert!( + waiting.description.starts_with("NEEDS INPUT"), + "{}", + waiting.description + ); + // And a waiting pane that also owns the keyboard still leads with the + // waiting: owning the keyboard is not the fact anyone is waiting for. + let waiting_and_selected = PaneSemantics::derive( + &panel(4, PaneStatus::NeedsInput), + true, + "local shell", + "LIVE PTY", + ); + assert!(waiting_and_selected.description.starts_with("NEEDS INPUT")); + } + + #[test] + fn a_panes_label_names_where_it_runs() { + // The execution target is one of the four facts the gate names, and it + // is the one a label is most easily written without: "Terminal 3" is a + // perfectly readable name that does not say which machine it is on. + let semantics = PaneSemantics::derive( + &panel(3, PaneStatus::Ready), + true, + "local · runtime session 7 · pid 4321", + "LIVE PTY", + ); + assert_eq!( + semantics.label, + "Terminal 3, local · runtime session 7 · pid 4321" + ); + assert_eq!(semantics.accessibility_id, "lumbridge.pane.3"); + } + + #[test] + fn the_standing_a_pane_announces_is_the_standing_it_prints() { + assert_eq!(pane_standing(true, false), "NEEDS INPUT"); + assert_eq!(pane_standing(true, true), "NEEDS INPUT"); + assert_eq!(pane_standing(false, true), "KEYBOARD OWNER"); + assert_eq!(pane_standing(false, false), "RUNNING"); + } + + #[test] + fn a_panes_identity_follows_the_pane_and_not_its_position() { + assert_eq!(pane_id(&panel(1, PaneStatus::Ready)), "lumbridge.pane.1"); + assert_eq!(pane_id(&panel(97, PaneStatus::Ready)), "lumbridge.pane.97"); + } + + #[test] + fn every_sidebar_row_the_renderer_draws_is_labelled_by_describe() { + // `describe` was written, tested, and called by nothing for the whole + // life of decision 0017. `sidebar_row_semantics` is the function + // `render_sidebar` now calls for every row it builds, so asserting that + // its label is `describe`'s output is asserting that the rendered + // sidebar is the described sidebar. + let mut input = SidebarInput::default(); + input.panes.push(PaneEntry { + id: 1, + title: "Terminal 1".to_owned(), + target: "local shell".to_owned(), + kind_glyph: "▸", + selected: true, + indicator: crate::sidebar::model::Indicator::None, + quota: None, + quota_critical: false, + }); + input.panes.push(PaneEntry { + id: 2, + title: "Notes".to_owned(), + target: "local shell".to_owned(), + kind_glyph: "¶", + selected: false, + indicator: crate::sidebar::model::Indicator::None, + quota: None, + quota_critical: false, + }); + + let rows = flatten(&input, &SidebarState::default()); + assert!(!rows.is_empty()); + for row in &rows { + let semantics = sidebar_row_semantics(row); + assert_eq!( + semantics.label, + describe(row), + "{:?} is labelled by something other than describe()", + row.key + ); + assert!(!semantics.accessibility_id.is_empty()); + } + + let selected = rows + .iter() + .find(|row| row.key == RowKey::Pane(1)) + .expect("the selected pane has a row"); + let unselected = rows + .iter() + .find(|row| row.key == RowKey::Pane(2)) + .expect("the unselected pane has a row"); + assert!(sidebar_row_semantics(selected).selected); + assert!(!sidebar_row_semantics(unselected).selected); + } + + #[test] + fn every_row_key_gets_its_own_identity() { + // Two rows sharing an accessibility id would collapse into one node on + // the accessibility bus, which is the same failure mode the surface tab + // ordinal test guards against and just as invisible on screen. + let mut input = SidebarInput::default(); + input.panes.push(PaneEntry { + id: 1, + title: "Terminal 1".to_owned(), + target: "local shell".to_owned(), + kind_glyph: "▸", + selected: true, + indicator: crate::sidebar::model::Indicator::None, + quota: None, + quota_critical: false, + }); + input.detached.push(PaneEntry { + id: 1, + title: "Terminal 1".to_owned(), + target: "local shell".to_owned(), + kind_glyph: "▸", + selected: false, + indicator: crate::sidebar::model::Indicator::None, + quota: None, + quota_critical: false, + }); + + let rows = flatten(&input, &SidebarState::default()); + let mut ids: Vec = rows.iter().map(|row| sidebar_row_id(&row.key)).collect(); + let before = ids.len(); + ids.sort_unstable(); + ids.dedup(); + assert_eq!( + before, + ids.len(), + "an attached pane and its detached namesake must not share a node" + ); + + // The section slug comes from the section's own title, so a renamed + // section cannot keep an identifier naming the old one. + assert_eq!( + sidebar_row_id(&RowKey::Header(Section::NeedsYou)), + "lumbridge.sidebar.header.needs-you" + ); + } + + #[test] + fn the_roles_lumbridge_claims_are_distinct() { + // The enum is the list of mappings stage 5 has to write. Two variants + // that compare equal would mean one of those mappings is missing. + let roles = [ + Role::Application, + Role::Region, + Role::Pane, + Role::Terminal, + Role::Status, + Role::Dialog, + Role::List, + Role::ListItem, + ]; + for (index, role) in roles.iter().enumerate() { + for other in &roles[index + 1..] { + assert_ne!(role, other); + } + } + } +}