[Harbor 3/4] Mode B (nested harbor run) + the vero harbor build compiler#5
[Harbor 3/4] Mode B (nested harbor run) + the vero harbor build compiler#5varunursekar wants to merge 5 commits into
Conversation
| # Execute | ||
| # ------------------------------------------------------------------ | ||
|
|
||
| def _build_command( |
There was a problem hiding this comment.
HarborConfig.n_attempts / max_retries are typed fields, which signals they are honored, but _build_command never emits flags for them — so every nested run uses harbor defaults. On a budget-graded run that is a real cost/correctness bug. Either map them to the real harbor run flags here, or drop the fields. (Confirms Greptile's P1.)
| continue | ||
| task_name = data.get("task_name") | ||
| if task_name: | ||
| trials[task_name] = data |
There was a problem hiding this comment.
trials[task_name] = data is last-write-wins over an unordered rglob; with retries or a reused jobs_dir, a failing attempt can clobber a passing one, feeding a wrong score into auto_best. Collect all matches and pick deterministically (latest mtime / highest attempt index), and/or namespace jobs_dir per attempt. (Confirms Greptile's P1.)
| values = [float(v) for v in rewards.values()] | ||
| return sum(values) / len(values) if values else 0.0 | ||
|
|
||
| def _existing(self, params: EvaluationParameters, sample_id: int) -> SampleResult | None: |
There was a problem hiding this comment.
_existing treats a persisted error sample as 'done', so a sample whose nested run failed once is permanently skipped on resume and silently degrades the candidate's score. Either don't persist error samples, or make the pending/skip predicate treat error results as not-done.
| ["tar", "xf", "-", "--strip-components", str(strip)], | ||
| cwd=dest, stdin=archive.stdout, check=True, | ||
| ) | ||
| archive.wait() |
There was a problem hiding this comment.
archive.wait()'s return code is discarded; if git archive fails, tar can still exit 0 on a truncated stream and you commit a near-empty baseline whose SHA gets baked into both images. Assert archive.wait() == 0 (and reap the Popen on the tar-failure path). (Confirms Greptile.)
| environment: | ||
| VERO_HOME_DIR: "/opt/vero_home" | ||
| {% for secret in secrets %} | ||
| {{ secret }}: "${{ '{' }}{{ secret }}{{ '}' }}" |
There was a problem hiding this comment.
This renders SECRET: "${SECRET}", resolved from the run host's env, not anything the compiler validates. An unset var interpolates to empty and the sidecar comes up credential-less, failing every eval opaquely. Consider failing the build / emitting a required-secrets manifest when a declared secret is missing. Note test_rendered_files_parse only asserts the literal ${...} string, which hides this.
| # ...except locked paths (e.g. the scorer): root-owned + unwritable. | ||
| if [ -e "/work/agent/{{ p }}" ]; then | ||
| chown -R root:root "/work/agent/{{ p }}" | ||
| chmod -R a-w "/work/agent/{{ p }}" |
There was a problem hiding this comment.
This chown root:root + chmod -R a-w applies to the working tree under /work/agent, but the verifier never executes the working tree: _transfer_commit fetches git blobs and the evaluator checks the commit out into a fresh temp copy. The agent can git add -f a modified scorer and commit it regardless of working-tree perms. So read_only_paths is not a tamper control — please document it as advisory-only and move scorer-provenance enforcement into the sidecar (see the Mode A scorer-provenance comment on #4).
- docs/harbor/architecture.md — what the integration is, the compiled-task topology, the two evaluation modes, the component map, and the leaderboard-integrity model. - docs/harbor/tutorial.md — build and run an optimization task end to end (both modes, the agent-side protocol), and a Harbor section in the README. - examples/gaia-optimization — a Mode-B example optimizing a GaiaAgent (a thin Terminus2 subclass with an editable prompt) on gaia/gaia via a nested harbor run on Modal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Mode B (runner.py): `HarborRunner`, an `EvalStrategy` that — for each candidate — runs a *nested* `harbor run` of the agent over the selected Harbor tasks (e.g. on Modal) and collates the verifier rewards into vero `SampleResult`s. One Harbor task = one sample; inference is fully delegated, scoring comes from Harbor's verifier. - The compiler (build/): `vero harbor build` renders a `BuildConfig` into a runnable Harbor task directory — a Docker Compose environment (optimizer workbench `main` + the eval sidecar + three volumes), two Dockerfiles, instruction.md, tests/test.sh, and the seed/solve scripts — baking the dataset/scorer/baseline repo and the sidecar's ServeConfig. Supports Mode A (local dataset/scorer) and Mode B (a registry or local Harbor benchmark, passed through to the HarborConfig). - `.gitignore`: un-ignore src/vero/harbor/build/ (the repo's `build/` rule was hiding the compiler package). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…xample, by-mode scorer Documentation accuracy fixes (review findings on PR #6): - architecture: soften the intro from a hard guarantee ("the optimizer cannot read hidden labels, modify the scorer, or bypass its budget") to best-effort, OS/process-level language describing what is actually enforced. - gaia build.yaml: correct "never reaches the optimizer". Because agent_repo is "." and build.yaml is git-tracked, the validation task ids ARE seeded into the optimizer's repo; only the per-sample scores are withheld. Acceptable for a public benchmark, with a caveat + mitigations for secret-identity benchmarks. - examples: gsm8k-agent is cited as the Mode A example but ships no build.yaml; repoint to gaia-optimization as the complete runnable example and pair gsm8k-agent with the tutorial's Mode A snippet. - architecture: document the current fail-open default for unlisted splits (and that it becomes fail-closed once the protocol fix lands), and split the "scorer is sidecar-only" claim by mode (true for Mode B; Mode A keeps the scorer in the agent's editable repo until the serve.py fix). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mkdtemp cleanup Mode B runner + build-compiler robustness (review findings on PR #5): - runner: emit --n-attempts / --max-retries (the typed HarborConfig fields were silently dropped); pick the best trial per task deterministically instead of last-write-wins over an unordered rglob; treat a persisted error sample as not-done so a transient failure is re-run on resume. - compiler: assert git archive exited 0 (and reap it) so a truncated stream cannot bake a near-empty baseline; validate declared secrets at build time and render compose secrets with a fail-fast guard so an unset host var fails loudly instead of producing a credential-less sidecar. - seed.sh: document that read_only_paths is advisory only and scorer provenance is enforced sidecar-side. - compiler: stage the dataset in a cleaned-up TemporaryDirectory (Greptile: the mkdtemp scratch dir was leaking, datasets can be gigabytes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
767570e to
38709b1
Compare
05cdeea to
44285bf
Compare
[Harbor 4/4] architecture docs, tutorial, and the GAIA example
Draft · Stack 3 of 4 — targets
harbor-2-sidecar.HarborRunner, anEvalStrategythat — per candidate — runs a nestedharbor runof the agent over the selected Harbor tasks (e.g. on Modal) and collates the verifier rewards into veroSampleResults. One Harbor task = one sample; inference is delegated, scoring comes from Harbor.vero harbor buildrenders aBuildConfiginto a runnable Harbor task — a Docker Compose env (optimizer workbench + eval sidecar + 3 volumes), two Dockerfiles,instruction.md,tests/test.sh, seed/solve scripts — baking the dataset/scorer/baseline repo and the sidecar'sServeConfig. Mode A and Mode B.Review focus: the nested-run + collation, and the trust-boundary plumbing baked into the generated compose (volumes,
root:600token, secrets → sidecar only). Also un-ignoressrc/vero/harbor/build/(the repo'sbuild/rule was hiding the package).Stack: [1/4] core → [2/4] sidecar → this → [4/4] docs.
🤖 Generated with Claude Code
Greptile Summary
This PR adds Mode B (nested
harbor run) as anEvalStrategyinrunner.pyand introduces thevero harbor buildcompiler (build/) that renders aBuildConfiginto a fully-wired Harbor task directory — Dockerfiles, docker-compose, seed/solve/test scripts, and a bakedServeConfig. The previous-thread fixes (best-trial selection,--n-attempts/--max-retriesflag emission) are implemented and tested inTestReviewFixes.HarborRunner(runner.py): per-candidate nestedharbor runthat filters already-done samples on resume, collates result JSONs using a deterministic best-trial ranking (clean > has-rewards >finished_at> mtime), and persistsSampleResults.compile_taskmaterialises the agent baseline viagit archive, registers the dataset into a bakedVERO_HOME, emits aServeConfig, and renders all task artifacts; secrets reach the sidecar container only via compose${VAR:?}guards, nevermain.root:600on a volume mounted read-only tomain, so the optimizer agent (agentuser) cannot read it;read_only_pathsadvisory chmod is documented as non-authoritative.Confidence Score: 4/5
Draft PR; do not merge until the git archive pipeline fixes from #9 are backported into this stack.
The compiler's _prepare_baseline_repo still carries the communicate()/stdout-close bug that breaks git-archive builds for the normal case — acknowledged in prior review threads as landing in #9. The test_baseline_archive_failure_raises test may not reach its expected RuntimeError until that fix lands. Both runner fixes (best-trial ranking, --n-attempts/--max-retries flag emission) are correctly implemented and tested in this PR.
vero/src/vero/harbor/build/compiler.py — the git archive pipeline (lines 89-111) needs the communicate()/stdout-close fix from #9 before the compiler is usable for git-tracked agent repos.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Agent as Optimizer Agent participant SC as eval-sidecar participant HR as HarborRunner participant IH as Inner Harbor participant VF as Verifier Agent->>SC: vero harbor eval --split train SC->>HR: produce_sample_results HR->>IH: harbor run -n N -i task1 -i task2 IH-->>HR: jobs_dir/ts/trial/result.json HR->>HR: _load_trials best rank per task HR->>SC: save SampleResult SC-->>Agent: aggregate score and budget Note over Agent,SC: optimizer iterates within budget VF->>SC: vero harbor finalize with admin token SC->>HR: produce_sample_results hidden test HR->>IH: harbor run test tasks IH-->>HR: result.json files HR->>SC: SampleResults SC-->>VF: reward.json final score%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Agent as Optimizer Agent participant SC as eval-sidecar participant HR as HarborRunner participant IH as Inner Harbor participant VF as Verifier Agent->>SC: vero harbor eval --split train SC->>HR: produce_sample_results HR->>IH: harbor run -n N -i task1 -i task2 IH-->>HR: jobs_dir/ts/trial/result.json HR->>HR: _load_trials best rank per task HR->>SC: save SampleResult SC-->>Agent: aggregate score and budget Note over Agent,SC: optimizer iterates within budget VF->>SC: vero harbor finalize with admin token SC->>HR: produce_sample_results hidden test HR->>IH: harbor run test tasks IH-->>HR: result.json files HR->>SC: SampleResults SC-->>VF: reward.json final scoreComments Outside Diff (2)
vero/src/vero/harbor/runner.py, line 739-752 (link)n_attemptsandmax_retriessilently dropped from harbor runHarborConfigexposesn_attemptsandmax_retriesas typed fields (notextra_argspass-throughs), signalling that callers expect to configure retry behavior. Neither field is translated to a CLI flag in_build_command, so everyharbor runinvocation uses whatever the harbor defaults are regardless of what the caller configured. A user who setsmax_retries=0to disable retries on a budget-sensitive run will get the default retry behavior and may blow their budget.Prompt To Fix With AI
vero/src/vero/harbor/runner.py, line 799-813 (link)task_namewhen harbor retries a taskrglob("result.json")visits all result files across all timestamp directories underjobs_dir. If harbor'sn_attemptsormax_retriescauses the same task to produce multiple trial entries with the sametask_name,trials[task_name] = datasimply overwrites with whichever filerglobhappens to yield last — filesystem iteration order is not defined. This can cause a passing retry to be silently replaced by a failing one (or vice versa), yielding an incorrect score for that sample.Prompt To Fix With AI
Reviews (4): Last reviewed commit: "Merge pull request #6 from scaleapi/harb..." | Re-trigger Greptile