From 0afc12e026469727049152140ebf171eebb1eb7f Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Fri, 10 Jul 2026 13:45:36 -0700 Subject: [PATCH 1/3] [jwies/review-shed-tuning] review: budget and shed tuning from the 07-10 production sheds The 07-10 reviews on #232/#238 shed claim validation and three whole-change dimensions on substantial PRs. Recalibrate the tier budgets (overhead-aware wall clocks; low/medium invocation caps that fit the seven-reviewer roster), exempt pipeline steps from the invocation cap, gate shedding the claim-validator on hard-ceiling pressure only, and split the skipped-dimension note wording so a planned budget shed no longer reads as an infrastructure failure. This repo's ROUTING re-raises .claude/skills/** and eval plan docs from the broad trivial-docs rule. --- .changeset/review-shed-tuning.md | 5 +++++ .github/aw/review/ROUTING | 10 ++++++++- workflows/review/lib/router.ts | 35 ++++++++++++++++++++++++------- workflows/review/review.md | 36 +++++++++++++++++++++++++------- 4 files changed, 70 insertions(+), 16 deletions(-) create mode 100644 .changeset/review-shed-tuning.md diff --git a/.changeset/review-shed-tuning.md b/.changeset/review-shed-tuning.md new file mode 100644 index 00000000..31c6c0b9 --- /dev/null +++ b/.changeset/review-shed-tuning.md @@ -0,0 +1,5 @@ +--- +"review": patch +--- + +Budget and shed tuning from the 2026-07-10 production sheds (the reviews on Khan/actions #232 and #238, which shed claim validation and three whole-change dimensions on substantial PRs that had routed to the bottom tiers). Three changes. The tier budget table is recalibrated against measured runs: wall clocks now account for the ~3 minutes of fixed staging/router/triage overhead a run spends before the first reviewer returns (trivial 3->6, low 6->10, medium 12->15), and the low/medium invocation caps now fit the standard seven-reviewer whole-change roster (low 4->8, medium 8->10), which the old low cap of 4 deterministically shed on every run. The invocation cap is now explicitly finding-producers only: pattern-triage, thread-reconciler, and the claim-validator are pipeline steps and never consume a slot (the 07-10 runs counted them inconsistently). And the claim-validator moves to the back of the shed order with a hard-ceiling gate: it may be shed only when the job timeout or credits cap is genuinely close, never at a mere soft-target breach, since its cost tracks the visible candidate count rather than the diff. Skipped-dimension notes now distinguish the two causes: a planned budget shed reads `shed under the -tier run budget` and the `output unavailable` wording is reserved for a sub-agent that was dispatched but returned nothing parseable, so an operator can tell budget arithmetic from an infrastructure failure. (This repo's ROUTING also re-raises `.claude/skills/**` and `workflows/review/eval/**/*.md` from the broad trivial-docs rule; that file is consumer config, not part of the package.) diff --git a/.github/aw/review/ROUTING b/.github/aw/review/ROUTING index fef3c570..a2a61a47 100644 --- a/.github/aw/review/ROUTING +++ b/.github/aw/review/ROUTING @@ -29,7 +29,7 @@ workflows/review/eval/** tier=low # are re-raised after the broad docs rule (last match wins). Any shared workflow's # prompt (and prompt imports beside it) is high; the docs living in the same # directory drop back to trivial. Single-star segments do not cross into -# subdirectories, so nothing under eval/ gets re-raised. +# subdirectories. **/*.md tier=trivial .changeset/*.md tier=trivial workflows/*/*.md tier=high @@ -37,6 +37,14 @@ workflows/*/README.md tier=trivial workflows/*/CHANGELOG.md tier=trivial .github/workflows/*.md tier=high +# More executable-not-docs prose the broad docs rule would drop to trivial: +# skills steer the Claude agents that operate on this repo, and eval plan docs +# drive the eval suite's implementation, so they keep the eval dir's dev-only +# tier. (The 2026-07-10 reviews on #232/#238 routed both as trivial docs and +# shed review dimensions for it.) +.claude/skills/** tier=medium +workflows/review/eval/**/*.md tier=low + # The reviewer's own consumer config: config.md carries the add-reviewer team # allowlist and bot token, ROUTING sets these tiers, and the prose imports steer # every sub-agent. A PR tampering with any of them must not run at the trivial diff --git a/workflows/review/lib/router.ts b/workflows/review/lib/router.ts index 28996187..26f46181 100644 --- a/workflows/review/lib/router.ts +++ b/workflows/review/lib/router.ts @@ -157,13 +157,23 @@ export type RunBudget = { tier: RiskTier; /** True when the misrouted floor lifted the budget above the touched tier. */ floored: boolean; - /** Upper bound on specialist+whole-change reviewer invocations for the run. */ + /** + * Upper bound on specialist+whole-change reviewer invocations for the + * run. Only finding-producing dispatches count; pipeline steps + * (`pattern-triage`, `thread-reconciler`, `claim-validator`) never + * consume a slot. + */ maxReviewerInvocations: number; /** Upper bound on investigation tool calls a single finding may spend. */ maxToolCallsPerFinding: number; /** Upper bound on investigation tool calls across the whole run. */ maxTotalToolCalls: number; - /** Soft wall-clock ceiling (minutes) the orchestrator should target. */ + /** + * Soft wall-clock ceiling (minutes) the orchestrator should target, + * measured from run start; it therefore includes the run's fixed + * staging/router/triage overhead (~3 minutes measured), not just + * reviewer time. + */ maxWallClockMinutes: number; /** Soft spend ceiling (USD) the orchestrator should target. */ maxUsd: number; @@ -254,6 +264,17 @@ export type RouteInput = { * per-run ceiling. Every field scales monotonically with the tier, and the * table is exported so the eval suite and consumers can override it without a * code change. + * + * Calibration (re-measured 2026-07-10 against production runs on + * Khan/actions #232/#238): a run spends ~3 minutes of fixed overhead + * (staging, router, provenance, pattern-triage) before the first reviewer + * returns, so wall clocks below ~6 minutes put a run past its shed threshold + * before any review work lands; and the standard enabled roster is seven + * whole-change reviewers (two defaults plus the five eval-justified opt-ins + * both current consumers enable), so an invocation cap of 4 deterministically + * shed dimensions on every low-tier run. Low and medium now fit the roster; + * trivial stays deliberately small (a trivial-only PR gets the two default + * reviewers, with the rest declared as budget sheds). */ export const DEFAULT_TIER_BUDGETS: Record = { trivial: { @@ -262,25 +283,25 @@ export const DEFAULT_TIER_BUDGETS: Record = { maxReviewerInvocations: 2, maxToolCallsPerFinding: 2, maxTotalToolCalls: 10, - maxWallClockMinutes: 3, + maxWallClockMinutes: 6, maxUsd: 0.5, }, low: { tier: "low", floored: false, - maxReviewerInvocations: 4, + maxReviewerInvocations: 8, maxToolCallsPerFinding: 3, maxTotalToolCalls: 20, - maxWallClockMinutes: 6, + maxWallClockMinutes: 10, maxUsd: 1.5, }, medium: { tier: "medium", floored: false, - maxReviewerInvocations: 8, + maxReviewerInvocations: 10, maxToolCallsPerFinding: 5, maxTotalToolCalls: 60, - maxWallClockMinutes: 12, + maxWallClockMinutes: 15, maxUsd: 4, }, high: { diff --git a/workflows/review/review.md b/workflows/review/review.md index 4d4c88ba..23cc9067 100644 --- a/workflows/review/review.md +++ b/workflows/review/review.md @@ -727,8 +727,10 @@ estimate dollars; watch the signals you can observe, as spend proxies: against the run start you recorded in Step 1 at each later checkpoint. This is the sharpest proxy, and the job-timeout ceiling it guards is just as fatal as the credits cap. -- **Dispatch count** vs `runBudget.maxReviewerInvocations`: reviewers and lenses - already dispatched plus still pending. +- **Dispatch count** vs `runBudget.maxReviewerInvocations`: finding-producing + reviewers and lenses already dispatched plus still pending. Only those count. + `pattern-triage`, `thread-reconciler`, and the `claim-validator` are pipeline + steps, not reviewers; they never consume a slot of this cap. - **Run-wide investigation usage** vs `runBudget.maxTotalToolCalls`: one line per authorised call in `/tmp/gh-aw/review/investigation-journal.log` (`wc -l`). - **Trajectory**: an unusually large diff, many sub-agents still pending, many @@ -742,8 +744,16 @@ order: a skipped dimension (Step 6 note). 2. Skip the risks/patterns comment and reviewer requests (Steps 7-8) if they have not happened yet. -3. If the `claim-validator` has not run, post the unvalidated candidates under the - existing missing-validator rule (Phase 3) with its skipped-dimension note. +3. Last, and never at the soft targets alone: the `claim-validator`. It is the + false-positive gate, and its cost scales with the candidate count (which you + can already see when deciding), not with the diff, so validating a small + candidate set costs less than one reviewer dispatch. Shed it only when a + hard ceiling is genuinely close (elapsed wall clock past three-quarters of + the job's `timeout-minutes`, or an equally direct signal that the credits + cap is near); at a mere soft-target breach, dispatch it anyway and shed + elsewhere. When it is shed, post the unvalidated candidates under the + missing-validator rule (Phase 3), using the planned-shed wording of the + skipped-dimension note (Step 6). Then go straight to Steps 4-6: compute the verdict from the findings already validated, post the surviving comments, and submit the review with one @@ -1017,10 +1027,20 @@ keep the body to a single line: Changes requested — see inline comments. ``` -**Skipped dimensions (either verdict).** If a sub-agent's output was unavailable this -run so a dimension could not be assessed (Step 3), append to the review body — after -any verdict-specific text above — one line per skipped dimension, exactly: -`Note: not assessed this run ( output unavailable).` If the +**Skipped dimensions (either verdict).** If a dimension could not be assessed this +run (Step 3), append to the review body — after +any verdict-specific text above — one line per skipped dimension, choosing the +wording by cause: + +- Planned shed (the budget rule stopped the sub-agent from being dispatched): + `Note: not assessed this run (shed under the -tier run budget).` +- The sub-agent was dispatched but its output was missing or unparseable: + `Note: not assessed this run ( output unavailable).` + +The two read very differently to an operator (a shed is budget arithmetic and +expected on small-tier runs; an unavailable output is a failure worth +investigating), so never use the `unavailable` wording for work you chose not +to start. If the change-provenance gate was skipped because `provenance.json` was missing or carried warnings (Step 3), also append exactly: `Note: change-provenance gate skipped this run (diff staging unparseable).` From a31b9405cd559eb3fa1136438e5a7773543ee664 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Fri, 10 Jul 2026 14:00:08 -0700 Subject: [PATCH 2/3] [jwies/review-shed-ranking] review: value-ranked shedding and dispatch; never shed reviewer requests The shed list's first bucket had no ordering, so a budget-pressed run shed arbitrarily and an under-cap dispatch filled slots by enablement mechanism instead of value. Rank both: shed generic dimensions first (conventions, first-principles, holistic, completeness/test-adequacy) and matched specialist lenses last; dispatch in the reverse order when the invocation cap cannot fit the roster. Reviewer requests (Step 8) leave the shed list entirely: one tool call, and the human fallback matters most when the bot's own coverage is partial. --- .changeset/review-shed-ranking.md | 5 +++++ workflows/review/review.md | 25 ++++++++++++++++++++----- 2 files changed, 25 insertions(+), 5 deletions(-) create mode 100644 .changeset/review-shed-ranking.md diff --git a/.changeset/review-shed-ranking.md b/.changeset/review-shed-ranking.md new file mode 100644 index 00000000..bc7558b2 --- /dev/null +++ b/.changeset/review-shed-ranking.md @@ -0,0 +1,5 @@ +--- +"review": patch +--- + +Value-ranked shedding and dispatch. The graceful-landing rule's first shed bucket previously said "skip any not-yet-dispatched opt-in reviewers and lenses" with no ordering, so a budget-pressed run shed arbitrarily; the 07-10 production runs showed the consequence (an invocation cap smaller than the roster filled its slots by enablement mechanism, not value, and a matched specialist lens could be shed to afford `conventions`). The shed order is now explicit, lowest value first: conventions, first-principles, holistic, completeness/test-adequacy, and path-triggered specialist lenses last, since a matched lens is the most targeted signal in the run. The same ranking read from the other end (defaults, lenses, targeted opt-ins, generic opt-ins) is now the mandated dispatch order when `maxReviewerInvocations` cannot fit the full roster, with every undispatched reviewer recorded as a planned shed. And reviewer requests (Step 8) are removed from the shed list entirely: a request is one tool call, and pulling a human in matters most on exactly the run whose own coverage is partial. The ranking is a first-cut editorial ordering; replace it with measured per-dimension must-catch contribution from the eval corpus when that data is compiled. diff --git a/workflows/review/review.md b/workflows/review/review.md index 23cc9067..643a1134 100644 --- a/workflows/review/review.md +++ b/workflows/review/review.md @@ -527,7 +527,11 @@ either run by default; a reviewer earns its `enable` line through the eval suite not by shipping). Dispatch the default reviewers (`correctness-reviewer`, `skill-auditor`, `thread-reconciler`) **plus** every reviewer named in `enabledReviewers` **plus** every lens named in `lensesToSpawn`, all **in parallel** -(one turn), and wait for all. +(one turn), and wait for all. If `runBudget.maxReviewerInvocations` cannot fit +that whole set, fill the slots by the dispatch ranking (the budget rule below, +Step 3 Phase 3): defaults first, then matched lenses, then the targeted opt-in +dimensions, then the generic ones. Never choose arbitrarily, and record every +reviewer left undispatched as a planned shed (Step 6 note). **One candidate contract.** Every finding-producing reviewer returns `findings[]` in the same shape (a `label` per finding, from the fixed label set in Step 4); a @@ -740,10 +744,21 @@ When any proxy passes roughly three-quarters of its soft target (or the trajecto is clearly expensive), stop starting new work and shed remaining work in this order: -1. Skip any not-yet-dispatched opt-in reviewers and specialist lenses; each becomes - a skipped dimension (Step 6 note). -2. Skip the risks/patterns comment and reviewer requests (Steps 7-8) if they have - not happened yet. +1. Skip not-yet-dispatched opt-in reviewers and specialist lenses in value + order, lowest value first; each becomes a skipped dimension (Step 6 note). + The ranking, from first-shed to last-shed: `conventions`, then + `first-principles`, then `holistic`, then `completeness` and + `test-adequacy`, and only then any path-triggered specialist lens from + `lensesToSpawn`. A matched lens is the most targeted signal in the run (the + router chose it for the specific files this PR touches), so it outranks + every generic dimension; shedding `security-auth` on an auth-path diff to + afford `conventions` is exactly backwards. This same ranking, read from the + other end (defaults, lenses, targeted opt-ins, generic opt-ins), is the + dispatch order when the invocation cap cannot fit the roster (Phase 2). +2. Skip the risks/patterns comment (Step 7) if it has not happened yet. + Reviewer requests (Step 8) are **never** shed: a request is a single tool + call, and pulling a human in matters most on exactly the run whose own + coverage is partial. 3. Last, and never at the soft targets alone: the `claim-validator`. It is the false-positive gate, and its cost scales with the candidate count (which you can already see when deciding), not with the diff, so validating a small From fd3ad8e54d684f2d8eea45ae5d602d20f64ab496 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Mon, 13 Jul 2026 10:48:49 -0700 Subject: [PATCH 3/3] [jwies/review-shed-ranking] review: shed-ranking review feedback (accurate cross-reference, provisional-ordering note, scoped step 8 rationale, minor bump) --- .changeset/review-shed-ranking.md | 4 ++-- workflows/review/review.md | 14 ++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/.changeset/review-shed-ranking.md b/.changeset/review-shed-ranking.md index bc7558b2..54756b81 100644 --- a/.changeset/review-shed-ranking.md +++ b/.changeset/review-shed-ranking.md @@ -1,5 +1,5 @@ --- -"review": patch +"review": minor --- -Value-ranked shedding and dispatch. The graceful-landing rule's first shed bucket previously said "skip any not-yet-dispatched opt-in reviewers and lenses" with no ordering, so a budget-pressed run shed arbitrarily; the 07-10 production runs showed the consequence (an invocation cap smaller than the roster filled its slots by enablement mechanism, not value, and a matched specialist lens could be shed to afford `conventions`). The shed order is now explicit, lowest value first: conventions, first-principles, holistic, completeness/test-adequacy, and path-triggered specialist lenses last, since a matched lens is the most targeted signal in the run. The same ranking read from the other end (defaults, lenses, targeted opt-ins, generic opt-ins) is now the mandated dispatch order when `maxReviewerInvocations` cannot fit the full roster, with every undispatched reviewer recorded as a planned shed. And reviewer requests (Step 8) are removed from the shed list entirely: a request is one tool call, and pulling a human in matters most on exactly the run whose own coverage is partial. The ranking is a first-cut editorial ordering; replace it with measured per-dimension must-catch contribution from the eval corpus when that data is compiled. +Value-ranked shedding and dispatch. The graceful-landing rule's first shed bucket previously said "skip any not-yet-dispatched opt-in reviewers and lenses" with no ordering, so a budget-pressed run shed arbitrarily; the 2026-07-10 production runs showed the consequence (an invocation cap smaller than the roster filled its slots by enablement mechanism, not value, and a matched specialist lens could be shed to afford `conventions`). The shed order is now explicit, lowest value first: conventions, first-principles, holistic, completeness/test-adequacy, and path-triggered specialist lenses last, since a matched lens is the most targeted signal in the run. The same ranking read from the other end (defaults, lenses, targeted opt-ins, generic opt-ins) is now the mandated dispatch order when `maxReviewerInvocations` cannot fit the full roster, with every undispatched reviewer recorded as a planned shed. And reviewer requests (Step 8) are removed from the shed list entirely: pulling a human in matters most on exactly the run whose own coverage is partial. The ranking is a first-cut editorial ordering; replace it with measured per-dimension must-catch contribution from the eval corpus when that data is compiled. diff --git a/workflows/review/review.md b/workflows/review/review.md index 643a1134..96124c34 100644 --- a/workflows/review/review.md +++ b/workflows/review/review.md @@ -528,9 +528,9 @@ not by shipping). Dispatch the default reviewers (`correctness-reviewer`, `skill-auditor`, `thread-reconciler`) **plus** every reviewer named in `enabledReviewers` **plus** every lens named in `lensesToSpawn`, all **in parallel** (one turn), and wait for all. If `runBudget.maxReviewerInvocations` cannot fit -that whole set, fill the slots by the dispatch ranking (the budget rule below, -Step 3 Phase 3): defaults first, then matched lenses, then the targeted opt-in -dimensions, then the generic ones. Never choose arbitrarily, and record every +that whole set, fill the slots by the dispatch ranking (the budget rule below: +Step 3, graceful-landing bucket 1): defaults first, then matched lenses, then +the targeted opt-in dimensions, then the generic ones. Never choose arbitrarily, and record every reviewer left undispatched as a planned shed (Step 6 note). **One candidate contract.** Every finding-producing reviewer returns `findings[]` @@ -755,10 +755,12 @@ order: afford `conventions` is exactly backwards. This same ranking, read from the other end (defaults, lenses, targeted opt-ins, generic opt-ins), is the dispatch order when the invocation cap cannot fit the roster (Phase 2). + The interior order is a first-cut editorial ranking; replace it with + measured per-dimension must-catch contribution once the eval corpus + yields that data. 2. Skip the risks/patterns comment (Step 7) if it has not happened yet. - Reviewer requests (Step 8) are **never** shed: a request is a single tool - call, and pulling a human in matters most on exactly the run whose own - coverage is partial. + Reviewer requests (Step 8) are **never** shed: pulling a human in matters + most on exactly the run whose own coverage is partial. 3. Last, and never at the soft targets alone: the `claim-validator`. It is the false-positive gate, and its cost scales with the candidate count (which you can already see when deciding), not with the diff, so validating a small