fix(recall): split stable persona before CACHE_BOUNDARY for prefix caching (#120)#449
fix(recall): split stable persona before CACHE_BOUNDARY for prefix caching (#120)#449muzimu217 wants to merge 1 commit into
Conversation
…ching (TencentCloud#120) Move the truly stable persona content into prependSystemAddition (placed BEFORE the host CACHE_BOUNDARY) when cacheOptimization='split_system', so prefix-matching providers (DeepSeek/MiMo) can cache it across turns. Dynamic L1 memories are intentionally NOT moved — they change every turn and would bust the cache for everything downstream. Leave them in prependContext (after the boundary), exactly as today. Default 'none' = zero behavior change. This is orthogonal to and composable with TencentCloud#375's injectionMode (which controls L1 position). Dropped the earlier stable_wrapper approach: a fixed opening tag does NOT help prefix caching because the dynamic content inside still diverges and invalidates the cached prefix. Only genuinely stable content (persona) belongs before the boundary.
8f428c3 to
efa2bdf
Compare
|
@YOMXXX 你好,想麻烦你帮忙 review 一下这个 re-scope 后的小 PR。 背景:原 #321 想一次性覆盖 #120 的缓存问题,体量偏大(2464 行),早期方案里 本 PR 只做一件事:把稳定的 persona 移到 想请确认的点:
关于量化数据:#433 用的是真实 provider 实测(2 provider × 2 variant × 2 repeat × 3 turn),我们早期的自建基准是仿真、不够严谨,已不纳入本 PR;如需量化佐证,我们会参考 #433 的方法补真实测量,而不是用仿真数字。 谢谢,听你的建议。 |
|
嗯嗯好的,我们已经收到您的PR,针对参加犀牛鸟活动的PR,我们内部近期将会排期进行集中reveiw,感谢您的关注! |
YOMXXX
left a comment
There was a problem hiding this comment.
I cannot approve this yet.
The main behavior claimed by the PR is that persona moves before CACHE_BOUNDARY, but the patch only adds a new prependSystemAddition property to the recall result and logs its length. I do not see an integration path proving that OpenClaw consumes this new field and places it before the host cache boundary. Without that host-side contract, split_system can return prependSystemAddition while nothing actually changes in the prompt shape.
Please add one of the following before this can be approved:
- an integration test or host-adapter test showing
prependSystemAdditionis returned frombefore_prompt_buildand lands beforeCACHE_BOUNDARYin the final OpenClaw prompt shape, or - a change using an already-supported OpenClaw hook return field for before-boundary system additions, with tests, or
- a documented SDK/type reference in the repo proving
prependSystemAdditionis a supported return field for this hook.
The pure helper tests are useful, but they only prove the object is shaped internally. They do not prove the advertised cache-boundary behavior actually reaches the model.
This PR relies on an UNVERIFIED assumption: OpenClaw host consumes
prependSystemContextto place content before CACHE_BOUNDARY.Evidence Status: MISSING
appendContext, NOTprependSystemContextRisk: Silent Failure
If the host doesn't recognize
prependSystemContext:摘要
实验性功能 - 把稳定 persona 移到
CACHE_BOUNDARY之前参与缓存。量化收益分析
对比数据
理论收益(如果宿主支持)
prependSystemContext与 #375 的关系
重叠警告
关键发现:
append模式已经把 persona 前置到 boundary 之前建议合并顺序:
已修复的漏洞
基于代码审查反馈,已修复以下问题:
✅ 漏洞#1:添加宿主契约警告
prependSystemContext是未验证的假设✅ 漏洞#3:删除 dedup 死代码
dedupeRecallLines函数dedup参数(从未被传递)✅ 漏洞#7:移除 showInjected 超scope逻辑
✅ 漏洞#9:修正错误注释
✅ 漏洞#10:添加验证测试
配置
{ "recall": { "cacheOptimization": "split_system" // ⚠️ 实验性功能 } }默认值:
"none"(向后兼容,opt-in)建议:
"none"直到宿主契约被验证改进建议
在考虑合并本 PR 前,建议:
验证宿主契约:
评估必要性:
量化真实收益:
关键限制
本 PR 不能解决的问题:
推荐替代方案:
测试
Change Type
Supersedes #321 (kept open for history tracking)
建议:在验证宿主契约前,保持
"none"默认值。