feat(eval): support memos api mode#116
Conversation
Duguce
changed the title
|
@hush-cd can you help me review this pr? thanks!!! |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for a new "memos-api" mode to the evaluation framework, enabling testing of MemOS functionality through API calls instead of just local mode. The changes expand the evaluation capabilities to include both local and API-based testing scenarios.
- Introduces new MemOSAPI client for HTTP-based communication with MemOS services
- Updates all evaluation scripts to support "memos-api" mode alongside existing modes
- Fixes search functionality by implementing proper top_k parameter handling
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| evaluation/scripts/utils/memos_api.py | New MemOSAPI client implementation for HTTP-based MemOS operations |
| evaluation/scripts/utils/client.py | Adds API mode support to memos_client function |
| evaluation/scripts/run_lme_eval.sh | Updates evaluation script configuration and adds new pipeline steps |
| evaluation/scripts/longmemeval/*.py | Updates all LongMemEval scripts to support memos-api mode |
| evaluation/scripts/locomo/*.py | Updates all LoCoMo scripts to support memos-api mode |
| """Register a user.""" | ||
| url = f"{self.base_url}/users/register" | ||
| payload = json.dumps({"user_id": user_id}) | ||
| response = requests.request("POST", url, data=payload, headers=self.headers) |
There was a problem hiding this comment.
[nitpick] Use requests.post() instead of requests.request("POST", ...) for better readability and consistency with HTTP method naming conventions.
| response = requests.request("POST", url, data=payload, headers=self.headers) | |
| response = requests.post(url, data=payload, headers=self.headers) |
| url = f"{self.base_url}/add" | ||
| payload = json.dumps({"messages": messages, "user_id": user_id, "mem_cube_id": cube_id}) | ||
|
|
||
| response = requests.request("POST", url, data=payload, headers=self.headers) |
There was a problem hiding this comment.
[nitpick] Use requests.post() instead of requests.request("POST", ...) for better readability and consistency with HTTP method naming conventions.
| response = requests.request("POST", url, data=payload, headers=self.headers) | |
| response = requests.post(url, data=payload, headers=self.headers) |
| } | ||
| ) | ||
|
|
||
| response = requests.request("POST", url, data=payload, headers=self.headers) |
There was a problem hiding this comment.
[nitpick] Use requests.post() instead of requests.request("POST", ...) for better readability and consistency with HTTP method naming conventions.
| response = requests.request("POST", url, data=payload, headers=self.headers) | |
| response = requests.post(url, data=payload, headers=self.headers) |
| response = requests.request("POST", url, data=payload, headers=self.headers) | ||
| return response.text | ||
|
|
||
| def search(self, query: str, user_id: str | None = None, top_k: int = 10): |
There was a problem hiding this comment.
The top_k parameter is not included in the API request payload, but it's used to slice the results locally. This may cause inconsistent behavior if the API returns fewer than top_k results or if server-side filtering would be more efficient.
| register_res = json.loads(self.user_register(user_id)) | ||
| cube_id = register_res["data"]["mem_cube_id"] |
There was a problem hiding this comment.
No error handling for JSON parsing failures. If user_register returns invalid JSON, this will raise an unhandled exception without a helpful error message.
| register_res = json.loads(self.user_register(user_id)) | |
| cube_id = register_res["data"]["mem_cube_id"] | |
| try: | |
| register_res = json.loads(self.user_register(user_id)) | |
| cube_id = register_res["data"]["mem_cube_id"] | |
| except json.JSONDecodeError as e: | |
| raise ValueError(f"Failed to parse JSON response from user_register: {e}") |
| result = json.loads(response.text)["data"]["text_mem"][0]["memories"] | ||
| text_memories = [item["memory"] for item in result][:top_k] | ||
| return text_memories |
There was a problem hiding this comment.
No error handling for JSON parsing or key access failures. This could raise KeyError or json.JSONDecodeError without helpful context about the API response structure.
| result = json.loads(response.text)["data"]["text_mem"][0]["memories"] | |
| text_memories = [item["memory"] for item in result][:top_k] | |
| return text_memories | |
| try: | |
| response_json = json.loads(response.text) | |
| data = response_json.get("data", {}) | |
| text_mem = data.get("text_mem", []) | |
| if not text_mem or not isinstance(text_mem, list) or "memories" not in text_mem[0]: | |
| raise KeyError("Expected 'text_mem' to be a non-empty list with 'memories' key.") | |
| result = text_mem[0]["memories"] | |
| text_memories = [item.get("memory", "") for item in result][:top_k] | |
| return text_memories | |
| except json.JSONDecodeError as e: | |
| raise ValueError(f"Failed to parse JSON response: {e}") | |
| except (KeyError, IndexError) as e: | |
| raise ValueError(f"Unexpected response structure: {e}") |
| type=str, | ||
| choices=["zep", "memos", "mem0", "mem0_graph", "langmem", "openai"], | ||
| choices=["zep", "memos", "mem0", "mem0_graph", "openai", "memos-api"], | ||
| help="Specify the memory framework (zep or memos or mem0 or mem0_graph)", |
There was a problem hiding this comment.
Update the help string to include "openai" and the newly added "memos-api" for consistency with the choices list.
Ensure corresponding updates are made in other relevant files where this argument is defined or documented to maintain coherence across the codebase.
| speaker_2_memories=speaker_b_context, | ||
| ) | ||
|
|
||
| print(query, context) |
There was a problem hiding this comment.
Are the print statements in multiple search functions necessary? Also, note that the zep_search function does not have a corresponding print statement.
* feat(eval): add eval dependencies * feat(eval): add configs example * docs(eval): update README.md * feat(eval): remove the dependency (pydantic) * feat(eval): add run locomo eval script * fix(eval): delete about memos redundant search branches * chore: fix format * feat(eval): add openai memory on locomo - eval guide * docs(eval): modify openai memory on locomo - eval guide * feat(eval): add longmemeval evaluation pipeline * chore(eval): formatter * chore: update * feat(eval): add configs example * fix(eval): bugs about longmemeval * fix(eval): search top k * chore(eval): update * feat(eval): support memos api mode * feat(eval): add memobase; fix bugs about share db
* feat(eval): add eval dependencies * feat(eval): add configs example * docs(eval): update README.md * feat(eval): remove the dependency (pydantic) * feat(eval): add run locomo eval script * fix(eval): delete about memos redundant search branches * chore: fix format * feat(eval): add openai memory on locomo - eval guide * docs(eval): modify openai memory on locomo - eval guide * feat(eval): add longmemeval evaluation pipeline * chore(eval): formatter * chore: update * feat(eval): add configs example * fix(eval): bugs about longmemeval * fix(eval): search top k * chore(eval): update * feat(eval): support memos api mode * feat(eval): add memobase; fix bugs about share db
Description
Summary: fix some bugs, such as search top k
Fix: #(issue)
Reviewer: @hush-cd
Checklist: