feat(metrics): per-(task version, agent, model) trial metrics - #1225
feat(metrics): per-(task version, agent, model) trial metrics#1225stateofkate wants to merge 6 commits into
Conversation
Adds task_version_model_metrics at a grain finer than task_version_browse_summaries, so pass rate and trajectory length can be selected per model rather than only filtered on. Rides the existing refresh_task_browse_summaries hook and its advisory locks instead of adding a second maintenance mechanism. Recompute per group, never delta: retried trials reset to RUNNING with reward and total_steps nulled, so they move backwards out of terminal buckets. Step counts are stored as a distribution split by outcome (min/p50/max via percentile_disc, each with its own _n) because a mean hides both a suspiciously cheap pass and a failure capped at the step ceiling. Table is inert until backfilled.
Populates task_version_model_metrics for existing task versions, which the migration deliberately leaves empty so it stays fast on a hot trials table. Keyset pagination over task version ids rather than "versions missing a row": a version whose trials are all out of scope legitimately produces no row, so a missing-row cursor hands back the same page forever. Resumable with --after-id. Tests cover the recompute (bucket split, percentile_disc medians, NULL steps excluded rather than zeroed, in-flight exclusion, agent/model grain, backwards transitions on retry, soft-delete and supersession, stale-row cleanup) and the backfill itself (terminates past a trial-less version, idempotent on re-run).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: medium. Left a non-blocking comment because Cursor Bugbot reported an unresolved high-severity finding (unscored counts include in-flight trials) and the Bugbot check finished as skipped. Assigned reviewers for the browse-summary metrics path.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Oddish previewCommit:
Vercel deployment URL: https://oddish-p6h21r2wj.oddish.app Plan:
This comment is updated by the PR Preview workflow. |
The recompute assumed its only caller was refresh_task_browse_summaries, which already holds a sorted advisory lock per version id. The backfill calls it directly and held no lock, so a backfill batch racing a live refresh could overwrite a fresh row with the snapshot it aggregated moments earlier -- silent staleness rather than a crash, and invisible to a single-threaded fixture test. pg_advisory_xact_lock is re-entrant within a transaction, so taking an already-held lock on the browse-summary path costs nothing, and the shared sort order keeps both callers in one global ordering. Proven by holding the lock from a second connection and asserting the recompute blocks; the test fails with DID NOT RAISE when the lock is removed.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 68508de. Configure here.
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. |
1 participant
Summary
Adds
task_version_model_metrics: trial metrics at(task_version_id, agent, model), a grain finer than the existingtask_version_browse_summaries. Makes per-model pass rate and trajectory length selectable rather than only filterable —_subject_metric_valuecomputes exactly this today and discards it, since it is only ever consumed by aWHERE.Step counts are stored as a distribution split by outcome (min / p50 / max, each with its own
_n), not a mean. A mean hides the two things worth catching, because they sit at opposite extremes and average to something unremarkable:The prod dry run found an instance in the first 2,400 task versions:
grok-buildonxai/v9m-rl-learnability-tp8, 3 passing trials at exactly 1 step each (min = p50 = max = 1).Design notes
Rides the existing refresh, no new maintenance mechanism. One call at the end of
refresh_task_browse_summaries, so all 18 existing hook sites cover the finer grain and it runs under the advisory locks already held. No trigger, no dirty queue, no reconciliation worker. The bucket taxonomy and scope predicate are reused verbatim fromtask_browse_metrics, so this does not add a third definition of either.Recompute per group, never delta.
trial_handler.pyresets a retried trial toRUNNINGwithrewardandtotal_stepsnulled, so trials move backwards out of terminal buckets. Delta arithmetic is only correct under exactly-once in-order delivery. A distribution cannot be maintained incrementally at all — you cannot add a trial to a median.percentile_disc, not_cont. Most groups hold four trials or fewer, where interpolation returns a step count no trial ever had.The table is inert until backfilled — the migration does no inline work, so it stays fast on a hot
trialstable.Verification
tvm_metrics_001at headpercentile_discmedians, NULL steps excluded rather than zeroed, in-flight exclusion, agent/model grain, backwards transitions, soft-delete/supersession, stale-row cleanup) and backfill (terminates past a trial-less version, idempotent on re-run)GroupAggregateover anIncremental SortwithPresorted Key: task_version_id— it seeks, no sequential scanruffclean. Pre-existing suite failures confirmed unrelated by toggling the hook off and getting byte-identical15 failed, 197 passedboth waysRollout
Merging applies the migration through the normal deploy. The table is empty until:
Prod has 119,750 task versions. Projected ~540k metric rows and ~1.2 minutes of query time — that figure measures the SELECT only, since the upsert could not be timed against a table that does not exist there yet, so treat it as a floor. The projection also extrapolates from the first 2,400 versions in id order, and rows-per-version rose steadily across batches, so the row count is likely understated.
Caveat worth knowing before reading the output
trials.total_stepswas only added 2026-06-18, so it is NULL on ~89% of historical trials and the step columns will be NULL for most older groups. The live path has been clean since late June (post-migration trials withharbor_shaset: 0.00% missing). Step data is trustworthy from roughly August onward; before that it is a thin, agent-biased slice. Backfillingtotal_stepsitself is a separate, optional workstream — out of scope here.Note
Medium Risk
Touches hot trial aggregation paths and adds concurrent refresh/backfill logic guarded by advisory locks; migration is schema-only but operational backfill is heavy on
trials.Overview
Introduces
task_version_model_metrics, a denormalized rollup one grain finer than task browse summaries: each row is (task_version_id, agent, model) with pass/partial/fail and unscored/cancelled counters, reward/runtime sums, and step distributions (min /percentile_discp50 / max) split by pass vs fail (partial is count-only). Ratios stay computed at read time; the migration creates the table and indexes only—no inline backfill on deploy.refresh_task_version_model_metricsfully recomputes affected groups from trials (same browse scope and bucket taxonomy as existing browse metrics), upserts rows, deletes stale (agent, model) keys, and takes the same per-version advisory locks as browse summary refresh. That refresh is now invoked at the end ofrefresh_task_browse_summaries, so existing summary hooks keep the new table warm without new call sites.A batched, resumable backfill (
backfill_task_version_model_metrics) walks task versions by id keyset (not “missing row”) for initial population after merge.Reviewed by Cursor Bugbot for commit 68508de. Bugbot is set up for automated code reviews on this repo. Configure here.