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

[shehabyasser-scale]

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 six live failure modes observed in the Pawn campaign: idempotent finalize caching, per-commit mean-pooling + git-tree dedup to defeat max-over-rows exploitation, bounded retries on every reward-critical eval, all-error-as-outage retry, fail-safe floor reversion to seed when the baseline is unmeasurable, and a new verifier_timeout config field that correctly sizes the verifier's budget to cover the full finalize battery.

• verifier.py: Wraps _finalize() in a lock-guarded idempotent shell, introduces _admin_eval_score() (retry loop, all-error detection, None-as-unmeasurable contract), replaces max-over-rows shortlisting with two-stage mean-pooling + GitWorkspace.tree_hash dedup, and adds fail-safe fallback to seed when the baseline floor eval cannot complete.
• config.py / compiler.py / task.toml.j2: Adds optional verifier_timeout field that defaults to timeout; the template now uses this value so finalize is not killed mid-battery.
• workspace/git.py: Adds tree_hash(ref) wrapping git rev-parse {ref}^{tree} to enable content-based commit dedup.

Confidence Score: 4/5

Safe to merge — the finalize path is significantly more robust than before, and the changes are well-tested against the specific live failure modes they target.

The core change (idempotent finalize, pooled shortlisting, _admin_eval_score retry loop, fail-safe floor) is logically sound and backed by 34 passing tests including direct regression pins for each new behavior. Two non-blocking code-quality issues exist in verifier.py: tree-hash resolution is sequential where an asyncio.gather() would suffice, and a rare silent-None return path in _admin_eval_score bypasses the retry loop without logging.

vero/src/vero/harbor/verifier.py — the two code-quality issues are both localized here.

Important Files Changed

