From bf799304a8fe29d29395fe30653aa3e0eb3b3efd Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 5 Aug 2026 16:06:52 +0000 Subject: [PATCH] Version Packages --- .changeset/review-defect-clustering.md | 103 ------------------------ workflows/review/CHANGELOG.md | 104 +++++++++++++++++++++++++ workflows/review/package.json | 2 +- workflows/review/review.md | 2 +- 4 files changed, 106 insertions(+), 105 deletions(-) delete mode 100644 .changeset/review-defect-clustering.md diff --git a/.changeset/review-defect-clustering.md b/.changeset/review-defect-clustering.md deleted file mode 100644 index ab4aed0e..00000000 --- a/.changeset/review-defect-clustering.md +++ /dev/null @@ -1,103 +0,0 @@ ---- -"review": minor ---- - -Cross-source dedup gains a second tier: a `claim-clusterer` sub-agent names the -candidate comments that describe ONE defect, and `dedup.ts` verifies that -assertion and merges them. Several reviewers finding one problem now post once. - -Run 30587343777 (webapp#41204) is the case. Four sources flagged one wrong doc -comment (`// Keeps at most 10 samples per key.` above `const maxSamples = 25`) at -window.go :8, :9, :8, :8, and `merges` recorded none of them; it was a FIRST -review at `depth: full`, so not a re-review artifact. Autofix later satisfied all -four with one rewritten comment, which is the proof they were one defect. - -Replaying that run's own claims.json showed the similarity tier is not close to -reaching it. Three of the four share the EXACT anchor and still score -0.060-0.068 Jaccard against a 0.14 floor with 0-1 shared bigrams against a floor -of 4: an order of magnitude below the tier, so no re-derivation from the fixtures -gets there (the floor that admits 0.06 admits everything). Each reviewer wrote -the same defect in different words, and the terser the claim the less text -arithmetic has to work with. Nor can reweighting the text recover the -discriminator: the pairs dedup deliberately keeps apart share MORE salient -tokens than the real duplicates do (run 29943085279's AddDate issue and its -"central behavior never exercised" thought sit on one line and share AddDate, -MemoryTTLDays, 180, 15). Duplicates are "same ask, different words"; those are -"same facts, different ask", which is a semantic judgment. - -Asking each finder for its own identity key would cluster deterministically at -zero dispatch, and it fails for the same reason. A finder mints its key blind to -the other reviewers, so two of them agreeing on one defect would have to -independently pick the same string; and where a blind key DOES agree is the case -that must not merge, since the AddDate bug and its missing-test todo would both -key on `AddDate`. The clusterer's grounding evidence is not that key: it is -chosen after reading the candidate set, which is what makes it both possible and -checkable against every member's text. - -So the unit of identity is now the defect, not the anchor. Tier 2 requires no -line agreement at all, which is what makes the same-defect-different-anchor shape -mergeable for the first time (one missing-test defect drew comments at three -anchors in run 29943085279); the line survives as tier-1 evidence and as the -survivor's posting anchor. - -The model contributes identity only. Every merge rule stays in code, and the -clusterer must ground each group in the code element its members share, which is -then checked: a group whose `evidence` names no identifier, literal, or quoted -text is discarded, and so is a member whose own text never mentions it. Same -path and different sources are enforced as in tier 1, and only a NON-BLOCKING -copy may be absorbed on a model's word — with the survivor always the -highest-severity copy, a false tier-2 merge can cost an advisory comment and can -never lose a blocking finding or soften a verdict. The accepted price: one defect -flagged blocking by two sources in different words still posts twice unless tier -1 reaches it. - -Degradation is soft in both directions. The clusterer is dispatched only when the -candidates hold a pair it could legally merge (two anchored claims on one path -from two sources, at least one non-blocking), so a run with nothing to find never -pays for the step. A missing definition or an unusable reply leaves the run on -tier 1, exactly today's behavior, and surfaces as a run warning plus a -`clustering` block in `dispatch-result.json` (`candidates`, `proposed`, -`clusterMerges` per group, `clusterMerged` per absorbed copy, and every rejected -member with the rule that stopped it) rather -than as an author-facing note: duplicate hygiene is not a review dimension. Each -merge in `merges` now carries `via` (`similarity`/`clusterer`/`both`) plus the -tier and anchor of each absorbed copy, so the merge rate reads off the artifact -instead of off a PR that autofix has already tidied. - -The "also flagged by" note names a source's line when it differs from the -survivor's, and quotes the subject of any copy tier 2 absorbed. One edit -discharging every member's ask does not mean every member asked in the same -words; run 30587343777's `conventions` copy wanted the symbol-name prefix, not -the corrected number. Tier 2 is exactly the case where the survivor's own -prose is known not to restate it (the text floor is what those copies could not -clear). Tier-1 copies are not quoted: clearing that floor against the survivor is -the evidence that they say the same thing, and repeating them would move the -duplicate noise into the surviving comment rather than remove it. - -The live A/B now runs dedup, which it never did: a change to the merge rules was -unmeasurable by construction before this. Tier 1 runs in both arms (it is shared -code and production has had it since #245) while tier 2 is carried by each arm's -own review.md, exactly like the provenance gate's anchor-snap emulation, so the -arm delta prices the clusterer alone and a false merge shows up as recall loss. -The report gains a "Cross-source claims merged (of candidates)" row with tier 2's -share, its dollars and wall-clock, and per-case dedup counts. - -Tier 2 can add merges and never subtract them, and that comes from the ORDER the -tiers run in rather than from any check on a proposal. Tier 1 settles completely, -and tier 2 then merges only the comments it left standing; a cluster member tier -1 has already absorbed is read at the comment it now posts under, and a comment -tier 2 absorbs carries its own tier-1 copies along. Screening each proposed -member before it was unioned was not enough, because a member can be legal in -every respect and still displace the survivor of a tier-1 group it was clustered -into, orphaning that group's other copies: with three copies folded into one -comment and a higher-confidence claim clustered with just one of them, the old -single-pass merge posted three comments where tier 1 alone posted one. The -structural pre-screen stays for what it does do (an illegal member cannot -out-rank the legal ones and take a good proposal down with it), as does the -per-member re-check against the elected survivor. - -`dispatch.ts` was at its 1000-line cap again, so the clustering step lands in -`dispatch-cluster.ts` (dispatch, contract parse, telemetry) rather than raising -the cap, and tier 2's rules in `lib/dedup-cluster.ts` beside the tests that -already carried the name; the eval's dedup stage splits to -`eval/live-dedup.ts` on the same principle. diff --git a/workflows/review/CHANGELOG.md b/workflows/review/CHANGELOG.md index 4a63525c..9ed1fc26 100644 --- a/workflows/review/CHANGELOG.md +++ b/workflows/review/CHANGELOG.md @@ -1,5 +1,109 @@ # review +## 1.12.0 + +### Minor Changes + +- cbcb494: Cross-source dedup gains a second tier: a `claim-clusterer` sub-agent names the + candidate comments that describe ONE defect, and `dedup.ts` verifies that + assertion and merges them. Several reviewers finding one problem now post once. + + Run 30587343777 (webapp#41204) is the case. Four sources flagged one wrong doc + comment (`// Keeps at most 10 samples per key.` above `const maxSamples = 25`) at + window.go :8, :9, :8, :8, and `merges` recorded none of them; it was a FIRST + review at `depth: full`, so not a re-review artifact. Autofix later satisfied all + four with one rewritten comment, which is the proof they were one defect. + + Replaying that run's own claims.json showed the similarity tier is not close to + reaching it. Three of the four share the EXACT anchor and still score + 0.060-0.068 Jaccard against a 0.14 floor with 0-1 shared bigrams against a floor + of 4: an order of magnitude below the tier, so no re-derivation from the fixtures + gets there (the floor that admits 0.06 admits everything). Each reviewer wrote + the same defect in different words, and the terser the claim the less text + arithmetic has to work with. Nor can reweighting the text recover the + discriminator: the pairs dedup deliberately keeps apart share MORE salient + tokens than the real duplicates do (run 29943085279's AddDate issue and its + "central behavior never exercised" thought sit on one line and share AddDate, + MemoryTTLDays, 180, 15). Duplicates are "same ask, different words"; those are + "same facts, different ask", which is a semantic judgment. + + Asking each finder for its own identity key would cluster deterministically at + zero dispatch, and it fails for the same reason. A finder mints its key blind to + the other reviewers, so two of them agreeing on one defect would have to + independently pick the same string; and where a blind key DOES agree is the case + that must not merge, since the AddDate bug and its missing-test todo would both + key on `AddDate`. The clusterer's grounding evidence is not that key: it is + chosen after reading the candidate set, which is what makes it both possible and + checkable against every member's text. + + So the unit of identity is now the defect, not the anchor. Tier 2 requires no + line agreement at all, which is what makes the same-defect-different-anchor shape + mergeable for the first time (one missing-test defect drew comments at three + anchors in run 29943085279); the line survives as tier-1 evidence and as the + survivor's posting anchor. + + The model contributes identity only. Every merge rule stays in code, and the + clusterer must ground each group in the code element its members share, which is + then checked: a group whose `evidence` names no identifier, literal, or quoted + text is discarded, and so is a member whose own text never mentions it. Same + path and different sources are enforced as in tier 1, and only a NON-BLOCKING + copy may be absorbed on a model's word — with the survivor always the + highest-severity copy, a false tier-2 merge can cost an advisory comment and can + never lose a blocking finding or soften a verdict. The accepted price: one defect + flagged blocking by two sources in different words still posts twice unless tier + 1 reaches it. + + Degradation is soft in both directions. The clusterer is dispatched only when the + candidates hold a pair it could legally merge (two anchored claims on one path + from two sources, at least one non-blocking), so a run with nothing to find never + pays for the step. A missing definition or an unusable reply leaves the run on + tier 1, exactly today's behavior, and surfaces as a run warning plus a + `clustering` block in `dispatch-result.json` (`candidates`, `proposed`, + `clusterMerges` per group, `clusterMerged` per absorbed copy, and every rejected + member with the rule that stopped it) rather + than as an author-facing note: duplicate hygiene is not a review dimension. Each + merge in `merges` now carries `via` (`similarity`/`clusterer`/`both`) plus the + tier and anchor of each absorbed copy, so the merge rate reads off the artifact + instead of off a PR that autofix has already tidied. + + The "also flagged by" note names a source's line when it differs from the + survivor's, and quotes the subject of any copy tier 2 absorbed. One edit + discharging every member's ask does not mean every member asked in the same + words; run 30587343777's `conventions` copy wanted the symbol-name prefix, not + the corrected number. Tier 2 is exactly the case where the survivor's own + prose is known not to restate it (the text floor is what those copies could not + clear). Tier-1 copies are not quoted: clearing that floor against the survivor is + the evidence that they say the same thing, and repeating them would move the + duplicate noise into the surviving comment rather than remove it. + + The live A/B now runs dedup, which it never did: a change to the merge rules was + unmeasurable by construction before this. Tier 1 runs in both arms (it is shared + code and production has had it since #245) while tier 2 is carried by each arm's + own review.md, exactly like the provenance gate's anchor-snap emulation, so the + arm delta prices the clusterer alone and a false merge shows up as recall loss. + The report gains a "Cross-source claims merged (of candidates)" row with tier 2's + share, its dollars and wall-clock, and per-case dedup counts. + + Tier 2 can add merges and never subtract them, and that comes from the ORDER the + tiers run in rather than from any check on a proposal. Tier 1 settles completely, + and tier 2 then merges only the comments it left standing; a cluster member tier + 1 has already absorbed is read at the comment it now posts under, and a comment + tier 2 absorbs carries its own tier-1 copies along. Screening each proposed + member before it was unioned was not enough, because a member can be legal in + every respect and still displace the survivor of a tier-1 group it was clustered + into, orphaning that group's other copies: with three copies folded into one + comment and a higher-confidence claim clustered with just one of them, the old + single-pass merge posted three comments where tier 1 alone posted one. The + structural pre-screen stays for what it does do (an illegal member cannot + out-rank the legal ones and take a good proposal down with it), as does the + per-member re-check against the elected survivor. + + `dispatch.ts` was at its 1000-line cap again, so the clustering step lands in + `dispatch-cluster.ts` (dispatch, contract parse, telemetry) rather than raising + the cap, and tier 2's rules in `lib/dedup-cluster.ts` beside the tests that + already carried the name; the eval's dedup stage splits to + `eval/live-dedup.ts` on the same principle. + ## 1.11.0 ### Minor Changes diff --git a/workflows/review/package.json b/workflows/review/package.json index 4975621e..27333f96 100644 --- a/workflows/review/package.json +++ b/workflows/review/package.json @@ -1,6 +1,6 @@ { "name": "review", - "version": "1.11.0", + "version": "1.12.0", "dependencies": { "octokit": "5.0.5", "@anthropic-ai/claude-agent-sdk": "0.3.205", diff --git a/workflows/review/review.md b/workflows/review/review.md index 70572971..3e8a38f4 100644 --- a/workflows/review/review.md +++ b/workflows/review/review.md @@ -227,7 +227,7 @@ pre-agent-steps: uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 with: repository: Khan/actions - ref: review-v1.11.0 + ref: review-v1.12.0 path: gh-aw-review-lib persist-credentials: false