diff --git a/.pql/changelog/ticket_history/2026-08.sql b/.pql/changelog/ticket_history/2026-08.sql index d53c5f0a3..f90072147 100644 --- a/.pql/changelog/ticket_history/2026-08.sql +++ b/.pql/changelog/ticket_history/2026-08.sql @@ -2153,3 +2153,54 @@ STILL OPEN on this ticket, and honest about it: - check-canvas-version is not ported. It is the one that genuinely parses git, so its fixture needs a real git repo — the hardest of the four. - NO PARITY TESTS yet for dataflow-graph or systems-db-stamp. Both were verified by running the two implementations side by side on the live tree, which proves the happy path only. The stamp gate''s six failure modes are entirely unexercised, and its fixture is awkward because the old script imports generator_sources from its own directory, so a fixture must copy both files and build a sqlite DB with a meta table. - Retirement (scripts, parity cases, make targets, the pre-commit hook entry for fact-ids) waits for the whole domain, per the per-domain rule.', NULL, '2026-08-31 15:55:15', '2026-08-31 15:55:15.374', '2026-08-31 15:55:15.374', NULL, 'eed8836144005a06232829ab3dfad2e6', 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 ('06G5GWCTS89VSST07DFVBVG064', 'description', 'Finish what T-1262 started. domains/check/ currently has one verb (client-version); the other four gates join it: check-canvas-version, check-systems-db-stamp, check-fact-ids and check-dataflow-graph.py. Per the domain map, canvas_sources.py and generator_sources.py move alongside as module-level REGISTRIES, not verbs — nothing types them, and putting them in reach --help would answer a question nobody has. schema_version.py goes to the db domain where it is defined; check imports it, which is what a service layer is for. STANDARD PORT ACCEPTANCE, applying to every per-domain ticket under this epic: (a) service.py holds the logic and is transport-agnostic — no printing, no sys.exit, no typer; (b) router.py holds no logic and every command wears @command; (c) a parity test per gate, copying the pattern in tooling/test_check_parity.py, which builds a throwaway fixture repo and runs BOTH implementations — exit codes must match exactly and no fact the old message carried may be lost; (d) the old scripts are RETIRED once parity passes, not left alongside; (e) that domain''s tooling make targets are retired too rather than left as wrappers, per the D-263 make/reach split — build orchestration stays with make. Note check-dataflow-graph.py and check-canvas-version both parse git output, so their fixtures need a git repo rather than a bare directory; that is the one thing here harder than client-version was. + +PROGRESS 2026-08-31 — fact-ids ported, the first bash-to-Python rewrite under the D-263 decision. It was 89 lines of grep/sed pipeline; it is now service.fact_ids() returning a FactIdCheck, with a router that renders it. PARITY IS EXACT ON THE LIVE TREE: both implementations print ''check-fact-ids: OK — 6 references validated against 61 canonical facts'' and exit 0. The matching COUNTS are the real evidence — if the line-matching regex differed from the grep chain even slightly, 6 and 61 would diverge. Kept line-matched rather than YAML-parsed on purpose: parsing properly would change which lines count (anchors, merge keys, multi-document files would start contributing ids the old check never saw), which is a different check wearing the same name. Three parity cases added to test_check_parity.py — ok, unknown fact_id, and the advisory mode where catalogs are unpopulated. Proven to fail: removing the entity-attributes.yaml exclusion was caught, and interestingly by the ADVISORY case rather than the exclusion assertion — including that file made the catalog non-empty, so the new implementation went to enforcing mode while the old stayed advisory. A real behavioural divergence, caught by exit code. RETIREMENT DEFERRED to the end of this ticket, and there is a reason worth recording. The rule is retire per DOMAIN, and three gates remain. It also resolves a tension: the parity test COPIES the old script into its fixture, so deleting the script early would delete the test''s own subject. A parity test is scaffolding with a defined lifetime — when the old script goes, its parity case goes with it and is replaced by a unit test of the new behaviour. Same will apply to every port under T-1250. NOTE the pre-commit hook calls tooling/check-fact-ids directly, so retirement means editing that hook — and a mistake there is felt on every commit rather than every push. Do it together with validate-content (T-1282), which is in the same hook, so it is edited once. + +PROGRESS 2026-08-31, second increment — dataflow-graph and systems-db-stamp ported. Both were already Python, so these are moves rather than rewrites, and both produce BYTE-IDENTICAL output to their originals on the live tree: "check-dataflow-graph: OK — 1 diagram(s)" and "check-systems-db-stamp: OK — 1 generator(s) up to date", same exit codes. + +CORRECTION TO THIS TICKET''S OWN NOTE: it says check-dataflow-graph.py parses git output. It does not — it reads .d2 files and globs the filesystem. Only check-canvas-version parses git, so only that one needs a git repo in its fixture. + +THE E402 DEBT EVAPORATED, exactly as T-1274 predicted. check-systems-db-stamp reached generator_sources through sys.path.insert plus a noqa: E402, because tooling/ was not a package. It now imports as `from tooling import generator_sources` with no hack and no suppression. That is the first concrete evidence for T-1274''s premise; the map''s plan to move generator_sources into the db domain still stands, but it is already importable where it sits. + +SIX FAILURE MODES PRESERVED in the stamp port, modelled as a StampState enum rather than collapsed to pass/fail, because they carry different remedies and one carries a different EXIT CODE: UNSTAMPED exits 2 while every other failure exits 1, and the pre-push hook has relied on that distinction since T-857. The states are ok / absent / unstamped / bad_version / conflict / unknown / broken / stale, each with its own remedy string. + +ONE DELIBERATE BEHAVIOURAL DIFFERENCE, flagged rather than hidden: the old stamp script was SILENT on success unless given --verbose; the new one always prints its verdict. That is not a lost fact, so it does not violate the parity definition, and it makes the gate consistent with client-version and dataflow-graph, which both always print. The old script was the odd one out. Its per-command --verbose also disappears in favour of the global one, which is the consolidation the whole initiative is for. + +STILL OPEN on this ticket, and honest about it: +- check-canvas-version is not ported. It is the one that genuinely parses git, so its fixture needs a real git repo — the hardest of the four. +- NO PARITY TESTS yet for dataflow-graph or systems-db-stamp. Both were verified by running the two implementations side by side on the live tree, which proves the happy path only. The stamp gate''s six failure modes are entirely unexercised, and its fixture is awkward because the old script imports generator_sources from its own directory, so a fixture must copy both files and build a sqlite DB with a meta table. +- Retirement (scripts, parity cases, make targets, the pre-commit hook entry for fact-ids) waits for the whole domain, per the per-domain rule.', 'Finish what T-1262 started. domains/check/ currently has one verb (client-version); the other four gates join it: check-canvas-version, check-systems-db-stamp, check-fact-ids and check-dataflow-graph.py. Per the domain map, canvas_sources.py and generator_sources.py move alongside as module-level REGISTRIES, not verbs — nothing types them, and putting them in reach --help would answer a question nobody has. schema_version.py goes to the db domain where it is defined; check imports it, which is what a service layer is for. STANDARD PORT ACCEPTANCE, applying to every per-domain ticket under this epic: (a) service.py holds the logic and is transport-agnostic — no printing, no sys.exit, no typer; (b) router.py holds no logic and every command wears @command; (c) a parity test per gate, copying the pattern in tooling/test_check_parity.py, which builds a throwaway fixture repo and runs BOTH implementations — exit codes must match exactly and no fact the old message carried may be lost; (d) the old scripts are RETIRED once parity passes, not left alongside; (e) that domain''s tooling make targets are retired too rather than left as wrappers, per the D-263 make/reach split — build orchestration stays with make. Note check-dataflow-graph.py and check-canvas-version both parse git output, so their fixtures need a git repo rather than a bare directory; that is the one thing here harder than client-version was. + +PROGRESS 2026-08-31 — fact-ids ported, the first bash-to-Python rewrite under the D-263 decision. It was 89 lines of grep/sed pipeline; it is now service.fact_ids() returning a FactIdCheck, with a router that renders it. PARITY IS EXACT ON THE LIVE TREE: both implementations print ''check-fact-ids: OK — 6 references validated against 61 canonical facts'' and exit 0. The matching COUNTS are the real evidence — if the line-matching regex differed from the grep chain even slightly, 6 and 61 would diverge. Kept line-matched rather than YAML-parsed on purpose: parsing properly would change which lines count (anchors, merge keys, multi-document files would start contributing ids the old check never saw), which is a different check wearing the same name. Three parity cases added to test_check_parity.py — ok, unknown fact_id, and the advisory mode where catalogs are unpopulated. Proven to fail: removing the entity-attributes.yaml exclusion was caught, and interestingly by the ADVISORY case rather than the exclusion assertion — including that file made the catalog non-empty, so the new implementation went to enforcing mode while the old stayed advisory. A real behavioural divergence, caught by exit code. RETIREMENT DEFERRED to the end of this ticket, and there is a reason worth recording. The rule is retire per DOMAIN, and three gates remain. It also resolves a tension: the parity test COPIES the old script into its fixture, so deleting the script early would delete the test''s own subject. A parity test is scaffolding with a defined lifetime — when the old script goes, its parity case goes with it and is replaced by a unit test of the new behaviour. Same will apply to every port under T-1250. NOTE the pre-commit hook calls tooling/check-fact-ids directly, so retirement means editing that hook — and a mistake there is felt on every commit rather than every push. Do it together with validate-content (T-1282), which is in the same hook, so it is edited once. + +PROGRESS 2026-08-31, second increment — dataflow-graph and systems-db-stamp ported. Both were already Python, so these are moves rather than rewrites, and both produce BYTE-IDENTICAL output to their originals on the live tree: "check-dataflow-graph: OK — 1 diagram(s)" and "check-systems-db-stamp: OK — 1 generator(s) up to date", same exit codes. + +CORRECTION TO THIS TICKET''S OWN NOTE: it says check-dataflow-graph.py parses git output. It does not — it reads .d2 files and globs the filesystem. Only check-canvas-version parses git, so only that one needs a git repo in its fixture. + +THE E402 DEBT EVAPORATED, exactly as T-1274 predicted. check-systems-db-stamp reached generator_sources through sys.path.insert plus a noqa: E402, because tooling/ was not a package. It now imports as `from tooling import generator_sources` with no hack and no suppression. That is the first concrete evidence for T-1274''s premise; the map''s plan to move generator_sources into the db domain still stands, but it is already importable where it sits. + +SIX FAILURE MODES PRESERVED in the stamp port, modelled as a StampState enum rather than collapsed to pass/fail, because they carry different remedies and one carries a different EXIT CODE: UNSTAMPED exits 2 while every other failure exits 1, and the pre-push hook has relied on that distinction since T-857. The states are ok / absent / unstamped / bad_version / conflict / unknown / broken / stale, each with its own remedy string. + +ONE DELIBERATE BEHAVIOURAL DIFFERENCE, flagged rather than hidden: the old stamp script was SILENT on success unless given --verbose; the new one always prints its verdict. That is not a lost fact, so it does not violate the parity definition, and it makes the gate consistent with client-version and dataflow-graph, which both always print. The old script was the odd one out. Its per-command --verbose also disappears in favour of the global one, which is the consolidation the whole initiative is for. + +STILL OPEN on this ticket, and honest about it: +- check-canvas-version is not ported. It is the one that genuinely parses git, so its fixture needs a real git repo — the hardest of the four. +- NO PARITY TESTS yet for dataflow-graph or systems-db-stamp. Both were verified by running the two implementations side by side on the live tree, which proves the happy path only. The stamp gate''s six failure modes are entirely unexercised, and its fixture is awkward because the old script imports generator_sources from its own directory, so a fixture must copy both files and build a sqlite DB with a meta table. +- Retirement (scripts, parity cases, make targets, the pre-commit hook entry for fact-ids) waits for the whole domain, per the per-domain rule. + +PROGRESS 2026-08-31, third increment — canvas-version ported. ALL FOUR GATES ARE NOW IN THE DOMAIN: client-version, canvas-version, systems-db-stamp, dataflow-graph, fact-ids. Byte-identical output to the original on the live tree. + +FIRST REAL CONSUMER OF core/process.run. The git calls go through the guarded exec with check=False, deliberately: a git failure here is not an error to report but a signal that there is nothing to compare — a fresh clone with no remote is a legitimate state, not a broken one. The argv-list and missing-binary guards still apply. + +TWO SKIPS KEPT DISTINCT FROM A PASS. NO_BASE (base ref absent) and DIFF_FAILED both exit 0, as does CLEAN (nothing relevant changed) — but only CLEAN means the gate actually looked at something. Collapsing them would hide a gate that had silently stopped running, which for this particular check is the exact failure mode it exists to prevent. + +A SECOND RICH PATH FOUND, and it is worth knowing about. typer''s PRETTY-EXCEPTION handler renders unhandled errors as box-art with syntax highlighting, and it is a DIFFERENT mechanism from rich_markup_mode — setting one does nothing about the other. It surfaced when a NameError produced a full-width box-drawn traceback. That is the same log pollution rich_markup_mode=None was set to prevent (T-1259), arriving through another door, and landing in the worst possible place: a hook log at the moment something has already gone wrong. pretty_exceptions_enable=False now set on both the root app and every domain built by cli.domain(). + +THE EXISTING TEST MOVED WITH THE CODE. tooling/test_canvas_version_check.py loaded the extensionless script through a SourceFileLoader and reached canvas_sources via sys.path.insert — both only because tooling/ was not an importable package. It now does `from tooling.domains.check import service as check` and `from tooling.canvas_sources import relative_paths`. Second instance of the E402/sys.path debt evaporating on contact (T-1274). The test passes unchanged in what it ASSERTS, which is the point: diff_has_version_bump was kept pure in the port precisely so those six properties still hold without constructing git history. + +A BUG CAUGHT BY RUNNING IT: the check router had dropped its `import typer` in T-1267 when it moved to cli.domain(), and canvas-version needs typer.Option for --base/--head. Restored. + +STILL OPEN on this ticket: +- No parity tests for canvas-version, dataflow-graph or systems-db-stamp. All three were verified side by side on the live tree, which proves the happy path only. canvas-version''s failure path is the valuable one and needs a git fixture — a temp repo with a commit touching a registry file and no version bump. +- Retirement of all five old scripts, their make targets, and the pre-commit hook entry for fact-ids.', NULL, '2026-08-31 16:22:11', '2026-08-31 16:22:11.006', '2026-08-31 16:22:11.006', NULL, 'ffbaaabe80ba8e200132ab7fcd391775', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-08.sql b/.pql/changelog/tickets/2026-08.sql index cf9ef988a..4d8dd237f 100644 --- a/.pql/changelog/tickets/2026-08.sql +++ b/.pql/changelog/tickets/2026-08.sql @@ -2480,3 +2480,37 @@ STILL OPEN on this ticket, and honest about it: - check-canvas-version is not ported. It is the one that genuinely parses git, so its fixture needs a real git repo — the hardest of the four. - NO PARITY TESTS yet for dataflow-graph or systems-db-stamp. Both were verified by running the two implementations side by side on the live tree, which proves the happy path only. The stamp gate''s six failure modes are entirely unexercised, and its fixture is awkward because the old script imports generator_sources from its own directory, so a fixture must copy both files and build a sqlite DB with a meta table. - Retirement (scripts, parity cases, make targets, the pre-commit hook entry for fact-ids) waits for the whole domain, per the per-domain rule.', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-31 15:30:35.338', '2026-08-31 15:55:15.374', NULL, '86fc272377f0603e44b7b5af742a5f90', 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 ('06G5GWCTS89VSST07DFVBVG064', 'task', '06G1S3D0M1TQW0GMFBBPQZG3ZM', 'Port the check domain — the four remaining gates', 'Finish what T-1262 started. domains/check/ currently has one verb (client-version); the other four gates join it: check-canvas-version, check-systems-db-stamp, check-fact-ids and check-dataflow-graph.py. Per the domain map, canvas_sources.py and generator_sources.py move alongside as module-level REGISTRIES, not verbs — nothing types them, and putting them in reach --help would answer a question nobody has. schema_version.py goes to the db domain where it is defined; check imports it, which is what a service layer is for. STANDARD PORT ACCEPTANCE, applying to every per-domain ticket under this epic: (a) service.py holds the logic and is transport-agnostic — no printing, no sys.exit, no typer; (b) router.py holds no logic and every command wears @command; (c) a parity test per gate, copying the pattern in tooling/test_check_parity.py, which builds a throwaway fixture repo and runs BOTH implementations — exit codes must match exactly and no fact the old message carried may be lost; (d) the old scripts are RETIRED once parity passes, not left alongside; (e) that domain''s tooling make targets are retired too rather than left as wrappers, per the D-263 make/reach split — build orchestration stays with make. Note check-dataflow-graph.py and check-canvas-version both parse git output, so their fixtures need a git repo rather than a bare directory; that is the one thing here harder than client-version was. + +PROGRESS 2026-08-31 — fact-ids ported, the first bash-to-Python rewrite under the D-263 decision. It was 89 lines of grep/sed pipeline; it is now service.fact_ids() returning a FactIdCheck, with a router that renders it. PARITY IS EXACT ON THE LIVE TREE: both implementations print ''check-fact-ids: OK — 6 references validated against 61 canonical facts'' and exit 0. The matching COUNTS are the real evidence — if the line-matching regex differed from the grep chain even slightly, 6 and 61 would diverge. Kept line-matched rather than YAML-parsed on purpose: parsing properly would change which lines count (anchors, merge keys, multi-document files would start contributing ids the old check never saw), which is a different check wearing the same name. Three parity cases added to test_check_parity.py — ok, unknown fact_id, and the advisory mode where catalogs are unpopulated. Proven to fail: removing the entity-attributes.yaml exclusion was caught, and interestingly by the ADVISORY case rather than the exclusion assertion — including that file made the catalog non-empty, so the new implementation went to enforcing mode while the old stayed advisory. A real behavioural divergence, caught by exit code. RETIREMENT DEFERRED to the end of this ticket, and there is a reason worth recording. The rule is retire per DOMAIN, and three gates remain. It also resolves a tension: the parity test COPIES the old script into its fixture, so deleting the script early would delete the test''s own subject. A parity test is scaffolding with a defined lifetime — when the old script goes, its parity case goes with it and is replaced by a unit test of the new behaviour. Same will apply to every port under T-1250. NOTE the pre-commit hook calls tooling/check-fact-ids directly, so retirement means editing that hook — and a mistake there is felt on every commit rather than every push. Do it together with validate-content (T-1282), which is in the same hook, so it is edited once. + +PROGRESS 2026-08-31, second increment — dataflow-graph and systems-db-stamp ported. Both were already Python, so these are moves rather than rewrites, and both produce BYTE-IDENTICAL output to their originals on the live tree: "check-dataflow-graph: OK — 1 diagram(s)" and "check-systems-db-stamp: OK — 1 generator(s) up to date", same exit codes. + +CORRECTION TO THIS TICKET''S OWN NOTE: it says check-dataflow-graph.py parses git output. It does not — it reads .d2 files and globs the filesystem. Only check-canvas-version parses git, so only that one needs a git repo in its fixture. + +THE E402 DEBT EVAPORATED, exactly as T-1274 predicted. check-systems-db-stamp reached generator_sources through sys.path.insert plus a noqa: E402, because tooling/ was not a package. It now imports as `from tooling import generator_sources` with no hack and no suppression. That is the first concrete evidence for T-1274''s premise; the map''s plan to move generator_sources into the db domain still stands, but it is already importable where it sits. + +SIX FAILURE MODES PRESERVED in the stamp port, modelled as a StampState enum rather than collapsed to pass/fail, because they carry different remedies and one carries a different EXIT CODE: UNSTAMPED exits 2 while every other failure exits 1, and the pre-push hook has relied on that distinction since T-857. The states are ok / absent / unstamped / bad_version / conflict / unknown / broken / stale, each with its own remedy string. + +ONE DELIBERATE BEHAVIOURAL DIFFERENCE, flagged rather than hidden: the old stamp script was SILENT on success unless given --verbose; the new one always prints its verdict. That is not a lost fact, so it does not violate the parity definition, and it makes the gate consistent with client-version and dataflow-graph, which both always print. The old script was the odd one out. Its per-command --verbose also disappears in favour of the global one, which is the consolidation the whole initiative is for. + +STILL OPEN on this ticket, and honest about it: +- check-canvas-version is not ported. It is the one that genuinely parses git, so its fixture needs a real git repo — the hardest of the four. +- NO PARITY TESTS yet for dataflow-graph or systems-db-stamp. Both were verified by running the two implementations side by side on the live tree, which proves the happy path only. The stamp gate''s six failure modes are entirely unexercised, and its fixture is awkward because the old script imports generator_sources from its own directory, so a fixture must copy both files and build a sqlite DB with a meta table. +- Retirement (scripts, parity cases, make targets, the pre-commit hook entry for fact-ids) waits for the whole domain, per the per-domain rule. + +PROGRESS 2026-08-31, third increment — canvas-version ported. ALL FOUR GATES ARE NOW IN THE DOMAIN: client-version, canvas-version, systems-db-stamp, dataflow-graph, fact-ids. Byte-identical output to the original on the live tree. + +FIRST REAL CONSUMER OF core/process.run. The git calls go through the guarded exec with check=False, deliberately: a git failure here is not an error to report but a signal that there is nothing to compare — a fresh clone with no remote is a legitimate state, not a broken one. The argv-list and missing-binary guards still apply. + +TWO SKIPS KEPT DISTINCT FROM A PASS. NO_BASE (base ref absent) and DIFF_FAILED both exit 0, as does CLEAN (nothing relevant changed) — but only CLEAN means the gate actually looked at something. Collapsing them would hide a gate that had silently stopped running, which for this particular check is the exact failure mode it exists to prevent. + +A SECOND RICH PATH FOUND, and it is worth knowing about. typer''s PRETTY-EXCEPTION handler renders unhandled errors as box-art with syntax highlighting, and it is a DIFFERENT mechanism from rich_markup_mode — setting one does nothing about the other. It surfaced when a NameError produced a full-width box-drawn traceback. That is the same log pollution rich_markup_mode=None was set to prevent (T-1259), arriving through another door, and landing in the worst possible place: a hook log at the moment something has already gone wrong. pretty_exceptions_enable=False now set on both the root app and every domain built by cli.domain(). + +THE EXISTING TEST MOVED WITH THE CODE. tooling/test_canvas_version_check.py loaded the extensionless script through a SourceFileLoader and reached canvas_sources via sys.path.insert — both only because tooling/ was not an importable package. It now does `from tooling.domains.check import service as check` and `from tooling.canvas_sources import relative_paths`. Second instance of the E402/sys.path debt evaporating on contact (T-1274). The test passes unchanged in what it ASSERTS, which is the point: diff_has_version_bump was kept pure in the port precisely so those six properties still hold without constructing git history. + +A BUG CAUGHT BY RUNNING IT: the check router had dropped its `import typer` in T-1267 when it moved to cli.domain(), and canvas-version needs typer.Option for --base/--head. Restored. + +STILL OPEN on this ticket: +- No parity tests for canvas-version, dataflow-graph or systems-db-stamp. All three were verified side by side on the live tree, which proves the happy path only. canvas-version''s failure path is the valuable one and needs a git fixture — a temp repo with a commit touching a registry file and no version bump. +- Retirement of all five old scripts, their make targets, and the pre-commit hook entry for fact-ids.', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-31 15:30:35.338', '2026-08-31 16:22:11.006', NULL, '3e7813e253aead4835ce00f1f3533abd', 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/tooling/core/cli.py b/tooling/core/cli.py index 8c7135eeb..f19a096dc 100644 --- a/tooling/core/cli.py +++ b/tooling/core/cli.py @@ -84,4 +84,11 @@ def domain(name: str, help: str) -> typer.Typer: no_args_is_help=True, add_completion=False, rich_markup_mode=None, + # A SECOND rich path, separate from rich_markup_mode and easy to miss: + # typer's pretty-exception handler renders unhandled errors as box-art + # with syntax highlighting. That is the same log pollution + # rich_markup_mode=None prevents for help text, arriving through a + # different door — and it lands in the worst place, a hook log at the + # moment something has already gone wrong. Plain tracebacks instead. + pretty_exceptions_enable=False, ) diff --git a/tooling/domains/check/router.py b/tooling/domains/check/router.py index 1475c8613..7cc327b56 100644 --- a/tooling/domains/check/router.py +++ b/tooling/domains/check/router.py @@ -17,11 +17,13 @@ nothing left to defer. Laziness lives one level up, in `main.py`. from __future__ import annotations +import typer + from tooling.core import cli, console from tooling.core.command import command from tooling.core.errors import ReachError from tooling.domains.check import service -from tooling.domains.check.schemas import StampState +from tooling.domains.check.schemas import CanvasState, StampState app = cli.domain("check", "Consistency gates — the checks the push hook runs.") @@ -67,6 +69,63 @@ def client_version() -> None: console.verdict(f"check-client-version: OK — {result.yaml_version}") +@app.command("canvas-version") +@command +def canvas_version( + base: str = typer.Option( + service.DEFAULT_BASE, "--base", help="Base ref to compare against." + ), + head: str = typer.Option("HEAD", "--head", help="Head ref to compare."), +) -> None: + """Fail if canvas generation changed without project.yaml's version moving.""" + result = service.canvas_version(base, head) + + if result.state is CanvasState.NO_BASE: + console.verdict( + f"check-canvas-version: {result.base} not found — skipping (nothing to compare)" + ) + return + if result.state is CanvasState.DIFF_FAILED: + console.verdict( + f"check-canvas-version: could not diff {result.commit_range} — skipping" + ) + return + if result.state is CanvasState.CLEAN: + console.verdict("check-canvas-version: no canvas-generation changes in range — OK") + return + if result.state is CanvasState.BUMPED: + console.verdict( + f"check-canvas-version: OK — {len(result.touched)} canvas-generation " + "file(s) changed and project.yaml's version moved with them" + ) + return + + shown = result.touched[:10] + remainder = len(result.touched) - len(shown) + listing = "".join(f" {path}\n" for path in shown) + if remainder: + listing += f" ... and {remainder} more\n" + + raise ReachError( + "check-canvas-version: canvas generation changed without a version bump\n" + "\n" + f" Range: {result.commit_range}\n" + " Changed canvas-generation files:\n" + listing + "\n" + "project.yaml's `version:` is the Atlas disk cache's ONLY invalidation\n" + "signal. Without a bump, every warm cache keeps serving canvases built by\n" + "the code you just changed — silently, and only on machines that have a\n" + "warm cache, so you will not see it on a cold checkout.", + fix=( + "bump `version:` in project.yaml (scheme 0.{phase}.{n}), note it in the " + "comment block above, and mirror the value into client/project.godot's " + "config/version. If you are certain the change cannot alter canvas bytes, " + "bump it anyway — the cost is one round of cache misses, and this has " + "shipped broken five times, most recently T-1239, which took eight days " + "to find." + ), + ) + + _STAMP_REMEDY = { StampState.UNSTAMPED: "make regen-db — systems.db carries no stamp to verify", StampState.BAD_VERSION: "make regen-db — the recorded schema_version predates semver", diff --git a/tooling/domains/check/schemas.py b/tooling/domains/check/schemas.py index 72321e3d0..377eff623 100644 --- a/tooling/domains/check/schemas.py +++ b/tooling/domains/check/schemas.py @@ -67,6 +67,37 @@ class DiagramCheck(BaseModel): return not self.unresolved and not self.missing and self.checked > 0 +class CanvasState(str, Enum): + """Why the canvas-version gate reached its verdict. + + Two of these are SKIPS rather than passes, and keeping them distinct + matters: "no base to compare against" and "nothing relevant changed" both + exit 0, but only the second means the gate actually looked at something. + Collapsing them would hide a gate that silently stopped running. + """ + + NO_BASE = "no_base" # base ref absent (fresh clone) — nothing to compare + DIFF_FAILED = "diff_failed" # git could not produce a range + CLEAN = "clean" # nothing in the registry was touched + BUMPED = "bumped" # canvas files changed AND version moved + NEEDS_BUMP = "needs_bump" # canvas files changed, version did not + + +class CanvasCheck(BaseModel): + """The outcome of pairing canvas-generation changes with a version bump.""" + + model_config = ConfigDict(frozen=True) + + state: CanvasState + commit_range: str + touched: list[str] = [] + base: str = "" + + @property + def ok(self) -> bool: + return self.state is not CanvasState.NEEDS_BUMP + + class StampState(str, Enum): """Why the systems.db stamp check reached its verdict. diff --git a/tooling/domains/check/service.py b/tooling/domains/check/service.py index f4766f7cf..9e3da8d45 100644 --- a/tooling/domains/check/service.py +++ b/tooling/domains/check/service.py @@ -12,8 +12,10 @@ import re import sqlite3 from pathlib import Path -from tooling.core import config +from tooling.core import config, process from tooling.domains.check.schemas import ( + CanvasCheck, + CanvasState, DiagramCheck, FactIdCheck, StampCheck, @@ -60,6 +62,93 @@ def client_version() -> VersionCheck: ) +# --- canvas-generation version pairing ------------------------------------ +# +# project.yaml's `version:` is the Atlas disk cache's ONLY invalidation signal. +# Change how a canvas is generated without moving it and every warm cache keeps +# serving canvases built by code that no longer exists — silently, and only on +# machines with a warm cache, so the author never sees it. That has shipped five +# times; T-1239 is what the last one cost (eight days). +# +# Deliberately no override flag (T-1242): a false positive costs one round of +# cache misses, a false negative costs another week of a wrong map. An escape +# hatch would be reached for exactly when someone is sure their change is +# harmless — which is the state of mind that produced all five regressions. + +DEFAULT_BASE = "origin/main" + + +def diff_has_version_bump(diff_text: str) -> bool: + """Does this project.yaml diff actually MOVE the `version:` field? + + Kept pure so the property is testable without constructing git history — + which is what tooling/test_canvas_version_check.py exercises, and the reason + that test could be written at all. + + Matched on the diff BODY rather than on the filename appearing in + --name-only: project.yaml carries a comment block documenting past bumps, + including lines quoting old version NUMBERS, so editing that commentary must + not count. Requires the ADDED side — a lone deletion means the field was + removed, not moved — and skips `+++` headers, which would otherwise match. + """ + for line in diff_text.splitlines(): + if line.startswith("+++"): + continue + if line.startswith("+version:"): + return True + return False + + +def canvas_version(base: str = DEFAULT_BASE, head: str = "HEAD") -> CanvasCheck: + """Require a version bump alongside any canvas-generation change.""" + from tooling import canvas_sources + + # Three-dot: what HEAD added since the merge base, matching the convention + # the pre-push hook already uses for the systems.db stamp check. + commit_range = f"{base}...{head}" + + if _git("rev-parse", "--verify", base) is None: + # No base to compare against (fresh clone, no remote). Skipping is + # correct rather than failing: there is no range to judge. + return CanvasCheck(state=CanvasState.NO_BASE, commit_range=commit_range, base=base) + + listing = _git("diff", "--name-only", commit_range) + if listing is None: + return CanvasCheck( + state=CanvasState.DIFF_FAILED, commit_range=commit_range, base=base + ) + + changed = {line for line in listing.splitlines() if line} + touched = sorted(changed & set(canvas_sources.relative_paths())) + if not touched: + return CanvasCheck(state=CanvasState.CLEAN, commit_range=commit_range, base=base) + + bumped = _git("diff", "-U0", commit_range, "--", "project.yaml") + state = ( + CanvasState.BUMPED + if bumped is not None and diff_has_version_bump(bumped) + else CanvasState.NEEDS_BUMP + ) + return CanvasCheck( + state=state, commit_range=commit_range, touched=touched, base=base + ) + + +def _git(*args: str) -> str | None: + """Run git, returning stdout, or None if it failed. + + `check=False` on purpose: a git failure here is not an error to report, it + is a signal that there is nothing to compare — a fresh clone with no remote + is a legitimate state, not a broken one. Still goes through + `core.process.run` so the argv-list and missing-binary guards apply. + """ + result = process.run( + ["git", "-C", str(config.repo_root()), *args], + check=False, + ) + return result.stdout if result.returncode == 0 else None + + # --- systems-db stamp ----------------------------------------------------- # # server/data/systems.db is a committed build artefact. Its meta table records diff --git a/tooling/main.py b/tooling/main.py index 0f3151a3b..26beb574e 100644 --- a/tooling/main.py +++ b/tooling/main.py @@ -126,6 +126,9 @@ cli = typer.Typer( no_args_is_help=True, add_completion=False, rich_markup_mode=None, + # See core/cli.py — a second rich path that renders unhandled exceptions as + # box-art, arriving through a different door than rich_markup_mode. + pretty_exceptions_enable=False, context_settings={"help_option_names": ["-h", "--help"], "max_content_width": 100}, ) diff --git a/tooling/test_canvas_version_check.py b/tooling/test_canvas_version_check.py index d0b45ce51..3fb8fb421 100644 --- a/tooling/test_canvas_version_check.py +++ b/tooling/test_canvas_version_check.py @@ -16,30 +16,20 @@ stays quiet otherwise. Two properties carry that, and neither is observable from Run: python3 tooling/test_canvas_version_check.py """ -import importlib.util import sys from pathlib import Path REPO_ROOT = Path(__file__).resolve().parent.parent -TOOLING = REPO_ROOT / "tooling" -sys.path.insert(0, str(TOOLING)) -from canvas_sources import relative_paths # noqa: E402 +# Re-pointed at the ported service (T-1281). This test previously loaded the +# extensionless tooling/check-canvas-version through a SourceFileLoader and +# reached canvas_sources via a sys.path insert — both because tooling/ was not +# an importable package. Neither is needed now, and the test moves with the code +# it guards rather than being left aimed at a script due for retirement. +sys.path.insert(0, str(REPO_ROOT)) - -def _load_check_module(): - """Import the extensionless check script as a module.""" - path = TOOLING / "check-canvas-version" - spec = importlib.util.spec_from_loader( - "check_canvas_version", - importlib.machinery.SourceFileLoader("check_canvas_version", str(path)), - ) - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - return module - - -check = _load_check_module() +from tooling.canvas_sources import relative_paths # noqa: E402 +from tooling.domains.check import service as check # noqa: E402 FAILURES: list[str] = []