fix: [no-ticket] harbor - honest measurement signals (summary qualifiers, versioned re-evals, dead-attempt causes)#35
Conversation
…alue, versioned re-evals, dead-attempt causes)
Four small fixes so recorded numbers say what they are:
1. summary.json now carries n_scored, n_errored, and score_se beside
mean_score: a mean over 3-of-18 scored samples, or one dominated by
errored zero-fills, is a different measurement than a clean
full-split mean, and both the agent and any auditor should see that
without per-sample access. All three are label-safe aggregates.
2. summary.json status now writes the enum VALUE ("success"), not
str(enum) ("ExperimentResultStatus.SUCCESS").
3. Result dirs are versioned per eval ({split}__{commit12}__eN instead
of wipe-and-rewrite keyed on (split, commit)): repeat measurements of
one commit (multifidelity confirms, champion re-evals) are exactly
the evidence worth comparing, and the second eval erased the first.
4. Mean-mode collation records dead_exception_types per sample: n_dead
alone hides WHY attempts died, and cause matters (rate-limit deaths
are infra noise, crashes point at the candidate; measured live,
110/129 UnicodeDecodeError deaths sat on two never-solved tasks).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if dest.exists(): # ordinal collision only on volume reuse; never merge | ||
| shutil.rmtree(dest) | ||
| dest.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
The comment says "ordinal collision only on volume reuse; never merge", but when a server restarts with a reused volume the first N evals quietly wipe whatever __e1 through __eN dirs survived from the prior session. Since the entire motivation of the versioned-dir change is that a re-measurement must not erase earlier evidence, a silent shutil.rmtree here is exactly the failure mode the PR is trying to prevent — it just occurs on restart rather than on same-session re-eval. At minimum a logger.warning(...) at this branch would surface the collision to auditors; the alternative is a guard on the server startup that moves the conflicting dirs to a timestamped backup name.
Prompt To Fix With AI
This is a comment left during a code review.
Path: vero/src/vero/harbor/server.py
Line: 291-293
Comment:
**Silent overwrite contradicts the preservation goal**
The comment says "ordinal collision only on volume reuse; never merge", but when a server restarts with a reused volume the first N evals quietly wipe whatever `__e1` through `__eN` dirs survived from the prior session. Since the entire motivation of the versioned-dir change is that a re-measurement must not erase earlier evidence, a silent `shutil.rmtree` here is exactly the failure mode the PR is trying to prevent — it just occurs on restart rather than on same-session re-eval. At minimum a `logger.warning(...)` at this branch would surface the collision to auditors; the alternative is a guard on the server startup that moves the conflicting dirs to a timestamped backup name.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Agreed, restart-with-reused-volume was exactly the erasure this change is supposed to prevent. Fixed in e1b1d6b (stack tip): on the first routed eval, _route_results scans the results root for surviving __eN dirs and resumes the ordinal past the max, so a restarted sidecar appends (e8, e9, ...) instead of wiping e1. Test: test_volume_reuse_resumes_ordinal_past_survivors.
| filled = [ | ||
| r.score if r.score is not None else 0.0 | ||
| for r in sample_results.values() | ||
| ] | ||
| score_se = None | ||
| if len(filled) > 1: | ||
| m = sum(filled) / len(filled) | ||
| var = sum((x - m) ** 2 for x in filled) / (len(filled) - 1) | ||
| score_se = (var / len(filled)) ** 0.5 |
There was a problem hiding this comment.
score_se is the SE of the zero-filled mean, but n_scored describes only the non-error population
filled zero-fills every sample whose score is None before computing the variance, which is consistent with mean_score (both use default_minimum_score = 0.0 as the fill). However, an agent reading n_scored=3 alongside score_se will naturally interpret SE as the uncertainty of the 3-sample mean — but it is actually the SE of the zero-filled-18-sample mean, which is smaller by roughly sqrt(n_scored / n_samples). Concretely: 3 of 18 scored with [1, 0.8, 0.9] gives SE ≈ 0.083 (zero-filled) vs ≈ 0.058 (scored-only); a reader inferring noise from the zero-filled SE will underestimate it. Adding a score_se_n field or a comment in the JSON clarifying that score_se is computed over n_samples (not n_scored) would avoid the misread.
Prompt To Fix With AI
This is a comment left during a code review.
Path: vero/src/vero/harbor/server.py
Line: 301-309
Comment:
**`score_se` is the SE of the zero-filled mean, but `n_scored` describes only the non-error population**
`filled` zero-fills every sample whose `score is None` before computing the variance, which is consistent with `mean_score` (both use `default_minimum_score = 0.0` as the fill). However, an agent reading `n_scored=3` alongside `score_se` will naturally interpret SE as the uncertainty of the 3-sample mean — but it is actually the SE of the zero-filled-18-sample mean, which is smaller by roughly `sqrt(n_scored / n_samples)`. Concretely: 3 of 18 scored with [1, 0.8, 0.9] gives SE ≈ 0.083 (zero-filled) vs ≈ 0.058 (scored-only); a reader inferring noise from the zero-filled SE will underestimate it. Adding a `score_se_n` field or a comment in the JSON clarifying that `score_se` is computed over `n_samples` (not `n_scored`) would avoid the misread.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fair: the pairing of n_scored with an SE computed over the zero-filled population invited misreading. Fixed in e1b1d6b (stack tip): renamed to mean_score_se to bind it to mean_score (both computed over the zero-filled n_samples population) and documented that an SE of the n_scored subset would be a different, larger number. Kept the zero-filled definition because it is the uncertainty of the number the agent actually optimizes against.
… floor default, ordinal resume, SE naming) - Free-baseline flag is claimed BEFORE the eval await and refunded on failure: setting it only after success reopened a window where two concurrent baseline evals both resolved free (asyncio interleaves at await points). Claim-then-refund keeps both properties: concurrent callers see the claim, and a failed eval does not burn the freebie. - build_status defaults k_anonymity_floor to 5, matching the sidecar's enforcement default: a caller that forgets to pass the floor must not advertise a laxer one than gets enforced. - _route_results resumes the eval ordinal past surviving __eN dirs on a reused volume: a restarted sidecar started back at e1 and silently wiped the prior session's evidence, the exact erasure the versioned dirs exist to prevent. - score_se renamed to mean_score_se and documented: it is the SE of the zero-filled mean_score over n_samples, not of the n_scored subset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Stacked on #34 (harbor-7-access-tiers). Part of the pre-paper hardening series; this is the "recorded numbers say what they are" group.
Changes
1. summary.json qualifies its mean (
server.py). The agent-visible aggregate now carriesn_scored,n_errored, andscore_sebesidemean_score. A mean over 3-of-18 scored samples, or one dominated by errored zero-fills, is a different measurement than a clean full-split mean; the agent (deciding whether a delta is real) and any auditor (reading the run afterward) should both see that without per-sample access. All three are label-safe aggregates, so they ride the same non_viewable-safe summary as before.2. Enum value, not repr (
server.py)."status": str(experiment.result.status)serialized as"ExperimentResultStatus.SUCCESS"(str-mixin enums lost their value-__str__in py3.11). Now writes.value->"success".3. Versioned result dirs (
server.py). Result dirs were keyed on(split, commit[:12])and wiped on re-eval, so a repeat measurement of the same commit erased the agent's earlier evidence; repeat measurements (multifidelity confirms, champion re-evals under a noisy scorer) are exactly the ones worth comparing. Each metered eval now gets{split}__{commit12}__eNandresult_pathin the response names the dir for that eval. The stale-file hazard the wipe used to prevent is gone structurally: no two evals share a dir.4. Dead-attempt causes (
runner.py). Mean-mode collation zero-fills attempts that died before scoring and counts them inn_dead; it now also recordsdead_exception_types(a per-cause count) in the sample output. Causes are not interchangeable: rate-limit deaths are infra noise that retunes with capacity, crashes point at the candidate, and deaths cluster hard by cause in practice (measured on 46 historical runs: 110 of 129 UnicodeDecodeError deaths sat on two never-solved tasks). Kept out ofmetrics(float-valued by contract) and rides the same viewable-only exposure path as attempt detail.Tests
__e1and__e2, both dirs alive.no_rewards_recorded; all-clean samples carry no key.🤖 Generated with Claude Code
Greptile Summary
This PR adds "honest measurement signals" to the Harbor evaluation sidecar:
summary.jsonnow carriesn_scored,n_errored, andscore_sealongsidemean_score; the enum status field is fixed to emit.valueinstead of thereprstring; result dirs are versioned per eval (__eN) so re-measurements preserve earlier evidence; and dead attempt causes are recorded asdead_exception_typesin mean-mode sample output.server.py):score_seis computed inline from the zero-filled score vector (same population asmean_score), andn_scored/n_erroredlet callers interpret the mean's quality without per-sample access.server.py):_eval_seq(a per-instance counter) replaces the wipe-and-rewrite strategy; each metered eval gets its own{split}__{commit12}__eNdir, preserving earlier runs for comparison.runner.py):dead_exception_typesis appended to mean-mode output only when there are dead attempts, kept out of float-onlymetrics, and exposed only on the viewable tier.Confidence Score: 4/5
The changes are well-scoped and the new fields ride existing access-tier guards correctly. The two rough edges — silent overwrite on restart-reuse and the SE population ambiguity — are non-breaking quality concerns, not data-corruption risks.
All four changes are individually correct and well-tested. The versioned-dir logic preserves earlier evidence within a session but silently wipes it on server restart with a reused volume, which is the exact scenario the PR aims to prevent. The SE field is mathematically consistent with mean_score but an agent pairing score_se with n_scored will likely interpret it as the SE of the scored-only mean rather than the zero-filled mean — a meaningful difference when error rates are high. Neither issue causes wrong data to be recorded in the normal single-session case.
vero/src/vero/harbor/server.py — the _write_results method's ordinal-collision branch and the score_se computation.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[evaluate request] --> B{admin?} B -- yes --> C[return None\nno result written] B -- no --> D{tier_for_split} D -- no_access --> C D -- partial / viewable --> E[_eval_seq += 1\ndest = split__commit12__eN] E --> F{dest.exists?} F -- yes\nordinal collision --> G[shutil.rmtree dest\nsilent overwrite] F -- no --> H[dest.mkdir] G --> H H --> I[compute filled scores\nzero-fill None scores] I --> J{len filled > 1?} J -- yes --> K[compute sample variance\nscore_se = sqrt var / n] J -- no --> L[score_se = None] K --> M[write summary.json\nn_samples n_scored n_errored\nmean_score score_se status.value] L --> M M --> N{tier == viewable?} N -- yes --> O[write per-sample JSON\nincludes dead_exception_types\nif present] N -- no partial --> P[summary only\nno per-sample files] O --> Q[return result_path = str dest] P --> Q%%{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[evaluate request] --> B{admin?} B -- yes --> C[return None\nno result written] B -- no --> D{tier_for_split} D -- no_access --> C D -- partial / viewable --> E[_eval_seq += 1\ndest = split__commit12__eN] E --> F{dest.exists?} F -- yes\nordinal collision --> G[shutil.rmtree dest\nsilent overwrite] F -- no --> H[dest.mkdir] G --> H H --> I[compute filled scores\nzero-fill None scores] I --> J{len filled > 1?} J -- yes --> K[compute sample variance\nscore_se = sqrt var / n] J -- no --> L[score_se = None] K --> M[write summary.json\nn_samples n_scored n_errored\nmean_score score_se status.value] L --> M M --> N{tier == viewable?} N -- yes --> O[write per-sample JSON\nincludes dead_exception_types\nif present] N -- no partial --> P[summary only\nno per-sample files] O --> Q[return result_path = str dest] P --> QPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(harbor): honest measurement signals ..." | Re-trigger Greptile