Skip to content

feat(harbor): the agent's first baseline eval is budget-free#25

Merged
varunursekar merged 6 commits into
harbor-2-mode-b-sample-timeout-warningfrom
harbor-2-free-baseline-eval
Jul 17, 2026
Merged

feat(harbor): the agent's first baseline eval is budget-free#25
varunursekar merged 6 commits into
harbor-2-mode-b-sample-timeout-warningfrom
harbor-2-free-baseline-eval

Conversation

@shehabyasser-scale

@shehabyasser-scale shehabyasser-scale commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #20 (top of the sidecar chain: #11 -> #19 -> #20 -> this).

The problem, observed twice in live trials

The seeded baseline is the reference every candidate is implicitly compared to, yet it can never win selection (auto_best excludes base_commit, correctly, so "do nothing" cannot win). Metering its evaluation therefore forces the optimizer into a bad choice:

  1. Pay for it: the incident behind fix(harbor): floor rewards on no-candidate outcome instead of erroring #11/docs(harbor): warn in the task instruction that baseline evals create no candidate #12, an optimizer spent its whole budget measuring the baseline, left the candidate pool empty, and the trial died (now floored by fix(harbor): floor rewards on no-candidate outcome instead of erroring #11, warned against by docs(harbor): warn in the task instruction that baseline evals create no candidate #12).
  2. Skip it and fly blind: today's TB2 trial (exp5 v3). The optimizer, warned off baseline evals by docs(harbor): warn in the task instruction that baseline evals create no candidate #12's instruction, never learned the baseline scored 0.375. Its first edit scored 0.375, and with no reference it could not tell a no-op from an improvement; its second edit scored 0.125; it rationally gave up with 3 of its 5 budgeted evals unspent. Final reward: 0.375, exactly baseline.

#12 treated the symptom (don't waste budget); this treats the cause (the reference measurement should not cost budget).

The fix

EvaluationSidecar.evaluate: when the transferred commit resolves to base_commit and the free eval hasn't been used, run it through the engine's unmetered (admin) path while still routing results through the agent's real split tier. Capped at exactly one, so free compute is bounded; subsequent baseline evals debit normally. /status now reports base_commit and free_baseline_available, so agents can discover the mechanism.

Selection integrity is untouched: the baseline experiment is recorded as before and remains excluded from auto_best by the existing base_commit filter.

Tests

Seven new tests: first baseline eval unmetered but tier-routed; second one metered; non-baseline commits always metered; no-base_commit tasks never free; /status surfaces and flips the flag.

Follow-up (separate, compiler layer): update the #12 instruction paragraph to say the first baseline eval is free once this lands.

🤖 Generated with Claude Code

Greptile Summary

