review: meter AI credits at Khan's real Anthropic rate, not list price - #314
review: meter AI credits at Khan's real Anthropic rate, not list price#314jwbron wants to merge 7 commits into
Conversation
🦋 Changeset detectedLatest commit: 879902b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
b253aa8 to
fc0a447
Compare
fc0a447 to
67de169
Compare
02da9e6 to
2766e4b
Compare
…an's real Anthropic rate, not list price Khan bills Anthropic list minus 50%, but the firewall api-proxy computes AI credits from a list-price catalog baked into its container image, so every dollar figure in the workflow reads 2x high: max-ai-credits cuts a run off at half the real spend it implies, the router's maxUsd soft targets are denominated in a currency twice as expensive as the real one, and the cost counters report list. Add gh-aw's `models.providers` operator overlay with per-token rates at 50% of list for every model the reviewer can run. The overlay compiles to the firewall's apiProxy.providers and is the highest-precedence pricing source, ahead of runtime discovery, the curated table, and the bundled models.dev catalog; it covers the threat-detection proxy as well as the agent's. Nothing else changes. Because a credit becomes $0.01 of real spend rather than $0.01 of list, the existing caps and maxUsd targets become correct as written, so budgets.ts, credit-cap.ts and the counters are untouched. Inert until the gh-aw toolchain reaches v0.84.1: apiProxy.providers landed in AWF v0.27.43, and gh-aw gates emitting it on that floor, so v0.83.4 (which compiled these locks, defaulting to AWF v0.27.42) drops the block silently. Not worked around with a forward sandbox.agent.version pin, which review.md explicitly forbids.
…erlay blocker to a stable-release gate gh-aw raises DefaultFirewallVersion to AWF v0.27.43 as of v0.84.0, but v0.83.5/v0.84.0/v0.84.1 are all flagged prerelease and releases/latest is still v0.83.4, so the overlay is blocked on that line going stable rather than on a version that already exists. Record why pinning the extension to a prerelease is not the workaround: nothing in CI runs gh aw compile, so a prerelease-compiled lock would be silently reverted by the next compile on stable, with no test failure. Add the post-bump verification step, since the failure mode is silent.
…de, trim the changeset The installed copy may differ from its pinned release only inside hunks carrying the KHAN/ACTIONS LOCAL OVERRIDE marker (review-pins.test.ts). The overlay hunk had none and passed only because diff -u context merged it into the adjacent marked max-ai-credits hunk, which would stop holding as soon as anything between them changed. Mark it, following the pattern the timeout-minutes override already uses, with the condition for dropping it. Changeset trimmed from 511 words to 203: the investigation notes belong in the PR, not the changelog.
The installed copy carried a second hand-maintained copy of the override table, marked as a local override that existed only to be deleted at the next installed-reviewer version bump. Since the overlay is inert until gh-aw v0.84.x is stable, that bought nothing now and cost a divergence to unwind later. Define the rates once, in the shared source. This repo's own reviewer picks them up the normal way, when a release bumps the installed pin. The installed copy and its lock are byte-identical to main again.
2766e4b to
90e373a
Compare
There was a problem hiding this comment.
Approved — no blocking issues found.
Re-reviewed the trimmed, config-only change (the models.providers overlay in workflows/review/review.md plus its changeset). The pricing math is correct — every rate is 50% of Anthropic list and consistent with the cache ratios (cache_read = 0.1x input, cache_write = 1.25x input) — and there is no duplicate top-level models: key in this file.
1 prior review thread still open (non-blocking)
- workflows/review/review.md:326 — the orphaned rationale sentence in the pricing-overlay comment block still applies (line 326 still runs "...broke run 30290472047." straight into "Khan bills Anthropic list minus 50%..."). Not resolving the thread since the code is unchanged there.
There was a problem hiding this comment.
Changes requested — see inline comments.
1 of 1 prior review thread is still unaddressed as of 83ee887:
1 non-blocking thread still open
- suggestion (non-blocking)
workflows/review/review.md:326: This sentence (Khan bills Anthropic list minus 50%, but the firewall api-proxy meters credits...) reads as orphaned he...
…6 in the overlay; detach the orphaned rationale sentence Review feedback on #314. The overlay covered the orchestrator model and four override candidates but not claude-sonnet-4-6, the pattern-triage pin, so that lens would have metered at full list (2x) once the overlay went live. One entry at 50% of its list rates fixes it; every in-use pin (opus-4-8 x20, fable-5 x2, sonnet-4-6 x1) is now covered. Also moves the 'Khan bills list minus 50%' sentence out of the sandbox-pinning paragraph into its own WHY THE OVERLAY paragraph.
|
Addressed the review feedback in 879902b:
Merge-gate status: gh-aw v0.85.4 is now the latest stable (>= the v0.84.x floor), so the remaining steps are the documented ones: upgrade the extension, recompile, and verify |
| # The pattern-triage sub-agent's pin (the cheap first pass; see its | ||
| # `model:` line below). Not an engine model, but dispatched inside | ||
| # the sandboxed agent step and metered by the same api-proxy. | ||
| claude-sonnet-4-6: |
There was a problem hiding this comment.
note (non-blocking): Sixth overlay model (claude-sonnet-4-6) is not in the description's five-row table. The description's What section says 'the five pinned models' and its table lists five, but the diff adds claude-sonnet-4-6 as a sixth entry. I confirmed it is genuinely dispatched (review.md:1502, the pattern-triage pin) and is covered by the changeset's broader 'every model the reviewer can run' wording, so this is a description/implementation mismatch rather than a defect — worth aligning the table with the six entries actually shipped.
| # to v0.27.43 as of v0.84.0, but v0.83.5/v0.84.0/v0.84.1 are all prereleases, | ||
| # so recompiling is blocked on that line going stable rather than on any edit | ||
| # here. Do NOT pin the extension to a prerelease to force it: nothing in CI | ||
| # runs `gh aw compile`, so the next compile on stable would silently drop |
There was a problem hiding this comment.
suggestion (non-blocking): Whole change hinges on a manual, un-CI-enforced source→lock recompile that can silently revert. The diff edits the distributable source (review.md) but deliberately leaves review.lock.yml — the artifact that actually runs — out of sync, and the comment itself notes nothing in CI recompiles, so activation and its persistence rely entirely on humans remembering to recompile and verify. A CI guard that fails when review.md and its compiled lock diverge (or asserts providers appears in both awf-config payloads once the AWF floor is met) would convert this silent-revert footgun into an enforced invariant.
…pricing mechanically Review feedback on #294: - The merge from the pricing-overlay base (#314) dropped the models.default-ai-credits-pricing block for the second time in this branch's history; without it every dispatch 400s on the stable toolchain (claude-opus-5 is not in firewall v0.27.42's curated table). Restored, with the prose reconciled to the merged 50% providers overlay: fallback rates stay at list because in the only window it binds, every other model bills at list too. - New model-pricing.test.ts turns both prose warnings into failing checks: every model pin must have a providers overlay entry, and the fallback must exist while claude-opus-5 is pinned (delete that test with the fallback block once gh-aw defaults to v0.27.43+). - Stale rationale comments updated: correctness-reviewer's Fable 5 justification now states what Opus 5 carries; first-principles records the move; the overlay's engine markers moved to opus-5, with opus-4-8 relabeled as the refusal-fallback target. - Changeset renamed to review-roster-opus-5 (roster-wide scope) and records why security-auth moves with the roster instead of a pre-emptive carve-out. Also merges the base branch (sonnet-4-6 overlay entry).
|
Merge-gate verification, run locally with gh-aw v0.85.4 (current stable) against this branch's shared source:
Per the gate's step 4, nothing in the branch needed editing; the compile was verification only and was not committed. Steps 1-3 of the gate are now satisfied. |
There was a problem hiding this comment.
Approved — no blocking issues found.
2 of 2 prior review threads are still unaddressed as of 879902b:
2 non-blocking threads still open
- suggestion (non-blocking)
workflows/review/review.md:316: Whole change hinges on a manual, un-CI-enforced source→lock recompile that can silently revert. The diff edits the distr... - note (non-blocking)
workflows/review/review.md:384: Sixth overlay model (claude-sonnet-4-6) is not in the description's five-row table. The description's What section says...
Full finding
claude-sonnet-5 overlay rates are half of Anthropic's introductory pricing ($2/$10 per MTok), which expires 2026-08-31 — after that, half-of-list is 1.5e-06/7.5e-06 and this entry under-meters sonnet-5 spend by 33%. Introduced by this change. The overlay prices claude-sonnet-5 at input 1e-06 / output 5e-06 (cache_read 1e-07, cache_write 1.25e-06), i.e. 50% of $2/$10 per MTok. I checked the current Anthropic model pricing catalog: Sonnet 5's $2/$10 is introductory pricing through 2026-08-31; standard list is $3/$15, so from 2026-09-01 the correct half-of-list rates are 1.5e-06/7.5e-06 (cache_read 1.5e-07, cache_write 1.875e-06 — identical to the claude-sonnet-4-6 entry below it). This interacts badly with the PR's own merge gate: the block is inert until gh-aw v0.84.x goes stable, so it plausibly activates only after the intro pricing has already expired — meaning the rates are stale from the moment they go live, and max-ai-credits / maxUsd caps would permit ~1.5x the intended real spend on any sonnet-5 run. Exposure today is limited — I grepped review.md's model pins and no in-repo agent uses claude-sonnet-5 (pins are claude-opus-4-8, claude-fable-5, and claude-sonnet-4-6 at pattern-triage), so this only bites consumers selecting sonnet-5 via an engine: override — but the entry exists precisely to price that case. The file's MAINTENANCE comment says to re-halve 'when Anthropic list prices move'; this particular move is already scheduled and dated, so encode the post-expiry standard-list rates now (briefly over-metering during the remaining intro window, while the block is inert anyway, is the safe direction) or at minimum date-stamp the comment with the 2026-08-31 expiry.
Review live A/BBaseline: Ruler: matcher deterministic+arbiter; corpus 5ca2d116455d (10 cases).
Adversarial hard gate: PASSED on the candidate arm. Single-run-stable rows: recall, verdict agreement, regressions, adversarial gate. Judge quality and noise are not: they jitter run-to-run at this corpus size, and a regressed reviewer can score HIGHER on judge quality (fewer, surer comments each read better). Recall against the labeled specs is the load-bearing metric. Measured noise floor (identical arms, run 29069228968, 2026-07-10, 6 arm-samples, full corpus x3, pre-arbiter; budget skips left the samples on unequal case sets, so these v1 bands also carry case-mix variance): must-catch recall 54%-86% (sd 10%), verdict agreement 75%-100% (sd 9%), noise (unmatched posted) 50%-60% (sd 3%), judge mean quality 82%-86% (sd 2%). A single-run delta whose arms both sit inside a band is indistinguishable from run-to-run wobble; use |
What
Adds gh-aw's
models.providerspricing overlay to the shared review workflow, metering AI credits at 50% of Anthropic list (Khan's real rate) for the five pinned models;cache_read/cache_writehalved alongside. Two files: the shared source and its changeset.claude-opus-4-8,claude-opus-5claude-sonnet-5claude-haiku-4-5claude-fable-5Why
The firewall api-proxy prices credits from a list-price catalog baked into its image, so every dollar figure reads 2x high:
max-ai-credits: 2500stops a run at ~$12.50 of real spend rather than $25, andbudgets.ts/counters.tsare denominated in the inflated currency. The overlay is the highest-precedence pricing source (awf-config-spec 10.7.1) and covers the threat-detection proxy too. With a credit worth $0.01 of real spend, the existing caps andmaxUsdtargets are correct as written: deliberately no change tobudgets.ts,credit-cap.ts, or the counters.The rates live only in
workflows/review/review.md; this repo's installed copy picks them up at its next pin bump (an interim local override was considered and removed as pure churn).Merge gate: gh-aw v0.84.x stable
apiProxy.providersneeds AWF >= v0.27.43. Current stable gh-aw (v0.83.4) defaults to v0.27.42 and silently drops the block: clean compile, no warning, rates reach only the informationalGH_AW_INFO_MODEL_COSTS. When v0.84.x is stable:gh extension upgrade gh-aw(confirm v0.84.x)gh aw compileprovidersappears inside bothawf-config.jsonpayloads inreview.lock.yml(presence inGH_AW_INFO_MODEL_COSTSalone means it is not live)Deliberately not worked around with a prerelease gh-aw pin (nothing in CI recompiles, so the next stable compile would silently revert the lock) or a forward
sandbox.agent.versionpin (forward pins recreate the stale-floor 403 failure from run 30290472047).Relationship to #323
#323 enforces the run's spend ceiling in code, superseding this PR's cap-denomination motivation. What remains here that #323 cannot do: price models correctly at the proxy itself, which is what prices
claude-opus-5for #294. Disposition (close vs merge for that purpose) is decided when #323 lands.Maintenance this introduces
Matching is per model, and an unlisted model bills at full list with no error: a new engine model needs an entry, and the rates need re-halving when Anthropic list prices move. The per-model spelling is deliberate; a bare
claude-opus-4prefix would also capture opus-4-0/4-1, which list at 3x the 4-5+ rate.Checks
gh aw compileclean; the installed workflow and its lock are byte-identical tomain; full suite green (69 files, 1618 tests); CI green.