fix(agent): query expansion panicked on non-ASCII and rewrote text inside words

Probing what QueryExpander::expand actually produces for LongMemEval questions
turned up two defects in the same helper.

`replace_word_case_insensitive` did a plain substring replace despite its
name, so acronym expansion fired inside ordinary words: "training" became
"trArtificial Intelligencening" ("ai") and "programming" became
"Pull Requestogramming" ("pr"). Nearly every acronym expansion of prose was
corrupt. Matching now requires word boundaries at both ends; real acronyms
(API, database) still expand in both directions.

The same helper searched `text.to_lowercase()` and then sliced `text` with the
offsets it found. That holds only while lowercasing preserves byte length, and
it does not — Turkish 'İ' is 2 bytes and lowercases to 3. Offsets after such a
character drifted, so output was silently corrupted ("İstanbul AI trip" lost a
character) or the slice landed inside a character or past the end and
panicked: `expand("İ AI")` was enough, from a plain query string. Matching now
walks the original string, comparing case-insensitively char by char, so
offsets are always valid.

Regression tests cover both, plus whole-word matching at string edges. The
morphological rules remain crude ("during" -> "dured"); that is a quality
limit, not a correctness bug, and is now documented as a reason to measure
before enabling expansion on a retrieval path.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

CHANGELOG.md

@@ -3,6 +3,19 @@
 ## Unreleased
 
 ### Retrieval quality
+- `clawhdf5-agent`: **`QueryExpander::expand` panicked on ordinary non-ASCII
+  input** — `"İ AI"` was enough. It searched a lowercased copy of the query and
+  then sliced the *original* with those offsets, which only works while
+  lowercasing preserves byte length (Turkish `İ` is 2 bytes and lowercases to
+  3). Depending on where the offsets drifted it either corrupted the output
+  ("İstanbul AI trip" lost a character) or panicked. Matching now walks the
+  original string.
+- `clawhdf5-agent`: query expansion no longer rewrites text inside words.
+  `replace_word_case_insensitive` did a plain substring replace despite its
+  name, so "training" became "trArtificial Intelligencening" and "programming"
+  became "Pull Requestogramming" — every acronym expansion of ordinary prose
+  was corrupt. Matches now require word boundaries; genuine acronyms
+  (`API`, `database`) still expand.
 - `clawhdf5-agent`: **the default fusion weights are now the measured ones.**
   A sweep of every 0.1 step over the full LongMemEval haystack (500 questions,
   real MiniLM embeddings) shows the long-standing `0.7/0.3` default is

crates/clawhdf5-agent/src/query_expand.rs

@@ -6,6 +6,11 @@
 //! - Temporal expansion (time-related rewrites)
 //! - Morphological variants (stemming-like transforms)
 //! - Knowledge graph expansion (entity aliases and neighbors)
+//!
+//! The morphological rules are crude suffix swaps, so some variants are not
+//! words ("during" -> "dured"). That is tolerable for a BM25 stage, which
+//! simply finds no postings for a nonsense term, but it means expansion is not
+//! free: measure before enabling it on a retrieval path.
 
 use crate::knowledge::KnowledgeCache;
 
@@ -340,18 +345,85 @@ fn contains_phrase(text: &str, phrase: &str) -> bool {
 
 /// Replace a phrase in `text` case-insensitively, preserving surrounding case.
 fn replace_word_case_insensitive(text: &str, from: &str, to: &str) -> String {
-    case_insensitive_replace(text, from, to)
+    replace_first(text, from, to, MatchKind::WholeWord)
 }
 
 fn case_insensitive_replace(text: &str, from: &str, to: &str) -> String {
-    let lower = text.to_lowercase();
-    let lower_from = from.to_lowercase();
-    if let Some(pos) = lower.find(&lower_from) {
-        let end = pos + from.len();
-        format!("{}{}{}", &text[..pos], to, &text[end..])
-    } else {
-        text.to_string()
+    replace_first(text, from, to, MatchKind::Substring)
 }
+
+/// Whether a match may fall inside a larger word.
+#[derive(Clone, Copy, PartialEq)]
+enum MatchKind {
+    /// Match anywhere, including inside another word.
+    Substring,
+    /// Match only when both ends sit on a word boundary.
+    WholeWord,
+}
+
+/// Replace the first case-insensitive match of `from` in `text` with `to`.
+///
+/// Matching walks the *original* string rather than a lowercased copy. The
+/// previous implementation searched `text.to_lowercase()` and then sliced
+/// `text` with the offsets it found, which only holds while lowercasing
+/// preserves byte length. It does not: Turkish `İ` (2 bytes) lowercases to
+/// `i` + U+0307 (3 bytes), so every later offset was wrong — silently
+/// corrupting the output, or panicking when an offset landed inside a
+/// character or past the end. `"İ AI"` was enough to panic.
+fn replace_first(text: &str, from: &str, to: &str, kind: MatchKind) -> String {
+    match find_case_insensitive(text, from, kind) {
+        Some((start, end)) => {
+            let mut out = String::with_capacity(text.len() - (end - start) + to.len());
+            out.push_str(&text[..start]);
+            out.push_str(to);
+            out.push_str(&text[end..]);
+            out
+        }
+        None => text.to_string(),
+    }
+}
+
+/// Byte range of the first case-insensitive match of `needle` in `haystack`.
+fn find_case_insensitive(haystack: &str, needle: &str, kind: MatchKind) -> Option<(usize, usize)> {
+    if needle.is_empty() {
+        return None;
+    }
+    let lowered: Vec<char> = needle.chars().flat_map(char::to_lowercase).collect();
+    let is_word = |c: char| c.is_alphanumeric() || c == '_';
+
+    for (start, _) in haystack.char_indices() {
+        if kind == MatchKind::WholeWord
+            && haystack[..start].chars().next_back().is_some_and(is_word)
+        {
+            continue; // mid-word: "ai" inside "training"
+        }
+        let mut matched = 0usize;
+        let mut end = start;
+        for (offset, ch) in haystack[start..].char_indices() {
+            if matched == lowered.len() {
+                break;
+            }
+            let mut consumed_all = true;
+            for lc in ch.to_lowercase() {
+                if lowered.get(matched) != Some(&lc) {
+                    consumed_all = false;
+                    break;
+                }
+                matched += 1;
+            }
+            if !consumed_all {
+                break;
+            }
+            end = start + offset + ch.len_utf8();
+        }
+        if matched == lowered.len()
+            && !(kind == MatchKind::WholeWord
+                && haystack[end..].chars().next().is_some_and(is_word))
+        {
+            return Some((start, end));
+        }
+    }
+    None
 }
 
 /// Simple whitespace/punctuation tokenizer.
@@ -637,4 +709,86 @@ mod tests {
             expanded.iter().map(|x| &x.text).collect::<Vec<_>>()
         );
     }
+    #[test]
+    fn acronyms_only_match_whole_words() {
+        let ex = QueryExpander::new(QueryExpansionConfig::default());
+        // "training" contains "ai", "programming" contains "pr". These used to
+        // be rewritten to "trArtificial Intelligencening" and
+        // "Pull Requestogramming".
+        for query in [
+            "How many miles during my marathon training?",
+            "Which programming language did I pick?",
+            "I updated the maintainer list",
+        ] {
+            for expansion in ex.expand(query) {
+                assert!(
+                    expansion.expansion_type != "acronym",
+                    "{query:?} produced {expansion:?}"
+                );
+            }
+        }
+        // A real acronym still expands, in both directions.
+        let texts: Vec<String> = ex
+            .expand("What about the API and the database?")
+            .into_iter()
+            .filter(|e| e.expansion_type == "acronym")
+            .map(|e| e.text)
+            .collect();
+        assert!(
+            texts
+                .iter()
+                .any(|t| t.contains("Application Programming Interface")),
+            "{texts:?}"
+        );
+        assert!(texts.iter().any(|t| t.contains("DB")), "{texts:?}");
+    }
+
+    #[test]
+    fn non_ascii_queries_do_not_panic_or_corrupt() {
+        let ex = QueryExpander::new(QueryExpansionConfig::default());
+        // Turkish 'İ' is 2 bytes but lowercases to 3, so offsets taken from a
+        // lowercased copy no longer line up with the original. `"İ AI"` used
+        // to panic; `"İstanbul AI trip"` used to silently eat a character.
+        for query in ["İ AI", "İé AI", "İİ ML", "İstanbul AI trip", "ǰ ML notes"] {
+            for expansion in ex.expand(query) {
+                assert!(
+                    expansion.text.contains('İ') || expansion.text.contains('ǰ'),
+                    "{query:?} lost its leading character: {expansion:?}"
+                );
+            }
+        }
+        let expanded = ex.expand("İstanbul AI trip");
+        assert!(
+            expanded
+                .iter()
+                .any(|e| e.text == "İstanbul Artificial Intelligence trip"),
+            "{expanded:?}"
+        );
+    }
+
+    #[test]
+    fn whole_word_matching_handles_string_edges_and_case() {
+        assert_eq!(
+            replace_word_case_insensitive("ai tools", "AI", "Artificial Intelligence"),
+            "Artificial Intelligence tools"
+        );
+        assert_eq!(
+            replace_word_case_insensitive("tools for ai", "AI", "Artificial Intelligence"),
+            "tools for Artificial Intelligence"
+        );
+        assert_eq!(
+            replace_word_case_insensitive("the aim", "AI", "Artificial Intelligence"),
+            "the aim",
+            "must not match inside a word"
+        );
+        assert_eq!(
+            replace_word_case_insensitive("no match here", "xyz", "abc"),
+            "no match here"
+        );
+        // Only the first occurrence is replaced, as before.
+        assert_eq!(
+            replace_word_case_insensitive("ai and ai", "ai", "ML"),
+            "ML and ai"
+        );
+    }
 }