From 2931d628492847942fb32887aa6a03a6b7fe2c15 Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Fri, 9 Oct 2026 19:52:35 -0700 Subject: [PATCH] fix(cli): report actionable spec and dataset file errors --- CHANGELOG.md | 7 +++ agentdescent/evolvespec.py | 51 +++++++++++----- docs/plugins.md | 9 +++ tests/test_spec_file_errors.py | 104 +++++++++++++++++++++++++++++++++ 4 files changed, 157 insertions(+), 14 deletions(-) create mode 100644 tests/test_spec_file_errors.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 324a29a0..607cc883 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ All notable changes to AgentDescent are documented here. The format follows ## [Unreleased] +### Fixed + +- Spec and dataset file failures now report a concise `SpecError` instead of a + CLI traceback. Invalid JSON names the input file and line/column; JSONL uses + the physical file line, including blank lines. Missing or unreadable specs, + invalid UTF-8, and non-object spec JSON also produce actionable errors. + ### Added - **`AggregatorConfig.audit_drain_per_step` — the L-value consumer, the audit diff --git a/agentdescent/evolvespec.py b/agentdescent/evolvespec.py index 8f5cf073..5b861791 100644 --- a/agentdescent/evolvespec.py +++ b/agentdescent/evolvespec.py @@ -229,6 +229,8 @@ class EvolveSpec: @classmethod def from_dict(cls, d: Mapping[str, Any]) -> "EvolveSpec": + if not isinstance(d, Mapping): + raise SpecError("spec must be a JSON object") known = {f for f in cls.__dataclass_fields__} unknown = sorted(set(d) - known) if unknown: @@ -308,8 +310,16 @@ def load_spec(path: str, *, absolutise: bool = True) -> EvolveSpec: Relative paths in it are resolved against the current directory (see :meth:`EvolveSpec.absolutise`), because the process that *runs* the spec is usually not this one. Pass ``absolutise=False`` to read it verbatim.""" - with open(os.path.expanduser(path), encoding="utf-8") as fh: - spec = EvolveSpec.from_dict(json.load(fh)) + path = os.path.expanduser(path) + try: + with open(path, encoding="utf-8") as fh: + payload = json.load(fh) + except json.JSONDecodeError as e: + raise SpecError(f"spec {path!r}: invalid JSON at line {e.lineno}, " + f"column {e.colno}: {e.msg}") from None + except (OSError, UnicodeError) as e: + raise SpecError(f"spec {path!r}: cannot read UTF-8 file: {e}") from None + spec = EvolveSpec.from_dict(payload) return spec.absolutise() if absolutise else spec @@ -372,18 +382,31 @@ def _read_rows_file(path: str) -> List[Dict[str, Any]]: if not os.path.exists(path): raise SpecError(f"data.path {path!r} does not exist") ext = os.path.splitext(path)[1].lower() - with open(path, encoding="utf-8") as fh: - if ext == ".jsonl": - return [json.loads(line) for line in fh if line.strip()] - if ext == ".json": - rows = json.load(fh) - if isinstance(rows, Mapping): - rows = rows.get("rows") or rows.get("data") or rows.get("tasks") - if not isinstance(rows, list): - raise SpecError(f"data.path {path!r}: expected a JSON list of rows") - return rows - if ext in (".csv", ".tsv"): - return list(csv.DictReader(fh, delimiter="\t" if ext == ".tsv" else ",")) + line_number = 0 + try: + with open(path, encoding="utf-8") as fh: + if ext == ".jsonl": + rows = [] + for line_number, line in enumerate(fh, 1): + if line.strip(): + rows.append(json.loads(line)) + return rows + if ext == ".json": + rows = json.load(fh) + if isinstance(rows, Mapping): + rows = rows.get("rows") or rows.get("data") or rows.get("tasks") + if not isinstance(rows, list): + raise SpecError(f"data.path {path!r}: expected a JSON list of rows") + return rows + if ext in (".csv", ".tsv"): + return list(csv.DictReader(fh, delimiter="\t" if ext == ".tsv" else ",")) + except json.JSONDecodeError as e: + # JSONL is parsed one physical line at a time, including blank lines. + line = line_number if ext == ".jsonl" else e.lineno + raise SpecError(f"data.path {path!r}: invalid JSON at line {line}, " + f"column {e.colno}: {e.msg}") from None + except (OSError, UnicodeError) as e: + raise SpecError(f"data.path {path!r}: cannot read UTF-8 file: {e}") from None raise SpecError(f"data.path {path!r}: use .json, .jsonl, .csv or .tsv") diff --git a/docs/plugins.md b/docs/plugins.md index e85ca858..881dfe82 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -493,3 +493,12 @@ result = comp.run() # == evolve(comp.tasks, comp.reward, **comp.kwarg * [Using the policy slots](policy-guide.md) — what the `policies` block can name * [Sandboxes](sandboxes.md) — the container provider the `plugin` kind wants * [Design record](plugin-design.md) — why it is built this way + +### Fixing input-file errors + +`agentdescent plan ` reports input errors and exits with status 2. +Spec-file read errors appear on stderr; dataset validation errors appear in the +plan response (`ok: false`). For malformed spec or dataset JSON, the diagnostic names the file, +line and column to edit. JSONL line numbers count physical lines, including +blank lines. Files must be readable UTF-8; a spec must be a JSON object. Fix +the named input, then rerun `plan` before starting an evolution. diff --git a/tests/test_spec_file_errors.py b/tests/test_spec_file_errors.py new file mode 100644 index 00000000..5ae80d34 --- /dev/null +++ b/tests/test_spec_file_errors.py @@ -0,0 +1,104 @@ +"""User-edited input files fail at the boundary, not with a CLI traceback.""" +import json +import subprocess +import sys + +import pytest + +from agentdescent import EvolveSpec, SpecError, load_spec +from agentdescent.evolvespec import load_rows + + +@pytest.mark.parametrize("payload", [None, 7, "text", [], ["kind"]]) +def test_spec_requires_object(payload): + with pytest.raises(SpecError, match="spec must be a JSON object"): + EvolveSpec.from_dict(payload) + + +def test_spec_json_location(tmp_path): + path = tmp_path / "broken.json" + path.write_text('{\n "kind": "text",\n}\n') + with pytest.raises(SpecError, match="line 3, column 1") as exc: + load_spec(str(path)) + assert str(path) in str(exc.value) + + +@pytest.mark.parametrize("kind", ["missing", "directory", "encoding"]) +def test_spec_read_errors(tmp_path, kind): + path = tmp_path / "spec.json" + if kind == "directory": + path.mkdir() + elif kind == "encoding": + path.write_bytes(b"\xff") + with pytest.raises(SpecError, match="cannot read UTF-8 file") as exc: + load_spec(str(path)) + assert str(path) in str(exc.value) + + +@pytest.mark.parametrize("ext,contents,line", [ + ("json", '[\n {"prompt": "ok"},\n]', 3), + ("jsonl", '\n{"prompt": "ok"}\n\n{"prompt": }\n', 4), +]) +def test_data_json_location(tmp_path, ext, contents, line): + path = tmp_path / ("cases." + ext) + path.write_text(contents) + with pytest.raises(SpecError, match=f"line {line}, column") as exc: + load_rows({"path": str(path)}) + assert str(path) in str(exc.value) + + +@pytest.mark.parametrize("ext", ["json", "jsonl", "csv", "tsv"]) +@pytest.mark.parametrize("kind", ["directory", "encoding"]) +def test_data_read_errors(tmp_path, ext, kind): + path = tmp_path / ("cases." + ext) + if kind == "directory": + path.mkdir() + else: + path.write_bytes(b"\xff") + with pytest.raises(SpecError, match="cannot read UTF-8 file"): + load_rows({"path": str(path)}) + + +@pytest.mark.parametrize("source", ["missing", "spec", "data"]) +def test_cli_plan_bad_file_has_no_traceback(tmp_path, source): + path = tmp_path / "spec.json" + if source == "spec": + path.write_text('{"kind": }') + elif source == "data": + data = tmp_path / "cases.jsonl" + data.write_text('\n{"prompt": }\n') + path.write_text(json.dumps({"kind": "text", "target": "instruction", + "data": {"path": str(data)}, + "agent": "openai_compatible"})) + result = subprocess.run([sys.executable, "-m", "agentdescent.cli", "plan", str(path)], + capture_output=True, text=True) + assert result.returncode == 2 + output = result.stdout + result.stderr + assert "Traceback" not in output + assert str(path if source != "data" else data) in output + if source == "data": + assert json.loads(result.stdout)["ok"] is False + assert "line 2, column" in output + else: + assert result.stderr.startswith("error: ") + + +@pytest.mark.parametrize("ext", ["json", "jsonl", "csv", "tsv"]) +def test_valid_data_still_loads(tmp_path, ext): + rows = [{"prompt": "hello", "gold": "world"}] + contents = {"json": json.dumps(rows), "jsonl": '\n' + json.dumps(rows[0]) + '\n\n', + "csv": 'prompt,gold\nhello,world\n', + "tsv": 'prompt\tgold\nhello\tworld\n'} + path = tmp_path / ("cases." + ext) + path.write_text(contents[ext]) + assert load_rows({"path": str(path)}) == rows + + +def test_valid_spec_still_loads(tmp_path): + payload = {"kind": "text", "target": "instruction", "data": {"inline": []}} + path = tmp_path / "spec.json" + path.write_text(json.dumps(payload)) + spec = load_spec(str(path), absolutise=False) + assert spec.kind == "text" + assert spec.target == "instruction" + assert spec.data == payload["data"]