Skip to content

fix: gate fork sessions on char-count upper bound in preflight (#553) - #558

Merged
ranxianglei merged 1 commit into
masterfrom
2026-09-05_fork-preflight-upper-bound
Sep 7, 2026
Merged

ranxianglei merged 1 commit into
masterfrom
2026-09-05_fork-preflight-upper-bound

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Problem (#553)

Anonymous prefix-affinity sessions minted after an ACP compression (forks) start with lastInputTokens = 0. Preflight then decided solely on the optimistic defaultCountTokens estimate (non-CJK 4 chars/token), which undercounts dense code/JSON replays by up to ~4x — so over-window payloads were forwarded raw upstream: no compression, no interception, doom-looping into #551's 300s transfer cutoffs and exceed_context_size_error 400s.

Fix

When a session has no measured baseline AND arrived anonymously (prefix-affinity resolution), judge payload size by a char-count upper bound — 1 token per char, which never undershoots BPE token counts since every token covers at least one character. Applied at three points:

  • Trigger: tokenCount = char upper instead of max(baseline, optimistic), so over-window dense replays enter the compress loop.
  • Nothing-compressible gate: such sessions fail fast (structured 502) instead of gambling a raw forward past the window.
  • Loop internals: char-based chunk packing (min-compress-range integrity preserved via a minUnits guard so sub-minimum chunks are never emitted), char-based netting, and a regime-aware PreflightResult.fitsWindow verdict — optimistic for measured-baseline sessions (preserves the fix: stitched-stream usage double-count after compress re-request (adapter-anthropic >0 guard) #300 stale-baseline contract byte-for-byte), char upper for unknown-baseline ones (closes the residual gap where the loop exits with failure while the optimistic figure still says "fits").

Explicit-identity zero-baseline sessions keep today's optimistic path: they are genuinely-new or post-native-compaction populations whose payloads are small or shrink, and they self-heal via the learned-window overflow path (persist.ts persists lastInputTokens across restarts, so zero-baseline anonymous sessions are exactly the fork/reload population).

Why not an estimator multiplier? Any fixed k<4 false-fires on repetitive prose that BPE-compresses at ~4+ chars/token (the codex-compact-e2e setup sits at 3.5x window in chars while truly fitting at 87% of window); k=4 is exactly the bug. Content is indistinguishable by char-ratio, so the conservative measure is scoped structurally (anonymous + zero-baseline) instead.

Tests

  • New tests/preflight-fork-estimate.test.ts: anonymous fork replay whose optimistic estimate fits but char upper does not → preflight fires, summarizes, forwards within window; small anonymous fresh session → no preflight (no over-fire). Both verified RED on old code / GREEN on fix.
  • Full suite: 948/949 (single failure is pre-existing env-only in launcher.test.ts, fails identically on clean tree — sandbox lacks client binaries).
  • npm run typecheck clean; npm run build clean.
  • E2E: E2E_CHECK=1 zero-token preflight passes against built dist. Full ACP_TEST_E2E=1 run is not completable in this sandbox (codex warmup fails identically on the clean tree before any change lands) — flagged for CI/manual run.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 On it — checking out 2026-09-05_fork-preflight-upper-bound, will verify the diff, re-run typecheck/test/build, and confirm the new test is RED on old code / GREEN on fix before reporting back.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 Review complete — LGTM with minor nits. I checked out the PR branch and verified every claim independently.

Verification (all executed, not taken on trust)

Check Result
npm run typecheck ✅ clean
Full suite ✅ 948/949 — the single failure is launcher.test.ts "resolveClientCommand: codex/claude resolve to themselves"; I re-ran it on a clean origin/master worktree and it fails identically there → pre-existing env-only (sandbox has a codex binary on PATH), as claimed
npm run build ✅ clean (dist/index.js 2.60 MB)
RED/GREEN ✅ new fork-replay test fails on master code, passes on the fix; the over-fire guard passes on both (correct regression-guard behavior)
E2E_CHECK=1 zero-token preflight ✅ passes against built dist
Full ACP_TEST_E2E=1 ⚠️ not completable in this sandbox (real upstream + codex warmup required; warmup fails identically before any change lands). Flagging for CI/manual-dispatch run before merge

Triage conclusions

  1. Bug is real and correctly layered. The root cause is in the measurement regime, not in chunking or folding: anonymous + lastInputTokens <= 0 sessions are exactly the fork/reload population (prefix-affinity resolution mints a fresh session id after ACP compression; persist.ts only persists a baseline that was ever measured), and for them the optimistic 4-chars/token estimate has no floor to lean on. Fixing the estimator multiplier was rightly rejected — any k<4 false-fires on prose, k=4 is the bug itself; content is indistinguishable by char-ratio, so structural scoping is the right layer.
  2. fix: stitched-stream usage double-count after compress re-request (adapter-anthropic >0 guard) #300 contract preserved. For measured-baseline sessions every measure reduces byte-for-byte to the old code path (countText = defaultCountTokens, credit-based netting, fitsWindow = payloadEstimate < limit) — I traced this line by line in src/preflight.ts. The stale-baseline forward contract is untouched.
  3. Both compression modes considered. Plugin-mode Anthropic clients early-return empty processedMessages in pluginMode, so the trigger can't fire there; opencode sends x-session-affinity (not anonymous). The new regime effectively covers proxy-mode anonymous clients — matching the design scope.
  4. Mechanics check out: applyRanges doesn't mutate originals in place (so post-fold char netting is correct); the minUnits guard can't infinite-loop (chunks always advance ≥1 message; sub-minimum trailing chunks hit the existing rangeChars < minChars skip → clean exhausted failure); explicit-identity zero-baseline sessions keep today's optimistic path as documented.

Nits / observations (non-blocking)

  • Cosmetic: src/server.ts:2264 — if (result.fitsWindow) { is indented 12 spaces instead of 4.
  • Accepted tradeoff, stated for the record: low-density-prose anonymous forks (BPE ≥ ~4 chars/token — e.g. the codex-compact-e2e setup at 3.5× window in chars while fitting at 87%) will now over-fire preflight, including the nothing-compressible → structured 502 case for a giant single-message conversation that upstream might have accepted. That's the deliberate conservative direction (fail-fast with a diagnostic over guaranteed doom-loop into 上游长时间 prefill(首 token 前无输出)被 undici 默认 bodyTimeout=300s 掐断,UPSTREAM_TIMEOUT_MS=10min 配置形同虚设,大上下文请求必超时 #551 cutoffs) — fine by me, just noting the edge.
  • Transient baseline inflation: after a char-regime preflight, lastInputTokens briefly holds the char-based upper bound until the forwarded response's usage report lands. Self-heals (the test asserts correction to the reported value); interim requests can only trigger extra conservative rounds, never a raw past-window forward.

Housekeeping

#553)

Semantic merge onto current master. The fork-predate relax walk (#544/
#575), wire overhead (#470), image floors (#488), side-request isolation
(#388) and pi host-usage (#590) all landed after this PR's fork point
(09ef208). Resolution:
- preflight.ts: char regime (unknownBaseline / estimateCoreMessagesUpper /
  char-based splitChunks + netting / fitsWindow) grafted onto the multi-
  range walk; the upper bound also carries the image+wire floors.
- server.ts: effectiveTokenCount feeds all three prepare branches; the
  trigger's tokenCount uses upper+floors only for anonymous zero-baseline
  sessions; unknown-baseline keeps the loop's fitsWindow judgment, known
  keeps master's rebuilt-payload re-estimate (#330).
- unknown-baseline + nothing-compressible fails fast (PR semantics);
  known-baseline falls through to preflightCompress so the #330 relax
  path still runs.
- Tests adapted to current master/kernel: fork conversation reshaped for
  the kernel's minPressureBenefit (5k) band and main-request shape
  (max_tokens>200) since #388 now routes the old shape to passthrough.
@ranxianglei
ranxianglei force-pushed the 2026-09-05_fork-preflight-upper-bound branch from 8403c7c to 9630396 Compare September 6, 2026 16:40
@ranxianglei

Copy link
Copy Markdown
Owner Author

已在 9630396 将本 PR 语义合并到当前 master 并重写为线性单提交(修 #575 同样的 diff-爆炸问题)。全量 1146/1146 通过、typecheck 0 错、build 成功。语义合并要点:#553 fork 上界:char regime 移植到 #575 多 range walk 上,上界补 #470/#488 floors;unknownBaseline 用 fitsWindow、known 用 master 重建实测;unknown+无range 才提前 failFast,known 落穿保 #330 relax;测试按新 kernel 阈值(minPressureBenefit 5k)与 #388 side-request 形态调整。CI 绿后请人工合并。

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-05_fork-preflight-upper-bound (9630396)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-558

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

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr558.tgz
npm install -g package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 重写后的 9630396 已独立复核完毕,全部通过。

验证结果(均实际执行)

检查 结果
线性单提交 ✅ 父提交 = #591 merge(3f57ce4);master 此后仅前进 v0.1.88 release commit(#592,只动 package.json/lock)→ 与 PR 文件零重叠,干净合并
npm run typecheck ✅ 0 错
全量测试 ✅ 本环境 1145/1146 —— 唯一失败是 launcher.test.ts 的 resolveClientCommand(本沙箱 PATH 上有 codex 二进制导致解析出绝对路径),在干净 master 2b510af 上同样失败 → env-only,与 PR 无关;无 codex 环境下即你的 1146/1146
npm run build ✅ 成功
E2E_CHECK=1 零 token preflight ✅ 对构建产物通过
完整 ACP_TEST_E2E ⚠️ 沙箱内不可完成(需真实 upstream + codex warmup),维持 CI/手动跑标记

Diff 审计(对照合并说明逐条核实)

一点范围观察(非阻塞)

effectiveTokenCount 不只改了 preflight trigger,也改变了喂给 kernel processTurn 的 tokenCount——匿名 zero-baseline fork 现在会在窗口下就收到 nudge,而不是"usage 0%、直到 overflow 才触发"。这超出了原 PR 范围,但正对 #553 "什么都不触发"的症状,作用域与原 regime 完全一致(anonymousPrefixAffinity && lastInputTokens == 0),方向保守(只可能提前压缩,不会放行超窗 payload),且有新测试覆盖。记录在案即可。

CI 绿后请人工合并(按仓库规则我不执行 merge):#558

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.

1 participant