feat: add filtered fulltext memory search - #230
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new application-facing endpoint and SDK support for “pure” MatrixOne full-text memory search, allowing callers to combine lexical matching with deterministic structured SQL pre-filters (metadata + fixed fields) without invoking embedding/vector/graph/hybrid retrieval.
Changes:
- Introduces
POST /v1/memories/fulltext-searchin the Rust API/service/storage stack, including centralized handling for MatrixOne “empty pattern” fulltext errors and consistentretrieval_scoremapping. - Adds sync/async Python SDK
memories.fulltext_search()methods with mirrored validation and corresponding unit tests. - Updates API/SDK documentation and changelogs to describe the new contract and behavior.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| skills/api-reference/SKILL.md | Documents the new REST endpoint contract and semantics (filters, ranking, limits). |
| sdk/python/tests/unit/test_memories.py | Adds sync SDK unit tests for request construction + validation behavior. |
| sdk/python/tests/unit/test_memories_async.py | Adds async SDK unit tests for the new method and runtime-type validation. |
| sdk/python/src/memoria/resources/memories.py | Implements fulltext_search() (sync/async) and shared client-side validation. |
| sdk/python/README.md | Adds usage example and notes about strict session_id and JSON type-family equality. |
| sdk/python/CHANGELOG.md | Changelog entry for the new SDK feature. |
| memoria/crates/memoria-storage/src/store.rs | Adds storage-level validation constants/helpers and implements structured fulltext SQL path with scoring + empty-pattern handling. |
| memoria/crates/memoria-storage/src/lib.rs | Re-exports new storage validation helpers and constants. |
| memoria/crates/memoria-storage/src/graph/store.rs | Refactors graph fulltext query to reuse centralized empty-pattern handling. |
| memoria/crates/memoria-service/src/service.rs | Adds FulltextSearchOptions and a service method to run structured fulltext search on a branch. |
| memoria/crates/memoria-service/src/lib.rs | Re-exports FulltextSearchOptions. |
| memoria/crates/memoria-api/tests/api_e2e.rs | Adds E2E coverage for structured prefilters, validation boundaries, and branch isolation. |
| memoria/crates/memoria-api/src/routes/memory.rs | Adds the /v1/memories/fulltext-search route handler wiring into the service. |
| memoria/crates/memoria-api/src/models.rs | Adds FulltextSearchRequest with strict validation and conversion into service options. |
| memoria/crates/memoria-api/src/lib.rs | Registers the new route in the API router. |
| memoria/CHANGELOG.md | Documents the new endpoint at the service level. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
gouhongshen
left a comment
There was a problem hiding this comment.
Reviewed current head 73c2c39. One actionable SDK contract finding is inline. Verification: cargo check for memoria-api/service/storage, both fulltext MatrixOne API E2E cases, the storage validation test, and 49 Python SDK unit tests passed.
aptend
left a comment
There was a problem hiding this comment.
Reviewed head 73c2c39. One blocking SDK contract issue remains:
[P1] Preserve subject_id and extra_metadata returned by the new endpoint. This PR is independently mergeable against main, whose Python Memory model does not define either field, and Memory.from_dict silently ignores unknown response keys. I reproduced fulltext response decoding and both fields disappear from the returned SDK object. Add the fields to this PR without breaking the existing positional field order, or make the dependency on #228 explicit and rebase after its compatible model change lands.
Verification on this head: cargo check for memoria-storage, memoria-service, and memoria-api passed; the storage validation test passed; 49 Python SDK unit tests passed; git diff --check passed.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (3)
memoria/crates/memoria-api/src/models.rs:188
parse_memory_types_optpreserves duplicate entries, and the storage query emits one placeholder and bind per entry. Because requests may be up to 2 MiB whileMemoryTypehas only six variants, a payload containing tens of thousands of repeated"semantic"values can generate an enormous prepared statement or exceed the database parameter limit instead of producing a validation error. Cap or deduplicate this list before constructing the options, and mirror that validation in the SDK.
memory_types: parse_memory_types_opt(self.memory_types.as_ref())?,
sdk/python/src/memoria/resources/memories.py:61
json.dumpscan raiseValueErrorfor an accepted arbitrarily large integer (Python's integer-string digit limit), and UTF-8 encoding can fail for a string containing a lone surrogate. In both casesfulltext_search()leaks a built-in exception instead of rejecting the oversized/invalid scalar withMemoriaValidationError. Translate serialization failures to the SDK's validation exception.
encoded_value = json.dumps(value, ensure_ascii=False, separators=(",", ":")).encode()
memoria/crates/memoria-storage/src/store.rs:5621
- The new E2E test leaves only one matching row and merely checks that its score is numeric, so neither relevance ordering nor the deterministic
memory_idtie-breaker introduced here is exercised. Add multiple eligible matches, including an equal-score pair, and assert their returned order.
ORDER BY ft_score DESC, memory_id DESC LIMIT ?"
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (1)
sdk/python/src/memoria/resources/memories.py:137
- A query containing an unpaired surrogate (for example,
"\ud800") raises a rawUnicodeEncodeErrorhere instead of the SDK's documentedMemoriaValidationError. This bypasses the validation contract even though equivalent metadata encoding failures are translated. Catch the encoding error before checking the byte limit.
if len(query.encode()) > _FULLTEXT_QUERY_MAX_BYTES:
raise MemoriaValidationError(
f"fulltext_search: query must not exceed {_FULLTEXT_QUERY_MAX_BYTES} bytes"
)
aptend
left a comment
There was a problem hiding this comment.
Reviewed current head 5bd56dc. The previous response-metadata blocker is fixed without breaking positional Memory construction, and the encoding/error handling, deduplication, ranking coverage, and fixed-field blank validation look good. One filter-broadening case remains. Targeted Rust tests passed; 55 sync/async Python SDK tests passed; ruff check and git diff --check passed. DB Tests are still running on GitHub.
|
[P1] Reconcile this branch with PR #228 before merge The current heads are not jointly mergeable cleanly. A three-way merge of main, PR #228 at ccf0249, and PR #230 at 132f9a2 reports conflicts in shared SDK and Rust files, including sdk/python/src/memoria/models.py and sdk/python/src/memoria/resources/memories.py. Both PRs also add the same Memory subject_id and extra_metadata fields. Since this PR describes itself as standalone, merging either PR first leaves the other needing manual conflict resolution and can duplicate the model change. Please choose the merge order or dependency explicitly, then rebase and resolve the combined diff before merge. |
## What type of PR is this? - [x] feat (new feature) - [x] fix (bug fix) - [x] docs (documentation) - [ ] style (formatting, no code change) - [x] refactor (code change that neither fixes a bug nor adds a feature) - [ ] perf (performance improvement) - [x] test (adding or updating tests) - [ ] chore (maintenance, tooling) - [ ] build / ci (build or CI changes) ## Which issue(s) this PR fixes Refs #229 and restores the changes from #230. ## What this PR does / why we need it PR #230 was squash-merged as `e46ba76`, but PR #228 was merged six minutes later from the same parent commit. The second squash commit replaced the first one on `main`, so the full-text search implementation disappeared even though #230 remains marked as merged. This PR reapplies #230 on top of the current `main`, which already contains #228, and resolves the shared-file overlap without duplicating the Python `Memory.subject_id` or `Memory.extra_metadata` fields. - Restores `POST /v1/memories/fulltext-search` backed by MatrixOne full-text search. - Restores exact metadata and fixed-field SQL pre-filters, deterministic score ordering, strict session filtering, and branch isolation. - Restores sync and async Python SDK `memories.fulltext_search()` support. - Keeps the structured `POST /v1/memories/query` API from #228 intact. - Keeps full-text search out of MCP as originally intended. - Restores the API reference, changelogs, SDK documentation, and regression coverage. Validation: - `cargo check --manifest-path memoria/Cargo.toml -p memoria-storage -p memoria-service -p memoria-api` - `cargo clippy --manifest-path memoria/Cargo.toml -p memoria-storage -p memoria-service -p memoria-api --lib -- -D warnings` - `cargo test --manifest-path memoria/Cargo.toml -p memoria-storage metadata_and_fulltext_validation_are_enforced_at_storage_boundary` - `cargo test --manifest-path memoria/Cargo.toml -p memoria-api --lib fulltext_memory_type_parser_rejects_blank_entries_but_allows_empty_arrays` - `cargo test --manifest-path memoria/Cargo.toml -p memoria-api --test api_e2e test_api_fulltext -- --nocapture` (`3 passed`) - `uv run --project sdk/python pytest sdk/python/tests/unit/test_memories.py sdk/python/tests/unit/test_memories_async.py -q` (`89 passed`) - `uv run --project sdk/python ruff check sdk/python/src/memoria/resources/memories.py sdk/python/tests/unit/test_memories.py sdk/python/tests/unit/test_memories_async.py` - `git diff --check`
What type of PR is this?
Which issue(s) this PR fixes
Fixes #229
What this PR does / why we need it
Adds an application-facing lexical search path for callers that need MatrixOne full-text matching combined with deterministic structured constraints, without invoking embedding, vector, graph, hybrid, temporal, or confidence scoring.
POST /v1/memories/fulltext-searchbacked by MatrixOneMATCH(content) AGAINST(... IN BOOLEAN MODE).extra_metadata_filter, subject, memory type, session, trust tier, user/group, active-memory, and branch SQL pre-filters before the Top-K limit.session_idequality; unlike retrieve/search session scoping, unscoped memories are excluded.2may match2.0, but does not match string"2".retrieval_score, ordered by score withmemory_idas a deterministic tie-breaker.memories.fulltext_search()methods with matching validation limits.Validation:
cargo check --manifest-path memoria/Cargo.toml -p memoria-storage -p memoria-service -p memoria-apicargo clippy --manifest-path memoria/Cargo.toml -p memoria-storage -p memoria-service -p memoria-api --lib -- -D warningscargo test --manifest-path memoria/Cargo.toml -p memoria-storage fulltext_filter_validation_is_enforced_at_storage_boundarycargo test --manifest-path memoria/Cargo.toml -p memoria-api --test api_e2e test_api_fulltext_search_with_structured_prefilters -- --exact --nocapturecargo test --manifest-path memoria/Cargo.toml -p memoria-api --test api_e2e test_api_fulltext_search_on_branch_is_isolated_from_main -- --exact --nocaptureuv run pytest tests/unit/test_memories.py tests/unit/test_memories_async.py -q(45 passed)uv run ruff check src/memoria/resources/memories.py tests/unit/test_memories.py tests/unit/test_memories_async.pyuv run ruff format --check src/memoria/resources/memories.pygit diff --check