[BENCH-766] Model pricing in Postgres: admin-recorded rates with history - #594
[BENCH-766] Model pricing in Postgres: admin-recorded rates with history#594cooperr1 wants to merge 1 commit into
Conversation
Adds benchmarks_v2.pricing_rates (migration 20260903_0027), an append-only log seeded with the 61 rates verified against providers' public pricing pages. A price changes through POST /v1/admin/pricing instead of a deploy, and every rate ever served stays on record: recording again for the same effective date corrects it (the latest wins, the earlier row is kept and marked superseded), a future effective date schedules a change, and null unit + price records that no public rate is known from that day. GET /v1/pricing reads the log live and serves the rate in force on ?as_of (default today, never later) with the earlier spans as history, behind the same roster and embargo filters as every other data endpoint. GET and POST /v1/admin/pricing require the coval org (require_coval_admin) and stamp each recording with the caller's Clerk user id and email. Every write passes the same PricingEntry rules as a ratesheet entry: the unit bills that benchmark, the price is a decimal string, the source is a real URL, the effective date is at most a year ahead. The downgrade refuses to drop the table once it holds a recording made after the seed. GRANTs are Terraform-managed: the api role needs SELECT and INSERT on pricing_rates.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coval-cale
left a comment
There was a problem hiding this comment.
Approving overall. I left two follow-up correctness comments around model identity across renames and unsupported benchmark validation.
| roster = {(m.benchmark, m.provider, m.model) for m in models} | ||
| rates: list[PricingRateOut] = [] | ||
| for key, timeline in sorted(timelines(recordings).items()): | ||
| if key not in roster or (key[1], key[2]) in hidden: |
There was a problem hiding this comment.
[P2] Keep pricing attached to an immutable model identity. This joins the pricing log to the current roster using the mutable (benchmark, provider, model) key. After a model is renamed, its old key becomes available to POST /admin/models; a different model later created under that key will silently inherit the original model's entire pricing timeline, potentially including embargoed rates. Please store an immutable model ID, migrate rates during rename, or reserve historical keys so they cannot be reused.
| """ | ||
| CREATE TABLE benchmarks_v2.pricing_rates ( | ||
| id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, | ||
| benchmark TEXT NOT NULL CHECK (benchmark IN ('STT', 'TTS', 'S2S')), |
There was a problem hiding this comment.
[P2] Reject unsupported modalities before inserting. AdminRateCreate accepts the full Benchmark enum, and an unpriced LLM entry skips PricingEntry's unit validation. For a registered LLM model, model_exists() therefore succeeds and this CHECK rejects the insert, surfacing an uncaught database error as a 500. Please restrict the admin schema to supported pricing modalities or make the database and validation rules agree.
Linear: BENCH-766
What
benchmarks_v2.pricing_rates(migration20260903_0027): append-only log, seeded with the 61 rates verified against providers' public pricing pages. Nothing updates or deletes a row; a correction appends and marks the earlier row superseded, a future date schedules a change, null unit + price records "no known public rate".GET /v1/pricing?as_of=: the rate in force on a day (default today) with earlier spans ashistory. Same roster and embargo filters as every other data endpoint.GET/POST /v1/admin/pricing: coval-org only via the existingrequire_coval_admin. Each recording is stamped with the caller's Clerk user id and email. Writes pass the ratesheetPricingEntryrules.Deploy
gcloud run jobs execute benchmarks-runner --region=us-east1 --project=coval-benchmarks-prod --args="db,migrate"pricing_rates, applied via Atlantis after the table exists.Until the grant lands,
GET /v1/pricingreturns 503 and admin writes fail closed. Nothing else is affected.Verification
cd runner && uv run ruff check . && uv run ruff format --check . && uv run mypy --strict src tests && uv run pytest -q --disable-socket --allow-unix-socket --allow-hosts=127.0.0.1,::1— 2010 passed. Live on the local stack with a mock Clerk issuer: 401 no token, 403 other org, 422 bad unit / float price / bad URL / two years out, 201 stamped recording, repeat 200, public read serves the new rate with the old one in history,?as_of=yesterdayserves the old one.