feat: write-path content-hash dedup for MemoryManager.add#2153
feat: write-path content-hash dedup for MemoryManager.add#2153Zhe-SH-CN wants to merge 4 commits into
Conversation
* Fix MemTensor#1540: fix: (MemTensor#1824) docs(memos-local-plugin): clarify install path and stale dir names (MemTensor#1540) The README's 'Quick start' section told users to use install.sh instead of npm install, but the warning was buried and users still tried 'npm install -g @memtensor/memos-local-plugin' first. The reporter in MemTensor#1540 encountered this on a Hermes deployment. This change: - Promotes the 'do not run npm install -g' notice to a prominent IMPORTANT callout explaining why global install is wrong (no agent-home deploy, no config.yaml, no bridge/viewer) and that the tarball intentionally ships built artifacts only. - Adds a Troubleshooting subsection covering the two specific symptoms in the bug report: the 'package not found' misread, and the stale web/ and site/ directory names (web/ is now viewer/, site/ was removed by commit 26e7e3d). - Mentions install.ps1 for Windows alongside install.sh. - CHANGELOG: record the docs fix and reference MemTensor#1540. Documentation-only change; no code or runtime behavior touched. Co-authored-by: MemOS AutoDev <autodev@memtensor.ai> Co-authored-by: Matthew <heimixiaozhuang@zju.edu.cn> * Fix MemTensor#1888: [Bug] test_system_parser.py: SystemParser.__init__() got an unexpected keyword a (MemTensor#1889) fix: remove invalid chunker parameter from SystemParser test instantiation - SystemParser.__init__() signature changed to (embedder, llm=None) - Test was still passing chunker=None causing TypeError - Fixes all 5 failing tests in test_system_parser.py Fixes MemTensor#1888 Co-authored-by: MemOS AutoDev <autodev@memos.ai> Co-authored-by: Matthew <heimixiaozhuang@zju.edu.cn> * Fix MemTensor#1525: [Bug] clean_json_response crashes with cryptic AttributeError when given None (MemTensor#1884) * test: add comprehensive tests for clean_json_response (issue MemTensor#1525) - Add test suite in tests/mem_os/test_format_utils.py - Cover None input ValueError with diagnostic message - Cover markdown removal, whitespace stripping, edge cases - Verify fix for AttributeError when LLM returns None * style: format clean_json_response tests --------- Co-authored-by: MemOS AutoDev <autodev@memos.ai> Co-authored-by: Matthew <heimixiaozhuang@zju.edu.cn> * Fix MemTensor#1901: share_cube_with_user passes swapped args to _validate_cube_access — fails for ev (MemTensor#1903) fix: validate current user not target in share_cube_with_user (MemTensor#1901) share_cube_with_user(cube_id, target_user_id) called _validate_cube_access(cube_id, target_user_id), but the validator signature is (user_id, cube_id). The cube_id therefore landed in the user_id slot and _validate_user_exists raised "User '<cube_id>' does not exist or is inactive" for every well-formed call, making the API unusable. The in-code comment "Validate current user has access to this cube" already documented the correct intent: the sharing user (self.user_id) must have access to the cube being shared, not the target. Switch the call to self._validate_cube_access(self.user_id, cube_id). The target user's existence is independently checked on the next line via validate_user(target_user_id), so that path is unchanged. Add regression tests in tests/mem_os/test_memos_core.py that pin down: - validate_user_cube_access is consulted with (self.user_id, cube_id), - add_user_to_cube is called with (target_user_id, cube_id) on success, - a missing target raises "Target user '<id>' does not exist". Closes MemTensor#1901 Co-authored-by: MemOS AutoDev Bot <autodev@memtensor.local> Co-authored-by: Matthew <heimixiaozhuang@zju.edu.cn> * feat(memos): chunk batch reflection scoring * Fix MemTensor#1897: bug(memos-local-plugin): Hermes background model status checks can trigger paid (MemTensor#1899) * Fix MemTensor#1897: fix(memos-local-plugin): add LLM circuit breaker for terminal provider errors Issue MemTensor#1897 reported ~12,900 paid LLM requests in 24 h on Hermes against a DeepSeek key with insufficient balance. The local `system_model_status` row count (12,900) closely tracked the provider-side `request_count` (11,344) for the same billing window. The naming is misleading: `system_model_status` is not a health probe; it is the audit row written once per LLM call (ok / fallback / error) inside `core/llm/client.ts`. With no circuit breaker, every pipeline subscriber (capture / session-relation / reward / L2 / L3 / skill / retrieval LLM filter / world-model) kept firing on every turn / closed episode / induction, generating one paid request each. Add a per-`LlmClient` circuit breaker: - Trips on terminal errors: HTTP 401/402/403 or messages containing `insufficient balance` / `invalid api key` / `unauthorized` / `account suspended` / `billing`. - Open: short-circuits subsequent calls inside the facade without contacting the provider. Throws `MemosError(LLM_UNAVAILABLE)` with `details.circuitOpen=true` so existing catch blocks still work. - Half-open after cool-down (default 5 min, configurable, min 30 s): next call probes the provider; success closes the breaker, terminal failure re-opens it for another cool-down. - Host fallback rescues a call without tripping the breaker — fallback exists precisely to keep going when the primary is down. - Coalesces `system_model_status="circuit_open"` audit rows to at most one per ~25 s while the breaker stays open, so we don't replace paid spam with audit-row spam. - Exposes `circuitOpen` / `circuitOpenUntil` / `circuitOpenedReason` via `LlmClientStats` for the Overview viewer card. - Enabled by default; legacy behaviour available via `circuitBreaker.enabled = false`. Tests: 9 new vitest cases under `tests/unit/llm/client.test.ts` covering trip on 402, trip on "insufficient balance" message, no trip on generic transient, coalescing, half-open close on success, host-fallback rescues without trip, disabled mode, stats fields, and re-open on terminal probe failure. All 59 LLM and 28 pipeline tests pass; `tsc --noEmit` clean. Out of scope (tracked separately): 429 `Retry-After` handling (issue MemTensor#1620), per-tool rate limits, daily budget caps. * Fix LLM breaker with host fallback --------- Co-authored-by: autodev-bot <autodev@memtensor.local> Co-authored-by: Jiang <33757498+hijzy@users.noreply.github.com> Co-authored-by: GU TIANCHUN <96930846+TianchunGu@users.noreply.github.com> Co-authored-by: Dubberman <48425266+whipser030@users.noreply.github.com> --------- Co-authored-by: Memtensor-AI <project@memtensor.cn> Co-authored-by: MemOS AutoDev <autodev@memtensor.ai> Co-authored-by: Matthew <heimixiaozhuang@zju.edu.cn> Co-authored-by: MemOS AutoDev <autodev@memos.ai> Co-authored-by: MemOS AutoDev Bot <autodev@memtensor.local> Co-authored-by: Jiang <33757498+hijzy@users.noreply.github.com> Co-authored-by: GU TIANCHUN <96930846+TianchunGu@users.noreply.github.com> Co-authored-by: Dubberman <48425266+whipser030@users.noreply.github.com>
…nsor#2145) This reverts commit 21a27ed. Co-authored-by: jiachengzhen <jiacz@memtensor.cn>
c27c7b5 to
2e364ee
Compare
🤖 Open Code ReviewTarget: PR #2153 🔍 OpenCodeReview found 3 issue(s) in this PR. 1.
|
2e364ee to
ffe0923
Compare
Fixes MemTensor#2141 Add server-side deduplication on the write path to prevent duplicate memories from replayed events, retry loops, or buggy client integrations. Implementation: - Compute sha1(memory_text) for each incoming TextualMemoryItem - Dedup within the batch using a set - Query graph_store for existing nodes with the same content_hash - Skip items whose hash exists within a configurable time window - Fail-open: if the dedup check errors, the write proceeds - Persist content_hash as a metadata property on the graph node - Toggles: MOS_DEDUP_ENABLED=0 to disable, MOS_DEDUP_WINDOW_DAYS to adjust the window (default 7 days)
ffe0923 to
5c60972
Compare
✅ Automated Test Results: PASSEDAll tests passed (8/8 executed). memos_python_core/changed-repo-python: 8/8. Duration: 5s [advisory, non-gating] AI-generated tests on branch test/auto-gen-258da3fb72ae3edf-20260723200033: 112/120 passed, 8 failed — these do NOT affect the PR verdict; review the branch manually. Branch: |
Description
MemOS has no server-side dedup on the write path. Any client that re-sends identical memory content (retry loops, replayed events, buggy integrations) creates duplicate nodes that pollute search results, graph structure, and reorganization.
This PR adds content-hash deduplication at
MemoryManager.add:sha1(memory_text)for each incomingTextualMemoryItemgraph_store.get_by_metadatafor existing nodes with the samecontent_hashcontent_hashas a metadata property on the graph nodeMOS_DEDUP_ENABLED=0to disable,MOS_DEDUP_WINDOW_DAYSto adjust the windowRelated Issue: Fixes #2141
Type of change
How Has This Been Tested
python3 -c "import ast; ast.parse(...)"syntax verificationChecklist