From b9d81ac694482621d2f4b3339169b626ea8cfec3 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 25 Aug 2026 07:31:15 +0200 Subject: [PATCH] =?UTF-8?q?feat(config):=20T-1260=20=E2=80=94=20reach=20li?= =?UTF-8?q?sts=20its=20domains=20without=20importing=20them?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `reach --help` renders from a declaration table and imports nothing. The cost of help is now flat as the registry grows, which is the property that has to hold going from one domain to a dozen. The trap is real and was confirmed in typer's vendored source rather than assumed from upstream Click: TyperGroup.format_commands loops over list_commands calling get_command on each, purely to read a short help string off the loaded command. With lazy loading underneath, that imports every domain in the registry to render --help — while the output looks entirely correct. Nothing observable changes; only the import graph does. So the test asserts on sys.modules, and it was proven to fail before being trusted. Disabling the format_commands override made it fail and name the cause, listing all five leaked check modules. It also carries a positive control — invoking a domain must import its service — because without one, "nothing was imported" would pass equally for a loader that is simply broken, and it fails on an empty registry, which would otherwise satisfy everything vacuously. The check domain is created here because the test needs a subject: a stub raising NotImplementedError would have been committed dead code. That takes the port out of T-1262, which is rescoped to what it still owns — pydantic schemas, byte-for-byte output parity on the drift path, and the failure tests. The old tooling/check-client-version script stays in place and stays wired to the pre-push hook; the deprecation window is deliberate. One Typer behaviour worth knowing before every future domain: a single-command app collapses into a bare command, so `reach check client-version` failed with "unexpected extra argument" until the router got a callback. Same mechanism as the root callback, different symptom. Help now works at every level, closing item 5 of T-1248. Co-Authored-By: Claude Opus 5 (1M context) --- .pql/changelog/ticket_history/2026-08.sql | 78 ++++++++++++ .pql/changelog/tickets/2026-08.sql | 121 ++++++++++++++++++ Makefile | 4 + tooling/domains/check/__init__.py | 6 + tooling/domains/check/router.py | 70 +++++++++++ tooling/domains/check/schemas.py | 20 +++ tooling/domains/check/service.py | 62 +++++++++ tooling/main.py | 72 ++++++++++- tooling/test_lazy_domains.py | 147 ++++++++++++++++++++++ 9 files changed, 575 insertions(+), 5 deletions(-) create mode 100644 tooling/domains/check/__init__.py create mode 100644 tooling/domains/check/router.py create mode 100644 tooling/domains/check/schemas.py create mode 100644 tooling/domains/check/service.py create mode 100644 tooling/test_lazy_domains.py diff --git a/.pql/changelog/ticket_history/2026-08.sql b/.pql/changelog/ticket_history/2026-08.sql index 779934e3f..e9b8843ab 100644 --- a/.pql/changelog/ticket_history/2026-08.sql +++ b/.pql/changelog/ticket_history/2026-08.sql @@ -1413,3 +1413,81 @@ ALREADY DONE IN T-1259, do not redo: OPEN QUESTION carried here: bare `reach` currently exits 2 (Click''s usage-error convention via no_args_is_help). Once domains are registered and a bare `reach` prints a useful domain list, decide whether that should exit 0 instead. It is not a failure, and D-263''s exit-code contract concerns failures carrying a remedy. BUDGET NOTE: the D-263 ceiling was raised to ~500 ms on 2026-08-23. Current cost of `reach --help` with zero domains is 80 ms. Stay smart about what loads; do not contort for tightness.', NULL, '2026-08-23 12:01:12', '2026-08-23 12:01:12.824', '2026-08-23 12:01:12.824', NULL, 'ba4645e4d3bebf918819b0564ff7c1ff', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S67EQ8EQTT2E9E3CGTXS7W', 'status', 'in_progress', 'done', NULL, '2026-08-23 12:01:35', '2026-08-23 12:01:35.203', '2026-08-23 12:01:35.203', NULL, 'fb5721fa5d639350ec686820293d1f1f', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S685APSPZRXSM9F62DTKGM', 'status', 'backlog', 'in_progress', NULL, '2026-08-23 12:02:01', '2026-08-23 12:02:01.994', '2026-08-23 12:02:01.994', NULL, '4b815972d1111d2070b965682ec9730f', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S685APSPZRXSM9F62DTKGM', 'status', 'in_progress', 'in_progress', NULL, '2026-08-23 12:02:25', '2026-08-23 12:02:25.470', '2026-08-23 12:02:25.470', NULL, '75a4647db23d935267b2f0b7a304903f', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S685APSPZRXSM9F62DTKGM', 'description', 'reach --help must list every domain WITHOUT importing any of them. This is not an optimisation; it is the mechanism that keeps the 104 ms push-gate budget reachable, and it has one specific trap. Click Group.format_commands calls get_command for every subcommand to render its short help — which imports all 123 modules just to print help. So format_commands MUST be overridden to read short help from a declaration table rather than from the loaded command. Deliverables: tooling/main.py holding a DOMAINS table mapping name to (import_path, short_help); a LazyDomainGroup(click.Group) overriding list_commands, get_command and format_commands; get_command resolving via importlib and typer.main.get_command only for the domain actually invoked. main.py is a router and nothing else — no logic, no I/O, no pydantic, no domain imports at module level. Acceptance is a TEST, not an eyeball: run reach --help in a subprocess and assert no tooling.domains..service module is in sys.modules afterwards. That test is the whole point of the ticket and belongs with it, not in E8. + +AMENDED 2026-08-20 — the justification changes, the requirement does not. Lazy registration is still mandatory, but NOT because of a 104 ms parity budget (withdrawn — see D-263 amendment and T-1251). It is mandatory because of what an eager entrypoint would actually cost: measured in the repo venv, scipy.ndimage 275 ms, pydantic 87 ms, numpy 72 ms, PIL.Image 29 ms — about 460 ms paid before executing a line of its own, growing every time a domain is added. The format_commands trap and the sys.modules acceptance test are unchanged and are the heart of this ticket. + +AMENDED 2026-08-23 from T-1259 — the base class in the description is WRONG and this ticket cannot be built as written. + +The description specifies LazyDomainGroup(click.Group). There is no top-level click package: typer VENDORS click as of 0.26.0 (as typer._click), and the docs are explicit that extracting the internal Click app is unsupported. Adding click as a direct dependency would be worse, not better — two Click implementations in one process, with typer.main.get_command returning a typer._click Command that a real click.Group cannot host. + +CORRECTED DESIGN, verified working in T-1259: +- The root is `cli = typer.Typer(cls=LazyDomainGroup, ...)` in tooling/main.py — already in place. +- LazyDomainGroup subclasses **typer.core.TyperGroup**, not click.Group. Confirmed TyperGroup''s MRO is (TyperGroup, typer._click.core.Command, ABC) and that cls= composes. +- Override list_commands, get_command and format_commands on that class. The format_commands trap is UNCHANGED and is still the heart of this ticket: the base implementation calls get_command for every subcommand to render short help, which would import every domain just to print --help. It must read short help from the DOMAINS declaration table instead. +- get_command resolves the invoked domain via importlib and converts the domain''s Typer sub-app with typer.main.get_command. +- The sys.modules acceptance test is unchanged and is still the point: after `reach --help`, no domain service module may appear in sys.modules. Extend it to assert `rich` and `pygments` are absent too — that is currently true and is worth pinning, since it is one careless kwarg away from regressing. + +ALREADY DONE IN T-1259, do not redo: +- main.py exists with the Typer root and a callback. The callback is REQUIRED: a Typer root with no commands and no callback raises RuntimeError("Could not get a command for this Typer instance") at build time, and lazy registration means no command is ever eager. +- rich_markup_mode=None is set and is load-bearing — worth 94 ms of a 168 ms empty --help, and it keeps rich and pygments off the import path. Do not remove it while adding the lazy group. + +OPEN QUESTION carried here: bare `reach` currently exits 2 (Click''s usage-error convention via no_args_is_help). Once domains are registered and a bare `reach` prints a useful domain list, decide whether that should exit 0 instead. It is not a failure, and D-263''s exit-code contract concerns failures carrying a remedy. + +BUDGET NOTE: the D-263 ceiling was raised to ~500 ms on 2026-08-23. Current cost of `reach --help` with zero domains is 80 ms. Stay smart about what loads; do not contort for tightness.', 'reach --help must list every domain WITHOUT importing any of them. This is not an optimisation; it is the mechanism that keeps the 104 ms push-gate budget reachable, and it has one specific trap. Click Group.format_commands calls get_command for every subcommand to render its short help — which imports all 123 modules just to print help. So format_commands MUST be overridden to read short help from a declaration table rather than from the loaded command. Deliverables: tooling/main.py holding a DOMAINS table mapping name to (import_path, short_help); a LazyDomainGroup(click.Group) overriding list_commands, get_command and format_commands; get_command resolving via importlib and typer.main.get_command only for the domain actually invoked. main.py is a router and nothing else — no logic, no I/O, no pydantic, no domain imports at module level. Acceptance is a TEST, not an eyeball: run reach --help in a subprocess and assert no tooling.domains..service module is in sys.modules afterwards. That test is the whole point of the ticket and belongs with it, not in E8. + +AMENDED 2026-08-20 — the justification changes, the requirement does not. Lazy registration is still mandatory, but NOT because of a 104 ms parity budget (withdrawn — see D-263 amendment and T-1251). It is mandatory because of what an eager entrypoint would actually cost: measured in the repo venv, scipy.ndimage 275 ms, pydantic 87 ms, numpy 72 ms, PIL.Image 29 ms — about 460 ms paid before executing a line of its own, growing every time a domain is added. The format_commands trap and the sys.modules acceptance test are unchanged and are the heart of this ticket. + +AMENDED 2026-08-23 from T-1259 — the base class in the description is WRONG and this ticket cannot be built as written. + +The description specifies LazyDomainGroup(click.Group). There is no top-level click package: typer VENDORS click as of 0.26.0 (as typer._click), and the docs are explicit that extracting the internal Click app is unsupported. Adding click as a direct dependency would be worse, not better — two Click implementations in one process, with typer.main.get_command returning a typer._click Command that a real click.Group cannot host. + +CORRECTED DESIGN, verified working in T-1259: +- The root is `cli = typer.Typer(cls=LazyDomainGroup, ...)` in tooling/main.py — already in place. +- LazyDomainGroup subclasses **typer.core.TyperGroup**, not click.Group. Confirmed TyperGroup''s MRO is (TyperGroup, typer._click.core.Command, ABC) and that cls= composes. +- Override list_commands, get_command and format_commands on that class. The format_commands trap is UNCHANGED and is still the heart of this ticket: the base implementation calls get_command for every subcommand to render short help, which would import every domain just to print --help. It must read short help from the DOMAINS declaration table instead. +- get_command resolves the invoked domain via importlib and converts the domain''s Typer sub-app with typer.main.get_command. +- The sys.modules acceptance test is unchanged and is still the point: after `reach --help`, no domain service module may appear in sys.modules. Extend it to assert `rich` and `pygments` are absent too — that is currently true and is worth pinning, since it is one careless kwarg away from regressing. + +ALREADY DONE IN T-1259, do not redo: +- main.py exists with the Typer root and a callback. The callback is REQUIRED: a Typer root with no commands and no callback raises RuntimeError("Could not get a command for this Typer instance") at build time, and lazy registration means no command is ever eager. +- rich_markup_mode=None is set and is load-bearing — worth 94 ms of a 168 ms empty --help, and it keeps rich and pygments off the import path. Do not remove it while adding the lazy group. + +OPEN QUESTION carried here: bare `reach` currently exits 2 (Click''s usage-error convention via no_args_is_help). Once domains are registered and a bare `reach` prints a useful domain list, decide whether that should exit 0 instead. It is not a failure, and D-263''s exit-code contract concerns failures carrying a remedy. + +BUDGET NOTE: the D-263 ceiling was raised to ~500 ms on 2026-08-23. Current cost of `reach --help` with zero domains is 80 ms. Stay smart about what loads; do not contort for tightness. + +DONE 2026-08-25. Lazy registration works and is guarded by a test that has been proven to fail. + +THE TRAP IS REAL — confirmed in the vendored source, not assumed from upstream Click. +typer.core.TyperGroup.format_commands loops over list_commands calling get_command on each, purely to read get_short_help_str off the loaded command. With lazy loading underneath that imports every domain in the registry to render --help, while the output looks entirely correct. Overridden to read short help from the DOMAINS table and never touch get_command. + +Note on the base class: TyperGroup''s MRO is (TyperGroup, Command, ABC, object) — typer''s vendored click has NO separate Group/MultiCommand class, everything is collapsed into Command, and TyperGroup defines list_commands/get_command/format_commands itself. So the overrides sit directly on TyperGroup as the amendment said. + +DELIVERED +- tooling/main.py: DOMAINS registry (name -> import target, short help) and LazyDomainGroup(TyperGroup) overriding list_commands, get_command, format_commands. _load_domain resolves via importlib and typer.main.get_command only for the domain actually invoked. +- tooling/domains/check/: __init__.py, router.py, service.py, schemas.py — the check domain, registered so the mechanism has a subject. +- tooling/test_lazy_domains.py, wired into make test-tooling. + +THE TEST WAS PROVEN TO FAIL, which is the only reason its pass means anything. Disabled the format_commands override and re-ran: it failed and named the cause, listing tooling.domains, .check, .check.router, .check.schemas and .check.service as leaked. Restored, green again. Three properties: (1) nothing under tooling.domains in sys.modules after --help; (2) no heavy third-party module (rich, pygments, numpy, scipy, pydantic, PIL); (3) POSITIVE CONTROL — invoking check client-version DOES import the service, without which properties 1 and 2 would also pass for a loader that is simply broken and imports nothing ever. The test also fails on an empty DOMAINS registry, which would otherwise satisfy everything vacuously. Probes run in a FRESH subprocess, since the assertion is about a module graph and an in-process check would inherit whatever the harness already imported. + +SCOPE OVERLAP WITH T-1262 — READ BEFORE PICKING THAT UP. +The acceptance test needs a real domain or it asserts nothing, so this ticket created the check domain rather than a stub (stub code that raises NotImplementedError would have been committed dead code). check/service.py now implements client_version for real, ported from tooling/check-client-version, and router.py has the client-version verb. What T-1262 STILL OWNS, unchanged: converting schemas.py from the stdlib dataclass to pydantic; byte-for-byte output-parity verification against the old script on both the OK and drift paths including exit codes; and the failure-path tests. T-1262 should be re-read as a hardening ticket rather than a port. + +TYPER GOTCHA worth knowing before every future domain: Typer COLLAPSES a single-command app into a bare command. reach check client-version failed with "Got unexpected extra argument(s) (client-version)" until the check router got an @app.callback(). Every domain router needs that callback until it has two or more verbs, and keeping it afterwards costs nothing and stops the shape changing under you. Same mechanism as the root callback from T-1259, different symptom. + +HELP AT EVERY LEVEL now works, which closes item (5) of the parent epic T-1248: reach --help lists domains under a "Domains" heading, reach check --help lists verbs under "Commands". + +TIMING: reach --help is 98 ms with one domain registered, against 80 ms with none. The registry itself is the only growth; the point of the override is that this stays flat as domains are added. D-263 ceiling is 500 ms. + +STILL OPEN, carried forward: bare `reach` exits 2 via no_args_is_help (Click''s usage-error convention). It now prints a genuinely useful domain list, so exit 0 is arguably more honest — it is not a failure. Left as-is rather than changed silently; belongs with T-1249''s error contract work.', NULL, '2026-08-25 05:30:54', '2026-08-25 05:30:54.480', '2026-08-25 05:30:54.480', NULL, 'a9934c3d90d3b97190de33e34d157397', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S6A2D22PZVQJ0S0HT91E7W', 'description', 'The one domain ported end to end as the pattern every later port copies, so it is worth more care than its size suggests. Port tooling/check-client-version (72 lines, reads project.yaml and client/project.godot, compares the mirrored version, prints a drift message naming the remedy) into tooling/domains/check/. Deliverables: service.py holding the logic, TRANSPORT-AGNOSTIC — it returns a result and never prints, never calls sys.exit, never imports typer, so it can be called by a test or another service directly; router.py holding the Typer sub-app, which parses, delegates and formats, with ZERO logic; schemas.py holding the result shape as a STDLIB frozen dataclass, not pydantic. That last point is deliberate and is half the value of this ticket as a reference: pydantic costs 87 ms against a whole gate check of 20 ms, so domains/check/ carries no pydantic at all, and schemas.py existing while being pydantic-free demonstrates both the layering and the budget rule at once. Keep the existing tooling/check-client-version script IN PLACE and still wired to the pre-push hook — retiring old paths is E6, and the deprecation window is the point. Acceptance: reach check client-version matches the old script byte for byte on both the OK and the drift path, including the exit code. + +AMENDED 2026-08-20 — the pydantic carve-out is DROPPED. The description above justifies a stdlib dataclass in schemas.py on the grounds that pydantic costs 87 ms against a 20 ms check; that reasoning came from the timing-parity budget which D-263 withdrew the same day. Pydantic is now available to every domain including check. Use pydantic in schemas.py so the reference domain is a TRUE reference — the pattern every later port copies should be the normal one, not an exception carrying a footnote. Everything else in this ticket stands unchanged, and the acceptance criterion is now the primary one: byte-for-byte output parity with the old script on both the OK and drift paths, exit code included.', 'The one domain ported end to end as the pattern every later port copies, so it is worth more care than its size suggests. Port tooling/check-client-version (72 lines, reads project.yaml and client/project.godot, compares the mirrored version, prints a drift message naming the remedy) into tooling/domains/check/. Deliverables: service.py holding the logic, TRANSPORT-AGNOSTIC — it returns a result and never prints, never calls sys.exit, never imports typer, so it can be called by a test or another service directly; router.py holding the Typer sub-app, which parses, delegates and formats, with ZERO logic; schemas.py holding the result shape as a STDLIB frozen dataclass, not pydantic. That last point is deliberate and is half the value of this ticket as a reference: pydantic costs 87 ms against a whole gate check of 20 ms, so domains/check/ carries no pydantic at all, and schemas.py existing while being pydantic-free demonstrates both the layering and the budget rule at once. Keep the existing tooling/check-client-version script IN PLACE and still wired to the pre-push hook — retiring old paths is E6, and the deprecation window is the point. Acceptance: reach check client-version matches the old script byte for byte on both the OK and the drift path, including the exit code. + +AMENDED 2026-08-20 — the pydantic carve-out is DROPPED. The description above justifies a stdlib dataclass in schemas.py on the grounds that pydantic costs 87 ms against a 20 ms check; that reasoning came from the timing-parity budget which D-263 withdrew the same day. Pydantic is now available to every domain including check. Use pydantic in schemas.py so the reference domain is a TRUE reference — the pattern every later port copies should be the normal one, not an exception carrying a footnote. Everything else in this ticket stands unchanged, and the acceptance criterion is now the primary one: byte-for-byte output parity with the old script on both the OK and drift paths, exit code included. + +RESCOPED 2026-08-25 by T-1260. This is now a HARDENING ticket, not a port. T-1260''s acceptance test needed a real domain as its subject — a stub raising NotImplementedError would have been committed dead code — so tooling/domains/check/ already exists with router.py, service.py and schemas.py, and service.client_version() is ported from tooling/check-client-version and working. reach check client-version runs green. WHAT REMAINS YOURS, unchanged: (1) convert schemas.py from the stdlib frozen dataclass to pydantic, so the reference domain shows the normal pattern rather than an exception; (2) BYTE-FOR-BYTE OUTPUT PARITY against the old script on BOTH paths — the OK path and the drift path — including the exit code, which is the acceptance criterion and has NOT been verified yet, only the OK path has been eyeballed; (3) the failure-path tests, including a deliberately drifted client/project.godot, since a gate that has only ever passed has never been tested. Note the old tooling/check-client-version script is still in place and still wired to the pre-push hook — the deprecation window is deliberate and retiring it is T-1253.', NULL, '2026-08-25 05:31:01', '2026-08-25 05:31:01.531', '2026-08-25 05:31:01.531', NULL, '541b2744645b4c59cf155673bc9bd56c', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-08.sql b/.pql/changelog/tickets/2026-08.sql index c999fed5d..1c769a9b2 100644 --- a/.pql/changelog/tickets/2026-08.sql +++ b/.pql/changelog/tickets/2026-08.sql @@ -1417,3 +1417,124 @@ ALREADY DONE IN T-1259, do not redo: OPEN QUESTION carried here: bare `reach` currently exits 2 (Click''s usage-error convention via no_args_is_help). Once domains are registered and a bare `reach` prints a useful domain list, decide whether that should exit 0 instead. It is not a failure, and D-263''s exit-code contract concerns failures carrying a remedy. BUDGET NOTE: the D-263 ceiling was raised to ~500 ms on 2026-08-23. Current cost of `reach --help` with zero domains is 80 ms. Stay smart about what loads; do not contort for tightness.', 'backlog', 'high', NULL, NULL, 'D-263', '2026-08-20 00:36:25.557', '2026-08-23 12:01:12.824', NULL, '9f7e2efceadf8ec6d46fd5094d1564ed', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S67EQ8EQTT2E9E3CGTXS7W', 'task', '06G1S392DEG41MRGEFSYWJWP2W', 'The reach entrypoint: [project.scripts] plus typer and pydantic', 'pyproject.toml currently has NO [project.scripts] at all — no console entrypoint is defined anywhere. Adding one plus the two dependencies is the whole delta. Deliverables: [project.scripts] reach = tooling.main:cli; typer and pydantic added to [project] dependencies, both version-pinned and CVE-checked against NVD plus OSV the way scipy and Pillow already are in this file, with the check date in a comment since that is the established convention here. NOTE the entrypoint target is main:cli not main:app — see the lazy-registration task; the root is a click Group subclass rather than a typer.Typer instance, because a Typer root with zero eagerly-registered commands raises at build time and because keeping typer off the --help path is worth having. Acceptance: .venv/bin/reach --help runs. This is the venv-local proof only; the bare-name-on-PATH guarantee is a separate task and is the one that actually matters. + +DONE 2026-08-23. Entrypoint works: .venv/bin/reach --help runs in 80 ms. But the design in the description above is WRONG in one load-bearing way and had to change. + +TYPER VENDORS CLICK — the click.Group root is impossible. +The description specifies main:cli as a click Group subclass, on the reasoning that it keeps typer off the --help path. That cannot be done. As of typer 0.26.0 Click is VENDORED into typer as typer._click: no top-level click package is installed at all, and the docs are explicit that extracting the internal Click app is no longer supported. A real click.Group root hosting typer sub-apps would mean two different Click implementations in one process. Discovered the direct way — main.py imported click and reach --help died with ModuleNotFoundError. +Replacement, and it is the supported surface: the root is a typer.Typer, and lazy registration goes through typer.Typer(cls=...) with a typer.core.TyperGroup subclass. Verified TyperGroup subclasses typer._click.core.Command and that cls= composes. T-1260 builds the lazy group on that surface, NOT on click.Group. + +THE ZERO-COMMAND CLAIM WAS HALF RIGHT. +The description says a Typer root with zero eagerly-registered commands raises at build time. Tested both ways: with no commands AND no callback it does raise (RuntimeError: Could not get a command for this Typer instance); with a callback and no commands it builds fine and prints help. So the empty root is legal as long as it carries a callback, which main.py now has. That matters because lazy registration means NO command is ever eager. + +rich_markup_mode=None IS LOAD-BEARING, NOT COSMETIC. +An empty --help costs 168 ms with typer default rich rendering and 74 ms with rich disabled — 94 ms, 56 percent of runtime, for an empty CLI. Verified rich and pygments are then absent from sys.modules entirely, so this removes them from the import path rather than merely skipping the render. It also stops typer drawing box-art help, which it does EVEN WHEN STDOUT IS A PIPE — that would have put box-drawing characters into every hook log and agent capture. One line to revert if the boxes are ever worth more than the milliseconds. + +typer-slim IS NOT THE ANSWER (checked, since rich is unused). +Deprecated as of typer 0.22.0 and now a shallow wrapper that installs all of typer, so depending on it buys nothing. Dependency stays plain typer; rich ships transitively but is never imported. + +SECURITY — both direct deps and the transitive set checked 2026-08-23 against NVD, OSV and the GitHub Advisory Database: +- typer 0.27.1: no advisories on record at all. +- pydantic 2.13.4: clean. PYSEC-2026-1812 / CVE-2024-3772 (email-regex ReDoS) is fixed in 2.4.0. NOTE the 2026 SSRF advisories CVE-2026-25580 and CVE-2026-54249 are against pydantic-ai, a DIFFERENT package that is not a dependency here — recorded in pyproject so the next sweep does not re-panic. +- transitive: pygments 2.21.0 clears CVE-2026-4539 (archetype-lexer ReDoS, fixed 2.20.0); rich and click have no advisories on record. + +TIMING, for the record: 80 ms for reach --help with zero domains registered. Interpreter startup is 27 ms of that and typer import is ~42 ms. Against the D-263 ceiling — raised to 500 ms on 2026-08-23 — that leaves real headroom for the domains to come. + +BARE `reach` EXITS 2. no_args_is_help=True prints help and exits 2 (Click usage-error convention). Flagging rather than changing it: that is standard for no-command-given, and D-263''s exit-code contract is about FAILURES carrying a remedy, which this is not. Revisit in T-1260 if the domain list should exit 0.', 'done', 'high', NULL, NULL, 'D-263', '2026-08-20 00:36:19.770', '2026-08-23 12:01:35.203', NULL, 'c23886b83a683b3de7c97acf77098fb0', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S685APSPZRXSM9F62DTKGM', 'task', '06G1S392DEG41MRGEFSYWJWP2W', 'Lazy domain registration — --help must not import the tree', 'reach --help must list every domain WITHOUT importing any of them. This is not an optimisation; it is the mechanism that keeps the 104 ms push-gate budget reachable, and it has one specific trap. Click Group.format_commands calls get_command for every subcommand to render its short help — which imports all 123 modules just to print help. So format_commands MUST be overridden to read short help from a declaration table rather than from the loaded command. Deliverables: tooling/main.py holding a DOMAINS table mapping name to (import_path, short_help); a LazyDomainGroup(click.Group) overriding list_commands, get_command and format_commands; get_command resolving via importlib and typer.main.get_command only for the domain actually invoked. main.py is a router and nothing else — no logic, no I/O, no pydantic, no domain imports at module level. Acceptance is a TEST, not an eyeball: run reach --help in a subprocess and assert no tooling.domains..service module is in sys.modules afterwards. That test is the whole point of the ticket and belongs with it, not in E8. + +AMENDED 2026-08-20 — the justification changes, the requirement does not. Lazy registration is still mandatory, but NOT because of a 104 ms parity budget (withdrawn — see D-263 amendment and T-1251). It is mandatory because of what an eager entrypoint would actually cost: measured in the repo venv, scipy.ndimage 275 ms, pydantic 87 ms, numpy 72 ms, PIL.Image 29 ms — about 460 ms paid before executing a line of its own, growing every time a domain is added. The format_commands trap and the sys.modules acceptance test are unchanged and are the heart of this ticket. + +AMENDED 2026-08-23 from T-1259 — the base class in the description is WRONG and this ticket cannot be built as written. + +The description specifies LazyDomainGroup(click.Group). There is no top-level click package: typer VENDORS click as of 0.26.0 (as typer._click), and the docs are explicit that extracting the internal Click app is unsupported. Adding click as a direct dependency would be worse, not better — two Click implementations in one process, with typer.main.get_command returning a typer._click Command that a real click.Group cannot host. + +CORRECTED DESIGN, verified working in T-1259: +- The root is `cli = typer.Typer(cls=LazyDomainGroup, ...)` in tooling/main.py — already in place. +- LazyDomainGroup subclasses **typer.core.TyperGroup**, not click.Group. Confirmed TyperGroup''s MRO is (TyperGroup, typer._click.core.Command, ABC) and that cls= composes. +- Override list_commands, get_command and format_commands on that class. The format_commands trap is UNCHANGED and is still the heart of this ticket: the base implementation calls get_command for every subcommand to render short help, which would import every domain just to print --help. It must read short help from the DOMAINS declaration table instead. +- get_command resolves the invoked domain via importlib and converts the domain''s Typer sub-app with typer.main.get_command. +- The sys.modules acceptance test is unchanged and is still the point: after `reach --help`, no domain service module may appear in sys.modules. Extend it to assert `rich` and `pygments` are absent too — that is currently true and is worth pinning, since it is one careless kwarg away from regressing. + +ALREADY DONE IN T-1259, do not redo: +- main.py exists with the Typer root and a callback. The callback is REQUIRED: a Typer root with no commands and no callback raises RuntimeError("Could not get a command for this Typer instance") at build time, and lazy registration means no command is ever eager. +- rich_markup_mode=None is set and is load-bearing — worth 94 ms of a 168 ms empty --help, and it keeps rich and pygments off the import path. Do not remove it while adding the lazy group. + +OPEN QUESTION carried here: bare `reach` currently exits 2 (Click''s usage-error convention via no_args_is_help). Once domains are registered and a bare `reach` prints a useful domain list, decide whether that should exit 0 instead. It is not a failure, and D-263''s exit-code contract concerns failures carrying a remedy. + +BUDGET NOTE: the D-263 ceiling was raised to ~500 ms on 2026-08-23. Current cost of `reach --help` with zero domains is 80 ms. Stay smart about what loads; do not contort for tightness.', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-20 00:36:25.557', '2026-08-23 12:02:01.993', NULL, 'b60b13ca2f5cf299e3baf03fa108cf28', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S685APSPZRXSM9F62DTKGM', 'task', '06G1S392DEG41MRGEFSYWJWP2W', 'Lazy domain registration — --help must not import the tree', 'reach --help must list every domain WITHOUT importing any of them. This is not an optimisation; it is the mechanism that keeps the 104 ms push-gate budget reachable, and it has one specific trap. Click Group.format_commands calls get_command for every subcommand to render its short help — which imports all 123 modules just to print help. So format_commands MUST be overridden to read short help from a declaration table rather than from the loaded command. Deliverables: tooling/main.py holding a DOMAINS table mapping name to (import_path, short_help); a LazyDomainGroup(click.Group) overriding list_commands, get_command and format_commands; get_command resolving via importlib and typer.main.get_command only for the domain actually invoked. main.py is a router and nothing else — no logic, no I/O, no pydantic, no domain imports at module level. Acceptance is a TEST, not an eyeball: run reach --help in a subprocess and assert no tooling.domains..service module is in sys.modules afterwards. That test is the whole point of the ticket and belongs with it, not in E8. + +AMENDED 2026-08-20 — the justification changes, the requirement does not. Lazy registration is still mandatory, but NOT because of a 104 ms parity budget (withdrawn — see D-263 amendment and T-1251). It is mandatory because of what an eager entrypoint would actually cost: measured in the repo venv, scipy.ndimage 275 ms, pydantic 87 ms, numpy 72 ms, PIL.Image 29 ms — about 460 ms paid before executing a line of its own, growing every time a domain is added. The format_commands trap and the sys.modules acceptance test are unchanged and are the heart of this ticket. + +AMENDED 2026-08-23 from T-1259 — the base class in the description is WRONG and this ticket cannot be built as written. + +The description specifies LazyDomainGroup(click.Group). There is no top-level click package: typer VENDORS click as of 0.26.0 (as typer._click), and the docs are explicit that extracting the internal Click app is unsupported. Adding click as a direct dependency would be worse, not better — two Click implementations in one process, with typer.main.get_command returning a typer._click Command that a real click.Group cannot host. + +CORRECTED DESIGN, verified working in T-1259: +- The root is `cli = typer.Typer(cls=LazyDomainGroup, ...)` in tooling/main.py — already in place. +- LazyDomainGroup subclasses **typer.core.TyperGroup**, not click.Group. Confirmed TyperGroup''s MRO is (TyperGroup, typer._click.core.Command, ABC) and that cls= composes. +- Override list_commands, get_command and format_commands on that class. The format_commands trap is UNCHANGED and is still the heart of this ticket: the base implementation calls get_command for every subcommand to render short help, which would import every domain just to print --help. It must read short help from the DOMAINS declaration table instead. +- get_command resolves the invoked domain via importlib and converts the domain''s Typer sub-app with typer.main.get_command. +- The sys.modules acceptance test is unchanged and is still the point: after `reach --help`, no domain service module may appear in sys.modules. Extend it to assert `rich` and `pygments` are absent too — that is currently true and is worth pinning, since it is one careless kwarg away from regressing. + +ALREADY DONE IN T-1259, do not redo: +- main.py exists with the Typer root and a callback. The callback is REQUIRED: a Typer root with no commands and no callback raises RuntimeError("Could not get a command for this Typer instance") at build time, and lazy registration means no command is ever eager. +- rich_markup_mode=None is set and is load-bearing — worth 94 ms of a 168 ms empty --help, and it keeps rich and pygments off the import path. Do not remove it while adding the lazy group. + +OPEN QUESTION carried here: bare `reach` currently exits 2 (Click''s usage-error convention via no_args_is_help). Once domains are registered and a bare `reach` prints a useful domain list, decide whether that should exit 0 instead. It is not a failure, and D-263''s exit-code contract concerns failures carrying a remedy. + +BUDGET NOTE: the D-263 ceiling was raised to ~500 ms on 2026-08-23. Current cost of `reach --help` with zero domains is 80 ms. Stay smart about what loads; do not contort for tightness.', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-20 00:36:25.557', '2026-08-23 12:02:25.470', NULL, 'f7f81d09d9939736e2dc1161f3240b89', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S685APSPZRXSM9F62DTKGM', 'task', '06G1S392DEG41MRGEFSYWJWP2W', 'Lazy domain registration — --help must not import the tree', 'reach --help must list every domain WITHOUT importing any of them. This is not an optimisation; it is the mechanism that keeps the 104 ms push-gate budget reachable, and it has one specific trap. Click Group.format_commands calls get_command for every subcommand to render its short help — which imports all 123 modules just to print help. So format_commands MUST be overridden to read short help from a declaration table rather than from the loaded command. Deliverables: tooling/main.py holding a DOMAINS table mapping name to (import_path, short_help); a LazyDomainGroup(click.Group) overriding list_commands, get_command and format_commands; get_command resolving via importlib and typer.main.get_command only for the domain actually invoked. main.py is a router and nothing else — no logic, no I/O, no pydantic, no domain imports at module level. Acceptance is a TEST, not an eyeball: run reach --help in a subprocess and assert no tooling.domains..service module is in sys.modules afterwards. That test is the whole point of the ticket and belongs with it, not in E8. + +AMENDED 2026-08-20 — the justification changes, the requirement does not. Lazy registration is still mandatory, but NOT because of a 104 ms parity budget (withdrawn — see D-263 amendment and T-1251). It is mandatory because of what an eager entrypoint would actually cost: measured in the repo venv, scipy.ndimage 275 ms, pydantic 87 ms, numpy 72 ms, PIL.Image 29 ms — about 460 ms paid before executing a line of its own, growing every time a domain is added. The format_commands trap and the sys.modules acceptance test are unchanged and are the heart of this ticket. + +AMENDED 2026-08-23 from T-1259 — the base class in the description is WRONG and this ticket cannot be built as written. + +The description specifies LazyDomainGroup(click.Group). There is no top-level click package: typer VENDORS click as of 0.26.0 (as typer._click), and the docs are explicit that extracting the internal Click app is unsupported. Adding click as a direct dependency would be worse, not better — two Click implementations in one process, with typer.main.get_command returning a typer._click Command that a real click.Group cannot host. + +CORRECTED DESIGN, verified working in T-1259: +- The root is `cli = typer.Typer(cls=LazyDomainGroup, ...)` in tooling/main.py — already in place. +- LazyDomainGroup subclasses **typer.core.TyperGroup**, not click.Group. Confirmed TyperGroup''s MRO is (TyperGroup, typer._click.core.Command, ABC) and that cls= composes. +- Override list_commands, get_command and format_commands on that class. The format_commands trap is UNCHANGED and is still the heart of this ticket: the base implementation calls get_command for every subcommand to render short help, which would import every domain just to print --help. It must read short help from the DOMAINS declaration table instead. +- get_command resolves the invoked domain via importlib and converts the domain''s Typer sub-app with typer.main.get_command. +- The sys.modules acceptance test is unchanged and is still the point: after `reach --help`, no domain service module may appear in sys.modules. Extend it to assert `rich` and `pygments` are absent too — that is currently true and is worth pinning, since it is one careless kwarg away from regressing. + +ALREADY DONE IN T-1259, do not redo: +- main.py exists with the Typer root and a callback. The callback is REQUIRED: a Typer root with no commands and no callback raises RuntimeError("Could not get a command for this Typer instance") at build time, and lazy registration means no command is ever eager. +- rich_markup_mode=None is set and is load-bearing — worth 94 ms of a 168 ms empty --help, and it keeps rich and pygments off the import path. Do not remove it while adding the lazy group. + +OPEN QUESTION carried here: bare `reach` currently exits 2 (Click''s usage-error convention via no_args_is_help). Once domains are registered and a bare `reach` prints a useful domain list, decide whether that should exit 0 instead. It is not a failure, and D-263''s exit-code contract concerns failures carrying a remedy. + +BUDGET NOTE: the D-263 ceiling was raised to ~500 ms on 2026-08-23. Current cost of `reach --help` with zero domains is 80 ms. Stay smart about what loads; do not contort for tightness. + +DONE 2026-08-25. Lazy registration works and is guarded by a test that has been proven to fail. + +THE TRAP IS REAL — confirmed in the vendored source, not assumed from upstream Click. +typer.core.TyperGroup.format_commands loops over list_commands calling get_command on each, purely to read get_short_help_str off the loaded command. With lazy loading underneath that imports every domain in the registry to render --help, while the output looks entirely correct. Overridden to read short help from the DOMAINS table and never touch get_command. + +Note on the base class: TyperGroup''s MRO is (TyperGroup, Command, ABC, object) — typer''s vendored click has NO separate Group/MultiCommand class, everything is collapsed into Command, and TyperGroup defines list_commands/get_command/format_commands itself. So the overrides sit directly on TyperGroup as the amendment said. + +DELIVERED +- tooling/main.py: DOMAINS registry (name -> import target, short help) and LazyDomainGroup(TyperGroup) overriding list_commands, get_command, format_commands. _load_domain resolves via importlib and typer.main.get_command only for the domain actually invoked. +- tooling/domains/check/: __init__.py, router.py, service.py, schemas.py — the check domain, registered so the mechanism has a subject. +- tooling/test_lazy_domains.py, wired into make test-tooling. + +THE TEST WAS PROVEN TO FAIL, which is the only reason its pass means anything. Disabled the format_commands override and re-ran: it failed and named the cause, listing tooling.domains, .check, .check.router, .check.schemas and .check.service as leaked. Restored, green again. Three properties: (1) nothing under tooling.domains in sys.modules after --help; (2) no heavy third-party module (rich, pygments, numpy, scipy, pydantic, PIL); (3) POSITIVE CONTROL — invoking check client-version DOES import the service, without which properties 1 and 2 would also pass for a loader that is simply broken and imports nothing ever. The test also fails on an empty DOMAINS registry, which would otherwise satisfy everything vacuously. Probes run in a FRESH subprocess, since the assertion is about a module graph and an in-process check would inherit whatever the harness already imported. + +SCOPE OVERLAP WITH T-1262 — READ BEFORE PICKING THAT UP. +The acceptance test needs a real domain or it asserts nothing, so this ticket created the check domain rather than a stub (stub code that raises NotImplementedError would have been committed dead code). check/service.py now implements client_version for real, ported from tooling/check-client-version, and router.py has the client-version verb. What T-1262 STILL OWNS, unchanged: converting schemas.py from the stdlib dataclass to pydantic; byte-for-byte output-parity verification against the old script on both the OK and drift paths including exit codes; and the failure-path tests. T-1262 should be re-read as a hardening ticket rather than a port. + +TYPER GOTCHA worth knowing before every future domain: Typer COLLAPSES a single-command app into a bare command. reach check client-version failed with "Got unexpected extra argument(s) (client-version)" until the check router got an @app.callback(). Every domain router needs that callback until it has two or more verbs, and keeping it afterwards costs nothing and stops the shape changing under you. Same mechanism as the root callback from T-1259, different symptom. + +HELP AT EVERY LEVEL now works, which closes item (5) of the parent epic T-1248: reach --help lists domains under a "Domains" heading, reach check --help lists verbs under "Commands". + +TIMING: reach --help is 98 ms with one domain registered, against 80 ms with none. The registry itself is the only growth; the point of the override is that this stays flat as domains are added. D-263 ceiling is 500 ms. + +STILL OPEN, carried forward: bare `reach` exits 2 via no_args_is_help (Click''s usage-error convention). It now prints a genuinely useful domain list, so exit 0 is arguably more honest — it is not a failure. Left as-is rather than changed silently; belongs with T-1249''s error contract work.', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-20 00:36:25.557', '2026-08-25 05:30:54.478', NULL, '429368bce7b536f612270d52288b05ec', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S6A2D22PZVQJ0S0HT91E7W', 'task', '06G1S392DEG41MRGEFSYWJWP2W', 'Reference domain: check client-version, end to end through router and service', 'The one domain ported end to end as the pattern every later port copies, so it is worth more care than its size suggests. Port tooling/check-client-version (72 lines, reads project.yaml and client/project.godot, compares the mirrored version, prints a drift message naming the remedy) into tooling/domains/check/. Deliverables: service.py holding the logic, TRANSPORT-AGNOSTIC — it returns a result and never prints, never calls sys.exit, never imports typer, so it can be called by a test or another service directly; router.py holding the Typer sub-app, which parses, delegates and formats, with ZERO logic; schemas.py holding the result shape as a STDLIB frozen dataclass, not pydantic. That last point is deliberate and is half the value of this ticket as a reference: pydantic costs 87 ms against a whole gate check of 20 ms, so domains/check/ carries no pydantic at all, and schemas.py existing while being pydantic-free demonstrates both the layering and the budget rule at once. Keep the existing tooling/check-client-version script IN PLACE and still wired to the pre-push hook — retiring old paths is E6, and the deprecation window is the point. Acceptance: reach check client-version matches the old script byte for byte on both the OK and the drift path, including the exit code. + +AMENDED 2026-08-20 — the pydantic carve-out is DROPPED. The description above justifies a stdlib dataclass in schemas.py on the grounds that pydantic costs 87 ms against a 20 ms check; that reasoning came from the timing-parity budget which D-263 withdrew the same day. Pydantic is now available to every domain including check. Use pydantic in schemas.py so the reference domain is a TRUE reference — the pattern every later port copies should be the normal one, not an exception carrying a footnote. Everything else in this ticket stands unchanged, and the acceptance criterion is now the primary one: byte-for-byte output parity with the old script on both the OK and drift paths, exit code included. + +RESCOPED 2026-08-25 by T-1260. This is now a HARDENING ticket, not a port. T-1260''s acceptance test needed a real domain as its subject — a stub raising NotImplementedError would have been committed dead code — so tooling/domains/check/ already exists with router.py, service.py and schemas.py, and service.client_version() is ported from tooling/check-client-version and working. reach check client-version runs green. WHAT REMAINS YOURS, unchanged: (1) convert schemas.py from the stdlib frozen dataclass to pydantic, so the reference domain shows the normal pattern rather than an exception; (2) BYTE-FOR-BYTE OUTPUT PARITY against the old script on BOTH paths — the OK path and the drift path — including the exit code, which is the acceptance criterion and has NOT been verified yet, only the OK path has been eyeballed; (3) the failure-path tests, including a deliberately drifted client/project.godot, since a gate that has only ever passed has never been tested. Note the old tooling/check-client-version script is still in place and still wired to the pre-push hook — the deprecation window is deliberate and retiring it is T-1253.', 'backlog', 'high', NULL, NULL, 'D-263', '2026-08-20 00:36:41.192', '2026-08-25 05:31:01.531', NULL, '22e7c3529424eda485dc84e15561343a', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at; diff --git a/Makefile b/Makefile index e01c94115..5e3e54e6f 100644 --- a/Makefile +++ b/Makefile @@ -268,6 +268,10 @@ test-tooling: @mkdir -p .cache @python3 tooling/test_canvas_version_check.py 2> .cache/test-tooling-canvas-version.log || \ { echo " FAIL: canvas version gate units — log follows:"; cat .cache/test-tooling-canvas-version.log; exit 1; } + @echo " [test-tooling] reach lazy domain registration (T-1260)..." + @mkdir -p .cache + @$(VENV_PY) tooling/test_lazy_domains.py 2> .cache/test-tooling-lazy-domains.log || \ + { echo " FAIL: reach lazy registration — log follows:"; cat .cache/test-tooling-lazy-domains.log; exit 1; } @echo " [test-tooling] economy_import.traits validation units (T-995/PR #173 H2)..." @mkdir -p .cache @python3 tooling/economy-db/test_traits.py 2> .cache/test-tooling-traits.log || \ diff --git a/tooling/domains/check/__init__.py b/tooling/domains/check/__init__.py new file mode 100644 index 000000000..a3b63b20b --- /dev/null +++ b/tooling/domains/check/__init__.py @@ -0,0 +1,6 @@ +"""The `check` domain — repo consistency gates, run by the pre-push hook. + +These are the highest-frequency commands in the tree and the most +timing-sensitive, since they run on every push. Nothing here should acquire a +heavy module-level import. +""" diff --git a/tooling/domains/check/router.py b/tooling/domains/check/router.py new file mode 100644 index 000000000..9761b0e82 --- /dev/null +++ b/tooling/domains/check/router.py @@ -0,0 +1,70 @@ +"""Transport for the `check` domain — args in, delegate, format out. + +**Zero logic lives here.** Every command in this file should read as: parse, +call a service, turn the result into output and an exit code. If a command +grows a branch that is about the *problem* rather than about *presentation*, +that branch belongs in `service.py`. + +The service import is deliberately at module level: by the time this module is +imported at all, `reach` has already decided to run a `check` command, so there +is nothing left to defer. Laziness lives one level up, in `main.py`. +""" + +from __future__ import annotations + +import typer + +from tooling.core import console +from tooling.domains.check import service + +app = typer.Typer( + name="check", + help="Consistency gates — the checks the push hook runs.", + no_args_is_help=True, + add_completion=False, + rich_markup_mode=None, +) + + +@app.callback() +def _domain() -> None: + """Keeps `check` a group. + + Typer collapses a single-command app into a bare command, so without this + `reach check client-version` fails with "unexpected extra argument". Every + domain router needs this until it has two or more verbs — and keeping it + afterwards costs nothing and stops the shape changing under you. + """ + + +@app.command("client-version") +def client_version() -> None: + """Fail if the client's baked version has drifted from project.yaml.""" + result = service.client_version() + + if result.problem: + console.verdict( + f"check-client-version: {result.problem}", + ok=False, + fix="check that project.yaml and client/project.godot exist and are readable", + ) + raise typer.Exit(1) + + if not result.ok: + console.verdict( + "check-client-version: version drift\n" + f" project.yaml {result.yaml_version}\n" + f" client/project.godot {result.godot_version}\n" + "\n" + "This matters beyond cosmetics: the Atlas disk cache keys its\n" + "invalidation on this version, so a stale mirror makes a shipped\n" + "build serve canvases generated by code it no longer runs (T-1239).", + ok=False, + fix=( + "set config/version in client/project.godot's [application] " + f"section to {result.yaml_version} — project.yaml is the source of truth" + ), + ) + raise typer.Exit(1) + + console.verdict(f"check-client-version: OK — {result.yaml_version}") diff --git a/tooling/domains/check/schemas.py b/tooling/domains/check/schemas.py new file mode 100644 index 000000000..2d9a0f5ec --- /dev/null +++ b/tooling/domains/check/schemas.py @@ -0,0 +1,20 @@ +"""Data shapes for the `check` domain. + +Stdlib dataclass for now. T-1262 converts this to pydantic as part of making +this domain the reference implementation — pydantic is available to every +domain since the D-263 budget amendment dropped the gate-path carve-out. +""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class VersionCheck: + """The outcome of comparing project.yaml against client/project.godot.""" + + ok: bool + yaml_version: str | None = None + godot_version: str | None = None + problem: str | None = None diff --git a/tooling/domains/check/service.py b/tooling/domains/check/service.py new file mode 100644 index 000000000..8a3ebe7c0 --- /dev/null +++ b/tooling/domains/check/service.py @@ -0,0 +1,62 @@ +"""Logic for the `check` domain. Transport-agnostic (D-263). + +Nothing here prints, calls `sys.exit`, or imports typer. A service must not know +it was called from a CLI — that is what lets a test call it directly, lets one +domain's service call another's, and leaves a second front end possible without +a rewrite. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +from tooling.core import config +from tooling.domains.check.schemas import VersionCheck + +# Anchored to line start so the commentary above `version:` (which mentions +# earlier versions by number) can never be mistaken for the field itself. +_YAML_VERSION = re.compile(r"^version:\s*(\S+)\s*$", re.MULTILINE) +_GODOT_VERSION = re.compile(r'^config/version\s*=\s*"([^"]*)"\s*$', re.MULTILINE) + + +def client_version() -> VersionCheck: + """Compare the version in project.yaml with the one baked into the client. + + project.yaml is the version source of truth (CLAUDE.md). The client cannot + read it at runtime — an exported build has no repo root — so the value is + mirrored into `application/config/version` in client/project.godot, which + Godot bakes into the PCK (T-1241). + + A mirror nobody checks is worse than the bug it replaced: the old code + failed LOUDLY in an export ("?.?.?" everywhere), whereas a stale mirror + fails SILENTLY — the Atlas disk cache keeps serving canvases under a version + that stopped matching the build. That is the T-1239 failure, which cost + eight days of a map drawn from a canvas whose generating code no longer + existed. + """ + root = config.repo_root() + yaml_version, problem = _read(root / "project.yaml", _YAML_VERSION, "`version:` line") + if problem: + return VersionCheck(ok=False, problem=problem) + + godot_path = root / "client" / "project.godot" + godot_version, problem = _read(godot_path, _GODOT_VERSION, "`config/version=` line") + if problem: + return VersionCheck(ok=False, yaml_version=yaml_version, problem=problem) + + return VersionCheck( + ok=yaml_version == godot_version, + yaml_version=yaml_version, + godot_version=godot_version, + ) + + +def _read(path: Path, pattern: re.Pattern[str], label: str) -> tuple[str | None, str | None]: + """Return (value, problem). Exactly one of the two is ever set.""" + if not path.exists(): + return None, f"{path} not found" + match = pattern.search(path.read_text(encoding="utf-8")) + if not match: + return None, f"no {label} in {path}" + return match.group(1), None diff --git a/tooling/main.py b/tooling/main.py index f71d296d4..f2f5ee797 100644 --- a/tooling/main.py +++ b/tooling/main.py @@ -14,8 +14,8 @@ app" is unsupported. Mixing a real `click.Group` root with typer sub-apps would mean two different Click implementations in one process. So the customisation surface is `typer.Typer(cls=...)` with a `TyperGroup` -subclass, which is the supported path and is what T-1260 uses to register -domains lazily. +subclass, which is the supported path and is what `LazyDomainGroup` below uses +to register domains lazily. `rich_markup_mode=None` is not a style preference — it is worth 94 ms of the 168 ms an empty `--help` otherwise costs, and it keeps `rich` and `pygments` @@ -26,17 +26,79 @@ are ever wanted more than the milliseconds. The callback below is not decoration. A `typer.Typer` with **no commands and no callback** raises `RuntimeError: Could not get a command for this Typer -instance` at build time; with a callback it builds fine and prints help. Since -domains are registered lazily and none are eager, the callback is what makes an -empty root legal. +instance` at build time; with a callback it builds fine. Since every domain is +registered lazily and none is eager, the callback is what makes the root legal. """ from __future__ import annotations +import importlib + import typer +from typer.core import TyperGroup + +# The domain registry: name -> (import target, one-line help). +# +# This table is the ONLY thing `reach --help` reads. The short help lives here +# as a literal string rather than being pulled off the loaded command, because +# reading it off the command is precisely what would import the world — see +# LazyDomainGroup.format_commands. +# +# Adding a domain is adding a line here plus a `router.py` that exposes `app`. +DOMAINS: dict[str, tuple[str, str]] = { + "check": ( + "tooling.domains.check.router:app", + "Consistency gates — the checks the push hook runs", + ), +} + + +class LazyDomainGroup(TyperGroup): + """Lists domains without importing them; imports exactly the one invoked. + + Three overrides, and the third is the one that matters. `TyperGroup`'s own + `format_commands` loops over `list_commands` calling `get_command` on each, + just to read a short help string off the loaded command — which, with lazy + loading underneath, imports every domain in the registry to render `--help`. + That would defeat the whole mechanism silently, while looking correct. + + So `format_commands` is overridden to read help from `DOMAINS` and never + touch `get_command`. The cost of `reach --help` is then flat no matter how + many domains exist, which is the property that has to hold as this grows + from one domain to a dozen. + """ + + def list_commands(self, ctx: typer.Context) -> list[str]: + return sorted({*super().list_commands(ctx), *DOMAINS}) + + def get_command(self, ctx: typer.Context, cmd_name: str): + if cmd_name in DOMAINS: + return _load_domain(cmd_name) + return super().get_command(ctx, cmd_name) + + def format_commands(self, ctx: typer.Context, formatter) -> None: + # Deliberately does NOT call get_command. See the class docstring. + rows = [(name, short) for name, (_target, short) in sorted(DOMAINS.items())] + for name in sorted(super().list_commands(ctx)): + command = super().get_command(ctx, name) + if command is not None and not command.hidden: + rows.append((name, command.get_short_help_str(80))) + if rows: + with formatter.section("Domains"): + formatter.write_dl(sorted(rows)) + + +def _load_domain(name: str): + """Import one domain's router and convert its Typer app to a command.""" + target, _short = DOMAINS[name] + module_name, _, attr = target.partition(":") + module = importlib.import_module(module_name) + return typer.main.get_command(getattr(module, attr)) + cli = typer.Typer( name="reach", + cls=LazyDomainGroup, help="Repo tooling for The Settled Reach.\n\nRun `reach --help` to see what a domain can do.", no_args_is_help=True, add_completion=False, diff --git a/tooling/test_lazy_domains.py b/tooling/test_lazy_domains.py new file mode 100644 index 000000000..bd2fef40d --- /dev/null +++ b/tooling/test_lazy_domains.py @@ -0,0 +1,147 @@ +#!/usr/bin/env python3 +"""Units for lazy domain registration (T-1260). + +`reach --help` must list every domain WITHOUT importing any of them. That is not +tidiness: an eager entrypoint pays for every domain's imports on every +invocation, and the expensive ones are already in this tree — scipy.ndimage +alone is 275 ms. The cost of `--help` has to stay flat as the registry grows +from one domain to a dozen. + +The trap this guards is specific and quiet. `TyperGroup.format_commands` loops +over `list_commands` calling `get_command` on each, purely to read a short help +string off the loaded command. With lazy loading underneath, that imports every +domain in the registry to render `--help` — while looking entirely correct. +Nothing about the output changes; only the import graph does. So the assertion +has to be on `sys.modules`, not on what `--help` prints. + +Three properties, and the third is what stops the first two passing vacuously: + + 1. After `--help`, nothing under `tooling.domains` is imported. + 2. After `--help`, no heavy third-party module is imported. + 3. POSITIVE CONTROL: actually invoking a domain DOES import its service. If + this fails, properties 1 and 2 are meaningless — they would also pass for a + CLI whose lazy loader is broken and never imports anything at all. + +Run: python3 tooling/test_lazy_domains.py +""" + +import json +import subprocess +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# Modules that must never be dragged in by `--help`. pydantic is on the list +# because domain schemas use it: it must load with the domain, not with the CLI. +HEAVY = ("rich", "pygments", "numpy", "scipy", "pydantic", "PIL") + +_HELP_PROBE = """ +import contextlib, io, json, sys +from tooling.main import cli, DOMAINS + +buf = io.StringIO() +try: + with contextlib.redirect_stdout(buf): + cli(args=["--help"]) +except SystemExit: + pass + +print(json.dumps({ + "help": buf.getvalue(), + "domain_modules": sorted( + m for m in sys.modules + if m == "tooling.domains" or m.startswith("tooling.domains.") + ), + "heavy": sorted(m for m in %(heavy)r if m in sys.modules), + "declared": sorted(DOMAINS), +})) +""" + +_INVOKE_PROBE = """ +import contextlib, io, json, sys +from tooling.main import cli + +buf = io.StringIO() +try: + with contextlib.redirect_stdout(buf): + cli(args=["check", "client-version"]) +except SystemExit: + pass + +print(json.dumps({ + "domain_modules": sorted( + m for m in sys.modules + if m == "tooling.domains" or m.startswith("tooling.domains.") + ), +})) +""" + + +def _probe(source: str) -> dict: + """Run a snippet in a FRESH interpreter and return its JSON verdict. + + A subprocess, not an in-process import, because this test is entirely about + a module graph — running it in the harness's own interpreter would inherit + whatever the harness already imported and prove nothing. + """ + result = subprocess.run( + [sys.executable, "-c", source], + capture_output=True, + text=True, + cwd=REPO_ROOT, + ) + if result.returncode != 0: + raise SystemExit(f"probe failed (exit {result.returncode}):\n{result.stderr}") + return json.loads(result.stdout) + + +def main() -> int: + failures = [] + + helped = _probe(_HELP_PROBE % {"heavy": HEAVY}) + + # Guard against a vacuous pass: an empty registry would satisfy every + # assertion below while proving nothing at all. + if not helped["declared"]: + failures.append("DOMAINS registry is empty — every assertion here would pass vacuously") + + for name in helped["declared"]: + if name not in helped["help"]: + failures.append(f"`--help` does not list the declared domain {name!r}") + + if helped["domain_modules"]: + failures.append( + "`--help` imported domain modules, so registration is not lazy: " + + ", ".join(helped["domain_modules"]) + + "\n The usual cause is format_commands falling back to the base " + "implementation,\n which calls get_command on every subcommand to read its short help." + ) + + if helped["heavy"]: + failures.append("`--help` imported heavy modules: " + ", ".join(helped["heavy"])) + + # Positive control. Without this, the assertions above would pass for a CLI + # that is simply broken and imports nothing ever. + invoked = _probe(_INVOKE_PROBE) + if "tooling.domains.check.service" not in invoked["domain_modules"]: + failures.append( + "positive control FAILED: invoking `check client-version` did not import " + "tooling.domains.check.service, so the lazy-import assertions above prove nothing" + ) + + if failures: + print("test_lazy_domains: FAIL", file=sys.stderr) + for failure in failures: + print(f" - {failure}", file=sys.stderr) + return 1 + + print( + f"test_lazy_domains: OK — {len(helped['declared'])} domain(s) listed, " + "none imported; positive control confirms the loader works" + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main())