feat(memory): add an opt-in conservative update policy for auto memory#2233
feat(memory): add an opt-in conservative update policy for auto memory#2233print-happy wants to merge 7 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| // Execute operations. | ||
| for _, op := range ops { | ||
| w.executeOperation(ctx, userKey, op) | ||
| if err := w.executeOperation(ctx, userKey, op); err != nil { |
There was a problem hiding this comment.
Legacy auto memory now returns persistence errors, breaking the previous best effort flow. Keep errors non fatal for UpdatePolicyLegacy.
中文
Legacy auto memory 现在会返回持久化错误,破坏了原来的尽力而为流程。请在 `UpdatePolicyLegacy` 下保持错误非致命。| @@ -1 +1 @@ | |||
| Subproject commit 1edee87bc8b4efa539329e6766fd0fcd4b03fd7b | |||
| Subproject commit 39f7b38c3c23b3b4405fc4bd67408c1d8839dfb9 | |||
There was a problem hiding this comment.
This submodule pin is only advertised by refs/pull/11/head, so it is not on a stable branch or tag. Pin a commit reachable from the benchmark repo main branch.
中文
这个 submodule 指针只由 `refs/pull/11/head` 发布,因此不在稳定分支或标签上。请改为指向 benchmark 仓库 main 分支可达的 commit。
Rememorio
left a comment
There was a problem hiding this comment.
The policy direction is promising, but the current 50-case result is not yet sufficient merge evidence. Please publish the exact manifest and sanitized case-level aggregate, reconcile the conflicting Mem0 denominators (50/50 in the table versus 35/49 in the text), and report per-type results. The 2.58x increase in active memories also needs retrieval-quality, latency, and cost measurements on both historical-state and current-state questions. A fixed development subset plus a blind holdout would make it much easier to distinguish a general improvement from benchmark-specific tuning.
中文
这个策略方向有价值,但目前的 50-case 结果还不足以作为合入依据。建议提供精确 manifest 和脱敏后的 case-level 聚合结果,修正 Mem0 分母矛盾(表格为 50/50,正文为 35/49),并补充各类别结果。活跃记忆增长到 2.58 倍后,还需要在历史状态问题和当前状态问题上评估检索质量、延迟与成本。使用固定开发子集和未参与调参的盲测集,也更容易区分通用改进与针对 benchmark 的调优。
|
|
||
| // UpdatePolicyProvider optionally declares an extractor's update policy. | ||
| // Extractors that do not implement this interface retain legacy behavior. | ||
| type UpdatePolicyProvider interface { |
There was a problem hiding this comment.
Do we need a named exported UpdatePolicyProvider interface here? Users only select one of a closed set of framework-defined policies; there is no supported scenario where they implement a new policy. Exporting this interface turns an internal configuration transport mechanism into a permanent extension point and permits partial implementations where the worker changes reconciliation behavior but the custom extractor does not apply the corresponding prompt semantics.
I suggest exposing only a string-backed UpdatePolicy enum plus WithUpdatePolicy(...). The effective values could describe behavior directly, for example model-managed, history-preserving, and add-only. The current behavior should be represented by an explicit non-empty value rather than "", initialized by NewExtractor, and recorded in metadata. Internally, the worker can either recognize the built-in concrete extractor or use an unexported capability interface; no named public provider interface is necessary.
中文
这里是否需要一个具名且公开的 UpdatePolicyProvider 接口?用户只会从框架提供的封闭策略集合中选择,并不存在自行实现新策略的受支持场景。导出该接口会把内部配置传递机制固化成公共扩展点,还可能出现不完整实现:worker 改变了 reconcile 行为,但自定义 extractor 并没有应用对应的 prompt 语义。
建议公共 API 只保留 string-backed 的 UpdatePolicy 枚举和 WithUpdatePolicy(...)。枚举值可以直接描述行为,例如 model-managed、history-preserving 和 add-only。现有行为也应使用显式的非空值表示,由 NewExtractor 初始化并写入 metadata。worker 内部可以只识别内置具体类型,或者使用未导出的 capability interface,不需要公开具名的 Provider 接口。
There was a problem hiding this comment.
Fixed. UpdatePolicyProvider interface has been removed. And the enumerate values are changed as followed: compatible for default and regular policy, strict stands for the strict update policy(which gets 40/50 on lme), and add-only represents completly banning update, delete and clear. And thus the prompt problem is fixed automatically.
| out = appendConservativeAdd(ctx, w, userKey, out, op, existing) | ||
| case extractor.OperationUpdate: | ||
| out = appendConservativeUpdate(ctx, w, userKey, out, op, byID[op.MemoryID]) | ||
| default: |
There was a problem hiding this comment.
The policy contract needs to cover destructive operations as well. This default branch currently passes model-generated Delete and Clear operations through unchanged, so a contradictory newer state can still erase historical memory even under the conservative policy.
If UpdatePolicyDisabled is renamed to add-only, the implementation must actually guarantee that automatic extraction can only produce Add operations: duplicates become no-ops, Update becomes Add, and Delete/Clear are rejected or filtered. For a history-preserving policy, destructive operations should at least require an explicit user forget/delete request. Please add tests covering every OperationType for each policy.
中文
策略契约还需要覆盖破坏性操作。当前这个 default 分支会原样放行模型生成的 Delete 和 Clear,因此即使启用了保守策略,矛盾的新状态仍可能删除历史记忆。
如果将 UpdatePolicyDisabled 重命名为 add-only,实现必须真正保证自动提取最终只产生 Add:重复项不操作,Update 转为 Add,Delete/Clear 被拒绝或过滤。对于 history-preserving 策略,破坏性操作至少应要求用户明确提出遗忘或删除请求。建议为每种策略补充覆盖全部 OperationType 的测试。
There was a problem hiding this comment.
Fixed. 现在strict模式下只有用户明确要求时才能调用delete,然后add only模式彻底禁止了delete/clear行为,只能通过显式调用memory_delete的方式删除。
| ) ([]*memory.Entry, error) { | ||
| query := buildSearchQuery(messages) | ||
| if updatePolicyFor(w.config.Extractor) != extractor.UpdatePolicyLegacy { | ||
| query = buildPolicySearchQuery(messages) |
There was a problem hiding this comment.
This makes the experiment change more than the update policy: non-legacy modes also change relevant-memory retrieval from the legacy user-only query to a user+assistant query. Conservative reconciliation additionally reuses the preloaded candidate set instead of issuing the legacy per-add searches. Therefore the reported 36% to 80% gain cannot currently be attributed to update semantics alone.
Please separate these controls or provide an ablation for: legacy reconciliation with the assistant-inclusive query, conservative reconciliation with the legacy query, and conservative reconciliation with the assistant-inclusive query. The effective query/reconciliation configuration should also be recorded in benchmark metadata.
中文
这里使实验改变的不只是 Update Policy:非 Legacy 模式还把相关记忆检索从原来的仅 user query 改成了 user+assistant query;保守 reconcile 又改为复用预加载候选集,而不是执行原来的逐 Add 搜索。因此当前 36% 到 80% 的提升无法只归因于更新语义。
建议拆分这些控制项,或者至少补充以下消融实验:Legacy reconcile + assistant-inclusive query、Conservative reconcile + Legacy query、Conservative reconcile + assistant-inclusive query。实际生效的 query/reconcile 配置也应记录到 benchmark metadata 中。
| ) | ||
|
|
||
| const ( | ||
| conservativeOldCoverage = 0.95 |
There was a problem hiding this comment.
These fixed lexical coverage thresholds and the language-specific number/negation/entity regular expressions define a substantial part of the policy behavior. Please document whether the values were selected on a development set or chosen heuristically, and add boundary tests for multilingual text, numeric changes, negation, same-name entities, and events on different dates.
Falling back to Add is safer than overwriting, but it can turn classification uncertainty into unbounded memory growth. The benchmark should report how often each decision path is taken and how the thresholds affect both accuracy and active-memory cardinality.
中文
这些固定 lexical coverage 阈值,以及针对特定语言的数字、否定词和实体正则,实际上定义了策略的大部分行为。建议说明这些值来自开发集调参还是经验设定,并补充多语言文本、数字变化、否定、同名实体和不同日期事件的边界测试。
不确定时回退到 Add 比覆盖原记忆更安全,但也可能把分类不确定性转化成无界的记忆增长。benchmark 应统计各决策路径的触发次数,并展示阈值对准确率和活跃记忆数量的共同影响。
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (78.53261%) is below the target coverage (85.00000%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2233 +/- ##
===================================================
- Coverage 89.88607% 89.85661% -0.02947%
===================================================
Files 1144 1146 +2
Lines 198637 198967 +330
===================================================
+ Hits 178547 178785 +238
- Misses 12588 12652 +64
- Partials 7502 7530 +28
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
0224c5b to
d4d8424
Compare
Summary
UpdatePolicyLegacy,UpdatePolicyConservative, andUpdatePolicyDisabled.WithUpdatePolicy(...)extractor option.Reason
Legacy Auto may update or discard information based on semantic similarity and topic overlap, even when the new memory contains an important detail not represented by its topics.
For example:
Alice visited Bob on December 1st, 2025.Alice visited Bob at 4pm on December 1st, 2025.The
4pmdetail may previously be discarded. Conservative mode recognizes this as a compatible enrichment and updates the existing event while preserving all original information. Corrections, state changes, different events, and uncertain matches are stored as separate memories.Impact
Legacy behavior remains the default, including its best-effort persistence semantics. Existing users, interfaces, database schemas, memory IDs, and explicit memory tools are unchanged.
Enable it explicitly:
The main trade-off is higher memory usage because uncertain information is retained instead of merged or discarded.
Result
Mem0 completed 49 cases, so its accuracy uses
35/49. Conservative improved accuracy by 44 percentage points over Legacy Auto, while increasing active memory entries by 2.58x.Test
go test ./memory/extractor ./memory/internal/memory