From 11bbc49840839e460ee495058fb2b0c840b73134 Mon Sep 17 00:00:00 2001 From: serrrfirat Date: Wed, 12 Aug 2026 20:15:02 +0300 Subject: [PATCH 1/3] fix(memory): rank memory retrieval by relevance instead of requiring every term MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Memory recall from a conversation almost never matched. `Filter::Fts` builds an AND over every non-stopword term of the raw user message, so a question worded even slightly differently from the saved sentence returned nothing: one missing word was enough. A fact saved as "Sarah prefers the standup meeting scheduled early on Thursday mornings" was invisible to "when does Sarah like her standup scheduled" purely because the stored text has no "like". Add an explicit ranked retrieval mode rather than flipping AND to OR for everyone. `Filter::FtsRanked { key, query, limit }` matches a record carrying ANY content term and orders by backend relevance — `bm25()` on libSQL, `ts_rank` over an OR `tsquery` on PostgreSQL, and distinct-term coverage in the in-memory reference so the three stay behaviorally consistent. Like `Filter::VectorNearest` it is a top-k operation: `limit` truncates after ranking and nesting it inside And/Or is `Unsupported` on every backend, because a predicate position would discard the ordering. `Filter::Fts` keeps its every-term semantics untouched. Its only production consumer is memory-native's search path, which moves to the ranked variant; the remaining uses are the filesystem crate's own contract tests. Tests: a three-backend `ranked_fts_contract` in the filesystem contract suite (libSQL + in-memory run locally, Postgres leg is docker-gated and unrun here) that opens by asserting the AND filter finds nothing, so it cannot pass under the old semantics; and a group_memory integration scenario driving the real composition, verified to fail on the previous behavior with "no captured system prompt containing \"Thursday mornings\"". It writes to a non-standing document so the always-on MEMORY.md lane from #7365 cannot satisfy it. Part of #7185, #7275 Co-Authored-By: Claude Fable 5 --- .../memory-native/src/repo/filesystem.rs | 11 +- .../ironclaw_filesystem/src/in_memory.rs | 93 +++++++++-- .../ironclaw_filesystem/src/index.rs | 28 ++++ .../ironclaw_filesystem/src/libsql.rs | 157 +++++++++++++---- .../ironclaw_filesystem/src/postgres.rs | 120 ++++++++++--- .../tests/db_root_filesystem_contract.rs | 158 ++++++++++++++++++ tests/integration/group_memory/main.rs | 10 ++ ...enario_paraphrased_prompt_recall_libsql.rs | 77 +++++++++ 8 files changed, 578 insertions(+), 76 deletions(-) create mode 100644 tests/integration/group_memory/scenario_paraphrased_prompt_recall_libsql.rs diff --git a/crates/extensions/packages/memory-native/src/repo/filesystem.rs b/crates/extensions/packages/memory-native/src/repo/filesystem.rs index f4d9686f572..15278a0f8b1 100644 --- a/crates/extensions/packages/memory-native/src/repo/filesystem.rs +++ b/crates/extensions/packages/memory-native/src/repo/filesystem.rs @@ -932,10 +932,19 @@ where .await?; let full_text_results = if request.full_text() { - let filter = Filter::Fts { + // #7185: memory recall is conversational — the query is a whole + // user sentence, and requiring EVERY content term (`Filter::Fts`) + // means a paraphrased question almost never matches the stored + // wording. `Filter::FtsRanked` matches on any content term and + // orders by backend relevance (bm25 / ts_rank), which is what the + // rank-then-fuse step below already assumes it is being handed. + let filter = Filter::FtsRanked { key: Self::index_key(fs_keys::CONTENT), query: request.query().to_string(), + limit: request.pre_fusion_limit().min(Page::MAX_LIMIT as usize) as u32, }; + // `FtsRanked` overrides `Page::limit` (it is a top-k operation); + // the bounded page still satisfies the API. let page = Page::new( 0, request.pre_fusion_limit().min(Page::MAX_LIMIT as usize) as u32, diff --git a/crates/substrates/ironclaw_filesystem/src/in_memory.rs b/crates/substrates/ironclaw_filesystem/src/in_memory.rs index 77cfa243b2d..1b2150850c6 100644 --- a/crates/substrates/ironclaw_filesystem/src/in_memory.rs +++ b/crates/substrates/ironclaw_filesystem/src/in_memory.rs @@ -303,16 +303,46 @@ impl RootFilesystem for InMemoryBackend { }) .collect()); } - // Audit finding F5: a `Filter::VectorNearest` nested inside - // `And`/`Or` is `Unsupported` on both SQL backends (the WHERE- - // fragment translator refuses to inline a ranking op as a - // predicate; the top of `query` only handles a top-level - // `VectorNearest`). The in-memory backend previously treated a - // nested `VectorNearest` as "any row with `IndexValue::Bytes` at - // `key`", silently changing semantics across backends. Align by + // `Filter::FtsRanked` is the second ranking operation: OR over the + // query's content terms, ordered by relevance. The reference score is + // term coverage (how many DISTINCT query terms the record carries), + // tie-broken by path so the reference is deterministic. That is a + // coarse stand-in for `bm25()` / `ts_rank`, but the ordering contract + // the three backends share — a record matching more query terms + // outranks one matching fewer — holds on all of them. + if let Some((key, query, limit)) = top_level_fts_ranked(filter) { + let terms = crate::index::plain_fts_terms(query); + let mut ranked: Vec<(&VirtualPath, &StoredEntry, usize)> = candidates + .into_iter() + .filter_map(|(candidate_path, stored)| { + let Some(IndexValue::Text(stored_text)) = stored.entry.indexed.get(key) else { + return None; + }; + let score = fts_term_coverage(stored_text, &terms); + (score > 0).then_some((candidate_path, stored, score)) + }) + .collect(); + ranked.sort_by(|a, b| b.2.cmp(&a.2).then_with(|| a.0.as_str().cmp(b.0.as_str()))); + ranked.truncate(limit as usize); + return Ok(ranked + .into_iter() + .map(|(matched_path, stored, _)| VersionedEntry { + path: matched_path.clone(), + entry: stored.entry.clone(), + version: stored.version, + }) + .collect()); + } + // Audit finding F5: a ranking filter (`Filter::VectorNearest`, and + // since #7185 `Filter::FtsRanked`) nested inside `And`/`Or` is + // `Unsupported` on both SQL backends (the WHERE-fragment translator + // refuses to inline a ranking op as a predicate; the top of `query` + // only handles the top-level shape). The in-memory backend previously + // treated a nested `VectorNearest` as "any row with `IndexValue::Bytes` + // at `key`", silently changing semantics across backends. Align by // surfacing the same `Unsupported` error before the scalar // filter loop runs. - if contains_nested_vector_nearest(filter) { + if contains_nested_ranking_filter(filter) { return Err(FilesystemError::Unsupported { path: path.clone(), operation: FilesystemOperation::Query, @@ -829,7 +859,10 @@ fn filter_matches( // `false` (the conservative answer; the caller already errored) so // we don't fall through to "match any row with a bytes value at // key" the way prior versions did. - Filter::VectorNearest { .. } => false, + // Same reasoning as `VectorNearest`: `FtsRanked` is a ranking + // operation handled at the top of `query`, and nested occurrences are + // rejected before this loop runs. + Filter::VectorNearest { .. } | Filter::FtsRanked { .. } => false, Filter::And(children) => children.iter().all(|f| filter_matches(f, indexed, fts)), Filter::Or(children) => children.iter().any(|f| filter_matches(f, indexed, fts)), } @@ -889,6 +922,35 @@ fn fts_naive_matches(stored: &str, terms: &[String]) -> bool { .all(|term| tokens.contains(term.to_lowercase().as_str())) } +/// Reference relevance score for [`Filter::FtsRanked`]: how many DISTINCT +/// query terms appear as whole tokens in `stored`. Zero means "no term +/// matched", which the ranked path treats as "not a hit" (OR semantics need at +/// least one term). Tokenization is the same whole-token rule +/// [`fts_naive_matches`] uses, so the reference agrees with FTS5's +/// `unicode61` tokenizer on what counts as a term occurrence. +fn fts_term_coverage(stored: &str, terms: &[String]) -> usize { + if terms.is_empty() { + return 0; + } + let stored_lower = stored.to_lowercase(); + let tokens: std::collections::HashSet<&str> = stored_lower + .split(|character: char| !character.is_alphanumeric()) + .filter(|token| !token.is_empty()) + .collect(); + terms + .iter() + .filter(|term| tokens.contains(term.to_lowercase().as_str())) + .count() +} + +/// If `filter` is a top-level [`Filter::FtsRanked`], return its components. +fn top_level_fts_ranked(filter: &Filter) -> Option<(&IndexKey, &str, u32)> { + if let Filter::FtsRanked { key, query, limit } = filter { + return Some((key, query.as_str(), *limit)); + } + None +} + /// If `filter` is a top-level `VectorNearest` (the only shape the SQL /// backends and this reference implementation evaluate by ranking rather /// than predication), return its components. @@ -904,14 +966,15 @@ fn top_level_vector_nearest(filter: &Filter) -> Option<(&IndexKey, &[f32], u32)> None } -/// Walk `filter` and report whether any `VectorNearest` occurs strictly -/// inside an `And`/`Or` compound. A top-level `VectorNearest` is handled -/// by the query method's ranking path; nested occurrences are rejected -/// with `Unsupported` to match the SQL backends (audit finding F5). -fn contains_nested_vector_nearest(filter: &Filter) -> bool { +/// Walk `filter` and report whether any ranking filter (`VectorNearest` or +/// `FtsRanked`) occurs strictly inside an `And`/`Or` compound. A top-level +/// ranking filter is handled by the query method's ranking paths; nested +/// occurrences are rejected with `Unsupported` to match the SQL backends +/// (audit finding F5). +fn contains_nested_ranking_filter(filter: &Filter) -> bool { fn walk(filter: &Filter, inside_compound: bool) -> bool { match filter { - Filter::VectorNearest { .. } => inside_compound, + Filter::VectorNearest { .. } | Filter::FtsRanked { .. } => inside_compound, Filter::And(children) | Filter::Or(children) => { children.iter().any(|child| walk(child, true)) } diff --git a/crates/substrates/ironclaw_filesystem/src/index.rs b/crates/substrates/ironclaw_filesystem/src/index.rs index 34f5e214a0a..c641fc65440 100644 --- a/crates/substrates/ironclaw_filesystem/src/index.rs +++ b/crates/substrates/ironclaw_filesystem/src/index.rs @@ -300,6 +300,33 @@ pub enum Filter { key: IndexKey, query: String, }, + /// Ranked full-text search on a text-valued indexed `key`. Requires the + /// index to be `IndexKind::Fts`, and `query` follows the same plain-text + /// contract as [`Filter::Fts`] (punctuation splits terms, English stop + /// words are dropped, `AND`/`OR`/`NOT` are never operators). + /// + /// The difference from [`Filter::Fts`] is the match rule and the ordering: + /// `Fts` requires EVERY content term and returns path-ordered results, + /// while `FtsRanked` matches a record carrying ANY content term and + /// returns them ordered by descending relevance (`bm25()` on libSQL, + /// `ts_rank` on PostgreSQL, and a term-coverage score in the in-memory + /// reference). Conversational retrieval needs the ranked-OR shape: a + /// paraphrased question shares only some content words with the stored + /// text, so requiring every term returns nothing. + /// + /// Like [`Filter::VectorNearest`] this is a top-k ranking operation: + /// `limit` truncates after ranking and overrides the caller's + /// [`Page::limit`](crate::Page::limit), and nesting it inside `And`/`Or` + /// would discard the ranking, so backends reject that with + /// [`FilesystemError::Unsupported`](crate::FilesystemError::Unsupported). + /// + /// A query with no content terms (empty, punctuation-only, or entirely + /// stop words) matches nothing on every backend. + FtsRanked { + key: IndexKey, + query: String, + limit: u32, + }, /// Vector-similarity search on a vector-valued indexed `key`. Requires /// the index to be `IndexKind::Vector { dim }` with a matching `dim`. /// `embedding` is the query vector; results are ranked by descending @@ -672,6 +699,7 @@ fn collect_equality_values<'a>( Filter::PrefixOn { .. } | Filter::Range { .. } | Filter::Fts { .. } + | Filter::FtsRanked { .. } | Filter::VectorNearest { .. } | Filter::Or(_) => false, } diff --git a/crates/substrates/ironclaw_filesystem/src/libsql.rs b/crates/substrates/ironclaw_filesystem/src/libsql.rs index 415bc467fed..36728b7c9ee 100644 --- a/crates/substrates/ironclaw_filesystem/src/libsql.rs +++ b/crates/substrates/ironclaw_filesystem/src/libsql.rs @@ -803,6 +803,11 @@ impl RootFilesystem for LibSqlRootFilesystem { .vector_nearest_query(path, key, embedding, *limit) .await; } + // Ranked full-text search is the second top-k ranking operation: it + // needs an ORDER BY over `bm25()`, which no WHERE fragment can carry. + if let Filter::FtsRanked { key, query, limit } = filter { + return self.fts_ranked_query(path, key, query, *limit).await; + } let fts_tables = self.discover_fts_tables_for_filter(path, filter).await?; let mut params: Vec = vec![libsql::Value::Text(path.as_str().to_string())]; let (prefix_lower, prefix_upper) = descendant_path_range(path); @@ -842,30 +847,7 @@ impl RootFilesystem for LibSqlRootFilesystem { .await .map_err(|error| libsql_db_error(path.clone(), FilesystemOperation::Query, error))? { - let row_path: String = row.get(0).map_err(|error| { - libsql_db_error(path.clone(), FilesystemOperation::Query, error) - })?; - let row_path = VirtualPath::new(row_path)?; - let body: Vec = row.get(1).map_err(|error| { - libsql_db_error(row_path.clone(), FilesystemOperation::Query, error) - })?; - let content_type_raw: String = row.get(2).map_err(|error| { - libsql_db_error(row_path.clone(), FilesystemOperation::Query, error) - })?; - let kind_raw: Option = row.get(3).ok(); - let indexed_raw: String = row.get(4).map_err(|error| { - libsql_db_error(row_path.clone(), FilesystemOperation::Query, error) - })?; - let version_raw: i64 = row.get(5).map_err(|error| { - libsql_db_error(row_path.clone(), FilesystemOperation::Query, error) - })?; - let entry = build_entry(&row_path, body, content_type_raw, kind_raw, indexed_raw)?; - let version = record_version_from_i64(&row_path, version_raw)?; - out.push(VersionedEntry { - path: row_path, - entry, - version, - }); + out.push(record_row_to_versioned_entry(path, &row)?); } Ok(out) } @@ -2392,6 +2374,77 @@ impl LibSqlRootFilesystem { Ok(out) } + /// Ranked full-text search over the FTS5 shadow table declared for `key`. + /// + /// The query's content terms are joined with FTS5's `OR` operator — a + /// record carrying ANY term is a hit — and results are ordered by + /// `bm25()`, whose score is more negative the more relevant the row, hence + /// `ORDER BY score ASC`. Terms come from the shared `plain_fts_terms` + /// parser and are quoted individually, so untrusted caller text can never + /// reach the MATCH grammar as syntax; only the `OR` joiner this method + /// writes is an operator. A query with no content terms matches nothing + /// without touching the database. + async fn fts_ranked_query( + &self, + path: &VirtualPath, + key: &IndexKey, + query: &str, + limit: u32, + ) -> Result, FilesystemError> { + let probe = Filter::FtsRanked { + key: key.clone(), + query: query.to_string(), + limit, + }; + let fts_tables = self.discover_fts_tables_for_filter(path, &probe).await?; + let Some(fts_table) = fts_tables.get(key.as_str()) else { + return Err(FilesystemError::Unsupported { + path: path.clone(), + operation: FilesystemOperation::Query, + }); + }; + let terms = crate::index::plain_fts_terms(query); + if terms.is_empty() { + return Ok(Vec::new()); + } + let match_query = terms + .into_iter() + .map(|term| format!("\"{term}\"")) + .collect::>() + .join(" OR "); + let (prefix_lower, prefix_upper) = descendant_path_range(path); + let params: Vec = vec![ + libsql::Value::Text(path.as_str().to_string()), + libsql::Value::Text(prefix_lower), + libsql::Value::Text(prefix_upper), + libsql::Value::Text(match_query), + libsql::Value::Integer(i64::from(limit.min(crate::Page::MAX_LIMIT))), + ]; + let sql = format!( + "SELECT r.path, r.contents, r.content_type, r.kind, r.indexed, r.version \ + FROM root_filesystem_entries r \ + JOIN (SELECT path AS ranked_path, bm25({fts_table}) AS score \ + FROM {fts_table} WHERE {fts_table} MATCH ?4) ranked \ + ON ranked.ranked_path = r.path \ + WHERE r.is_dir = 0 AND (r.path = ?1 OR (r.path >= ?2 AND r.path < ?3)) \ + ORDER BY ranked.score ASC, r.path ASC LIMIT ?5" + ); + let conn = self.read_connection().await?; + let mut rows = conn + .query(&sql, params) + .await + .map_err(|error| libsql_db_error(path.clone(), FilesystemOperation::Query, error))?; + let mut out = Vec::new(); + while let Some(row) = rows + .next() + .await + .map_err(|error| libsql_db_error(path.clone(), FilesystemOperation::Query, error))? + { + out.push(record_row_to_versioned_entry(path, &row)?); + } + Ok(out) + } + /// Brute-force cosine over candidates under `path` whose indexed /// projection has an `IndexValue::Bytes` value at `key` decoded as a /// little-endian f32 buffer of any non-zero length matching the query @@ -2858,15 +2911,17 @@ fn translate_filter( )); Ok(()) } - Filter::VectorNearest { .. } => Err(FilesystemError::Unsupported { - // VectorNearest is evaluated by the top-level `query` method, - // not inside the WHERE fragment. Reaching the translator - // means a caller composed it inside an And/Or — which would - // throw away the ranking. Surface as Unsupported so the - // caller restructures the query. - path: path.clone(), - operation: FilesystemOperation::Query, - }), + Filter::VectorNearest { .. } | Filter::FtsRanked { .. } => { + Err(FilesystemError::Unsupported { + // VectorNearest and FtsRanked are evaluated by the top-level + // `query` method, not inside the WHERE fragment. Reaching the + // translator means a caller composed one inside an And/Or — which + // would throw away the ranking. Surface as Unsupported so the + // caller restructures the query. + path: path.clone(), + operation: FilesystemOperation::Query, + }) + } Filter::And(children) => { translate_compound(path, children, " AND ", "TRUE", out, params, fts_tables) } @@ -2901,9 +2956,43 @@ fn translate_compound( out.push(')'); Ok(()) } +/// Map one `RECORD_QUERY_PREFIX_SQL`-shaped row (path, contents, +/// content_type, kind, indexed, version) onto a [`VersionedEntry`]. Shared by +/// the predicate query path and the ranked full-text path so both agree on +/// column order and error attribution. +fn record_row_to_versioned_entry( + path: &VirtualPath, + row: &libsql::Row, +) -> Result { + let row_path: String = row + .get(0) + .map_err(|error| libsql_db_error(path.clone(), FilesystemOperation::Query, error))?; + let row_path = VirtualPath::new(row_path)?; + let body: Vec = row + .get(1) + .map_err(|error| libsql_db_error(row_path.clone(), FilesystemOperation::Query, error))?; + let content_type_raw: String = row + .get(2) + .map_err(|error| libsql_db_error(row_path.clone(), FilesystemOperation::Query, error))?; + let kind_raw: Option = row.get(3).ok(); + let indexed_raw: String = row + .get(4) + .map_err(|error| libsql_db_error(row_path.clone(), FilesystemOperation::Query, error))?; + let version_raw: i64 = row + .get(5) + .map_err(|error| libsql_db_error(row_path.clone(), FilesystemOperation::Query, error))?; + let entry = build_entry(&row_path, body, content_type_raw, kind_raw, indexed_raw)?; + let version = record_version_from_i64(&row_path, version_raw)?; + Ok(VersionedEntry { + path: row_path, + entry, + version, + }) +} + fn collect_fts_keys(filter: &Filter, out: &mut Vec) { match filter { - Filter::Fts { key, .. } => { + Filter::Fts { key, .. } | Filter::FtsRanked { key, .. } => { let k = key.as_str().to_string(); if !out.contains(&k) { out.push(k); diff --git a/crates/substrates/ironclaw_filesystem/src/postgres.rs b/crates/substrates/ironclaw_filesystem/src/postgres.rs index fed7251be5f..83db9b4ac39 100644 --- a/crates/substrates/ironclaw_filesystem/src/postgres.rs +++ b/crates/substrates/ironclaw_filesystem/src/postgres.rs @@ -536,6 +536,11 @@ impl RootFilesystem for PostgresRootFilesystem { .vector_nearest_query(path, key, embedding, *limit) .await; } + // Ranked full-text search needs an ORDER BY over `ts_rank`, which no + // WHERE fragment can carry — same top-level treatment as vector search. + if let Filter::FtsRanked { key, query, limit } = filter { + return self.fts_ranked_query(path, key, query, *limit).await; + } let mut params: Vec> = Vec::new(); let path_str = path.as_str().to_string(); let (prefix_lower, prefix_upper) = descendant_path_range(path); @@ -579,25 +584,7 @@ impl RootFilesystem for PostgresRootFilesystem { .query(&statement, ¶ms_ref[..]) .await .map_err(|error| db_error(path.clone(), FilesystemOperation::Query, error))?; - rows.into_iter() - .map(|row| { - let row_path: String = row.get("path"); - let row_path = VirtualPath::new(row_path)?; - let body: Vec = row.get("contents"); - let content_type_raw: String = row.get("content_type"); - let kind_raw: Option = row.get("kind"); - let indexed_value: serde_json::Value = row.get("indexed"); - let version_raw: i64 = row.get("version"); - let entry = - build_entry(&row_path, body, content_type_raw, kind_raw, indexed_value)?; - let version = record_version_from_i64(&row_path, version_raw)?; - Ok(VersionedEntry { - path: row_path, - entry, - version, - }) - }) - .collect() + rows.iter().map(record_row_to_versioned_entry).collect() } async fn query_ordered( @@ -1235,6 +1222,63 @@ impl PostgresRootFilesystem { Ok(row.is_some()) } + /// Ranked full-text search: `ts_rank` over an OR `tsquery`. + /// + /// The query's content terms come from the shared `plain_fts_terms` parser + /// (so PostgreSQL, libSQL, and the in-memory reference agree on which words + /// are required matches) and are joined with `|`. Because that parser + /// returns Unicode-alphanumeric strings only, the joined expression is + /// bound as a parameter to `to_tsquery` and can never carry caller + /// punctuation into the tsquery grammar. Ordering is by descending + /// `ts_rank`, tie-broken by path so the result is deterministic. A query + /// with no content terms matches nothing without touching the database. + async fn fts_ranked_query( + &self, + path: &VirtualPath, + key: &IndexKey, + query: &str, + limit: u32, + ) -> Result, FilesystemError> { + let terms = crate::index::plain_fts_terms(query); + if terms.is_empty() { + return Ok(Vec::new()); + } + let tsquery = terms.join(" | "); + let (prefix_lower, prefix_upper) = descendant_path_range(path); + let params: Vec> = vec![ + Box::new(path.as_str().to_string()), + Box::new(prefix_lower), + Box::new(prefix_upper), + Box::new(tsquery), + Box::new(i64::from(limit.min(Page::MAX_LIMIT))), + ]; + // `key` is validated as `[A-Za-z_][A-Za-z0-9_]*` by `IndexKey`, so + // splicing it into the JSON path is safe — the same rule the + // predicate translator relies on. + let sql = format!( + "SELECT path, contents, content_type, kind, indexed, version \ + FROM root_filesystem_entries, to_tsquery('english', $4) AS ranked_query \ + WHERE is_dir = FALSE AND (path = $1 OR (path >= $2 AND path < $3)) \ + AND to_tsvector('english', COALESCE(indexed->>'{}', '')) @@ ranked_query \ + ORDER BY ts_rank(to_tsvector('english', COALESCE(indexed->>'{}', '')), ranked_query) \ + DESC, path ASC LIMIT $5", + key.as_str(), + key.as_str(), + ); + let client = self.client().await?; + let params_ref: Vec<&(dyn tokio_postgres::types::ToSql + Sync)> = + params.iter().map(|p| p.as_ref() as _).collect(); + let statement = client + .prepare_cached(sql.as_str()) + .await + .map_err(|error| db_error(path.clone(), FilesystemOperation::Query, error))?; + let rows = client + .query(&statement, ¶ms_ref[..]) + .await + .map_err(|error| db_error(path.clone(), FilesystemOperation::Query, error))?; + rows.iter().map(record_row_to_versioned_entry).collect() + } + /// Brute-force cosine ranker over candidate rows in this prefix whose /// indexed projection has an `IndexValue::Bytes` value at `key`. Used /// when the caller's filter is a top-level `VectorNearest`. @@ -2464,17 +2508,41 @@ fn translate_filter( )); Ok(()) } - Filter::VectorNearest { .. } => Err(FilesystemError::Unsupported { - // Same reason as libsql: VectorNearest is a ranking operation - // and is evaluated at the top-level `query` method, not as a - // WHERE-clause predicate. Nested usage is unsupported. - path: path.clone(), - operation: FilesystemOperation::Query, - }), + Filter::VectorNearest { .. } | Filter::FtsRanked { .. } => { + Err(FilesystemError::Unsupported { + // Same reason as libsql: VectorNearest and FtsRanked are ranking + // operations evaluated at the top-level `query` method, not as a + // WHERE-clause predicate. Nested usage is unsupported. + path: path.clone(), + operation: FilesystemOperation::Query, + }) + } Filter::And(children) => translate_compound(path, children, " AND ", "TRUE", out, params), Filter::Or(children) => translate_compound(path, children, " OR ", "FALSE", out, params), } } +/// Map one `(path, contents, content_type, kind, indexed, version)` row onto a +/// [`VersionedEntry`]. Shared by the predicate query path and the ranked +/// full-text path; both select the same named columns. +fn record_row_to_versioned_entry( + row: &tokio_postgres::Row, +) -> Result { + let row_path: String = row.get("path"); + let row_path = VirtualPath::new(row_path)?; + let body: Vec = row.get("contents"); + let content_type_raw: String = row.get("content_type"); + let kind_raw: Option = row.get("kind"); + let indexed_value: serde_json::Value = row.get("indexed"); + let version_raw: i64 = row.get("version"); + let entry = build_entry(&row_path, body, content_type_raw, kind_raw, indexed_value)?; + let version = record_version_from_i64(&row_path, version_raw)?; + Ok(VersionedEntry { + path: row_path, + entry, + version, + }) +} + fn translate_compound( path: &VirtualPath, children: &[Filter], diff --git a/crates/substrates/ironclaw_filesystem/tests/db_root_filesystem_contract.rs b/crates/substrates/ironclaw_filesystem/tests/db_root_filesystem_contract.rs index f57e865aa2d..519476280ed 100644 --- a/crates/substrates/ironclaw_filesystem/tests/db_root_filesystem_contract.rs +++ b/crates/substrates/ironclaw_filesystem/tests/db_root_filesystem_contract.rs @@ -1398,6 +1398,156 @@ async fn libsql_ordered_index_declaration_never_backfills_existing_rows() { ); } +/// Shared cross-backend body for `Filter::FtsRanked` (#7185). +/// +/// The scenario is conversational recall: a fact is stored as one sentence and +/// asked for in differently-worded question that shares only SOME of its +/// content words. `Filter::Fts` requires every content term, so it finds +/// nothing — the assertion this test opens with, which is what made memory +/// recall fail in practice. `Filter::FtsRanked` matches on any term and orders +/// by relevance, so the sentence sharing three query terms comes back ahead of +/// the one sharing a single term, and the unrelated record stays out. +/// +/// The AND-semantics assertion is load-bearing: without it a ranked-OR test +/// would also pass under the old behavior and prove nothing. +async fn ranked_fts_contract(filesystem: &F, base: &str) { + let content = IndexKey::new("content").unwrap(); + let kind = RecordKind::new("chunk").unwrap(); + let root = VirtualPath::new(base.to_string()).unwrap(); + let spec = IndexSpec::new( + IndexName::new("ranked_fts_content_v1").unwrap(), + vec![content.clone()], + IndexKind::Fts, + ); + filesystem.ensure_index(&root, &spec).await.unwrap(); + + for (leaf, body) in [ + ( + "a", + "Sarah prefers the standup meeting scheduled early on Thursday mornings", + ), + ("b", "Sarah keeps a spare umbrella at her desk"), + ("c", "Deployment runbook for the staging cluster"), + ] { + filesystem + .put( + &VirtualPath::new(format!("{base}/{leaf}")).unwrap(), + Entry::record(kind.clone(), &serde_json::json!({})) + .unwrap() + .with_indexed(content.clone(), IndexValue::Text(body.into())), + CasExpectation::Absent, + ) + .await + .unwrap(); + } + + // Content terms: sarah, like, standup, scheduled. Record `a` carries three + // of them but not `like`, so every-term matching returns nothing. + let question = "when does Sarah like her standup scheduled"; + + let and_results = filesystem + .query( + &root, + &Filter::Fts { + key: content.clone(), + query: question.into(), + }, + Page::default(), + ) + .await + .unwrap(); + assert!( + and_results.is_empty(), + "Filter::Fts requires every content term, so a paraphrased question must miss; got {:?}", + and_results + .iter() + .map(|e| e.path.as_str()) + .collect::>() + ); + + let ranked = filesystem + .query( + &root, + &Filter::FtsRanked { + key: content.clone(), + query: question.into(), + limit: 10, + }, + Page::default(), + ) + .await + .unwrap(); + let ranked_paths: Vec<&str> = ranked.iter().map(|entry| entry.path.as_str()).collect(); + assert_eq!( + ranked_paths, + vec![format!("{base}/a").as_str(), format!("{base}/b").as_str()], + "ranked OR must return both partial matches, most relevant first, and exclude the \ + unrelated record" + ); + + // `limit` truncates after ranking, so the top-1 request keeps the best hit. + let top_one = filesystem + .query( + &root, + &Filter::FtsRanked { + key: content.clone(), + query: question.into(), + limit: 1, + }, + Page::default(), + ) + .await + .unwrap(); + assert_eq!( + top_one.iter().map(|e| e.path.as_str()).collect::>(), + vec![format!("{base}/a").as_str()] + ); + + // A query with no content terms matches nothing rather than everything. + let stop_words_only = filesystem + .query( + &root, + &Filter::FtsRanked { + key: content.clone(), + query: "and the of to".into(), + limit: 10, + }, + Page::default(), + ) + .await + .unwrap(); + assert!(stop_words_only.is_empty()); + + // Nesting a ranking filter inside a compound would discard the ordering. + let nested = filesystem + .query( + &root, + &Filter::And(vec![Filter::FtsRanked { + key: content, + query: question.into(), + limit: 10, + }]), + Page::default(), + ) + .await; + assert!( + matches!(nested, Err(FilesystemError::Unsupported { .. })), + "nested FtsRanked must be Unsupported on every backend, got {nested:?}" + ); +} + +#[tokio::test] +async fn libsql_ranked_fts_finds_paraphrased_recall_in_relevance_order() { + let filesystem = libsql_root().await; + ranked_fts_contract(&*filesystem, "/memory/ranked-fts").await; +} + +#[tokio::test] +async fn in_memory_ranked_fts_finds_paraphrased_recall_in_relevance_order() { + let filesystem = ironclaw_filesystem::InMemoryBackend::new(); + ranked_fts_contract(&filesystem, "/memory/ranked-fts").await; +} + /// Shared cross-backend body: an ordered-index spec declared on an ancestor /// prefix serves queries at child paths, and those queries stay scoped to the /// queried child subtree. @@ -2569,6 +2719,14 @@ mod postgres_tests { super::ancestor_declared_ordered_index_contract(&fs, &prefix).await; } + #[tokio::test] + async fn postgres_ranked_fts_finds_paraphrased_recall_in_relevance_order() { + let Some((fs, prefix)) = postgres_root().await else { + return; + }; + super::ranked_fts_contract(&fs, &prefix).await; + } + #[tokio::test] async fn postgres_ordered_index_narrow_declaration_survives_root_declaration() { let Some((fs, prefix)) = postgres_root().await else { diff --git a/tests/integration/group_memory/main.rs b/tests/integration/group_memory/main.rs index f60b50a014e..176e0207b53 100644 --- a/tests/integration/group_memory/main.rs +++ b/tests/integration/group_memory/main.rs @@ -21,6 +21,7 @@ mod scenario_disabled_binding_offers_no_memory_tools; mod scenario_lifecycle_gates_host_memory_calls; mod scenario_memory_search_finds_seeded; mod scenario_memory_tree_reflects_structure; +mod scenario_paraphrased_prompt_recall_libsql; mod scenario_proactive_prompt_recall_libsql; mod scenario_write_then_read_cross_thread; @@ -89,5 +90,14 @@ async fn memory_group_e2e() { scenario_always_on_memory_recall_libsql::run().await, ); + // Scenario 8 (#7185): ranked-OR retrieval. A fact saved as one sentence is + // recalled by a differently-worded question sharing only some of its + // content words — the case every-term (AND) matching silently missed. Own + // group, same libSQL reason as scenario 6. + report.record( + "paraphrased_prompt_recall_libsql", + scenario_paraphrased_prompt_recall_libsql::run().await, + ); + report.assert_all_passed(); } diff --git a/tests/integration/group_memory/scenario_paraphrased_prompt_recall_libsql.rs b/tests/integration/group_memory/scenario_paraphrased_prompt_recall_libsql.rs new file mode 100644 index 00000000000..a337f79cdcd --- /dev/null +++ b/tests/integration/group_memory/scenario_paraphrased_prompt_recall_libsql.rs @@ -0,0 +1,77 @@ +//! Production-path regression for #7185: a fact saved as one sentence is +//! recalled by a differently-worded question that shares only SOME of its +//! content words. +//! +//! Retrieval used to require EVERY non-stopword term of the raw user message +//! (`Filter::Fts` is an AND), so conversational recall essentially never +//! matched: the reader's question below shares `sarah` / `standup` / +//! `scheduled` with the stored sentence but adds `like`, which the stored +//! sentence does not contain. One absent term was enough to return nothing. +//! Ranked OR retrieval (`Filter::FtsRanked`, bm25 on the shipping libSQL +//! backend) matches on any content term and orders by relevance. +//! +//! The fact is deliberately written to a NON-standing document +//! (`notes/standup.md`), not `MEMORY.md`: the always-on standing-document lane +//! added in #7365 prepends `MEMORY.md` regardless of the query, so writing +//! there would make this assertion pass without the search path working at +//! all. + +use ironclaw_host_runtime::MEMORY_WRITE_CAPABILITY_ID; +use serde_json::json; + +use super::reborn_support::group::{HarnessResult, RebornIntegrationGroup}; +use super::reborn_support::reply::RebornScriptedReply; + +/// Never written anywhere. Guards the positive assertion against a prompt that +/// simply carries everything. +const UNWRITTEN_MARKER: &str = "wombat-31"; + +pub async fn run() -> HarnessResult<()> { + let group = RebornIntegrationGroup::builtin_tools_with_native_memory_libsql().await?; + + let writer = group + .thread("conv-memory-paraphrase-writer") + .script([ + RebornScriptedReply::tool_call( + MEMORY_WRITE_CAPABILITY_ID, + json!({ + "target": "notes/standup.md", + "content": "Sarah prefers the standup meeting scheduled early on Thursday mornings", + "append": false + }), + ), + RebornScriptedReply::text("saved"), + ]) + .build() + .await?; + writer + .submit_turn("Remember when Sarah wants the standup.") + .await?; + writer + .assert_tool_invoked(MEMORY_WRITE_CAPABILITY_ID) + .await?; + drop(writer); + + // A different conversation, so only the host-managed proactive prompt lane + // can satisfy the assertion — the reader makes no memory tool call. + // + // Content terms of this question: sarah, like, standup, scheduled. The + // stored sentence carries three of the four and NOT `like`, so every-term + // (AND) matching returns nothing and this assertion fails. + let reader = group + .thread("conv-memory-paraphrase-reader") + .script([RebornScriptedReply::text("answered")]) + .build() + .await?; + reader + .submit_turn("when does Sarah like her standup scheduled") + .await?; + reader + .assert_system_prompt_contains("Thursday mornings") + .await?; + reader + .assert_system_prompt_excludes(UNWRITTEN_MARKER) + .await?; + + Ok(()) +} From b6a97e83217115ee72d1023e978f78fea9d0fd9c Mon Sep 17 00:00:00 2001 From: serrrfirat Date: Wed, 12 Aug 2026 21:19:05 +0300 Subject: [PATCH 2/3] fix(memory): make a failed memory retrieval distinguishable from an empty one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A memory backend that was down and a user with nothing relevant stored produced exactly the same thing: an empty prompt section. Every failure on the retrieval path degraded silently — the host adapter logged the lane error at `debug!` and returned an empty list, and the loop host cached that empty list in its per-run `OnceCell`, so one blip blanked memory for the whole run with no way to tell afterwards whether memory was empty or broken. Make the outcome typed instead of inferred. `MemoryPromptContextService` now returns `MemoryPromptContextLoad { snippets, degradations }`, mirroring `LoopContextBundle`'s existing shape of "successful payload plus an explicit, typed description of what was lost" (`recent_window_truncation`). A degradation names the lane (`short_term` / `long_term`) and a closed-vocabulary failure kind (`input` / `unavailable`) — never a backend message, a query, or a path. `degradations` is empty exactly when every queried lane answered, so an empty result from a healthy backend is no longer confusable with an outage. Retrieval stays best-effort and never fails a turn, and the per-run cache stays (it exists to stop a slow backend being re-hit on every model step) — but the cached value now RECORDS that it failed rather than laundering the failure into "empty". Operator visibility rides the milestone sink the context port already holds: a degraded load emits one `LoopDriverNoteKind::Context` driver note per run, which reaches the live work summary. This is the same route `publish_personal_context_admitted` uses and the same rationale as `EventSubscriptionTerminated` — a subsystem that stopped contributing must not be silently invisible. Deliberately NOT promoted to `warn!`/`info!`: those levels render in the REPL and corrupt the terminal UI, and this fires from a background prompt build. Tests: crate tier pins both directions in the host adapter (an outage records both lanes, a partial failure records only the failing one, and a healthy empty result records nothing); integration tier drives the real composition with two byte-identical turns differing only in whether the bound provider's lanes return `Err(unavailable)` or `Ok(vec![])`, verified to fail before the change with "no driver note reporting degraded memory retrieval; saw []". Part of #7185, #7275 Co-Authored-By: Claude Fable 5 --- .../ironclaw_loop_contracts/src/lib.rs | 4 +- .../src/memory_context.rs | 89 +++++++++++- .../tests/memory_prompt_context_service.rs | 6 +- .../src/memory_context.rs | 64 +++++++-- .../tests/memory_prompt_context.rs | 109 ++++++++++++--- crates/loop/ironclaw_loop_host/src/lib.rs | 21 ++- .../ironclaw_loop_host/src/memory_context.rs | 128 +++++++++++++++--- .../ironclaw_loop_host/tests/llm_gateway.rs | 16 ++- .../tests/thread_loop_host_contract.rs | 16 ++- tests/integration/group_memory/main.rs | 10 ++ ...rio_memory_retrieval_failure_is_visible.rs | 115 ++++++++++++++++ tests/integration/support/assertions.rs | 63 +++++++++ 12 files changed, 565 insertions(+), 76 deletions(-) create mode 100644 tests/integration/group_memory/scenario_memory_retrieval_failure_is_visible.rs diff --git a/crates/contracts/ironclaw_loop_contracts/src/lib.rs b/crates/contracts/ironclaw_loop_contracts/src/lib.rs index 3e7099dd1af..150b7280199 100644 --- a/crates/contracts/ironclaw_loop_contracts/src/lib.rs +++ b/crates/contracts/ironclaw_loop_contracts/src/lib.rs @@ -97,7 +97,9 @@ pub use loop_exit::{ LoopCompletionKind, LoopExit, LoopFailed, LoopFailureKind, }; pub use memory_context::{ - EmptyMemoryPromptContextService, MemoryPromptContextRequest, MemoryPromptContextService, + EmptyMemoryPromptContextService, MemoryPromptContextLoad, MemoryPromptContextRequest, + MemoryPromptContextService, MemoryRetrievalDegradation, MemoryRetrievalFailureKind, + MemoryRetrievalLane, }; pub use milestones::{ HookDecisionSummary, HookMilestoneSink, InMemoryHookMilestoneSink, diff --git a/crates/contracts/ironclaw_loop_contracts/src/memory_context.rs b/crates/contracts/ironclaw_loop_contracts/src/memory_context.rs index 58f5be8d5e6..df9116c3a7c 100644 --- a/crates/contracts/ironclaw_loop_contracts/src/memory_context.rs +++ b/crates/contracts/ironclaw_loop_contracts/src/memory_context.rs @@ -62,12 +62,95 @@ pub struct MemoryPromptContextRequest { /// [`AgentLoopHostErrorKind::Unavailable`](super::host::AgentLoopHostErrorKind::Unavailable). /// Raw backend error messages, filesystem paths, and internal identifiers must /// never appear in the error's `safe_summary`. +/// Which retrieval lane a degradation came from. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MemoryRetrievalLane { + ShortTerm, + LongTerm, +} + +impl MemoryRetrievalLane { + pub fn as_str(self) -> &'static str { + match self { + Self::ShortTerm => "short_term", + Self::LongTerm => "long_term", + } + } +} + +/// Why a retrieval lane produced nothing. A closed vocabulary, never free text +/// and never a backend message — the label is safe to record and to render in +/// operator surfaces. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MemoryRetrievalFailureKind { + /// The request itself was rejected (bad query or profile). + Input, + /// The provider or its storage backend was unreachable or errored. + Unavailable, +} + +impl MemoryRetrievalFailureKind { + pub fn as_str(self) -> &'static str { + match self { + Self::Input => "input", + Self::Unavailable => "unavailable", + } + } +} + +/// One lane that failed rather than returning nothing. +/// +/// This is the distinction the previous contract could not express: a lane +/// error and a lane with no matching memory both arrived as an empty snippet +/// list, so "the memory backend is down" was indistinguishable from "the user +/// has no memory about this" in tests and in operator diagnostics alike. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct MemoryRetrievalDegradation { + pub lane: MemoryRetrievalLane, + pub kind: MemoryRetrievalFailureKind, +} + +impl MemoryRetrievalDegradation { + pub fn new(lane: MemoryRetrievalLane, kind: MemoryRetrievalFailureKind) -> Self { + Self { lane, kind } + } +} + +/// The result of one memory prompt-context load: the snippets that were +/// admitted, plus a typed record of every lane that failed on the way. +/// +/// Mirrors [`LoopContextBundle`](super::host::LoopContextBundle), which carries +/// its successful payload alongside an explicit description of what was lost +/// (`recent_window_truncation`). Memory retrieval is best-effort — a failing +/// lane must never fail the turn — but "best-effort" is not "invisible": +/// `degradations` is empty exactly when every queried lane answered. +#[derive(Debug, Clone, Default)] +pub struct MemoryPromptContextLoad { + pub snippets: Vec, + pub degradations: Vec, +} + +impl MemoryPromptContextLoad { + /// A load where every queried lane answered. + pub fn healthy(snippets: Vec) -> Self { + Self { + snippets, + degradations: Vec::new(), + } + } + + /// Whether any queried lane failed rather than simply matching nothing. + pub fn is_degraded(&self) -> bool { + !self.degradations.is_empty() + } +} + #[async_trait] pub trait MemoryPromptContextService: Send + Sync { async fn load_memory_snippets( &self, request: MemoryPromptContextRequest, - ) -> Result, AgentLoopHostError>; + ) -> Result; } /// No-op implementation that always returns an empty snippet list. @@ -82,7 +165,7 @@ impl MemoryPromptContextService for EmptyMemoryPromptContextService { async fn load_memory_snippets( &self, _request: MemoryPromptContextRequest, - ) -> Result, AgentLoopHostError> { - Ok(Vec::new()) + ) -> Result { + Ok(MemoryPromptContextLoad::default()) } } diff --git a/crates/contracts/ironclaw_loop_contracts/tests/memory_prompt_context_service.rs b/crates/contracts/ironclaw_loop_contracts/tests/memory_prompt_context_service.rs index b950c2ed465..e6fc16af655 100644 --- a/crates/contracts/ironclaw_loop_contracts/tests/memory_prompt_context_service.rs +++ b/crates/contracts/ironclaw_loop_contracts/tests/memory_prompt_context_service.rs @@ -36,8 +36,10 @@ async fn empty_service_returns_empty_vec() { let result = service .load_memory_snippets(test_request("tenant-a", "user-x")) .await; - let snippets = result.expect("empty service should not fail"); - assert!(snippets.is_empty()); + let load = result.expect("empty service should not fail"); + assert!(load.snippets.is_empty()); + // "No memory backend wired" is not a retrieval failure. + assert!(!load.is_degraded()); } #[tokio::test] diff --git a/crates/kernel/ironclaw_host_runtime/src/memory_context.rs b/crates/kernel/ironclaw_host_runtime/src/memory_context.rs index 1216f1917c3..7ee5e43186c 100644 --- a/crates/kernel/ironclaw_host_runtime/src/memory_context.rs +++ b/crates/kernel/ironclaw_host_runtime/src/memory_context.rs @@ -24,8 +24,9 @@ use ironclaw_host_api::{ resource::ResourceScope, }; use ironclaw_loop_contracts::{ - AgentLoopHostError, AgentLoopHostErrorKind, LoopContextSnippet, MemoryPromptContextRequest, - MemoryPromptContextService, memory_snippet_display_ref, + AgentLoopHostError, AgentLoopHostErrorKind, LoopContextSnippet, MemoryPromptContextLoad, + MemoryPromptContextRequest, MemoryPromptContextService, MemoryRetrievalDegradation, + MemoryRetrievalFailureKind, MemoryRetrievalLane, memory_snippet_display_ref, }; use ironclaw_memory::{ MemoryContextProfileId, MemoryInvocation, MemoryService, MemoryServiceContextRequest, @@ -69,15 +70,15 @@ impl MemoryPromptContextService for ProductionMemoryPromptContextService { async fn load_memory_snippets( &self, request: MemoryPromptContextRequest, - ) -> Result, AgentLoopHostError> { + ) -> Result { if request.max_snippets == 0 { - return Ok(Vec::new()); + return Ok(MemoryPromptContextLoad::default()); } // Fail closed at the host before any provider call: a memory-disabled // profile returns no snippets without touching the memory service (the // memory service keeps an equivalent check as defense in depth). if memory_context_disabled(request.context_profile_id.as_str()) { - return Ok(Vec::new()); + return Ok(MemoryPromptContextLoad::default()); } // The host-resolved `ContextProfileId` is already validated, so this // construction won't fail in practice — but propagate rather than unwrap. @@ -95,7 +96,7 @@ impl MemoryPromptContextService for ProductionMemoryPromptContextService { let query_long = self.lifecycle.declares(MemoryLifecycleHook::ReadLongTerm); let query_short = self.lifecycle.declares(MemoryLifecycleHook::ReadShortTerm); if !query_long && !query_short { - return Ok(Vec::new()); + return Ok(MemoryPromptContextLoad::default()); } let invocation = invocation_for_context_request(&request); let expected = ExpectedScope::from_scope(&invocation.scope); @@ -128,11 +129,28 @@ impl MemoryPromptContextService for ProductionMemoryPromptContextService { } }, ); + let mut degradations = Vec::new(); let short_term = short_term - .map(|lane| admit_lane(&expected, lane, request.max_snippets, "short_term")) + .map(|lane| { + admit_lane( + &expected, + lane, + request.max_snippets, + MemoryRetrievalLane::ShortTerm, + &mut degradations, + ) + }) .unwrap_or_default(); let long_term = long_term - .map(|lane| admit_lane(&expected, lane, request.max_snippets, "long_term")) + .map(|lane| { + admit_lane( + &expected, + lane, + request.max_snippets, + MemoryRetrievalLane::LongTerm, + &mut degradations, + ) + }) .unwrap_or_default(); // Concatenate short-term before long-term so active-thread memory keeps @@ -155,20 +173,31 @@ impl MemoryPromptContextService for ProductionMemoryPromptContextService { total_bytes = total_bytes.saturating_add(snippet_bytes); admitted.push(loop_snippet); } - Ok(admitted) + Ok(MemoryPromptContextLoad { + snippets: admitted, + degradations, + }) } } /// Host admission for one raw provider lane: drop any out-of-scope snippet, /// sanitize the rest into untrusted-enveloped, size-capped text, and cap the -/// lane at `max_snippets`. A lane retrieval failure degrades to empty +/// lane at `max_snippets`. A lane retrieval failure still degrades to empty /// (best-effort: memory never breaks a turn) — including the `unavailable` an -/// unimplemented lane reports. +/// unimplemented lane reports — but it is now RECORDED in `degradations` +/// instead of vanishing into a log line, so the caller can tell "the backend +/// failed" from "there was nothing to recall". +/// +/// The log half stays at `debug!` deliberately: `info!`/`warn!` output is +/// rendered by the REPL and corrupts the terminal UI, and this runs on a +/// background prompt-build path. Operator visibility comes from the returned +/// value and the driver note the loop host emits from it, not from the level. fn admit_lane( expected: &ExpectedScope, lane: Result, MemoryServiceError>, max_snippets: usize, - lane_label: &'static str, + lane_label: MemoryRetrievalLane, + degradations: &mut Vec, ) -> Vec { match lane { Ok(raw) => raw @@ -177,11 +206,18 @@ fn admit_lane( .take(max_snippets) .collect(), Err(error) => { + let kind = match error.kind() { + MemoryServiceErrorKind::Input => MemoryRetrievalFailureKind::Input, + MemoryServiceErrorKind::Operation | MemoryServiceErrorKind::Unavailable => { + MemoryRetrievalFailureKind::Unavailable + } + }; tracing::debug!( - lane = lane_label, - kind = ?error.kind(), + lane = lane_label.as_str(), + kind = kind.as_str(), "memory context lane retrieval failed; degrading lane to empty" ); + degradations.push(MemoryRetrievalDegradation::new(lane_label, kind)); Vec::new() } } diff --git a/crates/kernel/ironclaw_host_runtime/tests/memory_prompt_context.rs b/crates/kernel/ironclaw_host_runtime/tests/memory_prompt_context.rs index 3c018be07eb..2cc0e12af7d 100644 --- a/crates/kernel/ironclaw_host_runtime/tests/memory_prompt_context.rs +++ b/crates/kernel/ironclaw_host_runtime/tests/memory_prompt_context.rs @@ -14,6 +14,7 @@ use ironclaw_host_api::ids::{AgentId, ProjectId, TenantId, ThreadId, UserId}; use ironclaw_host_api::turn::{TurnActor, TurnScope}; use ironclaw_loop_contracts::{ ContextProfileId, MemoryPromptContextRequest, MemoryPromptContextService, + MemoryRetrievalDegradation, MemoryRetrievalFailureKind, MemoryRetrievalLane, memory_snippet_display_ref, }; use ironclaw_memory::{ @@ -198,7 +199,8 @@ async fn empty_memory_returns_empty_snippets() { let result = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) .await - .unwrap(); + .unwrap() + .snippets; assert!(result.is_empty()); } @@ -240,7 +242,8 @@ async fn provider_supplied_cross_scope_snippets_are_dropped_by_the_host() { let snippets = make_service(memory_service) .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) .await - .unwrap(); + .unwrap() + .snippets; assert_eq!( snippets.len(), @@ -261,7 +264,8 @@ async fn max_snippets_zero_returns_empty_without_memory_service_call() { let snippets = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 0)) .await - .unwrap(); + .unwrap() + .snippets; assert!(snippets.is_empty()); assert!( @@ -284,7 +288,11 @@ async fn memory_disabled_context_profile_returns_empty_without_memory_service_ca let mut request = test_request("tenant-a", "user-x", None, None, 10); request.context_profile_id = ContextProfileId::new("memory_disabled").unwrap(); - let snippets = service.load_memory_snippets(request).await.unwrap(); + let snippets = service + .load_memory_snippets(request) + .await + .unwrap() + .snippets; assert!(snippets.is_empty()); assert!( @@ -300,11 +308,49 @@ async fn unavailable_memory_service_degrades_both_lanes_to_empty() { // replaces the pre-two-lane contract where an unavailable service surfaced a // host error — memory is now best-effort and never fails the turn. let service = make_service(Arc::new(MockMemoryService::with_error())); - let snippets = service + let load = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) .await .expect("a memory retrieval outage must not error the whole call"); - assert!(snippets.is_empty()); + assert!(load.snippets.is_empty()); + // #7185: degrading is not the same as having nothing to recall. The outage + // is recorded in the returned value, so callers and operator surfaces can + // tell a broken backend from an empty memory. + assert!(load.is_degraded()); + assert_eq!( + load.degradations, + vec![ + MemoryRetrievalDegradation::new( + MemoryRetrievalLane::ShortTerm, + MemoryRetrievalFailureKind::Unavailable + ), + MemoryRetrievalDegradation::new( + MemoryRetrievalLane::LongTerm, + MemoryRetrievalFailureKind::Unavailable + ), + ] + ); +} + +/// The other half of the distinction: a healthy backend that simply matched +/// nothing reports NO degradation. Without this pin, an implementation that +/// always reported degradation would satisfy the outage test above. +#[tokio::test] +async fn empty_result_from_healthy_lanes_reports_no_degradation() { + let service = make_service(Arc::new(MockMemoryService::with_lane_snippets( + Vec::new(), + Vec::new(), + ))); + let load = service + .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) + .await + .expect("an empty memory must not error"); + assert!(load.snippets.is_empty()); + assert!( + !load.is_degraded(), + "no matching memory is not a retrieval failure; got {:?}", + load.degradations + ); } #[tokio::test] @@ -373,7 +419,8 @@ async fn load_memory_snippets_fetches_both_short_term_and_long_term_lanes() { let snippets = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) .await - .unwrap(); + .unwrap() + .snippets; // Both lane methods were queried exactly once. let captured = memory_service.captured(); @@ -410,7 +457,8 @@ async fn undeclared_short_term_lane_is_not_queried() { let snippets = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) .await - .unwrap(); + .unwrap() + .snippets; let lanes: Vec = memory_service .captured() @@ -439,7 +487,8 @@ async fn empty_lifecycle_issues_no_retrieval_queries() { let snippets = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) .await - .unwrap(); + .unwrap() + .snippets; assert!(snippets.is_empty()); assert!( @@ -461,13 +510,25 @@ async fn load_memory_snippets_degrades_when_one_lane_fails() { )); let service = make_service(memory_service); - let snippets = service + let load = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) .await .expect("one lane failing must not error the whole call"); - assert_eq!(snippets.len(), 1); - assert_eq!(snippets[0].snippet_ref, expected_ref("notes/long-term.md")); + assert_eq!(load.snippets.len(), 1); + assert_eq!( + load.snippets[0].snippet_ref, + expected_ref("notes/long-term.md") + ); + // The surviving lane's results are returned AND the failed lane is named, + // so a partially degraded retrieval is not reported as a healthy one. + assert_eq!( + load.degradations, + vec![MemoryRetrievalDegradation::new( + MemoryRetrievalLane::ShortTerm, + MemoryRetrievalFailureKind::Unavailable + )] + ); } #[tokio::test] @@ -489,7 +550,8 @@ async fn load_memory_snippets_aggregate_budget_bounds_combined_lanes_short_term_ let snippets = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 40)) .await - .unwrap(); + .unwrap() + .snippets; assert!( !snippets.is_empty(), @@ -526,7 +588,8 @@ async fn host_hashes_reference_and_wraps_raw_provider_text() { let snippets = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) .await - .unwrap(); + .unwrap() + .snippets; assert_eq!(snippets.len(), 1); assert_eq!(snippets[0].snippet_ref, expected_ref("notes/plan.md")); @@ -566,7 +629,8 @@ async fn host_builds_stable_legacy_memory_snippet_reference() { 10, )) .await - .unwrap(); + .unwrap() + .snippets; assert_eq!(snippets.len(), 1); assert_eq!(snippets[0].snippet_ref, "memory-snippet:cb96ed00b13e6ae4"); @@ -587,7 +651,8 @@ async fn adapter_enforces_max_snippets_after_memory_service_returns() { let snippets = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 1)) .await - .unwrap(); + .unwrap() + .snippets; assert_eq!(snippets.len(), 1); assert_eq!(snippets[0].snippet_ref, expected_ref("notes/one.md")); @@ -623,7 +688,8 @@ async fn adapter_retains_security_prose_and_paths_redacts_credentials_and_drops_ let snippets = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) .await - .unwrap(); + .unwrap() + .snippets; assert_eq!(snippets.len(), 5); assert_eq!(snippets[0].snippet_ref, expected_ref("notes/clean.md")); @@ -681,7 +747,8 @@ async fn adapter_re_sanitizes_provider_supplied_untrusted_prefix() { let snippets = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) .await - .unwrap(); + .unwrap() + .snippets; assert_eq!(snippets.len(), 1); assert_eq!( @@ -704,7 +771,8 @@ async fn adapter_truncates_oversized_raw_snippet_text() { let snippets = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 10)) .await - .unwrap(); + .unwrap() + .snippets; assert_eq!(snippets.len(), 1); assert!(snippets[0].model_content.len() <= 512); @@ -730,7 +798,8 @@ async fn adapter_caps_aggregate_model_content_bytes() { let snippets = service .load_memory_snippets(test_request("tenant-a", "user-x", None, None, 20)) .await - .unwrap(); + .unwrap() + .snippets; let total_bytes: usize = snippets .iter() diff --git a/crates/loop/ironclaw_loop_host/src/lib.rs b/crates/loop/ironclaw_loop_host/src/lib.rs index 1b603f20544..3fb178fb174 100644 --- a/crates/loop/ironclaw_loop_host/src/lib.rs +++ b/crates/loop/ironclaw_loop_host/src/lib.rs @@ -211,9 +211,10 @@ use ironclaw_loop_contracts::{ LoopHostMilestoneSink, LoopInlineMessageBody, LoopInputCursor, LoopModelMessage, LoopModelPort, LoopModelRequest, LoopModelResponse, LoopModelUsage, LoopPromptBundleAuthority, LoopRequest, LoopRequestBatch, LoopRunContext, LoopRunInfoPort, LoopSafeSummary, LoopTranscriptPort, - MemoryPromptContextService, ModelProfileId, ModelStreamChunk, ParentLoopOutput, PromptMode, - UpdateAssistantDraft, VisibleCapabilityRequest, VisibleCapabilitySurface, resolution, - sanitize_model_visible_text, sort_instruction_snippets_for_prompt, + MemoryPromptContextLoad, MemoryPromptContextService, ModelProfileId, ModelStreamChunk, + ParentLoopOutput, PromptMode, UpdateAssistantDraft, VisibleCapabilityRequest, + VisibleCapabilitySurface, resolution, sanitize_model_visible_text, + sort_instruction_snippets_for_prompt, }; use ironclaw_outbound::{ OutboundError, ReplyAttachmentHandle, ReplyAttachmentIntent, ReplyAttachmentIntentPort, @@ -334,10 +335,17 @@ where /// non-optional null-object `user_profile_source`, this is a genuine `Option`.) // arch-exempt: optional_arc, deferred production wiring, issue #5013 memory_context_service: Option>, - /// Per-run cache for the fetched memory snippets. Shared across clones via + /// Per-run cache for the fetched memory load. Shared across clones via /// `Arc` so the "fetch once per run" guarantee holds even if the port is - /// cloned, exactly like `identity_candidates`. - memory_snippets_cache: Arc>>, + /// cloned, exactly like `identity_candidates`. The cached value is the + /// whole [`MemoryPromptContextLoad`], degradations included: a failed + /// fetch must not be remembered as a plain empty result for the rest of + /// the run. + memory_snippets_cache: Arc>, + /// One-shot guard so a degraded memory retrieval produces exactly ONE + /// operator-visible driver note per run, however many prompt builds read + /// the cached load. `Arc`-shared for the same reason as the cache itself. + memory_degradation_note_emitted: Arc>, /// Pre-resolved channel conversation history for shared-channel runs /// (UNTRUSTED third-party text carried on the run's persisted product /// context). Rendered as ONE framed system-context block per prompt @@ -412,6 +420,7 @@ where milestone_sink: None, memory_context_service: None, memory_snippets_cache: Arc::new(OnceCell::new()), + memory_degradation_note_emitted: Arc::new(OnceCell::new()), channel_conversation_context: None, } } diff --git a/crates/loop/ironclaw_loop_host/src/memory_context.rs b/crates/loop/ironclaw_loop_host/src/memory_context.rs index e6326792056..76b42eaac0b 100644 --- a/crates/loop/ironclaw_loop_host/src/memory_context.rs +++ b/crates/loop/ironclaw_loop_host/src/memory_context.rs @@ -1,4 +1,10 @@ -use ironclaw_loop_contracts::{LoopContextSnippet, MemoryPromptContextRequest}; +use std::sync::Arc; + +use ironclaw_loop_contracts::{ + LoopContextSnippet, LoopDriverNoteKind, LoopHostMilestoneEmitter, LoopSafeSummary, + MemoryPromptContextLoad, MemoryPromptContextRequest, MemoryRetrievalDegradation, + MemoryRetrievalFailureKind, MemoryRetrievalLane, +}; use ironclaw_threads::{ContextMessage, MessageKind, SessionThreadService}; use crate::ThreadBackedLoopContextPort; @@ -20,13 +26,30 @@ where /// per-iteration calls reuse the cached snippets (the "fetch once per run" /// guarantee). When no service is wired, or there is no actor / user message /// to scope a query to, this returns empty. A fetch failure degrades to empty - /// and never fails the turn. + /// and never fails the turn — but it is recorded, not laundered: see + /// [`Self::load_memory_context_once`]. pub(super) async fn load_memory_snippets_once( &self, context_messages: &[ContextMessage], ) -> Vec { + self.load_memory_context_once(context_messages) + .await + .snippets + } + + /// The full memory load for this run: the admitted snippets plus every lane + /// that FAILED rather than simply matching nothing. + /// + /// The distinction matters because both used to look identical downstream. + /// A memory backend that is down produced the same empty prompt section as + /// a user with nothing relevant stored, so "it forgot" and "retrieval broke" + /// were indistinguishable in tests and in operator diagnostics. + pub(super) async fn load_memory_context_once( + &self, + context_messages: &[ContextMessage], + ) -> MemoryPromptContextLoad { let Some(service) = self.memory_context_service.as_deref() else { - return Vec::new(); + return MemoryPromptContextLoad::default(); }; // Build the request BEFORE touching the cache. When there is no actor or no // user message yet, there is nothing to query: return empty WITHOUT seeding @@ -35,31 +58,104 @@ where // memory to empty for the rest of the run). Only seed the cell once a real // request exists. let Some(request) = self.build_memory_prompt_context_request(context_messages) else { - return Vec::new(); + return MemoryPromptContextLoad::default(); }; - // Fetch exactly once per run and CACHE the outcome - including an empty vec - // on failure. A down or slow memory service must not be re-hit on every - // model step of the run: the prior `get_or_try_init` left the cell - // uninitialized on error, so each iteration retried and could stack - // timeouts into latency spikes. A retrieval failure degrades to empty memory - // for the rest of the run rather than failing the turn; the per-run cache - // makes that decision exactly once. - let snippets = self + // Fetch exactly once per run and CACHE the outcome. A down or slow memory + // service must not be re-hit on every model step of the run: the prior + // `get_or_try_init` left the cell uninitialized on error, so each iteration + // retried and could stack timeouts into latency spikes. + // + // The cache therefore still holds a failed fetch for the rest of the run — + // but it caches the whole `MemoryPromptContextLoad`, so the cached value + // RECORDS that it failed instead of being indistinguishable from "no + // matching memory". A hard error from the service is folded into the same + // shape with an `Unavailable` degradation covering both lanes. + let load = self .memory_snippets_cache .get_or_init(|| async { match service.load_memory_snippets(request).await { - Ok(snippets) => snippets, + Ok(load) => load, Err(error) => { tracing::debug!( kind = ?error.kind, "memory context fetch failed; degrading to empty memory for this run" ); - Vec::new() + MemoryPromptContextLoad { + snippets: Vec::new(), + degradations: vec![ + MemoryRetrievalDegradation::new( + MemoryRetrievalLane::ShortTerm, + MemoryRetrievalFailureKind::Unavailable, + ), + MemoryRetrievalDegradation::new( + MemoryRetrievalLane::LongTerm, + MemoryRetrievalFailureKind::Unavailable, + ), + ], + } } } }) - .await; - snippets.clone() + .await + .clone(); + self.publish_memory_retrieval_degraded(&load.degradations); + load + } + + /// Surface a degraded memory retrieval to the operator as a driver note. + /// + /// The note is the operator-visible half of the typed degradation: it rides + /// the milestone sink this port already holds and reaches the live work + /// summary, the same route `publish_personal_context_admitted` uses and the + /// same rationale as `EventSubscriptionTerminated` — a subsystem that + /// stopped contributing must not be silently invisible. + /// + /// Deliberately NOT `warn!`/`info!`: those levels render in the REPL and + /// corrupt the terminal UI, and this fires from a background prompt build. + /// The summary carries only closed-vocabulary lane and failure labels, never + /// a backend message, a query, or a path. + /// + /// The per-run `OnceCell` above means the fetch happens once, but this is + /// called on every prompt build that reads the cache, so it is guarded by + /// its own cell to stay at one note per run. + fn publish_memory_retrieval_degraded(&self, degradations: &[MemoryRetrievalDegradation]) { + if degradations.is_empty() { + return; + } + let Some(milestone_sink) = self.milestone_sink.as_ref() else { + return; + }; + if self.memory_degradation_note_emitted.set(()).is_err() { + return; + } + let lanes = degradations + .iter() + .map(|degradation| { + format!( + "{}:{}", + degradation.lane.as_str(), + degradation.kind.as_str() + ) + }) + .collect::>() + .join(", "); + let summary = match LoopSafeSummary::new(format!("memory retrieval degraded ({lanes})")) { + Ok(summary) => summary, + Err(error) => { + tracing::debug!("failed to build memory degradation milestone: {error}"); + return; + } + }; + let context = self.run_context.clone(); + let milestone_sink = Arc::clone(milestone_sink); + tokio::spawn(async move { + if let Err(error) = LoopHostMilestoneEmitter::new(context, milestone_sink) + .driver_note(LoopDriverNoteKind::Context, summary) + .await + { + tracing::debug!("failed to emit memory degradation milestone: {error}"); + } + }); } /// Build the memory request from the run context. Returns `None` (no memory diff --git a/crates/loop/ironclaw_loop_host/tests/llm_gateway.rs b/crates/loop/ironclaw_loop_host/tests/llm_gateway.rs index 6d597e2da96..1b048dc53cc 100644 --- a/crates/loop/ironclaw_loop_host/tests/llm_gateway.rs +++ b/crates/loop/ironclaw_loop_host/tests/llm_gateway.rs @@ -4202,16 +4202,18 @@ impl MemoryPromptContextService for CountingMemoryContextService { async fn load_memory_snippets( &self, request: MemoryPromptContextRequest, - ) -> Result, AgentLoopHostError> { + ) -> Result { self.fetches.fetch_add(1, Ordering::SeqCst); *self.last_query.lock().unwrap() = Some(request.query.clone()); let content = format!("Untrusted memory content: {}", request.query); - Ok(vec![LoopContextSnippet { - snippet_ref: "memory-snippet:caller-test".to_string(), - model_content: content.clone(), - safe_summary: content, - metadata: None, - }]) + Ok(ironclaw_loop_contracts::MemoryPromptContextLoad::healthy( + vec![LoopContextSnippet { + snippet_ref: "memory-snippet:caller-test".to_string(), + model_content: content.clone(), + safe_summary: content, + metadata: None, + }], + )) } } diff --git a/crates/loop/ironclaw_loop_host/tests/thread_loop_host_contract.rs b/crates/loop/ironclaw_loop_host/tests/thread_loop_host_contract.rs index e6b0a072d66..8fe6d27ff87 100644 --- a/crates/loop/ironclaw_loop_host/tests/thread_loop_host_contract.rs +++ b/crates/loop/ironclaw_loop_host/tests/thread_loop_host_contract.rs @@ -6898,14 +6898,16 @@ impl ironclaw_loop_contracts::MemoryPromptContextService for RecordingMemoryProm async fn load_memory_snippets( &self, request: ironclaw_loop_contracts::MemoryPromptContextRequest, - ) -> Result, AgentLoopHostError> { + ) -> Result { self.calls.lock().expect("memory calls lock").push(request); - Ok(vec![LoopContextSnippet { - snippet_ref: "memory-snippet:test".to_string(), - model_content: "remembered fact".to_string(), - safe_summary: "remembered fact".to_string(), - metadata: None, - }]) + Ok(ironclaw_loop_contracts::MemoryPromptContextLoad::healthy( + vec![LoopContextSnippet { + snippet_ref: "memory-snippet:test".to_string(), + model_content: "remembered fact".to_string(), + safe_summary: "remembered fact".to_string(), + metadata: None, + }], + )) } } diff --git a/tests/integration/group_memory/main.rs b/tests/integration/group_memory/main.rs index 176e0207b53..5d77a364789 100644 --- a/tests/integration/group_memory/main.rs +++ b/tests/integration/group_memory/main.rs @@ -19,6 +19,7 @@ mod support; mod scenario_always_on_memory_recall_libsql; mod scenario_disabled_binding_offers_no_memory_tools; mod scenario_lifecycle_gates_host_memory_calls; +mod scenario_memory_retrieval_failure_is_visible; mod scenario_memory_search_finds_seeded; mod scenario_memory_tree_reflects_structure; mod scenario_paraphrased_prompt_recall_libsql; @@ -99,5 +100,14 @@ async fn memory_group_e2e() { scenario_paraphrased_prompt_recall_libsql::run().await, ); + // Scenario 9 (#7185/#7275 observability): a retrieval/backend FAILURE is + // distinguishable from "no matching memory" — same turn, same assertions, + // only the bound provider's lane outcome differs. Own groups (the provider + // is a construction input). + report.record( + "memory_retrieval_failure_is_visible", + scenario_memory_retrieval_failure_is_visible::run().await, + ); + report.assert_all_passed(); } diff --git a/tests/integration/group_memory/scenario_memory_retrieval_failure_is_visible.rs b/tests/integration/group_memory/scenario_memory_retrieval_failure_is_visible.rs new file mode 100644 index 00000000000..f9d8e244c9e --- /dev/null +++ b/tests/integration/group_memory/scenario_memory_retrieval_failure_is_visible.rs @@ -0,0 +1,115 @@ +//! #7185 / #7275 observability: a memory RETRIEVAL FAILURE is distinguishable +//! from "no matching memory", through the real composition. +//! +//! Both cases used to look identical from outside: a lane error was logged at +//! `debug!` and returned as an empty snippet list, so a memory backend that was +//! down produced exactly the same prompt — and exactly the same test evidence — +//! as a user with nothing relevant stored. Operators had no signal at all. +//! +//! The two arms below are byte-identical except for what the bound memory +//! provider returns from its retrieval lanes: `Err(unavailable)` versus +//! `Ok(vec![])`. Only the failing arm emits the operator-visible driver note. +//! +//! The note rides the milestone sink, not a log level: `info!`/`warn!` output +//! is rendered by the REPL and corrupts the terminal UI, and this fires from a +//! background prompt build. + +use std::sync::Arc; + +use async_trait::async_trait; +use ironclaw_extension_contracts::memory::{MemoryDescriptor, MemoryLifecycleHook}; +use ironclaw_memory::{ + MemoryInvocation, MemoryService, MemoryServiceContextRequest, MemoryServiceContextSnippet, + MemoryServiceError, MemoryServiceProfileReadResponse, MemoryServiceRecordRequest, + MemoryServiceRecordResponse, +}; + +use super::reborn_support::group::{HarnessResult, RebornIntegrationGroup}; +use super::reborn_support::reply::RebornScriptedReply; + +/// A bound memory provider whose retrieval lanes either fail or answer empty. +struct RetrievalOutcomeMemoryService { + lanes_fail: bool, +} + +#[async_trait] +impl MemoryService for RetrievalOutcomeMemoryService { + async fn read_long_term( + &self, + _invocation: MemoryInvocation, + _request: MemoryServiceContextRequest, + ) -> Result, MemoryServiceError> { + if self.lanes_fail { + return Err(MemoryServiceError::unavailable()); + } + Ok(Vec::new()) + } + + async fn read_short_term( + &self, + _invocation: MemoryInvocation, + _request: MemoryServiceContextRequest, + ) -> Result, MemoryServiceError> { + if self.lanes_fail { + return Err(MemoryServiceError::unavailable()); + } + Ok(Vec::new()) + } + + async fn record_interaction( + &self, + _invocation: MemoryInvocation, + _request: MemoryServiceRecordRequest, + ) -> Result { + Ok(MemoryServiceRecordResponse { recorded: true }) + } + + async fn profile_read( + &self, + _invocation: MemoryInvocation, + ) -> Result { + Ok(MemoryServiceProfileReadResponse { document: None }) + } +} + +async fn group_with_lanes(lanes_fail: bool) -> HarnessResult { + RebornIntegrationGroup::builder() + .with_bound_memory_provider( + Arc::new(RetrievalOutcomeMemoryService { lanes_fail }) as Arc, + MemoryDescriptor { + lifecycle: MemoryLifecycleHook::ALL.to_vec(), + ..MemoryDescriptor::default() + }, + ) + .builtin_tools() + .await +} + +pub async fn run() -> HarnessResult<()> { + // ── Arm 1: both retrieval lanes fail. The turn still completes (memory is + // best-effort and must never break a run) AND the failure is surfaced. ─── + let failing = group_with_lanes(true).await?; + let harness = failing + .thread("conv-memory-retrieval-failed") + .script([RebornScriptedReply::text("answered anyway")]) + .build() + .await?; + harness.submit_turn("what do you remember?").await?; + harness.assert_reply_contains("answered anyway").await?; + harness.assert_memory_retrieval_degraded_note().await?; + drop(harness); + + // ── Arm 2: healthy lanes with nothing to return. Identical turn, no note — + // this is what makes arm 1 evidence rather than noise. ─────────────────── + let healthy = group_with_lanes(false).await?; + let harness = healthy + .thread("conv-memory-retrieval-empty") + .script([RebornScriptedReply::text("answered anyway")]) + .build() + .await?; + harness.submit_turn("what do you remember?").await?; + harness.assert_reply_contains("answered anyway").await?; + harness.assert_no_memory_retrieval_degraded_note().await?; + + Ok(()) +} diff --git a/tests/integration/support/assertions.rs b/tests/integration/support/assertions.rs index 8c41175b97d..1646696a260 100644 --- a/tests/integration/support/assertions.rs +++ b/tests/integration/support/assertions.rs @@ -1192,6 +1192,69 @@ impl RebornIntegrationHarness { .into()) } + /// Every `DriverNote` milestone summary recorded by this harness, in order. + /// + /// A driver note is the operator-visible channel for "a subsystem stopped + /// contributing but the run continued" — it reaches the live work summary + /// rather than only a log line. + fn driver_note_summaries(&self) -> Vec { + self.loop_milestones() + .into_iter() + .filter_map(|milestone| match milestone.kind { + LoopHostMilestoneKind::DriverNote { safe_summary, .. } => { + Some(safe_summary.as_str().to_string()) + } + _ => None, + }) + .collect() + } + + /// Assert an operator-visible driver note reported degraded memory + /// retrieval — the evidence that a retrieval/backend FAILURE is + /// distinguishable from "no matching memory" outside the returned value. + /// + /// Driver notes are emitted from a spawned task, so poll over a bounded + /// window rather than racing the emit. + pub async fn assert_memory_retrieval_degraded_note(&self) -> HarnessResult<()> { + let mut waited = std::time::Duration::ZERO; + while waited < std::time::Duration::from_secs(5) { + if self + .driver_note_summaries() + .iter() + .any(|summary| summary.contains("memory retrieval degraded")) + { + return Ok(()); + } + tokio::time::sleep(std::time::Duration::from_millis(25)).await; + waited += std::time::Duration::from_millis(25); + } + Err(format!( + "no driver note reporting degraded memory retrieval; saw {:?}", + self.driver_note_summaries() + ) + .into()) + } + + /// Assert NO memory-degradation driver note was emitted. Pairs with + /// [`Self::assert_memory_retrieval_degraded_note`]: a healthy backend that + /// simply matched nothing must not look like an outage. + pub async fn assert_no_memory_retrieval_degraded_note(&self) -> HarnessResult<()> { + // Give a stray emit the same window the positive assertion waits on, so + // this cannot pass merely by checking too early. + tokio::time::sleep(std::time::Duration::from_millis(250)).await; + let summaries = self.driver_note_summaries(); + if summaries + .iter() + .any(|summary| summary.contains("memory retrieval degraded")) + { + return Err(format!( + "an empty-but-healthy memory retrieval reported degradation: {summaries:?}" + ) + .into()); + } + Ok(()) + } + /// Assert exactly one typed redaction milestone was emitted for the applied /// compaction since `baseline`, carrying `expected_redactions`. pub async fn assert_compaction_redacted_once_since( From 8fa650f059da23691ad2476ae3d86c351cbb8ec3 Mon Sep 17 00:00:00 2001 From: serrrfirat Date: Thu, 13 Aug 2026 13:05:20 +0300 Subject: [PATCH 3/3] =?UTF-8?q?fix(memory):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20backend=20parity,=20note=20retry,=20scenario=20ownership?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #7553: - libSQL answered a content-term-free ranked query with `Unsupported` when the prefix had no declared FTS index, while PostgreSQL and the in-memory backend answered "empty". The answer does not depend on an index, so it must not depend on the bound backend: read the query before resolving the FTS table. Pinned in the shared `ranked_fts_contract`, which runs on all three backends. - The degraded-retrieval driver note marked itself emitted before the publish succeeded, so one transient milestone-sink error suppressed it for the rest of the run — restoring the exact ambiguity between broken and empty memory this change exists to remove. Adopt the two-step guard `publish_personal_context_admitted` already uses (in-flight flag + set-on-success). Regression test verified red against the old guard. - `Filter::Fts` and `Filter::FtsRanked` carried separate copies of the in-memory tokenizer; they must agree on what counts as a term occurrence, so share one `fts_tokens`. - `scenario_paraphrased_prompt_recall_libsql` built its own group; the group-scenario contract puts that in the binary. It keeps a dedicated libSQL group rather than joining the shared one, because scenarios 6 and 7 assert which snippets reach the prompt and a ranked top-N lane over one store would couple those assertions to sibling seed data. - Register both new scenarios in `tests/CLAUDE.md` §3.4 and update counts. Co-Authored-By: Claude Opus 5 (1M context) --- crates/loop/ironclaw_loop_host/src/lib.rs | 6 ++ .../ironclaw_loop_host/src/memory_context.rs | 39 ++++++-- .../tests/thread_loop_host_contract.rs | 94 +++++++++++++++++++ .../ironclaw_filesystem/src/in_memory.rs | 30 +++--- .../ironclaw_filesystem/src/libsql.rs | 15 ++- .../tests/db_root_filesystem_contract.rs | 25 +++++ tests/CLAUDE.md | 10 +- tests/integration/group_memory/main.rs | 15 ++- ...enario_paraphrased_prompt_recall_libsql.rs | 4 +- 9 files changed, 205 insertions(+), 33 deletions(-) diff --git a/crates/loop/ironclaw_loop_host/src/lib.rs b/crates/loop/ironclaw_loop_host/src/lib.rs index 3fb178fb174..c3312a20c24 100644 --- a/crates/loop/ironclaw_loop_host/src/lib.rs +++ b/crates/loop/ironclaw_loop_host/src/lib.rs @@ -345,7 +345,12 @@ where /// One-shot guard so a degraded memory retrieval produces exactly ONE /// operator-visible driver note per run, however many prompt builds read /// the cached load. `Arc`-shared for the same reason as the cache itself. + /// Set only AFTER the note is published, so a transient sink failure does + /// not permanently suppress it; `memory_degradation_note_in_flight` keeps + /// the window between claim and publish from producing duplicates. Same + /// pair, and same rationale, as `personal_context_admitted`. memory_degradation_note_emitted: Arc>, + memory_degradation_note_in_flight: Arc, /// Pre-resolved channel conversation history for shared-channel runs /// (UNTRUSTED third-party text carried on the run's persisted product /// context). Rendered as ONE framed system-context block per prompt @@ -421,6 +426,7 @@ where memory_context_service: None, memory_snippets_cache: Arc::new(OnceCell::new()), memory_degradation_note_emitted: Arc::new(OnceCell::new()), + memory_degradation_note_in_flight: Arc::new(AtomicBool::new(false)), channel_conversation_context: None, } } diff --git a/crates/loop/ironclaw_loop_host/src/memory_context.rs b/crates/loop/ironclaw_loop_host/src/memory_context.rs index 76b42eaac0b..9f8198b8c72 100644 --- a/crates/loop/ironclaw_loop_host/src/memory_context.rs +++ b/crates/loop/ironclaw_loop_host/src/memory_context.rs @@ -1,4 +1,5 @@ use std::sync::Arc; +use std::sync::atomic::Ordering; use ironclaw_loop_contracts::{ LoopContextSnippet, LoopDriverNoteKind, LoopHostMilestoneEmitter, LoopSafeSummary, @@ -116,8 +117,16 @@ where /// a backend message, a query, or a path. /// /// The per-run `OnceCell` above means the fetch happens once, but this is - /// called on every prompt build that reads the cache, so it is guarded by - /// its own cell to stay at one note per run. + /// called on every prompt build that reads the cache, so it is guarded to + /// stay at one note per run. + /// + /// That guard is claimed in two steps, matching + /// `publish_personal_context_admitted`: an in-flight flag suppresses + /// duplicates while a publish is outstanding, and the `OnceCell` is set + /// only once a publish SUCCEEDS. Marking the note emitted up front would + /// mean a single transient sink error silently suppressed it for the rest + /// of the run — putting the operator back in exactly the state this change + /// exists to fix, unable to tell broken retrieval from empty memory. fn publish_memory_retrieval_degraded(&self, degradations: &[MemoryRetrievalDegradation]) { if degradations.is_empty() { return; @@ -125,7 +134,13 @@ where let Some(milestone_sink) = self.milestone_sink.as_ref() else { return; }; - if self.memory_degradation_note_emitted.set(()).is_err() { + if self.memory_degradation_note_emitted.get().is_some() { + return; + } + if self + .memory_degradation_note_in_flight + .swap(true, Ordering::AcqRel) + { return; } let lanes = degradations @@ -142,19 +157,29 @@ where let summary = match LoopSafeSummary::new(format!("memory retrieval degraded ({lanes})")) { Ok(summary) => summary, Err(error) => { + self.memory_degradation_note_in_flight + .store(false, Ordering::Release); tracing::debug!("failed to build memory degradation milestone: {error}"); return; } }; let context = self.run_context.clone(); let milestone_sink = Arc::clone(milestone_sink); + let emitted = Arc::clone(&self.memory_degradation_note_emitted); + let in_flight = Arc::clone(&self.memory_degradation_note_in_flight); tokio::spawn(async move { - if let Err(error) = LoopHostMilestoneEmitter::new(context, milestone_sink) + let publish_result = LoopHostMilestoneEmitter::new(context, milestone_sink) .driver_note(LoopDriverNoteKind::Context, summary) - .await - { - tracing::debug!("failed to emit memory degradation milestone: {error}"); + .await; + match publish_result { + Ok(()) => { + let _ = emitted.set(()); + } + Err(error) => { + tracing::debug!("failed to emit memory degradation milestone: {error}"); + } } + in_flight.store(false, Ordering::Release); }); } diff --git a/crates/loop/ironclaw_loop_host/tests/thread_loop_host_contract.rs b/crates/loop/ironclaw_loop_host/tests/thread_loop_host_contract.rs index 8fe6d27ff87..1ea9d6b9905 100644 --- a/crates/loop/ironclaw_loop_host/tests/thread_loop_host_contract.rs +++ b/crates/loop/ironclaw_loop_host/tests/thread_loop_host_contract.rs @@ -6970,3 +6970,97 @@ async fn thread_context_port_loads_memory_snippets_through_wired_service_once_pe assert_eq!(calls[0].scope, run_context.scope); assert_eq!(calls[0].actor.user_id, owner); } + +/// Reports a long-term lane that FAILED rather than matching nothing, so the +/// port has a degradation to publish. +struct DegradedMemoryPromptContextService; + +#[async_trait] +impl ironclaw_loop_contracts::MemoryPromptContextService for DegradedMemoryPromptContextService { + async fn load_memory_snippets( + &self, + _request: ironclaw_loop_contracts::MemoryPromptContextRequest, + ) -> Result { + Ok(ironclaw_loop_contracts::MemoryPromptContextLoad { + snippets: Vec::new(), + degradations: vec![ironclaw_loop_contracts::MemoryRetrievalDegradation::new( + ironclaw_loop_contracts::MemoryRetrievalLane::LongTerm, + ironclaw_loop_contracts::MemoryRetrievalFailureKind::Unavailable, + )], + }) + } +} + +/// A transient milestone-sink failure must not permanently suppress the +/// degraded-retrieval note. +/// +/// The note is the ONLY operator-visible signal that retrieval broke rather +/// than simply matching nothing. Marking it emitted before the publish +/// succeeds would mean one failed sink call put the operator back in exactly +/// the state this behavior exists to escape — for the rest of the run, with no +/// second chance. Same guarantee, and same two-step guard, as +/// `context_port_survives_personal_context_admitted_milestone_sink_failure`. +#[traced_test] +#[tokio::test] +async fn context_port_retries_memory_degradation_note_after_milestone_sink_failure() { + let fixture = ThreadFixture::new().await; + let owner = fixture + .thread_scope + .owner_user_id + .clone() + .expect("fixture thread scope carries an owner"); + let run_context = fixture + .run_context + .clone() + .with_actor(TurnActor::new(owner)); + let milestone_sink = Arc::new(FailOnceMilestoneSink::default()); + let adapter = ThreadBackedLoopContextPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + run_context, + 16, + ) + .with_memory_context_service(Arc::new(DegradedMemoryPromptContextService) as Arc<_>) + .with_milestone_sink(milestone_sink.clone()); + + let request = || LoopContextRequest { + after: None, + limit: 16, + mode: ironclaw_loop_contracts::PromptMode::TextOnly, + }; + + // First prompt build: the publish is attempted and fails. The turn survives. + adapter.load_loop_context(request()).await.unwrap(); + wait_for_fail_once_attempts(&milestone_sink, 1).await; + assert!( + milestone_sink.milestones().is_empty(), + "the first publish failed, so no note was delivered" + ); + assert!(logs_contain("failed to emit memory degradation milestone")); + + // Second prompt build of the SAME run: the cached load still records the + // degradation, and the note is retried rather than suppressed. + adapter.load_loop_context(request()).await.unwrap(); + wait_for_fail_once_attempts(&milestone_sink, 2).await; + let milestones = milestone_sink.milestones(); + assert_eq!( + milestones.len(), + 1, + "exactly one note reaches the operator: retried after the failure, not duplicated" + ); + assert!(matches!( + &milestones[0].kind, + LoopHostMilestoneKind::DriverNote { kind, safe_summary } + if *kind == LoopDriverNoteKind::Context + && safe_summary.as_str() == "memory retrieval degraded (long_term:unavailable)" + )); + + // A third build must NOT publish again — the success is remembered. + adapter.load_loop_context(request()).await.unwrap(); + tokio::task::yield_now().await; + assert_eq!( + milestone_sink.milestones().len(), + 1, + "the note stays at one per run once it has actually been delivered" + ); +} diff --git a/crates/substrates/ironclaw_filesystem/src/in_memory.rs b/crates/substrates/ironclaw_filesystem/src/in_memory.rs index 1b2150850c6..95e41149f5b 100644 --- a/crates/substrates/ironclaw_filesystem/src/in_memory.rs +++ b/crates/substrates/ironclaw_filesystem/src/in_memory.rs @@ -912,14 +912,24 @@ fn fts_naive_matches(stored: &str, terms: &[String]) -> bool { if terms.is_empty() { return false; } - let stored_lower = stored.to_lowercase(); - let tokens: std::collections::HashSet<&str> = stored_lower - .split(|character: char| !character.is_alphanumeric()) - .filter(|token| !token.is_empty()) - .collect(); + let tokens = fts_tokens(stored); terms .iter() - .all(|term| tokens.contains(term.to_lowercase().as_str())) + .all(|term| tokens.contains(&term.to_lowercase())) +} + +/// The whole-token set of `stored`, lowercased. THE single definition of "what +/// counts as a term occurrence" for this backend: `Filter::Fts` (all terms) and +/// `Filter::FtsRanked` (any term, ranked) must agree on it or the two matchers +/// stop being comparable, so both go through here rather than each carrying its +/// own copy of the split rule. +fn fts_tokens(stored: &str) -> std::collections::HashSet { + stored + .to_lowercase() + .split(|character: char| !character.is_alphanumeric()) + .filter(|token| !token.is_empty()) + .map(str::to_string) + .collect() } /// Reference relevance score for [`Filter::FtsRanked`]: how many DISTINCT @@ -932,14 +942,10 @@ fn fts_term_coverage(stored: &str, terms: &[String]) -> usize { if terms.is_empty() { return 0; } - let stored_lower = stored.to_lowercase(); - let tokens: std::collections::HashSet<&str> = stored_lower - .split(|character: char| !character.is_alphanumeric()) - .filter(|token| !token.is_empty()) - .collect(); + let tokens = fts_tokens(stored); terms .iter() - .filter(|term| tokens.contains(term.to_lowercase().as_str())) + .filter(|term| tokens.contains(&term.to_lowercase())) .count() } diff --git a/crates/substrates/ironclaw_filesystem/src/libsql.rs b/crates/substrates/ironclaw_filesystem/src/libsql.rs index 36728b7c9ee..85e28f9699a 100644 --- a/crates/substrates/ironclaw_filesystem/src/libsql.rs +++ b/crates/substrates/ironclaw_filesystem/src/libsql.rs @@ -2396,6 +2396,17 @@ impl LibSqlRootFilesystem { query: query.to_string(), limit, }; + // Answer the empty-term query BEFORE looking for a declared index. + // `Filter::FtsRanked` documents that a query with no content terms + // matches nothing on every backend, and PostgreSQL returns an empty + // result without ever consulting an index. Resolving the FTS table + // first made libSQL alone answer `Unsupported` for a stop-words-only + // query against an undeclared prefix — a backend-dependent answer to a + // question whose answer does not depend on any index. + let terms = crate::index::plain_fts_terms(query); + if terms.is_empty() { + return Ok(Vec::new()); + } let fts_tables = self.discover_fts_tables_for_filter(path, &probe).await?; let Some(fts_table) = fts_tables.get(key.as_str()) else { return Err(FilesystemError::Unsupported { @@ -2403,10 +2414,6 @@ impl LibSqlRootFilesystem { operation: FilesystemOperation::Query, }); }; - let terms = crate::index::plain_fts_terms(query); - if terms.is_empty() { - return Ok(Vec::new()); - } let match_query = terms .into_iter() .map(|term| format!("\"{term}\"")) diff --git a/crates/substrates/ironclaw_filesystem/tests/db_root_filesystem_contract.rs b/crates/substrates/ironclaw_filesystem/tests/db_root_filesystem_contract.rs index 519476280ed..76c7dc43635 100644 --- a/crates/substrates/ironclaw_filesystem/tests/db_root_filesystem_contract.rs +++ b/crates/substrates/ironclaw_filesystem/tests/db_root_filesystem_contract.rs @@ -1518,6 +1518,31 @@ async fn ranked_fts_contract(filesystem: &F, base: &str) { .unwrap(); assert!(stop_words_only.is_empty()); + // ...and it stays "nothing" on a prefix where no FTS index was ever + // declared. Every case above declares the index first, so this is the one + // combination that can expose an ordering difference INSIDE a backend: + // libSQL resolves the FTS table before reading the query, PostgreSQL reads + // the query and never consults an index. The answer to "does a query with + // no content terms match anything" does not depend on an index, so it must + // not depend on which backend is bound. + let undeclared = VirtualPath::new(format!("{base}-undeclared")).unwrap(); + let stop_words_only_undeclared = filesystem + .query( + &undeclared, + &Filter::FtsRanked { + key: content.clone(), + query: "and the of to".into(), + limit: 10, + }, + Page::default(), + ) + .await; + assert!( + matches!(&stop_words_only_undeclared, Ok(entries) if entries.is_empty()), + "a content-term-free ranked query must be an empty result on every backend even where no \ + FTS index is declared, got {stop_words_only_undeclared:?}" + ); + // Nesting a ranking filter inside a compound would discard the ordering. let nested = filesystem .query( diff --git a/tests/CLAUDE.md b/tests/CLAUDE.md index 7f07a54ace6..9268f44e936 100644 --- a/tests/CLAUDE.md +++ b/tests/CLAUDE.md @@ -51,7 +51,7 @@ Tier-selection rule: `.claude/rules/testing.md`. | Extension lifecycle | 14 | 6 | ✓ | ✓ | | Channels (Slack/Telegram/webhook) | 2 | 3 | ✓ | ✓ | | Triggers / automations / routines | 11 | 2 | ✓ | ✓ | -| Memory & workspace | 6 | 2 | — | ✓ | +| Memory & workspace | 8 | 2 | — | ✓ | | Skills | 1 | 1 | — | ✓ | | Multi-user / scope isolation | 5 | 2 | 9 | ✓ | | Tools & tool dispatch | — | 11 | ✓ | ✓ | @@ -62,7 +62,7 @@ Tier-selection rule: `.claude/rules/testing.md`. | Providers (Google/Slack/GitHub contracts) | — | — | ✓ | ✓ | | Coverage/meta gates | — | 2 | ✓ | ✓ | -Totals: **53** group scenarios · **55** flat integration bins (49 in +Totals: **55** group scenarios · **55** flat integration bins (49 in `tests/integration/`, 6 in `tests/integration/auth/`) · **39** top-level Rust bins · **102** Python scenario files (**869** test functions) registered in the active Reborn coverage map below. Section 6 separately inventories retained and legacy @@ -70,7 +70,7 @@ Python scenarios, so its exhaustive totals are intentionally broader. --- -## 3. Group scenarios — `tests/integration/group_*/` (53) +## 3. Group scenarios — `tests/integration/group_*/` (55) Multi-thread journeys over ONE shared runtime and ONE shared set of stores. These are the canonical "a user does X in one conversation and sees the effect in another" tests. @@ -118,7 +118,7 @@ the canonical "a user does X in one conversation and sees the effect in another" | Have a stored-but-expired credential rejected, reconnect, and have the tool retry **with the new credential** | `scenario_expired_credential_resume.rs` | | Not resolve another person's approval prompt — each user answers their own | `scenario_multi_actor_gate_isolation.rs` | -### 3.4 Memory — `group_memory/` (7) +### 3.4 Memory — `group_memory/` (9) | The user can… | Evidence | |---|---| @@ -129,6 +129,8 @@ the canonical "a user does X in one conversation and sees the effect in another" | Trust that only the memory hooks the provider declares actually fire | `scenario_lifecycle_gates_host_memory_calls.rs` | | Ask a natural punctuated question in a new chat and receive explicitly saved memory — and only your own, never another user's, and never another conversation's raw transcript — framed as a recollection to verify (#7294), through the proactive prompt lane on the shipping libSQL backend | `scenario_proactive_prompt_recall_libsql.rs` | | Have the assistant remember a preference you mentioned in passing and still know it in a later chat that opens on a completely unrelated subject — full-text retrieval cannot cover this, because it matches on the current message's words | `scenario_always_on_memory_recall_libsql.rs` | +| Ask about a stored fact in their OWN words rather than the words it was saved in, and still have it recalled — the paraphrase shares only some of the saved sentence's terms and adds one it never contained (#7185) | `scenario_paraphrased_prompt_recall_libsql.rs` | +| Tell from the run itself that memory retrieval BROKE rather than simply having nothing to say — the two used to be the same silent empty section (#7185/#7275) | `scenario_memory_retrieval_failure_is_visible.rs` | ### 3.5 Multi-user — `group_multiuser/` (5) diff --git a/tests/integration/group_memory/main.rs b/tests/integration/group_memory/main.rs index 5d77a364789..7ffc407d65c 100644 --- a/tests/integration/group_memory/main.rs +++ b/tests/integration/group_memory/main.rs @@ -93,11 +93,20 @@ async fn memory_group_e2e() { // Scenario 8 (#7185): ranked-OR retrieval. A fact saved as one sentence is // recalled by a differently-worded question sharing only some of its - // content words — the case every-term (AND) matching silently missed. Own - // group, same libSQL reason as scenario 6. + // content words — the case every-term (AND) matching silently missed. + // + // The binary owns the group, per the group-scenario contract. It is a + // SEPARATE libSQL group rather than the shared `g` above for scenario 6's + // reason (the backend binding differs), and separate from scenarios 6/7 on + // purpose: those assert on which snippets reach the prompt, and a ranked + // top-N lane over one shared store would couple each scenario's positive + // assertion to the documents its siblings happened to seed first. + let paraphrase_group = RebornIntegrationGroup::builtin_tools_with_native_memory_libsql() + .await + .expect("libSQL-backed native memory group builds"); report.record( "paraphrased_prompt_recall_libsql", - scenario_paraphrased_prompt_recall_libsql::run().await, + scenario_paraphrased_prompt_recall_libsql::run(¶phrase_group).await, ); // Scenario 9 (#7185/#7275 observability): a retrieval/backend FAILURE is diff --git a/tests/integration/group_memory/scenario_paraphrased_prompt_recall_libsql.rs b/tests/integration/group_memory/scenario_paraphrased_prompt_recall_libsql.rs index a337f79cdcd..beb75650d03 100644 --- a/tests/integration/group_memory/scenario_paraphrased_prompt_recall_libsql.rs +++ b/tests/integration/group_memory/scenario_paraphrased_prompt_recall_libsql.rs @@ -26,9 +26,7 @@ use super::reborn_support::reply::RebornScriptedReply; /// simply carries everything. const UNWRITTEN_MARKER: &str = "wombat-31"; -pub async fn run() -> HarnessResult<()> { - let group = RebornIntegrationGroup::builtin_tools_with_native_memory_libsql().await?; - +pub async fn run(group: &RebornIntegrationGroup) -> HarnessResult<()> { let writer = group .thread("conv-memory-paraphrase-writer") .script([