-
Notifications
You must be signed in to change notification settings - Fork 1
review: move the whole reviewer roster to Opus 5 #294
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.
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
base: jwies/aic-anthropic-discount-overlay
Are you sure you want to change the base?
Changes from all commits
183401b
15965a6
3c454c8
8245704
642ad5c
d3cafce
2cfb0fb
f94ea6b
c451b34
69ff09a
470b456
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| "review": minor | ||
| --- | ||
|
|
||
| Move the reviewer roster to Opus 5 (`claude-opus-5`). `pattern-triage` stays on Sonnet 4.6 as the cheap first pass. | ||
|
|
||
| The specialist lenses were kept off Fable 5 because cyber safety classifiers can refuse benign security-focused analysis, and a refused lens is a silent coverage hole: it surfaces as a missing agent result, not an error. Opus 5 can also return `stop_reason: "refusal"` on cyber-adjacent input, so this move re-opens that hole rather than closing it. The detector is the weekly drift corpus, where a refusing lens craters must-catch recall on security-adjacent cases while every other metric looks normal. On that signature, put the `security-auth` lens back on `claude-opus-4-8` first. Moving `security-auth` with the roster rather than carving it out pre-emptively is deliberate: the refusal hazard is intermittent and unproven on Opus 5, the one-hop runtime fallback (a refused agent re-dispatches once on `claude-opus-4-8`, recorded as `fellBackTo`) bounds each occurrence, and a uniform roster leaves one signature to watch and one revert to make instead of a permanent carve-out justified by a hazard that may not materialize. | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -516,7 +516,9 @@ Fable's cyber classifiers can refuse benign security analysis, while | |
| `correctness-reviewer` — the default roster's load-bearing recall agent — was | ||
| moved *onto* Fable 5 for its recall gain. Eval run 30656579898 caught it | ||
| refusing `incident-auth-bypass` and `adversarial-injection-approve` outright, | ||
| at 5,207 tokens (so not a context limit). | ||
| at 5,207 tokens (so not a context limit). The roster has since moved to Opus 5, | ||
| which carries its own elevated cyber safeguards, so the hazard moved with it | ||
| rather than being resolved by the pin change. | ||
|
|
||
| Refusals are **intermittent**: probe run 30658862532 saw the same Fable pin | ||
| clear both cases that run 30656579898 blocked. The ordinary retry still cannot | ||
|
|
@@ -527,7 +529,7 @@ refusing pin to a model with a different refusal profile: | |
| | Pinned model | Falls back to | Basis | | ||
| | --- | --- | --- | | ||
| | `claude-fable-5` | `claude-opus-4-8` | measured (run 30656579898) | | ||
| | `claude-opus-5` | `claude-opus-4-8` | pre-emptive; #294 notes Opus 5 also ships elevated cyber safeguards | | ||
| | `claude-opus-5` | `claude-opus-4-8` | pre-emptive; Opus 5 ships elevated cyber safeguards and can also return `stop_reason: "refusal"` | | ||
|
|
||
| Rules: **one hop**, never back to a model that already refused, and **no | ||
| fallback for an unlisted pin** — an unmapped model's refusal stands and is | ||
|
|
@@ -552,19 +554,19 @@ sub-agent models — this table is the human-facing summary: | |
|
|
||
| | Role | Model | Effort | Why | | ||
| | --- | --- | --- | --- | | ||
| | orchestrator | `claude-opus-4-8` | high | Owns every GitHub/safe-output decision | | ||
| | orchestrator | `claude-opus-5` | high | Owns every GitHub/safe-output decision | | ||
| | `pattern-triage` | `claude-sonnet-4-6` | medium | Cheap first-pass triage | | ||
| | `thread-reconciler` | `claude-opus-4-8` | medium | Reconciliation | | ||
| | `correctness-reviewer` | `claude-fable-5` | high | Whole-change reviewer; bug-finding recall is the load-bearing metric | | ||
| | `skill-auditor` | `claude-opus-4-8` | high | Whole-change reviewer | | ||
| | `holistic` | `claude-opus-4-8` | high | Opt-in whole-change reviewer (`enable` in `ROUTING`) | | ||
| | `completeness` | `claude-opus-4-8` | high | Opt-in whole-change reviewer (`enable` in `ROUTING`) | | ||
| | `test-adequacy` | `claude-opus-4-8` | high | Opt-in whole-change reviewer (`enable` in `ROUTING`) | | ||
| | `conventions` | `claude-opus-4-8` | medium | Opt-in advisory targeted check (`enable` in `ROUTING`) | | ||
| | `documentation` | `claude-opus-4-8` | medium | Opt-in advisory targeted check (`enable` in `ROUTING`) | | ||
| | `first-principles` | `claude-fable-5` | high | Opt-in advisory-only; reviews the change's justification | | ||
| | `claim-validator` | `claude-opus-4-8` | xhigh | Adversarial claim validation; stays Opus (the Fable arm did not improve precision) | | ||
| | specialist lenses | `claude-opus-4-8` | high | Opt-in via `lens=` in `ROUTING`; the security & auth lens is xhigh | | ||
| | `thread-reconciler` | `claude-opus-5` | medium | Reconciliation | | ||
| | `correctness-reviewer` | `claude-opus-5` | high | Whole-change reviewer; bug-finding recall is the load-bearing metric | | ||
| | `skill-auditor` | `claude-opus-5` | high | Whole-change reviewer | | ||
| | `holistic` | `claude-opus-5` | high | Opt-in whole-change reviewer (`enable` in `ROUTING`) | | ||
| | `completeness` | `claude-opus-5` | high | Opt-in whole-change reviewer (`enable` in `ROUTING`) | | ||
| | `test-adequacy` | `claude-opus-5` | high | Opt-in whole-change reviewer (`enable` in `ROUTING`) | | ||
| | `conventions` | `claude-opus-5` | medium | Opt-in advisory targeted check (`enable` in `ROUTING`) | | ||
| | `documentation` | `claude-opus-5` | medium | Opt-in advisory targeted check (`enable` in `ROUTING`) | | ||
| | `first-principles` | `claude-opus-5` | high | Opt-in advisory-only; reviews the change's justification | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question (non-blocking): This PR rewrites the models/effort table (README lines 557–569) to put the entire roster on |
||
| | `claim-validator` | `claude-opus-5` | xhigh | Adversarial claim validation; stays Opus (the Fable arm did not improve precision) | | ||
| | specialist lenses | `claude-opus-5` | high | Opt-in via `lens=` in `ROUTING`; the security & auth lens is xhigh | | ||
|
|
||
| Only the orchestrator and the default roster (`pattern-triage`, | ||
| `correctness-reviewer`, `skill-auditor`, `thread-reconciler`, `claim-validator`) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| /** | ||
| * Mechanical pricing gates for the model pins in review.md (#294 review | ||
| * feedback: the merge-ordering constraint "do not ship an un-priced pin" | ||
| * rested entirely on human memory of a draft PR). | ||
| * | ||
| * Two hazards, both prose-only until this file: | ||
| * | ||
| * - The `models.providers` overlay matches per model and an unlisted model | ||
| * silently bills at FULL list price (the overlay's own MAINTENANCE note); | ||
| * the overlay omitting `claude-sonnet-4-6` shipped exactly that way in an | ||
| * earlier draft of #314. | ||
| * - On the stable toolchain (gh-aw v0.83.x -> firewall v0.27.42) the | ||
| * `providers` block is dropped silently and the api-proxy's credit guard | ||
| * rejects a model its curated table does not price with a 400 before the | ||
| * request reaches the model (#266). `claude-opus-5` is not in that table, | ||
| * so the `default-ai-credits-pricing` fallback is load-bearing for every | ||
| * dispatch until a gh-aw release defaults the firewall to v0.27.43+. | ||
| * | ||
| * DELETE the fallback test (only it) together with the fallback block when | ||
| * the toolchain moves; the coverage test is permanent. | ||
| */ | ||
| import {readFileSync} from "node:fs"; | ||
| import {join} from "node:path"; | ||
|
|
||
| import {describe, it, expect} from "vitest"; | ||
|
|
||
| const reviewMd = readFileSync(join(__dirname, "..", "review.md"), "utf8"); | ||
|
|
||
| /** The workflow frontmatter (between the first pair of --- fences). */ | ||
| const frontmatter = reviewMd.split(/^---$/m)[1] ?? ""; | ||
|
|
||
| /** | ||
| * Every model pin in the file: the engine's indented `model:` line in the | ||
| * frontmatter plus each sub-agent's `model:` line in its block frontmatter. | ||
| */ | ||
| const pins = [ | ||
| ...new Set( | ||
| [...reviewMd.matchAll(/^\s*model:\s*(claude-[a-z0-9.-]+)\s*$/gm)].map( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick (non-blocking): The pin-extraction regex requires the model id to end the line, so a pin with a trailing inline comment silently escapes the coverage gate. Checked all 23 current pin lines in review.md — none carry inline comments today, so this is a latent gap introduced with the new test, not a live failure. Tolerating an optional trailing comment (or asserting the pin count against the number of agent blocks) would keep the gate from resting on that formatting convention. A sketch, not a committable replacement: |
||
| (match) => match[1], | ||
| ), | ||
| ), | ||
| ]; | ||
|
|
||
| /** | ||
| * The models the `providers` overlay prices: bare `claude-*:` mapping keys in | ||
| * the frontmatter (only the providers block declares them). | ||
| */ | ||
| const priced = new Set( | ||
| [...frontmatter.matchAll(/^\s+(claude-[a-z0-9.-]+):\s*$/gm)].map( | ||
| (match) => match[1], | ||
| ), | ||
| ); | ||
|
|
||
| describe("model pricing coverage (review.md frontmatter)", () => { | ||
| it("finds the pins and the overlay (guards the extraction itself)", () => { | ||
| // 22 agents plus the engine; a collapse to zero means the regexes | ||
| // rotted, not that the roster emptied. | ||
| expect(pins.length).toBeGreaterThanOrEqual(2); | ||
| expect(priced.size).toBeGreaterThanOrEqual(2); | ||
| }); | ||
|
|
||
| it("prices every pinned model in the providers overlay", () => { | ||
| const unpriced = pins.filter((pin) => !priced.has(pin)); | ||
| // An unlisted model silently bills at full list price; add an entry | ||
| // at 50% of its Anthropic list rate (see the MAINTENANCE note). | ||
| expect(unpriced).toEqual([]); | ||
| }); | ||
|
|
||
| it("keeps the stable-toolchain credit-guard fallback while claude-opus-5 is pinned", () => { | ||
| // Firewall v0.27.42's curated table does not price claude-opus-5; | ||
| // without `default-ai-credits-pricing` every dispatch 400s on the | ||
| // stable toolchain. Delete this test with the fallback block once a | ||
| // gh-aw release defaults the firewall to v0.27.43+. | ||
| if (pins.includes("claude-opus-5")) { | ||
| expect(frontmatter).toContain("default-ai-credits-pricing:"); | ||
| } | ||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.