This PR addresses the root cause of two observed trial failures: the seeded baseline commit, which can never win selection, was being charged against the agent's eval budget. It adds a budget-free path for the agent's first baseline eval, a auto_best selection floor that reverts to the seed when no candidate beats it, retry logic for score_baseline, and surfaces baseline outcomes durably via the finalize response.

  • Free first baseline eval (server.py): when the transferred commit resolves to base_commit and the slot is unused, the eval is routed through the engine's admin (unmetered) path; the flag is consumed only after a successful eval, so transient failures leave the slot available for a free retry.
  • auto_best baseline floor (verifier.py): after rescoring, if the best candidate's admin score does not strictly exceed the baseline's admin score on the selection split, finalize reverts to base_commit.
  • Structured finalize response + retry (verifier.py, cli.py): finalize now returns {\"rewards\": …, \"baseline\": …}; _maybe_score_baseline is retried and its outcome surfaced durably.

Confidence Score: 5/5

Safe to merge — the free-baseline slot is consumed only after a successful engine call, admin calls leave the slot intact, and the auto_best floor reverts cleanly to the seed when no candidate improves on it.

The two issues flagged in the previous review round were addressed in 0874f11 and are directly pinned by new tests. The baseline floor is consistent with existing error-handling conventions, the CLI backward-compat shim handles mixed-version sidecars, and test coverage is thorough.

No files require special attention.

Important Files Changed

Filename Overview
vero/src/vero/harbor/server.py Adds free-first-baseline-eval logic with correct post-success flag assignment, admin-call guard, and status exposure.
vero/src/vero/harbor/verifier.py Adds auto_best baseline floor and retry+structured-return for _maybe_score_baseline; consistent with existing error-handling conventions.
vero/src/vero/harbor/cli.py Backward-compatible unwrapping of the new finalize envelope.
vero/tests/test_harbor_server.py Seven new tests pin all free-baseline invariants.
vero/tests/test_harbor_verifier.py Adds TestAutoBestBaselineFloor suite and updates existing callers to unwrap the new finalize envelope.

Reviews (3): Last reviewed commit: "Merge pull request #27 from scaleapi/har..." | Re-trigger Greptile

The seeded baseline is the reference every candidate is implicitly
compared to, yet it can never win selection (auto_best excludes
base_commit by design), so metering it forced the optimizer to choose
between flying blind and paying a budgeted eval for an unselectable
commit. Observed live twice: the incident behind #11/#12 (an optimizer
burned its whole budget measuring the baseline), and exp5's optimizer
which, warned off by #12, skipped the reference entirely, could not tell
its no-op edit (train 0.375) from an improvement, and quit with 3 of 5
evals unspent.

The first eval whose transferred commit resolves to base_commit now runs
unmetered (engine admin path) while results still route through the
agent's real split tier. Capped at one: later baseline evals debit
normally, so free compute is bounded. /status exposes base_commit and
whether the free reference eval is still available.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread vero/src/vero/harbor/server.py Outdated
Comment thread vero/tests/test_harbor_server.py
@shehabyasser-scale

Copy link
Copy Markdown
Collaborator Author

Nit found by the review panel for #26 while verifying this PR's semantics: _free_baseline_used is a plain in-memory bool, while the budget ledger is persisted to <admin_volume>/ledger.json precisely so a sidecar restart cannot refund budget. After a restart the freebie re-arms: a second baseline eval is free and status reports free_baseline_available: true again. Agent-favorable (bounded free compute, never misleads the agent into wasting budget, instruction and status stay mutually consistent), so not a blocker; a follow-up could persist the flag next to ledger.json.

Setting _free_baseline_used before the await burned the agent's one free
baseline on a transient engine failure, forcing the retry to be metered: the
exact pay-for-it failure the feature prevents. Move the flag write to after a
successful evaluate (safe in the single-threaded asyncio loop, no await between
the check and the write) so a failed baseline eval can be retried for free.

Adds two tests: an admin re-score of the base commit must not consume the free
slot, and a failed baseline eval must leave the slot available for a free retry.

Addresses Greptile P1 on this PR.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
shehabyasser-scale added a commit that referenced this pull request Jul 4, 2026
… constant

The compiler introspects StatusSummary for the free-baseline field name; that
string was an unnamed literal, the sole coupling to the field PR #25 adds on a
separate chain. Hoist it to _FREE_BASELINE_FIELD so the compiler<->protocol
contract has one documented source. A hard import-time assert cannot live here:
the field is legitimately absent on this branch's base until the sidecar PR
merges, which is why the render is introspection-gated in the first place.

Addresses Greptile P2 on this PR.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines +83 to +98
free_baseline = (
not admin
and self.base_commit is not None
and sha == self.base_commit
and not self._free_baseline_used
)
exp = await self.engine.evaluate(
replace(req, commit=sha), admin=admin or free_baseline
)
# Consume the free slot only after the eval actually succeeds. Setting it
# before the await would burn the one free baseline on a transient engine
# failure (timeout, infra), forcing the agent to pay for the retry, which is
# the exact failure mode this feature prevents. Safe in the single-threaded
# asyncio loop: no await runs between the check above and this write.
if free_baseline:
self._free_baseline_used = True

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Race window defeats the "capped at one" guarantee

