feat(evaluator): expose per-task attempts with trial identity - #1224
Open
ngoncharenko wants to merge 7 commits into
Open
feat(evaluator): expose per-task attempts with trial identity#1224ngoncharenko wants to merge 7 commits into
ngoncharenko wants to merge 7 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe SDK now stores typed, trial-linked per-task metric values, uses them for pass@k, persists them in ChangesTask metric value handling
Harbor example configuration
Compose live test lifecycle
Sequence Diagram(s)sequenceDiagram
participant ScoreRecords
participant AgentEvalSummary
participant SummaryJson
participant GymInspector
ScoreRecords->>AgentEvalSummary: collect task_metric_values
AgentEvalSummary->>AgentEvalSummary: calculate pass@k
AgentEvalSummary->>SummaryJson: write typed trial records
SummaryJson->>GymInspector: load summary.json
GymInspector->>GymInspector: display task and trial values
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/results.py`:
- Around line 591-604: Update the output-schema tracking around output_keys and
excluded so exclusions are keyed by task.id as well as (metric_type, spec.name).
Apply exclusion checks only within the current task when filtering persisted
values and pass@k inputs, preserving retained outputs for other tasks; add
coverage where one task retains a model output and another excludes the same key
as a continuous score.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a940a624-881c-44d9-bf66-e89732790587
⛔ Files ignored due to path filters (1)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/results.pyis excluded by!sdk/**
📒 Files selected for processing (6)
packages/nemo_evaluator_sdk/examples/gym/README.mdpackages/nemo_evaluator_sdk/examples/gym/inspect_results.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/results.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_pass_at_k.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_persistence.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_task_metric_values.py
Contributor
|
ngoncharenko
changed the title
ngoncharenko
force-pushed
the
ngoncharenko/aalgo-310-passatk-harbor-runner
branch
from
August 11, 2026 18:51
b8d501a to
3d85361
Compare
Contributor
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/nemo_evaluator_sdk/examples/gym/inspect_results.py (1)
137-164: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocstring omits the new
unmeasuredverdict.The function now emits a fourth classification and counter. Add it to the docstring so the four printed states are documented.
📝 Proposed edit
``None`` is a trial that died: it counts as an attempt and never as a pass, so a task that passed once and crashed once reads as flaky rather than solved. + + An empty attempt list is ``unmeasured``: the task produced no usable measurement, either because + its metric failed or because no trial ran. """🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nemo_evaluator_sdk/examples/gym/inspect_results.py` around lines 137 - 164, Update the show_per_task docstring to document all four emitted verdicts: solved, flaky, failed, and unmeasured, including that unmeasured applies when a task has no recorded attempts. Keep the existing attempt and pass@k behavior description unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/nemo_evaluator_sdk/examples/gym/inspect_results.py`:
- Around line 137-164: Update the show_per_task docstring to document all four
emitted verdicts: solved, flaky, failed, and unmeasured, including that
unmeasured applies when a task has no recorded attempts. Keep the existing
attempt and pass@k behavior description unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a2eb5db6-98da-4d10-8bf5-c6d10e2e8fb1
⛔ Files ignored due to path filters (1)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/results.pyis excluded by!sdk/**
📒 Files selected for processing (8)
docs/evaluator/agent-eval/reading-results.mdxpackages/nemo_evaluator_sdk/examples/gym/README.mdpackages/nemo_evaluator_sdk/examples/gym/inspect_results.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/results.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_pass_at_k.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_persistence.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_task_metric_attempts.py
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/nemo_evaluator_sdk/tests/agent_eval/test_pass_at_k.py
- packages/nemo_evaluator_sdk/examples/gym/README.md
ngoncharenko
force-pushed
the
ngoncharenko/aalgo-310-passatk-harbor-runner
branch
from
August 11, 2026 19:18
378ba22 to
8193be9
Compare
Contributor
Author
|
@coderabbitai continue the review |
Contributor
|
|
| "'<metric_type>.<output>', plus per-semantic-view rollups named 'view.<name>'. " | ||
| "Failed or missing scores are surfaced as nan_count." | ||
| ), | ||
| examples=[ |
Contributor
There was a problem hiding this comment.
nit: Will these examples show up in openapi.yaml? In service mode, i am wondering if extensive example can blow up the size of openapi spec
Contributor
Author
There was a problem hiding this comment.
- On the service side, this is stored in summary.json and not exposed in the API spec
- Added this on purpose because the results is not a straightforward data structure and having illustrative examples should help both agents and humans.
| ), | ||
| examples=[ | ||
| { | ||
| "contract-review-msa-indemnity": { |
Contributor
There was a problem hiding this comment.
So this essentially builds the reverse index of results ie task : {runner: [{trials: (name, score)}]}
Contributor
Author
There was a problem hiding this comment.
yes, to match harbor format
ngoncharenko
force-pushed
the
ngoncharenko/aalgo-310-passatk-harbor-runner
branch
from
August 12, 2026 03:55
328f40d to
3eebc8b
Compare
| *, | ||
| metric_type: str, | ||
| output_name: str, | ||
| ) -> dict[str, list[float | None]]: |
Contributor
There was a problem hiding this comment.
I wonder if we should provide a stronger type here. Instead of:
{
"task-47": { "harbor_reward.reward": [
{ "trial_id": "task-47__7f3a9c", "value": 1.0 },
{ "trial_id": "task-47__1b8e42", "value": null }
]}
}
maybe use a model like:
class TrialOutcome:
trial_id: str
value: float | int | bool | None
class PerTaskOutcome(BaseModel):
metric_name: str
trials: list[TrialOutcome]
class PerTaskOutcomes(BaseModel):
task_id: str
outcomes: list[PerTaskOutcome]
We actually have the type AgentEvalAttemptValue which seems to match this TrialOutcome.
Comment on lines
+114
to
+118
| if "task_metric_attempts" not in payload: | ||
| raise SystemExit( | ||
| f"{bundle / 'summary.json'} predates summary.task_metric_attempts, which this script reads " | ||
| "per-task outcomes from. Re-run the eval to produce a current bundle." | ||
| ) |
Contributor
There was a problem hiding this comment.
SystemExit seems extreme for the exception, right? Maybe a custom exception would be appropriate here. Also, I assume the AgentEvalSummary.model_validate would have failed anyway? Why couldn't we just handle the model_validate failure instead?
Contributor
Author
There was a problem hiding this comment.
- AgentEvalSummary.model_validate won't raise since default value is a dict
- Updated SystemExit part
| "(trials.jsonl) and AgentEvalTaskScore.trial_id (scores.jsonl)." | ||
| ) | ||
| ) | ||
| value: float | None = Field( |
Contributor
There was a problem hiding this comment.
Are there more values than float | None? I think this would correspond to int | bool too, right? What about str? I know our native metrics can output labels too.
Contributor
Author
There was a problem hiding this comment.
Great catch. Modeled it after pass@k which operates on floats only. Fixed this
Add `AgentEvalSummary.task_metric_values`: the ordered per-attempt values for each task, keyed `<metric_type>.<output>`, persisted into `summary.json`. Answering "which tasks were flaky, and on which attempt?" previously meant regrouping the flat task x trial x metric score list by hand. Rebuild pass@k on top of that mapping instead of rescanning the scores, so the per-attempt view and the published pass@k figures cannot disagree. pass@k means are unchanged; a task that produced no trial at all now surfaces in `nan_count` rather than silently shrinking the denominator. Retention follows the declared output schema: continuous, discrete and boolean values are kept, while labels and free models (token measurements) stay out even when their emitted value happens to be numeric. Rework the gym `inspect_results.py` example to read the summary directly rather than re-deriving per-task outcomes from `scores.jsonl`. Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
ngoncharenko
force-pushed
the
ngoncharenko/aalgo-310-passatk-harbor-runner
branch
from
August 13, 2026 04:00
3eebc8b to
0fdd09e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nemo_evaluator_sdk/tests/agent_eval/test_sandbox_compose_provider_live.py (1)
255-261: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMake the
busybox:latestprerequisite explicit.The live test has no build or preload step for
busybox:latest. Withpull_policy="never", it fails before topology validation. Preload the image in the live-test setup or add an explicit CI setup step.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_sandbox_compose_provider_live.py` around lines 255 - 261, Update the live-test setup around DockerComposeSandboxProvider to explicitly ensure the busybox:latest image is available before constructing or starting the provider, either by preloading it in the test setup or by adding the required CI setup step. Preserve pull_policy="never" and the existing topology validation flow.
🧹 Nitpick comments (1)
packages/nemo_evaluator_sdk/tests/agent_eval/test_task_metric_values.py (1)
267-299: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd infinity coverage to the escape tests.
_SPECIAL_FLOAT_TOKENS_MAPalso escapesInfinityand-Infinity, andserialize_nanemits them for any non-finite float. OnlyNaNis exercised here, so a regression in either token would pass. One extra round trip covers both.💚 Suggested addition
def test_a_payload_without_value_type_still_loads() -> None:def test_infinite_metric_values_round_trip_as_strings() -> None: for value, token in ((float("inf"), "Infinity"), (float("-inf"), "-Infinity")): dumped = TrialMetricValue(trial_id="t0", value=value).model_dump(mode="json") assert dumped == {"trial_id": "t0", "value_type": "number", "value": token} assert TrialMetricValue.model_validate(json.loads(json.dumps(dumped))).value == value🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_task_metric_values.py` around lines 267 - 299, Add a test alongside test_nan_metric_values_survive_json_as_a_string and test_a_label_and_a_real_nan_are_distinguishable_on_the_wire that round-trips both float("inf") and float("-inf") through TrialMetricValue.model_dump(mode="json"), strict JSON serialization, and model_validate. Assert they serialize as "Infinity" and "-Infinity" with value_type "number", then verify the restored values equal their original infinities.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nemo_evaluator_sdk/examples/gym/inspect_results.py`:
- Around line 103-105: Update the per-task metric aggregation in show_per_task
so tasks missing the selected key in summary.task_metric_values are retained
with an empty list instead of being filtered out; preserve existing values for
present keys and add coverage for omitted or failed metrics.
---
Outside diff comments:
In
`@packages/nemo_evaluator_sdk/tests/agent_eval/test_sandbox_compose_provider_live.py`:
- Around line 255-261: Update the live-test setup around
DockerComposeSandboxProvider to explicitly ensure the busybox:latest image is
available before constructing or starting the provider, either by preloading it
in the test setup or by adding the required CI setup step. Preserve
pull_policy="never" and the existing topology validation flow.
---
Nitpick comments:
In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_task_metric_values.py`:
- Around line 267-299: Add a test alongside
test_nan_metric_values_survive_json_as_a_string and
test_a_label_and_a_real_nan_are_distinguishable_on_the_wire that round-trips
both float("inf") and float("-inf") through
TrialMetricValue.model_dump(mode="json"), strict JSON serialization, and
model_validate. Assert they serialize as "Infinity" and "-Infinity" with
value_type "number", then verify the restored values equal their original
infinities.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ff3df90b-246a-4919-a223-425283c05d5c
⛔ Files ignored due to path filters (2)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/results.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/scores.pyis excluded by!sdk/**
📒 Files selected for processing (12)
docs/evaluator/agent-eval/reading-results.mdxpackages/nemo_evaluator_sdk/examples/gym/README.mdpackages/nemo_evaluator_sdk/examples/gym/inspect_results.pypackages/nemo_evaluator_sdk/examples/harbor/run_harbor_example.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/results.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/scores.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_pass_at_k.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_persistence.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_result_display.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_sandbox_compose_provider_live.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_task_metric_values.py
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py
- docs/evaluator/agent-eval/reading-results.mdx
- packages/nemo_evaluator_sdk/tests/agent_eval/test_persistence.py
- packages/nemo_evaluator_sdk/examples/gym/README.md
- packages/nemo_evaluator_sdk/tests/agent_eval/test_pass_at_k.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/nemo_evaluator_sdk/tests/agent_eval/test_sandbox_compose_provider_live.py`:
- Around line 259-261: Update the sandbox compose provider test configuration
around startup_timeout_seconds to use pull_policy="missing" instead of "never",
ensuring busybox:latest is pulled automatically when unavailable while
preserving the existing timeout settings.
In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_task_metric_values.py`:
- Around line 602-611: Replace the repository-root namespace imports in
packages/nemo_evaluator_sdk/tests/agent_eval/test_task_metric_values.py:602-611
and :634-636 with file-path loading or a shared fixture that adds the gym
example directory to sys.path. Apply the same mechanism to import
BundleFormatError/load_bundle and per_task_outcomes/per_task_trial_values so
both tests resolve reliably during collection.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f1a7a7d1-48c9-47d6-b314-04c2b8013eb3
⛔ Files ignored due to path filters (2)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/results.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/scores.pyis excluded by!sdk/**
📒 Files selected for processing (12)
docs/evaluator/agent-eval/reading-results.mdxpackages/nemo_evaluator_sdk/examples/gym/README.mdpackages/nemo_evaluator_sdk/examples/gym/inspect_results.pypackages/nemo_evaluator_sdk/examples/harbor/run_harbor_example.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/results.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/scores.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_pass_at_k.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_persistence.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_result_display.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_sandbox_compose_provider_live.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_task_metric_values.py
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/nemo_evaluator_sdk/examples/gym/README.md
- packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py
- docs/evaluator/agent-eval/reading-results.mdx
- packages/nemo_evaluator_sdk/tests/agent_eval/test_pass_at_k.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_result_display.py
- packages/nemo_evaluator_sdk/examples/gym/inspect_results.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/results.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_persistence.py
- packages/nemo_evaluator_sdk/examples/harbor/run_harbor_example.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/scores.py
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
ngoncharenko
commented
Aug 13, 2026
|
|
||
| # Keep-alive containers ignore SIGTERM; use a 1s stop grace so teardown does not wait | ||
| # the provider default of 30s on every create/close cycle. | ||
| _SHUTDOWN_TIMEOUT_SECONDS = 1.0 |
Contributor
Author
There was a problem hiding this comment.
updated the test - was waiting for sandbox spin down for too long
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
3 participants
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.
Summary
AgentEvalSummary.task_metric_attempts— the ordered attempts per task and metric output — and persists it tosummary.json.trials.jsonl:Important notes
value: null= the trial died before scoring — an attempt that did not pass. A failed metric leaves no entry at all: unmeasured, not unsuccessful.Related Issue
reward_stats) and AALGO-428 (P3.2, exception rollup).How this change moves us closer to Harbor parity
Target shape:
AgentDatasetStats— Harbor's per-dataset aggregate, the thingsummary.jsonhas to be able to reproduce. Per-trial source isVerifierResult.rewards; it is populated byJobStats.incrementasreward_stats.setdefault(value, []).append(trial_result.trial_name).pass_at_ksummary.score("<metric>.<output>.pass@k").mean— pre-existingreward_statstask_metric_attempts— trial ids + raw floatsn_trialssummary.trial_countn_errorstrial_idwherevalue is Noneexception_statstrial_name; we grouped bytask_id.harbor_runtimealready stampstrial_nameontoAgentEvalTrial.id, so carryingtrial_idon each attempt makes the shapes the same, not merely similar:trial_name, nottask_id) and inner key type (rawfloat | int, not stringified). Pinned bytest_harbor_reward_stats_is_derivable_from_summary_task_metric_attempts.n_errorsneeds the trial id specifically — one dead trial contributes anullto every metric key, so a naive count over-counts by the number of metrics. Deduping requires identity.pass_at_kmatches in content but not shape (Harbor keys{k: float}, we emit named aggregates), andn_errorsis derivable but not implemented.Why attempts carry an id but no ordinal
Worth stating, since "shouldn't each attempt have an index?" is the obvious review question:
n_attemptsby discarding the loop index —job.py:416:for _ in range(self.config.n_attempts)— and runs the repeats concurrently (job.py:129). Each repeat gets a fresh randomtrial_name. There is no first attempt._pass_at_k(n, c, k)is a function of counts and never of position. An ordinal would imply an ordering the runs do not have.sorted(glob("*/result.json"))over a random suffix, i.e. arbitrary;trial_idis the only meaningful handle, which is why it is carried rather than parsed. For contrast, the experimentalist's_trial_attemptrecovers the ordinal by parsing the trial-name suffix, whichisdigit()-fails against Harbor's ShortUUID — so it returnsNoneon every real Harbor run (verified against a liven_attempts=2job:hello-world__4c3VrKY,hello-world__NXRG3pE).JobStats.n_retries) and its per-trialTrialResultcarries no retry field, so "did the retry do better?" is a gap in Harbor's model — not something the evaluator can synthesize. Flagging it in case it matters later.Follow-ups
reward_payload_from_resultstill walksresult.scoresand emits the legacy task-keyed, stringified shape; rewiring it onto the summary is now mechanical.exception_statsis the last gap. Withtrial_idpresent it's a join againsttrials.jsonl(already in the bundle), not a schema change. Recommend it first add a typederrortoAgentEvalTrial, replacing the untypedmetadata["exception_type"]convention.What to review
_task_metric_attemptsinagent_eval/results.py— the core. Docstring carries anin → outworked example; the two bullet lists map 1:1 onto the branches below them.value: null(counted inn); a failed metric is no entry (kept out ofn, so a judge timeout is never charged to the agent). Most likely thing to get wrong later.trial_id—persistence.pywrites withsort_keys=True(would reorder attempts lexicographically), and nothing enforces trial-id uniqueness, so a dict would collapse two attempts into one and silently drop pass@k'sn. Pinned bytest_duplicate_trial_ids_are_two_attempts_not_one.AgentEvalAttemptValueis frozen, andvaluehas no default — both guard the same class of silent corruption:persistwould save the rewrite.value:Nonemeans "the trial died" and pass@k counts it towardn, so a record that merely omits the key must not quietly become a failed attempt. Explicit"value": nullstill works.nan_countsemantics —test_a_task_that_produced_no_trial_is_unmeasured_and_counted_in_pass_at_k_nan. Previously such a task was absent from the denominator; now reported as missing coverage. Means unaffected. Note this is afrom_scorespath only — a full run still fails loudly on a trial-less task, and this PR deliberately does not relax that guard.trial.metadata["exception_type"]that only Harbor stamps, andexception_typealready means two opposite things in this codebase. Withtrial_idpresent it becomes a join againsttrials.jsonl.Changes
task_metric_attempts: dict[task_id, dict["<metric_type>.<output>", list[AgentEvalAttemptValue]]], whereAgentEvalAttemptValue = {trial_id, value}.values_by_task = [attempt_values(outputs[key]) ...]. Everything below it (measured,unmeasured,max_n,_pass_at_k) is byte-identical.tasks=Nonethere are no specs to filter on, so every numeric output observed is kept.pass@knan_countinstead of silently shrinking the denominator. Reachable only viaAgentEvalSummary.from_scorescalled directly with a task list wider than the scores; a full run cannot get here, because_score_trialsrefuses to score when a task produced no trial (test_run_rejects_tasks_without_trials).trial_idis Harbor'strial_name(_trial_from_harbor_resultstamps it), so Harbor's ownreward_statsis rebuildable from the summary alone. See the parity section above.NaNtoken and madesummary.jsonunparseable by strict readers. Serialized as"NaN", matchingMetricOutput; round-trips back to a float.(task_id, metric_type)once, replacing a per-task/per-key rescan.inspect_results.pyreads the summary directly;per_task_outcomeskeeps its bare-value shape, newper_task_attemptsexposes the records. It now rejects a bundle predating the field rather than loading it and silently showing no per-task section.make vendor, pinned byte-exact by a test.Type of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowTargeted validation:
pytest packages/nemo_evaluator_sdk/tests(live/docker deselected)pytest packages/nemo_evaluator_sdk/tests/agent_eval(live/docker deselected)pytest .../test_task_metric_attempts.pypytest plugins/nemo-evaluator/tests plugins/nemo-optimization/testsuv run ruff check packages/nemo_evaluator_sdkuv run ruff format --check packages/nemo_evaluator_sdkuv run --frozen ty check <changed files>make vendorpersist_run→inspect_resultssummary.json; per-task output unchangedpass@k invariance — a golden
{name: (mean, count, nan_count)}table was captured from the pre-change implementation before the producer was touched, over a fixture covering every branch (always-passes, dead trial, metric-raised, two never-measured). Post-change it compares identical, and is pinned bytest_pass_at_k_aggregates_are_unchanged_by_carrying_trial_ids.Not passing / not run:
uv-lockpre-commit hook: needs uv0.9.14to match CI; local toolchain is0.9.30. Environment mismatch, not from this PR — nopyproject.tomlis touched, anduv-lock-checkpasses.test_harbor_runtime_e2e.py,test_codex_runtime_live.py,test_sandbox_docker_provider_live.py,test_sandbox_compose_provider_live.pyneed Docker and provider credentials.Summary by CodeRabbit
New Features
Bug Fixes
Documentation