validate gold file parsing and add tests

tests/test_evaluate.py

@@ -8,9 +8,13 @@
 
 from __future__ import annotations
 
+import json
+import tempfile
+from pathlib import Path
+
 import pytest
 
-from vaultrag.evaluate import CaseResult, EvalReport, GoldCase, diff, run_eval
+from vaultrag.evaluate import CaseResult, EvalReport, GoldCase, diff, load_gold, run_eval
 from vaultrag.generate import FakeLLM
 
 _ANSWERS = FakeLLM('{"answer": "The engineering bonus is 10% of base.", "cited": [1], "conflict": false}')
@@ -167,3 +171,202 @@ def test_diff_reports_a_fix():
 def test_diff_reports_no_change_when_nothing_moved():
     r = [CaseResult("c1", "alice", recall=1.0)]
     assert diff(_report("v1", r), _report("v2", list(r)))["verdict"] == "NO CHANGE"
+
+
+# ----------------------------------------------------------------- gold file parsing & validation
+
+
+def _write_temp_gold(content: str | dict) -> str:
+    with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False, encoding="utf-8") as f:
+        if isinstance(content, str):
+            f.write(content)
+        else:
+            json.dump(content, f)
+        return f.name
+
+
+def test_load_gold_invalid_json_syntax():
+    tmp_path = _write_temp_gold("{ invalid json: ")
+    try:
+        with pytest.raises(ValueError, match="Invalid JSON"):
+            load_gold(tmp_path)
+    finally:
+        Path(tmp_path).unlink(missing_ok=True)
+
+
+def test_load_gold_missing_cases_key():
+    tmp_path = _write_temp_gold({"not_cases": []})
+    try:
+        with pytest.raises(ValueError, match="missing required 'cases' key"):
+            load_gold(tmp_path)
+    finally:
+        Path(tmp_path).unlink(missing_ok=True)
+
+
+def test_load_gold_cases_not_a_list():
+    tmp_path = _write_temp_gold({"cases": "not a list"})
+    try:
+        with pytest.raises(ValueError, match="'cases' must be a list"):
+            load_gold(tmp_path)
+    finally:
+        Path(tmp_path).unlink(missing_ok=True)
+
+
+def test_load_gold_root_not_dict():
+    tmp_path = _write_temp_gold(["not", "a", "dict"])
+    try:
+        with pytest.raises(ValueError, match="root must be a JSON object"):
+            load_gold(tmp_path)
+    finally:
+        Path(tmp_path).unlink(missing_ok=True)
+
+
+def test_load_gold_case_not_a_dict():
+    tmp_path = _write_temp_gold({"cases": ["not-a-dict"]})
+    try:
+        with pytest.raises(ValueError, match="must be a dictionary"):
+            load_gold(tmp_path)
+    finally:
+        Path(tmp_path).unlink(missing_ok=True)
+
+
+def test_load_gold_case_missing_required_field():
+    # missing id
+    tmp_path = _write_temp_gold(
+        {
+            "cases": [
+                {
+                    "user_id": "alice",
+                    "question": "what is the policy?",
+                }
+            ]
+        }
+    )
+    try:
+        with pytest.raises(ValueError, match="Missing required field 'id'"):
+            load_gold(tmp_path)
+    finally:
+        Path(tmp_path).unlink(missing_ok=True)
+
+    # missing user_id
+    tmp_path2 = _write_temp_gold(
+        {
+            "cases": [
+                {
+                    "id": "c1",
+                    "question": "what is the policy?",
+                }
+            ]
+        }
+    )
+    try:
+        with pytest.raises(ValueError, match="Missing required field 'user_id'"):
+            load_gold(tmp_path2)
+    finally:
+        Path(tmp_path2).unlink(missing_ok=True)
+
+    # missing question
+    tmp_path3 = _write_temp_gold(
+        {
+            "cases": [
+                {
+                    "id": "c1",
+                    "user_id": "alice",
+                }
+            ]
+        }
+    )
+    try:
+        with pytest.raises(ValueError, match="Missing required field 'question'"):
+            load_gold(tmp_path3)
+    finally:
+        Path(tmp_path3).unlink(missing_ok=True)
+
+
+def test_load_gold_invalid_optional_field_types():
+    # expected_docs not a list
+    tmp1 = _write_temp_gold(
+        {"cases": [{"id": "c1", "user_id": "u1", "question": "q", "expected_docs": "doc1"}]}
+    )
+    try:
+        with pytest.raises(ValueError, match="Field 'expected_docs' must be a list"):
+            load_gold(tmp1)
+    finally:
+        Path(tmp1).unlink(missing_ok=True)
+
+    # forbidden_docs not a list
+    tmp2 = _write_temp_gold(
+        {"cases": [{"id": "c1", "user_id": "u1", "question": "q", "forbidden_docs": 123}]}
+    )
+    try:
+        with pytest.raises(ValueError, match="Field 'forbidden_docs' must be a list"):
+            load_gold(tmp2)
+    finally:
+        Path(tmp2).unlink(missing_ok=True)
+
+    # should_answer not a bool
+    tmp3 = _write_temp_gold(
+        {"cases": [{"id": "c1", "user_id": "u1", "question": "q", "should_answer": "yes"}]}
+    )
+    try:
+        with pytest.raises(ValueError, match="Field 'should_answer' must be a boolean"):
+            load_gold(tmp3)
+    finally:
+        Path(tmp3).unlink(missing_ok=True)
+
+
+def test_load_gold_valid_file():
+    valid_data = {
+        "cases": [
+            {
+                "id": "c1",
+                "user_id": "alice",
+                "question": "what is the policy?",
+                "expected_docs": ["doc1"],
+                "forbidden_docs": ["doc2"],
+                "should_answer": True,
+            },
+            {
+                "id": "c2",
+                "user_id": "bob",
+                "question": "where is the office?",
+            },
+        ]
+    }
+    tmp_path = _write_temp_gold(valid_data)
+    try:
+        cases = load_gold(tmp_path)
+        assert len(cases) == 2
+        assert isinstance(cases[0], GoldCase)
+        assert cases[0].id == "c1"
+        assert cases[0].user_id == "alice"
+        assert cases[0].question == "what is the policy?"
+        assert cases[0].expected_docs == ["doc1"]
+        assert cases[0].forbidden_docs == ["doc2"]
+        assert cases[0].should_answer is True
+
+        assert isinstance(cases[1], GoldCase)
+        assert cases[1].id == "c2"
+        assert cases[1].user_id == "bob"
+        assert cases[1].question == "where is the office?"
+        assert cases[1].expected_docs == []
+        assert cases[1].forbidden_docs == []
+        assert cases[1].should_answer is True
+    finally:
+        Path(tmp_path).unlink(missing_ok=True)
+
+
+def test_load_gold_duplicate_case_ids():
+    tmp_path = _write_temp_gold(
+        {
+            "cases": [
+                {"id": "duplicate-case-id", "user_id": "alice", "question": "q1"},
+                {"id": "duplicate-case-id", "user_id": "bob", "question": "q2"},
+            ]
+        }
+    )
+    try:
+        with pytest.raises(ValueError, match="duplicate-case-id"):
+            load_gold(tmp_path)
+    finally:
+        Path(tmp_path).unlink(missing_ok=True)

vaultrag/evaluate.py

@@ -135,7 +135,79 @@ def to_json(self) -> str:
 
 
 def load_gold(path: str | Path) -> list[GoldCase]:
-    data = json.loads(Path(path).read_text())
+    path_obj = Path(path)
+    try:
+        raw = path_obj.read_text(encoding="utf-8")
+    except Exception as e:
+        raise ValueError(f"Failed to read gold file '{path}': {e}") from e
+
+    try:
+        data = json.loads(raw)
+    except json.JSONDecodeError as e:
+        raise ValueError(f"Invalid JSON in gold file '{path}': {e}") from e
+
+    if not isinstance(data, dict):
+        raise ValueError(
+            f"Invalid gold file format in '{path}': root must be a JSON object, got {type(data).__name__}"
+        )
+
+    if "cases" not in data:
+        raise ValueError(f"Invalid gold file format in '{path}': missing required 'cases' key")
+
+    if not isinstance(data["cases"], list):
+        raise ValueError(
+            f"Invalid gold file format in '{path}': 'cases' must be a list, got {type(data['cases']).__name__}"
+        )
+
+    required_fields = ("id", "user_id", "question")
+    seen_ids: set[str] = set()
+
+    for i, case in enumerate(data["cases"]):
+        ctx = f"case index {i}"
+        if not isinstance(case, dict):
+            raise ValueError(
+                f"Invalid case at index {i} in '{path}': case must be a dictionary, got {type(case).__name__}"
+            )
+
+        if "id" in case and isinstance(case["id"], str):
+            ctx = f"case '{case['id']}' (index {i})"
+
+        for req_field in required_fields:
+            if req_field not in case:
+                raise ValueError(f"Missing required field '{req_field}' in {ctx} in '{path}'")
+            if not isinstance(case[req_field], str):
+                raise ValueError(
+                    f"Field '{req_field}' must be a string in {ctx} in '{path}', got {type(case[req_field]).__name__}"
+                )
+
+        case_id = case["id"]
+        if case_id in seen_ids:
+            raise ValueError(
+                f"Duplicate case id '{case_id}' in {ctx} in '{path}'"
+            )
+        seen_ids.add(case_id)
+
+        if "expected_docs" in case and (
+            not isinstance(case["expected_docs"], list)
+            or not all(isinstance(d, str) for d in case["expected_docs"])
+        ):
+            raise ValueError(
+                f"Field 'expected_docs' must be a list of strings in {ctx} in '{path}'"
+            )
+
+        if "forbidden_docs" in case and (
+            not isinstance(case["forbidden_docs"], list)
+            or not all(isinstance(d, str) for d in case["forbidden_docs"])
+        ):
+            raise ValueError(
+                f"Field 'forbidden_docs' must be a list of strings in {ctx} in '{path}'"
+            )
+
+        if "should_answer" in case and not isinstance(case["should_answer"], bool):
+            raise ValueError(
+                f"Field 'should_answer' must be a boolean in {ctx} in '{path}', got {type(case['should_answer']).__name__}"
+            )
+
     return [GoldCase(**c) for c in data["cases"]]