From 61c7cdb65f0bdad431f6af6f58584ec07a459545 Mon Sep 17 00:00:00 2001 From: Lanre Shittu <136805224+Shizoqua@users.noreply.github.com> Date: Wed, 11 Nov 2026 18:53:06 +0000 Subject: [PATCH 1/2] fix(officeqa): keep a numeric 0 gold answer when loading JSON splits _normalize_row read the gold answer with str(row.get("ground_truth") or row.get("answer") or ""). A JSON split can store a numeric answer, and 0 or 0.0 is falsy, so it was skipped: the item loaded with an empty ground_truth and no answers, and a model answering "0" scored EM 0 and F1 0. Take the first field that is present and not blank instead. String values, the "answer" fallback and items with no gold answer behave as before; a whitespace-only ground_truth now also falls back to "answer". --- CHANGELOG.md | 3 ++ skillopt/envs/officeqa/dataloader.py | 15 ++++++++- tests/test_officeqa_dataloader.py | 46 ++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/test_officeqa_dataloader.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ef83812d9..5a6631ada 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -180,6 +180,9 @@ 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 an OfficeQA gold answer of numeric `0` when loading JSON splits instead + of treating it as missing, which left the item with an empty gold answer and + scored a correct `0` as 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 a9c22b466..9efd30851 100644 --- a/skillopt/envs/officeqa/dataloader.py +++ b/skillopt/envs/officeqa/dataloader.py @@ -29,10 +29,23 @@ 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. + """ + for key in keys: + value = row.get(key) + 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() 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 000000000..43f626074 --- /dev/null +++ b/tests/test_officeqa_dataloader.py @@ -0,0 +1,46 @@ +"""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", "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 + + +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"] == [] From ac950fa75c7b3b195a575186a9b667e50bd42297 Mon Sep 17 00:00:00 2001 From: Lanre Shittu <136805224+Shizoqua@users.noreply.github.com> Date: Sat, 14 Nov 2026 08:12:55 +0000 Subject: [PATCH 2/2] fix(officeqa): load integral float answers as integers and keep id 0 Two follow-ups from review of the numeric 0 gold answer fix: - A JSON float column stores 1776 as 1776.0. str() turned it into "1776.0", which the evaluator compares with "1776" character by character, so a correct integer answer still scored EM 0. Integral floats are now written as integers; non-integral floats and string values are unchanged. - The item id used the same `or` chain, so "uid": 0 or "id": 0 produced an empty id. It now goes through _first_text as well. --- CHANGELOG.md | 7 ++++--- skillopt/envs/officeqa/dataloader.py | 8 ++++++-- tests/test_officeqa_dataloader.py | 23 ++++++++++++++++++++++- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a6631ada..ed9f70926 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -180,9 +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 an OfficeQA gold answer of numeric `0` when loading JSON splits instead - of treating it as missing, which left the item with an empty gold answer and - scored a correct `0` as wrong (thanks @Shizoqua). +- 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 9efd30851..c9915ecb3 100644 --- a/skillopt/envs/officeqa/dataloader.py +++ b/skillopt/envs/officeqa/dataloader.py @@ -33,17 +33,21 @@ 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. + 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 = _first_text(row, "ground_truth", "answer") task_type = str(row.get("category") or row.get("difficulty") or "officeqa").strip() or "officeqa" diff --git a/tests/test_officeqa_dataloader.py b/tests/test_officeqa_dataloader.py index 43f626074..0bea2304b 100644 --- a/tests/test_officeqa_dataloader.py +++ b/tests/test_officeqa_dataloader.py @@ -17,7 +17,7 @@ def _load(tmp_path, rows: list[dict]) -> list[dict]: return OfficeQADataLoader().load_split_items(str(split)) -@pytest.mark.parametrize(("gold", "expected"), [(0, "0"), (0.0, "0.0"), ("0", "0")]) +@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}]) @@ -26,6 +26,27 @@ def test_zero_ground_truth_is_kept(tmp_path, gold, 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,