feat(core): inspect persisted conversation branches - #7185
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
282f6ed to
a632b8d
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
|
Thanks for the PR, @doudouOUC! Template looks good ✓ — all required sections present. Problem: This is a Direction: Session branch topology is a natural extension of the existing Size: 548 production logic lines ( Approach: The implementation is clean and well-structured. Since the last review, the author addressed all feedback: F1 (neutral-only roots exposed as phantom branches) is fixed and regression-tested, collapsed branch ordering follows physical terminal position, The core question remains whether this infrastructure should land now as a foundation, or wait until the write-side consumer is ready so the API can be shaped by actual usage. Moving on to code review. 🔍 中文说明感谢贡献,@doudouOUC! 模板完整 ✓ 问题: 这是一个 方向: 会话分支拓扑是现有 规模: 548 行生产逻辑代码,498 行测试,68 行设计文档。触及核心路径——按 500+ 生产行阈值标记需维护者关注。 方案: 实现干净且结构良好。自上次审查以来,作者解决了所有反馈:F1(纯元数据根被暴露为幽灵分支)已修复并有回归测试,折叠分支排序遵循物理终端位置, 核心问题仍然是:这个基础设施应该现在作为基础落地,还是等到写侧消费者准备好后再根据实际使用来塑造 API。进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewIndependent proposal: To build a read-only branch inspector, I would: (1) index records by UUID with first-occurrence semantics, (2) build a children map, (3) find physical leaves, (4) collapse metadata-only terminals to their nearest non-metadata ancestor (omitting neutral-only chains with no conversation ancestor), (5) deduplicate the resulting antichain, (6) walk parent chains from each leaf to produce summaries, (7) classify branches relative to rewind records, (8) report structural diagnostics. This is what the PR does — the approach matches and I don't see a simpler path. Findings: No correctness bugs, security issues, or regressions found. Changes since the last review pass:
The underlying implementation remains clean: first-occurrence semantics matching One latent concern noted by wenshao remains: the inspector indexes all records while TestingNon-user-visible change (pure read-side API with no runtime consumer). tmux captures the test, typecheck, and lint execution as evidence. All 28 tests pass (20 new + 8 existing conversation-chain). Core typecheck clean. ESLint clean on both new files. 中文说明代码审查独立方案: 构建纯读分支检查器:(1) 按 UUID 索引记录(首次出现语义),(2) 构建子节点映射,(3) 找物理叶子,(4) 将仅元数据的终端记录折叠到最近非元数据祖先(省略无会话祖先的纯中性链),(5) 去重得到反链,(6) 从每个叶子走父链生成摘要,(7) 根据 rewind 记录分类分支,(8) 报告结构性诊断。PR 的做法与此一致。 发现: 未发现正确性 bug、安全问题或回归。自上次审查以来的变更:F1 修复(纯元数据根被省略而非报告为幽灵分支)、折叠排序遵循物理终端位置、合成用户过滤覆盖 wenshao 指出的一个潜在问题仍存在:inspector 索引全部记录,而 测试非用户可见变更。tmux 捕获了测试、类型检查和 lint 执行作为证据。28 个测试全部通过。core typecheck 干净。ESLint 干净。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 4/5 — clean code, thorough tests, all prior feedback addressed, F1 fixed and regression-tested. The remaining point is the product-direction question (no runtime consumer), which is a maintainer call, not a code-quality concern. The code is solid. The implementation matches what I'd independently propose, and the three fix commits since the last review pass are all correct: neutral-only roots are now omitted (F1), collapsed branch ordering follows physical terminal position, and synthetic user filtering covers all known subtypes. 28 tests pass, typecheck and lint are clean. wenshao's independent runtime verification adds significant confidence: 5000-transcript fuzz with the real writer semantics (active-tail advance, metadata tails, rewind re-rooting, concurrent-writer siblings), 32/32 runtime assertions against the compiled functions, and mutation testing proving the suite catches regressions. The cross-check invariant (every returned leaf reconstructs gap-free via The reservation is unchanged from the last pass: this PR ships a 547-line public core API that nothing in the codebase calls. The PR is upfront about this. The author's rationale — validating a pure, deterministic read model independently of the write-side mutation and recovery policy — is reasonable, and the split does narrow the review surface. But the API surface is being designed without feedback from real usage, and if the write-side consumer lands differently than expected, the read-side may need reshaping. Given the code quality, test coverage, wenshao's runtime verification, and the author's responsiveness to feedback, approving the code. The timing question (land now as a foundation vs. wait for the consumer) is for the maintainer — flagging for awareness per the 500+ production line threshold on fork PRs touching core paths. 中文说明置信度:4/5 — 代码干净,测试充分,所有先前反馈已解决,F1 已修复并有回归测试。剩余一分是产品方向问题(无运行时消费者),这是维护者的判断,不是代码质量问题。 代码扎实。实现与我的独立方案一致,自上次审查以来的三个修复提交都正确:纯元数据根现在被省略(F1),折叠分支排序遵循物理终端位置,合成用户过滤覆盖所有已知子类型。28 个测试通过,typecheck 和 lint 干净。 wenshao 的独立运行时验证显著增强了信心:5000 条 transcript 模糊测试(使用真实 writer 语义),32/32 运行时断言,变异测试证明测试能捕获回归。交叉校验不变量在所有 5000 条 transcript 上成立。 顾虑与上次相同:本 PR 交付了一个 547 行的公共 core API,代码库中无处调用。作者的理由——独立于写侧变更验证纯确定性读模型——是合理的。但 API 表面设计没有来自真实使用的反馈。 鉴于代码质量、测试覆盖、wenshao 的运行时验证以及作者对反馈的积极响应,批准代码。时机问题(现在作为基础落地还是等消费者)由维护者判断——按 500+ 生产行阈值标记需维护者关注。 — Qwen Code · qwen3.7-max Reviewed at |
|
Thanks — no code change from this review. The split is intentional: this PR validates a pure, deterministic read model against the existing persisted ChatRecord schema and a sanitized incident topology, while the later restart/selection change owns mutation and recovery policy. Keeping those concerns separate narrows the risk and lets the topology rules be reviewed without write-side behavior; the later consumer can adapt at the call boundary without changing persisted-data analysis. Maintainer guidance on whether to land this foundation separately is welcome. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Addressed all three review suggestions in
Verification: full repository build passed; full workspace typecheck passed; 26 targeted conversation branch/chain tests passed. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
Local runtime verification — PR #7185 ✅I built and ran this PR locally against the real compiled code (not just vitest) as a merge reference. Net: the PR does what it claims — behavior is correct for every realistic transcript, and the tests have real teeth. One latent edge case is worth an optional follow‑up (F1 below); it is non‑blocking because nothing consumes this API yet. How I verified
Results
Confirmed at runtime, beyond the test file:
Mutation testing (proves the suite isn't vacuous):
Minor: the reviewer test‑plan step 1 says "expect all 23 tests"; head actually has 26 (the review‑feedback commit added 3). Harmless — just bump the number. Finding F1 — metadata‑only record with no non‑neutral ancestor → phantom "ordinary" branch (latent, non‑blocking)
Minimal repro: const recs = [{ uuid:'meta', parentUuid:null, sessionId:'s', timestamp:'2026-01-01T00:00:00.000Z',
type:'system', subtype:'session_artifact_event', cwd:'/w', version:'0' }];
inspectConversationBranches(recs).branches;
// => [{ leafUuid:'meta', classification:'ordinary', ... }] // phantom branch
buildOrderedUuidChain(recs, { leafUuid:'meta', detectGaps:true }); // => { uuids:[], gaps:[] } // cannot reconstructReachability: the only writer that emits artifact records is ACP Recommendation👍 Good to merge as the read‑side foundation. Treat F1 as a small follow‑up (or a cheap guard) before anything starts selecting/consuming these leaves. 🇨🇳 中文说明本地真实构建验证 — PR #7185 ✅作为合并参考,我在本地用真实编译产物(不仅是 vitest)构建并运行了本 PR。结论:PR 行为与描述一致 —— 对所有真实 transcript 都正确,且测试有实际约束力。 有一个潜在边界情况值得作为可选后续处理(下方 F1),但不阻塞合并,因为目前还没有任何运行时消费该 API。 验证方式
结果
在测试文件之外的运行时确认:
变异测试(证明测试非空洞):
小问题: reviewer 测试计划第 1 步写的是「23 个测试」,但 head 实际是 26 个(review‑feedback commit 新增了 3 个)。无害,更新下数字即可。 发现 F1 —— 无非中性祖先的纯元数据记录 → 幽灵 "ordinary" 分支(潜在,不阻塞)
最小复现: const recs = [{ uuid:'meta', parentUuid:null, sessionId:'s', timestamp:'2026-01-01T00:00:00.000Z',
type:'system', subtype:'session_artifact_event', cwd:'/w', version:'0' }];
inspectConversationBranches(recs).branches;
// => [{ leafUuid:'meta', classification:'ordinary', ... }] // 幽灵分支
buildOrderedUuidChain(recs, { leafUuid:'meta', detectGaps:true }); // => { uuids:[], gaps:[] } // 无法重建可达性: 唯一会写 artifact 记录的 writer 是 ACP 建议👍 作为读侧基础可以合并。把 F1 当作小的后续项(或加个便宜的护栏)即可 —— 在任何逻辑开始选择/消费这些叶子之前处理。 Verified locally on Linux, Node v22.22.2, against PR head |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
|
@wenshao Thanks for the thorough runtime verification. I independently reproduced F1 on the current head and confirmed that an ACP artifact can be persisted before the first conversation record.
Verification: full repository build passed, workspace typecheck passed, and the targeted branch/chain suites passed 27/27. |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Review follow-up summary for
Verification: full repository build passed, workspace typecheck passed, and branch/chain unit suites passed 28/28. No feedback was rejected or deferred. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Review follow-up summary for
Verification: full repository build passed, workspace typecheck passed, and branch/chain unit suites passed 28/28. No feedback was rejected or deferred. |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.20.1. |

What this PR does
Adds a read-only conversation topology inspector that identifies semantic leaves in persisted session trees, reports missing-parent, cycle, and conflicting-parent diagnostics, and produces deterministic summaries for each branch.
Normalizes metadata-only terminal records so custom-title and session-artifact writes do not appear as phantom conversation branches, while preserving recovery-significant system records such as rewind, compression, attribution, and file-history checkpoints.
Classifies rewind descendants and rewind siblings strictly as structural relationships. It does not select an active branch, mark a sibling abandoned, mutate the transcript, or change current resume, fork, daemon, ACP, or CLI behavior.
Why it's needed
Persisted sessions already form a tree through parent UUID links, but restart currently reconstructs one physically selected tail. Rewinds or concurrent writers can leave multiple valid sibling histories in the same JSONL file, so selecting only the physical tail can hide recoverable work. This PR provides the independent read-side foundation needed to enumerate and summarize those branches before a later write-side change adds exclusive ownership and durable branch selection.
Reviewer Test Plan
How to verify
cd packages/core && npx vitest run src/utils/conversation-branches.test.ts src/utils/conversation-chain.test.ts; expect all 28 tests to pass.Evidence (Before & After)
N/A — this PR adds a pure read-side core API and does not change user-visible behavior.
Tested on
Environment (optional)
Node.js v22.22.3, local non-sandboxed unit tests.
Risk & Scope
Linked Issues
N/A
中文说明
本 PR 做了什么
新增纯读的会话拓扑检查能力,用于识别持久化会话树中的语义叶子、报告父节点缺失、父链循环和重复 UUID 父节点冲突,并为每个分支生成确定性摘要。
对仅包含元数据的终端记录进行归一化,避免自定义标题和会话 artifact 写入被误识别为虚假会话分支,同时保留 rewind、压缩、归因和文件历史检查点等对恢复有意义的 system 记录。
将 rewind descendant 和 rewind sibling 严格定义为结构关系。本 PR 不选择 active branch,不把 sibling 标记为 abandoned,不修改 transcript,也不改变现有 resume、fork、daemon、ACP 或 CLI 行为。
为什么需要
持久化会话已经通过父 UUID 链形成树结构,但重启恢复目前只重建一个按物理位置选出的尾部。rewind 或并发 writer 可能在同一 JSONL 文件中留下多个有效 sibling history,只选择物理尾部会隐藏仍可恢复的工作。本 PR 提供独立的读侧基础能力,用于枚举和摘要这些分支;后续写侧变更再补充独占所有权和持久化分支选择。
Reviewer 测试计划
如何验证
cd packages/core && npx vitest run src/utils/conversation-branches.test.ts src/utils/conversation-chain.test.ts,预期 28 个测试全部通过。证据(Before & After)
N/A——本 PR 新增纯读 core API,不改变用户可见行为。
测试平台
环境(可选)
Node.js v22.22.3,本地无 sandbox 单元测试。
风险与范围
关联 Issue
N/A