Skip to content

Version Packages - #303

Merged
jwbron merged 1 commit into
mainfrom
changeset-release/main
Jul 30, 2026
Merged

Version Packages#303
jwbron merged 1 commit into
mainfrom
changeset-release/main

Conversation

@khan-actions-bot

@khan-actions-bot khan-actions-bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

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

autofix@0.1.0

Minor Changes

  • 7d62a7a: Add docs to the scope axis: autofix: docs / /autofix docs fixes only the documentation reviewer's threads, selected by the label that reviewer mints (suggestion (non-blocking, documentation)).

    It is a subset of nits, not a peer of it. Documentation findings are non-blocking, so nits already covers them and arming both is the same as arming nits; the containment runs one way only, and docs exists because arming nits to clear three stale comments also invites the fixer into every other cosmetic thread on the PR. The flat token namespace cannot express that, so scope.ts and the README both say it. findingLabelsForScope becomes an exhaustive switch rather than a blocking/non-blocking ternary, and the tests pin the containment and the absence of any blocking label from docs scope.

    docs is not loop-eligible, which is worth stating because it looks like the exception to "nits never loop" and is only half one: its deletion half converges (a comment that restates the code is either gone or not), while the documentation reviewer's missing explanation findings are answered with prose, and prose can always be wanted better. Ineligible until something measures which half dominates.

    One prompt rule comes with it: a documentation item changes text, never code. Deleting a redundant comment is the expected fix (and such findings often carry no suggestion block, since a deletion cannot be expressed as one), but if the honest fix would touch an executable line the item is left unfixed and reported rather than becoming a code change wearing a documentation label. That property — edits that cannot alter behavior — makes docs the safest scope to trial first in a repo new to autofix.

    Note the version coupling: autofix selects threads by parsing the label off each posted comment, so this scope finds threads only in repos whose installed review release mints the documentation label. Against an older reviewer it is not broken, just always empty.

  • 486bb92: Add the autofix workflow: opt-in, one-shot fixing of the PR reviewer's own feedback.

    Arm a PR with an autofix: blocking / autofix: nits label or an /autofix [scope] comment; the run fixes the reviewer's open threads in that scope, pushes one commit, replies in each thread, and removes the label. Both arming surfaces are peers resolving through one shared token vocabulary, and the trigger decides which is read, so a stale label cannot widen an explicit command.

    Everything except the code edit is deterministic. lib/stage.ts runs as a pre-agent step and fetches the inputs before the agent starts; lib/plan.ts then resolves scope, checks review currency, builds the work list, and renders the commit trailer. The plan is final: the prompt may execute it or stop, never widen it.

    Guards fail closed. Currency is checked per file so one unrelated push doesn't refuse the whole run; unparseable labels, outdated anchors, threads a human opened, an unreadable diff, and a head that moves mid-run are all excluded. Refusal is reserved for a PR with no review at all, and for a thread fetch that fails: GitHub reports GraphQL rate limits and node-access failures as HTTP 200 with an errors array, so staging treats any errors entry or an unparseable body as fatal rather than as "this PR has no threads", which would clear the arming label while the findings it was armed for stayed open.

    The reviewer's skip-ai-review label does not disarm autofix. It stops the reviewer's next run without withdrawing a review already posted, so a labelled PR can still carry current findings, and an explicit autofix: label or /autofix from someone with write access is the authorisation to act on them. A PR with no review is still refused, by the guard that checks for one.

    The push uses KHAN_ACTIONS_BOT_TOKEN, because GitHub creates no workflow runs for GITHUB_TOKEN-triggered events, and the re-review of the autofix commit is the intended verification a fix gets. That verification is best-effort rather than guaranteed: the chain from the push to a posted review has several links, whether a break is visible depends on how the consumer triggers its reviewer, and the human re-arming loop is the accepted backstop for v1. The run's summary comment says so on every push. Ships with a documented workaround for gh-aw's unbounded PR-branch fetch, which is otherwise fatal on large monorepos.

