fix(recommend): never advertise a range the fold gate always empties (#319)

[joshwapohlmann]

Problem

buildCompressibleRanges advertises ranges that the fold gate in src/compress.ts always empties, so acp_status offers a range and compress then refuses it:

compress FAILED m00239–m00256 → 0 blocks. Range would split 1 tool call/result pair(s) at the protected-zone boundary

The recommendation side filters protected messages out of the candidate set, but it evaluates turn integrity on a set the gate later sees differently: a tool call whose reasoning run, or whose result, sits inside the protected zone is advertised and then withdrawn, leaving nothing to fold.

Reviewing that path also turned up a second-order defect in the gate itself. INV1 (a kept tool call keeps its reasoning run) was evaluated before INV2 (a call and its result never split), so withdrawing a message for INV1 could leave an INV2 split standing, and one shape folded into a block holding only the tool turn's reasoning.

Fix

• computeIntegrityWithdrawals(messages, foldedIds) in src/turn-integrity.ts: re-checks both invariants to a fixed point and returns {withdrawn, splitTurnCount, splitPairCount}, so a withdrawal made for one rule cannot leave the other violated.
• src/compress.ts calls it where the single-pass withdrawal used to run.
• src/recommend.ts demotes the same messages out of the advertised candidate set and marks each withdrawn message as a gap, so the listing only offers ranges the gate will accept.

The fold-side allowance for the existing tool-pair rule (#564) is unchanged: this is a screening and fixed-point change, not a relaxation.

Tests

tests/recommend-range-integrity.test.ts: a fixture that folds once and then re-lists, with preserveRecentMessages: 4 and tool name edit (read sits in NEVER_PRESERVE_RECENT_TOOLS, so it never forms the split the test needs). Reverting only the recommend-side filter reproduces the original failure, advertised range m00003..m00004 must fold; with the filter both cases pass, and the fold produces a block that covers the whole advertised range.

Verification

npm test 806/806 on this branch (11 suites), 820/820 on the tree merged with current master; npm run typecheck clean. Verified against a live proxy afterwards: the previously failing session shape now folds its residue (compress requested 1 range(s) followed by Compressed … 1 block(s)), and no Range would split line appears for any advertised range.

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

Reviewing now. I'll verify the reported failure mode against the current code, re-run the suite on this branch and on a tree merged with master, check CI, and report back here with findings.

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

PR Review — #320 ✅ Recommend merge (not merged — merges are human-only here)

Duplicate screening — Searched open+closed issues/PRs for the advertised-but-refused-range / fold-gate-withdrawal topic: no duplicates. Prior related work (#684 turn grouping, #564 directional allowance) is correctly referenced and preserved. No other open PR touches src/recommend.ts or the fold gate in src/compress.ts, so no concurrent-file churn. This is the follow-up promised on #319.

Diff cleanliness — Clean. Four files, every line maps to the stated purpose; no lockfile/generated churn, no mass whitespace or reformatting:

• src/turn-integrity.ts: adds computeIntegrityWithdrawals(messages, foldedIds) → {withdrawn, splitTurnCount, splitPairCount}, re-checking INV1 (a kept call keeps its reasoning run) and INV2 (a call and its result never split) to a fixed point. Termination is monotone: remaining only shrinks, and each iteration acts only on previously-unhandled turns/pairs.
• src/compress.ts: applySingleRange's inline single-pass withdrawal replaced by a call to the shared helper; warning/error text paths unchanged.
• src/recommend.ts: buildCompressibleRanges screens its candidate set through the same helper; each withdrawn message becomes a gap so no advertised range spans it.
• tests/recommend-range-integrity.test.ts: regression fixture (fold once, re-list; preserveRecentMessages: 4, tool edit).

Bug verification — reproduced on current master (a679eca), not just trusted. Ran the PR's fixture shape against unfixed master:

• After folding head m00001..m00002, buildCompressibleRanges advertises residue m00003..m00004 (reasoning + call whose result sits in the protected zone) — the exact shape from compress advertises ranges the fold gate always refuses (protected-zone residue, plus a second-order turn split) #319.
• Folding it returns zero errors yet produces a block holding only the reasoning message (direct=["r1"]), leaving the tool call visible without its reasoning run — silent stream corruption that strict-echo providers (DeepSeek thinking mode) reject. Worse than the loud failure in the issue title.
• Ordering hole confirmed mechanically: the zone carve removes the result after boundary adjustment, then the old single pass withdraws the pair under INV2 after INV1 already ran, so the reasoning-only fold stands unchecked. And per the description's claim, new gate + old recommend filter fails loudly instead (advertised range m00003..m00004 must fold) — patching only one side leaves the other defect.

Root-cause layering: the reported symptom (advertised then refused) is one face; the deeper defects are (a) recommendation evaluating integrity on a different set than the gate sees (full candidate set vs. per-range effective set after the protected-zone carve), and (b) the gate's single-pass ordering. This fix addresses both layers.

Fix soundness

• Strictly stronger than the old gate: iteration 1 performs exactly the old two passes in the same order over the same set; later iterations only add withdrawals. It cannot admit a fold the old gate refused — no relaxation of tool-pair atomicity; the #564 reverse direction (reasoning/text kept while the pair folds) stays allowed and documented.
• No kernel-owned contract touched: message-id/ref immutability, wire format, single-owner content, lossless round-trip, pipeline node ordering, nudge invariants all untouched. Behavior delta = fewer/narrower advertised ranges and never fewer gate withdrawals.
• One residual edge, accepted: if a call/result pair were ever split across two different advertised ranges (requires a user message between call and result — these host loops don't produce that), the union-set screen wouldn't catch it; folding the first range would withdraw the local half at apply time (partial fold + warning, not guaranteed-empty). Pre-existing class, practically unreachable given adjacency.

Verification (run locally on the branch)

• npm run typecheck clean · npm test 806/806 (11 suites) · npm run build OK.
• Head bc0b7ee sits directly on top of current master a679eca (merge-base == base), so the branch tree is the merged-with-master tree and 806/806 is the merged-tree result. (The separate "820/820 merged" figure in the description doesn't correspond to the actual merged tree — presumably measured against a different local state; no impact on the conclusion.)
• New test fails on master as claimed (residue advertised instead of an empty listing) and passes on the branch; second test pins the positive case (wholly compressible turn still advertised and folds to exactly 1 block).

CI — Required checks green: test (22), test (24), pr-validation. The red build-artifact job fails only at npm publish with ENEEDAUTH: this is a fork PR and GitHub passes no base-repo secrets to fork workflows, so secrets.NPM_TOKEN resolves empty. Known systemic issue with a one-line owner-side fix already open in #288; the job's build steps themselves succeeded, and it is not a required check. Not caused by this PR.

Minor observations (non-blocking)

• tests/recommend-range-integrity.test.ts is not prettier-formatted (line-wrapping only). The PR's changed lines in src/recommend.ts/src/turn-integrity.ts are formatting-clean; the flagged lines there are pre-existing drift (master itself fails prettier --check on 109 files; CI runs no format job). Happy to reformat the test file if desired, but didn't want to push to the fork unprompted.
• Counts in the "Range would split N …" message shifted from violations-found-in-one-pass to actions-taken-to-fixpoint; monotonically ≥ old counts, cosmetic.

中文摘要： 审核了 #320——修复了推荐侧把"折叠门必然清空或损坏的范围"广播出去的问题（已在当前 master 上复现：残留范围被广播，折叠后静默生成只含 reasoning 的块、留下孤儿 tool-call，两个缺陷均属实且根因判断准确）；方案用共享定点函数同时约束折叠门与推荐侧，严格强于旧门、不放宽任何内核契约，diff 干净、测试充分（分支即合并后树，806/806），CI 红项是已知的 fork PR 发布问题（#288 待合，非必需检查），可以合并。

[joshwapohlmann]

Thanks for the review and for reproducing both layers on unfixed master. Answers to the three points:

The 820/820 figure. It is this branch's 806 plus the 14 tests of the companion wire codec (kernel PR #318), measured on the tree that merges the two, which is also the tree the local installation is built from. Re-measured today on a fresh merge of this branch with #318's codec onto a679eca: 820/820. Your reading of this branch's own tree is right: with head bc0b7ee sitting directly on a679eca, 806/806 is the merged-with-master result for this change alone.

Test-file formatting. Taken care of: tests/recommend-range-integrity.test.ts is now prettier-clean against the repo's defaults (whitespace only, 28 insertions / 6 deletions in that one file), committed as style(test): format the range-integrity fixture with the repo's prettier defaults. The branch still reports 806/806. No upstream file was touched, precisely because master itself is not a prettier fixed point.

The count in the "Range would split N …" message. Intentional: the number now reports actions taken until the fixed point rather than violations found in a single pass, so it is monotonically greater than or equal to the old count for the same fold. The wording paths themselves are unchanged.

The residual cross-range edge you accepted is recorded as you describe it: it requires a user message between a call and its result, which the host loops in question do not emit, so folding the first range withdraws the local half at apply time with a warning rather than folding empty. Pre-existing class, unchanged by this PR.

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

Independent second-agent review — verified locally, recommend merge.

I reproduced both defects on unfixed master before reading this diff, so I could check the patch against my own repros rather than the issue text alone:

1. Primary defect reproduced: after folding the head of a session whose newest turn keeps its result inside the recent zone, the next listing advertises the call-only residue, and folding it fails with exactly the reported error (Range would split 1 tool call/result pair(s) at the protected-zone boundary).
2. Second-order defect reproduced: folding [reasoning, call] with the result zoned produced a block holding only the reasoning run (directMessageIds = the reasoning message alone) — the visible call then loses its reasoning, which strict echo providers reject. Confirms the INV1-before-INV2 single-pass ordering as the cause.

On PR head 2dd51d5:

• tsc --noEmit clean; full suite 806/806 (11 suites) green locally.
• The shared fixed point checks out against both repros: the round-2 INV1 firing after the round-1 INV2 withdrawal is precisely what demotes the [reasoning, call] residue, and the reverse direction (#564: reasoning+text stay visible while call+result fold) still folds cleanly — a wholly-compressible turn remains advertised and folds into one block.
• Gate and recommend now share one verdict source, so the advertised-but-refused divergence class is closed for everything the gate sees in a single pass.

Two non-blocking notes:

1. CI build-artifact is infra, not code. The tarball builds fine (acp-kernel-0.0.75-pr.320.185, 161 files); the job dies at publish with npm error code ENEEDAUTH … need auth … logged in to https://registry.npmjs.org. The "PR Build Artifact" workflow needs its npm auth secret restored; pr-validation / test(24) / test(22) all pass.
2. Residual edge case (line-by-line analysis, no live repro — my sandbox ran out of disk mid-review). The recommend-side simulation runs once over the union of all candidate ids. A call/result pair that straddles a gap boundary and sits more than maxScan (20) messages apart is seen intact by the global sim (both halves are candidates) but split by a standalone fold, because adjustBoundariesForToolPairs extends at most 20 messages each way and cannot bridge the gap. An advertised range consisting solely of such a call would still fail at fold time. It requires a compression artifact or protected-tool turn between the halves plus >20 messages of separation, and under production minCompressRange defaults a sub-threshold fragment gets batched with its neighbor (which includes the result, healing the pair) or dropped — so exposure is very low, and the failure direction is safe (hard error, never a silent misfold). Suggested follow-up if you ever want it closed: run the simulation per gap-delimited segment (split the candidate runs on gapBefore before calling computeIntegrityWithdrawals) instead of over the whole candidate set. Not blocking.

Bottom line: both reported defects are fixed at the root (one fixed-point verdict shared by gate and recommendation), the new tests cover the failing shapes and the no-over-withdrawal shape, and nothing else regressed. Mergeable once the artifact workflow's npm auth is sorted (or accept the red build-artifact as known infra).

中文摘要:独立复现了两个缺陷并在本分支验证修复(共享定点撤回逻辑同时用于折叠门与推荐侧,typecheck 干净、806/806 全绿);build-artifact 的 CI 失败是 npm 发布凭据缺失的环境问题而非代码问题;另发现一个极罕见的残余边界情况(跨 gap 且相距超过 maxScan=20 的 call/result 对仍可能被广告后拒绝),方向安全、不阻塞合并,建议后续按 gap 分段模拟。

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

Update to note 2 above: now reproduced empirically (sandbox disk recovered mid-review). Minimal shape on head 2dd51d5 with preserveRecentMessages: 2, minCompressRange: 1:

u0        user
pc / pr   compress call+result     (gap: always-protected tool)
c1        pwsh call_1              <- isolated between the two gaps
pc2 / pr2 compress call+result     (gap)
f1..f19   assistant text fillers
t1        pwsh result call_1       <- 22 messages from c1 (> maxScan 20)
tail x2 + final user               (keeps t1 out of the recent zone)

The listing advertises m00004..m00004 (the lone call); folding it fails with exactly the issue's error — Range would split 1 tool call/result pair(s) at the protected-zone boundary. With only 6 fillers (t1 nine messages away, ≤ maxScan) the identical advertised range folds cleanly (blocks=1, zero warnings): the bidirectional boundary adjustment bridges the pair. So the residual is precisely gap-straddling pairs whose halves sit more than 20 messages apart — rarer than the general >maxScan case, and still safe-direction (hard error, never a silent misfold).

Still non-blocking per the earlier reasoning; the per-gap-segment simulation suggested in note 2 closes it if you want it closed.

[ranxianglei]

继续