From 6f9cfd9ff2bb3cb2e8c3fe4bf1a9b713494f40cf Mon Sep 17 00:00:00 2001 From: Erik Nilsen Date: Tue, 20 Oct 2026 08:06:43 -0700 Subject: [PATCH 1/2] feat(server): suggest near slugs when a direct read finds no entity Signed-off-by: Erik Nilsen --- docs/reference/failure-reasons.md | 2 +- .../crates/trogon-atlas-client/src/failure.rs | 35 +++- .../crates/trogon-atlas-server/src/conv.rs | 15 ++ .../crates/trogon-atlas-server/src/lib.rs | 1 + .../crates/trogon-atlas-server/src/service.rs | 63 ++++--- .../crates/trogon-atlas-server/src/suggest.rs | 167 ++++++++++++++++++ .../tests/cross_tenant_reads.rs | 41 +++++ 7 files changed, 300 insertions(+), 24 deletions(-) create mode 100644 rsworkspace/crates/trogon-atlas-server/src/suggest.rs diff --git a/docs/reference/failure-reasons.md b/docs/reference/failure-reasons.md index 65adb425..1562310d 100644 --- a/docs/reference/failure-reasons.md +++ b/docs/reference/failure-reasons.md @@ -26,7 +26,7 @@ instead of re-deriving category from code or reason on its own. | Reason | gRPC code | Category | Metadata | | --- | --- | --- | --- | -| `NOT_FOUND` | `NOT_FOUND` | `not_found` | none | +| `NOT_FOUND` | `NOT_FOUND` | `not_found` | `did_you_mean`, up to 3 comma-separated slugs of the same kind in the same namespace that are near the requested slug, closest first. Only `get_entity`, `get_slice_projection`, `get_storyboard_projection` and `get_event_model_projection` set it, and only for a namespace the caller can read. Absent when nothing is near. The client gives it as a list in the failure details (MCP `next_action_detail.did_you_mean`). | | `ALREADY_EXISTS` | `ALREADY_EXISTS` | `precondition` | none | | `STALE_PRECONDITION` | `ABORTED` | `stale_state` | none | | `VALIDATION_FAILED` | `INVALID_ARGUMENT` | `validation` | none | diff --git a/rsworkspace/crates/trogon-atlas-client/src/failure.rs b/rsworkspace/crates/trogon-atlas-client/src/failure.rs index a4a44072..7aceb1b3 100644 --- a/rsworkspace/crates/trogon-atlas-client/src/failure.rs +++ b/rsworkspace/crates/trogon-atlas-client/src/failure.rs @@ -397,9 +397,15 @@ impl Failure { ), _ => return None, }; - let details = match metadata.get("journal") { - Some(journal) => FailureDetails::from_json(json!({ "journal": journal })), - None => FailureDetails::empty(), + let details = if let Some(journal) = metadata.get("journal") { + FailureDetails::from_json(json!({ "journal": journal })) + } else if let Some(slugs) = metadata.get("did_you_mean") { + // ErrorInfo metadata is string-to-string, so the server joins the + // slugs with a comma, which a valid slug can never contain. + let slugs: Vec<&str> = slugs.split(',').filter(|s| !s.is_empty()).collect(); + FailureDetails::from_json(json!({ "did_you_mean": slugs })) + } else { + FailureDetails::empty() }; let failure = Self::new(category, status.message().to_string()) .with_code(FailureCode::known(code)) @@ -640,6 +646,29 @@ mod tests { assert_eq!(failure.details.0["journal"], json!("journal-123")); } + #[test] + fn not_found_with_did_you_mean_carries_the_slugs_as_a_list() { + use std::collections::HashMap; + + use tonic_types::{ErrorDetails, StatusExt}; + + let details = ErrorDetails::with_error_info( + "NOT_FOUND", + SERVER_ERROR_DOMAIN, + HashMap::from([( + "did_you_mean".to_string(), + "place-order-slice,place-orders".to_string(), + )]), + ); + let status = Status::with_error_details(Code::NotFound, "entity not found", details); + let failure = Failure::from_status(&status); + assert_eq!(failure.category, FailureCategory::NotFound); + assert_eq!( + failure.details.0["did_you_mean"], + json!(["place-order-slice", "place-orders"]) + ); + } + #[test] fn status_with_not_writer_reason_classifies_as_unavailable_with_its_metadata() { use std::collections::HashMap; diff --git a/rsworkspace/crates/trogon-atlas-server/src/conv.rs b/rsworkspace/crates/trogon-atlas-server/src/conv.rs index bb375e17..316d7d61 100644 --- a/rsworkspace/crates/trogon-atlas-server/src/conv.rs +++ b/rsworkspace/crates/trogon-atlas-server/src/conv.rs @@ -180,6 +180,21 @@ pub fn cursor_rejected_err(message: impl Into) -> Status { with_reason(Code::InvalidArgument, message, reason::CURSOR_REJECTED) } +/// `NOT_FOUND` carrying near-match slugs under `did_you_mean`, comma-joined +/// because `ErrorInfo` metadata values are strings. With no suggestions it is +/// the same status `store_err` gives, so the two cannot be told apart. +pub fn not_found_with_suggestions(suggestions: &[String]) -> Status { + if suggestions.is_empty() { + return store_err(StoreError::NotFound); + } + with_reason_and_metadata( + Code::NotFound, + "entity not found", + reason::NOT_FOUND, + HashMap::from([("did_you_mean".to_string(), suggestions.join(","))]), + ) +} + pub fn store_err(err: StoreError) -> Status { match err { StoreError::NotFound => with_reason(Code::NotFound, "entity not found", reason::NOT_FOUND), diff --git a/rsworkspace/crates/trogon-atlas-server/src/lib.rs b/rsworkspace/crates/trogon-atlas-server/src/lib.rs index a03ea201..ce628931 100644 --- a/rsworkspace/crates/trogon-atlas-server/src/lib.rs +++ b/rsworkspace/crates/trogon-atlas-server/src/lib.rs @@ -21,6 +21,7 @@ pub mod scope; pub mod search; pub mod service; pub mod spicedb; +pub mod suggest; pub mod telemetry; pub mod tls; pub mod token_reload; diff --git a/rsworkspace/crates/trogon-atlas-server/src/service.rs b/rsworkspace/crates/trogon-atlas-server/src/service.rs index 81247db5..3d82832f 100644 --- a/rsworkspace/crates/trogon-atlas-server/src/service.rs +++ b/rsworkspace/crates/trogon-atlas-server/src/service.rs @@ -1189,6 +1189,29 @@ impl EventModelServiceImpl { Err(Status::not_found("entity not found")) } + /// Map a direct read's store error, adding near-match slugs to a + /// `NOT_FOUND`. Call only after `require_visible` has passed: the + /// suggestions come from the lensed snapshot, but a hidden namespace must + /// get the bare `NOT_FOUND` that `require_visible` already gave it. + async fn read_err( + &self, + err: trogon_atlas_store::error::StoreError, + vis: &Visibility, + branch: Option<&str>, + kind: pb::EntityKind, + id: &pb::Id, + ) -> Status { + if !matches!(err, trogon_atlas_store::error::StoreError::NotFound) { + return store_err(err); + } + match self.snapshot_for(branch, vis).await { + Ok(all) => crate::conv::not_found_with_suggestions(&crate::suggest::did_you_mean( + kind, id, &all, + )), + Err(_) => store_err(err), + } + } + /// The set of namespaces this request may read, resolved once so the /// per-record filters that follow stay synchronous. async fn lens(&self, vis: &Visibility) -> Result { @@ -2730,11 +2753,10 @@ impl pb::event_model_service_server::EventModelService for EventModelServiceImpl // Direct-by-key reads never touch the snapshot, so the visibility // filter that covers every other read path does not reach them. self.require_visible(&vis, &id.namespace).await?; - let stored = self - .store - .get(kind, id, branch.as_deref()) - .await - .map_err(store_err)?; + let stored = match self.store.get(kind, id, branch.as_deref()).await { + Ok(stored) => stored, + Err(err) => return Err(self.read_err(err, &vis, branch.as_deref(), kind, id).await), + }; Ok(Response::new(pb::GetEntityResponse { entity: Some(stored.entity), etag: stored.etag, @@ -3651,11 +3673,10 @@ impl pb::event_model_service_server::EventModelService for EventModelServiceImpl // The root is fetched by key, so the lensed snapshot below // narrows the neighbourhood but never the root itself. self.require_visible(&vis, &id.namespace).await?; - let stored = self - .store - .get(kind, id, branch.as_deref()) - .await - .map_err(store_err)?; + let stored = match self.store.get(kind, id, branch.as_deref()).await { + Ok(stored) => stored, + Err(err) => return Err(self.read_err(err, &vis, branch.as_deref(), kind, id).await), + }; let slice = slice_from_entity(&stored.entity) .ok_or_else(|| Status::internal("slice entity kind mismatch"))?; @@ -3693,11 +3714,11 @@ impl pb::event_model_service_server::EventModelService for EventModelServiceImpl // The root is fetched by key, so the lensed snapshot below // narrows the neighbourhood but never the root itself. self.require_visible(&vis, &id.namespace).await?; - let stored = self - .store - .get(pb::EntityKind::Storyboard, id, branch.as_deref()) - .await - .map_err(store_err)?; + let kind = pb::EntityKind::Storyboard; + let stored = match self.store.get(kind, id, branch.as_deref()).await { + Ok(stored) => stored, + Err(err) => return Err(self.read_err(err, &vis, branch.as_deref(), kind, id).await), + }; let Some(pb::entity::Kind::Storyboard(storyboard)) = stored.entity.kind.clone() else { return Err(Status::internal("storyboard entity kind mismatch")); }; @@ -3767,11 +3788,13 @@ impl pb::event_model_service_server::EventModelService for EventModelServiceImpl // narrows the neighbourhood but never the root itself. self.require_visible(&vis, &id.namespace).await?; let (em, all, as_of_incomplete) = if req.as_of_changeset.is_empty() { - let stored = self - .store - .get(pb::EntityKind::EventModel, id, branch.as_deref()) - .await - .map_err(store_err)?; + let kind = pb::EntityKind::EventModel; + let stored = match self.store.get(kind, id, branch.as_deref()).await { + Ok(stored) => stored, + Err(err) => { + return Err(self.read_err(err, &vis, branch.as_deref(), kind, id).await) + } + }; let Some(pb::entity::Kind::EventModel(em)) = stored.entity.kind.clone() else { return Err(Status::internal("event_model entity kind mismatch")); }; diff --git a/rsworkspace/crates/trogon-atlas-server/src/suggest.rs b/rsworkspace/crates/trogon-atlas-server/src/suggest.rs new file mode 100644 index 00000000..273a2d64 --- /dev/null +++ b/rsworkspace/crates/trogon-atlas-server/src/suggest.rs @@ -0,0 +1,167 @@ +//! Near-match slugs for a direct read whose key does not resolve. + +use trogon_atlas_proto as pb; +use trogon_atlas_store::StoredEntity; + +use crate::conv::{entity_id, entity_kind}; + +/// Enough to catch a typo or a missing suffix without burying the answer. +pub const MAX_SUGGESTIONS: usize = 3; + +/// Slugs of the same kind in the same namespace as `missing`, closest first. +/// +/// `entities` must already be the caller's lensed snapshot: anything in it +/// can be named back to the caller, so it must hold nothing they cannot see. +pub fn did_you_mean( + kind: pb::EntityKind, + missing: &pb::Id, + entities: &[StoredEntity], +) -> Vec { + let mut ranked: Vec<(u8, usize, &str)> = entities + .iter() + .filter(|se| entity_kind(&se.entity).ok() == Some(kind)) + .filter_map(|se| entity_id(&se.entity).ok()) + .filter(|id| id.namespace == missing.namespace) + .filter_map(|id| { + let (tier, distance) = rank(&missing.slug, &id.slug)?; + Some((tier, distance, id.slug.as_str())) + }) + .collect(); + ranked.sort_unstable(); + ranked.dedup_by(|a, b| a.2 == b.2); + ranked + .into_iter() + .take(MAX_SUGGESTIONS) + .map(|(_, _, slug)| slug.to_string()) + .collect() +} + +/// A prefix or substring match beats a raw edit distance, because a slug +/// that is missing its suffix is the common mistake and can be far apart in +/// edits (`place-order` vs `place-order-slice`). +fn rank(missing: &str, candidate: &str) -> Option<(u8, usize)> { + if candidate == missing { + return None; + } + let distance = edit_distance(missing, candidate); + let tier = if candidate.starts_with(missing) || missing.starts_with(candidate) { + 0 + } else if candidate.contains(missing) || missing.contains(candidate) { + 1 + } else if distance <= (missing.len() / 3).max(1) { + 2 + } else { + return None; + }; + Some((tier, distance)) +} + +/// Levenshtein distance over bytes; slugs are ASCII by validation. +fn edit_distance(a: &str, b: &str) -> usize { + let (a, b) = (a.as_bytes(), b.as_bytes()); + let mut prev: Vec = (0..=b.len()).collect(); + let mut cur = vec![0; b.len() + 1]; + for (i, ca) in a.iter().enumerate() { + cur[0] = i + 1; + for (j, cb) in b.iter().enumerate() { + let substitute = prev[j] + usize::from(ca != cb); + cur[j + 1] = substitute.min(prev[j + 1] + 1).min(cur[j] + 1); + } + std::mem::swap(&mut prev, &mut cur); + } + prev[b.len()] +} + +#[cfg(test)] +mod tests { + use super::*; + + fn id(namespace: &str, slug: &str) -> pb::Id { + pb::Id { + namespace: namespace.into(), + slug: slug.into(), + version: 1, + } + } + + fn stored(kind: pb::entity::Kind) -> StoredEntity { + StoredEntity { + entity: pb::Entity { + system: None, + kind: Some(kind), + }, + etag: String::new(), + } + } + + fn command_slice(namespace: &str, slug: &str) -> StoredEntity { + stored(pb::entity::Kind::CommandSlice(pb::CommandSlice { + id: Some(id(namespace, slug)), + ..Default::default() + })) + } + + fn event(namespace: &str, slug: &str) -> StoredEntity { + stored(pb::entity::Kind::Event(pb::Event { + id: Some(id(namespace, slug)), + ..Default::default() + })) + } + + #[test] + fn a_missing_suffix_ranks_above_a_closer_typo() { + let entities = [ + command_slice("acme-storefront", "plac-order"), + command_slice("acme-storefront", "place-order-slice"), + ]; + let got = did_you_mean( + pb::EntityKind::CommandSlice, + &id("acme-storefront", "place-order"), + &entities, + ); + assert_eq!(got, ["place-order-slice", "plac-order"]); + } + + #[test] + fn only_the_same_kind_and_namespace_are_suggested() { + let entities = [ + event("acme-storefront", "place-order-slice"), + command_slice("acme-billing", "place-order-slice"), + command_slice("acme-storefront", "place-order-v2"), + ]; + let got = did_you_mean( + pb::EntityKind::CommandSlice, + &id("acme-storefront", "place-order"), + &entities, + ); + assert_eq!(got, ["place-order-v2"]); + } + + #[test] + fn at_most_three_distinct_slugs_are_suggested() { + let entities: Vec<_> = ["order-a", "order-b", "order-c", "order-d", "order-a"] + .into_iter() + .map(|slug| command_slice("acme-storefront", slug)) + .collect(); + let got = did_you_mean( + pb::EntityKind::CommandSlice, + &id("acme-storefront", "order"), + &entities, + ); + assert_eq!(got, ["order-a", "order-b", "order-c"]); + } + + #[test] + fn a_typo_is_suggested_but_an_unrelated_slug_is_not() { + let entities = [ + command_slice("acme-storefront", "place-ordr"), + command_slice("acme-storefront", "refund-payment"), + ]; + let got = did_you_mean( + pb::EntityKind::CommandSlice, + &id("acme-storefront", "place-order"), + &entities, + ); + assert_eq!(got, ["place-ordr"]); + } +} diff --git a/rsworkspace/crates/trogon-atlas-server/tests/cross_tenant_reads.rs b/rsworkspace/crates/trogon-atlas-server/tests/cross_tenant_reads.rs index 1130c902..06a82102 100644 --- a/rsworkspace/crates/trogon-atlas-server/tests/cross_tenant_reads.rs +++ b/rsworkspace/crates/trogon-atlas-server/tests/cross_tenant_reads.rs @@ -377,6 +377,47 @@ async fn get_slice_projection_does_not_project_another_owners_slice() { assert_hidden("get_slice_projection", out, |r| r.projection.is_none()); } +/// The `did_you_mean` slugs a not-found slice projection names, if any. +async fn suggestions_for( + c: &mut EventModelServiceClient, + slug: &str, + token: &str, +) -> Option { + use tonic_types::StatusExt; + + let status = c + .get_slice_projection(authed( + pb::GetSliceProjectionRequest { + kind: pb::EntityKind::CommandSlice as i32, + id: Some(id(slug)), + }, + token, + )) + .await + .expect_err("the slug does not exist"); + assert_eq!(status.code(), Code::NotFound, "{}", status.message()); + status + .get_details_error_info() + .and_then(|info| info.metadata.get("did_you_mean").cloned()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_not_found_read_suggests_near_slugs_to_their_owner() { + let mut c = world().await; + assert_eq!( + suggestions_for(&mut c, "plac", ACME_TOKEN).await.as_deref(), + Some("place"), + ); +} + +/// A suggestion would name a slug in the hidden namespace, which is the +/// existence oracle `require_visible` answers `NOT_FOUND` to prevent. +#[tokio::test(flavor = "multi_thread")] +async fn a_not_found_read_suggests_nothing_from_a_hidden_namespace() { + let mut c = world().await; + assert_eq!(suggestions_for(&mut c, "plac", BETA_TOKEN).await, None); +} + #[tokio::test(flavor = "multi_thread")] async fn extract_subgraph_does_not_extract_another_owners_subgraph() { let mut c = world().await; From 8a91321ae2b7b3e23b4055e133ed268b220a285f Mon Sep 17 00:00:00 2001 From: Erik Nilsen Date: Tue, 20 Oct 2026 08:31:30 -0700 Subject: [PATCH 2/2] fix(server): use a near-miss test slug the spell checker accepts Signed-off-by: Erik Nilsen --- rsworkspace/crates/trogon-atlas-server/src/suggest.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rsworkspace/crates/trogon-atlas-server/src/suggest.rs b/rsworkspace/crates/trogon-atlas-server/src/suggest.rs index 273a2d64..bf1f524a 100644 --- a/rsworkspace/crates/trogon-atlas-server/src/suggest.rs +++ b/rsworkspace/crates/trogon-atlas-server/src/suggest.rs @@ -154,7 +154,7 @@ mod tests { #[test] fn a_typo_is_suggested_but_an_unrelated_slug_is_not() { let entities = [ - command_slice("acme-storefront", "place-ordr"), + command_slice("acme-storefront", "place_order"), command_slice("acme-storefront", "refund-payment"), ]; let got = did_you_mean( @@ -162,6 +162,6 @@ mod tests { &id("acme-storefront", "place-order"), &entities, ); - assert_eq!(got, ["place-ordr"]); + assert_eq!(got, ["place_order"]); } }