joshtronic/igor

Watch

RFC: escalating review ladder (Sonnet -> Opus -> Fable), escalate-on-non-approval #308

New issue

Closed

opened 2026-07-01 15:57:43 +00:00 by igor · 4 comments

igor commented

2026-07-01 15:57:43 +00:00

Collaborator

Goal

Replace the single-tier code review (one claude_call on AGENT_MODEL_REVIEW) with an escalating ladder: the cheapest capable model reviews first, and a PR only climbs to a stronger (costlier) model when the current tier cannot get to APPROVE. A human is the final rung, reached only after every model tier is exhausted. Net effect: most PRs clear on Sonnet (cheaper + faster than always-Opus today); only the genuinely hard ones burn Opus/Fable. Better ceiling, lower average cost.

Today (for grounding)

do_review_tick runs ONE claude_call on AGENT_MODEL_REVIEW (currently Opus). Verdicts: APPROVE/COMMENT -> request the human; REQUEST_CHANGES -> assign the bot, drive the rework loop, capped at 3 rounds (rework_rounds) then escalate to the human. State lives per-PR under .review[<repo#num>] = {sha, verdict, ci, patch_id, rework_rounds, rework_crashes, pending_rc_body, ...}. Note AGENT_MODEL_REVIEW is ALSO read by two one-shot triage users: logwatch and maintenance-classify.

Proposed

Ladder config. AGENT_MODEL_REVIEW becomes a comma-ordered, cheapest-first ladder, e.g. claude-sonnet-5,claude-opus-4-8,claude-fable-5. do_review_tick splits it into tiers. A single value = todays behavior (degrades gracefully). The one-shot triage users (logwatch, maintenance) take the first rung (${AGENT_MODEL_REVIEW%%,*}) — they dont rework/escalate, and cheapest-tier triage is the right call (also the Sonnet-for-triage you wanted).
Per-PR tier in state. Add .review[key].tier (int, default 0). Review + rework calls use ladder[tier].
Escalate on failure-to-approve:
- APPROVE (any tier) -> terminal-good: request the human (who still approves + merges via the existing auto-merge gate). Done.
- REQUEST_CHANGES -> rework loop at the current tier as today; when rework_rounds hits the cap without APPROVE, bump tier, reset rework_rounds, re-review with the next model — instead of jumping straight to the human.
- COMMENT (cant confidently approve, nothing blocking) -> bump tier and re-review the same head with the next model.
- Top tier still cant approve -> escalate to the human, tagged "all model tiers exhausted." (Exactly todays escalate-to-human, now at the end of the ladder.)
- rework_crashes cap (igor#291) routes to tier-escalation too.
Unchanged: the security gate (its own AGENT_MODEL_SECURITY = Opus, NOT part of this ladder); igor never auto-merges itself (ladder is advisory there, human gates); per-SHA/patch-id dedup; CI-must-settle-before-review.

Why this shape

Cost/latency drops in the common case — most PRs approve at Sonnet, so we stop paying Opus on easy PRs while keeping Opus/Fable in reserve.
Escalate-on-non-approval matches the rule exactly — a Sonnet APPROVE is sufficient and terminal; only "cant approve" climbs.
Human is genuinely last — exhaust the model bench before spending your attention. "Automate before the human," applied to review.

Open questions (your calls)

COMMENT handling: escalate the tier (my rec — exhaust models first), or keep todays "-> human" on a COMMENT?
Env shape: AGENT_MODEL_REVIEW-as-CSV-ladder (my rec — one var; one-shot users take rung 0) vs. a separate AGENT_MODEL_REVIEW_LADDER var vs. explicit per-tier vars (_1/_2/_3). This is the ".env rethink" you flagged.
Rework rounds per tier: keep 3 at every tier ("3 sonnet AND 3 opus"), or fewer at the cheap tiers to escalate faster?
Fables role: does the top tier also get its own rework loop, or is Fable a single final verdict before the human?
One-shot triage (logwatch/maintenance): ride the cheapest rung (Sonnet, cheaper) as a side effect, or keep them explicitly on their own model?

Not in scope

Curing the exit-1 (#306); the security gate; the human-merge gate. Purely the code-review model-routing + escalation.

## Goal Replace the single-tier code review (one `claude_call` on `AGENT_MODEL_REVIEW`) with an **escalating ladder**: the cheapest capable model reviews first, and a PR only climbs to a stronger (costlier) model when the current tier **cannot get to APPROVE**. A human is the final rung, reached only after every model tier is exhausted. Net effect: most PRs clear on Sonnet (cheaper + faster than always-Opus today); only the genuinely hard ones burn Opus/Fable. Better ceiling, lower average cost. ## Today (for grounding) `do_review_tick` runs ONE `claude_call` on `AGENT_MODEL_REVIEW` (currently Opus). Verdicts: APPROVE/COMMENT -> request the human; REQUEST_CHANGES -> assign the bot, drive the rework loop, capped at 3 rounds (`rework_rounds`) then escalate to the human. State lives per-PR under `.review[<repo#num>]` = `{sha, verdict, ci, patch_id, rework_rounds, rework_crashes, pending_rc_body, ...}`. Note `AGENT_MODEL_REVIEW` is ALSO read by two one-shot triage users: logwatch and maintenance-classify. ## Proposed 1. **Ladder config.** `AGENT_MODEL_REVIEW` becomes a comma-ordered, cheapest-first ladder, e.g. `claude-sonnet-5,claude-opus-4-8,claude-fable-5`. `do_review_tick` splits it into tiers. A single value = todays behavior (degrades gracefully). The one-shot triage users (logwatch, maintenance) take the **first rung** (`${AGENT_MODEL_REVIEW%%,*}`) — they dont rework/escalate, and cheapest-tier triage is the right call (also the Sonnet-for-triage you wanted). 2. **Per-PR tier in state.** Add `.review[key].tier` (int, default 0). Review + rework calls use `ladder[tier]`. 3. **Escalate on failure-to-approve:** - **APPROVE** (any tier) -> terminal-good: request the human (who still approves + merges via the existing auto-merge gate). Done. - **REQUEST_CHANGES** -> rework loop at the current tier as today; when `rework_rounds` hits the cap without APPROVE, **bump `tier`, reset `rework_rounds`, re-review with the next model** — instead of jumping straight to the human. - **COMMENT** (cant confidently approve, nothing blocking) -> **bump `tier`** and re-review the same head with the next model. - **Top tier still cant approve** -> escalate to the human, tagged "all model tiers exhausted." (Exactly todays escalate-to-human, now at the end of the ladder.) - `rework_crashes` cap (igor#291) routes to tier-escalation too. 4. **Unchanged:** the security gate (its own `AGENT_MODEL_SECURITY` = Opus, NOT part of this ladder); igor never auto-merges itself (ladder is advisory there, human gates); per-SHA/patch-id dedup; CI-must-settle-before-review. ## Why this shape - **Cost/latency drops in the common case** — most PRs approve at Sonnet, so we stop paying Opus on easy PRs while keeping Opus/Fable in reserve. - **Escalate-on-non-approval matches the rule exactly** — a Sonnet APPROVE is sufficient and terminal; only "cant approve" climbs. - **Human is genuinely last** — exhaust the model bench before spending your attention. "Automate before the human," applied to review. ## Open questions (your calls) 1. **COMMENT handling:** escalate the tier (my rec — exhaust models first), or keep todays "-> human" on a COMMENT? 2. **Env shape:** `AGENT_MODEL_REVIEW`-as-CSV-ladder (my rec — one var; one-shot users take rung 0) vs. a separate `AGENT_MODEL_REVIEW_LADDER` var vs. explicit per-tier vars (`_1/_2/_3`). This is the ".env rethink" you flagged. 3. **Rework rounds per tier:** keep 3 at every tier ("3 sonnet AND 3 opus"), or fewer at the cheap tiers to escalate faster? 4. **Fables role:** does the top tier also get its own rework loop, or is Fable a single final verdict before the human? 5. **One-shot triage (logwatch/maintenance):** ride the cheapest rung (Sonnet, cheaper) as a side effect, or keep them explicitly on their own model? ## Not in scope Curing the exit-1 (#306); the security gate; the human-merge gate. Purely the code-review model-routing + escalation.

joshtronic was assigned by igor

2026-07-01 15:57:44 +00:00

igor commented

2026-07-01 17:12:38 +00:00

Author

Collaborator

Decisions from review (2026-07-01) + a new dimension

Locked:

Q1 — COMMENT escalates the tier (exhaust models before the human). Rationale: "you dont ping the CTO on every little thing; you escalate when theres a problem and you need their expert opinion."
Q2 — single CSV var AGENT_MODEL_REVIEW=sonnet,opus[,fable]. Flexible on which models / how many rungs. Fable is not available yet — stage it with the dual-line convention (active line + commented desired-end-state), Fable commented until its live.
Q5 — one-shot triage (logwatch, maintenance) rides rung 0 (Sonnet). Theyre classification/analysis, not judgment — "when you know the task, you dont need Opus+." Bonus: lowers higher-model usage. (Security gate stays Opus — adversarial judgment, not analysis.)

Riffed / proposed:

Q3 — rounds per tier: NOT 3/3/3 (=9; at 9 reviews theres a bigger problem). Proposal: a decreasing schedule, ~3 / 2 / 1, hardcoded (internal tuning constant, not an env knob). Logic: a more capable model needs fewer attempts to either fix it or conclude its unfixable; front-load the cheap tries, make the expensive tiers decisive; 6 total is a fair "we really tried" bar before the human.
Q4 — the LAST tier is a single "final boss" pass, then the human. Matches "1 pass is sufficient" + "Fable calls are costly." Generalizes cleanly: whatever the top rung is (Fable, or Opus if Fables omitted), it gets one decisive pass.

New dimension — effort ladder (great idea)

Today the harness sets no reasoning-effort at all. Proposal, orthogonal to the model ladder:

Worker effort scales UP each rework pass (default → high → xhigh): try harder as the problem proves hard.
Reviewer effort stays FLAT — so it doesnt inflate feedback / move the goalposts, which would prevent convergence. (Your instinct here is right and load-bearing.)

So two independent escalation axes:

axis	who	climbs when
model	reviewer	on failure-to-approve (sonnet→opus→fable), effort flat
effort	worker	each rework pass (med→high→xhigh), model = AGENT_MODEL

Caveat to confirm before building: the /effort youre picturing is the interactive command — I need to verify the headless claude --print invocation exposes a per-call effort flag. If it doesnt, the effort axis needs a different mechanism (or waits).

## Decisions from review (2026-07-01) + a new dimension **Locked:** - **Q1 — COMMENT escalates the tier** (exhaust models before the human). Rationale: "you dont ping the CTO on every little thing; you escalate when theres a problem and you need their expert opinion." - **Q2 — single CSV var** `AGENT_MODEL_REVIEW=sonnet,opus[,fable]`. Flexible on which models / how many rungs. **Fable is not available yet** — stage it with the dual-line convention (active line + commented desired-end-state), Fable commented until its live. - **Q5 — one-shot triage (logwatch, maintenance) rides rung 0 (Sonnet).** Theyre *classification/analysis*, not judgment — "when you know the task, you dont need Opus+." Bonus: lowers higher-model usage. (Security gate stays Opus — adversarial judgment, not analysis.) **Riffed / proposed:** - **Q3 — rounds per tier: NOT 3/3/3 (=9; at 9 reviews theres a bigger problem).** Proposal: a **decreasing** schedule, ~**3 / 2 / 1**, hardcoded (internal tuning constant, not an env knob). Logic: a more capable model needs fewer attempts to either fix it or conclude its unfixable; front-load the cheap tries, make the expensive tiers decisive; 6 total is a fair "we really tried" bar before the human. - **Q4 — the LAST tier is a single "final boss" pass**, then the human. Matches "1 pass is sufficient" + "Fable calls are costly." Generalizes cleanly: whatever the top rung is (Fable, or Opus if Fables omitted), it gets one decisive pass. ## New dimension — effort ladder (great idea) Today the harness sets **no reasoning-effort at all**. Proposal, orthogonal to the model ladder: - **Worker effort scales UP each rework pass** (default → high → xhigh): try harder as the problem proves hard. - **Reviewer effort stays FLAT** — so it doesnt inflate feedback / move the goalposts, which would prevent convergence. (Your instinct here is right and load-bearing.) So two independent escalation axes: | axis | who | climbs when | |---|---|---| | **model** | reviewer | on failure-to-approve (sonnet→opus→fable), effort flat | | **effort** | worker | each rework pass (med→high→xhigh), model = AGENT_MODEL | **Caveat to confirm before building:** the `/effort` youre picturing is the *interactive* command — I need to verify the headless `claude --print` invocation exposes a per-call effort flag. If it doesnt, the effort axis needs a different mechanism (or waits).

igor commented

2026-07-01 17:31:25 +00:00

Author

Collaborator

`--effort` confirmed + near-term shape (2026-07-01)

claude --effort <level> is a real CLI arg (verified via claude --help, alongside --model). So the effort axis is buildable, and better: a ladder rung can be a model:effort pair, which solves the "only 2 models while Fable is out" problem elegantly.

Near-term shape (Josh): dont run a dynamic variable-length model ladder yet. Instead:

Ladder = opus only, escalated by effort: opus (default) -> opus:high -> opus:xhigh, last rung a single "final boss" pass, then the human.
This keeps ONE model in .env (no dynamic 3-model logic to reason about), while still giving real escalation — opus:default vs opus:high is a meaningful capability step.
When Fable ships, it slots in as the 1-pass final boss; the rung format already supports it (fable or fable:high).

So the rung parser should accept model[:effort] and map to --model <m> [--effort <e>]. Effort default when unspecified = whatever the harness default is (medium). The worker effort-scaling axis (med->high->xhigh per rework pass) is unchanged and independent.

Rounds stay the hardcoded decreasing constant (3/2/1, last rung = 1). Igor has an opinion here; not a knob.

## `--effort` confirmed + near-term shape (2026-07-01) `claude --effort <level>` **is** a real CLI arg (verified via `claude --help`, alongside `--model`). So the effort axis is buildable, and better: **a ladder rung can be a `model:effort` pair**, which solves the "only 2 models while Fable is out" problem elegantly. **Near-term shape** (Josh): dont run a dynamic variable-length model ladder yet. Instead: - Ladder = **opus only**, escalated by **effort**: `opus` (default) -> `opus:high` -> `opus:xhigh`, last rung a single "final boss" pass, then the human. - This keeps ONE model in `.env` (no dynamic 3-model logic to reason about), while still giving real escalation — `opus:default` vs `opus:high` is a meaningful capability step. - When Fable ships, it slots in as the 1-pass final boss; the rung format already supports it (`fable` or `fable:high`). So the rung parser should accept `model[:effort]` and map to `--model <m> [--effort <e>]`. Effort default when unspecified = whatever the harness default is (medium). The **worker** effort-scaling axis (med->high->xhigh per rework pass) is unchanged and independent. Rounds stay the hardcoded decreasing constant (3/2/1, last rung = 1). Igor has an opinion here; not a knob.

igor commented

2026-07-01 18:06:25 +00:00

Author

Collaborator

Ladder simplified (2026-07-01) — Opus + effort only, no Fable

Per Josh: drop Fable (too fragile — weird guardrails, even for interactive Igor). CoS-Igor stays Opus. The ladder collapses to ONE model + effort:

No Sonnet rung for review — code review IS judgment, so start at Opus. One-shot triage (logwatch/maintenance) stays Sonnet (analysis, not judgment).
During the rework loop: reviewer effort FLAT (stable bar, no goalpost drift); the worker effort climbs each pass (high -> xhigh).
Final boss: a single opus:max terminal review before escalating to the human. MAX is justified because it's the tail — only PRs that resisted the loop reach it, and the alternative is Josh's time (the most expensive resource). Safe to bump here because it's terminal (no rework after it -> no goalpost drift). The ONE deliberate reviewer-effort bump.
Rounds: worker gets ~3 climbing-effort attempts against flat Opus review; unresolved -> opus:max final boss -> human.

Net: .env stays single-model Opus; escalation is pure effort. Simpler than the CSV model-ladder, nothing dynamic to reason about. The rung parser (model[:effort]) still generalizes if we ever re-add a model tier.

## Ladder simplified (2026-07-01) — Opus + effort only, no Fable Per Josh: **drop Fable** (too fragile — weird guardrails, even for interactive Igor). CoS-Igor stays Opus. The ladder collapses to ONE model + effort: - **No Sonnet rung for review** — code review IS judgment, so start at Opus. One-shot triage (logwatch/maintenance) stays **Sonnet** (analysis, not judgment). - **During the rework loop:** reviewer effort **FLAT** (stable bar, no goalpost drift); the **worker** effort climbs each pass (high -> xhigh). - **Final boss:** a single **`opus:max`** terminal review before escalating to the human. MAX is justified *because* it's the tail — only PRs that resisted the loop reach it, and the alternative is Josh's time (the most expensive resource). Safe to bump here because it's terminal (no rework after it -> no goalpost drift). The ONE deliberate reviewer-effort bump. - **Rounds:** worker gets ~3 climbing-effort attempts against flat Opus review; unresolved -> `opus:max` final boss -> human. Net: `.env` stays single-model Opus; escalation is pure effort. Simpler than the CSV model-ladder, nothing dynamic to reason about. The rung parser (`model[:effort]`) still generalizes if we ever re-add a model tier.

igor referenced this issue

2026-07-01 18:19:32 +00:00

Per-ticket audit trail: record model + effort + outcome ON the issue/PR #310

igor commented

2026-07-01 18:20:00 +00:00

Author

Collaborator

Flow clarification + worker-effort (2026-07-01)

Every path ends at Josh — he's the merge gate for everything. The difference is why:

APPROVE → straight to Josh to MERGE (happy path, cheapest). No further escalation — approve is terminal-good.
Can't approve → worker reworks at climbing effort; rounds exhaust → opus:max final boss → if still no approve, to Josh to ADJUDICATE ("I couldn't get this to pass, your call").

So "approve → you, don't keep escalating" is exactly right. Escalation (effort climb + final boss) only fires on non-approval.

Worker effort on tasks (Josh's Q)

v1: the worker (Sonnet) effort climbs on rework — default → high → xhigh — but the worker MODEL stays Sonnet. Two independent axes, both gated on non-approval:

reviewer: Opus, effort flat during the loop, opus:max terminal.
worker: Sonnet, effort climbs per rework pass.

Deliberately NOT building a worker→Opus model bump yet. If Sonnet-at-max-effort keeps failing review, THAT's a real signal to add one — but we should see it in the data first, not assume it. Which is exactly what the telemetry ticket (#310) is for: model×effort×outcome per surface tells us empirically whether a worker Opus rung earns its cost. Tune from data, don't speculate. ("Don't be the KDE of agents.")

## Flow clarification + worker-effort (2026-07-01) **Every path ends at Josh — he's the merge gate for everything.** The difference is *why*: - **APPROVE** → straight to Josh **to MERGE** (happy path, cheapest). No further escalation — approve is terminal-good. - **Can't approve** → worker reworks at climbing effort; rounds exhaust → `opus:max` final boss → if still no approve, to Josh **to ADJUDICATE** ("I couldn't get this to pass, your call"). So "approve → you, don't keep escalating" is exactly right. Escalation (effort climb + final boss) only fires on non-approval. ## Worker effort on tasks (Josh's Q) **v1: the worker (Sonnet) effort climbs on rework — default → high → xhigh — but the worker MODEL stays Sonnet.** Two independent axes, both gated on non-approval: - reviewer: Opus, effort flat during the loop, `opus:max` terminal. - worker: Sonnet, effort climbs per rework pass. **Deliberately NOT building** a worker→Opus model bump yet. If Sonnet-at-max-effort keeps failing review, THAT's a real signal to add one — but we should see it in the data first, not assume it. Which is exactly what the telemetry ticket (#310) is for: model×effort×outcome per surface tells us empirically whether a worker Opus rung earns its cost. Tune from data, don't speculate. ("Don't be the KDE of agents.")

igor referenced this issue from a pull request that will close it,

2026-07-01 20:13:59 +00:00

feat(review): escalating effort ladder -- reviewer flat->max, worker climbs (Closes #308) #312

igor referenced this issue from a commit

2026-07-01 20:13:59 +00:00

feat(review): escalating effort ladder -- reviewer flat->max, worker climbs (igor#308)

joshtronic closed this issue

2026-07-01 20:47:03 +00:00

joshtronic referenced this issue from a commit

2026-07-01 20:47:04 +00:00

Merge pull request 'feat(review): escalating effort ladder -- reviewer flat->max, worker climbs (Closes #308)' (#312) from agent/308-review-effort-ladder into master

igor referenced this issue from a commit