Scaffolding: trials.kind + experiments.shadow_of (no behavior change) - #1290
Scaffolding: trials.kind + experiments.shadow_of (no behavior change)#1290kyle-compute wants to merge 2 commits into
Conversation
Two chained migrations (trialkind01, shadowexp01) plus the model columns: trials.kind VARCHAR(32) NOT NULL DEFAULT 'agent' with a partial index on the non-agent rows, and experiments.shadow_of VARCHAR(64) with a partial unique index (one live shadow per experiment, enabling race-safe INSERT .. ON CONFLICT get-or-create). Nothing writes a non-agent kind or a shadow_of value yet. Kind-awareness lands ahead of the writers: first_party_spend_filter() gains a kind = 'agent' clause (so quota accounting and every cost surface that shares it exclude future analysis runs), analysis_spend_filter() (kind != 'agent') is added for the QA-cost surfaces, and EligibleTrialScope excludes non-agent kinds by default with an include_non_agent_kinds opt-out. frontend types.ts gains the TrialKind union (type only). No behavior change on existing data: every existing row is kind 'agent', so all filters select exactly what they selected before. Claude-Session: https://claude.ai/code/session_01ACF6SUXdbLFarpj3qwF1Ki
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Oddish previewCommit:
Vercel deployment URL: https://oddish-4b03bhxhp.oddish.app Plan:
This comment is updated by the PR Preview workflow. |
An interrupted CREATE INDEX CONCURRENTLY leaves a same-name INVALID index behind; IF NOT EXISTS then sees the relation and skips creation, so a retried migration completes with an index that serves no queries. For uq_experiments_shadow_of_live that silently breaks the shadow creator's INSERT .. ON CONFLICT arbiter inference, which only considers valid indexes. Before each concurrent CREATE, drop a same-name index whose pg_index.indisvalid is false so the CREATE rebuilds it. Drilled against Postgres 16: corrupting both indexes and rerunning the unfixed migrations left indisvalid=f and the ON CONFLICT insert failing with "no unique or exclusion constraint matching"; with this change the rerun rebuilds both (indisvalid=t) and the conflict path dedupes again. Claude-Session: https://claude.ai/code/session_01ACF6SUXdbLFarpj3qwF1Ki
|
Review fix (68ab5a8): both concurrent-index migrations now recover from an interrupted build. An interrupted Drilled against Postgres 16: corrupting both indexes and rerunning the unfixed migrations left |
1 participant
Important
This PR is stacked on #1287 (PR A). Its base is
qa-rebuild/a-delete-reports, notstaging.C cannot merge before A because its first migration comes after A's migration.
After A squash-merges:
Then this PR's base changes to
stagingand it can move out of draft.This is PR C of six (A–F) rebuilding analysis so analysis jobs run as normal trials. A is #1287 and B is #1288.
C only adds the database and filtering scaffolding needed for that system. Nothing writes analysis trials yet, so existing behavior is unchanged.
What this PR adds
Two new columns:
plus the filters and indexes needed to use them safely later.
trials.kindEvery trial now has:
Existing and normal user trials are:
Later PRs will create analysis trials such as:
Those analysis trials must not be counted, billed, or displayed as normal user runs.
The column lands now so every existing counter and cost query can exclude them before any analysis trials are created.
A partial index is also added:
It only indexes rows where:
Almost every trial will remain an
agenttrial, so this keeps the index small while making analysis-trial lookups cheap.experiments.shadow_ofNormal experiments keep:
Later, an experiment can have one hidden shadow experiment whose
shadow_ofpoints back to the real experiment.Analysis trials will live in that shadow experiment instead of the real one, so they never appear in the user's normal trial list.
A partial unique index enforces:
The uniqueness condition is:
That means two workers racing to create the same shadow cannot create duplicates, while soft-deleting a shadow allows a replacement to be created later.
I verified this directly on Postgres 16:
INSERT ... ON CONFLICT ... DO NOTHINGsafely loses the raceNULLvalues do not conflictCost filtering
first_party_spend_filter()now includes:That function is already the shared definition of spend that counts for:
Adding the condition once means all of those surfaces automatically exclude future analysis-trial costs.
This PR also adds:
which selects:
PR D will use that for analysis-cost reporting.
It intentionally uses
!= 'agent'instead of enumeratingqa,audit, etc. Any future analysis kind is therefore excluded from user spend automatically.Trial eligibility
EligibleTrialScopenow excludes analysis trials by default by adding:A caller that intentionally wants analysis trials can opt out with:
The frontend also gains the
TrialKindtype.Nothing reads that type yet.
Migrations
This PR adds two migrations:
The resulting Alembic head is:
I verified the migrations against Postgres 16 beyond a basic up/down test:
kind = 'agent'pg_indexcreate_alldatabases produce identical column and index definitionsALTER TABLEuses:so a deploy cannot wait indefinitely behind a long-running production query
The index-validity check matters because a failed concurrent index creation can leave an invalid index behind, after which
IF NOT EXISTSwould otherwise silently skip recreating it.Verification
The existing suites already have unrelated failures on fresh databases, so the regression check is a failure-set diff against this PR's parent, PR A.
Both branches were run against identical local Postgres 16 setups.
Result:
The only difference in the oddish-only harness is the two DB-backed tests added by this PR. They require the
organizationstable from the backend tree, so they fail when oddish is tested against an oddish-only database and pass when both trees are present.That is the same harness limitation as the existing
test_cost_excluded_llm_keys.pytest they are modeled on.pnpm buildalso exits 0.Tests added
This is an additive PR, so it includes a small set of tests for the new behavior:
schema assertions for both columns, defaults, and index names
compiled-SQL assertions proving
EligibleTrialScope:kind = 'agent'by defaultinclude_non_agent_kinds=TrueDB-backed cost tests proving:
$7.00kind='qa'trial is excluded from admin/user spendanalysis_spend_filter()selects that analysis trial$2.00agent trial is still counted as exactly$2.00What changes for users
Nothing yet.
After this PR:
agenttrialsThis PR only puts the invariants in place so PR D can safely create analysis trials without contaminating normal trial counts, costs, or experiment views.
Next: PR D — analysis-trial engine + cutover, stacked on this branch.