docs(client): fix search result example usage#1345
Open
haosenwang1018 wants to merge 5 commits intoMemMachine:mainfrom
Open
docs(client): fix search result example usage#1345haosenwang1018 wants to merge 5 commits intoMemMachine:mainfrom
haosenwang1018 wants to merge 5 commits intoMemMachine:mainfrom
Conversation
Contributor
|
@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. |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes Python SDK docs/demo to use typed result objects for memory search output, and extends the client/tooling to accept raw filter strings (plus episode type support in LangGraph).
Changes:
- Update docs and demo to use attribute-style access for search results (e.g.,
item.content). - Add
filter: str | Nonepassthrough toMemory.search()/Memory.list()and LangGraphsearch_memory(). - Add optional
episode_typehandling + normalization in LangGraph tools, with expanded tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/client/src/memmachine_client/memory.py | Adds raw filter string parameter and combines it with built-in / dict filters |
| packages/client/src/memmachine_client/langgraph.py | Adds episode_type support, normalizes enum values, and passes raw filter through |
| packages/client/client_tests/test_memory.py | Adds unit tests for raw filter strings and updates filter_dict expectations |
| packages/client/client_tests/test_langgraph.py | Updates tool tests for new filter/episode_type behavior |
| packages/client/client_tests/test_integration_complete.py | Aligns integration test filter_dict keys with metadata. prefix |
| examples/memmachine_client_demo.py | Switches demo to attribute access for episodic memory items |
| docs/api_reference/python/client.mdx | Updates docs example to attribute access for search results |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
459
to
468
| def list( | ||
| self, | ||
| memory_type: MemoryType = MemoryType.Episodic, | ||
| page_size: int = 100, | ||
| page_num: int = 0, | ||
| filter_dict: dict[str, str] | None = None, | ||
| filter: str | None = None, | ||
| set_metadata: dict[str, JsonValue] | None = None, | ||
| timeout: int | None = None, | ||
| ) -> ListResult: |
Comment on lines
362
to
+370
| self, | ||
| query: str, | ||
| limit: int | None = None, | ||
| expand_context: int = 0, | ||
| score_threshold: float | None = None, | ||
| filter_dict: dict[str, str] | None = None, | ||
| timeout: int | None = None, | ||
| *, | ||
| filter: str | None = None, |
| print("Results found:") | ||
| for idx, item in enumerate(results.get('episodic_memory', [])): | ||
| print(f"[{idx+1}] {item['content']} (Role: {item['producer_role']})") | ||
| for idx, item in enumerate(results.episodic_memory or []): |
Comment on lines
+39
to
+49
| episodic_memories = results.episodic_memory or [] | ||
| if not episodic_memories: | ||
| print(" No memories found.") | ||
| return | ||
|
|
||
| episodic_memories = results["episodic_memory"] | ||
| if episodic_memories and len(episodic_memories) > 0: | ||
| print(f" Found {len(episodic_memories[0])} relevant memories:") | ||
| for i, memory in enumerate(episodic_memories[0][:3], 1): # Show top 3 | ||
| print(f" Time: {memory['timestamp']}") | ||
| print(f" {i}. {memory['content']}") | ||
| if memory.get("user_metadata"): | ||
| print(f" Metadata: {memory['user_metadata']}") | ||
| print() | ||
| print(f" Found {len(episodic_memories)} relevant memories:") | ||
| for i, memory in enumerate(episodic_memories[:3], 1): # Show top 3 | ||
| print(f" Time: {memory.timestamp}") | ||
| print(f" {i}. {memory.content}") | ||
| if memory.user_metadata: | ||
| print(f" Metadata: {memory.user_metadata}") |
Comment on lines
392
to
+394
| timeout: Request timeout in seconds (uses client default if not provided) | ||
| filter: Optional raw filter string. If provided together with built-in or | ||
| `filter_dict` filters, all filters are combined with AND. |
Contributor
There was a problem hiding this comment.
Hi @haosenwang1018!
For this PR (and all your other PRs) to merge, we need you to sign your commits.
Also, please take a look at the feedback that CoPilot has generated and respond to it accordingly. Conflicts are especially important to respond to.
Kind Regards,
Sarah
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
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.
Purpose of the change
Fix the Python client docs/demo so they use the actual
SearchResultobject API instead of treating search results like a dict.Description
Fixes #973
The current Python SDK example and demo access search results like:
results.get('episodic_memory', [])item['content']But the Python client returns typed objects (
SearchResult,EpisodicMemory), which causes runtime failures like:AttributeError: 'SearchResult' object has no attribute 'get'This change updates:
docs/api_reference/python/client.mdxexamples/memmachine_client_demo.pyto use object-style access instead:
results.episodic_memory or []item.contentitem.producer_rolememory.timestampmemory.user_metadataType of change
How Has This Been Tested?
Manual verification:
results.episodic_memory or []item.contentresults.get(...)or dict indexing for search resultsChecklist
Maintainer Checklist