Repository navigation
[PR] Roll back the incremental canonical when the upstream forward fails | fix(lane): forward 失败后回滚增量 canonical,客户端重试同一 delta 不再被迫全量 resync - #4881
Conversation
ranxianglei
changed the title
📦 Built Package ArtifactBranch: Latest build — install this exact versionnpm install -g billion-context@0.0.9176-pr.4881.f50444bThis exact version is immutable and reproducible. The rolling Recent builds (commit → version)
Download artifact
tar xzf billion-context-pr4881.tgz
npm install -g packageInstall instructions are refreshed on each push. |
|
(ai: vllm-qwen/qwen3.8-27b) Behind-alert resolved: no new master commits since this morning's sync (master tip still 23158db, branch head 66ff3e0 already contains it — mergeable=true, 0 behind); the repeated alerts came from a stale local mirror ref of this branch, now fast-forwarded to 66ff3e0 to match GitHub. |
(ai: vllm-qwen/qwen3.8-27b) Merge-guard task closed out: (1) synced origin/master into this branch and pushed 66ff3e0 — clean merge, zero conflicts; (2) the repeated "behind" alerts turned out to be a stale local mirror ref of this branch (frozen at pre-sync 226c25c), which I fast-forwarded to 66ff3e0 after verifying its attached worktree was clean and idle. Final state: head 66ff3e0 contains all of master tip 23158db, mergeable=true, 0 behind, CI green on all guard jobs with test lanes completing. Nothing pending from me — ready for human merge once CI is fully green. |
…already flushed) Review supplement for #4881: the rollback gate in handleIncrementalTurn only fires while the ack cannot have reached the client, but the new suite only asserted the roll-back side. Add the negative-direction case: a forward that throws AFTER the 2xx head (carrying x-bili-applied-seq) was flushed must NOT roll back — the client already advanced its pointer, and a rollback would be reverse drift whose next delta 409s.
ranxianglei
added
bug
(ai: vllm-qwen/qwen3.8-27b) 查重: 不重复 —— 检索 open+closed 的 issue/PR(canonical rollback / forward failure / 409 resync 家族),无同机制条目;本 PR 即 #4867 的唯一修复。 标签: bug + dsh/omp/opencode/pi(全部插件增量车道)+ scope/wiring(请求管道)+ p2(可用但每次上游抖动白付一次全量重发)。 独立审核结论我没有照抄 issue 侧分析,重新读了 diff 与相关代码路径,并本地定向验证。 快照完整性(修法核心):对照 门禁条件 vs 客户端契约: wrap-up 一致性:非 2xx 完成路径回滚后, 回归清点(逐项):成功路径行为不变;异常路径中 2xx 头已发出的场景前后都不回滚(不变);ack 头契约(#3344)不变;turn lock/pendingPtrWork 生命周期不变(finally 在异常路径完整释放);probe 端点在失败后报告回合前值(更诚实,且 adoption 另有 boundary hash 门,不会被污染)。未发现回归。 本地验证(定向):新测试文件 4/4 绿;既有 8 个增量车道套件(incremental-lane / ack-pull / client-recovery / issue3304 / issue3343 / issue3433 / omp-native-incremental / opencode-incremental)58/58 绿;typecheck 干净。上一 head(66ff3e0,含 master 合入)CI 全绿:test 全部分片(ubuntu/windows × node22/24)、static-gates、kernel-guard/lane-guard/config-surface-guard/version-guard/docs-sync/lock-sync/build-artifact 均 success。 Review 发现一处小问题 → 已直接修掉回滚门禁只测了正向(seed/delta/resync 失败→回滚→重试落地;抛异常→回滚),负向没有钉住:「2xx 头已 flush 后抛异常 → 不回滚」这条分支无任何测试,违反仓库 §3 Gate Test Discipline(#3617:每个 gate 必须双向断言)。已在 PR 分支补上该用例(forward 在 writeHead(200) 后抛错 → probe 断言 canonical 保持推进长度 → 已采纳 ack 的客户端从 seq 2 继续无需 resync),独立标注 commit 3b8a7cd,本地 5/5 + typecheck 干净,新 head CI 排队中。 结构化八问
判定:方向正确、实现完整、回归面受控、双向门禁现已齐备;唯一发现(负向测试缺失)已在 PR 分支修掉并推送。等 3b8a7cd 的 CI 转绿即可合并。 |
(ai: vllm-qwen/qwen3.8-27b) CI 确认: 新 head 3b8a7cd(补负向门禁测试的 commit)的 CI 已全部通过——完整翻页核对全部 check runs(129 条、42 个不同检查项,按检查名取最新一次):28 success + 14 skipped,零红零挂起。期间出现的 cancelled 条目是同一 push 触发重复 run 被并发策略取消的旧副本,每个检查名的最新一次均为绿。PR 可以合并了。 |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Final merge-guard state: after the review lane's follow-up push, branch head is now 3b8a7cd (adds the missing negative-direction rollback-gate test) — verified locally it still fully contains master tip 23158db (0 behind, 3 ahead), so no further sync is needed from me. My work on this PR is complete: master sync merged+pushed (66ff3e0), stale local mirror ref converged, outcome comments posted. Handing off to the review lane + human merge once CI on 3b8a7cd is green. No further action expected from me on this PR. |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Merge-guard sync #2: master had genuinely moved since the morning sync (23158db → 0cf53c8, +62 commits across the dsh/WS/anchor/catalog lanes) and none of them touch this PR's two files, so origin/master merged into the branch cleanly with zero conflicts — pushed as d2ca83c (now contains all of master: 0 behind, 4 ahead); PR mergeable=true, CI re-running on the new head. Also re-converged the stale local mirror ref of this branch (it had lagged at 66ff3e0 after yesterday's external push) so the merge guard's local view matches GitHub again. |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Behind-alert #4 was genuine — synced: master had moved 0cf53c8 → e4cc685 (+5: hermes ACP session binding #4856/#4855 + docs/goldens, zero overlap with this PR's files); merged cleanly into the branch and pushed as 1421a8a (zero conflicts). Verified via API: head 1421a8a on base e4cc685, mergeable=true, checks green so far (20 success / gated lanes skipped / 2 queued); local mirror ref re-converged in step. |
…back snapshot Merging master brought #5155's per-turn render cache into Canonical (renderPrefix + marksVersion). applyTurn bumps marksVersion and nulls renderPrefix on resync, and laneItemsForTurn refreshes the prefix over the NEW history before the forward lands. The #4867 pre-turn snapshot predates those fields, so a rolled-back turn left a prefix cache rendered from the rejected branch: if its len/marksVersion gates happened to match a later turn's cut, laneItemsForTurn would splice rejected-branch bytes into an accepted body. Snapshot and restore both fields.
(ai: vllm-qwen/qwen3.8-27b) Both new "conflict" triggers verified stale — no action taken: head |
…cold-tail storage-mode pin Clears the inherited #5140 cold-restored-parent raw-tail CI red on this branch. Net delta vs f679c7d is exactly tests/plugin-fork.test.ts (+9/-1): pin BILI_STORAGE_UNIFIED="full" for the raw-tail proof (the legacy runner baseline has no unified rows so forkSnapshot fails closed with 409 SNAPSHOT_UNAVAILABLE) and restore the ambient value in finally. Docs/config registrations from ci-fixes were already present via master@44067a5bc. No source changes.
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) CI 修复(auto-fix 车道):本轮 6 个红分片全部是继承自 master 的 #5140 冷尾 fork flake(叠加 Windows 环境 flake 家族),非本 PR 代码。#5140 现已在 ci-fixes 定位修复(一个测试存储模式钉),我已将该修复合入本分支并推送。 证据(逐条可核)
#5140 根因(已在 ci-fixes 定位修复):该用例证明 forkSnapshot 能从 UNIFIED 索引重建「非保留」父会话的冷恢复 raw tail,但用例自身没钉存储模式;CI 全局基线 本次动作:本分支(f679c7d,含 master@44067a5bc)缺这个测试钉,故仍撞 #5140。已按 CI 协调纪律 结构化八问
收工判定:本 PR 内容就绪、可合;本轮已把继承的 #5140 修复合入并推送( |
…ward fails applyTurn advanced the warm canonical BEFORE the assembled request was forwarded upstream; when that forward failed (thrown transport error, or a completed non-2xx), the canonical stayed ahead while the client received no ack (x-bili-applied-seq is stamped on 2xx only, #3344) — so the client's plain retry of the SAME delta hit a 409 seq-mismatch and paid a full resync. Every upstream blip became one full-history resend. Snapshot the pre-turn state (msgs ref, CLONED marks map, system/boundary scalars) right before applyTurn and restore it when the forward does not land: - thrown forward: roll back only while the ack cannot have reached the client (a flushed 2xx head means the client already advanced on it); - completed non-2xx / headless end: roll back — no ack went out. Append turns truncate back to the pre-turn length; resync turns restore the replaced prefix whole (generalizing truncation). Rows of the rejected branch stay in the append-only table — the existing 409->resync path owns them. On the resolved-non-2xx path the wrap-up chain now records pointers onto the restored old-branch rows, keeping the warm and cold views consistent. Regression tests (all red before the fix, green after): tests/issue4867-incremental-forward-failure-rollback.test.ts — seed/delta/resync retry-after-failure over HTTP plus the direct-call thrown-forward case. (cherry picked from commit 226c25c)
…already flushed) Review supplement for #4881: the rollback gate in handleIncrementalTurn only fires while the ack cannot have reached the client, but the new suite only asserted the roll-back side. Add the negative-direction case: a forward that throws AFTER the 2xx head (carrying x-bili-applied-seq) was flushed must NOT roll back — the client already advanced its pointer, and a rollback would be reverse drift whose next delta 409s. (cherry picked from commit 3b8a7cd)
…back snapshot Merging master brought #5155's per-turn render cache into Canonical (renderPrefix + marksVersion). applyTurn bumps marksVersion and nulls renderPrefix on resync, and laneItemsForTurn refreshes the prefix over the NEW history before the forward lands. The #4867 pre-turn snapshot predates those fields, so a rolled-back turn left a prefix cache rendered from the rejected branch: if its len/marksVersion gates happened to match a later turn's cut, laneItemsForTurn would splice rejected-branch bytes into an accepted body. Snapshot and restore both fields. (cherry picked from commit d553129)
cec85bb to
6787c75
Compare
Closes #4867
问题
增量回合并入服务端 canonical(canonical 前进)之后,转发上游失败(抛异常,或完成但非 2xx)时 canonical 不回滚:客户端拿不到 ack 头(#3344 只在 2xx 打),下一回合用同一 delta 重试 →
sinceSeq !== serverSeq→ 409 seq-mismatch → 被迫全量 resync。每次上游 5xx/断流都白付一次整段历史重发。修法(
src/incremental-lane.ts)applyTurn之前对回合前状态做一轮快照:msgs保留旧数组引用(applyTurn 赋的是新数组)、marks克隆(applyTurn 原地改 Map)、外加 system/boundary 标量;边界说明:2xx 头已发出后流中断的场景,ack 视为已送达、不回滚——该场景本就不产生 409 循环,不在本次范围。
行为变更披露(旧值 → 新值)
验证
tests/issue4867-incremental-forward-failure-rollback.test.ts(4 例:seed/delta/resync 失败后重试 + direct-call forward 抛异常):打补丁前全部红(精确复现 409 / canonical 停在推进后长度),打补丁后全部绿;