fix: honor gl-report format flags

Author: alexph-dev

02_docs/architecture.md

@@ -66,6 +66,8 @@ flowchart TD
 ### Output Layer
 
 - `output()` dispatches to text/JSON/TSV modes.
+- `_resolve_fmt()` defines the shared precedence rule: command-specific `--output/--format`
+  overrides global `-f/--format`.
 - `_normalize_output_data()` centralizes dict/list normalization shared by text/TSV output.
 - `_output_kv()` handles readable single-entity rendering.
 
@@ -76,6 +78,8 @@ flowchart TD
 - Report builders format that tree into text, JSON-like structures, or transaction lists.
 - `cmd_gl_report()` remains the orchestration point for account discovery, customer resolution,
   date handling, and final rendering.
+- `gl-report` follows the shared format resolver for `text`/`json`, adds `txns` and `expanded`,
+  and rejects unsupported `tsv` explicitly.
 
 ## Safety Boundaries
 

03_notes/2026-03-19-gl-format-review-merge.md

@@ -0,0 +1,14 @@
+# 2026-03-19 gl-format review merge
+
+- Scope: review local refactor work, verify merge readiness, push `main`.
+- Finding: `gl-report` did not honor shared format resolution for global `-f json` and did not accept
+  subcommand `--format json`.
+- Fix: route `gl-report` through `_resolve_fmt()`, add `--format` alias, reject unsupported `tsv`
+  explicitly, add parser/handler coverage, update docs.
+- Verification:
+  - `uv run ruff check qbo_cli/cli.py tests/test_commands.py tests/test_live.py README.md`
+  - `uv run mypy qbo_cli/cli.py tests`
+  - `uv run pytest -q`
+  - `uv run pytest -m live -q`
+- Note: repo-wide Ruff still reports pre-existing unrelated issues already present on `origin/main`
+  in untouched test files.

README.md

@@ -214,10 +214,15 @@ qbo gl-report -c "John Smith" -a 125 --currency USD
 # Custom date range
 qbo gl-report -c "John Smith" -a "Revenue" --start 2025-01-01 --end 2025-12-31
 
+# Structured JSON output
+qbo gl-report -a 125 --format json
+
 # Dates default to: first transaction → today
 qbo gl-report -c "John Smith" -a 125
 ```
 
+`gl-report` supports `text`, `json`, `txns`, and `expanded`. `tsv` is not available for this command.
+
 ### Output formats
 
 ```bash

THEORY.MD

@@ -1,62 +1,64 @@
-# Theory: Safe Internal Refactor for qbo-cli
+# Theory: Safe CLI Refactor With Format Contract Verification
 
 ## Problem
 
-`qbo_cli/cli.py` is still the center of the application, but before this pass it
-mixed parser construction, command dispatch, repeated command glue, and output
-normalization in ways that made small changes noisier than necessary. The risk
-was not a broken CLI; the risk was that future edits would keep paying a
-complexity tax in the least interesting parts of the file.
+`qbo_cli/cli.py` still carries the whole CLI surface, so small refactors can
+silently change behavior in places where parser defaults, command glue, and
+output selection interact. The structural cleanup already improved the file, but
+that kind of cleanup is only safe if the format contract stays consistent across
+commands. `gl-report` was the highest-risk spot because it has custom output
+modes and heavier orchestration than the basic CRUD/report commands.
 
 ## Operating Theory
 
-The project is safest to improve at its seams. Unit tests already protect the
-public behavior of common command handlers, parser aliases, output formatting,
-client pagination/retry behavior, and many pure report helpers. The less-tested
-surfaces are OAuth callback handling, token persistence internals, and the
-orchestration-heavy parts of `cmd_gl_report()`. That makes the correct lever a
-local refactor of parser/dispatch and command plumbing, not a large module
-split.
+The right way to stabilize this codebase is to keep changes at the seams and
+verify the public contract aggressively. The refactor reduced duplication by
+centralizing parser construction, runtime creation, dispatch, stdin handling,
+report param building, and output emission. That increases maintainability, but
+it also means one mismatch in format resolution can affect an entire command
+surface. The leverage point is therefore not more extraction; it is making sure
+every command follows the same format-selection rules unless it has an explicit
+reason not to.
 
 ## Strategy
 
-Keep behavior stable while reducing repetition:
+Treat the refactor as successful only if the CLI contract is demonstrably
+preserved:
 
-- shrink `main()` into orchestration only
-- move parser building and dispatch decisions into dedicated helpers
-- centralize repeated command concerns: client creation, output emission, stdin
-  JSON loading, report param building
-- centralize output normalization shared by text and TSV rendering
-- document the logical architecture so the file can stay understandable even
-  before a future module split
+- review local commits and uncommitted fixes against `origin/main`
+- verify parser behavior directly, not only through handler unit tests
+- patch only the format-resolution gap at the source
+- add tests that lock the contract in place for both command-level and parser-level flows
+- validate with unit tests, live tests, and static checks that matter for the touched surface
 
