fix(review): budget the composed body against GitHub's review limit - #9247
Conversation
A review body over 65,536 characters is rejected by the API whole — every blocker it carries with it — and until now nothing measured the composed body. Only two contributors were individually bounded (the ledger marker's 8 KiB, the deferral list's 20 × 240); the unresolved-blocker list, the disclosure sentences and the body Criticals are model-written prose with no upstream cap. A probe on #9118 composed 67,039 characters, which would have lost that review's blockers to a 422. compose-review now measures what it is about to return, holding back room for the marker it appends afterwards (only when the plan names a PR — the same check the marker itself asks, so the two cannot disagree), and trims in a fixed order when the body would overflow: the deferral display first (rank 1 — the display of findings the review deliberately did NOT request), the not-reviewed disclosures second (rank 2 — the verdict's cap already carries that claim), and the blockers, the undecided-blocker list, the caps and the openers never. Each trim is disclosed in the body with a count and repeated on stderr, because a list silently shortened reads as a list that was complete. When the un-trimmable remainder still overflows — blockers alone past the limit — the body is truncated rather than composed into a rejection: the bilingual fold is dropped first so the cut cannot leave unbalanced markup on the PR page, the cut lands on a code-point boundary, and a loud notice names the truncation. Posting a truncated review beats posting none. Closes #9177.
|
Re-run on the current head — 21 commits and seven autofix review rounds since the first pass, so every stage below is a fresh assessment, not a refresh. Template looks good ✓ Problem: real and measured, not theoretical. A probe during #9118's review composed 67,039 characters against GitHub's 65,536 limit, and the linked issue (#9177, Direction: aligned — this closes #9177 at the one place that sees the whole composed body, and the review pipeline's own robustness is core to this repo's dogfooded tooling. Size: 9 files — 643 production logic lines (compose-review.ts +530/−57, review-footer.ts +19/−1, save-artifact.ts +36), 1,030 test lines, 20 doc lines (SKILL.md +3/−1, web-shell README +8/−8 alignment-only). The only core-path touch is the 4-line SKILL.md doc update. Author is a collaborator/maintainer and this is a Approach: matches what I'd propose independently — measure at the composition site, degrade in least-loss order, truncate only as a code-point-safe last resort, reserve room for the ledger marker, and disclose every trim. The rounds since the first pass earned their additions rather than padding: the bilingual fold now yields before any content does, the last-resort cut spends already-delivered prose before this round's only-copy blockers, the footer's interpolations are capped at the source, and the artifact and verdict line track what the body actually carries. One hygiene note: the Risk: no elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明本轮为当前 head 的重新运行——自首次通过后已有 21 个提交与 7 轮 autofix 评审,以下各阶段均为全新评估而非刷新。 模板完整 ✓ 问题:真实且有实测,不是理论性问题。#9118 评审期间的探针组合出 67,039 字符,超过 GitHub 65,536 上限;关联 issue(#9177, 方向:对齐——在唯一知晓完整正文的位置闭合 #9177,评审流水线自身的健壮性是本仓库自 dogfood 工具的核心。 规模:9 个文件——643 行生产逻辑(compose-review.ts +530/−57、review-footer.ts +19/−1、save-artifact.ts +36),1,030 行测试,20 行文档(SKILL.md +3/−1、web-shell README +8/−8 仅为对齐)。核心路径改动仅 SKILL.md 的 4 行文档更新。作者为协作者/维护者且类型为 方案:与我的独立设想一致——在组合点测量、按最小损失顺序降级、仅在码点安全的最后手段截断、为 ledger marker 预留空间、每次裁剪都披露。首次通过之后的各轮迭代所增加的都有据可依而非堆砌:双语折叠先于任何内容让位;末位切割先消耗作者已收到的文本、再触及本轮唯一副本的阻断项;页脚插值在源头设上限;工件与裁决行跟随正文实际承载的内容。一处卫生提示: 风险:无升级风险信号——未匹配高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewNo blocking issues found. My independent proposal before reading the diff was: measure the body at the one composition site, degrade in least-loss order (bilingual fold first, then disclosure sections, blockers never), truncate at a code-point boundary with a loud notice only as a last resort, reserve headroom for the post-composition ledger marker, disclose every trim, and persist the trim state so downstream claims stay truthful. The PR matches that baseline and is stricter where it counts. What I traced against the code at this commit:
One non-blocking hygiene note, carried over from Stage 1: the Test evidenceCI on the reviewed commit is green, and — unlike the first pass — this PR's suites actually executed there: the pre-existing
Real-scenario (tmux) testing: N/A — pipeline behavior with no TUI surface, per the PR's own test plan; and this is an unattended run, where the live-behavior lane is the sandboxed job named above. 中文说明代码审查未发现阻断问题。我在读 diff 前的独立方案是:在唯一组合点测量正文、按最小损失顺序降级(先双语折叠、再披露段落、阻断项永不裁剪)、仅在码点边界做带醒目通知的最后截断、为组合后追加的 ledger marker 预留空间、每次裁剪都披露、并将裁剪状态持久化以保证下游声明如实。PR 与该基线一致,且在关键处更严格。 已对照当前提交逐行核验:预算数学端到端闭合(最坏 65,024 ≤ 65,536, 一处非阻断卫生提示(与 Stage 1 一致):web-shell README 改动仅为表格宽度重对齐、无内容变化,与本 PR 目标无关。 测试证据评审提交上的 CI 为绿,且与首轮不同——本 PR 的套件确实在 CI 中执行过:此前挡住流水线进入测试步骤的预先存在的 真实场景(tmux)测试:不适用——按 PR 自身测试计划,此为流水线行为、无 TUI 界面;且本次为无人值守运行,实时行为通道即上述沙箱任务。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean across every stage; the single nit is the unrelated web-shell README re-alignment riding in the diff. Stepping back: this started as a small fix for a measured hole — a probe composed 67,039 characters, past the 65,536 line where GitHub rejects a review body whole, blockers and all. What landed is larger (643 production lines) because seven review rounds each closed a real hole rather than adding polish: the fold must yield before any content does, the last-resort cut must spend already-delivered prose before this round's only-copy blockers, the footer's interpolations must be bounded at the source or the cut can be emptied, the marker reserve and the marker must ask one shared question, and everything the trim leaves behind must still describe the body truthfully — verdict line and artifact included. Each of those is pinned by a test that names the mutant it exists to catch; the suite is the strongest evidence this PR carries, and it ran green in CI on this commit (the first pass's one reservation — tests that had never executed in CI — is gone now that #9238's build fix merged in). Against my independent proposal, the PR matches on the shape and exceeds on the seams I would have gotten wrong: I would have appended the truncation notice at the end, which is exactly the placement the rounds rejected after three hand-models of page rendering diverged; above the cut is the bounded choice. I would also have capped the footer defensively at the budget rather than at the source — weaker, because an unbounded footer can still empty the cut. The scope is justified for a module whose whole contract is truth-telling: a silent truncation would leave the verdict line and the artifact describing a body that no longer exists. What keeps this at 4 rather than 5 is only hygiene, not substance: the Approving, pinned to the reviewed commit. Heads-up for the author and maintainers: the standing CHANGES_REQUESTED reviews in this thread are the autofix review rounds' verdicts on earlier commits — this approval attests to the current head, and those stale requests will need dismissing (or will fall to branch protection) before merge. 中文说明回顾全局:这始于一个已实测缺口的小修复——探针组合出 67,039 字符,超过 GitHub 65,536 上限时评审正文会被整条拒收、连同所有阻断项。最终落地的改动更大(643 行生产代码),因为七轮评审每一轮都在闭合真实缺口而非堆砌:折叠必须先于任何内容让位;末位切割必须先消耗作者已收到的文本、再触及本轮唯一副本的阻断项;页脚插值必须在源头设界、否则切口可被清空;marker 预留与 marker 必须共用同一判定;裁剪之后留下的每句话仍须如实描述正文——裁决行与工件同样如此。每条都有钉住其对应变种体的测试;这套测试是本 PR 最有力的证据,且已在本提交的 CI 中跑绿(首轮的保留——测试从未在 CI 执行——随 #9238 构建修复合入而消失)。 对照我的独立方案:形态一致,且在我会做错的接缝处更严格——我会把截断通知追加在末尾,而那正是各轮在三种页面渲染手工模型各自发散后否决的位置;置于切口之上才是有界的选择。我也会在预算侧而非源头为页脚设防——更弱,因为无界页脚仍可清空切口。对一个契约即如实的模块,这一范围是正当的:静默截断会让裁决行与工件描述一个已不存在的正文。 评为 4 而非 5 只在卫生、不在实质:web-shell README 改动是无内容变化的表格宽度重对齐,与正文预算毫无关系——未来触碰时值得从本分支拿掉,但不值得扣住 PR。按 re-run 规则记录、不据此延后。 予以批准,钉在所评审的提交上。提醒作者与维护者:线程中现存的 CHANGES_REQUESTED 评审是 autofix 各轮对更早提交的判定——本次批准针对当前 head,合并前那些过期请求需要被 dismiss(或交由分支保护处理)。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.12)
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Round-1 review of the budget found the disclosures drift from the cut: - The verdict line claimed N deferrals were "listed in the body" after the trim had removed the list; `bodyTrim` now rides on the composed result (and through the saved artifact) and the line turns with it. - The trim notice was the last element of `head`, so the last-resort prefix cut could slice off the very sentence disclosing the drop; it now rides first and in the protected tail. The hard "TRUNCATED" claim fires only on an actual cut — a bilingual-fold-only overflow says so. - The last resort ordered by position, spending this round's only copy of the blockers before prose the author already has; parts now carry a `keep` rank and the cut spends them in that order. - The trim note names the kinds it dropped rather than counting them. - `planNamesPr` routes through the shared `isPositivePrNumber`. - The unlicensed-deferral disclosure no longer promises adjacency it cannot keep once its neighbour can be trimmed away. SKILL.md's promise that a trimmed section "stays whole in the artifact" was true only of the deferral list: the artifact persists findings, counts and the trimmed body, so a trimmed disclosure has no other durable copy. The sentence now says which is which, and the reviewer is told to repeat the trimmed sections in the terminal summary — the stderr line asks for the same thing. Tests: the order test now uses a shape where dropping rank 1 alone fits, so a mutant that trims both together fails it; the truncation guard gets an astral-plane body, where removing the surrogate check leaves a lone high surrogate; and the artifact's `bodyTrim` gets the refuse/default arms its sibling count already had.
An adversarial pass over the previous commit reproduced five defects, three of them regressions it introduced: - The `keep` sort spent the undecided-blocker list FIRST — it carried no rank, so it sorted below the body Criticals — while the trim notice went on asserting "Nothing blocking was trimmed". The list is blocker-grade (`keep: 2`), and the claim is dropped exactly when a cut happened, where the truncation notice takes over the subject. - The COMMENT path merges clauses 1-4 into one paragraph and copied only their text, discarding every `keep` tag — so the merged opener, carrying the downgrade disclosure, became the first thing the cut spent. The merge now inherits the strongest retention among the clauses it merges. - The `keep` sort also ran on the fold-only exit, reordering a body that is never cut: "Unresolved, please confirm" read as a footnote to the blocker above it. That exit composes in document order. - The fold-only exit swallowed the per-rank stderr line and called the English text complete after the rank loop had dropped sections out of it. Both exits now push the naming line, and the notice says what is missing. - The notice and `bodyTrim.sections` counted RANKS, not sections: one rank routinely carries four `Not reviewed:` paragraphs, so a body that dropped five sections said two — and persisted two into the artifact. SKILL.md's Step 8 said a deferred finding's durable record on the PR is the posted deferral list; the budget can now drop that list whole, so the sentence is qualified and pinned beside the Step 7 one. The astral-plane truncation test could not fail — the cut for its fixture landed between surrogate pairs. A one-character prefix moves the boundary inside one, and each new test here was checked by mutating the code it pins and confirming it goes red.
Self-audit round on
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 3)": none — the full chunk (diff lines 389–778) and all supporting source it depends on were read to completion..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 3)":none — the full chunk (diff lines 389–778) and all supporting source it depends on were read to completion.。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
@qwen-code /resolve |
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
|
Qwen Code attempted to resolve merge conflicts but the run did not complete successfully. Check the workflow run for full logs. |
…ings Round 2 found no blockers; the twelve Suggestions are all about the budget telling the truth and the tests being able to fail. The degradation ladder changes shape. The bilingual fold is a translation of the English above it, so dropping it costs the author no content at all — while every other rung costs a finding or a disclosure. It now yields FIRST: measured on a bilingual body, a mild overflow used to spend the whole deferral list with ~24,000 characters of headroom sitting unused behind the fold. `bodyTrim` records the fold drop alongside the rest, and nothing claims a fold was dropped on a body that never had one. Also: - The trim notice's "deferred findings in this run's findings artifact" pointer rode unconditionally, so a rank-2-only trim (any run with disclosures and no posture deferrals) sent the author to a list that does not exist. It is conditional now, as its stderr sibling always was. - The clause-3 openers — including the two `Review incomplete — …` sentences — carried no retention rank, so a COMMENT merge containing only them still defaulted to the weakest and the cut spent them before any blocker. - The surrogate fixup is a loop: quoted text can already carry an unpaired high, and a cut inside the astral pair that follows leaves two halves. Tests: the fits-untouched guard forbade a phrase no code path emits (a spurious trim banner passed it); the fold-balance oracle compared presence rather than counts; the surrogate oracle was one-sided; and the fold and truncation exits' stderr lines were asserted by nothing. Every pin added here was checked by mutating what it pins — eleven mutants, eleven kills. SKILL.md carries the new order, and both budget pins now cover the clauses that do the work rather than a prefix of them.
…e cut path A self-audit of the reordered ladder found the reorder had left its own false record: - The fold-drop record was taken on the way INTO the tail cut, so every truncated bilingual body recorded `fold: true` and pushed "the English body is complete" — on a body cut mid-blocker whose text disclosed no fold at all. The stderr line is persisted with the verdict, so that was durable. The cut path now says the English text is truncated as well, in the body and on stderr, and the fold notice rides in the cut's protected head so the reader sees it. - The fold notice was appended at the bottom, 64,000 characters below the body it qualifies — the same shape the trim notice's own comment rejects, and a contradiction of SKILL.md's "every trim is disclosed at the top of the body". Both notices ride at the top now, fold first, each describing what the other left. - The fold-balance oracle compared 0 to 0: its fixture's only part has `zh === en`, so no fold was ever built. Rung 3 renders English only, so the assertion is now that the posted body carries NO fold markup — which a mutant appending a bare `<details>` opener fails.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 5)": none — no check was cut short.; "agent reverse-audit (round 4)": none — the chunk read came back complete (lines 1078–1199 of 2588, untruncated) and every check above ran to a verdict..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 5)":none — no check was cut short.;"agent reverse-audit (round 4)":none — the chunk read came back complete (lines 1078–1199 of 2588, untruncated) and every check above ran to a verdict.。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const RAW_TEXT_ELEMENTS: ReadonlySet<string> = new Set([ | ||
| 'script', | ||
| 'style', |
There was a problem hiding this comment.
[Critical] R5-1 (class-level re-report, carried id from the round-5 ledger): the rung-3 cut's page-state certification — noticeRidesClean / scanBrowserState / openBlockAtEnd — still hand-models how the posted PR page renders over model-written blocker markdown. This is an UNBOUNDED surface, disproved in every round since round 3; round 7 executes four divergence classes against this commit:
(a) Unclosed <details> — probe: bodyLen=65012, <details>=1 </details>=0, notice ancestors ["details","p"]: the truncation notice and footer render nested inside a collapsed details:not([open]) element, invisible to the author.
(b) A type-1 <script> html_block carrying <!--<script> before its </script> line — scanBrowserState models no script-data escape levels; parse5 places notice+footer as script text (notice ancestors: ["script"]).
(c) plaintext/template are absent from RAW_TEXT_ELEMENTS despite the docstring naming plaintext and the dead handling at :776/:938 — probes show both ship with no closer while set-member controls ride clean.
(e) skipTagBody (~:710) treats a quote in attribute-NAME position as a value delimiter — a cut ending <div "x><script>" y> is certified clean; parse5 notice ancestors ["html","body","div","script"]. Flip check: the expectValue fix appends the closer, notice ancestors become ["div","p"], and the suite stays 306/306 green.
(d, claimed at low confidence — GitHub's svg preservation unprobed) foreign-content CDATA in <svg>/<math>: the markup arm ends <![CDATA[ at the first >; parse5 notice ancestors ["svg"].
Failure scenario: a body Critical quoting PR code overflows into rung 3 with the cut landing after one of these openers → no closer/rewind → the author reads a truncated body as a complete review — the exact failure this machinery exists to prevent.
Fix (structural — entrance-by-entrance patching produced a new divergence class in rounds 3, 4, 5 and now 7): certify the tail with a real HTML5 tokenizer (parse5 is in the tree transitively via jsdom) over the actually-rendered HTML, or fail closed whenever any raw-text/swallow/collapsed-hiding container opener precedes the cut without real-parser certification. Interim: add plaintext/template to RAW_TEXT_ELEMENTS, track <details> nesting depth with \n</details> closer candidates, and consume quoted spans in skipTagBody only in before-attribute-value position (expectValue flag set on =).
中文说明
[Critical] R5-1(类级复报,沿用第 5 轮账本 id):rung-3 切割的页面状态认证——noticeRidesClean / scanBrowserState / openBlockAtEnd——仍在手工建模「发布后的 PR 页面如何渲染模型写出的阻断项 markdown」。这是一个无界表面,自第 3 轮起每轮都被证伪;本轮在该提交上实测出四个分歧类别:(a) 未闭合 <details>——探针:bodyLen=65012、<details>=1 </details>=0、通知祖先 ["details","p"]:截断通知与页脚被嵌套进折叠的 details:not([open]) 元素,作者不可见;(b) 携带 <!--<script> 且位于 </script> 行之前的 type-1 <script> html 块——scanBrowserState 不建模 script-data 转义层级;parse5 将通知+页脚置为 script 文本;(c) plaintext/template 不在 RAW_TEXT_ELEMENTS 中,尽管 docstring 点名了 plaintext 且 :776/:938 存在死代码处理——探针显示两者均无闭合器发出,而集合成员对照组正常闭合;(e) skipTagBody(约 :710)把属性名位置上的引号当作值定界符——以 <div "x><script>" y> 结尾的切割被认证为干净;parse5 通知祖先 ["html","body","div","script"]。翻转验证:expectValue 修复后闭合器被补上、通知祖先变为 ["div","p"],且测试套件保持 306/306 全绿。(d,低置信声称——GitHub 是否保留 svg 外来内容未证实) <svg>/<math> 外来内容中的 CDATA:markup 分支在第一个 > 处结束 <![CDATA[;parse5 通知祖先 ["svg"]。失败场景:引用 PR 代码的 body Critical 溢出进入 rung 3、切割落在上述某个开标签之后 → 无闭合器/无回退 → 作者把被截断的正文当作完整评审阅读——正是这套机制本要防止的失败。修复(结构性——逐入口打补丁已在第 3、4、5 以及本轮各产生一个新分歧类别):用真正的 HTML5 分词器(parse5 经 jsdom 已在依赖树中)对实际渲染出的 HTML 认证尾部,或在任何 raw-text/吞吸/折叠隐藏容器开标签未被真解析器认证时失败关闭。临时措施:把 plaintext/template 加入 RAW_TEXT_ELEMENTS、跟踪 <details> 嵌套深度并提供 \n</details> 闭合候选、让 skipTagBody 仅在「属性值之前」位置(在 = 上设置 expectValue 标志)消费引号包裹的片段。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
The interim fix from this finding landed this round; the structural rewrite is deferred to the follow-up queue.
Reproduced first — a probe against this commit (markdown-it render → parse5 DOM) confirmed four divergence classes: (a) the notice hidden inside an unclosed collapsed <details> (ancestors ["details","p"]); (b) the notice swallowed by <script> through the <!\-\-/<script escape levels the scan never modelled; (c) <plaintext>/<template> (mid-line and block forms) shipping with no closer — the notice swallowed or parked in inert template content; (e) a quote in attribute-NAME position (<span "x><script>") read as a value delimiter, certifying a tail the real <script> swallows. (d) (foreign-content CDATA) stays open — GitHub's <svg> preservation is unprobed, per the finding's own low-confidence note.
Fixed — the interim prescription, plus the escape-level class: plaintext/template added to RAW_TEXT_ELEMENTS (and template to the raw1 closer copy); scanBrowserState now tracks <details> nesting depth and the cut offers one \n</details> per open level — combined with the swallow's own closer when both are open — every candidate verified against the rendered page before it ships, with the fail-closed rewind when no closer can be certified (plaintext); skipTagBody consumes quoted spans only in before-attribute-value position (after =); the script arm models the two escape levels, so a </script> spent inside either exits the level, not the element.
Fixtures — every reproduced class has a cut fixture asserting, through a jsdom-parsed oracle, that the notice lands outside every swallowing element and every collapsed <details>; all eight failed against the pre-fix code (measured).
Deferred — the structural fix itself (a real HTML5 tokenizer certifying the tail in production code) requires declaring parse5/jsdom as a production dependency of this package (package.json + lockfile), a supply-chain change outside this PR's footprint; it is recorded in the PR's deferred-findings queue, and the thread stays open for it.
中文说明
本发现的临时修复已于本轮落地;结构性重写延后到跟进队列。
先复现——针对本提交的探针(markdown-it 渲染 → parse5 DOM)确认了四个分歧类别:(a) 通知被隐藏在未闭合折叠的 <details> 内(祖先 ["details","p"]);(b) 通知经由扫描从未建模的 <!\-\-/<script 转义层级被 <script> 吞吸;(c) <plaintext>/<template>(行中与块形态)无闭合器发出——通知被吞吸或置于惰性的 template 内容中;(e) 属性名位置的引号(<span "x><script>")被当作值定界符,认证了实际被 <script> 吞吸的尾部。(d)(外来内容 CDATA)保持开放——按本发现自身的低置信标注,GitHub 是否保留 <svg> 未证实。
已修复——临时处方 + 转义层级类别:plaintext/template 加入 RAW_TEXT_ELEMENTS(且 template 加入 raw1 闭合器副本);scanBrowserState 现在跟踪 <details> 嵌套深度,切割按每个未闭合层级提供一个 \n</details>——两者都开放时与吞吸元素自身的闭合器组合——每个候选在发布前对渲染页面验证,无法认证闭合器时失败关闭回退(plaintext);skipTagBody 仅在属性值之前位置(= 之后)消费引号包裹片段;script 分支建模两个转义层级,任一层级内消费的 </script> 只退出层级、不关闭元素。
夹具——每个已复现类别都有一个切割夹具,经 jsdom 解析的 oracle 断言通知落在所有吞吸元素与所有折叠 <details> 之外;全部八个在修复前代码上失败(实测)。
延后——结构性修复本身(在生产代码中用真正的 HTML5 分词器认证尾部)需要把 parse5/jsdom 声明为本包的生产依赖(package.json + lockfile),属于本 PR 足迹之外的供应链变更;已记录进本 PR 的延后发现队列,本线程为其保持开放。
| trim: 2, | ||
| en: `Not linted (tool limitation, not a blocker): ${gateDisclosed.join('; ')}.`, |
There was a problem hiding this comment.
[Suggestion] R5-10 (ledger id carried forward — still stands): the trim: 2 tags on deferredBlock (here) and testPlanBlock (~:2770) have no overflow oracle — neither block's rendered string ("Not linted (tool limitation…", "Test Plan (not a blocker)…") appears anywhere in the 6,966-line test file; the rank-scan overflow fixture exercises only their sibling repositoryContextBlock.
Failure scenario: mutation run at this commit — deleting trim: 2 from both blocks ships the whole suite green (306/306, measured): the notes become un-trimmable, so a borderline overflowing body that rung 2 would settle by dropping the disclosures falls through to the rung-3 cut and shaves blocker text instead; 2 → 1 also ships green and drops the notes WITH the deferral display, inverting the order SKILL.md pins.
Fix: extend the 'ranks the plan-gate disclosures…' self-calibrating scan with fixtures feeding scriptLintGate/testPlanGate notes, requiring the same both-shapes-exist property for each block.
中文说明
[建议] R5-10(账本 id 沿用——仍然存在):deferredBlock(此处)与 testPlanBlock(约 :2770)上的 trim: 2 标签没有溢出 oracle——两个块的渲染字符串("Not linted (tool limitation…"、"Test Plan (not a blocker)…")在 6,966 行测试文件中任何位置都未被断言;rank 扫描溢出夹具只演练了它们的同族 repositoryContextBlock。失败场景:在本提交上运行变异——删除两个块的 trim: 2 后整个套件全绿(306/306,实测):这两条通知变得不可裁剪,于是原本靠 rung 2 丢弃披露即可收敛的临界溢出正文会落入 rung-3 切割、改为削阻断项文本;2 → 1 同样全绿,并把这两条通知连同延后展示一起丢弃,颠倒 SKILL.md 钉住的顺序。修复:在 'ranks the plan-gate disclosures…' 自标定扫描中补入 scriptLintGate/testPlanGate 通知的夹具,对每个块要求同样的「两种形态都存在」性质。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Deferred to the next round, not dropped — this round's batch was capped (~8 findings) with the Critical (R5-1) and its paired oracle rewrite (R7-9) taking priority. The finding stands as reported: the trim: 2 tags on deferredBlock/testPlanBlock still have no overflow oracle; the proposed self-calibrating scan extension queues next.
中文说明
延后到下一轮,并非丢弃——本轮批次上限约 8 项,Critical(R5-1)及其配套 oracle 重写(R7-9)优先。本发现按原样成立:deferredBlock/testPlanBlock 上的 trim: 2 标签仍无溢出 oracle;所提的自标定扫描扩展排入下一轮。
| keep: 1, | ||
| en: '⚠️ The reverse-audit findings file could not be read at compose time, so this run cannot show its findings were verified.', |
There was a problem hiding this comment.
[Suggestion] R5-3 (ledger id carried forward — still stands): the keep: 1 on both unverifiedTagsBlock branches (this arm and the surviving-tag arm at ~:2794) has no rung-3 truncation-survival oracle — the still carried the fixtures (compose-review.test.ts:6318-6400) test the findingsPath tag-CAP feature and never overflow (their largest blocker is ~50 chars), so the tag never rides a rung-3 cut.
Failure scenario: mutation at this commit — deleting keep: 1 from both arms ships the suite green (306/306, measured); a tagged findings file plus a >65k body Critical then truncates and the cut spends the tag disclosure before the blockers — the author never learns verification was outstanding. The fixture promised in round 6 did not land.
Fix: a tagged findings file plus a deterministic 70k blocker, asserting bodyTrim.truncated and toContain('still carried the').
中文说明
[建议] R5-3(账本 id 沿用——仍然存在):unverifiedTagsBlock 两个分支(此臂与约 :2794 的存活标记臂)上的 keep: 1 没有 rung-3 截断存活 oracle——still carried the 夹具(compose-review.test.ts:6318-6400)测试的是 findingsPath 标记上限功能且从不出溢(最大阻断项约 50 字符),因此该标记从未经历过 rung-3 切割。失败场景:在本提交上变异——删除两臂的 keep: 1 套件全绿(306/306,实测);带标记的 findings 文件加上超过 65k 的 body Critical 时发生截断,切割会先消费标记披露、再轮到阻断项——作者永远不知道验证未完成。第 6 轮承诺的夹具没有落地。修复:带标记的 findings 文件加一个确定性 70k 阻断项,断言 bodyTrim.truncated 与 toContain('still carried the')。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Deferred to the next round, not dropped — this round's batch was capped (~8 findings) with the Critical (R5-1) and its paired oracle rewrite (R7-9) taking priority. The finding stands as reported: the keep: 1 on both unverifiedTagsBlock arms still has no rung-3 truncation-survival oracle; the tagged-findings-plus-70k-blocker fixture queues next.
中文说明
延后到下一轮,并非丢弃——本轮批次上限约 8 项,Critical(R5-1)及其配套 oracle 重写(R7-9)优先。本发现按原样成立:unverifiedTagsBlock 两臂上的 keep: 1 仍无 rung-3 截断存活 oracle;带标记 findings 文件 + 70k 阻断项的夹具排入下一轮。
| keep: 1, | ||
| en: '⚠️ This run could not certify that any of this diff was reviewed.', |
There was a problem hiding this comment.
[Suggestion] R7-2 (new this round, R5-9 family): round 7 added keep: 1 to three further clauses with no truncation oracle — the nothingCertified arm of coverageOpener (here), Suggestions are inline. (:3075), and the suggestionsDiscarded clause (:3087). All three tag deletions ship green (measured). Note: the two suggestions clauses are COMMENT-path-only and merge into an opener that always carries a keep:1 sibling, so deleting either tag is behaviorally inert at this commit — the LIVE exposure is this nothingCertified arm, which renders as a standalone part on the REQUEST_CHANGES path.
Failure scenario: deleting keep: 1 from the nothingCertified arm ships the suite green; a run that could not certify any diff beside an over-budget blocker then truncates and the cut spends the trust warning before the blockers — the author gets a truncated body with no sentence saying the run itself was uncertified.
Fix: one truncation fixture per clause in the pattern of 'keeps the context-unavailable trust warning through a truncation' — the nothingCertified arm is the load-bearing one.
中文说明
[建议] R7-2(本轮新发现,R5-9 族):第 7 轮又给三个没有截断 oracle 的子句加了 keep: 1——coverageOpener 的 nothingCertified 臂(此处)、Suggestions are inline.(约 :3075)、suggestionsDiscarded 子句(约 :3087)。三处标签删除均全绿上船(实测)。注意:两个 suggestions 子句只在 COMMENT 路径出现,且会并入总带有 keep:1 兄弟的开场白,因此删除这两个标签在本提交上行为惰性——实际暴露是这个 nothingCertified 臂,它在 REQUEST_CHANGES 路径上作为独立段落渲染。失败场景:删除 nothingCertified 臂的 keep: 1 套件全绿;一次无法认证任何 diff 的评审在阻断项超预算时截断,切割先消费信任警告、再轮到阻断项——作者拿到被截断的正文,却没有任何一句说明本次运行本身未被认证。修复:仿照 'keeps the context-unavailable trust warning through a truncation' 为每个子句加一个截断夹具——nothingCertified 臂是承重的那个。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Deferred to the next round, not dropped — this round's batch was capped (~8 findings) with the Critical (R5-1) and its paired oracle rewrite (R7-9) taking priority. The finding stands as reported: the keep: 1 on the nothingCertified arm (the load-bearing one) and the two COMMENT-path suggestions clauses still has no truncation oracle; one fixture per clause queues next.
中文说明
延后到下一轮,并非丢弃——本轮批次上限约 8 项,Critical(R5-1)及其配套 oracle 重写(R7-9)优先。本发现按原样成立:nothingCertified 臂(承重者)与两个 COMMENT 路径 suggestions 子句上的 keep: 1 仍无截断 oracle;每子句一个夹具排入下一轮。
| expect(budget).not.toContain('the English body is complete'); | ||
| }); | ||
|
|
||
| it('does not reorder a body it never cuts', () => { |
There was a problem hiding this comment.
[Suggestion] R5-14 (ledger id carried forward — still stands): 'does not reorder a body it never cuts' is pinned only for the fold-only (rung-1) exit — it asserts the fold-drop notice to prove its branch. The rung-2 rank-drop exit also returns a body that survives whole and has no order oracle.
Failure scenario: mutation at this commit — applying the keep-sort to the rung-2 exit ships 306/306 green, and the probe flips: clean tree undecidedAt=340 < unlicensedAt=397 (natural order); mutant undecidedAt=64701, unlicensedAt=315 — the keep:1 disclosure is filed before the 64k undecided-blocker block.
Fix: a rung-2 fixture sized to drop rank 1 and fit, with the same order-inverting pair (a cannotTell entry beside unlicensed deferrals) asserting natural order survives.
中文说明
[建议] R5-14(账本 id 沿用——仍然存在):'does not reorder a body it never cuts' 仍只为 fold-only(rung-1)出口钉住——它靠断言 fold 丢弃通知来证明自己走的是该分支。rung-2 的整 rank 丢弃出口同样返回完整存活的正文,却没有顺序 oracle。失败场景:在本提交上变异——把 keep 排序应用到 rung-2 出口,306/306 全绿,且探针翻转:未改动树 undecidedAt=340 < unlicensedAt=397(自然顺序);变异体 undecidedAt=64701、unlicensedAt=315——keep:1 披露被排到 64k 未决阻断块之前。修复:一个 sized 到丢弃 rank 1 后恰好收敛的 rung-2 夹具,带同样的顺序反转对(cannotTell 条目 + 无许可延后披露),断言自然顺序存活。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Deferred to the next round, not dropped — this round's batch was capped (~8 findings) with the Critical (R5-1) and its paired oracle rewrite (R7-9) taking priority. The finding stands as reported: the no-reorder property still has no rung-2 exit oracle; the rung-2 fixture with the order-inverting pair queues next.
中文说明
延后到下一轮,并非丢弃——本轮批次上限约 8 项,Critical(R5-1)及其配套 oracle 重写(R7-9)优先。本发现按原样成立:不重排性质仍无 rung-2 出口 oracle;带顺序反转对的 rung-2 夹具排入下一轮。
| ['a processing instruction', '<?php\necho 1;\n', '\n?>'], | ||
| ['a markup declaration', '<!ELEMENT review EMPTY\n', '\n>'], | ||
| ['a CDATA section', '<![CDATA[raw data\n', '\n]]>'], | ||
| ])( |
There was a problem hiding this comment.
[Suggestion] R7-6: no fixture exercises a <pre> type-1 html_block — the one CommonMark type-1 opener production's raw1 closer regex names that has zero coverage (grep: only a comment mention at :5937). Verified at this commit: markdown-it parses raw:\n<pre>\ncode line\n\nSENTINEL as one html_block running to EOF, swallowing the appended tail. <pre> is additionally the only type-1 member the browser does NOT swallow, so weaker mutants lose the closer with the notice visibly rendered inside a giant code block — neither shape observed by any test.
Failure scenario: mutation at this commit — deleting pre from the raw1 alternation ships the whole suite green (306/306, measured); a real blocker quoting PR HTML with a <pre> block then overflows into rung 3, falls through to closers: [], and takes the fail-closed branch whose own comment documents 'the posted body was notice and footer only, every blocker dropped' — although the </pre> closer was known. Measured flip: the proposed fixture fails under the mutant and passes pristine.
Fix: one fixture in the textarea pattern: const preBlock = 'raw:\n<pre>\n' + 'code line\n'.repeat(9_000); asserting noticeOutsideCode, toContain('</pre>'), closer-before-notice, and toContain('code line').
中文说明
[建议] R7-6:没有任何夹具演练 <pre> type-1 html_block——它是生产 raw1 闭合器正则点名的 CommonMark type-1 开标签中唯一零覆盖的(grep:仅 :5937 的注释提及)。本提交上验证:markdown-it 把 raw:\n<pre>\ncode line\n\nSENTINEL 解析为一个延伸到 EOF 的 html_block,吞掉追加的尾部。<pre> 还是唯一不被浏览器吞吸的 type-1 成员,因此更弱的变异体只是丢失闭合器、通知会可见地渲染在巨大代码块内部——两种形态都没有任何测试观察。失败场景:在本提交上变异——从 raw1 备选项中删除 pre 整个套件全绿(306/306,实测);真实阻断项引用带 <pre> 块的 PR HTML 时溢出进入 rung 3,落入 closers: [],走其注释自述「发布正文只剩通知和页脚、所有阻断项被丢」的失败关闭分支——尽管 </pre> 闭合器是已知的。实测翻转:建议的夹具在变异下失败、未改动时通过。修复:仿 textarea 形态加一个夹具:const preBlock = 'raw:\n<pre>\n' + 'code line\n'.repeat(9_000);,断言 noticeOutsideCode、toContain('</pre>')、闭合器在通知之前、toContain('code line')。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| 'xmp', | ||
| 'iframe', | ||
| 'noembed', |
There was a problem hiding this comment.
[Suggestion] R7-8: six of the nine RAW_TEXT_ELEMENTS members (style, xmp, iframe, noembed, noframes, noscript) have zero truncation fixtures — grep confirms they appear in the test file only inside the test-side browserSwallowOpen helper's own list and one comment; the suite's only cut fixtures for this family are script (two), textarea (two), title (one). The line-start shapes are doubly fragile: their closer comes from the separate raw1 regex copy in openBlockAtEnd, so the two sets can drift apart green.
Failure scenario: a future edit (e.g. the structural rewrite R5-1 recommends) drops noscript from the set: a blocker quoting a mid-line <noscript> fragment overflows into rung 3 — with the member gone, scanBrowserState never enters the raw state, noticeRidesClean certifies clean, no closer is appended, and the element swallows the notice, footer, and marker on the rendered page. Measured: dropping noscript ships 306/306 green; a mid-line fixture asserting the closer fails under the mutant.
Fix: one it.each over the six members in the mid-line-raw-opener shape (70k filler; assert noticeOutsideCode, the \n</tag> closer before the notice, was TRUNCATED to fit), plus the line-start form for the longest closers (noscript/noframes); extend browserSwallowOpen's swallow list to match.
中文说明
[建议] R7-8:RAW_TEXT_ELEMENTS 九个成员中有六个(style、xmp、iframe、noembed、noframes、noscript)没有任何截断夹具——grep 确认它们在测试文件中只出现在测试侧 browserSwallowOpen 助手自己的列表和一处注释里;套件对该族仅有的切割夹具是 script(两个)、textarea(两个)、title(一个)。行首形态 doubly 脆弱:其闭合器来自 openBlockAtEnd 中另一份 raw1 正则副本,两个集合可以各自漂移而全绿。失败场景:未来某次编辑(例如 R5-1 建议的结构重写)把 noscript 从集合中删掉:引用行中 <noscript> 片段的阻断项溢出进入 rung 3——成员消失后 scanBrowserState 永不进入 raw 状态、noticeRidesClean 认证干净、不补闭合器,渲染页面上该元素吞掉通知、页脚与 marker。实测:删除 noscript 全绿(306/306);断言闭合器的行中夹具在变异下失败。修复:对六个成员加一个行中原始开标签形态的 it.each(70k 填充;断言 noticeOutsideCode、通知之前的 \n</tag> 闭合器、was TRUNCATED to fit),再为最长闭合器(noscript/noframes)加行首形态;同步扩展 browserSwallowOpen 的吞吸列表。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const swallows = [ | ||
| ['<!--', '-->'], | ||
| ['<script', '</script'], | ||
| ['<style', '</style'], | ||
| ['<textarea', '</textarea'], | ||
| ] as const; |
There was a problem hiding this comment.
[Suggestion] R7-9: the block's browser-layer oracle browserSwallowOpen (consumed by noticeOutsideCode at ~16 assertion sites) is a second hand-model of the HTML tokenizer with the same blind-spot family R5-1 names on the source side — it knows only comment/script/style/textarea, so it cannot see plaintext/template/title-RCDATA/xmp/iframe/noscript/noembed swallows, collapsed-hiding <details>, or script-data escape-level nesting. The <title> fixture already bypasses it with manual index assertions because the helper cannot see title elements; no analog exists for the remaining classes. Round 4 also found it prefix-matches end-tag names (</scripter> closes <script) and indexOf-matches closers inside quoted attribute values.
Failure scenario: probe at this commit — a mid-line <plaintext> body: the suite's own oracle noticeOutsideCode returns true (certifies clean) while parse5 places the notice's ancestors as ["#document","html","body","plaintext"] — swallowed on the real HTML tree. A fixture using noticeOutsideCode for such a shape ships green while the PR page renders the tail swallowed.
Fix: apply R5-1's structural fix to the oracle too — parse the rendered HTML with parse5 and assert the notice text node sits outside every swallowing element and outside a non-open <details>; interim: extend the swallows list with the RCDATA/raw-text/plaintext classes and model the script-data escaped states.
中文说明
[建议] R7-9:本块的浏览器层 oracle browserSwallowOpen(被约 16 处断言点的 noticeOutsideCode 消费)是第二个手工 HTML 分词模型,带着与源码侧 R5-1 同族的盲区——只认识 comment/script/style/textarea,看不见 plaintext/template/title-RCDATA/xmp/iframe/noscript/noembed 吞吸、折叠隐藏的 <details>、script-data 转义层级嵌套。<title> 夹具已因助手看不见 title 元素而改用手工索引断言绕过它;其余类别没有对应处理。第 4 轮还发现它前缀匹配结束标签名(</scripter> 会闭合 <script>)、并会在引号属性值内部 indexOf 匹配到闭合器。失败场景:本提交上探针——行中 <plaintext> 正文:套件自己的 oracle noticeOutsideCode 返回 true(认证干净),而 parse5 把通知祖先置为 ["#document","html","body","plaintext"]——在真实 HTML 树上被吞吸。对此形态使用 noticeOutsideCode 的夹具全绿上船,而 PR 页面渲染出的尾部被吞。修复:把 R5-1 的结构修复同样应用到 oracle——用 parse5 解析渲染后的 HTML,断言通知文本节点位于所有吞吸元素之外、且位于非 open 的 <details> 之外;临时措施:把 RCDATA/raw-text/plaintext 类别加入吞吸列表并建模 script-data 转义态。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // Otherwise the block is lost: the cut moves back before the opener | ||
| // and scans again — the rewind can expose an earlier open block. | ||
| cut = cut.slice(0, open.blockStart); |
There was a problem hiding this comment.
[Suggestion] R7-13: the rung-3 loop's final rewind fallback (this line) — the loop's only progress guarantee when no verified closer fits and the shave guard doesn't apply — has zero fixture coverage. The deepTicks fixture's comment claims branch coverage it does not provide: with shortest=13 vs room=12 and cut.length − 1 > blockStart the fixture exercises the SHAVE branch, never the rewind. The fail-closed closers: [] arm that forces the rewind has no coverage either (grep: zero plaintext fixtures).
Failure scenario: mutation at this commit — deleting this rewind line ships 306/306 green with no hang (measured): every fixture resolves via open === null, a chosen closer, or candidate rejection. A regression on this line then ships with no red test: the first real shape that needs it hangs the compose (loop with no progress) or silently spends every blocker before the open block. Correction to the round-5 finding's proposed fixture: a <!DOCTYPE review shape resolves via open === null and does NOT force the rewind; the verified working shape is an unclosed type-6 block at the cut tail: 'evidence line\n<div class="x\n' + 'y'.repeat(80_000) (pristine resolves it; under the rewind-deletion mutant the run hangs to the 120s timeout).
Fix: one fixture of that verified shape asserting termination, noticeOutsideCode, and content survival before the opener.
中文说明
[建议] R7-13:rung-3 循环的最终回退兜底(本行)——当没有可验证的闭合器合适且 shave 守卫不适用时,循环唯一的前进保证——零夹具覆盖。deepTicks 夹具的注释声称了它并未提供的分支覆盖:shortest=13 对 room=12 且 cut.length − 1 > blockStart 时,该夹具走的是 SHAVE 分支,从不走回退。强制回退的失败关闭 closers: [] 臂同样无覆盖(grep:零 plaintext 夹具)。失败场景:在本提交上变异——删除这行回退后 306/306 全绿且不挂起(实测):每个夹具都经由 open === null、选定闭合器或候选拒绝而解。此后该行上的回归将无红测试上船:第一个真正需要它的形态要么让组合挂起(循环失去前进)、要么悄悄消费掉开标签之前的全部阻断项。对第 5 轮发现所提夹具的更正:<!DOCTYPE review 形态经 open === null 解决、并不强制回退;经验证有效的形态是切割尾部未闭合的 type-6 块:'evidence line\n<div class="x\n' + 'y'.repeat(80_000)(原代码可解;删除回退的变异体下运行挂到 120s 超时)。修复:为该经验证形态加一个夹具,断言终止、noticeOutsideCode 与开标签之前内容存活。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| expect(existsSync(paths.out)).toBe(false); | ||
| }); | ||
|
|
||
| it('refuses a present bodyTrim with no `fold` — that shape never shipped', () => { |
There was a problem hiding this comment.
[Suggestion] R7-15: the suite pins missing-field strictness for a present bodyTrim only for fold; the missing-field arms of sections, deferralList, and truncated have zero deciding coverage. The validator code itself is strict for all four fields — only the tests are partial.
Failure scenario: a future 'be kind to partial records' edit granting tolerance to one of the other three fields passes the whole suite (executed mutants: missing-truncated/deferralList/sections tolerance each ship 34 passed | 1 skipped, baseline-identical; control: the identical mutant on fold fails this no-fold test). A corrupted composed JSON lacking that field then saves into the durable artifact with a validator-fabricated default — truncated: false over a body that was truncated — laundering 'a present record that says nothing true', the exact shape the adjacent test comment declares must not exist.
Fix: generalize the no-fold test over all four keys: it.each(['sections', 'deferralList', 'fold', 'truncated']) removing the named key and expecting the throw.
中文说明
[建议] R7-15:套件只为 fold 钉住了「present bodyTrim 缺字段即拒绝」的严格性;sections、deferralList、truncated 的缺字段分支零判定覆盖。验证器代码本身对四个字段都严格——只是测试不全。失败场景:未来某次「对部分记录宽容」的编辑给另外三个字段之一放行容错,整个套件照样通过(实测变异:missing-truncated/deferralList/sections 容错各自 34 通过 | 1 跳过,与基线一致;对照:fold 上的同样变异会使本 no-fold 测试失败)。缺该字段的损坏 composed JSON 随后带着验证器伪造的默认值存进持久工件——被截断的正文上盖 truncated: false——把「一条什么真话都没说的 present 记录」洗白,正是相邻测试注释宣称绝不能存在的形态。修复:把 no-fold 测试推广到全部四个键:it.each(['sections', 'deferralList', 'fold', 'truncated']),删除指名键并期望抛错。
— qwen3.8-max via Qwen Code /review (v0.21.12)
…cape levels The rung-3 cut's page-state certification hand-modelled four HTML5 behaviours the page does not follow, each reproduced against this commit with a parse5 probe: an unclosed <details> hides the notice collapsed, script-data escape levels spend the appended </script> without closing the element, plaintext/template were absent from the swallow set, and a quote in attribute-name position was read as a value delimiter. Track <details> depth with per-level closers, model the two script escape levels, add plaintext/template, and consume quoted spans only after '='. The test oracle is now a real parser (jsdom) instead of a second hand model.
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 4/100 轮)。改动内容与我反驳保留之处如下: Review-feedback round summary — PR #9247Commit: Feedback points and dispositions[rc:3793216282] R5-1 (Critical, CHANGES_REQUESTED) — fixed (interim prescription), structural rewrite deferred. Reproduced first, on this commit, with a markdown-it → parse5 probe over composed bodies: (a) the truncation notice rendered hidden inside an unclosed collapsed Changes (the finding's interim prescription, plus the escape-level class):
Eight new cut fixtures cover the reproduced classes; all eight FAILED against the pre-fix code and pass now. The structural fix the finding prefers — a real HTML5 tokenizer certifying the tail in production code — requires declaring parse5/jsdom as a production dependency (package.json + lockfile), a supply-chain change outside this PR's footprint; it is recorded in [rc:3793216301] R7-9 — resolved. The test-side oracle [rc:3793216292] R7-5 — resolved. The [rc:3793216295] R7-6 — resolved. New [rc:3793216299] R7-8 — resolved. One [rc:3793216302] R7-13 — resolved. New fixture for the rung-3 loop's final rewind fallback: an unclosed type-6 [rc:3793216290] R6-1 — resolved. The reserve fixture is re-sized so its post-rank-drop body lands IN the reserve window (56,830 < body ≤ 65,024: 59,600-char blocker + four 3k disclosures, PR-named plan) and now asserts [rc:3793216304] R7-15 — resolved. The no-fold strictness test is generalized to Deferred to the next round (batch cap ~8, Critical first) — replies posted on each thread:
[rv:4947724711] review body (CHANGES_REQUESTED) — carries no findings, only the automated reviewer's own coverage disclosures (Integration Tests skipped in CI, reverse audit not converged within its round cap). Nothing actionable in code. Conflict notesNone — VerificationCommands actually run, in order:
中文说明评审反馈轮次总结 — PR #9247提交: 反馈点与处置[rc:3793216282] R5-1(Critical,CHANGES_REQUESTED)— 已修复(临时处方),结构性重写延后。 先在本提交上复现:用 markdown-it → parse5 探针对组合出的正文实测:(a) 截断通知被渲染隐藏在未闭合折叠的 改动(本发现的临时处方 + 转义层级类别):
八个新切割夹具覆盖已复现类别;全部八个在修复前代码上失败、修复后通过。本发现更倾向的结构性修复——在生产代码中用真正的 HTML5 分词器认证尾部——需要把 parse5/jsdom 声明为生产依赖(package.json + lockfile),属于本 PR 足迹之外的供应链变更;已记入 [rc:3793216301] R7-9 — 已解决。 测试侧 oracle [rc:3793216292] R7-5 — 已解决。 [rc:3793216295] R7-6 — 已解决。 新增 [rc:3793216299] R7-8 — 已解决。 对六个未有夹具的 [rc:3793216302] R7-13 — 已解决。 为 rung-3 循环的最终回退兜底新增夹具:切割尾部未闭合的 type-6 [rc:3793216290] R6-1 — 已解决。 预留夹具重新定尺,使其 rank 丢弃后的正文落入预留窗口(56,830 < 正文 ≤ 65,024:59,600 字符阻断项 + 四条 3k 披露、PR 命名 plan),现断言 [rc:3793216304] R7-15 — 已解决。 no-fold 严格性测试推广为 延后到下一轮(批次上限约 8 项,Critical 优先)— 各线程已回复:
[rv:4947724711] 评审正文(CHANGES_REQUESTED) — 不含发现,只有自动审查器自身的覆盖披露(CI 中集成测试被跳过、反向审计在其轮数上限内未收敛)。代码层面无需处理。 冲突说明无 — 验证实际执行的命令,按顺序:
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:2787 — [review] R5-3 still stands: keep:1 on unverifiedTagsBlock has no rung-3 truncation-survival oracle (deletion mutation ships green)packages/cli/src/commands/review/compose-review.ts:2759 — [review] R5-10 still stands: trim:2 on deferredBlock/testPlanBlock has no overflow oracle (both mutations ship green)packages/cli/src/commands/review/compose-review.ts:2739 — [review] R7-2 still stands: keep:1 on nothingCertified arm / 'Suggestions are inline.' / suggestions-dropped clause has no truncation oraclepackages/cli/src/commands/review/compose-review.test.ts:5720 — [review] R5-14 still stands: the no-reorder oracle pins only the fold-only exit, not the rung-2 rank-drop exitpackages/cli/src/commands/review/compose-review.ts:685 — [review] R7-8 still stands: line-start raw1 fixtures still missing for style/xmp/iframe/noembed (+title); the two element copies can drift greenpackages/cli/src/commands/review/compose-review.ts:790 — [review] D8-1: the 'tag' resume arm inherits a pending raw-text opener — wrong swallow attribution feeds over-rewind (ship-safety fails closed)packages/cli/src/commands/review/compose-review.test.ts:5899 — [review] D8-2: tilde/list-nested/TAIL-sentinel fence fixtures lack content-survival oracles — a rewind-discard ships greenpackages/cli/src/commands/review/compose-review.test.ts:6122 — [review] D8-3: mid-line RAW_TEXT/template fixtures' ordering assertion is vacuously true when the closer is absentpackages/cli/src/commands/review/compose-review.test.ts:5288 — [review] D8-4: noticeOutsideCode walks Text nodes only — blind to a comment-node swallow when the body quotes the phrase earlierpackages/cli/src/commands/review/compose-review.test.ts:7095 — [review] D8-5: the over-cap cannot-tell test lacks a capped-entry survival oracle — a drop mutation ships greenpackages/cli/src/commands/review/compose-review.test.ts:6007 — [review] D8-6: the script-escape-level fixture passes green via rewind-discard under the live c1 divergence
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:reverse audit — did not converge within the reverse-audit round cap of 5。
收敛姿态下延后(第 8 轮,非阻断)——已记录,本轮不要求修改:共 11 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| name += s.charAt(k).toLowerCase(); | ||
| k++; | ||
| } | ||
| if (name === 'details') depth = Math.max(0, depth - 1); |
There was a problem hiding this comment.
[Critical] R5-1 (class-level re-report, carried id from the round-5 ledger): the rung-3 cut's page-state certification — noticeRidesClean / scanBrowserState / openBlockAtEnd — still hand-models how the posted PR page renders over model-written blocker markdown. This is an UNBOUNDED surface (a re-implementation of HTML5 tokenization and tree construction), disproved in every round since round 3. Round 7's interim fix landed at this commit — and this round falsifies the class with eight NEW executed divergence classes:
- (c1) a matching
</script>at escape level 1 demotes a level instead of closing the element (~:839) - (c2) the comment-end recognizer accepts only
-->/--!>, not the HTML5 dash/bang runs--->,---->,--!->(~:761) - (c3)
templateinRAW_TEXT_ELEMENTS— browsers tokenize template content as data-state HTML (~:696) - (c4) the raw arm indexes the original string with offsets taken from a
toLowerCase()copy —'İ'expands to 2 code units, shifting every offset into phantom closers (~:752) - (c5)
skipTagBodytreats any=as a value introducer; HTML5 treats=in before-attribute-name position as an attribute-NAME character (~:728) - (c6)
isTagNameChartruncates tag names at '.', so dotted names (</details.foo>,<style.foo>) misclassify (~:706) - (c7) the raw-text model applies inside
<svg>/<math>foreign content, where HTML5 has no raw-text states (~:939) - (c8, anchored here) the details-depth counter decrements unconditionally on
</details>, while HTML5 tree construction IGNORES that end tag under an unclosed<table>/<select>/<object>/<marquee>above it (~:917)
— Failure scenario: a body Critical quoting code overflows into rung 3 with the cut landing after one of these openers (e.g. a details fold containing a hand-written unclosed table — the shape of this repo's own benchmark folds) → noticeRidesClean certifies the tail clean → no closer/rewind → the truncation notice, footer and ledger marker render invisible on the PR page and the author reads a truncated body as a complete review — the exact failure this machinery exists to prevent.
Witness (probes at this commit, markdown-it + jsdom/parse5 oracle):
c1: noticeRidesClean('<script><!-- </script> <style> </script>','') → true;
jsdom sentinel INSIDE <style> in <head>, body empty
c8: parse5 '<details><table></details><p>NOTICE' → inside-details: true
(control with closed table: outside)
E2E c8 via composeReview: 65,012-char body ships, oracle notice.visible=false,
ancestor chain p > td > tr > tbody > table > details
minimal fail-closed patch flips every probe to visible; suite stays green
Suggested fix (structural — entrance-by-entrance patching produced a new divergence class in every round 3–8): certify the tail with a real HTML5 tokenizer (parse5 is in the tree transitively via jsdom) over the actually-rendered HTML, or fail closed whenever any raw-text/swallow/foreign-content/mode-blocking opener precedes the cut without real-parser certification.
中文说明
[Critical] R5-1(类级复报,沿用第 5 轮账本 id):rung-3 切割的页面状态认证——noticeRidesClean / scanBrowserState / openBlockAtEnd——仍在手工建模「发布后的 PR 页面如何渲染模型写出的阻断项 markdown」。这是一个无界表面(对 HTML5 分词与树构造的重新实现),自第 3 轮起每轮都被证伪。第 7 轮的临时修复已落在本提交——本轮又实测出八个新的分歧类别:(c1) 转义层级 1 上匹配的 </script> 只降级不闭合(:839);(c2) 注释结束识别器只接受 :761);(c3) -->/--!>,不接受 HTML5 的连字符/感叹号串 --->、---->、--!->(RAW_TEXT_ELEMENTS 中的 template——浏览器按数据状态对 template 内容分词(:696);(c4) raw 分支用 :752);(c5) toLowerCase() 副本的偏移去索引原字符串——'İ' 展开为 2 个码元,令所有偏移错位、产生幻影闭合器(skipTagBody 把任何 = 都当作值引导符,而 HTML5 在「属性名之前」位置把 = 当作属性名字符(:728);(c6) :706);(c7) raw-text 模型被应用于 isTagNameChar 在 '.' 处截断标签名,带点名称(</details.foo>、<style.foo>)被误分类(<svg>/<math> 外来内容内部,而 HTML5 在外来内容中没有 raw-text 状态(:939);(c8,锚点所在行) details 深度计数器在 :917)。— 失败场景:引用代码的 body Critical 溢出进入 rung 3、切割落在上述某个开标签之后(例如包含手写未闭合表格的 details 折叠——本仓库自身基准折叠的形态)→ </details> 上无条件递减,而 HTML5 树构造在上方存在未闭合 <table>/<select>/<object>/<marquee> 时会忽略该结束标签(noticeRidesClean 认证尾部干净 → 无闭合器/无回退 → 截断通知、页脚与 ledger marker 在 PR 页面上不可见,作者把被截断的正文当作完整评审阅读——正是这套机制本要防止的失败。证据(本提交上的探针,markdown-it + jsdom/parse5 预言机):c1 noticeRidesClean('<script><!-- </script> <style> </script>','') → true,jsdom 哨兵位于 的 <style> 内、body 为空;c8 parse5 '<details><table></details><p>NOTICE' → 通知位于 details 内(闭合表格对照组在外);E2E 经 composeReview:65,012 字符正文发出,oracle notice.visible=false,祖先链 p > td > tr > tbody > table > details;最小失败关闭补丁可翻转全部探针且套件保持全绿。建议修复(结构性——逐入口打补丁已在第 3–8 轮每轮产生一个新分歧类别):用真正的 HTML5 分词器(parse5 已经 jsdom 传递性地在依赖树中)对实际渲染出的 HTML 认证尾部,或在任何 raw-text/吞吸/外来内容/模式阻塞开标签未被真解析器认证时失败关闭。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const closers = [`\n${prefix}${run}`]; | ||
| if (prefix) closers.push(`\n${run}`); | ||
| return { closers, blockStart }; |
There was a problem hiding this comment.
[Critical] The markdown-layer closer arms of openBlockAtEnd (this fence arm and every html_block arm: comment, <?, CDATA, <!a, raw1) return candidate closers WITHOUT the '\n</details>'.repeat(detailsDepth) suffix the browser-state arm appends (~:1023). With an unclosed <details> in the cut, noticeRidesClean (which requires detailsDepth === 0) rejects every candidate, the shave branch never fires (rejection by gate, not by room), and the rewind discards the ENTIRE swallowing block even when ~64k of budget sits unused — spending the keep-2 body Critical's only-copy evidence first, and making the posted hardNote's claim false. — Failure scenario: a body Critical of a fenced code block (or <script>/<style> html_block) wrapped in <details> — the repo's own fold idiom, which model-written findings quote — is cut inside the fence → the posted body is 355–645 chars with ALL quoted evidence gone, while the identical block without the wrapper keeps its 65,016-char content through the same cut.
Witness (E2E through composeReview at this commit):
wrapped: len=626 keepsFenceContent=false
control (no wrap): len=65016 keepsFenceContent=true
line-start <script>: len=626 keepsScriptContent=false
fix arm: len=65024 keepsFenceContent=true (suite 324/324 green; reverted)
Suggested fix: run the same scanBrowserState probe the swallow-null arm already performs over the rendered cut, and append '\n</details>'.repeat(scan.detailsDepth) to every markdown-layer candidate; add a combination fixture (fence inside an unclosed <details>, asserting the fence content survives).
中文说明
[Critical] openBlockAtEnd 的 markdown 层闭合臂(此 fence 臂以及全部 html_block 臂:注释、<?、CDATA、<!a、raw1)返回的候选闭合器不带浏览器状态臂(~:1023)所附加的 '\n</details>'.repeat(detailsDepth) 后缀。当切割中存在未闭合的 <details> 时,noticeRidesClean(要求 detailsDepth === 0)拒绝所有候选,shave 分支永不触发(这是门拒绝而非余量拒绝),即便预算还剩约 64k,回退也会丢弃整个吞吸块——先消耗 keep-2 body Critical 的唯一副本证据,并使发出的 hardNote 声明失真。— 失败场景:被 <details> 包裹的 fenced 代码块(或 <script>/<style> html 块)body Critical——本仓库自身的折叠写法,模型写出的发现经常引用——切割落在 fence 内部 → 发出正文仅 355–645 字符、全部引用证据丢失,而无包裹的相同块经同一切割保留 65,016 字符内容。证据(本提交上经 composeReview 的 E2E):包裹 len=626 keepsFenceContent=false;对照(无包裹)len=65016 keepsFenceContent=true;行首 <script> len=626 keepsScriptContent=false;修复臂 len=65024 keepsFenceContent=true(套件 324/324 全绿,已还原)。建议修复:对渲染后的切割执行 swallow-null 臂已在做的同一 scanBrowserState 探测,把 '\n</details>'.repeat(scan.detailsDepth) 附加到每个 markdown 层候选上;增加组合夹具(未闭合 <details> 内的 fence,断言 fence 内容存活)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| let swallow: (typeof tokens)[number] | null = null; | ||
| for (let i = tokens.length - 1; i >= 0; i--) { |
There was a problem hiding this comment.
[Critical] This backward swallow search skips the tail's own inline token and keeps scanning, so when the cut QUOTES the NOTICE_SENTINEL literal inside an earlier CLOSED fence/html_block it attributes the swallow to that closed block — the one sentinel read in this module without the tail-occurrence guard that noticeRidesClean (break on inline) and this function's own render scan (html.lastIndexOf) both apply. The closed fence's closer fails verification, no shave fires, and the rewind deletes everything from the quoted fence to the end — including the actual swallower and every blocker after it. The module's own docstring anticipates this input ('every read resolves the TAIL occurrence'); the trigger is self-hosting, because the sentinel literal lives in this file's code and fixtures. — Failure scenario: cut = 'See this log:\n\nQWENREVIEWCUTSENTINEL\n\n\nfoo <style>' → the loop picks the closed quoted fence instead of returning null to the raw branch; the caller rewinds past the real swallower and every blocker after it, instead of the raw branch's one-paragraph rewind + \n</style> close.
Witness (executed at this commit):
PR: len=599 keepsK=false keepsQuotedFence=false styleCloser=false
FIX: len=65021 keepsK=true keepsQuotedFence=true styleCloser=true
single-variable arms: inline-break patch alone flips this finding
(details-closer patch alone leaves it broken, and vice versa)
Suggested fix:
for (let i = tokens.length - 1; i >= 0; i--) {
const tok = tokens[i];
if (!tok.content.includes(NOTICE_SENTINEL)) continue;
if (tok.type === 'inline') break; // tail rides clean at the markdown layer
if (tok.type === 'fence' || tok.type === 'html_block') { swallow = tok; break; }
}中文说明
[Critical] 这个向后吞吸搜索会跳过尾部自身的 inline token 继续向前扫描,因此当切割引用了更早的某个已闭合 fence/html_block 中的 NOTICE_SENTINEL 字面量时,它会把吞吸归因到那个已闭合的块——这是本模块中唯一一处没有「尾部出现位置」守卫的哨兵读取,而 noticeRidesClean(在 inline 处 break)与本函数自身的渲染扫描(html.lastIndexOf)都有该守卫。已闭合 fence 的闭合器无法通过验证,shave 不触发,回退会删除从被引用 fence 直到末尾的一切——包括真正的吞吸者及其后的每个阻断项。模块自身的 docstring 已预期这种输入(「每次读取都解析尾部出现位置」);触发是自宿主的,因为哨兵字面量就存在于本文件的代码与夹具中。— 失败场景:cut = 'See this log:\n\nQWENREVIEWCUTSENTINEL\n\n\nfoo <style>' → 循环选中已闭合的被引用 fence 而不是向 raw 分支返回 null;调用方回退越过真正的吞吸者及其后的每个阻断项,而不是 raw 分支的单段落回退 + \n</style> 闭合。证据(本提交上执行):PR len=599 keepsK=false keepsQuotedFence=false styleCloser=false;修复 len=65021 全部保留;单变量臂证明:仅 inline-break 补丁即可翻转本发现(details-closer 补丁单独无效,反之亦然)。建议修复:见英文部分代码——在尾部 inline token 处 break,镜像 noticeRidesClean。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // A plaintext swallow has no closer, and an unrecognised state fails | ||
| // closed over the whole cut rather than ship an unverified closer. | ||
| return { closers: [], blockStart }; |
There was a problem hiding this comment.
[Critical] The browser-state arm proposes no closer for end.kind === 'tag' — a tag left open with an unclosed quoted attribute value at the cut — even though \n"> / \n'> / \n> are gate-verifiable closers inside CLOSER_RESERVE (12). The fail-closed comment is misplaced: every candidate is verified by noticeRidesClean before shipping, so proposing is safe; plaintext is the only state with no closer. Combined with the detailsLine latch finding below, a 14-char salvage becomes total loss (final cut length 0). — Failure scenario: blocker \n<div class="a">\n<span title=" + V×80,000 (cut lands mid quoted attribute) → the final body is 299 chars — no <div class="a">, no Vs, total loss of the blocker — although noticeRidesClean(cut, '\n">') returns TRUE.
Witness (E2E at this commit):
PR: len=299 keeps '<div class="a">'=false
patch (arm proposing \n">/\n'/>/\n> + detailsCloser):
len=65015 keeps=true, notice visible (p > span > div > body > html)
reverted
Suggested fix: add an arm before the fail-closed return: if (end.kind === 'tag') return { closers: ['\n">', "\n'>", '\n>'].map((c) => \${c}${detailsCloser}`), blockStart };— the gate rejects the wrong quote variant; keepclosers: []` for plaintext.
中文说明
[Critical] 浏览器状态臂对 end.kind === 'tag'(切割处留下带未闭合引号属性值的标签)不提出任何闭合器——尽管 \n"> / \n'> / \n> 都是 CLOSER_RESERVE(12)之内、可经门验证的闭合器。失败关闭的注释放错了位置:每个候选在发出前都由 noticeRidesClean 验证,提出候选是安全的;plaintext 才是唯一没有闭合器的状态。与下方 detailsLine 锁存发现叠加时,14 字符的挽救会变成全损(最终切割长度为 0)。— 失败场景:阻断项 \n<div class="a">\n<span title=" + V×80,000(切割落在引号属性中间)→ 最终正文 299 字符——无 <div class="a">、无 V、阻断项全损——而 noticeRidesClean(cut, '\n">') 返回 TRUE。证据(本提交上 E2E):PR len=299 keeps=false;补丁臂(提出 \n">/\n'/>/\n> + detailsCloser)len=65015 keeps=true、通知可见;已还原。建议修复:在失败关闭返回之前增加一个臂(见英文部分),门会拒绝错误的引号变体;plaintext 保持 closers: []。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const openLines = [openLine, detailsLine].filter((l) => l !== -1); | ||
| const blockStart = lineStart( |
There was a problem hiding this comment.
[Critical] detailsLine latches the FIRST-ever depth increase (detailsLine === -1 && r.detailsDepth > depth), so a closed-then-reopened <details> pins it to the ALREADY-CLOSED fold, and Math.min(openLine, detailsLine) retreats to that line even when openLine alone suffices. The rewind fires on any browser-branch failure (the missing tag arm above, a fail-closed nested-opener html_block such as a first line <div><style>, a plaintext swallow, or a gate rejection). — Failure scenario: body = <details> fold one </details>, evidence paragraph, reopened <details> (line 8), then a type-6 html_block cut mid quoted attribute (line 12) → blockStart 0 (the line of the CLOSED fold) → the loop discards all 127 chars (final cut length 0), while the module's own gate certifies three cheaper salvages — whole cut + \n">\n</details> (14 chars, the shave branch pays the 2-char reserve delta), cut.slice(0, lineStart(12)) + \n</details>, cut.slice(0, lineStart(8)) + \n</details>.
Witness (E2E at this commit):
PR: len=283 keeps 'fold one'=false keeps 'evidence paragraph'=false (total loss)
stack patch: len=371 keeps both, notice visible
independence: tag-arm patch alone fixes the finding above but leaves this at 283;
stack patch alone fixes this but leaves the above at 299 — both reverted
Suggested fix: replace the latch with a stack of opener lines (push when r.detailsDepth > depth, pop when it drops, read the bottom), and compute blockStart from openLine !== -1 ? openLine : detailsLine instead of Math.min.
中文说明
[Critical] detailsLine 锁存首次深度增加(detailsLine === -1 && r.detailsDepth > depth),因此「闭合后再打开」的 <details> 会把它钉在已闭合的折叠上,Math.min(openLine, detailsLine) 即便 openLine 单独已足够也会退到那一行。任何浏览器分支失败都会触发回退(上面缺失的 tag 臂、失败关闭的嵌套开标签 html_block 如首行 <div><style>、plaintext 吞吸、门拒绝)。— 失败场景:正文 = <details> 折叠一 </details>、证据段落、重新打开的 <details>(第 8 行)、随后在引号属性中间被切割的 type-6 html_block(第 12 行)→ blockStart 0(已闭合折叠的行)→ 循环丢弃全部 127 字符(最终切割长度 0),而模块自身的门认证了三个更便宜的挽救——整个切割 + \n">\n</details>(14 字符,shave 分支支付 2 字符预留差)、cut.slice(0, lineStart(12)) + \n</details>、cut.slice(0, lineStart(8)) + \n</details>。证据(本提交上 E2E):PR len=283 两者皆失(全损);栈补丁 len=371 两者保留、通知可见;独立性:仅 tag 臂补丁修复上一条但本条仍 283;仅栈补丁修复本条但上一条仍 299——均已还原。建议修复:把锁存换成开标签行栈(深度增加时 push、减少时 pop、读栈底),blockStart 用 openLine !== -1 ? openLine : detailsLine 计算而非 Math.min。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (raw1) { | ||
| return { closers: [`\n</${raw1[1].toLowerCase()}>`], blockStart }; | ||
| } |
There was a problem hiding this comment.
[Critical] A budget cut landing INSIDE a start tag (before its >) proposes only the element's end tag, which the browser absorbs into the still-open start tag; the gate correctly rejects it and no salvage candidate is offered, so the caller always rewinds the whole block. Distinct from the end.kind === 'tag' finding above (unclosed QUOTED attribute): unquoted/no-attribute cuts surface as kind raw/data and bypass that fix. — Failure scenario: cut 'Intro.\n\n<script' → openBlockAtEnd returns closers ["\n</script>"] with gate [false] (the closer's own > completes the open start tag; </script becomes attribute junk, the element opens and swallows the notice); shortest 11 ≤ reserve 12 so no shave; the caller rewinds to 'Intro.\n\n'. Same for '<details' and '<details class=x'.
Witness (unit + E2E at this commit):
gate(cut, '\n</script>')=false
gate(cut, '\n<x>\n</script>')=true ← gate-verifiable salvage, never offered
gate(cut, '\n>\n</script>')=true
E2E A/B: shape A (cut 5 chars inside '<script aaa…') ships without the block
(scriptPresent=false); shape B (cut 28 chars later, past the completed
start tag) keeps it with the verified closer
Severity note, stated honestly: because the cut is a prefix, the block's remaining content was never in the cut — the rewind's net loss is bounded to the partial start tag's own lines, and the direction is fail-safe (never ships a swallowed notice). The defect is unnecessary loss where a verified salvage exists, in the same class as the findings above.
Suggested fix: when the scan shows the swallow opener's start tag never completed (the closer candidate's own > would be the first >), also propose a two-part closer shipping a raw > before the end tag — '\n<x>\n</' + tag + '>' (and '\n>\n</details>' while a type-6/7 html_block is still open to carry it raw) — so the gate can verify and the shave branch can fund it.
中文说明
[Critical] 预算切割落在开始标签内部(其 > 之前)时只提出该元素的结束标签,而浏览器会把它吸收进仍未闭合的开始标签;门正确地拒绝了它,且没有提供任何挽救候选,于是调用方总是回退整个块。与上方 end.kind === 'tag' 的发现(未闭合引号属性)不同:无引号/无属性的切割以 kind raw/data 出现,绕过那个修复。— 失败场景:cut 'Intro.\n\n<script' → openBlockAtEnd 返回 closers ["\n</script>"]、门 [false](闭合器自身的 > 补全了未闭合的开始标签;</script 变成属性垃圾,元素随后打开并吞掉通知);shortest 11 ≤ 预留 12 故无 shave;调用方回退到 'Intro.\n\n'。'<details' 与 '<details class=x' 同样。证据(本提交上单元 + E2E):gate(cut,'\n</script>')=false,而 gate(cut,'\n\n</script>')=true(可经门验证的挽救,从未被提出)、gate(cut,'\n>\n</script>')=true;E2E A/B:形态 A(切割在 '<script aaa…' 内 5 字符)发出时块丢失(scriptPresent=false);形态 B(切割晚 28 字符、越过已完成的开始标签)以验证过的闭合器保留该块。严重度说明(如实陈述):由于切割是前缀,块的其余内容本就不在切割中——回退的净损失限于部分开始标签自身的若干行,且方向是失败安全的(绝不发出被吞吸的通知)。本缺陷属于「存在已验证挽救却仍不必要丢失」的类别,与上述发现同类。建议修复:当扫描显示吞吸开标签的开始标签从未完成(闭合器候选自身的 > 将是第一个 >)时,同时提出一个两段式闭合器——在结束标签之前先发一个原始 >,如 '\n<x>\n</' + tag + '>'(当仍有 type-6/7 html_block 承载时用 '\n>\n</details>')——让门可以验证、shave 分支可以支付。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (child.type === 'html_inline') { | ||
| frags.push({ content: child.content, line: tok.map?.[0] ?? 0 }); | ||
| } |
There was a problem hiding this comment.
[Critical] Every html_inline fragment is attributed to the FIRST line of its inline token (tok.map?.[0] ?? 0 — markdown-it inline children carry no line maps), so an inline swallower on a LATER line of a multi-line paragraph rewinds to the paragraph start — discarding clean lines. The code's own comment in this arm promises 'the rewind loses the swallower, not the whole cut'; the measured outcome lost the whole cut. The same early attribution feeds detailsLine for inline <details>. — Failure scenario: cut 'Line one of the paragraph stays.\nLine two has <plaintext> inline.' — markdown layer clean, browser arm finds end.kind 'raw' tag plaintext (no closer exists, correctly) but returns blockStart 0 instead of offset 33; cut.slice(0, 33) rides clean (gate true) and could have shipped — instead the entire ~64.7k cut is discarded and the body is tail-only.
Witness (unit + E2E at this commit):
openBlockAtEnd(…) → {closers:[], blockStart:0} (correct blockStart=33)
gate('Line one of the paragraph stays.\nLine two has ','') = true
E2E: len=283, lineOneSurvives=false — the entire cut discarded
Suggested fix: track a per-inline-token source cursor — for each html_inline child, text.indexOf(child.content, cursor) within the token's map range, count newlines to derive its line, advance the cursor; fall back to tok.map[0] only if not found.
中文说明
[Critical] 每个 html_inline 片段都被归因到其 inline token 的第一行(tok.map?.[0] ?? 0——markdown-it 的 inline 子节点不带行映射),因此多行段落后面某行上的 inline 吞吸者会回退到段落开头——丢弃干净的行。本分支代码自身的注释承诺「回退丢失的是吞吸者,而不是整个切割」;实测结果丢掉了整个切割。同样的过早归因也作用于 inline <details> 的 detailsLine。— 失败场景:cut 'Line one of the paragraph stays.\nLine two has <plaintext> inline.'——markdown 层干净,浏览器臂发现 end.kind 'raw' tag plaintext(正确:不存在闭合器),但返回 blockStart 0 而不是偏移 33;cut.slice(0, 33) 可干净搭载(门为 true)本可以发出——结果整个约 64.7k 的切割被丢弃,正文只剩尾部。证据(本提交上单元 + E2E):openBlockAtEnd(…) → {closers:[], blockStart:0}(正确应为 33);门对前 33 字符前缀为 true;E2E len=283,lineOneSurvives=false——整个切割被丢弃。建议修复:为每个 inline token 维护一个源码游标——对每个 html_inline 子节点,在 token 的 map 范围内 text.indexOf(child.content, cursor),按换行计数推导其行号并推进游标;仅当找不到时回退到 tok.map[0]。
— qwen3.8-max via Qwen Code /review (v0.21.13)
# Conflicts: # packages/cli/src/commands/review/compose-review.test.ts # packages/cli/src/commands/review/compose-review.ts # packages/core/src/skills/bundled/review/SKILL.md
…lasses Close the eight falsified classes in the page-state scan: script end tags close at escape level 1, comment ends accept dash and bang runs, offsets index the original string (no folded copy), a bare '=' is an attribute-name character, tag names run to whitespace/slash/'>' (raw closers take the attribute tail), template content is data state, and foreign content or a still-open mode blocker fails closed. Salvage what the gate can verify: markdown-layer closers carry the details/template suffixes, the dominant browser state's closers ride every swallowed block's candidates, the swallow search stops at the tail's inline token, inline fragments attribute to their own line, opener-line stacks replace the first-increase latch, cut-open start tags and unclosed quoted attributes get gate-verifiable completions, and the reserve funds the cheapest VERIFIED closer, not the shortest text.
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 5/100 轮)。改动内容与我反驳保留之处如下: Round summary — PR #9247 (address-review, round 9)Critical-only mode is active (diff-growth engagement). All seven Critical Conflict resolution (
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped at the round cap of 5 without converging (round 5 reported findings; every round 1–5 audited its chunks).
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:1270 — [review] stale openLine: a closed swallow still pins blockStart → over-rewind (probe-confirmed, fail-closed excess loss)packages/cli/src/commands/review/compose-review.ts:1270 — [review] no opener-line attribution for uncertain causes (open mode blocker / foreign) → rewind to line 0 discards the whole headpackages/cli/src/commands/review/compose-review.test.ts:5834 — [review] low-surrogate fixture junction assertion has only 4,714 chars of slack — spurious red on a PR-named-plan variantpackages/cli/src/commands/review/compose-review.ts:1254 — [review] table-cell html_inline fragments attributed to the first occurrence anywhere → over-rewind when no closer fits (R8-6 descendant)packages/cli/src/commands/review/compose-review.test.ts:6894 — [review] c7/c8 fixtures pass green on a total-loss body — no content-survival assertion (probe: 283-char bodies)packages/cli/src/commands/review/compose-review.test.ts:5854 — [review] lone-high-surrogate fixture fails SILENT under the same 4.7k slack — strip-loop mutant ships green on the plan variantpackages/cli/src/commands/review/compose-review.test.ts:5582 — [review] oracle walks doc.body only — a notice swallowed into template.content is invisible (latent)packages/cli/src/commands/review/compose-review.ts:2795 — [review] untagged duplicatesBlock/continuityBlock bypass rung 2; rung-3 keep-sort spends cannotTell first — inverted cost order (probe-confirmed)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — stopped at the round cap of 5 without converging (round 5 reported findings; every round 1–5 audited its chunks)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后(第 9 轮,非阻断)——已记录,本轮不要求修改:共 8 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
…y budget (#9247) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 6/100 轮)。改动内容与我反驳保留之处如下: Round summary — PR #9247 (address-review, round 10)Critical-only mode is active (5 change-producing rounds complete; diff growth src 721 / test 839 vs 400/400 budgets). Only the two Critical inline findings were actionable this round; the Feedback points and dispositions[rv:4952504261] CHANGES_REQUESTED — "Partially reviewed — gaps disclosed"Container review. Its actionable content is the two Critical inline findings below; its "Not reviewed" disclosures (reverse audit stopped at the round cap; Integration Tests CLI/No-Sandbox skipped in CI and not run locally) and its eight round-9 deferred items are explicitly "recorded, not requested in this round" — no code action taken on them. [rc:3797095873] Critical R9-1 — the last-resort tail is never measured against the body budget → FIXEDReproduced on the pre-round commit with focused failing tests before changing anything:
Root cause: the footer interpolates Regression tests (all three failed on the pre-round code — the verification gate's pre-round check):
[rc:3797095867] Critical R5-1 (class-level re-report) — hand-modeled HTML5 page-state certification → ESCALATED, thread left UNRESOLVEDVerified real by source inspection at this commit (three of the seven corners read directly in code): corner 5 — Not implemented this round — this is a maintainer's decision, not mine:
The escalation with options and a recommendation is posted as a reply on the finding's thread ( Changes made
Diff: +81 / −1 lines (source +23 incl. doc comments, tests +59) — subtractive in spirit: bounding the contributor the budget rationale missed, instead of adding a new measurement rung. Conflict notes
Verification
中文说明轮次总结 — PR #9247(address-review,第 10 轮)仅处理 Critical 的模式已生效(已完成 5 个产生改动的轮次;diff 增长 源码 721 / 测试 839,预算 400/400)。本轮可操作的仅有两条 Critical 内联发现; 反馈点与处置[rv:4952504261] CHANGES_REQUESTED — "Partially reviewed — gaps disclosed"(部分审查,缺口已披露)容器评审。其可操作内容即下方两条 Critical 内联发现;其中的 "Not reviewed"(未审查)披露(reverse audit 在轮次上限处停止;Integration Tests CLI/No-Sandbox 在 CI 中被跳过且本地未运行)以及第 9 轮的八条延后条目均明确为「已记录、本轮不要求修改」——未对它们采取代码行动。 [rc:3797095873] Critical R9-1 — 末位 tail 从未对 body 预算计量 → 已修复在改动任何代码之前,先在本轮之前的提交上用聚焦失败测试复现:
根因:页脚逐字内嵌 回归测试(三条在本轮之前的代码上全部失败 — 满足验证门的前置检查):
[rc:3797095867] Critical R5-1(类级复报)— 手工建模的 HTML5 页面状态认证 → 已上报,线程保持未解决通过本提交上的源码检视确认属实(七个角落中三个可直接在代码中读到):角落 5 — 本轮未实施 — 这是维护者的决策,不是我的:
带有选项与建议的上报已作为回复发布在该发现的线程中( 改动内容
Diff:+81 / −1 行(源码 +23 含文档注释,测试 +59)— 精神上减法:给预算理由清单遗漏的贡献者设限,而不是新增一层计量。 冲突说明
验证
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete and the PR's diff grew src 721 / test 839 net lines beyond this counting window's baseline (budgets: 400/400). The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式:已完成 5 个产生改动的轮次,且本计数窗口内 diff 净增长已达 源码 721 / 测试 839 行(预算 400/400)。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
…page model R5-1, re-reported as one class across five rounds: the rung-3 cut certified its trailing notice against a hand model of how GitHub renders the page — markdown block grammar plus HTML5 tokenization, tree construction, raw-text and escape states, `<template>` inert content. That surface is unbounded. Three models were tried and each shipped a new class of divergence; round 9 named seven more corners at the previous commit. The notice does not need to be down there. Every other disclosure this budget writes already rides at the TOP of the body — the fold notice, the trim notice — precisely so nothing below can swallow them. Moving the truncation notice up with them removes the question the model existed to answer: above the cut, no open construct can reach it, and the most an unbalanced fence can still absorb is the footer's attribution line, which says nothing the review needs. The ledger marker is read from raw text and never rendered, so it is unaffected either way. Deleted with it: `openBlockAtEnd`, `noticeRidesClean`, `scanBrowserState` and their tables, the closer/shave/rewind loop, the `markdown-it` parse at compose time, and the 23 fixtures that pinned the model's corners. What remains of the last resort is a prefix cut, a surrogate-strip loop, and a subtraction — 1,534 lines lighter. The tail's own bound stays where the previous commit put it: `reviewFooter` caps `modelId`, so the subtraction can never empty the cut. A second guard here would be a branch for a state that can no longer occur.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- compose-review.test.ts:6004 — the no-reorder pin guards only the rung-1 exit (no rung-2 counterpart) — already reported as R5-14 (round-5 finding; deferred in the round-8 list)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped at the round cap of 5 without converging (round 5 reported findings; every round 1–5 audited its chunks).
Deferred under the convergence posture (round 10, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:2479 — [probe] D10-1: rung-3 cut spends the resumed-run continuity note before already-delivered cannotTell prose, inverting the stated loss order; the comment certifies the wrong orderpackages/core/src/skills/bundled/review/SKILL.test.ts:387 — [probe] D10-2: the pin's loss-order comment asserts the undecided list is spent first; on resumed runs the continuity note goes firstpackages/cli/src/commands/review/compose-review.test.ts:6009 — [probe] D10-3: comment calls the undecided-blocker block "keep: 2"; it is deliberately untagged (rank 3, spent first)packages/cli/src/commands/review/compose-review.ts:2052 — [probe] D10-4: comment claims attribution off removes the fold; the fold depends only on bilingual && zh !== enpackages/cli/src/commands/review/compose-review.ts:3113 — [probe] D10-5: Bi.keep docstring's tier-3 "prose the author already received" is false on resumed runs — the continuity note was never received
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:reverse audit — stopped at the round cap of 5 without converging (round 5 reported findings; every round 1–5 audited its chunks)。
收敛姿态下延后(第 10 轮,非阻断)——已记录,本轮不要求修改:共 5 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| modelId.length <= MODEL_ID_MAX_CHARS | ||
| ? modelId | ||
| : `${modelId.slice(0, MODEL_ID_MAX_CHARS - 1)}…`; | ||
| return `_— ${name} ${FOOTER_MARKER} (v${cliVersion})_`; |
There was a problem hiding this comment.
[Critical] R10-1: The footer is still an UNBOUNDED budget contributor through its second interpolated input — cliVersion is interpolated with no length cap, while the rung-3 comment in compose-review.ts now asserts the tail is a BOUNDED contributor because the footer caps modelId at MODEL_ID_MAX_CHARS, so bodyBudget - footerTail.length can never empty the cut. footerVersion validates charset only (no length bound) and getCliVersion() returns process.env['CLI_VERSION'] unchecked — both env vars are wrapper-controlled. Same class as the modelId hole 0773ad4 capped, through the sibling interpolation (the R9-1 mechanism's second entrance; the footer interpolates exactly two inputs). — Failure scenario: a version-shaped string of ~57k+ chars passes validation, the footer grows past bodyBudget (65,536 − 512 margin − marker reserve), the rung-3 cut clamps to empty and render returns the footer alone — every blocker silently dropped when the tail still fits under 65,536, and at ~66k+ the POST is 422-rejected whole with every blocker lost.
Witness (probe on the unmodified PR head, flipped by a one-line version clamp):
body.length = 70042 / fits 65536 = false / blocker(1k) present = false / hardNote present = false
body begins "\n\n_— test-model via Qwen Code /review (vvvvv…"
→ after clamp: fits 65536 = true / blocker present = true / hardNote present = true
(all 313 existing tests in compose-review.test.ts + review-footer.test.ts pass under the fix)
Suggested fix — clamp the version slot where the modelId slot is clamped:
const version =
cliVersion.length <= MODEL_ID_MAX_CHARS
? cliVersion
: `${cliVersion.slice(0, MODEL_ID_MAX_CHARS - 1)}…`;
return `_— ${name} ${FOOTER_MARKER} (v${version})_`;(or have footerVersion return undefined for an over-long stamp so the caller falls back to the bounded getCliVersion(); disclose the clamp on the remediation channel like the modelId clamp)
中文说明
[Critical] R10-1:页脚仍有一个无界的预算贡献者——第二个被插值的输入 cliVersion 没有长度上限;而 compose-review.ts 的 rung-3 注释现在声称尾部是有界贡献者(因为页脚已将 modelId 限制在 MODEL_ID_MAX_CHARS),所以 bodyBudget - footerTail.length 永远不会把切割清空。footerVersion 只校验字符集(无长度限制),getCliVersion() 不加检查地返回 process.env['CLI_VERSION']——两个环境变量都可由外部包装器设置。这与 0773ad4 加上限的 modelId 漏洞同类,只是经由相邻的插值进入(R9-1 机制的第二个入口;页脚恰好插值两个输入)。— 失败场景:约 57k+ 字符的版本形字符串通过校验,页脚超过 bodyBudget(65,536 − 512 余量 − marker 预留),rung-3 切割被清空,render 只返回页脚——当尾部仍低于 65,536 时全部阻断项被静默丢弃;约 66k+ 时整个 POST 被 422 拒收,连同每一条阻断项。证据:在未改动的 PR 代码上探针实测 body.length = 70042 / fits 65536 = false / blocker present = false / hardNote present = false,正文以 "\n\n_— test-model via Qwen Code /review (vvvvv…" 开头;一行版本 clamp 即可翻转(fits = true / blocker present = true / hardNote present = true),且全部 313 个既有测试通过。建议修复:像 modelId 一样对 version 做 clamp(见英文部分代码),或让 footerVersion 拒绝过长戳记、回退到有界的 getCliVersion(),并像 modelId 一样在 remediation 通道披露。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| `GitHub's ${BODY_MAX_CHARS}-character review limit; the ` + | ||
| `English text below is ${ | ||
| cut | ||
| ? 'truncated as well — see the notice at the end' |
There was a problem hiding this comment.
[Critical] R10-2: The fold-drop notice's cut branch points the reader to a notice "at the end", but this PR's round-10 commit moved the truncation notice (hardNote) to the TOP of the rung-3 body (keep:1, sorted first). The body's end carries only the attribution footer and the invisible ledger marker — no notice; rung 3 is the only cut path, so there is no state in which the truncation notice rides at the end. Same false-record class this PR fixes in three sibling places (verdictLine "listed in the body", unlicensed-deferral "They are listed below", stderr "the English body is complete"), and it contradicts the SKILL.md paragraph this PR adds ("that notice rides above the cut"). — Failure scenario: any bilingual run reaching rung 3 (the shape pinned by this PR's own fixture 'a truncated bilingual body discloses its fold too') posts the TRUNCATED notice as paragraph 1 and the fold notice as paragraph 2; the author following "see the notice at the end" scrolls past up to ~65,000 characters to a spot where no notice exists, and may conclude the truncation was undisclosed. Shipped because the test pins only the prefix 'the English text below is truncated as well', not the sentence's tail.
Witness (probe on the unmodified PR head):
truncation-notice index = 20 / pointer index = 378 / truncation-notice occurrences = 1
any TRUNCATION notice after the pointer = false
last 200 chars = "CCC…CCC\n\n_— test-model via Qwen Code /review (vunknown)_"
| ? 'truncated as well — see the notice at the end' | |
| ? 'truncated as well — see the notice above' |
(or drop the pointer clause entirely, since the truncation notice rides directly above the fold notice; pin the new wording in the existing bilingual-truncation test)
中文说明
[Critical] R10-2:折叠丢弃通知的「切割」分支把读者指向「末尾的通知」,但本 PR 第 10 轮的提交已把截断通知(hardNote)移到 rung-3 正文的最前(keep:1,排序在最前)。正文末尾只有署名页脚和不可见的 ledger marker——没有任何通知;rung 3 是唯一的切割路径,因此不存在截断通知位于末尾的状态。这与本 PR 在另外三处修复的「虚假记录」同类(verdictLine 的 "listed in the body"、未授权延后的 "They are listed below"、stderr 的 "the English body is complete"),也与本 PR 新增的 SKILL.md 段落("that notice rides above the cut")矛盾。— 失败场景:任何到达 rung 3 的双语正文(本 PR 自己的夹具 'a truncated bilingual body discloses its fold too' 钉住的形态)会在第 1 段发出 TRUNCATED 通知、第 2 段发出折叠通知;作者按 "see the notice at the end" 翻过最多约 65,000 字符到一个没有任何通知的位置,可能以为截断未被披露。之所以全绿上船,是因为测试只钉了前缀 'the English text below is truncated as well',没钉句尾。证据:探针实测截断通知在索引 20、指针在索引 378、通知只出现一次、指针之后无任何 TRUNCATION 通知;正文最后 200 字符是阻断项文本 + 署名页脚。建议:把指针改为指向上方(见 suggestion),或直接删掉指针子句(截断通知就在折叠通知正上方),并在现有双语截断测试中钉住新措辞。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 7/100 轮)。改动内容与我反驳保留之处如下: Round 11 — Critical-only mode (5 change-producing rounds complete)Two Critical findings were actionable this round; both were reproduced on the [Critical] R10-1 (rc:3799298167) — the footer's unbounded
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped at the round cap of 5 without converging (round 4 reported one confirmed finding; round 5 dry; every chunk audited each round).
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 11, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/review-footer.test.ts:98 — [probe] CANONICAL_LGTM_RE version slot {1,100} desynced from the new 200-char footer cappackages/core/src/skills/bundled/review/SKILL.test.ts:425 — [review] pinned Step 8 sentence "an overflowing body can carry none of it" contradicts the fold-first rung
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — stopped at the round cap of 5 without converging (round 4 reported one confirmed finding; round 5 dry; every chunk audited each round)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后(第 11 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round — no action takenThis round found nothing actionable on PR #9247:
Critical-only mode is active (5 change-producing rounds are complete in this counting window), so the one deferred non-Critical item — the automated reviewer's review (pullrequestreview-4956334258) — is excluded from this round per the workflow's deterministic brake and remains open for human follow-up. No code was changed, no threads were resolved, and no base merge was performed ( The branch stays at its current head ( 中文说明Autofix 轮次 — 未采取任何操作本轮在 PR #9247 上没有发现可处理的内容:
当前处于仅处理 Critical 的模式(本计数窗口已完成 5 个产生改动的轮次),因此唯一一条被延后的非 Critical 条目 —— 自动评审器的评审(pullrequestreview-4956334258)—— 按工作流的确定性刹车机制从本轮中排除,保持开放,留待人工跟进。未修改任何代码,未解决任何讨论串,也未执行 base 分支合并( 分支保持在当前 head( Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式:已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 128 passed · 0 failed · 128 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:128 通过 · 0 失败 · 128 总计 Verification reportVerification report — PR #9247
|
| cell | scenario | base len | head len | verdict |
|---|---|---|---|---|
| 00 | fits untouched (zero collateral) | 281 ✓ | 281 ✓ | byte-identical across arms |
| 01 | model blockers alone past the limit (80k) | 80,231 ✗ | 65,024 ✓ | REJECTION → POSTS, TRUNCATED notice, 50k of blockers kept |
| 02 | bilingual overflow absorbed by the fold | 101,419 ✗ | 51,171 ✓ | fold dropped; deferral list + disclosures SURVIVE (content not spent) |
| 03 | trim order: rank 1 alone suffices | 65,758 ✗ | 64,801 ✓ | deferral dropped, Not reviewed: survives, (1 section(s)), verdict line says "trimmed from the body" |
| 04 | trim order: both ranks go | 67,038 ✗ | 63,475 ✓ | both kinds named, note count == bodyTrim.sections |
| 05 | marker reserve (plan names a PR; posted value) | 80,641 ✗ | 57,021 ✓ | posted body+marker ≤ limit; marker round-trips via parseLedger |
| 06 | astral band (surrogate pair at the cut) | 100,231 ✗ | 65,023 ✓ | no lone surrogate, no \uFFFD, astral chars before the cut survive |
| 07 | run of pre-existing unpaired highs across the cut | 80,231 ✗ | 60,593 ✓ | char handed to the tail is not an unpaired high (loop, not one pass) |
| 08 | footer modelId 70k |
150,221 ✗ | 65,024 ✓ | cut keeps 50k of blockers; clamp disclosed on stderr |
| 09 | footer cliVersion 70k |
150,224 ✗ | 65,024 ✓ | same hole closed on the second interpolation |
| 10 | planNamesPr drift (prNumber: "0") |
80,641 ✗ + marker | 65,024 ✓, no marker | predicate unified (see Observations) |
| 11 | unpaired LOW at the cut | 80,231 ✗ | 65,024 ✓ | author's bytes kept (documented policy) |
Flip: 11/11 over-limit cells go REJECTED → POSTS; the under-budget cell is byte-identical. Base control cells are assertions that the broken shape reproduces — all 15 passed as predicted.
Mutation matrix (PR's own suite as oracle, 02-mutation-matrix.png)
Unmutated control green; every mutant red with the intended behavioural assertion (quoted, not an import/compile break):
| mutant | guard | result | first failing assertion |
|---|---|---|---|
| — | unmutated control | GREEN | — |
| M0 | budget early return removed (central hunk revert) | KILLED, 26 red | deferral list not trimmed / bodyTrim.sections 0≠4 |
| M1 | surrogate strip loop removed | KILLED | LONE_SURROGATE true ≠ false |
| M2 | fold rung disabled | KILLED | content spent while fold headroom sits (sections 1≠0) |
| M3 | keep sort inverted |
KILLED | truncation notice itself cut from the body |
| M4 | footer interpolations unclamped | KILLED | 70039 ≤ 65536 fails |
| M5 | verdict line always "listed in the body" | KILLED | verdict-line pointer mismatch |
| M6 | bodyTrim.fold absence tolerated in save-artifact |
KILLED | expected [Function] to throw |
| M7 | marker reserve never applied | KILLED | posted body 66553 ≤ 65536 fails |
| M8 | SKILL.md budget paragraph reverted | KILLED | doc pin: rejected by the API **whole** missing |
No survivors; the positive control (M0) lands 26 red on the length/trim oracles, so the kill counts are trustworthy.
Boundary probes (31/31)
- Budget edge is
<=not<: a body at exactly the budget (65,024) composes untouched; +1 char with nothing trimmable trims/cuts and still fits. planNamesPrtype ladder (13 shapes: 0, "0", -1, 1.5, "1.5", "abc", "-3", null, true, [42], …): marker rides iff positive integer or positive-integer string; every shape composes within the limit, none throws.- Missing and unparseable plan files: budget still holds (truncation path), compose never taken down.
- Attribution off: no footer, budget holds.
Targeted gates
packages/clireview suite at head: 3372 passed | 4 skipped (82 files), exit 0.packages/coreSKILL.test.ts: 21 passed, exit 0.packages/clitsc --noEmit: exit 0.- Live-gate proof: a planted failing test inside the same gate command was reported (
1 failed | 82 passed, exit 1) — the green above is a measurement, not an assumption. - Scripted adjudication of every evidence log (20/20):
04-scripted-adjudication.png.
Observations (non-blocking)
- Notice-cost stacking: at budget+1 with a small trimmable section, the trim notice (~350 chars) can cost more than the section it discloses, so the body records
sections: 1andtruncated: true. Both channels disclose both facts; this is inherent to honest disclosure taking space, not a defect. - Unpaired low surrogate policy: the cut keeps an author-written lone low at the junction (asserted in cell 11). Whether GitHub's API accepts a lone low in a review body is untested here (no credentials); the PR documents the choice ("rewriting it is spending the author's bytes").
- Predicate unification is observable: base appended a ledger marker for
prNumber: "0"(old/^\d+$/check) while anchor consumers read such plans as PR-less; head routesledgerMarkerFor,prevRoundFor,scriptLintReportName,testPlanGatethrough oneisPositivePrNumber. Intentional and commented; recorded as a hardening, not a regression.
Not covered
- Per-commit attribution: the checkout is depth 2 (1 commit reachable locally vs 21 in the metadata snapshot); the aggregate
HEAD^1..HEADdiff was verified, per-commit claims were not individually exercised. - Real GitHub POST: no credentials in this lane. The oracle is GitHub's documented 65,536-character limit applied to the exact string
submitposts; a live 422-vs-201 wire test was not run. - Repo-wide gates (full suite, lint, format) — the PR's own CI covers them; only the affected suites plus cli typecheck ran here.
packages/web-shell/README.mdtable reflow is cosmetic prettier output; not exercised.- save-artifact CLI round-trip end-to-end (handler path) — covered at the validation-function level by its suite and M6 only.
Methodology
Environment: node:22-bookworm CI container, refs/pull/9247/merge at depth 2; npm ci + npm run build pre-ran at HEAD. Harnesses (harness/*.mjs, rerunnable) drive the compiled dist/ of each arm with file-based plan fixtures — no mocks of the code under test; the base arm is a scratch worktree at HEAD^1 rebuilt with tsc only, sharing the root node_modules (justified: lockfile untouched, and the compiled compose-review.js resolves no @qwen-code/* import — the internal-symlink hazard was checked via readlink -f and found absent for the unit under test). Mutations ran in a second scratch worktree with per-package node_modules symlinked; each mutant was applied, run, and reverted (worktrees removed afterwards). Evidence images rasterised with scripts/verify-capture.mjs; raw per-cell and per-mutant logs live in logs/ (head-ab.json/base-ab.json carry the cell-level records, verify-matrix.log the scripted adjudication of every log). Assertion totals: 62 + 15 + 31 + 20 = 128.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round summary — PR #9247 (no action taken)ResultNo action was taken in this round. No code was changed and no commit was made. Why no action was taken
Working tree state
Verification
中文说明Autofix 轮次总结 — PR #9247(未采取任何操作)结果本轮未采取任何操作。未改动任何代码,也未创建任何提交。 未采取行动的原因
工作树状态
验证
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式:已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |




What this PR does
Makes
compose-reviewmeasure the review body it is about to return, and keep it inside GitHub's 65,536-character limit.When the body would overflow, it trims in a fixed order — the Chinese fold first (a translation of the English above it, so it costs no content at all), then the deferral display, then the not-reviewed disclosures, and the blockers, the undecided-blocker list, the caps and the openers never — and discloses every trim in the body with a count, repeating it on stderr for the operator. When what remains is un-trimmable by that policy and still overflows (blockers alone past the limit), the body is truncated rather than composed into something the API rejects: the bilingual fold is dropped first so the cut cannot leave unbalanced markup on the PR page, the cut lands on a code-point boundary, and a loud notice names the truncation. Room for the ledger marker is held back whenever the plan names a PR — the same check the marker itself asks, so the budget and the marker cannot disagree about whether one will ride.
Everything the trim leaves behind still describes the body truthfully, which is what the first review round was spent on: the trim notice rides first, above the sentences it corrects, and names the kinds it dropped rather than counting them; the verdict line says
trimmed from the body … whole in the findings artifactinstead of "listed in the body" once the list is gone (ComposeReviewResult.bodyTrim, persisted into the artifact); the last-resort cut spends parts in an explicitkeeporder, so already-delivered prior-round prose goes before this round's only copy of a blocker; and the loud "TRUNCATED" claim fires only on an actual cut — an overflow that the bilingual fold alone caused says exactly that instead.Why it's needed
A review body over the limit is rejected by the API whole — every blocker it carries with it. That is the worst failure this module has: a run that found the bug and could not say it.
Until now nothing measured the composed body. Two contributors were individually bounded (the ledger marker's 8 KiB, the deferral list's 20 × 240), but the unresolved-blocker list, the disclosure sentences and the body Criticals are model-written prose with no upstream cap, so the rejection shape stayed constructible: a probe during #9118's review composed 67,039 characters. The exposure is older than that PR and independent of it — this closes it at the one place that knows the whole body.
Reviewer Test Plan
How to verify
the deferred-findings list did not fit (1 section(s))) and the not-reviewed disclosures survive; with the body still overflowing, those go next and the notice names both. The blocker text survives whole in either case, and the verdict line stops claiming the deferrals are listed in the body.was TRUNCATED to fit, the footer, balanced<details>markup (the fold is dropped, not cut), and no lone surrogate — pinned with an astral-plane body, where the cut lands on a surrogate pair.bodyTrim.truncatedstays false).submitposts.cd packages/cli && npx vitest run src/commands/review/andcd packages/core && npx vitest run src/skills/bundled/review/SKILL.test.ts.Evidence (Before & After)
N/A — pipeline behavior; no TUI change. Before: a body over the limit was handed to
submitand the POST was rejected whole. After: the same input composes a trimmed-and-disclosed body that posts.Tested on
Environment (optional)
Unit tests per package; typecheck and the review suites verified in a clean detached worktree. The unrelated
npm cifailure this branch was blocked on (packages/sdk-typescriptbrowser daemon bundle 194,607 bytes vs. its 190 KiB limit, grown by #9180) is fixed by #9238, merged into this branch.Risk & Scope
ComposeReviewResultgainsbodyTrim(withfold) (and the artifact a matching field, read as untrimmed when absent, so a composed file written by an older CLI still saves); only the composed body can differ, and only when it would previously have been rejected.Linked Issues
Closes #9177. Extracted from #9118's review rounds.
中文说明
本 PR 做了什么
让
compose-review测量它将要返回的评审正文,并使其保持在 GitHub 65,536 字符上限之内。正文超限时按固定顺序裁剪——先丢中文折叠(它是上方英文的翻译,不损失任何内容),再延后展示,再未审披露;阻断项、未决 blocker 清单、cap 与开场白永不裁剪——每次裁剪都在正文中带计数披露,并在 stderr 向操作者重复。当按该策略不可裁剪的部分仍然超限(阻断项本身就超过上限)时,正文被截断而不是组合成一条会被 API 拒收的内容:先丢弃双语折叠(避免截断留下未闭合标记)、按码点边界切、并附醒目通知。只要 plan 指向 PR,就为随后追加的 ledger marker 预留空间——预留与 marker 自身使用同一个判定,两者不可能不一致。
裁剪之后留下的每句话仍须如实描述正文,这是第一轮评审的全部工作:裁剪通知位于最前、先于它所纠正的句子,并点名所丢弃的类别而非仅计数;清单被丢弃后,裁决行改为
trimmed from the body … whole in the findings artifact而非「listed in the body」(ComposeReviewResult.bodyTrim,并持久化进工件);末位切割按显式keep顺序消费,先消耗作者上一轮已收到的文本,再触及本轮阻断项的唯一副本;醒目的「TRUNCATED」声明仅在确有切割时发出——仅因双语折叠而超限时,如实说明的是折叠被丢弃。为什么需要
超限的评审正文会被 API 整条拒收,连同它携带的每一条阻断项。这是本模块最坏的失败形态:找到了 bug 却说不出来。
此前没有任何环节测量组合后的正文。两个贡献者各自有界(ledger marker 的 8 KiB、延后清单的 20 × 240),但未决 blocker 清单、披露句与 body Critical 都是没有上游上限的模型文本,因此拒收形态始终可构造:#9118 评审期间的探针组合出 67,039 字符。该暴露先于那个 PR 存在且与之无关——本 PR 在唯一知晓完整正文的位置将其闭合。
评审验证计划
见上方英文部分:未超限不变、裁剪顺序(先延后展示、后未审披露、阻断项完整存活)、末位截断(含折叠平衡与无孤立代理)、marker 预留后 POST 正文仍合规,以及两个测试套件命令。
注:本分支此前受阻的
npm ci失败(packages/sdk-typescript浏览器 daemon bundle 194,607 字节,超过 190 KiB 上限,由 #9180 增长所致)与本分支无关,已由 #9238 修复并合入本分支。风险与范围
ComposeReviewResult新增bodyTrim(含fold)(工件同步新增该字段,缺省读作「未裁剪」,旧版 CLI 写出的 composed 文件仍可保存);只有原本会被拒收的正文才会不同。关联 Issue
Closes #9177,提取自 #9118 的评审轮次。