From 19173774e37002ee747fcdba2dd76e08cbeb2289 Mon Sep 17 00:00:00 2001 From: Miguel de Benito Delgado Date: Sat, 15 Aug 2026 14:09:20 +0200 Subject: [PATCH] Add a `--trigger-mode full|delta` flag to mental-models update --- hindsight-cli/.openapi-coverage.toml | 2 +- hindsight-cli/src/commands/mental_model.rs | 163 ++++++++++++++++++--- hindsight-cli/src/main.rs | 8 + hindsight-cli/tests/cli_integration.rs | 134 +++++++++++++++++ 4 files changed, 287 insertions(+), 20 deletions(-) diff --git a/hindsight-cli/.openapi-coverage.toml b/hindsight-cli/.openapi-coverage.toml index c47e1e7eb8..5569bcc0a7 100644 --- a/hindsight-cli/.openapi-coverage.toml +++ b/hindsight-cli/.openapi-coverage.toml @@ -149,4 +149,4 @@ trigger = "Exposed as --mode / --fact-types on `knowledge-base create-page`; the trigger = "Exposed as --trigger-refresh-after-consolidation on `mental-model create` (other nested trigger fields like fact_types/tag_groups are not exposed yet)." [fields.update_mental_model] -trigger = "Exposed as --trigger-refresh-after-consolidation on `mental-model update` (other nested trigger fields like fact_types/tag_groups are not exposed yet)." +trigger = "Exposed as --trigger-refresh-after-consolidation and --trigger-mode on `mental-model update` (other nested trigger fields like fact_types/tag_groups are not exposed yet)." diff --git a/hindsight-cli/src/commands/mental_model.rs b/hindsight-cli/src/commands/mental_model.rs index 57ed274d8d..58109b566a 100644 --- a/hindsight-cli/src/commands/mental_model.rs +++ b/hindsight-cli/src/commands/mental_model.rs @@ -23,6 +23,47 @@ fn parse_tags_match(value: &str) -> Result { }) } +/// Parse a --trigger-mode value into the generated refresh-mode enum +fn parse_trigger_mode(value: &str) -> Result { + Ok(match value.to_lowercase().as_str() { + "full" => types::Mode::Full, + "delta" => types::Mode::Delta, + other => anyhow::bail!("invalid --trigger-mode '{other}': expected one of full, delta"), + }) +} + +/// Build the trigger override sent on update when at least one trigger flag +/// was passed. `None` (no trigger flags) leaves the stored trigger config +/// untouched on the server. When an override IS sent, the server replaces the +/// whole trigger, so fields the user did not specify fall back to their +/// defaults. +fn build_trigger_override( + trigger_mode: Option<&str>, + trigger_refresh_after_consolidation: Option, +) -> Result> { + if trigger_mode.is_none() && trigger_refresh_after_consolidation.is_none() { + return Ok(None); + } + Ok(Some(types::MentalModelTriggerInput { + mode: trigger_mode + .map(parse_trigger_mode) + .transpose()? + .unwrap_or(types::Mode::Full), + refresh_after_consolidation: trigger_refresh_after_consolidation.unwrap_or(false), + refresh_cron: None, + exclude_mental_models: false, + exclude_mental_model_ids: None, + fact_types: None, + tag_groups: None, + tags_match: None, + include_chunks: None, + recall_max_tokens: None, + recall_chunks_max_tokens: None, + response_schema: None, + keep_trace: false, + })) +} + /// List mental models for a bank pub fn list( client: &ApiClient, @@ -194,6 +235,7 @@ pub fn update( max_tokens: Option, tags: Option>, trigger_refresh_after_consolidation: Option, + trigger_mode: Option<&str>, verbose: bool, output_format: OutputFormat, ) -> Result<()> { @@ -202,10 +244,11 @@ pub fn update( && max_tokens.is_none() && tags.is_none() && trigger_refresh_after_consolidation.is_none() + && trigger_mode.is_none() { anyhow::bail!( - "At least one of --name, --source-query, --max-tokens, --tags, or \ - --trigger-refresh-after-consolidation must be provided" + "At least one of --name, --source-query, --max-tokens, --tags, \ + --trigger-mode, or --trigger-refresh-after-consolidation must be provided" ); } @@ -215,23 +258,11 @@ pub fn update( None }; - // Only build a trigger override when the user actually passed the flag; - // sending None leaves the existing trigger config untouched on the server. - let trigger = trigger_refresh_after_consolidation.map(|refresh| types::MentalModelTriggerInput { - mode: types::Mode::Full, - refresh_after_consolidation: refresh, - refresh_cron: None, - exclude_mental_models: false, - exclude_mental_model_ids: None, - fact_types: None, - tag_groups: None, - tags_match: None, - include_chunks: None, - recall_max_tokens: None, - recall_chunks_max_tokens: None, - response_schema: None, - keep_trace: false, - }); + // Only build a trigger override when the user actually passed a trigger + // flag; sending None leaves the existing trigger config untouched on the + // server. When an override IS sent, the server replaces the whole trigger, + // so fields the user did not specify fall back to their defaults. + let trigger = build_trigger_override(trigger_mode, trigger_refresh_after_consolidation)?; let request = types::UpdateMentalModelRequest { name, @@ -533,4 +564,98 @@ mod tests { let err = parse_tags_match("most").unwrap_err().to_string(); assert!(err.contains("invalid --tags-match 'most'"), "unexpected error: {err}"); } + + #[test] + fn parse_trigger_mode_accepts_both_modes() { + assert_eq!(parse_trigger_mode("full").unwrap(), types::Mode::Full); + assert_eq!(parse_trigger_mode("delta").unwrap(), types::Mode::Delta); + } + + #[test] + fn parse_trigger_mode_is_case_insensitive() { + assert_eq!(parse_trigger_mode("DELTA").unwrap(), types::Mode::Delta); + } + + #[test] + fn parse_trigger_mode_rejects_unknown() { + let err = parse_trigger_mode("incremental").unwrap_err().to_string(); + assert!( + err.contains("invalid --trigger-mode 'incremental'"), + "unexpected error: {err}" + ); + } + + #[test] + fn build_trigger_override_none_when_no_flags() { + assert!(build_trigger_override(None, None).unwrap().is_none()); + } + + #[test] + fn build_trigger_override_mode_only() { + let trigger = build_trigger_override(Some("delta"), None) + .unwrap() + .unwrap(); + assert_eq!(trigger.mode, types::Mode::Delta); + assert!(!trigger.refresh_after_consolidation); + } + + #[test] + fn build_trigger_override_refresh_only_defaults_to_full_mode() { + let trigger = build_trigger_override(None, Some(true)).unwrap().unwrap(); + assert_eq!(trigger.mode, types::Mode::Full); + assert!(trigger.refresh_after_consolidation); + + // Explicit false must stay distinguishable from omission. + let trigger = build_trigger_override(None, Some(false)).unwrap().unwrap(); + assert_eq!(trigger.mode, types::Mode::Full); + assert!(!trigger.refresh_after_consolidation); + } + + #[test] + fn build_trigger_override_both_flags() { + let trigger = build_trigger_override(Some("FULL"), Some(true)) + .unwrap() + .unwrap(); + assert_eq!(trigger.mode, types::Mode::Full); + assert!(trigger.refresh_after_consolidation); + } + + #[test] + fn build_trigger_override_propagates_parse_error() { + let err = build_trigger_override(Some("incremental"), None) + .unwrap_err() + .to_string(); + assert!( + err.contains("invalid --trigger-mode 'incremental'"), + "unexpected error: {err}" + ); + } + + #[test] + fn update_request_serializes_delta_trigger() { + let request = types::UpdateMentalModelRequest { + name: None, + source_query: None, + max_tokens: None, + tags: None, + trigger: build_trigger_override(Some("delta"), Some(true)).unwrap(), + }; + let value = serde_json::to_value(&request).unwrap(); + assert_eq!(value["trigger"]["mode"], "delta"); + assert_eq!(value["trigger"]["refresh_after_consolidation"], true); + } + + #[test] + fn update_request_omits_trigger_when_no_flags() { + let request = types::UpdateMentalModelRequest { + name: Some("renamed".to_string()), + source_query: None, + max_tokens: None, + tags: None, + trigger: build_trigger_override(None, None).unwrap(), + }; + let value = serde_json::to_value(&request).unwrap(); + assert_eq!(value["name"], "renamed"); + assert!(value.get("trigger").is_none() || value["trigger"].is_null()); + } } diff --git a/hindsight-cli/src/main.rs b/hindsight-cli/src/main.rs index 2d9c15d521..cc9915344a 100644 --- a/hindsight-cli/src/main.rs +++ b/hindsight-cli/src/main.rs @@ -1073,6 +1073,12 @@ enum MentalModelCommands { /// Enable/disable automatic refresh after observations consolidation #[arg(long)] trigger_refresh_after_consolidation: Option, + + /// Refresh mode: full or delta. Passing a trigger flag replaces the + /// stored trigger configuration: trigger settings you do not pass + /// reset to their API defaults. + #[arg(long)] + trigger_mode: Option, }, /// Delete a mental model @@ -1851,6 +1857,7 @@ fn run() -> Result<()> { max_tokens, tags, trigger_refresh_after_consolidation, + trigger_mode, } => commands::mental_model::update( &client, &bank_id, @@ -1860,6 +1867,7 @@ fn run() -> Result<()> { max_tokens, tags, trigger_refresh_after_consolidation, + trigger_mode.as_deref(), verbose, output_format, ), diff --git a/hindsight-cli/tests/cli_integration.rs b/hindsight-cli/tests/cli_integration.rs index f27067eb43..356c50cba1 100644 --- a/hindsight-cli/tests/cli_integration.rs +++ b/hindsight-cli/tests/cli_integration.rs @@ -741,6 +741,140 @@ fn test_mental_model_update() { let _ = run_hindsight(&["bank", "delete", &bank_id, "-y"]); } +#[test] +fn test_mental_model_update_trigger_mode() { + skip_if_no_server!(); + + let bank_id = test_bank_id("mm-trigger-mode"); + + // Create the bank first + let output = run_hindsight(&["bank", "create", &bank_id, "--name", "Test Bank"]); + assert!( + output.status.success(), + "bank create failed: stderr={}", + String::from_utf8_lossy(&output.stderr) + ); + + // Create a mental model + let output = run_hindsight(&[ + "mental-model", + "create", + &bank_id, + "Test Trigger Mode Model", + "What are the key facts?", + ]); + assert!( + output.status.success(), + "mental-model create failed: stdout={}, stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + // List to get the ID + let output = run_hindsight(&["mental-model", "list", &bank_id, "-o", "json"]); + assert!( + output.status.success(), + "mental-model list failed: stderr={}", + String::from_utf8_lossy(&output.stderr) + ); + let result: serde_json::Value = serde_json::from_str(&String::from_utf8_lossy(&output.stdout)) + .expect("list output is valid JSON"); + let id = result + .get("items") + .and_then(|v| v.as_array()) + .expect("list response has items") + .iter() + .find(|item| item.get("name").and_then(|v| v.as_str()) == Some("Test Trigger Mode Model")) + .expect("created mental model appears in list") + .get("id") + .and_then(|v| v.as_str()) + .expect("mental model has an id") + .to_string(); + + // Assert the mode currently stored on the server via `get -o json`. + let assert_stored_mode = |expected: &str| { + let output = run_hindsight(&["mental-model", "get", &bank_id, &id, "-o", "json"]); + assert!( + output.status.success(), + "mental-model get failed: stderr={}", + String::from_utf8_lossy(&output.stderr) + ); + let result: serde_json::Value = + serde_json::from_str(&String::from_utf8_lossy(&output.stdout)) + .expect("get output is valid JSON"); + assert_eq!( + result + .get("trigger") + .and_then(|t| t.get("mode")) + .and_then(|v| v.as_str()), + Some(expected), + "stored trigger mode should be {expected}: {result}" + ); + }; + + // Switch the trigger to delta and verify it round-trips. + let output = run_hindsight(&[ + "mental-model", + "update", + &bank_id, + &id, + "--trigger-mode", + "delta", + ]); + assert!( + output.status.success(), + "update --trigger-mode delta failed: stdout={}, stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert_stored_mode("delta"); + + // And back to full. + let output = run_hindsight(&[ + "mental-model", + "update", + &bank_id, + &id, + "--trigger-mode", + "full", + ]); + assert!( + output.status.success(), + "update --trigger-mode full failed: stdout={}, stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert_stored_mode("full"); + + // Unknown modes must fail loudly. + let output = run_hindsight(&[ + "mental-model", + "update", + &bank_id, + &id, + "--trigger-mode", + "incremental", + ]); + assert!( + !output.status.success(), + "update --trigger-mode incremental should fail" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("invalid --trigger-mode 'incremental'"), + "unexpected stderr: {}", + stderr + ); + + // Clean up + let output = run_hindsight(&["bank", "delete", &bank_id, "-y"]); + assert!( + output.status.success(), + "bank delete failed: stderr={}", + String::from_utf8_lossy(&output.stderr) + ); +} + #[test] fn test_mental_model_refresh() { skip_if_no_server!();