Skip to content

fix(summarize): 缺 setDistillCursor 时降级内存游标而非抛错 - #279

Merged
modusensus merged 2 commits into
mainfrom
fix/persist-cursor-fallback
Sep 21, 2026
Merged

modusensus merged 2 commits into
mainfrom
fix/persist-cursor-fallback

Conversation

@modusensus

@modusensus modusensus commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

背景

#274 复审(Anans-Ivresse APPROVED)留了三条非阻塞建议,本 PR 落第 1 条;第 2、3 条经评估不做,理由见下。

变更

persistCursorservice.setDistillCursor 缺失时不再抛错,降级为 warn + 内存游标(lastDistilledSeq Map,#274 之前的语义):

测试

  • 新增回归用例:删掉 service.setDistillCursor 后蒸馏照跑、记忆落库、无持久化游标、第二轮不重蒸(内存游标生效)。
  • npm test:1248 tests,1247 passed + 1 skipped,0 fail(含新用例)。
  • npm run sync + check-sync 通过(src ↔ lib 平价)。

不做的两条(Anans 建议第 2、3 条)

  • 第 2 条「读路径内存优先」不改memoryDir 允许双宿主共用(store 的 WAL + busy_timeout 多进程就绪,AGENTS.md 并行协作节)。读库优先是跨进程游标一致的依据——两个宿主进程各持内存 map 互相看不见对方推进的游标,恰好复活 [Bug] lastDistilledSeq 为进程内存态:宿主重启后蒸馏窗口从头重放,已蒸馏历史被重复处理 #229 要修的重复蒸馏,只是从「跨重启」变成「跨进程」。SQLite 主键点查微秒级,turn/end 是人类频率,收益不抵风险。
  • 第 3 条「峰值时段/最小窗口 × 持久化游标交叉用例」不补:restart 恢复与 bypassPeak 各有既有覆盖,两条路在 previousSeq 计算处汇合,交叉场景结构上已走通;补测属于为覆盖数字写测试。

Fixes #274(复审建议跟进)

Summary by CodeRabbit

  • Bug Fixes
    • 兼容未提供持久化游标功能的旧版服务:摘要处理不再因此中止,改为使用进程内游标继续运行。
    • 增加警告日志,帮助识别未启用持久化游标的运行环境。
    • 同一进程内重复处理相同事件时,仍可避免重复摘要。

Anans 复审留下的非阻塞建议第 1 条:第三方宿主用旧版 service 构造时,
persistCursor 在方法缺失分支硬抛错,会把 #274 之前能正常工作的宿主
变成每轮蒸馏即崩。降级为 warn + 内存游标(#274 前语义,本进程不重蒸,
跨重启重放由 saveWithDedupe 三元组兜底);方法存在但抛错仍向上传播,
恰一次回滚语义不动(对应回滚测试用例保持原样)。

补回归测试:缺方法时蒸馏照跑、记忆落库、不持久化、进程内不重蒸。
Anans 建议第 2 条(内存优先读)经评估不改:memoryDir 允许双宿主共用,
DB 优先是跨进程游标一致的依据,内存优先会复活跨进程重复蒸馏。
Copilot AI lite review requested due to automatic review settings September 21, 2026 12:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Repository: slow-stack/dsh-mneme/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: d2d67467-77a2-4c28-ab80-b65864bee85b

📥 Commits

Reviewing files that changed from the base of the PR and between e1e9a94 and e5139f6.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: slow-stack/dsh-mneme/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 8d903ae6-2045-4cc9-8fad-13a0ec549ec8

📥 Commits

Reviewing files that changed from the base of the PR and between 360f7c8 and e1e9a94.

📒 Files selected for processing (3)
  • dsh-mneme/lib/summarize.js
  • dsh-mneme/src/summarize.js
  • dsh-mneme/test/summarize.test.js

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

service 缺少 setDistillCursor 时,persistCursor 现在记录警告并返回。流程改用内存游标。方法存在但抛错时,错误仍向上传播。新增回归测试覆盖该兼容行为。

Changes

游标兼容性

Layer / File(s) Summary
内存游标降级与回归验证
dsh-mneme/src/summarize.js, dsh-mneme/lib/summarize.js, dsh-mneme/test/summarize.test.js
setDistillCursor 不是函数时,代码记录警告并跳过持久化。首次处理仍执行 LLM 蒸馏并保存记忆。同一进程再次处理相同窗口时,内存游标阻止重复蒸馏。现有的持久化写入错误仍继续传播。

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: z2ace0107

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了主要变更:当缺少 setDistillCursor 时,改为使用内存游标而不是抛出错误。标题简洁且与代码变更和 PR 目标一致。
Linked Issues check ✅ Passed [#274] 的持久化游标、事务回滚和失败传播实现已在评审头中保留。本次 diff 同步修改 src/summarize.jslib/summarize.js:当旧版 service 缺少 setDistillCursor 时记录 warning 并使用进程内游标;当方法存在但抛错时仍传播错误。新增测试验证蒸馏、记忆写入、无游标持久化和进程内去重。该兼容分支不改变 [#274
Out of Scope Changes check ✅ Passed 本次变更仅涉及 [#274] 的游标兼容逻辑、src/lib/ 的同步,以及对应回归测试。warning、旧版宿主降级和进程内去重都直接服务于游标兼容目标。未发现与 [#274] 无关的代码或测试变更。
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot AI review requested due to automatic review settings September 21, 2026 15:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@modusensus
modusensus merged commit a586213 into main Sep 21, 2026
9 checks passed
@modusensus
modusensus deleted the fix/persist-cursor-fallback branch September 21, 2026 15:31
modusensus added a commit that referenced this pull request Sep 21, 2026
CHANGELOG 的 [Unreleased] 转为 v0.8.6 小节,并补上三份未登账的改动:
工具暴露开关与描述纪律(#276)、蒸馏游标持久化(#229/#274)与旧宿主降级路径
(#279)、能力说明第 5 条文案精确性(#277)、CI 与徽章口径收敛(#281/#273/#272)。
两个 README 的 v0.8.6 占位行补描述(版本历史 + 路线图),测试徽章与注释数按
badge:sync 对齐到 1275。
modusensus added a commit that referenced this pull request Sep 21, 2026
* chore: v0.8.6 发布准备(版本号 + 测试徽章 + CHANGELOG/路线图占位行)

请在 CHANGELOG 与两个 README 的占位行补描述后再合并。

* chore(release): v0.8.6 发布说明与路线图补齐

CHANGELOG 的 [Unreleased] 转为 v0.8.6 小节,并补上三份未登账的改动:
工具暴露开关与描述纪律(#276)、蒸馏游标持久化(#229/#274)与旧宿主降级路径
(#279)、能力说明第 5 条文案精确性(#277)、CI 与徽章口径收敛(#281/#273/#272)。
两个 README 的 v0.8.6 占位行补描述(版本历史 + 路线图),测试徽章与注释数按
badge:sync 对齐到 1275。

---------

Co-authored-by: modusensus <modusensus@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants