feat: add memory normalizer pipeline#222
feat: add memory normalizer pipeline#222Victor-dw wants to merge 1 commit intoCortexReach:masterfrom
Conversation
rwmjhb
left a comment
There was a problem hiding this comment.
Review
核心思路有价值——auto-capture 的原始文本经过清洗和结构化后确实能提升记忆质量。atomic metadata schema(版本化、类型安全)、三级 fallback(LLM → 规则 → 原始)、opt-in 设计都是正确的方向。
但当前 PR 不适合直接合并,有以下问题:
必须解决
-
未声明的 Breaking Change:PR 删除了
memory upgrade和reindex-ftsCLI 命令(移除了memory-upgrader.ts/llm-client.ts依赖),但 PR 描述里没有提及。现有用户升级后会丢失这些功能。 -
个人化内容泄漏:
normalization-rules.ts的deriveTagsFromText()硬编码了"lossless-claw"、"tushare"、"acpx-with-proxy"、"7897"等个人项目名称,测试用例中也大量使用这些内容。这些对通用用户无意义,且"proxy"、"7897"这样的规则会对无关记忆产生误打标。 -
中英文混杂的 CLI 输出:
"还有 ${n} 条"、"命中噪声/会话元信息规则"、"包含 pending/计划态信号"等中文字符串直接出现在 CLI 输出中,非中文用户无法理解。
建议改进
-
PR 范围过大:+3397 行,同时引入 normalizer 管道、atomic metadata schema、governance 报告、search observation、review packet 对比、auto-capture 重构。应拆分为独立 PR,便于 review 和 rollback。
-
isRuntimeChatter正则过宽:/\bstatus\b/i、/\bpoll(?:ing)?\b/i会把 "I prefer polling over webhooks" 这样的合法内容判为 chatter 并丢弃。 -
测试覆盖不均:PR 有 3 个测试文件(641 行),但主要覆盖集成路径。
normalization-rules.ts(正则匹配、chatter 检测、标签推导)和normalization-validate.ts(分支逻辑)这两个最容易出错的模块缺少独立的单元测试。
建议
建议将有价值的部分拆分为独立 PR 逐步提交:
- PR A:atomic metadata schema(
src/atomic-memory.ts)+ auto-capture 重构(src/auto-capture.ts)— 这是最干净、最容易合并的部分 - PR B:normalizer 管道核心(
src/normalizer.ts+src/normalization-*.ts)— 清理个人化内容后提交 - PR C:governance / audit CLI 工具 — 独立功能,单独 review
每个 PR 都不应删除现有 CLI 命令。建议关闭当前 PR,按上述拆分重新提交。
|
关闭此 PR。核心思路有价值,建议按 review 中的拆分方案重新提交。 |
Summary
memory_storeand regex auto-capture writes through atomic metadata aware normalizationWhat Changed
src/normalizer.tsplus normalization rule/validation/types helpersindex.tsto initialize the normalizer from plugin config and use it in auto-capture fallbacksrc/tools.tssomemory_storecan accept atomic input or normalize free-form candidate text before persistenceopenclaw.plugin.jsonwith normalization settingsmemory_storenormalization, auto-capture normalization, and a live A/B harnessValidation
node test/memory-store-normalizer.mjsnode test/normalizer-auto-capture.mjsnode test/cli-smoke.mjsnode test/plugin-manifest-regression.mjsnode test/normalizer-phase1-live-ab.mjs(skips unlessMEMORY_NORMALIZER_API_KEYis set)Notes
CortexReach/masterand intentionally excludes the earlier local stable-merge history