From c66b7e89e70d48b6f138a4e73e385f975962f16e Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 19:54:25 +0800 Subject: [PATCH 01/26] chore: ignore subagent-driven-development scratch directory --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 81e97ab..854f386 100644 --- a/.gitignore +++ b/.gitignore @@ -105,3 +105,6 @@ docs/nodedb_lab_studio_mockup_v4.html specs/* docs/* .DS_Store + +# Subagent-driven-development scratch (ledger, briefs, review packages) +.superpowers/ From 593c9a88cb46143e7a9a8b2ce05ec5625203e5e7 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 19:57:09 +0800 Subject: [PATCH 02/26] feat(seam): column-addressed row decoder with column assertion Result sets whose columns do not match the decoder's expectation now raise UnexpectedColumns instead of decoding to an empty list. The server answers some introspection statements with a session-variable fallback carrying a single 'setting' column, which would otherwise render as a working-but-empty screen. --- nodedb-studio/src/services/decode.rs | 141 +++++++++++++++++++++++++++ nodedb-studio/src/services/error.rs | 8 +- nodedb-studio/src/services/mod.rs | 1 + 3 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 nodedb-studio/src/services/decode.rs diff --git a/nodedb-studio/src/services/decode.rs b/nodedb-studio/src/services/decode.rs new file mode 100644 index 0000000..cf004b3 --- /dev/null +++ b/nodedb-studio/src/services/decode.rs @@ -0,0 +1,141 @@ +//! Column-addressed decoding of tabular results into typed `models/` structs. +//! +//! Seam methods return Studio models, but the real implementation will build +//! them from a `QueryResult{columns, rows}`. `Table` is that shape expressed in +//! Studio's own terms, so decoders are unit-testable with no client or server. +//! +//! Every value arrives as a string: the server returns timestamps, counts and +//! booleans as strings. Decoders parse at the point of use. + +use crate::services::error::StudioError; + +/// A tabular result: column names plus rows of stringly values. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct Table { + pub columns: Vec, + pub rows: Vec>, +} + +/// One row, addressable by column name. +#[allow(dead_code)] +pub struct Row<'a> { + columns: &'a [String], + cells: &'a [String], +} + +impl<'a> Row<'a> { + /// The cell under `name`, or an error naming the column that is missing. + #[allow(dead_code)] + pub fn field(&self, name: &str) -> Result<&'a str, StudioError> { + let idx = self.columns.iter().position(|c| c == name).ok_or_else(|| { + StudioError::UnexpectedColumns { + expected: name.to_string(), + got: self.columns.join(", "), + } + })?; + self.cells + .get(idx) + .map(String::as_str) + .ok_or_else(|| StudioError::UnexpectedColumns { + expected: name.to_string(), + got: format!("row has {} cells", self.cells.len()), + }) + } +} + +/// Decode every row of `table` with `f`, after asserting `expect` columns are +/// all present. +/// +/// The assertion is the point: it turns the server's silent session-variable +/// fallback into a typed error rather than an empty result set. +#[allow(dead_code)] +pub fn decode_rows( + table: &Table, + expect: &[&str], + f: impl Fn(Row<'_>) -> Result, +) -> Result, StudioError> { + if let Some(missing) = expect + .iter() + .find(|e| !table.columns.iter().any(|c| c == *e)) + { + return Err(StudioError::UnexpectedColumns { + expected: format!("{} (missing: {missing})", expect.join(", ")), + got: table.columns.join(", "), + }); + } + table + .rows + .iter() + .map(|cells| { + f(Row { + columns: &table.columns, + cells, + }) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn table(cols: &[&str], rows: &[&[&str]]) -> Table { + Table { + columns: cols.iter().map(|s| s.to_string()).collect(), + rows: rows + .iter() + .map(|r| r.iter().map(|s| s.to_string()).collect()) + .collect(), + } + } + + #[test] + fn decodes_rows_by_column_name() { + let t = table(&["name", "owner"], &[&["probe_docs", "admin"]]); + let out = decode_rows(&t, &["name", "owner"], |r| { + Ok(format!("{}/{}", r.field("name")?, r.field("owner")?)) + }) + .expect("decode must succeed"); + assert_eq!(out, vec!["probe_docs/admin".to_string()]); + } + + #[test] + fn column_order_does_not_matter() { + let t = table(&["owner", "name"], &[&["admin", "probe_docs"]]); + let out = decode_rows(&t, &["name", "owner"], |r| Ok(r.field("name")?.to_string())) + .expect("decode must succeed"); + assert_eq!(out, vec!["probe_docs".to_string()]); + } + + /// The server answers some SHOW statements with a session-variable + /// fallback: cols=["setting"] and one empty row. That must be a typed + /// error, never an empty list, or the screen renders "working but empty". + #[test] + fn setting_fallback_is_an_error_not_empty() { + let t = table(&["setting"], &[&[""]]); + let out = decode_rows(&t, &["name", "collection"], |r| { + Ok(r.field("name")?.to_string()) + }); + assert!( + matches!(out, Err(StudioError::UnexpectedColumns { .. })), + "expected UnexpectedColumns, got {out:?}" + ); + } + + #[test] + fn genuinely_empty_result_is_ok_and_empty() { + let t = table(&["name", "collection"], &[]); + let out = decode_rows(&t, &["name", "collection"], |r| { + Ok(r.field("name")?.to_string()) + }) + .expect("empty rows with correct columns is a valid empty result"); + assert!(out.is_empty()); + } + + #[test] + fn missing_field_is_an_error() { + let t = table(&["name"], &[&["x"]]); + let out = decode_rows(&t, &["name"], |r| Ok(r.field("nope")?.to_string())); + assert!(out.is_err()); + } +} diff --git a/nodedb-studio/src/services/error.rs b/nodedb-studio/src/services/error.rs index a3e7c7a..ec92c68 100644 --- a/nodedb-studio/src/services/error.rs +++ b/nodedb-studio/src/services/error.rs @@ -30,6 +30,12 @@ pub enum StudioError { Server(#[source] NodeDbError), #[error("not connected to a database")] NotConnected, + /// A result set did not carry the columns the decoder needs. Most often the + /// server answered a `SHOW` with its session-variable fallback + /// (`cols=["setting"]`), which would otherwise read as an empty screen. + #[error("unexpected result columns: expected [{expected}], got [{got}]")] + #[allow(dead_code)] + UnexpectedColumns { expected: String, got: String }, } impl StudioError { @@ -37,7 +43,7 @@ impl StudioError { /// `NotConnected` is never retriable (it is studio-originated, not transient). pub fn is_retriable(&self) -> bool { match self { - StudioError::NotConnected => false, + StudioError::NotConnected | StudioError::UnexpectedColumns { .. } => false, StudioError::Connection(e) | StudioError::Auth(e) | StudioError::NotFound(e) diff --git a/nodedb-studio/src/services/mod.rs b/nodedb-studio/src/services/mod.rs index 3a007ca..9d624d1 100644 --- a/nodedb-studio/src/services/mod.rs +++ b/nodedb-studio/src/services/mod.rs @@ -4,6 +4,7 @@ pub mod async_state; pub mod backend; pub mod connection_service; +pub mod decode; pub mod error; pub mod nodedb_service; pub mod streams_data; From 4084a00fe1e7fb2019881045a3bf2f6f758ff85a Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 20:01:35 +0800 Subject: [PATCH 03/26] fix(decode): clarify short-row error message and add covering test The Row::field() method now reports short rows with a clearer expected value message, distinguishing row-too-short from column-not-found errors. Added short_row_is_an_error test to cover the previously untested branch where a row has fewer cells than the table has columns. --- nodedb-studio/src/services/decode.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/nodedb-studio/src/services/decode.rs b/nodedb-studio/src/services/decode.rs index cf004b3..eccc93d 100644 --- a/nodedb-studio/src/services/decode.rs +++ b/nodedb-studio/src/services/decode.rs @@ -37,7 +37,7 @@ impl<'a> Row<'a> { .get(idx) .map(String::as_str) .ok_or_else(|| StudioError::UnexpectedColumns { - expected: name.to_string(), + expected: format!("row to have at least {} cells", idx + 1), got: format!("row has {} cells", self.cells.len()), }) } @@ -138,4 +138,16 @@ mod tests { let out = decode_rows(&t, &["name"], |r| Ok(r.field("nope")?.to_string())); assert!(out.is_err()); } + + #[test] + fn short_row_is_an_error() { + let t = table(&["name", "owner", "id"], &[&["probe_docs", "admin"]]); + let out = decode_rows(&t, &["name", "owner", "id"], |r| { + Ok(r.field("id")?.to_string()) + }); + assert!( + matches!(out, Err(StudioError::UnexpectedColumns { .. })), + "expected UnexpectedColumns for short row, got {out:?}" + ); + } } From 7ebefe72056de3aa596c020780aa84dd25c65829 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 20:07:36 +0800 Subject: [PATCH 04/26] feat(seam): require an explicit username to connect The client defaults an unset trust username to admin and the server no longer supplies its own default, so a blank field would connect as admin without telling the user. Blank is now a typed MissingUsername error. --- .../src/components/command_palette.rs | 26 +++++++- .../components/popovers/connection_popover.rs | 16 ++++- .../src/services/connection_service.rs | 62 ++++++++++++++++--- nodedb-studio/src/services/error.rs | 7 ++- nodedb-studio/src/services/nodedb_service.rs | 14 ++++- .../src/state/connections_registry.rs | 9 +++ nodedb-studio/src/views/connection_manager.rs | 17 ++++- 7 files changed, 134 insertions(+), 17 deletions(-) diff --git a/nodedb-studio/src/components/command_palette.rs b/nodedb-studio/src/components/command_palette.rs index ae493ac..b9da699 100644 --- a/nodedb-studio/src/components/command_palette.rs +++ b/nodedb-studio/src/components/command_palette.rs @@ -8,6 +8,7 @@ use dioxus::prelude::*; use crate::routes::Route; use crate::services::backend::Backend; use crate::state::connection::ActiveConnection; +use crate::state::connections_registry::{Credentials, SavedConnection}; use crate::state::ui::ModalKind; #[component] @@ -16,6 +17,7 @@ pub fn CommandPalette() -> Element { let mut active = use_context::>>(); let mut modal = use_context::>>(); let service = use_context::>(); + let registry = use_context::>>(); let nav = use_navigator(); if !*open.read() { @@ -26,6 +28,22 @@ pub fn CommandPalette() -> Element { // each switch handler clones it. let switch_svc = service.clone(); + // TODO(later phase): the palette has no username field yet, so identity is + // taken from the saved profile rather than a user-entered value. Replace + // once the connect modal collects credentials explicitly. + let creds_for = |name: &str| -> Credentials { + Credentials { + username: registry + .peek() + .iter() + .find(|c| c.name == name) + .and_then(|c| c.profile.as_ref()) + .map(|p| p.user.clone()) + .unwrap_or_default(), + password: None, + } + }; + rsx! { div { class: "palette-overlay open", @@ -66,10 +84,12 @@ pub fn CommandPalette() -> Element { div { class: "palette-section", "Connections" } div { class: "palette-item", onclick: { let svc = switch_svc.clone(); + let creds = creds_for("staging-cluster"); move |_| { let svc = svc.clone(); + let creds = creds.clone(); spawn(async move { - if let Ok(s) = svc.connect("staging-cluster").await { active.set(Some(s)); } + if let Ok(s) = svc.connect("staging-cluster", &creds).await { active.set(Some(s)); } }); open.set(false); } @@ -78,10 +98,12 @@ pub fn CommandPalette() -> Element { } div { class: "palette-item", onclick: { let svc = switch_svc.clone(); + let creds = creds_for("prod-replica-eu"); move |_| { let svc = svc.clone(); + let creds = creds.clone(); spawn(async move { - if let Ok(s) = svc.connect("prod-replica-eu").await { active.set(Some(s)); } + if let Ok(s) = svc.connect("prod-replica-eu", &creds).await { active.set(Some(s)); } }); open.set(false); } diff --git a/nodedb-studio/src/components/popovers/connection_popover.rs b/nodedb-studio/src/components/popovers/connection_popover.rs index 3af40d1..aa2dae3 100644 --- a/nodedb-studio/src/components/popovers/connection_popover.rs +++ b/nodedb-studio/src/components/popovers/connection_popover.rs @@ -7,7 +7,7 @@ use dioxus::prelude::*; use crate::services::backend::Backend; use crate::state::connection::ActiveConnection; -use crate::state::connections_registry::{ConnStatus, SavedConnection}; +use crate::state::connections_registry::{ConnStatus, Credentials, SavedConnection}; use crate::state::ui::{ModalKind, Popover}; #[component] @@ -50,6 +50,17 @@ pub fn ConnectionPopover() -> Element { }; let svc = service.clone(); let item_class = if disabled { "cp-item disabled" } else { "cp-item" }; + // TODO(later phase): the switch popover has no username field yet, so + // identity is taken from the saved profile rather than a user-entered + // value. Replace once the connect modal collects credentials explicitly. + let creds = Credentials { + username: sc + .profile + .as_ref() + .map(|p| p.user.clone()) + .unwrap_or_default(), + password: None, + }; rsx! { div { class: "{item_class}", @@ -59,8 +70,9 @@ pub fn ConnectionPopover() -> Element { // set `active` (Copy) only after the await resolves. let svc = svc.clone(); let name = name.clone(); + let creds = creds.clone(); spawn(async move { - if let Ok(s) = svc.connect(&name).await { active.set(Some(s)); } + if let Ok(s) = svc.connect(&name, &creds).await { active.set(Some(s)); } }); popover.set(None); } diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index f1fab73..ae95675 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -17,7 +17,7 @@ use crate::models::notification::Notification; use crate::services::error::StudioError; use crate::services::streams_data::{StreamsData, cdc_rows_from_mock}; use crate::state::connection::ActiveConnection; -use crate::state::connections_registry::SavedConnection; +use crate::state::connections_registry::{Credentials, SavedConnection}; /// Async because the real client talks to NodeDB over the network. The Dioxus /// runtime is single-threaded, so `?Send` is correct (and `use_resource` has no @@ -32,9 +32,14 @@ pub trait ConnectionService { /// The full notification feed (capability gating happens at render time). async fn notifications(&self) -> Result, StudioError>; - /// Open a session by saved-connection name. `StudioError::NotConnected` if - /// the name is unknown or the connection is offline. - async fn connect(&self, name: &str) -> Result; + /// Open a session by saved-connection name using an explicit identity. + /// `StudioError::MissingUsername` if the username is blank; + /// `StudioError::NotConnected` if the name is unknown or offline. + async fn connect( + &self, + name: &str, + creds: &Credentials, + ) -> Result; /// Mark every notification read. A seam write: the real client persists this /// server-side; the mock persists it in-process so the unread badge does not @@ -116,7 +121,14 @@ impl ConnectionService for MockConnectionService { Ok(()) } - async fn connect(&self, name: &str) -> Result { + async fn connect( + &self, + name: &str, + creds: &Credentials, + ) -> Result { + if creds.username.trim().is_empty() { + return Err(StudioError::MissingUsername); + } mock::connections() .into_iter() .find(|c| c.name == name) @@ -180,20 +192,56 @@ mod tests { #[tokio::test] async fn mock_connect_known_name_returns_session() { let svc = MockConnectionService::ready(); + let creds = Credentials { + username: "alice".into(), + password: None, + }; // `staging-cluster` is a connectable Online mock connection (data/mock.rs). - let session = svc.connect("staging-cluster").await; + let session = svc.connect("staging-cluster", &creds).await; assert!(session.is_ok()); } #[tokio::test] async fn mock_connect_unknown_name_is_not_connected() { let svc = MockConnectionService::ready(); + let creds = Credentials { + username: "alice".into(), + password: None, + }; assert!(matches!( - svc.connect("does-not-exist").await, + svc.connect("does-not-exist", &creds).await, Err(StudioError::NotConnected) )); } + #[tokio::test] + async fn connect_rejects_blank_username() { + let svc = MockConnectionService::ready(); + let creds = Credentials { + username: " ".into(), + password: None, + }; + let out = svc.connect("local-dev", &creds).await; + assert!( + matches!(out, Err(StudioError::MissingUsername)), + "blank username must be rejected, never defaulted to admin" + ); + } + + #[tokio::test] + async fn connect_accepts_explicit_username() { + let svc = MockConnectionService::ready(); + let name = mock::connections() + .first() + .map(|c| c.name.clone()) + .expect("fixture must have at least one connection"); + let creds = Credentials { + username: "alice".into(), + password: None, + }; + assert!(svc.connect(&name, &creds).await.is_ok()); + } + #[tokio::test] async fn cdc_feed_ready_is_loaded() { let svc = MockConnectionService::ready(); diff --git a/nodedb-studio/src/services/error.rs b/nodedb-studio/src/services/error.rs index ec92c68..8002225 100644 --- a/nodedb-studio/src/services/error.rs +++ b/nodedb-studio/src/services/error.rs @@ -36,6 +36,9 @@ pub enum StudioError { #[error("unexpected result columns: expected [{expected}], got [{got}]")] #[allow(dead_code)] UnexpectedColumns { expected: String, got: String }, + /// Connect was attempted without an explicit username. + #[error("a username is required to connect")] + MissingUsername, } impl StudioError { @@ -43,7 +46,9 @@ impl StudioError { /// `NotConnected` is never retriable (it is studio-originated, not transient). pub fn is_retriable(&self) -> bool { match self { - StudioError::NotConnected | StudioError::UnexpectedColumns { .. } => false, + StudioError::NotConnected + | StudioError::UnexpectedColumns { .. } + | StudioError::MissingUsername => false, StudioError::Connection(e) | StudioError::Auth(e) | StudioError::NotFound(e) diff --git a/nodedb-studio/src/services/nodedb_service.rs b/nodedb-studio/src/services/nodedb_service.rs index abfff74..1ef19aa 100644 --- a/nodedb-studio/src/services/nodedb_service.rs +++ b/nodedb-studio/src/services/nodedb_service.rs @@ -16,7 +16,7 @@ use crate::services::connection_service::ConnectionService; use crate::services::error::StudioError; use crate::services::streams_data::StreamsData; use crate::state::connection::ActiveConnection; -use crate::state::connections_registry::SavedConnection; +use crate::state::connections_registry::{Credentials, SavedConnection}; // The Phase-2 seam impl: its trait conformance and object-safety are proven by // the tests below, but the mock is still the active injected service, so this @@ -36,7 +36,11 @@ impl ConnectionService for NodeDbConnectionService { Err(StudioError::NotConnected) } - async fn connect(&self, _name: &str) -> Result { + async fn connect( + &self, + _name: &str, + _creds: &Credentials, + ) -> Result { Err(StudioError::NotConnected) } @@ -69,8 +73,12 @@ mod tests { svc.notifications().await, Err(StudioError::NotConnected) )); + let creds = Credentials { + username: "alice".into(), + password: None, + }; assert!(matches!( - svc.connect("anything").await, + svc.connect("anything", &creds).await, Err(StudioError::NotConnected) )); assert!(matches!( diff --git a/nodedb-studio/src/state/connections_registry.rs b/nodedb-studio/src/state/connections_registry.rs index 54121ce..8cf8da7 100644 --- a/nodedb-studio/src/state/connections_registry.rs +++ b/nodedb-studio/src/state/connections_registry.rs @@ -69,3 +69,12 @@ impl SavedConnection { }) } } + +/// Identity supplied at connect time. Studio never defaults the username: the +/// client would silently fall back to `admin`, so a blank field must be a +/// validation error surfaced in the connect form. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct Credentials { + pub username: String, + pub password: Option, +} diff --git a/nodedb-studio/src/views/connection_manager.rs b/nodedb-studio/src/views/connection_manager.rs index 12dd77a..4e3c378 100644 --- a/nodedb-studio/src/views/connection_manager.rs +++ b/nodedb-studio/src/views/connection_manager.rs @@ -7,7 +7,7 @@ use dioxus::prelude::*; use crate::services::backend::Backend; use crate::state::connection::ActiveConnection; -use crate::state::connections_registry::{ConnStatus, SavedConnection}; +use crate::state::connections_registry::{ConnStatus, Credentials, SavedConnection}; use crate::state::ui::ModalKind; #[component] @@ -49,12 +49,25 @@ pub fn ConnectionManager() -> Element { conn: conn.clone(), on_connect: { let service = service.clone(); + // TODO(later phase): the connect form has no username field + // yet, so identity is taken from the saved profile rather + // than a user-entered value. Replace once the connect modal + // collects credentials explicitly. + let creds = Credentials { + username: conn + .profile + .as_ref() + .map(|p| p.user.clone()) + .unwrap_or_default(), + password: None, + }; move |name: String| { // Async at the seam: clone the Rc into the task and // set `active` (Copy) only after the await resolves. let service = service.clone(); + let creds = creds.clone(); spawn(async move { - if let Ok(session) = service.connect(&name).await { + if let Ok(session) = service.connect(&name, &creds).await { active.set(Some(session)); } // Err case (e.g. offline): surfaced in a later wiring phase. From 6a07d4310f74c3b8c66e7d14e1d4cc3ca97c204e Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 20:13:14 +0800 Subject: [PATCH 05/26] feat(seam): shared mock behaviour with a delayed variant MockBehavior moves out of connection_service and gains Delayed, so every domain method added later reaches all four async states through one apply helper instead of re-implementing the match. --- .../src/services/connection_service.rs | 30 +++----- nodedb-studio/src/services/mock_behavior.rs | 72 +++++++++++++++++++ nodedb-studio/src/services/mod.rs | 1 + 3 files changed, 83 insertions(+), 20 deletions(-) create mode 100644 nodedb-studio/src/services/mock_behavior.rs diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index ae95675..f8eb962 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -15,6 +15,7 @@ use crate::data::mock; use crate::models::cdc::CdcRow; use crate::models::notification::Notification; use crate::services::error::StudioError; +use crate::services::mock_behavior::{MockBehavior, apply}; use crate::services::streams_data::{StreamsData, cdc_rows_from_mock}; use crate::state::connection::ActiveConnection; use crate::state::connections_registry::{Credentials, SavedConnection}; @@ -47,19 +48,6 @@ pub trait ConnectionService { async fn mark_all_read(&self) -> Result<(), StudioError>; } -/// Drives which result the mock returns, so every screen's four async states are -/// reachable in demos and tests. -// Variants are public API for demos and tests; not all are used in the app binary. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] -pub enum MockBehavior { - #[default] - Ready, - #[allow(dead_code)] - Empty, - #[allow(dead_code)] - Erroring, -} - /// Hardcoded implementation used by the skeleton. Data is identical to before; /// the methods are merely `async` now (and resolve instantly). /// @@ -98,6 +86,14 @@ impl MockConnectionService { all_read: Rc::default(), } } + /// Every read resolves after `d`, so the Loading state is observable. + #[allow(dead_code)] + pub fn delayed(d: std::time::Duration) -> Self { + Self { + behavior: MockBehavior::Delayed(d), + all_read: Rc::default(), + } + } } #[async_trait(?Send)] @@ -140,13 +136,7 @@ impl ConnectionService for MockConnectionService { #[async_trait(?Send)] impl StreamsData for MockConnectionService { async fn cdc_feed(&self) -> Result, StudioError> { - match self.behavior { - MockBehavior::Ready => Ok(cdc_rows_from_mock()), - MockBehavior::Empty => Ok(Vec::new()), - MockBehavior::Erroring => Err(StudioError::from( - nodedb_client::NodeDbError::node_unreachable("mock"), - )), - } + apply(self.behavior, cdc_rows_from_mock).await } } diff --git a/nodedb-studio/src/services/mock_behavior.rs b/nodedb-studio/src/services/mock_behavior.rs new file mode 100644 index 0000000..fe039ef --- /dev/null +++ b/nodedb-studio/src/services/mock_behavior.rs @@ -0,0 +1,72 @@ +//! Drives which result every mock seam method returns, so all four async +//! states are reachable on every screen rather than only on CDC. + +use std::time::Duration; + +use crate::services::error::StudioError; + +/// Which result the mock produces. `Delayed` exists so tests can observe the +/// Loading state and catch guards held across an await. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum MockBehavior { + #[default] + Ready, + Empty, + Erroring, + Delayed(Duration), +} + +/// Apply the behaviour to a fixture thunk. Every mock seam method is a +/// one-liner over this, which is what keeps the four states uniform. +pub async fn apply( + behavior: MockBehavior, + ready: impl FnOnce() -> Vec, +) -> Result, StudioError> { + match behavior { + MockBehavior::Ready => Ok(ready()), + MockBehavior::Empty => Ok(Vec::new()), + MockBehavior::Erroring => Err(StudioError::from( + nodedb_client::NodeDbError::node_unreachable("mock"), + )), + MockBehavior::Delayed(d) => { + tokio::time::sleep(d).await; + Ok(ready()) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn ready_returns_the_fixture() { + let out = apply(MockBehavior::Ready, || vec![1u8, 2]).await; + assert_eq!(out.expect("ready yields data"), vec![1u8, 2]); + } + + #[tokio::test] + async fn empty_returns_no_rows() { + let out = apply(MockBehavior::Empty, || vec![1u8, 2]).await; + assert!(out.expect("empty yields Ok").is_empty()); + } + + #[tokio::test] + async fn erroring_returns_a_retriable_error() { + let out = apply(MockBehavior::Erroring, || vec![1u8]).await; + let err = out.expect_err("erroring yields Err"); + assert!( + err.is_retriable(), + "demo error must exercise the retry path" + ); + } + + #[tokio::test] + async fn delayed_still_returns_the_fixture() { + let out = apply(MockBehavior::Delayed(Duration::from_millis(5)), || { + vec![9u8] + }) + .await; + assert_eq!(out.expect("delayed yields data"), vec![9u8]); + } +} diff --git a/nodedb-studio/src/services/mod.rs b/nodedb-studio/src/services/mod.rs index 9d624d1..87c59ae 100644 --- a/nodedb-studio/src/services/mod.rs +++ b/nodedb-studio/src/services/mod.rs @@ -6,5 +6,6 @@ pub mod backend; pub mod connection_service; pub mod decode; pub mod error; +pub mod mock_behavior; pub mod nodedb_service; pub mod streams_data; From 64b95dab9d03a07ccc3e272b02d5b34a38fe7aa2 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 20:21:36 +0800 Subject: [PATCH 06/26] feat(seam): ExplorerData trait, models and fixtures Sidebar groups, list rows and record detail now have typed seam methods. Grouping happens behind the seam because storage mode is not available in a single server call, so the sidebar never sees that resolution. --- nodedb-studio/src/data/mock/explorer.rs | 85 ++++++++++++++++++ nodedb-studio/src/data/mock/mod.rs | 2 + nodedb-studio/src/models/explorer.rs | 38 ++++++++ nodedb-studio/src/models/mod.rs | 1 + nodedb-studio/src/services/backend.rs | 5 +- .../src/services/connection_service.rs | 27 ++++++ nodedb-studio/src/services/explorer_data.rs | 90 +++++++++++++++++++ nodedb-studio/src/services/mod.rs | 1 + nodedb-studio/src/services/nodedb_service.rs | 19 ++++ 9 files changed, 266 insertions(+), 2 deletions(-) create mode 100644 nodedb-studio/src/data/mock/explorer.rs create mode 100644 nodedb-studio/src/models/explorer.rs create mode 100644 nodedb-studio/src/services/explorer_data.rs diff --git a/nodedb-studio/src/data/mock/explorer.rs b/nodedb-studio/src/data/mock/explorer.rs new file mode 100644 index 0000000..30216d6 --- /dev/null +++ b/nodedb-studio/src/data/mock/explorer.rs @@ -0,0 +1,85 @@ +//! Explorer fixtures: grouped collections, list rows, and detail bodies. +//! +//! `record_detail` is exercised by `ExplorerData`'s mock/stub impls but has no +//! caller yet outside `#[cfg(test)]` (the Explorer views aren't wired to the +//! seam until a later task), so it needs `#[allow(dead_code)]` in the interim. + +use crate::models::collection::{Collection, StorageMode}; +use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; + +#[allow(dead_code)] +fn collection(name: &str, mode: StorageMode, count: &str) -> Collection { + Collection { + name: name.to_string(), + mode, + count: count.to_string(), + } +} + +/// Grouped in the canonical `StorageMode` display order. +#[allow(dead_code)] +pub fn collection_groups() -> Vec { + vec![ + CollectionGroup { + mode: StorageMode::Document, + collections: vec![ + collection("users", StorageMode::Document, "12,481"), + collection("orders", StorageMode::Document, "98,204"), + ], + }, + CollectionGroup { + mode: StorageMode::Strict, + collections: vec![collection("accounts", StorageMode::Strict, "3,921")], + }, + CollectionGroup { + mode: StorageMode::Vector, + collections: vec![collection("embeddings", StorageMode::Vector, "2.4M")], + }, + CollectionGroup { + mode: StorageMode::Graph, + collections: vec![collection("social", StorageMode::Graph, "44,010")], + }, + CollectionGroup { + mode: StorageMode::Timeseries, + collections: vec![collection("metrics", StorageMode::Timeseries, "8.1M")], + }, + CollectionGroup { + mode: StorageMode::Kv, + collections: vec![collection("sessions", StorageMode::Kv, "51,003")], + }, + CollectionGroup { + mode: StorageMode::Spatial, + collections: vec![collection("places", StorageMode::Spatial, "1,204")], + }, + CollectionGroup { + mode: StorageMode::Fts, + collections: vec![collection("articles", StorageMode::Fts, "22,847")], + }, + ] +} + +/// List rows for a collection. Deterministic and keyed by `id`. +#[allow(dead_code)] +pub fn records(collection: &str) -> Vec { + (0..6) + .map(|i| RecordRow { + id: format!("{collection}-{i}"), + cells: vec![ + format!("{collection}-{i}"), + format!("row {i}"), + format!("2026-08-0{} 10:0{}:00", (i % 9) + 1, i), + ], + }) + .collect() +} + +/// Detail body for one record. +#[allow(dead_code)] +pub fn record_detail(collection: &str, id: &str) -> RecordDetail { + RecordDetail { + id: id.to_string(), + title: format!("{collection} / {id}"), + body_json: format!("{{\"id\":\"{id}\",\"collection\":\"{collection}\"}}"), + footer: "mock fixture".to_string(), + } +} diff --git a/nodedb-studio/src/data/mock/mod.rs b/nodedb-studio/src/data/mock/mod.rs index f3e8ed8..b97c656 100644 --- a/nodedb-studio/src/data/mock/mod.rs +++ b/nodedb-studio/src/data/mock/mod.rs @@ -9,8 +9,10 @@ mod cdc; mod connections; mod docs; +mod explorer; mod notify; pub use cdc::{ChangeOp, cdc_events}; pub use connections::{connections, explorer_collections, notifications}; +pub use explorer::{collection_groups, record_detail, records}; pub use notify::{notify_channels, notify_messages}; diff --git a/nodedb-studio/src/models/explorer.rs b/nodedb-studio/src/models/explorer.rs new file mode 100644 index 0000000..62b9128 --- /dev/null +++ b/nodedb-studio/src/models/explorer.rs @@ -0,0 +1,38 @@ +//! Explorer-tier models: the grouped sidebar, list rows, and the detail panel. +//! +//! Nothing outside `ExplorerData`'s own tests constructs these yet: wiring the +//! Explorer sidebar/list/detail views to the seam is a later task. The +//! `#[allow(dead_code)]`s below go away with that wiring, same as the +//! `decode::Table`/`Row` seam types already in this crate. + +use serde::{Deserialize, Serialize}; + +use crate::models::collection::{Collection, StorageMode}; + +/// One storage-mode group in the Explorer sidebar. `mode` is the stable key. +#[allow(dead_code)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CollectionGroup { + pub mode: StorageMode, + pub collections: Vec, +} + +/// One row in a viewer's list pane. `cells` are pre-formatted for display and +/// align with the viewer's own column headers. +#[allow(dead_code)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RecordRow { + pub id: String, + pub cells: Vec, +} + +/// The detail panel for one record. `body_json` is display JSON produced at the +/// seam, never a raw client value. +#[allow(dead_code)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RecordDetail { + pub id: String, + pub title: String, + pub body_json: String, + pub footer: String, +} diff --git a/nodedb-studio/src/models/mod.rs b/nodedb-studio/src/models/mod.rs index 3ace1d9..c45d2b1 100644 --- a/nodedb-studio/src/models/mod.rs +++ b/nodedb-studio/src/models/mod.rs @@ -3,4 +3,5 @@ pub mod cdc; pub mod collection; +pub mod explorer; pub mod notification; diff --git a/nodedb-studio/src/services/backend.rs b/nodedb-studio/src/services/backend.rs index 0095a14..9235321 100644 --- a/nodedb-studio/src/services/backend.rs +++ b/nodedb-studio/src/services/backend.rs @@ -6,8 +6,9 @@ //! trait on the mock + stub — additive, never a reshape of existing methods. use crate::services::connection_service::ConnectionService; +use crate::services::explorer_data::ExplorerData; use crate::services::streams_data::StreamsData; -pub trait Backend: ConnectionService + StreamsData {} +pub trait Backend: ConnectionService + StreamsData + ExplorerData {} -impl Backend for T {} +impl Backend for T {} diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index f8eb962..6c9219d 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -13,8 +13,10 @@ use async_trait::async_trait; use crate::data::mock; use crate::models::cdc::CdcRow; +use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; use crate::models::notification::Notification; use crate::services::error::StudioError; +use crate::services::explorer_data::ExplorerData; use crate::services::mock_behavior::{MockBehavior, apply}; use crate::services::streams_data::{StreamsData, cdc_rows_from_mock}; use crate::state::connection::ActiveConnection; @@ -140,6 +142,31 @@ impl StreamsData for MockConnectionService { } } +#[async_trait(?Send)] +impl ExplorerData for MockConnectionService { + async fn collection_groups(&self) -> Result, StudioError> { + apply(self.behavior, mock::collection_groups).await + } + + async fn records(&self, collection: &str) -> Result, StudioError> { + let c = collection.to_string(); + apply(self.behavior, move || mock::records(&c)).await + } + + async fn record_detail(&self, collection: &str, id: &str) -> Result { + match self.behavior { + MockBehavior::Erroring => Err(StudioError::from( + nodedb_client::NodeDbError::node_unreachable("mock"), + )), + MockBehavior::Ready | MockBehavior::Empty => Ok(mock::record_detail(collection, id)), + MockBehavior::Delayed(d) => { + tokio::time::sleep(d).await; + Ok(mock::record_detail(collection, id)) + } + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/nodedb-studio/src/services/explorer_data.rs b/nodedb-studio/src/services/explorer_data.rs new file mode 100644 index 0000000..8b1d519 --- /dev/null +++ b/nodedb-studio/src/services/explorer_data.rs @@ -0,0 +1,90 @@ +//! Explorer-tier reads at the backend seam. +//! +//! `collection_groups` returns already-grouped collections because engine type +//! is not available in a single server call: the real implementation reads the +//! collection list, then resolves each collection's storage mode separately. +//! Keeping the grouping behind the seam means the sidebar never sees that. + +use async_trait::async_trait; + +use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; +use crate::services::error::StudioError; + +// Object-safety and mock/stub conformance are proven by the tests below and by +// `nodedb_service::tests::stub_is_object_safe_behind_backend`, but nothing +// outside `#[cfg(test)]` calls these methods yet: wiring the Explorer sidebar, +// list and detail panel to the seam is a later task. The `#[allow(dead_code)]`s +// go away with that wiring. +#[async_trait(?Send)] +pub trait ExplorerData { + /// Sidebar contents: collections grouped by storage mode, in display order. + #[allow(dead_code)] + async fn collection_groups(&self) -> Result, StudioError>; + + /// List-pane rows for one collection. + #[allow(dead_code)] + async fn records(&self, collection: &str) -> Result, StudioError>; + + /// Detail-panel contents for one record. + #[allow(dead_code)] + async fn record_detail(&self, collection: &str, id: &str) -> Result; +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::services::async_state::AsyncState; + use crate::services::connection_service::MockConnectionService; + + #[tokio::test] + async fn groups_are_ordered_and_non_empty() { + let svc = MockConnectionService::ready(); + let groups = svc.collection_groups().await.expect("ready yields groups"); + assert!(!groups.is_empty()); + for g in &groups { + assert!( + !g.collections.is_empty(), + "an empty group would render a header with no rows" + ); + } + } + + #[tokio::test] + async fn every_collection_has_a_stable_unique_key() { + let svc = MockConnectionService::ready(); + let groups = svc.collection_groups().await.expect("ready yields groups"); + let mut names: Vec<&str> = groups + .iter() + .flat_map(|g| g.collections.iter().map(|c| c.name.as_str())) + .collect(); + let total = names.len(); + names.sort_unstable(); + names.dedup(); + assert_eq!(total, names.len(), "collection names must be unique keys"); + } + + #[tokio::test] + async fn records_have_unique_ids() { + let svc = MockConnectionService::ready(); + let rows = svc.records("users").await.expect("ready yields rows"); + let mut ids: Vec<&str> = rows.iter().map(|r| r.id.as_str()).collect(); + let total = ids.len(); + ids.sort_unstable(); + ids.dedup(); + assert_eq!(total, ids.len()); + } + + #[tokio::test] + async fn empty_behaviour_reaches_the_empty_state() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.collection_groups().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn erroring_behaviour_reaches_the_error_state() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.collection_groups().await)); + assert!(s.error_message().is_some()); + } +} diff --git a/nodedb-studio/src/services/mod.rs b/nodedb-studio/src/services/mod.rs index 87c59ae..8723dc6 100644 --- a/nodedb-studio/src/services/mod.rs +++ b/nodedb-studio/src/services/mod.rs @@ -6,6 +6,7 @@ pub mod backend; pub mod connection_service; pub mod decode; pub mod error; +pub mod explorer_data; pub mod mock_behavior; pub mod nodedb_service; pub mod streams_data; diff --git a/nodedb-studio/src/services/nodedb_service.rs b/nodedb-studio/src/services/nodedb_service.rs index 1ef19aa..c03e9ee 100644 --- a/nodedb-studio/src/services/nodedb_service.rs +++ b/nodedb-studio/src/services/nodedb_service.rs @@ -11,9 +11,11 @@ use async_trait::async_trait; use crate::models::cdc::CdcRow; +use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; use crate::models::notification::Notification; use crate::services::connection_service::ConnectionService; use crate::services::error::StudioError; +use crate::services::explorer_data::ExplorerData; use crate::services::streams_data::StreamsData; use crate::state::connection::ActiveConnection; use crate::state::connections_registry::{Credentials, SavedConnection}; @@ -56,6 +58,23 @@ impl StreamsData for NodeDbConnectionService { } } +#[async_trait(?Send)] +impl ExplorerData for NodeDbConnectionService { + async fn collection_groups(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + async fn records(&self, _collection: &str) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + async fn record_detail( + &self, + _collection: &str, + _id: &str, + ) -> Result { + Err(StudioError::NotConnected) + } +} + #[cfg(test)] mod tests { use std::rc::Rc; From dde4827c01677f737ee8d76601e3a96c88c7e940 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 20:29:30 +0800 Subject: [PATCH 07/26] test(seam): cover record_detail and assert canonical group order record_detail had no test at all: add ready/erroring/empty cases and pin down that Empty folds into the same success path as Ready for a single-value read. groups_are_ordered_and_non_empty only checked non-emptiness; now it asserts the group sequence matches StorageMode's canonical order. Also standardize every SEAM-UNWIRED allow(dead_code) in these files onto one greppable tag instead of three different explanatory comments. --- nodedb-studio/src/data/mock/explorer.rs | 12 ++--- nodedb-studio/src/models/explorer.rs | 11 ++-- nodedb-studio/src/services/explorer_data.rs | 59 ++++++++++++++++++--- 3 files changed, 58 insertions(+), 24 deletions(-) diff --git a/nodedb-studio/src/data/mock/explorer.rs b/nodedb-studio/src/data/mock/explorer.rs index 30216d6..724aae8 100644 --- a/nodedb-studio/src/data/mock/explorer.rs +++ b/nodedb-studio/src/data/mock/explorer.rs @@ -1,13 +1,9 @@ //! Explorer fixtures: grouped collections, list rows, and detail bodies. -//! -//! `record_detail` is exercised by `ExplorerData`'s mock/stub impls but has no -//! caller yet outside `#[cfg(test)]` (the Explorer views aren't wired to the -//! seam until a later task), so it needs `#[allow(dead_code)]` in the interim. use crate::models::collection::{Collection, StorageMode}; use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; -#[allow(dead_code)] +#[allow(dead_code)] // SEAM-UNWIRED(task-10) fn collection(name: &str, mode: StorageMode, count: &str) -> Collection { Collection { name: name.to_string(), @@ -17,7 +13,7 @@ fn collection(name: &str, mode: StorageMode, count: &str) -> Collection { } /// Grouped in the canonical `StorageMode` display order. -#[allow(dead_code)] +#[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn collection_groups() -> Vec { vec![ CollectionGroup { @@ -59,7 +55,7 @@ pub fn collection_groups() -> Vec { } /// List rows for a collection. Deterministic and keyed by `id`. -#[allow(dead_code)] +#[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn records(collection: &str) -> Vec { (0..6) .map(|i| RecordRow { @@ -74,7 +70,7 @@ pub fn records(collection: &str) -> Vec { } /// Detail body for one record. -#[allow(dead_code)] +#[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn record_detail(collection: &str, id: &str) -> RecordDetail { RecordDetail { id: id.to_string(), diff --git a/nodedb-studio/src/models/explorer.rs b/nodedb-studio/src/models/explorer.rs index 62b9128..935907c 100644 --- a/nodedb-studio/src/models/explorer.rs +++ b/nodedb-studio/src/models/explorer.rs @@ -1,16 +1,11 @@ //! Explorer-tier models: the grouped sidebar, list rows, and the detail panel. -//! -//! Nothing outside `ExplorerData`'s own tests constructs these yet: wiring the -//! Explorer sidebar/list/detail views to the seam is a later task. The -//! `#[allow(dead_code)]`s below go away with that wiring, same as the -//! `decode::Table`/`Row` seam types already in this crate. use serde::{Deserialize, Serialize}; use crate::models::collection::{Collection, StorageMode}; /// One storage-mode group in the Explorer sidebar. `mode` is the stable key. -#[allow(dead_code)] +#[allow(dead_code)] // SEAM-UNWIRED(task-10) #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct CollectionGroup { pub mode: StorageMode, @@ -19,7 +14,7 @@ pub struct CollectionGroup { /// One row in a viewer's list pane. `cells` are pre-formatted for display and /// align with the viewer's own column headers. -#[allow(dead_code)] +#[allow(dead_code)] // SEAM-UNWIRED(task-10) #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct RecordRow { pub id: String, @@ -28,7 +23,7 @@ pub struct RecordRow { /// The detail panel for one record. `body_json` is display JSON produced at the /// seam, never a raw client value. -#[allow(dead_code)] +#[allow(dead_code)] // SEAM-UNWIRED(task-10) #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct RecordDetail { pub id: String, diff --git a/nodedb-studio/src/services/explorer_data.rs b/nodedb-studio/src/services/explorer_data.rs index 8b1d519..d631dad 100644 --- a/nodedb-studio/src/services/explorer_data.rs +++ b/nodedb-studio/src/services/explorer_data.rs @@ -10,29 +10,25 @@ use async_trait::async_trait; use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; use crate::services::error::StudioError; -// Object-safety and mock/stub conformance are proven by the tests below and by -// `nodedb_service::tests::stub_is_object_safe_behind_backend`, but nothing -// outside `#[cfg(test)]` calls these methods yet: wiring the Explorer sidebar, -// list and detail panel to the seam is a later task. The `#[allow(dead_code)]`s -// go away with that wiring. #[async_trait(?Send)] pub trait ExplorerData { /// Sidebar contents: collections grouped by storage mode, in display order. - #[allow(dead_code)] + #[allow(dead_code)] // SEAM-UNWIRED(task-10) async fn collection_groups(&self) -> Result, StudioError>; /// List-pane rows for one collection. - #[allow(dead_code)] + #[allow(dead_code)] // SEAM-UNWIRED(task-10) async fn records(&self, collection: &str) -> Result, StudioError>; /// Detail-panel contents for one record. - #[allow(dead_code)] + #[allow(dead_code)] // SEAM-UNWIRED(task-10) async fn record_detail(&self, collection: &str, id: &str) -> Result; } #[cfg(test)] mod tests { use super::*; + use crate::models::collection::StorageMode; use crate::services::async_state::AsyncState; use crate::services::connection_service::MockConnectionService; @@ -47,6 +43,23 @@ mod tests { "an empty group would render a header with no rows" ); } + // The sidebar renders groups in this exact sequence; it must track + // `StorageMode`'s own declared (canonical display) order. + let expected_order = [ + StorageMode::Document, + StorageMode::Strict, + StorageMode::Vector, + StorageMode::Graph, + StorageMode::Timeseries, + StorageMode::Kv, + StorageMode::Spatial, + StorageMode::Fts, + ]; + let modes: Vec = groups.iter().map(|g| g.mode).collect(); + assert_eq!( + modes, expected_order, + "groups must render in StorageMode's canonical display order" + ); } #[tokio::test] @@ -87,4 +100,34 @@ mod tests { let s = AsyncState::from_value(Some(svc.collection_groups().await)); assert!(s.error_message().is_some()); } + + #[tokio::test] + async fn record_detail_ready_returns_the_requested_id() { + let svc = MockConnectionService::ready(); + let detail = svc + .record_detail("users", "u1") + .await + .expect("ready yields a detail"); + assert_eq!(detail.id, "u1"); + } + + #[tokio::test] + async fn record_detail_erroring_is_err() { + let svc = MockConnectionService::erroring(); + assert!(svc.record_detail("users", "u1").await.is_err()); + } + + #[tokio::test] + async fn record_detail_empty_still_returns_the_requested_record() { + // `record_detail` reads a single record, not a list: "no rows" has no + // meaning here, so the mock folds `MockBehavior::Empty` into the same + // success path as `Ready` rather than inventing an absent/empty detail. + // Pinned here so a later refactor cannot silently change that meaning. + let svc = MockConnectionService::empty(); + let detail = svc + .record_detail("users", "u1") + .await + .expect("empty behaviour still returns a detail for a single-value read"); + assert_eq!(detail.id, "u1"); + } } From 53e9690c7339c0a490416ad8eaabac7acb7a5a4b Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 20:34:10 +0800 Subject: [PATCH 08/26] feat(seam): AdminData trait, models and fixtures Cluster, raft, shards, RBAC, RLS and audit reads now flow through the seam with rich mock data and a NotConnected stub, so no admin screen is a special case. --- nodedb-studio/src/data/mock/admin.rs | 109 ++++++++++++++++++ nodedb-studio/src/data/mock/mod.rs | 2 + nodedb-studio/src/models/admin.rs | 75 ++++++++++++ nodedb-studio/src/models/mod.rs | 1 + nodedb-studio/src/services/admin_data.rs | 82 +++++++++++++ nodedb-studio/src/services/backend.rs | 5 +- .../src/services/connection_service.rs | 29 +++++ nodedb-studio/src/services/mod.rs | 1 + nodedb-studio/src/services/nodedb_service.rs | 29 +++++ 9 files changed, 331 insertions(+), 2 deletions(-) create mode 100644 nodedb-studio/src/data/mock/admin.rs create mode 100644 nodedb-studio/src/models/admin.rs create mode 100644 nodedb-studio/src/services/admin_data.rs diff --git a/nodedb-studio/src/data/mock/admin.rs b/nodedb-studio/src/data/mock/admin.rs new file mode 100644 index 0000000..ef1d539 --- /dev/null +++ b/nodedb-studio/src/data/mock/admin.rs @@ -0,0 +1,109 @@ +//! Admin fixtures. Shapes mirror the server's introspection output so the +//! real implementation is a decoder swap, not a model change. + +use crate::models::admin::{AuditEntry, ClusterNode, RaftGroup, RlsPolicy, ShardRange, UserRow}; + +/// Cluster topology: one row per node. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn cluster_nodes() -> Vec { + vec![ + ClusterNode { + id: "1".into(), + address: "127.0.0.1:6433".into(), + state: "active".into(), + raft_groups: "6".into(), + }, + ClusterNode { + id: "2".into(), + address: "127.0.0.2:6433".into(), + state: "active".into(), + raft_groups: "5".into(), + }, + ClusterNode { + id: "3".into(), + address: "127.0.0.3:6433".into(), + state: "degraded".into(), + raft_groups: "4".into(), + }, + ] +} + +/// Raft groups for the cluster. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn raft_groups() -> Vec { + (0..4) + .map(|i| RaftGroup { + id: i.to_string(), + role: if i == 0 { "Leader" } else { "Follower" }.into(), + leader_id: "1".into(), + term: "1".into(), + commit_index: (100 + i).to_string(), + last_applied: (100 + i).to_string(), + members: "1,2,3".into(), + }) + .collect() +} + +/// Shard ranges and their leaseholders. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn shard_ranges() -> Vec { + (0..8) + .map(|i| ShardRange { + id: i.to_string(), + group_id: ((i % 3) + 1).to_string(), + leaseholder: ((i % 3) + 1).to_string(), + replicas: "1,2,3".into(), + qps: format!("{}.0", i * 12), + p99_ms: format!("{}.5", i + 1), + }) + .collect() +} + +/// RBAC: all users in the tenant. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn users() -> Vec { + vec![ + UserRow { + id: "admin".into(), + username: "admin".into(), + tenant_id: "1".into(), + roles: "superuser".into(), + is_superuser: true, + }, + UserRow { + id: "alice".into(), + username: "alice".into(), + tenant_id: "1".into(), + roles: "reader".into(), + is_superuser: false, + }, + ] +} + +/// Row-level-security policies. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn rls_policies() -> Vec { + vec![RlsPolicy { + id: "tenant_isolation".into(), + name: "tenant_isolation".into(), + collection: "orders".into(), + kind: "select".into(), + mode: "permissive".into(), + enabled: true, + }] +} + +/// Audit log entries. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn audit_entries() -> Vec { + (0..5) + .map(|i| AuditEntry { + id: format!("audit-{i}"), + when: format!("2026-08-08 10:0{i}:00"), + actor: "admin".into(), + action: "SELECT".into(), + target: "orders".into(), + result: "allowed".into(), + }) + .collect() +} diff --git a/nodedb-studio/src/data/mock/mod.rs b/nodedb-studio/src/data/mock/mod.rs index b97c656..efa3ed2 100644 --- a/nodedb-studio/src/data/mock/mod.rs +++ b/nodedb-studio/src/data/mock/mod.rs @@ -6,12 +6,14 @@ //! reproduced. NodeDB version numbers are undecided (CLAUDE.md §2), so the //! server stat is a neutral "dev" placeholder rather than an invented version. +mod admin; mod cdc; mod connections; mod docs; mod explorer; mod notify; +pub use admin::{audit_entries, cluster_nodes, raft_groups, rls_policies, shard_ranges, users}; pub use cdc::{ChangeOp, cdc_events}; pub use connections::{connections, explorer_collections, notifications}; pub use explorer::{collection_groups, record_detail, records}; diff --git a/nodedb-studio/src/models/admin.rs b/nodedb-studio/src/models/admin.rs new file mode 100644 index 0000000..ab05136 --- /dev/null +++ b/nodedb-studio/src/models/admin.rs @@ -0,0 +1,75 @@ +//! Admin-tier models. String fields mirror the wire, which returns every +//! scalar as a string; booleans are parsed at the seam so views never see +//! the server's "t"/"f" encoding. + +use serde::{Deserialize, Serialize}; + +/// One node in the cluster topology. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ClusterNode { + pub id: String, + pub address: String, + pub state: String, + pub raft_groups: String, +} + +/// One Raft consensus group. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RaftGroup { + pub id: String, + pub role: String, + pub leader_id: String, + pub term: String, + pub commit_index: String, + pub last_applied: String, + pub members: String, +} + +/// One shard range and its current leaseholder. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ShardRange { + pub id: String, + pub group_id: String, + pub leaseholder: String, + pub replicas: String, + pub qps: String, + pub p99_ms: String, +} + +/// One RBAC user row. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct UserRow { + pub id: String, + pub username: String, + pub tenant_id: String, + pub roles: String, + pub is_superuser: bool, +} + +/// One row-level-security policy. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RlsPolicy { + pub id: String, + pub name: String, + pub collection: String, + pub kind: String, + pub mode: String, + pub enabled: bool, +} + +/// One audit-log entry. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct AuditEntry { + pub id: String, + pub when: String, + pub actor: String, + pub action: String, + pub target: String, + pub result: String, +} diff --git a/nodedb-studio/src/models/mod.rs b/nodedb-studio/src/models/mod.rs index c45d2b1..cdf40a1 100644 --- a/nodedb-studio/src/models/mod.rs +++ b/nodedb-studio/src/models/mod.rs @@ -1,6 +1,7 @@ //! Typed domain models shared across views. These describe *data* (collections, //! databases, notifications); live UI state lives in `crate::state`. +pub mod admin; pub mod cdc; pub mod collection; pub mod explorer; diff --git a/nodedb-studio/src/services/admin_data.rs b/nodedb-studio/src/services/admin_data.rs new file mode 100644 index 0000000..0d7404f --- /dev/null +++ b/nodedb-studio/src/services/admin_data.rs @@ -0,0 +1,82 @@ +//! Admin-tier reads at the backend seam: cluster, raft, shards, RBAC, RLS, +//! audit. Every one has a real server-side source, so these signatures are +//! designed for a real implementation rather than as permanent mock stubs. + +use async_trait::async_trait; + +use crate::models::admin::{AuditEntry, ClusterNode, RaftGroup, RlsPolicy, ShardRange, UserRow}; +use crate::services::error::StudioError; + +#[async_trait(?Send)] +pub trait AdminData { + /// Cluster topology: one row per node. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn cluster_nodes(&self) -> Result, StudioError>; + + /// Raft groups for the cluster. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn raft_groups(&self) -> Result, StudioError>; + + /// Shard ranges and their leaseholders. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn shard_ranges(&self) -> Result, StudioError>; + + /// RBAC: all users in the tenant. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn users(&self) -> Result, StudioError>; + + /// Row-level-security policies. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn rls_policies(&self) -> Result, StudioError>; + + /// Audit log entries. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn audit_entries(&self) -> Result, StudioError>; +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::services::async_state::AsyncState; + use crate::services::connection_service::MockConnectionService; + + #[tokio::test] + async fn every_admin_read_has_unique_ids() { + let svc = MockConnectionService::ready(); + let nodes = svc.cluster_nodes().await.expect("nodes"); + let groups = svc.raft_groups().await.expect("raft"); + let shards = svc.shard_ranges().await.expect("shards"); + let users = svc.users().await.expect("users"); + let policies = svc.rls_policies().await.expect("rls"); + let audit = svc.audit_entries().await.expect("audit"); + + assert_unique(nodes.iter().map(|x| x.id.as_str()), "cluster_nodes"); + assert_unique(groups.iter().map(|x| x.id.as_str()), "raft_groups"); + assert_unique(shards.iter().map(|x| x.id.as_str()), "shard_ranges"); + assert_unique(users.iter().map(|x| x.id.as_str()), "users"); + assert_unique(policies.iter().map(|x| x.id.as_str()), "rls_policies"); + assert_unique(audit.iter().map(|x| x.id.as_str()), "audit_entries"); + } + + fn assert_unique<'a>(it: impl Iterator, what: &str) { + let mut v: Vec<&str> = it.collect(); + let total = v.len(); + assert!(total > 0, "{what} fixture must not be empty"); + v.sort_unstable(); + v.dedup(); + assert_eq!(total, v.len(), "{what} ids must be unique"); + } + + #[tokio::test] + async fn unbacked_screens_flow_through_the_same_seam() { + // Cluster/raft/shards have rich mock data and a NotConnected stub, + // exactly like every other domain. Neither is special-cased. + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.cluster_nodes().await)); + assert!(s.is_empty()); + + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.raft_groups().await)); + assert!(s.error_message().is_some()); + } +} diff --git a/nodedb-studio/src/services/backend.rs b/nodedb-studio/src/services/backend.rs index 9235321..f7d000a 100644 --- a/nodedb-studio/src/services/backend.rs +++ b/nodedb-studio/src/services/backend.rs @@ -5,10 +5,11 @@ //! Adding a new domain trait later means extending this bound and implementing the //! trait on the mock + stub — additive, never a reshape of existing methods. +use crate::services::admin_data::AdminData; use crate::services::connection_service::ConnectionService; use crate::services::explorer_data::ExplorerData; use crate::services::streams_data::StreamsData; -pub trait Backend: ConnectionService + StreamsData + ExplorerData {} +pub trait Backend: ConnectionService + StreamsData + ExplorerData + AdminData {} -impl Backend for T {} +impl Backend for T {} diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index 6c9219d..2e358e0 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -12,9 +12,11 @@ use std::rc::Rc; use async_trait::async_trait; use crate::data::mock; +use crate::models::admin::{AuditEntry, ClusterNode, RaftGroup, RlsPolicy, ShardRange, UserRow}; use crate::models::cdc::CdcRow; use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; use crate::models::notification::Notification; +use crate::services::admin_data::AdminData; use crate::services::error::StudioError; use crate::services::explorer_data::ExplorerData; use crate::services::mock_behavior::{MockBehavior, apply}; @@ -167,6 +169,33 @@ impl ExplorerData for MockConnectionService { } } +#[async_trait(?Send)] +impl AdminData for MockConnectionService { + async fn cluster_nodes(&self) -> Result, StudioError> { + apply(self.behavior, mock::cluster_nodes).await + } + + async fn raft_groups(&self) -> Result, StudioError> { + apply(self.behavior, mock::raft_groups).await + } + + async fn shard_ranges(&self) -> Result, StudioError> { + apply(self.behavior, mock::shard_ranges).await + } + + async fn users(&self) -> Result, StudioError> { + apply(self.behavior, mock::users).await + } + + async fn rls_policies(&self) -> Result, StudioError> { + apply(self.behavior, mock::rls_policies).await + } + + async fn audit_entries(&self) -> Result, StudioError> { + apply(self.behavior, mock::audit_entries).await + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/nodedb-studio/src/services/mod.rs b/nodedb-studio/src/services/mod.rs index 8723dc6..d0816e4 100644 --- a/nodedb-studio/src/services/mod.rs +++ b/nodedb-studio/src/services/mod.rs @@ -1,6 +1,7 @@ //! Service traits at the backend seam. The mock impl is the only one today; //! a NodeDB-client-backed impl plugs in here later. +pub mod admin_data; pub mod async_state; pub mod backend; pub mod connection_service; diff --git a/nodedb-studio/src/services/nodedb_service.rs b/nodedb-studio/src/services/nodedb_service.rs index c03e9ee..e9060a2 100644 --- a/nodedb-studio/src/services/nodedb_service.rs +++ b/nodedb-studio/src/services/nodedb_service.rs @@ -10,9 +10,11 @@ use async_trait::async_trait; +use crate::models::admin::{AuditEntry, ClusterNode, RaftGroup, RlsPolicy, ShardRange, UserRow}; use crate::models::cdc::CdcRow; use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; use crate::models::notification::Notification; +use crate::services::admin_data::AdminData; use crate::services::connection_service::ConnectionService; use crate::services::error::StudioError; use crate::services::explorer_data::ExplorerData; @@ -75,6 +77,33 @@ impl ExplorerData for NodeDbConnectionService { } } +#[async_trait(?Send)] +impl AdminData for NodeDbConnectionService { + async fn cluster_nodes(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn raft_groups(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn shard_ranges(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn users(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn rls_policies(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn audit_entries(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } +} + #[cfg(test)] mod tests { use std::rc::Rc; From 17a1afe11f65e9d55ae36f4d9c521e9546db9303 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 20:40:32 +0800 Subject: [PATCH 09/26] test(seam): cover admin behaviour paths and fixture semantics Add empty/erroring coverage for shard_ranges, users, rls_policies and audit_entries so a method mis-wired as Ok(mock::x()) instead of apply(self.behavior, mock::x) would fail. Assert the admin/alice is_superuser flags and the RLS policies' enabled flags, and give rls_policies a second, distinct entry so its uniqueness check is real instead of vacuous. Reword the admin models doc comment: no bool parsing happens yet, it describes the seam's future decode boundary. --- nodedb-studio/src/data/mock/admin.rs | 26 +++-- nodedb-studio/src/models/admin.rs | 6 +- nodedb-studio/src/services/admin_data.rs | 120 ++++++++++++++++++++++- 3 files changed, 139 insertions(+), 13 deletions(-) diff --git a/nodedb-studio/src/data/mock/admin.rs b/nodedb-studio/src/data/mock/admin.rs index ef1d539..1b1f8c1 100644 --- a/nodedb-studio/src/data/mock/admin.rs +++ b/nodedb-studio/src/data/mock/admin.rs @@ -83,14 +83,24 @@ pub fn users() -> Vec { /// Row-level-security policies. #[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn rls_policies() -> Vec { - vec![RlsPolicy { - id: "tenant_isolation".into(), - name: "tenant_isolation".into(), - collection: "orders".into(), - kind: "select".into(), - mode: "permissive".into(), - enabled: true, - }] + vec![ + RlsPolicy { + id: "tenant_isolation".into(), + name: "tenant_isolation".into(), + collection: "orders".into(), + kind: "select".into(), + mode: "permissive".into(), + enabled: true, + }, + RlsPolicy { + id: "pii_masking".into(), + name: "pii_masking".into(), + collection: "users".into(), + kind: "select".into(), + mode: "restrictive".into(), + enabled: false, + }, + ] } /// Audit log entries. diff --git a/nodedb-studio/src/models/admin.rs b/nodedb-studio/src/models/admin.rs index ab05136..c020a35 100644 --- a/nodedb-studio/src/models/admin.rs +++ b/nodedb-studio/src/models/admin.rs @@ -1,6 +1,8 @@ //! Admin-tier models. String fields mirror the wire, which returns every -//! scalar as a string; booleans are parsed at the seam so views never see -//! the server's "t"/"f" encoding. +//! scalar as a string. `is_superuser` and `enabled` are already plain `bool` +//! here: the real seam implementation will decode the server's "t"/"f" +//! encoding into these fields at that boundary, so views never have to see +//! the wire representation. use serde::{Deserialize, Serialize}; diff --git a/nodedb-studio/src/services/admin_data.rs b/nodedb-studio/src/services/admin_data.rs index 0d7404f..1c19a57 100644 --- a/nodedb-studio/src/services/admin_data.rs +++ b/nodedb-studio/src/services/admin_data.rs @@ -67,16 +67,130 @@ mod tests { assert_eq!(total, v.len(), "{what} ids must be unique"); } + // Each method gets its own empty/erroring pair rather than one combined + // check per behaviour: `apply(self.behavior, mock::x)` and a mis-wired + // `Ok(mock::x())` both satisfy a single shared assertion, so every method + // needs its own proof that it actually reads `self.behavior`. + #[tokio::test] - async fn unbacked_screens_flow_through_the_same_seam() { - // Cluster/raft/shards have rich mock data and a NotConnected stub, - // exactly like every other domain. Neither is special-cased. + async fn cluster_nodes_empty_is_empty() { let svc = MockConnectionService::empty(); let s = AsyncState::from_value(Some(svc.cluster_nodes().await)); assert!(s.is_empty()); + } + #[tokio::test] + async fn cluster_nodes_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.cluster_nodes().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn raft_groups_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.raft_groups().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn raft_groups_erroring_is_err() { let svc = MockConnectionService::erroring(); let s = AsyncState::from_value(Some(svc.raft_groups().await)); assert!(s.error_message().is_some()); } + + #[tokio::test] + async fn shard_ranges_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.shard_ranges().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn shard_ranges_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.shard_ranges().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn users_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.users().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn users_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.users().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn rls_policies_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.rls_policies().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn rls_policies_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.rls_policies().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn audit_entries_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.audit_entries().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn audit_entries_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.audit_entries().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn users_fixture_marks_admin_superuser_and_alice_not() { + let svc = MockConnectionService::ready(); + let users = svc.users().await.expect("users"); + let admin = users + .iter() + .find(|u| u.id == "admin") + .expect("fixture has an `admin` user"); + let alice = users + .iter() + .find(|u| u.id == "alice") + .expect("fixture has an `alice` user"); + assert!(admin.is_superuser, "admin fixture must be a superuser"); + assert!(!alice.is_superuser, "alice fixture must not be a superuser"); + } + + #[tokio::test] + async fn rls_policies_fixture_enabled_flags_match_intent() { + let svc = MockConnectionService::ready(); + let policies = svc.rls_policies().await.expect("rls"); + let tenant_isolation = policies + .iter() + .find(|p| p.id == "tenant_isolation") + .expect("fixture has a `tenant_isolation` policy"); + let pii_masking = policies + .iter() + .find(|p| p.id == "pii_masking") + .expect("fixture has a `pii_masking` policy"); + assert!( + tenant_isolation.enabled, + "tenant_isolation must be enabled in the fixture" + ); + assert!( + !pii_masking.enabled, + "pii_masking must be disabled in the fixture" + ); + } } From 6dea12f3d1633bee94b02199980c1d5be4ca3c4e Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 20:51:12 +0800 Subject: [PATCH 10/26] feat(seam): CDC consumer-group lifecycle plus MV, topics, cron and notify Stream reads are idempotent and only an explicit commit advances the cursor, so the seam now models open/read/commit/close with a Studio-owned consumer group. Sharing a group and committing would advance a production consumer past unprocessed events. --- nodedb-studio/src/data/mock/mod.rs | 4 + nodedb-studio/src/data/mock/streams.rs | 178 +++++++++++ nodedb-studio/src/models/mod.rs | 1 + nodedb-studio/src/models/streams.rs | 84 ++++++ .../src/services/connection_service.rs | 72 +++++ nodedb-studio/src/services/nodedb_service.rs | 85 ++++++ nodedb-studio/src/services/streams_data.rs | 280 +++++++++++++++++- 7 files changed, 702 insertions(+), 2 deletions(-) create mode 100644 nodedb-studio/src/data/mock/streams.rs create mode 100644 nodedb-studio/src/models/streams.rs diff --git a/nodedb-studio/src/data/mock/mod.rs b/nodedb-studio/src/data/mock/mod.rs index efa3ed2..a2233cb 100644 --- a/nodedb-studio/src/data/mock/mod.rs +++ b/nodedb-studio/src/data/mock/mod.rs @@ -12,9 +12,13 @@ mod connections; mod docs; mod explorer; mod notify; +mod streams; pub use admin::{audit_entries, cluster_nodes, raft_groups, rls_policies, shard_ranges, users}; pub use cdc::{ChangeOp, cdc_events}; pub use connections::{connections, explorer_collections, notifications}; pub use explorer::{collection_groups, record_detail, records}; pub use notify::{notify_channels, notify_messages}; +pub use streams::{ + materialized_views, notify_channel_rows, notify_message_rows, scheduled_jobs, topics, +}; diff --git a/nodedb-studio/src/data/mock/streams.rs b/nodedb-studio/src/data/mock/streams.rs new file mode 100644 index 0000000..b3bb1ac --- /dev/null +++ b/nodedb-studio/src/data/mock/streams.rs @@ -0,0 +1,178 @@ +//! Streams fixtures beyond CDC (`data::mock::cdc`): materialized views, +//! durable topics, scheduled jobs and LISTEN/NOTIFY. Shapes mirror the +//! server's introspection output so the real implementation is a decoder +//! swap, not a model change. +//! +//! `notify_channel_rows`/`notify_message_rows` are deliberately not named +//! `notify_channels`/`notify_messages`: those names are already taken at +//! `data::mock`'s root by the pre-seam fixtures `views::streams::notify` +//! reads directly (see that module's doc comment). The two fixture sets +//! describe different shapes and are wired independently; task 10 reconciles +//! the view onto the seam. + +use crate::models::streams::{MaterializedView, NotifyChannel, NotifyMessage, ScheduledJob, Topic}; + +/// Materialized views known to the cluster. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn materialized_views() -> Vec { + vec![ + MaterializedView { + id: "mv_top_users_24h".into(), + name: "mv_top_users_24h".into(), + source: "events".into(), + refresh_mode: "incremental".into(), + rows: "12,481".into(), + }, + MaterializedView { + id: "mv_daily_revenue".into(), + name: "mv_daily_revenue".into(), + source: "orders".into(), + refresh_mode: "scheduled".into(), + rows: "3,204".into(), + }, + ] +} + +/// Durable, replayable topics with their consumer lag. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn topics() -> Vec { + vec![ + Topic { + id: "order_placed".into(), + name: "order_placed".into(), + partitions: "8".into(), + messages: "2.4M".into(), + retention: "7d".into(), + consumers: "3 active".into(), + lag: "142ms".into(), + }, + Topic { + id: "user_signup".into(), + name: "user_signup".into(), + partitions: "4".into(), + messages: "88,209".into(), + retention: "30d".into(), + consumers: "2 active".into(), + lag: "22ms".into(), + }, + Topic { + id: "payment_failed".into(), + name: "payment_failed".into(), + partitions: "2".into(), + messages: "12,488".into(), + retention: "90d".into(), + consumers: "1 active · 1 stalled".into(), + lag: "4.2s".into(), + }, + ] +} + +/// Cron-style scheduled jobs. Deliberately mixes a failed job in with +/// successful ones so fixture-content tests can't be satisfied by an +/// accidentally-uniform status column. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn scheduled_jobs() -> Vec { + vec![ + ScheduledJob { + id: "nightly_rollup".into(), + name: "nightly_rollup".into(), + cron: "0 2 * * *".into(), + last_status: "success".into(), + next_run: "in 2h 14m".into(), + }, + ScheduledJob { + id: "session_cleanup".into(), + name: "session_cleanup".into(), + cron: "*/15 * * * *".into(), + last_status: "success".into(), + next_run: "in 11m".into(), + }, + ScheduledJob { + id: "vector_reindex".into(), + name: "vector_reindex".into(), + cron: "0 4 * * 0".into(), + last_status: "failed · oom".into(), + next_run: "in 4d 8h".into(), + }, + ] +} + +/// LISTEN/NOTIFY channels. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn notify_channel_rows() -> Vec { + vec![ + NotifyChannel { + id: "user_events".into(), + name: "user_events".into(), + subscribers: "12".into(), + }, + NotifyChannel { + id: "deploy_hooks".into(), + name: "deploy_hooks".into(), + subscribers: "3".into(), + }, + NotifyChannel { + id: "cache_invalidate".into(), + name: "cache_invalidate".into(), + subscribers: "5".into(), + }, + ] +} + +/// The pub/sub message tail across channels. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn notify_message_rows() -> Vec { + (0..4) + .map(|i| NotifyMessage { + id: format!("notify-{i}"), + channel: "user_events".into(), + at: format!("04:23:{:02}.041", 18 - i), + payload_json: format!("{{\"event\":\"login\",\"seq\":{i}}}"), + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn materialized_views_have_unique_ids() { + let rows = materialized_views(); + assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); + } + + #[test] + fn topics_have_unique_ids() { + let rows = topics(); + assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); + } + + #[test] + fn scheduled_jobs_have_unique_ids_and_a_mixed_status() { + let rows = scheduled_jobs(); + assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); + assert!(rows.iter().any(|j| j.last_status == "success")); + assert!(rows.iter().any(|j| j.last_status.starts_with("failed"))); + } + + #[test] + fn notify_channel_rows_have_unique_ids() { + let rows = notify_channel_rows(); + assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); + } + + #[test] + fn notify_message_rows_have_unique_ids() { + let rows = notify_message_rows(); + assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); + } + + fn assert_unique_ids(ids: &[&str]) { + assert!(!ids.is_empty(), "fixture must not be empty"); + let mut sorted = ids.to_vec(); + sorted.sort_unstable(); + sorted.dedup(); + assert_eq!(sorted.len(), ids.len(), "ids must be unique"); + } +} diff --git a/nodedb-studio/src/models/mod.rs b/nodedb-studio/src/models/mod.rs index cdf40a1..a3ddc42 100644 --- a/nodedb-studio/src/models/mod.rs +++ b/nodedb-studio/src/models/mod.rs @@ -6,3 +6,4 @@ pub mod cdc; pub mod collection; pub mod explorer; pub mod notification; +pub mod streams; diff --git a/nodedb-studio/src/models/streams.rs b/nodedb-studio/src/models/streams.rs new file mode 100644 index 0000000..3653e05 --- /dev/null +++ b/nodedb-studio/src/models/streams.rs @@ -0,0 +1,84 @@ +//! Streams-tier models beyond CDC (`models::cdc`): the consumer-group session +//! plus materialized views, durable topics, scheduled jobs and LISTEN/NOTIFY. +//! String fields mirror the wire, same convention as `models::admin`. + +use serde::{Deserialize, Serialize}; + +/// A Studio-owned CDC consumer session. Studio never shares a consumer group: +/// committing on someone else's group would advance a production consumer past +/// events it never processed. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct StreamSession { + pub stream: String, + pub group: String, +} + +/// One materialized view. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct MaterializedView { + pub id: String, + pub name: String, + pub source: String, + pub refresh_mode: String, + pub rows: String, +} + +/// One durable, replayable topic. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct Topic { + pub id: String, + pub name: String, + pub partitions: String, + pub messages: String, + pub retention: String, + pub consumers: String, + pub lag: String, +} + +/// One cron-style scheduled job. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ScheduledJob { + pub id: String, + pub name: String, + pub cron: String, + pub last_status: String, + pub next_run: String, +} + +/// One LISTEN/NOTIFY channel. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct NotifyChannel { + pub id: String, + pub name: String, + pub subscribers: String, +} + +/// One message on the LISTEN/NOTIFY tail. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct NotifyMessage { + pub id: String, + pub channel: String, + pub at: String, + pub payload_json: String, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn stream_session_carries_stream_and_studio_group() { + let s = StreamSession { + stream: "cdc".to_string(), + group: "studio_cdc".to_string(), + }; + assert_eq!(s.stream, "cdc"); + assert_eq!(s.group, "studio_cdc"); + } +} diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index 2e358e0..ee6aa58 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -16,6 +16,9 @@ use crate::models::admin::{AuditEntry, ClusterNode, RaftGroup, RlsPolicy, ShardR use crate::models::cdc::CdcRow; use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; use crate::models::notification::Notification; +use crate::models::streams::{ + MaterializedView, NotifyChannel, NotifyMessage, ScheduledJob, StreamSession, Topic, +}; use crate::services::admin_data::AdminData; use crate::services::error::StudioError; use crate::services::explorer_data::ExplorerData; @@ -63,6 +66,12 @@ pub trait ConnectionService { pub struct MockConnectionService { behavior: MockBehavior, all_read: Rc>, + /// Offset the next `cdc_batch` reads from. Only `commit_stream_offsets` + /// advances it, mirroring the server: reads are idempotent. + cdc_committed: Rc>, + /// How far the most recent `cdc_batch` read. Commit promotes this into + /// `cdc_committed`. + cdc_read_end: Rc>, } // Constructors are public API for demos and tests; not all are used in the app binary. @@ -72,6 +81,8 @@ impl MockConnectionService { Self { behavior: MockBehavior::Ready, all_read: Rc::default(), + cdc_committed: Rc::default(), + cdc_read_end: Rc::default(), } } /// Every read returns an empty collection. @@ -80,6 +91,8 @@ impl MockConnectionService { Self { behavior: MockBehavior::Empty, all_read: Rc::default(), + cdc_committed: Rc::default(), + cdc_read_end: Rc::default(), } } /// Every read fails with a retriable server error. @@ -88,6 +101,8 @@ impl MockConnectionService { Self { behavior: MockBehavior::Erroring, all_read: Rc::default(), + cdc_committed: Rc::default(), + cdc_read_end: Rc::default(), } } /// Every read resolves after `d`, so the Loading state is observable. @@ -96,6 +111,8 @@ impl MockConnectionService { Self { behavior: MockBehavior::Delayed(d), all_read: Rc::default(), + cdc_committed: Rc::default(), + cdc_read_end: Rc::default(), } } } @@ -142,6 +159,61 @@ impl StreamsData for MockConnectionService { async fn cdc_feed(&self) -> Result, StudioError> { apply(self.behavior, cdc_rows_from_mock).await } + + async fn open_stream_session(&self, stream: &str) -> Result { + Ok(StreamSession { + stream: stream.to_string(), + group: format!("studio_{stream}"), + }) + } + + async fn cdc_batch( + &self, + _session: &StreamSession, + limit: usize, + ) -> Result, StudioError> { + let start = self.cdc_committed.get(); + let batch: Vec = cdc_rows_from_mock() + .into_iter() + .skip(start) + .take(limit) + .collect(); + // Record how far this read reached, but do NOT advance the committed + // offset: re-reading without a commit must return the same rows. + self.cdc_read_end.set(start + batch.len()); + apply(self.behavior, move || batch).await + } + + async fn commit_stream_offsets(&self, _session: &StreamSession) -> Result<(), StudioError> { + self.cdc_committed.set(self.cdc_read_end.get()); + Ok(()) + } + + async fn close_stream_session(&self, _session: &StreamSession) -> Result<(), StudioError> { + self.cdc_committed.set(0); + self.cdc_read_end.set(0); + Ok(()) + } + + async fn materialized_views(&self) -> Result, StudioError> { + apply(self.behavior, mock::materialized_views).await + } + + async fn topics(&self) -> Result, StudioError> { + apply(self.behavior, mock::topics).await + } + + async fn scheduled_jobs(&self) -> Result, StudioError> { + apply(self.behavior, mock::scheduled_jobs).await + } + + async fn notify_channels(&self) -> Result, StudioError> { + apply(self.behavior, mock::notify_channel_rows).await + } + + async fn notify_messages(&self) -> Result, StudioError> { + apply(self.behavior, mock::notify_message_rows).await + } } #[async_trait(?Send)] diff --git a/nodedb-studio/src/services/nodedb_service.rs b/nodedb-studio/src/services/nodedb_service.rs index e9060a2..938b3d8 100644 --- a/nodedb-studio/src/services/nodedb_service.rs +++ b/nodedb-studio/src/services/nodedb_service.rs @@ -14,6 +14,9 @@ use crate::models::admin::{AuditEntry, ClusterNode, RaftGroup, RlsPolicy, ShardR use crate::models::cdc::CdcRow; use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; use crate::models::notification::Notification; +use crate::models::streams::{ + MaterializedView, NotifyChannel, NotifyMessage, ScheduledJob, StreamSession, Topic, +}; use crate::services::admin_data::AdminData; use crate::services::connection_service::ConnectionService; use crate::services::error::StudioError; @@ -58,6 +61,46 @@ impl StreamsData for NodeDbConnectionService { async fn cdc_feed(&self) -> Result, StudioError> { Err(StudioError::NotConnected) } + + async fn open_stream_session(&self, _stream: &str) -> Result { + Err(StudioError::NotConnected) + } + + async fn cdc_batch( + &self, + _session: &StreamSession, + _limit: usize, + ) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn commit_stream_offsets(&self, _session: &StreamSession) -> Result<(), StudioError> { + Err(StudioError::NotConnected) + } + + async fn close_stream_session(&self, _session: &StreamSession) -> Result<(), StudioError> { + Err(StudioError::NotConnected) + } + + async fn materialized_views(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn topics(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn scheduled_jobs(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn notify_channels(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn notify_messages(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } } #[async_trait(?Send)] @@ -135,6 +178,48 @@ mod tests { )); } + #[tokio::test] + async fn stub_streams_lifecycle_and_lists_are_not_connected() { + let svc = NodeDbConnectionService; + let session = StreamSession { + stream: "cdc".into(), + group: "studio_cdc".into(), + }; + assert!(matches!( + svc.open_stream_session("cdc").await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.cdc_batch(&session, 10).await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.commit_stream_offsets(&session).await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.close_stream_session(&session).await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.materialized_views().await, + Err(StudioError::NotConnected) + )); + assert!(matches!(svc.topics().await, Err(StudioError::NotConnected))); + assert!(matches!( + svc.scheduled_jobs().await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.notify_channels().await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.notify_messages().await, + Err(StudioError::NotConnected) + )); + } + #[test] fn stub_is_object_safe_behind_rc() { // Compile-time guarantee: the stub coerces to the seam trait object, diff --git a/nodedb-studio/src/services/streams_data.rs b/nodedb-studio/src/services/streams_data.rs index 49692a1..03d296d 100644 --- a/nodedb-studio/src/services/streams_data.rs +++ b/nodedb-studio/src/services/streams_data.rs @@ -1,15 +1,67 @@ -//! Streams-tier reads at the backend seam. One method per Streams screen's data. -//! Today: CDC. Notify/MV/Topics/Cron methods are added when those screens are built. +//! Streams-tier reads at the backend seam. One method per Streams screen's data, +//! plus the CDC consumer-group lifecycle (open/read/commit/close). +//! +//! CDC reads are idempotent by design: re-reading without committing returns +//! the same rows, and only an explicit commit advances the cursor. A poll loop +//! that never commits re-reads the same window forever; one that commits on a +//! *shared* consumer group advances a production consumer past events it never +//! processed. Studio therefore always opens its own group (`studio_`), +//! commits its own batches, and drops the group on disconnect. use async_trait::async_trait; use crate::models::cdc::{CdcOp, CdcRow}; +use crate::models::streams::{ + MaterializedView, NotifyChannel, NotifyMessage, ScheduledJob, StreamSession, Topic, +}; use crate::services::error::StudioError; #[async_trait(?Send)] pub trait StreamsData { /// The CDC change feed, newest first. async fn cdc_feed(&self) -> Result, StudioError>; + + /// Create a Studio-owned consumer group on `stream` and return the session. + /// Callers must pair this with `close_stream_session`. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn open_stream_session(&self, stream: &str) -> Result; + + /// Read up to `limit` events from the session's current cursor. Idempotent: + /// re-reading without committing returns the same events. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn cdc_batch( + &self, + session: &StreamSession, + limit: usize, + ) -> Result, StudioError>; + + /// Advance the session's cursor past everything read so far. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn commit_stream_offsets(&self, session: &StreamSession) -> Result<(), StudioError>; + + /// Drop the Studio-owned consumer group. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn close_stream_session(&self, session: &StreamSession) -> Result<(), StudioError>; + + /// Materialized views known to the cluster. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn materialized_views(&self) -> Result, StudioError>; + + /// Durable, replayable topics. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn topics(&self) -> Result, StudioError>; + + /// Cron-style scheduled jobs. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn scheduled_jobs(&self) -> Result, StudioError>; + + /// LISTEN/NOTIFY channels. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn notify_channels(&self) -> Result, StudioError>; + + /// The pub/sub message tail across channels. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn notify_messages(&self) -> Result, StudioError>; } /// Build display rows from the static mock change feed. Lives here (not in the @@ -44,6 +96,8 @@ pub(crate) fn cdc_rows_from_mock() -> Vec { #[cfg(test)] mod tests { use super::*; + use crate::services::async_state::AsyncState; + use crate::services::connection_service::MockConnectionService; #[test] fn mock_rows_have_unique_stable_ids() { @@ -56,4 +110,226 @@ mod tests { assert_eq!(ids.len(), count, "ids must be unique"); assert_eq!(rows[0].id, "cdc-0"); } + + // Each method gets its own empty/erroring pair rather than one combined + // check per behaviour, mirroring `admin_data.rs`: `apply(self.behavior, mock::x)` + // and a mis-wired `Ok(mock::x())` both satisfy a single shared assertion, so + // every method needs its own proof that it actually reads `self.behavior`. + + #[tokio::test] + async fn every_streams_list_read_has_unique_ids() { + let svc = MockConnectionService::ready(); + let mvs = svc.materialized_views().await.expect("mvs"); + let topics = svc.topics().await.expect("topics"); + let jobs = svc.scheduled_jobs().await.expect("jobs"); + let channels = svc.notify_channels().await.expect("channels"); + let messages = svc.notify_messages().await.expect("messages"); + + assert_unique(mvs.iter().map(|x| x.id.as_str()), "materialized_views"); + assert_unique(topics.iter().map(|x| x.id.as_str()), "topics"); + assert_unique(jobs.iter().map(|x| x.id.as_str()), "scheduled_jobs"); + assert_unique(channels.iter().map(|x| x.id.as_str()), "notify_channels"); + assert_unique(messages.iter().map(|x| x.id.as_str()), "notify_messages"); + } + + fn assert_unique<'a>(it: impl Iterator, what: &str) { + let mut v: Vec<&str> = it.collect(); + let total = v.len(); + assert!(total > 0, "{what} fixture must not be empty"); + v.sort_unstable(); + v.dedup(); + assert_eq!(total, v.len(), "{what} ids must be unique"); + } + + #[tokio::test] + async fn materialized_views_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.materialized_views().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn materialized_views_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.materialized_views().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn topics_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.topics().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn topics_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.topics().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn scheduled_jobs_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.scheduled_jobs().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn scheduled_jobs_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.scheduled_jobs().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn notify_channels_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.notify_channels().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn notify_channels_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.notify_channels().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn notify_messages_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.notify_messages().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn notify_messages_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.notify_messages().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn scheduled_jobs_fixture_has_a_failed_and_a_successful_job() { + let svc = MockConnectionService::ready(); + let jobs = svc.scheduled_jobs().await.expect("jobs"); + assert!( + jobs.iter().any(|j| j.last_status == "success"), + "fixture must include a successful job" + ); + assert!( + jobs.iter().any(|j| j.last_status.starts_with("failed")), + "fixture must include a failed job so the status column isn't uniform" + ); + } +} + +#[cfg(test)] +mod lifecycle_tests { + use super::*; + use crate::services::connection_service::MockConnectionService; + + #[tokio::test] + async fn session_group_is_studio_scoped() { + let svc = MockConnectionService::ready(); + let s = svc.open_stream_session("cdc").await.expect("session opens"); + assert!( + s.group.starts_with("studio_"), + "Studio must use its own consumer group, got {}", + s.group + ); + } + + #[tokio::test] + async fn open_stream_session_preserves_the_requested_stream_name() { + let svc = MockConnectionService::ready(); + let s = svc.open_stream_session("cdc").await.expect("session opens"); + assert_eq!(s.stream, "cdc"); + assert_eq!(s.group, "studio_cdc"); + } + + /// Reads do not advance the cursor: two reads without a commit return the + /// same rows. A naive poll loop would therefore repeat forever. + #[tokio::test] + async fn reads_are_idempotent_until_committed() { + let svc = MockConnectionService::ready(); + let s = svc.open_stream_session("cdc").await.expect("session"); + let first = svc.cdc_batch(&s, 10).await.expect("first read"); + let second = svc.cdc_batch(&s, 10).await.expect("second read"); + assert_eq!(first, second, "an uncommitted re-read must be identical"); + assert!(!first.is_empty()); + } + + #[tokio::test] + async fn commit_advances_past_the_batch() { + let svc = MockConnectionService::ready(); + let s = svc.open_stream_session("cdc").await.expect("session"); + let first = svc.cdc_batch(&s, 10).await.expect("first read"); + svc.commit_stream_offsets(&s).await.expect("commit"); + let after = svc.cdc_batch(&s, 10).await.expect("read after commit"); + assert!( + after.len() < first.len() || after.is_empty(), + "commit must advance the cursor" + ); + } + + #[tokio::test] + async fn limit_is_respected() { + let svc = MockConnectionService::ready(); + let s = svc.open_stream_session("cdc").await.expect("session"); + assert!(svc.cdc_batch(&s, 2).await.expect("read").len() <= 2); + } + + /// `close_stream_session` drops the group by resetting the cursor, so a + /// freshly opened session sees the same window a brand-new one would. + #[tokio::test] + async fn close_stream_session_resets_the_cursor_for_the_next_session() { + let svc = MockConnectionService::ready(); + let s = svc.open_stream_session("cdc").await.expect("session"); + let first = svc.cdc_batch(&s, 10).await.expect("first read"); + svc.commit_stream_offsets(&s).await.expect("commit"); + svc.close_stream_session(&s).await.expect("close"); + + let s2 = svc + .open_stream_session("cdc") + .await + .expect("reopened session"); + let after_reopen = svc.cdc_batch(&s2, 10).await.expect("read after reopen"); + assert_eq!( + first, after_reopen, + "closing must drop the committed offset, not carry it forward" + ); + } + + /// Writes are not gated by the mock's read behaviour: `commit_stream_offsets` + /// and `close_stream_session` always succeed, exactly like `mark_all_read` + /// does for notifications, even when the configured behaviour makes reads fail. + #[tokio::test] + async fn commit_and_close_succeed_even_when_reads_error() { + let svc = MockConnectionService::erroring(); + let s = svc + .open_stream_session("cdc") + .await + .expect("open is not a read"); + assert!(svc.cdc_batch(&s, 10).await.is_err(), "reads still fail"); + assert!(svc.commit_stream_offsets(&s).await.is_ok()); + assert!(svc.close_stream_session(&s).await.is_ok()); + } + + #[tokio::test] + async fn cdc_batch_empty_behavior_returns_no_rows() { + let svc = MockConnectionService::empty(); + let s = svc.open_stream_session("cdc").await.expect("session"); + let batch = svc.cdc_batch(&s, 10).await.expect("empty read is Ok"); + assert!(batch.is_empty()); + } + + #[tokio::test] + async fn cdc_batch_erroring_behavior_is_err() { + let svc = MockConnectionService::erroring(); + let s = svc.open_stream_session("cdc").await.expect("session"); + assert!(svc.cdc_batch(&s, 10).await.is_err()); + } } From 335ebb28f7da4ebb34254342f50de8f9be192e38 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 21:01:25 +0800 Subject: [PATCH 11/26] fix(seam): stop CDC cursor advancing on empty or erroring reads cdc_batch recorded cdc_read_end from the computed batch before consulting self.behavior, so an empty or erroring read still moved the cursor as if every row had been delivered. A following commit then promoted that phantom offset into cdc_committed, silently skipping events the caller never saw. Now cdc_read_end is set only from what apply actually delivered. Also gives MV/Topic/ScheduledJob/NotifyChannel fixtures distinct id/name pairs so a wrong-field-keyed list bug is visible. --- nodedb-studio/src/data/mock/streams.rs | 60 +++++++++++------ .../src/services/connection_service.rs | 30 +++++++-- nodedb-studio/src/services/streams_data.rs | 67 +++++++++++++++++++ 3 files changed, 134 insertions(+), 23 deletions(-) diff --git a/nodedb-studio/src/data/mock/streams.rs b/nodedb-studio/src/data/mock/streams.rs index b3bb1ac..0b8f365 100644 --- a/nodedb-studio/src/data/mock/streams.rs +++ b/nodedb-studio/src/data/mock/streams.rs @@ -12,19 +12,21 @@ use crate::models::streams::{MaterializedView, NotifyChannel, NotifyMessage, ScheduledJob, Topic}; -/// Materialized views known to the cluster. +/// Materialized views known to the cluster. `id` deliberately differs from +/// `name` (a `mv-N` handle vs. the human-readable view name) so a later bug +/// that keys a list by the wrong field is visible instead of invisible. #[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn materialized_views() -> Vec { vec![ MaterializedView { - id: "mv_top_users_24h".into(), + id: "mv-1".into(), name: "mv_top_users_24h".into(), source: "events".into(), refresh_mode: "incremental".into(), rows: "12,481".into(), }, MaterializedView { - id: "mv_daily_revenue".into(), + id: "mv-2".into(), name: "mv_daily_revenue".into(), source: "orders".into(), refresh_mode: "scheduled".into(), @@ -33,12 +35,13 @@ pub fn materialized_views() -> Vec { ] } -/// Durable, replayable topics with their consumer lag. +/// Durable, replayable topics with their consumer lag. `id` deliberately +/// differs from `name`, same reasoning as `materialized_views`. #[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn topics() -> Vec { vec![ Topic { - id: "order_placed".into(), + id: "topic-1".into(), name: "order_placed".into(), partitions: "8".into(), messages: "2.4M".into(), @@ -47,7 +50,7 @@ pub fn topics() -> Vec { lag: "142ms".into(), }, Topic { - id: "user_signup".into(), + id: "topic-2".into(), name: "user_signup".into(), partitions: "4".into(), messages: "88,209".into(), @@ -56,7 +59,7 @@ pub fn topics() -> Vec { lag: "22ms".into(), }, Topic { - id: "payment_failed".into(), + id: "topic-3".into(), name: "payment_failed".into(), partitions: "2".into(), messages: "12,488".into(), @@ -69,26 +72,27 @@ pub fn topics() -> Vec { /// Cron-style scheduled jobs. Deliberately mixes a failed job in with /// successful ones so fixture-content tests can't be satisfied by an -/// accidentally-uniform status column. +/// accidentally-uniform status column. `id` deliberately differs from `name`, +/// same reasoning as `materialized_views`. #[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn scheduled_jobs() -> Vec { vec![ ScheduledJob { - id: "nightly_rollup".into(), + id: "job-1".into(), name: "nightly_rollup".into(), cron: "0 2 * * *".into(), last_status: "success".into(), next_run: "in 2h 14m".into(), }, ScheduledJob { - id: "session_cleanup".into(), + id: "job-2".into(), name: "session_cleanup".into(), cron: "*/15 * * * *".into(), last_status: "success".into(), next_run: "in 11m".into(), }, ScheduledJob { - id: "vector_reindex".into(), + id: "job-3".into(), name: "vector_reindex".into(), cron: "0 4 * * 0".into(), last_status: "failed · oom".into(), @@ -97,22 +101,23 @@ pub fn scheduled_jobs() -> Vec { ] } -/// LISTEN/NOTIFY channels. +/// LISTEN/NOTIFY channels. `id` deliberately differs from `name`, same +/// reasoning as `materialized_views`. #[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn notify_channel_rows() -> Vec { vec![ NotifyChannel { - id: "user_events".into(), + id: "channel-1".into(), name: "user_events".into(), subscribers: "12".into(), }, NotifyChannel { - id: "deploy_hooks".into(), + id: "channel-2".into(), name: "deploy_hooks".into(), subscribers: "3".into(), }, NotifyChannel { - id: "cache_invalidate".into(), + id: "channel-3".into(), name: "cache_invalidate".into(), subscribers: "5".into(), }, @@ -137,29 +142,33 @@ mod tests { use super::*; #[test] - fn materialized_views_have_unique_ids() { + fn materialized_views_have_unique_ids_distinct_from_name() { let rows = materialized_views(); assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); + assert_ids_distinct_from_names(rows.iter().map(|r| (r.id.as_str(), r.name.as_str()))); } #[test] - fn topics_have_unique_ids() { + fn topics_have_unique_ids_distinct_from_name() { let rows = topics(); assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); + assert_ids_distinct_from_names(rows.iter().map(|r| (r.id.as_str(), r.name.as_str()))); } #[test] - fn scheduled_jobs_have_unique_ids_and_a_mixed_status() { + fn scheduled_jobs_have_unique_ids_distinct_from_name_and_a_mixed_status() { let rows = scheduled_jobs(); assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); + assert_ids_distinct_from_names(rows.iter().map(|r| (r.id.as_str(), r.name.as_str()))); assert!(rows.iter().any(|j| j.last_status == "success")); assert!(rows.iter().any(|j| j.last_status.starts_with("failed"))); } #[test] - fn notify_channel_rows_have_unique_ids() { + fn notify_channel_rows_have_unique_ids_distinct_from_name() { let rows = notify_channel_rows(); assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); + assert_ids_distinct_from_names(rows.iter().map(|r| (r.id.as_str(), r.name.as_str()))); } #[test] @@ -175,4 +184,17 @@ mod tests { sorted.dedup(); assert_eq!(sorted.len(), ids.len(), "ids must be unique"); } + + /// A list keyed by the wrong field (e.g. `name` instead of `id`) still + /// passes `assert_unique_ids` when the fixture happens to have `id == + /// name`, so that mistake stays invisible. Fixtures here deliberately + /// give every row a distinct `id`/`name` pair to make it visible. + fn assert_ids_distinct_from_names<'a>(rows: impl Iterator) { + let mut saw_any = false; + for (id, name) in rows { + saw_any = true; + assert_ne!(id, name, "id must not equal name: {id}"); + } + assert!(saw_any, "fixture must not be empty"); + } } diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index ee6aa58..e435d1c 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -115,6 +115,22 @@ impl MockConnectionService { cdc_read_end: Rc::default(), } } + + /// Test-only: a clone that shares this instance's CDC cursor cells but + /// answers reads with a different `MockBehavior`. Lets a test simulate a + /// single session whose connection recovers after an error (or stops + /// being empty), without losing whatever the cursor already recorded — + /// which is exactly the scenario the commit-after-a-failed-read + /// regression needs to observe. + #[cfg(test)] + pub(crate) fn with_shared_cursor(&self, behavior: MockBehavior) -> Self { + Self { + behavior, + all_read: self.all_read.clone(), + cdc_committed: self.cdc_committed.clone(), + cdc_read_end: self.cdc_read_end.clone(), + } + } } #[async_trait(?Send)] @@ -178,10 +194,16 @@ impl StreamsData for MockConnectionService { .skip(start) .take(limit) .collect(); - // Record how far this read reached, but do NOT advance the committed - // offset: re-reading without a commit must return the same rows. - self.cdc_read_end.set(start + batch.len()); - apply(self.behavior, move || batch).await + // Run the behaviour first: `Erroring` returns before the cursor is + // touched (the `?`), and `Empty` delivers no rows. Only what was + // actually delivered may move `cdc_read_end` — otherwise a commit + // after a failed or empty read would promote a phantom offset into + // `cdc_committed` and silently skip events the caller never saw. + let delivered = apply(self.behavior, move || batch).await?; + // Do NOT advance the committed offset here: re-reading without a + // commit must return the same rows. + self.cdc_read_end.set(start + delivered.len()); + Ok(delivered) } async fn commit_stream_offsets(&self, _session: &StreamSession) -> Result<(), StudioError> { diff --git a/nodedb-studio/src/services/streams_data.rs b/nodedb-studio/src/services/streams_data.rs index 03d296d..a973344 100644 --- a/nodedb-studio/src/services/streams_data.rs +++ b/nodedb-studio/src/services/streams_data.rs @@ -230,6 +230,7 @@ mod tests { mod lifecycle_tests { use super::*; use crate::services::connection_service::MockConnectionService; + use crate::services::mock_behavior::MockBehavior; #[tokio::test] async fn session_group_is_studio_scoped() { @@ -332,4 +333,70 @@ mod lifecycle_tests { let s = svc.open_stream_session("cdc").await.expect("session"); assert!(svc.cdc_batch(&s, 10).await.is_err()); } + + /// Regression: `cdc_batch` used to record `cdc_read_end` from the + /// computed batch *before* consulting `self.behavior`, so an erroring + /// read still moved the cursor as if it had delivered every row. A + /// following commit then promoted that phantom offset into + /// `cdc_committed`, silently skipping events the caller never saw. This + /// simulates the same session's connection recovering (shared cursor + /// cells, `Erroring` swapped for `Ready`) and proves the events are + /// still there to read. + #[tokio::test] + async fn erroring_read_then_commit_does_not_skip_events_once_reads_recover() { + let erroring = MockConnectionService::erroring(); + let s = erroring + .open_stream_session("cdc") + .await + .expect("open is not a read"); + assert!( + erroring.cdc_batch(&s, 10).await.is_err(), + "read fails as configured" + ); + erroring + .commit_stream_offsets(&s) + .await + .expect("commit always succeeds, even after a failed read"); + + let recovered = erroring.with_shared_cursor(MockBehavior::Ready); + let after = recovered + .cdc_batch(&s, 10) + .await + .expect("the recovered read succeeds"); + assert!( + !after.is_empty(), + "a commit after a failed read must not have advanced the cursor \ + past events the caller never saw" + ); + } + + /// Same regression as above, for the `Empty` behaviour: an empty read + /// delivers zero rows, so a following commit must leave the cursor + /// exactly where it was, not wherever the (discarded) full batch would + /// have ended. + #[tokio::test] + async fn empty_read_then_commit_does_not_skip_events_once_reads_recover() { + let empty = MockConnectionService::empty(); + let s = empty + .open_stream_session("cdc") + .await + .expect("open is not a read"); + let first = empty.cdc_batch(&s, 10).await.expect("empty read is Ok"); + assert!(first.is_empty(), "Empty behaviour delivers no rows"); + empty + .commit_stream_offsets(&s) + .await + .expect("commit always succeeds, even after an empty read"); + + let recovered = empty.with_shared_cursor(MockBehavior::Ready); + let after = recovered + .cdc_batch(&s, 10) + .await + .expect("the recovered read succeeds"); + assert!( + !after.is_empty(), + "a commit after an empty read must not have advanced the cursor \ + past events the caller never saw" + ); + } } From 5b11ec335d6928dbe58c9b78d879fe4ce632f90f Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 21:07:50 +0800 Subject: [PATCH 12/26] feat(seam): WorkbenchData trait for query, explain and schema tree Result sets are paginated at the seam because the client buffers whole result sets with no cursor. --- nodedb-studio/src/data/mock/mod.rs | 2 + nodedb-studio/src/data/mock/workbench.rs | 78 ++++++++ nodedb-studio/src/models/mod.rs | 1 + nodedb-studio/src/models/workbench.rs | 34 ++++ nodedb-studio/src/services/backend.rs | 8 +- .../src/services/connection_service.rs | 35 ++++ nodedb-studio/src/services/mod.rs | 1 + nodedb-studio/src/services/nodedb_service.rs | 34 ++++ nodedb-studio/src/services/workbench_data.rs | 171 ++++++++++++++++++ 9 files changed, 362 insertions(+), 2 deletions(-) create mode 100644 nodedb-studio/src/data/mock/workbench.rs create mode 100644 nodedb-studio/src/models/workbench.rs create mode 100644 nodedb-studio/src/services/workbench_data.rs diff --git a/nodedb-studio/src/data/mock/mod.rs b/nodedb-studio/src/data/mock/mod.rs index a2233cb..93c1955 100644 --- a/nodedb-studio/src/data/mock/mod.rs +++ b/nodedb-studio/src/data/mock/mod.rs @@ -13,6 +13,7 @@ mod docs; mod explorer; mod notify; mod streams; +mod workbench; pub use admin::{audit_entries, cluster_nodes, raft_groups, rls_policies, shard_ranges, users}; pub use cdc::{ChangeOp, cdc_events}; @@ -22,3 +23,4 @@ pub use notify::{notify_channels, notify_messages}; pub use streams::{ materialized_views, notify_channel_rows, notify_message_rows, scheduled_jobs, topics, }; +pub use workbench::{query_plan, result_set, schema_tree}; diff --git a/nodedb-studio/src/data/mock/workbench.rs b/nodedb-studio/src/data/mock/workbench.rs new file mode 100644 index 0000000..ac40faf --- /dev/null +++ b/nodedb-studio/src/data/mock/workbench.rs @@ -0,0 +1,78 @@ +//! Workbench fixtures: a deterministic query result, a short explain plan, +//! and a schema tree with path-like ids. + +use crate::models::explorer::RecordRow; +use crate::models::workbench::{QueryPlan, ResultSet, SchemaNode}; + +/// A deterministic 3-column, 4-row result set. Every query text answers with +/// the same shape today; the real implementation decodes whatever the server +/// returned for `sql`. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn result_set(sql: &str) -> ResultSet { + ResultSet { + columns: vec!["id".into(), "name".into(), "created_at".into()], + rows: (0..4) + .map(|i| RecordRow { + id: format!("row-{i}"), + cells: vec![ + format!("row-{i}"), + format!("item {i}"), + format!("2026-08-0{} 10:0{}:00", (i % 9) + 1, i), + ], + }) + .collect(), + elapsed_ms: 12, + scanned: format!("4 rows for `{sql}`"), + } +} + +/// A short, deterministic EXPLAIN plan for `sql`. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn query_plan(sql: &str) -> QueryPlan { + QueryPlan { + text: format!("Seq Scan on users (cost=0.00..1.04 rows=4)\n -- {sql}"), + } +} + +/// A two-level schema tree (database -> collections -> fields) with +/// path-like ids, so uniqueness is structural rather than accidental. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn schema_tree() -> Vec { + vec![SchemaNode { + id: "db".into(), + label: "db".into(), + kind: "database".into(), + children: vec![ + SchemaNode { + id: "db/users".into(), + label: "users".into(), + kind: "collection".into(), + children: vec![ + SchemaNode { + id: "db/users/id".into(), + label: "id".into(), + kind: "field".into(), + children: Vec::new(), + }, + SchemaNode { + id: "db/users/name".into(), + label: "name".into(), + kind: "field".into(), + children: Vec::new(), + }, + ], + }, + SchemaNode { + id: "db/orders".into(), + label: "orders".into(), + kind: "collection".into(), + children: vec![SchemaNode { + id: "db/orders/id".into(), + label: "id".into(), + kind: "field".into(), + children: Vec::new(), + }], + }, + ], + }] +} diff --git a/nodedb-studio/src/models/mod.rs b/nodedb-studio/src/models/mod.rs index a3ddc42..89082f5 100644 --- a/nodedb-studio/src/models/mod.rs +++ b/nodedb-studio/src/models/mod.rs @@ -7,3 +7,4 @@ pub mod collection; pub mod explorer; pub mod notification; pub mod streams; +pub mod workbench; diff --git a/nodedb-studio/src/models/workbench.rs b/nodedb-studio/src/models/workbench.rs new file mode 100644 index 0000000..3dd81f9 --- /dev/null +++ b/nodedb-studio/src/models/workbench.rs @@ -0,0 +1,34 @@ +//! Workbench models: result sets, plans, and the schema tree. + +use serde::{Deserialize, Serialize}; + +use crate::models::explorer::RecordRow; + +/// One page of query output. Pagination is the seam's responsibility: the +/// client buffers whole result sets, so the real implementation emits +/// LIMIT/OFFSET rather than holding a cursor. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ResultSet { + pub columns: Vec, + pub rows: Vec, + pub elapsed_ms: u32, + pub scanned: String, +} + +/// The query planner's EXPLAIN output for one statement. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct QueryPlan { + pub text: String, +} + +/// One node in the schema tree (database / collection / field, recursively). +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SchemaNode { + pub id: String, + pub label: String, + pub kind: String, + pub children: Vec, +} diff --git a/nodedb-studio/src/services/backend.rs b/nodedb-studio/src/services/backend.rs index f7d000a..bcc94e9 100644 --- a/nodedb-studio/src/services/backend.rs +++ b/nodedb-studio/src/services/backend.rs @@ -9,7 +9,11 @@ use crate::services::admin_data::AdminData; use crate::services::connection_service::ConnectionService; use crate::services::explorer_data::ExplorerData; use crate::services::streams_data::StreamsData; +use crate::services::workbench_data::WorkbenchData; -pub trait Backend: ConnectionService + StreamsData + ExplorerData + AdminData {} +pub trait Backend: + ConnectionService + StreamsData + ExplorerData + AdminData + WorkbenchData +{ +} -impl Backend for T {} +impl Backend for T {} diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index e435d1c..0c7f5ee 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -19,11 +19,13 @@ use crate::models::notification::Notification; use crate::models::streams::{ MaterializedView, NotifyChannel, NotifyMessage, ScheduledJob, StreamSession, Topic, }; +use crate::models::workbench::{QueryPlan, ResultSet, SchemaNode}; use crate::services::admin_data::AdminData; use crate::services::error::StudioError; use crate::services::explorer_data::ExplorerData; use crate::services::mock_behavior::{MockBehavior, apply}; use crate::services::streams_data::{StreamsData, cdc_rows_from_mock}; +use crate::services::workbench_data::WorkbenchData; use crate::state::connection::ActiveConnection; use crate::state::connections_registry::{Credentials, SavedConnection}; @@ -290,6 +292,39 @@ impl AdminData for MockConnectionService { } } +#[async_trait(?Send)] +impl WorkbenchData for MockConnectionService { + async fn run_query(&self, sql: &str) -> Result { + match self.behavior { + MockBehavior::Erroring => Err(StudioError::from( + nodedb_client::NodeDbError::node_unreachable("mock"), + )), + MockBehavior::Ready | MockBehavior::Empty => Ok(mock::result_set(sql)), + MockBehavior::Delayed(d) => { + tokio::time::sleep(d).await; + Ok(mock::result_set(sql)) + } + } + } + + async fn explain(&self, sql: &str) -> Result { + match self.behavior { + MockBehavior::Erroring => Err(StudioError::from( + nodedb_client::NodeDbError::node_unreachable("mock"), + )), + MockBehavior::Ready | MockBehavior::Empty => Ok(mock::query_plan(sql)), + MockBehavior::Delayed(d) => { + tokio::time::sleep(d).await; + Ok(mock::query_plan(sql)) + } + } + } + + async fn schema_tree(&self) -> Result, StudioError> { + apply(self.behavior, mock::schema_tree).await + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/nodedb-studio/src/services/mod.rs b/nodedb-studio/src/services/mod.rs index d0816e4..9be639b 100644 --- a/nodedb-studio/src/services/mod.rs +++ b/nodedb-studio/src/services/mod.rs @@ -11,3 +11,4 @@ pub mod explorer_data; pub mod mock_behavior; pub mod nodedb_service; pub mod streams_data; +pub mod workbench_data; diff --git a/nodedb-studio/src/services/nodedb_service.rs b/nodedb-studio/src/services/nodedb_service.rs index 938b3d8..28a590a 100644 --- a/nodedb-studio/src/services/nodedb_service.rs +++ b/nodedb-studio/src/services/nodedb_service.rs @@ -17,11 +17,13 @@ use crate::models::notification::Notification; use crate::models::streams::{ MaterializedView, NotifyChannel, NotifyMessage, ScheduledJob, StreamSession, Topic, }; +use crate::models::workbench::{QueryPlan, ResultSet, SchemaNode}; use crate::services::admin_data::AdminData; use crate::services::connection_service::ConnectionService; use crate::services::error::StudioError; use crate::services::explorer_data::ExplorerData; use crate::services::streams_data::StreamsData; +use crate::services::workbench_data::WorkbenchData; use crate::state::connection::ActiveConnection; use crate::state::connections_registry::{Credentials, SavedConnection}; @@ -147,6 +149,21 @@ impl AdminData for NodeDbConnectionService { } } +#[async_trait(?Send)] +impl WorkbenchData for NodeDbConnectionService { + async fn run_query(&self, _sql: &str) -> Result { + Err(StudioError::NotConnected) + } + + async fn explain(&self, _sql: &str) -> Result { + Err(StudioError::NotConnected) + } + + async fn schema_tree(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } +} + #[cfg(test)] mod tests { use std::rc::Rc; @@ -220,6 +237,23 @@ mod tests { )); } + #[tokio::test] + async fn stub_workbench_reads_are_not_connected() { + let svc = NodeDbConnectionService; + assert!(matches!( + svc.run_query("SELECT 1").await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.explain("SELECT 1").await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.schema_tree().await, + Err(StudioError::NotConnected) + )); + } + #[test] fn stub_is_object_safe_behind_rc() { // Compile-time guarantee: the stub coerces to the seam trait object, diff --git a/nodedb-studio/src/services/workbench_data.rs b/nodedb-studio/src/services/workbench_data.rs new file mode 100644 index 0000000..45e370a --- /dev/null +++ b/nodedb-studio/src/services/workbench_data.rs @@ -0,0 +1,171 @@ +//! Workbench-tier reads at the backend seam: query execution, EXPLAIN, and the +//! schema tree. +//! +//! `run_query` returns a `ResultSet` rather than a raw table because +//! pagination is the seam's job: the real client buffers whole result sets +//! with no cursor, so the eventual implementation emits LIMIT/OFFSET here +//! rather than holding one. + +use async_trait::async_trait; + +use crate::models::workbench::{QueryPlan, ResultSet, SchemaNode}; +use crate::services::error::StudioError; + +#[async_trait(?Send)] +pub trait WorkbenchData { + /// Execute `sql` and return one page of results. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn run_query(&self, sql: &str) -> Result; + + /// The query planner's EXPLAIN output for `sql`. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn explain(&self, sql: &str) -> Result; + + /// The schema tree for the connected database. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn schema_tree(&self) -> Result, StudioError>; +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::services::async_state::AsyncState; + use crate::services::connection_service::MockConnectionService; + + #[tokio::test] + async fn run_query_returns_columns_and_keyed_rows() { + let svc = MockConnectionService::ready(); + let rs = svc.run_query("SELECT 1").await.expect("query runs"); + assert!(!rs.columns.is_empty()); + for r in &rs.rows { + assert_eq!( + r.cells.len(), + rs.columns.len(), + "row width must match header" + ); + assert!(!r.id.is_empty(), "rows need a stable key"); + } + } + + #[tokio::test] + async fn run_query_ready_fixture_has_three_columns_and_four_rows() { + // Expectation authored independently of the fixture body: the brief + // pins the shape at "3-column, 4-row", so this asserts those literal + // numbers rather than deriving them from the call under test. + let svc = MockConnectionService::ready(); + let rs = svc.run_query("SELECT 1").await.expect("query runs"); + assert_eq!(rs.columns.len(), 3, "fixture is documented as 3 columns"); + assert_eq!(rs.rows.len(), 4, "fixture is documented as 4 rows"); + } + + #[tokio::test] + async fn run_query_empty_behaviour_still_returns_a_result_set() { + // `run_query` reads a single result set, not a list: "no rows" has no + // meaning at this call boundary, so `MockBehavior::Empty` folds into + // the same success path as `Ready`, mirroring `record_detail`. + let svc = MockConnectionService::empty(); + let rs = svc + .run_query("SELECT 1") + .await + .expect("empty behaviour still returns a result set for a single-value read"); + assert!(!rs.columns.is_empty()); + } + + #[tokio::test] + async fn erroring_surfaces_through_run_query() { + let svc = MockConnectionService::erroring(); + assert!(svc.run_query("SELECT 1").await.is_err()); + } + + #[tokio::test] + async fn explain_returns_a_plan() { + let svc = MockConnectionService::ready(); + let p = svc.explain("SELECT 1").await.expect("explain runs"); + assert!(!p.text.is_empty()); + } + + #[tokio::test] + async fn explain_ready_fixture_mentions_a_scan() { + // Independently-authored expectation: the fixture is documented as a + // deterministic plan string, so this checks for a marker the fixture + // is known to contain rather than re-deriving it from the same call. + let svc = MockConnectionService::ready(); + let p = svc.explain("SELECT 1").await.expect("explain runs"); + assert!( + p.text.contains("Scan"), + "fixture plan text must describe a scan" + ); + } + + #[tokio::test] + async fn explain_empty_behaviour_still_returns_a_plan() { + let svc = MockConnectionService::empty(); + let p = svc + .explain("SELECT 1") + .await + .expect("empty behaviour still returns a plan for a single-value read"); + assert!(!p.text.is_empty()); + } + + #[tokio::test] + async fn explain_erroring_is_err() { + let svc = MockConnectionService::erroring(); + assert!(svc.explain("SELECT 1").await.is_err()); + } + + #[tokio::test] + async fn schema_tree_nodes_have_unique_ids() { + let svc = MockConnectionService::ready(); + let tree = svc.schema_tree().await.expect("schema"); + let mut ids = Vec::new(); + fn walk<'a>(ns: &'a [SchemaNode], out: &mut Vec<&'a str>) { + for n in ns { + out.push(n.id.as_str()); + walk(&n.children, out); + } + } + walk(&tree, &mut ids); + let total = ids.len(); + ids.sort_unstable(); + ids.dedup(); + assert_eq!(total, ids.len(), "schema node ids must be unique"); + } + + #[tokio::test] + async fn schema_tree_ready_fixture_is_at_least_two_levels_deep_with_path_like_ids() { + // Expectation authored independently: the brief requires a fixture + // that is structurally (not accidentally) unique, rooted at "db" + // with a "db/users" descendant. + let svc = MockConnectionService::ready(); + let tree = svc.schema_tree().await.expect("schema"); + let root = tree.first().expect("schema tree has a root"); + assert_eq!(root.id, "db"); + assert!( + root.children.iter().any(|c| c.id == "db/users"), + "root must have a db/users child" + ); + let users = root + .children + .iter() + .find(|c| c.id == "db/users") + .expect("db/users child exists"); + assert!( + !users.children.is_empty(), + "db/users must have its own children for a two-level tree" + ); + } + + #[tokio::test] + async fn schema_tree_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.schema_tree().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn schema_tree_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.schema_tree().await)); + assert!(s.error_message().is_some()); + } +} From 23f84db54198b0a6a4a5d6f6c735fdd4f9910d4d Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 21:14:10 +0800 Subject: [PATCH 13/26] feat(seam): ViewersData trait for graph, vector, series, spatial, FTS and sync Models carry their own display fields because the client decodes graph properties and vector metadata as empty; the real implementation populates them from raw rows. --- nodedb-studio/src/data/mock/mod.rs | 2 + nodedb-studio/src/data/mock/viewers.rs | 123 +++++++++ nodedb-studio/src/models/mod.rs | 1 + nodedb-studio/src/models/viewers.rs | 88 +++++++ nodedb-studio/src/services/backend.rs | 8 +- .../src/services/connection_service.rs | 42 +++ nodedb-studio/src/services/mod.rs | 1 + nodedb-studio/src/services/nodedb_service.rs | 60 +++++ nodedb-studio/src/services/viewers_data.rs | 246 ++++++++++++++++++ 9 files changed, 569 insertions(+), 2 deletions(-) create mode 100644 nodedb-studio/src/data/mock/viewers.rs create mode 100644 nodedb-studio/src/models/viewers.rs create mode 100644 nodedb-studio/src/services/viewers_data.rs diff --git a/nodedb-studio/src/data/mock/mod.rs b/nodedb-studio/src/data/mock/mod.rs index 93c1955..e40f194 100644 --- a/nodedb-studio/src/data/mock/mod.rs +++ b/nodedb-studio/src/data/mock/mod.rs @@ -13,6 +13,7 @@ mod docs; mod explorer; mod notify; mod streams; +mod viewers; mod workbench; pub use admin::{audit_entries, cluster_nodes, raft_groups, rls_policies, shard_ranges, users}; @@ -23,4 +24,5 @@ pub use notify::{notify_channels, notify_messages}; pub use streams::{ materialized_views, notify_channel_rows, notify_message_rows, scheduled_jobs, topics, }; +pub use viewers::{fts_hits, series, spatial_features, sub_graph, sync_peers, vector_points}; pub use workbench::{query_plan, result_set, schema_tree}; diff --git a/nodedb-studio/src/data/mock/viewers.rs b/nodedb-studio/src/data/mock/viewers.rs new file mode 100644 index 0000000..1d535c8 --- /dev/null +++ b/nodedb-studio/src/data/mock/viewers.rs @@ -0,0 +1,123 @@ +//! Specialized-viewer fixtures: graph, vector, timeseries, spatial, FTS, sync. +//! +//! The client decodes `SubGraph` node/edge properties and `SearchResult.metadata` +//! as empty, which is why `models::viewers` defines its own display-carrying +//! types instead of reusing client types here. These fixtures are deterministic +//! stand-ins for what the real implementation will decode from raw SQL rows. + +use crate::models::viewers::{ + FtsHit, GraphEdge, GraphNode, SeriesPoint, SpatialFeature, SubGraph, SyncPeer, VectorPoint, +}; + +/// A small connected graph: every edge references a node present in `nodes`. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn sub_graph() -> SubGraph { + let nodes = vec![ + GraphNode { + id: "n1".into(), + label: "alice".into(), + x: 0.0, + y: 0.0, + }, + GraphNode { + id: "n2".into(), + label: "bob".into(), + x: 1.0, + y: 0.5, + }, + GraphNode { + id: "n3".into(), + label: "carol".into(), + x: 2.0, + y: 1.0, + }, + ]; + let edges = vec![ + GraphEdge { + id: "e1".into(), + from: "n1".into(), + to: "n2".into(), + label: "follows".into(), + }, + GraphEdge { + id: "e2".into(), + from: "n2".into(), + to: "n3".into(), + label: "follows".into(), + }, + ]; + SubGraph { nodes, edges } +} + +/// A 2D projection of embeddings, grouped into a couple of clusters. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn vector_points() -> Vec { + (0..6) + .map(|i| VectorPoint { + id: format!("vec-{i}"), + x: i as f32 * 0.3, + y: (i % 3) as f32 * 0.7, + cluster: if i % 2 == 0 { "a" } else { "b" }.into(), + }) + .collect() +} + +/// Samples for one timeseries metric. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn series(metric: &str) -> Vec { + (0..8) + .map(|i| SeriesPoint { + id: format!("{metric}-{i}"), + t: format!("2026-08-08T10:0{i}:00Z"), + value: 10.0 + i as f32, + }) + .collect() +} + +/// Spatial features with placeholder GeoJSON geometry. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn spatial_features() -> Vec { + vec![ + SpatialFeature { + id: "kl-tower".into(), + name: "KL Tower".into(), + geometry_json: r#"{"type":"Point","coordinates":[101.7038,3.1528]}"#.into(), + }, + SpatialFeature { + id: "petronas".into(), + name: "Petronas Towers".into(), + geometry_json: r#"{"type":"Point","coordinates":[101.7119,3.1579]}"#.into(), + }, + ] +} + +/// Full-text-search hits for `query`. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn fts_hits(query: &str) -> Vec { + (0..3) + .map(|i| FtsHit { + id: format!("hit-{i}"), + excerpt: format!("...an excerpt mentioning {query}..."), + score: format!("0.{}", 9 - i), + }) + .collect() +} + +/// Sync/replication peers. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn sync_peers() -> Vec { + vec![ + SyncPeer { + id: "peer-1".into(), + name: "eu-west".into(), + state: "synced".into(), + lag: "0ms".into(), + }, + SyncPeer { + id: "peer-2".into(), + name: "ap-south".into(), + state: "lagging".into(), + lag: "820ms".into(), + }, + ] +} diff --git a/nodedb-studio/src/models/mod.rs b/nodedb-studio/src/models/mod.rs index 89082f5..084e6b1 100644 --- a/nodedb-studio/src/models/mod.rs +++ b/nodedb-studio/src/models/mod.rs @@ -7,4 +7,5 @@ pub mod collection; pub mod explorer; pub mod notification; pub mod streams; +pub mod viewers; pub mod workbench; diff --git a/nodedb-studio/src/models/viewers.rs b/nodedb-studio/src/models/viewers.rs new file mode 100644 index 0000000..6be4969 --- /dev/null +++ b/nodedb-studio/src/models/viewers.rs @@ -0,0 +1,88 @@ +//! Specialized-viewer models: graph, vector, timeseries, spatial, FTS, sync. +//! +//! The client decodes `SubGraph` node/edge properties and `SearchResult.metadata` +//! as empty, so Studio cannot get display fields (labels, coordinates, excerpts) +//! by calling those typed client methods. These models carry the fields the +//! viewers actually render; the real seam implementation populates them by +//! decoding raw SQL rows rather than the client's typed graph/search types. + +use serde::{Deserialize, Serialize}; + +/// One node in a graph viewer. `x`/`y` are a laid-out display position, not +/// stored coordinates. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct GraphNode { + pub id: String, + pub label: String, + pub x: f32, + pub y: f32, +} + +/// One edge in a graph viewer. `from`/`to` reference `GraphNode::id`. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct GraphEdge { + pub id: String, + pub from: String, + pub to: String, + pub label: String, +} + +/// A graph viewer's full render input: every edge must reference a node +/// present in `nodes`. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SubGraph { + pub nodes: Vec, + pub edges: Vec, +} + +/// One point in a vector viewer's projection. `x`/`y` are a 2D projection of +/// the embedding, not the raw vector. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct VectorPoint { + pub id: String, + pub x: f32, + pub y: f32, + pub cluster: String, +} + +/// One sample in a timeseries metric. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SeriesPoint { + pub id: String, + pub t: String, + pub value: f32, +} + +/// One feature in a spatial viewer. `geometry_json` is display GeoJSON +/// produced at the seam, never a raw client value. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SpatialFeature { + pub id: String, + pub name: String, + pub geometry_json: String, +} + +/// One full-text-search hit. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct FtsHit { + pub id: String, + pub excerpt: String, + pub score: String, +} + +/// One peer in the sync/replication topology. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SyncPeer { + pub id: String, + pub name: String, + pub state: String, + pub lag: String, +} diff --git a/nodedb-studio/src/services/backend.rs b/nodedb-studio/src/services/backend.rs index bcc94e9..9e57413 100644 --- a/nodedb-studio/src/services/backend.rs +++ b/nodedb-studio/src/services/backend.rs @@ -9,11 +9,15 @@ use crate::services::admin_data::AdminData; use crate::services::connection_service::ConnectionService; use crate::services::explorer_data::ExplorerData; use crate::services::streams_data::StreamsData; +use crate::services::viewers_data::ViewersData; use crate::services::workbench_data::WorkbenchData; pub trait Backend: - ConnectionService + StreamsData + ExplorerData + AdminData + WorkbenchData + ConnectionService + StreamsData + ExplorerData + AdminData + WorkbenchData + ViewersData { } -impl Backend for T {} +impl + Backend for T +{ +} diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index 0c7f5ee..fd9878d 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -19,12 +19,16 @@ use crate::models::notification::Notification; use crate::models::streams::{ MaterializedView, NotifyChannel, NotifyMessage, ScheduledJob, StreamSession, Topic, }; +use crate::models::viewers::{ + FtsHit, SeriesPoint, SpatialFeature, SubGraph, SyncPeer, VectorPoint, +}; use crate::models::workbench::{QueryPlan, ResultSet, SchemaNode}; use crate::services::admin_data::AdminData; use crate::services::error::StudioError; use crate::services::explorer_data::ExplorerData; use crate::services::mock_behavior::{MockBehavior, apply}; use crate::services::streams_data::{StreamsData, cdc_rows_from_mock}; +use crate::services::viewers_data::ViewersData; use crate::services::workbench_data::WorkbenchData; use crate::state::connection::ActiveConnection; use crate::state::connections_registry::{Credentials, SavedConnection}; @@ -325,6 +329,44 @@ impl WorkbenchData for MockConnectionService { } } +#[async_trait(?Send)] +impl ViewersData for MockConnectionService { + async fn sub_graph(&self) -> Result { + match self.behavior { + MockBehavior::Erroring => Err(StudioError::from( + nodedb_client::NodeDbError::node_unreachable("mock"), + )), + MockBehavior::Ready | MockBehavior::Empty => Ok(mock::sub_graph()), + MockBehavior::Delayed(d) => { + tokio::time::sleep(d).await; + Ok(mock::sub_graph()) + } + } + } + + async fn vector_points(&self) -> Result, StudioError> { + apply(self.behavior, mock::vector_points).await + } + + async fn series(&self, metric: &str) -> Result, StudioError> { + let m = metric.to_string(); + apply(self.behavior, move || mock::series(&m)).await + } + + async fn spatial_features(&self) -> Result, StudioError> { + apply(self.behavior, mock::spatial_features).await + } + + async fn fts_hits(&self, query: &str) -> Result, StudioError> { + let q = query.to_string(); + apply(self.behavior, move || mock::fts_hits(&q)).await + } + + async fn sync_peers(&self) -> Result, StudioError> { + apply(self.behavior, mock::sync_peers).await + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/nodedb-studio/src/services/mod.rs b/nodedb-studio/src/services/mod.rs index 9be639b..765c6fe 100644 --- a/nodedb-studio/src/services/mod.rs +++ b/nodedb-studio/src/services/mod.rs @@ -11,4 +11,5 @@ pub mod explorer_data; pub mod mock_behavior; pub mod nodedb_service; pub mod streams_data; +pub mod viewers_data; pub mod workbench_data; diff --git a/nodedb-studio/src/services/nodedb_service.rs b/nodedb-studio/src/services/nodedb_service.rs index 28a590a..83d5a63 100644 --- a/nodedb-studio/src/services/nodedb_service.rs +++ b/nodedb-studio/src/services/nodedb_service.rs @@ -17,12 +17,16 @@ use crate::models::notification::Notification; use crate::models::streams::{ MaterializedView, NotifyChannel, NotifyMessage, ScheduledJob, StreamSession, Topic, }; +use crate::models::viewers::{ + FtsHit, SeriesPoint, SpatialFeature, SubGraph, SyncPeer, VectorPoint, +}; use crate::models::workbench::{QueryPlan, ResultSet, SchemaNode}; use crate::services::admin_data::AdminData; use crate::services::connection_service::ConnectionService; use crate::services::error::StudioError; use crate::services::explorer_data::ExplorerData; use crate::services::streams_data::StreamsData; +use crate::services::viewers_data::ViewersData; use crate::services::workbench_data::WorkbenchData; use crate::state::connection::ActiveConnection; use crate::state::connections_registry::{Credentials, SavedConnection}; @@ -164,6 +168,33 @@ impl WorkbenchData for NodeDbConnectionService { } } +#[async_trait(?Send)] +impl ViewersData for NodeDbConnectionService { + async fn sub_graph(&self) -> Result { + Err(StudioError::NotConnected) + } + + async fn vector_points(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn series(&self, _metric: &str) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn spatial_features(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn fts_hits(&self, _query: &str) -> Result, StudioError> { + Err(StudioError::NotConnected) + } + + async fn sync_peers(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } +} + #[cfg(test)] mod tests { use std::rc::Rc; @@ -254,6 +285,35 @@ mod tests { )); } + #[tokio::test] + async fn stub_viewer_reads_are_not_connected() { + let svc = NodeDbConnectionService; + assert!(matches!( + svc.sub_graph().await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.vector_points().await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.series("qps").await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.spatial_features().await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.fts_hits("nodedb").await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.sync_peers().await, + Err(StudioError::NotConnected) + )); + } + #[test] fn stub_is_object_safe_behind_rc() { // Compile-time guarantee: the stub coerces to the seam trait object, diff --git a/nodedb-studio/src/services/viewers_data.rs b/nodedb-studio/src/services/viewers_data.rs new file mode 100644 index 0000000..6ca2fc7 --- /dev/null +++ b/nodedb-studio/src/services/viewers_data.rs @@ -0,0 +1,246 @@ +//! Specialized-viewer reads at the backend seam: graph, vector, timeseries, +//! spatial, FTS, and sync. +//! +//! `sub_graph` returns a single `SubGraph` rather than a list: unlike the +//! other five reads it cannot be expressed as `apply(self.behavior, ...)`, +//! which only knows how to fold `MockBehavior::Empty` into `Vec::new()`. It +//! is implemented with the same explicit four-arm match `record_detail` and +//! `run_query` use. + +use async_trait::async_trait; + +use crate::models::viewers::{ + FtsHit, SeriesPoint, SpatialFeature, SubGraph, SyncPeer, VectorPoint, +}; +use crate::services::error::StudioError; + +#[async_trait(?Send)] +pub trait ViewersData { + /// One graph viewer's full render input (nodes + edges). + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn sub_graph(&self) -> Result; + + /// A 2D projection of vector embeddings for the vector viewer. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn vector_points(&self) -> Result, StudioError>; + + /// Samples for one timeseries metric. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn series(&self, metric: &str) -> Result, StudioError>; + + /// Features for the spatial viewer's map. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn spatial_features(&self) -> Result, StudioError>; + + /// Full-text-search hits for `query`. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn fts_hits(&self, query: &str) -> Result, StudioError>; + + /// Sync/replication peers. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn sync_peers(&self) -> Result, StudioError>; +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::services::async_state::AsyncState; + use crate::services::connection_service::MockConnectionService; + + #[tokio::test] + async fn subgraph_edges_reference_existing_nodes() { + let svc = MockConnectionService::ready(); + let g = svc.sub_graph().await.expect("graph"); + let ids: Vec<&str> = g.nodes.iter().map(|n| n.id.as_str()).collect(); + assert!(!g.nodes.is_empty() && !g.edges.is_empty()); + for e in &g.edges { + assert!( + ids.contains(&e.from.as_str()), + "dangling edge from {}", + e.from + ); + assert!(ids.contains(&e.to.as_str()), "dangling edge to {}", e.to); + } + } + + #[tokio::test] + async fn every_viewer_read_is_keyed_and_non_empty() { + let svc = MockConnectionService::ready(); + assert!(!svc.vector_points().await.expect("vec").is_empty()); + assert!(!svc.series("qps").await.expect("series").is_empty()); + assert!(!svc.spatial_features().await.expect("geo").is_empty()); + assert!(!svc.fts_hits("nodedb").await.expect("fts").is_empty()); + assert!(!svc.sync_peers().await.expect("peers").is_empty()); + } + + #[tokio::test] + async fn empty_and_error_states_reachable() { + assert!( + MockConnectionService::empty() + .vector_points() + .await + .expect("empty is Ok") + .is_empty() + ); + assert!( + MockConnectionService::erroring() + .sync_peers() + .await + .is_err() + ); + } + + // Each method below gets its own named ready/empty/erroring coverage: + // `apply(self.behavior, mock::x)` and a mis-wired `Ok(mock::x())` both + // satisfy a single shared assertion, so every method needs its own proof + // that it actually reads `self.behavior` (see admin_data.rs). + + #[tokio::test] + async fn sub_graph_ready_returns_nodes_and_edges() { + let svc = MockConnectionService::ready(); + let g = svc.sub_graph().await.expect("ready yields a graph"); + assert!(!g.nodes.is_empty(), "fixture must have nodes"); + assert!(!g.edges.is_empty(), "fixture must have edges"); + } + + #[tokio::test] + async fn sub_graph_empty_behaviour_still_returns_a_graph() { + // `sub_graph` reads a single value, not a list: "no rows" has no + // meaning here, so the mock folds `MockBehavior::Empty` into the same + // success path as `Ready`, mirroring `record_detail` and `run_query`. + // Pinned here so a later refactor cannot silently change that meaning. + let svc = MockConnectionService::empty(); + let g = svc + .sub_graph() + .await + .expect("empty behaviour still returns a graph for a single-value read"); + assert!( + !g.nodes.is_empty(), + "folded-Empty graph must still have nodes" + ); + } + + #[tokio::test] + async fn sub_graph_erroring_is_err() { + let svc = MockConnectionService::erroring(); + assert!(svc.sub_graph().await.is_err()); + } + + #[tokio::test] + async fn vector_points_ready_has_two_clusters() { + // Expectation authored independently of the fixture body: the fixture + // alternates clusters "a"/"b", so both must be present. + let svc = MockConnectionService::ready(); + let pts = svc.vector_points().await.expect("ready yields points"); + assert!(pts.iter().any(|p| p.cluster == "a")); + assert!(pts.iter().any(|p| p.cluster == "b")); + } + + #[tokio::test] + async fn vector_points_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.vector_points().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn vector_points_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.vector_points().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn series_ready_ids_are_prefixed_with_the_requested_metric() { + let svc = MockConnectionService::ready(); + let points = svc.series("qps").await.expect("ready yields points"); + assert!( + points.iter().all(|p| p.id.starts_with("qps-")), + "series ids must key off the requested metric, not a fixed name" + ); + } + + #[tokio::test] + async fn series_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.series("qps").await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn series_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.series("qps").await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn spatial_features_ready_have_geometry() { + let svc = MockConnectionService::ready(); + let feats = svc.spatial_features().await.expect("ready yields features"); + assert!( + feats.iter().all(|f| !f.geometry_json.is_empty()), + "every feature must carry display geometry" + ); + } + + #[tokio::test] + async fn spatial_features_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.spatial_features().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn spatial_features_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.spatial_features().await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn fts_hits_ready_excerpts_mention_the_query() { + let svc = MockConnectionService::ready(); + let hits = svc.fts_hits("nodedb").await.expect("ready yields hits"); + assert!( + hits.iter().all(|h| h.excerpt.contains("nodedb")), + "excerpts must reflect the requested query, not a fixed string" + ); + } + + #[tokio::test] + async fn fts_hits_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.fts_hits("nodedb").await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn fts_hits_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.fts_hits("nodedb").await)); + assert!(s.error_message().is_some()); + } + + #[tokio::test] + async fn sync_peers_ready_has_a_lagging_and_a_synced_peer() { + let svc = MockConnectionService::ready(); + let peers = svc.sync_peers().await.expect("ready yields peers"); + assert!(peers.iter().any(|p| p.state == "synced")); + assert!(peers.iter().any(|p| p.state == "lagging")); + } + + #[tokio::test] + async fn sync_peers_empty_is_empty() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.sync_peers().await)); + assert!(s.is_empty()); + } + + #[tokio::test] + async fn sync_peers_erroring_is_err() { + let svc = MockConnectionService::erroring(); + let s = AsyncState::from_value(Some(svc.sync_peers().await)); + assert!(s.error_message().is_some()); + } +} From efc3ff444bf0c450d755770888600a6c60367d66 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 21:27:45 +0800 Subject: [PATCH 14/26] feat(seam): nav badges, session info and database list Shell chrome reads through the seam instead of rendering literals. --- nodedb-studio/src/data/mock/connections.rs | 37 ++++ nodedb-studio/src/data/mock/mod.rs | 4 +- nodedb-studio/src/models/mod.rs | 1 + nodedb-studio/src/models/shell.rs | 25 +++ .../src/services/connection_service.rs | 167 ++++++++++++++++++ nodedb-studio/src/services/nodedb_service.rs | 30 ++++ 6 files changed, 263 insertions(+), 1 deletion(-) create mode 100644 nodedb-studio/src/models/shell.rs diff --git a/nodedb-studio/src/data/mock/connections.rs b/nodedb-studio/src/data/mock/connections.rs index 9311e34..cd9f448 100644 --- a/nodedb-studio/src/data/mock/connections.rs +++ b/nodedb-studio/src/data/mock/connections.rs @@ -1,5 +1,6 @@ use crate::models::collection::{Collection, StorageMode}; use crate::models::notification::{Notification, NotificationTarget, Severity}; +use crate::models::shell::{NavBadges, SessionInfo}; use crate::state::connection::{Capabilities, Capability}; use crate::state::connections_registry::{ConnStatus, ConnectionProfile, SavedConnection}; @@ -222,3 +223,39 @@ pub fn notifications() -> Vec { }, ] } + +/// Nav-rail badge counts: pending items on the Query and Streams entries. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn nav_badges() -> NavBadges { + NavBadges { + query: 3, + streams: 2, + } +} + +/// The active session summary shown in the statusbar. `server_version` is a +/// neutral "dev" placeholder: NodeDB version numbers are undecided, so no +/// specific version is invented here (see the module note in `mod.rs`). +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn session_info() -> SessionInfo { + SessionInfo { + database: "analytics".into(), + role: "admin".into(), + server_version: "dev".into(), + timezone: "UTC".into(), + read_only: false, + } +} + +/// Databases visible on the active connection, matching `local-nodedb-dev`'s +/// profile above. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +pub fn databases() -> Vec { + vec![ + "analytics".into(), + "events_log".into(), + "social_graph".into(), + "iot_telemetry".into(), + "docs_corpus".into(), + ] +} diff --git a/nodedb-studio/src/data/mock/mod.rs b/nodedb-studio/src/data/mock/mod.rs index e40f194..97d40ff 100644 --- a/nodedb-studio/src/data/mock/mod.rs +++ b/nodedb-studio/src/data/mock/mod.rs @@ -18,7 +18,9 @@ mod workbench; pub use admin::{audit_entries, cluster_nodes, raft_groups, rls_policies, shard_ranges, users}; pub use cdc::{ChangeOp, cdc_events}; -pub use connections::{connections, explorer_collections, notifications}; +pub use connections::{ + connections, databases, explorer_collections, nav_badges, notifications, session_info, +}; pub use explorer::{collection_groups, record_detail, records}; pub use notify::{notify_channels, notify_messages}; pub use streams::{ diff --git a/nodedb-studio/src/models/mod.rs b/nodedb-studio/src/models/mod.rs index 084e6b1..fa3a72f 100644 --- a/nodedb-studio/src/models/mod.rs +++ b/nodedb-studio/src/models/mod.rs @@ -6,6 +6,7 @@ pub mod cdc; pub mod collection; pub mod explorer; pub mod notification; +pub mod shell; pub mod streams; pub mod viewers; pub mod workbench; diff --git a/nodedb-studio/src/models/shell.rs b/nodedb-studio/src/models/shell.rs new file mode 100644 index 0000000..7262bb1 --- /dev/null +++ b/nodedb-studio/src/models/shell.rs @@ -0,0 +1,25 @@ +//! Shell chrome models: nav-rail badge counts and the statusbar's session +//! summary. Both are single values (not lists), which is why the mock impl +//! cannot use `services::mock_behavior::apply` and instead matches +//! `MockBehavior` explicitly, the same way `record_detail` and `run_query` do. + +use serde::{Deserialize, Serialize}; + +/// Badge counts shown on the nav rail's Query and Streams entries. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub struct NavBadges { + pub query: u32, + pub streams: u32, +} + +/// The active session summary shown in the statusbar. +#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SessionInfo { + pub database: String, + pub role: String, + pub server_version: String, + pub timezone: String, + pub read_only: bool, +} diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index fd9878d..e48f4db 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -16,6 +16,7 @@ use crate::models::admin::{AuditEntry, ClusterNode, RaftGroup, RlsPolicy, ShardR use crate::models::cdc::CdcRow; use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; use crate::models::notification::Notification; +use crate::models::shell::{NavBadges, SessionInfo}; use crate::models::streams::{ MaterializedView, NotifyChannel, NotifyMessage, ScheduledJob, StreamSession, Topic, }; @@ -59,6 +60,18 @@ pub trait ConnectionService { /// server-side; the mock persists it in-process so the unread badge does not /// revert on reload (POP-03). async fn mark_all_read(&self) -> Result<(), StudioError>; + + /// Badge counts for the nav rail's Query and Streams entries. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn nav_badges(&self) -> Result; + + /// The active session summary shown in the statusbar. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn session_info(&self) -> Result; + + /// All databases visible on the active connection. + #[allow(dead_code)] // SEAM-UNWIRED(task-10) + async fn databases(&self) -> Result, StudioError>; } /// Hardcoded implementation used by the skeleton. Data is identical to before; @@ -174,6 +187,36 @@ impl ConnectionService for MockConnectionService { .and_then(|c| c.open()) .ok_or(StudioError::NotConnected) } + + async fn nav_badges(&self) -> Result { + match self.behavior { + MockBehavior::Erroring => Err(StudioError::from( + nodedb_client::NodeDbError::node_unreachable("mock"), + )), + MockBehavior::Ready | MockBehavior::Empty => Ok(mock::nav_badges()), + MockBehavior::Delayed(d) => { + tokio::time::sleep(d).await; + Ok(mock::nav_badges()) + } + } + } + + async fn session_info(&self) -> Result { + match self.behavior { + MockBehavior::Erroring => Err(StudioError::from( + nodedb_client::NodeDbError::node_unreachable("mock"), + )), + MockBehavior::Ready | MockBehavior::Empty => Ok(mock::session_info()), + MockBehavior::Delayed(d) => { + tokio::time::sleep(d).await; + Ok(mock::session_info()) + } + } + } + + async fn databases(&self) -> Result, StudioError> { + apply(self.behavior, mock::databases).await + } } #[async_trait(?Send)] @@ -459,6 +502,130 @@ mod tests { assert!(svc.connect(&name, &creds).await.is_ok()); } + #[tokio::test] + async fn session_info_populates_the_statusbar() { + let svc = MockConnectionService::ready(); + let s = svc.session_info().await.expect("session info"); + assert!(!s.database.is_empty()); + assert!(!s.role.is_empty()); + assert!(!s.server_version.is_empty()); + assert!(!s.timezone.is_empty()); + } + + #[tokio::test] + async fn session_info_reports_the_fixture_values() { + // Independently-authored expectations (not derived from the call under + // test), so this can actually fail if the fixture drifts. + let svc = MockConnectionService::ready(); + let s = svc.session_info().await.expect("session info"); + assert_eq!(s.database, "analytics"); + assert_eq!(s.role, "admin"); + assert!(!s.read_only, "fixture session is a read-write admin"); + } + + #[tokio::test] + async fn session_info_empty_behavior_still_returns_the_fixture() { + // Single-value reads have no "empty" shape, so Empty folds into Ready + // — matching the `record_detail` / `run_query` precedent. + let svc = MockConnectionService::empty(); + let s = svc.session_info().await.expect("empty folds into ready"); + assert_eq!(s.database, "analytics"); + } + + #[tokio::test] + async fn session_info_erroring_is_retriable_error() { + let svc = MockConnectionService::erroring(); + let err = svc.session_info().await.expect_err("erroring must fail"); + assert!(err.is_retriable()); + } + + #[tokio::test] + async fn session_info_delayed_still_resolves() { + let svc = MockConnectionService::delayed(std::time::Duration::from_millis(5)); + let s = svc.session_info().await.expect("delayed still resolves"); + assert_eq!(s.role, "admin"); + } + + #[tokio::test] + async fn nav_badges_come_from_the_seam() { + let svc = MockConnectionService::ready(); + let b = svc.nav_badges().await.expect("badges"); + assert!(b.query > 0 || b.streams > 0, "fixture should show badges"); + } + + #[tokio::test] + async fn nav_badges_reports_the_fixture_counts() { + let svc = MockConnectionService::ready(); + let b = svc.nav_badges().await.expect("badges"); + assert_eq!(b.query, 3); + assert_eq!(b.streams, 2); + } + + #[tokio::test] + async fn nav_badges_empty_behavior_still_returns_the_fixture() { + let svc = MockConnectionService::empty(); + let b = svc.nav_badges().await.expect("empty folds into ready"); + assert!(b.query > 0 || b.streams > 0); + } + + #[tokio::test] + async fn nav_badges_erroring_is_retriable_error() { + let svc = MockConnectionService::erroring(); + let err = svc.nav_badges().await.expect_err("erroring must fail"); + assert!(err.is_retriable()); + } + + #[tokio::test] + async fn nav_badges_delayed_still_resolves() { + let svc = MockConnectionService::delayed(std::time::Duration::from_millis(5)); + let b = svc.nav_badges().await.expect("delayed still resolves"); + assert_eq!(b.query, 3); + } + + #[tokio::test] + async fn databases_are_unique() { + let svc = MockConnectionService::ready(); + let mut dbs = svc.databases().await.expect("databases"); + let total = dbs.len(); + dbs.sort(); + dbs.dedup(); + assert_eq!(total, dbs.len()); + } + + #[tokio::test] + async fn databases_fixture_has_more_than_one_entry() { + // Pins the precondition `databases_are_unique` relies on: with a + // single-entry fixture the dedup check above could never fire. + let svc = MockConnectionService::ready(); + let dbs = svc.databases().await.expect("databases"); + assert!( + dbs.len() > 1, + "fixture must list more than one database for the dedup check to be meaningful" + ); + assert!(dbs.contains(&"analytics".to_string())); + } + + #[tokio::test] + async fn databases_empty_behavior_returns_no_rows() { + let svc = MockConnectionService::empty(); + let dbs = svc.databases().await.expect("mock databases is infallible"); + assert!(dbs.is_empty()); + } + + #[tokio::test] + async fn databases_erroring_is_retriable_error() { + let svc = MockConnectionService::erroring(); + let err = svc.databases().await.expect_err("erroring must fail"); + assert!(err.is_retriable()); + } + + #[tokio::test] + async fn databases_delayed_still_returns_the_fixture() { + let svc = MockConnectionService::delayed(std::time::Duration::from_millis(5)); + let dbs = svc.databases().await.expect("delayed still resolves"); + assert!(!dbs.is_empty()); + } + #[tokio::test] async fn cdc_feed_ready_is_loaded() { let svc = MockConnectionService::ready(); diff --git a/nodedb-studio/src/services/nodedb_service.rs b/nodedb-studio/src/services/nodedb_service.rs index 83d5a63..bb8dc8e 100644 --- a/nodedb-studio/src/services/nodedb_service.rs +++ b/nodedb-studio/src/services/nodedb_service.rs @@ -14,6 +14,7 @@ use crate::models::admin::{AuditEntry, ClusterNode, RaftGroup, RlsPolicy, ShardR use crate::models::cdc::CdcRow; use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; use crate::models::notification::Notification; +use crate::models::shell::{NavBadges, SessionInfo}; use crate::models::streams::{ MaterializedView, NotifyChannel, NotifyMessage, ScheduledJob, StreamSession, Topic, }; @@ -60,6 +61,18 @@ impl ConnectionService for NodeDbConnectionService { async fn mark_all_read(&self) -> Result<(), StudioError> { Err(StudioError::NotConnected) } + + async fn nav_badges(&self) -> Result { + Err(StudioError::NotConnected) + } + + async fn session_info(&self) -> Result { + Err(StudioError::NotConnected) + } + + async fn databases(&self) -> Result, StudioError> { + Err(StudioError::NotConnected) + } } #[async_trait(?Send)] @@ -226,6 +239,23 @@ mod tests { )); } + #[tokio::test] + async fn stub_shell_chrome_reads_are_not_connected() { + let svc = NodeDbConnectionService; + assert!(matches!( + svc.nav_badges().await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.session_info().await, + Err(StudioError::NotConnected) + )); + assert!(matches!( + svc.databases().await, + Err(StudioError::NotConnected) + )); + } + #[tokio::test] async fn stub_streams_lifecycle_and_lists_are_not_connected() { let svc = NodeDbConnectionService; From ebef86ba4011a6baa553dc2d19f6a6f43373f4f3 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 21:35:36 +0800 Subject: [PATCH 15/26] feat(seam): wire Explorer sidebar to collection_groups, add seam-discipline gate Split ExplorerSidebar into a fetch wrapper (use_resource over Backend::collection_groups) and a presentational SidebarGroups component driven by AsyncState, following the CDC screen's pattern. Groups and collections are keyed by StorageMode::key()/name instead of Debug/index. Removed the now-orphaned mock::explorer_collections fixture (superseded by mock::collection_groups) and the SEAM-UNWIRED(task-10) tags on the collection_groups path now that it has a real non-test caller. Added tests/seam_discipline.rs: an integration test that scans src/views, src/components and src/modals for data::mock/mock:: references outside code comments, failing with file:line detail on any new violation. views/streams/notify.rs is the one documented exception (its seam models are missing fields the current view renders; rewiring is deliberately deferred), verified to actually fail without the exception before restoring it. --- nodedb-studio/src/data/mock/connections.rs | 27 --- nodedb-studio/src/data/mock/explorer.rs | 2 - nodedb-studio/src/data/mock/mod.rs | 4 +- nodedb-studio/src/models/explorer.rs | 1 - nodedb-studio/src/services/explorer_data.rs | 1 - nodedb-studio/src/views/explorer/sidebar.rs | 212 +++++++++++++++++--- nodedb-studio/tests/seam_discipline.rs | 123 ++++++++++++ 7 files changed, 303 insertions(+), 67 deletions(-) create mode 100644 nodedb-studio/tests/seam_discipline.rs diff --git a/nodedb-studio/src/data/mock/connections.rs b/nodedb-studio/src/data/mock/connections.rs index cd9f448..c7ade82 100644 --- a/nodedb-studio/src/data/mock/connections.rs +++ b/nodedb-studio/src/data/mock/connections.rs @@ -1,4 +1,3 @@ -use crate::models::collection::{Collection, StorageMode}; use crate::models::notification::{Notification, NotificationTarget, Severity}; use crate::models::shell::{NavBadges, SessionInfo}; use crate::state::connection::{Capabilities, Capability}; @@ -125,32 +124,6 @@ pub fn connections() -> Vec { ] } -/// Explorer collections, in sidebar display order (grouped by storage mode). -/// One NodeDB instance exposes all eight modes; these are not separate engines. -pub fn explorer_collections() -> Vec { - let c = |name: &str, mode, count: &str| Collection { - name: name.to_string(), - mode, - count: count.to_string(), - }; - vec![ - c("users", StorageMode::Document, "12,481"), - c("events", StorageMode::Document, "2.4M"), - c("sessions", StorageMode::Document, "88,209"), - c("orders", StorageMode::Strict, "442,003"), - c("invoices", StorageMode::Strict, "95,818"), - c("doc_embeddings", StorageMode::Vector, "1.1M"), - c("product_embeds", StorageMode::Vector, "88,400"), - c("social_graph", StorageMode::Graph, "3.2M"), - c("metrics", StorageMode::Timeseries, "48M"), - c("sensor_temps", StorageMode::Timeseries, "5.1M"), - c("sessions_cache", StorageMode::Kv, "18,200"), - c("feature_flags", StorageMode::Kv, "42"), - c("store_locations", StorageMode::Spatial, "2,108"), - c("articles_idx", StorageMode::Fts, "241,005"), - ] -} - /// The notification feed. Capability gating is applied at render time against /// the active connection (see `state::notifications`). pub fn notifications() -> Vec { diff --git a/nodedb-studio/src/data/mock/explorer.rs b/nodedb-studio/src/data/mock/explorer.rs index 724aae8..70195d6 100644 --- a/nodedb-studio/src/data/mock/explorer.rs +++ b/nodedb-studio/src/data/mock/explorer.rs @@ -3,7 +3,6 @@ use crate::models::collection::{Collection, StorageMode}; use crate::models::explorer::{CollectionGroup, RecordDetail, RecordRow}; -#[allow(dead_code)] // SEAM-UNWIRED(task-10) fn collection(name: &str, mode: StorageMode, count: &str) -> Collection { Collection { name: name.to_string(), @@ -13,7 +12,6 @@ fn collection(name: &str, mode: StorageMode, count: &str) -> Collection { } /// Grouped in the canonical `StorageMode` display order. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn collection_groups() -> Vec { vec![ CollectionGroup { diff --git a/nodedb-studio/src/data/mock/mod.rs b/nodedb-studio/src/data/mock/mod.rs index 97d40ff..7146e45 100644 --- a/nodedb-studio/src/data/mock/mod.rs +++ b/nodedb-studio/src/data/mock/mod.rs @@ -18,9 +18,7 @@ mod workbench; pub use admin::{audit_entries, cluster_nodes, raft_groups, rls_policies, shard_ranges, users}; pub use cdc::{ChangeOp, cdc_events}; -pub use connections::{ - connections, databases, explorer_collections, nav_badges, notifications, session_info, -}; +pub use connections::{connections, databases, nav_badges, notifications, session_info}; pub use explorer::{collection_groups, record_detail, records}; pub use notify::{notify_channels, notify_messages}; pub use streams::{ diff --git a/nodedb-studio/src/models/explorer.rs b/nodedb-studio/src/models/explorer.rs index 935907c..53fdfe8 100644 --- a/nodedb-studio/src/models/explorer.rs +++ b/nodedb-studio/src/models/explorer.rs @@ -5,7 +5,6 @@ use serde::{Deserialize, Serialize}; use crate::models::collection::{Collection, StorageMode}; /// One storage-mode group in the Explorer sidebar. `mode` is the stable key. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct CollectionGroup { pub mode: StorageMode, diff --git a/nodedb-studio/src/services/explorer_data.rs b/nodedb-studio/src/services/explorer_data.rs index d631dad..c4075ff 100644 --- a/nodedb-studio/src/services/explorer_data.rs +++ b/nodedb-studio/src/services/explorer_data.rs @@ -13,7 +13,6 @@ use crate::services::error::StudioError; #[async_trait(?Send)] pub trait ExplorerData { /// Sidebar contents: collections grouped by storage mode, in display order. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) async fn collection_groups(&self) -> Result, StudioError>; /// List-pane rows for one collection. diff --git a/nodedb-studio/src/views/explorer/sidebar.rs b/nodedb-studio/src/views/explorer/sidebar.rs index 45c3029..c31c723 100644 --- a/nodedb-studio/src/views/explorer/sidebar.rs +++ b/nodedb-studio/src/views/explorer/sidebar.rs @@ -1,23 +1,50 @@ -//! Explorer sidebar: collections grouped by storage mode. Clicking a -//! collection updates the shared selection, which swaps the viewer pane. +//! Explorer sidebar: collections grouped by storage mode, read through the +//! seam. Split into a fetch wrapper (`ExplorerSidebar`, owns the async read) +//! and a pure presentational component (`SidebarGroups`, takes `AsyncState` +//! as input) so the four states are render-testable without a runtime. +//! +//! Clicking a collection updates the shared selection, which swaps the +//! viewer pane. + +use std::rc::Rc; use dioxus::prelude::*; -use crate::data::mock; -use crate::models::collection::{Collection, StorageMode}; +use crate::components::async_view::AsyncView; +use crate::models::explorer::CollectionGroup; +use crate::services::async_state::AsyncState; +use crate::services::backend::Backend; use crate::views::explorer::Selected; #[component] pub fn ExplorerSidebar(selected: Signal) -> Element { - // Group collections by mode, preserving the mock's order. - let collections = use_hook(mock::explorer_collections); - let mut groups: Vec<(StorageMode, Vec)> = Vec::new(); - for col in &collections { - match groups.last_mut() { - Some((mode, items)) if *mode == col.mode => items.push(col.clone()), - _ => groups.push((col.mode, vec![col.clone()])), - } + let backend = use_context::>(); + let mut groups = use_resource(move || { + let backend = backend.clone(); + async move { backend.collection_groups().await } + }); + + // Clone the resource value out of its guard immediately — never hold a + // read guard across an await; there is none here. + let state = AsyncState::from_value(groups.read().clone()); + + rsx! { + SidebarGroups { state, selected, on_retry: move |_| groups.restart() } } +} + +#[derive(Props, Clone, PartialEq)] +pub struct SidebarGroupsProps { + pub state: AsyncState>, + pub selected: Signal, + #[props(default)] + pub on_retry: EventHandler<()>, +} + +#[component] +pub fn SidebarGroups(props: SidebarGroupsProps) -> Element { + let state = &props.state; + let mut selected = props.selected; rsx! { aside { class: "explorer-sidebar", @@ -25,27 +52,38 @@ pub fn ExplorerSidebar(selected: Signal) -> Element { input { placeholder: "Filter collections…" } button { class: "btn small ghost", title: "New collection", "+" } } - for (mode, items) in groups { - div { class: "engine-group", - div { class: "engine-group-header", - span { class: "chev", "▾" } - " {mode.label().to_uppercase()}" - } - for col in items { - { - let sel = selected.read(); - let is_active = sel.name == col.name && sel.mode == col.mode; - drop(sel); - let item_class = if is_active { "collection active" } else { "collection" }; - let name = col.name.clone(); - let mode = col.mode; - rsx! { - div { - class: "{item_class}", - onclick: move |_| selected.set(Selected { name: name.clone(), mode }), - span { class: "ico", "{col.mode.icon_letter()}" } - " {col.name} " - span { class: "count", "{col.count}" } + AsyncView { + loading: state.is_loading(), + empty: state.is_empty(), + error: state.error_message(), + retriable: state.is_retriable(), + on_retry: move |_| props.on_retry.call(()), + empty_message: "No collections.".to_string(), + } + if let Some(groups) = state.loaded() { + for group in groups { + div { key: "{group.mode.key()}", class: "engine-group", + div { class: "engine-group-header", + span { class: "chev", "▾" } + " {group.mode.label().to_uppercase()}" + } + for col in &group.collections { + { + let sel = selected.read(); + let is_active = sel.name == col.name && sel.mode == col.mode; + drop(sel); + let item_class = if is_active { "collection active" } else { "collection" }; + let name = col.name.clone(); + let mode = col.mode; + rsx! { + div { + key: "{col.name}", + class: "{item_class}", + onclick: move |_| selected.set(Selected { name: name.clone(), mode }), + span { class: "ico", "{col.mode.icon_letter()}" } + " {col.name} " + span { class: "count", "{col.count}" } + } } } } @@ -55,3 +93,111 @@ pub fn ExplorerSidebar(selected: Signal) -> Element { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::collection::{Collection, StorageMode}; + use crate::services::error::StudioError; + + fn render(app: fn() -> Element) -> String { + let mut dom = VirtualDom::new(app); + dom.rebuild_in_place(); + dioxus_ssr::render(&dom) + } + + fn sample_groups() -> Vec { + vec![ + CollectionGroup { + mode: StorageMode::Document, + collections: vec![Collection { + name: "users".to_string(), + mode: StorageMode::Document, + count: "12,481".to_string(), + }], + }, + CollectionGroup { + mode: StorageMode::Vector, + collections: vec![Collection { + name: "embeddings".to_string(), + mode: StorageMode::Vector, + count: "2.4M".to_string(), + }], + }, + ] + } + + fn selected_signal() -> Signal { + Signal::new(Selected { + name: "users".to_string(), + mode: StorageMode::Document, + }) + } + + fn app_loading() -> Element { + rsx! { + SidebarGroups { + state: AsyncState::Loading, + selected: selected_signal(), + } + } + } + + fn app_empty() -> Element { + rsx! { + SidebarGroups { + state: AsyncState::from_value(Some(Ok(Vec::::new()))), + selected: selected_signal(), + } + } + } + + fn app_error() -> Element { + rsx! { + SidebarGroups { + state: AsyncState::Error(StudioError::NotConnected), + selected: selected_signal(), + } + } + } + + fn app_ready() -> Element { + rsx! { + SidebarGroups { + state: AsyncState::from_value(Some(Ok(sample_groups()))), + selected: selected_signal(), + } + } + } + + #[test] + fn loading_state_renders_spinner() { + let html = render(app_loading); + assert!(html.contains("async-loading")); + } + + #[test] + fn empty_state_renders_message() { + let html = render(app_empty); + assert!(html.contains("async-empty")); + assert!(html.contains("No collections")); + } + + #[test] + fn error_state_renders_error() { + let html = render(app_error); + assert!(html.contains("async-error")); + } + + #[test] + fn ready_state_renders_keyed_groups_and_collections() { + let html = render(app_ready); + assert!(html.contains("engine-group")); + assert!(html.contains("DOCUMENT")); + assert!(html.contains("VECTOR")); + assert!(html.contains("users")); + assert!(html.contains("embeddings")); + // The selected collection ("users") renders with the active class. + assert!(html.contains("collection active")); + } +} diff --git a/nodedb-studio/tests/seam_discipline.rs b/nodedb-studio/tests/seam_discipline.rs new file mode 100644 index 0000000..d78bdc9 --- /dev/null +++ b/nodedb-studio/tests/seam_discipline.rs @@ -0,0 +1,123 @@ +//! Structural gate: views/components/modals must read data through the +//! backend seam (`services::backend::Backend`), never straight from +//! `data::mock`. Reaching past the seam is exactly the bug the seam exists +//! to prevent — a screen that "works" against mock data but silently breaks +//! (or never connects) once a real `Backend` impl lands. +//! +//! This is a plain filesystem/text scan, not a `syn`-based check: the crate +//! has no lib target (bin-only, see AGENTS.md), so an integration test here +//! cannot `use` crate items at all. Scanning source text is the only option +//! available at this layer, and it is enough to catch the pattern we care +//! about (`data::mock` / `mock::` reference-by-name). + +use std::fs; +use std::path::{Path, PathBuf}; + +/// Directories (relative to the crate root) that must stay seam-only. +const SCANNED_ROOTS: &[&str] = &["src/views", "src/components", "src/modals"]; + +/// The one documented exception. `views/streams/notify.rs` renders +/// `models::streams::NotifyChannel`/`NotifyMessage`, but those seam models +/// are missing the `active` and `source` fields the current notify view +/// renders (a gap flagged in Task 6's review). Rewiring notify would force a +/// UI redesign decision that is deliberately deferred rather than papered +/// over here. Remove this exception the moment notify is rewired to the +/// seam — at that point this test must go back to zero exceptions. +const ALLOWED_EXCEPTIONS: &[&str] = &["views/streams/notify.rs"]; + +/// Recursively collect every `.rs` file under `dir`. +fn collect_rs_files(dir: &Path, out: &mut Vec) { + let Ok(entries) = fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + collect_rs_files(&path, out); + } else if path.extension().is_some_and(|ext| ext == "rs") { + out.push(path); + } + } +} + +/// Strip a trailing `//` line comment (if any) so matches inside comments do +/// not count as violations. This is a simple substring split, not a real +/// tokenizer — sufficient here because none of the scanned files put `//` +/// inside a string literal ahead of real code on the same line. +fn code_part(line: &str) -> &str { + match line.find("//") { + Some(idx) => &line[..idx], + None => line, + } +} + +fn references_mock(line: &str) -> bool { + let code = code_part(line); + code.contains("data::mock") || code.contains("mock::") +} + +#[test] +fn views_components_and_modals_read_only_through_the_seam() { + let manifest_dir = Path::new(env!("CARGO_MANIFEST_DIR")); + + let mut files = Vec::new(); + for root in SCANNED_ROOTS { + collect_rs_files(&manifest_dir.join(root), &mut files); + } + assert!( + !files.is_empty(), + "expected to find .rs files under {SCANNED_ROOTS:?} — scan roots may be wrong" + ); + + let src_dir = manifest_dir.join("src"); + let mut violations: Vec = Vec::new(); + + for file in &files { + let rel = file + .strip_prefix(&src_dir) + .unwrap_or(file) + .to_string_lossy() + .replace('\\', "/"); + + if ALLOWED_EXCEPTIONS.contains(&rel.as_str()) { + continue; + } + + let Ok(contents) = fs::read_to_string(file) else { + continue; + }; + for (idx, line) in contents.lines().enumerate() { + if references_mock(line) { + violations.push(format!("{rel}:{} : {}", idx + 1, line.trim())); + } + } + } + + assert!( + violations.is_empty(), + "found {} reference(s) to data::mock outside the allowed exception \ + ({:?}) — views/components/modals must read through `services::backend::Backend`, \ + not `data::mock` directly:\n{}", + violations.len(), + ALLOWED_EXCEPTIONS, + violations.join("\n") + ); +} + +#[test] +fn the_documented_exception_still_exists_and_still_needs_it() { + // Guards against the exception silently becoming stale: if + // `views/streams/notify.rs` stops referencing `data::mock`, the + // exception entry above is dead and must be deleted along with this + // test's assumption. + let manifest_dir = Path::new(env!("CARGO_MANIFEST_DIR")); + let notify = manifest_dir.join("src/views/streams/notify.rs"); + let contents = fs::read_to_string(¬ify) + .unwrap_or_else(|e| panic!("expected {} to exist: {e}", notify.display())); + let still_uses_mock = contents.lines().any(references_mock); + assert!( + still_uses_mock, + "views/streams/notify.rs no longer references data::mock — remove it from \ + ALLOWED_EXCEPTIONS in this test file, the exception is no longer needed" + ); +} From 622ced4b4c76d9d5b763fcac4b67be27e4662ff6 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 21:50:44 +0800 Subject: [PATCH 16/26] fix(services): unblock AsyncView for single-value seam reads Six seam methods (session_info, nav_badges, record_detail, run_query, explain, sub_graph) returned a single value rather than a Vec, so they could not satisfy AsyncState's IsEmpty bound and were unrenderable through the AsyncView primitive. Add explicit IsEmpty impls (always false, a fetched single value is never "empty") for each of the six models, with a test proving from_value(Some(Ok(_))) now yields Loaded. Also collapse the six byte-identical MockBehavior match blocks in connection_service.rs into a new apply_one helper (mirroring apply's four-arm semantics), and route list_connections/notifications/connect through apply/apply_one so all nine mock methods honour every MockBehavior variant, not just the six added most recently. connect's blank-username guard still runs before any behaviour branch. --- nodedb-studio/src/services/async_state.rs | 95 +++++++++ .../src/services/connection_service.rs | 181 +++++++++++------- nodedb-studio/src/services/mock_behavior.rs | 51 +++++ 3 files changed, 253 insertions(+), 74 deletions(-) diff --git a/nodedb-studio/src/services/async_state.rs b/nodedb-studio/src/services/async_state.rs index 28b150c..c886ef3 100644 --- a/nodedb-studio/src/services/async_state.rs +++ b/nodedb-studio/src/services/async_state.rs @@ -4,6 +4,10 @@ //! testable. Every later wiring phase maps a `use_resource` result into an //! `AsyncState` via `from_value` and hands it to the `AsyncView` component. +use crate::models::explorer::RecordDetail; +use crate::models::shell::{NavBadges, SessionInfo}; +use crate::models::viewers::SubGraph; +use crate::models::workbench::{QueryPlan, ResultSet}; use crate::services::error::StudioError; /// Anything that can report emptiness, so `from_value` can distinguish a @@ -18,6 +22,48 @@ impl IsEmpty for Vec { } } +// Single-value seam reads have no "empty" shape: a fetched value is never +// "empty", it either arrived or it errored (`MockBehavior::Empty` folds into +// `Ready` for these — see `record_detail`/`run_query` precedent). Each impl is +// listed explicitly, deliberately not a blanket `impl IsEmpty for T`, so a +// future single-value model must opt in here rather than silently inheriting +// a meaning that may not fit it. +impl IsEmpty for SessionInfo { + fn is_empty(&self) -> bool { + false + } +} + +impl IsEmpty for NavBadges { + fn is_empty(&self) -> bool { + false + } +} + +impl IsEmpty for RecordDetail { + fn is_empty(&self) -> bool { + false + } +} + +impl IsEmpty for ResultSet { + fn is_empty(&self) -> bool { + false + } +} + +impl IsEmpty for QueryPlan { + fn is_empty(&self) -> bool { + false + } +} + +impl IsEmpty for SubGraph { + fn is_empty(&self) -> bool { + false + } +} + /// The canonical, unit-tested mapping from a `use_resource` read to the four UI /// states. Wired views call `from_value` directly (cloning the resource value /// out of its guard — `StudioError` is `Clone`) and then drive `AsyncView` via @@ -217,6 +263,55 @@ mod tests { ); } + #[test] + fn single_value_seam_models_are_loaded_not_empty() { + // Before their `IsEmpty` impls existed, these six single-value seam + // models could not satisfy `AsyncState::from_value`'s `T: IsEmpty` + // bound at all, so they could never be rendered through `AsyncView`. + // Each must map a fetched value straight to `Loaded`, never `Empty`. + let session_info = AsyncState::from_value(Some(Ok(SessionInfo { + database: String::new(), + role: String::new(), + server_version: String::new(), + timezone: String::new(), + read_only: false, + }))); + assert!(matches!(session_info, AsyncState::Loaded(_))); + + let nav_badges = AsyncState::from_value(Some(Ok(NavBadges { + query: 0, + streams: 0, + }))); + assert!(matches!(nav_badges, AsyncState::Loaded(_))); + + let record_detail = AsyncState::from_value(Some(Ok(RecordDetail { + id: String::new(), + title: String::new(), + body_json: String::new(), + footer: String::new(), + }))); + assert!(matches!(record_detail, AsyncState::Loaded(_))); + + let result_set = AsyncState::from_value(Some(Ok(ResultSet { + columns: Vec::new(), + rows: Vec::new(), + elapsed_ms: 0, + scanned: String::new(), + }))); + assert!(matches!(result_set, AsyncState::Loaded(_))); + + let query_plan = AsyncState::from_value(Some(Ok(QueryPlan { + text: String::new(), + }))); + assert!(matches!(query_plan, AsyncState::Loaded(_))); + + let sub_graph = AsyncState::from_value(Some(Ok(SubGraph { + nodes: Vec::new(), + edges: Vec::new(), + }))); + assert!(matches!(sub_graph, AsyncState::Loaded(_))); + } + #[test] fn project_filters_loaded_and_redrives_empty() { // A filter that keeps elements -> Loaded with the filtered set. diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index e48f4db..951fe2c 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -27,7 +27,7 @@ use crate::models::workbench::{QueryPlan, ResultSet, SchemaNode}; use crate::services::admin_data::AdminData; use crate::services::error::StudioError; use crate::services::explorer_data::ExplorerData; -use crate::services::mock_behavior::{MockBehavior, apply}; +use crate::services::mock_behavior::{MockBehavior, apply, apply_one}; use crate::services::streams_data::{StreamsData, cdc_rows_from_mock}; use crate::services::viewers_data::ViewersData; use crate::services::workbench_data::WorkbenchData; @@ -155,17 +155,21 @@ impl MockConnectionService { #[async_trait(?Send)] impl ConnectionService for MockConnectionService { async fn list_connections(&self) -> Result, StudioError> { - Ok(mock::connections()) + apply(self.behavior, mock::connections).await } async fn notifications(&self) -> Result, StudioError> { - let mut notifs = mock::notifications(); - if self.all_read.get() { - for n in &mut notifs { - n.unread = false; + let all_read = self.all_read.get(); + apply(self.behavior, move || { + let mut notifs = mock::notifications(); + if all_read { + for n in &mut notifs { + n.unread = false; + } } - } - Ok(notifs) + notifs + }) + .await } async fn mark_all_read(&self) -> Result<(), StudioError> { @@ -178,40 +182,29 @@ impl ConnectionService for MockConnectionService { name: &str, creds: &Credentials, ) -> Result { + // The blank-username guard must run before any behaviour branch: a + // caller with bad input gets `MissingUsername`, never a behaviour + // error, even when the service is configured `Erroring`. if creds.username.trim().is_empty() { return Err(StudioError::MissingUsername); } - mock::connections() - .into_iter() - .find(|c| c.name == name) - .and_then(|c| c.open()) - .ok_or(StudioError::NotConnected) + let name = name.to_string(); + let session = apply_one(self.behavior, move || { + mock::connections() + .into_iter() + .find(|c| c.name == name) + .and_then(|c| c.open()) + }) + .await?; + session.ok_or(StudioError::NotConnected) } async fn nav_badges(&self) -> Result { - match self.behavior { - MockBehavior::Erroring => Err(StudioError::from( - nodedb_client::NodeDbError::node_unreachable("mock"), - )), - MockBehavior::Ready | MockBehavior::Empty => Ok(mock::nav_badges()), - MockBehavior::Delayed(d) => { - tokio::time::sleep(d).await; - Ok(mock::nav_badges()) - } - } + apply_one(self.behavior, mock::nav_badges).await } async fn session_info(&self) -> Result { - match self.behavior { - MockBehavior::Erroring => Err(StudioError::from( - nodedb_client::NodeDbError::node_unreachable("mock"), - )), - MockBehavior::Ready | MockBehavior::Empty => Ok(mock::session_info()), - MockBehavior::Delayed(d) => { - tokio::time::sleep(d).await; - Ok(mock::session_info()) - } - } + apply_one(self.behavior, mock::session_info).await } async fn databases(&self) -> Result, StudioError> { @@ -299,16 +292,9 @@ impl ExplorerData for MockConnectionService { } async fn record_detail(&self, collection: &str, id: &str) -> Result { - match self.behavior { - MockBehavior::Erroring => Err(StudioError::from( - nodedb_client::NodeDbError::node_unreachable("mock"), - )), - MockBehavior::Ready | MockBehavior::Empty => Ok(mock::record_detail(collection, id)), - MockBehavior::Delayed(d) => { - tokio::time::sleep(d).await; - Ok(mock::record_detail(collection, id)) - } - } + let c = collection.to_string(); + let i = id.to_string(); + apply_one(self.behavior, move || mock::record_detail(&c, &i)).await } } @@ -342,29 +328,13 @@ impl AdminData for MockConnectionService { #[async_trait(?Send)] impl WorkbenchData for MockConnectionService { async fn run_query(&self, sql: &str) -> Result { - match self.behavior { - MockBehavior::Erroring => Err(StudioError::from( - nodedb_client::NodeDbError::node_unreachable("mock"), - )), - MockBehavior::Ready | MockBehavior::Empty => Ok(mock::result_set(sql)), - MockBehavior::Delayed(d) => { - tokio::time::sleep(d).await; - Ok(mock::result_set(sql)) - } - } + let s = sql.to_string(); + apply_one(self.behavior, move || mock::result_set(&s)).await } async fn explain(&self, sql: &str) -> Result { - match self.behavior { - MockBehavior::Erroring => Err(StudioError::from( - nodedb_client::NodeDbError::node_unreachable("mock"), - )), - MockBehavior::Ready | MockBehavior::Empty => Ok(mock::query_plan(sql)), - MockBehavior::Delayed(d) => { - tokio::time::sleep(d).await; - Ok(mock::query_plan(sql)) - } - } + let s = sql.to_string(); + apply_one(self.behavior, move || mock::query_plan(&s)).await } async fn schema_tree(&self) -> Result, StudioError> { @@ -375,16 +345,7 @@ impl WorkbenchData for MockConnectionService { #[async_trait(?Send)] impl ViewersData for MockConnectionService { async fn sub_graph(&self) -> Result { - match self.behavior { - MockBehavior::Erroring => Err(StudioError::from( - nodedb_client::NodeDbError::node_unreachable("mock"), - )), - MockBehavior::Ready | MockBehavior::Empty => Ok(mock::sub_graph()), - MockBehavior::Delayed(d) => { - tokio::time::sleep(d).await; - Ok(mock::sub_graph()) - } - } + apply_one(self.behavior, mock::sub_graph).await } async fn vector_points(&self) -> Result, StudioError> { @@ -449,6 +410,37 @@ mod tests { assert!(!conns.is_empty()); } + #[tokio::test] + async fn list_connections_empty_behavior_returns_no_rows() { + let svc = MockConnectionService::empty(); + let conns = svc.list_connections().await.expect("empty behaviour is Ok"); + assert!(conns.is_empty()); + } + + #[tokio::test] + async fn list_connections_erroring_is_retriable_error() { + let svc = MockConnectionService::erroring(); + let err = svc + .list_connections() + .await + .expect_err("erroring must fail"); + assert!(err.is_retriable()); + } + + #[tokio::test] + async fn notifications_empty_behavior_returns_no_rows() { + let svc = MockConnectionService::empty(); + let notifs = svc.notifications().await.expect("empty behaviour is Ok"); + assert!(notifs.is_empty()); + } + + #[tokio::test] + async fn notifications_erroring_is_retriable_error() { + let svc = MockConnectionService::erroring(); + let err = svc.notifications().await.expect_err("erroring must fail"); + assert!(err.is_retriable()); + } + #[tokio::test] async fn mock_connect_known_name_returns_session() { let svc = MockConnectionService::ready(); @@ -502,6 +494,47 @@ mod tests { assert!(svc.connect(&name, &creds).await.is_ok()); } + #[tokio::test] + async fn connect_erroring_is_retriable_error() { + let svc = MockConnectionService::erroring(); + let creds = Credentials { + username: "alice".into(), + password: None, + }; + let err = svc + .connect("staging-cluster", &creds) + .await + .expect_err("erroring must fail"); + assert!(err.is_retriable()); + } + + #[tokio::test] + async fn connect_delayed_still_resolves() { + let svc = MockConnectionService::delayed(std::time::Duration::from_millis(5)); + let creds = Credentials { + username: "alice".into(), + password: None, + }; + assert!(svc.connect("staging-cluster", &creds).await.is_ok()); + } + + #[tokio::test] + async fn connect_checks_missing_username_before_consulting_behavior() { + // A blank username must surface `MissingUsername` even when the + // service is configured `Erroring` — the guard must not become + // skippable by routing `connect` through the behaviour matcher. + let svc = MockConnectionService::erroring(); + let creds = Credentials { + username: " ".into(), + password: None, + }; + let out = svc.connect("staging-cluster", &creds).await; + assert!( + matches!(out, Err(StudioError::MissingUsername)), + "blank username must win over the configured Erroring behavior" + ); + } + #[tokio::test] async fn session_info_populates_the_statusbar() { let svc = MockConnectionService::ready(); @@ -558,7 +591,7 @@ mod tests { let svc = MockConnectionService::ready(); let b = svc.nav_badges().await.expect("badges"); assert_eq!(b.query, 3); - assert_eq!(b.streams, 2); + assert_eq!(b.streams, 6); } #[tokio::test] diff --git a/nodedb-studio/src/services/mock_behavior.rs b/nodedb-studio/src/services/mock_behavior.rs index fe039ef..5b720be 100644 --- a/nodedb-studio/src/services/mock_behavior.rs +++ b/nodedb-studio/src/services/mock_behavior.rs @@ -35,6 +35,27 @@ pub async fn apply( } } +/// Apply the behaviour to a single-value fixture thunk (used by seam methods +/// that read one value rather than a list — `session_info`, `nav_badges`, +/// `record_detail`, `run_query`, `explain`, `sub_graph`). `Ready` and `Empty` +/// both call the thunk: a fetched single value has no "empty" shape, so +/// `Empty` folds into `Ready` here rather than inventing an absent value. +pub async fn apply_one( + behavior: MockBehavior, + ready: impl FnOnce() -> T, +) -> Result { + match behavior { + MockBehavior::Ready | MockBehavior::Empty => Ok(ready()), + MockBehavior::Erroring => Err(StudioError::from( + nodedb_client::NodeDbError::node_unreachable("mock"), + )), + MockBehavior::Delayed(d) => { + tokio::time::sleep(d).await; + Ok(ready()) + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -69,4 +90,34 @@ mod tests { .await; assert_eq!(out.expect("delayed yields data"), vec![9u8]); } + + #[tokio::test] + async fn apply_one_ready_returns_the_fixture() { + let out = apply_one(MockBehavior::Ready, || 7u8).await; + assert_eq!(out.expect("ready yields data"), 7u8); + } + + #[tokio::test] + async fn apply_one_empty_still_returns_the_fixture() { + // Unlike `apply`, a single value has no "empty" shape: `Empty` folds + // into `Ready` rather than inventing an absent value. + let out = apply_one(MockBehavior::Empty, || 7u8).await; + assert_eq!(out.expect("empty folds into ready"), 7u8); + } + + #[tokio::test] + async fn apply_one_erroring_returns_a_retriable_error() { + let out = apply_one(MockBehavior::Erroring, || 7u8).await; + let err = out.expect_err("erroring yields Err"); + assert!( + err.is_retriable(), + "demo error must exercise the retry path" + ); + } + + #[tokio::test] + async fn apply_one_delayed_still_returns_the_fixture() { + let out = apply_one(MockBehavior::Delayed(Duration::from_millis(5)), || 9u8).await; + assert_eq!(out.expect("delayed yields data"), 9u8); + } } From 1f8e0318d757faba6d965a8d883d0c2edc811d79 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 21:50:49 +0800 Subject: [PATCH 17/26] chore(services): retag decoder dead-code allows as SEAM-UNWIRED decode.rs and error.rs used bare #[allow(dead_code)] instead of the project's #[allow(dead_code)] // SEAM-UNWIRED(task-10) convention, hiding the decoder module from the grep that enumerates unwired seam surface. --- nodedb-studio/src/services/decode.rs | 6 +++--- nodedb-studio/src/services/error.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nodedb-studio/src/services/decode.rs b/nodedb-studio/src/services/decode.rs index eccc93d..b3953be 100644 --- a/nodedb-studio/src/services/decode.rs +++ b/nodedb-studio/src/services/decode.rs @@ -17,7 +17,7 @@ pub struct Table { } /// One row, addressable by column name. -#[allow(dead_code)] +#[allow(dead_code)] // SEAM-UNWIRED(task-10) pub struct Row<'a> { columns: &'a [String], cells: &'a [String], @@ -25,7 +25,7 @@ pub struct Row<'a> { impl<'a> Row<'a> { /// The cell under `name`, or an error naming the column that is missing. - #[allow(dead_code)] + #[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn field(&self, name: &str) -> Result<&'a str, StudioError> { let idx = self.columns.iter().position(|c| c == name).ok_or_else(|| { StudioError::UnexpectedColumns { @@ -48,7 +48,7 @@ impl<'a> Row<'a> { /// /// The assertion is the point: it turns the server's silent session-variable /// fallback into a typed error rather than an empty result set. -#[allow(dead_code)] +#[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn decode_rows( table: &Table, expect: &[&str], diff --git a/nodedb-studio/src/services/error.rs b/nodedb-studio/src/services/error.rs index 8002225..593e2ef 100644 --- a/nodedb-studio/src/services/error.rs +++ b/nodedb-studio/src/services/error.rs @@ -34,7 +34,7 @@ pub enum StudioError { /// server answered a `SHOW` with its session-variable fallback /// (`cols=["setting"]`), which would otherwise read as an empty screen. #[error("unexpected result columns: expected [{expected}], got [{got}]")] - #[allow(dead_code)] + #[allow(dead_code)] // SEAM-UNWIRED(task-10) UnexpectedColumns { expected: String, got: String }, /// Connect was attempted without an explicit username. #[error("a username is required to connect")] From 478ade580b8647ea36261a38cbfd773a8effe464 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 21:50:55 +0800 Subject: [PATCH 18/26] test: remove or repair three weak seam tests - models/streams.rs: delete stream_session_carries_stream_and_studio_group, a tautology that only echoes the struct literal it builds. - viewers_data.rs: every_viewer_read_is_keyed_and_non_empty now actually asserts every item's id is non-empty, matching what its name claims; previously it only checked collection non-emptiness. - viewers_data.rs: delete empty_and_error_states_reachable, fully subsumed by the later per-method vector_points_empty_is_empty and sync_peers_erroring_is_err. --- nodedb-studio/src/models/streams.rs | 15 ------- nodedb-studio/src/services/viewers_data.rs | 47 ++++++++++++++-------- 2 files changed, 30 insertions(+), 32 deletions(-) diff --git a/nodedb-studio/src/models/streams.rs b/nodedb-studio/src/models/streams.rs index 3653e05..7d2e094 100644 --- a/nodedb-studio/src/models/streams.rs +++ b/nodedb-studio/src/models/streams.rs @@ -67,18 +67,3 @@ pub struct NotifyMessage { pub at: String, pub payload_json: String, } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn stream_session_carries_stream_and_studio_group() { - let s = StreamSession { - stream: "cdc".to_string(), - group: "studio_cdc".to_string(), - }; - assert_eq!(s.stream, "cdc"); - assert_eq!(s.group, "studio_cdc"); - } -} diff --git a/nodedb-studio/src/services/viewers_data.rs b/nodedb-studio/src/services/viewers_data.rs index 6ca2fc7..cf845ae 100644 --- a/nodedb-studio/src/services/viewers_data.rs +++ b/nodedb-studio/src/services/viewers_data.rs @@ -66,27 +66,40 @@ mod tests { #[tokio::test] async fn every_viewer_read_is_keyed_and_non_empty() { let svc = MockConnectionService::ready(); - assert!(!svc.vector_points().await.expect("vec").is_empty()); - assert!(!svc.series("qps").await.expect("series").is_empty()); - assert!(!svc.spatial_features().await.expect("geo").is_empty()); - assert!(!svc.fts_hits("nodedb").await.expect("fts").is_empty()); - assert!(!svc.sync_peers().await.expect("peers").is_empty()); - } - #[tokio::test] - async fn empty_and_error_states_reachable() { + let vector = svc.vector_points().await.expect("vec"); + assert!(!vector.is_empty()); + assert!( + vector.iter().all(|p| !p.id.is_empty()), + "every vector point needs a stable key" + ); + + let series = svc.series("qps").await.expect("series"); + assert!(!series.is_empty()); + assert!( + series.iter().all(|p| !p.id.is_empty()), + "every series point needs a stable key" + ); + + let spatial = svc.spatial_features().await.expect("geo"); + assert!(!spatial.is_empty()); + assert!( + spatial.iter().all(|f| !f.id.is_empty()), + "every spatial feature needs a stable key" + ); + + let fts = svc.fts_hits("nodedb").await.expect("fts"); + assert!(!fts.is_empty()); assert!( - MockConnectionService::empty() - .vector_points() - .await - .expect("empty is Ok") - .is_empty() + fts.iter().all(|h| !h.id.is_empty()), + "every fts hit needs a stable key" ); + + let peers = svc.sync_peers().await.expect("peers"); + assert!(!peers.is_empty()); assert!( - MockConnectionService::erroring() - .sync_peers() - .await - .is_err() + peers.iter().all(|p| !p.id.is_empty()), + "every sync peer needs a stable key" ); } From 7c60ef936acdd38ab0abc587d281b9b68cdc8601 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 21:51:02 +0800 Subject: [PATCH 19/26] fix(state): redact Credentials password from Debug output Credentials derived Debug on a struct carrying an Option password. Harmless while the field is always None, but the moment the connect form populates it, any {:?} or error-chain print would leak the secret. Hand-write Debug instead: it prints username verbatim and always renders password as a fixed "" marker, whether Some or None, so a print cannot even reveal presence-vs-absence. --- .../src/state/connections_registry.rs | 49 ++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/nodedb-studio/src/state/connections_registry.rs b/nodedb-studio/src/state/connections_registry.rs index 8cf8da7..1cad126 100644 --- a/nodedb-studio/src/state/connections_registry.rs +++ b/nodedb-studio/src/state/connections_registry.rs @@ -73,8 +73,55 @@ impl SavedConnection { /// Identity supplied at connect time. Studio never defaults the username: the /// client would silently fall back to `admin`, so a blank field must be a /// validation error surfaced in the connect form. -#[derive(Debug, Clone, Default, PartialEq, Eq)] +/// +/// `Debug` is hand-written, not derived: a derived impl would print +/// `password` verbatim once the connect form starts populating it. The +/// redaction marker is rendered unconditionally (`Some` and `None` look +/// identical) so a `{:?}` print cannot even leak whether a password was set. +#[derive(Clone, Default, PartialEq, Eq)] pub struct Credentials { pub username: String, pub password: Option, } + +impl std::fmt::Debug for Credentials { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Credentials") + .field("username", &self.username) + .field("password", &"") + .finish() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn debug_never_leaks_a_populated_password() { + let creds = Credentials { + username: "alice".into(), + password: Some("hunter2".into()), + }; + let printed = format!("{creds:?}"); + assert!(!printed.contains("hunter2"), "password leaked: {printed}"); + assert!(printed.contains("")); + } + + #[test] + fn debug_reads_identically_whether_a_password_is_set_or_not() { + // The redaction marker must not double as a presence/absence signal. + let with_password = Credentials { + username: "alice".into(), + password: Some("hunter2".into()), + }; + let without_password = Credentials { + username: "alice".into(), + password: None, + }; + assert_eq!( + format!("{with_password:?}"), + format!("{without_password:?}") + ); + } +} From 92449d15577d86dd1982633f495bb0aa7025ff0d Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 8 Aug 2026 21:51:06 +0800 Subject: [PATCH 20/26] fix(data): align nav_badges streams fixture with the rail literal mock::nav_badges() returned streams: 2 while components::rail still renders a hardcoded Streams badge of "6". Update the fixture to 6 so the eventual swap onto the seam is a visual no-op; the rail itself is left unwired, as that is a later phase. --- nodedb-studio/src/data/mock/connections.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nodedb-studio/src/data/mock/connections.rs b/nodedb-studio/src/data/mock/connections.rs index c7ade82..74ed23d 100644 --- a/nodedb-studio/src/data/mock/connections.rs +++ b/nodedb-studio/src/data/mock/connections.rs @@ -198,11 +198,14 @@ pub fn notifications() -> Vec { } /// Nav-rail badge counts: pending items on the Query and Streams entries. +/// These must agree with the hardcoded literals in `components::rail` until +/// that later phase swaps the rail onto this seam method, so the eventual +/// wiring is a visual no-op. #[allow(dead_code)] // SEAM-UNWIRED(task-10) pub fn nav_badges() -> NavBadges { NavBadges { query: 3, - streams: 2, + streams: 6, } } From 6480ebc684d1b1f3f8f9ed0b4e2cd5766925da9a Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sun, 9 Aug 2026 10:59:51 +0800 Subject: [PATCH 21/26] fix(views): derive Explorer's default selection from loaded collections The Explorer hardcoded its default selection as "events" / Document, a collection that does not exist in collection_groups() (an earlier fixture change dropped it). Opening the Explorer showed a viewer header for a collection the sidebar didn't have, with no row highlighted. Replace the hardcoded literal with default_selection(), which derives the default from the loaded data (first collection of the first group). Selected is now Option: there is no selection at all while the seam read is loading, empty, or errored, and the main pane says so instead of fabricating a name. --- nodedb-studio/src/views/explorer/mod.rs | 2 +- nodedb-studio/src/views/explorer/sidebar.rs | 60 ++++++-- nodedb-studio/src/views/explorer/view.rs | 152 ++++++++++++++++---- 3 files changed, 179 insertions(+), 35 deletions(-) diff --git a/nodedb-studio/src/views/explorer/mod.rs b/nodedb-studio/src/views/explorer/mod.rs index 581b6b0..ebe1643 100644 --- a/nodedb-studio/src/views/explorer/mod.rs +++ b/nodedb-studio/src/views/explorer/mod.rs @@ -4,4 +4,4 @@ pub mod sidebar; mod view; pub mod viewers; -pub use view::{Explorer, Selected}; +pub use view::{Explorer, Selected, default_selection}; diff --git a/nodedb-studio/src/views/explorer/sidebar.rs b/nodedb-studio/src/views/explorer/sidebar.rs index c31c723..4830da6 100644 --- a/nodedb-studio/src/views/explorer/sidebar.rs +++ b/nodedb-studio/src/views/explorer/sidebar.rs @@ -4,7 +4,10 @@ //! as input) so the four states are render-testable without a runtime. //! //! Clicking a collection updates the shared selection, which swaps the -//! viewer pane. +//! viewer pane. `ExplorerSidebar` also owns defaulting that selection: once +//! `collection_groups()` loads with data and nothing is selected yet, it +//! picks the first collection of the first group (`default_selection`) — +//! there is no selection at all while loading, empty, or errored. use std::rc::Rc; @@ -14,10 +17,10 @@ use crate::components::async_view::AsyncView; use crate::models::explorer::CollectionGroup; use crate::services::async_state::AsyncState; use crate::services::backend::Backend; -use crate::views::explorer::Selected; +use crate::views::explorer::{Selected, default_selection}; #[component] -pub fn ExplorerSidebar(selected: Signal) -> Element { +pub fn ExplorerSidebar(selected: Signal>) -> Element { let backend = use_context::>(); let mut groups = use_resource(move || { let backend = backend.clone(); @@ -28,6 +31,23 @@ pub fn ExplorerSidebar(selected: Signal) -> Element { // read guard across an await; there is none here. let state = AsyncState::from_value(groups.read().clone()); + // Default the selection once real data is in, but only while nothing has + // been picked yet — a later reload (`on_retry`) must never clobber a + // selection the user already made. Reads `groups` (the resource itself, + // not the derived `state` local) inside the effect so it reruns exactly + // when the resource changes; `selected.peek()` reads without subscribing. + use_effect(move || { + let value = groups.read().clone(); + if selected.peek().is_some() { + return; + } + if let Some(Ok(gs)) = value + && let Some(first) = default_selection(&gs) + { + selected.set(Some(first)); + } + }); + rsx! { SidebarGroups { state, selected, on_retry: move |_| groups.restart() } } @@ -36,7 +56,7 @@ pub fn ExplorerSidebar(selected: Signal) -> Element { #[derive(Props, Clone, PartialEq)] pub struct SidebarGroupsProps { pub state: AsyncState>, - pub selected: Signal, + pub selected: Signal>, #[props(default)] pub on_retry: EventHandler<()>, } @@ -70,7 +90,9 @@ pub fn SidebarGroups(props: SidebarGroupsProps) -> Element { for col in &group.collections { { let sel = selected.read(); - let is_active = sel.name == col.name && sel.mode == col.mode; + let is_active = sel + .as_ref() + .is_some_and(|s| s.name == col.name && s.mode == col.mode); drop(sel); let item_class = if is_active { "collection active" } else { "collection" }; let name = col.name.clone(); @@ -79,7 +101,7 @@ pub fn SidebarGroups(props: SidebarGroupsProps) -> Element { div { key: "{col.name}", class: "{item_class}", - onclick: move |_| selected.set(Selected { name: name.clone(), mode }), + onclick: move |_| selected.set(Some(Selected { name: name.clone(), mode })), span { class: "ico", "{col.mode.icon_letter()}" } " {col.name} " span { class: "count", "{col.count}" } @@ -127,11 +149,15 @@ mod tests { ] } - fn selected_signal() -> Signal { - Signal::new(Selected { + fn selected_signal() -> Signal> { + Signal::new(Some(Selected { name: "users".to_string(), mode: StorageMode::Document, - }) + })) + } + + fn no_selection_signal() -> Signal> { + Signal::new(None) } fn app_loading() -> Element { @@ -170,6 +196,15 @@ mod tests { } } + fn app_ready_no_selection() -> Element { + rsx! { + SidebarGroups { + state: AsyncState::from_value(Some(Ok(sample_groups()))), + selected: no_selection_signal(), + } + } + } + #[test] fn loading_state_renders_spinner() { let html = render(app_loading); @@ -200,4 +235,11 @@ mod tests { // The selected collection ("users") renders with the active class. assert!(html.contains("collection active")); } + + #[test] + fn no_selection_highlights_no_row() { + // Honest "nothing picked yet" state: no row gets the active class. + let html = render(app_ready_no_selection); + assert!(!html.contains("collection active")); + } } diff --git a/nodedb-studio/src/views/explorer/view.rs b/nodedb-studio/src/views/explorer/view.rs index d092913..2cf06fb 100644 --- a/nodedb-studio/src/views/explorer/view.rs +++ b/nodedb-studio/src/views/explorer/view.rs @@ -4,10 +4,17 @@ //! collections grouped by mode; selecting one swaps the main pane to that //! mode's purpose-built viewer. The selected collection is shared between the //! sidebar and the viewer pane via a signal owned here. +//! +//! There is no fabricated default selection. Until the sidebar's seam read +//! resolves — or if it resolves to no collections at all — `selected` stays +//! `None` and the main pane says so honestly. Once `collection_groups()` +//! loads with data, `ExplorerSidebar` defaults `selected` to the first +//! collection of the first group via `default_selection` below. use dioxus::prelude::*; use crate::models::collection::StorageMode; +use crate::models::explorer::CollectionGroup; use crate::views::explorer::sidebar::ExplorerSidebar; use crate::views::explorer::viewers::document::DocumentViewer; use crate::views::explorer::viewers::fts::FtsViewer; @@ -25,12 +32,24 @@ pub struct Selected { pub mode: StorageMode, } +/// The Explorer's default selection: the first collection of the first +/// group, in `collection_groups()`'s own display order. `None` when there +/// are no groups (loading, empty, or errored) — there is no fallback name, +/// because any hardcoded name silently rots the moment the fixture changes +/// (see the `events` regression this replaced: the collection it named had +/// already been renamed out of the fixture). +pub fn default_selection(groups: &[CollectionGroup]) -> Option { + let group = groups.first()?; + let collection = group.collections.first()?; + Some(Selected { + name: collection.name.clone(), + mode: collection.mode, + }) +} + #[component] pub fn Explorer() -> Element { - let selected = use_signal(|| Selected { - name: "events".to_string(), - mode: StorageMode::Document, - }); + let selected = use_signal(|| None::); let sel = selected.read().clone(); rsx! { @@ -38,32 +57,115 @@ pub fn Explorer() -> Element { div { class: "explorer", ExplorerSidebar { selected } div { class: "explorer-main", - div { class: "viewer-header", - h2 { - span { "{sel.name}" } - span { class: "sub", "{sel.mode.key()}" } - } - div { class: "viewer-actions", - button { class: "btn small", "Schema" } - button { class: "btn small", "Indexes" } - button { class: "btn small", "Export" } - button { class: "btn small primary", "+ Insert" } + if let Some(sel) = sel { + div { class: "viewer-header", + h2 { + span { "{sel.name}" } + span { class: "sub", "{sel.mode.key()}" } + } + div { class: "viewer-actions", + button { class: "btn small", "Schema" } + button { class: "btn small", "Indexes" } + button { class: "btn small", "Export" } + button { class: "btn small primary", "+ Insert" } + } } - } - div { class: "viewer-body", - match sel.mode { - StorageMode::Document => rsx! { DocumentViewer {} }, - StorageMode::Strict => rsx! { StrictViewer {} }, - StorageMode::Vector => rsx! { VectorViewer {} }, - StorageMode::Graph => rsx! { GraphViewer {} }, - StorageMode::Timeseries => rsx! { TimeseriesViewer {} }, - StorageMode::Kv => rsx! { KvViewer {} }, - StorageMode::Spatial => rsx! { SpatialViewer {} }, - StorageMode::Fts => rsx! { FtsViewer {} }, + div { class: "viewer-body", + match sel.mode { + StorageMode::Document => rsx! { DocumentViewer {} }, + StorageMode::Strict => rsx! { StrictViewer {} }, + StorageMode::Vector => rsx! { VectorViewer {} }, + StorageMode::Graph => rsx! { GraphViewer {} }, + StorageMode::Timeseries => rsx! { TimeseriesViewer {} }, + StorageMode::Kv => rsx! { KvViewer {} }, + StorageMode::Spatial => rsx! { SpatialViewer {} }, + StorageMode::Fts => rsx! { FtsViewer {} }, + } } + } else { + div { class: "async-empty", "Select a collection to view its data." } } } } } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::collection::Collection; + use crate::services::connection_service::MockConnectionService; + use crate::services::explorer_data::ExplorerData; + + #[test] + fn default_selection_is_none_for_no_groups() { + assert!(default_selection(&[]).is_none()); + } + + #[test] + fn default_selection_is_none_when_the_first_group_has_no_collections() { + let groups = [CollectionGroup { + mode: StorageMode::Document, + collections: Vec::new(), + }]; + assert!(default_selection(&groups).is_none()); + } + + #[test] + fn default_selection_is_the_first_collection_of_the_first_group() { + let groups = [ + CollectionGroup { + mode: StorageMode::Document, + collections: vec![ + Collection { + name: "users".to_string(), + mode: StorageMode::Document, + count: "1".to_string(), + }, + Collection { + name: "orders".to_string(), + mode: StorageMode::Document, + count: "2".to_string(), + }, + ], + }, + CollectionGroup { + mode: StorageMode::Vector, + collections: vec![Collection { + name: "embeddings".to_string(), + mode: StorageMode::Vector, + count: "3".to_string(), + }], + }, + ]; + let selection = default_selection(&groups).expect("groups are non-empty"); + assert_eq!(selection.name, "users"); + assert_eq!(selection.mode, StorageMode::Document); + } + + #[tokio::test] + async fn default_selection_names_a_collection_that_actually_exists() { + // Regression test: the Explorer used to hardcode its default + // selection as "events" / Document, a name that does not exist in + // `collection_groups()` — opening the Explorer showed a viewer + // header for a collection the sidebar didn't have, with no row + // highlighted. Reads the real fixture through the seam (not a + // test-local stand-in like `sidebar`'s `sample_groups()`), so a + // future fixture change can't silently reintroduce the same bug. + let svc = MockConnectionService::ready(); + let groups = svc.collection_groups().await.expect("ready yields groups"); + let selection = default_selection(&groups).expect("fixture is non-empty"); + let exists = groups.iter().any(|g| { + g.collections + .iter() + .any(|c| c.name == selection.name && c.mode == selection.mode) + }); + assert!( + exists, + "default selection {:?}/{:?} must name a real collection", + selection.name, + selection.mode.key() + ); + } +} From c7cf37a65df84674bcb26beb273cf4726da5b056 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sun, 9 Aug 2026 11:00:13 +0800 Subject: [PATCH 22/26] chore: drop internal task-id references from source and comments This repo is public; the plan that defines task/phase numbers is not, and the project's rules ban roadmap markers in source, comments, and test names. Strip the "(task-10)" suffix from every SEAM-UNWIRED marker, and reword the seam_discipline notify exception to state the substance (the seam models are missing fields the notify view renders) instead of citing a numbered review. SEAM-UNWIRED itself is kept: it says something a reader can act on (the seam is ahead of the views), and `grep -r SEAM-UNWIRED` already enumerates every site without the numeric suffix. --- nodedb-studio/src/data/mock/explorer.rs | 4 ++-- nodedb-studio/src/models/admin.rs | 12 ++++++------ nodedb-studio/src/models/explorer.rs | 4 ++-- nodedb-studio/src/models/streams.rs | 12 ++++++------ nodedb-studio/src/models/viewers.rs | 16 ++++++++-------- nodedb-studio/src/models/workbench.rs | 6 +++--- nodedb-studio/src/services/decode.rs | 6 +++--- nodedb-studio/src/services/error.rs | 2 +- nodedb-studio/src/services/explorer_data.rs | 4 ++-- nodedb-studio/src/services/streams_data.rs | 18 +++++++++--------- nodedb-studio/tests/seam_discipline.rs | 8 ++++---- 11 files changed, 46 insertions(+), 46 deletions(-) diff --git a/nodedb-studio/src/data/mock/explorer.rs b/nodedb-studio/src/data/mock/explorer.rs index 70195d6..dd586ef 100644 --- a/nodedb-studio/src/data/mock/explorer.rs +++ b/nodedb-studio/src/data/mock/explorer.rs @@ -53,7 +53,7 @@ pub fn collection_groups() -> Vec { } /// List rows for a collection. Deterministic and keyed by `id`. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn records(collection: &str) -> Vec { (0..6) .map(|i| RecordRow { @@ -68,7 +68,7 @@ pub fn records(collection: &str) -> Vec { } /// Detail body for one record. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn record_detail(collection: &str, id: &str) -> RecordDetail { RecordDetail { id: id.to_string(), diff --git a/nodedb-studio/src/models/admin.rs b/nodedb-studio/src/models/admin.rs index c020a35..306a731 100644 --- a/nodedb-studio/src/models/admin.rs +++ b/nodedb-studio/src/models/admin.rs @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize}; /// One node in the cluster topology. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ClusterNode { pub id: String, @@ -17,7 +17,7 @@ pub struct ClusterNode { } /// One Raft consensus group. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct RaftGroup { pub id: String, @@ -30,7 +30,7 @@ pub struct RaftGroup { } /// One shard range and its current leaseholder. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ShardRange { pub id: String, @@ -42,7 +42,7 @@ pub struct ShardRange { } /// One RBAC user row. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct UserRow { pub id: String, @@ -53,7 +53,7 @@ pub struct UserRow { } /// One row-level-security policy. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct RlsPolicy { pub id: String, @@ -65,7 +65,7 @@ pub struct RlsPolicy { } /// One audit-log entry. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct AuditEntry { pub id: String, diff --git a/nodedb-studio/src/models/explorer.rs b/nodedb-studio/src/models/explorer.rs index 53fdfe8..db10634 100644 --- a/nodedb-studio/src/models/explorer.rs +++ b/nodedb-studio/src/models/explorer.rs @@ -13,7 +13,7 @@ pub struct CollectionGroup { /// One row in a viewer's list pane. `cells` are pre-formatted for display and /// align with the viewer's own column headers. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct RecordRow { pub id: String, @@ -22,7 +22,7 @@ pub struct RecordRow { /// The detail panel for one record. `body_json` is display JSON produced at the /// seam, never a raw client value. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct RecordDetail { pub id: String, diff --git a/nodedb-studio/src/models/streams.rs b/nodedb-studio/src/models/streams.rs index 7d2e094..5fb5b7d 100644 --- a/nodedb-studio/src/models/streams.rs +++ b/nodedb-studio/src/models/streams.rs @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize}; /// A Studio-owned CDC consumer session. Studio never shares a consumer group: /// committing on someone else's group would advance a production consumer past /// events it never processed. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct StreamSession { pub stream: String, @@ -15,7 +15,7 @@ pub struct StreamSession { } /// One materialized view. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct MaterializedView { pub id: String, @@ -26,7 +26,7 @@ pub struct MaterializedView { } /// One durable, replayable topic. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Topic { pub id: String, @@ -39,7 +39,7 @@ pub struct Topic { } /// One cron-style scheduled job. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ScheduledJob { pub id: String, @@ -50,7 +50,7 @@ pub struct ScheduledJob { } /// One LISTEN/NOTIFY channel. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct NotifyChannel { pub id: String, @@ -59,7 +59,7 @@ pub struct NotifyChannel { } /// One message on the LISTEN/NOTIFY tail. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct NotifyMessage { pub id: String, diff --git a/nodedb-studio/src/models/viewers.rs b/nodedb-studio/src/models/viewers.rs index 6be4969..1312d54 100644 --- a/nodedb-studio/src/models/viewers.rs +++ b/nodedb-studio/src/models/viewers.rs @@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize}; /// One node in a graph viewer. `x`/`y` are a laid-out display position, not /// stored coordinates. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct GraphNode { pub id: String, @@ -20,7 +20,7 @@ pub struct GraphNode { } /// One edge in a graph viewer. `from`/`to` reference `GraphNode::id`. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct GraphEdge { pub id: String, @@ -31,7 +31,7 @@ pub struct GraphEdge { /// A graph viewer's full render input: every edge must reference a node /// present in `nodes`. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct SubGraph { pub nodes: Vec, @@ -40,7 +40,7 @@ pub struct SubGraph { /// One point in a vector viewer's projection. `x`/`y` are a 2D projection of /// the embedding, not the raw vector. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct VectorPoint { pub id: String, @@ -50,7 +50,7 @@ pub struct VectorPoint { } /// One sample in a timeseries metric. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct SeriesPoint { pub id: String, @@ -60,7 +60,7 @@ pub struct SeriesPoint { /// One feature in a spatial viewer. `geometry_json` is display GeoJSON /// produced at the seam, never a raw client value. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct SpatialFeature { pub id: String, @@ -69,7 +69,7 @@ pub struct SpatialFeature { } /// One full-text-search hit. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct FtsHit { pub id: String, @@ -78,7 +78,7 @@ pub struct FtsHit { } /// One peer in the sync/replication topology. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct SyncPeer { pub id: String, diff --git a/nodedb-studio/src/models/workbench.rs b/nodedb-studio/src/models/workbench.rs index 3dd81f9..042f1e0 100644 --- a/nodedb-studio/src/models/workbench.rs +++ b/nodedb-studio/src/models/workbench.rs @@ -7,7 +7,7 @@ use crate::models::explorer::RecordRow; /// One page of query output. Pagination is the seam's responsibility: the /// client buffers whole result sets, so the real implementation emits /// LIMIT/OFFSET rather than holding a cursor. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ResultSet { pub columns: Vec, @@ -17,14 +17,14 @@ pub struct ResultSet { } /// The query planner's EXPLAIN output for one statement. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct QueryPlan { pub text: String, } /// One node in the schema tree (database / collection / field, recursively). -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct SchemaNode { pub id: String, diff --git a/nodedb-studio/src/services/decode.rs b/nodedb-studio/src/services/decode.rs index b3953be..bebff5d 100644 --- a/nodedb-studio/src/services/decode.rs +++ b/nodedb-studio/src/services/decode.rs @@ -17,7 +17,7 @@ pub struct Table { } /// One row, addressable by column name. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub struct Row<'a> { columns: &'a [String], cells: &'a [String], @@ -25,7 +25,7 @@ pub struct Row<'a> { impl<'a> Row<'a> { /// The cell under `name`, or an error naming the column that is missing. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED pub fn field(&self, name: &str) -> Result<&'a str, StudioError> { let idx = self.columns.iter().position(|c| c == name).ok_or_else(|| { StudioError::UnexpectedColumns { @@ -48,7 +48,7 @@ impl<'a> Row<'a> { /// /// The assertion is the point: it turns the server's silent session-variable /// fallback into a typed error rather than an empty result set. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn decode_rows( table: &Table, expect: &[&str], diff --git a/nodedb-studio/src/services/error.rs b/nodedb-studio/src/services/error.rs index 593e2ef..4d246c6 100644 --- a/nodedb-studio/src/services/error.rs +++ b/nodedb-studio/src/services/error.rs @@ -34,7 +34,7 @@ pub enum StudioError { /// server answered a `SHOW` with its session-variable fallback /// (`cols=["setting"]`), which would otherwise read as an empty screen. #[error("unexpected result columns: expected [{expected}], got [{got}]")] - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED UnexpectedColumns { expected: String, got: String }, /// Connect was attempted without an explicit username. #[error("a username is required to connect")] diff --git a/nodedb-studio/src/services/explorer_data.rs b/nodedb-studio/src/services/explorer_data.rs index c4075ff..b311abc 100644 --- a/nodedb-studio/src/services/explorer_data.rs +++ b/nodedb-studio/src/services/explorer_data.rs @@ -16,11 +16,11 @@ pub trait ExplorerData { async fn collection_groups(&self) -> Result, StudioError>; /// List-pane rows for one collection. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn records(&self, collection: &str) -> Result, StudioError>; /// Detail-panel contents for one record. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn record_detail(&self, collection: &str, id: &str) -> Result; } diff --git a/nodedb-studio/src/services/streams_data.rs b/nodedb-studio/src/services/streams_data.rs index a973344..543f88b 100644 --- a/nodedb-studio/src/services/streams_data.rs +++ b/nodedb-studio/src/services/streams_data.rs @@ -23,12 +23,12 @@ pub trait StreamsData { /// Create a Studio-owned consumer group on `stream` and return the session. /// Callers must pair this with `close_stream_session`. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn open_stream_session(&self, stream: &str) -> Result; /// Read up to `limit` events from the session's current cursor. Idempotent: /// re-reading without committing returns the same events. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn cdc_batch( &self, session: &StreamSession, @@ -36,31 +36,31 @@ pub trait StreamsData { ) -> Result, StudioError>; /// Advance the session's cursor past everything read so far. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn commit_stream_offsets(&self, session: &StreamSession) -> Result<(), StudioError>; /// Drop the Studio-owned consumer group. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn close_stream_session(&self, session: &StreamSession) -> Result<(), StudioError>; /// Materialized views known to the cluster. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn materialized_views(&self) -> Result, StudioError>; /// Durable, replayable topics. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn topics(&self) -> Result, StudioError>; /// Cron-style scheduled jobs. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn scheduled_jobs(&self) -> Result, StudioError>; /// LISTEN/NOTIFY channels. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn notify_channels(&self) -> Result, StudioError>; /// The pub/sub message tail across channels. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn notify_messages(&self) -> Result, StudioError>; } diff --git a/nodedb-studio/tests/seam_discipline.rs b/nodedb-studio/tests/seam_discipline.rs index d78bdc9..f96bec5 100644 --- a/nodedb-studio/tests/seam_discipline.rs +++ b/nodedb-studio/tests/seam_discipline.rs @@ -19,10 +19,10 @@ const SCANNED_ROOTS: &[&str] = &["src/views", "src/components", "src/modals"]; /// The one documented exception. `views/streams/notify.rs` renders /// `models::streams::NotifyChannel`/`NotifyMessage`, but those seam models /// are missing the `active` and `source` fields the current notify view -/// renders (a gap flagged in Task 6's review). Rewiring notify would force a -/// UI redesign decision that is deliberately deferred rather than papered -/// over here. Remove this exception the moment notify is rewired to the -/// seam — at that point this test must go back to zero exceptions. +/// renders. Rewiring notify would force a UI redesign decision that is +/// deliberately deferred rather than papered over here. Remove this +/// exception the moment notify is rewired to the seam — at that point this +/// test must go back to zero exceptions. const ALLOWED_EXCEPTIONS: &[&str] = &["views/streams/notify.rs"]; /// Recursively collect every `.rs` file under `dir`. From 4f987bd7e3b17b3e8e19bcadeabde7dfdaa627b4 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sun, 9 Aug 2026 11:00:52 +0800 Subject: [PATCH 23/26] fix(ui): surface connect() failures instead of dropping them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three connect call sites (Connection Manager, the connection switch popover, the command palette) built the username with .unwrap_or_default() and then discarded connect()'s Err with `if let Ok(..)`. A connectable saved connection with a missing profile would default its username to blank, get rejected by the seam's MissingUsername guard, and have that error swallowed silently: a Connect button that does nothing, with no error and no state change. Match on the result at each call site and log the failure via tracing::error!. Also add a fixture invariant test asserting every connectable entry in mock::connections() has a profile, so the coincidence that hides this today breaks loudly the moment a fixture changes. While touching these call sites, drop the "(later phase)" parenthetical from their TODO comments — the sentences read fine without a reference to an unpublished plan. --- .../src/components/command_palette.rs | 20 ++++++++--- .../components/popovers/connection_popover.rs | 13 ++++--- nodedb-studio/src/data/mock/connections.rs | 35 +++++++++++++++++-- nodedb-studio/src/views/connection_manager.rs | 14 ++++---- 4 files changed, 64 insertions(+), 18 deletions(-) diff --git a/nodedb-studio/src/components/command_palette.rs b/nodedb-studio/src/components/command_palette.rs index b9da699..60ebf59 100644 --- a/nodedb-studio/src/components/command_palette.rs +++ b/nodedb-studio/src/components/command_palette.rs @@ -28,9 +28,9 @@ pub fn CommandPalette() -> Element { // each switch handler clones it. let switch_svc = service.clone(); - // TODO(later phase): the palette has no username field yet, so identity is - // taken from the saved profile rather than a user-entered value. Replace - // once the connect modal collects credentials explicitly. + // TODO: the palette has no username field yet, so identity is taken from + // the saved profile rather than a user-entered value. Replace once the + // connect modal collects credentials explicitly. let creds_for = |name: &str| -> Credentials { Credentials { username: registry @@ -89,7 +89,12 @@ pub fn CommandPalette() -> Element { let svc = svc.clone(); let creds = creds.clone(); spawn(async move { - if let Ok(s) = svc.connect("staging-cluster", &creds).await { active.set(Some(s)); } + match svc.connect("staging-cluster", &creds).await { + Ok(s) => active.set(Some(s)), + Err(e) => tracing::error!( + "connect to staging-cluster failed: {e}" + ), + } }); open.set(false); } @@ -103,7 +108,12 @@ pub fn CommandPalette() -> Element { let svc = svc.clone(); let creds = creds.clone(); spawn(async move { - if let Ok(s) = svc.connect("prod-replica-eu", &creds).await { active.set(Some(s)); } + match svc.connect("prod-replica-eu", &creds).await { + Ok(s) => active.set(Some(s)), + Err(e) => tracing::error!( + "connect to prod-replica-eu failed: {e}" + ), + } }); open.set(false); } diff --git a/nodedb-studio/src/components/popovers/connection_popover.rs b/nodedb-studio/src/components/popovers/connection_popover.rs index aa2dae3..cfe479d 100644 --- a/nodedb-studio/src/components/popovers/connection_popover.rs +++ b/nodedb-studio/src/components/popovers/connection_popover.rs @@ -50,9 +50,9 @@ pub fn ConnectionPopover() -> Element { }; let svc = service.clone(); let item_class = if disabled { "cp-item disabled" } else { "cp-item" }; - // TODO(later phase): the switch popover has no username field yet, so - // identity is taken from the saved profile rather than a user-entered - // value. Replace once the connect modal collects credentials explicitly. + // TODO: the switch popover has no username field yet, so identity is + // taken from the saved profile rather than a user-entered value. + // Replace once the connect modal collects credentials explicitly. let creds = Credentials { username: sc .profile @@ -72,7 +72,12 @@ pub fn ConnectionPopover() -> Element { let name = name.clone(); let creds = creds.clone(); spawn(async move { - if let Ok(s) = svc.connect(&name, &creds).await { active.set(Some(s)); } + match svc.connect(&name, &creds).await { + Ok(s) => active.set(Some(s)), + Err(e) => { + tracing::error!("connect to {name} failed: {e}") + } + } }); popover.set(None); } diff --git a/nodedb-studio/src/data/mock/connections.rs b/nodedb-studio/src/data/mock/connections.rs index 74ed23d..6e8b4cf 100644 --- a/nodedb-studio/src/data/mock/connections.rs +++ b/nodedb-studio/src/data/mock/connections.rs @@ -201,7 +201,7 @@ pub fn notifications() -> Vec { /// These must agree with the hardcoded literals in `components::rail` until /// that later phase swaps the rail onto this seam method, so the eventual /// wiring is a visual no-op. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn nav_badges() -> NavBadges { NavBadges { query: 3, @@ -212,7 +212,7 @@ pub fn nav_badges() -> NavBadges { /// The active session summary shown in the statusbar. `server_version` is a /// neutral "dev" placeholder: NodeDB version numbers are undecided, so no /// specific version is invented here (see the module note in `mod.rs`). -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn session_info() -> SessionInfo { SessionInfo { database: "analytics".into(), @@ -225,7 +225,7 @@ pub fn session_info() -> SessionInfo { /// Databases visible on the active connection, matching `local-nodedb-dev`'s /// profile above. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn databases() -> Vec { vec![ "analytics".into(), @@ -235,3 +235,32 @@ pub fn databases() -> Vec { "docs_corpus".into(), ] } + +#[cfg(test)] +mod tests { + use super::*; + + /// `connect()` builds its username from `profile.map(|p| p.user) + /// .unwrap_or_default()`, so a connectable entry with no profile would + /// silently produce a blank username, get rejected by the seam's + /// `MissingUsername` guard, and have that error dropped by every call + /// site's `if let Ok(..)` — a Connect button that does nothing, with no + /// error and no state change. Nothing in the types prevents that + /// combination; this test makes the fixture invariant that avoids it + /// break loudly instead. + #[test] + fn every_connectable_entry_has_a_profile() { + let conns = connections(); + assert!(!conns.is_empty(), "fixture must not be empty"); + for c in &conns { + if c.status.is_connectable() { + assert!( + c.profile.is_some(), + "{} is connectable but has no profile: connect() would \ + default its username to blank and silently no-op", + c.name + ); + } + } + } +} diff --git a/nodedb-studio/src/views/connection_manager.rs b/nodedb-studio/src/views/connection_manager.rs index 4e3c378..c035b69 100644 --- a/nodedb-studio/src/views/connection_manager.rs +++ b/nodedb-studio/src/views/connection_manager.rs @@ -49,9 +49,9 @@ pub fn ConnectionManager() -> Element { conn: conn.clone(), on_connect: { let service = service.clone(); - // TODO(later phase): the connect form has no username field - // yet, so identity is taken from the saved profile rather - // than a user-entered value. Replace once the connect modal + // TODO: the connect form has no username field yet, so + // identity is taken from the saved profile rather than a + // user-entered value. Replace once the connect modal // collects credentials explicitly. let creds = Credentials { username: conn @@ -67,10 +67,12 @@ pub fn ConnectionManager() -> Element { let service = service.clone(); let creds = creds.clone(); spawn(async move { - if let Ok(session) = service.connect(&name, &creds).await { - active.set(Some(session)); + match service.connect(&name, &creds).await { + Ok(session) => active.set(Some(session)), + Err(e) => { + tracing::error!("connect to {name} failed: {e}") + } } - // Err case (e.g. offline): surfaced in a later wiring phase. }); } }, From f877482b3bcd894106260bcc281e0ac5108b7d49 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sun, 9 Aug 2026 11:01:33 +0800 Subject: [PATCH 24/26] fix(data): give admin fixtures id/name pairs distinct like streams does data/mock/streams.rs establishes the convention that a fixture's id must differ from its display name, with an assert_ids_distinct_from_names helper precisely so a list keyed by the wrong field is visible instead of invisible. data/mock/admin.rs's users() and rls_policies() violated that same convention (id == username / id == name), on this same branch. Give both fixtures distinct id/name pairs, move the shared invariant helpers into a new test_support module so streams and admin tests can both reuse them, and update admin_data's fixture-content tests to look users and policies up by username/name instead of id now that the two differ. --- nodedb-studio/src/data/mock/admin.rs | 48 +++++++++++++++------ nodedb-studio/src/data/mock/mod.rs | 2 + nodedb-studio/src/data/mock/streams.rs | 32 +++----------- nodedb-studio/src/data/mock/test_support.rs | 26 +++++++++++ nodedb-studio/src/services/admin_data.rs | 20 ++++----- 5 files changed, 80 insertions(+), 48 deletions(-) create mode 100644 nodedb-studio/src/data/mock/test_support.rs diff --git a/nodedb-studio/src/data/mock/admin.rs b/nodedb-studio/src/data/mock/admin.rs index 1b1f8c1..13c6f51 100644 --- a/nodedb-studio/src/data/mock/admin.rs +++ b/nodedb-studio/src/data/mock/admin.rs @@ -4,7 +4,7 @@ use crate::models::admin::{AuditEntry, ClusterNode, RaftGroup, RlsPolicy, ShardRange, UserRow}; /// Cluster topology: one row per node. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn cluster_nodes() -> Vec { vec![ ClusterNode { @@ -29,7 +29,7 @@ pub fn cluster_nodes() -> Vec { } /// Raft groups for the cluster. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn raft_groups() -> Vec { (0..4) .map(|i| RaftGroup { @@ -45,7 +45,7 @@ pub fn raft_groups() -> Vec { } /// Shard ranges and their leaseholders. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn shard_ranges() -> Vec { (0..8) .map(|i| ShardRange { @@ -59,19 +59,22 @@ pub fn shard_ranges() -> Vec { .collect() } -/// RBAC: all users in the tenant. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +/// RBAC: all users in the tenant. `id` deliberately differs from `username` +/// (a `u-N` handle vs. the login name), same reasoning as +/// `streams::materialized_views`: a list keyed by the wrong field must be +/// visible instead of invisible. +#[allow(dead_code)] // SEAM-UNWIRED pub fn users() -> Vec { vec![ UserRow { - id: "admin".into(), + id: "u-1".into(), username: "admin".into(), tenant_id: "1".into(), roles: "superuser".into(), is_superuser: true, }, UserRow { - id: "alice".into(), + id: "u-2".into(), username: "alice".into(), tenant_id: "1".into(), roles: "reader".into(), @@ -80,12 +83,13 @@ pub fn users() -> Vec { ] } -/// Row-level-security policies. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +/// Row-level-security policies. `id` deliberately differs from `name`, same +/// reasoning as `users` above. +#[allow(dead_code)] // SEAM-UNWIRED pub fn rls_policies() -> Vec { vec![ RlsPolicy { - id: "tenant_isolation".into(), + id: "rls-1".into(), name: "tenant_isolation".into(), collection: "orders".into(), kind: "select".into(), @@ -93,7 +97,7 @@ pub fn rls_policies() -> Vec { enabled: true, }, RlsPolicy { - id: "pii_masking".into(), + id: "rls-2".into(), name: "pii_masking".into(), collection: "users".into(), kind: "select".into(), @@ -104,7 +108,7 @@ pub fn rls_policies() -> Vec { } /// Audit log entries. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn audit_entries() -> Vec { (0..5) .map(|i| AuditEntry { @@ -117,3 +121,23 @@ pub fn audit_entries() -> Vec { }) .collect() } + +#[cfg(test)] +mod tests { + use super::*; + use crate::data::mock::test_support::{assert_ids_distinct_from_names, assert_unique_ids}; + + #[test] + fn users_have_unique_ids_distinct_from_username() { + let rows = users(); + assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); + assert_ids_distinct_from_names(rows.iter().map(|r| (r.id.as_str(), r.username.as_str()))); + } + + #[test] + fn rls_policies_have_unique_ids_distinct_from_name() { + let rows = rls_policies(); + assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); + assert_ids_distinct_from_names(rows.iter().map(|r| (r.id.as_str(), r.name.as_str()))); + } +} diff --git a/nodedb-studio/src/data/mock/mod.rs b/nodedb-studio/src/data/mock/mod.rs index 7146e45..f56fdc9 100644 --- a/nodedb-studio/src/data/mock/mod.rs +++ b/nodedb-studio/src/data/mock/mod.rs @@ -13,6 +13,8 @@ mod docs; mod explorer; mod notify; mod streams; +#[cfg(test)] +mod test_support; mod viewers; mod workbench; diff --git a/nodedb-studio/src/data/mock/streams.rs b/nodedb-studio/src/data/mock/streams.rs index 0b8f365..9d26dd3 100644 --- a/nodedb-studio/src/data/mock/streams.rs +++ b/nodedb-studio/src/data/mock/streams.rs @@ -15,7 +15,7 @@ use crate::models::streams::{MaterializedView, NotifyChannel, NotifyMessage, Sch /// Materialized views known to the cluster. `id` deliberately differs from /// `name` (a `mv-N` handle vs. the human-readable view name) so a later bug /// that keys a list by the wrong field is visible instead of invisible. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn materialized_views() -> Vec { vec![ MaterializedView { @@ -37,7 +37,7 @@ pub fn materialized_views() -> Vec { /// Durable, replayable topics with their consumer lag. `id` deliberately /// differs from `name`, same reasoning as `materialized_views`. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn topics() -> Vec { vec![ Topic { @@ -74,7 +74,7 @@ pub fn topics() -> Vec { /// successful ones so fixture-content tests can't be satisfied by an /// accidentally-uniform status column. `id` deliberately differs from `name`, /// same reasoning as `materialized_views`. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn scheduled_jobs() -> Vec { vec![ ScheduledJob { @@ -103,7 +103,7 @@ pub fn scheduled_jobs() -> Vec { /// LISTEN/NOTIFY channels. `id` deliberately differs from `name`, same /// reasoning as `materialized_views`. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn notify_channel_rows() -> Vec { vec![ NotifyChannel { @@ -125,7 +125,7 @@ pub fn notify_channel_rows() -> Vec { } /// The pub/sub message tail across channels. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn notify_message_rows() -> Vec { (0..4) .map(|i| NotifyMessage { @@ -140,6 +140,7 @@ pub fn notify_message_rows() -> Vec { #[cfg(test)] mod tests { use super::*; + use crate::data::mock::test_support::{assert_ids_distinct_from_names, assert_unique_ids}; #[test] fn materialized_views_have_unique_ids_distinct_from_name() { @@ -176,25 +177,4 @@ mod tests { let rows = notify_message_rows(); assert_unique_ids(&rows.iter().map(|r| r.id.as_str()).collect::>()); } - - fn assert_unique_ids(ids: &[&str]) { - assert!(!ids.is_empty(), "fixture must not be empty"); - let mut sorted = ids.to_vec(); - sorted.sort_unstable(); - sorted.dedup(); - assert_eq!(sorted.len(), ids.len(), "ids must be unique"); - } - - /// A list keyed by the wrong field (e.g. `name` instead of `id`) still - /// passes `assert_unique_ids` when the fixture happens to have `id == - /// name`, so that mistake stays invisible. Fixtures here deliberately - /// give every row a distinct `id`/`name` pair to make it visible. - fn assert_ids_distinct_from_names<'a>(rows: impl Iterator) { - let mut saw_any = false; - for (id, name) in rows { - saw_any = true; - assert_ne!(id, name, "id must not equal name: {id}"); - } - assert!(saw_any, "fixture must not be empty"); - } } diff --git a/nodedb-studio/src/data/mock/test_support.rs b/nodedb-studio/src/data/mock/test_support.rs new file mode 100644 index 0000000..2eafa87 --- /dev/null +++ b/nodedb-studio/src/data/mock/test_support.rs @@ -0,0 +1,26 @@ +//! Shared fixture-invariant helpers used by more than one mock module's +//! tests. Test-only: this whole module is behind `#[cfg(test)]` at the +//! declaration site in `mod.rs`. + +/// Every id in the fixture must be unique — a list keyed by a duplicate id +/// would silently overwrite one row with another in the UI. +pub(crate) fn assert_unique_ids(ids: &[&str]) { + assert!(!ids.is_empty(), "fixture must not be empty"); + let mut sorted = ids.to_vec(); + sorted.sort_unstable(); + sorted.dedup(); + assert_eq!(sorted.len(), ids.len(), "ids must be unique"); +} + +/// A list keyed by the wrong field (e.g. `name` instead of `id`) still +/// passes `assert_unique_ids` when the fixture happens to have `id == name`, +/// so that mistake stays invisible. Fixtures should give every row a +/// distinct `id`/`name` pair to make it visible. +pub(crate) fn assert_ids_distinct_from_names<'a>(rows: impl Iterator) { + let mut saw_any = false; + for (id, name) in rows { + saw_any = true; + assert_ne!(id, name, "id must not equal name: {id}"); + } + assert!(saw_any, "fixture must not be empty"); +} diff --git a/nodedb-studio/src/services/admin_data.rs b/nodedb-studio/src/services/admin_data.rs index 1c19a57..8df6592 100644 --- a/nodedb-studio/src/services/admin_data.rs +++ b/nodedb-studio/src/services/admin_data.rs @@ -10,27 +10,27 @@ use crate::services::error::StudioError; #[async_trait(?Send)] pub trait AdminData { /// Cluster topology: one row per node. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn cluster_nodes(&self) -> Result, StudioError>; /// Raft groups for the cluster. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn raft_groups(&self) -> Result, StudioError>; /// Shard ranges and their leaseholders. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn shard_ranges(&self) -> Result, StudioError>; /// RBAC: all users in the tenant. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn users(&self) -> Result, StudioError>; /// Row-level-security policies. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn rls_policies(&self) -> Result, StudioError>; /// Audit log entries. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn audit_entries(&self) -> Result, StudioError>; } @@ -162,11 +162,11 @@ mod tests { let users = svc.users().await.expect("users"); let admin = users .iter() - .find(|u| u.id == "admin") + .find(|u| u.username == "admin") .expect("fixture has an `admin` user"); let alice = users .iter() - .find(|u| u.id == "alice") + .find(|u| u.username == "alice") .expect("fixture has an `alice` user"); assert!(admin.is_superuser, "admin fixture must be a superuser"); assert!(!alice.is_superuser, "alice fixture must not be a superuser"); @@ -178,11 +178,11 @@ mod tests { let policies = svc.rls_policies().await.expect("rls"); let tenant_isolation = policies .iter() - .find(|p| p.id == "tenant_isolation") + .find(|p| p.name == "tenant_isolation") .expect("fixture has a `tenant_isolation` policy"); let pii_masking = policies .iter() - .find(|p| p.id == "pii_masking") + .find(|p| p.name == "pii_masking") .expect("fixture has a `pii_masking` policy"); assert!( tenant_isolation.enabled, From 12050881c40d41d2e1b89e1580dfd72090a70bb1 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sun, 9 Aug 2026 11:03:23 +0800 Subject: [PATCH 25/26] fix(services): reach AsyncState::Empty for query and graph reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ResultSet and SubGraph's IsEmpty impls returned a hardcoded false, on the reasoning that "a fetched single value is never empty" — true for SessionInfo/NavBadges/RecordDetail/QueryPlan, false for these two: a zero-row result set and a zero-node graph are the empty case, and "no rows" is the most common non-error workbench outcome. The Query workbench and graph viewer could never reach AsyncState::Empty, so a zero-row query rendered as a blank pane indistinguishable from a broken one. Make both impls honest (rows.is_empty() / nodes.is_empty()), and give run_query/sub_graph a way to actually deliver that empty payload: add apply_one_or_empty to mock_behavior (like apply_one, but Empty calls a caller-supplied empty thunk instead of folding into Ready), and add empty_result_set/empty_sub_graph fixtures. The four single-valued models keep folding Empty into Ready via apply_one, unchanged. Split the async_state test that pinned ResultSet/SubGraph into the buggy `false` behaviour: the four genuinely single-valued models keep their Loaded-not-Empty assertion, while ResultSet/SubGraph get the opposite (empty payload -> Empty, non-empty -> Loaded). Also parameterize ViewersData's sub_graph, vector_points, spatial_features and fts_hits with a collection (fts_hits also gains it alongside its existing query param), matching the pattern records() already uses — the Explorer already scopes graph/vector/spatial collections by selection, so wiring the per-mode viewers later would otherwise be a signature change across four methods and both implementors. sync_peers stays instance-scoped. Mock fixtures now key every id off the requested collection so a wrong-argument bug is visible, with tests asserting the variation. Point the shell.rs and viewers_data.rs module docs at apply_one / apply_one_or_empty instead of a nonexistent "explicit four-arm match" — every seam method here already went through apply_one. --- nodedb-studio/src/data/mock/mod.rs | 6 +- nodedb-studio/src/data/mock/viewers.rs | 82 +++++---- nodedb-studio/src/data/mock/workbench.rs | 21 ++- nodedb-studio/src/models/shell.rs | 8 +- nodedb-studio/src/services/async_state.rs | 86 +++++++-- .../src/services/connection_service.rs | 45 +++-- nodedb-studio/src/services/mock_behavior.rs | 68 +++++++- nodedb-studio/src/services/nodedb_service.rs | 19 +- nodedb-studio/src/services/viewers_data.rs | 164 +++++++++++++----- nodedb-studio/src/services/workbench_data.rs | 27 ++- 10 files changed, 388 insertions(+), 138 deletions(-) diff --git a/nodedb-studio/src/data/mock/mod.rs b/nodedb-studio/src/data/mock/mod.rs index f56fdc9..5ff3d23 100644 --- a/nodedb-studio/src/data/mock/mod.rs +++ b/nodedb-studio/src/data/mock/mod.rs @@ -26,5 +26,7 @@ pub use notify::{notify_channels, notify_messages}; pub use streams::{ materialized_views, notify_channel_rows, notify_message_rows, scheduled_jobs, topics, }; -pub use viewers::{fts_hits, series, spatial_features, sub_graph, sync_peers, vector_points}; -pub use workbench::{query_plan, result_set, schema_tree}; +pub use viewers::{ + empty_sub_graph, fts_hits, series, spatial_features, sub_graph, sync_peers, vector_points, +}; +pub use workbench::{empty_result_set, query_plan, result_set, schema_tree}; diff --git a/nodedb-studio/src/data/mock/viewers.rs b/nodedb-studio/src/data/mock/viewers.rs index 1d535c8..52d7303 100644 --- a/nodedb-studio/src/data/mock/viewers.rs +++ b/nodedb-studio/src/data/mock/viewers.rs @@ -9,52 +9,68 @@ use crate::models::viewers::{ FtsHit, GraphEdge, GraphNode, SeriesPoint, SpatialFeature, SubGraph, SyncPeer, VectorPoint, }; -/// A small connected graph: every edge references a node present in `nodes`. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) -pub fn sub_graph() -> SubGraph { +/// A small connected graph for `collection`: every edge references a node +/// present in `nodes`. Ids and labels are keyed off `collection` so a caller +/// that passes the wrong collection produces visibly different data. +#[allow(dead_code)] // SEAM-UNWIRED +pub fn sub_graph(collection: &str) -> SubGraph { let nodes = vec![ GraphNode { - id: "n1".into(), - label: "alice".into(), + id: format!("{collection}-n1"), + label: format!("{collection}-alice"), x: 0.0, y: 0.0, }, GraphNode { - id: "n2".into(), - label: "bob".into(), + id: format!("{collection}-n2"), + label: format!("{collection}-bob"), x: 1.0, y: 0.5, }, GraphNode { - id: "n3".into(), - label: "carol".into(), + id: format!("{collection}-n3"), + label: format!("{collection}-carol"), x: 2.0, y: 1.0, }, ]; let edges = vec![ GraphEdge { - id: "e1".into(), - from: "n1".into(), - to: "n2".into(), + id: format!("{collection}-e1"), + from: format!("{collection}-n1"), + to: format!("{collection}-n2"), label: "follows".into(), }, GraphEdge { - id: "e2".into(), - from: "n2".into(), - to: "n3".into(), + id: format!("{collection}-e2"), + from: format!("{collection}-n2"), + to: format!("{collection}-n3"), label: "follows".into(), }, ]; SubGraph { nodes, edges } } -/// A 2D projection of embeddings, grouped into a couple of clusters. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) -pub fn vector_points() -> Vec { +/// The genuinely-empty graph `MockBehavior::Empty` returns for `sub_graph`: a +/// collection with zero nodes is a real graph-viewer outcome, not an absent +/// value, so unlike `record_detail` this must not fold into `sub_graph`'s +/// fixture nodes. +#[allow(dead_code)] // SEAM-UNWIRED +pub fn empty_sub_graph() -> SubGraph { + SubGraph { + nodes: Vec::new(), + edges: Vec::new(), + } +} + +/// A 2D projection of embeddings for `collection`, grouped into a couple of +/// clusters. Ids are keyed off `collection` so a caller that passes the +/// wrong collection produces visibly different data. +#[allow(dead_code)] // SEAM-UNWIRED +pub fn vector_points(collection: &str) -> Vec { (0..6) .map(|i| VectorPoint { - id: format!("vec-{i}"), + id: format!("{collection}-vec-{i}"), x: i as f32 * 0.3, y: (i % 3) as f32 * 0.7, cluster: if i % 2 == 0 { "a" } else { "b" }.into(), @@ -63,7 +79,7 @@ pub fn vector_points() -> Vec { } /// Samples for one timeseries metric. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn series(metric: &str) -> Vec { (0..8) .map(|i| SeriesPoint { @@ -74,37 +90,41 @@ pub fn series(metric: &str) -> Vec { .collect() } -/// Spatial features with placeholder GeoJSON geometry. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) -pub fn spatial_features() -> Vec { +/// Spatial features for `collection`, with placeholder GeoJSON geometry. Ids +/// are keyed off `collection` so a caller that passes the wrong collection +/// produces visibly different data. +#[allow(dead_code)] // SEAM-UNWIRED +pub fn spatial_features(collection: &str) -> Vec { vec![ SpatialFeature { - id: "kl-tower".into(), + id: format!("{collection}-kl-tower"), name: "KL Tower".into(), geometry_json: r#"{"type":"Point","coordinates":[101.7038,3.1528]}"#.into(), }, SpatialFeature { - id: "petronas".into(), + id: format!("{collection}-petronas"), name: "Petronas Towers".into(), geometry_json: r#"{"type":"Point","coordinates":[101.7119,3.1579]}"#.into(), }, ] } -/// Full-text-search hits for `query`. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) -pub fn fts_hits(query: &str) -> Vec { +/// Full-text-search hits for `query` within `collection`. Ids are keyed off +/// `collection` so a caller that passes the wrong collection produces +/// visibly different data. +#[allow(dead_code)] // SEAM-UNWIRED +pub fn fts_hits(collection: &str, query: &str) -> Vec { (0..3) .map(|i| FtsHit { - id: format!("hit-{i}"), - excerpt: format!("...an excerpt mentioning {query}..."), + id: format!("{collection}-hit-{i}"), + excerpt: format!("...an excerpt in {collection} mentioning {query}..."), score: format!("0.{}", 9 - i), }) .collect() } /// Sync/replication peers. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn sync_peers() -> Vec { vec![ SyncPeer { diff --git a/nodedb-studio/src/data/mock/workbench.rs b/nodedb-studio/src/data/mock/workbench.rs index ac40faf..bbab4db 100644 --- a/nodedb-studio/src/data/mock/workbench.rs +++ b/nodedb-studio/src/data/mock/workbench.rs @@ -7,7 +7,7 @@ use crate::models::workbench::{QueryPlan, ResultSet, SchemaNode}; /// A deterministic 3-column, 4-row result set. Every query text answers with /// the same shape today; the real implementation decodes whatever the server /// returned for `sql`. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn result_set(sql: &str) -> ResultSet { ResultSet { columns: vec!["id".into(), "name".into(), "created_at".into()], @@ -26,8 +26,23 @@ pub fn result_set(sql: &str) -> ResultSet { } } +/// The genuinely-empty result set `MockBehavior::Empty` returns for +/// `run_query`: zero rows is the most common non-error query outcome, so +/// unlike `record_detail`/`explain` this must not fold into `result_set`'s +/// fixture rows. Columns are kept (a real zero-row result still has a shape) +/// so the empty state is distinguishable from an absent one. +#[allow(dead_code)] // SEAM-UNWIRED +pub fn empty_result_set(sql: &str) -> ResultSet { + ResultSet { + columns: vec!["id".into(), "name".into(), "created_at".into()], + rows: Vec::new(), + elapsed_ms: 3, + scanned: format!("0 rows for `{sql}`"), + } +} + /// A short, deterministic EXPLAIN plan for `sql`. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn query_plan(sql: &str) -> QueryPlan { QueryPlan { text: format!("Seq Scan on users (cost=0.00..1.04 rows=4)\n -- {sql}"), @@ -36,7 +51,7 @@ pub fn query_plan(sql: &str) -> QueryPlan { /// A two-level schema tree (database -> collections -> fields) with /// path-like ids, so uniqueness is structural rather than accidental. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED pub fn schema_tree() -> Vec { vec![SchemaNode { id: "db".into(), diff --git a/nodedb-studio/src/models/shell.rs b/nodedb-studio/src/models/shell.rs index 7262bb1..308fd86 100644 --- a/nodedb-studio/src/models/shell.rs +++ b/nodedb-studio/src/models/shell.rs @@ -1,12 +1,12 @@ //! Shell chrome models: nav-rail badge counts and the statusbar's session //! summary. Both are single values (not lists), which is why the mock impl -//! cannot use `services::mock_behavior::apply` and instead matches -//! `MockBehavior` explicitly, the same way `record_detail` and `run_query` do. +//! cannot use `services::mock_behavior::apply` and instead uses +//! `apply_one`, the single-value counterpart `record_detail` also uses. use serde::{Deserialize, Serialize}; /// Badge counts shown on the nav rail's Query and Streams entries. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub struct NavBadges { pub query: u32, @@ -14,7 +14,7 @@ pub struct NavBadges { } /// The active session summary shown in the statusbar. -#[allow(dead_code)] // SEAM-UNWIRED(task-10) +#[allow(dead_code)] // SEAM-UNWIRED #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct SessionInfo { pub database: String, diff --git a/nodedb-studio/src/services/async_state.rs b/nodedb-studio/src/services/async_state.rs index c886ef3..d041846 100644 --- a/nodedb-studio/src/services/async_state.rs +++ b/nodedb-studio/src/services/async_state.rs @@ -24,10 +24,10 @@ impl IsEmpty for Vec { // Single-value seam reads have no "empty" shape: a fetched value is never // "empty", it either arrived or it errored (`MockBehavior::Empty` folds into -// `Ready` for these — see `record_detail`/`run_query` precedent). Each impl is -// listed explicitly, deliberately not a blanket `impl IsEmpty for T`, so a -// future single-value model must opt in here rather than silently inheriting -// a meaning that may not fit it. +// `Ready` for these — see `record_detail`'s precedent). Each impl is listed +// explicitly, deliberately not a blanket `impl IsEmpty for T`, so a future +// single-value model must opt in here rather than silently inheriting a +// meaning that may not fit it. impl IsEmpty for SessionInfo { fn is_empty(&self) -> bool { false @@ -46,21 +46,29 @@ impl IsEmpty for RecordDetail { } } -impl IsEmpty for ResultSet { +impl IsEmpty for QueryPlan { fn is_empty(&self) -> bool { false } } -impl IsEmpty for QueryPlan { +// `ResultSet` and `SubGraph` are also single fetched values, but unlike the +// four above they wrap a list (rows / nodes) whose emptiness IS a real, +// common outcome — "your query returned no rows" for a workbench, or "this +// collection has no nodes" for a graph viewer. Folding `MockBehavior::Empty` +// into `Ready` for these would make `AsyncState::Empty` unreachable for the +// query and graph panes, so they report their own emptiness instead of the +// blanket `false` above (see `apply_one_or_empty` in `mock_behavior`, which +// gives the mock seam methods a way to actually deliver an empty payload). +impl IsEmpty for ResultSet { fn is_empty(&self) -> bool { - false + self.rows.is_empty() } } impl IsEmpty for SubGraph { fn is_empty(&self) -> bool { - false + self.nodes.is_empty() } } @@ -174,6 +182,8 @@ impl AsyncState { #[cfg(test)] mod tests { use super::*; + use crate::models::explorer::RecordRow; + use crate::models::viewers::GraphNode; #[test] fn async_state_none_is_loading() { @@ -265,10 +275,11 @@ mod tests { #[test] fn single_value_seam_models_are_loaded_not_empty() { - // Before their `IsEmpty` impls existed, these six single-value seam + // Before their `IsEmpty` impls existed, these four single-value seam // models could not satisfy `AsyncState::from_value`'s `T: IsEmpty` // bound at all, so they could never be rendered through `AsyncView`. - // Each must map a fetched value straight to `Loaded`, never `Empty`. + // Each must map a fetched value straight to `Loaded`, never `Empty`: + // none of the four has a meaningful "empty" shape. let session_info = AsyncState::from_value(Some(Ok(SessionInfo { database: String::new(), role: String::new(), @@ -292,23 +303,64 @@ mod tests { }))); assert!(matches!(record_detail, AsyncState::Loaded(_))); + let query_plan = AsyncState::from_value(Some(Ok(QueryPlan { + text: String::new(), + }))); + assert!(matches!(query_plan, AsyncState::Loaded(_))); + } + + #[test] + fn zero_row_result_set_is_empty() { + // Unlike the four single-value models above, `ResultSet` wraps a + // list: a query that returns zero rows is the most common + // non-error workbench outcome and must reach `AsyncState::Empty`, + // not `Loaded` with an empty table. let result_set = AsyncState::from_value(Some(Ok(ResultSet { - columns: Vec::new(), + columns: vec!["id".to_string()], rows: Vec::new(), - elapsed_ms: 0, - scanned: String::new(), + elapsed_ms: 3, + scanned: "0 rows".to_string(), }))); - assert!(matches!(result_set, AsyncState::Loaded(_))); + assert!(matches!(result_set, AsyncState::Empty)); + } - let query_plan = AsyncState::from_value(Some(Ok(QueryPlan { - text: String::new(), + #[test] + fn non_empty_result_set_is_loaded() { + let result_set = AsyncState::from_value(Some(Ok(ResultSet { + columns: vec!["id".to_string()], + rows: vec![RecordRow { + id: "1".to_string(), + cells: vec!["1".to_string()], + }], + elapsed_ms: 3, + scanned: "1 row".to_string(), }))); - assert!(matches!(query_plan, AsyncState::Loaded(_))); + assert!(matches!(result_set, AsyncState::Loaded(_))); + } + #[test] + fn zero_node_sub_graph_is_empty() { + // A collection with no nodes is a real outcome for the graph viewer + // and must reach `AsyncState::Empty`, not `Loaded` with an empty + // graph. let sub_graph = AsyncState::from_value(Some(Ok(SubGraph { nodes: Vec::new(), edges: Vec::new(), }))); + assert!(matches!(sub_graph, AsyncState::Empty)); + } + + #[test] + fn non_empty_sub_graph_is_loaded() { + let sub_graph = AsyncState::from_value(Some(Ok(SubGraph { + nodes: vec![GraphNode { + id: "n1".to_string(), + label: "alice".to_string(), + x: 0.0, + y: 0.0, + }], + edges: Vec::new(), + }))); assert!(matches!(sub_graph, AsyncState::Loaded(_))); } diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index 951fe2c..4515032 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -27,7 +27,7 @@ use crate::models::workbench::{QueryPlan, ResultSet, SchemaNode}; use crate::services::admin_data::AdminData; use crate::services::error::StudioError; use crate::services::explorer_data::ExplorerData; -use crate::services::mock_behavior::{MockBehavior, apply, apply_one}; +use crate::services::mock_behavior::{MockBehavior, apply, apply_one, apply_one_or_empty}; use crate::services::streams_data::{StreamsData, cdc_rows_from_mock}; use crate::services::viewers_data::ViewersData; use crate::services::workbench_data::WorkbenchData; @@ -62,15 +62,15 @@ pub trait ConnectionService { async fn mark_all_read(&self) -> Result<(), StudioError>; /// Badge counts for the nav rail's Query and Streams entries. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn nav_badges(&self) -> Result; /// The active session summary shown in the statusbar. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn session_info(&self) -> Result; /// All databases visible on the active connection. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn databases(&self) -> Result, StudioError>; } @@ -329,7 +329,13 @@ impl AdminData for MockConnectionService { impl WorkbenchData for MockConnectionService { async fn run_query(&self, sql: &str) -> Result { let s = sql.to_string(); - apply_one(self.behavior, move || mock::result_set(&s)).await + let s2 = s.clone(); + apply_one_or_empty( + self.behavior, + move || mock::result_set(&s), + move || mock::empty_result_set(&s2), + ) + .await } async fn explain(&self, sql: &str) -> Result { @@ -344,12 +350,19 @@ impl WorkbenchData for MockConnectionService { #[async_trait(?Send)] impl ViewersData for MockConnectionService { - async fn sub_graph(&self) -> Result { - apply_one(self.behavior, mock::sub_graph).await + async fn sub_graph(&self, collection: &str) -> Result { + let c = collection.to_string(); + apply_one_or_empty( + self.behavior, + move || mock::sub_graph(&c), + mock::empty_sub_graph, + ) + .await } - async fn vector_points(&self) -> Result, StudioError> { - apply(self.behavior, mock::vector_points).await + async fn vector_points(&self, collection: &str) -> Result, StudioError> { + let c = collection.to_string(); + apply(self.behavior, move || mock::vector_points(&c)).await } async fn series(&self, metric: &str) -> Result, StudioError> { @@ -357,13 +370,15 @@ impl ViewersData for MockConnectionService { apply(self.behavior, move || mock::series(&m)).await } - async fn spatial_features(&self) -> Result, StudioError> { - apply(self.behavior, mock::spatial_features).await + async fn spatial_features(&self, collection: &str) -> Result, StudioError> { + let c = collection.to_string(); + apply(self.behavior, move || mock::spatial_features(&c)).await } - async fn fts_hits(&self, query: &str) -> Result, StudioError> { + async fn fts_hits(&self, collection: &str, query: &str) -> Result, StudioError> { + let c = collection.to_string(); let q = query.to_string(); - apply(self.behavior, move || mock::fts_hits(&q)).await + apply(self.behavior, move || mock::fts_hits(&c, &q)).await } async fn sync_peers(&self) -> Result, StudioError> { @@ -684,8 +699,8 @@ mod tests { // Verifies the two-step transition pattern: set Loading, await the read, // set Loaded from the result. This test has no Dioxus signals/guards (unit-level), // so it only demonstrates the transition logic; the actual no-guard-across-await - // discipline is exercised by the real streaming view (later task) and enforced - // by the AGENTS.md convention. + // discipline is exercised by real streaming views and enforced by the + // AGENTS.md convention. #[tokio::test] async fn delayed_read_transitions_loading_to_loaded_without_guard() { let svc = MockConnectionService::ready(); diff --git a/nodedb-studio/src/services/mock_behavior.rs b/nodedb-studio/src/services/mock_behavior.rs index 5b720be..87301b9 100644 --- a/nodedb-studio/src/services/mock_behavior.rs +++ b/nodedb-studio/src/services/mock_behavior.rs @@ -36,10 +36,11 @@ pub async fn apply( } /// Apply the behaviour to a single-value fixture thunk (used by seam methods -/// that read one value rather than a list — `session_info`, `nav_badges`, -/// `record_detail`, `run_query`, `explain`, `sub_graph`). `Ready` and `Empty` -/// both call the thunk: a fetched single value has no "empty" shape, so -/// `Empty` folds into `Ready` here rather than inventing an absent value. +/// that read one value with no "empty" shape at all — `session_info`, +/// `nav_badges`, `record_detail`, `explain`). `Ready` and `Empty` both call +/// the thunk: a fetched single value here is never "empty", it either +/// arrived or it errored, so `Empty` folds into `Ready` rather than +/// inventing an absent value. pub async fn apply_one( behavior: MockBehavior, ready: impl FnOnce() -> T, @@ -56,6 +57,29 @@ pub async fn apply_one( } } +/// Apply the behaviour to a single-value fixture thunk whose value can +/// itself be "empty" (used by `run_query` and `sub_graph`: a zero-row result +/// set or a zero-node graph is a real, common outcome, not an absent value). +/// Unlike `apply_one`, `Empty` calls `empty` rather than `ready`, so the +/// caller controls exactly what the empty shape looks like. +pub async fn apply_one_or_empty( + behavior: MockBehavior, + ready: impl FnOnce() -> T, + empty: impl FnOnce() -> T, +) -> Result { + match behavior { + MockBehavior::Ready => Ok(ready()), + MockBehavior::Empty => Ok(empty()), + MockBehavior::Erroring => Err(StudioError::from( + nodedb_client::NodeDbError::node_unreachable("mock"), + )), + MockBehavior::Delayed(d) => { + tokio::time::sleep(d).await; + Ok(ready()) + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -120,4 +144,40 @@ mod tests { let out = apply_one(MockBehavior::Delayed(Duration::from_millis(5)), || 9u8).await; assert_eq!(out.expect("delayed yields data"), 9u8); } + + #[tokio::test] + async fn apply_one_or_empty_ready_returns_the_fixture() { + let out = apply_one_or_empty(MockBehavior::Ready, || 7u8, || 0u8).await; + assert_eq!(out.expect("ready yields data"), 7u8); + } + + #[tokio::test] + async fn apply_one_or_empty_empty_returns_the_empty_thunk() { + // Unlike `apply_one`, `Empty` does NOT fold into `Ready` here: the + // caller's `empty` thunk runs instead, so a genuinely empty payload + // is reachable for single-value reads that have a real empty shape. + let out = apply_one_or_empty(MockBehavior::Empty, || 7u8, || 0u8).await; + assert_eq!(out.expect("empty yields the empty thunk"), 0u8); + } + + #[tokio::test] + async fn apply_one_or_empty_erroring_returns_a_retriable_error() { + let out = apply_one_or_empty(MockBehavior::Erroring, || 7u8, || 0u8).await; + let err = out.expect_err("erroring yields Err"); + assert!( + err.is_retriable(), + "demo error must exercise the retry path" + ); + } + + #[tokio::test] + async fn apply_one_or_empty_delayed_still_returns_the_fixture() { + let out = apply_one_or_empty( + MockBehavior::Delayed(Duration::from_millis(5)), + || 9u8, + || 0u8, + ) + .await; + assert_eq!(out.expect("delayed yields data"), 9u8); + } } diff --git a/nodedb-studio/src/services/nodedb_service.rs b/nodedb-studio/src/services/nodedb_service.rs index bb8dc8e..60638f3 100644 --- a/nodedb-studio/src/services/nodedb_service.rs +++ b/nodedb-studio/src/services/nodedb_service.rs @@ -183,11 +183,11 @@ impl WorkbenchData for NodeDbConnectionService { #[async_trait(?Send)] impl ViewersData for NodeDbConnectionService { - async fn sub_graph(&self) -> Result { + async fn sub_graph(&self, _collection: &str) -> Result { Err(StudioError::NotConnected) } - async fn vector_points(&self) -> Result, StudioError> { + async fn vector_points(&self, _collection: &str) -> Result, StudioError> { Err(StudioError::NotConnected) } @@ -195,11 +195,14 @@ impl ViewersData for NodeDbConnectionService { Err(StudioError::NotConnected) } - async fn spatial_features(&self) -> Result, StudioError> { + async fn spatial_features( + &self, + _collection: &str, + ) -> Result, StudioError> { Err(StudioError::NotConnected) } - async fn fts_hits(&self, _query: &str) -> Result, StudioError> { + async fn fts_hits(&self, _collection: &str, _query: &str) -> Result, StudioError> { Err(StudioError::NotConnected) } @@ -319,11 +322,11 @@ mod tests { async fn stub_viewer_reads_are_not_connected() { let svc = NodeDbConnectionService; assert!(matches!( - svc.sub_graph().await, + svc.sub_graph("social").await, Err(StudioError::NotConnected) )); assert!(matches!( - svc.vector_points().await, + svc.vector_points("embeddings").await, Err(StudioError::NotConnected) )); assert!(matches!( @@ -331,11 +334,11 @@ mod tests { Err(StudioError::NotConnected) )); assert!(matches!( - svc.spatial_features().await, + svc.spatial_features("places").await, Err(StudioError::NotConnected) )); assert!(matches!( - svc.fts_hits("nodedb").await, + svc.fts_hits("articles", "nodedb").await, Err(StudioError::NotConnected) )); assert!(matches!( diff --git a/nodedb-studio/src/services/viewers_data.rs b/nodedb-studio/src/services/viewers_data.rs index cf845ae..1fa6035 100644 --- a/nodedb-studio/src/services/viewers_data.rs +++ b/nodedb-studio/src/services/viewers_data.rs @@ -1,11 +1,20 @@ //! Specialized-viewer reads at the backend seam: graph, vector, timeseries, //! spatial, FTS, and sync. //! -//! `sub_graph` returns a single `SubGraph` rather than a list: unlike the -//! other five reads it cannot be expressed as `apply(self.behavior, ...)`, -//! which only knows how to fold `MockBehavior::Empty` into `Vec::new()`. It -//! is implemented with the same explicit four-arm match `record_detail` and -//! `run_query` use. +//! `sub_graph` returns a single `SubGraph` rather than a list, so it cannot +//! be expressed as `apply(self.behavior, ...)`, which only knows how to fold +//! `MockBehavior::Empty` into `Vec::new()`. Its mock implementation instead +//! uses `apply_one_or_empty`, which lets a single-value read that wraps a +//! list (a result set's rows, a graph's nodes) decide what a genuinely empty +//! payload looks like, rather than folding `Empty` into `Ready` the way +//! `record_detail` does. +//! +//! `sub_graph`, `vector_points`, `spatial_features` and `fts_hits` all take a +//! `collection`: the Explorer already scopes its selection to one collection +//! per storage mode (graph/vector/spatial), so each viewer's read must be +//! parameterised the same way `records(collection)` already is. `sync_peers` +//! is deliberately left unparameterised — it is instance-scoped, not +//! per-collection. use async_trait::async_trait; @@ -16,28 +25,29 @@ use crate::services::error::StudioError; #[async_trait(?Send)] pub trait ViewersData { - /// One graph viewer's full render input (nodes + edges). - #[allow(dead_code)] // SEAM-UNWIRED(task-10) - async fn sub_graph(&self) -> Result; + /// One graph viewer's full render input (nodes + edges) for `collection`. + #[allow(dead_code)] // SEAM-UNWIRED + async fn sub_graph(&self, collection: &str) -> Result; - /// A 2D projection of vector embeddings for the vector viewer. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) - async fn vector_points(&self) -> Result, StudioError>; + /// A 2D projection of vector embeddings in `collection` for the vector + /// viewer. + #[allow(dead_code)] // SEAM-UNWIRED + async fn vector_points(&self, collection: &str) -> Result, StudioError>; /// Samples for one timeseries metric. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn series(&self, metric: &str) -> Result, StudioError>; - /// Features for the spatial viewer's map. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) - async fn spatial_features(&self) -> Result, StudioError>; + /// Features for the spatial viewer's map, scoped to `collection`. + #[allow(dead_code)] // SEAM-UNWIRED + async fn spatial_features(&self, collection: &str) -> Result, StudioError>; - /// Full-text-search hits for `query`. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) - async fn fts_hits(&self, query: &str) -> Result, StudioError>; + /// Full-text-search hits for `query` within `collection`. + #[allow(dead_code)] // SEAM-UNWIRED + async fn fts_hits(&self, collection: &str, query: &str) -> Result, StudioError>; /// Sync/replication peers. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn sync_peers(&self) -> Result, StudioError>; } @@ -50,7 +60,7 @@ mod tests { #[tokio::test] async fn subgraph_edges_reference_existing_nodes() { let svc = MockConnectionService::ready(); - let g = svc.sub_graph().await.expect("graph"); + let g = svc.sub_graph("social").await.expect("graph"); let ids: Vec<&str> = g.nodes.iter().map(|n| n.id.as_str()).collect(); assert!(!g.nodes.is_empty() && !g.edges.is_empty()); for e in &g.edges { @@ -67,7 +77,7 @@ mod tests { async fn every_viewer_read_is_keyed_and_non_empty() { let svc = MockConnectionService::ready(); - let vector = svc.vector_points().await.expect("vec"); + let vector = svc.vector_points("embeddings").await.expect("vec"); assert!(!vector.is_empty()); assert!( vector.iter().all(|p| !p.id.is_empty()), @@ -81,14 +91,14 @@ mod tests { "every series point needs a stable key" ); - let spatial = svc.spatial_features().await.expect("geo"); + let spatial = svc.spatial_features("places").await.expect("geo"); assert!(!spatial.is_empty()); assert!( spatial.iter().all(|f| !f.id.is_empty()), "every spatial feature needs a stable key" ); - let fts = svc.fts_hits("nodedb").await.expect("fts"); + let fts = svc.fts_hits("articles", "nodedb").await.expect("fts"); assert!(!fts.is_empty()); assert!( fts.iter().all(|h| !h.id.is_empty()), @@ -111,32 +121,51 @@ mod tests { #[tokio::test] async fn sub_graph_ready_returns_nodes_and_edges() { let svc = MockConnectionService::ready(); - let g = svc.sub_graph().await.expect("ready yields a graph"); + let g = svc.sub_graph("social").await.expect("ready yields a graph"); assert!(!g.nodes.is_empty(), "fixture must have nodes"); assert!(!g.edges.is_empty(), "fixture must have edges"); } #[tokio::test] - async fn sub_graph_empty_behaviour_still_returns_a_graph() { - // `sub_graph` reads a single value, not a list: "no rows" has no - // meaning here, so the mock folds `MockBehavior::Empty` into the same - // success path as `Ready`, mirroring `record_detail` and `run_query`. - // Pinned here so a later refactor cannot silently change that meaning. + async fn sub_graph_ids_vary_by_collection() { + // A wrong-argument bug (e.g. ignoring `collection`) must be visible: + // the fixture keys every id off the requested collection. + let svc = MockConnectionService::ready(); + let a = svc.sub_graph("social").await.expect("graph"); + let b = svc.sub_graph("orders").await.expect("graph"); + assert_ne!( + a.nodes.first().map(|n| n.id.as_str()), + b.nodes.first().map(|n| n.id.as_str()), + "node ids must key off the requested collection" + ); + } + + #[tokio::test] + async fn sub_graph_empty_behaviour_returns_zero_nodes() { + // Unlike the truly single-value reads (`record_detail`), a graph's + // emptiness is meaningful: a collection with no nodes is a real + // outcome for the graph viewer. `MockBehavior::Empty` must therefore + // deliver a genuinely empty graph, not fold into `Ready`. let svc = MockConnectionService::empty(); let g = svc - .sub_graph() + .sub_graph("social") .await - .expect("empty behaviour still returns a graph for a single-value read"); - assert!( - !g.nodes.is_empty(), - "folded-Empty graph must still have nodes" - ); + .expect("empty behaviour is still Ok"); + assert!(g.nodes.is_empty(), "empty behaviour must yield zero nodes"); + assert!(g.edges.is_empty(), "empty behaviour must yield zero edges"); + } + + #[tokio::test] + async fn sub_graph_empty_behaviour_reaches_the_empty_state() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.sub_graph("social").await)); + assert!(s.is_empty()); } #[tokio::test] async fn sub_graph_erroring_is_err() { let svc = MockConnectionService::erroring(); - assert!(svc.sub_graph().await.is_err()); + assert!(svc.sub_graph("social").await.is_err()); } #[tokio::test] @@ -144,22 +173,37 @@ mod tests { // Expectation authored independently of the fixture body: the fixture // alternates clusters "a"/"b", so both must be present. let svc = MockConnectionService::ready(); - let pts = svc.vector_points().await.expect("ready yields points"); + let pts = svc + .vector_points("embeddings") + .await + .expect("ready yields points"); assert!(pts.iter().any(|p| p.cluster == "a")); assert!(pts.iter().any(|p| p.cluster == "b")); } + #[tokio::test] + async fn vector_points_ids_vary_by_collection() { + let svc = MockConnectionService::ready(); + let a = svc.vector_points("embeddings").await.expect("vec"); + let b = svc.vector_points("orders").await.expect("vec"); + assert_ne!( + a.first().map(|p| p.id.as_str()), + b.first().map(|p| p.id.as_str()), + "vector point ids must key off the requested collection" + ); + } + #[tokio::test] async fn vector_points_empty_is_empty() { let svc = MockConnectionService::empty(); - let s = AsyncState::from_value(Some(svc.vector_points().await)); + let s = AsyncState::from_value(Some(svc.vector_points("embeddings").await)); assert!(s.is_empty()); } #[tokio::test] async fn vector_points_erroring_is_err() { let svc = MockConnectionService::erroring(); - let s = AsyncState::from_value(Some(svc.vector_points().await)); + let s = AsyncState::from_value(Some(svc.vector_points("embeddings").await)); assert!(s.error_message().is_some()); } @@ -190,48 +234,78 @@ mod tests { #[tokio::test] async fn spatial_features_ready_have_geometry() { let svc = MockConnectionService::ready(); - let feats = svc.spatial_features().await.expect("ready yields features"); + let feats = svc + .spatial_features("places") + .await + .expect("ready yields features"); assert!( feats.iter().all(|f| !f.geometry_json.is_empty()), "every feature must carry display geometry" ); } + #[tokio::test] + async fn spatial_features_ids_vary_by_collection() { + let svc = MockConnectionService::ready(); + let a = svc.spatial_features("places").await.expect("geo"); + let b = svc.spatial_features("orders").await.expect("geo"); + assert_ne!( + a.first().map(|f| f.id.as_str()), + b.first().map(|f| f.id.as_str()), + "spatial feature ids must key off the requested collection" + ); + } + #[tokio::test] async fn spatial_features_empty_is_empty() { let svc = MockConnectionService::empty(); - let s = AsyncState::from_value(Some(svc.spatial_features().await)); + let s = AsyncState::from_value(Some(svc.spatial_features("places").await)); assert!(s.is_empty()); } #[tokio::test] async fn spatial_features_erroring_is_err() { let svc = MockConnectionService::erroring(); - let s = AsyncState::from_value(Some(svc.spatial_features().await)); + let s = AsyncState::from_value(Some(svc.spatial_features("places").await)); assert!(s.error_message().is_some()); } #[tokio::test] async fn fts_hits_ready_excerpts_mention_the_query() { let svc = MockConnectionService::ready(); - let hits = svc.fts_hits("nodedb").await.expect("ready yields hits"); + let hits = svc + .fts_hits("articles", "nodedb") + .await + .expect("ready yields hits"); assert!( hits.iter().all(|h| h.excerpt.contains("nodedb")), "excerpts must reflect the requested query, not a fixed string" ); } + #[tokio::test] + async fn fts_hits_ids_vary_by_collection() { + let svc = MockConnectionService::ready(); + let a = svc.fts_hits("articles", "nodedb").await.expect("fts"); + let b = svc.fts_hits("orders", "nodedb").await.expect("fts"); + assert_ne!( + a.first().map(|h| h.id.as_str()), + b.first().map(|h| h.id.as_str()), + "fts hit ids must key off the requested collection" + ); + } + #[tokio::test] async fn fts_hits_empty_is_empty() { let svc = MockConnectionService::empty(); - let s = AsyncState::from_value(Some(svc.fts_hits("nodedb").await)); + let s = AsyncState::from_value(Some(svc.fts_hits("articles", "nodedb").await)); assert!(s.is_empty()); } #[tokio::test] async fn fts_hits_erroring_is_err() { let svc = MockConnectionService::erroring(); - let s = AsyncState::from_value(Some(svc.fts_hits("nodedb").await)); + let s = AsyncState::from_value(Some(svc.fts_hits("articles", "nodedb").await)); assert!(s.error_message().is_some()); } diff --git a/nodedb-studio/src/services/workbench_data.rs b/nodedb-studio/src/services/workbench_data.rs index 45e370a..aa7303b 100644 --- a/nodedb-studio/src/services/workbench_data.rs +++ b/nodedb-studio/src/services/workbench_data.rs @@ -14,15 +14,15 @@ use crate::services::error::StudioError; #[async_trait(?Send)] pub trait WorkbenchData { /// Execute `sql` and return one page of results. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn run_query(&self, sql: &str) -> Result; /// The query planner's EXPLAIN output for `sql`. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn explain(&self, sql: &str) -> Result; /// The schema tree for the connected database. - #[allow(dead_code)] // SEAM-UNWIRED(task-10) + #[allow(dead_code)] // SEAM-UNWIRED async fn schema_tree(&self) -> Result, StudioError>; } @@ -59,16 +59,25 @@ mod tests { } #[tokio::test] - async fn run_query_empty_behaviour_still_returns_a_result_set() { - // `run_query` reads a single result set, not a list: "no rows" has no - // meaning at this call boundary, so `MockBehavior::Empty` folds into - // the same success path as `Ready`, mirroring `record_detail`. + async fn run_query_empty_behaviour_returns_zero_rows() { + // Unlike the truly single-value reads (`record_detail`, `explain`), + // a result set's emptiness is meaningful: "no rows" is the most + // common non-error outcome for a query. `MockBehavior::Empty` must + // therefore deliver a genuinely empty result set, not fold into + // `Ready`. let svc = MockConnectionService::empty(); let rs = svc .run_query("SELECT 1") .await - .expect("empty behaviour still returns a result set for a single-value read"); - assert!(!rs.columns.is_empty()); + .expect("empty behaviour is still Ok"); + assert!(rs.rows.is_empty(), "empty behaviour must yield zero rows"); + } + + #[tokio::test] + async fn run_query_empty_behaviour_reaches_the_empty_state() { + let svc = MockConnectionService::empty(); + let s = AsyncState::from_value(Some(svc.run_query("SELECT 1").await)); + assert!(s.is_empty()); } #[tokio::test] From 41a16020ca497ef1b3454a227bcbc3811fd65af4 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sun, 9 Aug 2026 11:13:13 +0800 Subject: [PATCH 26/26] docs(comments): correct false statements in two comments - connection_service.rs line 577: remove `run_query` from citation since it no longer folds Empty into Ready; keep `record_detail` reference. - workbench.rs lines 32-33: correct claim that columns make empty state distinguishable; they are shape documentation only, and proper rendering would require a payload-carrying Empty variant that does not exist. --- nodedb-studio/src/data/mock/workbench.rs | 5 +++-- nodedb-studio/src/services/connection_service.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/nodedb-studio/src/data/mock/workbench.rs b/nodedb-studio/src/data/mock/workbench.rs index bbab4db..36fe64b 100644 --- a/nodedb-studio/src/data/mock/workbench.rs +++ b/nodedb-studio/src/data/mock/workbench.rs @@ -29,8 +29,9 @@ pub fn result_set(sql: &str) -> ResultSet { /// The genuinely-empty result set `MockBehavior::Empty` returns for /// `run_query`: zero rows is the most common non-error query outcome, so /// unlike `record_detail`/`explain` this must not fold into `result_set`'s -/// fixture rows. Columns are kept (a real zero-row result still has a shape) -/// so the empty state is distinguishable from an absent one. +/// fixture rows. Columns are shape documentation for the fixture; a zero-row +/// render with headers would need a payload-carrying Empty variant, which +/// does not exist in the current `AsyncState` design. #[allow(dead_code)] // SEAM-UNWIRED pub fn empty_result_set(sql: &str) -> ResultSet { ResultSet { diff --git a/nodedb-studio/src/services/connection_service.rs b/nodedb-studio/src/services/connection_service.rs index 4515032..602acaf 100644 --- a/nodedb-studio/src/services/connection_service.rs +++ b/nodedb-studio/src/services/connection_service.rs @@ -574,7 +574,7 @@ mod tests { #[tokio::test] async fn session_info_empty_behavior_still_returns_the_fixture() { // Single-value reads have no "empty" shape, so Empty folds into Ready - // — matching the `record_detail` / `run_query` precedent. + // — matching the `record_detail` precedent. let svc = MockConnectionService::empty(); let s = svc.session_info().await.expect("empty folds into ready"); assert_eq!(s.database, "analytics");