diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d59e396..883a81f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -245,7 +245,7 @@ breaking changes may land in a minor release. from #190 — one JSON object on stdout, inline `schema_version`, additive-only evolution, errors → stderr with empty stdout — now live in one module with shared `emit`/`add_json_flag` helpers; `status --json` uses them (output byte-identical) and the duplicated token-total math - folded into `_run_token_totals`. All four `--json` commands share the contract (#195); `--json` + folded into `run_token_totals`. All four `--json` commands share the contract (#195); `--json` adoption on more commands is tracked in #196. - **Backend-neutral naming for the seam-backed helpers and operator messages.** The multiplexer seam has non-tmux backends now, so the helpers that wrap it drop their legacy tmux names — diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 2bd0f43f..cc444ccc 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -46,13 +46,13 @@ from .documents import STATUS_SCHEMA_VERSION # noqa: F401 — re-export from .documents import VALIDATE_SCHEMA_VERSION # noqa: F401 — re-export from .documents import ( - _clean_document, - _cleanup_document, - _decisions_document, - _list_document, - _run_token_totals, - _status_document, - _validate_document, + clean_document, + cleanup_document, + decisions_document, + list_document, + run_token_totals, + status_document, + validate_document, ) from .engine import Engine from .journal import Journal, load_state, save_state @@ -504,7 +504,7 @@ def cmd_validate(args: argparse.Namespace) -> int: if getattr(args, "json", False): # getattr, not args.json: cmd_validate is called directly by tests (and by # anything holding a hand-built Namespace) that predate the flag. - machine.emit(_validate_document(report, stories_on, spec_folder)) + machine.emit(validate_document(report, stories_on, spec_folder)) else: report.render() return 0 if report.passed else 1 @@ -1521,7 +1521,7 @@ def cmd_decisions(args: argparse.Namespace) -> int: # document, not the text line), and regardless of --list: --json *is* # the listing. It cannot fall through to the prompter, which reads stdin # and prints per-answer progress — neither survives a pure document. - machine.emit(_decisions_document(pending)) + machine.emit(decisions_document(pending)) return 0 if not pending: print("no unanswered decisions from past sweeps") @@ -1568,7 +1568,7 @@ def cmd_status(args: argparse.Namespace) -> int: return 1 state = load_state(run_dir) if args.json: - machine.emit(_status_document(state)) + machine.emit(status_document(state)) return 0 kind = f" [{state.run_type}]" if state.run_type != "story" else "" print(f"run {state.run_id}{kind} started {state.started_at}") @@ -1578,7 +1578,7 @@ def cmd_status(args: argparse.Namespace) -> int: print(f"status: PAUSED ({state.paused_stage}) — {state.paused_reason}") else: print("status: in progress (or interrupted)") - raw_total, weighted_total, weight = _run_token_totals(state) + raw_total, weighted_total, weight = run_token_totals(state) if raw_total: print( f"tokens: {weighted_total:,} weighted " @@ -1624,7 +1624,7 @@ def cmd_list(args: argparse.Namespace) -> int: project = _project(args) infos = discover_runs(project) # oldest first if args.json: - machine.emit(_list_document(infos)) + machine.emit(list_document(infos)) return 0 if not infos: print("no runs found") @@ -1762,7 +1762,7 @@ def cmd_cleanup(args: argparse.Namespace) -> int: ) if args.json: machine.emit( - _cleanup_document( + cleanup_document( dry_run=args.dry_run, killed=killed, live=live, unknown=unknown, windows=windows ) ) @@ -1881,7 +1881,7 @@ def cmd_clean(args: argparse.Namespace) -> int: if args.json: machine.emit( - _clean_document( + clean_document( dry_run=dry, retain=retain, cleanup_policy=pol.cleanup, diff --git a/src/bmad_loop/documents.py b/src/bmad_loop/documents.py index 1b846932..23eb61dd 100644 --- a/src/bmad_loop/documents.py +++ b/src/bmad_loop/documents.py @@ -24,6 +24,16 @@ So: add a new command's builder here, not in ``cli.py``. ``cli.py`` re-imports these names, which is what keeps ``cli.STATUS_SCHEMA_VERSION`` and friends resolving for existing callers and tests. + +Every name in this module is public, and a new builder must be too: the +projection surface *is* the API, so a leading underscore would say "do not +import this" about the one thing the module exists to be imported for. The +builders carried one until #212 only because they were born private inside +``cli.py``. ``run_token_totals`` is public for the same reason even though it +projects no document of its own — ``cmd_status`` calls it across the module +boundary to render text, and a private name imported by another module is the +contradiction this module is meant not to have. Keep genuinely intra-module +helpers private, as :mod:`bmad_loop.probe` and :mod:`bmad_loop.diagnostics` do. """ from __future__ import annotations @@ -43,7 +53,7 @@ VALIDATE_SCHEMA_VERSION = 1 -def _validate_document(report: ValidationReport, stories_on: bool, spec_folder: str) -> dict: +def validate_document(report: ValidationReport, stories_on: bool, spec_folder: str) -> dict: """The `validate --json` document: the verdict plus every check that produced it. Obeys the pure-document contract in machine.py (additive-only evolution; @@ -76,7 +86,7 @@ def _validate_document(report: ValidationReport, stories_on: bool, spec_folder: "ok": report.passed, "mode": "stories" if stories_on else "sprint", # "" rather than null in sprint mode, where a spec folder is inapplicable — - # the same convention as _list_document's paused_stage. + # the same convention as list_document's paused_stage. "spec_folder": spec_folder if stories_on else "", "counts": report.counts(), "findings": [ @@ -94,7 +104,7 @@ def _validate_document(report: ValidationReport, stories_on: bool, spec_folder: DECISIONS_SCHEMA_VERSION = 1 -def _decisions_document(pending: list[Decision]) -> dict[str, object]: +def decisions_document(pending: list[Decision]) -> dict[str, object]: """The `decisions --json` document: every pending decision, in DW order. Obeys the pure-document contract in machine.py (additive-only evolution; @@ -140,7 +150,7 @@ def _decisions_document(pending: list[Decision]) -> dict[str, object]: STATUS_SCHEMA_VERSION = 1 -def _run_token_totals(state: RunState) -> tuple[int, int, float]: +def run_token_totals(state: RunState) -> tuple[int, int, float]: """Run-level token totals as ``(raw, weighted, weight)``. The weight is the run's persisted snapshot — never live policy — so the @@ -156,7 +166,7 @@ def _run_token_totals(state: RunState) -> tuple[int, int, float]: return raw, weighted, weight -def _status_document(state: RunState) -> dict[str, object]: +def status_document(state: RunState) -> dict[str, object]: """The `status --json` document: the stable machine-readable contract. Obeys the pure-document contract in machine.py (additive-only evolution; @@ -164,9 +174,9 @@ def _status_document(state: RunState) -> dict[str, object]: status text, which is best-effort and free to change. Everything here is derived from state.json alone — never from live policy or other project files — so a consumer can reproduce the document, and the weight matches - what the run actually enforced (see _run_token_totals). + what the run actually enforced (see run_token_totals). """ - raw_total, weighted_total, weight = _run_token_totals(state) + raw_total, weighted_total, weight = run_token_totals(state) if state.finished: status = "finished" elif state.paused: @@ -220,7 +230,7 @@ def _status_document(state: RunState) -> dict[str, object]: LIST_SCHEMA_VERSION = 1 -def _list_document(infos: list[RunInfo]) -> dict[str, object]: +def list_document(infos: list[RunInfo]) -> dict[str, object]: """The `list --json` document: one entry per run, oldest first. Obeys the pure-document contract in machine.py (additive-only evolution; @@ -252,7 +262,7 @@ def _list_document(infos: list[RunInfo]) -> dict[str, object]: CLEANUP_SCHEMA_VERSION = 1 -def _cleanup_document( +def cleanup_document( *, dry_run: bool, killed: list[str], @@ -290,7 +300,7 @@ def _cleanup_document( CLEAN_SCHEMA_VERSION = 1 -def _clean_document( +def clean_document( *, dry_run: bool, retain: int, diff --git a/tests/conftest.py b/tests/conftest.py index c8ea2fcc..274fc56e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -174,7 +174,7 @@ def make_validate_document(findings, *, stories_on: bool = False, spec_folder: s document (a specific severity mix, a specific detail shape) and no subprocess. - It is built by driving the same ValidationReport -> _validate_document path + It is built by driving the same ValidationReport -> validate_document path the CLI drives, so the shape cannot drift from the contract by being hand-written. Going through ValidationReport.add also means its assert (checks.py) rejects invented check ids: a test cannot quietly pin behaviour @@ -183,7 +183,7 @@ def make_validate_document(findings, *, stories_on: bool = False, spec_folder: s report = ValidationReport() for check, severity, message, detail in findings: report.add(check, severity, message, detail) - return documents._validate_document(report, stories_on, spec_folder) + return documents.validate_document(report, stories_on, spec_folder) @pytest.fixture(scope="session") diff --git a/tests/test_cli.py b/tests/test_cli.py index f2a3cc20..fffb493f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1057,6 +1057,60 @@ def test_list_json_paused_run_carries_stage(project, capsys): assert entry["paused_stage"] == PAUSE_ESCALATION +# ---------------------------------------- documents.py as an imported library + +# documents.py exists so a non-CLI frontend (the planned web backend) can import +# the builders and serialize them itself, never shelling out to the CLI to parse +# its stdout. These two pin that promise the only way that matters: drive one +# fixture down BOTH paths and assert the same dict comes back. Without them the +# library path has no coverage of its own — every other --json test reaches the +# builders through `cli.main`, so a change that reached `status --json` but not +# `status_document` (or the reverse) would land green and only break in a +# consumer. Equality is asserted against the RAW builder return, not a +# json round-trip: a consumer holds this dict before serializing it, so a tuple +# where a list belongs is a real defect the round-trip would hide. + + +def test_status_document_library_call_matches_the_cli(project, capsys): + from bmad_loop.documents import status_document + from bmad_loop.journal import load_state + from bmad_loop.model import Phase, StoryTask, TokenUsage + + task = StoryTask(story_key="1-1-login", epic=1, phase=Phase.DONE) + task.tokens = TokenUsage( + input_tokens=100, output_tokens=50, cache_creation_tokens=10, cache_read_tokens=1000 + ) + run_dir = _make_run_with_tokens(project, {"1-1-login": task}, weight=0.5) + + from_cli = _status_json(project, capsys) + from_library = status_document(load_state(run_dir)) + + assert from_library == from_cli + + +def test_list_document_library_call_matches_the_cli(project, capsys): + # cmd_list sources its RunInfos the same lazy way — data.py has no textual + # imports, so this does not drag the TUI into a library consumer's process. + from bmad_loop.documents import list_document + from bmad_loop.tui.data import discover_runs + + # Deterministic statuses only: running/interrupted probe pid liveness and + # would flake (see _make_list_run). + _make_list_run(project, "20260101-000000-aaaa", started_at="2026-01-01T00:00:00", finished=True) + _make_list_run( + project, + "20260102-000000-bbbb", + started_at="2026-01-02T00:00:00", + run_type="sweep", + stopped=True, + ) + + from_cli = _list_json(project, capsys) + from_library = list_document(discover_runs(project.project)) + + assert from_library == from_cli + + def test_attach_records_return_pane_inside_tmux(project, monkeypatch): from bmad_loop.tui import launch @@ -3192,7 +3246,7 @@ def test_validate_json_mode_and_spec_folder_reflect_the_flag(project, capsys, sp """`--spec` forces stories mode, and the document says which queue was gated. spec_folder is "" — not null — in sprint mode, where it is inapplicable: the - same ""-for-inapplicable convention as _list_document's paused_stage.""" + same ""-for-inapplicable convention as list_document's paused_stage.""" install_bmad_config(project) _setup_stories_fixture(project, [_stories_entry("1")]) _write_policy(project.project, CLAUDE_ONLY_POLICY) # sprint policy either way