fix(loaders): refuse what cannot be scored honestly, in both loaders and the pricing file

CHANGELOG.md

@@ -83,6 +83,17 @@ different event from one that moved because it was wrong.
 
 ### Fixed
 
+- The JevBench loader skipped the duplicate-id check the JSONL loader makes, so
+  repeated ids loaded silently (#44). Both loaders now share it. The dataset
+  loader read a byte order mark as part of the first row and refused it, turned
+  a label of `null`, `true` or `1` into the text "None", "True" or "1", accepted
+  empty labels and descriptions of options that do not exist, and failed on a
+  non-UTF-8 file without naming it (#45). It now reads the mark as nothing,
+  refuses each of the others with the reason, and names the file and line that
+  is not UTF-8. The pricing loader accepted NaN, infinite and negative prices,
+  `as_of` values such as `20260901` or `2026-W36-1`, and dates in the future,
+  which kept the report from ever calling a price stale; each is refused now,
+  and a byte order mark is read as nothing there too.
 - The CLI accepted options that misbehaved or crashed (#41): `--limit 0` ran
   every row and a negative limit sliced from the end, `--boot 0` failed with a
   traceback after the run had been paid for, an option the adapter does not

docs/datasets.md

@@ -7,8 +7,8 @@ for exactly this, so a clone never commits it.
 
 ## The format
 
-One JSON object per line (JSONL), UTF-8. Blank lines are skipped. Every other
-line is one case.
+One JSON object per line (JSONL), UTF-8, with or without a byte order mark.
+Blank lines are skipped. Every other line is one case.
 
 | field | required | what it is |
 |---|---|---|
@@ -46,7 +46,11 @@ A row that cannot be scored honestly is refused with its line number, and the
 rest of the file still runs. `--strict` refuses the whole run instead.
 
 - A missing required field, an empty `id` or `text`, or `labels` that is not a
-  list of at least two distinct entries.
+  list of at least two distinct, non-empty strings. A label written as `null`,
+  `true` or a number is refused rather than turned into the text `"None"`,
+  `"True"` or `"1"`.
+- A `label_descriptions` entry for something that is not one of the row's
+  options, or a description that is not a non-empty string.
 - A `gold_label` that is not one of the row's own `labels`. Scoring it would
   mark every model wrong on that row, which reads as a model failure and is a
   data error.
@@ -55,8 +59,9 @@ rest of the file still runs. `--strict` refuses the whole run instead.
 - An `id` already used earlier in the file.
 - A line that is not a JSON object.
 
-A file with no rows at all, or a path that does not exist, is an error rather
-than an empty run.
+A file with no rows at all, a path that does not exist, or a file that is not
+UTF-8 is an error rather than an empty run; the last names the line with the
+byte that is not.
 
 ## Running it
 

src/plumbline/config.py

@@ -20,6 +20,8 @@
 from __future__ import annotations
 
 import json
+import math
+import re
 from collections.abc import Mapping
 from datetime import date
 from pathlib import Path
@@ -122,6 +124,8 @@ def _anthropic(input_usd: float, output_usd: float) -> Pricing:
 #: carries it has not been filled in, so it is refused rather than priced.
 TEMPLATE_AS_OF = "YYYY-MM-DD"
 
+_ISO_DATE = re.compile(r"\d{4}-\d{2}-\d{2}")
+
 
 def load_pricing_file(path: Path | str) -> PricingTable:
     """Read a pricing table the operator wrote, requiring provenance on every entry.
@@ -138,7 +142,7 @@ def load_pricing_file(path: Path | str) -> PricingTable:
     """
     path = Path(path)
     try:
-        raw = json.loads(path.read_text(encoding="utf-8"))
+        raw = json.loads(path.read_text(encoding="utf-8-sig"))
     except FileNotFoundError:
         raise PricingConfigError(
             f"no pricing file at {path}. plumbline never guesses a pricing file "
@@ -245,6 +249,15 @@ def _price(name: str, value: Mapping[str, Any], field: str, path: Path) -> float
             "or null. Use null where the vendor publishes no price; null and 0 are "
             "different claims and plumbline keeps them apart."
         )
+    if not math.isfinite(number):
+        raise PricingConfigError(
+            f"entry {name!r} in {path} has {field}={number!r}; a price must be finite, "
+            "or every row it prices becomes NaN or infinite."
+        )
+    if number < 0:
+        raise PricingConfigError(
+            f"entry {name!r} in {path} has {field}={number!r}; a price cannot be negative."
+        )
     return float(number)
 
 
@@ -256,11 +269,26 @@ def _as_of(name: str, value: Any, path: Path) -> date:
             "them; an unedited copy of docs/pricing.example.json is refused rather than "
             "used to price a run."
         )
+    # fromisoformat also accepts 20260901 and week dates such as 2026-W36-1,
+    # which nobody writes meaning a date, so the shape is checked first.
+    if not (isinstance(value, str) and _ISO_DATE.fullmatch(value)):
+        raise PricingConfigError(
+            f"entry {name!r} in {path} has as_of={value!r}, which is not an ISO date "
+            "written YYYY-MM-DD. The date a price was read is what lets the report say the price "
+            "may be out of date, so it is not optional and not free-form."
+        )
     try:
-        return date.fromisoformat(str(value))
+        read_on = date.fromisoformat(value)
     except ValueError:
         raise PricingConfigError(
             f"entry {name!r} in {path} has as_of={value!r}, which is not an ISO date "
             "(YYYY-MM-DD). The date a price was read is what lets the report say the "
             "price may be out of date, so it is not optional and not free-form."
         ) from None
+    if read_on > date.today():
+        raise PricingConfigError(
+            f"entry {name!r} in {path} has as_of={value!r}, which is in the future. A "
+            "price cannot have been read on a day that has not happened, and a future date "
+            "would keep the report from ever calling it out of date."
+        )
+    return read_on

src/plumbline/datasets/loader.py

@@ -131,14 +131,7 @@ def load_jsonl(path: Path | str) -> LoadReport:
             refusals.append(RowRefusal(number, case_id, str(refused)))
             continue
         if case.id in seen:
-            refusals.append(
-                RowRefusal(
-                    number,
-                    case.id,
-                    f"duplicate id {case.id!r}; results are keyed by id, so a repeat would "
-                    "overwrite an earlier row",
-                )
-            )
+            refusals.append(_duplicate(number, case.id))
             continue
         seen.add(case.id)
         cases.append(case)
@@ -179,6 +172,7 @@ def load_jevbench(path: Path | str) -> LoadReport:
     cases: list[Case] = []
     refusals: list[RowRefusal] = []
     types: dict[str, int] = {}
+    seen: set[str] = set()
     normalized = 0
     dropped_criteria = 0
 
@@ -199,15 +193,19 @@ def load_jevbench(path: Path | str) -> LoadReport:
             refusals.append(RowRefusal(number, case_id, str(refused)))
             continue
 
+        if case.id in seen:
+            refusals.append(_duplicate(number, case.id))
+            continue
+        seen.add(case.id)
         normalized += int(was_normalized)
         dropped_criteria += int(lost_criteria)
         cases.append(case)
 
     notes = [
         "Translated from JevBench: the case text is the row's question above its state, "
         "and each row is asked as the question type it states. plumbline's harness, "
-        "prompts and "
-        "scoring differ from JevBench's, so these numbers are not comparable with theirs."
+        "prompts and scoring differ from JevBench's, so these numbers are not comparable "
+        "with theirs."
     ]
     if normalized:
         notes.append(
@@ -248,19 +246,20 @@ def _read_rows(path: Path) -> list[tuple[int, Mapping[str, Any] | str]]:
         )
 
     rows: list[tuple[int, Mapping[str, Any] | str]] = []
-    with path.open(encoding="utf-8") as handle:
-        for number, line in enumerate(handle, start=1):
-            if not line.strip():
-                continue  # A blank line is formatting, not a row.
-            try:
-                parsed = json.loads(line)
-            except json.JSONDecodeError as broken:
-                rows.append((number, f"line is not valid JSON: {broken.msg}"))
-                continue
-            if not isinstance(parsed, dict):
-                rows.append((number, f"line is a {type(parsed).__name__}, expected an object"))
-                continue
-            rows.append((number, parsed))
+    # Decoded line by line, so a file that is not UTF-8 is named with the line
+    # that is not. The first line is read as utf-8-sig, so a byte order mark
+    # (which Windows editors and spreadsheet exports write) is read as nothing
+    # rather than refusing the first row.
+    for number, raw in enumerate(path.read_bytes().splitlines(keepends=True), start=1):
+        try:
+            line = raw.decode("utf-8-sig" if number == 1 else "utf-8")
+        except UnicodeDecodeError as undecodable:
+            raise DatasetError(
+                f"{path} is not UTF-8 text: line {number} holds the byte "
+                f"{raw[undecodable.start]:#04x}, which UTF-8 does not allow. Save the "
+                "file as UTF-8 and load it again."
+            ) from None
+        _parse_line(rows, number, line)
 
     if not rows:
         raise DatasetError(
@@ -270,6 +269,31 @@ def _read_rows(path: Path) -> list[tuple[int, Mapping[str, Any] | str]]:
     return rows
 
 
+def _parse_line(rows: list[tuple[int, Mapping[str, Any] | str]], number: int, line: str) -> None:
+    """One line of the file: a row, a refusal to record, or nothing if it is blank."""
+    if not line.strip():
+        return  # A blank line is formatting, not a row.
+    try:
+        parsed = json.loads(line)
+    except json.JSONDecodeError as broken:
+        rows.append((number, f"line is not valid JSON: {broken.msg}"))
+        return
+    if not isinstance(parsed, dict):
+        rows.append((number, f"line is a {type(parsed).__name__}, expected an object"))
+        return
+    rows.append((number, parsed))
+
+
+def _duplicate(number: int, case_id: str) -> RowRefusal:
+    """The refusal for an id already seen, the same from either loader."""
+    return RowRefusal(
+        number,
+        case_id,
+        f"duplicate id {case_id!r}; results are keyed by id, so a repeat would "
+        "overwrite an earlier row",
+    )
+
+
 def _case_from_record(record: Mapping[str, Any]) -> Case:
     """Build one Case, or refuse with the reason spelled out."""
     for name in ("id", "text", "labels", "gold_label"):
@@ -287,7 +311,15 @@ def _case_from_record(record: Mapping[str, Any]) -> Case:
     raw_labels = record["labels"]
     if not isinstance(raw_labels, Sequence) or isinstance(raw_labels, str | bytes):
         raise DatasetError("field 'labels' must be a list of strings")
-    labels = tuple(str(label) for label in raw_labels)
+    # Each label is taken as written. str() would have turned null into "None"
+    # and true into "True", and loaded an option nobody wrote.
+    for label in raw_labels:
+        if not isinstance(label, str) or not label.strip():
+            raise DatasetError(
+                f"every label must be a non-empty string, and {label!r} is not; labels are "
+                "the options the model chooses between, as written"
+            )
+    labels = tuple(raw_labels)
     if len(labels) < 2:
         raise DatasetError(f"a case needs at least 2 labels, got {len(labels)}")
     if len(set(labels)) != len(labels):
@@ -302,8 +334,21 @@ def _case_from_record(record: Mapping[str, Any]) -> Case:
         )
 
     descriptions = record.get("label_descriptions")
-    if descriptions is not None and not isinstance(descriptions, Mapping):
-        raise DatasetError("field 'label_descriptions' must be an object")
+    if descriptions is not None:
+        if not isinstance(descriptions, Mapping):
+            raise DatasetError("field 'label_descriptions' must be an object")
+        unknown = sorted(str(key) for key in descriptions if key not in labels)
+        if unknown:
+            raise DatasetError(
+                f"label_descriptions describes {unknown!r}, which are not among this row's "
+                f"options {list(labels)!r}; a description has to belong to an option"
+            )
+        for option, description in descriptions.items():
+            if not isinstance(description, str) or not description.strip():
+                raise DatasetError(
+                    f"label_descriptions gives {option!r} the description {description!r}; "
+                    "each description is a non-empty string"
+                )
 
     question_type = record.get("question_type", "choice")
     if question_type not in QUESTION_TYPES:

tests/test_datasets.py

@@ -341,3 +341,86 @@ def test_the_example_in_the_dataset_docs_loads_as_documented(tmp_path) -> None:
     kinds = sorted(case.question_type for case in report.cases)
     assert kinds == ["choice", "choice", "choice", "noul", "score"]
     assert len(report.scoreable) == 4  # the score row loads and is held back
+
+
+# Validation the loaders were missing (#44, #45)
+
+
+def jevbench_row(case_id: str) -> dict:
+    return {
+        "id": case_id,
+        "expected": "yes",
+        "labels": ["yes", "no"],
+        "question": {"type": "noul", "instructions": "Well?", "criteria": {}},
+        "state": f"something happened to {case_id}",
+    }
+
+
+def test_the_jevbench_loader_refuses_a_duplicate_id_like_the_jsonl_one(tmp_path: Path) -> None:
+    path = write_jsonl(tmp_path / "j.jsonl", [jevbench_row("dup"), jevbench_row("dup")])
+
+    report = loader.load_jevbench(path)
+
+    assert len(report.cases) == 1
+    assert len(report.refusals) == 1 and "duplicate id" in report.refusals[0].reason
+
+
+def test_a_byte_order_mark_is_read_as_utf8_rather_than_refused(tmp_path: Path) -> None:
+    """Windows editors and spreadsheet exports write one; the first row must still load."""
+    path = tmp_path / "bom.jsonl"
+    path.write_text(json.dumps(a_row()) + "\n", encoding="utf-8-sig")
+
+    report = loader.load_jsonl(path)
+
+    assert len(report.cases) == 1 and not report.refusals
+
+
+def test_a_file_that_is_not_utf8_names_the_file_and_the_line(tmp_path: Path) -> None:
+    path = tmp_path / "latin1.jsonl"
+    path.write_bytes(
+        (json.dumps(a_row()) + "\n").encode("utf-8")
+        # ensure_ascii=False keeps the é as a character, so latin-1 writes it as 0xE9.
+        + (json.dumps(a_row(id="two", text="café"), ensure_ascii=False) + "\n").encode("latin-1")
+    )
+
+    with pytest.raises(DatasetError, match=r"latin1\.jsonl.*line 2"):
+        loader.load_jsonl(path)
+
+
+@pytest.mark.parametrize(
+    ("labels", "gold"),
+    [
+        ([None, "a"], "a"),
+        ([True, False], "True"),
+        (["a", ""], "a"),
+        (["a", "   "], "a"),
+        ([1, 2], "1"),
+    ],
+    ids=["null", "bools", "empty", "blank", "numbers"],
+)
+def test_every_label_must_be_a_non_empty_string(
+    tmp_path: Path, labels: list[object], gold: str
+) -> None:
+    """str() used to turn null into 'None' and true into 'True', and load the row."""
+    path = write_jsonl(tmp_path / "d.jsonl", [a_row(labels=labels, gold_label=gold)])
+
+    report = loader.load_jsonl(path)
+
+    assert not report.cases
+    assert "non-empty string" in report.refusals[0].reason
+
+
+@pytest.mark.parametrize(
+    "descriptions",
+    [{"zzz": "not an option"}, {"billing": None}, {"billing": 3}],
+    ids=["unknown-key", "null-value", "number-value"],
+)
+def test_label_descriptions_must_describe_the_options_in_words(
+    tmp_path: Path, descriptions: dict[str, object]
+) -> None:
+    path = write_jsonl(tmp_path / "d.jsonl", [a_row(label_descriptions=descriptions)])
+
+    report = loader.load_jsonl(path)
+
+    assert not report.cases
+    assert "label_descriptions" in report.refusals[0].reason

tests/test_pricing_config.py

@@ -158,3 +158,30 @@ def test_a_bool_is_not_accepted_as_a_price(tmp_path: Path) -> None:
     payload = {"m": {**ENTRY, "output_usd_per_million": True}}
     with pytest.raises(config.PricingConfigError, match="not a number"):
         config.load_pricing_file(write(tmp_path, payload))
+
+
+@pytest.mark.parametrize(
+    ("field", "value", "expected"),
+    [
+        ("input_usd_per_million", float("nan"), "finite"),
+        ("output_usd_per_million", float("inf"), "finite"),
+        ("input_usd_per_million", -1.0, "negative"),
+        ("as_of", "20260901", "YYYY-MM-DD"),
+        ("as_of", "2026-W36-1", "YYYY-MM-DD"),
+        ("as_of", "2099-01-01", "future"),
+    ],
+)
+def test_prices_must_be_finite_and_dates_real_and_past(
+    tmp_path: Path, field: str, value: object, expected: str
+) -> None:
+    """A NaN price costs every row as NaN; a future date defeats the staleness check (#45)."""
+    path = tmp_path / "pricing.json"
+    path.write_text(json.dumps({"m": {**ENTRY, field: value}}), encoding="utf-8")
+    with pytest.raises(config.PricingConfigError, match=expected):
+        config.load_pricing_file(path)
+
+
+def test_a_pricing_file_with_a_byte_order_mark_loads(tmp_path: Path) -> None:
+    path = tmp_path / "pricing.json"
+    path.write_text(json.dumps({"m": ENTRY}), encoding="utf-8-sig")
+    assert "m" in config.load_pricing_file(path)