Skip to content

fix: snap compress range endpoints onto hidden orphan calls (#396) - #397

Merged
ranxianglei merged 1 commit into
masterfrom
2026-09-25_orphan-endpoint-snap
Sep 25, 2026
Merged

ranxianglei merged 1 commit into
masterfrom
2026-09-25_orphan-endpoint-snap

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Summary

Fixes #396 — when a compress range endpoint lands on a failed compress call that was hidden by orphan-hiding, the whole range was rejected as cannot be anchored, wasting a compress attempt and consuming bcp's MAX_COMPRESS_ATTEMPTS quota, even though the rest of the range was visible and valid.

Root cause (and why not a blanket snap)

In resolveAnchorIndex (src/boundaries.ts), a message endpoint that is present in messageRefs.byRef but absent from the visible indexByMessageId falls through to a BoundaryNotFoundError("consumed"). That single code path serves two different cases that need opposite handling:

From (visible messages + state) alone these two are indistinguishable: both appear as "ref in byRef/byRaw, rawId not in the visible index, no block covers it." The only distinguishing signal is which refs the hide pass removed, which is dropped between processTurn and applyCompression (the host strips orphans from the carried conversation). So the hide pass now records them in state.

Change

  • src/types.ts — add optional CompressionState.hiddenOrphanRefs?: string[] (padded refs of orphan compress call/result messages currently hidden by the orphan window). Optional ⇒ old persisted states lack it ⇒ treated as empty (no migration).
  • src/hide-consumed.ts — hideConsumedCompressCalls additionally returns the padded refs of the orphan call/result messages it hides (skips non-orphans whose toolCallId belongs to a block, skips BLOCKED_REF, dedups).
  • src/compress.ts — hideCompressCallsNode writes hiddenOrphanRefs into state each turn; cloneState preserves it (the clone is field-by-field, so it had to be added explicitly or applyCompression's clone would silently drop it).
  • src/boundaries.ts — in resolveAnchorIndex, after the existing active-block anchor check: if the endpoint ref ∈ hiddenOrphanRefs, snap to the nearest adjacent visible message (start → next higher ref, end → previous lower ref; protected/BLOCKED and synthetic messages are auto-skipped because they are absent from byRaw) and return a snapped hint like the block-anchor path. If no neighbor exists in the required direction, throw a consumed error that names the real cause ("hidden orphan compress call with no adjacent visible message"). If the ref is not in hiddenOrphanRefs, fall through to the unchanged existing throw (preserves fix: protected-zone rejection carries acp_status guidance; leanPack ref-stability wording (#385) #387 byte-for-byte).

No pipeline-node ordering change, no wire-format change, no new dependency.

Behavior change (flagged per owner policy)

Case Before After
Endpoint on a hidden orphan compress call whole range rejected: cannot be anchored … likely consumed by an existing block (misattributed cause) endpoint snaps to the adjacent visible message; the range proceeds with a snapped warning
Hidden-orphan endpoint with no neighbor in that direction cannot be anchored … likely consumed by an existing block cannot be anchored … hidden orphan compress call with no adjacent visible message … run acp_status (accurate cause)
Drifted / dangling ref (#387) clear "cannot be anchored … dangling" error unchanged

Review focus (§7.5)

Tests

New tests/orphan-endpoint-snap.test.ts (6 tests, all pass):

  1. hide pass records exactly the hidden orphan refs (["m00001","m00002"]).
  2. start endpoint snaps forward to the next visible message.
  3. end endpoint snaps backward to the previous visible message.
  4. a dangling ref not in hiddenOrphanRefs still throws consumed (guards fix: protected-zone rejection carries acp_status guidance; leanPack ref-stability wording (#385) #387).
  5. hidden-orphan endpoint with no neighbor in the required direction → accurate error.
  6. end-to-end applyCompression: range m00005..m00013 with m00005 a hidden orphan → errors empty, blocksCreated 1, warnings include snapped.

Full suite: 1047 pass / 0 fail. typecheck + build green.

Formatting note

All files touched by this PR conform to Prettier except src/hide-consumed.ts, which uses 4-space indentation repo-wide (a pre-existing inconsistency — the baseline version already fails prettier --check). Reformatting it would be unrelated whole-file churn, so it was left matching its existing style. The full-tree format:check also surfaces other pre-existing failures (json/config/some test files under prettier 3.9.6 vs the devDep floor ^3.3.0) unrelated to this change.


中文摘要:修复了 compress 区间端点落在「被 orphan-hiding 隐藏的失败 compress 调用」上时整条区间被误判为 cannot be anchored 的问题——隐藏路径现在把被隐藏孤儿的 ref 记进 state,边界解析把这种端点吸附到最近的相邻可见消息(并给出 snapped 警告);#387 的漂移引用语义保持不变。改动仅新增一个可选 state 字段,属 §7.3 状态结构变更,需人工合并。

When a compress range endpoint lands on a failed compress call that was hidden by orphan-hiding, resolveAnchorIndex rejected the whole range as 'cannot be anchored' even though the rest was visible and valid. The hide pass now records the padded refs of hidden orphan call/result messages in state.hiddenOrphanRefs; boundary resolution snaps such an endpoint to the nearest adjacent visible message (start -> next, end -> prev) and emits a 'snapped' warning, mirroring the existing block-anchor path. A dangling ref NOT in hiddenOrphanRefs still throws the existing consumed error (preserves #387 drift semantics); when no neighbor exists in the required direction the error now names the real cause (hidden orphan with no adjacent visible message).

Adds optional CompressionState.hiddenOrphanRefs (absent on old persisted states = empty). No version bump.

Fixes #396
@github-actions

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-25_orphan-endpoint-snap (5a664f5)

Option A — Install from npm PR tag (recommended)

In your adapter project:

npm install acp-kernel@pr-397

Each push to this PR publishes a new version under the pr-397 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf acp-kernel-pr397.tgz
npm install ./package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 收到,开始按 PR Review 规范审查本 PR(fix: 端点吸附到被 orphan-hiding 隐藏的失败 compress 调用):先核对原 issue #396 的方向与根因判断,再拉分支审代码、回归面、测试有效性与 diff 整洁度,并确认 CI 状态。完成后回复完整结论。

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Reviewed PR #397 (2026-09-25_orphan-endpoint-snap, 5a664f5, base master 77b651a) against issue #396. CI is fully green (test Node 22 + 24, pr-validation, build-artifact) and the branch merges cleanly. Verdict: ready for human merge (§7.3 state-schema gate applies, as the PR notes). Details below.

① Original-issue direction — correct. I traced the failure path in resolveAnchorIndex (src/boundaries.ts): ref found in byRef → rawId absent from the visible indexByMessageId → no active block covers it → BoundaryNotFoundError("consumed") with the misleading "likely consumed by an existing block" text. That is exactly #396's root cause (endpoint lands on an intact-but-hidden failed compress call, not a consumed message), and the PR fixes the cause, not the symptom: option 1 (snap to nearest visible neighbor, start→forward, end→backward, with a snapped hint mirroring the existing block-anchor path) plus option 3 as a bonus (the no-neighbor case now names the real cause instead of misattributing it).

② Regression audit — existing behaviors touched, one by one:

  • unknown-ref error (typo/wrong session): unchanged.
  • Active-block anchor path (activeOwnerAnchor): unchanged, and still takes precedence over the new snap (correct order — a block-covered message is never an orphan).
  • fix: protected-zone rejection carries acp_status guidance; leanPack ref-stability wording (#385) #387 dangling/drifted-ref error: byte-for-byte preserved. A drifted ref is not in hiddenOrphanRefs (its message was replaced, not hidden by the orphan pass), so it falls through to the original throw. Guarded twice: the pre-existing drift test at tests/compress.test.ts:661 (untouched) and new test 4.
  • Final generic "consumed" throw: kept as fall-through for everything else.
  • Pre-feature persisted states: hiddenOrphanRefs is optional; state.hiddenOrphanRefs?.includes(...) is a no-op when absent → old behavior exactly. No migration needed (claim verified).
  • Intended behavior change (disclosure): a range endpoint landing on a hidden orphan compress call now resolves to the adjacent visible message with a snapped warning, instead of rejecting the whole range with a misleading error. Known edge: if both endpoints are hidden orphans and nothing visible lies between them, the snaps can land crossed and the pre-existing start/end swap then widens the range slightly beyond the request. Bounded by the min-size gate and overlap detection, consistent with the option-1 semantics the issue proposed — accepted, flagged for awareness.
  • Soft-protection is not bypassed: snapping only chooses the anchor index; applySingleRange's protected-zone backstop still applies downstream.
  • HideConsumedResult gains a required field (hiddenOrphanRefs) — additive for destructuring consumers (hosts take {messages, hidden}); any external constructor breaks at compile time (semver-minor surface, fine for a 0.x kernel).
  • One gap found (not blocking, see ③): two other field-by-field state copies don't carry the new field.

③ Code review. Verified: ref-width consistency (REF_WIDTH=5 fixed in src/refs.ts:3, indexToRef padStart(5) == formatPaddedRef padStart(5); membership check normalizes via formatPaddedRef(numericId) so non-canonical input like m5 works); nearest-by-ref == nearest-by-position holds because refs are assigned monotonically and never recycled (the AGENTS.md no-reuse invariant — the code comment documents this); neighbor candidates correctly exclude BLOCKED_REF protected messages and synthetic summary/retrieved messages (no byRaw entry); rememberHiddenRef correctly skips messages whose toolCallId belongs to any block, so inactive-block pairs keep their own (already accurate) error path and don't pollute the snap set; both call and result refs are recorded, dedup'd; no input-state mutation (hide node returns a fresh state object; cloneState copies the array). The cloneState addition is load-bearing — applyCompression clones state before boundary resolution (src/compress.ts:235), and without it the field would be dropped and the e2e test would fail.

  • Gap (optional hardening, your call): mergeCompressionState (src/persist/state-merge.ts) and syncBlocks (src/sync.ts) also copy state field-by-field and omit hiddenOrphanRefs, unlike cloneState. Impact is nil in practice: the field is derived data, processTurn recomputes it via the hide-compress-calls node (which runs after sync-blocks in the pipeline) on every turn, and hosts feed applyCompression the same-turn processTurn output — so worst case (reload + compress before any new turn) self-heals within one turn and never affects pre-feature behavior. If you want belt-and-braces, the two lines are: hiddenOrphanRefs: parsed.hiddenOrphanRefs ?? undefined in state-merge.ts and hiddenOrphanRefs: state.hiddenOrphanRefs ? [...state.hiddenOrphanRefs] : undefined in sync.ts. I deliberately left this to you rather than pushing onto the PR branch, since it's a scoping decision (carry derived state or not) on a human-gated PR.

④ Tests — real teeth, not ceremony. All 6 new tests assert concrete outcomes: exact snap indices (direction-sensitive — a wrong-direction implementation fails), deepEqual on the recorded ref set, kind+endpoint+message-regex on throws, and the e2e asserts errors.length === 0 && blocksCreated === 1 && warnings ~ /snapped/ through the full applyCompression path (pinning the cloneState fix). The #387 regression is explicitly guarded. Minor untested corners: the allBlockCallIds skip in rememberHiddenRef has no direct unit test, and there's no test for a reloaded state lacking the field (safe by construction via ?.). Acceptable.

⑤ Diff cleanliness. 5 files, +394/-2, every line serves the fix; test file is new; no generated/lockfile noise; version untouched; branch naming compliant; single commit closing #396. The formatting note checks out: src/hide-consumed.ts is 4-space-indented on master too (no .prettierrc, Prettier defaults to 2), so tree-wide format:check failures pre-date this PR and the changed lines correctly match the file's local style.

Open item: the optional 2-line carry-over in ③. Otherwise nothing blocks merge — over to you per §7.3.

修复了「压缩范围端点落在被孤儿隐藏窗口藏掉的失败 compress 调用上时整段范围被误报 cannot be anchored」的问题(端点吸附到相邻可见消息并给出 snapped 提示,#387 悬空引用报错保持不变);代码、回归面、测试与 diff 均审过无阻塞项,仅剩一处可选的两行加固(state-merge/sync 不携带新派生字段,可自愈),可以合并(按 §7.3 由人工执行)。

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.

compress 端点落在被 orphan-hiding 隐藏的失败 compress 调用上时整条区间被拒为 cannot be anchored(应吸附到相邻可见消息)

1 participant