Skip to content

fix: make the harness's fallback substitutions visible - #2

Closed
idIing wants to merge 1 commit into
mainfrom
fix/surface-fallback-substitutions
Closed

idIing wants to merge 1 commit into
mainfrom
fix/surface-fallback-substitutions

Conversation

@idIing

@idIing idIing commented Sep 4, 2026

Copy link
Copy Markdown
Owner

The defect

Found by cold-cloning this repo and running the documented loop as an outsider: write a
contributed agent following docs/adding-an-agent.md (priciest-shop — buys the priciest
affordable Joker instead of the cheapest), then compare it. I made one ordinary mistake — I did
not add the imports its shop policy needs, because src/bench/agents.py does not import
FactoredAction / get_fallback_action / the action constants, and the guide never mentions it.

The policy raised NameError on all 40 of its shop decisions. Every step of the advertised
loop reported success:

agent broken (raised on every shop decision) same agent fixed
evaluate.py says 16/16 games ... 0 fails 16/16 games ... 0 fails
mean highest ante 1.375 3.125
paired vs greedy-shop −1.750 [−2.500, −0.875] — "interval excludes zero" +0.000 [+0.000, +0.000] — "interval includes zero"
shop decisions that actually ran 0 of 40 (fallback-error:NameError) 90 of 90

The broken run publishes a statistically significant finding from code that never executed once.

build_decider legality-gates every slot output and wraps every slot call in except Exception,
substituting a legal fallback. That is correct for battery robustness — one bad state must not kill
240 seeds — and this PR does not change it. The problem is that the substitution is invisible
everywhere a human looks:

  • evaluate.py prints 0 fails — a "fail" is a crashed game, not a dead policy.
  • the result artifact's summary had no field for it (run_depth, blind_stats, win_rate,
    advance_curve only).
  • scripts/inspect_run.py — the tool whose entire job is step 5, "inspect failures" — never
    mentioned reasoning or was_fallback. Its timeline for the broken agent looked completely
    normal.

The truth existed only in events[].reasoning in the raw JSONL, which you have to already know to
look for. A missing import is the mild version; the dangerous version is an AttributeError or
IndexError on a rare state, which silently degrades part of a run and shifts a headline number by
an unknowable amount that no artifact records.

For a product whose one purpose is trustworthy paired comparison, this is the worst-shaped defect
available — it doesn't break, it lies.

The fix — fallback stays, its silence goes

  • metrics.fallback_stats(runs) — counts substitutions, runs affected, and a by-reason
    breakdown, isolating fallback-error:* (the policy raised — always an agent defect) from
    fallback-illegal and the benign fallback-phase.
  • summary.fallback in the result artifact via metrics.summarize. Additive; no existing
    field changes and jackhammer.result/v1 consumers are unaffected.
  • evaluate.py prints each arm's substitution rate on every run, not only bad ones, and
    warns explicitly when any arm's policy raised.
  • inspect_run.py labels every decision with its recorded reasoning, marks substituted ones
    !! FALLBACK, banners the per-run rate, and flags affected runs in --list.
  • Docs — the swallow and how to check it, the import gap that trapped me, a fallback-rate
    reporting requirement for submissions (CONTRIBUTING.md), the artifact field
    (docs/result-artifacts.md), and a known-limits.md entry for the residual: substitution is now
    surfaced, not prevented, and a partial rate still shifts a number by an amount the artifact
    records but does not correct for.

The same broken run now reads:

priciest-shop: 8 runs -> .../priciest-shop.result.json
  mean highest ante: 1.375
  fallback substitutions: 40 / 180 decisions (22.2%) across 7/8 runs -- fallback-error:NameError=40
  ERROR: 40 decision(s) raised inside the priciest-shop policy (fallback-error:*). Those decisions
  were made by the harness, not by priciest-shop. Inspect with: scripts/inspect_run.py <runs.jsonl> --list

paired (8 seeds): priciest-shop - greedy-shop = -1.750 ante [-2.500, -0.875] boot-95
  interval excludes zero

WARNING: at least one arm's policy raised during the battery. The harness substituted a legal
fallback for those decisions, so the run completed and the numbers above are well-formed -- but
they are not measurements of that agent. Fix the agent and re-run before reporting anything from it.

and scripts/inspect_run.py --seed PVRQ4K5A:

fallback=5/16 decisions (31.2%)  fallback-error:NameError=5
  !! 5 decision(s) were substituted by the harness, not chosen by the agent.
  !! 5 of those raised inside the policy (fallback-error:*). That is a bug in the agent;
     this run does not measure the agent, and no number derived from it is about it.

decisions:
    0  ante=1  PlayHand  hand_type=Flush score=316  [greedy-tactical]
    1  ante=1  Reroll  [!! FALLBACK fallback-error:NameError]
    2  ante=1  NextRound  [!! FALLBACK fallback-error:NameError]

