feat(summary): auto-enqueue a trajectory summary for every finished trial - #1277
Open
stateofkate wants to merge 1 commit into
Open
feat(summary): auto-enqueue a trajectory summary for every finished trial#1277stateofkate wants to merge 1 commit into
stateofkate wants to merge 1 commit into
Conversation
…rial A trajectory summary only ever existed where something asked for one: the post-trial classifier's component map, which runs on `run_analysis` tasks, or a dashboard/share page read. Trials on tasks with QA off carried no summary until a human opened them. `_run_post_trial_hooks` now enqueues one per finished trial, behind `ODDISH_AUTO_TRAJECTORY_SUMMARY` (off by default). Eligibility is real agent trials only -- terminal, not cancelled, not superseded, not a probe, not a nop/oracle baseline, and with a fetchable trajectory. Baselines and probes are excluded because their trajectories are near-empty or harness noise and each would still pay for a full LLM call. The enqueue itself is delegated to `get_or_enqueue_summary_job` through a seam, because that function owns the payload and its `schema_version` idempotency key. A second enqueue site with its own key would not find the first one's job, so a page view after a trial finished would pay for the same summary twice. Eligibility deliberately does not skip a trial that already has a mirrored summary: the mirror can be at an older schema, and only the seam knows the current one. Runs in the hook's own transaction, after the stage transition, so a hook that rolls back leaves no paid job behind and the QA job enqueued above it finds the summary cached rather than building its own.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Oddish previewCommit:
Vercel deployment URL: https://oddish-1f4ot593j.oddish.app Plan:
This comment is updated by the PR Preview workflow. |
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
1 participant
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.
What
A trajectory summary only ever existed where something asked for one: the post-trial classifier's component map (which runs only on
run_analysistasks), or a dashboard / share-page read. Trials on tasks with QA off carried no summary until a human opened them._run_post_trial_hooksnow enqueues one per finished trial, behindODDISH_AUTO_TRAJECTORY_SUMMARY, off by default — so merging this changes nothing until the env var is set.Eligibility
Real agent trials only: terminal (
SUCCESS/FAILED), not cancelled, not superseded, not a probe, not a nop/oracle baseline, and with a fetchable trajectory (which includes old grok-build trials whosegrok-build.jsonsynthesizes to ATIF). Baselines and probes are excluded because their trajectories are near-empty or sanctioned-harness noise, and each would still pay for a full LLM call.Why it delegates instead of enqueueing directly
The job itself (
ANALYZER+payload.mode = "trajectory_summary") already exists from #1261. The enqueue shape stays inget_or_enqueue_summary_job, reached fromoddish/through a new enqueuer seam, because that function owns the payload and itsschema_versionidempotency key — and onlybackend/knowsSCHEMA_VERSION.A second enqueue site with its own key would not find the first one's job, so a page view after a trial finished would have paid for the same summary twice. For the same reason, eligibility deliberately does not skip a trial that already has a mirrored
trials.trajectory_summary: the mirror can be at an older schema, and only the seam knows the current one.Placement
The call runs in the hook's own transaction, after the stage transition. A hook that rolls back leaves no paid job behind, and the QA job enqueued just above it finds the summary cached rather than building its own.
Standalone
oddishregisters no enqueuer, so the whole path is a no-op there — correct, since there is no generator to reach.Cost
One streamed LLM call per eligible trial. On
run_analysis=Truetasks this mostly moves existing spend earlier (generation is read-through cached, so the classifier then pays nothing); the genuinely new spend is trials on tasks with QA off.Existing cancellation covers these for free:
cancel_tasks_runsmatches onsubject_table='trials'regardless of kind, so cancelling a sweep retires its pending summary jobs. No UI change either — the frontend only special-cases job kindstrial/qa/analysis, so nothing mistakes these for a running analysis.Testing
oddish/tests/test_trajectory_summary_auto_enqueue.py— 17 tests: flag on/off, unregistered enqueuer, the 9-case eligibility table, FAILED-trial and grok-build inclusion, stale-mirror pass-through, and the hook wiring.backend/tests/test_trajectory_summary_auto_enqueue_seam.py— importing the worker's provider module installs the enqueuer, and the enqueuer delegates toget_or_enqueue_summary_job. A missing registration is otherwise silent: theoddishside just stops enqueueing.Each guard was proven by mutation — the flag check, the unregistered-enqueuer check, the eligibility gate, the hook call, and the registration line were each removed in turn and the matching test watched go red.
Full
oddishandbackendsuites were run against this branch and against cleanstaging; the failure sets are identical, so no regressions. (Thetest_new_model_rerunsteardown errors flake at the same rate on both — 20 errored instances across 6 runs either way.)