Filename	Overview
vero/src/vero/harbor/verifier.py	Core of the PR — adds idempotent finalize (lock + cached result), per-commit mean-pooling and git-tree dedup for shortlisting, _admin_eval_score with bounded retry for every reward-critical eval, all-error eval retry, and fail-safe floor fallback. Logic is sound; two minor code-quality concerns noted.
vero/src/vero/harbor/build/config.py	Adds optional `verifier_timeout: int
vero/src/vero/harbor/build/compiler.py	Passes verifier_timeout to the task compile step, correctly falling back to config.timeout when unset.
vero/src/vero/harbor/build/templates/task.toml.j2	Switches timeout_sec in the verifier block to use the new verifier_timeout variable; comment correctly documents the sizing formula.
vero/src/vero/workspace/git.py	Adds tree_hash(ref) as a thin wrapper around git rev-parse {ref}^{tree} to support content-based dedup in the verifier.
vero/tests/test_harbor_verifier.py	Adds 7 targeted tests covering idempotency, commit-pooling, tree-dedup, floor fail-safe, retry-then-floor with error marker, transient recovery, and all-error retry. Coverage is appropriate for the new paths.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["finalize() called"] --> B{_finalize_result cached?}
    B -- Yes --> C["Return cached result (idempotent)"]
    B -- No --> D["Acquire _finalize_lock"]
    D --> E["_select_commit()"]
    E --> F{reward_mode}
    F -- submit --> G["Read submission.json"]
    F -- auto_best --> H["_best_from_db()"]
    H --> H1["Load experiments_df, filter split/dataset/base_commit"]
    H1 --> H2["groupby candidate_commit → per-commit mean_score"]
    H2 --> H3["Resolve tree_hash for each commit (sequential)"]
    H3 --> H4["groupby _tree → tree-level mean_score, newest commit represents group"]
    H4 --> H5["shortlist = top-K by pooled mean_score"]
    H5 --> H6["Re-score each shortlisted commit via _admin_eval_score"]
    H6 --> H7{auto_best_baseline_floor?}
    H7 -- Yes --> H8["_admin_eval_score baseline on selection split"]
    H8 --> H9{baseline measurable?}
    H9 -- No --> H10["Fail-safe: return base_commit (seed)"]
    H9 -- Yes --> H11{best_score > base_score?}
    H11 -- No --> H10
    H11 -- Yes --> H12["Return best_commit"]
    H7 -- No --> H12
    G --> I["Score selected commit on each target via _admin_eval_score"]
    H12 --> I
    H10 --> I
    I --> J{score returned?}
    J -- Yes --> K["rewards[key] = score"]
    J -- No --> L["rewards[key] = default_minimum_score + target_errors[key]"]
    K --> M["_maybe_score_baseline()"]
    L --> M
    M --> N["Cache & return result"]
    N --> C

    subgraph "_admin_eval_score (retry loop)"
        R1["evaluate_admin()"] --> R2{score fill_score=None is None?}
        R2 -- Yes, all errored --> R3{More attempts?}
        R3 -- Yes --> R1
        R3 -- No --> R4["Return None"]
        R2 -- No --> R5["Return score()"]
        R1 -- Exception --> R6{More attempts?}
        R6 -- Yes --> R1
        R6 -- No --> R4
    end

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() called"] --> B{_finalize_result cached?}
    B -- Yes --> C["Return cached result (idempotent)"]
    B -- No --> D["Acquire _finalize_lock"]
    D --> E["_select_commit()"]
    E --> F{reward_mode}
    F -- submit --> G["Read submission.json"]
    F -- auto_best --> H["_best_from_db()"]
    H --> H1["Load experiments_df, filter split/dataset/base_commit"]
    H1 --> H2["groupby candidate_commit → per-commit mean_score"]
    H2 --> H3["Resolve tree_hash for each commit (sequential)"]
    H3 --> H4["groupby _tree → tree-level mean_score, newest commit represents group"]
    H4 --> H5["shortlist = top-K by pooled mean_score"]
    H5 --> H6["Re-score each shortlisted commit via _admin_eval_score"]
    H6 --> H7{auto_best_baseline_floor?}
    H7 -- Yes --> H8["_admin_eval_score baseline on selection split"]
    H8 --> H9{baseline measurable?}
    H9 -- No --> H10["Fail-safe: return base_commit (seed)"]
    H9 -- Yes --> H11{best_score > base_score?}
    H11 -- No --> H10
    H11 -- Yes --> H12["Return best_commit"]
    H7 -- No --> H12
    G --> I["Score selected commit on each target via _admin_eval_score"]
    H12 --> I
    H10 --> I
    I --> J{score returned?}
    J -- Yes --> K["rewards[key] = score"]
    J -- No --> L["rewards[key] = default_minimum_score + target_errors[key]"]
    K --> M["_maybe_score_baseline()"]
    L --> M
    M --> N["Cache & return result"]
    N --> C

    subgraph "_admin_eval_score (retry loop)"
        R1["evaluate_admin()"] --> R2{score fill_score=None is None?}
        R2 -- Yes, all errored --> R3{More attempts?}
        R3 -- Yes --> R1
        R3 -- No --> R4["Return None"]
        R2 -- No --> R5["Return score()"]
        R1 -- Exception --> R6{More attempts?}
        R6 -- Yes --> R1
        R6 -- No --> R4
    end

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
vero/src/vero/harbor/verifier.py:393-397
**Sequential tree-hash resolution adds latency for large candidate pools**

Each `_tree_of()` call is a `git rev-parse` subprocess awaited one at a time. For a shortlist of N commits this serializes N git calls. With 10–50 recorded candidates this is noticeable wall-clock latency inside what is otherwise an I/O-bound async flow. These lookups are independent and can be issued concurrently with `asyncio.gather()`.

### Issue 2 of 2
vero/src/vero/harbor/verifier.py:203-204
**Silent `None` return bypasses the retry loop**

After the all-error guard (`fill_score=None` is not `None`) confirms at least one sample scored, `exp.result.score()` is called with default fill. If — due to an edge case in the result object — `score()` returns `None` here, the function returns `None` immediately from within the loop without retrying or emitting a warning. Callers interpret `None` as "could not measure" and apply their fail-safe (floor, or revert to seed for the baseline floor check), so a single such occurrence silently triggers the most conservative outcome without any retry or log entry. A simple `continue` (or raising into the retry machinery) here would keep the retry semantics consistent.

_{Reviews (1): Last reviewed commit: "fix(harbor): selection and finalize inte..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[greptile-apps]

Silent None return bypasses the retry loop

After the all-error guard (fill_score=None is not None) confirms at least one sample scored, exp.result.score() is called with default fill. If — due to an edge case in the result object — score() returns None here, the function returns None immediately from within the loop without retrying or emitting a warning. Callers interpret None as "could not measure" and apply their fail-safe (floor, or revert to seed for the baseline floor check), so a single such occurrence silently triggers the most conservative outcome without any retry or log entry. A simple continue (or raising into the retry machinery) here would keep the retry semantics consistent.

Prompt To Fix With AI

This is a comment left during a code review.
Path: vero/src/vero/harbor/verifier.py
Line: 203-204

Comment:
**Silent `None` return bypasses the retry loop**

After the all-error guard (`fill_score=None` is not `None`) confirms at least one sample scored, `exp.result.score()` is called with default fill. If — due to an edge case in the result object — `score()` returns `None` here, the function returns `None` immediately from within the loop without retrying or emitting a warning. Callers interpret `None` as "could not measure" and apply their fail-safe (floor, or revert to seed for the baseline floor check), so a single such occurrence silently triggers the most conservative outcome without any retry or log entry. A simple `continue` (or raising into the retry machinery) here would keep the retry semantics consistent.

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