Skip to content

One challenger registry, and stop a new model moving the #46 human pass (#122) - #128

Merged
jonfroehlich merged 8 commits into
mainfrom
feat/challenger-registry-122
Aug 18, 2026
Merged

One challenger registry, and stop a new model moving the #46 human pass (#122)#128
jonfroehlich merged 8 commits into
mainfrom
feat/challenger-registry-122

Conversation

@jonfroehlich

Copy link
Copy Markdown
Member

Closes #122.

Stacked on #123 — base this on feat/claude-sonnet-5-leg, not main. #123 touches the
_compare_args shim this PR replaces, and its Claude flags are now registry entries. Retarget
to main once #123 lands.

The hazard was real, and here it is measured

silent_witness.py computes the RampNet misses no other model witnessed. A further witness can
only shrink that set — and that set is the item list for the #46 tagging pass, which is finished,
with committed per-rater verdicts.

Adding the already-published gemini-3.7-flash leg (#120) to the roster would have done this:

unwitnessed sparse-union excess near-field lower bound
frozen pool (7 models) 59 +39.0 ~19 ramps 0.0092
plus gemini-3.7-flash 58 +35.6 ~18.1 ramps 0.0088

The single item that drops out is morgantown:356422300574178 at (0.09899, 0.54838). It is one
of the 50 crops already rated, and it carries one of only two definition verdicts in the whole
pass
— the rarest category in it. Nothing would have raised a hand: the gallery's manifest digest
would change, the published bracket would move, and the verdict file would still look fine.

What changed

rampnet/roster.py is the single table. CHALLENGERS, SPARSE, DENSE, SCORED_SPECS and
the per-provider defaults all derive from it. It lives in the package because both scripts/analysis/*
and scripts/model_comparison/* need it and it must import without torch, so a fresh clone can
score published detections on a laptop.

WITNESS_POOL_46 is written out literally, not derived — that is the whole mechanism. It is
silent_witness.py's new --models default.

  • silent_witness.py takes --models, and derives sparse/dense from the pool it is handed.
  • An unregistered or unmeasured model now raises instead of being silently filed as dense,
    which is what spec in SPARSE else "DENSE" did. Only the sparse union feeds a headline, so a
    wrong guess there moves a published number.
  • "union of the 5 sparse models" reads its count off the pool.
  • Both silent_witness.json and fp_taxonomy.json record the pool they ran over — same reasoning
    as the detector signature already inside every published detections file.
  • Five copies of the provider defaults (compare.py, _compare_args, null_recall.py,
    dump_detections.py, and null_recall.DEFAULT_MODELS) collapse to one. A drift there never
    crashed; it changed the cache key and silently missed every already-paid detection.
  • gemini-3.7-flash is registered as published-but-off-roster. Its density is measured, not
    assumed: 1.90 boxes/pano over 2,109 panos, against gemini-3.6-flash's 2.34 and OWLv2's 72.77.

Adding a model is now one registry entry.

Docs stop counting the roster by hand

Two committed docs disagreed with each other — "all 8" in ten matrix cells of
docs/model_comparison.md against "seven-model roster" in docs/replication.md, both true about
different things, neither saying so, while a ninth leg sat published in the repo.

The doc now carries a table generated by python -m rampnet.roster, and tests/test_roster.py
fails if the doc and the registry disagree. The stale-phrase guard collapses whitespace first —
every one of those phrases was wrapped across a line break, so a naive substring check passes while
the docs are still wrong.

Four errors fixed in passages this touched anyway:

  • curb_ramp_data_sourcing.md said the Qualitative failure taxonomy: bucket each model's FP/FN from cached detections #46 reviewer pass was not done. It was completed and
    committed 2026-07-31: 50/50, digest 360b5ddf8751dcd0, visible 41 / unclear 4 /
    context-only 3 / definition 2.
  • its verdict table listed six of the eight verdicts, dropping context-only and definition
    — the two that most affect what a visible rate means, and both actually used.
  • replication.md pointed at benchmark/miss_taxonomy_46/silent__manifest.json, which does not exist.
  • replication.md said --verify skips "nine files"; there are ten.

Verification

  • pytest -q1033 passed, 2 skipped. tests/test_roster.py is new (21 tests).
  • silent_witness.py re-run: unwitnessed identical in content and order, all 59 items; every
    other number unchanged; models is the only added key.
  • fp_taxonomy.py re-run: same, models the only added key.
  • export_model_cache.py --verify: 68/68 pairs still score identically to the cache, so the
    spec_labelroster.label_for swap is behaviourally inert.
  • The AST drift guard in test_fp_taxonomy.py now resolves _D["key"] against the registry
    rather than skipping it, so the provider flags stay inside the check and it fails if compare.py
    ever stops reading the registry.

Deliberately not done

🤖 Generated with Claude Code (claude-opus-5[1m])

jonfroehlich and others added 3 commits August 18, 2026 11:03
… made under (#122)

`CHALLENGERS`, `SPARSE` and `DENSE` were three tuples kept in step by hand, and
`silent_witness.py` iterated the roster directly with no way to point it anywhere
else. That made adding a model to the benchmark quietly expensive in the one place
nobody would look: a finished human pass.

`rampnet/roster.py` is now the single table. It carries each model's spec, label,
density class, the date it joined, and whether it is scored in the roster tables;
`CHALLENGERS`, `SPARSE`, `DENSE` and the per-provider defaults all derive from it.

The load-bearing part is `WITNESS_POOL_46`, written out literally rather than
derived, and it is `silent_witness.py`'s new `--models` default. The hazard is not
hypothetical — measured here:

  frozen pool (7 models)   59 unwitnessed, sparse-union excess +39.0
  plus gemini-3.7-flash    58 unwitnessed, sparse-union excess +35.6

The one item that drops out is morgantown:356422300574178 at (0.09899, 0.54838).
It is one of the 50 crops already tagged, and it carries one of only two
`definition` verdicts in the whole pass. Adding an already-published leg to the
roster would have orphaned it, changed the gallery's manifest digest, and moved the
published lower bound from 0.0092 to 0.0088 recall points, with nothing raising a
hand. Now a new challenger moves the comparison tables and leaves the human pass
alone by construction.

Also:

- `silent_witness.py` takes `--models`, and derives its sparse/dense split from the
  pool it was given. An unregistered or unmeasured model now raises instead of being
  silently filed as dense, which is what `spec in SPARSE else "DENSE"` did.
- The "union of the 5 sparse models" / "2 dense detectors" strings read their counts
  off the pool instead of having them baked in.
- Both `silent_witness.json` and `fp_taxonomy.json` record the pool they ran over,
  so a verdict file can be matched to the pool that produced its items — the same
  reasoning as the detector signature inside each published detections file.
- gemini-3.7-flash is registered as published-but-off-roster (#120). Its density is
  measured, not assumed: 1.90 boxes/pano over 2,109 panos, against gemini-3.6-flash's
  2.34 and OWLv2's 72.77.

Both committed artifacts regenerate with the `models` block as the only diff: every
number is unchanged, and `unwitnessed` is identical in content and order.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sers (#122)

`--gemini-model`, `--qwen-model`, `--owlv2-model`, `--gdino-model`, `--molmo-model`
and the three Claude flags were written out four times: compare.py's parser,
fp_taxonomy's `_compare_args` shim, null_recall.py and dump_detections.py. They all
feed `build_detector` and therefore the detection signature and the cache key, so a
copy that drifts does not crash — it changes the key, misses every already-paid
cached detection, and reports a model with zero detections.

They now all read `roster.PROVIDER_DEFAULTS`. `null_recall.DEFAULT_MODELS` was a
fifth private copy of the roster itself; it is `roster.SCORED_SPECS` now, and it
gains the Claude flags it was missing.

`export_model_cache.spec_label` delegates to `roster.label_for`, which is the same
torch-free resolution plus a hook for providers whose `model_id` slot is not a model
id. Verified unchanged: 68/68 published (model, split) pairs still score identically
to the cache.

The AST drift guard in test_fp_taxonomy.py now resolves `_D["key"]` subscripts
against the registry rather than skipping them. Resolving instead of skipping is the
point — it keeps the provider flags inside the check, and it fails if compare.py
ever stops reading the registry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
)

The roster count was written by hand in four documents, and two of them disagreed:
docs/model_comparison.md said "all 8" in ten coverage-matrix cells while
docs/replication.md said "seven-model roster" — both true, about different things,
neither saying so — with a ninth leg published in the repo at the time.

docs/model_comparison.md now carries a table generated by `python -m rampnet.roster`,
and `tests/test_roster.py` fails if the doc and the registry disagree. The matrix
cells say "full roster" rather than a number. The stale-phrase guard collapses
whitespace before checking, because every one of these was wrapped across a line
break and a naive substring check finds none of them.

Four errors fixed in passages this touched anyway:

- curb_ramp_data_sourcing.md said the #46 reviewer pass was not done. It was
  completed and committed on 2026-07-31 — 50/50 tagged, digest 360b5ddf8751dcd0,
  `visible` 41 / `unclear` 4 / `context-only` 3 / `definition` 2.
- its verdict table listed six of the eight verdicts, silently dropping
  `context-only` and `definition` — the two that most affect what the `visible` rate
  means, and both actually used in the pass.
- replication.md pointed at benchmark/miss_taxonomy_46/silent__manifest.json, which
  does not exist; the digest is in silent_gallery/manifest.json.
- replication.md said `--verify` skips "nine files"; there are ten.

The bracket section now states which pool it is against, and says plainly that
adding gemini-3.7-flash would move it from 0.0092 to 0.0088 and drop an
already-rated item. scripts/analysis/README.md stops restating the bracket at all —
it is a function of the pool, so it belongs next to the pool, not in a script index.

The point estimate is still not computed, deliberately: it needs a decision about
whether `context-only` and `definition` count toward the sourcing target, and it
rests on one rater with no agreement statistic (#46, #74).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jonfroehlich
jonfroehlich changed the base branch from feat/claude-sonnet-5-leg to main August 18, 2026 18:14
jonfroehlich and others added 2 commits August 18, 2026 11:14
…r Claude legs (#122)

The registry claimed to be "every model the benchmark knows about, scored or
not". It covered 78 of the 112 files in benchmark/model_detections/. The three
supervised YOLO pano arms (30 files, #51) and the four annapolis Claude legs (4
files, #122) had been run, scored, verified and written up, and the one place
that is supposed to enumerate every model said nothing about them.

That gap is now a test rather than a promise. Two directions, both checked
against the directory instead of against prose:

  test_every_published_detections_file_belongs_to_a_registered_leg
  test_every_registered_leg_has_published_detections

plus test_each_published_file_names_the_leg_it_says_it_is, which is the one
check a clean clone can run with no cache at all: the filename, the
published_as recorded inside, the signature's model_id and the registry all
have to agree.

Registering the Claude legs needed a concept the registry did not have. Effort
is in the detection signature, so one model id is several legs with different
cache keys and different results, and both claude-sonnet-5 legs wanted the same
filename. Hence two fields:

  pins          the signature-entering knobs this leg holds, e.g.
                (("claude_effort", "high"),)
  published_as  the filename stem, defaulting to label -- because label cannot
                carry the pin. It is baked into cache keys that were already
                paid for, so renaming it orphans the detections.

BY_SPEC stays single-valued by holding only default legs (pins that match
PROVIDER_DEFAULTS), which is what a bare --models spec reproduces.
SCORED_SPECS is unchanged, so no analysis default moved: the YOLO arms in
particular must stay out of it, since it is the default --models of
fp_taxonomy, null_recall and silent_witness and a yolo: spec carries a local
checkpoint path.

Densities are measured off the published detections, per the registry's own
rule that density is evidence. The YOLO arms are 2.02-2.60 boxes/pano at the
headline conf 0.25 (4.42-9.39 at the 0.05 export floor); the Claude legs
1.56-3.73. All sparse.

Also: label_for now knows the yolo stem rule, mirroring YoloDetector's own
identity rule, so a yolo:<path> label is derived rather than overridden; slug()
moves to the roster so the directory test can spell a filename without
importing the exporter; and docs/replication.md's detection ledger now accounts
for all 112 files instead of 82, with the verification status of each group
stated rather than implied.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
report_usage warns when a leg finishes with no log destination. That warning
cannot save the run it fires on: by the time it prints, the tokens are bought,
and token counts are the one artifact that cannot be back-filled -- a re-run
reads the detection cache, makes zero API calls, and can never reproduce them.
The four Claude legs' $28.82 is unrecoverable for exactly that reason.

So the check moves to before the money is spent. compare.py now errors out on
--usage-log none when any selected provider is paid, with
--allow-unrecorded-spend as a named, visible override -- a guard with no
override gets edited out instead, which is worse.

Which providers cost money is roster.PAID_PROVIDERS, and a test asserts that
every model priced in pricing.py has its provider in that set, so the guard
cannot develop a hole exactly where money is being spent.

The end-of-run warning stays: it still covers the case where a log path existed
but could not be written.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jonfroehlich
jonfroehlich force-pushed the feat/challenger-registry-122 branch from 5dc8333 to e4d3d1d Compare August 18, 2026 18:18
@jonfroehlich

Copy link
Copy Markdown
Member Author

Restacked on the current #123 tip, and closed the gap that restack exposed

This branch was stacked on feat/claude-sonnet-5-leg as it stood before the two review commits landed, so it did not have the four published Claude legs in its tree. Rebased onto the current tip (2bdcbd3); the conflicts were the ones you would expect — MODELS_HELP, the two new --claude-* flags, and the ledger prose — and claude_image_format / claude_temperature now come from PROVIDER_DEFAULTS with the others.

With the Claude files present, the registry could be checked against the directory instead of against prose. It did not survive that:

rampnet/roster.py said it was "every model the benchmark knows about, scored or not". It covered 78 of the 112 files in benchmark/model_detections/. The three supervised YOLO pano arms (30 files, #51) and the four annapolis Claude legs (4 files) had all been run, scored, verified and written up while the one place that is supposed to enumerate every model said nothing about them. That is also what the unremarked 78 → 108 jump in the ledger's own drift list was.

What is in the two new commits

0437d67 — register the missing legs. Seven entries, and the concept the registry lacked to hold four of them. Effort is in the detection signature, so one model id is several legs with different cache keys and different results, and both claude-sonnet-5 legs wanted the same filename. Two fields:

field what it is
pins the signature-entering knobs this leg holds, e.g. (("claude_effort", "high"),)
published_as the filename stem, defaulting to label — because label cannot carry the pin. It is baked into cache keys that were already paid for, so renaming it orphans the detections.

BY_SPEC stays single-valued by holding only default legs (pins matching PROVIDER_DEFAULTS), which is what a bare --models <spec> reproduces. SCORED_SPECS is unchanged, so no analysis default moved — and the YOLO arms in particular have to stay out of it: it is the default --models of fp_taxonomy, null_recall and silent_witness, and a yolo: spec carries a local checkpoint path, so promoting them would break those three scripts on any clone without the weights.

Densities are measured off the published detections, per the registry's own rule that density is evidence: the YOLO arms are 2.02–2.60 boxes/pano at the headline conf 0.25 (4.42–9.39 at the 0.05 export floor), the Claude legs 1.56–3.73. All sparse.

The guard is a bijection, in both directions, against the directory:

  • test_every_published_detections_file_belongs_to_a_registered_leg
  • test_every_registered_leg_has_published_detections
  • test_each_published_file_names_the_leg_it_says_it_is — the one check a clean clone can run with no cache at all: the filename, the published_as recorded inside, the signature's model_id and the registry all have to agree.

docs/replication.md's ledger now accounts for all 112 files as a table rather than 82 in a sentence, and says which of them --verify covers by default (68) and which were verified by their own invocation against their own machine-local cache (44).

e4d3d1d — refuse to start a paid leg that will not record what it spent. The report_usage warning added in #123 cannot save the run it fires on: by then the tokens are bought, and token counts are the one artifact that cannot be back-filled, since a re-run reads the detection cache and makes zero API calls. So compare.py now errors out on --usage-log none when any selected provider is paid, with --allow-unrecorded-spend as a named, visible override — a guard with no override gets edited out instead. Which providers cost money is roster.PAID_PROVIDERS, and a test asserts every model priced in pricing.py has its provider in that set, so the guard cannot develop a hole exactly where money is being spent.

Suite: 1,100 passed, 2 skipped (was 1,067 on #123).

#129 is rebased on this and pushed; it needed vistas in PROVIDERS, the ledger count at 114, and one real fix — test_every_published_leg_is_named_in_the_ledger knew that slug() turns / into __ but not that it does the same to +, so it read mask2former-vistas-curb-cut+curb as undocumented.

🤖 Generated with Claude Code (claude-opus-5[1m])

@jonfroehlich

Copy link
Copy Markdown
Member Author

Review: 15 findings, all verified against the code

/code-review 128 high — diffed against the stated base feat/claude-sonnet-5-leg, suite re-run in a throwaway worktree (1,100 passed, 2 skipped), promotion path simulated. Every finding below was re-checked by hand before posting; none is speculative. Ten of the fifteen are in code added today, which is the useful part of the result.

Nothing is fixed yet — this comment is the record of what the review said. Fixes follow in a separate commit with a resolution table.

# severity file what
1 high tests/test_model_comparison.py:1293 a default-suite test can issue a real billed API call
2 high rampnet/roster.py:314 the documented one-field promotion path is broken for a pinned leg
3 med-high rampnet/roster.py:339 label_for yolo stem rule misses the --yolo-model path
4 medium scripts/analysis/fp_taxonomy.py:436 unregistered spec crashes after the whole analysis, losing it
5 medium tests/test_roster.py:315 test_provider_defaults_match_compare_pys_parser skips unconditionally — dead
6 medium scripts/model_comparison/detectors.py:1575 five more copies of the provider defaults survive
7 medium rampnet/roster.py:131 comment says SCORED_SPECS is silent_witness's default; it is WITNESS_POOL_46
8 medium scripts/analysis/export_model_cache.py:133 the exporter never asks the registry for a leg's published name
9 medium rampnet/roster.py:283 published_filename / BY_PUBLISHED have zero call sites
10 medium scripts/model_comparison/compare.py:676 spend guard blocks runs that provably cannot spend
11 low-med rampnet/roster.py:258 PAID_PROVIDERS docstring names a consumer that does not exist
12 low scripts/analysis/silent_witness.py:62 vestigial SPARSE/DENSE + unused CHALLENGERS import
13 low docs/replication.md:87 drift list ends at 108 while the table above it says 112
14 low tests/test_roster.py:82 stale-phrase guard is over-broad and partly redundant
15 low tests/test_model_comparison.py:1262 _run_compare docstring promises a two-tuple

1. A test in the default suite can make a paid API call

test_unrecorded_spend_stays_possible_but_deliberate drives compare.main() all the way through build_detector into score_model with --models claude:claude-opus-5 --usage-log none --allow-unrecorded-spend --limit 1. ClaudeDetector constructs lazily, so nothing stops before score_model, and the only thing standing between pytest -q and a billed request is a .model_cache hit.

Verified preconditions on this machine: anthropic 0.122.0 installed in .venv, .env present at repo root (90 B), and benchmark/annapolis/panos/ populated with 125 jpgs in the primary checkout. On a cache miss that is a live paid call with --usage-log none — the precise thing this PR exists to prevent, written by the commit that prevents it. It also violates the repo rule that a test needing a network call belongs behind a skip, and it writes into the real REPO_ROOT/.model_cache.

The intent was to prove the override still works. That should assert on the parser, not on a run.

2. The one-field promotion path is broken for a pinned leg

CHALLENGERS keys on c.spec, but SPARSE/DENSE resolve each spec back through BY_SPEC, which by design holds only default legs. Simulated: set standing=True on the claude-opus-5-effort-high entry and

claude spec in CHALLENGERS: True
BY_SPEC resolves that spec to: claude-opus-5-effort-low

so the roster table would claim the high leg while fp_taxonomy/null_recall score the low leg's detections, and the high leg's density would be read off the low leg's measurement — exactly the "density is evidence, not configuration" guarantee the module docstring makes.

There is a latent second half: promote a pinned leg whose spec has no default-leg sibling and BY_SPEC[s] at line 314 raises KeyError at module import, taking down all six scripts that import roster at module scope. Not reachable with today's entries — both claude: specs happen to have a default leg — but nothing prevents it, and test_a_bare_spec_names_exactly_one_leg only inspects default legs.

3. label_for's yolo rule misses the --yolo-model path

The stem reduction added here fires only when the path arrives inside the spec:

label_for("yolo", cargs with yolo_model="runs/detect/train/weights/best.pt")
  -> 'runs/detect/train/weights/best.pt'
build_detector for the same args      -> 'best'

export_model_cache.spec_label is label_for, so --models yolo --yolo-model <path> slugs a whole path into a filename that was never written, then falls back to a cache key that misses on every pano — reported as "no cache entry" and silently excluded rather than as an error. Same drift class the PR was written to close.

The rest, briefly

  • 4. pool_record is called at fp_taxonomy.py:436, inside the if args.json_out: block, so an unregistered spec raises KeyError only after every model is scored and every table printed — and no JSON is written. silent_witness.py:183 validates its pool up front for exactly this reason.
  • 5. hasattr(compare, "build_parser") is always False (the parser is built inline in main()), so the test skips unconditionally and asserts nothing. The real coupling check exists once, in test_fp_taxonomy._compare_parser_defaults. A reader trusting the name believes the most load-bearing invariant here is covered twice.
  • 6. build_detector keeps its own fallbacks (owlv2 1575, gdino 1580, molmo 1586, effort 1564, tool_choice 1565), and scripts/analysis/size_analysis.py:36 and depth_extract_da3.py:44 each define a private Args with hardcoded model ids handed straight to build_detector. Change PROVIDER_DEFAULTS and these five sites keep the old value — a different signature, silently missing every already-paid detection.
  • 7. roster.py:131 and docs/model_comparison.md both say SCORED_SPECS is the default --models of silent_witness. It is WITNESS_POOL_46 (silent_witness.py:170) — changed by this same PR. Someone reasoning about whether promoting a YOLO arm disturbs the frozen Qualitative failure taxonomy: bucket each model's FP/FN from cached detections #46 pool reads this and concludes it would, which is the opposite of what was built.
  • 8 + 9. These are one defect. published_filename() and BY_PUBLISHED have zero call sites — while export_model_cache still derives every filename from label plus a hand-typed --publish-as, and the new tests hand-roll slug(published_name(c)) + "__" + city inline, which is published_filename's body. Re-export the effort-high leg without the flag and it writes claude-opus-5__annapolis.json: no registry entry, no collision with an existing file, so the signature guard stays quiet and it surfaces later as a red bijection test. The registry holds the answer and the one place that writes the filename never asks. (BY_PUBLISHED's comment also claims its key is "unique by construction" — a dict comprehension drops duplicates silently; the uniqueness is held by test_published_names_are_unique, not by the dict.)
  • 10. The spend guard keys on provider alone, so a fully cached --usage-log none re-score exits 2 even though score_model would print "all N panos already cached; model load skipped" (compare.py:272), skip prepare(), make zero calls, and report_usage would return at its first line. null_recall.cache_coverage() already does the cache lookup that would discriminate.
  • 11. PAID_PROVIDERS' comment says "two separate checks need it: the cost estimate in pricing.py, and compare.py's refusal". grep returns three hits total — definition, compare.py:678, one test. pricing.py never imports the roster and prices by model id. The stated reason for the constant's location is false.
  • 12. silent_witness.SPARSE/DENSE (62–63) have no remaining runtime use — the script runs off pool_sparse/pool_dense — but still sit under a comment about what feeds the headline, bound to the standing roster rather than the frozen pool. CHALLENGERS on line 53 is now an unused import.
  • 13. "It drifted three times (61 → 68 …, 68 → 78 …, 78 → 108 unremarked)" — the 108 → 112 step is the four Claude legs, which the table 20 lines above lists as its own row. Stale in the passage rewritten to say the count "is now a test rather than a promise".
  • 14. "all 8" is a strict prefix of "all 8 model groups", so two of five entries can never fire independently, and any future legitimate "all 8 splits" in those three files fails the suite. scripts/analysis/README.md, which this PR edits and which carries per-model prose, is outside the loop.
  • 15. _run_compare's docstring promises (code, stderr); the body returns the code alone.

Read

Findings 1, 2, 3, 8/9 and 10 are the same shape: the registry now knows something the code around it does not ask it. Worth fixing as one lookup rather than patching each site — the exporter, label_for and the promotion path should all resolve through one entry lookup.

🤖 Generated with Claude Code (claude-opus-5[1m], effort: high)

Two were real defects with teeth; the rest were the registry knowing something
the code around it never asked.

**A test could spend money.** test_unrecorded_spend_stays_possible_but_deliberate
drove compare.main() into score_model with a paid model and --usage-log none, so
`pytest -q` was one cache miss away from a billed call -- written by the commit
that exists to stop unrecorded spend. All four spend tests now drive score_model
directly with a stub detector whose prepare() raises, so a guard that let a run
through fails loudly instead of quietly reaching for the network. No bundle, no
credentials, 0.42 s.

**The guard also refused runs that cannot spend.** Keying on provider at parse
time rejected a fully cached re-score, which skips the model load and makes zero
calls -- the exact path the published detections exist to keep open. The check
moved into score_model, at the first uncached pano: still before any money moves,
and now it raises UnrecordedSpend, which main() deliberately does not swallow
into its "not runnable" handler.

**Promotion was broken for a pinned leg.** standing=True on a pinned entry put
its spec in CHALLENGERS while BY_SPEC resolved it to the sibling default leg, so
the tables would claim one leg and fp_taxonomy/null_recall would score the
other's detections and read its density. Now rejected at import with an
explanation, SPARSE/DENSE derive from entries rather than a spec lookup that
could KeyError at module scope, and a test covers it.

The rest:

- label_for applies the yolo stem rule to --yolo-model too, not just to a path
  inside the spec; both spellings now agree with YoloDetector (was: a whole path
  slugged into a filename that was never written, then a cache miss on every
  pano reported as "no cache entry").
- The exporter asks the registry for a leg's published name (new roster.leg_for
  + publication_name) instead of depending on --publish-as being remembered.
  Forgetting it wrote the bare model id, which collides with nothing, so the
  overwrite guard stayed quiet. published_filename/BY_PUBLISHED are now used
  rather than hand-rolled in the tests.
- compare.py grows build_parser(), so test_provider_defaults_match_compare_pys_parser
  actually runs. It had skipped unconditionally its whole life while reading as
  coverage of the most load-bearing invariant here.
- build_detector's fallbacks and the private Args in size_analysis.py and
  depth_extract_da3.py read PROVIDER_DEFAULTS; five more copies of values that
  feed the cache signature are gone.
- fp_taxonomy builds its pool record BEFORE scoring, so an unregistered spec
  fails in a second instead of after every model is scored -- and only for runs
  that were going to write an artifact.
- Two comments stated things that are false: SCORED_SPECS is not silent_witness's
  default (this PR made that WITNESS_POOL_46, which is the point of the freeze),
  and pricing.py does not consume PAID_PROVIDERS.
- silent_witness's vestigial SPARSE/DENSE and unused CHALLENGERS import are gone;
  their tests now assert over the frozen pool the script actually runs.
- The stale-phrase doc guard no longer has an entry that is a prefix of another,
  no longer rejects a legitimate "all 8 splits", and covers scripts/analysis/README.md.
- Ledger drift list runs to 112; _run_compare's wrong docstring went with the
  test rewrite.

Suite 1,100 -> 1,102 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jonfroehlich

Copy link
Copy Markdown
Member Author

All 15 findings fixed — 16315ea

Suite 1,100 → 1,102 passed, 1 skipped. Every fix below was re-run before commit.

# finding resolution
1 test could make a paid API call fixed — all four spend tests drive score_model directly with a stub whose prepare() raises; no bundle, no credentials, 0.42 s
2 promotion path broken for a pinned leg fixed — rejected at import with an explanation; SPARSE/DENSE derive from entries, so the KeyError half is gone too
3 label_for yolo rule missed --yolo-model fixedweights_stem() applied on both paths; both spellings now agree with YoloDetector
4 fp_taxonomy validated the pool after the run fixed — pool record built before scoring; unregistered spec now fails in ~1 s
5 dead test_provider_defaults_match_compare_pys_parser fixedcompare.build_parser() extracted; the test is now unconditional and passes with 0 mismatches across 33 options
6 five more copies of the provider defaults fixedbuild_detector fallbacks + both private Args classes read PROVIDER_DEFAULTS
7 comment: SCORED_SPECS is silent_witness's default fixed — corrected in roster.py and docs/model_comparison.md; it is WITNESS_POOL_46, which is the point of the freeze
8 exporter never asked the registry for a name fixedroster.leg_for() + publication_name(); --publish-as is now an override, not a requirement
9 published_filename / BY_PUBLISHED dead fixed — both in use; tests no longer hand-roll the filename
10 spend guard refused runs that cannot spend fixed — check moved to the first uncached pano inside score_model
11 PAID_PROVIDERS docstring named a nonexistent consumer fixed — says what actually reads it, and names the test that keeps it in step with pricing.py
12 vestigial SPARSE/DENSE in silent_witness fixed — removed with the unused import; tests retargeted to the frozen pool
13 ledger drift list stopped at 108 fixed — runs to 112, and says which step was which
14 stale-phrase guard over-broad fixed — no entry is a prefix of another, all 8 splits no longer rejected, scripts/analysis/README.md now in scope
15 _run_compare docstring fixed — the helper went with the test rewrite

The two that were more than cleanup

Finding 1 deserves restating because it is the same class of mistake the PR is about. test_unrecorded_spend_stays_possible_but_deliberate asserted that --allow-unrecorded-spend lets a run proceed — by letting it proceed, through build_detector and into score_model, with claude:claude-opus-5. ClaudeDetector builds lazily, so nothing stopped it; the only thing between pytest -q and a billed request was a .model_cache hit. It passed in a worktree because that worktree had no panos. The lesson generalizes: a test that proves an override works must not exercise the thing the override permits.

Finding 10 is the other half of the same idea. The first version of the guard refused at parse time on provider alone, which also refuses a fully cached re-score — a run that skips the model load entirely and makes zero calls. Guarding on intent to spend rather than possibility of spending would have blocked the clean-clone re-scoring path the published detections exist to provide. The check now sits at the first uncached pano: still before any money moves, and no false refusals. It raises UnrecordedSpend, which main() re-raises rather than folding into its "not runnable" handler — otherwise the next paid leg in the same --models list would spend unrecorded anyway.

One judgement call worth flagging

Fixing finding 8 changed observable behaviour: a registered pinned leg now publishes under its registry name without --publish-as. Two collision tests were retargeted accordingly — the scenario they constructed (a bare claude-sonnet-5__annapolis.json to collide with) can no longer arise for a registered leg, which is the point. The overwrite guard is unchanged and still covers unregistered legs and explicit --publish-as.

🤖 Generated with Claude Code (claude-opus-5[1m], effort: high)

jonfroehlich added a commit that referenced this pull request Aug 18, 2026
The rebase brings in build_detector's _D() lookup, so the Vistas branch's own
literals (VISTAS_CHECKPOINT, min_area 16, dtype float16, class set "curb-cut")
now come from PROVIDER_DEFAULTS like every other provider's, and
dump_detections.py's three --vistas-* args do too. That was review finding 8 on
this PR: line 183 of that file promises "defaults from rampnet.roster.
PROVIDER_DEFAULTS, so this overlay reproduces exactly what compare.py cached
rather than a near-miss", and the vistas args passed None.

Worth recording how the last piece was found. #128 gave compare.py a
build_parser() so that test_provider_defaults_match_compare_pys_parser could
stop skipping unconditionally. Registering "vistas_class_set" made it fail
immediately -- KeyError, no matching flag -- because the class set was reachable
only through the --models spec. So compare.py grows --vistas-class-set, and the
test that had asserted nothing for its whole life caught a real drift on its
first live run.

The other 14 findings on this PR are NOT addressed here; three of them change
what the write-up claims and are answered separately.

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

108 of the 114 files in benchmark/model_detections/ were exported before the
`published_as` field existed, so re-running the exporter on any of them produced
a file that differed from the committed one -- identical detections, different
envelope. docs/replication.md promises that a derived artifact can be PROVEN
identical when regenerated, and that was not true of 95% of this corpus.

I first assumed closing it meant re-exporting from .model_cache, i.e. finding
the machine that produced each leg -- Hyak for the open detectors, makelab2 for
the YOLO trio. It does not. The serialization is deterministic (now DUMP_KW, one
definition), all 114 files round-trip byte-identically through it, and for every
one of these the published name is recoverable from the file itself, so the
result is exactly what a real re-export would write.

  python scripts/analysis/export_model_cache.py --canonicalize [--write]

Cache-free, detections untouched, and it REFUSES rather than guesses when a
file's published name cannot be derived from its contents -- guessing there is
the silent rename this mechanism exists to prevent. Measured on the corpus: 108
files rewritten, 0 detections changed, `published_as` the only key that differs.

Three tests keep it shut: every file equals its own canonical re-dump, every
file declares the name it is published under and agrees with its filename, and
canonicalize cannot alter a detection.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jonfroehlich added a commit that referenced this pull request Aug 18, 2026
The rebase brings in build_detector's _D() lookup, so the Vistas branch's own
literals (VISTAS_CHECKPOINT, min_area 16, dtype float16, class set "curb-cut")
now come from PROVIDER_DEFAULTS like every other provider's, and
dump_detections.py's three --vistas-* args do too. That was review finding 8 on
this PR: line 183 of that file promises "defaults from rampnet.roster.
PROVIDER_DEFAULTS, so this overlay reproduces exactly what compare.py cached
rather than a near-miss", and the vistas args passed None.

Worth recording how the last piece was found. #128 gave compare.py a
build_parser() so that test_provider_defaults_match_compare_pys_parser could
stop skipping unconditionally. Registering "vistas_class_set" made it fail
immediately -- KeyError, no matching flag -- because the class set was reachable
only through the --models spec. So compare.py grows --vistas-class-set, and the
test that had asserted nothing for its whole life caught a real drift on its
first live run.

The other 14 findings on this PR are NOT addressed here; three of them change
what the write-up claims and are answered separately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jonfroehlich

Copy link
Copy Markdown
Member Author

Published detections are now in canonical form — ca9275a

Follow-up to the thing the #129 review surfaced and misattributed: 108 of the 114 files in benchmark/model_detections/ were exported before the published_as field existed, so re-running the exporter on any of them produced a file that differed from the committed one — identical detections, different envelope. docs/replication.md promises a derived artifact can be proven identical when regenerated, and that was not true of 95% of the corpus.

I first assumed closing it meant re-exporting from .model_cache, i.e. finding the machine that produced each leg — Hyak for the open detectors, makelab2 for the YOLO trio. It does not. The serialization is deterministic (now DUMP_KW, one definition), all 114 files round-trip byte-identically through it, and for every one of these the published name is recoverable from the file itself. So the result is exactly what a real re-export would write:

python scripts/analysis/export_model_cache.py --canonicalize          # report
python scripts/analysis/export_model_cache.py --canonicalize --write  # apply

Measured on the corpus: 108 files rewritten, 0 detections changed, published_as the only key that differs. It refuses rather than guesses when a file's published name cannot be derived from its contents — guessing there is the silent rename the whole published_as mechanism exists to prevent — and it reported 0 such files.

Three tests keep it shut: every file equals its own canonical re-dump, every file declares the name it is published under and agrees with its own filename, and canonicalize cannot alter a detection. Suite 1,102 → 1,105.

Worth carrying forward: the next envelope change is not a reason to go find the producing machines either. Only a file whose published name is not recoverable from its contents needs a real re-export, and the tool names those instead of guessing.

🤖 Generated with Claude Code (claude-opus-5[1m], effort: high)

…try-122

# Conflicts:
#	docs/curb_ramp_data_sourcing.md
@jonfroehlich
jonfroehlich merged commit ecdea69 into main Aug 18, 2026
4 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.

Make adding a challenger cheap: one registry, and stop it moving the #46 human-pass item list

1 participant