feat(admin): exclude models and experiments from spend, and label what's left - #1271
Open
charlesyhuang wants to merge 7 commits into
Open
feat(admin): exclude models and experiments from spend, and label what's left#1271charlesyhuang wants to merge 7 commits into
charlesyhuang wants to merge 7 commits into
Conversation
…t's left Some spend Oddish records was never actually paid for: provider-sponsored capacity, free preview tiers, vendor credits, a bake-off someone comped. Operators can now declare it along two axes, from the admin panel or the CLI. Replaces the LLM-key exclusion list, which keyed off a hash nobody could read back and only covered the "we have a free key" case. The two axes people actually reach for: * Model (`cost_excluded_models`) -- global and retroactive. Every trial that ever ran on it stops counting, because the reason its spend isn't real is a property of the model. * Experiment (`cost_excluded_experiments`) -- scoped to trials homed in it, so a collection that merely gathers other experiments' trials can't launder their cost. Both fold into `first_party_spend_filter` and the quota inflight predicates, so excluded spend leaves the admin cost dashboards and stops counting against caps together, and re-including it restores every dollar. Dropped from accounting, but deliberately not hidden. Experiment, task, and trial surfaces still price the work that ran and now mark it as not real, so a reader reconciling an experiment tile against the admin dashboard finds an explanation instead of a silent gap. The experiment rollup splits its existing groups on the exclusion predicate to report `excluded_cost_usd`; trials carry `cost_exclusion_reason`, which is null both for real spend and for callers that never resolved exclusions -- absence is not a claim that the money was real. `core/cost_exclusions.py` owns the SQL predicates and the `CostExclusions` Python twin used to label already-loaded rows; keep the two in step. `trials.llm_key_hash` is left in place and is now write-only -- dropping a column off the hot trials table is a separate, riskier change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zmdrcCqnkVM6hWjhPu2T9
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Oddish previewCommit:
Vercel deployment URL: https://oddish-l8qkvtu8r.oddish.app Plan:
This comment is updated by the PR Preview workflow. |
The key list is gone from the code paths; take out what only existed to feed it. `trials.llm_key_hash` had exactly one reader — the exclusion probe — so with the probe gone the column, the fingerprinting module, and both stamping sites are dead weight. Removes `core/llm_key_fingerprint.py` and its test, the `llm_key_hash` column and ORM field, the settlement-time stamp in `trial_handler`, the local-runner stamp, and the `carl_tools` field allowlist entry. BYOK's own `key_hint` is a different feature and is untouched. `costexcl02` now drops the column alongside the table, under a short `lock_timeout` — dropping a column is catalog-only in Postgres but still takes a brief ACCESS EXCLUSIVE lock on a hot table, and failing fast beats queueing behind a long read with every writer stacked up behind us. One subtlety at the claim site: the conditional UPDATE there was never really about the hash. Its rowcount is the "this attempt is still ours" check that aborts a stale or reassigned worker before it does any expensive work. Kept as an UPDATE with the same predicate so the check stays atomic against a concurrent finish, now writing `updated_at`, since an UPDATE needs a SET. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zmdrcCqnkVM6hWjhPu2T9
Four of these are real defects, two of them regressions from the two commits above. Quota sweep no longer cancels excluded trials. `_active_trial_predicates` lost the key-exclusion filter when that feature was deleted and gained nothing in its place, so `cancel_trials_if_quota_reached` would kill every active trial in an excluded experiment — trials that contributed nothing to the number that tripped the cap and reserve nothing against it, so killing them frees no headroom. Adds both filters and a test, since the test that used to cover this was rewritten around the deleted feature. Model exclusions no longer silently no-op. `trials.model` is written by the agent-aware `normalize_trial_model`, which re-routes ids — `kimi-k2` is stored as `moonshot/kimi-k2`, Claude models as Bedrock ids — so storing what the operator typed registered a row that matched nothing, with no feedback. The endpoint now resolves the typed spelling against the values trials actually store, registers every match (one model can have two stored spellings, and excluding one would leave half the spend counting), and 404s when nothing matches. The SQL predicate and its Python twin now both compare `trials.model` verbatim, so they can no longer disagree about which rows are excluded. The CLI could never write. Mutations gated on `can_manage_api_keys`, which rejects API-key auth by design because it guards key management itself — so every `oddish cost-exclusions add` was a guaranteed 403 telling the operator to use the credential they were already using. Operator-org membership is the gate; `require_admin` already counts a full-scope key as admin. The experiment Cost tile claimed gathered spend was comped. It covers member trials including gathered ones, but was passed the whole-experiment excluded flag, which by design only excludes homed trials. Excluding a collection is now rejected outright for the same reason: it homes no trials, so it would exclude nothing while claiming everything was free. Also: migration resets `SET LOCAL lock_timeout` (all pending revisions share one transaction, so it leaked into later ones) and touches `trials` last on downgrade as it already did on upgrade; restores the `llm_key_hash` entry in Carl's SQL deny-list, which is inert once the column is gone but is the only guard where code runs ahead of schema; CLI emits JSON on every error branch and resolves non-canonical model spellings on remove. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zmdrcCqnkVM6hWjhPu2T9
Both deploy workflows run migrations before the code deploy — AGENTS.md states the ordering explicitly, because the backend can hard-require new schema on its hot paths. That is the expand pattern: safe for ADD, wrong for DROP. Dropping the column here leaves the still-serving previous release mapping a column that no longer exists, so every `select(TrialModel)` fails for the length of the Modal image build, and in-flight trials on old containers cannot persist their results. The column is inert either way — nothing reads or writes it after the commits above — so keeping it costs one release and removes the window entirely. The drop belongs in a follow-up migration, once a release that never touches the column is fully rolled out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zmdrcCqnkVM6hWjhPu2T9
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Removes comments and docstrings from the files this PR introduces, and
takes out what the red-team pass found was dead or duplicated. Net -613
lines across 27 files.
Dead code removed: the `byok_env` plumbing through `_settle_trial_metering`
and `store_trial_results`, whose only reader was the deleted key-hash
stamp; the unused `exclusions` parameter on `build_task_status_response`,
which no caller passed because `task_detail` rebuilds the trial list
itself; and `CostExclusions.__bool__`.
Deduplicated: the four Next.js proxy routes now use the app's shared
`proxyBackendJson` / `proxyJsonRequest` instead of reimplementing them —
83 lines down to 10 — which also restores request-id propagation, upstream
server-timing, and abort-signal forwarding, and drops the bespoke
`{error, details}` envelope in favour of the backend payload the rest of
the app already forwards. The two routers share `unavailable` and
`soft_delete`; their genuinely different halves (model resolution against
`trials.model` returning a list, experiment id-or-name resolution
rejecting collections) stay separate.
Pre-existing prose in shared modules is untouched — the accounting
rationale in `cost_basis`, `experiment_cost`, `quotas` and `helpers`
documents other features and this PR has no business deleting it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017zmdrcCqnkVM6hWjhPu2T9
…-exclusion-a5mcpf
Retitles the admin card to "Remove Spend Tracking" and replaces the paragraph under each heading with a single plain sentence. Same rename and trim in DOCS.md, plus a pointer to where the card lives in the dashboard. The two behaviours worth knowing are kept as a short note under the options, because both are things an operator hits in practice: `add model` matches the spelling trials actually store, and a model nothing has run on is rejected rather than saved as an entry that matches nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zmdrcCqnkVM6hWjhPu2T9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some spend Oddish records was never actually paid for: provider-sponsored capacity, free preview tiers, vendor credits, a bake-off someone comped. Operators can now declare it along two axes, from the admin panel or the CLI.
This replaces the cost-excluded LLM-key list, which keyed off a hash nobody could read back and only covered the "we have a free key" case. It supersedes #1132 (experiment exclusions), which had gone stale against
staging.The two axes
cost_excluded_models) — global and retroactive. Every trial that ever ran on it stops counting, because the reason its spend isn't real is a property of the model, not of when it ran.cost_excluded_experiments) — scoped to trials homed in it (trials.experiment_id), so a collection that merely gathers other experiments' trials can't launder their cost. A gathered trial keeps counting wherever it was really spent. Excluding a collection is rejected outright: it homes no trials, so it would exclude nothing while claiming everything was free.Both fold into
first_party_spend_filter, the quota inflight predicates, and the quota cancellation sweep, so excluded spend leaves the admin cost dashboards, stops counting against caps, and is not killed when a cap trips — removing an entry restores every dollar to all three.Dropped from accounting, deliberately not hidden
Experiment, task, and trial surfaces still price the work that ran, and now mark it as not real. Without that, a reader reconciling an experiment tile against the admin dashboard finds a gap with no explanation.
(agent, model, billed, owned)groups on the exclusion predicate to reportexcluded_cost_usd/owned_excluded_cost_usd, plusexperiment_cost_excluded. Only the owned tile may claim the whole figure is comped; the all-member Cost tile covers gathered trials that are still counted elsewhere.cost_exclusion_reason("model"/"experiment"), threaded throughbuild_trial_responsevia a per-request snapshot. Null means real spend or "this caller didn't resolve exclusions" — absence is not a claim the money was real.Matching the spelling trials actually store
trials.modelis written by the agent-awarenormalize_trial_model, which re-routes ids:kimi-k2is stored asmoonshot/kimi-k2, Claude models as Bedrock ids,fireworks/glm-5.2asglm-5p2. Storing what an operator typed would register a row matching nothing, with no feedback and no way to discover the stored spelling.So
POSTresolves the typed value against the values on disk, registers every match (one model can legitimately have two stored spellings — baregrok-free-previewand routedxai/grok-free-preview— and excluding one would leave half the spend counting), and 404s when nothing matches. The SQL predicate and itsCostExclusionsPython twin both comparetrials.modelverbatim, so the query and the label can't disagree about which rows are excluded.CLI
removeaccepts the row id, the model name, or an experiment name/id, canonicalizes model spellings, and rejects ambiguous experiment names rather than guessing. Mutations are gated on operator-org membership;require_adminalready counts a full-scope API key as admin, which is the CLI's only credential.The key list is deleted
Nobody was using it, so this removes everything that only existed to feed it:
core/llm_key_fingerprint.pyand its test, the ORM field, the settlement-time and local-runner stamps, and the now-deadbyok_envplumbing they were the only readers of. BYOK's ownkey_hintis a different feature and is untouched.trials.llm_key_hashis deliberately not dropped here. Both deploy workflows run migrations before the code deploy (AGENTS.md: "the backend can hard-require new schema on its hot paths"). That is the expand pattern — safe for ADD, wrong for DROP. Dropping it would leave the still-serving previous release mapping a column that no longer exists, failing everyselect(TrialModel)for the length of the image build. The column is inert either way, so it costs one release and the window disappears. Follow-up PR to drop it, once this is rolled out. Its entry stays in Carl's SQL deny-list meanwhile, which is the only guard where code runs ahead of schema.One subtlety at the claim site in
trial_handler: the conditional UPDATE there was never about the hash — its rowcount is the "this attempt is still ours" check that aborts a stale or reassigned worker. It stays an UPDATE with the same predicate so the check remains atomic, now writingupdated_at(whichonupdatewas already writing on every claim, so no new writes).Reviewer notes
Three read-only red-team passes ran over this; their findings are in commit
fc07695. The two that were regressions from earlier commits in this PR — the quota sweep cancelling excluded trials, and model exclusions silently matching nothing — both now have tests, since the coverage that would have caught the first was itself rewritten around the deleted feature.test_quota_enforcement.py::test_org_quota_cancels_every_users_active_trialsbuilt its preserved-task case on an excluded key, so it was reworked to a plain second task;test_cost_exclusions.pycarries the exclusion-specific coverage now.Comments and docstrings are stripped from the files this PR introduces, per request. Pre-existing prose in shared modules (
cost_basis,experiment_cost,quotas,helpers) documents other features and is left alone.Testing
test_cost_exclusions.pycovering both axes dropping fromget_cost_breakdown_coreandsum_org_cost_usd, removal re-including, inflight reservations skipping excluded spend, and the cancellation sweep sparing it.ruff checkat the same 41 findings as the base branch,pnpm tsc --noEmitclean, prettier applied.🤖 Generated with Claude Code
Generated by Claude Code