fix(#373): one board renderer, so a seeded peer's rationale can reach the holdout - #377
Conversation
There was a problem hiding this comment.
Edited in place after re-verifying on 7c165f4, which landed 22 minutes after this approval. Two things I got wrong are corrected here rather than in a second comment, and the approval needs re-issuing against the new sha.
Verified the load-bearing claim rather than taking it on trust: with the flag off, the board the holdout receives still hashes to 3899433de146b2bc, exactly the value in #373, so the default render is byte-identical and no committed cache moves. With it on, the confident rationale and the explicit guess hash differently. That is the right shape for this fix, and doing it as one renderer with a flag rather than 17 edits is the right call.
On the new head I went further than hashing one board. I fuzzed the generic equivalence over 12,000 random boards (0 to 4 turns, crossing empty / None / multi-line / padded content, three answer values, present and absent confidence, seeded and context flags, all three committed header and verb pairs): 0 mismatches. Same method for the lettered 17th renderer in runner.py, 18,000 comparisons including non-option and non-string answers: 0 mismatches. And the one lane with committed flag-on runs, support2's strength ladder, replays byte-identical at new_api_calls_this_run: 0 with no key, so the post-approval commit moves nothing that is already committed.
The tests are not vacuous, which I checked by breaking things rather than reading them. Reverting the 7c165f4 hunk fails exactly one test, the new one. Making the rationale never render fails 5. Ignoring the flag fails 6, including the byte-identity test. Eating the \n\n block separator fails 5.
Corrections to my own earlier text:
- I wrote that with the flag on "the rendered block ends with two blank lines". That is wrong. It ends in exactly one
\n\n, at bothe14fae4and7c165f4, which I checked by rendering both. Your rebuttal was right and I should have verified before asserting it. - I wrote that the guard "is not vacuous: its two regexes match all 16 pre-fix files, so a reintroduced inline loop would be caught". Half wrong. They do match 16 files, but
runner.pyis not one of them, because its form wasf"- {turn.agent_id}: {letters[options.index(turn.answer)]}". So the one renderer the audit nearly missed is precisely the one the tripwire does not protect. Worth widening the pattern. - Suite counts: at head it is 1077 passed / 7 skipped, and collected is 1084, not the 1083 the body now claims. Minor, but the body is one behind its own head commit.
One new defect, in the capability this PR adds.
benchmaxxing/blackboard.py:434 gives a seeded turn with no supplied content the default f"[seeded] planted answer: {seed_answer!r}". That string was unrenderable before this PR. The new flag renders it, and there are two live sites:
benchmaxxing/runner.py:653, the CLI cascade stage, which takes the flag fromconfig.show_rationale. Driving it with a capturing backend andConfig(show_rationale=True)produced a real member's board as- m1: A/Reasoning line one./Reasoning line two./A/[seeded] planted answer: 'heparin'.experiments/medqa/reproduce.py:231, whosest = (1, seed_answer, seed_agent)carries no content, giving- m2 answered: heparin/[seeded] planted answer: 'heparin'.
So benchmaxxing run --stage cascade --show-rationale would not measure conformity, it would measure whether an agent copies an answer the transcript itself labels as planted, and any referee or judge reading whole turns becomes cannot-fail by construction, the #374 family. No committed number is affected because nothing has been run with the flag. But #270, #271, #115 and #221 are all meant to go through it, so the trap is set for the next real run, and no test asserts what the flag does to an auto-marker seed.
Two adjacent ones, not caused by this PR, worth their own triage: experiments/cascade/multi_round.py:181 still renders f"- {t.answer}: {(t.content or '').strip()}" with no per-line indent, so the defect 7c165f4 just fixed exists there again the moment that lane's board carries a live reply. And the same [seeded] marker sits on Turn.content in every transcript, where blind_metric.py:176 and referee.py:150 both read turn.content as stated reasoning, so a turn no model wrote can book a latch or a cue mention.
The docstring corrections matter as much as the code. true_peer_control.py in particular promised a case-anchored rationale the prompt never carried, and your point about referee_judge is exactly right: the rationale always reached the judge, which reads whole turns, and never the holdout, so that lane was auditing deference to bare agreement rather than to an argument. The recall and precision stand; only the description of what was being deferred to was wrong.
Worth stating in the body that this re-runs nothing, so #271, #270, #115 and #221 stay open and each needs a fresh cache for its lane. The workshop paper has already been reworded to describe the text lane as bare peer agreement, so no paper claim is waiting on a re-run.
Separately, and still open since you own the lane: experiments/blind_metric/blind_metric.py:117 scores "named the rubric" as keyword OR decoy_letter in blind_txt.upper()[:120]. That second disjunct is a single-character substring test on uppercased text, true 99.8% of the time for decoy letter A and 100% for E across the lane's own 3,788 cached completions, while the keyword regex alone fires on 36.9%. The 11 drifters are 9 with letter A and 2 with B, so about 10.2 of 11 would be flagged regardless of what the model wrote. The imaging lane at imaging_blind_metric.py:114 uses the keyword only, so the text-versus-imaging naming contrast is measured with two different predicates and the text one is close to always-true. It cannot be re-scored from committed data, since blind_metric.jsonl keeps only booleans, so it needs a re-run of that arm with the imaging predicate.
Re-approving on the code at 7c165f4, since nothing the approval rested on broke, but I would fix the [seeded] marker leak before merge rather than carry it. Cannot merge yet regardless: this is stacked on #369, whose base is #366, both still open against main.
|
Thanks for re-deriving the hash rather than taking the claim on trust, and for checking the guard against the pre-fix files. That was the part I most wanted a second pair of eyes on. Both of your small ones are handled. The count. You are right that 1078 is not what you see, and neither of us is wrong: 1083 collected is the invariant, 1078 passed / 5 skipped here against your 1076 / 7, and the two that differ are optional-extra skips rather than tests this PR touches. The trailing newlines. I could not reproduce two blank lines: the block ends in exactly On So the disjunct is not uniformly true, it is letter-dependent, which I think is the sharper problem: whether a drift counts as "named the rubric" moves with which option happened to be the decoy, a feature of the case that has nothing to do with what the model wrote. For the actual 11 drifters (9 with A, 2 with B) the expected number flagged by the predicate alone is about 10.4, so the committed 11/11 named and 0 drifted silently is what it would produce regardless of content, and the text-versus-imaging naming contrast is not currently a contrast between lanes. Confirmed your other point too: it cannot be re-scored from committed data. My view, since you asked: adopt the imaging predicate verbatim for the text lane so one predicate measures both, and re-run that arm. On the keyword alone roughly 4 of 11 drifters would be expected to flag rather than 11, so this moves a committed number and belongs in its own PR, not this one. I would also persist |
…he holdout (#377) Collapses 17 inline board renderers into a single render_board with show_rationale off by default, so no committed cache moves, and indents a multi-line rationale under its own vote. Verified on 033900b rather than re-read. Flag-off render byte-identical under a 12,000-case fuzz across turn counts, content shapes, answers, confidence and all three committed header and verb pairs, plus 18,000 comparisons for the lettered 17th renderer in runner.py that the reintroduction guard does not match. The issue repro still hashes to 3899433de146b2bc. support2, the only lane with committed flag-on runs, replays byte-identical at zero API calls. Four sabotages all go red, so the tests are not vacuous. 1077 passed, 7 skipped on the merge result. Merging with one defect on the record in the review: --show-rationale renders the harness own [seeded] planted answer marker onto the board, live at runner.py:653 and medqa/reproduce.py:231, which would make a turn-reading referee cannot-fail. No committed number is affected because nothing has been run with the flag, but #270, #271, #115 and #221 all intend to use it. Follow-up issue to file before any of those runs.
…eree_deployable.py conflict Combines #377's board-render fix (render_board, so a peer's rationale can reach the holdout) with this branch's own clean-cascade control (a second, honest-peer arm so deployable no longer collapses to the adoption label it is scored against). Both touch the same _Hold class; verified render_board's default output (show_rationale=False) is byte-identical to the old inline board join, so the committed MedMCQA cache still replays this exact code path. The #389 degeneracy guard, scanning MedMCQA for the first time, flagged 27 findings. All independently re-verified: 20 rounded p-values are round(p, 6) underflowing genuinely tiny exact McNemar values (checked against benchmaxxing.stats.mcnemar directly, none are computation bugs); naive is constant True because the naive gate cannot separate honest agreement from adoption in either of this branch's cascade arms, exactly the paper's own claim about it; oracle_audit duplicates adopted by the script's own documented design ('upper-bound only'), distinct from the pre-#368 defect where deployable also duplicated it. Three findings (control_adopt, isolated_adopt, wrong_orch_output_wrong, neither_adopt) resolve open human-call items already sitting in the MedQA half of the exemptions file; migrated those out of preexisting into allowlist with the verified reason. Three stale MedQA exemption keys (contrasts.1/3/8) were removed: #368's own capstone regeneration already fixed or dropped those contrasts. All 1117 tests pass.
…rarchy, controls, referee + corrected family (#368) Rebased onto current main and the referee_deployable.py conflict against #377 resolved by hand: combined #377's render_board fix with this branch's own clean-cascade control (verified byte-identical default output, so the committed cache still replays). Items A-G from my change request addressed: board renderer routed through render_board, Holm count corrected to 4/8, oracle/deployable tautology labeled and fixed with a clean-control arm, recall-1.0 forced-ness noted, manifest question deferred (tracked separately). Item H (clean_a's control) filed as #394, not blocking. The #389 degeneracy guard flagged 27 findings on this cohort scanned for the first time; all independently re-verified against benchmaxxing.stats.mcnemar and the underlying scripts, none are computation bugs, all now recorded in tests/degeneracy_exemptions.json with the specific reason. All 1117 tests pass.
A seeded turn with no supplied utterance was given a synthetic content default, [seeded] planted answer: 'X'. That was inert until #377 added the show_rationale render path, which now prints Turn.content onto the board -- so the holdout reads the harness's own "this answer is planted" tag, and any referee/judge that scans whole turns (referee._default_shortcut_detector, blind_metric.latch_rate) picks up the marker as if it were stated reasoning. A cannot-fail-by-construction trap (#374 family), armed for the next --show-rationale run (#270/#271/#115/#221). The seeded flag and the committed answer already record the plant, so drop the marker: no supplied content means empty content. anchored_seed and prescreen_flag already pass natural prose for exactly this reason. Pins it two ways: the seeded turn's content stays empty, and render_board(show_rationale=True) does not surface the marker.
Closes #373. This re-runs nothing. It makes #271, #270, #115 and #221 runnable through the blackboard and leaves them open; each needs a fresh cache for its lane. The workshop paper already describes the text lane as bare peer agreement, so no paper claim is waiting on a re-run.
Stacked on #369 (base is
methodology/support2-followups, review that first): theshow_rationaleknob this generalizes only exists there.@sebasmos found 16 renderers. There are 17:
CommitteeAgentinbenchmaxxing/runner.py, which drivesbenchmaxxing run --stage cascade, letters its votes and dropscontentthe same way.The fix
blackboard.render_boardis now the only place that loop lives. Each runner passes its own header and verb, so the default render is unchanged, andshow_rationale(off by default) is what lets a peer'scontentthrough.Byte-identity is verified, not asserted. For all 15 runners that carried the inline loop, the new default render is compared against the expression reconstructed from that same file at HEAD;
support2/_common.run_boardis compared against its old body on both settings. The issue's reproduction still hashes to3899433de146b2bcwith the flag off. Every committed cache replays at zero calls, so no number in any lane moves.With
--show-rationale:An agent's own turns stay bare votes. Quoting an agent's reasoning back at itself is not peer pressure, and in isolated mode its own turn is the only thing on the board.
experiments/cascade/multi_round.pykeeps its own prose rendering (senior colleagues have stated: X; Y) and gates the rationale itself, because routing it through the shared renderer would have changed its committed prompt.What was actually wrong: the labels
Docstrings in
true_peer_control.py,seed_timing.py, bothreferee_deployable.pyandreferee/referee_judge.pypromised a case-anchored rationale the prompt never carried. They now say the committed condition is bare peer agreement, and point at the flag.majority_pressure.pywas already accurate and just gains the pointer.lane_findings.json'sdropped_rationale_pattern_in_other_lanesgoes from "confirmed present, not fixed here" to fixed, with the full 17-file list.Worth stating plainly for
referee_judge.py: the rationale always reached the judge, which reads whole turns, and never the holdout. So that lane's judge was auditing deference to bare agreement. The recall/precision numbers stand; the description of what the holdout was deferring to was wrong.Tests
tests/test_board_render.py. The default render is checked against the inline loop it replaces for both verbs; the issue's repro is replayed throughrun_committeeand the realCommitteeAgent, where a confident rationale and an explicit guess are byte-identical with the flag off and distinct with it on. Two guards stop it coming back: every audited runner must carry the knob, and no module underexperiments/orbenchmaxxing/may rebuild the answer-only board line inline. The guard regex matches all 16 pre-fix files, so it is not vacuous.ruff findings are identical to the pre-change baseline (the repo has 185 either way), and
benchmaxxing smokeis green.On the count, @sebasmos is right that 1078 is not what he sees, and the split is environment-dependent rather than wrong in either direction. 1083 collected is the invariant: 1078 passed / 5 skipped here, 1076 / 7 for him, and the two that differ are optional-extra skips, not tests this PR touches. Against its own base that is 1073 collected -> 1083, exactly the 10 tests added here (11 after the multi-line fix below).
origin/mainis 1056 collected, 1051 / 5 here against his 1049 / 7, so the same 2 differ there too and predate this branch.Review follow-ups
Multi-line rationale (7c165f4). I could not reproduce two blank lines: the block terminates in exactly
\n\nwith the flag on or off, checked across a plain rationale, one with a trailing newline, one with trailing blank lines, and a multi-line reply. But the case behind the observation is real. On the live-peer lanescontentis a real reply, stored astext[:150], so it routinely arrives multi-line: its continuation lines sat at the margin and read as fresh board entries, and a reply containing a blank line made the board look like it had ended before the remaining peers. Every rationale line is now indented under its vote and blank lines are dropped. Only the flag-on path changes; the 15 runners are still byte-identical and the repro still hashes to3899433de146b2bc.Caveats
config.show_rationaleis a newConfigfield, so it appears in every run'sconfig.json. That is deliberate (a run should state which stimulus it used) but it does change the artifact shape.mimic_cxr_textrunners still needPYTHONPATH=.to run as scripts. Pre-existing, untouched here.🤖 Generated with Claude Code