diff --git a/lab/README.md b/lab/README.md index 58a4896..885a230 100644 --- a/lab/README.md +++ b/lab/README.md @@ -44,7 +44,9 @@ A record: `kind` (test, equiv, bench, profile, submit, finding), `session`, `sna used for a verdict. The writer stamps `id`, `at` and `schema`. Heavy raw files (diffs, bundles) go through `ledger.put_blob` and are referenced by their content-hashed path under `blobs/`. A held-out record (`config.split == "heldout"`) carries no `raw` and one aggregate per metric -(`base`, `new`, `delta_pct`, `band_pct`, `verdict`); the writer refuses anything finer. +(`base`, `new`, `delta_pct`, `band_pct`, `verdict`); the writer refuses anything finer. A `"split": +"heldout"` anywhere else in a record (outside `claim`) is refused, so held-out data can't hide under +`result`; tools write held-out results with `Toolbox._record_heldout`. ## Runtime diff --git a/lab/ledger.py b/lab/ledger.py index 6922fbd..8308a7f 100644 --- a/lab/ledger.py +++ b/lab/ledger.py @@ -46,6 +46,15 @@ def _lock(root: Path): fcntl.flock(lf, fcntl.LOCK_UN) +def _mentions_heldout(obj: Any) -> bool: + """A `"split": "heldout"` anywhere in a nested value, so held-out data can't slip in under another field.""" + if isinstance(obj, dict): + return obj.get("split") == "heldout" or any(_mentions_heldout(v) for v in obj.values()) + if isinstance(obj, list): + return any(_mentions_heldout(v) for v in obj) + return False + + def _check(record: dict) -> None: """Integrity rules only: known kind, writer-owned fields, no held-out detail beyond one aggregate per metric.""" if record.get("kind") not in KINDS: @@ -55,8 +64,12 @@ def _check(record: dict) -> None: config = record.get("config") if config is not None and not isinstance(config, dict): raise LedgerError("config must be an object") - if (config or {}).get("split") != "heldout": + # `claim` is untrusted text that may name the split; everything else marks the record held-out + if not _mentions_heldout({k: v for k, v in record.items() if k != "claim"}): return + if (config or {}).get("split") != "heldout": + raise LedgerError("held-out data must be declared with config.split == 'heldout' and written as " + "top-level config + metrics, not nested under another field") extra = record.keys() - HELDOUT_FIELDS if extra: raise LedgerError(f"held-out records may only carry {sorted(HELDOUT_FIELDS)}; got {sorted(extra)}") diff --git a/lab/tools.py b/lab/tools.py index 4c2cf78..a332ac5 100644 --- a/lab/tools.py +++ b/lab/tools.py @@ -34,6 +34,14 @@ def _record(self, kind: str, tool: str, args: dict, result: dict, snapshot, usd: "patch": snapshot.patch, "result": result, "cost": {"usd": usd}} return ledger.append(body, self.s.ledger_root) + def _record_heldout(self, kind: str, tool: str, args: dict, config: dict, metrics: dict, snapshot, + usd: float = 0.0) -> dict: + """A held-out result: top-level config + one aggregate per metric, the only shape the ledger accepts.""" + body = {"kind": kind, "run": self.s.run_id, "session": self.s.session_id, "tool": tool, "args": args, + "snapshot": snapshot.id, "snapshot_blob": snapshot.blob, "patch": snapshot.patch, + "config": {**config, "split": "heldout"}, "metrics": metrics, "cost": {"usd": usd}} + return ledger.append(body, self.s.ledger_root) + def _audited(self, tool: str, args: dict): """Snapshot, and stop the run on a surface violation: integrity failures are the one hard rule.""" snap = self.s.workspace.snapshot() diff --git a/tests/test_lab_ledger.py b/tests/test_lab_ledger.py index 9bccaaa..202ff4b 100644 --- a/tests/test_lab_ledger.py +++ b/tests/test_lab_ledger.py @@ -23,6 +23,10 @@ def test_append_stamps_and_reads_back(tmp_path): {"kind": "submit", "config": {"split": "heldout"}, "raw": {"per_request": "blobs/x"}}, {"kind": "submit", "config": {"split": "heldout"}, "metrics": {"ttft_p50_ms": {"delta_pct": -3, "by_class": {"cold_start": -9}}}}, + # held-out data nested where the guard used to look away: under result, args, or a list + {"kind": "submit", "result": {"split": "heldout", "per_request": [1, 2, 3]}}, + {"kind": "bench", "args": {"split": "heldout"}, "result": {"metrics": {}}}, + {"kind": "submit", "result": {"runs": [{"split": "heldout", "rows": "blobs/x"}]}}, ]) def test_rejects(tmp_path, record): with pytest.raises(ledger.LedgerError): @@ -30,6 +34,18 @@ def test_rejects(tmp_path, record): assert list(ledger.records(tmp_path)) == [] +def test_a_tool_record_shaped_like_tools_record_cannot_carry_heldout(tmp_path): + record = {"kind": "submit", "run": "r", "session": "s", "tool": "submit", "args": {}, "snapshot": "x", + "snapshot_blob": "blobs/a", "patch": "blobs/b", "cost": {"usd": 0}, + "result": {"split": "heldout", "metrics": {"ttft_p50_ms": {"delta_pct": -3}}}} + with pytest.raises(ledger.LedgerError): + ledger.append(record, tmp_path) + + +def test_claim_may_name_the_split(tmp_path): + ledger.append({"kind": "note", "claim": {"text": "checked", "split": "heldout"}}, tmp_path) + + def test_heldout_aggregate_is_allowed(tmp_path): m = {"ttft_p50_ms": {"base": 800, "new": 700, "delta_pct": -12.5, "band_pct": 3, "verdict": "win"}} ledger.append({"kind": "submit", "config": {"split": "heldout"}, "metrics": m}, tmp_path) diff --git a/tests/test_lab_runtime.py b/tests/test_lab_runtime.py index 263e48c..ba4f038 100644 --- a/tests/test_lab_runtime.py +++ b/tests/test_lab_runtime.py @@ -187,3 +187,15 @@ def test_cli_parses(monkeypatch, cfg): session.main(["--goal", "g", "--budget", "3", "--base", "HEAD", "--max-sessions", "2"]) assert seen["cfg"].goal == "g" and seen["cfg"].budget_usd == 3.0 and seen["cfg"].max_sessions == 2 assert json.dumps({"ok": 1}) + + +def test_heldout_results_go_through_the_aggregate_only_writer(cfg): + from lab.tools import Toolbox + snap = type("Snap", (), {"id": "x", "blob": "blobs/a", "patch": "blobs/b"})() + sess = type("S", (), {"run_id": "r", "session_id": "s", "ledger_root": cfg.ledger_root})() + tb = Toolbox(sess) + m = {"ttft_p50_ms": {"base": 800, "new": 700, "delta_pct": -12.5, "band_pct": 3, "verdict": "win"}} + rec = tb._record_heldout("submit", "submit", {}, {"class": "steady_interactive"}, m, snap) + assert rec["config"]["split"] == "heldout" and "result" not in rec + with pytest.raises(ledger.LedgerError): + tb._record("submit", "submit", {}, {"split": "heldout", "per_request": [1]}, snap)