fix: [no-ticket] harbor - scoring integrity (dead attempts, best-rank monotonicity, strict reward key)

[shehabyasser-scale]

What

Three scoring-integrity fixes in HarborRunner, each closing a channel a live optimizer exploited or could exploit:

1. Dead attempts count 0.0 in mean aggregation (new n_dead metric). Attempts dying before the verifier scored were silently dropped from the mean, so the score estimated P(pass | attempt survived). Measured live in the Pawn replication: a no-retry candidate won selection at an inflated 0.233 while its retry-hardened successors measured an honest ~0.19 and lost; the optimizer documented the artifact in its own notes ("retry HELPS true quality but can LOWER measured train score"). All-dead samples error loudly instead of scoring 0.0.
2. 'best' trial ranking is monotone in the reward. The rank was (clean, has_rewards, recency), so with concurrent attempts 'best' meant "last clean attempt to finish" and a later clean 0.0 could clobber an earlier clean 1.0, violating the invariant documented in _load_trials. Reward now precedes recency in the key.
3. Strict reward_key extraction. A configured reward_key missing from a rewards dict no longer silently falls back to 'pass'/'reward' (attempts within one mean could be scored on different metrics), and several unrecognized keys are refused instead of averaged (a candidate could inflate the unweighted mean by emitting easy auxiliary metrics beside the real one). Sole-key dicts stay accepted.

Tests

tests/test_harbor_runner.py: 48 pass, including new pins for zero-fill (0.5 not 1.0), all-dead-errors, rank monotonicity, strict key, and refuse-to-average.

Stack

Based on #31 (harbor-4-mode-config-split). Part of the pre-paper hardening series; found by an 18-agent adversarial code sweep + live optimizer behavior.

🤖 Generated with Claude Code

Greptile Summary

This PR closes three scoring-integrity channels in HarborRunner: dead attempts (verifier never scored) now count 0.0 instead of being dropped from the mean; the _trial_rank sort key now includes the reward value so a later clean 0.0 cannot clobber an earlier clean 1.0; and _extract_reward is strict — a configured reward_key missing from a rewards dict returns None instead of falling back to pass/reward.

• Dead-attempt zero-fill (_sample_result mean path): every attempt in the group contributes to measured; only n_scored == 0 (all-dead) falls through to an explicit error rather than a silent 0.0.
• Rank monotonicity (_trial_rank): reward now precedes recency in the sort key, so the selection invariant documented in _load_trials is actually enforced.
• Strict reward key (_extract_reward): returns float | None; callers now branch on None and error loudly rather than silently averaging arbitrary auxiliary metrics.

Confidence Score: 4/5

Safe to merge; the three fixes are directionally correct and well-tested, with 48 tests passing including 6 new pins for each changed invariant.

The core logic changes in the mean aggregation loop and _trial_rank are correct and the tests directly validate the motivating scenarios. The one rough edge is that n_dead in the metrics bundles truly-dead attempts with reward-key-mismatch attempts under a single counter, which could mislead operators doing infra vs. config triage. There is also no test pinning the reward-key-mismatch zero-fill case in mean mode, leaving a gap for that specific path to regress silently.

The mean-aggregation loop in runner.py (lines 341–384) deserves a second look specifically around the n_dead counter semantics and the missing test for reward-key-mismatch zero-fill.

Important Files Changed

Filename	Overview
vero/src/vero/harbor/runner.py	Three integrity fixes: dead attempts now count 0.0 in mean aggregation (n_dead metric added), _trial_rank now includes reward in sort key for monotonicity, _extract_reward is strict with no silent fallback. One minor observability concern: n_dead can also increment for reward-key-mismatch attempts (verifier ran, wrong key), not only truly pre-verifier failures, which could mislead operators diagnosing infra vs config issues.
vero/tests/test_harbor_runner.py	Adds 6 new tests covering zero-fill for dead attempts (asserting 0.5 not 1.0), all-dead-errors, rank monotonicity, strict reward_key (no fallback), and refuse-to-average multi-key rewards. Existing tests updated to match new warning message format. Good coverage of the three fixes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[_sample_result called] --> B{attempts present\nAND mean mode?}
    B -- No --> G[Single-trial path]
    B -- Yes --> C[Loop over attempts]
    C --> D{_extract_reward\nreturns non-None?}
    D -- Yes\nscored attempt --> E[measured.append reward\nn_scored++\nmaybe n_clean++]
    D -- No\ndead OR key-mismatch --> F[measured.append 0.0\nn_dead++]
    E --> C
    F --> C
    C -- done --> H{n_scored > 0?}
    H -- No\nall dead --> G
    H -- Yes --> I[mean = sum/len\nreturn SampleResult\nwith n_dead metric]
    G --> J[rewards = trial rewards]
    J --> K{rewards empty?}
    K -- Yes --> L[SampleResult error:\nNo verifier rewards]
    K -- No --> M{_extract_reward\nreturns None?}
    M -- Yes --> N[SampleResult error:\nno usable metric]
    M -- No --> O[SampleResult score=reward]

    style F fill:#f9c74f
    style N fill:#f9844a
    style L fill:#f9844a
    style I fill:#90be6d

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[_sample_result called] --> B{attempts present\nAND mean mode?}
    B -- No --> G[Single-trial path]
    B -- Yes --> C[Loop over attempts]
    C --> D{_extract_reward\nreturns non-None?}
    D -- Yes\nscored attempt --> E[measured.append reward\nn_scored++\nmaybe n_clean++]
    D -- No\ndead OR key-mismatch --> F[measured.append 0.0\nn_dead++]
    E --> C
    F --> C
    C -- done --> H{n_scored > 0?}
    H -- No\nall dead --> G
    H -- Yes --> I[mean = sum/len\nreturn SampleResult\nwith n_dead metric]
    G --> J[rewards = trial rewards]
    J --> K{rewards empty?}
    K -- Yes --> L[SampleResult error:\nNo verifier rewards]
    K -- No --> M{_extract_reward\nreturns None?}
    M -- Yes --> N[SampleResult error:\nno usable metric]
    M -- No --> O[SampleResult score=reward]

    style F fill:#f9c74f
    style N fill:#f9844a
    style L fill:#f9844a
    style I fill:#90be6d

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/runner.py:354-356
**`n_dead` blends two distinct failure modes**

