review: always submit REQUEST_CHANGES with a non-empty body - #262
Merged
Conversation
…on-empty body (GitHub rejects an empty one)
🦋 Changeset detectedLatest commit: 4dd59a0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
khan-actions-bot
requested review from
a team,
jeresig and
somewhatabstract
and removed request for
a team
July 14, 2026 01:11
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
Review live A/BBaseline: Ruler: matcher deterministic+arbiter; corpus 0c1680e66eae (9 cases).
Adversarial hard gate: PASSED on the candidate arm. Agent failures
Single-run-stable rows: recall, verdict agreement, regressions, adversarial gate. Judge quality and noise are not: they jitter run-to-run at this corpus size, and a regressed reviewer can score HIGHER on judge quality (fewer, surer comments each read better). Recall against the labeled specs is the load-bearing metric. Measured noise floor (identical arms, run 29069228968, 2026-07-10, 6 arm-samples, full corpus x3, pre-arbiter; budget skips left the samples on unequal case sets, so these v1 bands also carry case-mix variance): must-catch recall 54%-86% (sd 10%), verdict agreement 75%-100% (sd 9%), noise (unmatched posted) 50%-60% (sd 3%), judge mean quality 82%-86% (sd 2%). A single-run delta whose arms both sit inside a band is indistinguishable from run-to-run wobble; use |
…ROVE; align the deterministic renderReviewBody mirror (review feedback)
Contributor
Review Guidancegithub-actions (2 files)
|
Member
|
Ahh ok - that's too bad! |
jwbron
added a commit
that referenced
this pull request
Jul 21, 2026
## Why The reviewer installed on this repo (`.github/workflows/review.md`) was still pinned at `review-v1.4.1`, the version `gh aw add` recorded on install (#241, 2026-07-10), and had never been updated since: five releases behind the shared source (now bumped all the way to v1.7.0, released today in #278 with the Fable-correctness reviewer, the recap fixes, and the unbounded-read lens). This surfaced on PR #266, where re-review 4739236283 posted a REQUEST_CHANGES whose entire body was "Changes requested — see inline comments." with **zero** inline comments attached (the blocking thread lives on an earlier review). Later releases address exactly that: - **v1.5.0 (#244/#246)**: the code-rendered re-review accountability section, which enumerates every still-open prior thread (linked, blocking first, "as of \<sha\>") in the verdict body. - **v1.6.1 (#262)**: REQUEST_CHANGES is always submitted with a non-empty body; the empty-body path silently downgraded a blocking verdict to COMMENTED (observed on Khan/webapp#40793). ## Why not `gh aw update` The documented update path fails on this repo: gh-aw v0.81.6's `resolveLatestRef` only treats the pinned ref as a tag when `isSemanticVersionTag()` accepts it, and that check (`semverutil.IsValid`) rejects changesets-style prefixed tags like `review-v1.4.1`. It falls through to branch resolution and 404s (`failed to get latest commit for branch review-v1.4.1`). Might be worth an upstream issue; until then, updates of this workflow are manual. ## What this PR does - Replicates `gh aw update`'s 3-way merge by hand: base = `review-v1.4.1` source, ours = installed copy, theirs = `review-v1.7.0` source (identical to current main), then `gh aw compile review`. Diff vs the v1.6.1 source is exactly the documented `KHAN/ACTIONS LOCAL OVERRIDE` edits and nothing else: - the same-repo fork guard in `if:` (public-repo hardening); - the `observability:` block stays commented out (the `GH_AW_OTEL_SENTRY_*` secrets still exist neither on this repo nor at org level; the block hard-fails the agent job when they are missing); - `max-ai-credits: 2500`, now mirrored into the new `REVIEW_MAX_AI_CREDITS` env var per the KEEP-IN-SYNC rule v1.6.0 added; - `source:` and the lib checkout `ref:` both bumped to `review-v1.7.0` (kept in lockstep, enforced by `review-pins.test.ts`). - **Per review: the "every difference is a documented override" claim is now CI-enforced, not hand-audited.** `review-pins.test.ts` diffs the installed copy against `workflows/review/review.md` at the pinned tag (fetching the tag when the checkout is shallow) and fails when any hunk lacks a `KHAN/ACTIONS LOCAL OVERRIDE` marker, so the next manual bump cannot silently drop an override or an upstream hunk. - **Per review: the re-review mode dial is no longer part of this PR.** The motivating defect is fixed by the version bump alone (the non-empty-body rule and the accountability section apply at the default `full` mode), so enabling a reduced-depth mode is a separable decision; it follows in its own PR starting at `scoped` rather than `flip-gated`, per the review's depth argument, once a re-review or two has been observed on v1.7.0 (#277). ## Verification - `pnpm test`: passes, including the new content guard, `review-pins.test.ts` pin consistency (source/ref/lock literals all `review-v1.7.0`), and `version-sync.test.ts`. The guard was negative-tested: an unmarked stray edit to the installed copy fails it. - `gh aw compile review`: 0 errors, 0 warnings. - No changeset needed: all three files are under `.github/`, the check's default exclusion, and the shared `workflows/review` package is untouched. Author: jwbron Reviewers: github-actions[bot], jwbron, jeresig, kevinb-khan Required Reviewers: Approved By: github-actions[bot], jeresig Checks: ✅ 8 checks were successful, ⏭️ 1 check has been skipped Pull Request URL: #276
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
2 participants
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.
GitHub rejects a REQUEST_CHANGES review event with an empty body, and Step 6 explicitly instructed submitting one ("...so submit it with an empty body", dating to #197). Under gh-aw v0.81.6 the safe-output flow posts the review event separately from the inline comments, so the comments do not make the event non-empty.
Observed live on the v1.5.0 behavior test (Khan/webapp#40793): the reviewer computed REQUEST_CHANGES correctly for two blocking findings, the submission failed with "Review body is required for REQUEST_CHANGES", and the PR was left with a COMMENTED review and no blocking verdict. The instruction survived this long because models usually filled the body anyway; this run followed it literally.
The fix: the REQUEST_CHANGES body's first line is now always
Changes requested — see inline comments., with the accountability section, note lines, and fingerprint stamp appended as before. The APPROVE empty-body case is unchanged (GitHub allows it).cc @jeresig