Repository navigation
fix(memory): sanitize FTS queries so natural-language recall works on libSQL (#7275) - #7281
Closed
serrrfirat wants to merge 1 commit into
Closed
serrrfirat wants to merge 1 commit into
serrrfirat wants to merge 1 commit into
Conversation
… libSQL (nearai#7275) Issue nearai#7275 asks for caller-level verification that explicit persistent memory written in conversation A is searchable and proactively recalled in conversation B on the production composition path. Building that verification on the real embedded-libSQL standalone composition exposed a genuine production defect: the libSQL backend passes the memory search query verbatim to FTS5, which treats ?, !, (, ), quotes, and + - * : ^ as operators — any natural-language query containing them makes the MATCH expression invalid, so memory_search fails with OperationFailed and the proactive prompt-memory lane (query = the user's latest message) degrades to empty. User messages almost always carry punctuation, so explicit recall across conversations was unreliable in production. - memory-native: sanitize queries in MemorySearchRequest::new to whitespace-joined alphanumeric tokens (the same treatment the unicode61 tokenizer applies to indexed content); punctuation-only queries still fail as invalid input. - composition factory tests (issue nearai#7275 acceptance criteria 1-6, on the production composition path with the shipping embedded-libSQL backend, not an in-memory harness): - conv A write with provider-issued write evidence (status/path/ content_length), durable across a full runtime teardown/rebuild; - conv B (different thread, same tenant/user/agent/project) finds the marker via memory_search and receives it through the production prompt-memory lane (same derivation build_reborn_runtime wires) without any memory tool call, including natural-language queries with punctuation; - fail-closed for a different user and for an isolated project axis (successful empty results on both surfaces, with non-vacuous same-scope controls); - retrieval failure distinguishable from no-match: no-match is a successful invocation with result_count 0, invalid input fails with InputEncode, backend Unavailable errors degrade the prompt lane to empty while emitting the operator diagnostic (captured via a thread-local subscriber). - regression tests fail without the sanitizer fix.
github-actions
Bot
added
size: XL
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes add FTS query sanitization and production-path memory tests. The tests cover durable cross-conversation recall, scope isolation, prompt-lane retrieval, empty results, backend failures, and operator diagnostics. ChangesPersistent memory recall validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ConversationA
participant MemoryService
participant MemoryStore
participant ConversationB
participant PromptContextService
ConversationA->>MemoryService: durable memory write
MemoryService->>MemoryStore: persist memory
ConversationB->>MemoryService: explicit memory search
MemoryService->>MemoryStore: search saved memory
MemoryStore-->>MemoryService: matching snippets
ConversationB->>PromptContextService: prompt-lane request
PromptContextService->>MemoryStore: retrieve scoped memory
MemoryStore-->>PromptContextService: matching snippets
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/extensions/packages/memory-native/src/search.rs (1)
79-86: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDistinguish "empty query" from "no searchable tokens" in the error.
After sanitization,
valueis the empty string for any punctuation-only input. The caller receivesvalue: ""andreason: "query must not be empty", so an operator cannot tell an empty query from a query that lost every token to the tokenizer. Repo guidance requires errors to carry useful context.🐛 Proposed fix: report the token-loss case explicitly
- let query = sanitize_fts_query(query); - if query.trim().is_empty() { + let raw: String = query.into(); + let query = sanitize_fts_query(&raw); + if query.is_empty() { return Err(HostApiError::InvalidId { kind: "memory search query", - value: query, - reason: "query must not be empty".to_string(), + value: raw, + reason: "query has no alphanumeric search tokens".to_string(), }); }
sanitize_fts_queryalready returns a whitespace-trimmed value, sotrim()is no longer required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/extensions/packages/memory-native/src/search.rs` around lines 79 - 86, Update the validation around sanitize_fts_query so genuinely empty input and punctuation-only input produce distinct InvalidId errors with useful original-query context. Remove the redundant trim check because sanitize_fts_query already returns a trimmed value, and preserve the existing non-empty searchable-query flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/app/ironclaw_composition/src/factory/tests.rs`:
- Around line 871-877: Extract the self-contained `#7275` memory-recall
block—including its helpers, four tests, CaptureWriter, and
UnavailableMemoryService—from the factory tests file into a sibling
memory_recall_tests.rs module. Declare it from the parent with #[cfg(test)] mod
memory_recall_tests;, and preserve the existing test behavior and required
imports while reducing the size of the original file.
- Around line 1279-1294: Update the invalid-input case in the invoke_json test
to send a syntactically valid request whose query contains only punctuation, so
it reaches MemorySearchRequest::new and exercises sanitizer rejection before
retrieval. Assert the caller-visible failure kind mapped from
HostApiError::InvalidId, replacing the current InputEncode expectation while
preserving the public invocation path.
- Around line 1151-1218: Add a positive prompt-lane control in this test before
the cross-user and isolated-project negative checks, using the existing
same-scope user/project context and the written “isolation marker” data. Assert
that the lane returns a non-empty snippet containing the marker, then retain the
existing assertions proving different users and projects return empty results;
anchor the change to the prompt_lane_service flow and
prompt_lane_turn_scope/prompt_lane_request helpers.
---
Outside diff comments:
In `@crates/extensions/packages/memory-native/src/search.rs`:
- Around line 79-86: Update the validation around sanitize_fts_query so
genuinely empty input and punctuation-only input produce distinct InvalidId
errors with useful original-query context. Remove the redundant trim check
because sanitize_fts_query already returns a trimmed value, and preserve the
existing non-empty searchable-query flow.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 364d2872-308d-44bc-a840-56f8858e6225
📒 Files selected for processing (2)
crates/app/ironclaw_composition/src/factory/tests.rscrates/extensions/packages/memory-native/src/search.rs
Comment on lines
+871
to
+877
| // ──────────────────────────────────────────────────────────────────────────── | ||
| // Issue #7275 — verify explicit persistent memory recall across conversations | ||
| // on the PRODUCTION composition path (standalone build over the embedded | ||
| // libSQL root filesystem — the active shipping backend of the local-dev | ||
| // deployment), not an in-memory harness. Each assertion maps to one | ||
| // acceptance criterion of the issue; see the PR body for the criterion table. | ||
| // ──────────────────────────────────────────────────────────────────────────── |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Move the #7275 memory-recall block into its own test module file.
This block adds about 530 lines to a file that is already over 3,000 lines. The repo rule for crates/**/*.rs requires a decomposition tracking issue for files over 3,000 lines, and inline justification for additions over 200 lines. The block is self-contained: helpers, four tests, CaptureWriter, and UnavailableMemoryService. Extract it to a sibling module, for example crates/app/ironclaw_composition/src/factory/memory_recall_tests.rs, and declare it with #[cfg(test)] mod memory_recall_tests;.
As per coding guidelines: "Keep new Rust files below 800 lines where possible. Existing files over 1,500 lines should become shorter when touched … files over 3,000 lines require a decomposition tracking issue, and additions exceeding 200 lines require inline justification."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/app/ironclaw_composition/src/factory/tests.rs` around lines 871 - 877,
Extract the self-contained `#7275` memory-recall block—including its helpers, four
tests, CaptureWriter, and UnavailableMemoryService—from the factory tests file
into a sibling memory_recall_tests.rs module. Declare it from the parent with
#[cfg(test)] mod memory_recall_tests;, and preserve the existing test behavior
and required imports while reducing the size of the original file.
Source: Coding guidelines
Comment on lines
+1151
to
+1218
| let lane = prompt_lane_service(&services).expect("native binding wires the prompt lane"); | ||
|
|
||
| // Different USER: search succeeds with zero results (no leak), and the | ||
| // prompt lane returns no snippet — both are SUCCESSFUL empty outcomes, | ||
| // the fail-closed shape, not errors. | ||
| let other_user_search = invoke_json( | ||
| &services, | ||
| MEMORY_SEARCH_CAPABILITY_ID, | ||
| memory_context_for( | ||
| MEMORY_SEARCH_CAPABILITY_ID, | ||
| "issue-7275-other-user", | ||
| "conv-c", | ||
| None, | ||
| ), | ||
| serde_json::json!({"query": "isolation marker", "limit": 5}), | ||
| ) | ||
| .await | ||
| .expect("cross-user search succeeds (fail-closed, not an error)"); | ||
| assert_eq!(other_user_search["result_count"], serde_json::json!(0)); | ||
| assert_eq!( | ||
| other_user_search["search_scope"], | ||
| serde_json::json!("reborn_internal_persistent_memory") | ||
| ); | ||
| let other_scope = prompt_lane_turn_scope("issue-7275-other-user", "conv-c", None); | ||
| let other_snippets = lane | ||
| .load_memory_snippets(prompt_lane_request( | ||
| &other_scope, | ||
| "issue-7275-other-user", | ||
| "isolation marker?", | ||
| )) | ||
| .await | ||
| .expect("cross-user prompt lane retrieval succeeds"); | ||
| assert!( | ||
| other_snippets.is_empty(), | ||
| "different user must not receive the marker through the prompt lane" | ||
| ); | ||
|
|
||
| // Isolated scope axis: same user, DIFFERENT project → same fail-closed | ||
| // shape on both surfaces (the memory document scope is | ||
| // tenant/user/agent/project, so a different project is a different | ||
| // memory partition). | ||
| let isolated_search = invoke_json( | ||
| &services, | ||
| MEMORY_SEARCH_CAPABILITY_ID, | ||
| memory_context_for( | ||
| MEMORY_SEARCH_CAPABILITY_ID, | ||
| user, | ||
| "conv-d", | ||
| Some("isolated-project"), | ||
| ), | ||
| serde_json::json!({"query": "isolation marker", "limit": 5}), | ||
| ) | ||
| .await | ||
| .expect("isolated-project search succeeds (fail-closed, not an error)"); | ||
| assert_eq!(isolated_search["result_count"], serde_json::json!(0)); | ||
| let isolated_scope = prompt_lane_turn_scope(user, "conv-d", Some("isolated-project")); | ||
| let isolated_snippets = lane | ||
| .load_memory_snippets(prompt_lane_request( | ||
| &isolated_scope, | ||
| user, | ||
| "isolation marker?", | ||
| )) | ||
| .await | ||
| .expect("isolated-project prompt lane retrieval succeeds"); | ||
| assert!( | ||
| isolated_snippets.is_empty(), | ||
| "an isolated scope axis must not receive the marker through the prompt lane" | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add a positive prompt-lane control so the isolation assertions are non-vacuous.
The control at Line 1141 exercises the tool surface only. Both prompt-lane assertions here are negative. If prompt_lane_turn_scope derived a tenant or agent that never matches the write path, other_snippets.is_empty() and isolated_snippets.is_empty() would still pass and prove nothing. The positive lane proof lives in memory_recall_across_conversations_on_production_path, which uses a different tempdir, owner, and user, so it does not cover this test.
💚 Proposed fix: assert the same-scope lane hit first
let lane = prompt_lane_service(&services).expect("native binding wires the prompt lane");
+ // Non-vacuity control for the lane: same user, same project, different
+ // thread must receive the marker, so the negative lane assertions below
+ // cannot pass because of a mis-derived scope.
+ let control_scope = prompt_lane_turn_scope(user, "conv-b", None);
+ let control_snippets = lane
+ .load_memory_snippets(prompt_lane_request(&control_scope, user, "isolation marker?"))
+ .await
+ .expect("same-scope prompt lane retrieval succeeds");
+ assert!(
+ control_snippets
+ .iter()
+ .any(|snippet| snippet.safe_summary.contains(MARKER)),
+ "lane control must find the marker: {control_snippets:?}"
+ );
+
// Different USER: search succeeds with zero results (no leak), and theAs per coding guidelines: "Destructive operations require an explicit product contract, authorization, and tests proving scope isolation."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let lane = prompt_lane_service(&services).expect("native binding wires the prompt lane"); | |
| // Different USER: search succeeds with zero results (no leak), and the | |
| // prompt lane returns no snippet — both are SUCCESSFUL empty outcomes, | |
| // the fail-closed shape, not errors. | |
| let other_user_search = invoke_json( | |
| &services, | |
| MEMORY_SEARCH_CAPABILITY_ID, | |
| memory_context_for( | |
| MEMORY_SEARCH_CAPABILITY_ID, | |
| "issue-7275-other-user", | |
| "conv-c", | |
| None, | |
| ), | |
| serde_json::json!({"query": "isolation marker", "limit": 5}), | |
| ) | |
| .await | |
| .expect("cross-user search succeeds (fail-closed, not an error)"); | |
| assert_eq!(other_user_search["result_count"], serde_json::json!(0)); | |
| assert_eq!( | |
| other_user_search["search_scope"], | |
| serde_json::json!("reborn_internal_persistent_memory") | |
| ); | |
| let other_scope = prompt_lane_turn_scope("issue-7275-other-user", "conv-c", None); | |
| let other_snippets = lane | |
| .load_memory_snippets(prompt_lane_request( | |
| &other_scope, | |
| "issue-7275-other-user", | |
| "isolation marker?", | |
| )) | |
| .await | |
| .expect("cross-user prompt lane retrieval succeeds"); | |
| assert!( | |
| other_snippets.is_empty(), | |
| "different user must not receive the marker through the prompt lane" | |
| ); | |
| // Isolated scope axis: same user, DIFFERENT project → same fail-closed | |
| // shape on both surfaces (the memory document scope is | |
| // tenant/user/agent/project, so a different project is a different | |
| // memory partition). | |
| let isolated_search = invoke_json( | |
| &services, | |
| MEMORY_SEARCH_CAPABILITY_ID, | |
| memory_context_for( | |
| MEMORY_SEARCH_CAPABILITY_ID, | |
| user, | |
| "conv-d", | |
| Some("isolated-project"), | |
| ), | |
| serde_json::json!({"query": "isolation marker", "limit": 5}), | |
| ) | |
| .await | |
| .expect("isolated-project search succeeds (fail-closed, not an error)"); | |
| assert_eq!(isolated_search["result_count"], serde_json::json!(0)); | |
| let isolated_scope = prompt_lane_turn_scope(user, "conv-d", Some("isolated-project")); | |
| let isolated_snippets = lane | |
| .load_memory_snippets(prompt_lane_request( | |
| &isolated_scope, | |
| user, | |
| "isolation marker?", | |
| )) | |
| .await | |
| .expect("isolated-project prompt lane retrieval succeeds"); | |
| assert!( | |
| isolated_snippets.is_empty(), | |
| "an isolated scope axis must not receive the marker through the prompt lane" | |
| ); | |
| let lane = prompt_lane_service(&services).expect("native binding wires the prompt lane"); | |
| // Non-vacuity control for the lane: same user, same project, different | |
| // thread must receive the marker, so the negative lane assertions below | |
| // cannot pass because of a mis-derived scope. | |
| let control_scope = prompt_lane_turn_scope(user, "conv-b", None); | |
| let control_snippets = lane | |
| .load_memory_snippets(prompt_lane_request(&control_scope, user, "isolation marker?")) | |
| .await | |
| .expect("same-scope prompt lane retrieval succeeds"); | |
| assert!( | |
| control_snippets | |
| .iter() | |
| .any(|snippet| snippet.safe_summary.contains(MARKER)), | |
| "lane control must find the marker: {control_snippets:?}" | |
| ); | |
| // Different USER: search succeeds with zero results (no leak), and the | |
| // prompt lane returns no snippet — both are SUCCESSFUL empty outcomes, | |
| // the fail-closed shape, not errors. | |
| let other_user_search = invoke_json( | |
| &services, | |
| MEMORY_SEARCH_CAPABILITY_ID, | |
| memory_context_for( | |
| MEMORY_SEARCH_CAPABILITY_ID, | |
| "issue-7275-other-user", | |
| "conv-c", | |
| None, | |
| ), | |
| serde_json::json!({"query": "isolation marker", "limit": 5}), | |
| ) | |
| .await | |
| .expect("cross-user search succeeds (fail-closed, not an error)"); | |
| assert_eq!(other_user_search["result_count"], serde_json::json!(0)); | |
| assert_eq!( | |
| other_user_search["search_scope"], | |
| serde_json::json!("reborn_internal_persistent_memory") | |
| ); | |
| let other_scope = prompt_lane_turn_scope("issue-7275-other-user", "conv-c", None); | |
| let other_snippets = lane | |
| .load_memory_snippets(prompt_lane_request( | |
| &other_scope, | |
| "issue-7275-other-user", | |
| "isolation marker?", | |
| )) | |
| .await | |
| .expect("cross-user prompt lane retrieval succeeds"); | |
| assert!( | |
| other_snippets.is_empty(), | |
| "different user must not receive the marker through the prompt lane" | |
| ); | |
| // Isolated scope axis: same user, DIFFERENT project → same fail-closed | |
| // shape on both surfaces (the memory document scope is | |
| // tenant/user/agent/project, so a different project is a different | |
| // memory partition). | |
| let isolated_search = invoke_json( | |
| &services, | |
| MEMORY_SEARCH_CAPABILITY_ID, | |
| memory_context_for( | |
| MEMORY_SEARCH_CAPABILITY_ID, | |
| user, | |
| "conv-d", | |
| Some("isolated-project"), | |
| ), | |
| serde_json::json!({"query": "isolation marker", "limit": 5}), | |
| ) | |
| .await | |
| .expect("isolated-project search succeeds (fail-closed, not an error)"); | |
| assert_eq!(isolated_search["result_count"], serde_json::json!(0)); | |
| let isolated_scope = prompt_lane_turn_scope(user, "conv-d", Some("isolated-project")); | |
| let isolated_snippets = lane | |
| .load_memory_snippets(prompt_lane_request( | |
| &isolated_scope, | |
| user, | |
| "isolation marker?", | |
| )) | |
| .await | |
| .expect("isolated-project prompt lane retrieval succeeds"); | |
| assert!( | |
| isolated_snippets.is_empty(), | |
| "an isolated scope axis must not receive the marker through the prompt lane" | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/app/ironclaw_composition/src/factory/tests.rs` around lines 1151 -
1218, Add a positive prompt-lane control in this test before the cross-user and
isolated-project negative checks, using the existing same-scope user/project
context and the written “isolation marker” data. Assert that the lane returns a
non-empty snippet containing the marker, then retain the existing assertions
proving different users and projects return empty results; anchor the change to
the prompt_lane_service flow and prompt_lane_turn_scope/prompt_lane_request
helpers.
Source: Coding guidelines
Comment on lines
+1279
to
+1294
| // Retrieval failure (invalid input on the same surface) → the invocation | ||
| // FAILS with a distinct failure kind instead of fabricating an empty | ||
| // result set: failure is observable at the caller. | ||
| let input_failure = invoke_json( | ||
| &services, | ||
| MEMORY_SEARCH_CAPABILITY_ID, | ||
| memory_context_for(MEMORY_SEARCH_CAPABILITY_ID, user, "conv-c", None), | ||
| serde_json::json!({}), | ||
| ) | ||
| .await | ||
| .expect_err("an empty query must fail the search invocation, not return empty results"); | ||
| assert_eq!( | ||
| input_failure, | ||
| FailureKind::InputEncode, | ||
| "invalid search input must surface as a distinct failure kind" | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The invalid-input case does not exercise the changed rejection path.
The payload {} omits query, so the failure originates in request deserialization, not in MemorySearchRequest::new. The comment states the empty query "is rejected before any retrieval", which describes a different mechanism. This PR makes punctuation-only queries newly invalid, and no caller-level test covers that. Add a probe with a punctuation-only query.
💚 Proposed fix: cover the sanitizer rejection at the caller
+ // Punctuation-only query: the sanitizer leaves no tokens, so the
+ // invocation FAILS instead of returning an empty result set.
+ let punctuation_failure = invoke_json(
+ &services,
+ MEMORY_SEARCH_CAPABILITY_ID,
+ memory_context_for(MEMORY_SEARCH_CAPABILITY_ID, user, "conv-e", None),
+ serde_json::json!({"query": "!!!???", "limit": 5}),
+ )
+ .await
+ .expect_err("a punctuation-only query must fail the search invocation");
+ assert_ne!(
+ punctuation_failure,
+ FailureKind::InputEncode,
+ "placeholder: assert the kind the tool surface maps the sanitizer rejection to"
+ );Replace the placeholder assertion with the failure kind the tool surface actually maps HostApiError::InvalidId to.
As per coding guidelines: "Test through the public caller when a predicate, classifier, or transform gates a side effect and wrappers or computed inputs intervene; helper-only tests are insufficient."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/app/ironclaw_composition/src/factory/tests.rs` around lines 1279 -
1294, Update the invalid-input case in the invoke_json test to send a
syntactically valid request whose query contains only punctuation, so it reaches
MemorySearchRequest::new and exercises sanitizer rejection before retrieval.
Assert the caller-visible failure kind mapped from HostApiError::InvalidId,
replacing the current InputEncode expectation while preserving the public
invocation path.
Source: Coding guidelines
Contributor
🔎 Review · PR #7281
1 actionable findings →Reviewed the complete trusted base-to-head comparison. The punctuation normalization improves several queries, but it does not fully make input safe for FTS5: alphabetic FTS operators remain executable and can still fail or alter natural-language recall. Automatic · PR opened · attempt 1 of 3 · completed in 1m 42s Run details
|
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 Review complete · PR #7281
Reviewed the complete trusted base-to-head comparison. The punctuation normalization improves several queries, but it does not fully make input safe for FTS5: alphabetic FTS operators remain executable and can still fail or alter natural-language recall.
Findings
- 🟠 Medium · FTS5 keyword operators survive query sanitization —
crates/extensions/packages/memory-native/src/search.rs:79
Details are attached to the relevant diff.
Validation and technical details
- Verified refs/ironloop/base and refs/ironloop/head resolve exactly to c69ed2d and 2e2439a.
- Inspected both changed files and surrounding memory service, repository, composition, and libSQL MATCH translation paths.
- Reproduced FTS5 behavior with SQLite:
ANDandalpha ORraise syntax errors;alpha OR betais interpreted as an operator expression. git diff --check refs/ironloop/base..refs/ironloop/headcompleted cleanly.- Rust tests could not be executed because
cargois unavailable in the review environment. - Base:
main - Head:
implement-issue-7275-testat2e2439a - Run:
c07e1127-3872-454e-82af-c971c19d6dd9
| // `plainto_tsquery`. Content is tokenized by unicode61 the same | ||
| // way, so sanitizing the query to whitespace-joined alphanumeric | ||
| // tokens is faithful, not lossy. | ||
| let query = sanitize_fts_query(query); |
Contributor
There was a problem hiding this comment.
🟠 Medium · FTS5 keyword operators survive query sanitization
sanitize_fts_query removes punctuation but preserves alphabetic FTS5 operators such as uppercase AND, OR, and NOT. Because the libSQL backend passes this value directly to MATCH, queries such as "AND" or "alpha OR" still produce FTS5 syntax errors, while "alpha OR beta" changes the intended all-token search into an OR expression. This leaves the reported production failure reproducible for valid natural-language/boolean-looking input and can broaden recall unexpectedly. Encode the sanitized tokens as FTS5 literals at the libSQL boundary (or otherwise use a parser-safe backend representation), and add production-backend regression cases for these keywords.
Collaborator
Author
Railway preview QA — BLOCKEDTested head: Required cases (blocked)The PR's user-visible contract — cross-conversation recall of an explicitly saved memory when conversation B asks in natural language with punctuation (
Status derivation
Supplemental evidence (does not upgrade BLOCKED)
Remaining risks / how to unblock
|
7 tasks
Collaborator
Author
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7285
August 6, 2026 12:23
Destroyed
This was referenced Aug 6, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
? ! ( ) " + - * : ^as operators. Any natural-language query containing them makes the MATCH expression invalid:memory_searchfails withOperationFailed, and the proactive prompt-memory lane (whose query is the user's latest message) degrades to empty. User messages almost always carry punctuation — so explicit recall across conversations was unreliable in production. This is the likely root cause of the Memory not reliably recalled across conversations #7185 feedback.MemorySearchRequest::new(memory-native) to whitespace-joined alphanumeric tokens — the same treatment the unicode61 tokenizer applies to indexed content, so it is faithful, not lossy. Punctuation-only queries still fail as invalid input.Change Type
Linked Issue
Closes #7275
Validation
cargo fmt --all -- --checkcargo clippy -p ironclaw_composition -p ironclaw_memory_native --tests(clean)cargo test -p ironclaw_composition --lib— 505 passedcargo test -p ironclaw_memory_native— all suites passed (52+39+13+25+4+4)cargo test -p ironclaw_host_runtime --test memory_prompt_context+ libmemory_*— passedcargo test -p ironclaw_integration_tests --test reborn_group_memory— passedcargo test -p ironclaw_integration_tests --test reborn_integration_wiring_parity— passedreborn_group_multiuserstack-overflows on the base commit too (pre-existing, reproduced with changes stashed)Acceptance-criteria evidence (issue #7275)
memory_recall_across_conversations_on_production_path: tool response carriesstatus:"written",path,append,content_length; tree read-back; full runtime teardown + rebuild over the same libSQL rootmemory_searchresult_count: 1, marker in content,search_scopemarkermemory_context_service(exactmemory_lifecycle_consumersderivationbuild_reborn_runtimewires) returns the marker snippet for conv B's run scope; request mirrors the loop's own buildermemory_recall_fails_closed_for_other_users_and_isolated_scope_axes: cross-user and cross-project both return successfulresult_count: 0and empty prompt lane; non-vacuous same-scope control finds the markermemory_retrieval_failure_is_distinct_from_no_matching_memory+memory_prompt_lane_failure_emits_operator_diagnostic: no-match = successful invocation withresult_count: 0; empty query = failed invocation (InputEncode); backendUnavailable= typedErr(Unavailable), prompt lane degrades to empty while thememory context lane retrieval failedoperator diagnostic is emitted (captured via thread-local subscriber)build_runtime_substrate(local_filesystem_build_input(..))(embedded libSQL, native memory provider)2e2439af4Test Strategy
User behavior: recall of explicitly saved memory across conversations must not depend on whether the user's phrasing carries punctuation.
Risk areas:
Tests added or updated:
MemorySearchRequestsanitizer tests (memory-nativesearch.rs)reborn_group_memory+wiring_parityre-run green)What the tests prove: the full issue #7275 acceptance-criteria matrix above, on the production composition path; plus the FTS5 punctuation defect and its fix.
Commands run: see Validation.