The counter increments both when an attempt died before the verifier ran (`rewards` is empty/None) and when the verifier ran but produced rewards under the wrong key (`_extract_reward` returns `None` on a non-empty dict, e.g. `reward_key="acc"` but the attempt emitted `{"pass": 1.0}`). The warning message labels both as "dead counted 0.0", but a misconfigured reward key is a config error, not an infra failure. An operator seeing `n_dead=2` cannot tell whether two attempts crashed or whether the verifier ran fine but the key was wrong. A separate `n_unscorable` counter (non-empty rewards, `_extract_reward` returned `None`) would let those two diagnoses diverge.

### Issue 2 of 2
vero/tests/test_harbor_runner.py:348-365
**No test for reward-key-mismatch zero-fill in mean mode**

The new zero-fill tests cover truly dead attempts (verifier never ran). There is no test for the parallel case where `reward_key` is configured, the verifier ran and produced rewards, but under the wrong key — e.g. `reward_key="acc"`, attempt 0 emits `{"acc": 1.0}`, attempt 1 emits `{"pass": 1.0}`. Per the current code, attempt 1 would be counted as `n_dead` (0.0 zero-filled) and the mean would be 0.5. This is the intended behavior per the strict-key contract, but pinning it in a test would guard against unintentional regression (it is an easy path to accidentally revert when softening the fallback logic later).

_{Reviews (1): Last reviewed commit: "fix(harbor): close three scoring-integri..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

[greptile-apps]

n_dead blends two distinct failure modes

The counter increments both when an attempt died before the verifier ran (rewards is empty/None) and when the verifier ran but produced rewards under the wrong key (_extract_reward returns None on a non-empty dict, e.g. reward_key="acc" but the attempt emitted {"pass": 1.0}). The warning message labels both as "dead counted 0.0", but a misconfigured reward key is a config error, not an infra failure. An operator seeing n_dead=2 cannot tell whether two attempts crashed or whether the verifier ran fine but the key was wrong. A separate n_unscorable counter (non-empty rewards, _extract_reward returned None) would let those two diagnoses diverge.

Prompt To Fix With AI

This is a comment left during a code review.
Path: vero/src/vero/harbor/runner.py
Line: 354-356

Comment:
**`n_dead` blends two distinct failure modes**

The counter increments both when an attempt died before the verifier ran (`rewards` is empty/None) and when the verifier ran but produced rewards under the wrong key (`_extract_reward` returns `None` on a non-empty dict, e.g. `reward_key="acc"` but the attempt emitted `{"pass": 1.0}`). The warning message labels both as "dead counted 0.0", but a misconfigured reward key is a config error, not an infra failure. An operator seeing `n_dead=2` cannot tell whether two attempts crashed or whether the verifier ran fine but the key was wrong. A separate `n_unscorable` counter (non-empty rewards, `_extract_reward` returned `None`) would let those two diagnoses diverge.

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

[greptile-apps]

No test for reward-key-mismatch zero-fill in mean mode

The new zero-fill tests cover truly dead attempts (verifier never ran). There is no test for the parallel case where reward_key is configured, the verifier ran and produced rewards, but under the wrong key — e.g. reward_key="acc", attempt 0 emits {"acc": 1.0}, attempt 1 emits {"pass": 1.0}. Per the current code, attempt 1 would be counted as n_dead (0.0 zero-filled) and the mean would be 0.5. This is the intended behavior per the strict-key contract, but pinning it in a test would guard against unintentional regression (it is an easy path to accidentally revert when softening the fallback logic later).

Prompt To Fix With AI

This is a comment left during a code review.
Path: vero/tests/test_harbor_runner.py
Line: 348-365

Comment:
**No test for reward-key-mismatch zero-fill in mean mode**

The new zero-fill tests cover truly dead attempts (verifier never ran). There is no test for the parallel case where `reward_key` is configured, the verifier ran and produced rewards, but under the wrong key — e.g. `reward_key="acc"`, attempt 0 emits `{"acc": 1.0}`, attempt 1 emits `{"pass": 1.0}`. Per the current code, attempt 1 would be counted as `n_dead` (0.0 zero-filled) and the mean would be 0.5. This is the intended behavior per the strict-key contract, but pinning it in a test would guard against unintentional regression (it is an easy path to accidentally revert when softening the fallback logic later).

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

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!