Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/review-shed-tuning.md
Original file line number Diff line number Diff line change
@@ -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.)
10 changes: 9 additions & 1 deletion .github/aw/review/ROUTING
Original file line number Diff line number Diff line change
Expand Up @@ -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

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.

nitpick (non-blocking): This pattern is wider than the tier= alignment column the rest of the file keeps, so tier=low lands one space off the grid. Accept it as the intentional outlier, or widen the column across all rules.

Low-confidence (1)
  • workflows/review/lib/router.ts:286 — Baking the measured ~3 min overhead into every tier's wall clock is fragile; measuring elapsed time from triage-end (a second timestamp) instead would keep the table overhead-invariant.


# 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
Expand Down
35 changes: 28 additions & 7 deletions workflows/review/lib/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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: {
Expand All @@ -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,
Comment thread
khan-actions-bot marked this conversation as resolved.
Comment thread
khan-actions-bot marked this conversation as resolved.
maxToolCallsPerFinding: 3,
maxTotalToolCalls: 20,
maxWallClockMinutes: 6,
maxWallClockMinutes: 10,
Comment thread
khan-actions-bot marked this conversation as resolved.
maxUsd: 1.5,
},
medium: {
tier: "medium",
floored: false,
maxReviewerInvocations: 8,
maxReviewerInvocations: 10,
maxToolCallsPerFinding: 5,
maxTotalToolCalls: 60,
maxWallClockMinutes: 12,
maxWallClockMinutes: 15,
maxUsd: 4,
},
high: {
Expand Down
36 changes: 28 additions & 8 deletions workflows/review/review.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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: <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).`
Expand Down
Loading