Repository navigation
Conversation
|
make format |
Kaap10
left a comment
Kaap10
left a comment
There was a problem hiding this comment.
Thanks for working on this @saahil08! The core implementation of the memory management tools (dynavec_remember, dynavec_get, dynavec_forget) and the --enable-write safeguard is solid and all 13 unit tests pass cleanly.
However, we cannot merge this PR in its current state due to unrelated formatting changes and title mismatch:
- Revert unrelated file modifications (80 files changed): The PR reformatted 76 unrelated files across
benchmarks/,src/, andtests/. PRs should only contain diffs directly relevant to the feature (src/dynavec/mcp/server.py,src/dynavec/cli.py,tests/test_mcp_server.py,CHANGELOG.md). - Revert
README.md: An auto-formatter escaped markdown characters (\*\*,\(, etc.) creating 1,600+ lines of mangled documentation. - Fix PR Title: The title currently says "support ollama and huggingface embedders (#350)", but the PR implements MCP memory tools. Please update it to reflect the actual feature (e.g.,
feat(mcp): add opt-in memory management tools (remember, get, forget)).
Once rebased cleanly with only the MCP files touched, we will be happy to merge this!
|
|
||
| [![Python versions]\(https\://img.shields.io/badge/python-3.9%20%7C%203.10%20%7C%203.11%20%7C%203.12-3776ab?style=flat-square&logo=python&logoColor=white)]\(https\://pypi.org/project/dynavec/) | ||
|
|
||
| [![CI]\(https\://img.shields.io/github/actions/workflow/status/codeforstartups/dynavec/ci.yml?branch=development&style=flat-square&label=CI)]\(https\://github.com/codeforstartups/dynavec/actions/workflows/ci.yml) |
There was a problem hiding this comment.
Please revert README.md. An automated formatter inadvertently escaped markdown syntax (\*\*, \(, etc.), creating 1,600+ lines of unintended diff.
| recalls.append( | ||
| len(truth_set & prediction_set) / k | ||
| ) | ||
| recalls.append(len(truth_set & prediction_set) / k) |
There was a problem hiding this comment.
Please revert changes to this and other unrelated files across benchmarks/, src/, and tests/. Please keep the PR focused strictly on the MCP feature files.
| def _get_db() -> Dynavec: | ||
| return db if db is not None else client_from_env() | ||
|
|
||
| def dynavec_remember( |
There was a problem hiding this comment.
The memory tools (dynavec_remember, dynavec_get, dynavec_forget) and the opt-in --enable-write flag look clean and all unit tests pass. Once the PR title and unrelated formatting diffs are reverted, this will be ready to merge.
Summary
Adds opt-in MCP memory management tools to Dynavec, allowing MCP clients to store, retrieve, and delete memories.
Changes
dynavec_rememberto store memories with optional document IDs, metadata, namespaces, and TTL.dynavec_getto retrieve memories by ID.dynavec_forgetto delete memories by ID.--enable-writeflag to enable memory management tools.Security
Memory write and management tools are opt-in and remain disabled by default. Users must explicitly enable them with
--enable-write.Testing
make formatmake run-ci(lint, mypy, and tests)Closes #350