Skip to content

No committed regression test guards reproduce.py's MedQA answer parser (the function behind every paper MedQA number) #345

Description

@sebasmos

Problem

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.
  • Once this exists, any future proposal to swap 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.

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingpriority: highDo this soon; unblocks the paper or other workstats

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions