Skip to content

Commit 410b650

Browse files
alexkromanclaude
andauthored
Add guards for stale help examples and JSON output purity (#197)
## Summary Adds two new test suites that enforce repo-wide invariants across all leaf commands: 1. **Help example freshness** — validates that `--help` examples reference only flags and subcommands that actually exist, catching silent rot from renames (e.g., `--api-key` → `--with-api-key`) or removed subcommands. 2. **JSON output purity** — verifies the "Errors → stderr, data → stdout" contract holds for every leaf command, ensuring pipeline safety when chaining `assembly … --json | next-tool`. ## Key Changes - **`tests/test_help_example_flags.py`** — New test module that: - Parses the `Examples` epilog from every leaf command's `--help` output - Extracts `$ assembly …` command lines and splits pipelines into per-`assembly` segments - Walks the live Typer tree to validate that each flag and subcommand path exists - Reports stale examples with the exact unknown flags they reference - Includes unit tests proving the detection logic catches renamed/removed flags and correctly handles shell syntax (pipes, redirects, command substitution) - **`tests/test_json_stdout_purity.py`** — New test module that: - Parametrizes over every leaf command path - Invokes each with an unknown flag (deterministic parse error before command body) - Asserts `exit_code == 2` (standard Click usage error) - Verifies `stdout == ""` (no data leaked during error) - Validates that stderr contains only valid JSON (no human-prose leaks) - Tests both `--json` mode (machine-readable error envelope) and human mode (error on stderr, not stdout) ## Implementation Details - **Conservative parsing** — The help-example validator skips segments where the `assembly` token is glued to shell syntax (e.g., `$(assembly …`) rather than mis-parsing, since those same commands appear unglued elsewhere in the examples. - **Uniform error path** — JSON purity uses an unknown flag as the trigger because it fails during Click's argument parsing, before credential resolution or network calls, making it deterministic and uniform across all commands. - **Parametrized coverage** — Both suites use `leaf_command_items()` and `leaf_command_argvs()` helpers (from `tests/_cli_tree`) to automatically cover every leaf command without manual enumeration. These guards close gaps left by existing checks: `test_help_examples_coverage` proves examples exist but not that they're valid; `docs_consistency_gate.py` keeps REFERENCE.md/README in sync but doesn't check in-`--help` examples. https://claude.ai/code/session_01A8VXBitwFzqz5VsLCyepiY Co-authored-by: Claude <noreply@anthropic.com>
1 parent 95effaf commit 410b650

2 files changed

Lines changed: 199 additions & 0 deletions

File tree

tests/test_help_example_flags.py

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
"""Guard that every ``--help`` example still parses against the live CLI tree.
2+
3+
``test_help_examples_coverage`` proves each leaf command *has* an ``Examples``
4+
epilog, but not that the example commands are real. The examples are the snippets
5+
users copy-paste, so a flag rename (the ``login --api-key`` → ``--with-api-key``
6+
deprecation is the canonical case) or a removed subcommand silently rots them.
7+
``docs_consistency_gate.py`` keeps REFERENCE.md/README in sync with the code, but
8+
nothing checked the in-``--help`` examples — this closes that gap.
9+
10+
The check is deliberately scoped to *flags and subcommand paths* (the parts that
11+
break on a rename), not full argument validation: examples carry placeholders
12+
(``<file>``, ``TRANSCRIPT_ID``) that aren't real paths, so parsing them with
13+
Click would false-positive. Pipelines (``a | assembly … | assembly …``) are split
14+
into per-``assembly`` segments; a segment whose ``assembly`` token is glued to
15+
other shell syntax (``$(assembly …``) is skipped rather than mis-parsed —
16+
conservative by design, since those same commands appear unglued elsewhere.
17+
"""
18+
19+
from __future__ import annotations
20+
21+
import shlex
22+
23+
import typer
24+
25+
from aai_cli.main import app
26+
from tests._cli_tree import leaf_command_items
27+
28+
# Shell tokens that end one command and start another; an example may chain several
29+
# `assembly` invocations through a pipe, so each segment is validated independently.
30+
_BOUNDARIES = frozenset({"|", ">", ">>", "<", ";", "&&", "||", "&"})
31+
32+
33+
def _option_names(command):
34+
"""Every flag spelling a Click command accepts (long, short, and --no- forms)."""
35+
names = {"--help"}
36+
for param in command.params:
37+
if param.param_type_name == "option":
38+
names.update(param.opts)
39+
names.update(param.secondary_opts)
40+
return names
41+
42+
43+
def _example_commands(command):
44+
"""The ``$ …`` command lines from a leaf command's rendered examples epilog."""
45+
epilog = getattr(command, "epilog", None) or ""
46+
return [line.strip()[2:] for line in epilog.splitlines() if line.strip().startswith("$ ")]
47+
48+
49+
def _assembly_segments(tokens):
50+
"""Split a token stream into the argv of each literal ``assembly`` invocation.
51+
52+
Each ``assembly`` token opens a fresh segment (appended up front, then grown in
53+
place), and a shell boundary closes the current one — so tokens belonging to a
54+
non-``assembly`` command (``ls``, ``jq``, ``$(assembly …``) are dropped.
55+
"""
56+
segments: list[list[str]] = []
57+
current: list[str] | None = None
58+
for token in tokens:
59+
if token == "assembly":
60+
current = []
61+
segments.append(current)
62+
elif token in _BOUNDARIES:
63+
current = None
64+
elif current is not None:
65+
current.append(token)
66+
return segments
67+
68+
69+
def _unknown_flags(argv, root):
70+
"""Flags in one ``assembly`` argv that no command at their position accepts.
71+
72+
Walks the tree token by token: a token matching a subcommand descends, and a
73+
flag is checked against whatever command is current (so a root flag like
74+
``--sandbox`` is validated against the root, a leaf flag against the leaf).
75+
"""
76+
command = root
77+
bad = []
78+
for token in argv:
79+
sub = getattr(command, "commands", None)
80+
if sub and token in sub:
81+
command = sub[token]
82+
continue
83+
if token.startswith("-") and token not in ("-", "--"):
84+
flag = token.split("=", 1)[0] # --model=x → --model
85+
if flag not in _option_names(command):
86+
bad.append(flag)
87+
return bad
88+
89+
90+
def _stale_examples(items, root):
91+
"""Map each command path to the (example, unknown-flags) pairs it ships, if any."""
92+
stale: dict[str, list[tuple[str, list[str]]]] = {}
93+
for path, command in items:
94+
for example in _example_commands(command):
95+
for segment in _assembly_segments(shlex.split(example)):
96+
bad = _unknown_flags(segment, root)
97+
if bad:
98+
stale.setdefault(" ".join(path), []).append((example, bad))
99+
return stale
100+
101+
102+
def test_help_examples_reference_only_real_flags():
103+
root = typer.main.get_command(app)
104+
stale = _stale_examples(leaf_command_items(), root)
105+
assert stale == {}, f"--help examples reference flags the CLI no longer accepts: {stale}"
106+
107+
108+
class _FakeLeaf:
109+
def __init__(self, epilog):
110+
self.epilog = epilog
111+
112+
113+
def test_stale_examples_detects_renamed_and_removed_flags():
114+
# Drives the detection path the real examples (correctly) never trigger: a stale
115+
# flag is reported under its command, and a command with no epilog contributes
116+
# nothing — proving the guard would actually fail on drift, not just pass vacuously.
117+
root = typer.main.get_command(app)
118+
items = [
119+
(("renamed",), _FakeLeaf("[bold]Examples[/bold]\n\n$ assembly transcribe x --gone-flag")),
120+
(("blank",), _FakeLeaf(None)),
121+
]
122+
assert _stale_examples(items, root) == {
123+
"renamed": [("assembly transcribe x --gone-flag", ["--gone-flag"])]
124+
}
125+
126+
127+
def test_assembly_segments_splits_pipelines_and_drops_foreign_commands():
128+
# The parser splits a chained pipeline into per-`assembly` argv and drops tokens
129+
# owned by a non-`assembly` command (the leading `ls`).
130+
tokens = shlex.split("ls *.wav | assembly stream --from-stdin | assembly llm -f")
131+
assert _assembly_segments(tokens) == [["stream", "--from-stdin"], ["llm", "-f"]]

tests/test_json_stdout_purity.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
"""Stream-discipline sweep over every leaf command.
2+
3+
The repo-wide invariant is *"Errors → stderr, data → stdout"* (root ``AGENTS.md``)
4+
— it's what keeps ``assembly … --json | next-tool`` pipeline-safe. Individual
5+
commands assert their own happy-path JSON shape, but nothing swept the contract
6+
across *all* of them, so a command that leaked a human-readable line onto stdout
7+
in ``--json`` mode (or printed an error payload to stdout) would pass the whole
8+
gate. This walks the live Typer tree and pins the contract for every leaf.
9+
10+
The trigger is an **unknown flag**: it fails during Click's argument parsing,
11+
before the command body, before any credential resolution or network — so it is
12+
the one error path that is uniform across all commands *and* deterministic under
13+
the suite's ``--disable-socket`` (no command-specific required-arg knowledge, and
14+
no risk of an interactive command like ``login``/``stream`` blocking on a browser
15+
or a mic). Click 8.2+ keeps ``result.stdout`` and ``result.stderr`` as separate
16+
streams on the ``CliRunner`` ``Result``, so the split is observable directly.
17+
"""
18+
19+
from __future__ import annotations
20+
21+
import json
22+
23+
import pytest
24+
from typer.testing import CliRunner
25+
26+
from aai_cli.main import app
27+
from tests._cli_tree import leaf_command_argvs
28+
29+
runner = CliRunner()
30+
31+
# A flag no command defines, so it always trips Click's "No such option" parse
32+
# error rather than reaching a command body.
33+
UNKNOWN_FLAG = "--__definitely-not-a-real-flag__"
34+
35+
36+
def _json_lines(stream: str) -> list[object]:
37+
"""Parse a stream as NDJSON, asserting every line is valid JSON.
38+
39+
A human-prose leak (a Rich-rendered error, a bare status line) is exactly a
40+
line that fails ``json.loads`` — so this raises rather than silently skipping.
41+
"""
42+
return [json.loads(line) for line in stream.splitlines()]
43+
44+
45+
@pytest.mark.parametrize("path", leaf_command_argvs(), ids=lambda p: " ".join(p))
46+
def test_json_mode_keeps_stdout_clean_and_error_on_stderr(path: list[str]) -> None:
47+
result = runner.invoke(app, [*path, UNKNOWN_FLAG, "--json"])
48+
49+
# Usage/parse error: the stable exit code for a bad flag (REFERENCE.md table).
50+
assert result.exit_code == 2
51+
# The whole point of the pipeline contract: a parse error puts *nothing* on
52+
# stdout, so a downstream consumer never sees a partial/garbage record.
53+
assert result.stdout == ""
54+
# The error rides stderr as the uniform JSON envelope — machine-readable, and
55+
# every emitted line parses (a human-prose leak onto stderr fails here too).
56+
objs = _json_lines(result.stderr)
57+
assert {"error": {"type": "usage_error", "message": "No such option: " + UNKNOWN_FLAG}} in objs
58+
59+
60+
@pytest.mark.parametrize("path", leaf_command_argvs(), ids=lambda p: " ".join(p))
61+
def test_human_mode_routes_errors_to_stderr(path: list[str]) -> None:
62+
# The same contract without --json: a human error still belongs on stderr, never
63+
# stdout, so `assembly … -o text > out` keeps the error out of the data file.
64+
result = runner.invoke(app, [*path, UNKNOWN_FLAG])
65+
66+
assert result.exit_code == 2
67+
assert result.stdout == ""
68+
assert "No such option" in result.stderr

0 commit comments

Comments
 (0)