review@1.9.0

Minor Changes

  • 6532c6b: Add an opt-in documentation reviewer: an advisory-only whole-change reviewer that checks the comments and prose docs a diff adds or changes against a documentation policy. It flags a comment that restates the code, one that narrates the change rather than describing the code, one this diff falsified, commented-out code, and (the other direction) a specific unexplained constant, workaround, or ordering constraint the change introduces. The policy lives inline in the reviewer's definition rather than in a fifth required consumer import: the baseline is universal, and repo-specific calibration already has a home in the per-directory REVIEW.md contracts. It never reasons about whether a human or a model wrote the text (it cannot tell, and the policy is the same either way), and it does not review the PR title or description. Off everywhere until a repo adds enable documentation to its ROUTING file.

    Its findings render with a new non-blocking label, suggestion (non-blocking, documentation), code-assigned from the lens exactly as the , best-practice variants are. The variant is not cosmetic: the autofix workflow selects threads by parsing the Conventional-Comment label off each posted comment, so this label is the only channel by which a documentation-scoped autofix can tell a documentation thread from any other nit. The scripted dispatcher now assigns the lens of a label-shape reviewer by name (dispatch-contracts.ts: previously conventions for skill-auditor and correctness for every other one), so a documentation finding carries lens: documentation downstream and labelForFinding agrees with the label the reviewer emitted. There is deliberately no blocking documentation variant, so BLOCKING_LABELS (what the blocking autofix scope acts on) is unchanged. thumbs-sweep-github.ts picks the new label up automatically; verdict.ts is unaffected, since the verdict counts blocking labels only.

    Two supporting changes. The claim-validator gains a documentation-claim rule: verify the quoted comment text and the code it is contrasted with, and refute whenever the comment carries information the code does not show, which is this reviewer's characteristic false positive. And the live eval producer now dispatches the opt-in reviewers a case enables (routerConfig.enabledReviewers, canonical order, unknown names throw) and maps their label-shape output; before this, no opt-in reviewer had a live arm at all and so could not earn its enable line through the eval suite the way the policy says it must. Cases that enable nothing are unaffected.

    New eval corpus pair, both live-enabled: golden-documentation-stale-and-narrated (a sound behavior change that leaves a falsified comment and a change-narrating comment, and carries a must-not-flag trap whose comment reads like a restatement but records a real constraint) and clean-documentation-earned-comments (every added comment earns its line; the run must post nothing).

    Eval-harness fix that this PR's own corpus pair needs: an opt-in reviewer a case enables but the arm's review.md does not define is now recorded as an absent dimension instead of throwing. The baseline arm of the A/B that graduates any new reviewer is built from the base tip and so cannot define it, and runArm does not wrap its produce call, so the throw killed the whole run before any report. The name is already validated against ENABLEABLE_REVIEWERS, so tolerating absence cannot mask a typo; every other roster member (the always-on finders, the validator, the reconciler, a routed lens) still hard-errors. The A/B report calls the asymmetry out under Arm asymmetry, since the candidate arm's findings on that dimension are gain by construction rather than a measured improvement.

    Three discriminating corpus cases, added after the first A/B measured a zero recall delta on the original pair: the baseline arm caught both seeded defects 6/6 without a documentation reviewer, because correctness-reviewer flags a stale comment and a change-narrating comment itself when the diff holds nothing else. Those two defects have a truth-value problem (a comment the change falsified reads as a bug risk), which is what made them visible to a correctness pass. The new cases seed defects with no truth-value problem at all, inside diffs carrying real logic work: a docstring that restates the signature (golden-documentation-restated-docstring), the one added constant whose neighbours explain themselves and it does not (golden-documentation-missing-why), and commented-out code as a matched pair, one call kept with a dated reason and a ticket, one with nothing (golden-documentation-commented-out-code). Each carries its must-not-flag traps in the same diff, and every diff is verified against computeChangedLines with every added line matching the fixture tree byte for byte.

Patch Changes

  • 5943268: Split the Step 3 roster concern out of dispatch.ts into dispatch-roster.ts (DEFAULT_FINDERS, SHED_RANKING, Roster, RosterShed, computeRoster), re-exported from dispatch.ts so callers and tests keep one import surface. No behaviour change: the moved code is byte-identical and every existing test passes unmodified.

    The trigger was a lint failure on main that neither contributing PR could see. @khanacademy/eslint-config caps a file at 1000 lines; [🔥AUDIT🔥] review: fix open-thread suppression, unreachable on every conforming run #302 took dispatch.ts to exactly 1000, and the one line review: add an opt-in documentation reviewer #299 added to the shed ranking took it to 1001. Both were green against their own bases, so the collision existed only in the merge. dispatch.ts is now 920 lines, and the split follows the precedent dispatch-contracts.ts already set for the same budget.

  • 157449c: Open-thread suppression was unreachable on every conforming run, so re-reviews re-posted findings an open bot thread already tracked. openThreadsFromStaged accepted an opener author of github-actions[bot] only, but get_review_comments — the tool review.md tells the orchestrator to copy threads.json from, verbatim — renders that same account as bare github-actions, and the filter fails closed, so a correct staging produced zero usable threads. It also read path from the thread while the tool carries path per comment, and suppressOpenThreadDuplicates matches on path, so a thread that cleared the author check still suppressed nothing. Both spellings are now accepted (BOT_AUTHORS, documented against the REST user.login surface that legitimately renders the bracketed form and that stage-pr.ts reads for prior reviews), and path falls back to the opening comment. Measured on webapp#41197's three-round seeded lifecycle, where suppression reported threadSuppressions: [] in all three rounds while the re-reviews duplicated open threads: 6 of round 3's 8 comments landed on the exact path and line of a thread that was already open, and the reconciler's own keep list held those thread IDs in the same run, so the data suppression needed was present and unusable. Every unit fixture spelled the bot the way the code did, which is why the suite passed throughout; the regression cases now use the tool's real shape verbatim, assert an end-to-end suppression (with the blocking thread still flooring the verdict) rather than only the parse, and keep a human-opened thread refused so the widened author check cannot drop a finding outright. The prompt's selection layer is widened to match, which is the same premise one layer up: review.md told the orchestrator to stage "the unresolved github-actions[bot] threads" and to classify a human thread as "any author other than github-actions[bot]", both bracketed-only, so a literal reading could stage zero bot threads before the widened code filter ever ran — or worse, misfile a bot thread into human-threads.json, where it lands in skipLines and makes the submission drop a fresh finding on that line rather than merely duplicate one. Both instructions now name either spelling and say why. Second half of the fix, since a fail-open guard that cannot be seen failing is how this survived a whole release: stagedThreadShapeFailure reports a staging whose shape defeats the filter entirely, as a threadSuppressionUnavailable field on dispatch-result.json and a run-log warning, so "nothing to suppress" is no longer indistinguishable from "suppression silently did nothing." It counts staged threads per thread_id against the reconciler's resolved set rather than by list length, so a long resolve list cannot mask a total shape failure in a short staging, and it documents its own limit: one usable thread returns nothing, making it a total-failure tripwire rather than a per-thread audit.

@khan-actions-bot
khan-actions-bot requested review from a team, jeresig and kevinb-khan and removed request for a team July 30, 2026 22:04
@github-actions
github-actions Bot force-pushed the changeset-release/main branch 3 times, most recently from d58abfa to dea14a8 Compare July 30, 2026 22:19
@github-actions
github-actions Bot force-pushed the changeset-release/main branch from dea14a8 to d299b64 Compare July 30, 2026 22:19
@jwbron
jwbron merged commit ff31322 into main Jul 30, 2026
@jwbron
jwbron deleted the changeset-release/main branch July 30, 2026 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants