[AgentGymLeader]

This PR contains:

• New features
• Changes to dev-tools e.g. CI config / github tooling
• Docs
• Bug fixes
• Code refactor

What is the current behavior? (You can also link to an open issue here)

Reading a subset of samples from a large .eval log currently has only two options, both O(N) or worse: read_eval_log(...) parses every sample, and looping read_eval_log_sample(...) rebuilds the AsyncZipReader and re-parses the zip central directory on every call (O(K·N)). See #2873.

What is the new behavior?

Adds read_eval_log_samples_by_id(log, samples, *, concurrency=...) (sync) and read_eval_log_samples_by_id_async(...): fetch only the requested (id, epoch) samples, concurrently, over a single shared AsyncZipReader (central directory parsed once). Reuses the existing read_eval_log_sample_async(reader=...) path; preserves request order; bounds fan-out with a semaphore; composes with exclude_fields; raises IndexError for a missing (id, epoch).

Benchmark (50k-sample .eval, fetching K=200, concurrency=8, median of 3): read_eval_log + pick K = 27.3s; sequential read_eval_log_sample loop = 20.2s; read_eval_log_samples_by_id = 0.11s (~176x / ~238x).

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

No — purely additive (two new public functions). No existing API or behavior changes.

Other information:

This is the "read fewer samples" (slicing) piece from #2873, distinct from the per-sample-cost work (validate=False / orjson, and exclude_fields in #3816 / #3990). It does not by itself speed up eval-retry or a full samples_df (those touch ~all samples), but it's the building block for a faster partial eval-retry (read summaries → load only the incomplete samples). Opened per @WatchTree-19's suggestion on #2873. Tests in tests/log/test_read_samples_by_id.py; make check (ruff + mypy) and the tests/log suite are clean.

[WatchTree-19]

nice. the (id, epoch) primitive is the right shape and the 27s/20s -> 0.11s win on the K=200/N=50k subset is convincing - that's the O(K) vs O(K*N) central-directory cost showing up clearly.

three minor things:

1. validate=False parallel kwarg. once the per-sample-cost PR lands (skip Pydantic on our-own-logs read), composing both - this for K-of-N, validate=False for cost-per-sample - is what makes partial eval-retry actually O(unfinished). worth adding the kwarg here so the composition is wire-ready when the other PR merges. happy to do that in a follow-up if you'd rather keep this PR focused.

2. samples: Iterable[tuple[str | int, int]]. the tests cover the int-id path (auto-assigned 1..n_samples). the string-id path (custom sample.id) - does AsyncZipReader look up by the same key format the central directory writes? worth a test if it's not obviously covered by read_eval_log_sample.

3. AsyncZipReader concurrency under one shared reader with semaphore=8. zipfile.ZipFile isn't thread-safe for concurrent reads through one handle; assuming AsyncZipReader serialises the actual member fetches and the parallelism is on the post-fetch parse + validate. if so worth a one-line docstring note so callers don't expect linear scaling on concurrency.

the "partial eval-retry consumes this + summaries to find incomplete samples, then loads only those" follow-up is the right next move and ties cleanly to the per-sample-cost PR. happy to wire that up after both this and Option 1 land if useful.

[AgentGymLeader]

Thanks, this is a great review.

2 (string-id): added a test — Sample(id="q-alpha", ...) flows through the same _sample_filename (samples/{id}_epoch_{epoch}.json), so the string path works; now covered explicitly.

3 (concurrency): added a docstring note. One small correction to the premise while I was in there — AsyncZipReader doesn't share a single zipfile handle/cursor: the central directory is parsed once (cached, lock-guarded) and each member is fetched via an independent byte-range read, so the fetches aren't serialized. concurrency bounds in-flight reads (range fetch + json parse + validate), so it's not a linear-scaling knob — but the parallelism isn't limited to post-fetch parse either.

1 (validate=False): agreed — that's the composition that makes partial eval-retry O(unfinished). Since validate lands with the per-sample-cost PR, I left the kwarg out here so it doesn't reference a param that doesn't exist yet; happy to add the passthrough once Option 1 merges, or fold it into your follow-up — whichever keeps the two shapes from overlapping.

And yes — summaries → find incomplete → read_eval_log_samples_by_id for partial eval-retry is exactly the follow-up I had in mind. Happy to coordinate sequencing after this and Option 1 land.

[WatchTree-19]

the rebase + expanded test coverage looks good, and deferring validate=False until the per-sample-cost PR lands is the right call - keeps the two shapes from cross-contaminating. lgtm.

[WatchTree-19]

the two adds look right: the string-id test confirms _sample_filename consistency across id types, and the docstring note correctly flags that concurrency bounds the fetch + parse, not parse-only. and yes, deferring the validate=False kwarg until Option 1 lands keeps the two PRs from racing on the same signature.

can wire the partial-eval-retry consumer (summaries to incomplete sample ids to read_eval_log_samples_by_id) as a follow-up once both this and Option 1 merge. ping on either thread if useful.

LGTM, ready to merge from my side.