-The aim is structural clarity with minimal movement of risky code.
+The intent is still minimal movement. One root fix in the parser/output path is
+better than special-casing downstream rendering.
 
 ## Key Discoveries
 
-The earlier readonly pass was useful because it separated environment gaps from
-real quality issues. Once the local toolchain was installed, the important fact
-was that `pytest` was already green, `ruff` problems were isolated to tests,
-and `mypy` failures in `qbo_cli/cli.py` were small enough to eliminate as part
-of the refactor. That turned the work from "safe cleanup only" into "safe
-cleanup plus static-check hardening" without widening the risk surface.
-
-The current shape now matches the intended strategy: `main()` delegates to
-`_build_parser()`, `_build_runtime()`, and `_dispatch_command()`, command
-handlers share helper glue, and output normalization is centralized. The code
-is still a single-file CLI, but with cleaner internal boundaries.
-
-The strongest confidence signal came from the live suite. Running all
-`@pytest.mark.live` tests against the configured QuickBooks account passed
-without changes to test code or to the refactored command paths. That matters
-because it validates the actual packaged CLI entrypoint, subprocess execution,
-auth status, report endpoints, and General Ledger smoke flows against the real
-service rather than only mocked unit-level contracts.
+The critical discovery from review was not inside report generation itself but
+at the CLI boundary: `gl-report` emitted JSON correctly when `args.output` was
+set to `json`, but it did not actually honor the shared format contract. Global
+`-f json` left `args.output` at `"text"`, and `gl-report --format json` was not
+accepted at all. That meant the refactor had made the common `_resolve_fmt()`
+path feel shared while `gl-report` still behaved like a special case.
+
+The fix is small and structural. `gl-report` now resolves output through the
+same format selector as other commands, accepts `--format` as an alias, and
+fails explicitly for `tsv`, which remains unsupported for that command. The
+tests now cover both direct handler behavior and parser wiring for `gl-report`,
+which was the missing safety net.
+
+Verification stayed strong after the fix. Full non-live `pytest`, full
+`@pytest.mark.live` smoke tests, and `mypy` all passed. Ruff also passes for the
+touched surface. Repo-wide Ruff failures still exist on `origin/main`, but they
+are pre-existing test-file issues unrelated to this refactor.
 
 ## Open Questions
 
-The next meaningful architectural step is still blocked on test depth, not on
-mechanical extraction work. Before splitting `cli.py` into separate modules, the
-project should add stronger tests around `TokenManager`, OAuth callback state
-handling, and `cmd_gl_report()` output modes. Without that, larger movement
-would trade readability gains for regression risk.
+The main remaining uncertainty is still test depth around the most stateful
+paths: `TokenManager`, OAuth callback handling, and the more exotic
+`cmd_gl_report()` modes such as `txns`, `expanded`, and explicit unsupported
+format cases. Future extraction work should wait until those contracts are
+covered more directly, otherwise readability gains will come with regression
+risk.

qbo_cli/cli.py

@@ -1163,7 +1163,9 @@ def cmd_gl_report(args, config, token_mgr):
             display_start = actual_first
 
     # Output
-    out_mode = args.output
+    out_mode = _resolve_fmt(args)
+    if out_mode == "tsv":
+        die("gl-report does not support tsv output. Use text, json, txns, or expanded.")
     title = f"General Ledger Report - {cust_name}" if cust_name else "General Ledger Report"
     date_range = _format_date_range(display_start, end_date)
     currency = args.currency
@@ -1207,7 +1209,7 @@ def tree_to_dict(node):
             report_data["customer"] = cust_name
             report_data["customer_id"] = cust_id
 
-        output(report_data)
+        output(report_data, out_mode)
 
     elif out_mode == "txns":
         lines = [title, date_range, ""]
@@ -1654,7 +1656,9 @@ def _build_parser() -> tuple[argparse.ArgumentParser, argparse.ArgumentParser]:
     gl_p.add_argument(
         "-o",
         "--output",
-        default="text",
+        "--format",
+        dest="output",
+        default=None,
         choices=GL_OUTPUT_FORMATS,
         help="Output format: text (default), json, txns (flat transaction list), expanded (tree + transactions)",
     )

tests/test_commands.py

@@ -10,17 +10,17 @@
 
 from qbo_cli.cli import (
     QBOClient,
+    _resolve_fmt,
     cmd_create,
+    cmd_gl_report,
     cmd_query,
-    cmd_search,
     cmd_report,
+    cmd_search,
     cmd_update,
     main,
-    _resolve_fmt,
 )
 from tests.conftest import make_args
 
