diff --git a/docs/cookbook/06-serving-and-ops.md b/docs/cookbook/06-serving-and-ops.md index 0f5e697..a6d9cf8 100644 --- a/docs/cookbook/06-serving-and-ops.md +++ b/docs/cookbook/06-serving-and-ops.md @@ -1707,6 +1707,11 @@ Exit codes are part of the interface: `0` did the job, `1` ran and the answer was negative (two runs differed, a run id had no events, no backend was usable), `2` could not run at all (missing file, missing component, unknown model spec). +`trace`, `metrics` and `viz` also exit `2` when a trace contains an invalid +event or non-UTF-8 bytes. The error names the file and its 1-based line; with +`--json` it is one failure document on stdout and stderr is empty. These +strict readers refuse the file rather than present a partial audit trail. + A whole session, verbatim (run ids are random per run and durations are wall-clock; the test maps the former, masks the latter, and byte-compares every other character): diff --git a/docs/deep-dive.md b/docs/deep-dive.md index a7aee89..e0e8b88 100644 --- a/docs/deep-dive.md +++ b/docs/deep-dive.md @@ -254,7 +254,7 @@ A stable system is not one that claims to have no edges — it is one whose edge - **`.env` and `grapharc.toml` follow the same discovery rule: the working directory, and nowhere else.** Neither searches parent directories — a run must not be governed by a file you did not know about, and must not be *billed* to one either. **This is a behaviour change:** the credential loader used to walk up to `/`, so a `.env` in an ancestor directory (a `$HOME` one on a shared box, a client project one above a demo checkout) was picked up silently. If you relied on that, move the file into the directory you run from, `export` the variable, or pass `env_file=` to name it explicitly. A real environment variable still beats any file. - **`grapharc run` has no budget unless you give it one.** Set any of `--max-tokens`, `--max-iterations`, `--max-seconds`, or `--max-concurrency`; without them each dimension is unlimited and the gate admits a topology of any worst-case cost. -**Verified this pass:** `pytest` → green, 2,225 selected and 13 deselected (the live ones); `ruff check .` clean; all eight `grapharc demo` stages green, plus the `trace` / `metrics` / `viz` / `replay` tour against a freshly recorded demo trace; the wheel builds and imports all submodules in a clean virtualenv with `[all]`, and `0.1.8` on PyPI is that wheel. The counts are a snapshot, not a property of the project — `pytest` re-derives them in one command, which is the only reason they are quoted, and `tests/test_deep_dive.py` fails this line rather than letting it drift. +**Verified this pass:** `pytest` → green, 2,252 selected and 13 deselected (the live ones); `ruff check .` clean; all eight `grapharc demo` stages green, plus the `trace` / `metrics` / `viz` / `replay` tour against a freshly recorded demo trace; the wheel builds and imports all submodules in a clean virtualenv with `[all]`, and `0.1.8` on PyPI is that wheel. The counts are a snapshot, not a property of the project — `pytest` re-derives them in one command, which is the only reason they are quoted, and `tests/test_deep_dive.py` fails this line rather than letting it drift. [ROADMAP.md](../ROADMAP.md) tracks what is built and what is not, item by item. diff --git a/grapharc/observe/trace.py b/grapharc/observe/trace.py index 0507479..5051c29 100644 --- a/grapharc/observe/trace.py +++ b/grapharc/observe/trace.py @@ -232,13 +232,19 @@ def read_events(self, run_id: str | None = None) -> list[TraceEvent]: if not self.path.exists(): return [] events = [] - with self.path.open(encoding="utf-8") as f: - for line_number, line in enumerate(f, start=1): - if not line.strip(): - continue + # Decode one JSONL line at a time. TextIO's buffered decoder can fail + # before yielding an earlier valid line when later bytes are not UTF-8, + # losing both the trace-error boundary and the offending line number. + with self.path.open("rb") as f: + # Preserve the text reader's LF, CRLF and CR line boundaries. + lines = (line for raw in f for line in raw.splitlines(keepends=True)) + for line_number, raw in enumerate(lines, start=1): try: + line = raw.decode("utf-8") + if not line.strip(): + continue ev = TraceEvent.model_validate_json(line) - except ValidationError as exc: + except (UnicodeDecodeError, ValidationError) as exc: raise TraceReadError(self.path, line_number, exc) from exc if run_id is None or ev.run_id == run_id: events.append(ev) diff --git a/tests/test_trace_encoding.py b/tests/test_trace_encoding.py new file mode 100644 index 0000000..2657810 --- /dev/null +++ b/tests/test_trace_encoding.py @@ -0,0 +1,84 @@ +"""Strict trace readers report corrupt encoding at its actual JSONL line.""" + +from __future__ import annotations + +import json + +import pytest + +from grapharc.cli.main import main +from grapharc.observe.trace import TraceReadError, TraceRecorder + +BAD_LINES = [ + pytest.param(b'{"node":"\xff"}\n', id="invalid-byte"), + pytest.param(b"\xff\xfe\x00binary\n", id="utf16"), + pytest.param(b'{"node":"\xe2\x82"}\n', id="incomplete-character"), +] +READERS = [ + pytest.param(["trace"], id="trace"), + pytest.param(["metrics", "r1"], id="metrics"), + pytest.param(["viz", "r1"], id="viz"), +] + + +def _trace(tmp_path, bad_line: bytes, line_number: int = 2): + recorder = TraceRecorder(tmp_path / "bad.jsonl") + if line_number == 2: + recorder.event(run_id="r1", graph="g", node="मॉडल", phase="start", step=1) + with recorder.path.open("ab") as handle: + handle.write(bad_line) + return recorder + + +@pytest.mark.parametrize("bad_line", BAD_LINES) +@pytest.mark.parametrize("line_number", [1, 2]) +def test_encoding_errors_use_the_trace_error_and_actual_line(tmp_path, bad_line, line_number): + recorder = _trace(tmp_path, bad_line, line_number) + + with pytest.raises(TraceReadError) as caught: + recorder.read_events("r1") + + assert caught.value.path == recorder.path + assert caught.value.line_number == line_number + assert isinstance(caught.value.cause, UnicodeDecodeError) + + +@pytest.mark.parametrize("argv", READERS) +@pytest.mark.parametrize("bad_line", BAD_LINES) +@pytest.mark.parametrize("as_json", [False, True], ids=["text", "json"]) +def test_cli_reports_non_utf8_trace_without_a_traceback(tmp_path, capsys, argv, bad_line, as_json): + recorder = _trace(tmp_path, bad_line) + flags = ["--json"] if as_json else [] + + assert main([argv[0], str(recorder.path), *argv[1:], *flags]) == 2 + + captured = capsys.readouterr() + message = f"unreadable trace file: {recorder.path}: line 2 is not a trace event" + if as_json: + assert json.loads(captured.out) == {"ok": False, "command": argv[0], "error": message} + assert captured.err == "" + else: + assert captured.out == "" + assert captured.err == f"error: {message}\n" + + +@pytest.mark.parametrize("newline", [b"\n", b"\r\n", b"\r"], ids=["lf", "crlf", "cr"]) +def test_utf8_content_and_blank_lines_keep_their_meaning(tmp_path, newline): + recorder = TraceRecorder(tmp_path / "valid.jsonl") + recorder.path.write_text("\u2003\n", encoding="utf-8") + recorder.event( + run_id="r1", + graph="g", + node="मॉडल", + phase="end", + step=1, + state_delta={"answer": "café ✓"}, + ) + # A valid final event does not require a trailing newline. + recorder.path.write_bytes(recorder.path.read_bytes().rstrip(b"\n").replace(b"\n", newline)) + + events = recorder.read_events("r1") + + assert len(events) == 1 + assert events[0].node == "मॉडल" + assert events[0].state_delta == {"answer": "café ✓"}