Repository navigation
Conversation
0b6e9aa to
2b96508
Compare
CI: the three red shards are a pre-existing #5140 flake, not this changeThe failing subtest is identical in all three: Evidence that it predates this PR:
Same sha Locally, on this branch: None of this PR's files are involved: the change is entirely in the tag-echo filter ( All other checks are green (35 success across the same 24-job matrix, incl. @ranxianglei — this looks like the #5140 cold-raw-tail test needs a retry/robustness pass rather than a change in this PR. Would you rather I (a) leave it and you re-run, or (b) take the #5140 flake as a separate issue?
|
Definitive: the red shards are the #5140 flake, and master HEAD is red with it RIGHT NOWI dug into the whole history instead of just my own run. The failing subtest is byte-identical everywhere: Failure census across master commits (via
|
| sha | role | failing checks | failing subtest |
|---|---|---|---|
09a5b5b7 |
master HEAD | 8 | #5140 |
083594fa |
master history | 3 | #5140 (same line) |
2b965084 |
this PR | 3 | #5140 (same line) |
3a5d5326 … 11 earlier |
master history | 0 | — |
So the flake appears exactly when #5144 (cold raw-tail fork, merged 10-29) landed on master: 083594fa is the first master commit that carries it, and it also hits green on re-runs of the same sha (09a5b5b7 has both 8 failures and several fully-green runs: 40960500370, 40960457155, 40960356832, 40960328690, 40960322230).
Why this PR cannot be the cause
- It touches 5 files:
src/loop/tag-echo-filter.ts,src/degenerate-turn.ts,src/plugin.ts,src/loop/core.ts,tests/issue5175-drift-name-leak.test.ts. tests/plugin-fork.test.ts— not touched.- The
#5140path (src/fork.ts/reconstructRawTailForForkinsrc/plugin.ts) — the onlysrc/plugin.tsedits are the 3endsWithDraftClose→isDraftDumpcall sites (lines 2663, 4074) plus the import; no fork/snapshot code changed. - Locally:
tests/plugin-fork.test.ts→ 54 pass / 0 fail, 8 consecutive runs on this branch.
All other 34 checks are green on the same matrix, including static-gates (ubuntu-24) / (windows-22), kernel-guard, unused-exports, mysql-contract, and every test shard not carrying plugin-fork.
@ranxianglei — options:
- Re-run CI — you have admin;
gh run rerun <run> --failedon run40968352705.--failedre-runs only the 4 red shards (~7 min). This is the cheapest and proves the post-flake state. - Treat [Bug][fork] Cold restore cannot use an intact unified raw tail for a non-retained parent (#4266 item 4) #5140's test as a flake issue — I'd rather file a separate
[Bug][CI] flaky: cold-restored non-retained parent … 409 !== 200 (intermittent)and let this PR pass on a green re-run. - I keep adding empty commits to retrigger, which pollutes the history — not something I want to do to your repo.
My preference is 1 (re-run), with 2 as the durable fix.
🤖 Powered by ework · deepseek-v41-flash
(ai: lab-v41/deepseek-v41-flash)
Follow-up pushed: the SSE lane now adjudicates too (
|
| Hook | Why a shape and not a name |
|---|---|
DRIFT_TAIL |
holds an unresolved drifted opening at the buffer tail so its pair can resolve in one buffer |
dp in the match race |
a COMPLETE drifted pair is one dead unit; drop it whole exactly as whole-text does |
| ``stripDriftResidue` on each released span | the same pure rules as whole-text, so the two lanes cannot disagree at rest |
All three are gated off every canonical NAME and off real HTML tag names. The HTML list is the one legitimate enumeration here: it is a closed set, so account/div/table/td/title are never touched (#2190 round 3 contract, verified).
Also widened the gate itself (containsRenderTagText). The gate decides whether the stripper is even called, so an enumeration there fails identically to an enumeration in the pattern — acpt returned false and the production path never invoked stripAcpTags at all.
Corpus measurement (8225 real tag-shaped fields, 4..12 char chunks)
Streaming deviations from the whole-text verdict:
| deviations | |
|---|---|
master |
45 |
| this PR | 33 |
| fixed | 12 |
| newly broken | 0 |
Verified end to end: 9 echo-filter suites 204 pass / 0 fail; tsc --noEmit clean; build + check-dist-import-annotations/check-dist-external OK.
Known residual (pinned in the tests, not silently accepted)
Two shapes remain and cannot be fixed from this lane:
- The reported paragraph opens with
acpt>— an orphan half whose\x3cwas already lost upstream (the GUI rendered the bare tag as HTML). It cannot start a tag, so no matcher can hold it. - Trailing orphan closes that whole-text strips by line — the streaming lane has no line state, and adding it collides with the
#2190balanced-markup contract (attempted, reverted).
Both carry no citation, and both are byte-identical at every chunk size — #3198 chunk-split determinism holds even in the residual. The tests assert that invariant: ref count must match and removing drift-shaped fragments must reproduce the whole-text verdict exactly.
CI
The 4 red shards on the previous head are the pre-existing #5144 cold-raw-tail flake (409 !== 200, plugin-fork.test.ts), reproduced on master HEAD itself and reported per policy in #4599. This head is re-running.
2b3e93b to
a473a28
Compare
Rebased onto master Re-verified on the rebased tree: 204 pass / 0 fail across the 9 echo-filter suites, |
|
| suite | on my head | on pristine master |
|---|---|---|
fix-1439-responses-json-triggers |
3/4 (1 red) | 4/4 green |
issue4323-drift-escalation |
5/6 | 5/6 (red with identical signature) |
So the first was mine, the second is inherited.
The regression
<acp_status></acct_status> is an "unknown open name + empty body" — exactly the shape my stripEmptyPairs rule drops. But it is not a drift name: it is one of the four tags the kernel publishes as a closed set (ACP_TEXT_OPEN / ACP_STATUS_OPEN / ACP_SEARCH_OPEN / ACP_DECOMPRESS_OPEN). acp_status is a read-only trigger that the non-streaming Responses loop executes via extractResponsesTextTriggers, so my rule ate the trigger before the loop could run it.
The fix is a second closed-set exclusion beside the HTML one, applied to every drift rule (head, empty-pair, mismatch-pair) and to both per-match Set tests.
This does not contradict the PR's thesis. The distinction is exactly which sets are closed:
| set | closed? | enumerable? |
|---|---|---|
| real HTML tag names | yes | yes |
| kernel protocol tags | yes | yes |
| the model's drifted render names | no | no |
Red → green, both directions
With the exclusion disabled the new test fails; with it enabled it passes, whole-text and at chunk sizes 1 and 5:
not ok 5 - #5175 A: the kernel's OWN protocol tags are never read as drift # disabled
# pass 16 # fail 0 # enabled
Surface now
10 suites, 188 pass / 0 fail (issue5175-… grew to 16 by adding this test), tsc --noEmit clean, build + dist-import-annotations + dist-external OK.
I also opened a throwaway worktree at upstream/master to run baselines side by side; that is how I separated the inherited failure from mine instead of asserting it.
|
| suite | this PR before 8464062e |
pristine upstream/master |
|---|---|---|
plugin-passthrough-chat-tag-strip |
16/20 (4 red) | 20/20 green |
Cause
filterFor routes thinking / reasoning_content / reasoning to an identity filter, because any rewrite desyncs a signature and bricks replay (#1960, #2229 unified invariant: "the thinking channel is NEVER rewritten, only the visible text channel is").
My drift adjudication runs on the released span, and I wired it into pushField for every field. So it reintroduced exactly the rewrite that invariant forbids, through the back door. All four failures were on reasoning bytes expected to ride verbatim.
Fix
An isVerbatimField predicate placed beside filterFor, so the identity-filter decision and the "never post-process" decision sit in one place and cannot drift apart:
const isVerbatimField = (field: string): boolean =>
field === "thinking" || (protocol === "openai" && (field === "reasoning_content" || field === "reasoning"));
...
const released = s.filter.push(text);
const clean = isVerbatimField(field) ? released : stripDriftResidue(released);The visible text channel keeps the adjudication; the thinking channel is untouched.
Red → green, both directions
Remove only the guard → 16/20 again. Restore it → 20/20.
Surface
All 27 dependent suites: 0 red. tsc --noEmit clean, build + both dist gates OK.
Honest note on process
Both regressions in this PR were mine, and both were found by widening the run surface rather than by reviewing the diff. The lesson recorded: after touching tag-echo-filter.ts, enumerate importers instead of picking suites by name. I have added a 17th test to issue5175-… documenting why the field split exists, so the next person changing pushField sees the invariant without re-deriving it.
Incoming red shards are inherited from master — measured with a clean worktree
The That is the same fixed failure set #5195 already attributed to What I can and cannot claim
Both self-inflicted regressions, both fixed, both pinned
Both were found by enumerating every importer of the two touched modules ( Added a characterization test for the scope note I wrote earlierI previously claimed in this thread that the
Suite is now 18 pass / 0 fail. Also checked: #5192 (render-tag suffix leak)
So #5192 is not a variant of the fragment I fix here, and this PR neither fixes nor worsens it — it needs the tag to arrive with its head already missing, which is an upstream emit problem, not a stripper problem. Flagging it so nobody assumes this PR closed it. |
|
| commit | test ( checks |
pass | fail | cancelled |
|---|---|---|---|---|
4bb2bc1e |
1 | 0 | 0 | 1 |
a8c907dd (#5169) |
11 | 2 | 9 | 0 |
4bb2bc1e has exactly one shard check and it is cancelled — its shards never ran, so "0 failures" there means "did not execute", not "passed". Scanning the last 30 master commits for a commit whose shards actually completed green, the last one is 91800538; the first genuinely red-with-completed-shards commit is 083594fa — which is before #5169 in the history.
So the #5169 attribution is unsupported on the current evidence, and reverting it would not clear the red set. The discriminating data is: which of these failures still reproduce on a clean local checkout, which is cheap to get (node --import tsx --test <suite> against a worktree at the suspect sha) and is how I found this one.
(ai: vllm-qwen/qwen3.8-27b) CI 红了 4 个 test shard。我逐条拉了这 4 个 job 的原始日志核对(不依赖任何转述)。结论:全部是继承性 flake,不是本 PR 的代码问题,本分支无需任何改动。 四个红 shard 的真实失败(每片恰好一条 not-ok):
结构化八问
判定:本 PR 代码本身健康、其自有测试 CI 全绿;4 个红 shard 均为继承性 flake,已在 #4599 登记(见我对该 issue 的状态行)。建议 owner/admin 重跑 run 40968352705 的 4 个失败 job 即可转绿;持久修复由哨兵在 ci-fixes 收敛后并入。我不合 PR、不自触发重跑(避免重复触发风暴),交回哨兵/owner 处置。 |
The test that mattered was missing, and that is on me. This PR had 18 passing tests while the reported paragraph still reached the The SSE fast path releases a delta untouched when all four Red baseline, driving the real pipeline ( Green with the fix ( Two things I got wrong on the way here, both worth recording:
Suite: 19 pass / 0 fail. All 11 suites importing the filter: 0 fails |
CI status: every red shard on this head is also red on
Per-suite verdicts, each established on a pristine
None of these suites import What I did fix, both pre-existing and both reproduced on the pristine worktree first:
Both are the same shape — a diagnostic message was reworded and its assertion was not — which is invisible in a diff and only shows up when you run the suite. Fixing them here keeps this PR from inheriting two red shards it did not cause. Green on my surface: 19 pass / 0 fail in |
2e33511 to
5225625
Compare
Pushed
The product invariant is fine. The assertion was measuring the wrong thing. Instrumenting the exact same sequence: The reset lands at exactly So I assert the minimum observed during the loop rather than the end state. That is the contract — "the configured value reached the pragma" — and it still discriminates: with the 8 MiB default the floor is
|
Pushed
So on Windows the warning read
Combined run of all four touched suites: 37 pass / 0 fail. What is still red, and why I did not touch it:
|
3439248 to
fb9f245
Compare
CI: the red shards are inherited — the same shard set is red on master before my branch existedHead Measured shard sets (not asserted —
So three of my four red shards are red on The suites behind them:
I cannot re-run these myself ( |
…t envelopes Two thinking-leak shapes reached the client verbatim. ## Defect A — drifted open-tag names Every open-side matcher keys on ACP_NAME_ALT (buildAcplikeName, :55-85). That enum allows the three-letter cores plus one INSERTION at every position, and three attested drifts. It has no tail-append arm, so `acpt` / `acct` / `accent` / `acad` / `actp` / `acme` / `acc` match nothing, and three gates miss the span together: RENDER_TAG_DETECT is enum-keyed, BROAD_TAG_HEAD_TAIL only matches a tail BEGINNING with `<` (an already-formed `<acpt m17138>` never does), and PARTIAL_TAIL is a suffix probe an interior tag never satisfies. stripAcpTags therefore returned `<acpt m17138></acpt>` unchanged. Enumerating names cannot close this. The CLOSE side already learned the lesson: DEAD_REF_CLOSE keys on an open ASCII class because "the close-name drift census is open (ap/ck/ak/fullwidth …)" (ranxianglei#2190). The open side never got the same treatment. A blanket any-name rule is wrong: ranxianglei#2190 round 3 pins genuine markup byte-exact (`<account>m00375</account>`, `<acmap>`, `<acstep>`, `<acpipe>`). The two signatures added here are render-tag imitation and nothing else — the NAME is the part that drifts, so the name stays open: DRIFT_PAIR <N mNNNNN(–mMMMMM)></N> ref in the ATTRIBUTE slot, body empty; structurally impossible as prose DRIFT_ATTR <N … tokens=… >refs</N> the emitter's own attribute A matched pair licenses removing that name's orphan halves (a lone `</acct>` is indistinguishable from prose on its own, so the pair is the proof — ranxianglei#1760's rule reused). Residue LINES are adjudicated whole: a line whose tokens are all fragments or bare refs and that contains an orphan is the line the imitation is written on. Prose contains words, and a word is neither, so ordinary sentences never match. ## Defect B — mid-turn draft envelope DRAFT_CLOSE_TAIL is `$`-anchored, so it only recognised a draft whose `</analysis>` was the last thing in the text. The reported turn put it MID-text with the whole `<summary>` body after it, so the anchor could never match and the dump released as prose. DRAFT_ENVELOPE covers that form; the `$` arm is kept so the historical tail shape still detects. `isDraftDump` replaces `endsWithDraftClose` at all three call sites (plugin.ts:2663, plugin.ts:4074, loop/core.ts:1122). ## Out of scope The SSE streaming path is unchanged. Its state machine owns a tag-structure hold (held / preHeld / swallowed) unrelated to a line-scoped adjudication; wiring the drift rule into it truncated `<account>` bodies in the ranxianglei#2190 round-3 suite. Recorded as a follow-up on the issue. Refs ranxianglei#5175, ranxianglei#2190, ranxianglei#1760, ranxianglei#1039, ranxianglei#2303, ranxianglei#2612, ranxianglei#2348, ranxianglei#4107.
…st drift tags The state machine's matchers are all enumeration-scoped (`NAME` feeds PAIRED/LONE_OPEN/LONE_CLOSE/PARTIAL_TAIL), so a drifted render name (`acpt`/`acct`/`accent`...) never entered it: its bytes were released one push at a time, and the whole-text pass only caught them long after the client had rendered the frames. That is the reported leak. Three additions, each keyed to a SHAPE rather than a name: - `DRIFT_TAIL` holds an unresolved drifted opening at the buffer tail. Without it PARTIAL_TAIL's trailing partial-ref arm claims the `m` of `<acct ...>m00257` and the opening ahead of it is released as prose, so the pair can never resolve inside one buffer. Gated off every canonical name and off real HTML tag names, so anything the machine already understands keeps its exact behaviour. - `dp` puts a COMPLETE drifted pair into the match race, so it dies whole exactly as whole-text drops it — no swallow session needed, nothing left to wait for. - `stripDriftResidue` runs on each released span (wired into the plugin's pushField), using the same pure rules as the whole-text pass so the two lanes cannot disagree on a span at rest. Corpus check (8225 real tag-shaped fields, 4..12 char chunks): streaming deviations from whole-text drop from 45 to 33 — 12 fixed, 0 newly broken. Remaining leftovers are orphan halves whose `\x3c` was already lost upstream plus line-licensed closes that need line state; they carry no citation and are byte-identical at every chunk size (ranxianglei#3198). Also widens `containsRenderTagText` by shape: the gate decides whether the stripper is CALLED at all, so an enumeration there failed the same way the pattern did.
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Review @
|
…ndow refusal, ranxianglei#5969 LiteLLM overflow counts Advances the base from 1f311dd to 39fc2ee (8 commits, 8 files). The only file this lane shares with the delta is scripts/live-files.mjs, and both sides are byte-identical (md5 295a1317030b28dbff36539f8e97d176): upstream landed the same ranxianglei#1248 registration this lane already carried from the ci-check commit 156d0c8, so the merge is a no-op there and git auto-resolved everything else. Notable upstream commits adopted: - 4ec9778 fix: empty-slot direct send now refuses over-window bodies locally (ranxianglei#5996) - 12b18e9 fix(native-intercept): name the floored local bar distinctly from the resolved window in the empty-slot refusal (ranxianglei#5996 review) - 8b25694 fix: parse LiteLLM input and completion overflow counts (ranxianglei#5969) - 911737c test: drop duplicate overflowArmCompletionReserve assertion - 831b645 fix(ci): register tests/issue5955-wedged-same-lane-takeover.test.ts in LIVE_FILES
|
(ai: deepseek/deepseek-v4-flash) Merged The
|
| suite | result |
|---|---|
tests/issue5175-drift-name-leak.test.ts |
58 / 0 |
tests/tag-echo.test.ts |
79 / 0 |
tests/issue2190-degen-close.test.ts |
32 / 0 |
tests/golden-storage-render.test.ts |
5 / 0 |
tests/storage-engine-proofs.test.ts |
12 / 0 |
tests/native-compact-terminal.test.ts |
1 / 0 |
tests/issue4666-migration-dedup.test.ts |
2 / 0 |
tests/native-intercept.test.ts |
82 / 0 |
tests/context-overflow.test.ts |
40 / 0 |
tests/e2e-issue3900-overflow-teaches.test.ts |
3 / 0 |
npx tsc --noEmit = 0, all four gates = 0, node tools/gen-config-docs.mjs generate leaves every generated doc byte-identical.
Lane anchors intact: src/server/relay.ts:2120 still holds the non-streaming compact egress, src/storage/mysql.ts:77 still exports MYSQL_SCHEMA_VERSION = 11, and the strip symbols are unchanged (stripDriftResidue 6, isDraftDump 6, containsRenderTagText 21, mayStartDegenerateRenderTag 6, DRIFT_BARE_MISMATCH 3, DRAFT_CLOSE_TAIL 3). Footprint stays at 18 files (the live-files registration is now redundant against master but harmless — dropping it here would re-red the matrix until the merge order is known).
Still waiting on the human merge per AGENTS.md §4; nothing here is self-merged.
…ocale translations, ranxianglei#5956 reasoning-replay after cut, ranxianglei#5957 non-stream anthropic proxy loop, ranxianglei#5958 degenerate retry error shape
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Re-sync verification @
|
|
(ai: deepseek/deepseek-v4-flash) Merged What came in
Two files overlap mine: The one overlap worth naming
So it does not become a new egress — the relay still ends the turn through the one-shot rewriter, which is where my strip gate lives. The lane's non-streaming compact egress moved one line and still guards: await pipePluginJson(toClient, res, undefined, prepared.protocol, undefined, undefined, undefined, reply);at Verified on the merged tree
Strip symbols unchanged: Still waiting on the human merge per AGENTS.md §4. |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Re-sync verification @
|
…ME translation
|
Sync with master Upstream delta since No overlap with this lane's 17 files, so nothing of mine was touched; branch head is now Verification on the merged tree: kernel rebuilt (126ms), CI tally on On master itself the three symbols are still 0 / 0 / 6 ( |
…ission (whole-text, gate, seam, streaming)
(ai: vllm-qwen/qwen3.8-27b) 查重: Not duplicate — this PR is the only in-flight fix for this failure family (drifted render-tag imitations reaching the client + draft-dump turns slipping past the one-shot continuation retry). No parallel PR or issue working the same mechanism found among open/closed items; GitHub #5175/#5179 are the upstream reports this PR closes. Note: internal mirror #5175 is an unrelated auto-CI tracker — the bare "#N" in the title resolves against GitHub, not the mirror. 标签: applied Scope note (landed mid-review): two commits arrived while I was reading — ① Direction (original issues) — verified against pre-fix code at master@406193b20 rather than trusting the report. Both gaps are real and sit at the mechanism level: (A) every open-side detection in the streaming lane is scoped to an acplike NAME enum, so drifted openers ( ② Regression inventory (existing behaviors on touched paths, pre-change code read):
③ Code focus: rules are narrow and evidence-gated — pair rules keep their ④ Test validity: the 1348-line suite drives the REAL pipelines (chat/responses/plugin-json pipes + all four JSON rewriters), not predicate copies — a hand-rolled copy is exactly what hid the first bug. Byte-cut sweeps at every boundary on four wires (>200 cuts/lane) assert zero kernel refs and zero drifted openers reach the client; parity asserts preserved shapes byte-identical to SOURCE; negatives pin what must survive (genuine markup, protocol tags, verbatim channels, mismatched canonical pairs, prose containing "acp"); the two unfixable-from-streaming residuals are pinned as KNOWN with rationale; defect B gets split-at-envelope-boundary cuts plus complete-envelope-not-retried and retried-turn-no-double-answer controls; the pre-fix predicate miss is asserted as a regression control. Teeth-bearing: each bug class fails these pre-fix. ⑤/⑤b Diff cleanliness & hygiene: 17 files, +2052/-64 at final head; no whitespace churn, no unrelated files, no lockfile/generated churn, no version bumps, kernel-guard green. Naming carries intent, no dead code, no magic literals without named constants, style matches module conventions. One non-blocker observation: the suite file is named CI (run 42592398719, head c6eaf1f): 13 success, 3 skipped, 14 in_progress at my last cached read — static gates, kernel-guard, unused-exports, docs-sync, architecture-docs all green, zero failures; full test matrix still running. I will post the final tally when it lands. 八问(中文)
判定:代码审查通过(PASS)——方向到根因、回归已清点并披露、测试有牙。合并前剩两道门:① 按 §7.4 需人工签核(wire/message-shape 变更影响两种压缩模式 + 新增导出面,不合格 auto-merge);② c6eaf1f 上的完整 CI 矩阵转绿(目前零失败,分片仍在跑)。另建议 PR 描述补挂 #5179 编号(lane 现已覆盖该缺陷)。CI 终态落地后我会补一条最终计数。 |
(ai: vllm-qwen/qwen3.8-27b) CI status addendum: the run cited in my review above (42592398719) was superseded while in flight — a fresh full run (42596043917 and its sibling workflows) was triggered on the SAME head |
…incompatibility notice Master moved five commits (8 files) under the notice-registry surface: 1c069c6 adds `src/pck-notice.ts` plus `src/notice-store.ts`/`src/notices.ts` wiring, 65fcba1ba narrows the warning to enabled installs, 549bc54 restores the built-in notice banners' `.show` class, 8d202c3 stops the i18n ref scanner treating filenames like `package.json` as catalog keys, and 63a4060 is the PR merge. Zero overlap with this lane's 17 files; the only shared directory is `src/web/` and the two sides touch different files there (`i18n.ts`/`client.ts` upstream, none of mine). Nothing of mine was touched.
|
Sync with master Upstream delta since Zero overlap with this lane's 17 files. Upstream touches Verification on the merged tree: kernel rebuilt (119 ms),
Branch head is now |
ranxianglei
left a comment
There was a problem hiding this comment.
APPROVE — final head ef41839587, full matrix green (15 success / 15 skipped / 0 failed).
Independent end-to-end review complete (the full first-round review at c6eaf1fabc is in the earlier comment above; this round covers everything since).
Delta since the previous review round:
ef41839587— merge of upstream/master @63a40600e(#6015 iot-thinking incompatibility notice). Verified byte-identical adoption: the merge's non-lane portion equals the upstream master delta exactly — no conflict-resolution edits, zero file overlap with this lane's 17 files.e374969e81— test-only plus a rule-site doc comment: pins the deliberate width of DRIFT_COUNT (any acp-prefixed name, not just underscore-joined ones) in BOTH directions — underscore-free siblings<acp_tool>5</acp_tool>/<acpx>910</acpx>are stripped whole-text and at every wire cut, while genuine tool-call XML survives (TOOLCALL_MISMATCH sweep + MUST_PRESERVE set). Rationale documented at the rule site.
No kernel/ changes, no kernel-change declaration → no human-tier authorization gate applies to this PR's content. No small findings requiring in-place fixes this round.
八问(中文)
- 是什么问题:模型把内核渲染标签模仿成漂移名字/计数体后原样漏给客户端(开侧检测全部按名字枚举圈定,开放集漂移一次绕过所有门);压缩草稿以未完结
</analysis><summary>信封出现在正文中段时,$ 锚定谓词看不见它,单发续写重试不触发,整轮死在草稿上。 - 什么原因导致的:检测机制按"名字"(开放集)而非"载荷形状"(封闭集)判定——名字漂移空间模型可自由生成(#2190 round 2 已证明枚举追不上),而载荷形状(ref 在属性槽、tokens= 属性、纯数字体+acp 前缀名)由发射器固定且散文结构上产生不了。修复把判定键换成形状,属机制级修复(消除漏检类的生产者);枚举补名才是堵。
- 影响范围与严重程度:所有 plugin 模式宿主、四条 wire(openai/anthropic/google/responses)、流式与非流式全车道;p2——会话可用,代价是客户端收到垃圾标记行(首行污染、可能随回放放大)+个别轮次死在草稿直到 nudge;非不可用、非缓存泄露。
- 怎么修复的:形状键控规则组(DRIFT_PAIR/ATTR/BARE-MISMATCH/COUNT+残行裁决)+规范名排除集(HTML ~70+协议 4)门控每条规则;孤儿半体仅由同文本真配对授权移除(#1760 纪律);流式缝与非流式共用同一规则集保证判决一致;草稿重试谓词扩为"未完结信封"判据(isDraftDump,三调用点全换),完结信封+答案不重试(负例钉死防双答)。
- 是否有回归问题:逐项清点见第一轮审查评论——干净内容四 wire×全切点字节级钉死、协议标签/签名思考通道/工具参数不受触、退化轮记账探针刻意解耦;有意变更 5 处均已披露旧值→新值(漂移字节被剥、全剥轮触发一次续写重试、未完结信封触发重试、重试计数按尝试重置、原生压缩非 SSE JSON 出口改走剥离管道+整包缓冲+handoff 对齐)。高危模式逐项过:无语义漂移、无 fail-fast 变吞错(parse 失败仍 fail-open 原文转发)、无阈值/时序静默变化。本轮新增提交为纯测试+注释,零行为面变化。
- 是否破坏缓存:不涉及——前缀缓存/会话状态/pf 水位均未触碰;剥离发生在生成后的出口侧,重试计数为请求内一次性状态。
- 是否改了设计方向:不涉及——沿用既有出口净化架构(#206/#2190 谱系),无新模块、无数据流转向;结构性补充仅是形状门统一进 containsRenderTagText 供两车道共用同一判定。
- 是否引入了新的配置:不涉及——零新 env/config/默认值;docs 变更仅为恢复 master 已有 #5319 chat 包的配置文档种子(en/zh/website,生成器不动点成立)。
Merge readiness: approved — per repo rules the merge itself stays human-only (§7.4: wire/message-shape change affecting both compression modes).
(ai: vllm-qwen/qwen3.8-27b) Review round 2 complete — formal APPROVE review posted on head Delta since the previous round, both reviewed: (1) clean master sync @ CI on |
|
ework,这个是不是有无关commit进入?需要重新审核 .对业务逻辑也需要重新审核 |
(ai: vllm-qwen/qwen3.8-27b) 重新审核完成(含业务逻辑)。直接回答: 是的,有无关 commit 进入。 分支累计 57 个非合并提交里,有 19 个非核心提交(另有 4 个 iot-thinking 提交是上游 #6015 经 merge 带入的 master 内容,不算本车道产出)。进入原因:分支存活期间 master 多次变红,为保持本车道 CI 绿,worker 把红灯门禁直接在分支里修掉了(每个提交都带来源 issue 标注),没有收敛到 ci-fixes——这是流程偏差,不是内容错误。分类与现状(对照今日 master
即:实际会随本 PR 落地的无关内容 = test-only hunk(4 文件)+ config-docs 种子行(5 文件)+ 1 行 mysql export,全部带标签、全部测试/文档级、全部是保持本分支 CI 与静态门禁绿所必需。我的建议:保留在分支内——拆出去会让本分支 CI 立刻重新变红,且它们不触碰任何生产行为。 一个真实发现:#4788 的 业务逻辑复审结论:通过。
合并前强制前置:分支现在落后 master 53 个提交(上次同步停在 八问
结论:内容复审通过、无拦下项;合并前置 = ① dev 车道同步至 |
|
(ai: vllm-qwen/qwen3.8-27b) Review update after the owner-requested re-audit: content PASSES re-review — every core fix verified intact at Merge precondition: the branch is now 53 commits behind master (last synced |
(ai: vllm-qwen/qwen3.8-27b) 第三轮复审已交付完毕,两条产出均已落地:
当前状态:审核车道工作完成,无拦下项;等待 dev 车道重同步 + 人工合并决定。(上面这条转发内容是平台把我刚发的 GitHub PR 评论回显进线程,非新问题。) |
…178-conflict # Conflicts: # docs/configuration.md # docs/configuration.zh-CN.md # website/docs/index.html # website/main.js
(ai: vllm-qwen/qwen3.8-27b) Conflict resolved, branch synced to current master, pushed. Merged The conflict (4 files, one root cause). Master removed the experimental Verification on the merged tree (targeted, per local-testing discipline): typecheck clean; the PR's own suite Live state (checked this run): PR is Behavior-change flag (deliverable 4): the PR branch's docs previously listed 结构化八问:
结论:冲突已解决并推送(新 head |
[ework] CI fix verified green ✓ |
What this fixes
Two shapes of thinking content reaching the client verbatim, both reported against 0.1.208/0.1.209 in DSH plugin mode.
A. Drifted render-tag names.
<acpt m17138></apct>,<acct …>,<accent …>,<acad …>,<actp …>— the model writes the canonical ACP render tag with a drifted name, and it reaches the visible transcript with the refs intact.B. Mid-turn draft envelopes.
write the summary now.\n</analysis>\n\n<summary>## 1. Primary Request…— the model's compaction draft, streamed as assistant text.Root cause A: three layers, all keyed to an ENUMERATED name set
src/loop/tag-echo-filter.tsbuilds its render-tag vocabulary frombuildAcplikeName(), which enumeratesacp/apc/cap/cpa/pac/pcaplus single-insertion variants and a short attested-drift list. Three independent layers consult it, and all three missacpt:ACP_NAME_ALT/NAMEcontainsRenderTagText()→RENDER_TAG_DETECTmayStartDegenerateRenderTag()→BROAD_TAG_HEAD_TAIL(master) anchored\x3c…$<is heldThe close side already learned this lesson:
DEAD_REF_CLOSEuses an open name class, with the comment "#2190's census showed the close-name drift set is open (ap/ck/ak/fullwidth …)". This PR applies the same reasoning to the open side and to the gate.The decisive miss is the streaming fast path.
processOpenai/pipePluginResponsesWithStriprelease a delta verbatim when four probes all say "not a tag", and one of them ismayStartDegenerateRenderTag(raw). The reported paragraph ENDS with a closed</apct>, so a tail-anchored probe can never fire ⇒ the bytes are released and the stripper is never consulted. Reproduced end-to-end throughpipePluginChatWithStrip, not through a hand-rolled copy of the predicate:Root cause B: the draft envelope is an ENVELOPE, not a tail
degenerate-turn.tsonly detected a draft when the tag was the LAST thing in the text:The real transcript has
</analysis>followed by the entire<summary>body — tens of thousands of characters — so a$-anchored test never matches.isDraftDump()adds the envelope shape (</analysis>followed by<summary>); the three call sites aresrc/plugin.ts:2742,src/plugin.ts:4191,src/loop/core.ts:1172.Fixes, in commit order
4225a1e6acp_compress/acp_status/acp_search/acp_decompressare a CLOSED set (kernel/src/compress-tools.ts:30-31) andextractResponsesTextTriggersexecutes on themfff60bc8thinking/reasoning_content/reasoningcarry an identity filter by invariant (#1960 KDD#10, #2229)ba23a2997bee49ad2ed07a47fc402db4Plus two guards that must be green before ANY test shard runs (
ci.yml:252makestest:need all guards), so without them the suite reportsin progress 0sinstead of running:cadbf770sessions.hardMaxBytes— #4788 D2 added the knob without aprocess.yamlseed, sodocs-sync/docs-coverage/config-surface-guard/lifecycle-ledger-guardwere all red91ef0581D:\…\sessions/index.db, which is both a real product bug and the #5103 assertionInherited regressions fixed here (each verified on a pristine
upstream/masterworktree first)d7f84894issue4788-resident-minimal4/4 fail:Promise resolution is still pending but the event loop has already resolvedafterNextFlushparks a resolver on the debounce timer, but the timer isunref()'d — when it is the last pending handle the loop drains and the promise never settlesdda83795(d) envelope-frozen routing…—EBUSY: resource busy or locked, unlink …\index.dbcancelAll()(timers only); the sqlite handle stayed open and Windows refuses to unlink it3eb22b17mysql-contractred since #4796002b5fa4bumped four of FIVE stamp pins to v9; the fifth sits in the DSN-gated case, invisible without MySQLcc418fb9#5140 cold-restored parent…red on 3 shards since #5144BILI_STORAGE_UNIFIED=fullbut never set it — it inherited the mode from an earlier case;preload-storage-baseline.mjspins??= "legacy"for CIThe
dda83795/cc418fb9pair also documents why they were invisible: the #5149 deadlock killed the suite before teardown, so the EBUSY leak had no chance to fail, and the mode inheritance only breaks under the CI preload chain.Verification
6403897cc:SUCCESS=38 SKIPPED=15 FAILURE=0— the full matrix, tenth consecutive green run on this branch.grep -ln "tag-echo-filter\|degenerate-turn" tests/*.test.ts— enumerated importers, not a hand-picked list).tests/issue5175-drift-name-leak.test.ts→ 58 pass / 0 fail, covering the chat lane (openai),processAnthropic,processGoogleandpipePluginResponsesWithStripseparately, each with a per-byte cut sweep (287 cuts × 5 shapes); plus the non-streamingpipePluginJsonseam (four wires, master leaks 2 refs + 2 openers on each); plus the four non-streaming JSON rewriters (rewriteJsonResponseAsync/-Openai/-Responses/-Google) that a realstream:falseinjected turn goes through — measured on pristinemaster: refs=1 openers=1 for each of the four; plus genuine-markup controls on every seam.tests/native-compact-terminal.test.ts→ 1 pass / 0 fail, driving the REALstartServer()proxy over/responses/compactwith a plain-JSON (non-SSE) upstream answer. Covers the native-compaction JSON egress (src/server/relay.ts), which fell through to a rawpipeThrough: master leaks 2 refs + 2 openers there, this branch leaks 0, and genuine markup (x<sup>2</sup>,<span class="a">z</span>) survives byte-for-byte.a kernel ref reached the client: …→ 0 refs#5175 A: the kernel's OWN protocol tags are never read as drift→ 16/16plugin-passthrough-tag-strip-chat16/20 → 20/20#5140under the CI preload chain:not ok 1→ok 1plugin-forkwhole file under the CI preload chain:56 pass / 0 failnpx tsc --noEmitclean;npm run buildsuccess;check-dist-import-annotations/check-dist-external/check-unused-exportsall clean.Correction: defect A had a second wire, and the first cut missed it
The initial push (through
a1450f6b) verified A on the chat pipe only. The responses pipe had no drift adjudication at all:src/plugin.ts'sresponse.output_text.deltabranch calledtagFilter.push(delta)and forwarded the result, while the chat branch ranstripDriftResidue(released)on the same shape. Driving the realpipePluginResponsesWithStripshowed the leak intact there:Fixing it surfaced a second, worse bug: emptying an entire delta without recording stripped-EVIDENCE makes the #732/#821 bare-turn gate judge the turn "bare: no thinking, no stripped echo, no released markup" and pass a silently empty assistant message — no leak, no trace, no diagnostic. Both wires now set
sawStrippedEcho(+releasedMarkupCharson chat, where the #2756 markup ledger lives).Both fixes are pinned by tests that fail when either half is reverted, and they use a non-enumerated close name (
</epct>) — theREPORTEDliteral's names are all insideACP_NAME_ALT, so a test built on it is vacuously green for the drift rule.Correction 2: the "known residual" was a third shape with no owner at all
I had pinned the remaining leak as
KNOWN RESIDUAL— "a drifted pair split acrosstwo pushes". Replaying the shipped corpus after the first cut showed that diagnosis
was wrong:
createTagEchoFilteron a whole payload strips cleanly at everychunk size (whole / 40 / 7 / 3 / 1). The bytes that still reached the client were a
compound shape no rule owned:
DRIFT_PAIRandDRIFT_ATTRboth carry a\1backreference, so they require theopening and closing names to agree. Nothing owned an emitter attribute slot
followed by bare refs followed by a disagreeing close.
Unit-level isolation on the pre-fix tree was decisive (this is the state the two
mismatch rules were written against):
<acpt m17138></apct>(same name)<acpt m17138></apct>(different name)<acpt></apct>(anonymous empty body)The empty-body row is a correction to an earlier revision of these rules: an
earlier cut owned anonymous empty pairs by shape alone.
714d69a16removed that — asame-name empty pair is valid XML and appears in real content, so shape is not
evidence of an emitter.
DRIFT_EMPTY_MISMATCHno longer exists;DRIFT_ATTR_MISMATCHand
DRIFT_BARE_MISMATCHrequire a non-empty ref body, which is why the thirdrow stays kept.
They strip the matched span only — deliberately NOT licensing orphan-half
removal, because the opening name here can be a genuine
tool_calland proving itwould let the orphan pass delete real tool-call markup elsewhere in the field (#1039).
The second half of that fix is a gate gap. The #2190 broad head probe had no
_(so
<tool_callnever engaged) and stopped at the name (so a still-growingattribute slot read as prose).
BROAD_TAG_OPEN_TAILreplacesBROAD_TAG_HEAD_TAILin both the gate (
src/loop/tag-echo-filter.ts:937) and the filter's own tail chain(
:1663) — changing only the gate left the pipeline leaking, because the filterreleases through its own chain.
Tool-call XML is still untouched (#295/#361)
The newly-stripped corpus spans carry names like
tool_call,bashandacp_bash_state, so the #295/#361 contract needs a direct check rather than anassumption. Measured on the current head:
<function_calls><invoke name="Bash"><parameter name="command">ls -la</parameter></invoke></function_calls>tokens=, no kernel ref<parameter name="command">grep -rn foo src/</parameter><tool_call>read the file and report back</tool_call>the model wrote <tool_call name="bash">cd /tmp</tool_call> as text<tool_call tokens="0" type="text">m10234</acctokens>tokens=and a kernel ref<acp_bash_state tokens="182" type="text">m07866</apc_bash_state>tokens=and a ref<bash tokens="60" type="text">m08847</bash>tokens=and a kernel refThe discriminator is
tokens=plus a kernel ref, not the tag name:acp_bash_stateappears nowhere in the tracked tree, so it can only be a model invention, while
tool_callis real markup — and the rule tells them apart on content, not name.Corpus census over the 8225 shipped fields (differential against master)
Both trees'
stripAcpTagswere run over the same 8225 fields and the outputs diffed,so only newly stripped spans are counted — canonical
<acp>stripping happens onboth sides and cancels out:
The last row is the safety argument: every newly-removed region contains a kernel
reference, i.e. it is emitter echo, never ordinary content. 0 unsafe.
Scope
Whole-text, the streaming fast path, and the attribute-slot/mismatched-close shape are
all covered. What is NOT covered, stated plainly: a drifted pair whose two halves are
pushed in different chunks still degrades to a ref-less orphan close fragment
(
</apct>with the opening half already consumed upstream). Measured: 0 of 287byte-boundary splits leak a kernel reference on any of the four wires; the 50/287
residue is that fragment, with
refCuts: 0. Adding line-level adjudication therecollides with the #2190 round-3 balanced-markup contract (four separate attempts, all
reverted).
Tool-call XML is untouched: stripping it is forbidden (#295/#361) because a
shape-based match cannot distinguish "trying to call" from "discussing such markup".
Related: #5175, #4764, #5140, #4788, #4666, #5103, #2303, #2190, #2183, #1760, #1720, #1039.
Landing order (re-verified on the current head)
095b39bf3adds a rule tosrc/loop/tag-echo-filter.ts, the same file #5216 touches,so the order was replayed rather than assumed: #4785 → #5216 → #5178 from master
c0fa5b2d6gives 0 + 0 + 2 conflicting files (src/plugin.ts,src/loop/core.ts), both a union of intent —isDraftDumpfor the gate,draftTailKind+ the shape-routed nudges for the retry. Separately, #5216'sopener-less pass must run AFTER this PR's drift adjudication (drift resolves names
only as intact PAIRS); that is a 10-line move, pinned by a negative control
(reverting it → exactly four red, restoring → green). Composed tree:
tscclean,229/229 across 13 contention suites, build OK.