fix(cli): propagate benchmark failures and isolate benchmark runs

memoria/crates/memoria-cli/src/benchmark.rs

@@ -22,6 +22,27 @@ pub use benchmark_taxonomy::{official_category, scenario_question_type, scenario
 
 use std::collections::HashSet;
 
+/// Scenario IDs that collapse to the same execution namespace.
+///
+/// The executor lowercases `scenario_id` to build each scenario's user and
+/// session namespace, so `CaseA` and `casea` are distinct to a case-sensitive
+/// uniqueness check yet share one namespace at run time — letting one
+/// scenario's writes, purges and corrections hit the other. Uniqueness has to
+/// be judged on the normalized form the executor actually uses.
+pub fn colliding_scenario_namespaces(scenarios: &[Scenario]) -> Vec<String> {
+    let mut seen = HashSet::new();
+    let mut errors = Vec::new();
+    for s in scenarios {
+        if !seen.insert(s.scenario_id.to_lowercase()) {
+            errors.push(format!(
+                "duplicate scenario_id (case-insensitive): {}",
+                s.scenario_id
+            ));
+        }
+    }
+    errors
+}
+
 pub fn validate_dataset(content: &str) -> Vec<String> {
     let mut errors = vec![];
     let dataset: ScenarioDataset = match serde_json::from_str(content) {
@@ -31,11 +52,8 @@ pub fn validate_dataset(content: &str) -> Vec<String> {
             return errors;
         }
     };
-    let mut ids = HashSet::new();
+    errors.extend(colliding_scenario_namespaces(&dataset.scenarios));
     for s in &dataset.scenarios {
-        if !ids.insert(&s.scenario_id) {
-            errors.push(format!("duplicate scenario_id: {}", s.scenario_id));
-        }
         if s.seed_memories.is_empty() {
             errors.push(format!("{}: no seed_memories", s.scenario_id));
         }
@@ -127,9 +145,9 @@ mod tests {
             _scenario_id: id.into(),
             step_results: vec![],
             assertion_results: vec![AssertionResult {
-                _query: "query".into(),
+                query: "query".into(),
                 returned_contents: vec!["memory".into()],
-                _error: None,
+                error: None,
             }],
             error: None,
         }
@@ -154,9 +172,9 @@ mod tests {
                     _scenario_id: "lme-2".into(),
                     step_results: vec![],
                     assertion_results: vec![AssertionResult {
-                        _query: "query".into(),
+                        query: "query".into(),
                         returned_contents: vec![],
-                        _error: None,
+                        error: None,
                     }],
                     error: None,
                 },
@@ -198,4 +216,96 @@ mod tests {
             Some("Instruction Following")
         );
     }
+
+    // ── scenario_id uniqueness must match the execution namespace ────────────
+
+    fn dataset_json(ids: &[&str]) -> String {
+        let scenarios: Vec<_> = ids
+            .iter()
+            .map(|id| {
+                json!({
+                    "scenario_id": id,
+                    "title": "t",
+                    "difficulty": "L1",
+                    "horizon": "short",
+                    "tags": [],
+                    "seed_memories": [{"content": "memory"}],
+                    "assertions": [{"query": "q", "expected_contents": ["memory"]}],
+                })
+            })
+            .collect();
+        json!({"dataset_id": "d", "version": "1", "scenarios": scenarios}).to_string()
+    }
+
+    #[test]
+    fn validate_rejects_case_distinct_scenario_ids_that_share_a_namespace() {
+        let errors = validate_dataset(&dataset_json(&["CaseA", "casea"]));
+        assert!(
+            errors.iter().any(|e| e.contains("duplicate scenario_id")),
+            "case-distinct ids collide in the execution namespace: {errors:?}"
+        );
+    }
+
+    #[test]
+    fn validate_accepts_genuinely_distinct_scenario_ids() {
+        assert!(validate_dataset(&dataset_json(&["case-a", "case-b"])).is_empty());
+    }
+
+    #[test]
+    fn colliding_namespaces_are_detectable_without_full_validation() {
+        // cmd_benchmark enforces this on every run, not just --validate-only,
+        // so the check has to be reachable independently of validate_dataset.
+        let collide: ScenarioDataset =
+            serde_json::from_str(&dataset_json(&["CaseA", "casea"])).unwrap();
+        let distinct: ScenarioDataset =
+            serde_json::from_str(&dataset_json(&["case-a", "case-b"])).unwrap();
+        assert_eq!(colliding_scenario_namespaces(&collide.scenarios).len(), 1);
+        assert!(colliding_scenario_namespaces(&distinct.scenarios).is_empty());
+    }
+
+    // ── a failed retrieval is not an empty-but-successful one ────────────────
+
+    #[test]
+    fn errored_assertion_does_not_pass_on_empty_expectations() {
+        let mut scenario = scenario_with_metadata("err-1", "beam", "event_ordering");
+        // No expected/excluded contents: scoring an errored retrieval like a valid
+        // empty answer would hand this a free pass (recall and noise both 100).
+        scenario.assertions = vec![MemoryAssertion {
+            query: "query".into(),
+            top_k: 3,
+            expected_contents: vec![],
+            excluded_contents: vec![],
+        }];
+
+        let errored_exec = ScenarioExecution {
+            _scenario_id: "err-1".into(),
+            step_results: vec![],
+            assertion_results: vec![AssertionResult {
+                query: "query".into(),
+                returned_contents: vec![],
+                error: Some("HTTP status server error (500)".into()),
+            }],
+            error: None,
+        };
+        let empty_exec = ScenarioExecution {
+            _scenario_id: "err-1".into(),
+            step_results: vec![],
+            assertion_results: vec![AssertionResult {
+                query: "query".into(),
+                returned_contents: vec![],
+                error: None,
+            }],
+            error: None,
+        };
+
+        let errored = score_scenario(&scenario, &errored_exec);
+        let empty = score_scenario(&scenario, &empty_exec);
+        assert_eq!(errored.aus_assertion_pass, 0.0);
+        assert_eq!(errored.mqs_recall, 0.0);
+        assert_eq!(errored.mqs_noise_rejection, 0.0);
+        assert_eq!(
+            empty.aus_assertion_pass, 100.0,
+            "a genuinely empty answer still scores on its own merits"
+        );
+    }
 }

memoria/crates/memoria-cli/src/benchmark_executor.rs

@@ -13,10 +13,14 @@ pub struct BenchmarkExecutor {
 
 impl BenchmarkExecutor {
     pub fn new(api_url: &str, token: &str) -> Self {
-        let run_id = SystemTime::now()
+        // A second-granularity timestamp alone collides: independent runs started
+        // in the same second share a namespace and can purge/correct each other's
+        // memories. Mix in random bits so each run is isolated.
+        let secs = SystemTime::now()
             .duration_since(UNIX_EPOCH)
-            .map(|d| d.as_secs().to_string())
+            .map(|d| d.as_secs())
             .unwrap_or_default();
+        let run_id = format!("{secs}-{:08x}", fastrand::u32(..));
         Self {
             base_url: api_url.trim_end_matches('/').into(),
             token: token.into(),
@@ -131,32 +135,52 @@ impl BenchmarkExecutor {
         let resp = client
             .post(format!("{}/v1/memories", self.base_url))
             .json(&body)
-            .send()?;
+            .send()?
+            .error_for_status()?;
         let data: Value = resp.json()?;
-        Ok(data["memory_id"].as_str().unwrap_or("").to_string())
+        match data["memory_id"].as_str() {
+            Some(id) if !id.is_empty() => Ok(id.to_string()),
+            // A 2xx without a memory_id is a response-shape failure, not a write
+            // that happened to return nothing.
+            _ => anyhow::bail!("store response carried no memory_id"),
+        }
     }
 
-    fn retrieve(&self, client: &Client, query: &str, session_id: &str, top_k: i64) -> Vec<String> {
+    /// `Ok(vec![])` means the service answered with no matches; an `Err` means the
+    /// query never produced an answer. Collapsing both into an empty vector hid
+    /// service failures behind valid-looking empty retrievals.
+    fn retrieve(
+        &self,
+        client: &Client,
+        query: &str,
+        session_id: &str,
+        top_k: i64,
+    ) -> anyhow::Result<Vec<String>> {
         let resp = client
             .post(format!("{}/v1/memories/retrieve", self.base_url))
             .json(&json!({"query": query, "top_k": top_k, "session_id": session_id}))
-            .send();
-        let data: Value = match resp.and_then(|r| r.json()) {
-            Ok(v) => v,
-            Err(_) => return vec![],
-        };
+            .send()?
+            .error_for_status()?;
+        let data: Value = resp.json()?;
         let items = if data.is_array() {
             data.as_array()
         } else {
             data["results"].as_array()
         };
+        let items = items.ok_or_else(|| {
+            anyhow::anyhow!("retrieve response was neither an array nor {{\"results\": [...]}}")
+        })?;
+        // filter_map would silently drop an entry whose content is missing or not
+        // a string, so a malformed backend response could still score full marks.
         items
-            .map(|arr| {
-                arr.iter()
-                    .filter_map(|i| i["content"].as_str().map(String::from))
-                    .collect()
+            .iter()
+            .enumerate()
+            .map(|(i, item)| {
+                item["content"].as_str().map(String::from).ok_or_else(|| {
+                    anyhow::anyhow!("retrieve result [{i}] has no string \"content\" field")
+                })
             })
-            .unwrap_or_default()
+            .collect()
     }
 
     fn run_step(&self, client: &Client, step: &ScenarioStep, session_id: &str) -> StepResult {
@@ -180,7 +204,7 @@ impl BenchmarkExecutor {
                         step.query.as_deref().unwrap_or(""),
                         session_id,
                         step.top_k.unwrap_or(5),
-                    );
+                    )?;
                 }
                 "search" => {
                     client
@@ -214,14 +238,14 @@ impl BenchmarkExecutor {
         })();
         match result {
             Ok(()) => StepResult {
-                _action: action,
+                action,
                 success: true,
-                _error: None,
+                error: None,
             },
             Err(e) => StepResult {
-                _action: action,
+                action,
                 success: false,
-                _error: Some(e.to_string()),
+                error: Some(format!("{e:#}")),
             },
         }
     }
@@ -232,11 +256,17 @@ impl BenchmarkExecutor {
         assertion: &MemoryAssertion,
         session_id: &str,
     ) -> AssertionResult {
-        let contents = self.retrieve(client, &assertion.query, session_id, assertion.top_k);
-        AssertionResult {
-            _query: assertion.query.clone(),
-            returned_contents: contents,
-            _error: None,
+        match self.retrieve(client, &assertion.query, session_id, assertion.top_k) {
+            Ok(contents) => AssertionResult {
+                query: assertion.query.clone(),
+                returned_contents: contents,
+                error: None,
+            },
+            Err(e) => AssertionResult {
+                query: assertion.query.clone(),
+                returned_contents: vec![],
+                error: Some(format!("{e:#}")),
+            },
         }
     }
 

memoria/crates/memoria-cli/src/benchmark_schema.rs

@@ -80,15 +80,15 @@ fn default_top_k() -> i64 {
 }
 
 pub struct StepResult {
-    pub _action: String,
+    pub action: String,
     pub success: bool,
-    pub _error: Option<String>,
+    pub error: Option<String>,
 }
 
 pub struct AssertionResult {
-    pub _query: String,
+    pub query: String,
     pub returned_contents: Vec<String>,
-    pub _error: Option<String>,
+    pub error: Option<String>,
 }
 
 pub struct ScenarioExecution {

memoria/crates/memoria-cli/src/benchmark_scoring.rs

@@ -88,12 +88,17 @@ pub fn score_scenario(scenario: &Scenario, exec: &ScenarioExecution) -> Scenario
     let mut noises = vec![];
     let mut passed_count = 0usize;
     for (i, assertion) in scenario.assertions.iter().enumerate() {
-        let returned = exec
-            .assertion_results
-            .get(i)
-            .map(|r| r.returned_contents.as_slice())
-            .unwrap_or(&[]);
-        let (p, r, n, ok) = score_contents(assertion, returned);
+        let result = exec.assertion_results.get(i);
+        // A retrieval that failed tells us nothing about recall or noise. Scoring
+        // it like an empty-but-successful answer would let an assertion with no
+        // expected_contents pass on a service failure.
+        let (p, r, n, ok) = match result {
+            Some(result) if result.error.is_some() => (0.0, 0.0, 0.0, false),
+            _ => score_contents(
+                assertion,
+                result.map(|r| r.returned_contents.as_slice()).unwrap_or(&[]),
+            ),
+        };
         precisions.push(p);
         recalls.push(r);
         noises.push(n);