Version Packages - #326
Merged
Merged
Conversation
khan-actions-bot
requested review from
a team,
jeresig and
somewhatabstract
and removed request for
a team
August 5, 2026 16:07
jwbron
added a commit
that referenced
this pull request
Aug 5, 2026
The reviewer installed on this repo (`.github/workflows/review.md`) was pinned at `review-v1.11.0`; #326 released review@1.12.0, so the shared package's cross-source dedup tier 2 runs in consuming repos but not here. What v1.12.0 adds, and what this bump picks up: - The `claim-clusterer` sub-agent (sonnet, read-only, identity judgment only) names the candidate comments that describe ONE defect, and `dedup.ts` verifies that assertion and merges them, so one problem found by several sources posts once. Tier 2 requires no line agreement, so the same-defect-different-anchor shape is mergeable for the first time; every merge rule stays in code, only a NON-BLOCKING copy may be absorbed on the model's word, and the survivor is always the highest-severity copy. - Dispatch is conditional (only when the candidates hold a legally mergeable pair), and a missing definition or unusable reply leaves the run on tier 1, i.e. today's behavior. - The prompt's Step 3 dedup description gains the clusterer and the `clustering` block in the dispatch result. Mechanics, same as #318: `gh aw update` cannot resolve changesets-style prefixed tags (`review-v1.12.0`), so the bump is a hand-run 3-way merge (base = `review-v1.11.0` source, ours = installed copy, theirs = `review-v1.12.0` source) followed by `gh aw compile review`. - `source:` and the lib checkout `ref:` move to `review-v1.12.0` in lockstep; the recompiled lock changes only those literals plus its metadata hashes (the prompt body reaches the runner through `{{#runtime-import .github/workflows/review.md}}`, and inline sub-agents are restored from the activation artifact, so the new agent section does not appear in the lock). - The local overrides are unchanged and still the only differences from the pinned source: the same-repo fork guard in `if:` plus its `roles: all` comment, the commented-out `observability:` block, the `max-ai-credits: 2500` / `REVIEW_MAX_AI_CREDITS` pair, and the comment on the lib checkout `ref:`. - No consumer-config change: `.github/aw/review/ROUTING` needs no dial for clustering. Verification: `pnpm vitest run` 1655 tests across 71 files pass, including `review-pins.test.ts` (every literal `review-v1.12.0`; every hunk against the pinned source carries a `KHAN/ACTIONS LOCAL OVERRIDE` marker) and `version-sync.test.ts`; `pnpm typecheck` clean; `gh aw compile review` 0 errors, 0 warnings. No changeset: both files are under `.github/`, the check's default exclusion, and the shared `workflows/review` package is untouched. Auditors: jeresig
jwbron
added a commit
that referenced
this pull request
Aug 5, 2026
🖍 _This is an audit!_ 🖍 ## Summary: ## Why The reviewer installed on this repo (`.github/workflows/review.md`) was pinned at `review-v1.11.0`; #326 released review@1.12.0, so the shared package's cross-source dedup tier 2 runs in consuming repos but not here. What v1.12.0 adds, and what this bump picks up: - The `claim-clusterer` sub-agent (sonnet, read-only, identity judgment only) names the candidate comments that describe ONE defect, and `dedup.ts` verifies that assertion and merges them, so one problem found by several sources posts once. Tier 2 requires no line agreement, so the same-defect-different-anchor shape is mergeable for the first time; every merge rule stays in code, only a NON-BLOCKING copy may be absorbed on the model's word, and the survivor is always the highest-severity copy. - Dispatch is conditional (only when the candidates hold a legally mergeable pair), and a missing definition or unusable reply leaves the run on tier 1, i.e. today's behavior. - The prompt's Step 3 dedup description gains the clusterer and the `clustering` block in the dispatch result. ## Why not `gh aw update` Same as #318 and #276: gh-aw's `resolveLatestRef` rejects changesets-style prefixed tags (`review-v1.12.0`) as non-semver, falls through to branch resolution, and 404s. Updates of this workflow stay manual. ## What this PR does - Replicates `gh aw update`'s 3-way merge by hand: base = `review-v1.11.0` source, ours = installed copy, theirs = `review-v1.12.0` source, then `gh aw compile review`. The only conflict was the lib checkout `ref:` line inside its override comment block; resolved as comment plus new ref. - `source:` and the lib checkout `ref:` move to `review-v1.12.0` in lockstep. The recompiled lock changes only those literals plus its metadata hashes: the prompt body reaches the runner through `{{#runtime-import .github/workflows/review.md}}` and inline sub-agents are restored from the activation artifact, so the new agent section does not appear in the lock. - The local overrides are unchanged and still the only differences from the pinned source, enforced by `review-pins.test.ts`: the same-repo fork guard in `if:` and its `roles: all` comment (public-repo hardening), the commented-out `observability:` block (the `GH_AW_OTEL_SENTRY_*` secrets still exist neither on this repo nor at org level), `max-ai-credits: 2500` with its `REVIEW_MAX_AI_CREDITS` mirror, and the comment on the lib checkout `ref:`. - No consumer-config change is needed: `.github/aw/review/ROUTING` needs no dial for clustering, and the roster it enables is untouched. ## Verification - `pnpm vitest run`: 1655 tests across 71 files pass, including `review-pins.test.ts` (source/ref/lock literals all `review-v1.12.0`, and every hunk against the pinned source carries a `KHAN/ACTIONS LOCAL OVERRIDE` marker) and `version-sync.test.ts`. - `pnpm typecheck`: clean. - `gh aw compile review`: 0 errors, 0 warnings. - No changeset needed: both files are under `.github/`, the check's default exclusion, and the shared `workflows/review` package is untouched. - This PR's own review does not exercise tier 2, and that is expected: for a `pull_request` event the lock runs from the head (so the lib is checked out at `review-v1.12.0`), but `restore_base_github_folders.sh` overwrites `.github/` with the base-branch snapshot and the activation job extracts the inline sub-agents from that same base markdown, so the prompt and the `## agent:` set both come from main at `review-v1.11.0`. With no `claim-clusterer` definition, dispatch leaves the run on tier 1 and records a run warning: the documented soft-degradation path. Both halves come from main once this merges. Author: jwbron Auditors: jeresig, jaredly Required Reviewers: Approved By: Checks: ⏭️ 8 checks have been skipped, ⌛ 1 check is pending, ✅ 2 checks were successful Pull Request URL: #327
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR was opened by the Changesets release GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated.
Releases
review@1.12.0
Minor Changes
cbcb494: Cross-source dedup gains a second tier: a
claim-clusterersub-agent names thecandidate comments that describe ONE defect, and
dedup.tsverifies thatassertion 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.aboveconst maxSamples = 25) atwindow.go :8, :9, :8, :8, and
mergesrecorded none of them; it was a FIRSTreview at
depth: full, so not a re-review artifact. Autofix later satisfied allfour 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 ischosen 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
evidencenames no identifier, literal, or quotedtext 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
clusteringblock indispatch-result.json(candidates,proposed,clusterMergesper group,clusterMergedper absorbed copy, and every rejectedmember with the rule that stopped it) rather
than as an author-facing note: duplicate hygiene is not a review dimension. Each
merge in
mergesnow carriesvia(similarity/clusterer/both) plus thetier 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
conventionscopy wanted the symbol-name prefix, notthe 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 review: hand off out-of-lane observations instead of dropping them #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.tswas at its 1000-line cap again, so the clustering step lands indispatch-cluster.ts(dispatch, contract parse, telemetry) rather than raisingthe cap, and tier 2's rules in
lib/dedup-cluster.tsbeside the tests thatalready carried the name; the eval's dedup stage splits to
eval/live-dedup.tson the same principle.