Skip to content

fix: [no-ticket] harbor - selection/finalize integrity (idempotency, tree pooling, retries, fail-safe floor)#33

Open
shehabyasser-scale wants to merge 2 commits into
harbor-5-scoring-integrityfrom
harbor-6-selection-integrity
Open

fix: [no-ticket] harbor - selection/finalize integrity (idempotency, tree pooling, retries, fail-safe floor)#33
shehabyasser-scale wants to merge 2 commits into
harbor-5-scoring-integrityfrom
harbor-6-selection-integrity

Conversation

@shehabyasser-scale

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

Copy link
Copy Markdown
Collaborator

What

Hardens the champion-selection/finalize path against the failure modes and exploits observed in the Pawn campaign:

  1. Idempotent finalize: the first completed result is cached and replayed verbatim on any retry. A re-run would re-rank against a DB that now also contains the first finalize's own admin evals, so a retried finalize could crown a different champion than the one already reported to Harbor.
  2. Pooled shortlisting: recorded evals of the same commit average into one score (not max-of-draws), and commits with identical git trees collapse into one candidate group (GitWorkspace.tree_hash). Two live optimizers Goodharted max-over-rows selection in opposite directions: one farmed empty re-commits as "clean independent lottery tickets for max-based selection" (its words), the other refused to ever re-measure its champion to protect a lucky 0.3 draw. Pooling makes re-measurement variance-reducing, and tree-dedup stops identical content from stuffing the top-K shortlist or collecting several admin re-score draws.
  3. Retry for every reward-critical eval (targets, shortlist re-scores, floor, baseline; same bounded-attempts knob as baseline scoring). A target that persistently fails is floored WITH a durable target_errors marker in the finalize payload instead of aborting finalize: a trial that ships no reward.json loses its result entirely, which happened live to an 8-hour run on a disk-full host.
  4. All-error evals retry, never quietly zero: an eval where no sample scored (score(fill_score=None) is None) is an outage, indistinguishable from infrastructure failure, and is retried like an exception.
  5. Fail-safe floor: when the baseline itself cannot be measured, selection reverts to the seed rather than shipping an unverified candidate.
  6. verifier_timeout build field: Harbor's [verifier] timeout_sec must cover the whole finalize battery (up to top-K re-scores + floor + targets + baseline attempts, each a full nested eval in Mode B), not one eval. Defaults to timeout for backward compatibility; docstring gives the sizing formula.

Tests

tests/test_harbor_verifier.py: 34 pass, including 7 new pins: idempotent replay, same-commit pooling to mean, identical-tree dedup (no shortlist stuffing, one re-score per tree), floor fail-safe, target retry-then-floor-with-marker, transient recovery, all-error-retries. Build/serve suites green (43 pass).

Stack

Based on #32 (harbor-5-scoring-integrity). Part of the pre-paper hardening series.

🤖 Generated with Claude Code

Greptile Summary

This PR hardens the Harbor champion-selection and finalize path against failure modes observed in the Pawn campaign, introducing idempotency, pooled shortlisting, bounded retries for every reward-critical eval, and a fail-safe floor.

  • Idempotent finalize: an asyncio.Lock + instance-level cache ensures a replayed /finalize call returns the first completed result verbatim, preventing re-ranking against a DB already polluted by the first run's admin evals.
  • Pooled shortlisting: recorded evals of the same commit are averaged (eliminating max-over-draws lottery farming), and commits sharing an identical git tree are collapsed into one candidate group via GitWorkspace.tree_hash, preventing tree-stuffing of the top-K shortlist.
  • _admin_eval_score retry helper: all reward-critical evals (targets, shortlist re-scores, floor, baseline) now share a bounded-retry path that treats all-error evals as outages rather than silently zeroing them; persistent failures floor the target reward with a durable target_errors marker so reward.json always ships.

Confidence Score: 5/5

Safe to merge; all changed paths have targeted test coverage and the fail-safe defaults are conservative.

The finalize logic is thoroughly tested with 7 new pins covering every new invariant. The idempotency cache is correctly scoped to the Verifier instance and serialized with an asyncio.Lock. Tree pooling and per-commit mean averaging are straightforward pandas operations with correct groupby semantics. The two non-blocking observations do not affect correctness.

vero/src/vero/harbor/verifier.py — the _maybe_score_baseline retry loop is the only path that does not use the new _admin_eval_score helper, leaving it with subtly different retry semantics; worth unifying in a follow-up.

Important Files Changed

