fix(evaluator-sdk): make Harbor's per-trial resume reachable with agent_dir set

[ngoncharenko]

Fixes AALGO-430. Refs AALGO-427.

Targets ngoncharenko/aalgo-312-wire-eval-into-optimizer, which this branch builds on.

The bug

scoped_harbor_agent_import minted the synthetic package name with a fresh uuid4() on every call. That string becomes AgentConfig.import_path and so part of Harbor's JobConfig, which Harbor compares field-by-field before resuming a job directory. The comparison therefore failed on every rerun, Harbor raised FileExistsError instead of resuming, and its per-trial resume was unreachable for any caller that sets agent_dir — which is every Experimentalist run.

AALGO-427 had papered over this by discarding the job directory on every cache miss (force_rerun whenever agent_dir was set). That removed the crash by never resuming.

Example

Run 1
Agent unchanged. UUID happens to be deadbeef:

import_path = "_nemo_evaluator_harbor_agents.agent_deadbeef.harbor_wrapper:WrappedAgent"

Harbor writes jobs/resume-job/config.json with that path, finishes trial 1 of 2, then the process exits mid-run (or you only asked for 1 attempt and later bump to 2).

Run 2 - same agent files, same job dir
New UUID cafef00d:

import_path = "_nemo_evaluator_harbor_agents.agent_cafef00d.harbor_wrapper:WrappedAgent"

Harbor compares the new JobConfig to the saved one:

saved:  ...agent_deadbeef...
new:    ...agent_cafef00d...

→ configs differ → Harbor raises FileExistsError: ... resumed with a different config
→ cannot resume; completed Docker work is stranded.

Nothing about the agent changed. Only the random suffix did.

After the fix
Suffix = content digest of agent_dir:

both runs, identical agent files:

_nemo_evaluator_harbor_agents.agent_3f8a91c2b4e1.harbor_wrapper:WrappedAgent

Same contents → same path → Harbor’s equality check passes → missing trials resume.

Edit the agent (wrapper.py changes) → digest changes → path changes → Harbor correctly refuses resume (stale results for a different agent).

Why UUID was there originally?
UUID avoided collisions if two different agent folders named agent were imported in one process. That isolation goal was valid; the bug was making the name part of a persisted config that must be stable across reruns. Content-addressing keeps isolation and stability.

Before the fix, Experimentalist always set agent_dir, so every eval with a custom agent hit this path and could never use Harbor’s per-trial resume.

The fix

Content-addressed import path. The suffix now comes from a content digest of agent_dir, so an unchanged agent produces a stable JobConfig and an edited one invalidates the job dir on Harbor's own terms. Identical contents share a package name, so the sys.modules injection is refcounted and torn down on the last scope exit. The digest takes the same jobs_dir exclusion _cache_stamp uses, so a jobs_dir nested under agent_dir can't shift the import path as results accumulate.

Handling Harbor's refusal. Removing the unconditional discard exposed a second problem: the SDK cache stamp is deliberately looser than Harbor's JobConfig.__eq__. quiet, n_concurrent_trials and the task_names filter change the JobConfig without changing the results, and keying on them would discard every completed candidate when an experiment resumes on a box with a different core count (the Experimentalist defaults n_concurrent_trials to os.cpu_count()). So a directory that passes the stamp can still be refused — and Harbor refuses by raising.

_build_native_job now identifies that refusal positively — bare message, no errno, naming this job dir — discards, re-runs, and logs which field forced it (n_concurrent_trials: 10 -> 4). Any other FileExistsError propagates untouched rather than costing a job directory.

Why not align the config instead

Mirroring Harbor's compared set would fix the crash at the source but regress two intentional behaviours: a core-count change would discard completed candidates, and keying on task_names would break subset-of-a-cached-job hits that _stamp_coverage exists to support. The stamp answers "did these inputs produce these results?"; Harbor answers "can I resume this directory?" — different questions, and the SDK's is the right one to drive a cache. The divergence is now documented and guarded rather than removed.

Tests

