Repository navigation
[PR] fix: clear compress-failure accounting on rebase; exempt post-rebase stale-ref from the model breaker (#5984) - #5985
ranxianglei wants to merge 1 commit into
Conversation
📦 Built Package ArtifactBranch: Latest build — install this exact versionnpm install -g billion-context@0.0.8907-pr.5985.f8034aaThis exact version is immutable and reproducible. The rolling Recent builds (commit → version)
Download artifact
tar xzf billion-context-pr5985.tgz
npm install -g packageInstall instructions are refreshed on each push. |
ranxianglei
changed the title
(ai: vllm-qwen/qwen3.8-27b) 已按 [PR] 审核流程处理(不重做):拉取分支 结构化八问
行为变更披露(old → new)
测试有效性:新增 6 例含正负双向。另做一次"拔牙"验证——临时把豁免条件置假,test B 立即以 issue 原文 CI 状态(live 核过):pending —— 3 个 test shard in_progress,静态门禁 34 项全绿,无失败;我未能从 CI 拿到最终绿。本地定向 78 项(6 新 + 72 关联)+ typecheck 干净作为佐证,最终裁决以远程 CI 为准。 合入判定:代码/行为层面通过,建议 CI 转绿后人工合入。不建议 auto-merge(§7.4:改了模型可见回执内容 = wire/message-shape,且动了 session 状态元数据 → 人工档)。标签 bug/p1/scope/kernel 已齐且恰当,无需调整。本次未发现需另立 issue 的独立缺陷。 |
…stale-ref from the model breaker A host-native compaction rebase restarts the ref space but left two compress-failure accounting fields on the session — the #2146 consecutive- failure breaker streak and the #847 exact-spec repeat guard — which are only cleared on a successful compress. Two consequences: 1. A pre-rebase failure streak could carry across the generation break and arm mid-loop on the new refs. 2. The stale-ref failures that are inevitable while the model still carries its pre-rebase refs in its own persisted history armed the breaker after three; the armed hard-stop receipt then scrubs the acp_status recovery guidance — the only way out — leaving the model dead-ended ("circuit breaker says stop"). Fixes: - resetSessionCompression now clears both keys at the single choke point every rebase funnels through (reconcileNativeCompactionBoundary); the key names move to session.ts so both modules share one source of truth. - noteCompressLoopFailure treats a stale-ref failure inside a 30-min post-rebase grace window as proxy-caused (bili itself rebased) and does not advance the model-failure breaker, keeping the non-armed recovery hints visible until one success clears everything. Non-stale causes still count even right after a rebase; no threshold or breaker wording changes. Behavior change (old -> new): - rebase now clears the compress-failure streak + exact-spec guard (they previously survived the boundary). - post-rebase (<=30 min) stale-ref failures no longer advance the model breaker (they previously did, arming at 3). Closes #5984
c36686c to
436d391
Compare
(ai: vllm-qwen/qwen3.8-27b) Merge conflict resolved & pushed. Rebased the PR onto the current master head and force-pushed the result.
结构化八问
收工判定:冲突已解决并推送,分支现直接落在当前 master 头之上,合并状态应转干净。镜像 check 缓存此刻仍是旧 |
[ework] CI fix verified green ✓ |
ranxianglei
left a comment
ranxianglei
left a comment
There was a problem hiding this comment.
Note: posted as a review comment because GitHub blocks self-approval of one's own PR; the verdict below stands.
(ai: vllm-qwen/qwen3.8-27b)
Verdict: APPROVE — clean, correct, well-scoped root-cause fix. Independently re-derived from the diff (not from the issue's own analysis).
What I checked
- Direction (issue #5984). After a host-native compaction rebase the ref space restarts at m00001, but the status line mixed a pre-rebase
compressibleRangesspan with the post-rebasehighest, so the model copied a range that can never resolve → 3 guaranteedunknownfailures → breaker arms → the armed receipt scrubs the veryacp_statusrecovery guidance that is the only exit. The PR attacks that mechanism, not the symptom. - Fix A — rebase clears failure accounting.
resetSessionCompression(src/session.ts:1731) now deletes bothCOMPRESS_LOOP_KEY(compressFailStreak, #2146 streak) andFAIL_STREAK_KEY(compressFailKeys, #847 guard). VerifiedresetSessionCompressionhas exactly ONE src caller —reconcileNativeCompactionBoundary(src/session.ts:1808, call at :1820) — reached only from the prepare/rebase paths (anthropic/google/openai/responses) and gated onnativeCompactionBoundary.pendingRebase === true. So the clear fires ONLY on a genuine rebase, never on an unrelated reset. Also confirmed these are the ONLY two compress-failure accounting fields on session metadata (completeness — no third survivor). - Fix B — post-rebase stale-ref ≠ model failure.
noteCompressLoopFailure(src/stream.ts:332) derivesfreshCausefirst; when it starts with"stale-ref"ANDwithinRebaseGrace(≤30 min sincenativeCompactionAt) it logs and returns""WITHOUT advancing the streak. Verified reachability:compressFailureCause(src/stream.ts:441) returns"stale-ref — …"for the exactevery ref is unknown to this sessiontext. VerifiednativeCompactionAthas a single writer (session.ts:1796), sowithinRebaseGracecannot false-trigger from any other path. Verified the receipt (src/stream.ts:1522-1524) selects the full recovery cluster (rebaseNote+currentRefsSnapshot+ …) precisely whenloopGuard === ""— returning""is exactly what keeps the exit visible instead of the armed "do not poll acp_status" paragraph. - No silent regression from skipping the streak write. acp_status's
compressLastFailureCause(src/acp-status.ts:273) feeds only the armed-breaker annotation block; a pure stale-ref-during-grace never arms, so dropping the write loses nothing. Non-stale causes (parse / covered / content-changed) still advance normally inside the grace window. HoistingfreshCausebefore the streak read is behavior-preserving for the non-exempted path —compressFailureCauseis pure over(errs, metadata[METADATA_DRIFT_ESCALATED]), independent of the streak. - Tests (tests/issue5984-rebase-breaker.test.ts, 6 cases). Real functions exercised (
reconcileNativeCompactionBoundary,noteCompressLoopFailure); positive+negative per gate — A: rebase clears both keys / no-boundary leaves them untouched; B: grace exempts / no-rebase still arms-at-3 / grace-elapsed arms-at-3 / non-stale arms-at-3. Assertions have teeth (each fails if its mechanism regresses); not tautological. - Conventions & hygiene. No
kernel/orpi-subagents/touch (so no kernel authz gate applies).REBASE_STALE_GRACE_MS = 30minis hardcoded like the existingCOMPRESS_LOOP_*constants (config-surface discipline — no new knob invented).ctx.logused, notconsole.error; noas any; comments match this file's established why-with-issue-ref style. Diff is tight (+180/−15, every line on-purpose, no lockfile/whitespace churn). Behavior drift disclosed in the PR body (old→new ×2). - CI. Mirror cache for head
436d391: 15 success / 15 skipped / 0 failed — green.
One considered trade-off (not a defect, no action): compressFailureCause lumps rebase-killed refs, typos, and pruned gaps under one stale-ref label, so a genuine typo'd ref within 30 min of a rebase is also exempted from the breaker. That is the intended, deliberately-bounded cost of the fix — right after a rebase the model's persisted history is dominated by dead pre-rebase refs, so attribution to the proxy is overwhelmingly correct; exposure is capped at the 30-min window and non-stale causes still arm immediately. Acknowledged and acceptable as designed.
No small findings to fix on the spot; no decision items.
结构化八问（供人工复核）
- 是什么问题: 宿主原生压缩触发 rebase 后，压缩失败计数没有清零；模型沿用旧 ref 必然失败、连续三次熔断；而熔断回执又把唯一能救场的 acp_status 恢复指引抹掉，模型本轮彻底放弃压缩（死锁）。
- 什么原因导致的: rebase 重启了 ref 空间（resetSessionCompression），但两个失败会计字段（#2146 streak / #847 guard）历史上只在「成功压缩」时清，跨代残留；同时把「ref 全部 unknown」一律记成模型失败，未区分「是 bili 自己 rebase 弄死的旧 ref」。
- 影响范围与严重程度: 仅「宿主原生压缩 + rebase」路径触发（plugin native 模式，如 dsh/codex/omp 的 /compact）；命中后本轮压缩完全放弃。属体验级死锁，非数据丢失——特定条件下有问题。
- 怎么修复的: ① 在 rebase 唯一咽喉点 resetSessionCompression 里删掉两个会计键（它们描述的 ref 空间已不存在）；② rebase 后 30 分钟宽限期内，stale-ref 失败判为代理自身所致、不推进模型断路器，返回空串让恢复提示簇（rebaseNote + currentRefsSnapshot + acp_status 指引）保持可见，直到一次成功清空全部。
- 是否有回归问题: 逐项审过既有行为——非 stale 原因（parse/covered/content-changed）在宽限期内照常计数；无 rebase / 宽限过期仍按 3 次熔断；无 pending boundary 不做任何清理；freshCause 提前求值对非豁免路径纯函数无副作用；跳过写 streak 不影响 acp_status（其 lastCause 只在 armed 分支使用，纯 stale-ref 宽限内不会 armed）。高危模式过一遍：无语义漂移、无 fail-fast 变吞错、阈值/时序/输出格式均未变（COMPRESS_LOOP_THRESHOLD=3 / DECAY=10min 原样保留，只新增独立的 30min 宽限常量）。
- 是否破坏缓存: 不涉及——改动仅在失败回执文案与元数据会计，不动 KV/prefix 缓存、重试游标、去重键。
- 是否改了设计方向: 不涉及架构转向；只是在既有断路器机制上加一条「代理自因」归因旁路 + rebase 时清会计，边界与数据流不变。
- 是否引入了新的配置: 无新 env/config/CLI —— REBASE_STALE_GRACE_MS（30min）为硬编码常量，与既有 COMPRESS_LOOP_* 同风格，符合配置面纪律。
判定：可合。 根因修复、测试有牙、diff 干净、无 kernel 触碰、CI 绿。
[bot] 🏷 Reviewed GitHub PR #5985 → verdict: APPROVE. End-to-end review, independently re-derived from the diff (not copied from issue #5984): read the full diff (src/session.ts, src/stream.ts, new test file), traced both fixes against their real code paths, checked test teeth, the regression surface, house conventions, and CI.
Verdict posted as a formal review comment on the PR — GitHub blocks self-approval of one's own PR ( No small findings to fix on the spot; no 🔶 需要决策 items. 结构化八问（中文，供人工复核）
判定：可合。 根因修复、测试有牙、diff 干净、无 kernel 触碰、CI 绿。 |
What & why
After a host-native compaction rebase, the model is repeatedly steered into a compress call that must fail, and after three such failures the circuit breaker arms a hard stop that then hides the only recovery path — the model dead-ends ("circuit breaker says stop") and gives up compressing for the round.
Two defects on the rebase path, both confirmed by reading the code:
Failure accounting survives the rebase. A native-compaction rebase restarts the ref space (
resetSessionCompressionresets state and pins refs at a floor), but it did not clear the two compress-failure accounting fields that live on session metadata — the#2146consecutive-failure breaker streak (compressFailStreak) and the#847exact-spec repeat guard (compressFailKeys). Those were cleared only on a successful compress. So a pre-rebase streak could carry across the generation break and arm mid-loop on the new refs.The post-rebase stale-ref storm arms a dead-end. While the model still carries its pre-rebase refs in its own persisted history, its compress calls resolve to "every ref is unknown". Each advanced the model-failure breaker; at three, the armed receipt drops the rebase note, scrubs the
acp_statusrecovery guidance, and orders "do not poll acp_status" — i.e. it removes the exact step that would let the model out. This is the reported "circuit breaker says stop".Note: within a single turn the status line's
highestandcompressibleRangescome from the sameprocessTurnresult (internally consistent), so this is not a same-turn mix-up — it's temporal: the model acts on hints it retained from its own history across the rebase.Fixes
resetSessionCompressionnow deletes both keys; it runs insidereconcileNativeCompactionBoundary, the single choke point every rebase funnels through (openai/codex/responses/google/anthropic prepare paths). The key names move tosession.tsso both modules share one source of truth (stream already imports session; no new import cycle).noteCompressLoopFailuretreats astale-reffailure inside a 30-min post-rebase grace window as caused by bili's own rebase and does NOT advance the model-failure breaker — returning "" keeps the non-armed recovery cluster (rebase note + current-refs snapshot + kernelacp_statusguidance) visible until one success clears everything. Non-stale causes (parse / covered-by-block / content-changed) still count even right after a rebase. No threshold or breaker wording changes.Behavior change (old → new)
Tests
New
tests/issue5984-rebase-breaker.test.ts(6 cases): rebase clears both keys; no-boundary guard leaves them untouched; post-rebase stale-ref never arms; plus three guards pinning unchanged behavior (no-rebase still arms at 3, grace elapsed arms at 3, non-stale causes still arm). All pass. Existing breaker/rebase suites (issue2146,issue2592,issue2432-breaker-awareness,issue4812,issue847,issue3126,issue2432-dsh-local-compaction,issue2117,issue1933,issue1502,issue1820,compress-observability) all green locally. Full-suite verdict = remote CI.Typecheck clean. No
kernel//pi-subagents/touch.Closes #5984