Skip to content

fix(preflight): preserve complete content when recovering blocked ranges - #668

Open
Wuxie233 wants to merge 1 commit into
ranxianglei:masterfrom
Wuxie233:2026-09-10_preflight-covered-range-recovery
Open

fix(preflight): preserve complete content when recovering blocked ranges#668
Wuxie233 wants to merge 1 commit into
ranxianglei:masterfrom
Wuxie233:2026-09-10_preflight-covered-range-recovery

Conversation

@Wuxie233

@Wuxie233 Wuxie233 commented Sep 9, 2026

Copy link
Copy Markdown

When preflight selects a raw range already covered by an active block, it can exhaust its candidates without reducing an oversized request. Additionally, kernel tool/reasoning pairing can expand a requested range beyond the text previously supplied to the summarizer.

This change previews the kernel's actual compression boundary without mutating the session, skips rejected ranges, and collects exactly the content to be consumed using the original state so active child blocks remain summaries. Large content is summarized in bounded segments, and compression is applied only after every segment succeeds and the assembled summary satisfies the configured limit. Non-positive maximum summary lengths retain the kernel's unlimited semantics.

If ordinary candidates are exhausted while the request remains over-window, preflight retries with recent soft protection relaxed. Hard tool protection remains enforced; unknown-baseline decisions use the conservative upper bound. Summary carriers, persistent history, and the configured model window are unchanged. The shared preflight preserves the kernel contract in both modes; this does not change the plugin/proxy wire-carrier distinction.

Refs #663. This is a standalone range-recovery fix on current master, complementary to #665. It intentionally does not include the output-limit or Lite-header compatibility patches from that PR.

Validation:

  • Eight targeted regressions cover already-covered ranges, hard protection, full paired tool-result coverage, unknown baselines, missing/oversized segmented summaries, and unlimited summary lengths.
  • Standalone branch: typecheck/build pass and all 1254 tests pass in the serial unit suite. Upstream CI also passes on Ubuntu and Windows with Node 22 and 24; version guard passes.
  • The combined production candidate previously passed 1260 tests before the final unlimited-length correction, followed by focused tests for that correction.
  • An isolated real-upstream replay using the combined candidate reduced the failing request from approximately 287565 to 16389 tokens across three ranges, without changing its source session. All 262 sampled positions from the largest tool result entered summary requests; synthetic tests additionally assert full result coverage.
  • The deployed combined candidate passed shared-daemon reply and harmless tool-execution probes. These live checks include the separate Responses compatibility fixes and are not evidence that this standalone branch resolves those compatibility errors.

The repository's gated four-phase client E2E suite has not been rerun for this standalone branch; it remains a pre-merge check. No private history, deployment scripts, credentials, or generated bundles are included.

CI note: the separate build-artifact workflow successfully builds but fails at publishing its npm PR tag with ENEEDAUTH because the fork PR has no npm publishing credential. The four test-matrix jobs pass. This is a publishing-workflow limitation, not a compile/test failure.

@Wuxie233

Wuxie233 commented Sep 9, 2026

Copy link
Copy Markdown
Author

Validation is complete for commit 71c6695: typecheck/build pass, all 1254 standalone serial tests pass, and all four upstream CI test jobs pass (Ubuntu/Windows, Node 22/24). Version guard also passes.

The separate build-artifact job built successfully but failed at the npm PR-tag publish step with ENEEDAUTH: fork PRs do not receive the publishing credential. No credential or workflow permission change is needed to review this patch. Please treat that as a separate publishing-workflow issue; the branch and exact source diff are available in this PR. The gated real-client E2E suite remains a pre-merge check, as documented in the PR body.

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

Review of 71c6695 complete — all claims independently verified.

Diff: single commit, two files — src/preflight.ts (+84/−28) and new tests/preflight-covered-ranges.test.ts (+111). No unrelated changes, no version or lockfile drift. Clean.

Verification (run locally on the PR head)

  • npm run typecheck — pass
  • npm test — 1254/1254 pass (matches your report)

Design review against acp-kernel 0.0.60

  • The preview is mutation-safe: kernel applyCompression starts with cloneState(input.state), so deps.session.state is untouched by the probe.
  • Blocked-range recovery is real: raw refs to messages consumed by an active block snap to the covering block's anchor (resolveAnchorIndex), the preview yields a parent block with nestedBlockIds, and collectBlockContent(originalState, planned, { full: false }) renders the direct raw messages in full with active children as summaries — so the summarizer input exactly matches what the kernel will fold. The old renderRange path only saw raw index-span text and could lose child-block content.
  • The folded credit in the unknown-baseline regime now derives from planned.directMessageIds (the kernel's actual boundary after applyPairBoundaryAdjustments), not the requested index span. Slightly conservative (child-summary chars not netted) — the safe direction for the fit decision.
  • Segmented summarization: splitSummaryContent always advances ≥1 char (no infinite loop even when a single char exceeds budget); all segments must succeed and the assembled candidate must satisfy maxSummaryLength (<= 0 = unlimited, matching kernel validation) before anything is applied. Incomplete or over-limit sets leave the original content untouched — covered by the two new regression tests.
  • Both relaxation gates (top-of-round for unknown baselines via finalUpper, and the new end-of-round gate when every range was rejected) only relax preserveRecentMessages/Tokens; hard protectedTools stays enforced. No wire-carrier, system, or tool-injection changes — mode-neutral across plugin and proxy modes.

Remaining pre-merge check: the gated four-phase real-client E2E (tests/e2e/e2e-codex.test.ts, ACP_TEST_E2E=1) — required for preflight-path changes per AGENTS.md, and not runnable in this environment (needs the codex CLI + a live upstream). Keep it as the last gate before merge.

ENEEDAUTH on the build-artifact job: confirmed as a separate workflow defect. .github/workflows/pr-artifact.yml triggers on all PRs to master and authenticates the publish step with secrets.NPM_TOKEN, which GitHub does not share with fork PRs — so the job fails post-build and skips the tarball/artifact/comment steps for every fork PR. No credential or permission change is needed to review or merge this patch. Filed separately as #670 with a suggested fix (gate the publish step to non-fork PRs).

中文摘要:PR 已独立复核(typecheck 与 1254 项测试全部通过),preflight 预览/分段摘要/软保护放宽逻辑与 kernel 0.0.60 实际行为一致、diff 干净,审查通过,合并前仅剩 E2E 门控;fork PR 的 ENEEDAUTH 属发布工作流缺陷,已另开 #670

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