-
 # ─── cmd_query ────────────────────────────────────────────────────────────────
 
 
@@ -169,6 +169,83 @@ def test_report_forwards_params(self, fake_config, fake_token_mgr, capsys):
         assert params["accounting_method"] == "Cash"
 
 
+# ─── cmd_gl_report ────────────────────────────────────────────────────────────
+
+
+class TestCmdGlReport:
+    def test_gl_report_json_output(self, fake_config, fake_token_mgr, capsys):
+        client = MagicMock()
+        client.report = MagicMock(return_value={"Header": {"Option": []}, "Rows": {}})
+        args = make_args(
+            command="gl-report",
+            customer="R-CB1",
+            account="125",
+            start="2026-02-01",
+            end="2026-02-28",
+            method="Cash",
+            currency="THB",
+            list_accounts=False,
+            output="json",
+            no_sub=False,
+            by_customer=False,
+            sort="alpha",
+        )
+
+        with (
+            patch("qbo_cli.cli.QBOClient", return_value=client),
+            patch("qbo_cli.cli._resolve_customer", return_value=("104", "PM:R-CB1")),
+            patch("qbo_cli.cli._discover_account_tree", return_value={"name": "PM Owner Funds", "id": "125", "children": []}),
+            patch("qbo_cli.cli._parse_gl_rows", return_value=[]),
+            patch("qbo_cli.cli._build_section_index", return_value={}),
+            patch("qbo_cli.cli._extract_dates_from_gl", return_value=(None, None)),
+            patch("qbo_cli.cli._compute_subtotal", return_value=(123.45, 0)),
+            patch("qbo_cli.cli._find_gl_section", return_value=None),
+        ):
+            cmd_gl_report(args, fake_config, fake_token_mgr)
+
+        data = json.loads(capsys.readouterr().out)
+        assert data["customer"] == "PM:R-CB1"
+        assert data["account"]["name"] == "PM Owner Funds"
+        assert data["total"] == 123.45
+
+    def test_gl_report_respects_global_format_flag(self, fake_config, fake_token_mgr, capsys):
+        client = MagicMock()
+        client.report = MagicMock(return_value={"Header": {"Option": []}, "Rows": {}})
+        args = make_args(
+            command="gl-report",
+            customer=None,
+            account="125",
+            start="2026-02-01",
+            end="2026-02-28",
+            method="Cash",
+            currency="THB",
+            list_accounts=False,
+            output=None,
+            format="json",
+            no_sub=False,
+            by_customer=False,
+            sort="alpha",
+        )
+
+        with (
+            patch("qbo_cli.cli.QBOClient", return_value=client),
+            patch(
+                "qbo_cli.cli._discover_account_tree",
+                return_value={"name": "PM Owner Funds", "id": "125", "children": []},
+            ),
+            patch("qbo_cli.cli._parse_gl_rows", return_value=[]),
+            patch("qbo_cli.cli._build_section_index", return_value={}),
+            patch("qbo_cli.cli._extract_dates_from_gl", return_value=(None, None)),
+            patch("qbo_cli.cli._compute_subtotal", return_value=(123.45, 0)),
+            patch("qbo_cli.cli._find_gl_section", return_value=None),
+        ):
+            cmd_gl_report(args, fake_config, fake_token_mgr)
+
+        data = json.loads(capsys.readouterr().out)
+        assert data["account"]["name"] == "PM Owner Funds"
+        assert data["total"] == 123.45
+
+
 # ─── cmd_create / cmd_update ──────────────────────────────────────────────────
 
 
@@ -252,6 +329,7 @@ class TestSubcommandFormatAlias:
             (["qbo", "delete", "Customer", "1", "--format", "json"], "cmd_delete"),
             (["qbo", "report", "ProfitAndLoss", "--format", "json"], "cmd_report"),
             (["qbo", "raw", "GET", "companyinfo/1", "--format", "json"], "cmd_raw"),
+            (["qbo", "gl-report", "-a", "125", "--format", "json"], "cmd_gl_report"),
         ],
     )
     def test_format_alias_after_subcommand_maps_to_output(self, argv, handler_name):

tests/test_live.py

@@ -125,11 +125,10 @@ def test_smoke_gl_report_json():
     )
     if result.returncode != 0:
         pytest.skip(f"GL report returned no data for account {acct_id}")
-    # -o json in gl-report renders as key-value text (not raw JSON)
-    # Verify structural text output contains expected fields
-    assert "start_date" in result.stdout
-    assert "end_date" in result.stdout
-    assert "total" in result.stdout
+    data = json.loads(result.stdout)
+    assert "start_date" in data
+    assert "end_date" in data
+    assert "total" in data
 
 
 @pytest.mark.live