Skip to content

feat: MedMCQA dataset adapter (closes #112) - #142

Merged
sebasmos merged 1 commit into
mainfrom
feat/dataset-medmcqa
Jul 22, 2026
Merged

feat: MedMCQA dataset adapter (closes #112)#142
sebasmos merged 1 commit into
mainfrom
feat/dataset-medmcqa

Conversation

@felipeocampoos

Copy link
Copy Markdown
Collaborator

What

Implements benchmaxxing/datasets/medmcqa.py, a text-lane (MCQ) dataset adapter for MedMCQA (Pal et al. 2022), giving Lane B a second source alongside MedQA.

  • Maps question -> Case.question, opa/opb/opc/opd -> an ordered Case.options tuple, cop (0-3) -> Case.answer_index directly, and subject_name/topic_name -> Case.meta.
  • Accepts the raw file itself or a directory containing dev.json (JSON/JSONL, one record per line).
  • Raises a clear FileNotFoundError on missing data and ValueError on an out-of-range cop -- no fabricated rows.
  • case_id uses a provided id (id/question_id/case_id) if present, else a generated medmcqa-{index}, matching the medqa adapter's convention.
  • Registered in registry.py, so benchmaxxing datasets now lists medmcqa.

Follows the same adapter pattern as benchmaxxing/datasets/medqa.py (from #66), which the issue names as the reference.

Note on Case.meta

Case.meta is populated by the adapter, but the manifest round trip (write_manifest/load_cases) does not yet preserve the meta column on main -- that support lands in #100 (already reviewed/approved, not yet merged). The meta-mapping test therefore checks Case.meta directly off _case_from_obj's output rather than after a manifest round trip, so it isn't coupled to an unmerged PR. Once #100 lands, the meta column will round-trip for this adapter automatically -- no further change needed here.

Testing

  • New tests/test_medmcqa_adapter.py: fixture round trip (options order, answer_index, case_id generation), directory resolution, limit, a provided id, missing-data FileNotFoundError, out-of-range cop ValueError, and the meta mapping.
  • Updated tests/test_datasets.py: EXPECTED_DATASETS now includes medmcqa; test_all_adapters_build_manifest_raise accepts FileNotFoundError too, since medmcqa (implemented, unlike the remaining stub adapters) raises that instead of NotImplementedError on an empty raw_root.
  • Suite: 403 passed, 6 skipped, ruff check . clean. The one failure I see locally (test_checksum_is_stable_and_matches_hashlib) is pre-existing on main (Windows CRLF vs a literal-string hash fixture) and unrelated to this change.
  • Manual round trip verified: a one-row manifest builds and reloads with options[answer_index] equal to the correct choice.

Closes #112.

Copilot AI review requested due to automatic review settings July 20, 2026 14:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sebasmos

Copy link
Copy Markdown
Member

Thanks @felipeocampoos, MedMCQA is a welcome second Lane-B source. Two things before it can land: (1) a merge conflict in tests/test_datasets.py against current main, and (2) ruff check reports ~17 errors on the branch. Could you rebase onto main, run ruff check benchmaxxing tests + pytest, and push? Happy to review right after.

@Yehudha-kennedy Yehudha-kennedy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased on main and tests pass. Ruff check is completely clean too. Great work, merging this now!

@Yehudha-kennedy
Yehudha-kennedy self-requested a review July 22, 2026 01:50
Implements benchmaxxing/datasets/medmcqa.py mapping MedMCQA's JSON/JSONL rows
(question, opa..opd, cop, subject_name, topic_name) to the shared Case schema,
following the same pattern as the medqa adapter: opa..opd become an ordered
options tuple, cop maps directly to answer_index, and subject_name/topic_name
flow into Case.meta. Registers the adapter in registry.py, so `benchmaxxing
datasets` lists medmcqa alongside the existing five.

Tests cover the fixture round trip, directory resolution, limit, provided ids,
missing-data and out-of-range-cop errors, and the meta mapping (checked on the
Case object directly, since the manifest round trip does not yet preserve meta
on main -- that lands separately in PR #100).

Suite: 403 passed, 6 skipped, ruff clean (the one pre-existing failure,
test_checksum_is_stable_and_matches_hashlib, is a Windows CRLF issue on main,
unrelated to this change).
@felipeocampoos

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main -- the conflict in tests/test_datasets.py is resolved (medmcqa now sits alongside pubmedqa in EXPECTED_DATASETS and the registry). ruff check . is clean (no errors, the earlier ~17 were from a stale branch state that predated the rebase). 589 passed, 6 skipped. Ready for another look.

@sebasmos

Copy link
Copy Markdown
Member

Verified independently after the rebase: diff vs main is purely additive (medmcqa.py + registry + tests, no deletions), 588 passed, 7 skipped, ruff clean. Merging on top of @Yehudha-kennedy's approval.

@sebasmos
sebasmos merged commit f1553a1 into main Jul 22, 2026
@sebasmos
sebasmos deleted the feat/dataset-medmcqa branch July 24, 2026 00:13
sebasmos added a commit that referenced this pull request Aug 4, 2026
feat: MedMCQA dataset adapter (closes #112)
sebasmos added a commit that referenced this pull request Aug 4, 2026
…corrected (closes #131)

Rebased fresh off current main (the original branch predated #142/#143/#146/#148/#154/#157/
#161/#219 and would have deleted all of that merged work if landed as-is).

Addresses @Agastya191's review on #159:
- flash_lite_reference was hardcoded to scale_c's PRE-parser-fix numbers (0.33/0.51 at n=150,
  p<1e-4). Now reads dynamically from --scale-c-summary (scale_c's committed summary, PR #141)
  so it cannot drift out of sync with a future fix there. Regenerated: 0.729/0.847 at n=85,
  p=0.041.
- Also caught in the same pass: the docstring and README's own flash numbers were ALSO stale
  (60 hard cases, generic 0.10/anchored 0.083) versus the actually-committed, already-corrected
  summary (28 hard cases, generic 0.714/anchored 0.679). Rewrote both to match the real data
  and restated the finding precisely: both tiers conform substantially to a bare peer (~0.71-0.73),
  but only flash-lite's conformity climbs further under a case-anchored rationale (+0.12, p=0.041);
  flash's does not move. The anchoring lever is model-dependent, not conformity itself.

Verified end-to-end: keyless reproduction confirmed with the key unset (new_api_calls_this_run: 0,
n_hard_cases: 28, matches exactly), ruff clean, 625 tests pass on the rebased base, no hardcoded
personal paths, 0 em dashes.
sebasmos added a commit that referenced this pull request Aug 4, 2026
…g subgroup (closes #185, closes #214)

Pure re-analysis of the already-committed imaging cascade transcripts, zero API calls.

#185: quantifies claim 4 (contagion rides on case plausibility, not the cue's own solo potency).
Spearman of solo flip-above-noise vs cascade contagion across the four cues is -1.0 (n=4,
descriptive): the weakest solo cue (cable) has the strongest cascade, the opposite of what a
cue-potency account predicts. The sharper test, per-case cross-cue agreement across the shared
35 cases, shows corner-tag/watermark/laterality adopt on the EXACT same cases (phi=1.0,
Jaccard=1.0); Cochran's Q across all four cues is not significant (p=0.392). Both support a
case-driven, not cue-driven, account of contagion.

#214: per-finding contagion breakdown (exploratory, n=35, cells as small as 1). No finding is
categorically immune or uniquely susceptible at this sample size; Wilson intervals are wide and
overlapping. Deliberately does not force a paired test across findings, since different findings
involve different cases (not a repeated-measures design), which would be an invalid comparison.

Also fixed along the way: PR #166's branch had gone stale relative to current main (predated
#219/#142/#143 merged since it was opened); merged main into the same branch in place (zero
reviews yet, so no review thread to disrupt) and re-verified keyless reproduction. This PR is
stacked on top of #166 since it needs the imaging results that only exist there.

Verified: ruff clean, 625 tests pass, no hardcoded personal paths, 0 em dashes, purely additive
(no existing file modified).
sebasmos added a commit that referenced this pull request Aug 4, 2026
…loses #108)

Rebased fresh off current main (the original branch predated #142/#143/#146/#148/#154/#157/
#161/#219 and would have deleted all of that merged work if landed as-is, same stale-base
issue as #143/#150/#141/#159).

Addresses @Agastya191's review on #158:
- README/PR body had stale pre-fix numbers (accuracy 0.29/0.14, flash flip 0.76 vs 1.00 at
  p=9.8e-5) versus the already-corrected committed summary (accuracy 0.89/0.78, flash flip
  0.079 vs 0.455 at p=3.3e-3, flash-lite 0.090 vs 0.636 at p=4.4e-7). Rewrote to match.
- ever_flipped collapsed the three cues into a per-case boolean that doesn't match the stated
  headline flip rate (0.063/0.117). Now reports both per_record_flip_rate (the headline
  quantity) and ever_flipped_rate, clearly labeled and distinguished.
- Grading-standard mismatch: clean_correct was imported from the prior solo pipeline rather
  than re-verified. Now re-graded by exact match against ground truth, the same standard as
  options_only_correct. Re-graded values are identical to the imported ones (0.89/0.78), so no
  actual discrepancy, but the standard is now transparent and consistent.
- Cache race: _Cache.complete checked the store under lock but called the API outside it, so
  two threads could both miss the same key and both append a duplicate entry (confirmed: the
  committed cache had exactly one). Lock now spans the full miss-check-fetch-store sequence.
  Cache also pruned from 6054 shared-cache entries down to the 400 this script actually
  requests (100 cases x 2 models x 2 probes).

Verified end-to-end: keyless reproduction confirmed with the key unset (new_api_calls_this_run:
0, all numbers match exactly), ruff clean, 625 tests pass, no hardcoded personal paths, 0 em
dashes.
sebasmos added a commit that referenced this pull request Aug 4, 2026
Rebased fresh off current main (the original branch predated #142/#143/#146/#148/#154/#157/
#161/#219 and would have deleted all of that merged work if landed as-is, same stale-base
issue as #143/#150/#141/#159/#158).

Addresses @Agastya191's review on #155:
- README/PR body carried pre-fix numbers (shared 0.175/0.15/0.175/0.15/0.15, p=1.0) versus the
  already-corrected committed summary (0.275/0.3/0.275/0.325/0.325, gained 3 lost 1 p=0.625).
  Rewrote to match and restated the finding as no significant compounding at p=0.625.
- Real bug: the isolated holdout's prompt was byte-identical to the bare prompt every round
  (board text was built only from peer votes, which are never visible in isolated mode), so at
  temperature 0 it deterministically reproduced round 1's answer every round. Isolated adoption
  was 0 by construction, not by measurement. Fixed by reminding the isolated holdout of its own
  previous-round answer (visible to it in isolated mode; only peer turns are hidden), so the
  prompt genuinely differs round to round. The shared arm's board construction is UNCHANGED, so
  the shared numbers are identical to what was already reviewed; only the isolated arm is new.

Result: isolated adoption is now a genuine (if still near-floor) measurement: 0.0/0.025/0.0/
0.025/0.0, at most 1 of 40 cases per round, rather than a trivial constant 0.0.

Verified end-to-end: ran for real with the isolated fix (206 new calls), then re-verified keyless
reproduction with the key unset (new_api_calls_this_run: 0, exact match on both curves), ruff
clean, 629 tests pass, no hardcoded personal paths, 0 em dashes.
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.

Dataset adapter: MedMCQA (text/MCQ)

5 participants