From 9aaf3cf6b677bc40fb5acb236fc041c0884cdcb8 Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Singh Date: Sat, 26 Sep 2026 17:37:28 +0530 Subject: [PATCH] tests: the styling gate covers run, trace and metrics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #17. The contract — strip the escapes from what a terminal receives and it equals what a pipe receives, exactly — was gated for four commands: `plan`, `models`, `models --check`, `demo stage0`. The commands most likely to be piped were the uncovered ones: `run --check-only` is a linter in this repository's own CI, `trace | grep` and `metrics` in a script are the obvious uses, and `run`'s REFUSED block tints the key *and* the value of its verdict line, a shape no covered case reached. Four cases added, and the gap is demonstrated rather than asserted. Painting `trace`'s step column unconditionally leaves the old four-case gate **fully green** while both new `trace` cases fail. That is the whole claim of the issue, reproduced. A case is a builder rather than a literal argv now, because these commands take a file argument. The `workspace` fixture writes an admitted topology, a refused one carrying two objections so the per-objection row runs more than once, and a trace from one `demo stage0` run. The hermetic argv matters more than it looks. `run` resolves its registry and policy from the working directory, and this repository's root carries a `registry.py` and a `grapharc.toml` that are **gitignored** — dogfooding residue. A case leaning on those would read one registry locally and another in CI and then compare output that differs for reasons unrelated to styling. So the registry is named explicitly and `--config` points at an empty file. One normaliser added, under protest. `run`'s fingerprint is not stable across two loads of the same topology: `Subgraph.proposal_id` defaults to a fresh `uuid4` and `fingerprint()` hashes the whole model, that field included. So the same topology file yields a different fingerprint every invocation, while `graphrun.py` prints it under the comment "the fingerprint is what a later run is compared against". Normalising it keeps the ADMITTED block's styling in the comparison instead of dropping `run` over one token; the underlying problem is filed separately, and when it is fixed this normaliser should go and the comparison gets stricter for it. Verified: 2205 selected, 13 deselected, ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- docs/deep-dive.md | 2 +- tests/test_cli_style.py | 125 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 115 insertions(+), 12 deletions(-) diff --git a/docs/deep-dive.md b/docs/deep-dive.md index 6ade549..3d1c6a4 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,197 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,205 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/tests/test_cli_style.py b/tests/test_cli_style.py index 331bfd0..680482b 100644 --- a/tests/test_cli_style.py +++ b/tests/test_cli_style.py @@ -17,6 +17,7 @@ from __future__ import annotations +import json import os import re import selectors @@ -30,14 +31,41 @@ not hasattr(os, "openpty"), reason="needs a pty, which Windows has no equivalent for" ) +def _fixed(*args: str): + """A case whose argv needs nothing built first.""" + + def build(_workspace: dict) -> list[str]: + return list(args) + + return build + + # Commands with styled human-mode output. Exit codes are deliberately not pinned # here: `models --check` exits 1 when the host can reach no real provider, which # is correct and is what a machine with no credentials does. +# +# A case is a *builder* rather than a literal argv, because the styled commands +# that were missing from this gate all take a file argument — a topology, a +# trace — and those have to be made first. `run`, `trace` and `metrics` are +# exactly the commands most likely to be piped (`run --check-only` is a linter +# in this repo's own CI, `trace | grep`, `metrics` in a script), so they are the +# ones whose piped bytes matter most, and they were the ones nothing checked. STYLED = [ - pytest.param(["plan", "investigate the checkout outage", "--scripted"], id="plan"), - pytest.param(["models"], id="models"), - pytest.param(["models", "--check"], id="models-check"), - pytest.param(["demo", "stage0"], id="demo-stage0"), + pytest.param(_fixed("plan", "investigate the checkout outage", "--scripted"), id="plan"), + pytest.param(_fixed("models"), id="models"), + pytest.param(_fixed("models", "--check"), id="models-check"), + pytest.param(_fixed("demo", "stage0"), id="demo-stage0"), + # The ADMITTED verdict block and its accent-tinted fingerprint. + pytest.param( + lambda w: ["run", str(w["admitted"]), "--check-only", *w["hermetic"]], + id="run-check-only", + ), + # The REFUSED block, which tints the key *and* the value of its verdict line + # and then paints a row per objection — a shape no other case reaches. + pytest.param(lambda w: ["run", str(w["refused"]), *w["hermetic"]], id="run-refused"), + # A painted row per trace event: dim, cell, accent and err in one output. + pytest.param(lambda w: ["trace", str(w["trace"])], id="trace"), + pytest.param(lambda w: ["metrics", str(w["trace"]), w["run_id"]], id="metrics"), ] # The subset whose output is reproducible enough to compare byte-for-byte across @@ -58,6 +86,19 @@ # The default trace directory stamp: two invocations of one command are two # runs with two stamps, and the comparison is about styling, not clocks. _RUNDIR = re.compile(r"\d{8}-\d{6}-[0-9a-f]{6}") +# `run`'s fingerprint, which is *not* stable across two loads of the same +# topology file: `Subgraph.proposal_id` defaults to a fresh `uuid4` and +# `fingerprint()` hashes the whole model, `proposal_id` included. Normalised +# here so this file can still compare the styling of the line it appears on — +# which is the whole point of covering the ADMITTED block — rather than dropping +# `run` out of the comparison over one token. +# +# It is normalised under protest. `graphrun.py` prints it under the comment "the +# fingerprint is what a later run is compared against", and a value that differs +# on every invocation cannot do that job. Filed separately; if that is fixed so +# the fingerprint follows the topology, this normaliser should be deleted and the +# comparison will be stricter for it. +_FINGERPRINT = re.compile(r"(?<=fingerprint: )[0-9a-f]{16}") def _env(**extra: str) -> dict[str, str]: @@ -127,11 +168,72 @@ def _on_pty(args: list[str], **extra: str) -> tuple[str, int]: def _normalise(text: str) -> str: text = _TMPDIR.sub("/tmp/grapharc-NORMALISED", text) - return _RUNDIR.sub("RUNDIR-NORMALISED", text) - - -@pytest.mark.parametrize("args", STYLED) -def test_a_terminal_gets_escapes_and_a_pipe_gets_none(args): + text = _RUNDIR.sub("RUNDIR-NORMALISED", text) + return _FINGERPRINT.sub("FINGERPRINT-NORMALISED", text) + + +@pytest.fixture(scope="module") +def workspace(tmp_path_factory): + """Files the file-taking styled commands need, built once for the module. + + Hermetic on purpose, and the `hermetic` argv is the load-bearing part. + `run` resolves its registry and policy from the working directory when not + told otherwise, and this repository's root carries a `registry.py` and a + `grapharc.toml` that are *gitignored* — dogfooding residue. A case that + relied on them would read one registry here and a different one in CI, and + compare output that differs for a reason that has nothing to do with + styling. So the registry is named explicitly and `--config` points at an + empty file, which is what keeps `./grapharc.toml` out of it. + """ + root = tmp_path_factory.mktemp("cli-style") + + def topology(nodes, edges): + return json.dumps({"nodes": nodes, "edges": edges}) + "\n" + + admitted = root / "admitted.json" + admitted.write_text( + topology( + [{"name": "gather", "kind": "collect_context"}], + [ + {"source": "__start__", "target": "gather"}, + {"source": "gather", "target": "__end__"}, + ], + ), + encoding="utf-8", + ) + # Two objections rather than one, so the per-objection row is exercised more + # than once: a sentinel pointing the wrong way, and an unregistered kind. + refused = root / "refused.json" + refused.write_text( + topology( + [{"name": "triage", "kind": "not_a_registered_kind"}], + [ + {"source": "__end__", "target": "triage"}, + {"source": "triage", "target": "__end__"}, + ], + ), + encoding="utf-8", + ) + config = root / "empty.toml" + config.write_text("", encoding="utf-8") + + trace = root / "trace.jsonl" + out, err, code = _piped(["demo", "stage0", "--trace", str(trace)]) + assert code == 0, f"could not produce a trace to style:\n{out}\n{err}" + run_id = json.loads(trace.read_text(encoding="utf-8").splitlines()[0])["run_id"] + + return { + "admitted": admitted, + "refused": refused, + "trace": trace, + "run_id": run_id, + "hermetic": ["--registry", "grapharc.stdlib:build_registry", "--config", str(config)], + } + + +@pytest.mark.parametrize("build", STYLED) +def test_a_terminal_gets_escapes_and_a_pipe_gets_none(build, workspace): + args = build(workspace) """Both halves in one test: styling must be real, and confined to a terminal.""" on_pty, pty_code = _on_pty(args) out, err, piped_code = _piped(args) @@ -145,14 +247,15 @@ def test_a_terminal_gets_escapes_and_a_pipe_gets_none(args): assert "\x1b" not in err, "an escape reached piped stderr" -@pytest.mark.parametrize("args", COMPARABLE) -def test_stripping_the_escapes_reproduces_the_piped_output_exactly(args): +@pytest.mark.parametrize("build", COMPARABLE) +def test_stripping_the_escapes_reproduces_the_piped_output_exactly(build, workspace): """The property the byte-compared doc pages depend on. Colour must be the *only* difference between what a terminal shows and what a pipe carries. A tty-only change to spacing, alignment or line count would pass the leak check above and still make README describe output nobody sees. """ + args = build(workspace) on_pty, _ = _on_pty(args) out, err, _ = _piped(args)