Skip to content

review: budget and shed tuning from the 07-10 production sheds #258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jwbron merged 4 commits into from
Jul 13, 2026
Merged

review: budget and shed tuning from the 07-10 production sheds #258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0afc12e
[jwies/review-shed-tuning] review: budget and shed tuning from the 07…
jwbron Jul 10, 2026
651912b
[jwies/review-shed-tuning] review: lock the invocation caps to the ro…
jwbron Jul 13, 2026
61ff935
[jwies/review-shed-tuning] review: the low cap fits the lens-free ros…
jwbron Jul 13, 2026
5889d75
[jwies/review-shed-tuning] Merge remote-tracking branch 'origin/main'…
jwbron Jul 13, 2026
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
View file
Open in desktop
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
View file
Open in desktop
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

@github-actions github-actions Bot Jul 10, 2026

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
16 changes: 16 additions & 0 deletions workflows/review/lib/router.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
computeRunBudget,
DEFAULT_MISROUTED_FLOOR_TIER,
DEFAULT_TIER_BUDGETS,
ENABLEABLE_REVIEWERS,
isGenerated,
matchesGlob,
parseGitattributesGenerated,
Expand Down Expand Up @@ -384,6 +385,21 @@ describe("computeRunBudget: scaling", () => {
expect(result.runBudget.tier).toBe("high");
expect(result.runBudget.floored).toBe(false);
});

it("fits the whole-change roster within the low and medium caps", () => {
// Two always-on defaults (correctness-reviewer, skill-auditor; the
// other pipeline steps never consume a slot) plus every opt-in
// reviewer. Pinned to the roster size, not a literal: if a reviewer
// joins ENABLEABLE_REVIEWERS without a cap bump, low/medium runs
// would silently shed dimensions again while the suite stayed green.
const rosterSize = 2 + ENABLEABLE_REVIEWERS.length;
expect(
DEFAULT_TIER_BUDGETS.low.maxReviewerInvocations,
).toBeGreaterThanOrEqual(rosterSize);
expect(
DEFAULT_TIER_BUDGETS.medium.maxReviewerInvocations,
).toBeGreaterThanOrEqual(rosterSize);
});
});

/* -------------------------------------------------------------------------- */
Expand Down
42 changes: 35 additions & 7 deletions workflows/review/lib/router.ts
View file
Open in desktop
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,24 @@ 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). Raising the low cap to
* 8 is deliberate: a lens-free low-tier run can dispatch the full roster.
* Path-matched specialist lenses also consume slots, so a low PR matching
* two or more lenses still sheds from the bottom of the value ranking; that
* residual is sized when the modes are priced, not here. `maxUsd` is
* deliberately left uncalibrated
* (per-run cost measurement is deferred to #249), so the dollar column is
* still the original estimate, not a re-measured figure.
*/
export const DEFAULT_TIER_BUDGETS: Record<RiskTier, RunBudget> = {
trivial: {
Expand All @@ -262,25 +290,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
35 changes: 27 additions & 8 deletions workflows/review/review.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,8 +750,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 @@ -765,8 +767,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 @@ -1060,11 +1070,20 @@ append nothing. Never rephrase, reorder, or summarize it; if `rereview.json` is
missing or unparseable, submit the body without the section (do not hand-compose a
replacement).

**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
**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 and the re-review accountability section above — one line
per skipped dimension, exactly:
`Note: <dimension> not assessed this run (<sub-agent> output unavailable).` If the
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