Version Packages

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

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",

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