diff --git a/.console/backlog.md b/.console/backlog.md index 2ed3c014d..d1357f1cb 100644 --- a/.console/backlog.md +++ b/.console/backlog.md @@ -4,6 +4,115 @@ _Durable work inventory. Update after each meaningful chunk of progress._ ## Done +### 2026-07-15: Stage 4 — Refactor existing code to use the new shared helper (✅ COMPLETE) +- **Objective**: Independently re-verify Stage 2's migration against the "refactor existing + code" acceptance bar (identified/updated all relevant callsites, replaced redundant + console output code, behavior identical, consistent patterns) rather than take that + stage's own summary at face value. This closes the `print_structured()` objective. +- **Status**: ✅ COMPLETE — no source changes needed; migration re-confirmed correct. +- **Verification performed**: + - Swept the full source tree for remaining `typer.echo(json.dumps(...))` / + `console.print(json.dumps(...))` bypass patterns — none found outside `cli_output.py`'s + own docstring. + - Walked every remaining `json.dumps`/`console.print` occurrence in the 9 migrated files + and confirmed each is legitimately out of `print_structured`'s scope (inline debug + markup, disk writes, the `--pretty`/non-`--pretty` dual mode in `observer/cli.py show`, + the `ExtractionReportFormatter`-routed combined-output branch in `query-flaky-tests`, a + discarded serializability guard). + - Confirmed `artifact_index/cli.py`'s `default=str` (new) vs. `default=_path_default` + (old) swap is behavior-neutral — both migrated sites' payloads already pre-stringify + every `Path` before assembly, so the `default=` fallback was dead code pre-migration. + - Re-ran `ruff check`/`ruff format --check` (clean, 15 touched files) and the full suite: + 10298 passed, 6 failed (same pre-existing sandbox/timing failures as Stages 2-3), 21 + skipped, 2 xfailed — zero new failures. +- **Acceptance Criteria — ALL MET** ✅ (all relevant callsites identified/updated; redundant + console output code replaced with `print_structured` calls; behavior identical to + original; all refactored calls use consistent patterns). + +### 2026-07-15: Stage 3 — Write comprehensive tests for the helper function (✅ COMPLETE) +- **Objective**: Ensure `print_structured()` has comprehensive, 100%-coverage unit tests + proving normal-case and edge-case correctness (this was flagged as the closing stage in + Stage 2's "Next Stage" note). +- **Status**: ✅ COMPLETE — Stage 2 had already added 15 tests at 100% line/branch coverage; + extended to 22 tests by adding cases the docstring documents but didn't yet exercise: + `str` input (proves it's rendered as a JSON string scalar, not parsed — the documented + "callers must pass data, not `model_dump_json()`" contract), `bool`/`int`/`float` + primitives, a `dict` subclass (`OrderedDict`, pinning that it takes the passthrough + branch rather than the non-dict-`Mapping` branch), non-ASCII/unicode preservation + (`ensure_ascii=False`), and pretty-print indentation on nested payloads. +- **Changes**: `tests/unit/test_cli_output.py` — added + `TestMappingInput.test_dict_subclass_passthrough_not_routed_through_mapping_branch`, 4 + new tests in `TestOtherJsonNativeInputs` (bool/int/float/str), and a new + `TestUnicodeAndFormatting` class (2 tests). No production code changed. +- **Verification**: `ruff check`/`ruff format --check` clean on the test file. Coverage: + `src/operations_center/cli_output.py` 100.00% line, 100.00% branch (15/15 stmts, 6/6 + branches). Full suite: 10298 passed, 6 failed (same 6 pre-existing sandbox/timing + failures as Stage 2's run — `test_race_condition_guards.py` ×2, + `test_check_signal_collector.py`, `test_custodian_sweep.py`, + `test_dependency_drift_collector.py`, `test_snapshot_edge_cases.py` — zero new + failures), 21 skipped, 2 xfailed. +- **Acceptance Criteria — ALL MET** ✅ + 1. ✅ Unit tests cover normal cases and edge cases (22 tests: dict/BaseModel/dataclass/ + Mapping/dict-subclass/list/None/empty/bool/int/float/str/sort_keys/default=str + fallback/soft-wrap/no-ANSI/unicode/indent formatting) + 2. ✅ Tests verify output format and structure (JSON parse-back assertions throughout; + explicit indent/multi-line and no-escape-sequence checks) + 3. ✅ Tests pass with 100% coverage of helper code (verified via `--cov`) + 4. ✅ Test file placed in appropriate test directory (`tests/unit/test_cli_output.py`, + matching the module's own `tests/unit/` convention) + +### 2026-07-15: Stage 2 — Implement `print_structured()` and migrate call sites (✅ COMPLETE) +- **Objective**: Replace 4 inconsistent JSON-output mechanisms across CLI files with one + shared helper that always renders structured payloads via `console.print_json(...)`. +- **Status**: ✅ COMPLETE — helper implemented, 9 target files migrated (15 call sites), + tests added, 0 lint violations, 0 new test failures. +- **Changes**: + - `src/operations_center/cli_output.py` (NEW) — `print_structured(console, output, *, + sort_keys=False)` per Stage 1 design. + - Migrated: `entrypoints/audit/main.py` (3 sites, incl. 1 not in Stage 1's table — + `list-active --json`), `entrypoints/calibration/main.py` (3), `entrypoints/run_show/main.py`, + `entrypoints/worker_backend_status/main.py`, `entrypoints/worker_backend_probe/main.py`, + `run_memory/cli.py`, `artifact_index/cli.py` (2 of 3 sites — see below), + `entrypoints/governance/main.py` (2), `observer/cli.py` (4, incl. `show --pretty` which + previously used the "already-correct" `print_json(json_string)` pattern). + - Deleted the stale `observer/cli.py` soft-wrap comment that justified a since-removed + `typer.echo` call. + - **Corrected scope from Stage 1's design doc**: `artifact_index/cli.py`'s + `get-artifact --print-content` call site was mischaracterized in the design doc as a + "read-json command"; it's actually a raw content dump (JSON or text, per + `content_type`) with `--max-bytes` truncation — left unmigrated since `print_structured` + has no truncation equivalent. `observer/cli.py`'s `query-flaky-tests` combined-JSON + branch routes through `ExtractionReportFormatter` (a different existing abstraction), + not a naked `json.dumps` bypass — also left alone, consistent with not being in Stage 1's + scoped table. + - `tests/unit/test_cli_output.py` (NEW, 15 tests) — dict/BaseModel/dataclass(nested)/ + Mapping/list/None/empty-collection inputs, `sort_keys` both ways, `default=str` + fallback, soft-wrap regression, no-ANSI-on-non-tty. + - Updated 7 existing tests across `test_main_cov.py` in `audit`/`calibration`/`governance` + entrypoints — they mocked the *old* `model_dump_json()`/`typer.echo` mechanism using + `SimpleNamespace`/bare `MagicMock` fakes that don't satisfy `print_structured`'s + `isinstance(BaseModel)`/`is_dataclass` checks; rewrote to assert the CLI calls + `print_structured(console, )` with the right object (serialization itself is + covered by `test_cli_output.py`). +- **Verification**: `ruff check .` 0 violations; `ruff format --check` clean on all touched + files (68 unrelated pre-existing drifted files elsewhere, confirmed unrelated). Full suite: + 10291 passed, 6 failed — all 6 reproduce identically on the pre-change branch tip (sandbox + race conditions + 1 unrelated `test_custodian_sweep.py` assertion), zero new failures. +- **Acceptance Criteria — ALL MET** ✅ (helper implemented w/ full type hints+docstrings; + handles dict/BaseModel/dataclass/Mapping/list scenarios; integrates via `console.print_json`; + follows project style — flat module placement, SPDX header, ruff-clean). + +### Add shared `print_structured(console, output)` helper for Rich console output — Stages 0-1 +- **Stage 0** (Analyze codebase for Rich console usage patterns and identify requirements) — + ✅ COMPLETE 2026-07-15. See `.console/STAGE0_RICH_CONSOLE_HELPER_ANALYSIS.md` and + `.console/task.md`. +- **Stage 1** (Design the helper's signature, module location, and migration plan) — + ✅ COMPLETE 2026-07-15. See `.console/STAGE1_PRINT_STRUCTURED_DESIGN.md`. Signature: + `print_structured(console: Console, output: Any, *, sort_keys: bool = False) -> None` + in new module `src/operations_center/cli_output.py`; concrete per-file migration table + for 13 call sites across 9 files; verified `console.print_json` never soft-wraps + (resolves the `observer/cli.py:1075` typer.echo comment's stated concern). + ### 2026-06-26: Stage 5 — Verify complete implementation and test suite (✅ COMPLETE) - **Objective**: Full test-suite, linting, and verification that the `message_quality_rate` implementation is complete and mergeable - **Status**: ✅ COMPLETE — All acceptance criteria met; branch green and PR-ready diff --git a/.console/log.md b/.console/log.md index b53211d28..a6fed6845 100644 --- a/.console/log.md +++ b/.console/log.md @@ -1,3 +1,209 @@ +## 2026-07-15 — Stage 4: Refactor existing code to use the new shared helper (objective DONE) + +Stage 2 already performed the actual migration (15 call sites across 9 +files routed through `print_structured`). This stage's job was to +independently re-verify that migration against the "refactor existing +code" acceptance bar rather than take Stage 2's own summary at face value. + +Checks performed: +- Swept the full source tree for any remaining `typer.echo(json.dumps(...))` + / `console.print(json.dumps(...))` bypass patterns outside + `cli_output.py`'s own docstring — none found. +- Walked every remaining `json.dumps`/`console.print` occurrence in the 9 + migrated files (`observer/cli.py` has the most) and confirmed each is + legitimately out of scope: inline `[dim]` debug context inside a markup + string, disk writes with no console involved, the deliberate `--pretty` + vs. non-`--pretty` raw-string dual mode in `show`, the + `ExtractionReportFormatter`-routed combined-output branch in + `query-flaky-tests` (shares one `output` variable across json/markdown/ + table branches, so migrating just the json arm would break the shared + path), and a serializability guard whose `json.dumps` result is + discarded, never printed. +- Checked a real behavioral difference in the diff: `artifact_index/cli.py` + previously used `default=_path_default` (raises `TypeError` on anything + but a `Path`) while `print_structured` uses `default=str` (stringifies + anything unrecognized). Confirmed both migrated call sites' payloads + already pre-stringify every `Path` before assembly, so `default=` was + dead code at both sites pre-migration — no behavior change from the + swap. +- Re-ran `ruff check`/`ruff format --check` (clean on all 15 touched + files) and the full suite: 10298 passed, 6 failed, 21 skipped, 2 + xfailed — the same 6 pre-existing sandbox/timing failures as Stage 2/3's + baseline, zero new failures. + +No source changes were needed this stage; it's a verification pass, not a +fix. This closes the `print_structured()` objective: helper implemented, +all in-scope call sites migrated, tests comprehensive (22, 100% coverage), +full-suite/lint clean across three independent verification passes +(Stages 2, 3, 4). + +## 2026-07-15 — Stage 3: Write comprehensive tests for the helper function + +Stage 2 already shipped `tests/unit/test_cli_output.py` with 15 tests at +100% line/branch coverage on `print_structured()`. This stage's job was to +audit that suite against the helper's own documented contract (docstring + +Stage 1 design doc §4/§6) rather than just its coverage number, since +line/branch coverage can hit 100% while still missing documented-but-untested +behaviors. + +Found and closed 5 such gaps, adding 7 tests (22 total): +- The docstring explicitly states callers "must pass data, not + `model.model_dump_json()`" because a bare `str` is rendered as a JSON + string scalar, not parsed — this contract had no test. Added one that + renders a JSON-looking string and asserts it comes back as a quoted + scalar, not the object it encodes. +- `bool`/`int`/`float` primitive passthrough (the "any other + JSON-serializable value" branch) had no direct test. +- The `dict`-subclass dispatch path was untested: `OrderedDict` IS a + `dict`, so it must hit the `else` passthrough branch, not the + non-`dict`-`Mapping` branch — both produce correct output, but only one + is the intended code path, so this pins the dispatch logic itself, not + just its output. +- `ensure_ascii=False` (unicode preserved, not escaped to `\uXXXX`) and the + `indent=2` pretty-print formatting were both baked into the + `console.print_json` call but never asserted. + +No production code changed — `cli_output.py` was already correct. +Verification: `ruff check`/`ruff format --check` clean; `pytest --cov` +confirms 100.00% line + 100.00% branch coverage (unchanged, since the new +tests exercise already-covered lines through previously-untested inputs, +not new lines). Full suite: 10298 passed, 6 failed, 21 skipped, 2 xfailed — +the same 6 pre-existing sandbox/timing failures as Stage 2's baseline run +(`test_race_condition_guards.py` ×2, `test_check_signal_collector.py`, +`test_custodian_sweep.py`, `test_dependency_drift_collector.py`, +`test_snapshot_edge_cases.py`), zero new failures. + +Per the Overall Plan, Stage 4 (final full-suite/lint verification) remains +technically next, but this stage's own verification run already satisfies +it in substance — flagged in task.md as likely a quick confirmation rather +than new work. + +## 2026-07-15 — Stage 2: Implement `print_structured()` and migrate call sites + +Created `src/operations_center/cli_output.py` per the Stage 1 design exactly +(`print_structured(console: Console, output: Any, *, sort_keys: bool = False) +-> None`), then migrated all 9 target files (15 call sites total — 13 from +the design doc's table plus 2 found while implementing). + +Two corrections to Stage 1's design doc, found by re-reading the actual code +during migration rather than trusting the earlier table: +- `entrypoints/audit/main.py`'s `list-active --json` command bypasses + `Console` via `typer.echo(_json.dumps(...))` too — not caught by either + Stage 0 or Stage 1's analysis. Migrated for consistency with the rest of + the file. +- `artifact_index/cli.py`'s `get-artifact --print-content` call site — + labeled "read-json command" in the design doc — is actually a raw + content dump (JSON or text, chosen by `content_type`) with `--max-bytes` + truncation logic applied uniformly to both. `print_structured` has no + truncation equivalent, so migrating it would silently drop a real CLI + feature. Left unmigrated; `_path_default` and the `json` import both stay + since this is their only remaining caller. Also left alone: + `observer/cli.py`'s `query-flaky-tests` combined-JSON branch, which + routes through `ExtractionReportFormatter` (a distinct pre-existing + formatting abstraction with its own json/markdown/table methods), not a + naked `json.dumps` bypass — never in Stage 1's scoped table to begin + with. + +Migrating `observer/cli.py`'s `show --pretty` command required extra care: +its `pretty` flag isn't gated by `--quiet` today (pre-existing asymmetry, +not something to fix here), and the same code path serves both `--format +json` and `--format yaml`. Preserved both quirks exactly — `print_structured` +now handles only the json+pretty combination; yaml+pretty keeps calling +`console.print_json(output)` on the pre-serialized YAML string as before +(a latent oddity, unrelated to this change). + +Migrating broke 7 existing tests in `test_main_cov.py` (audit ×3, +calibration ×3, governance ×1) — they mocked the *old* +`model_dump_json()`/`typer.echo` mechanism with `SimpleNamespace`/bare +`MagicMock` fakes. `print_structured` type-dispatches via +`isinstance(BaseModel)`/`dataclasses.is_dataclass`, which those fakes don't +satisfy, so they fell through to the `default=str` catch-all and printed a +stringified mock repr instead of the payload. Rewrote each to assert the +CLI calls `print_structured(console, )` with the right +argument, rather than re-testing `print_structured`'s own serialization +(that's `tests/unit/test_cli_output.py`'s job, 15 tests, new). + +Verification: `ruff check .` 0 violations repo-wide; `ruff format --check` +clean on every touched file (68 unrelated files elsewhere have pre-existing +formatting drift, confirmed by name and by reproducing on the unmodified +branch tip). Full suite: 10291 passed, 6 failed, 21 skipped, 2 xfailed — all +6 failures reproduce identically before this stage's changes (sandbox +race-condition tests in observer/collectors + one unrelated +`test_custodian_sweep.py` assertion), so zero new failures. Updated +`.console/task.md`/`backlog.md` with Stage 2 completion; this objective has +no further stage queued (see task.md's "Next Stage" note on optional +Stage 3 full-suite re-verification if the operator wants it as a distinct +closing step). + +## 2026-07-15 — Stage 1: Design `print_structured()` signature, module location, migration plan + +Design (no code change) complete — see `.console/STAGE1_PRINT_STRUCTURED_DESIGN.md`. + +Signature: `print_structured(console: Console, output: Any, *, sort_keys: bool = False) -> None`. +The `sort_keys` kwarg wasn't in Stage 0's requirements summary — added after +reading all 9 target files' actual `json.dumps` calls and finding 4 of them +(`run_show`, `worker_backend_status`, `worker_backend_probe`, `run_memory/cli.py`) +pass `sort_keys=True` today for deterministic, automation-consumed output; a +signature without it would silently reorder those files' keys on migration. + +Module location: new flat top-level module `src/operations_center/cli_output.py` +(sibling to `capability_ownership.py`/`close_invariants.py`/etc.), not nested +under `entrypoints/` — 3 of the 9 target files (`observer/cli.py`, +`artifact_index/cli.py`, `run_memory/cli.py`) are themselves top-level packages, +not `entrypoints/` submodules, and there's no existing convention for them to +import shared utilities from `entrypoints/`. `contracts/common.py` was considered +and rejected — domain-model package, no existing `rich` dependency. + +Key empirical finding (verified against installed `rich==15.0.0`, not assumed): +`console.print_json()` never soft-wraps output regardless of `Console.width` +(hardcoded `soft_wrap=True` inside Rich's own implementation), and produces no +ANSI codes on non-tty output. This directly resolves the concern behind the +comment at `observer/cli.py:1075-1076` ("typer.echo ... so piped/redirected +JSON is not soft-wrapped — the watchdog collector parses this from a file") — +that comment is correct about `console.print(json.dumps(...))` wrapping, but +`print_json` doesn't have that problem, so that call site (and the other 5 +`typer.echo` sites) can safely migrate. Also corrected Stage 0's file-level +categorization: `artifact_index/cli.py` has 2 of 3 JSON call sites bypassing +`Console` via `typer.echo`, not just the one "unhighlighted plain text" pattern +Stage 0's medium-priority label implied — flagged in the design doc so Stage 2 +doesn't under-scope that file's changeset. + +Produced a concrete per-file before/after migration table (13 call sites across +9 files) so Stage 2 is a mechanical implementation pass, not another discovery +pass. Updated `.console/task.md` (Stage 1 acceptance criteria, Stage 2 starting +point) and `.console/backlog.md`. No source files changed this stage. + +## 2026-07-15 — Stage 0: Analyze Rich console usage, scope `print_structured()` helper + +New objective from operator/issue tracker: add a shared helper (e.g. +`print_structured(console, output)`) so CLI commands stop hand-rolling the +JSON/table print path independently. Stage 0 (analysis only, no code change) +complete — see `.console/STAGE0_RICH_CONSOLE_HELPER_ANALYSIS.md`. + +Findings: 16 production files construct their own `rich.console.Console` and +implement a `--json`/`--format json` vs. table/text branch. The structured +(JSON) branch alone is done 4 inconsistent ways — only +`observer/cli.py:589` uses `console.print_json()` (the correct pattern); 7 +files bypass `Console` entirely via `typer.echo(json.dumps(...))` +(`entrypoints/audit`, `calibration`, `run_show`, `worker_backend_status`, +`worker_backend_probe`, `run_memory/cli.py`); 3 more route through `Console` +but print a pre-serialized string so it loses syntax highlighting +(`artifact_index/cli.py`, `entrypoints/governance/main.py`, plus one other +command in `observer/cli.py` itself). Also found: `status_color` ternary +duplicated 4× across `entrypoints/regression/main.py` and +`entrypoints/replay/main.py` — a candidate for a companion helper, not this +one. `entrypoints/setup/main.py` (interactive wizard) and +`observer/extraction_health_dashboard.py` (Panel/Table dashboard) are +confirmed out of scope for `print_structured` — too heterogeneous to +generalize profitably. + +Decision: scope `print_structured(console, output)` narrowly to the +structured/JSON path only (normalize dict/BaseModel/dataclass → +`console.print_json`); leave table, panel, and interactive-prompt rendering +untouched. Updated `.console/task.md` with the new objective, Stage 0 +completion, and Stage 1 starting point (design signature + migration plan for +the 9 high/medium-priority files). No source files changed this stage. + ## 2026-07-14 — feat(reviewer): C1 cross-family council for guardrail PRs (COUNCIL_VERDICT.md) Council spec Phase 2 (C1) — keyless change control for guardrail surfaces. A PR diff --git a/.console/task.md b/.console/task.md index abc2be27b..259da42b8 100644 --- a/.console/task.md +++ b/.console/task.md @@ -3,107 +3,320 @@ _The active assignment. One objective at a time._ _Replace contents when the objective changes. History belongs in log.md._ -## Objective — ✅ COMPLETE (verified 2026-07-14) +## Objective — ✅ COMPLETE (2026-07-15) -**COMPLETE** (shipped in PR #374, verified again 2026-07-07 and 2026-07-13 watchdog cycles). -Expose the sample `gaps` and `edge_cases` lists in the extraction-health CLI so an operator can inspect them directly. Definition of done: the CLI surfaces both lists; tests cover the new output — both met (111/111 tests passing in tests/unit/observer/test_extraction_health_queries.py + test_cli_extraction_health.py). +Add a shared helper (e.g. `print_structured(console, output)`) for all Rich +console structured-output printing, so CLI commands stop hand-rolling the +JSON/table print path independently. -No active operator directive. Proceeding to standard watchdog steps. - -Shipped in PR #374 (a675c1f7). Verified 2026-07-14: `ExtractionHealth.gaps`/`.edge_cases` -fields, sample collection in `get_extraction_health()`, and table-format CLI sections all -present; 111/111 tests pass. No operator directive is currently active — proceeding to -standard watchdog steps. Awaiting a new objective from the operator. +Prior objective (gaps/edge_cases CLI exposure) shipped in PR #374 and was +re-verified 2026-07-07/2026-07-13/2026-07-14; see log.md for that history. ## Overall Plan -Expose sample gaps and edge_cases lists in CLI for operator inspection. +0. Analyze current Rich `Console` usage across the codebase; define what + "structured output" means for the helper and which files it should target. +1. Design the helper's exact signature, module location, and migration plan. +2. Implement `print_structured()` and migrate the identified call sites. +3. Test: unit tests for the helper (dict/BaseModel/dataclass inputs, JSON + highlighting via a buffered `Console`) plus regression coverage on migrated + commands' `--json` output. +4. Final verification: full suite + lint, no new failures. ## Current Stage -**Stage 1: Design CLI output format for gaps and edge_cases exposure** ✅ COMPLETE +**Stage 4: Refactor existing code to use the new shared helper** ✅ COMPLETE +(2026-07-15) + +- Independently re-verified Stage 2's migration against the "refactor + existing code" acceptance bar: swept the full source tree for any + remaining `typer.echo(json.dumps(...))` / `console.print(json.dumps(...))` + bypass patterns outside `cli_output.py`'s own docstring — none found, so + no target call site was missed. +- Walked every `json.dumps`/`console.print` occurrence still present in the + 9 migrated files (`observer/cli.py` has the most: lines ~335, ~348, + ~579-596, ~702-715, ~903, ~1073) and confirmed each is legitimately out + of `print_structured`'s scope: inline `[dim]` debug context inside a + markup string, disk writes (`output.write_text`, no console involved), + the deliberate `--pretty` vs. non-`--pretty` raw-string dual mode in + `show`, the `ExtractionReportFormatter`-routed combined-output branch in + `query-flaky-tests` (shares one `output` variable across json/markdown/ + table branches — restructuring just the json arm would break that + shared path), and a serializability guard whose `json.dumps` result is + discarded, not printed. None are missed migrations. +- Checked the one behavioral wrinkle in the diff: `artifact_index/cli.py`'s + two migrated sites previously used `default=_path_default` (raises + `TypeError` on anything but a `Path`) while `print_structured` uses + `default=str` (stringifies anything). Confirmed both call sites' payload + dicts (`_run_summary()` + the `skipped` list) already pre-stringify every + `Path` before assembly, so the `default=` fallback was dead code at both + sites pre-migration — no behavior change. +- Re-ran `ruff check` / `ruff format --check` on all 15 touched files + (clean) and the full suite: 10298 passed, 6 failed, 21 skipped, 2 + xfailed — the same 6 pre-existing failures named in Stage 2/3 (sandbox + race conditions + one unrelated `test_custodian_sweep.py` assertion), + zero new failures. +- No source changes were needed this stage — Stage 2's migration already + satisfied the "refactor existing code to use the shared helper" + objective; this stage is the independent verification pass confirming + that. + +## Stage 4 Acceptance Criteria — ALL MET ✅ + +1. ✅ **Identified and updated all relevant callsites** — swept for + remaining bypass patterns (none found); all 9 Stage-0/1 target files + migrated; the 2 deliberately-excluded sites (`artifact_index/cli.py` + truncated content dump, `observer/cli.py` formatter-routed combined + output) re-confirmed genuinely out of scope, not oversights. +2. ✅ **Replaced redundant console output code with calls to + print_structured** — 15 call sites across 9 files now route through + the shared helper instead of hand-rolled `json.dumps`/`typer.echo`/ + pre-serialized `console.print`. +3. ✅ **Behavior remains identical to original implementation** — + `default=str` vs. the old `default=_path_default` verified as a no-op + difference (no raw `Path` ever reaches either call site's `json.dumps` + pre-migration); `sort_keys` preserved per-site; soft-wrap and no-ANSI + behavior verified in `test_cli_output.py`. +4. ✅ **All refactored calls use consistent patterns** — every migrated + site now calls `print_structured(console, [, sort_keys=True])` + with no per-file variation in how the payload is serialized or + rendered. + +Prior: Stage 3 ✅ COMPLETE (2026-07-15) + +- Stage 2 already added `tests/unit/test_cli_output.py` (15 tests, 100% + line/branch coverage). This stage audited that suite against the + helper's documented contract (docstring + Stage 1 design doc §4) and + found five behaviors it described but didn't yet exercise: `str` inputs + (proves the "callers must pass data, not `model_dump_json()`" contract — + a `str` is rendered as a JSON string scalar, not parsed), `bool`/`int`/ + `float` primitive passthrough, a `dict` *subclass* (`OrderedDict`, to pin + that it takes the `else` passthrough branch rather than the non-`dict` + `Mapping` branch — both would produce correct output, but only one is + the intended dispatch path), non-ASCII/unicode preservation + (`ensure_ascii=False`), and pretty-print indentation on nested payloads. +- Added 7 new tests (22 total): `TestMappingInput.test_dict_subclass_ + passthrough_not_routed_through_mapping_branch`; `TestOtherJsonNativeInputs. + test_bool_renders_as_json_literal` / `test_int_renders_as_json_number` / + `test_float_renders_as_json_number` / + `test_str_input_is_not_parsed_as_json_but_rendered_as_a_string_scalar`; + new `TestUnicodeAndFormatting` class with + `test_non_ascii_characters_are_not_escaped` and + `test_nested_payload_is_pretty_printed_with_indent`. +- No production code changed — `src/operations_center/cli_output.py` was + already correct and already at 100% coverage; this stage strengthens the + proof, it doesn't fix a gap. +- Verification: `ruff check`/`ruff format --check` clean on the test file. + `pytest --cov=operations_center.cli_output`: 22 passed, 100.00% line + coverage, 100.00% branch coverage (15/15 stmts, 6/6 branches). Full + suite: 10298 passed, 6 failed, 21 skipped, 2 xfailed — the 6 failures are + the identical named tests that failed on Stage 2's pre-change baseline + (`test_race_condition_guards.py` ×2, `test_check_signal_collector.py`, + `test_custodian_sweep.py`, `test_dependency_drift_collector.py`, + `test_snapshot_edge_cases.py`), zero new failures introduced. + +## Stage 3 Acceptance Criteria — ALL MET ✅ + +1. ✅ **Unit tests cover normal cases and edge cases** — 22 tests spanning + `dict`/`BaseModel`/`dataclass` (incl. nested)/non-`dict` `Mapping`/`dict` + subclass/`list`/`None`/empty dict & list/`bool`/`int`/`float`/`str`/ + `sort_keys` both ways/`default=str` fallback/soft-wrap regression/no-ANSI + on non-tty/unicode preservation/indent formatting. +2. ✅ **Tests verify output format and structure** — every test parses the + rendered output back with `json.loads` and asserts structural equality + (or, for scalars/formatting, asserts the exact rendered text); dedicated + checks for indentation (multi-line, 2-space) and absence of `\uXXXX` + escapes / ANSI codes. +3. ✅ **Tests pass with 100% coverage of helper code** — `pytest --cov` + reports 100.00% line and 100.00% branch coverage on + `src/operations_center/cli_output.py` (15/15 statements, 6/6 branches). +4. ✅ **Test file placed in appropriate test directory** — + `tests/unit/test_cli_output.py`, matching the flat-module-under-`tests/ + unit/` convention already used for the other Stage-2-migrated modules. + +Prior: Stage 2 ✅ COMPLETE (2026-07-15) + +- Created `src/operations_center/cli_output.py` with `print_structured()` + exactly per the Stage 1 design (`console: Console, output: Any, *, + sort_keys: bool = False`), full docstrings, type hints. +- Migrated all 9 target files (15 call sites — 13 from the design doc's + table plus 2 discovered while implementing: a `list-active --json` branch + in `entrypoints/audit/main.py` that bypassed `Console` via + `typer.echo(_json.dumps(...))`, not previously catalogued, and + `observer/cli.py`'s `show --pretty --format json` branch which now routes + through `print_structured` instead of `console.print_json(json_string)`). +- Deleted the stale soft-wrap comment at `observer/cli.py` (the + `typer.echo` call it justified no longer exists). +- Left `artifact_index/cli.py`'s `get-artifact --print-content` call site + (json.dumps + `_path_default`) **unmigrated** — Stage 1's design doc + mischaracterized it as a "read-json command"; it's actually a raw + content dump (JSON *or* text, chosen by `content_type`) with + `--max-bytes` truncation logic that `print_structured` has no equivalent + for. Migrating it would silently drop truncation. `_path_default` and + the `json` import both stay, since this is their only remaining caller. + `observer/cli.py`'s `query-flaky-tests --format json` combined-output + branch (lines ~886-899) was also left alone — it goes through + `ExtractionReportFormatter`, a different pre-existing formatting + abstraction, not a naked `json.dumps` bypass, and wasn't in Stage 1's + scoped table. +- Added `tests/unit/test_cli_output.py` (15 tests): dict / `BaseModel` + (verifies `mode="json"` datetime/Path conversion) / dataclass (incl. + nested) / non-`dict` `Mapping` / list / `None` / empty dict/list / + `sort_keys` True vs False / `default=str` fallback / soft-wrap + regression (220-char value stays on one line at `width=80`) / no ANSI + codes on non-tty output. +- Updated 4 existing CLI tests that asserted on the *old* serialization + mechanism (`model_dump_json()`/`typer.echo` mocks) in + `tests/unit/entrypoints/audit/test_main_cov.py` (3 tests) and + `tests/unit/entrypoints/calibration/test_main_cov.py` (3 tests) and + `tests/unit/entrypoints/governance/test_main_cov.py` (1 test) — the + test doubles are `SimpleNamespace`/bare `MagicMock` fakes, not real + `BaseModel`/dataclass instances, so they no longer match + `print_structured`'s type-dispatch; rewrote these to assert the CLI + calls `print_structured(console, )` with the right object, + rather than re-asserting on `print_structured`'s own serialization + (already covered by `test_cli_output.py`). +- Verification: `ruff check .` — 0 violations. `ruff format --check` on + all touched files — clean (repo-wide pre-existing drift in 68 unrelated + files, confirmed unrelated by name and by reproducing on unmodified + branch). Full suite: 10291 passed, 21 skipped, 2 xfailed, 6 failed — all + 6 failures reproduce identically on the branch tip *before* this + stage's changes (sandbox race conditions in + `test_race_condition_guards.py`/`test_check_signal_collector.py`/ + `test_dependency_drift_collector.py`/`test_snapshot_edge_cases.py`, plus + one unrelated `test_custodian_sweep.py` assertion) — zero new failures + introduced. + +Full design: `.console/STAGE1_PRINT_STRUCTURED_DESIGN.md`. + +Prior: Stage 1 ✅ COMPLETE (2026-07-15) — `.console/STAGE1_PRINT_STRUCTURED_DESIGN.md`. +Stage 0 ✅ COMPLETE (2026-07-15) — `.console/STAGE0_RICH_CONSOLE_HELPER_ANALYSIS.md`. + +## Stage 0 Acceptance Criteria — ALL MET ✅ + +1. ✅ **Identified all files using Rich console in the codebase** + - 16 production files import and use `rich.console.Console` (full list in + the analysis doc, Part 1). A broader `grep -rli rich` match of 51 files + was mostly false positives (the substring "rich" inside *enrich*/*richer* + in docstrings/comments) — confirmed by checking each for an actual + `rich` import. + - 2 test files construct `Console` directly for output capture/markup + assertions (not shared-helper consumers). + +2. ✅ **Documented current output patterns and use cases** + - Status/severity markup convention (red=error, yellow=warning, + green=success, dim=muted) — consistent in meaning but re-implemented + inline in every file, including a `status_color` ternary duplicated + 4 times across `regression/main.py` and `replay/main.py`. + - Dual-mode (human vs. JSON) output — implemented 4 different, + inconsistent ways across files; only `observer/cli.py` uses + `console.print_json()`, 7 files bypass `Console` entirely via + `typer.echo(json.dumps(...))`. + - Table rendering — one-off `rich.table.Table` construction per command; + too heterogeneous to generalize, left out of scope. + - Panel/dashboard composition (`observer/extraction_health_dashboard.py`) + and interactive wizard (`entrypoints/setup/main.py`) — distinct + sub-patterns, secondary/non-fit for this helper. + - See analysis doc Part 2 for full detail and code references. + +3. ✅ **Clarified what "structured output" means for this helper** + - Defined as the machine-readable JSON payload path of a dual-mode CLI + command (the current `--json`/`--format json` branch), not the + human-readable table/text branch. + - `print_structured(console, output)` normalizes `dict` / Pydantic + `BaseModel` / dataclass-derived payloads to one JSON string + (`indent=2, ensure_ascii=False, default=str`) and always renders via + `console.print_json(...)` so structured output never again bypasses the + caller's `Console`. Full contract in analysis doc Part 3. + +4. ✅ **Listed files that would benefit from the shared helper** + - High priority (bypasses `Console` today): `entrypoints/audit/main.py`, + `entrypoints/calibration/main.py`, `entrypoints/run_show/main.py`, + `entrypoints/worker_backend_status/main.py`, + `entrypoints/worker_backend_probe/main.py`, `run_memory/cli.py`. + - Medium priority (routes through `Console` but unhighlighted): + `artifact_index/cli.py`, `entrypoints/governance/main.py`, + `observer/cli.py`. + - Low priority / partial fit: `observer/extraction_health_dashboard.py`. + - Not applicable (no JSON branch / table-only / interactive-only): + `entrypoints/artifacts/main.py`, `entrypoints/fixtures/main.py`, + `entrypoints/regression/main.py`, `entrypoints/replay/main.py`, + `entrypoints/setup/main.py`, `entrypoints/setup/providers.py`. + - Full ranked list with rationale in analysis doc Part 4. ## Stage 1 Acceptance Criteria — ALL MET ✅ -1. ✅ **Define output format for sample gaps list** - - Format: `list[str]` — flat array of pytest node ID strings - - JSON key: `"gaps"` inside the existing ExtractionHealth JSON object - - Example: `"gaps": ["test_module::test_missing_both", "tests/unit/foo.py::TestBar::test_baz"]` - - Cap: up to 10 samples; full count already carried by `no_extraction` - -2. ✅ **Define output format for sample edge_cases list** - - Format: `list[dict]` — each entry has `test_id` (string) and `issue` (string) - - JSON key: `"edge_cases"` inside the existing ExtractionHealth JSON object - - Example: `"edge_cases": [{"test_id": "test_module::test_foo", "issue": "truncated_message"}]` - - Cap: up to 10 entries; a test with 2 issues produces 2 entries - - Full counts per issue type already carried by `edge_case_summary` - -3. ✅ **Determine what fields to include per gap** - - Single field: `test_id` (the pytest node ID string, e.g. `"test_module::test_missing_both"`) - - No per-item `reason` field needed — all gaps share the same reason (both `test_name` - and `assertion_message` are None); reason is implicit from being in the `gaps` array - -4. ✅ **Determine what fields to include per edge_case** - - `test_id`: string — full pytest node ID - - `issue`: string — one of `"truncated_message"`, `"special_chars"`, `"malformed_exception"` - (singular form; maps to the corresponding `edge_case_summary` counter) - -5. ✅ **Plan integration with existing extraction-health command** - - **JSON mode** (`--format json`): zero CLI changes needed — `asdict(health)` auto-includes - new dataclass fields. New output adds `"gaps"` and `"edge_cases"` keys alongside - `"success_rate"`, `"no_extraction"`, etc. - - **Table mode** (`--format table`): one additional branch in `cli.py:1049-1055` to print - gap and edge_case sample lines below the summary line when either list is non-empty: - ``` - extraction success_rate=80.0% complete=4 partial=0 none=1 - gaps (1 test, showing 1): - test_module::test_missing_both - edge_cases (2 issues, showing 2): - test_module::test_truncated [truncated_message] - test_module::test_special [special_chars] - ``` - - **Baseline**: existing tests (26/26) confirmed passing before any code change - -## Full JSON Output Shape (after Stage 2 implementation) - -```json -{ - "success_rate": 80.0, - "complete_extraction": 4, - "partial_extraction": 0, - "no_extraction": 1, - "edge_case_summary": { - "truncated_messages": 2, - "special_chars": 1, - "malformed_exceptions": 0 - }, - "gaps": [ - "test_module::test_missing_both" - ], - "edge_cases": [ - {"test_id": "test_module::test_complete_extraction", "issue": "truncated_message"}, - {"test_id": "test_module::test_complete_extraction", "issue": "special_chars"} - ], - "history": { ... } -} -``` - -## Implementation Path (Stage 2) - -1. **`query_flaky.py:98-117`** — `ExtractionHealth` dataclass: add two fields: - ```python - gaps: list[str] = dataclass_field(default_factory=list) - edge_cases: list[dict] = dataclass_field(default_factory=list) - ``` - -2. **`query_flaky.py:358-395`** — `get_extraction_health()` loop: collect samples while - iterating (first 10 of each); append to local lists before `return ExtractionHealth(...)`. - -3. **`cli.py:1049-1055`** — table branch: after the summary line, print gap and edge_case - sample sections when either list is non-empty. - -4. **Tests**: - - `tests/unit/observer/test_extraction_health_queries.py` — add `TestExtractionHealthGaps` - and `TestExtractionHealthEdgeCases` classes + update `TestExtractionHealthDataclass` - - `tests/unit/observer/test_cli_extraction_health.py` — add JSON-shape assertions for - `gaps`/`edge_cases` keys and table-format section tests +1. ✅ **Defined function signature with parameter types and return value** + - `print_structured(console: Console, output: Any, *, sort_keys: bool = False) -> None` + in new module `src/operations_center/cli_output.py`. The `sort_keys` + keyword was discovered (not assumed) by reading all 9 target files' actual + `json.dumps` calls — 4 of them pass `sort_keys=True` for deterministic, + automation-consumed output and would silently regress without it. + +2. ✅ **Decided on module/file location for the shared helper** + - New flat top-level module `src/operations_center/cli_output.py`, sibling + to existing single-file cross-cutting modules (`capability_ownership.py`, + `close_invariants.py`, `impact_analysis.py`, etc.) rather than nested + under `entrypoints/`, because 3 of the 9 target files + (`observer/cli.py`, `artifact_index/cli.py`, `run_memory/cli.py`) are + top-level packages themselves, not `entrypoints/` submodules. Full + rationale in the design doc §2. + +3. ✅ **Documented expected behavior, output format(s), and edge cases** + - Normalization rules for `BaseModel`/`dataclass`/`Mapping`/other (§3); + verified empirically against the installed `rich==15.0.0` that + `console.print_json` never soft-wraps (hardcoded `soft_wrap=True` + internally) and emits no ANSI codes on non-tty output — this directly + resolves the soft-wrap concern behind the `typer.echo` comment at + `observer/cli.py:1075-1076`, meaning that call site (and the other 5 + `typer.echo` sites) are safe to migrate. Also documented: pre-serialized + JSON strings are NOT auto-parsed (callers must pass data, not a + `model_dump_json()` string), `None`/empty-collection behavior, circular + references, and the dual console+disk-write case in `governance/main.py`. + Full detail in design doc §4. + +4. ✅ **Verified design aligns with existing Rich console patterns in codebase** + - Always renders via `console.print_json(...)`, the one already-correct + existing pattern (`observer/cli.py:589`); takes the caller's own + `Console` instance (no new global); matches existing `ensure_ascii=False`/ + `indent=2`/`default=str` conventions. Concrete per-file migration table + (13 call sites across 9 files) in design doc §5, including a correction + to Stage 0's file-level categorization of `artifact_index/cli.py` (2 of + its 3 JSON call sites bypass `Console` via `typer.echo`, not just the one + Stage 0's summary implied). + +Full design: `.console/STAGE1_PRINT_STRUCTURED_DESIGN.md`. + +## Stage 2 Acceptance Criteria — ALL MET ✅ + +1. ✅ **Helper function implemented with full type hints and docstrings** — + `src/operations_center/cli_output.py`. +2. ✅ **Handles various structured output scenarios** — `dict`, `BaseModel`, + `dataclass` (incl. nested), non-`dict` `Mapping`, list, `None`, empty + collections, `sort_keys` True/False, `default=str` fallback; all covered + by `tests/unit/test_cli_output.py` (15 tests). +3. ✅ **Integrates properly with Rich console** — always renders via + `console.print_json(data=...)`; verified no soft-wrap (regression test) + and no ANSI codes on non-tty output. +4. ✅ **Code follows project style and conventions** — SPDX header, flat + module placement matching existing single-file utilities, `ruff check`/ + `ruff format` clean. + +Migration status: all 9 Stage-1 target files migrated (15 call sites — 13 +from the design table plus 2 found during implementation); one call site +(`artifact_index/cli.py` `get-artifact --print-content`) deliberately left +unmigrated (truncation semantics `print_structured` doesn't support — see +Current Stage notes above for full rationale). Full verification results +(tests/lint) also in Current Stage above. + +## Next Stage + +None queued. This objective (`print_structured()` helper + migration) is +now complete across all 5 stages (0-4): helper implemented, all in-scope +call sites migrated and independently re-verified against the "refactor +existing code" acceptance bar, tests comprehensive (22, 100% coverage), +full-suite/lint re-verified twice with zero new failures. Remaining open +items are explicitly out of scope for this ticket (see Stage 1 design doc +§6): a companion status-message helper for the red/yellow/green/dim +severity convention, and `artifact_index/cli.py`'s truncated +raw-content-dump path. Objective can be marked DONE. diff --git a/src/operations_center/artifact_index/cli.py b/src/operations_center/artifact_index/cli.py index b365f6a11..354506257 100644 --- a/src/operations_center/artifact_index/cli.py +++ b/src/operations_center/artifact_index/cli.py @@ -26,6 +26,8 @@ from rich.console import Console from rich.table import Table +from operations_center.cli_output import print_structured + from .errors import ( ArtifactNotFoundError, ArtifactPathUnresolvableError, @@ -94,17 +96,13 @@ def cmd_index( ) if json_output: - typer.echo( - json.dumps( - { - "search_root": str(idx.search_root), - "runs": [_run_summary(r) for r in idx.runs], - "skipped": [(str(p), reason) for p, reason in idx.skipped_paths], - }, - indent=2, - default=_path_default, - ensure_ascii=False, - ) + print_structured( + console, + { + "search_root": str(idx.search_root), + "runs": [_run_summary(r) for r in idx.runs], + "skipped": [(str(p), reason) for p, reason in idx.skipped_paths], + }, ) if not idx.runs: raise typer.Exit(code=2) @@ -184,31 +182,27 @@ def cmd_index_show( artifacts = [a for a in artifacts if a.status.value == "missing"] if json_output: - typer.echo( - json.dumps( - { - "run": _run_summary(run), - "artifacts": [ - { - "artifact_id": a.artifact_id, - "artifact_kind": a.artifact_kind, - "location": a.location.value, - "path_role": a.path_role.value, - "source_stage": a.source_stage, - "status": a.status.value, - "path": a.path, - "resolved_path": str(a.resolved_path) if a.resolved_path else None, - "exists_on_disk": a.exists_on_disk, - "is_repo_singleton": a.is_repo_singleton, - "size_bytes": a.size_bytes, - } - for a in artifacts - ], - }, - indent=2, - default=_path_default, - ensure_ascii=False, - ) + print_structured( + console, + { + "run": _run_summary(run), + "artifacts": [ + { + "artifact_id": a.artifact_id, + "artifact_kind": a.artifact_kind, + "location": a.location.value, + "path_role": a.path_role.value, + "source_stage": a.source_stage, + "status": a.status.value, + "path": a.path, + "resolved_path": str(a.resolved_path) if a.resolved_path else None, + "exists_on_disk": a.exists_on_disk, + "is_repo_singleton": a.is_repo_singleton, + "size_bytes": a.size_bytes, + } + for a in artifacts + ], + }, ) return diff --git a/src/operations_center/cli_output.py b/src/operations_center/cli_output.py new file mode 100644 index 000000000..f7b74bc1b --- /dev/null +++ b/src/operations_center/cli_output.py @@ -0,0 +1,60 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 ProtocolWarden +"""Shared structured-output rendering for CLI commands. + +Every CLI entrypoint that supports a machine-readable ``--json``/ +``--format json`` mode owns its own ``rich.console.Console`` and, before +this module existed, serialized that mode's payload independently — +some via ``typer.echo(json.dumps(...))`` (bypassing ``Console`` +entirely), others via ``console.print(json.dumps(...))`` (losing syntax +highlighting and risking soft-wrap on long values). ``print_structured`` +is the one call every such command should make instead: it normalizes +the payload and always renders it through ``Console.print_json``, which +does not soft-wrap and emits no ANSI codes on non-tty output. + +See ``.console/STAGE1_PRINT_STRUCTURED_DESIGN.md`` for the design +rationale and the per-file migration table. +""" + +from __future__ import annotations + +import dataclasses +from collections.abc import Mapping +from typing import Any + +from pydantic import BaseModel +from rich.console import Console + + +def print_structured(console: Console, output: Any, *, sort_keys: bool = False) -> None: + """Render `output` as syntax-highlighted, unwrapped JSON via `console`. + + `output` may be a `dict`, a Pydantic `BaseModel`, a `dataclass` + instance, another `Mapping`, or any other JSON-serializable value + (list, primitive). It is normalized to a plain JSON-native value and + passed to `Console.print_json`, which never soft-wraps regardless of + `console.width` and never emits ANSI escapes when `console.file` is + not a tty. + + Pre-serialized JSON strings are not accepted specially — a `str` + argument is rendered as a JSON string scalar, not parsed and + re-rendered as an object. Callers must pass data, not + `model.model_dump_json()`. + + Args: + console: the caller's own `Console` instance. + output: the structured payload to render. + sort_keys: forwarded to the underlying JSON encoder; set `True` + for output consumed by automation/diffing that depends on a + deterministic key order. + """ + if isinstance(output, BaseModel): + payload: Any = output.model_dump(mode="json") + elif dataclasses.is_dataclass(output) and not isinstance(output, type): + payload = dataclasses.asdict(output) + elif isinstance(output, Mapping) and not isinstance(output, dict): + payload = dict(output) + else: + payload = output + + console.print_json(data=payload, indent=2, ensure_ascii=False, default=str, sort_keys=sort_keys) diff --git a/src/operations_center/entrypoints/audit/main.py b/src/operations_center/entrypoints/audit/main.py index 28c901c0a..42961371b 100644 --- a/src/operations_center/entrypoints/audit/main.py +++ b/src/operations_center/entrypoints/audit/main.py @@ -48,6 +48,7 @@ load_run_status_entrypoint, resolve_artifact_manifest_path, ) +from operations_center.cli_output import print_structured app = typer.Typer( help="Managed repo audit dispatch commands.", @@ -126,7 +127,7 @@ def cmd_run( raise typer.Exit(code=3) from exc if json_output: - typer.echo(result.model_dump_json(indent=2)) + print_structured(console, result) else: _print_dispatch_result(result) @@ -150,7 +151,7 @@ def cmd_status( raise typer.Exit(code=2) from exc if json_output: - typer.echo(run_status.model_dump_json(indent=2)) + print_structured(console, run_status) else: t = Table(title="Run Status") t.add_column("Field") @@ -220,24 +221,15 @@ def cmd_list_active( json_output: bool = typer.Option(False, "--json", help="Output as JSON."), ) -> None: """List currently-held audit dispatch locks across all OpsCenter processes.""" - import json as _json from datetime import UTC, datetime store = get_global_registry().store active = store.list_active() if json_output: - typer.echo( - _json.dumps( - [ - { - **p.to_json(), - **p.liveness_summary(), - } - for p in active - ], - indent=2, - ) + print_structured( + console, + [{**p.to_json(), **p.liveness_summary()} for p in active], ) return diff --git a/src/operations_center/entrypoints/calibration/main.py b/src/operations_center/entrypoints/calibration/main.py index c02b5b948..5aafba757 100644 --- a/src/operations_center/entrypoints/calibration/main.py +++ b/src/operations_center/entrypoints/calibration/main.py @@ -40,6 +40,7 @@ load_calibration_report, write_calibration_report, ) +from operations_center.cli_output import print_structured app = typer.Typer( help="Managed repo audit behavior calibration commands.", @@ -114,7 +115,7 @@ def cmd_analyze( report = analyze_artifacts(calibration_input) if json_output: - typer.echo(report.model_dump_json(indent=2)) + print_structured(console, report) else: _print_report_summary(report) @@ -152,7 +153,7 @@ def cmd_tune_autonomy( report = analyze_artifacts(calibration_input) if json_output: - typer.echo(report.model_dump_json(indent=2)) + print_structured(console, report) else: _print_report_summary(report) if report.recommendations: @@ -188,7 +189,7 @@ def cmd_report( raise typer.Exit(code=2) from exc if json_output: - typer.echo(report.model_dump_json(indent=2)) + print_structured(console, report) else: _print_report_summary(report) diff --git a/src/operations_center/entrypoints/governance/main.py b/src/operations_center/entrypoints/governance/main.py index 50c95dcdb..a9706f0de 100644 --- a/src/operations_center/entrypoints/governance/main.py +++ b/src/operations_center/entrypoints/governance/main.py @@ -40,6 +40,7 @@ run_governed_audit, ) from operations_center.audit_governance.models import AuditUrgency +from operations_center.cli_output import print_structured app = typer.Typer( help="Full audit governance — approve, deny, and dispatch managed repo audits.", @@ -89,12 +90,11 @@ def cmd_request( console.print(f"[red]Invalid request:[/red] {exc}") raise typer.Exit(code=2) from exc - payload = req.model_dump_json(indent=2) if output: - Path(output).write_text(payload, encoding="utf-8") + Path(output).write_text(req.model_dump_json(indent=2), encoding="utf-8") console.print(f"Request written to [bold]{output}[/bold] (request_id={req.request_id})") else: - console.print(payload) + print_structured(console, req) @app.command("evaluate") @@ -179,14 +179,13 @@ def cmd_approve( console.print(f"[red]Approval validation failed:[/red] {exc}") raise typer.Exit(code=3) - payload = approval.model_dump_json(indent=2) if output: - Path(output).write_text(payload, encoding="utf-8") + Path(output).write_text(approval.model_dump_json(indent=2), encoding="utf-8") console.print( f"Approval written to [bold]{output}[/bold] (approval_id={approval.approval_id})" ) else: - console.print(payload) + print_structured(console, approval) @app.command("run") diff --git a/src/operations_center/entrypoints/run_show/main.py b/src/operations_center/entrypoints/run_show/main.py index 7a3154e8e..b16a54801 100644 --- a/src/operations_center/entrypoints/run_show/main.py +++ b/src/operations_center/entrypoints/run_show/main.py @@ -28,6 +28,8 @@ from rich.console import Console from rich.table import Table +from operations_center.cli_output import print_structured + app = typer.Typer( help="Print one run's provenance chain from execution_trace.json alone.", no_args_is_help=True, @@ -218,7 +220,7 @@ def show( payload = json.loads(trace_path.read_text(encoding="utf-8")) if as_json: - typer.echo(json.dumps(payload, indent=2, sort_keys=True, ensure_ascii=False)) + print_structured(_console, payload, sort_keys=True) return _console.print(f"[dim]source: {trace_path}[/dim]\n") _print_trace(payload) diff --git a/src/operations_center/entrypoints/worker_backend_probe/main.py b/src/operations_center/entrypoints/worker_backend_probe/main.py index 36759462a..0a587b500 100644 --- a/src/operations_center/entrypoints/worker_backend_probe/main.py +++ b/src/operations_center/entrypoints/worker_backend_probe/main.py @@ -11,7 +11,6 @@ from __future__ import annotations from datetime import UTC, datetime -import json from pathlib import Path import typer @@ -21,6 +20,7 @@ DEFAULT_PROBE_TIMEOUT_SECONDS, refresh_cooldowns, ) +from operations_center.cli_output import print_structured from operations_center.execution.usage_store import UsageStore app = typer.Typer( @@ -58,13 +58,10 @@ def _command( logger=None if as_json else (lambda msg: _console.print(f" {msg}")), ) if as_json: - typer.echo( - json.dumps( - {"observed_at": now.isoformat(), "report": report, "usage_path": str(store.path)}, - indent=2, - sort_keys=True, - ensure_ascii=False, - ) + print_structured( + _console, + {"observed_at": now.isoformat(), "report": report, "usage_path": str(store.path)}, + sort_keys=True, ) return if not report: diff --git a/src/operations_center/entrypoints/worker_backend_status/main.py b/src/operations_center/entrypoints/worker_backend_status/main.py index 2fe26e422..2ee765e60 100644 --- a/src/operations_center/entrypoints/worker_backend_status/main.py +++ b/src/operations_center/entrypoints/worker_backend_status/main.py @@ -4,7 +4,6 @@ from __future__ import annotations -import json from datetime import UTC, datetime from pathlib import Path @@ -12,6 +11,7 @@ from rich.console import Console from rich.table import Table +from operations_center.cli_output import print_structured from operations_center.execution.usage_store import UsageStore app = typer.Typer( @@ -44,7 +44,7 @@ def _command( "usage_path": str(store.path), } if as_json: - typer.echo(json.dumps(payload, indent=2, sort_keys=True, ensure_ascii=False)) + print_structured(_console, payload, sort_keys=True) return _console.print("[bold]Worker backend cooldowns[/bold]") diff --git a/src/operations_center/observer/cli.py b/src/operations_center/observer/cli.py index 2572d4f42..c2fea338d 100644 --- a/src/operations_center/observer/cli.py +++ b/src/operations_center/observer/cli.py @@ -21,6 +21,7 @@ from rich.console import Console from rich.table import Table +from operations_center.cli_output import print_structured from operations_center.observer.alert_channels import AlertChannelFactory from operations_center.observer.collectors.extraction_history_collector import ( ExtractionHistoryCollector, @@ -516,7 +517,7 @@ def cmd_list( elif format_str == "json": snapshots = [{"run_id": d.name} for d in snapshot_dirs] if not quiet: - console.print(json.dumps(snapshots, indent=2, ensure_ascii=False)) + print_structured(console, snapshots) except Exception as e: if not quiet: @@ -586,7 +587,10 @@ def cmd_show( raise typer.Exit(EXIT_CONFIG_ERROR) if pretty: - console.print_json(output) + if format_str == "json": + print_structured(console, obj) + else: + console.print_json(output) else: if not quiet: console.print(output) @@ -1072,9 +1076,7 @@ def cmd_extraction_health( logger.debug("extraction history augmentation skipped: %s", e) if format_str == "json": - # typer.echo (not the rich console) so piped/redirected JSON is not - # soft-wrapped — the watchdog collector parses this from a file. - typer.echo(json.dumps(payload, indent=2, ensure_ascii=False)) + print_structured(console, payload) else: # table console.print( f"extraction success_rate={payload['success_rate']:.1f}% " @@ -1182,7 +1184,7 @@ def cmd_extraction_health_dashboard( ) if format_str == "json": - typer.echo(json.dumps(data.to_dict(), indent=2, ensure_ascii=False)) + print_structured(console, data.to_dict()) else: renderer = ExtractionHealthDashboardRenderer() renderer.render(data, console) diff --git a/src/operations_center/run_memory/cli.py b/src/operations_center/run_memory/cli.py index c74e6d97d..8f4bbb3fe 100644 --- a/src/operations_center/run_memory/cli.py +++ b/src/operations_center/run_memory/cli.py @@ -4,13 +4,14 @@ from __future__ import annotations -import json from pathlib import Path import typer from rich.console import Console from rich.table import Table +from operations_center.cli_output import print_structured + from .index import ( RunMemoryQueryService, rebuild_index_from_artifacts, @@ -48,11 +49,7 @@ def query_cmd( ) records = svc.query(q) if json_out: - typer.echo( - json.dumps( - [r.to_jsonl() for r in records], sort_keys=True, indent=2, ensure_ascii=False - ) - ) + print_structured(_console, [r.to_jsonl() for r in records], sort_keys=True) return table = Table(title=f"Run Memory ({len(records)} matches)") table.add_column("created_at") diff --git a/tests/unit/entrypoints/audit/test_main_cov.py b/tests/unit/entrypoints/audit/test_main_cov.py index 8329bb778..7724e9a8f 100644 --- a/tests/unit/entrypoints/audit/test_main_cov.py +++ b/tests/unit/entrypoints/audit/test_main_cov.py @@ -178,12 +178,12 @@ def test_cmd_run_failure_exit_code_1(monkeypatch): def test_cmd_run_json_output(monkeypatch): result = _make_result(succeeded=True) monkeypatch.setattr(main, "dispatch_managed_audit", mock.Mock(return_value=result)) - echo = mock.Mock() - monkeypatch.setattr(main.typer, "echo", echo) + ps = mock.Mock() + monkeypatch.setattr(main, "print_structured", ps) with pytest.raises(typer.Exit) as ei: _run(monkeypatch, json_output=True) assert _exit_code(ei) == 0 - echo.assert_called_once_with('{"ok": true}') + ps.assert_called_once_with(main.console, result) def test_cmd_run_log_dir_passed(monkeypatch): @@ -237,10 +237,10 @@ def test_cmd_status_table(monkeypatch): def test_cmd_status_json(monkeypatch): rs = _make_run_status() monkeypatch.setattr(main, "load_run_status_entrypoint", mock.Mock(return_value=rs)) - echo = mock.Mock() - monkeypatch.setattr(main.typer, "echo", echo) + ps = mock.Mock() + monkeypatch.setattr(main, "print_structured", ps) main.cmd_status(run_status_path="/tmp/rs.json", json_output=True) - echo.assert_called_once_with('{"run": "status"}') + ps.assert_called_once_with(main.console, rs) def test_cmd_status_not_found_exit_1(monkeypatch): @@ -389,13 +389,13 @@ def _patch_store(monkeypatch, active): def test_list_active_json(monkeypatch): payload = _make_payload(oc_pid_alive=True, audit_pid_alive=False) _patch_store(monkeypatch, [payload]) - echo = mock.Mock() - monkeypatch.setattr(main.typer, "echo", echo) + ps = mock.Mock() + monkeypatch.setattr(main, "print_structured", ps) main.cmd_list_active(json_output=True) - echo.assert_called_once() - out = echo.call_args.args[0] - assert "run-1" in out - assert "oc_pid_alive" in out + ps.assert_called_once() + out = ps.call_args.args[1] + assert out == [{**payload.to_json(), **payload.liveness_summary()}] + assert "oc_pid_alive" in out[0] def test_list_active_empty_text(monkeypatch): diff --git a/tests/unit/entrypoints/calibration/test_main_cov.py b/tests/unit/entrypoints/calibration/test_main_cov.py index d35adbd0f..47bcb4c56 100644 --- a/tests/unit/entrypoints/calibration/test_main_cov.py +++ b/tests/unit/entrypoints/calibration/test_main_cov.py @@ -167,11 +167,12 @@ def test_analyze_has_errors_exit1(patched): assert result.exit_code == 1 -def test_analyze_json_output(patched): +def test_analyze_json_output(patched, monkeypatch): + ps = MagicMock() + monkeypatch.setattr(mod, "print_structured", ps) result = runner.invoke(mod.app, ["analyze", "-m", "/m.json", "--json"]) assert result.exit_code == 0 - assert '{"json": true}' in result.output - patched.report.model_dump_json.assert_called_once_with(indent=2) + ps.assert_called_once_with(mod.console, patched.report) def test_analyze_include_content_flag(patched): @@ -212,10 +213,12 @@ def test_tune_autonomy_uses_recommendation_profile(patched): assert ci.analysis_profile is AnalysisProfile.RECOMMENDATION -def test_tune_autonomy_json(patched): +def test_tune_autonomy_json(patched, monkeypatch): + ps = MagicMock() + monkeypatch.setattr(mod, "print_structured", ps) result = runner.invoke(mod.app, ["tune-autonomy", "-m", "/m.json", "--json"]) assert result.exit_code == 0 - assert '{"json": true}' in result.output + ps.assert_called_once_with(mod.console, patched.report) def test_tune_autonomy_with_recommendations_table(patched): @@ -253,10 +256,12 @@ def test_report_happy(patched): assert "Calibration Report" in result.output -def test_report_json(patched): +def test_report_json(patched, monkeypatch): + ps = MagicMock() + monkeypatch.setattr(mod, "print_structured", ps) result = runner.invoke(mod.app, ["report", "/some/report.json", "--json"]) assert result.exit_code == 0 - assert '{"json": true}' in result.output + ps.assert_called_once_with(mod.console, patched.report) def test_report_not_found(patched): diff --git a/tests/unit/entrypoints/governance/test_main_cov.py b/tests/unit/entrypoints/governance/test_main_cov.py index f579d077f..b07a45375 100644 --- a/tests/unit/entrypoints/governance/test_main_cov.py +++ b/tests/unit/entrypoints/governance/test_main_cov.py @@ -296,6 +296,8 @@ def test_approve_success_stdout(tmp_path, monkeypatch): approval.model_dump_json.return_value = '{"approval_id": "AID"}' approval.approval_id = "AID" monkeypatch.setattr(mod, "make_manual_approval", MagicMock(return_value=approval)) + ps = MagicMock() + monkeypatch.setattr(mod, "print_structured", ps) result = runner.invoke( mod.app, [ @@ -311,7 +313,7 @@ def test_approve_success_stdout(tmp_path, monkeypatch): ], ) assert result.exit_code == 0 - assert "AID" in result.stdout + ps.assert_called_once_with(mod.console, approval) def test_approve_success_to_file(tmp_path, monkeypatch): diff --git a/tests/unit/test_cli_output.py b/tests/unit/test_cli_output.py new file mode 100644 index 000000000..47189ef25 --- /dev/null +++ b/tests/unit/test_cli_output.py @@ -0,0 +1,168 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 ProtocolWarden +"""Tests for operations_center.cli_output.""" + +from __future__ import annotations + +import json +import types +from collections import OrderedDict +from dataclasses import dataclass +from datetime import UTC, datetime +from io import StringIO +from pathlib import Path + +from pydantic import BaseModel +from rich.console import Console + +from operations_center.cli_output import print_structured + + +def _render(output: object, *, sort_keys: bool = False, width: int | None = None) -> str: + buf = StringIO() + kwargs = {"file": buf, "no_color": True} + if width is not None: + kwargs["width"] = width + console = Console(**kwargs) + print_structured(console, output, sort_keys=sort_keys) + return buf.getvalue() + + +class _Model(BaseModel): + name: str + created_at: datetime + path: Path + + +@dataclass +class _Point: + x: int + y: int + + +@dataclass +class _Line: + start: _Point + end: _Point + + +class TestDictInput: + def test_plain_dict_renders_as_json_object(self) -> None: + result = _render({"a": 1, "b": "two"}) + assert json.loads(result) == {"a": 1, "b": "two"} + + def test_nested_dict_preserved(self) -> None: + payload = {"outer": {"inner": [1, 2, 3]}} + assert json.loads(_render(payload)) == payload + + +class TestBaseModelInput: + def test_model_normalized_via_model_dump_json_mode(self) -> None: + model = _Model(name="x", created_at=datetime(2026, 1, 1, tzinfo=UTC), path=Path("/tmp/x")) + result = json.loads(_render(model)) + assert result["name"] == "x" + assert result["path"] == "/tmp/x" + # datetime is converted to an ISO string by mode="json", not left as + # a python object that would need `default=str` to rescue it. + assert result["created_at"] == "2026-01-01T00:00:00Z" + + +class TestDataclassInput: + def test_dataclass_normalized_via_asdict(self) -> None: + result = json.loads(_render(_Point(x=1, y=2))) + assert result == {"x": 1, "y": 2} + + def test_nested_dataclass_recursively_normalized(self) -> None: + line = _Line(start=_Point(x=0, y=0), end=_Point(x=1, y=1)) + result = json.loads(_render(line)) + assert result == {"start": {"x": 0, "y": 0}, "end": {"x": 1, "y": 1}} + + +class TestMappingInput: + def test_non_dict_mapping_normalized_to_dict(self) -> None: + mapping = types.MappingProxyType({"k": "v"}) + result = json.loads(_render(mapping)) + assert result == {"k": "v"} + + def test_dict_subclass_passthrough_not_routed_through_mapping_branch(self) -> None: + # OrderedDict IS a dict, so it must hit the `else` passthrough branch, + # not the non-dict-Mapping branch (which would still work, but this + # pins the intended dispatch and confirms `dict(...)` isn't called + # redundantly on an already-dict value). + result = json.loads(_render(OrderedDict([("b", 1), ("a", 2)]))) + assert result == {"b": 1, "a": 2} + + +class TestOtherJsonNativeInputs: + def test_list_passthrough(self) -> None: + assert json.loads(_render([1, 2, 3])) == [1, 2, 3] + + def test_none_renders_as_null(self) -> None: + assert _render(None).strip() == "null" + + def test_empty_dict_renders_as_empty_object(self) -> None: + assert _render({}).strip() == "{}" + + def test_empty_list_renders_as_empty_array(self) -> None: + assert _render([]).strip() == "[]" + + def test_bool_renders_as_json_literal(self) -> None: + assert _render(True).strip() == "true" + + def test_int_renders_as_json_number(self) -> None: + assert _render(7).strip() == "7" + + def test_float_renders_as_json_number(self) -> None: + assert _render(1.5).strip() == "1.5" + + def test_str_input_is_not_parsed_as_json_but_rendered_as_a_string_scalar(self) -> None: + # Documented contract: callers must pass data, not a pre-serialized + # `model.model_dump_json()` string — a `str` argument is rendered as + # a JSON string scalar, quotes and all, not parsed and re-emitted as + # the object/array it might encode. + result = _render('{"already": "json"}') + assert result.strip() == json.dumps('{"already": "json"}') + assert json.loads(result) == '{"already": "json"}' + + +class TestUnicodeAndFormatting: + def test_non_ascii_characters_are_not_escaped(self) -> None: + # ensure_ascii=False: multibyte characters should appear literally, + # not as \uXXXX escapes. + result = _render({"name": "café ❤"}) + assert "café ❤" in result + assert "\\u" not in result + + def test_nested_payload_is_pretty_printed_with_indent(self) -> None: + result = _render({"outer": {"inner": 1}}) + lines = [line for line in result.splitlines() if line.strip()] + assert len(lines) > 1 + assert any(line.startswith(" ") for line in lines) + + +class TestSortKeys: + def test_sort_keys_false_preserves_insertion_order(self) -> None: + result = _render({"z": 1, "a": 2}, sort_keys=False) + assert result.index('"z"') < result.index('"a"') + + def test_sort_keys_true_sorts_alphabetically(self) -> None: + result = _render({"z": 1, "a": 2}, sort_keys=True) + assert result.index('"a"') < result.index('"z"') + + +class TestDefaultStrFallback: + def test_unserializable_nested_value_stringified(self) -> None: + result = json.loads(_render({"path": Path("/tmp/foo")})) + assert result == {"path": "/tmp/foo"} + + +class TestSoftWrapRegression: + def test_long_string_value_stays_on_one_line_regardless_of_width(self) -> None: + long_value = "x" * 220 + result = _render({"value": long_value}, width=80) + lines = [line for line in result.splitlines() if line.strip()] + assert any(long_value in line for line in lines) + + def test_no_ansi_escape_codes_on_non_tty_output(self) -> None: + result = _render({"a": 1}) + assert "\x1b[" not in result