Verification

  • No published number moves. Re-running the broken agent after the change yields artifacts
    byte-identical for both arms outside the new fallback block, and the same
    -1.750 [-2.500, -0.875] — now under the warning above.
  • Fixed agent end-to-end: 0 / 328 substitutions, +0.000 [+0.000, +0.000].
  • Healthy baseline arms report fallback substitutions: 0 / 345 decisions — a clean battery is
    genuinely 0.0%, so any nonzero rate is signal.
  • uv run ruff check src scripts tests — clean.
  • uv run pytest — 79 passed, including new coverage of the broken-agent case in all three
    surfaces (tests/test_playground_metrics.py, tests/test_inspect_run.py, new
    tests/test_evaluate.py).
  • The import block now documented in docs/adding-an-agent.md was executed verbatim to confirm
    every name resolves.

Open question for the maintainer

I deliberately did not touch docs/protocol-v1.md's reporting checklist. Adding "fallback
rate" there is arguably a change to the evaluation procedure, which the freeze says mints v2. The
requirement currently lives in CONTRIBUTING.md instead. If you'd rather it be a protocol-level
requirement, that's a v1.1/v2 call, not mine.

The priciest-shop agent was only the reproduction vehicle and is not included here.

🤖 Generated with Claude Code

`build_decider` legality-gates every slot output and catches every slot
exception, substituting a legal fallback so one bad state cannot kill a
240-seed battery. That robustness is correct and stays. Its invisibility does
not: nothing a reader looks at distinguishes a result produced by an agent from
one produced by the harness standing in for it.

Reproduced by writing a contributed agent from docs/adding-an-agent.md and
making one ordinary mistake -- omitting the imports its shop policy needs,
which src/bench/agents.py does not already have. The policy then raised
NameError on all 40 of its shop decisions. Every step of the advertised loop
reported success: evaluate.py printed `0 fails` (a "fail" is a crashed game,
not a dead policy), the artifact summary had no field for it, inspect_run.py --
the tool whose job is "inspect failures" -- rendered a normal-looking timeline,
and the paired comparison published -1.750 ante [-2.500, -0.875], an interval
excluding zero, from code that never executed once. The truth existed only in
events[].reasoning in the raw JSONL.

Fallback stays; the silence goes.

- metrics.fallback_stats(runs): counts substitutions, runs affected, and a
  by-reason breakdown, isolating `fallback-error:*` (the policy raised -- always
  an agent defect) from `fallback-illegal` and the benign `fallback-phase`.
- summary.fallback in the result artifact, via metrics.summarize. Additive, so
  v1 consumers are unaffected; no existing field changes.
- evaluate.py prints each arm's substitution rate on every run, not only bad
  ones, and warns explicitly when any arm's policy raised.
- inspect_run.py labels every decision with its recorded reasoning, marks
  substituted ones `!! FALLBACK`, banners the per-run rate, and flags affected
  runs in --list.
- docs: the swallow and how to check it, the import gap that triggered this,
  a fallback-rate reporting requirement for submissions, the artifact field,
  and a known-limits entry for the residual (substitution is now surfaced, not
  prevented).

Verified: no published number moves. Re-running the broken agent yields
byte-identical artifacts for both arms outside the new block, and the same
-1.750 [-2.500, -0.875] -- now under an unmissable warning. The fixed agent
reports 0/328 substitutions and +0.000 [+0.000, +0.000]. ruff clean; 79 tests
pass, including new coverage of the broken-agent case in all three surfaces.

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

idIing commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Closing in favour of #6, which fixes the underlying defect this PR set out to surface.

The failure this PR found is real and it reproduces: a genuinely broken agent (missing _NEXT_ROUND import) yields mean highest ante: 1.375 with fallback substitutions: 40 / 180 decisions (22.2%) and nothing in the published output saying the agent never ran. That was worth catching.

The counter it added cannot see it, though. harness.py computed was_fallback = reasoning.startswith("fallback") where reasoning is the slot name, so the flag meant "the wrapper stepped in", not "this was a fallback action". A policy that calls get_fallback_action itself is recorded as having decided — demonstrated with an abstain-shop that defers every shop decision: identical mean highest ante: 1.375, but 0 / 180 substitutions and inspect_run.py reporting every decision came from the agent. Same non-decision, opposite verdict. This PR's new import block teaches contributors exactly that idiom.

#6 addresses it from the other end: was_fallback is now passed explicitly rather than inferred from a string, and the residual limit — that a slot reaching for a default internally is invisible from the composer — is documented at _packet instead of being implied away. It also removes build_decider's except Exception, which is the thing that actually let the broken-agent run exit 0; scripts/evaluate.py's _play_one already isolates per game and exits nonzero, so with that net gone the broken agent reports 7 fails each named with its seed and exception.

The fallback-phase row this PR shipped is also now wrong in a way worth stating: under #6 there is no phase left for it to fire on. It fired zero times on the whole v1 slate anyway — get_fallback_action was called 0 times across 240 seeds × all three baselines, positive control abstain-shop = 62.

Not closed for being wrong about the problem. Reopen the visibility question against #6's ground if the residual gap needs its own instrument.

@idIing idIing closed this Sep 4, 2026
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