Skip to content

Commit 2e4b836

Browse files
tests: the styling gate covers run, trace and metrics (#126)
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) <noreply@anthropic.com>
1 parent 34c11ec commit 2e4b836

2 files changed

Lines changed: 115 additions & 12 deletions

File tree

‎docs/deep-dive.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ A stable system is not one that claims to have no edges — it is one whose edge
254254
- **`.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.
255255
- **`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.
256256

257-
**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.
257+
**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.
258258

259259
[ROADMAP.md](../ROADMAP.md) tracks what is built and what is not, item by item.
260260

‎tests/test_cli_style.py‎

Lines changed: 114 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
from __future__ import annotations
1919

20+
import json
2021
import os
2122
import re
2223
import selectors
@@ -30,14 +31,41 @@
3031
not hasattr(os, "openpty"), reason="needs a pty, which Windows has no equivalent for"
3132
)
3233

34+
def _fixed(*args: str):
35+
"""A case whose argv needs nothing built first."""
36+
37+
def build(_workspace: dict) -> list[str]:
38+
return list(args)
39+
40+
return build
41+
42+
3343
# Commands with styled human-mode output. Exit codes are deliberately not pinned
3444
# here: `models --check` exits 1 when the host can reach no real provider, which
3545
# is correct and is what a machine with no credentials does.
46+
#
47+
# A case is a *builder* rather than a literal argv, because the styled commands
48+
# that were missing from this gate all take a file argument — a topology, a
49+
# trace — and those have to be made first. `run`, `trace` and `metrics` are
50+
# exactly the commands most likely to be piped (`run --check-only` is a linter
51+
# in this repo's own CI, `trace | grep`, `metrics` in a script), so they are the
52+
# ones whose piped bytes matter most, and they were the ones nothing checked.
3653
STYLED = [
37-
pytest.param(["plan", "investigate the checkout outage", "--scripted"], id="plan"),
38-
pytest.param(["models"], id="models"),
39-
pytest.param(["models", "--check"], id="models-check"),
40-
pytest.param(["demo", "stage0"], id="demo-stage0"),
54+
pytest.param(_fixed("plan", "investigate the checkout outage", "--scripted"), id="plan"),
55+
pytest.param(_fixed("models"), id="models"),
56+
pytest.param(_fixed("models", "--check"), id="models-check"),
57+
pytest.param(_fixed("demo", "stage0"), id="demo-stage0"),
58+
# The ADMITTED verdict block and its accent-tinted fingerprint.
59+
pytest.param(
60+
lambda w: ["run", str(w["admitted"]), "--check-only", *w["hermetic"]],
61+
id="run-check-only",
62+
),
63+
# The REFUSED block, which tints the key *and* the value of its verdict line
64+
# and then paints a row per objection — a shape no other case reaches.
65+
pytest.param(lambda w: ["run", str(w["refused"]), *w["hermetic"]], id="run-refused"),
66+
# A painted row per trace event: dim, cell, accent and err in one output.
67+
pytest.param(lambda w: ["trace", str(w["trace"])], id="trace"),
68+
pytest.param(lambda w: ["metrics", str(w["trace"]), w["run_id"]], id="metrics"),
4169
]
4270

4371
# The subset whose output is reproducible enough to compare byte-for-byte across
@@ -58,6 +86,19 @@
5886
# The default trace directory stamp: two invocations of one command are two
5987
# runs with two stamps, and the comparison is about styling, not clocks.
6088
_RUNDIR = re.compile(r"\d{8}-\d{6}-[0-9a-f]{6}")
89+
# `run`'s fingerprint, which is *not* stable across two loads of the same
90+
# topology file: `Subgraph.proposal_id` defaults to a fresh `uuid4` and
91+
# `fingerprint()` hashes the whole model, `proposal_id` included. Normalised
92+
# here so this file can still compare the styling of the line it appears on —
93+
# which is the whole point of covering the ADMITTED block — rather than dropping
94+
# `run` out of the comparison over one token.
95+
#
96+
# It is normalised under protest. `graphrun.py` prints it under the comment "the
97+
# fingerprint is what a later run is compared against", and a value that differs
98+
# on every invocation cannot do that job. Filed separately; if that is fixed so
99+
# the fingerprint follows the topology, this normaliser should be deleted and the
100+
# comparison will be stricter for it.
101+
_FINGERPRINT = re.compile(r"(?<=fingerprint: )[0-9a-f]{16}")
61102

62103