The _free_baseline_used read at line 87 (inside free_baseline = (... and not self._free_baseline_used)) is separated from the write at line 98 by await self.engine.evaluate(...). If two concurrent evaluate coroutines arrive while none have completed yet, both will read False, both will set free_baseline = True, and both will be served as admin calls — violating the explicit "capped at exactly one" design goal.

The comment "no await runs between the check above and this write" is true about the if free_baseline: … assignment site alone, but that is not the relevant check. The check that controls the boolean is not self._free_baseline_used in the free_baseline tuple expression, and that read is separated from the write by a full await. In a single-threaded asyncio event loop two coroutines can both be suspended at await self.engine.evaluate(...) having already recorded free_baseline = True.

Fixing this requires setting the flag before the await (then restoring it on exception), or guarding it with a sentinel that is set synchronously before yielding — the mirror of the reasoning that justified moving the assignment after the await in the previous review.

Prompt To Fix With AI
This is a comment left during a code review.
Path: vero/src/vero/harbor/server.py
Line: 83-98

Comment:
**Race window defeats the "capped at one" guarantee**

The `_free_baseline_used` read at line 87 (inside `free_baseline = (... and not self._free_baseline_used)`) is separated from the write at line 98 by `await self.engine.evaluate(...)`. If two concurrent `evaluate` coroutines arrive while none have completed yet, both will read `False`, both will set `free_baseline = True`, and both will be served as admin calls — violating the explicit "capped at exactly one" design goal.

The comment "no await runs between the check above and this write" is true about the `if free_baseline: …` assignment site alone, but that is not the relevant check. The check that controls the boolean is `not self._free_baseline_used` in the `free_baseline` tuple expression, and that read is separated from the write by a full `await`. In a single-threaded asyncio event loop two coroutines can both be suspended at `await self.engine.evaluate(...)` having already recorded `free_baseline = True`.

Fixing this requires setting the flag **before** the await (then restoring it on exception), or guarding it with a sentinel that is set synchronously before yielding — the mirror of the reasoning that justified moving the assignment *after* the await in the previous review.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Cursor Fix in Claude Code Fix in Codex

shehabyasser-scale and others added 2 commits July 4, 2026 18:37
A live trial silently skipped baseline scoring: the nested baseline eval failed
transiently, the failure was swallowed by a warn-and-continue except, and the only
record (a log line) died with the sidecar container at teardown. The admin volume
that baseline.json was written to does not survive teardown either, so there was no
durable evidence of whether the baseline was scored, skipped, or crashed.

Two changes:
- Retry the baseline eval (default 2 attempts) so a single transient nested-run
  failure does not drop the regression check.
- finalize() now returns {"rewards": ..., "baseline": ...}; the baseline outcome
  (scores, a skip reason, or an error) is surfaced in the finalize response. The CLI
  writes only rewards to reward.json (the outer harness consumes its keys, unchanged)
  and echoes the full payload to stdout, which is captured into the trial's stdout on
  the host, the one channel that survives teardown. The CLI tolerates the old
  bare-rewards shape for a mixed-version sidecar.

Baseline scoring still never fails the trial.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s it

auto_best excludes base_commit from the candidate pool, so when every candidate
regressed it still selected the least-bad one and shipped a regression (observed
live: an opus optimizer on a weak haiku inner model produced only below-baseline
candidates and finalize shipped one 0.10 below the baseline, even though the free
baseline reference was available). Visibility alone did not prevent the harm;
nothing acted on it.

Add a selection floor: after the admin re-score picks the best candidate, admin-
score the untouched base_commit on the selection split and revert to it when the
best candidate does not strictly beat it (a statistical tie reverts too: if the
optimizer cannot show an improvement, shipping the seed is the safe outcome). On
by default, gated on a base_commit being set; costs one extra admin eval.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(harbor): auto_best reverts to the baseline when no candidate beats it
fix(harbor): make baseline scoring at finalize durable and retried
@varunursekar
varunursekar merged commit a0b5bcb into harbor-2-mode-b-sample-timeout-warning Jul 17, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants