Repository navigation
test(client): update remaining filter prefix tests - #1341
haosenwang1018 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Aligns remaining Python client tests with the stricter “prefixed metadata filter key” behavior introduced earlier.
Changes:
- Updates
_dict_to_filter_string()unit test inputs/expectations to use prefixed keys (e.g.,metadata.*). - Updates integration test search filter example to use
metadata.time. - Adjusts an integration test comment to clarify the metadata prefix requirement.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/client/client_tests/test_memory.py | Updates _dict_to_filter_string() test cases to expect prefixed filter keys. |
| packages/client/client_tests/test_integration_complete.py | Updates integration search filter example/comment to use metadata.-prefixed keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
|
|
||
| filter_dict = {"category": "work", "type": "preference"} | ||
| filter_dict = {"metadata.category": "work", "m.type": "preference"} |
There was a problem hiding this comment.
This test mixes two metadata key prefixes (metadata. and m.). In the same PR you describe a strict metadata. prefix requirement, so this inconsistency can confuse future readers and may codify an alias that isn’t actually supported across the stack. Prefer using a single canonical prefix in tests (e.g., metadata.type) unless m. is an explicitly supported alias—if it is, consider adding a dedicated test (and/or a clarifying comment) that m. is accepted.
| ) | ||
|
|
||
| # Search with filter (filter_dict is converted to SQL-like string format) | ||
| # Search with filter (user metadata fields use the metadata. prefix in filter_dict) |
There was a problem hiding this comment.
Given the unit tests in this PR use both metadata. and m. prefixes, this comment may be misleading if m. is also supported. Either align the tests to only use metadata. (preferred if the behavior is strict), or update this comment to mention any additional supported prefixes/aliases to avoid conflicting guidance.
| # Search with filter (user metadata fields use the metadata. prefix in filter_dict) | |
| # Search with filter (this test uses the metadata. prefix for user metadata fields) |
|
@haosenwang1018 thank you for the pull request submission. Please sign your commits, resolve the unit test failures, and review the CoPilot feedback. You only need to resolve the relevant items. Thanks. |
…ion (MemMachine#1403) * feat(client+langgraph): raw filter strings and EpisodeType normalization Add three small features to the Python client and its LangGraph wrapper so callers can pass structured filter expressions and either-enum-or-string episode types directly. 1. `Memory.search(filter=...)` and `Memory.list(filter=...)` Accept an optional raw filter string alongside `filter_dict`. When both are provided, the two are combined with `AND`. The raw filter is passed through to the v2 `SearchMemoriesSpec.filter` / `ListMemoriesSpec.filter` fields unchanged. 2. `MemMachineTools.search_memory(filter=...)` Pipes the same raw filter through the LangGraph search-memory tool. 3. `MemMachineTools.add_memory(episode_type=...)` Accept either an `EpisodeType` enum or its string value (e.g. `"message"`), normalizing strings via `EpisodeType(...)` before delegating to `Memory.add`. The factory tool's return-type annotation was widened to match. The `filter` parameter shadows the Python builtin, which is the same trade-off `memmachine_common.api.SearchMemoriesSpec` already made for its `filter:` field — keeping the parameter name aligned with the API field. `# noqa: A002` is applied at the three call sites with a comment pointing at the API spec. This commit consolidates the substantive work from haosenwang1018's 9-commit stack (MemMachine#1341 → MemMachine#1349) into a single rebased+linted commit against current `main`. The original stack's prefix-style doc and test changes have been omitted because they have already landed on `main` via MemMachine#1352 and MemMachine#1311. The original commits authored by haosenwang1018: - 921b55f feat(client): support raw filter strings - e0849bb feat(langgraph): support raw filter strings - 53b489e fix(langgraph): normalize episode type strings Closes MemMachine#1341, MemMachine#1342, MemMachine#1343, MemMachine#1344, MemMachine#1345, MemMachine#1346, MemMachine#1347, MemMachine#1348, MemMachine#1349 Co-authored-by: Steve Scargall <[email protected]> Signed-off-by: Steve Scargall <[email protected]> * docs(langgraph): document filter and episode type support --------- Signed-off-by: Steve Scargall <[email protected]> Co-authored-by: haosenwang1018 <[email protected]> Co-authored-by: Shu Wang <[email protected]>
Purpose of the change
Finish aligning Python client tests with the strict filter prefix behavior introduced by PR #1291.
Description
Follow-up to #1305
This updates the remaining Python client tests I found that still used unqualified metadata filter keys:
packages/client/client_tests/test_memory.pypackages/client/client_tests/test_integration_complete.pyChanges include:
_dict_to_filter_string()test examples from bare keys likecategory,type, andnameto prefixed keys likemetadata.category,m.type, andmetadata.namefilter_dict={"time": "morning"}tofilter_dict={"metadata.time": "morning"}metadata.prefix requirementType of change
How Has This Been Tested?
Manual verification:
test_memory.pynow expects prefixed filter keys in_dict_to_filter_string()examplestest_integration_complete.pynow usesfilter_dict={"metadata.time": "morning"}Checklist
Maintainer Checklist