63104
def _env(**extra: str) -> dict[str, str]:
@@ -127,11 +168,72 @@ def _on_pty(args: list[str], **extra: str) -> tuple[str, int]:
127168

128169
def _normalise(text: str) -> str:
129170
text = _TMPDIR.sub("/tmp/grapharc-NORMALISED", text)
130-
return _RUNDIR.sub("RUNDIR-NORMALISED", text)
131-
132-
133-
@pytest.mark.parametrize("args", STYLED)
134-
def test_a_terminal_gets_escapes_and_a_pipe_gets_none(args):
171+
text = _RUNDIR.sub("RUNDIR-NORMALISED", text)
172+
return _FINGERPRINT.sub("FINGERPRINT-NORMALISED", text)
173+
174+
175+
@pytest.fixture(scope="module")
176+
def workspace(tmp_path_factory):
177+
"""Files the file-taking styled commands need, built once for the module.
178+
179+
Hermetic on purpose, and the `hermetic` argv is the load-bearing part.
180+
`run` resolves its registry and policy from the working directory when not
181+
told otherwise, and this repository's root carries a `registry.py` and a
182+
`grapharc.toml` that are *gitignored* — dogfooding residue. A case that
183+
relied on them would read one registry here and a different one in CI, and
184+
compare output that differs for a reason that has nothing to do with
185+
styling. So the registry is named explicitly and `--config` points at an
186+
empty file, which is what keeps `./grapharc.toml` out of it.
187+
"""
188+
root = tmp_path_factory.mktemp("cli-style")
189+
190+
def topology(nodes, edges):
191+
return json.dumps({"nodes": nodes, "edges": edges}) + "\n"
192+
193+
admitted = root / "admitted.json"
194+
admitted.write_text(
195+
topology(
196+
[{"name": "gather", "kind": "collect_context"}],
197+
[
198+
{"source": "__start__", "target": "gather"},
199+
{"source": "gather", "target": "__end__"},
200+
],
201+
),
202+
encoding="utf-8",
203+
)
204+
# Two objections rather than one, so the per-objection row is exercised more
205+
# than once: a sentinel pointing the wrong way, and an unregistered kind.
206+
refused = root / "refused.json"
207+
refused.write_text(
208+
topology(
209+
[{"name": "triage", "kind": "not_a_registered_kind"}],
210+
[
211+
{"source": "__end__", "target": "triage"},
212+
{"source": "triage", "target": "__end__"},
213+
],
214+
),
215+
encoding="utf-8",
216+
)
217+
config = root / "empty.toml"
218+
config.write_text("", encoding="utf-8")
219+
220+
trace = root / "trace.jsonl"
221+
out, err, code = _piped(["demo", "stage0", "--trace", str(trace)])
222+
assert code == 0, f"could not produce a trace to style:\n{out}\n{err}"
223+
run_id = json.loads(trace.read_text(encoding="utf-8").splitlines()[0])["run_id"]
224+
225+
return {
226+
"admitted": admitted,
227+
"refused": refused,
228+
"trace": trace,
229+
"run_id": run_id,
230+
"hermetic": ["--registry", "grapharc.stdlib:build_registry", "--config", str(config)],
231+
}
232+
233+
234+
@pytest.mark.parametrize("build", STYLED)
235+
def test_a_terminal_gets_escapes_and_a_pipe_gets_none(build, workspace):
236+
args = build(workspace)
135237
"""Both halves in one test: styling must be real, and confined to a terminal."""
136238
on_pty, pty_code = _on_pty(args)
137239
out, err, piped_code = _piped(args)
@@ -145,14 +247,15 @@ def test_a_terminal_gets_escapes_and_a_pipe_gets_none(args):
145247
assert "\x1b" not in err, "an escape reached piped stderr"
146248

147249

148-
@pytest.mark.parametrize("args", COMPARABLE)
149-
def test_stripping_the_escapes_reproduces_the_piped_output_exactly(args):
250+
@pytest.mark.parametrize("build", COMPARABLE)
251+
def test_stripping_the_escapes_reproduces_the_piped_output_exactly(build, workspace):
150252
"""The property the byte-compared doc pages depend on.
151253
152254
Colour must be the *only* difference between what a terminal shows and what a
153255
pipe carries. A tty-only change to spacing, alignment or line count would pass
154256
the leak check above and still make README describe output nobody sees.
155257
"""
258+
args = build(workspace)
156259
on_pty, _ = _on_pty(args)
157260
out, err, _ = _piped(args)
158261

0 commit comments

Comments
 (0)