feat: add structured memory queries - #228
Conversation
There was a problem hiding this comment.
Pull request overview
Adds deterministic structured memory querying across the REST API, storage layer, and Python SDK.
Changes:
- Adds filtered, cursor-paginated
POST /v1/memories/query. - Adds sync/async Python SDK support and response fields.
- Adds validation, E2E/unit tests, and documentation.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
sdk/python/tests/unit/test_memories.py |
Tests sync querying and validation. |
sdk/python/tests/unit/test_memories_async.py |
Tests async querying. |
sdk/python/src/memoria/resources/memories.py |
Implements SDK query methods. |
sdk/python/src/memoria/models.py |
Exposes subject and metadata fields. |
sdk/python/README.md |
Documents structured queries. |
sdk/python/CHANGELOG.md |
Records the SDK feature. |
memoria/crates/memoria-storage/src/store.rs |
Implements validation and SQL filtering. |
memoria/crates/memoria-storage/src/lib.rs |
Exports storage query utilities. |
memoria/crates/memoria-service/src/service.rs |
Adds structured query service options and path. |
memoria/crates/memoria-service/src/lib.rs |
Exports structured query options. |
memoria/crates/memoria-api/tests/api_e2e.rs |
Tests filtering, pagination, and branches. |
memoria/crates/memoria-api/src/routes/memory.rs |
Handles structured query requests. |
memoria/crates/memoria-api/src/models.rs |
Defines and validates query requests. |
memoria/crates/memoria-api/src/lib.rs |
Registers the query endpoint. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (2)
memoria/crates/memoria-storage/src/store.rs:5363
- The new SQL path's tenant and active-record predicates are not exercised by the added structured-query tests: those tests use only one user and never deactivate/supersede a matching memory. Add query-specific MatrixOne E2E coverage showing that another user/group scope cannot see a match and that an inactive matching row is excluded, since existing list tests execute a different storage method.
let mut inner =
format!("SELECT memory_id FROM {table} WHERE user_id = ? AND is_active = 1");
sdk/python/src/memoria/models.py:45
- Adding
subject_idbefore the existing optional fields changes the public dataclass's positional constructor: callers that previously passedsession_id(and later fields) positionally will now bind those values to the wrong attributes without an error. Keep all existing fields in their original order and append the new fields afterretrieval_score.
subject_id: str | None = None
gouhongshen
left a comment
There was a problem hiding this comment.
Reviewed current head e8bc4d3. One actionable compatibility finding is inline. Verification: cargo check for memoria-api/service/storage, the structured-query MatrixOne API E2E cases, the storage validation test, and 42 Python SDK unit tests passed.
aptend
left a comment
There was a problem hiding this comment.
Reviewed head e8bc4d3. Two blocking contract issues remain:
-
[P1] Keep the public Memory dataclass positional field order compatible. Inserting subject_id before session_id silently shifts every existing optional positional argument. I reproduced an old positional construction where session-old is now stored as subject_id and the old retrieval score is stored as created_at. Append new fields after the existing fields or make them keyword-only, and add a compatibility regression test.
-
[P1] Reject blank selectors instead of silently dropping them. structured_options normalizes whitespace-only subject_id, session_id, trust_tier, and branch to None. When another selector is present, a request such as extra_metadata_filter plus subject_id set to spaces succeeds as a broader metadata-only query; a blank branch can even fall back to the active branch. This contradicts the documented rule that all supplied selectors are combined with AND. Apply the same non-empty validation in REST and sync/async SDK paths.
Verification on this head: cargo check for memoria-storage, memoria-service, and memoria-api passed; the storage validation test passed; 42 Python SDK unit tests passed; git diff --check passed.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (2)
memoria/crates/memoria-api/tests/api_e2e.rs:431
- This query does not actually verify the
urgentormemory_typespredicates: every fixture hasurgent: trueand is stored with the default semantic type, so the test still passes if either SQL predicate is omitted. Add near-matching fixtures withurgent: falseand a non-semantic type to exercise these filters.
"extra_metadata_filter": {"scene": "incident", "rank": 2, "urgent": true},
"memory_types": ["semantic"],
sdk/python/src/memoria/resources/memories.py:62
- For a key longer than 64 bytes, this error states only rules that the key already satisfies, so SDK callers cannot tell why validation failed. Include the 64-byte limit in the message, matching the server-side validation error.
"query: extra_metadata_filter keys must start with an ASCII letter or "
"underscore and contain only ASCII letters, digits, or underscore"
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
memoria/crates/memoria-storage/src/store.rs:5357
memory_typesis unbounded, and duplicate valid values survive parsing. A request near the 2 MB body limit can therefore make this code generate and bind tens of thousands ofINplaceholders, even though the model has only six possible memory types, causing avoidable allocation/DB load or a parameter-limit failure. Reject lists larger than the canonical type set (or deduplicate them) before constructing the SQL.
if let Some(types) = memory_types.filter(|types| !types.is_empty()) {
inner.push_str(" AND memory_type IN (");
inner.push_str(&vec!["?"; types.len()].join(", "));
inner.push(')');
aptend
left a comment
There was a problem hiding this comment.
Reviewed current head 7ef61b2. The positional Memory compatibility fix, predicate/scope coverage, and memory-type deduplication are good, but one previously requested contract issue is still unresolved and one SDK validation edge remains. Targeted Rust tests passed; 47 sync/async Python SDK tests passed; git diff --check passed.
## 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 #227
What this PR does / why we need it
Adds a dedicated structured memory query path for applications that need deterministic filtering without running embedding, vector, keyword, fulltext, graph, or relevance-ranking retrieval.
POST /v1/memories/querywith exact, type-sensitive scalarextra_metadata_filtermatching.AND.items,next_cursor) with stable memory ID ordering.memories.query()methods and exposessubject_id/extra_metadataon the SDKMemorymodel.retrieveandsearchunchanged and intentionally does not add an MCP tool, preserving the existing agent-facing MCP surface.Validation:
cargo check -p memoria-api -p memoria-service -p memoria-storagecargo test -p memoria-api --test api_e2e test_api_structured_query_by_extra_metadata -- --nocapture.venv/bin/python -m pytest tests/unit/test_memories.py tests/unit/test_memories_async.py -q(36 passed).venv/bin/python -m ruff check src/memoria/models.py src/memoria/resources/memories.py