mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-21 14:46:19 +00:00
a20a724ec0
* refactor(review): build the incremental scope from the PR's diff, not a check The containment oracle proved, after the fact, that a separately captured `anchor..head` carried no hunk the PR's own `base..head` diff lacked — because a comment anchored on a line GitHub does not display answers 422 and takes the whole all-or-nothing Create Review call with it. That proof was a hand-written match over two rendered unified diffs, and six review rounds each closed the reported entrances while the next found new ones: count-less headers, deletion junctions, lossy UTF-8 decodes that collapse distinct bytes onto U+FFFD, a budget spent across hunks, content matched without position. Every one was the same shape — something the delta carried that the PR's diff did not display, arriving through a gap in the match. The surface is unbounded because it is a match over arbitrary content. So the scope is no longer checked against the PR's diff; it is built from it. The delta is read only for the post-image ranges it touched, and the published text is assembled out of the full capture's own hunks. Every line the review sees is a line GitHub displays, by construction rather than by proof. The oracle goes, and with it the two refusal reasons that existed to report its verdicts: `hunks-outside-pr-diff` and `containment-unverified`. One reason replaces them, and it names a fact rather than a failed proof — `nothing-to-narrow`, when the PR's diff has no hunk left in the range that changed since the anchor. That is the "undo per feedback" round, where the commits since the anchor put lines back the way the base had them: there is genuinely nothing there to re-review, and the round keeps the full range, which is the review it would have done anyway. The invariant is asserted directly against real-git captures rather than argued per shape: every line of the narrowed output must appear in the full capture, checked over the whole output on each scenario, including the undo-per-feedback history that defeated the oracle six times. A mutant that assembles from the delta's bytes instead fails three of the four. Net effect on the tree: -255 lines of production code and the ~765-line battery that existed to pin it, against +29 and a four-case integration suite. * docs(review): retire the oracle's refusal reasons from the skill #9100 has landed, so the paragraphs this would have conflicted with are settled and the deferred half of this PR can go in. SKILL.md's recovery taxonomy still enumerated `hunks-outside-pr-diff` and `containment-unverified` — reasons the report can no longer carry, since the oracle that produced them is gone. It names `nothing-to-narrow` now, and says what actually produces it: an "undo per feedback" revert, which puts lines back the way the base had them so the PR's diff no longer shows that region, and a capture whose bytes do not survive a UTF-8 round trip. The retry classification moves with it. The old sentence said "the containment reasons re-rule identically"; the new one says the narrowing re-narrows identically, and why — the same two captures select the same hunks, and a capture that failed a round trip fails it again. Both remain deterministic for the same sha and outside the retryable set. Pinned in SKILL.test.ts beside its siblings: moving the reason into the retryable set fails that test rather than shipping green. * fix(review): keep narrowed rounds displayed-only across rename, mode, and huge-hunk shapes - Refuse to narrow when a delta path does not cross into the full capture's keys: git's rename detection can resolve differently across the two ranges, and the unmatched section is a displayed change that would silently drop from the published scope while the round still certified head. The round keeps the full range instead. - Emit a full section whole when the delta touches it without hunks: a since-anchor mode change, pure rename, or binary replacement lives in the section header, and the old guard dropped the whole section while reporting `effective: true`. - Weld `incremental.diffBase` to the merge base, not the anchor: the published hunks are byte-identical hunks of `mergeBase..head`, so Agent 7's test-efficacy probe must recompute that range; the anchor range can carry undo hunks the PR's diff does not display at all. - Assemble the narrowed text without spreading selected hunks into a single `push`: a hunk past the ~125k-line argument ceiling crashed the whole fetch-pr round with a RangeError instead of degrading. - Validate the full capture with a fatal UTF-8 decode instead of re-encoding a full-size copy to compare, removing ~N of peak memory on the large long-lived PR workload. Pinned by real-git integration scenarios for each shape, including the deletion-acceptance control and an unconditional null fallback; the producer→consumer weld test now asserts the merge base end to end. Each new test was verified to fail against the pre-round code. * fix(review): close the rename-key divergence in incremental narrowing - Fail closed on a rename the full capture does not key as the SAME rename: when round 1 rewrites a file below git's rename threshold and round 2 renames it, `base..head` nets the chain to an addition plus a deletion while `anchor..head` carries a 100%-similarity rename keyed on the new path. The path guard passed — the new path is in the full capture, as the addition — while the rename's deletion half sat under the old path and dropped from the published scope under `effective: true`. `parseDiff` now exposes `rename from`, and the narrowing join refuses unless both captures key the same rename; the round keeps the full range, which still displays the deletion. - Fatal-decode the delta symmetrically with the full capture: `narrowToDelta` takes the delta's raw bytes and decodes them itself. A lossily pre-decoded delta folded an invalid path byte onto U+FFFD, which could collide with a legitimate U+FFFD path in the full capture and publish an unchanged file's hunks. - Record the executable bit through git itself in the mode scenarios (`git update-index --chmod=...` beside the filesystem chmod): `chmodSync` is invisible to git on Windows — libuv cannot set the exec bit and `core.fileMode` is false — so the two mode tests failed on the Windows merge-queue leg. Verified against a Windows-git model (`core.fileMode=false`): 2 failed before, 16/16 after. - Assert the narrowing outright where the scenarios are constructed to narrow: the null guards on the undo-per-feedback and post-anchor-file tests let an all-or-nothing refusal ship green with zero assertions executed. - Pin the unpinned emission shapes: a whole-file deletion riding the `+0,0` clamp and inclusive `overlaps`, a mode-only full section the delta touches with content hunks, and both hunks of a two-region file surviving the join — each verified red against the corresponding mutant. - Enumerate all four null shapes under `nothing-to-narrow` in the report's union doc, the demotion arm, and the skill's reason bullet; the routing (deterministic, never retried) already held for all four. The rewrite-then-rename regression test fails against the pre-round code (the probe published [new.ts, other.ts] with the deletion absent); the mode scenarios fail under the Windows model before the index-native recording. * fix(review): keep header-level changes in the narrowed incremental scope A delta section whose hunks all miss the full capture used to be dropped whole, taking a post-anchor mode flip or rename out of the published scope while the round still reported effective. Track header-level delta changes per path and emit such sections whole, the hunk-less treatment. Also close the measured battery-power gaps: the rename guard's pass-through arm, rename-plus-hunks emission, a single delta hunk overlapping two full hunks, the multi-file section drop, the capture-failed disk assertion, the diffBase seam's division of labor, and the skill's reason taxonomy plus retryable set. (#9267) * test(review): close the narrowing battery's mutation holes (#9267) The headerTouched miss-branch tests never asserted the section's surviving content hunks, the delta-side UTF-8 refusal test stayed green with its guard removed, and the battery carried no no-trailing-newline marker pin and no binary delta section. Add the missing assertions and the two scenarios, and rebuild the delta-side refusal test around the U+FFFD collision its comment describes, so each shape now fails under the mutant it is meant to catch — all four mutants probed against the battery. Also name what the merge-base clamp actually prevents in the skill's `base-untrusted` clause, whose "those" lost its antecedent when the containment reasons were retired. * fix(review): carry position-divergent hunks through the narrowing join (#9267) Myers aligns a change inside a run of identical lines against whatever surrounds it, and the two captures' old sides differ — so the same post-anchor change can sit at disjoint head-side ranges in `base..head` and `anchor..head`. The range join then dropped a change the PR's diff displays while the round still reported `effective: true`, and the ledger certified head over it — the change never re-entered any later scope. Fail closed per hunk: a missed delta hunk whose changed lines the full section also changed emits the section whole — every line of it is displayed, and clean siblings still narrow. A netted-out undo contributes no line the full section displays, so the deliberate section drop is untouched. Covers both probe-confirmed shapes: the whole-section miss and the partial miss where a matched sibling hunk kept the file visible. * fix(review): fail closed when a delta hunk lacks a corroborating full hunk (#9267) * fix(review): key changed-line corroboration by new-side junction (#9267) * fix(review): narrow between files, not within them Four consecutive rounds reported the same class and each fix was defeated by the next round's entrance: whole-section miss, partial miss, then two shapes defeating the position-divergence guard's conjuncts, then content-only corroboration, then junction-keyed corroboration defeated by a delta hunk carrying two changes. The reviews were right about the cause each time and right about the pattern: matching hunks across the two captures is a heuristic over arbitrary content, which is what the containment oracle this file replaced also was. The two captures are independent Myers alignments over overlapping content, so which HUNK a change lands in is not stable between them — a run of identical lines lets the same edit be attributed to the run's front in one and its back in the other. Every guard here tried to recognise that divergence; none could, because it is a property of the alignment and not of the change. What IS stable is which FILE a change belongs to, and the path and rename guards already fail closed on the one way that could differ. So the unit of narrowing is the file: a section the delta touched is emitted whole, a section it did not touch is dropped. Nothing the delta performed can fall out of a section emitted entire, and the whole position-divergence family stops existing rather than being caught. That failure direction is the reason this could not stay as it was. A dropped hunk left the round reporting `effective: true`, and the ledger then certified head as the next anchor, so the change was never reviewed by any round. The deleted oracle failed toward more review; this failed toward silently less, recorded as complete. Cost, stated plainly: within a touched file the round now reviews all of that file's PR hunks, not only the ones that moved since the anchor. The saving incremental review exists for is the untouched files — a round touching 2 of 40 reviews 2 — and that is unaffected. Also: a base-free round no longer reports `capture-failed`. The fetch succeeded and `git merge-base` found no common ancestor, so nothing threw; naming an infrastructure fault put a deterministic state into the class the recovery flow retries. It reports `nothing-to-narrow`, and the two pins that asserted the old reason move with it. * fix(review): split a base-free round by why the base is missing The R11-1 fix keyed on `mergeBaseSha === null` alone, but that null has two causes and only one is deterministic. A base that could not be FETCHED — a fresh CI clone with no local base ref, hitting a transient fault — is infrastructure: something did fail, and the re-run re-runs exactly the component that failed. Reporting `nothing-to-narrow` there put a retryable state into the never-retried class and pointed operators at "nothing to narrow" instead of a fetch failure. The arms are split by `baseFetchFailed` now: fetch failed keeps `capture-failed`, no-common-ancestor keeps `nothing-to-narrow`. This is the same distinction SKILL.md's recovery paragraph already draws for a planless `partition-failed`, applied where the code makes the same choice. The pin that was supposed to cover this asserted `nothing-to-narrow` over a `{sha: null, baseFetchFailed: true}` fixture while its comment said "the fetch succeeded" — a fixture contradicting its own comment, which is what kept the gap invisible. It is one test over both fixtures now, each with the reason its cause implies. * fix(review): split merge-base surface failures from deterministic refusals (#9267) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|---|---|---|
| .. | ||
| scripts | ||
| src | ||
| vendor | ||
| index.ts | ||
| package.json | ||
| test-setup.ts | ||
| tsconfig.json | ||
| vitest.config.ts | ||