• Real-Docker e2e for the resume this change exists to enable (restoring uuid4 makes it fail with the original FileExistsError)
• Refusal predicate pinned against both real Harbor messages, an OS-level EEXIST, and a refusal naming a different job dir
• Regression: a job dir that exists plus an unrelated mid-run FileExistsError must propagate and leave the directory intact
• Drift guard: every JobConfig field must be classified as Harbor-ignored, never-set-by-the-SDK, stamp-covered, or knowingly-unkeyed-with-a-reason — a Harbor upgrade adding a compared field fails the build
• Wording pin so a Harbor reword of its refusal surfaces at upgrade time
• Import-path stability, change detection, agent isolation, overlapping-scope survival, refcount teardown

All guards mutation-checked: each fix reverted individually makes the intended test fail.

Verification

uv run --frozen pytest packages/nemo_evaluator_sdk/tests/agent_eval -q   # 319 passed, 1 skipped (Docker e2e included)
uv run --frozen pytest plugins/nemo-experimentalist/tests -q             # 611 passed, 2 skipped
uv run ruff check packages/nemo_evaluator_sdk                            # clean
uv run --frozen ty check packages/nemo_evaluator_sdk                     # 122 diagnostics, unchanged from base

The vendored SDK is a separate commit, regenerated with make vendor — import-path rewrites only.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved Harbor job resumption when custom agent directories are used.
  • Preserves completed work instead of unnecessarily discarding cached jobs.
  • Automatically retries jobs when Harbor explicitly refuses to resume, while preserving unrelated errors and valid job data.
  • Improved handling of agent changes and partial job recovery.

• Tests

  • Added comprehensive coverage for caching, resumption, import isolation, error handling, and end-to-end recovery scenarios.

[ngoncharenko]

@claude review the PR

[ngoncharenko]

@coderabbitai review the PR

[coderabbitai]

@ngoncharenko Reviewing PR #964.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

📝 Walkthrough

Walkthrough

Harbor native jobs now preserve reusable directories, retry only Harbor-specific resume refusals, and use deterministic content-addressed agent imports with refcounted teardown. Unit tests cover cache, import, error, and config-drift behavior; an end-to-end test validates resuming a partial custom-agent job.

Changes

Harbor resume behavior

Layer / File(s)	Summary
Content-addressed scoped agent imports packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py, packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py	Synthetic package names derive from agent contents, configured exclusions prevent jobs_dir changes from affecting identity, and overlapping import scopes use refcounted teardown.
Native cache and resume control flow packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py, packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py	Native jobs discard stale inputs, allow Harbor to resume under-covered jobs, and delete and retry only for recognized Harbor resume refusals. Tests cover unrelated FileExistsError, config drift, and cache-stamp field coverage.
End-to-end partial-job validation packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime_e2e.py	A custom agent test removes a trial result and verifies that a subsequent evaluation resumes and completes with reward 1.0.

Sequence Diagram(s)

sequenceDiagram
  participant HarborAgentTaskRunner
  participant HarborJob
  participant job_dir
  HarborAgentTaskRunner->>HarborJob: create or run native job
  HarborJob-->>HarborAgentTaskRunner: FileExistsError
  HarborAgentTaskRunner->>HarborAgentTaskRunner: identify Harbor resume refusal
  HarborAgentTaskRunner->>job_dir: delete refused directory
  HarborAgentTaskRunner->>HarborJob: rerun native job

Loading

Possibly related PRs

• NVIDIA-NeMo/nemo-platform#544: Introduced the Harbor runtime that this change extends.
• NVIDIA-NeMo/nemo-platform#620: Added related native Harbor resume and scoped-import behavior.
• NVIDIA-NeMo/nemo-platform#955: Added related Harbor task-runner cache and scoped-import tests.

Suggested reviewers: arpitsardhana, sandychapman

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: enabling Harbor per-trial resume when agent_dir is set.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ngoncharenko/aalgo-427-setup-cache

---

_{Comment @coderabbitai help to get the list of available commands.}

[SandyChapman]

lgtm