Repository navigation
fix(inject): read assistant body from data.message.content (Issue #129) - #136
Conversation
…w-stack#129) assistant/message(以及 tool/result)的消息体嵌在 data.message 下(DSH 侧由 @deepseek-ai/dsh-session 的 assertMessageEventShape 写死:type === "user/message" ? record : record?.["message"]),而 textOf 只读 event.data.content,于是热记忆块里 每一轮的 A 恒为空。 改用 ?? 兜底而不是直接替换:user 消息与旧的扁平形状都照旧可用。 同时修正一处测试夹具错误:原用例给 assistant 事件喂的是 data.content,与当时有 bug 的实现是同一个错误假设,因此永远抓不到这个回归;已改为真实形状,并补两个用例 (N 轮 A 不空 / 扁平形状仍兼容)。 npm test: 814 tests, 813 pass, 0 fail, 1 skipped.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesHot-memory content extraction
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The hot-memory extraction now supports nested assistant content while retaining the legacy flat message shape, with regression coverage for both paths. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
感谢精准的定位,尤其是把测试夹具里「与 bug 相同的错误假设」一并挖出来——这正是这个回归能溜过去的原因,这个修复质量很高。已 squash 合并进 main,将随下一版发布(Closes #129)。 |
问题
热记忆块(
[短期上下文])里每一轮的A恒为空。根因在
src/inject.js的textOf:它只读event?.data?.content。而 DSH 的事件契约对两种消息用两种嵌套层级(@deepseek-ai/dsh-session的assertMessageEventShape一行写死):即
user/message的正文在data.content,而assistant/message(以及tool/result)的消息体嵌在data.message,正文在data.message.content。同仓库的src/summarize.js走的正是正确路径(case "assistant/message": const msg = data?.message;),所以这是inject.js单处的漏改。改动
src/inject.js:用??兜底,而不是直接替换成data.message.content—— 这样 user 消息与旧的扁平形状都照旧可用。lib/inject.js由npm run sync生成(本仓库约定:src/是唯一事实源,不手改lib/);test/lib-smoke.test.js的 src↔lib 逐文件一致性守卫通过。测试
顺带修正了一处夹具错误(我认为这正是这个回归能溜过去的原因):
原用例
extracts hot rounds from snapshotEvents() when Session.events is absent给assistant/message喂的是data: { content: [...] }—— 与当时有 bug 的实现是同一个错误假设,所以它对这个回归永远是无感的。已改为真实形状:新增两个用例:
Issue #129: response 取自 data.message.content,N 轮不再出现空 A—— issue 里的复现(两轮,断言四个片段都在)。Issue #129: 扁平的 data.content 形状仍被兼容—— 锁住??的向后兼容语义,防止将来被「简化」成直接替换。验证
(Node v24.18.0 / npm 11.16.0;
npm ci后全新环境跑出。)影响面
只改热记忆块的读取路径。
textOf仍是纯读取 + 容错(非数组即返回""),没有引入新的失败点,符合仓库「后台路径 fail-safe、不阻塞主流程」的约定。Closes #129
Summary by CodeRabbit
Bug Fixes
Tests