Persist large V1 traces off the event loop - #1814
Merged
Merged
Conversation
Contributor
ApprovabilityVerdict: Approved This is a focused performance improvement that moves blocking trace serialization off the event loop using standard async patterns. Changes are limited to trace persistence code with consistent updates to all call sites. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ed6625dd6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
macroscopeapp
Bot
dismissed
their stale review
June 21, 2026 14:13
Dismissing prior approval to re-evaluate d97c79d
pull Bot
pushed a commit
to Stars1233/verifiers
that referenced
this pull request
Jun 23, 2026
* Persist large traces off the event loop * Preserve queued trace writes on cancellation * Use public Pydantic JSON serialization
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
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.
Overview
This moves V1 trace persistence off the event loop while retaining one durable JSONL append per completed trace.
TypeAdapter.dump_jsonbytes API and append binary JSONL, avoiding the text decode/re-encode path.Why
Large traces previously called
model_dump_json()and performed a text-mode append synchronously from completion callbacks. Three traces completing together could monopolize the event loop for the entire combined serialization window. The text path also converted Pydantic's UTF-8 output to a Python string and encoded it again for the file, adding roughly one payload-sized transient allocation.A per-run lock keeps serialization and writes ordered without introducing a persistent writer lifecycle. Awaiting the shielded persistence task preserves queued traces, error propagation, and the existing per-trace open/write/close boundary.
Observed impact
The event-loop workload used three simultaneous 29,128,201-byte traces (87,384,603 bytes total). Five-run component-wise medians:
A single 29,256,201-byte Unicode trace measured the serialization allocation path:
The serialized JSON representation, subclass fields, ordering, write-error propagation, and per-trace file-close behavior remain unchanged.
Note
Low Risk
Changes are localized to eval output I/O and completion callbacks; JSONL format and per-trace durability semantics are preserved, with only concurrency/cancellation behavior around writes being new.
Overview
V1 eval trace persistence no longer blocks the asyncio event loop during large
results.jsonlappends. Each completed trace still gets one durable JSONL line with the same ordering guarantees as before.append_traceis now async: Pydantic serializes to UTF-8 bytes viaTypeAdapter.dump_json, and the file append runs in a worker thread (asyncio.to_thread) behind a per-runasyncio.Lockso concurrent completions cannot interleave lines.asyncio.shieldkeeps an in-flight persist from being dropped on caller cancellation.Episode.runtakes an optional asyncon_completehook (awaited when each trace is finalized). Nativerun_eval, env-serverrun_eval_server, andrun_legacy_evaleach create a shared write lock and awaitappend_tracefrom their completion paths.Reviewed by Cursor Bugbot for commit 35372c2. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Persist large V1 traces off the event loop using async writes and a shared lock
append_tracecalls with an async version inoutput.pythat usesasyncio.to_threadto run file writes in a worker thread, keeping the event loop unblocked.asyncio.Lockshared across rollouts, preserving whole-line ordering inresults.jsonl.Episode.run,run_eval,run_eval_server, andrun_legacy_evalto pass the shared lock and await the async callback.on_completecallback inEpisode.runis now async; the default isNoneinstead of a no-op lambda.Macroscope summarized 35372c2.