Add first-class Harbor integration for agent-eval campaigns#5029
Add first-class Harbor integration for agent-eval campaigns#5029htahir1 wants to merge 23 commits into
Conversation
Promote the sandbox_harbor example into zenml.integrations.harbor: typed campaign models with stable trial identities, deterministic single-cell shard packing (trials_per_step knob), a shard-result materializer that archives Harbor's job tree with queryable metadata and a markdown visualization, reusable build/run/report steps, and the ZenMLSandboxEnvironment bridge relocated from the example. The example becomes a thin consumer of the integration. Sharding maps each mapped step onto one single-cell Harbor job, so step-level caching reruns only failed or new shards of a campaign and a failure invalidates the narrowest possible slice. Harbor stays optional everywhere: registry-facing modules never import it, requirements are gated to Python >= 3.12, and the shard step pins its materializer explicitly so a transitive-dependency version drift cannot silently degrade artifacts to the generic pydantic fallback. Verified end-to-end on Modal sandboxes (local and remote tracking servers), plus 64 unit tests including a Harbor 0.8 API canary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Documentation Link Check Results✅ Absolute links check passed |
Rename examples/sandbox_harbor to examples/harbor_agent_evals and turn the default invocation into a hermetic head-to-head: oracle (reference solution, reward 1.0) vs nop (does nothing, reward 0.0), 3 trials per cell packed 2 per shard — so one command demonstrates matrix expansion, shard packing, and a report with contrasting rows, still with no LLM keys required. After the run the example prints its own receipts: the campaign report, the losing shards found via the logged harbor.* step metadata, and one shard's Harbor job tree restored from the artifact store. Rerunning the same command demonstrates per-shard caching (completed shards cache-hit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
On CI legs without Harbor installed, pytest.importorskip("harbor")
passed anyway: with no __init__.py, the test directory itself satisfies
a bare `import harbor` as a namespace package (real distributions win
when installed, which is why this never reproduced locally), and the
gated modules then crashed on `harbor.job`. Skip on a concrete
submodule a namespace package can never provide.
Also point the materializer table at the GitHub source instead of the
sdkdocs page that only exists after the next release.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bandit (B108) flags hardcoded temp paths at medium severity, failing the security check. The strings were inert fixture placeholders, so plain non-tmp values work just as well. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Shards resolve datasets client-side, so the Harbor job only registers metrics under its adhoc bucket. Forwarding the dataset name as the task's `source` made Harbor 0.8's quiet-mode progress hook look up the missing dataset bucket and crash with an IndexError on every scored registry trial. Keep the provenance on the shard spec instead and restore it onto trial results after the run. Verified against the live registry: terminal-bench-sample@2.0, 3 tasks under the oracle agent on Modal, all rewards 1.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Allow dataset:NAME@VERSION:N on the CLI to slice a registry dataset to its first N tasks, so a first benchmark run does not fan out the full task set. Replace the hypothetical registry section in the README with the verified terminal-bench-sample run and its actual report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
15b0144 to
a8ff442
Compare
Both learned by running the branch against live benchmarks: provider keys reach the sandbox from the host environment (with the model on the agent spec, and cost/tokens flowing into results, metadata and the report), and fail_on_trial_error rescues the shard result plus log archive as a named manual artifact before failing the step — while without the flag an errored shard completes and gets cached. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A dedicated page under the Sandboxes component guide covering the campaign pipeline shape, shard semantics (retry/cache unit, the errored-shard caching caveat and its rescue artifact), artifacts and queryable metadata, real-LLM-agent key handling, and the environment bridge with its documented limitations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The component guide is for stack component flavors; evaluating agents with Harbor is a user workflow and sits naturally alongside the other standalone agents-guide pages. Keep a cross-link from the Sandboxes component page. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Positions the improvement loop Harbor unlocks on top of existing ZenML primitives: regression gates joined by stable trial identities, SFT dataset construction from reward-filtered trajectories in the archived job trees, rejection-sampling loops using trials_per_cell as a scored rollout generator, config/prompt sweeps keyed by identity-distinct agent configurations, and eval-gated promotion via the Model Control Plane. Every stage is a pipeline consuming campaign artifacts — no new framework surface required. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The evals-and-training story is a workflow of its own, not an appendix to the agents-guide tour: user-guide/agent-evals-guide/ now carries an overview (why campaigns are an operations problem), Run an eval campaign, Benchmark operations (delta reruns, error gating, cross-run joins, regression gates), and From evals to training. The agents guide returns to its original chapters; the sandboxes component page links to the new guide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The user guide, component guide and concepts docs publish as separate GitBook spaces, so relative links between them break on the published site. Cross-space references now use docs.zenml.io URLs that resolve today; the sandboxes page names the not-yet-published guide without a link to keep the absolute-link check green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
B1 ran on branch spike/b1-harbor-k8s (off open PR #5029, pushed, deliberately unmerged — the docs are the deliverable). Hermetic Harbor campaigns pass on the K8s sandbox flavor untouched; the docker_image wall was proven hollow (KubernetesSandboxSettings.image already exists, spike patch boots real Terminal-Bench images on EKS at oracle 1.000). FINDINGS.md Theme 5 extended with the second ecosystem test; wrap-vs- plugin assessment for Hamza lives on the spike branch.
|
cc @htahir1 Ran this integration on the Kubernetes sandbox flavor (staging EKS) as part of the RL-spike follow-ups — validation so far was Modal-only, so here's the second-flavor datapoint. Full writeup: The good news: everything behind the sandbox abstraction reproduces untouched. Hermetic oracle-vs-nop: 6/6 trials, oracle 1.000 / nop 0.000, ~30s wall clock; identical rerun cache-hits all shards in 8s with zero pods; metadata, report, and The Two small review-relevant observations from the failure path:
|
The Harbor->RL trajectory exporter (branch spike/b3-exporter, merges nowhere) answered the eval->training lineage question: expressible and demonstrated on B1's staging campaigns, but List[ArtifactVersionResponse] step inputs and .map() over historical artifacts are both unsupported, the #5029 trial_identity fragments across task-specification routes, and no layer records the resolved sandbox image.
|
Follow-up from the same RL-spike track (see K8s comment above): I built the "From evals to training" story this PR's docs promise — a trajectory exporter that turns campaign shard artifacts into an accumulating RL-episode-candidates dataset artifact, run against real campaigns. Full write-up:
|
Addresses strickvl's review findings on the Harbor integration (PR #5029), from the K8s and trajectory-exporter spikes: - trial_identity now hashes the task's canonical pin coordinates (TaskRef.to_string()) instead of the full ref dump, so dataset-resolved and direct git+ refs to the byte-identical pinned task get the same identity. Shipped identities become baselines people join against, so this lands before merge. - Surface Harbor's task_checksum on HarborTrialResult as the content-addressed complement to the coordinate-based identity. - Record sandbox provenance (flavor + resolved docker_image override) per trial. The environment bridge is the only party that knows which image actually backed a trial; reconstructing it from stack config is provably wrong for docker_image-pinned tasks. - Carry the original start() failure into the errors Harbor's cleanup path triggers on a never-started environment, so the root cause is no longer buried under "used before start()" noise. - Stop double-counting errored trials: Harbor's n_completed includes errored trials, so reports and metadata now use n_succeeded (harbor.n_succeeded), and an all-errored shard logs harbor.mean_reward=0.0 so reward-threshold regression gates fail safe. The docker_image -> KubernetesSandboxSettings translation from the same review is deliberately left as a fast-follow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Both landed in 7d3a3bd: Also from your K8s comment: The |
Fixes the confirmed findings from an adversarial review of the previous commit: - TaskRef.to_string() returned "" for registry package tasks (name/ref only, no path or git URL), collapsing every such task into one trial_identity and shard_id. Package refs now render as NAME@REF, and trial_identity refuses refs with no coordinates at all. - The harbor.mean_reward=0.0 sentinel was gated on the wrong condition: it fired on healthy-but-unscored shards (Harbor's verifier rewards are optional) and was bypassed by partially-errored shards whose survivor-only mean passed reward gates. It now fires only when every trial errored, and every shard logs harbor.error_rate so gates can pair reward thresholds with an error check. Docs updated to match. - A failed start() now discards this session's provenance entry, so a retried trial that never opened a sandbox can't inherit an earlier attempt's image; the shard runner clears the whole registry after stamping, so verifier-session entries no longer accumulate. - start() cancellation (Harbor's build timeout, a BaseException) is now recorded as the root cause, and a successful restart clears the stale failure note. - The materializer's artifact metadata and visualization now use n_succeeded, matching the report and step metadata. - The job-runner test seeds provenance through the public functions instead of the private registry dict. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cleanup pass over the two review-fix commits (no behavior change beyond one additive artifact-metadata key): - Replace the pop-per-trial + clear-afterwards provenance API with one atomic drain_session_provenance(): the shard runner drains right after job.run(), so the registry is reset even if result processing raises (the old ordering leaked entries when from_harbor raised). - Promote error_rate to a HarborShardResult property next to n_succeeded/mean_reward and reuse it in step metadata; the materializer now emits it too, keeping artifact metadata in step parity. - Collapse the twin start() except arms into one, store the exception object instead of a flattened string, and chain it into the _live_session RuntimeError (raise ... from) so Harbor's cleanup noise carries the full cause, not just a summary line. - Deduplicate the n_succeeded/registry/sentinel rationale comments to their one canonical home each. - Test hygiene: shared _patch_active_sandbox/_modal_sandbox helpers and an autouse registry-drain fixture in the environment tests; the steps fixture now errors trials from the end so the partial-error test uses it directly instead of hand-rebuilding trials. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pydoclint (DOC503) correctly flagged that the docstrings drifted when the start-failure bookkeeping was restructured: start() now re-raises CancelledError/Exception from its except arm, and the log-dir teardown re-raise lives in _start(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
harbor 0.8 requires datasets>=4.4.1 while the huggingface integration pins datasets>=2.16.0,<4.0.0, so installing every integration into one environment has no resolution — the >=3.12 lint/test jobs failed at Setup environment. Skip harbor there like tensorflow/deepchecks; its unit tests are importorskip-gated and keep running in environments that have Harbor installed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses the confirmed findings of a follow-up adversarial review: - The session-provenance registry moves from a module global to a ContextVar scope (session_provenance_scope). Dynamic mapped shard steps run concurrently as threads in one process (DagRunner's startup executor), so a global registry let one shard drain or reset another shard's entries; the scope rides the ambient context into Harbor's asyncio tasks and isolates per step thread. - A trial cancelled mid-create_session (Harbor's environment build timeout) abandoned the worker thread and leaked whatever sandbox it eventually created. Creation now runs on a dedicated executor whose concurrent future gets a reap callback on cancellation — the session is destroyed in the worker thread the moment it materializes. (The asyncio run_in_executor wrapper can't do this: it reports itself cancelled while the worker keeps running.) - Restore Harbor-backed CI coverage lost when harbor was excluded from the omnibus environment: a dedicated harbor-unit-test job in ci-fast installs zenml without integrations plus harbor on Python 3.12 and runs the gated tests, including the Harbor API canary. - TaskRef.parse now rejects git pins that are not commit SHAs (branches, tags, empty pins) — mutable pins move underneath shard caching and trial-identity baselines. Docs updated to match. - Sentinel docs/docstrings tightened: the 0.0 mean-reward sentinel applies to all-errored shards with no scored trials; errored-but- scored trials keep their real mean and are caught by the error-rate gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@strickvl Can you take a look? |
Follow-up fixes for the PR #5029 review; each item targets a specific concern raised there. W1 (agent_env secrets persisted in cleartext): agent_env is serialized verbatim into the harbor_shards artifact and every shard result, so a raw provider key placed there leaks into the artifact store. Document this explicitly in the campaign docs (use the ambient sandbox env or ZenML secrets, never raw values) and log a warning from build_harbor_matrix when an agent_env key looks credential-shaped. The value is never scrubbed — silent mutation would be worse than a loud warning, since dropping a var the agent needs fails opaquely. W2 (temp job dir leak on long-lived processes): the materializer now prunes the local zenml-harbor- temp job tree once it has archived it into the artifact store (the durable copy). Deletion is bounded to a direct child of the system temp dir carrying the zenml-harbor- prefix, so a user-supplied job_dir is never removed. This must live in the materializer, not the step's finally: output materialization runs after the step function returns. W3 (global debug verbosity in CI): no change needed. This branch already scopes ZENML_DEBUG to the harbor-unit-test job's own env block and never added ZENML_LOGGING_VERBOSITY=debug to install-zenml-dev.sh. W4 (trial to identity pairing by order): expand the comment to state that shard trials are exchangeable replicas of one (task, agent, model) cell and the index is not a stable per-attempt identity. When Harbor returns fewer results than identities, the missing trials are now recorded as errored (counted in both n_completed and n_errored, matching Harbor's convention that an errored trial is a finished one) so a fail_on_trial_error gate trips on them instead of letting them vanish. Also strengthen the upstream-API canary to assert the protected members the bridge relies on (_merge_env, _validate_definition) and document the harbor/huggingface co-resolution incompatibility (datasets>=4.4.1 vs <4) in the integration docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Describe changes
Promotes the Harbor example into a first-class
zenml.integrations.harborintegration for running Harbor agent-eval campaigns as ZenML pipelines. ZenML owns the outer orchestration (matrix expansion, per-shard steps, retries, caching, artifacts, reports); Harbor keeps the trial eval kernel (task loading, agent loop, verifier, reward). Trials execute inside the active stack's Sandbox component via theZenMLSandboxEnvironmentbridge (relocated from the example, behavior-identical).Integration (
src/zenml/integrations/harbor/):models.py/utils.py(harbor-free): typed campaign contracts, stable sha256 trial identities (task + agent + model + kwargs + env + index — the cross-run baseline join key), deterministic single-cell shard packing with atrials_per_stepknob.job_runner.py: one single-cell HarborJobConfigper shard, run viaawait Job.create(...)/job.run(), per-trial results from the in-memoryJobResult.trial_results. Datasetsourceis deliberately kept off the Harbor-side config (fixes an IndexError in Harbor 0.8's quiet-mode display for client-side-resolved registry datasets) and restored onto results afterward.materializers/:HarborShardResultMaterializerpersists the flat summary plus Harbor's full job tree (agent/verifier logs, trajectories) as a tar archive; queryable metadata, markdown visualization, on-demanddownload_jobs_dir(). Pinned on the step viaoutput_materializersso transitive-dependency drift can't silently degrade to the pydantic fallback.steps/:build_harbor_matrix→run_harbor_shard(mapped; logsharbor.*step metadata; optionalfail_on_trial_errorrescues the shard result + log archive asharbor_shard_result_<id>_failedbefore failing) →build_harbor_report.Example (
examples/harbor_agent_evals/, renamed fromsandbox_harbor): default is a hermetic oracle-vs-nop head-to-head (no keys) that prints its own receipts — report, metadata scan, restored log tree;dataset:NAME@VERSION[:N]CLI slicing for registry benchmarks.Docs: new top-level Agent evals guide (
user-guide/agent-evals-guide/): overview, Run an eval campaign, Benchmark operations (delta reruns, error gating, cross-run identity joins, regression gates), From evals to training (SFT from winning trajectories, rejection sampling, config sweeps, eval-gated promotion). Plus the materializers table row and a sandboxes-page cross-reference. Cross-space references use absolute docs.zenml.io URLs.Validation (all live, on Modal sandboxes)
terminal-bench-sample@2.0— chess-best-move, polyglot-c-py, sqlite-with-gcov, all reward 1.000 under oracle, incl. per-taskdocker_imageoverrides.fail_on_trial_error: live rescue artifact verified (archive restorable,exception.txt+job.logpresent).claude-code+claude-sonnet-4-5on chess-best-move — clean run, reward 0.000 (a genuine benchmark result), $0.1193 tracked through trial → metadata → report.harbor.job— namespace-package-proof).Areas needing reviewer attention
get_requirements()returns[]below (quiet on the interpreter fallback),check_installation()is version-guarded._merge_env,_validate_definitionoverride) — Harbor's intended subclass hooks.tests/unit/integrations/harbor/deliberately has NO__init__.py(would shadow the realharborpackage); see the conftest guard.harborships nopy.typed→ignore_missing_imports+disallow_subclassing_any=falsescoped to the environment module./stackslinks to/stacks/sandboxesand link the guide from the sandboxes page once published.Non-goals / extension points
No Harbor Hub upload, no built-in baseline store, no multi-step tasks or separate verifier sandboxes (rejected with
NotImplementedError), no Sandbox core changes. Known bridge limitations (no resource translation, noexec(user=...),allow_internet=falserefused,docker_imageoverride Modal-only) preserved and documented. Trajectory→training-dataset utilities are the natural follow-up (the archives already carry rollouts + rewards; see the From evals to training guide).Related: #5030 (remote image builders re-apply
.dockerignoreand drop generated context files), found while validating the Modal-orchestrator path.Pre-requisites
developand the open PR is targetingdevelop.Types of changes