fix: host-declared summaryOfBlockId — plain ranges skip host checkpoints (#335) - #336
ranxianglei wants to merge 1 commit into
Conversation
📦 Built Package ArtifactBranch: Option A — Install from npm PR tag (recommended)In your adapter project: npm install acp-kernel@pr-336Each push to this PR publishes a new version under the Option B — Download artifact
tar xzf acp-kernel-pr336.tgz
npm install ./packageThis comment is automatically updated on each push. |
[bot] 🏷 Reviewing PR #336 now. Initial status: CI is green (test 24/22, pr-validation, build-artifact all pass) and the diff is tight (+306/−1 across src/compress.ts, src/types.ts, tests/checkpoint-span.test.ts — no lockfile/generated churn). Pulling the branch to run local verification (typecheck/test/build + red-pre-fix check on master) and reading the full diff before giving a verdict. Will follow up with findings. |
[bot] 🏷 Review verdict: APPROVED — ready for human merge. Second-agent review complete (satisfies AGENTS.md §6 two-reviewer rule alongside the implementing session). No code changes needed; I found nothing to fix directly. Independent verification (not just trusting CI)
Diff cleanlinessTight: Code-level findings
One process note: strictly per AGENTS.md §7.4 this adds a small opt-in adapter-facing field rather than being a pure 中文摘要:独立复核了 PR #336(第二审查人)——在真实 master(v0.0.80)上验证了修复前红/修复后绿、本地 typecheck+865/865 测试+build 全过、diff 干净无无关改动、无重复条目;修复位置正确(与 |
|
Superseded by #338, which implements the same host-declared marker with two strict improvements over this PR: (1) liveness check — only carriers of still-active blocks are skipped, so stale carriers (block already consumed / unknown id) fold like ordinary messages and a dead block's leftover checkpoint cannot linger forever (this PR would have pinned it permanently); (2) kernel-side Verified before closing: read the full #338 diff; ran its head locally — typecheck clean, 868/868 tests pass, build OK; and cross-ran all four regression tests from this PR (#336) against #338 — all pass (liveness-at-call-start keeps the characterization flip intact even when the same call consumes the carrier's block). Closing to keep one issue = one open PR; #335 should track #338 going forward. |
[bot] 🏷 Acknowledged — supersession verified independently, no objection to closing. I checked both claims against the merged #338 diff (
Local runs at #338 head: Closing is correct — one issue = one open PR; #335 tracks #338 going forward. Nothing further needed on #336. 中文摘要:已独立核实 #338 的两项改进(liveness 判定 + kernel 侧 warnings 上报)确实存在于已合并 diff 中,本地 typecheck/869 测试/build 全绿、本 PR 的 4 条回归测试在 #338 代码上全部通过;同意关闭 #336,后续以 #335→#338 为准。 |
Closes #335
Problem (reproduced on v0.0.80)
applyCompressiononly treats kernel-renderedacp_summary_*ids as view-only summary placeholders (isSummaryMessageId, src/prune.ts:21). A host-managed-surface integrator (billion-context-dsh) projects its own compression checkpoint as an ordinaryuser/message; a PLAIN range spanning it folded the checkpoint into the new block'seffectiveMessageIds, silently dropping the previous distillation from the visible context with no supersedence signal in the result.Repro pre-fix: the new regression test below asserts the checkpoint is NOT in the new block's coverage and fails on current master with
host checkpoint must not be folded into the new block's coverage.Fix
Host-declarable marker instead of the kernel-private prefix:
CoreMessage.summaryOfBlockId?: string(src/types.ts) — the host declares "this message is my surface's rendering of block X, not primary content" (follows the existingthinkingTokenshost-field precedent).applySingleRange(src/compress.ts): for PLAIN ranges only (boundaryKind !== "block", i.e. both ends are mNNNNN refs), messages carrying the marker are excluded fromrangeMessageIds— the same choke point whereacp_summary_*ids are already filtered, applied after pair-boundary adjustment so it also covers widened spans. Block-boundary ranges (T2/T3 tier distillation) deliberately cross checkpoints and keep folding them.Unchanged by design: nested-block consumption/lineage (
directBlockIds, parents), tier output (plain range stays T1), soft protection, Bug-39 protected-tool filtering, unmarked-message behavior (the marker is opt-in).Tests — tests/checkpoint-span.test.ts (new)
Note: the test file referenced in #335 was never merged to main (no such file or PR exists upstream); this PR provides it.
directBlockIds [b1], b1 deactivated); prune keeps the checkpoint visible alongside the new rendered summary — the reported defect. Red pre-fix, green post-fix.Evidence:
npm run typecheckclean;npm test865/865 pass;npm run buildOK. Diff: +22 lines across src/types.ts and src/compress.ts plus the new test file; no lockfile/generated churn.中文摘要:修复了宿主自管 surface 的 checkpoint 消息被 plain range 当普通消息折叠的问题(#335)——新增
CoreMessage.summaryOfBlockId宿主声明标记,plain range(两端均为 mNNNNN)跳过带标记的消息、block-boundary 蒸馏路径行为不变;issue 中提到的回归测试此前并未合入 main,本 PR 已补齐。