You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
experiments/medqa/reproduce.py has its own hand-written _parse_choice(text, options) function, separate from the shared benchmaxxing/extract.py::parse_mcq_choice. This is the function that produces every MedQA number currently in the paper (solo flip rates, cascade contagion, referee precision/recall, everything downstream of reproduce.py's output). It has no dedicated, committed test suite of its own; its correctness has only ever been checked by hand, ad hoc, when something went wrong.
This is not hypothetical risk. PR #342 attempted to replace _parse_choice with the shared parse_mcq_choice (behind parse_legacy_string), as part of centralizing the #102 parser refactor. Independent review from two people (myself and @maximinl) plus my own empirical diff against all 6,054 real cached MedQA responses found the new shared parser disagrees with the old one on ~5.5% of real responses, including cases where it silently picks the wrong (always-last-mentioned) option when a model writes "The correct answer is D." in bold, a common Gemini formatting style. This was only caught because someone happened to run an ad hoc empirical diff against the real cache; there is no standing test that would catch a similar regression automatically on the next refactor.
Compounding this, tests/test_parse_regression.py (added in #342 as the intended regression guard) collects zero actual test_* functions, so pytest silently runs nothing from it. The intent to guard this exists; the guard itself does not.
Why this matters for the paper
reproduce.py's parser is the single most paper-critical piece of code in the repository: every MedQA number in the current draft depends on it parsing real Gemini responses correctly. Right now the only thing standing between a future refactor and silently corrupted paper numbers is whoever happens to notice and manually re-run the kind of empirical diff done for #342. That is not a repeatable safeguard.
What "done" looks like
A real, collected (test_* named) pytest suite for reproduce.py's answer-parsing function, covering: bare letter ("D"), \boxed{X} and \boxed{\text{X. ...}} forms, explicit declarations in various phrasings ("The correct answer is D.", "the answer is: D"), multi-option-text-match ambiguity (a long chain-of-thought response that names more than one option before concluding), and the option-text-named-at-the-end case.
At least a sample of the real cached responses in experiments/medqa/results/call_cache.jsonl (or a frozen, committed subset of them) used as real-world fixtures, not only synthetic examples, since the synthetic-only test in fix(parser): centralize MCQ extraction and resolve MedQA bug (Issue #102) #342 (tests/test_parse_regression.py) is exactly what missed the multi-match fall-through bug.
The suite must fail loudly (not silently collect zero tests) if it is ever renamed, moved, or its test functions are dropped; consider a CI check that asserts a minimum test count for parser-related test files.
The fixtures must be drawn from real cached Gemini responses already in the repo, not fabricated text, so the suite actually exercises the failure modes real model output produces.
Related: PR #342 (the attempted swap that surfaced this gap), the shared benchmaxxing/extract.py::parse_mcq_choice.
Problem
experiments/medqa/reproduce.pyhas its own hand-written_parse_choice(text, options)function, separate from the sharedbenchmaxxing/extract.py::parse_mcq_choice. This is the function that produces every MedQA number currently in the paper (solo flip rates, cascade contagion, referee precision/recall, everything downstream ofreproduce.py's output). It has no dedicated, committed test suite of its own; its correctness has only ever been checked by hand, ad hoc, when something went wrong.This is not hypothetical risk. PR #342 attempted to replace
_parse_choicewith the sharedparse_mcq_choice(behindparse_legacy_string), as part of centralizing the #102 parser refactor. Independent review from two people (myself and @maximinl) plus my own empirical diff against all 6,054 real cached MedQA responses found the new shared parser disagrees with the old one on ~5.5% of real responses, including cases where it silently picks the wrong (always-last-mentioned) option when a model writes "The correct answer is D." in bold, a common Gemini formatting style. This was only caught because someone happened to run an ad hoc empirical diff against the real cache; there is no standing test that would catch a similar regression automatically on the next refactor.Compounding this,
tests/test_parse_regression.py(added in #342 as the intended regression guard) collects zero actualtest_*functions, so pytest silently runs nothing from it. The intent to guard this exists; the guard itself does not.Why this matters for the paper
reproduce.py's parser is the single most paper-critical piece of code in the repository: every MedQA number in the current draft depends on it parsing real Gemini responses correctly. Right now the only thing standing between a future refactor and silently corrupted paper numbers is whoever happens to notice and manually re-run the kind of empirical diff done for #342. That is not a repeatable safeguard.What "done" looks like
test_*named) pytest suite forreproduce.py's answer-parsing function, covering: bare letter ("D"),\boxed{X}and\boxed{\text{X. ...}}forms, explicit declarations in various phrasings ("The correct answer is D.", "the answer is: D"), multi-option-text-match ambiguity (a long chain-of-thought response that names more than one option before concluding), and the option-text-named-at-the-end case.experiments/medqa/results/call_cache.jsonl(or a frozen, committed subset of them) used as real-world fixtures, not only synthetic examples, since the synthetic-only test in fix(parser): centralize MCQ extraction and resolve MedQA bug (Issue #102) #342 (tests/test_parse_regression.py) is exactly what missed the multi-match fall-through bug.reproduce.py's parser (like fix(parser): centralize MCQ extraction and resolve MedQA bug (Issue #102) #342) can point to a passing/failing suite instead of requiring a manual ad hoc diff.Real-data rule
The fixtures must be drawn from real cached Gemini responses already in the repo, not fabricated text, so the suite actually exercises the failure modes real model output produces.
Related: PR #342 (the attempted swap that surfaced this gap), the shared
benchmaxxing/extract.py::parse_mcq_choice.