Skip to content

cli: the tty-vs-piped styling gate covers four commands; run, trace and metrics are styled but untested #17

Description

@Shashankss1205

Summary

The styling contract — ANSI-escape-stripped tty output must equal piped output, byte for byte — is gated by tests/test_cli_style.py, but only for four commands. PR #11 shipped the restyle with this noted as a known gap ("proven ad-hoc but not committed"); the committed gate closed it for a subset:

# 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.
STYLED = [
    pytest.param(["plan", "investigate the checkout outage"], id="plan"),
    pytest.param(["models"], id="models"),
    pytest.param(["models", "--check"], id="models-check"),
    pytest.param(["demo", "stage0"], id="demo-stage0"),
]

Every other styled command is outside the gate. grapharc run prints tinted ADMITTED/REFUSED verdict blocks (grapharc/cli/graphrun.py:204-286), grapharc trace paints every event row (grapharc/cli/main.py:496-504), grapharc metrics styles its kv_block (grapharc/cli/main.py:521-526) — none is covered by test_a_terminal_gets_escapes_and_a_pipe_gets_none or test_stripping_the_escapes_reproduces_the_piped_output_exactly. A call site in any of them that painted unconditionally, or a tty-only layout change, would leak escapes into pipes (or make the docs describe output nobody sees) and no test would notice.

Why this matters

The piped bytes of these commands are load-bearing, not cosmetic: tests/test_readme.py and the cookbook tests byte-compare a subset of command output against fenced doc blocks, and CI consumes run --check-only as a linter. The style module's whole safety argument (grapharc/cli/style.py docstring: "a pipe gets exactly the bytes it got before this module existed") is currently enforced by tests for plan, models and demo stage0 only. The commands most likely to be piped or captured — run in CI, trace | grep, metrics in a script — are exactly the uncovered ones, and run's REFUSED path styles both the key and the value of its verdict line, a shape no covered command exercises.

Where in the code

  • tests/test_cli_style.py:36-49STYLED and COMPARABLE stop at four commands; the pty/pipe machinery and _TMPDIR normalisation already in this file are everything the new cases need
  • grapharc/cli/graphrun.py:204-286 — styled run output: header, ADMITTED/REFUSED verdicts, tinted rejection rows — uncovered
  • grapharc/cli/main.py:496-504 — styled trace rows (style.dim, style.cell, style.accent, style.err) — uncovered
  • grapharc/cli/main.py:521-526 — styled metrics output — uncovered

Confirm the gap for yourself — run --check-only emits escapes on a tty that nothing asserts on:

uv run pytest tests/test_cli_style.py -q --collect-only   # 4 styled ids: plan, models, models-check, demo-stage0
printf '{"nodes": [{"name": "triage"}], "edges": [{"source": "__start__", "target": "triage"}, {"source": "triage", "target": "__end__"}]}\n' > /tmp/topo.json
script -qec "uv run python -m grapharc.cli.main run /tmp/topo.json --check-only" /dev/null | cat -v | grep '\^\['
# ^[[38;5;… escapes are emitted on a tty; no test compares them against the piped form

What to change

  1. Add hermetic cases to STYLED/COMPARABLE in tests/test_cli_style.py for the uncovered styled commands: run <topo> --check-only (admitted path), run <bad-topo> (REFUSED path, which styles key and value tints no current case reaches), trace <trace.jsonl>, and metrics <trace.jsonl> <run-id>. A fixture can write the topology file and produce the trace with one demo stage0 --trace run, the way test_viz_on_a_terminal_stays_pasteable_mermaid already does.
  2. Keep the existing judgement calls: exit codes stay unpinned, and any command whose output varies run-to-run stays out of COMPARABLE (the run refused/admitted text is deterministic apart from the mkdtemp trace path, which the existing _TMPDIR normaliser already handles).
  3. If parametrising requires per-case setup (a file argument), a small indirection — building the argv in the test from a tmp_path-scoped fixture — is fine; do not weaken the assertion (stripped tty output must equal piped stdout+stderr exactly).

Out of scope: serve (blocks on a socket), demo --model live runs and agent (need a real provider), replay/diff (their text is the engine formatter's, deliberately unstyled — see grapharc/cli/main.py and grapharc/cli/replay.py), and any change to grapharc/cli/style.py itself.

How to verify

uv run pytest -q
uv run ruff check .

The new parametrised cases in tests/test_cli_style.py are the proof. Check they can go red: add a stray style.ok(...) guarded by nothing (or an unconditional "\x1b[32m") to one line of the run verdict block in grapharc/cli/graphrun.py and the new run cases must fail while the existing four stay green — that is precisely the blind spot being closed.

Acceptance criteria

  • run (admitted and refused), trace and metrics are covered by both the escapes-on-a-tty/none-in-a-pipe test and the strip-equality test
  • The new cases are hermetic: no network, no credentials, no reliance on the host's terminal, and they pass on a machine with no provider configured
  • Existing four cases and their ids are unchanged
  • uv run pytest stays green and uv run ruff check . is clean
  • Any README or cookbook sentence this changes is updated in the same pull request (several are byte-compared against real output by the test suite)

Skill level

good first issue — the pty harness, environment scrubbing and path normalisation are already written in the same file; the work is producing fixtures for four more argv lists and wiring them into the existing parametrisation. The one subtlety is keeping the cases deterministic (fixture files, not fresh mkdtemp paths, wherever the output embeds one) — copy how test_viz_on_a_terminal_stays_pasteable_mermaid obtains its trace. Questions welcome on the issue.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions