feat(memory): enforce capacity budgets for long-lived Memories - #1746
222twotwotwo wants to merge 2 commits into
Conversation
5731493 to
e001436
Compare
AlexStocks
left a comment
There was a problem hiding this comment.
结论
实现本身我没找到正确性缺陷。我在自己的双树上(base 1b728e95 / head e001436c)独立验证了 RFC 的声明,全部成立;本 PR 引入的是两个面向面/一致性问题,都属于"合并前改文档或补一行代码就完事"的量级,但都会实际误导使用者,所以按 REQUEST_CHANGES 提。
我独立验证过的(不看作者脚本)
| 声明 | 我的探针结果 |
|---|---|
| 上报字节 = 存储写入的规范化字节 | capacity().manifest_bytes = 925 = len(memory_content_bytes(head.content)) = 从 pc_artifacts.content 原始 JSON 重新算出的字节数(三者一致) |
| 拒绝写入不落盘 | 拒写后 pc_artifacts/pc_memory_entry_versions/pc_memory_entry_heads 行数不变、head 不变 |
| 压缩不删正文/历史、零投影写入 | 行数增量 [artifacts +1, entry_versions +0, entry_heads +0];语句级统计对 pc_memory_entry_heads/pc_memory_entry_fts 的 INSERT/UPDATE/DELETE = 0 |
| 压缩前引用仍有效 | 被移除条目的旧 citation 仍能 validate_citation 通过,且已不在 head 清单里 |
| 标签保护 | 带标签墓碑在压缩后存活,标签集合逐字不变 |
| dry-run 不写版本 | preview 后 head 未变;preview/commit 的 entry_ids 与 reclaimed_bytes 一致 |
| 压缩后可写 | 压缩后 remember() 成功且 exceeded == [] |
| 既有行为无变化 | 用只走公开 API 的探针在双树各跑一遍(remember/去重/revise/forget/reactivate/organize/changes/revisions/revision/validate_citation),s1–s11 输出逐字节相同,唯一差异是新出现的 capacity()。service.py 把 memory_content_hash(content) 换成 sha256(memory_content_bytes(content)).hexdigest() 与 canonical.py:220 的定义完全等价,content hash 未变 |
| 全量回归 | tests/builtin/artifacts/memory + test_memory.py + test_artifacts.py + test_integration_manifest.py:base 84 passed / head 101 passed, 13 skipped(OceanBase,本机无库) |
| 四道闸 | ruff check / ruff format --check / ty check --python-version 3.11(改动源文件)全过;CI 23 项 check-runs 全 success |
_require_capacity 的"只拦增长、不拦持平/收缩"设计我逐条对过 RFC 的 acceptance criteria:forget/organize/compact 的 growth 分别是 ∅/∅/∅,reactivate 只查 active_entries,_prepare_commit(add + revise)查全部三维——与文档表一致。压缩侧与 replace_tags 的并发也成立:两者都先拿 pc_artifact_heads 那一行的写锁(persistence/tags.py:166 的 UPDATE ... SET revision=revision / artifacts.revise 的 CAS),所以"提交前的再读"能把并发新增的标签拦下来,注释里的说法是准确的。
合并闸门
mergeStateStatus: BLOCKED 与代码无关:ruleset 9930477(master)要求 required_approving_review_count: 1,bypass_actors: []、current_user_can_bypass: "never",required_review_thread_resolution: false,require_extra_approval_for_unattributed_changes: true(本 PR 单 commit、无 co-author trailer,这条已满足)。当前 reviewDecision: REVIEW_REQUIRED、评审线程数 0 —— 也就是只差 1 个批准。
提的两个 P1
docs/en|zh/development/memory-layer.md:告诉读者"Enable it explicitly onRuntimeConfig",但memory_compaction_enabled目前没有任何消费方(行内评论给了 grep 与实测证据)。integrations/codex/.../hooks/bind_tools.py:加进_CURRENT_OPERATIONS的get_memory_capacity不在_MCP_OPERATION_IDS里,这条分支永不可达;同时暴露出"HTTP/client/JS 契约有、MCP 工具面没有"的面向面不一致。
另有两个 P2(growth 用裸字符串导致拼写错误会静默关掉预算检查、capacity() 的展开代价)见行内评论。
下面三条无法锚到具体行,一并写在这里:
docs/zh/development/memory-layer.md:101与英文侧:104是同一处措辞,需要同步改。- 本 PR 的 RFC 我按行核过 acceptance criteria,除"operator can recover capacity"这一句因为上面第 1 条而偏乐观外,其余都能对上;en/zh 两版 RFC 内容一致。
- 我在
.worktrees/下为本次评审建了pr1746-review、pr1746-base2(另有一个半损坏的pr1746-base)以及主仓.workbuddy/pr1746/探针目录。删除在当前沙箱被守卫拦死(fail-closed),请你在真实机器上清掉:git worktree remove --force .worktrees/pr1746-review .worktrees/pr1746-base2、git worktree prune、直接删.worktrees/pr1746-base、.workbuddy/pr1746/。
AI usage:本次评审的代码阅读、探针编写与双树对照由 WorkBuddy 内的 assistant 完成,结论中的每一个数字都是在本机当场跑出来的。
a9a2163 to
e817194
Compare
Which issue or RFC does this PR close?
Closes #1718
Rationale for this change
#1709 removed projection write amplification, but each Memory Revision still stores a complete manifest. Inactive entries remain as tombstones, writes have no explicit entry or byte ceiling, and callers cannot inspect remaining capacity.
This PR adds observable capacity budgets and deterministic refusal when a write exceeds them. Explicit tombstone compaction lets operators recover current-manifest capacity while preserving historical content and exact citations.
What changes are included in this PR?
This implements the explicit-failure option in feat(memory): define capacity contract for long-lived Memories #1718. Physical erasure and retention policies remain under RFC: define retention, erasure, and lifecycle policies #1425; entry and history pagination remain under feat(memory): add bounded cursor pagination for Memory entries #1656 and feat(memory): add bounded pagination for Memory changes and history #1657.
Are there any user-facing changes?
Yes.
No database schema migration is required. The budgets constrain current Revision content and history-read expansion; retained historical data continues to consume storage.
How was this change tested?
Focused capacity, service, persistence, HTTP/client, and API contract regressions:
uv run --locked --no-sync python -m pytest
tests/builtin/artifacts/memory/test_capacity.py
tests/builtin/artifacts/memory/test_service.py
tests/e2e/test_memory_capacity.py
tests/test_api_contract.py
tests/test_js_operations.py
tests/builtin/persistence/test_memory.py -q
80 passed, 13 skipped. The skipped cases require an OceanBase connection.
Full repository CI remains pending.
AI usage statement
Codex assisted with code review, documentation updates and validation.