diff --git a/CHANGELOG.md b/CHANGELOG.md index ef83812d..ed9f7092 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -180,6 +180,10 @@ All notable changes to SkillOpt are documented here. This project adheres to Code, Codex and Pi transcript timestamps as instants rather than strings. East of UTC, sessions that ended in the hours after a harvest were never harvested (thanks @1AbdullahButt). +- Keep numeric `0` gold answers and ids when loading OfficeQA JSON splits + instead of treating them as missing, and load integral floats such as + `1776.0` as `1776`, so a correct integer answer is no longer scored wrong + (thanks @Shizoqua). ### Security - Prevent managed-identity credentials from being sent to non-Azure or diff --git a/skillopt/envs/officeqa/dataloader.py b/skillopt/envs/officeqa/dataloader.py index a9c22b46..c9915ecb 100644 --- a/skillopt/envs/officeqa/dataloader.py +++ b/skillopt/envs/officeqa/dataloader.py @@ -29,10 +29,27 @@ def _parse_list_field(value: str | list[str] | None) -> list[str]: return [text] +def _first_text(row: dict, *keys: str) -> str: + """Return the first field that is present and non-blank, as stripped text. + + Unlike ``row.get(a) or row.get(b)``, a numeric ``0`` from JSON counts as + present instead of being skipped as falsy. An integral float such as + ``1776.0`` (a JSON float column) is written as ``"1776"`` so it matches an + integer answer. + """ + for key in keys: + value = row.get(key) + if isinstance(value, float) and value.is_integer(): + value = int(value) + if value is not None and str(value).strip(): + return str(value).strip() + return "" + + def _normalize_row(row: dict[str, str]) -> dict: - item_id = str(row.get("uid") or row.get("id") or "").strip() + item_id = _first_text(row, "uid", "id") question = str(row.get("question") or "").strip() - ground_truth = str(row.get("ground_truth") or row.get("answer") or "").strip() + ground_truth = _first_text(row, "ground_truth", "answer") task_type = str(row.get("category") or row.get("difficulty") or "officeqa").strip() or "officeqa" source_files = _parse_list_field(row.get("source_files")) source_docs = _parse_list_field(row.get("source_docs")) diff --git a/tests/test_officeqa_dataloader.py b/tests/test_officeqa_dataloader.py new file mode 100644 index 00000000..0bea2304 --- /dev/null +++ b/tests/test_officeqa_dataloader.py @@ -0,0 +1,67 @@ +"""OfficeQA split loading: gold answers that are numeric zero must survive.""" + +from __future__ import annotations + +import json + +import pytest + +from skillopt.envs.officeqa.dataloader import OfficeQADataLoader +from skillopt.envs.officeqa.evaluator import evaluate + + +def _load(tmp_path, rows: list[dict]) -> list[dict]: + split = tmp_path / "val" + split.mkdir() + (split / "items.json").write_text(json.dumps(rows), encoding="utf-8") + return OfficeQADataLoader().load_split_items(str(split)) + + +@pytest.mark.parametrize(("gold", "expected"), [(0, "0"), (0.0, "0"), ("0", "0")]) +def test_zero_ground_truth_is_kept(tmp_path, gold, expected): + (item,) = _load(tmp_path, [{"uid": "q1", "question": "How many bonds were issued?", "ground_truth": gold}]) + + assert item["ground_truth"] == expected + assert item["answers"] == [expected] + assert evaluate(expected, item["ground_truth"])["em"] == 1.0 + + +@pytest.mark.parametrize(("gold", "expected"), [(1776.0, "1776"), (12.5, "12.5"), (-3.0, "-3"), ("1776.0", "1776.0")]) +def test_integral_float_ground_truth_loads_as_an_integer(tmp_path, gold, expected): + # A float column written by e.g. pandas to_json stores 1776 as 1776.0. + (item,) = _load(tmp_path, [{"uid": "q1", "question": "Year?", "ground_truth": gold}]) + + assert item["ground_truth"] == expected + assert evaluate(expected, item["ground_truth"])["em"] == 1.0 + + +def test_numeric_zero_id_is_kept(tmp_path): + items = _load( + tmp_path, + [ + {"uid": 0, "question": "Year?", "ground_truth": "1776"}, + {"id": 0, "question": "Year?", "ground_truth": "1789"}, + ], + ) + + assert [(item["id"], item["uid"]) for item in items] == [("0", "0"), ("0", "0")] + + +def test_answer_field_is_used_when_ground_truth_is_missing_or_empty(tmp_path): + items = _load( + tmp_path, + [ + {"uid": "q1", "question": "Year?", "answer": "1776"}, + {"uid": "q2", "question": "Year?", "ground_truth": "", "answer": 0}, + {"uid": "q3", "question": "Year?", "ground_truth": None, "answer": "1789"}, + ], + ) + + assert [item["ground_truth"] for item in items] == ["1776", "0", "1789"] + + +def test_missing_gold_answer_stays_empty(tmp_path): + (item,) = _load(tmp_path, [{"uid": "q1", "question": "Year?"}]) + + assert item["ground_truth"] == "" + assert item["answers"] == []