review: value-ranked shedding and dispatch; never shed reviewer requests

.changeset/review-shed-ranking.md

@@ -0,0 +1,5 @@
+---
+"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 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.

.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>-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.)

.github/aw/review/ROUTING

@@ -29,14 +29,22 @@ 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
 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

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<RiskTier, RunBudget> = {
     trivial: {
@@ -262,25 +283,25 @@ export const DEFAULT_TIER_BUDGETS: Record<RiskTier, RunBudget> = {
         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: {

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, 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[]`
 in the same shape (a `label` per finding, from the fixed label set in Step 4); a
@@ -727,8 +731,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
@@ -738,12 +744,33 @@ 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.
-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.
+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).
+   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: 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
+   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 +1044,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: <dimension> not assessed this run (<sub-agent> 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: <dimension> not assessed this run (shed under the <tier>-tier run budget).`
+- The sub-agent was dispatched but its output was missing or unparseable:
+  `Note: <dimension> not assessed this run (<sub-agent> 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).`