Filename Overview
vero/src/vero/harbor/verifier.py Core hardening of finalize: idempotent wrapper with asyncio.Lock, new _admin_eval_score retry helper, per-commit mean pooling + git-tree deduplication, fail-safe floor on unmeasurable baseline. Logic is correct; minor style points around sequential tree-hash resolution and _maybe_score_baseline using a separate retry approach.
vero/src/vero/workspace/git.py Adds tree_hash() method using git rev-parse {ref}^{tree}; straightforward and correctly delegated through _git().
vero/src/vero/harbor/build/config.py Adds verifier_timeout: int
vero/src/vero/harbor/build/compiler.py Passes verifier_timeout (defaulting to timeout) to compile_task; straightforward backward-compatible change.
vero/src/vero/harbor/build/templates/task.toml.j2 Switches timeout_sec from {{ timeout }} to {{ verifier_timeout }}; safe because compiler.py guarantees verifier_timeout is always set.
vero/tests/test_harbor_verifier.py 7 new tests covering idempotency, commit pooling, tree dedup, fail-safe floor, target retry-then-floor, transient recovery, and all-error retry. Coverage is thorough and test assertions are specific.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[finalize call] --> B{_finalize_result cached?}
    B -- yes --> C[Return cached result idempotent]
    B -- no --> D[Acquire asyncio.Lock]
    D --> E[_select_commit]
    E -- submit mode --> F[Read submission.json]
    E -- auto_best mode --> G[get_experiments_df]
    G --> H[Filter split / dataset / exclude base_commit]
    H --> I[Per-commit mean pooling]
    I --> J[Resolve tree hashes]
    J --> K[Tree dedup pooling]
    K --> L[Rank and shortlist top-K]
    L --> M[Admin re-score each shortlisted commit]
    M --> N{auto_best_baseline_floor?}
    N -- yes --> O[Score baseline with retry]
    O -- None returned --> P[Fail-safe: return base_commit]
    O -- score returned --> Q{best_score > base_score?}
    Q -- no --> P
    Q -- yes --> R[Return best_commit]
    N -- no --> R
    F --> S[sha = selected commit]
    R --> S
    P --> S
    S --> T[Score each target on sha with retry]
    T -- score returned --> U[rewards key = score]
    T -- None after all retries --> V[rewards key = floor + target_errors marker]
    U --> W[_maybe_score_baseline optional]
    V --> W
    W --> X[Cache and return result]
Loading
%%{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"}}}%%
flowchart TD
    A[finalize call] --> B{_finalize_result cached?}
    B -- yes --> C[Return cached result idempotent]
    B -- no --> D[Acquire asyncio.Lock]
    D --> E[_select_commit]
    E -- submit mode --> F[Read submission.json]
    E -- auto_best mode --> G[get_experiments_df]
    G --> H[Filter split / dataset / exclude base_commit]
    H --> I[Per-commit mean pooling]
    I --> J[Resolve tree hashes]
    J --> K[Tree dedup pooling]
    K --> L[Rank and shortlist top-K]
    L --> M[Admin re-score each shortlisted commit]
    M --> N{auto_best_baseline_floor?}
    N -- yes --> O[Score baseline with retry]
    O -- None returned --> P[Fail-safe: return base_commit]
    O -- score returned --> Q{best_score > base_score?}
    Q -- no --> P
    Q -- yes --> R[Return best_commit]
    N -- no --> R
    F --> S[sha = selected commit]
    R --> S
    P --> S
    S --> T[Score each target on sha with retry]
    T -- score returned --> U[rewards key = score]
    T -- None after all retries --> V[rewards key = floor + target_errors marker]
    U --> W[_maybe_score_baseline optional]
    V --> W
    W --> X[Cache and return result]
Loading

Reviews (2): Last reviewed commit: "fix(harbor): a None aggregate score cons..." | Re-trigger Greptile

Comment thread vero/src/vero/harbor/verifier.py Outdated
shehabyasser-scale and others added 2 commits July 7, 2026 16:29
…retries, fail-safe floor)

Five gaps in the champion-selection/finalize path, all observed or provoked
live:

1. Idempotent finalize: the first completed result is cached and replayed on
   any retry. Re-running would re-rank against a DB that now contains the
   first finalize's own admin evals, so a retried finalize could crown a
   different champion than the one already reported.
2. Pooled shortlisting: recorded evals of the same commit average (not max),
   and commits with identical git TREES collapse into one candidate group.
   Max-over-rows made every re-measurement an independent lottery draw; one
   live optimizer farmed empty re-commits as 'clean independent lottery
   tickets', another refused to re-measure its champion to protect a lucky
   draw. Pooling makes re-measurement variance-reducing and stops identical
   content from stuffing the top-K shortlist.
3. Every reward-critical finalize eval (targets, shortlist re-scores, floor,
   baseline) retries transient failures; targets that persistently fail are
   floored WITH a durable target_errors marker instead of aborting finalize
   (a trial that ships no reward.json loses its result: happened live to an
   8-hour run on a disk-full host).
4. All-error evals (score(fill_score=None) is None) retry like exceptions:
   an outage must never quietly become a measured 0.0.
5. Fail-safe floor: when the baseline itself cannot be measured, revert to
   the seed rather than shipping an unverified candidate (the floor exists to
   stop shipped regressions; skipping it re-opens that hole).

Plus: verifier_timeout build field sizes Harbor's [verifier] timeout_sec for
the whole finalize battery (the old value covered ~one eval, so Harbor could
kill finalize mid-flight), and GitWorkspace.tree_hash() resolves commit
content identity for the pooling.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…the loop (review follow-up)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant