Skip to content

review: move deep-reasoning reviewer roles to Fable 5 - #266

Merged
jwbron merged 5 commits into
mainfrom
jwies/review-fable-swap
Jul 21, 2026
Merged

review: move deep-reasoning reviewer roles to Fable 5#266
jwbron merged 5 commits into
mainfrom
jwies/review-fable-swap

Conversation

@jwbron

@jwbron jwbron commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

De-scoped 2026-07-21 per review: the swap covers correctness-reviewer only. Now based on main (the corpus stack #270/#273 and the drift/budget PRs have landed); the de-scoped roster has its own powered A/B below.

What

Switches the review workflow's correctness-reviewer from claude-opus-4-8 to claude-fable-5 (effort high, unchanged). Bug-finding recall is the workflow's load-bearing metric, and both the original five-role bundle A/B and the de-scoped re-run concentrate their gain in correctness-adjacent rows.

Deliberately NOT moved, per review (jeresig) and per the measurements:

  • claim-validator stays Opus at xhigh: the bundle's Fable validator did not improve the precision gate, and the de-scoped re-run confirms the noise increase never came from the validator (it reproduces without it; see below).
  • holistic / completeness / test-adequacy stay Opus: the bundle A/B could not attribute any gain to them, and both known consumers enable them in ROUTING, so they price into every full review.
  • The orchestrator, thread-reconciler / skill-auditor / conventions, and all specialist lenses stay Opus (the lenses deliberately: Fable's cyber safety classifiers can false-positive refuse benign security analysis, and a refused lens is a silent coverage hole). pattern-triage stays Sonnet 4.6; first-principles already ran Fable 5.

Each stay-on-Opus reason is pinned in the agent definition in review.md. The weekly drift budget is sized from the de-scoped roster's measured rate (~$1.08/case-arm-run): 180 case-arm-runs ~ $194/week, cap 220.

The de-scoped measurement (powered A/B)

Nine pooled dispatches (runs 29855626692-29855643020), same shard-and-pool structure as the bundle run: 30 live cases x 3 repeats per arm; baseline = all-Opus roster, candidate = this branch. Aggregated with eval/aggregate.ts.

Metric Opus 4.8 Fable 5 correctness-only (five-role bundle, superseded)
Must-catch recall 82% (101/123) 89% (110/123) 83% -> 90%
True misses 22 13 21 -> 12
Verdict agreement 96% 96% 94% -> 97%
Noise (unmatched posted) 43% 49% 43% -> 49%
Judge mean quality 0.88 0.86 0.87 -> 0.86
Adversarial hard gate PASSED (all shards) PASSED
Cost $72.12 $97.19 (+35%) +73%

The de-scope kept the entire recall gain at half the cost delta. The improved rows are the same previously in-band/floor spec rows as the bundle run (incident-sql-missing-index:dm-default-backfill 1/3 -> 3/3, trial-retention-deletion:erasure-default-limit-one 0/3 -> 2/3, :erasure-ignores-delete-helper 1/3 -> 2/3, :erasure-prune-error-swallowed 0/3 -> 1/3, golden-retention-fix-push:quota-exceeded-never-true 2/3 -> 3/3, golden-retention-lifecycle-2:retention-prune-unhandled-rejection 2/3 -> 3/3), so the bundle's gain is now attributed: it was the correctness reviewer.

What the re-run disambiguated. The noise increase (43% -> 49%) reproduces without the Fable validator or whole-change reviewers: it is the Fable correctness reviewer posting more companion findings (219 posted vs 178). Likewise the one repeated verdict regression: golden-retention-lifecycle-3 (expected-clean re-review) 3/3 -> 1/3, where two candidate samples flag the deliberate single-batch PRUNE_BATCH prune as blocking. Note both arms in this run predate #271 (merged since), whose correctness-lens language explicitly names the bounded-batch prune as the fix shape, not a defect; the first drift run on merged main will show whether that attenuates this. If not, the corpus-side option is a bounded-prune must-not-flag trap on lifecycle-3.

Statistical honesty. The pooled +7-point recall delta is z ~ 1.7 (suggestive, not conclusive at 95%; pooled intervals are optimistic per the eval README). As with the bundle run, the per-case pattern is the stronger evidence: six hard rows improved, none regressed on recall, true misses down 41%. The aggregate stamps the expected mixed-corpus-hash warning (three shard groups select different case subsets); within every shard both arms share the same ruler.

The superseded five-role bundle measurement (runs 29783522498-29783549087) is preserved in the edit history and summarized in the table's last column.

Known risks to watch in production

  • Occasional refusal stop on cyber-adjacent diffs hitting correctness-reviewer (surfaces as a missing agent result; none observed across ~380 live case-arm-runs to date). The weekly drift corpus is the soft-refusal detector: a silently refusing correctness reviewer craters must-catch recall on the security-adjacent cases.
  • Author-facing chattiness: the Fable correctness reviewer posts more companion findings (noise 49% vs 43% on the eval's strict matcher). The reviewer-side dedup gap (two comments per defect) is the known follow-up that would absorb most of this.
  • Over-blocking on expected-clean re-review pushes (lifecycle-3 above); watch the drift series now that review: teach the correctness and caching lenses to flag unbounded reads #271's lens carve-out is on main.

@changeset-bot

changeset-bot Bot commented Jul 20, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 77e0278

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
review Minor

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

@github-actions

This comment has been minimized.

Comment thread workflows/review/review.md
Comment thread workflows/review/README.md Outdated
@jwbron
jwbron force-pushed the jwies/review-fable-swap branch from cc8b371 to f690b0c Compare July 20, 2026 17:04
@jwbron
jwbron changed the base branch from main to review-eval/corpus-discrimination July 20, 2026 17:04
@jwbron
jwbron force-pushed the jwies/review-fable-swap branch from f690b0c to d74de4b Compare July 20, 2026 17:18
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Comment thread workflows/review/eval/README.md

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested — see inline comments.

Comment thread .github/workflows/review-eval-drift.yml Outdated
@jwbron
jwbron force-pushed the jwies/review-fable-swap branch from efb2d56 to 45424f8 Compare July 20, 2026 21:27
@jwbron
jwbron changed the base branch from review-eval/corpus-discrimination to review-eval/fable-case-calibration July 20, 2026 21:27

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested — see inline comments.

@jwbron
jwbron marked this pull request as ready for review July 20, 2026 23:33
@khan-actions-bot
khan-actions-bot requested review from a team, jeresig and kevinb-khan and removed request for a team July 20, 2026 23:33
@github-actions

This comment has been minimized.

@khan-actions-bot
khan-actions-bot requested a review from a team July 20, 2026 23:47

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — no blocking issues found.

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review live A/B

Baseline: origin/main (review.md 1f18e0251d8f); candidate: working tree (review.md 56aa662fbaad).

Ruler: matcher deterministic+arbiter; corpus cc11988c0918 (9 cases).

Metric Baseline Candidate Delta
Must-catch recall 100% 100% +0%
Verdict agreement 89% 89% +0%
Noise (unmatched posted) 50% 61% +11%
Clean false flags 0 0
Judge mean quality 0.91 0.90 -0.01
Cost $7.32 $10.17
Wall clock 1066s 1309s
Cases run / skipped 9 / 0 9 / 0
Misses found-but-dropped 0 0
Findings anchor-snapped 0 0

Adversarial hard gate: PASSED on the candidate arm.

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 --repeats to resolve smaller effects.

@github-actions

This comment has been minimized.

Comment thread .github/workflows/review-eval-drift.yml Outdated
Comment thread workflows/review/README.md Outdated
| `holistic` | `claude-opus-4-8` | high | Opt-in whole-change reviewer (`enable` in `ROUTING`) |
| `completeness` | `claude-opus-4-8` | high | Opt-in whole-change reviewer (`enable` in `ROUTING`) |
| `test-adequacy` | `claude-opus-4-8` | high | Opt-in whole-change reviewer (`enable` in `ROUTING`) |
| `holistic` | `claude-fable-5` | high | Opt-in whole-change reviewer (`enable` in `ROUTING`) |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait - all of these are on fable?? That seems like extreme overkill here - and not just that but also with high levels of effort.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if I'd call this "extreme overkill". These are all reasoning tasks and fable performs significantly better at reasoning on high.

I'm not set on this selection though, maybe it makes sense to drop some of these back to opus but it will require more evals. The results from this experiment are promising though, fable is clearly a notably better reviewer than opus.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and de-scoped in 48c367f: only correctness-reviewer moves to Fable now (plus first-principles, which already ran it). The bundle A/B's own data supports the narrowing: the recall gain concentrated in correctness-adjacent rows, the Fable claim-validator measurably did not improve the precision gate (noise 43% -> 49%, one wrong blocking flag on a clean case), and the whole-change reviewers' contribution was unattributed while their cost is real; both this repo and webapp enable them in ROUTING, so the "free until enabled" framing was wrong in practice. Each stay-on-Opus reason is now pinned in the agent definition, the drift budget is provisionally re-derived (240 -> 200), and a fresh A/B of the de-scoped roster is running (runs 29855626692-29855643020, same shard-and-pool structure); the PR description will be updated with its numbers before this merges. One caveat for the record: attributing the gain to the correctness reviewer is inference from where the improved rows sit, not a per-role measurement, which is exactly what the re-run checks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fable's analysis above. Running a full A/B test now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

De-scoped A/B results are in (runs 29855626692-29855643020, 30 cases x 3 repeats per arm): the correctness-only swap keeps the entire recall gain (82% -> 89%, true misses 22 -> 13, matching the five-role bundle within noise) at half the cost delta (+35% vs +73%). The re-run also settled the attribution question in your favor: the noise increase and the one verdict regression (over-blocking on the expected-clean lifecycle-3 re-review case) reproduce without the Fable validator or whole-change reviewers, so those roles were adding cost, not signal. Full table and analysis in the updated PR description; drift budget re-sized from the measured rate (~$1.08/case-arm-run -> cap 220).

@jwbron
jwbron force-pushed the jwies/review-fable-swap branch 2 times, most recently from 1aa1dbd to 48c367f Compare July 21, 2026 18:03
@jwbron
jwbron force-pushed the review-eval/fable-case-calibration branch from df5935b to 85469db Compare July 21, 2026 18:13
jwbron added a commit that referenced this pull request Jul 21, 2026
…rift budget (#267)

## What

Two follow-ups from the first scheduled drift report (#265):

1. **Identical-arm eval reports no longer read as an A/B.** When both arms ran the same review.md (only possible under `--force-arms`: the weekly drift watch and manual wobble controls), the single-run, multi-repeat, and aggregate renderers now retitle themselves "Review wobble control (identical arms)", relabel Baseline/Candidate to Arm A/Arm B, and state up front that between-arm deltas are run-to-run wobble, not a prompt effect. The aggregate additionally leads with the noise-floor bands: on an identical-arm pool the bands are the product and the per-case table is the raw material. The aggregate keys the relabel off `noiseFloor` being present, which is computed iff every pooled sample ran identical arms.

2. **A contaminated noise floor is a red run, not a footnote.** The 2026-07-20 run spent $84.33 of its $85 cap and budget-skipped cases, which the report flags as contaminating the noise-floor bands with case-mix variance; but the flag lives inside the report body, so the degradation persists silently until someone reads it. The drift job now fails when the aggregate reports `caseAsymmetry` (after the artifact upload and visibility PR, both `if: always()`), so a corpus outgrowing the budget surfaces as a red run. The budget resize itself (85 -> 120 -> 240) moved to the corpus-growth stack (#270/#266), which owns the sizing; three open PRs were editing the same lines.

## Why the relabel matters

#265 renders a "Baseline vs Candidate" per-case table with small deltas between two runs of the same prompt; that framing invites reading wobble as a result. The drift run's whole point is that within-run deltas are noise, so the report should say so in its own headline.

## Testing

- `pnpm test --run workflows/review/eval/` (new cases cover the multi-repeat, single-run, and aggregate relabels, the noise-floor-first ordering, the identical-arm drop-note prefix, and that A/B reports keep the Baseline/Candidate framing)
- `pnpm typecheck`, prettier clean on changed files

Author: jwbron

Reviewers: github-actions[bot], jwbron, jeresig, kevinb-khan

Required Reviewers:

Approved By: github-actions[bot], jeresig

Checks: ✅ 9 checks were successful, ⏭️  1 check has been skipped

Pull Request URL: #267
jwbron added a commit that referenced this pull request Jul 21, 2026
…ads (#271)

## What

Closes the one genuine reviewer blind spot from the 2026-07-20 drift triage (#265, follow-up to #269/#270): `retention-unbounded-prune` was missed in all 6 drift samples, and the reviewer's own suggested fix for an adjacent bug recommended the same unbounded-read pattern (`pageSize: "all"`), so no lens was reasoning about memory-bounded reads at all.

Two prompt changes:

1. **Correctness lens, line scan**: unbounded reads and accumulation join the named defect classes; a query/fetch/scan that materializes a result set that grows with user data (missing LIMIT, `pageSize: "all"`, loading a whole table to act on a subset, unpaginated buffering). The rule also names the expected shape (page or batch it), so the fix pattern (a bounded read that deliberately processes one batch per invocation) is not itself flagged; that matters because `golden-retention-lifecycle-2/3`'s `PRUNE_BATCH` prune is exactly that shape and must stay clean.
2. **caching-resource specialist**: a matching "No unbounded reads" review rule and an `unbounded-read-materialization` tri-state hunt. The correctness lens is the always-on catch; the specialist adds depth when routed. (Note per review: no ROUTING routes `caching-resource` today, in this repo or webapp, so the correctness-lens rule is the operative change; the specialist half is the documented home for depth once a consuming repo routes storage-heavy paths to it.)

## Measurement

Recall-affecting by design. The original acceptance recipe targeted `golden-retention-lifecycle-1:retention-unbounded-prune`, but the corpus stack (#270/#273/#266) retires that spec: the lifecycle-1 facade documents no batch limit, so under a lens-less ruler the defect was ruled not evidenceable in-tree, and the slot now tracks the void-prune unhandled rejection. This PR is exactly the lens that makes the retired row fair again (the lens supplies the norm the tree does not state), so the plan is:

1. Land the corpus stack first, then rebase this PR onto main.
2. Re-add the retired `retention-unbounded-prune` spec to `golden-retention-lifecycle-1` in this PR (spec text is in git history at #270's parent), as the measurement instrument for the lens: without it, no corpus row tracks flagging an *undocumented* unbounded read (#270's review noted the gap).
3. Run the powered recipe:

```
gh workflow run review-eval-ab.yml --ref review-unbounded-read-lens \
  -f base_ref=origin/main -f cases=golden-retention-lifecycle-1 -f repeats=10 -f max_usd=30
```

Success: candidate arm catches `retention-unbounded-prune` at a real rate (baseline is 0/6 in the drift record) with lifecycle-2/3 clean on the bounded `PRUNE_BATCH` prune (no false flags), and no new flags on `mutation-retention-batch-delete-limit`'s already-caught defect beyond the existing catch.

Author: jwbron

Reviewers: github-actions[bot], jwbron, jeresig, jaredly, somewhatabstract

Required Reviewers:

Approved By: github-actions[bot], jeresig

Checks: ⏭️  1 check has been skipped, ✅ 9 checks were successful

Pull Request URL: #271
@github-actions

Copy link
Copy Markdown
Contributor

Review Guidance

github-actions (2 files)
File Reason
review.md Shared reviewer prompt executed in every consuming repo; swaps the default-roster correctness-reviewer's pinned model to claude-fable-5.
review-eval-drift.yml Weekly scheduled workflow; raises the drift-run spend cap (120 to 200), which gates how much of the corpus runs before budget-skipping.

Common patterns

2 files: correctness-reviewer model swapped from claude-opus-4-8 to claude-fable-5 (workflows/review/review.md, workflows/review/README.md).

-model: claude-opus-4-8
+model: claude-fable-5

2 files: eval budget cap re-sized for the Fable corpus (30 live cases x3) — default and recipe max_usd raised to 200 (.github/workflows/review-eval-drift.yml, workflows/review/eval/README.md).

-        default: "120"
+        default: "200"

Comment thread .changeset/review-fable-deep-reasoning.md
jwbron added a commit that referenced this pull request Jul 21, 2026
…calibration finding (#273)

> Stacked on #270. Companion to #266 (the Fable model swap), split out so the model swap stays a pure, revertable diff and the corpus work is reviewable on its own.

## What

Grows the live eval corpus by seven cases in this PR's diff (nine across the stack, counting the two minted in #270) across seven defect families it previously had no coverage for, each with a verified ground-truth defect, an exact git-generated diff, and deterministically matched specs (0 anchor snaps and 0 found-but-dropped across every calibration run):

| Case | Family |
| --- | --- |
| mutation-removed-behavior-logout-cache | Removed-behavior regression (refactor drops cache invalidation) |
| mutation-crossfile-report-redaction | Cross-file inference chain (PII to a partner webhook) |
| mutation-needle-paging-rename | Needle in 13 files of mechanical churn, plus a mustNotFlagSpec precision probe on a correctly migrated call site |
| mutation-retry-nonidempotent-capture | Partial-failure / distributed semantics (retry double-charge) |
| mutation-toctou-invite-limit | Check-then-act concurrency (seat-limit race) |
| mutation-window-boundary-doublecount | Boundary composition (inclusive endpoints double-count) |
| mutation-largetree-digest-category-bypass | 29-file cross-subsystem tree; the violated invariant sits two unchanged hops from the diff |

All are tagged `live` (never `smoke`), so per-PR cost is unchanged; they run in full-corpus, powered, and weekly drift selections.

## The calibration finding (the important part)

Every case was calibrated with identical-Opus-arm `--force-arms` runs before any band claim (runs 29763213774, 29764855482, 29765059892, 29765275168, 29767404342): all nine, plus the two minted in #270, read 100% recall across ~56 identical-arm samples, including a hardening pass that removed every stated invariant and the 29-file scale test. Hand-authored synthetics do not reach the 20-80% discrimination band at any feasibly authorable size; every case that has ever calibrated in-band or at floor derives from real material. This is now documented in `eval/README.md` under "Growing the corpus", with the operational guidance: author synthetics as deliberate family tripwires, grow recall discrimination from golden human-comment cases and production incident repros, and calibrate every new case before claiming a band.

The unmatched-posted findings on these cases were audited: they are companion findings on the already-caught defect from a second angle (the known reviewer-side dedup gap #270 documented), deliberately not annotated as specs.

## Notes

- Case descriptions carry their calibration outcomes and run ids inline, so the drift series and future authors see the classification without archaeology.
- The weekly drift budget bump for the grown corpus rides in #266 (its sizing depends on the Fable-arm rates that PR introduces).

Author: jwbron

Reviewers: github-actions[bot], jwbron, jeresig, kevinb-khan

Required Reviewers:

Approved By: github-actions[bot], jeresig

Checks: ✅ 10 checks were successful

Pull Request URL: #273
@jwbron
jwbron changed the base branch from review-eval/fable-case-calibration to main July 21, 2026 18:34
An error occurred while trying to automatically change base from review-eval/fable-case-calibration to main July 21, 2026 18:34
jwbron added 4 commits July 21, 2026 11:38
…viewer roles (correctness, claim-validator, opt-in whole-change) to Fable 5
…count to 30 and re-derive the cap to 240 (review feedback)
… for the Fable-arm corpus

The recipe figures moved here from the wobble-relabel PR so a single PR
owns the sizing: full-corpus dispatches at post-swap rates run ~$185
(vs main) to ~$225 (identical Fable arms) and need max_usd=240, in
line with the drift workflow default this PR sets.
…ectness reviewer

Per review (jeresig): the pooled A/B measured the five-role bundle, but
its recall gain concentrated in correctness-adjacent rows, the Fable
claim-validator measurably did not improve the precision gate (noise
43% -> 49%, one wrong blocking flag on a clean case), and the opt-in
whole-change reviewers' contribution is unattributed while their cost
is real (both known consumers enable them in ROUTING, so 'free until
enabled' was wrong in practice). Only correctness-reviewer moves to
Fable 5 (first-principles already ran it); claim-validator, the
whole-change reviewers, the procedural roles, and the lenses stay on
Opus 4.8, each with its reason pinned in the agent definition. Drift
budget re-derived provisionally (240 -> 200) pending the de-scoped
A/B's measured rate; eval README recipes re-sized to match.
@jwbron
jwbron force-pushed the jwies/review-fable-swap branch from 48c367f to 4eea3fe Compare July 21, 2026 18:38
…coped A/B's measured rate

The pooled A/B (runs 29855626692-29855643020) measured the
Fable-correctness roster at ~$1.08 per case-arm-run ($97.19 / 90), so
a drift week at 180 case-arm-runs is ~$194: the provisional 200 left
~3% headroom against a cap whose breach is now a red run. 220 clears it
with margin; eval README recipe figures updated to the measured rates.
@jwbron
jwbron force-pushed the jwies/review-fable-swap branch from 642472a to 77e0278 Compare July 21, 2026 19:01
@jwbron
jwbron merged commit f4deabf into main Jul 21, 2026
11 checks passed
@jwbron
jwbron deleted the jwies/review-fable-swap branch July 21, 2026 19:42
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
jwbron added a commit that referenced this pull request Jul 31, 2026
…ck my rebase dropped

Re-applying only the pin changes onto main's frontmatter lost the load-bearing
half of this PR: the models.default-ai-credits-pricing block. Without it every
dispatch 400s before reaching the model, so the branch as I rebased it would
have failed every run.

Verified rather than assumed: claude-opus-5 is absent from the firewall
api-proxy's curated pricing table at v0.27.42, the release gh-aw v0.83.4
defaults to now that this workflow's sandbox.agent.version pin is retired. The
table carries claude-opus-4-5 through 4-8 and claude-fable-5 and stops there.

The blast radius is worse than when this PR was written: main retired the
sandbox pin on the grounds that v0.27.42 prices claude-fable-5, which is true
and irrelevant once the roster is on Opus 5. And with all 22 pins on the
un-priced model, a missing fallback 400s the orchestrator and every default
agent rather than the two opt-in dispatches that #266 cost.

The gh-aw >= v0.83.0 floor this field needs is already met (v0.83.4), so the
compile-time constraint on consumers stands as documented but this repo clears
it.
jwbron added a commit that referenced this pull request Jul 31, 2026
…ck my rebase dropped

Re-applying only the pin changes onto main's frontmatter lost the load-bearing
half of this PR: the models.default-ai-credits-pricing block. Without it every
dispatch 400s before reaching the model, so the branch as I rebased it would
have failed every run.

Verified rather than assumed: claude-opus-5 is absent from the firewall
api-proxy's curated pricing table at v0.27.42, the release gh-aw v0.83.4
defaults to now that this workflow's sandbox.agent.version pin is retired. The
table carries claude-opus-4-5 through 4-8 and claude-fable-5 and stops there.

The blast radius is worse than when this PR was written: main retired the
sandbox pin on the grounds that v0.27.42 prices claude-fable-5, which is true
and irrelevant once the roster is on Opus 5. And with all 22 pins on the
un-priced model, a missing fallback 400s the orchestrator and every default
agent rather than the two opt-in dispatches that #266 cost.

The gh-aw >= v0.83.0 floor this field needs is already met (v0.83.4), so the
compile-time constraint on consumers stands as documented but this repo clears
it.
jwbron added a commit that referenced this pull request Jul 31, 2026
…ck my rebase dropped

Re-applying only the pin changes onto main's frontmatter lost the load-bearing
half of this PR: the models.default-ai-credits-pricing block. Without it every
dispatch 400s before reaching the model, so the branch as I rebased it would
have failed every run.

Verified rather than assumed: claude-opus-5 is absent from the firewall
api-proxy's curated pricing table at v0.27.42, the release gh-aw v0.83.4
defaults to now that this workflow's sandbox.agent.version pin is retired. The
table carries claude-opus-4-5 through 4-8 and claude-fable-5 and stops there.

The blast radius is worse than when this PR was written: main retired the
sandbox pin on the grounds that v0.27.42 prices claude-fable-5, which is true
and irrelevant once the roster is on Opus 5. And with all 22 pins on the
un-priced model, a missing fallback 400s the orchestrator and every default
agent rather than the two opt-in dispatches that #266 cost.

The gh-aw >= v0.83.0 floor this field needs is already met (v0.83.4), so the
compile-time constraint on consumers stands as documented but this repo clears
it.
jwbron added a commit that referenced this pull request Jul 31, 2026
…ck my rebase dropped

Re-applying only the pin changes onto main's frontmatter lost the load-bearing
half of this PR: the models.default-ai-credits-pricing block. Without it every
dispatch 400s before reaching the model, so the branch as I rebased it would
have failed every run.

Verified rather than assumed: claude-opus-5 is absent from the firewall
api-proxy's curated pricing table at v0.27.42, the release gh-aw v0.83.4
defaults to now that this workflow's sandbox.agent.version pin is retired. The
table carries claude-opus-4-5 through 4-8 and claude-fable-5 and stops there.

The blast radius is worse than when this PR was written: main retired the
sandbox pin on the grounds that v0.27.42 prices claude-fable-5, which is true
and irrelevant once the roster is on Opus 5. And with all 22 pins on the
un-priced model, a missing fallback 400s the orchestrator and every default
agent rather than the two opt-in dispatches that #266 cost.

The gh-aw >= v0.83.0 floor this field needs is already met (v0.83.4), so the
compile-time constraint on consumers stands as documented but this repo clears
it.
jwbron added a commit that referenced this pull request Jul 31, 2026
…ck my rebase dropped

Re-applying only the pin changes onto main's frontmatter lost the load-bearing
half of this PR: the models.default-ai-credits-pricing block. Without it every
dispatch 400s before reaching the model, so the branch as I rebased it would
have failed every run.

Verified rather than assumed: claude-opus-5 is absent from the firewall
api-proxy's curated pricing table at v0.27.42, the release gh-aw v0.83.4
defaults to now that this workflow's sandbox.agent.version pin is retired. The
table carries claude-opus-4-5 through 4-8 and claude-fable-5 and stops there.

The blast radius is worse than when this PR was written: main retired the
sandbox pin on the grounds that v0.27.42 prices claude-fable-5, which is true
and irrelevant once the roster is on Opus 5. And with all 22 pins on the
un-priced model, a missing fallback 400s the orchestrator and every default
agent rather than the two opt-in dispatches that #266 cost.

The gh-aw >= v0.83.0 floor this field needs is already met (v0.83.4), so the
compile-time constraint on consumers stands as documented but this repo clears
it.
jwbron added a commit that referenced this pull request Jul 31, 2026
…ck my rebase dropped

Re-applying only the pin changes onto main's frontmatter lost the load-bearing
half of this PR: the models.default-ai-credits-pricing block. Without it every
dispatch 400s before reaching the model, so the branch as I rebased it would
have failed every run.

Verified rather than assumed: claude-opus-5 is absent from the firewall
api-proxy's curated pricing table at v0.27.42, the release gh-aw v0.83.4
defaults to now that this workflow's sandbox.agent.version pin is retired. The
table carries claude-opus-4-5 through 4-8 and claude-fable-5 and stops there.

The blast radius is worse than when this PR was written: main retired the
sandbox pin on the grounds that v0.27.42 prices claude-fable-5, which is true
and irrelevant once the roster is on Opus 5. And with all 22 pins on the
un-priced model, a missing fallback 400s the orchestrator and every default
agent rather than the two opt-in dispatches that #266 cost.

The gh-aw >= v0.83.0 floor this field needs is already met (v0.83.4), so the
compile-time constraint on consumers stands as documented but this repo clears
it.
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