diff --git a/docs/JOURNAL.md b/docs/JOURNAL.md index 13a8715..0e35963 100644 --- a/docs/JOURNAL.md +++ b/docs/JOURNAL.md @@ -18,6 +18,40 @@ protocol. **Newest entries at the top.** Tag each entry with one or more of: --- +## 2026-07-24 — RLPR WebInstruct math grader ignored \boxed{...}, scoring correct answers 0 #mistake #fix #repro +**Context:** issue #170 — `_check_rlpr_webinstruct` in `src/trinity/orchestration/reward.py` scores +WebInstruct-verified RLPR items (`WebInstruct-verified-val_Avg2`). +**Expected:** a math item answered in the requested `\boxed{...}` format grades 1.0 when correct. +**Actual:** it scored 0.0. The choice-letter branch aside, the checker fed the **raw candidate +prose** straight into `math_equal` / `normalize_math_answer`, neither of which strips `\boxed{...}`: +```python +from trinity.orchestration import reward as R +ref = {"ground_truth": "15", "source": "WebInstruct-verified-val_Avg2"} +R.score_text("rlpr", r"The area is \boxed{15}.", ref) # -> 0.0 (should be 1.0) +R.score_text("rlpr", "The answer is 15", ref) # -> 1.0 (plain number worked) +R.score_text("math500", r"The area is \boxed{15}.", "15") # -> 1.0 (math path extracts the box) +``` +**Root cause:** unlike `_check_math`, which calls `extract_boxed` / `extract_last_number` before +comparing, this path skipped extraction. `normalize_math_answer` only strips a leading "answer is" +prefix, so a plain trailing number happened to match but a boxed one never did — even though +`format_hint("rlpr")` explicitly tells the worker to "use a boxed final answer for math items". So +the *requested* output format was the one graded wrong: every boxed WebInstruct math answer was a +false negative, understating that benchmark's accuracy and corrupting its reward signal. +**Fix / decision:** extract before the compare, mirroring `_check_math` exactly: +`cand_math = extract_boxed(cand) or extract_last_number(cand) or cand` and +`gold_math = extract_boxed(gold) or gold`, then `math_equal(cand_math, gold_math)`. The `or cand` +fallback is load-bearing: when the candidate has neither a box nor a number (a free-text answer), +`cand_math` is the full candidate, so non-math text answers grade exactly as before — the behaviour +only diverges when an answer is actually extracted. Distinct from #116/#122, which is the RLPR +**MMLU-Pro choice** path (letters E–J); this is the **WebInstruct math** branch. Added +`tests/test_reward_rlpr_webinstruct_boxed.py` (offline): the boxed repro, a boxed value winning over +a stray earlier number, the number-in-prose fallback, a boxed LaTeX fraction, a boxed gold, plus +guards that the plain-number, free-text, and letter-choice paths are unchanged (5 of the cases fail +on the old code; the 10 guards pass either way). Full root suite: 261 passed. +**Follow-up:** the number-extraction tradeoff is inherited from `_check_math` — an unboxed text gold +with a trailing number (e.g. `"15 apples"` restated verbatim) can lose its unit; boxing the answer, +which is what the format hint asks for, sidesteps it. + ## 2026-07-12 — code grader: add resource limits on top of the HOME/secrets fix #security #decision **Context:** issue #71 — the code grader (`run_pass_at_1`) runs untrusted miner/LLM candidate code. The core secret-leak fix (isolated throwaway HOME/cwd, scrubbed env, `python -I`) already landed on main. **Finding:** main's sandbox closes the HOME/secrets exfiltration vector but has **no resource limits** — an untrusted candidate can still exhaust host memory or fork-bomb the eval box within its wall-clock timeout (verified: a 4 GiB `bytearray` allocation runs to completion and "passes" on main). diff --git a/src/trinity/orchestration/reward.py b/src/trinity/orchestration/reward.py index b1da624..7b5824d 100644 --- a/src/trinity/orchestration/reward.py +++ b/src/trinity/orchestration/reward.py @@ -481,10 +481,16 @@ def _check_rlpr_webinstruct(candidate: str, reference: object) -> bool: if gold_letter is not None and cand_letter is not None: return cand_letter == gold_letter - if math_equal(cand, gold): + # Math items: extract the answer before comparing, mirroring `_check_math`. + # WebInstruct math items are answered in the requested `\boxed{...}` format + # (see `format_hint("rlpr")`), but feeding the raw candidate prose straight + # into `math_equal` never strips the box, so a correct boxed answer scored 0. + cand_math = extract_boxed(cand) or extract_last_number(cand) or cand + gold_math = extract_boxed(gold) or gold + if math_equal(cand_math, gold_math): return True - return normalize_math_answer(cand) == normalize_math_answer(gold) + return normalize_math_answer(cand_math) == normalize_math_answer(gold_math) # --------------------------------------------------------------------------- diff --git a/tests/test_reward_rlpr_webinstruct_boxed.py b/tests/test_reward_rlpr_webinstruct_boxed.py new file mode 100644 index 0000000..cefa229 --- /dev/null +++ b/tests/test_reward_rlpr_webinstruct_boxed.py @@ -0,0 +1,97 @@ +r"""Offline tests for boxed-answer grading on RLPR WebInstruct math items (#170). + +`_check_rlpr_webinstruct` fed the raw candidate prose straight into `math_equal`, +which never strips `\boxed{...}` — so a WebInstruct math item answered in the +requested boxed format (`format_hint("rlpr")` explicitly asks for it) scored 0 +even when correct. The fix extracts the answer before the math compare, mirroring +`_check_math`. These tests pin the fix and guard that the letter and plain-text +paths are unchanged. Pure functions; no GPU/network. +""" +from __future__ import annotations + +import pytest + +import trinity.orchestration.reward as R + +_SOURCE = "WebInstruct-verified-val_Avg2" + + +def _score(candidate: str, gold: object) -> float: + return R.score_text("rlpr", candidate, {"ground_truth": gold, "source": _SOURCE}) + + +# --------------------------------------------------------------------------- # +# The bug: a correct boxed answer was scored 0 +# --------------------------------------------------------------------------- # +def test_boxed_answer_now_grades_correct(): + # Exact repro from the issue: was 0.0 before the fix. + assert _score(r"The area is \boxed{15}.", "15") == 1.0 + + +def test_wrong_boxed_answer_still_fails(): + assert _score(r"The area is \boxed{16}.", "15") == 0.0 + + +def test_boxed_answer_beats_other_numbers_in_the_prose(): + # extract_boxed wins over a stray earlier number, so the committed \boxed{} + # value is what gets graded — not the first number mentioned. + assert _score(r"We ran 3 trials; the final answer is \boxed{15}.", "15") == 1.0 + + +def test_number_in_prose_without_a_box_is_extracted(): + # No box, but a trailing number: extract_last_number recovers it. + assert _score("The area of the triangle is 15 square units.", "15") == 1.0 + + +def test_boxed_latex_fraction_matches_plain_fraction(): + assert _score(r"Hence \boxed{\frac{1}{2}}.", "1/2") == 1.0 + + +def test_gold_may_itself_be_boxed(): + # Datasets vary: a boxed gold is unwrapped before comparison. + assert _score(r"\boxed{15}", r"\boxed{15}") == 1.0 + + +# --------------------------------------------------------------------------- # +# Guards: paths that must NOT change +# --------------------------------------------------------------------------- # +def test_plain_number_answer_unchanged(): + # Already worked before the fix (via the "answer is" prefix strip); must stay. + assert _score("The answer is 15", "15") == 1.0 + assert _score("The answer is 14", "15") == 0.0 + + +def test_free_text_answer_unchanged(): + # No box and no number -> candidate falls back to itself, so non-math text + # answers grade exactly as before (the `or cand` fallback). + assert _score("The answer is Paris", "Paris") == 1.0 + assert _score("The answer is London", "Paris") == 0.0 + + +def test_letter_choice_branch_unchanged(): + # A gold letter + a committed letter still routes through the choice compare. + assert _score("The answer is (A).", "A") == 1.0 + assert _score("The answer is (B).", "A") == 0.0 + + +# --------------------------------------------------------------------------- # +# Direct unit calls on the checker +# --------------------------------------------------------------------------- # +@pytest.mark.parametrize( + "candidate,gold,expected", + [ + (r"\boxed{15}", "15", True), + (r"\boxed{15}", "16", False), + ("42", "42", True), + ("41", "42", False), + ("Paris", "Paris", True), + ], +) +def test_check_rlpr_webinstruct_direct(candidate, gold, expected): + assert R._check_rlpr_webinstruct(candidate, gold) is expected + + +def test_empty_candidate_or_gold_is_false(): + assert R._check_rlpr_webinstruct("", "15") is False + assert R._check_rlpr_webinstruct(r"\boxed{15}", "") is False + assert R._check_rlpr_webinstruct(r"\boxed{15}", None) is False