From 0b54690ff108c525710ad4def95c18a6305be7a8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 07:58:10 +0000 Subject: [PATCH 1/2] rust(feat): default explore links to a single source type Agents building an Explore link were passing both `assets` and `runs` on their own, opening the view on two sources when the user asked about one. A run is already scoped to its asset, so the asset only widens the selection. MCP `explore_url` now rejects `assets` and `runs` together with INVALID_PARAMS unless the new `include_assets_and_runs` flag is set, and its description and the installed skill tell agents to send one source type. `sift-cli import` links to the run it imported into, falling back to the asset only when there is no run. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Gw582tAF2wdN5if7FxLfT3 --- rust/crates/sift_cli/CHANGELOG.md | 6 ++ .../skills/sift/references/explore-links.md | 6 ++ rust/crates/sift_cli/src/util/explore_url.rs | 35 ++++++---- rust/crates/sift_mcp/src/service/url/mod.rs | 20 +++++- rust/crates/sift_mcp/src/service/url/test.rs | 66 +++++++++++++++++++ rust/crates/sift_mcp/src/tool/explore/mod.rs | 15 ++++- rust/crates/sift_mcp/src/tool/explore/test.rs | 45 +++++++++++++ 7 files changed, 175 insertions(+), 18 deletions(-) diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index d8d0acbce..8b1c16709 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -7,6 +7,12 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### What's New +- Explore links now carry one source type. The MCP `explore_url` tool rejects + `assets` and `runs` together with `INVALID_PARAMS` unless the new + `include_assets_and_runs` flag is set, so an agent no longer mixes an asset + and a run into one view on its own. `sift-cli import` links to the run it + imported into, and falls back to the asset only when there is no run. + ## [v0.4.4] - August 24, 2026 ### What's New diff --git a/rust/crates/sift_cli/assets/skills/sift/references/explore-links.md b/rust/crates/sift_cli/assets/skills/sift/references/explore-links.md index 182072d98..8aa9a7bd4 100644 --- a/rust/crates/sift_cli/assets/skills/sift/references/explore-links.md +++ b/rust/crates/sift_cli/assets/skills/sift/references/explore-links.md @@ -3,6 +3,12 @@ Build the link with `explore_url`, then surface the URL to the user as plain text, in full. +Send one source type. Pass `runs` when the request names a run, `assets` +otherwise. A run is already scoped to its asset, so adding the asset opens the +view on a second, wider source. `explore_url` rejects both with +`INVALID_PARAMS`; set `include_assets_and_runs` to true only when the user asked +to see runs and assets together in one view. + Pick the `panel_type` that fits the request: `timeseries` (the default), `histogram`, `table`, `fft`, `metrics`, `scatter-plot`, or `geo-map`. The tool rejects an unknown value with `INVALID_PARAMS` and names the accepted set in diff --git a/rust/crates/sift_cli/src/util/explore_url.rs b/rust/crates/sift_cli/src/util/explore_url.rs index bed2d177f..1e350e405 100644 --- a/rust/crates/sift_cli/src/util/explore_url.rs +++ b/rust/crates/sift_cli/src/util/explore_url.rs @@ -54,6 +54,9 @@ pub fn pending_import_tip(location: &str, explore_url: Option<&str>) -> String { tip } +/// Builds a link to a single Explore data source: the run when the import targets one, and the +/// asset otherwise. A run is already scoped to its asset, so naming both would open the view on +/// two sources instead of the one the caller imported into. pub fn build_explore_url( app_uri: Option<&str>, asset_name: &str, @@ -61,11 +64,11 @@ pub fn build_explore_url( ) -> Option { let host = app_uri.and_then(normalize_app_uri)?; - let mut url = format!("{host}/explore?method=single&assets={}", encode(asset_name)); - if let Some(run) = run { - url.push_str(&format!("&runs={}", encode(run))); - } - Some(url) + let source = match run { + Some(run) => format!("runs={}", encode(run)), + None => format!("assets={}", encode(asset_name)), + }; + Some(format!("{host}/explore?method=single&{source}")) } #[cfg(test)] @@ -82,9 +85,16 @@ mod tests { ); assert_eq!( target.explore_url.as_deref(), - Some( - "https://sift.example.net/explore?method=single&assets=Engine%20%2F%207&runs=Test%20Run" - ) + Some("https://sift.example.net/explore?method=single&runs=Test%20Run") + ); + } + + #[test] + fn an_import_without_a_run_links_to_the_asset() { + let target = import_target("Engine / 7", None, None, Some("https://sift.example.net")); + assert_eq!( + target.explore_url.as_deref(), + Some("https://sift.example.net/explore?method=single&assets=Engine%20%2F%207") ); } @@ -96,12 +106,9 @@ mod tests { Some("run-id"), Some("https://app.siftstack.com"), ); - assert!( - target - .explore_url - .as_deref() - .unwrap() - .ends_with("&runs=run-id") + assert_eq!( + target.explore_url.as_deref(), + Some("https://app.siftstack.com/explore?method=single&runs=run-id") ); } diff --git a/rust/crates/sift_mcp/src/service/url/mod.rs b/rust/crates/sift_mcp/src/service/url/mod.rs index dd029df5a..1087afd5e 100644 --- a/rust/crates/sift_mcp/src/service/url/mod.rs +++ b/rust/crates/sift_mcp/src/service/url/mod.rs @@ -28,6 +28,7 @@ pub struct ExploreUrlRequest { pub panel_type: Option, pub start_time_unix_nanos: Option, pub end_time_unix_nanos: Option, + pub include_assets_and_runs: bool, } #[derive(Clone)] @@ -48,10 +49,14 @@ impl UrlService { panel_type, start_time_unix_nanos, end_time_unix_nanos, + include_assets_and_runs, } = request; - let no_selection = assets.as_ref().is_none_or(|v| v.is_empty()) - && runs.as_ref().is_none_or(|v| v.is_empty()) + let has_assets = assets.as_ref().is_some_and(|v| !v.is_empty()); + let has_runs = runs.as_ref().is_some_and(|v| !v.is_empty()); + + let no_selection = !has_assets + && !has_runs && channels.as_ref().is_none_or(|v| v.is_empty()) && panel_type.is_none() && start_time_unix_nanos.is_none() @@ -64,6 +69,17 @@ impl UrlService { )); } + if has_assets && has_runs && !include_assets_and_runs { + return Err(ErrorData::invalid_params( + "`assets` and `runs` were both set. An Explore link that mixes asset-scoped \ + and run-scoped sources is not the default: pass `runs` alone when the request \ + names a run, or `assets` alone otherwise. Set `include_assets_and_runs` to \ + true only when the user explicitly asked to see runs and assets together in \ + one view.", + None, + )); + } + if let (Some(start), Some(end)) = (start_time_unix_nanos, end_time_unix_nanos) && end < start { diff --git a/rust/crates/sift_mcp/src/service/url/test.rs b/rust/crates/sift_mcp/src/service/url/test.rs index 3bdea6dd2..27e3fc13d 100644 --- a/rust/crates/sift_mcp/src/service/url/test.rs +++ b/rust/crates/sift_mcp/src/service/url/test.rs @@ -16,6 +16,7 @@ fn full_url_with_all_params() { panel_type: Some(String::from("scatter-plot")), start_time_unix_nanos: Some(0), end_time_unix_nanos: Some(1_700_000_000_000_000_000), + include_assets_and_runs: true, }) .unwrap(); assert_eq!( @@ -30,6 +31,71 @@ fn full_url_with_all_params() { ); } +#[test] +fn assets_and_runs_together_are_rejected_by_default() { + let err = service() + .build_explore_url(ExploreUrlRequest { + assets: Some(vec![String::from("Engine-7")]), + runs: Some(vec![String::from("2025-thrust-test")]), + ..Default::default() + }) + .unwrap_err(); + assert_eq!(err.code.0, -32602); + assert!( + err.message.contains("include_assets_and_runs"), + "the error should name the opt-in, got `{}`", + err.message + ); +} + +#[test] +fn assets_and_runs_together_are_allowed_when_requested() { + let url = service() + .build_explore_url(ExploreUrlRequest { + assets: Some(vec![String::from("Engine-7")]), + runs: Some(vec![String::from("2025-thrust-test")]), + include_assets_and_runs: true, + ..Default::default() + }) + .unwrap(); + assert_eq!( + url, + "https://app.siftstack.com/explore?method=single\ + &assets=Engine-7\ + &runs=2025-thrust-test" + ); +} + +#[test] +fn an_empty_asset_list_does_not_conflict_with_runs() { + let url = service() + .build_explore_url(ExploreUrlRequest { + assets: Some(vec![]), + runs: Some(vec![String::from("2025-thrust-test")]), + ..Default::default() + }) + .unwrap(); + assert_eq!( + url, + "https://app.siftstack.com/explore?method=single&runs=2025-thrust-test" + ); +} + +#[test] +fn the_opt_in_is_ignored_for_a_single_source_type() { + let url = service() + .build_explore_url(ExploreUrlRequest { + runs: Some(vec![String::from("2025-thrust-test")]), + include_assets_and_runs: true, + ..Default::default() + }) + .unwrap(); + assert_eq!( + url, + "https://app.siftstack.com/explore?method=single&runs=2025-thrust-test" + ); +} + #[test] fn axis_prefix_colon_is_preserved() { let url = service() diff --git a/rust/crates/sift_mcp/src/tool/explore/mod.rs b/rust/crates/sift_mcp/src/tool/explore/mod.rs index a5c3df000..b1a042405 100644 --- a/rust/crates/sift_mcp/src/tool/explore/mod.rs +++ b/rust/crates/sift_mcp/src/tool/explore/mod.rs @@ -19,6 +19,7 @@ pub struct ExploreUrlParams { panel_type: Option, start_time_unix_nanos: Option, end_time_unix_nanos: Option, + include_assets_and_runs: Option, } #[tool_router(router = explore_router, vis = "pub(crate)")] @@ -37,6 +38,10 @@ impl SiftMcpServer { Parameters: - `assets`: optional list of asset names or UUIDs. The Explore service resolves either form. - `runs`: optional list of run names or UUIDs. Same resolution rules as `assets`. + - `include_assets_and_runs`: optional, defaults to false. A link carries one source type by + default: pass `runs` when the request names a run, `assets` otherwise. Setting both `assets` and + `runs` without this flag is rejected. Set it to true only when the user explicitly asked to see + runs and assets together in one view. Ignored unless both `assets` and `runs` are set. - `channels`: optional list of channel names, UUIDs, or prefixed forms. Axis prefixes (`L1:foo`, `L2:bar`) bind a channel to a Y-axis for multi-axis plots. Role prefixes (`x:foo`, `y:foo`, `color:foo` for scatter; `lat:foo`, `lon:foo`, `color:foo` for geo-map) bind a channel to a panel @@ -46,13 +51,17 @@ impl SiftMcpServer { - `start_time_unix_nanos`, `end_time_unix_nanos`: optional time window. Provided as Unix nanoseconds for parity with `get_data`; the tool converts to ISO 8601 UTC for the URL. Errors: - - `INVALID_PARAMS` if no selection or time parameter is set (the URL would be useless), if - `panel_type` is not in the known set, or if `end_time_unix_nanos < start_time_unix_nanos`. + - `INVALID_PARAMS` if no selection or time parameter is set (the URL would be useless), if both + `assets` and `runs` are set without `include_assets_and_runs`, if `panel_type` is not in the + known set, or if `end_time_unix_nanos < start_time_unix_nanos`. Guidance: - Reach for this tool when the user asks to \"see\", \"view\", \"graph\", \"plot\", \"visualize\", or \"open\" data in Sift. Pair it with `get_data` only when the user also wants the data locally for SQL or further processing. + - Do not add the asset alongside a run to be thorough. A run is already scoped to its asset, so + the asset adds a second, wider source to the view. Send both only on an explicit request for + both. - The tool does not validate that the named asset/run/channel exists — Explore resolves at page load. Use names you have already retrieved from `list_*` tools to avoid 404s on click. ", @@ -66,6 +75,7 @@ impl SiftMcpServer { panel_type, start_time_unix_nanos, end_time_unix_nanos, + include_assets_and_runs, }) = params; let url = self.url_service.build_explore_url(ExploreUrlRequest { @@ -75,6 +85,7 @@ impl SiftMcpServer { panel_type, start_time_unix_nanos, end_time_unix_nanos, + include_assets_and_runs: include_assets_and_runs.unwrap_or(false), })?; let next_step = format!( diff --git a/rust/crates/sift_mcp/src/tool/explore/test.rs b/rust/crates/sift_mcp/src/tool/explore/test.rs index f69fd3c7b..99bd4c8e3 100644 --- a/rust/crates/sift_mcp/src/tool/explore/test.rs +++ b/rust/crates/sift_mcp/src/tool/explore/test.rs @@ -33,6 +33,7 @@ async fn handler_returns_structured_url_and_text_content() { panel_type: None, start_time_unix_nanos: None, end_time_unix_nanos: None, + include_assets_and_runs: None, }; let result = server.explore_url(Parameters(params)).await.unwrap(); @@ -53,3 +54,47 @@ async fn handler_returns_structured_url_and_text_content() { "expected one ContentBlock::text wrapping the next_step" ); } + +#[tokio::test] +async fn handler_rejects_assets_and_runs_without_the_opt_in() { + let server = server_for_explore(APP_URI).await; + let params = ExploreUrlParams { + assets: Some(vec![String::from("Engine-7")]), + runs: Some(vec![String::from("2025-thrust-test")]), + channels: None, + panel_type: None, + start_time_unix_nanos: None, + end_time_unix_nanos: None, + include_assets_and_runs: None, + }; + + let err = server.explore_url(Parameters(params)).await.unwrap_err(); + assert_eq!(err.code.0, -32602); + assert!( + err.message.contains("include_assets_and_runs"), + "the error should name the opt-in, got `{}`", + err.message + ); +} + +#[tokio::test] +async fn handler_keeps_both_source_types_when_the_opt_in_is_set() { + let server = server_for_explore(APP_URI).await; + let params = ExploreUrlParams { + assets: Some(vec![String::from("Engine-7")]), + runs: Some(vec![String::from("2025-thrust-test")]), + channels: None, + panel_type: None, + start_time_unix_nanos: None, + end_time_unix_nanos: None, + include_assets_and_runs: Some(true), + }; + + let result = server.explore_url(Parameters(params)).await.unwrap(); + assert_eq!( + structured_field(result, "url").as_str(), + Some( + "https://app.siftstack.com/explore?method=single&assets=Engine-7&runs=2025-thrust-test" + ) + ); +} From 140d0f3a7c18c0c35e9bcaa3a8a9d6b284498a40 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Thu, 27 Aug 2026 13:01:35 -0700 Subject: [PATCH 2/2] fix: require IDs for Explore links --- rust/crates/sift_cli/CHANGELOG.md | 9 +++-- .../skills/sift/references/explore-links.md | 7 ++-- rust/crates/sift_mcp/src/service/url/mod.rs | 26 ++++++------- rust/crates/sift_mcp/src/service/url/test.rs | 38 +++++++++---------- rust/crates/sift_mcp/src/tool/explore/mod.rs | 28 +++++++------- rust/crates/sift_mcp/src/tool/explore/test.rs | 29 +++++++++----- 6 files changed, 74 insertions(+), 63 deletions(-) diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index 8b1c16709..d7e4c9ea3 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -7,10 +7,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### What's New -- Explore links now carry one source type. The MCP `explore_url` tool rejects - `assets` and `runs` together with `INVALID_PARAMS` unless the new - `include_assets_and_runs` flag is set, so an agent no longer mixes an asset - and a run into one view on its own. `sift-cli import` links to the run it +- Explore links now carry one source type. The MCP `explore_url` tool accepts + only `asset_ids` and `run_ids`, and rejects them together with + `INVALID_PARAMS` unless the new `include_assets_and_runs` flag is set. This + prevents ambiguous name resolution and stops an agent from mixing an asset + and run into one view on its own. `sift-cli import` links to the run it imported into, and falls back to the asset only when there is no run. ## [v0.4.4] - August 24, 2026 diff --git a/rust/crates/sift_cli/assets/skills/sift/references/explore-links.md b/rust/crates/sift_cli/assets/skills/sift/references/explore-links.md index 8aa9a7bd4..ab571b87d 100644 --- a/rust/crates/sift_cli/assets/skills/sift/references/explore-links.md +++ b/rust/crates/sift_cli/assets/skills/sift/references/explore-links.md @@ -3,9 +3,10 @@ Build the link with `explore_url`, then surface the URL to the user as plain text, in full. -Send one source type. Pass `runs` when the request names a run, `assets` -otherwise. A run is already scoped to its asset, so adding the asset opens the -view on a second, wider source. `explore_url` rejects both with +Send one source type. Pass `run_ids` when the request names a run and +`asset_ids` otherwise. Use IDs returned by `list_runs` and `list_assets`; the +tool does not accept names. A run is already scoped to its asset, so adding the +asset opens the view on a second, wider source. `explore_url` rejects both with `INVALID_PARAMS`; set `include_assets_and_runs` to true only when the user asked to see runs and assets together in one view. diff --git a/rust/crates/sift_mcp/src/service/url/mod.rs b/rust/crates/sift_mcp/src/service/url/mod.rs index 1087afd5e..db33a9cae 100644 --- a/rust/crates/sift_mcp/src/service/url/mod.rs +++ b/rust/crates/sift_mcp/src/service/url/mod.rs @@ -22,8 +22,8 @@ const VALUE_ENCODE_SET: &percent_encoding::AsciiSet = &percent_encoding::NON_ALP #[derive(Debug, Default)] pub struct ExploreUrlRequest { - pub assets: Option>, - pub runs: Option>, + pub asset_ids: Option>, + pub run_ids: Option>, pub channels: Option>, pub panel_type: Option, pub start_time_unix_nanos: Option, @@ -43,8 +43,8 @@ impl UrlService { pub fn build_explore_url(&self, request: ExploreUrlRequest) -> Result { let ExploreUrlRequest { - assets, - runs, + asset_ids, + run_ids, channels, panel_type, start_time_unix_nanos, @@ -52,8 +52,8 @@ impl UrlService { include_assets_and_runs, } = request; - let has_assets = assets.as_ref().is_some_and(|v| !v.is_empty()); - let has_runs = runs.as_ref().is_some_and(|v| !v.is_empty()); + let has_assets = asset_ids.as_ref().is_some_and(|v| !v.is_empty()); + let has_runs = run_ids.as_ref().is_some_and(|v| !v.is_empty()); let no_selection = !has_assets && !has_runs @@ -71,11 +71,11 @@ impl UrlService { if has_assets && has_runs && !include_assets_and_runs { return Err(ErrorData::invalid_params( - "`assets` and `runs` were both set. An Explore link that mixes asset-scoped \ - and run-scoped sources is not the default: pass `runs` alone when the request \ - names a run, or `assets` alone otherwise. Set `include_assets_and_runs` to \ - true only when the user explicitly asked to see runs and assets together in \ - one view.", + "`asset_ids` and `run_ids` were both set. An Explore link that mixes \ + asset-scoped and run-scoped sources is not the default: pass `run_ids` alone \ + when the request names a run, or `asset_ids` alone otherwise. Set \ + `include_assets_and_runs` to true only when the user explicitly asked to see \ + runs and assets together in one view.", None, )); } @@ -104,11 +104,11 @@ impl UrlService { let host = self.app_host()?; let mut query = String::from("method=single"); - if let Some(v) = assets.as_ref().filter(|v| !v.is_empty()) { + if let Some(v) = asset_ids.as_ref().filter(|v| !v.is_empty()) { query.push_str("&assets="); query.push_str(&join_encoded(v)); } - if let Some(v) = runs.as_ref().filter(|v| !v.is_empty()) { + if let Some(v) = run_ids.as_ref().filter(|v| !v.is_empty()) { query.push_str("&runs="); query.push_str(&join_encoded(v)); } diff --git a/rust/crates/sift_mcp/src/service/url/test.rs b/rust/crates/sift_mcp/src/service/url/test.rs index 27e3fc13d..eb7c247c0 100644 --- a/rust/crates/sift_mcp/src/service/url/test.rs +++ b/rust/crates/sift_mcp/src/service/url/test.rs @@ -10,8 +10,8 @@ fn service() -> UrlService { fn full_url_with_all_params() { let url = service() .build_explore_url(ExploreUrlRequest { - assets: Some(vec![String::from("Engine-7")]), - runs: Some(vec![String::from("2025-thrust-test")]), + asset_ids: Some(vec![String::from("asset-id")]), + run_ids: Some(vec![String::from("run-id")]), channels: Some(vec![String::from("temperature"), String::from("pressure")]), panel_type: Some(String::from("scatter-plot")), start_time_unix_nanos: Some(0), @@ -22,8 +22,8 @@ fn full_url_with_all_params() { assert_eq!( url, "https://app.siftstack.com/explore?method=single\ - &assets=Engine-7\ - &runs=2025-thrust-test\ + &assets=asset-id\ + &runs=run-id\ &channels=temperature,pressure\ &panelType=scatter-plot\ &startTime=1970-01-01T00:00:00.000Z\ @@ -35,8 +35,8 @@ fn full_url_with_all_params() { fn assets_and_runs_together_are_rejected_by_default() { let err = service() .build_explore_url(ExploreUrlRequest { - assets: Some(vec![String::from("Engine-7")]), - runs: Some(vec![String::from("2025-thrust-test")]), + asset_ids: Some(vec![String::from("asset-id")]), + run_ids: Some(vec![String::from("run-id")]), ..Default::default() }) .unwrap_err(); @@ -52,8 +52,8 @@ fn assets_and_runs_together_are_rejected_by_default() { fn assets_and_runs_together_are_allowed_when_requested() { let url = service() .build_explore_url(ExploreUrlRequest { - assets: Some(vec![String::from("Engine-7")]), - runs: Some(vec![String::from("2025-thrust-test")]), + asset_ids: Some(vec![String::from("asset-id")]), + run_ids: Some(vec![String::from("run-id")]), include_assets_and_runs: true, ..Default::default() }) @@ -61,8 +61,8 @@ fn assets_and_runs_together_are_allowed_when_requested() { assert_eq!( url, "https://app.siftstack.com/explore?method=single\ - &assets=Engine-7\ - &runs=2025-thrust-test" + &assets=asset-id\ + &runs=run-id" ); } @@ -70,14 +70,14 @@ fn assets_and_runs_together_are_allowed_when_requested() { fn an_empty_asset_list_does_not_conflict_with_runs() { let url = service() .build_explore_url(ExploreUrlRequest { - assets: Some(vec![]), - runs: Some(vec![String::from("2025-thrust-test")]), + asset_ids: Some(vec![]), + run_ids: Some(vec![String::from("run-id")]), ..Default::default() }) .unwrap(); assert_eq!( url, - "https://app.siftstack.com/explore?method=single&runs=2025-thrust-test" + "https://app.siftstack.com/explore?method=single&runs=run-id" ); } @@ -85,14 +85,14 @@ fn an_empty_asset_list_does_not_conflict_with_runs() { fn the_opt_in_is_ignored_for_a_single_source_type() { let url = service() .build_explore_url(ExploreUrlRequest { - runs: Some(vec![String::from("2025-thrust-test")]), + run_ids: Some(vec![String::from("run-id")]), include_assets_and_runs: true, ..Default::default() }) .unwrap(); assert_eq!( url, - "https://app.siftstack.com/explore?method=single&runs=2025-thrust-test" + "https://app.siftstack.com/explore?method=single&runs=run-id" ); } @@ -128,7 +128,7 @@ fn comma_inside_single_value_is_encoded() { fn unknown_panel_type_is_rejected() { let err = service() .build_explore_url(ExploreUrlRequest { - assets: Some(vec![String::from("a")]), + asset_ids: Some(vec![String::from("asset-id")]), panel_type: Some(String::from("bogus")), ..Default::default() }) @@ -149,8 +149,8 @@ fn empty_request_is_rejected() { fn empty_vecs_are_treated_as_missing() { let err = service() .build_explore_url(ExploreUrlRequest { - assets: Some(vec![]), - runs: Some(vec![]), + asset_ids: Some(vec![]), + run_ids: Some(vec![]), channels: Some(vec![]), ..Default::default() }) @@ -163,7 +163,7 @@ fn configured_app_uri_trims_a_trailing_slash() { let svc = UrlService::new(String::from("https://sift.example.net/")); let url = svc .build_explore_url(ExploreUrlRequest { - assets: Some(vec![String::from("a")]), + asset_ids: Some(vec![String::from("asset-id")]), ..Default::default() }) .unwrap(); diff --git a/rust/crates/sift_mcp/src/tool/explore/mod.rs b/rust/crates/sift_mcp/src/tool/explore/mod.rs index b1a042405..86bf42c25 100644 --- a/rust/crates/sift_mcp/src/tool/explore/mod.rs +++ b/rust/crates/sift_mcp/src/tool/explore/mod.rs @@ -13,8 +13,8 @@ mod test; #[derive(Debug, Deserialize, JsonSchema)] pub struct ExploreUrlParams { - assets: Option>, - runs: Option>, + asset_ids: Option>, + run_ids: Option>, channels: Option>, panel_type: Option, start_time_unix_nanos: Option, @@ -36,12 +36,12 @@ impl SiftMcpServer { instructs you on how to surface it. Parameters: - - `assets`: optional list of asset names or UUIDs. The Explore service resolves either form. - - `runs`: optional list of run names or UUIDs. Same resolution rules as `assets`. + - `asset_ids`: optional list of asset IDs returned by `list_assets`. + - `run_ids`: optional list of run IDs returned by `list_runs`. - `include_assets_and_runs`: optional, defaults to false. A link carries one source type by - default: pass `runs` when the request names a run, `assets` otherwise. Setting both `assets` and - `runs` without this flag is rejected. Set it to true only when the user explicitly asked to see - runs and assets together in one view. Ignored unless both `assets` and `runs` are set. + default: pass `run_ids` when the request names a run, `asset_ids` otherwise. Setting both + `asset_ids` and `run_ids` without this flag is rejected. Set it to true only when the user + explicitly asked to see runs and assets together in one view. Ignored unless both are set. - `channels`: optional list of channel names, UUIDs, or prefixed forms. Axis prefixes (`L1:foo`, `L2:bar`) bind a channel to a Y-axis for multi-axis plots. Role prefixes (`x:foo`, `y:foo`, `color:foo` for scatter; `lat:foo`, `lon:foo`, `color:foo` for geo-map) bind a channel to a panel @@ -52,7 +52,7 @@ impl SiftMcpServer { for parity with `get_data`; the tool converts to ISO 8601 UTC for the URL. Errors: - `INVALID_PARAMS` if no selection or time parameter is set (the URL would be useless), if both - `assets` and `runs` are set without `include_assets_and_runs`, if `panel_type` is not in the + `asset_ids` and `run_ids` are set without `include_assets_and_runs`, if `panel_type` is not in the known set, or if `end_time_unix_nanos < start_time_unix_nanos`. Guidance: @@ -62,15 +62,15 @@ impl SiftMcpServer { - Do not add the asset alongside a run to be thorough. A run is already scoped to its asset, so the asset adds a second, wider source to the view. Send both only on an explicit request for both. - - The tool does not validate that the named asset/run/channel exists — Explore resolves at page - load. Use names you have already retrieved from `list_*` tools to avoid 404s on click. + - The tool does not validate that the provided asset/run/channel exists — Explore resolves at page + load. Use IDs and channel names you have already retrieved from `list_*` tools to avoid 404s. ", annotations(title = "explore/explore_url", read_only_hint = true) )] pub async fn explore_url(&self, params: Parameters) -> error::McpResult { let Parameters(ExploreUrlParams { - assets, - runs, + asset_ids, + run_ids, channels, panel_type, start_time_unix_nanos, @@ -79,8 +79,8 @@ impl SiftMcpServer { }) = params; let url = self.url_service.build_explore_url(ExploreUrlRequest { - assets, - runs, + asset_ids, + run_ids, channels, panel_type, start_time_unix_nanos, diff --git a/rust/crates/sift_mcp/src/tool/explore/test.rs b/rust/crates/sift_mcp/src/tool/explore/test.rs index 99bd4c8e3..b67788304 100644 --- a/rust/crates/sift_mcp/src/tool/explore/test.rs +++ b/rust/crates/sift_mcp/src/tool/explore/test.rs @@ -23,12 +23,23 @@ fn structured_field(result: rmcp::model::CallToolResult, key: &str) -> Value { .take() } +#[test] +fn schema_requires_source_ids() { + let schema = serde_json::to_value(schemars::schema_for!(ExploreUrlParams)).unwrap(); + let properties = schema["properties"].as_object().unwrap(); + + assert!(properties.contains_key("asset_ids")); + assert!(properties.contains_key("run_ids")); + assert!(!properties.contains_key("assets")); + assert!(!properties.contains_key("runs")); +} + #[tokio::test] async fn handler_returns_structured_url_and_text_content() { let server = server_for_explore(APP_URI).await; let params = ExploreUrlParams { - assets: Some(vec![String::from("Engine-7")]), - runs: None, + asset_ids: Some(vec![String::from("asset-id")]), + run_ids: None, channels: None, panel_type: None, start_time_unix_nanos: None, @@ -37,7 +48,7 @@ async fn handler_returns_structured_url_and_text_content() { }; let result = server.explore_url(Parameters(params)).await.unwrap(); - let expected_url = "https://app.siftstack.com/explore?method=single&assets=Engine-7"; + let expected_url = "https://app.siftstack.com/explore?method=single&assets=asset-id"; let url = structured_field(result.clone(), "url"); assert_eq!(url.as_str(), Some(expected_url)); @@ -59,8 +70,8 @@ async fn handler_returns_structured_url_and_text_content() { async fn handler_rejects_assets_and_runs_without_the_opt_in() { let server = server_for_explore(APP_URI).await; let params = ExploreUrlParams { - assets: Some(vec![String::from("Engine-7")]), - runs: Some(vec![String::from("2025-thrust-test")]), + asset_ids: Some(vec![String::from("asset-id")]), + run_ids: Some(vec![String::from("run-id")]), channels: None, panel_type: None, start_time_unix_nanos: None, @@ -81,8 +92,8 @@ async fn handler_rejects_assets_and_runs_without_the_opt_in() { async fn handler_keeps_both_source_types_when_the_opt_in_is_set() { let server = server_for_explore(APP_URI).await; let params = ExploreUrlParams { - assets: Some(vec![String::from("Engine-7")]), - runs: Some(vec![String::from("2025-thrust-test")]), + asset_ids: Some(vec![String::from("asset-id")]), + run_ids: Some(vec![String::from("run-id")]), channels: None, panel_type: None, start_time_unix_nanos: None, @@ -93,8 +104,6 @@ async fn handler_keeps_both_source_types_when_the_opt_in_is_set() { let result = server.explore_url(Parameters(params)).await.unwrap(); assert_eq!( structured_field(result, "url").as_str(), - Some( - "https://app.siftstack.com/explore?method=single&assets=Engine-7&runs=2025-thrust-test" - ) + Some("https://app.siftstack.com/explore?method=single&assets=asset-id&runs=run-id") ); }