From 7f20bd303b118ec576033420e9c09ea4cabc3a7a Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 2 Sep 2026 12:48:56 +0200 Subject: [PATCH] =?UTF-8?q?feat(config):=20T-1282=20=E2=80=94=20the=20vali?= =?UTF-8?q?date=20domain,=20and=20a=20move=20that=20broke=20a=20root?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reach validate content / checklist / ron / name-collisions. The three old scripts are retired, their make targets with them. Print statements go through the logging sink rather than a collector. The validators emit their findings as console events as they run, so a long content validation streams instead of going quiet and dumping at the end — the message strings and their order are unchanged, only the destination. That also satisfies the conformance rule forbidding print() in the package, which is what forced the question. validate-ron was three languages deep: bash dispatching on a flag, a Python heredoc doing collision detection, cargo run for schema validation. Logic embedded in a shell string cannot be imported, tested, or found by anything that indexes Python, so it became Python; the cargo call became a guarded exec. It also split into two verbs, because --check-name-collisions answered a different question from the default path: whether the SET of cultures is coherent, versus whether ONE file is well-formed. The move broke something, quietly, which is the point of doing these one at a time. validate-checklist computed ROOT as Path(__file__).parent.parent — the repo root while it lived at tooling/validate-checklist, and tooling/domains once moved. Both its schema and gauntlet paths silently repointed at nothing, the gauntlet directory "did not exist", and it reported success having checked zero files. Caught by running it beside the original: old exit 1, new exit 0. Now config.repo_root(), and load_schema raises ReachError instead of calling sys.exit, which a service must not do. Parity on the live tree: content reproduces the original byte for byte including its counts, name-collisions likewise. Tests pin what those runs cannot reach — the detection path, since the repo currently has no collisions, and the argument errors. Two things found and left alone: validate-content FAILS on the live tree with 13 missing schemas, pre-existing and unrelated to this port; and the ticket's claim that validate-content sits in the pre-commit hook is wrong — that hook runs only check-fact-ids and pql decisions validate, so there was no shared edit to coordinate. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/settings.json | 11 -- .pql/changelog/ticket_history/2026-08.sql | 128 +++++++++++++ .pql/changelog/ticket_history/2026-09.sql | 2 + .pql/changelog/tickets/2026-08.sql | 146 ++++++++++++++ .pql/changelog/tickets/2026-09.sql | 2 + Makefile | 27 ++- docs/DEVOPS.md | 2 +- tooling/domains/validate/__init__.py | 6 + .../validate/checklist.py} | 74 ++++--- .../validate/content.py} | 119 ++++++------ tooling/domains/validate/ron.py | 112 +++++++++++ tooling/domains/validate/router.py | 100 ++++++++++ tooling/main.py | 4 + tooling/test_validate.py | 180 ++++++++++++++++++ tooling/validate-ron | 137 ------------- 15 files changed, 798 insertions(+), 252 deletions(-) create mode 100644 .pql/changelog/ticket_history/2026-09.sql create mode 100644 .pql/changelog/tickets/2026-09.sql create mode 100644 tooling/domains/validate/__init__.py rename tooling/{validate-checklist => domains/validate/checklist.py} (62%) rename tooling/{validate-content => domains/validate/content.py} (81%) create mode 100644 tooling/domains/validate/ron.py create mode 100644 tooling/domains/validate/router.py create mode 100644 tooling/test_validate.py delete mode 100755 tooling/validate-ron diff --git a/.claude/settings.json b/.claude/settings.json index 63d58ada0..1b8a0dbfe 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -24,25 +24,19 @@ "Bash(git rm *)", "Bash(git ls-tree *)", "Bash(git rev-parse --show-toplevel)", - "Bash(tooling/db/audio-generate *)", "Bash(tooling/db/audio-health)", "Bash(tooling/db/audio-post *)", "Bash(tooling/db/audio-batch *)", - "Bash(make *)", "Bash(make)", - "Bash(pql)", "Bash(pql *)", - "Bash(reach)", "Bash(reach *)", - "Bash(tea *)", "Bash(tooling/tea-comment *)", "Bash(tooling/worktree-setup:*)", - "Bash(cargo test *)", "Bash(cargo test)", "Bash(cargo build *)", @@ -54,27 +48,22 @@ "Bash(ruff check *)", "Bash(ruff check)", "Bash(tests/run-*)", - "Bash(godot4:*)", "Bash(gdformat:*)", "Bash(tooling/atlas:*)", - "Bash(tooling/validate-ron:*)", "Bash(tooling/godot-cold-parse:*)", "Bash(tooling/pr-watchlist-diff:*)", - "Bash(chmod *)", "Bash(ls *)", "Bash(find *)", "Bash(list *)", "Bash(tree *)", "Bash(sed -n *)", - "Read(//var/mnt/data/projects/settled-reach/**)", "Edit(//var/mnt/data/projects/settled-reach/**)", "Read(//tmp/**)", "Grep", "Glob", - "Skill(git-commit)", "Skill(whats-next)", "Skill(pr-process)", diff --git a/.pql/changelog/ticket_history/2026-08.sql b/.pql/changelog/ticket_history/2026-08.sql index 2e8f6c4ed..088819126 100644 --- a/.pql/changelog/ticket_history/2026-08.sql +++ b/.pql/changelog/ticket_history/2026-08.sql @@ -2292,3 +2292,131 @@ PROVEN TO FAIL: truncating the touched-file set made canvas-needs-bump report bo SHAs are hardcoded because history is immutable, and each case asserts the OLD script still behaves as the case claims — so if a rebase ever invalidated a range, the test says so by name rather than silently checking nothing. STILL OPEN on this ticket: parity tests for dataflow-graph and systems-db-stamp (happy path verified side by side only), and retirement of all five old scripts, their make targets, and the pre-commit hook entry for fact-ids.', NULL, '2026-08-31 18:07:14', '2026-08-31 18:07:14.123', '2026-08-31 18:07:14.123', NULL, 'eacbb029d58e1ea83aae262aed0c8e89', 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. + +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. + +PROGRESS 2026-08-31, fourth increment — canvas-version parity, five cases, including its failure path. + +A FIXTURE REPO DOES NOT WORK FOR THIS GATE, and finding out why exposed a real inconsistency in the port. canvas_sources.relative_paths() globs from a __file__-derived REPO_ROOT, so under SR_REPO_ROOT the new service would DIFF the fixture while GLOBBING the real tree. The service resolves git through config.repo_root() but the registry does not — the two disagree about which repo is being checked. Harmless in production (they are the same repo) but it makes fixture-based parity impossible, and it is the same asymmetry the domain map already noted about old scripts having no root override, showing up inside the new code. +NOT FIXED HERE: making it dynamic means restructuring six module-level path constants in a module the old script also imports, which is more churn than this ticket should carry. Filed as a follow-up consideration for when canvas_sources moves into the domain per the map. + +SO PARITY USES REAL HISTORY INSTEAD, which is better anyway: both implementations see byte-identical input, nothing is mutated, and no fixture can drift from the thing it models. Five cases: +- canvas-needs-bump: 4e503c356 (T-1237 one-course-per-river) — changed canvas generation, did NOT bump. Both exit 1. +- canvas-needs-bump-2: 566b56651 (T-1194 relief/vegetation texture) — same. Both exit 1. +- canvas-bumped: 9b146f9e1 — changed canvas AND bumped. Both exit 0. +- canvas-clean: 6b31111cd — governance only. Both exit 0. +- canvas-no-base: a nonexistent base ref — a SKIP, not a failure. Both exit 0. +The two failure cases are GENUINE historical instances of the regression this gate exists to catch, both bumped after the fact. That is the history that produced the check, used as its own test. + +THE TEST ALSO ASSERTS NO CHANGED FILE IS DROPPED from the failure message. The list of touched files is the actionable half — a failure saying "something changed" without saying what leaves the reader to re-derive the intersection by hand. + +PROVEN TO FAIL: truncating the touched-file set made canvas-needs-bump report both the exit-code divergence AND all three omitted filenames by name. + +SHAs are hardcoded because history is immutable, and each case asserts the OLD script still behaves as the case claims — so if a rebase ever invalidated a range, the test says so by name rather than silently checking nothing. + +STILL OPEN on this ticket: parity tests for dataflow-graph and systems-db-stamp (happy path verified side by side only), and retirement of all five old scripts, their make targets, and the pre-commit hook entry for fact-ids.', '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. + +PROGRESS 2026-08-31, fourth increment — canvas-version parity, five cases, including its failure path. + +A FIXTURE REPO DOES NOT WORK FOR THIS GATE, and finding out why exposed a real inconsistency in the port. canvas_sources.relative_paths() globs from a __file__-derived REPO_ROOT, so under SR_REPO_ROOT the new service would DIFF the fixture while GLOBBING the real tree. The service resolves git through config.repo_root() but the registry does not — the two disagree about which repo is being checked. Harmless in production (they are the same repo) but it makes fixture-based parity impossible, and it is the same asymmetry the domain map already noted about old scripts having no root override, showing up inside the new code. +NOT FIXED HERE: making it dynamic means restructuring six module-level path constants in a module the old script also imports, which is more churn than this ticket should carry. Filed as a follow-up consideration for when canvas_sources moves into the domain per the map. + +SO PARITY USES REAL HISTORY INSTEAD, which is better anyway: both implementations see byte-identical input, nothing is mutated, and no fixture can drift from the thing it models. Five cases: +- canvas-needs-bump: 4e503c356 (T-1237 one-course-per-river) — changed canvas generation, did NOT bump. Both exit 1. +- canvas-needs-bump-2: 566b56651 (T-1194 relief/vegetation texture) — same. Both exit 1. +- canvas-bumped: 9b146f9e1 — changed canvas AND bumped. Both exit 0. +- canvas-clean: 6b31111cd — governance only. Both exit 0. +- canvas-no-base: a nonexistent base ref — a SKIP, not a failure. Both exit 0. +The two failure cases are GENUINE historical instances of the regression this gate exists to catch, both bumped after the fact. That is the history that produced the check, used as its own test. + +THE TEST ALSO ASSERTS NO CHANGED FILE IS DROPPED from the failure message. The list of touched files is the actionable half — a failure saying "something changed" without saying what leaves the reader to re-derive the intersection by hand. + +PROVEN TO FAIL: truncating the touched-file set made canvas-needs-bump report both the exit-code divergence AND all three omitted filenames by name. + +SHAs are hardcoded because history is immutable, and each case asserts the OLD script still behaves as the case claims — so if a rebase ever invalidated a range, the test says so by name rather than silently checking nothing. + +STILL OPEN on this ticket: parity tests for dataflow-graph and systems-db-stamp (happy path verified side by side only), and retirement of all five old scripts, their make targets, and the pre-commit hook entry for fact-ids. + +DONE 2026-08-31. All five gates ported, tested against their failure paths, old scripts retired, hooks and make targets rewired, and both hooks exercised for real. + +FINAL STATE: reach check client-version / canvas-version / systems-db-stamp / dataflow-graph / fact-ids. tooling/check-* no longer exists. + +VERIFIED BY THE HOOKS THEMSELVES, not by reading them. The retirement commit ran the rewired PRE-COMMIT hook, which executed reach check fact-ids and emitted its verdict. The push ran the rewired PRE-PUSH hook, which executed client-version, canvas-version and dataflow-graph through reach and passed. That is the only evidence that matters here — a hook edit verified by inspection is a hook edit that has not been tested. + +TESTS: tooling/test_check.py, 19 cases across five gates, every gate exercising at least one failure. Renamed from test_check_parity.py because its subjects no longer exist. Every case began as a parity case running both implementations; that evidence is recorded above and in the commits. With the scripts gone there is nothing to compare against, so the assertions become the spec. A parity test is scaffolding with a defined lifetime — keeping one after its subject is deleted would mean keeping the subject alive to be compared with, which is the opposite of a migration. + +TWO GATES COULD NOT BE FIXTURE-TESTED AGAINST THEIR ORIGINALS, and both reasons are findings rather than obstacles: +- canvas-version: canvas_sources globs from a __file__-derived root while the service resolves git through config.repo_root(), so a fixture would DIFF one tree and GLOB another. Real history used instead — five ranges including two GENUINE instances of the regression this gate exists to catch (T-1237 and T-1194, both bumped after the fact). The history that produced the check, used as its own fixture. +- systems-db-stamp: generator_sources raises at IMPORT time when the economy-db tree is absent, so the old script died at line 39 in every fixture, before any logic. Its failure modes were never testable in isolation. The ported service imports it lazily and only after the absent/unstamped checks, which is precisely why absent/unstamped/empty/unknown/bad-version are reachable now and were not before. STALE still needs the full registered source set and is covered by the live repo on every push. + +MAKE TARGETS RETIRED, NOT WRAPPED, per the D-263 split, with the mapping left as a comment where they used to be so the next reader finds the answer rather than an absence. Wrapping would leave two ways to invoke each gate and reach --help would stop being the answer to "what tooling exists" while the Makefile stayed a competing index. pre-pr-validate and pre-pr-content keep their orchestration role and drop the individual target dependency. + +DOCS: CLAUDE.md and docs/DEVOPS.md updated to name the reach verbs. Sprint archives and workshop notes still reference the old paths and are deliberately left alone — they record what was true when written, and rewriting history to match the present is how a record stops being evidence. + +NET: 1040 lines deleted, 399 added.', NULL, '2026-08-31 18:42:59', '2026-08-31 18:42:59.700', '2026-08-31 18:42:59.700', NULL, '43a455cdc2ed0f006229bf85b97cff0b', 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', 'status', 'in_progress', 'done', NULL, '2026-08-31 18:43:00', '2026-08-31 18:43:00.154', '2026-08-31 18:43:00.154', NULL, 'a1e28007b5eaeed5eb27e74a98927937', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/ticket_history/2026-09.sql b/.pql/changelog/ticket_history/2026-09.sql new file mode 100644 index 000000000..11d6d2a56 --- /dev/null +++ b/.pql/changelog/ticket_history/2026-09.sql @@ -0,0 +1,2 @@ +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 ('06G5GWDPQ95RSCKK51K8DTYRCM', 'status', 'backlog', 'in_progress', NULL, '2026-09-01 15:22:13', '2026-09-01 15:22:13.360', '2026-09-01 15:22:13.360', NULL, '524f353821f4dc123ec999ca0e84f25e', 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 ('06G5GWDPQ95RSCKK51K8DTYRCM', 'status', 'in_progress', 'in_progress', NULL, '2026-09-01 15:22:27', '2026-09-01 15:22:27.856', '2026-09-01 15:22:27.856', NULL, '3c5c1770ada99b9f3c0822537836737e', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-08.sql b/.pql/changelog/tickets/2026-08.sql index c7fbcd7ba..0f1c0ab90 100644 --- a/.pql/changelog/tickets/2026-08.sql +++ b/.pql/changelog/tickets/2026-08.sql @@ -2569,3 +2569,149 @@ PROVEN TO FAIL: truncating the touched-file set made canvas-needs-bump report bo SHAs are hardcoded because history is immutable, and each case asserts the OLD script still behaves as the case claims — so if a rebase ever invalidated a range, the test says so by name rather than silently checking nothing. STILL OPEN on this ticket: parity tests for dataflow-graph and systems-db-stamp (happy path verified side by side only), and 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 18:07:14.123', NULL, 'b6d293132ca2394ec8e11441243d174f', 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. + +PROGRESS 2026-08-31, fourth increment — canvas-version parity, five cases, including its failure path. + +A FIXTURE REPO DOES NOT WORK FOR THIS GATE, and finding out why exposed a real inconsistency in the port. canvas_sources.relative_paths() globs from a __file__-derived REPO_ROOT, so under SR_REPO_ROOT the new service would DIFF the fixture while GLOBBING the real tree. The service resolves git through config.repo_root() but the registry does not — the two disagree about which repo is being checked. Harmless in production (they are the same repo) but it makes fixture-based parity impossible, and it is the same asymmetry the domain map already noted about old scripts having no root override, showing up inside the new code. +NOT FIXED HERE: making it dynamic means restructuring six module-level path constants in a module the old script also imports, which is more churn than this ticket should carry. Filed as a follow-up consideration for when canvas_sources moves into the domain per the map. + +SO PARITY USES REAL HISTORY INSTEAD, which is better anyway: both implementations see byte-identical input, nothing is mutated, and no fixture can drift from the thing it models. Five cases: +- canvas-needs-bump: 4e503c356 (T-1237 one-course-per-river) — changed canvas generation, did NOT bump. Both exit 1. +- canvas-needs-bump-2: 566b56651 (T-1194 relief/vegetation texture) — same. Both exit 1. +- canvas-bumped: 9b146f9e1 — changed canvas AND bumped. Both exit 0. +- canvas-clean: 6b31111cd — governance only. Both exit 0. +- canvas-no-base: a nonexistent base ref — a SKIP, not a failure. Both exit 0. +The two failure cases are GENUINE historical instances of the regression this gate exists to catch, both bumped after the fact. That is the history that produced the check, used as its own test. + +THE TEST ALSO ASSERTS NO CHANGED FILE IS DROPPED from the failure message. The list of touched files is the actionable half — a failure saying "something changed" without saying what leaves the reader to re-derive the intersection by hand. + +PROVEN TO FAIL: truncating the touched-file set made canvas-needs-bump report both the exit-code divergence AND all three omitted filenames by name. + +SHAs are hardcoded because history is immutable, and each case asserts the OLD script still behaves as the case claims — so if a rebase ever invalidated a range, the test says so by name rather than silently checking nothing. + +STILL OPEN on this ticket: parity tests for dataflow-graph and systems-db-stamp (happy path verified side by side only), and retirement of all five old scripts, their make targets, and the pre-commit hook entry for fact-ids. + +DONE 2026-08-31. All five gates ported, tested against their failure paths, old scripts retired, hooks and make targets rewired, and both hooks exercised for real. + +FINAL STATE: reach check client-version / canvas-version / systems-db-stamp / dataflow-graph / fact-ids. tooling/check-* no longer exists. + +VERIFIED BY THE HOOKS THEMSELVES, not by reading them. The retirement commit ran the rewired PRE-COMMIT hook, which executed reach check fact-ids and emitted its verdict. The push ran the rewired PRE-PUSH hook, which executed client-version, canvas-version and dataflow-graph through reach and passed. That is the only evidence that matters here — a hook edit verified by inspection is a hook edit that has not been tested. + +TESTS: tooling/test_check.py, 19 cases across five gates, every gate exercising at least one failure. Renamed from test_check_parity.py because its subjects no longer exist. Every case began as a parity case running both implementations; that evidence is recorded above and in the commits. With the scripts gone there is nothing to compare against, so the assertions become the spec. A parity test is scaffolding with a defined lifetime — keeping one after its subject is deleted would mean keeping the subject alive to be compared with, which is the opposite of a migration. + +TWO GATES COULD NOT BE FIXTURE-TESTED AGAINST THEIR ORIGINALS, and both reasons are findings rather than obstacles: +- canvas-version: canvas_sources globs from a __file__-derived root while the service resolves git through config.repo_root(), so a fixture would DIFF one tree and GLOB another. Real history used instead — five ranges including two GENUINE instances of the regression this gate exists to catch (T-1237 and T-1194, both bumped after the fact). The history that produced the check, used as its own fixture. +- systems-db-stamp: generator_sources raises at IMPORT time when the economy-db tree is absent, so the old script died at line 39 in every fixture, before any logic. Its failure modes were never testable in isolation. The ported service imports it lazily and only after the absent/unstamped checks, which is precisely why absent/unstamped/empty/unknown/bad-version are reachable now and were not before. STALE still needs the full registered source set and is covered by the live repo on every push. + +MAKE TARGETS RETIRED, NOT WRAPPED, per the D-263 split, with the mapping left as a comment where they used to be so the next reader finds the answer rather than an absence. Wrapping would leave two ways to invoke each gate and reach --help would stop being the answer to "what tooling exists" while the Makefile stayed a competing index. pre-pr-validate and pre-pr-content keep their orchestration role and drop the individual target dependency. + +DOCS: CLAUDE.md and docs/DEVOPS.md updated to name the reach verbs. Sprint archives and workshop notes still reference the old paths and are deliberately left alone — they record what was true when written, and rewriting history to match the present is how a record stops being evidence. + +NET: 1040 lines deleted, 399 added.', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-31 15:30:35.338', '2026-08-31 18:42:59.700', NULL, '0f6e6001ef4474934dd9da781def12e4', 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. + +PROGRESS 2026-08-31, fourth increment — canvas-version parity, five cases, including its failure path. + +A FIXTURE REPO DOES NOT WORK FOR THIS GATE, and finding out why exposed a real inconsistency in the port. canvas_sources.relative_paths() globs from a __file__-derived REPO_ROOT, so under SR_REPO_ROOT the new service would DIFF the fixture while GLOBBING the real tree. The service resolves git through config.repo_root() but the registry does not — the two disagree about which repo is being checked. Harmless in production (they are the same repo) but it makes fixture-based parity impossible, and it is the same asymmetry the domain map already noted about old scripts having no root override, showing up inside the new code. +NOT FIXED HERE: making it dynamic means restructuring six module-level path constants in a module the old script also imports, which is more churn than this ticket should carry. Filed as a follow-up consideration for when canvas_sources moves into the domain per the map. + +SO PARITY USES REAL HISTORY INSTEAD, which is better anyway: both implementations see byte-identical input, nothing is mutated, and no fixture can drift from the thing it models. Five cases: +- canvas-needs-bump: 4e503c356 (T-1237 one-course-per-river) — changed canvas generation, did NOT bump. Both exit 1. +- canvas-needs-bump-2: 566b56651 (T-1194 relief/vegetation texture) — same. Both exit 1. +- canvas-bumped: 9b146f9e1 — changed canvas AND bumped. Both exit 0. +- canvas-clean: 6b31111cd — governance only. Both exit 0. +- canvas-no-base: a nonexistent base ref — a SKIP, not a failure. Both exit 0. +The two failure cases are GENUINE historical instances of the regression this gate exists to catch, both bumped after the fact. That is the history that produced the check, used as its own test. + +THE TEST ALSO ASSERTS NO CHANGED FILE IS DROPPED from the failure message. The list of touched files is the actionable half — a failure saying "something changed" without saying what leaves the reader to re-derive the intersection by hand. + +PROVEN TO FAIL: truncating the touched-file set made canvas-needs-bump report both the exit-code divergence AND all three omitted filenames by name. + +SHAs are hardcoded because history is immutable, and each case asserts the OLD script still behaves as the case claims — so if a rebase ever invalidated a range, the test says so by name rather than silently checking nothing. + +STILL OPEN on this ticket: parity tests for dataflow-graph and systems-db-stamp (happy path verified side by side only), and retirement of all five old scripts, their make targets, and the pre-commit hook entry for fact-ids. + +DONE 2026-08-31. All five gates ported, tested against their failure paths, old scripts retired, hooks and make targets rewired, and both hooks exercised for real. + +FINAL STATE: reach check client-version / canvas-version / systems-db-stamp / dataflow-graph / fact-ids. tooling/check-* no longer exists. + +VERIFIED BY THE HOOKS THEMSELVES, not by reading them. The retirement commit ran the rewired PRE-COMMIT hook, which executed reach check fact-ids and emitted its verdict. The push ran the rewired PRE-PUSH hook, which executed client-version, canvas-version and dataflow-graph through reach and passed. That is the only evidence that matters here — a hook edit verified by inspection is a hook edit that has not been tested. + +TESTS: tooling/test_check.py, 19 cases across five gates, every gate exercising at least one failure. Renamed from test_check_parity.py because its subjects no longer exist. Every case began as a parity case running both implementations; that evidence is recorded above and in the commits. With the scripts gone there is nothing to compare against, so the assertions become the spec. A parity test is scaffolding with a defined lifetime — keeping one after its subject is deleted would mean keeping the subject alive to be compared with, which is the opposite of a migration. + +TWO GATES COULD NOT BE FIXTURE-TESTED AGAINST THEIR ORIGINALS, and both reasons are findings rather than obstacles: +- canvas-version: canvas_sources globs from a __file__-derived root while the service resolves git through config.repo_root(), so a fixture would DIFF one tree and GLOB another. Real history used instead — five ranges including two GENUINE instances of the regression this gate exists to catch (T-1237 and T-1194, both bumped after the fact). The history that produced the check, used as its own fixture. +- systems-db-stamp: generator_sources raises at IMPORT time when the economy-db tree is absent, so the old script died at line 39 in every fixture, before any logic. Its failure modes were never testable in isolation. The ported service imports it lazily and only after the absent/unstamped checks, which is precisely why absent/unstamped/empty/unknown/bad-version are reachable now and were not before. STALE still needs the full registered source set and is covered by the live repo on every push. + +MAKE TARGETS RETIRED, NOT WRAPPED, per the D-263 split, with the mapping left as a comment where they used to be so the next reader finds the answer rather than an absence. Wrapping would leave two ways to invoke each gate and reach --help would stop being the answer to "what tooling exists" while the Makefile stayed a competing index. pre-pr-validate and pre-pr-content keep their orchestration role and drop the individual target dependency. + +DOCS: CLAUDE.md and docs/DEVOPS.md updated to name the reach verbs. Sprint archives and workshop notes still reference the old paths and are deliberately left alone — they record what was true when written, and rewriting history to match the present is how a record stops being evidence. + +NET: 1040 lines deleted, 399 added.', 'done', 'high', NULL, NULL, 'D-263', '2026-08-31 15:30:35.338', '2026-08-31 18:43:00.154', NULL, '3b6d263bb556f6eee249ffc29d3d52dc', 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/.pql/changelog/tickets/2026-09.sql b/.pql/changelog/tickets/2026-09.sql new file mode 100644 index 000000000..909032872 --- /dev/null +++ b/.pql/changelog/tickets/2026-09.sql @@ -0,0 +1,2 @@ +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 ('06G5GWDPQ95RSCKK51K8DTYRCM', 'task', '06G1S3D0M1TQW0GMFBBPQZG3ZM', 'Port the validate domain — content, checklist, ron', 'validate-content, validate-checklist and validate-ron into domains/validate/. Standard port acceptance as defined on T-1281: transport-agnostic service, logic-free router, every command decorated, a parity test per verb against the old script, old scripts retired once parity passes, and that domain''s tooling make targets retired rather than wrapped. Note validate-content is wired into the PRE-COMMIT hook (via make validate-content) as well as being a make target, so the hook is part of this port''s blast radius, not just the Makefile — and a pre-commit failure is felt on every commit rather than every push, so getting the exit codes right matters more here than for a push-only gate. check-fact-ids is ALSO in the pre-commit hook but belongs to the check domain (T-1281); coordinate so the hook is edited once rather than twice.', 'in_progress', 'medium', NULL, NULL, 'D-263', '2026-08-31 15:30:42.490', '2026-09-01 15:22:13.360', NULL, 'd8cc608ee1be87634289397ebae7fd61', 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 ('06G5GWDPQ95RSCKK51K8DTYRCM', 'task', '06G1S3D0M1TQW0GMFBBPQZG3ZM', 'Port the validate domain — content, checklist, ron', 'validate-content, validate-checklist and validate-ron into domains/validate/. Standard port acceptance as defined on T-1281: transport-agnostic service, logic-free router, every command decorated, a parity test per verb against the old script, old scripts retired once parity passes, and that domain''s tooling make targets retired rather than wrapped. Note validate-content is wired into the PRE-COMMIT hook (via make validate-content) as well as being a make target, so the hook is part of this port''s blast radius, not just the Makefile — and a pre-commit failure is felt on every commit rather than every push, so getting the exit codes right matters more here than for a push-only gate. check-fact-ids is ALSO in the pre-commit hook but belongs to the check domain (T-1281); coordinate so the hook is edited once rather than twice.', 'in_progress', 'medium', NULL, NULL, 'D-263', '2026-08-31 15:30:42.490', '2026-09-01 15:22:27.856', NULL, '89c48e98147081cb5465e7373234ec9f', 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 f0ac24d4f..864d512bd 100644 --- a/Makefile +++ b/Makefile @@ -3,12 +3,12 @@ GODOT := $(shell command -v godot4 2>/dev/null || command -v godot 2>/dev/null) .PHONY: help setup build client server game atlas stop test test-tooling lint lint-python setup-venv ci ci-client ci-server clean \ install-reach reach-repoint \ decisions-sync decisions-active decisions-validate \ - validate-content setup-hooks install-hooks \ + setup-hooks install-hooks \ audit deny atlas-verify economy-db regen-db \ pre-pr pre-pr-lint pre-pr-build pre-pr-test pre-pr-validate pre-pr-fixtures \ pre-pr-server pre-pr-client pre-pr-content \ fixtures-client fixtures-gauntlet golden-diff golden-update \ - checklist-validate checklist-generate check-star-map star-map-data \ + check-star-map star-map-data \ build-sr-voice run-sr-voice test-voice-mock test-voice-real \ perf-baseline debug-schedule \ test-ipc-fixtures test-ipc-protocol test-ipc-integration test-ipc-benchmark \ @@ -55,7 +55,6 @@ help: @echo " make decisions-validate Validate decision records — malformed-record gate (pql)" @echo " make audit Run cargo audit (security advisory check)" @echo " make deny Run cargo deny check (license/ban policy)" - @echo " make validate-content Validate content YAML against schemas" @echo " make atlas-verify Verify atlas proposal JSONs (all in docs/atlas/proposals/)" @echo " make star-map-data Regenerate client/data/star_map_data.json from systems.db + wiki" @echo " make check-star-map Assert star_map_data.json is up to date (part of pre-pr-client)" @@ -65,8 +64,6 @@ help: @echo " make fixtures-client Generate GDScript->Rust cross-encoder fixtures (#475)" @echo " make golden-diff Show diff if golden file output has changed" @echo " make golden-update Regenerate golden file and stage for commit" - @echo " make checklist-validate Validate checklist YAML against schema" - @echo " make checklist-generate Validate checklists + print condition summary" @echo " make perf-baseline Run performance benchmarks and save baseline" @echo "" @echo " make screenshot Ad-hoc visual capture (SCENARIO=name, default: atlas_GJ820Bc_Global)" @@ -315,6 +312,10 @@ test-tooling: @mkdir -p .cache @$(VENV_PY) tooling/test_conformance.py 2> .cache/test-tooling-conformance.log || \ { echo " FAIL: reach conformance — log follows:"; cat .cache/test-tooling-conformance.log; exit 1; } + @echo " [test-tooling] reach validate verbs — behaviour (T-1282)..." + @mkdir -p .cache + @$(VENV_PY) tooling/test_validate.py 2> .cache/test-tooling-validate.log || \ + { echo " FAIL: validate verbs — log follows:"; cat .cache/test-tooling-validate.log; exit 1; } @echo " [test-tooling] reach check gates — behaviour (T-1281)..." @mkdir -p .cache @$(VENV_PY) tooling/test_check.py 2> .cache/test-tooling-check.log || \ @@ -378,7 +379,7 @@ pre-pr-build: build-server build-client pre-pr-test: test-server test-client @echo "--- Tests: PASS ---" -pre-pr-validate: validate-content check-star-map +pre-pr-validate: check-star-map @echo "--- Content validation: PASS ---" pre-pr-fixtures: @@ -423,7 +424,7 @@ pre-pr-server: lint-server build-server test-server pre-pr-fixtures audit deny pre-pr-client: lint-client build-client test-client check-star-map @echo "=== Client pre-PR: PASSED ===" -pre-pr-content: validate-content checklist-validate atlas-verify +pre-pr-content: atlas-verify @echo "=== Content pre-PR: PASSED ===" # --- CI (run locally) --- @@ -468,6 +469,9 @@ regen-db: ## Regenerate systems.db from all sources and stamp meta table (#855, # make check-canvas-version -> reach check canvas-version # make check-fact-ids -> reach check fact-ids # make check-dataflow-graph -> reach check dataflow-graph +# make validate-content -> reach validate content +# make checklist-validate -> reach validate checklist --check +# make checklist-generate -> reach validate checklist # # Wrapping them would leave two ways to invoke each, and `reach --help` would # stop being the answer to "what tooling exists" because the Makefile would @@ -501,9 +505,6 @@ decisions-validate: # --- Content Validation --- -validate-content: - @tooling/validate-content - atlas-verify: @tooling/atlas-verify docs/atlas/proposals/*.json @@ -513,12 +514,6 @@ audit: deny: cd server && cargo deny check -checklist-validate: - @tooling/validate-checklist --check - -checklist-generate: - @tooling/validate-checklist - check-star-map: @python3 tooling/generate-star-map-data.py --check diff --git a/docs/DEVOPS.md b/docs/DEVOPS.md index f5b64b07b..20b1c5f16 100644 --- a/docs/DEVOPS.md +++ b/docs/DEVOPS.md @@ -238,7 +238,7 @@ make clean # Remove build artifacts and .cache/ contents ### Content Validation ```bash -make validate-content # Validate content YAML against JSON schemas +reach validate content # Validate content YAML against JSON schemas make check-fact-ids # Check fact_id references against knowledge catalogs ``` diff --git a/tooling/domains/validate/__init__.py b/tooling/domains/validate/__init__.py new file mode 100644 index 000000000..68e148ef0 --- /dev/null +++ b/tooling/domains/validate/__init__.py @@ -0,0 +1,6 @@ +"""The `validate` domain — content, checklists and RON against their schemas. + +Three validators with three shapes: `content` and `checklist` were Python and +moved; `ron` was bash wrapping a Python heredoc wrapping a Rust binary, so its +logic became Python and its binary call became a guarded exec (D-263). +""" diff --git a/tooling/validate-checklist b/tooling/domains/validate/checklist.py similarity index 62% rename from tooling/validate-checklist rename to tooling/domains/validate/checklist.py index c7b30579b..792bd45b7 100755 --- a/tooling/validate-checklist +++ b/tooling/domains/validate/checklist.py @@ -9,22 +9,37 @@ Exit code 0 = all valid, 1 = validation errors found. """ import json -import sys from pathlib import Path +from tooling.core import config, console +from tooling.core.errors import ReachError + import jsonschema import yaml -ROOT = Path(__file__).resolve().parent.parent +# config.repo_root(), NOT __file__-relative. The original computed +# Path(__file__).parent.parent, which meant the repo root while this file lived +# at tooling/validate-checklist and means tooling/domains now. Moving the file +# silently repointed both paths at nothing, the gauntlet directory "did not +# exist", and the validator reported success having checked zero files — the +# exact shape of failure this whole initiative keeps finding. +ROOT = config.repo_root() SCHEMA_PATH = ROOT / "server" / "content" / "_schema" / "checklist.schema.json" GAUNTLET_DIR = ROOT / "server" / "content" / "gauntlet" + + + def load_schema(): if not SCHEMA_PATH.exists(): - print(f"ERROR: Schema file not found at {_rel(SCHEMA_PATH)}") - print("Expected: content/_schema/checklist.schema.json") - sys.exit(1) + # ReachError, not sys.exit: a service must not decide to end the + # process, and the caller gets a remedy rather than a bare 1. + raise ReachError( + f"checklist schema not found at {_rel(SCHEMA_PATH)}", + fix="expected server/content/_schema/checklist.schema.json — " + "restore it or correct the path", + ) with open(SCHEMA_PATH) as f: return json.load(f) @@ -59,12 +74,12 @@ def validate_schema(files, schema): with open(path) as f: data = yaml.safe_load(f) except yaml.YAMLError as e: - print(f"YAML ERROR: {rel}: {e}") + console.event(f"YAML ERROR: {rel}: {e}") errors += 1 continue if data is None: - print(f"EMPTY: {rel}") + console.event(f"EMPTY: {rel}") errors += 1 continue @@ -72,10 +87,10 @@ def validate_schema(files, schema): jsonschema.validate(instance=data, schema=schema) validated += 1 except jsonschema.ValidationError as e: - print(f"INVALID: {rel}") - print(f" Error: {e.message}") + console.event(f"INVALID: {rel}") + console.event(f" Error: {e.message}") if e.absolute_path: - print(f" Path: {'.'.join(str(p) for p in e.absolute_path)}") + console.event(f" Path: {'.'.join(str(p) for p in e.absolute_path)}") errors += 1 return validated, errors @@ -102,14 +117,14 @@ def check_id_uniqueness(files): if not cid: continue if cid in local_seen: - print(f'ID ERROR: duplicate condition id "{cid}" in {rel}') + console.event(f'ID ERROR: duplicate condition id "{cid}" in {rel}') errors += 1 local_seen.add(cid) if cid in global_ids and global_ids[cid] != path: - print(f'ID ERROR: condition id "{cid}" used in multiple files') - print(f" First: {_rel(global_ids[cid])}") - print(f" Also: {rel}") + console.event(f'ID ERROR: condition id "{cid}" used in multiple files') + console.event(f" First: {_rel(global_ids[cid])}") + console.event(f" Also: {rel}") errors += 1 elif cid not in global_ids: global_ids[cid] = path @@ -135,60 +150,57 @@ def summarize(files): count = len(conditions) total += count room = data.get("room_id", "cross_room") - print(f" {room}: {count} conditions") + console.event(f" {room}: {count} conditions") for cond in conditions: ct = cond.get("condition_type", "unknown") type_counts[ct] = type_counts.get(ct, 0) + 1 - print(f"\n Total: {total} conditions across {len(files)} files") + console.event(f"\n Total: {total} conditions across {len(files)} files") if type_counts: - print(" By type:") + console.event(" By type:") for ct in sorted(type_counts): - print(f" {ct}: {type_counts[ct]}") + console.event(f" {ct}: {type_counts[ct]}") -def main(): - check_only = "--check" in sys.argv +def validate(check_only: bool = False) -> int: if not GAUNTLET_DIR.exists(): - print(f"No gauntlet directory at {_rel(GAUNTLET_DIR)}") - print("Checklist validation skipped (no content yet).") + console.event(f"No gauntlet directory at {_rel(GAUNTLET_DIR)}") + console.event("Checklist validation skipped (no content yet).") return 0 files = find_checklists() if not files: - print("No checklist files found under content/gauntlet/.") - print("Checklist validation skipped.") + console.event("No checklist files found under content/gauntlet/.") + console.event("Checklist validation skipped.") return 0 schema = load_schema() # Pass 1: Schema validation validated, schema_errors = validate_schema(files, schema) - print(f"Pass 1 (schema): {validated} valid, {schema_errors} errors") + console.event(f"Pass 1 (schema): {validated} valid, {schema_errors} errors") if schema_errors > 0: - print(f"\nSchema validation failed ({schema_errors} errors) — skipping ID checks") + console.event(f"\nSchema validation failed ({schema_errors} errors) — skipping ID checks") return 1 # Pass 2: Condition ID uniqueness id_errors = check_id_uniqueness(files) if id_errors > 0: - print(f"Pass 2 (IDs): {id_errors} errors") + console.event(f"Pass 2 (IDs): {id_errors} errors") total_errors = schema_errors + id_errors - print(f"\nChecklist validation: {validated} valid, {total_errors} errors") + console.event(f"\nChecklist validation: {validated} valid, {total_errors} errors") if total_errors > 0: return 1 if not check_only: - print("\nChecklist summary:") + console.event("\nChecklist summary:") summarize(files) return 0 -if __name__ == "__main__": - sys.exit(main()) diff --git a/tooling/validate-content b/tooling/domains/validate/content.py similarity index 81% rename from tooling/validate-content rename to tooling/domains/validate/content.py index cf853a6f3..4d58583c3 100755 --- a/tooling/validate-content +++ b/tooling/domains/validate/content.py @@ -23,13 +23,17 @@ Exit code 0 = all valid, 1 = validation errors found. import json import re -import sys from pathlib import Path import jsonschema import yaml -CONTENT_DIR = Path(__file__).resolve().parent.parent / "server" / "content" +from tooling.core import config, console + + + + +CONTENT_DIR = config.path("server", "content") SCHEMA_DIR = CONTENT_DIR / "_schema" # Map directory parent name (or filename) to schema file @@ -222,9 +226,9 @@ class ContentIndex: if not cid: continue if cid in seen: - print(f'XREF ERROR: duplicate canonical_id "{cid}"') - print(f" Defined in: {_rel(seen[cid])}") - print(f" Duplicate in: {_rel(path)}") + console.event(f'XREF ERROR: duplicate canonical_id "{cid}"') + console.event(f" Defined in: {_rel(seen[cid])}") + console.event(f" Duplicate in: {_rel(path)}") errors += 1 else: seen[cid] = path @@ -243,9 +247,9 @@ class ContentIndex: target = rel.get("target") if target and target not in self.npcs: known = sorted(self.npcs.keys()) - print(f'XREF ERROR: unresolved relationship target "{target}"') - print(f" In: {_rel(path)}") - print(f" Known canonical_ids: {', '.join(known[:10])}" + console.event(f'XREF ERROR: unresolved relationship target "{target}"') + console.event(f" In: {_rel(path)}") + console.event(f" Known canonical_ids: {', '.join(known[:10])}" + (f" (and {len(known) - 10} more)" if len(known) > 10 else "")) errors += 1 return errors @@ -261,9 +265,9 @@ class ContentIndex: for slug in locs: expected = locations_dir / f"{slug}.yaml" if not expected.exists(): - print(f'XREF ERROR: location slug "{slug}" not found') - print(f" In: {_rel(path)}") - print(f" Expected file: {_rel(expected)}") + console.event(f'XREF ERROR: location slug "{slug}" not found') + console.event(f" In: {_rel(path)}") + console.event(f" Expected file: {_rel(expected)}") errors += 1 return errors @@ -280,21 +284,21 @@ class ContentIndex: district_key = str(district_dir) district_locs = self.locations.get(district_key, set()) if not district_locs: - print(f'XREF ERROR: dialogue location "{location}" references district with no locations declared') - print(f" In: {_rel(path)}") - print(f" District: {_rel(district_dir / 'district.yaml')}") + console.event(f'XREF ERROR: dialogue location "{location}" references district with no locations declared') + console.event(f" In: {_rel(path)}") + console.event(f" District: {_rel(district_dir / 'district.yaml')}") errors += 1 elif location not in district_locs: - print(f'XREF ERROR: dialogue location "{location}" not in district') - print(f" In: {_rel(path)}") - print(f" District locations: {', '.join(sorted(district_locs))}") + console.event(f'XREF ERROR: dialogue location "{location}" not in district') + console.event(f" In: {_rel(path)}") + console.event(f" District locations: {', '.join(sorted(district_locs))}") errors += 1 return errors def _check_5_fact_ids(self) -> int: """Check that all referenced fact_ids resolve to knowledge catalogs.""" if not self.fact_ids: - print(" Check 5 (fact_ids): SKIPPED — no canonical fact_ids in knowledge catalogs yet") + console.event(" Check 5 (fact_ids): SKIPPED — no canonical fact_ids in knowledge catalogs yet") return 0 errors = 0 @@ -303,12 +307,12 @@ class ContentIndex: for fact_id, sources in sorted(refs.items()): if fact_id not in self.fact_ids: - print(f'XREF ERROR: unknown fact_id "{fact_id}"') + console.event(f'XREF ERROR: unknown fact_id "{fact_id}"') for src in sources[:3]: - print(f" In: {_rel(src)}") + console.event(f" In: {_rel(src)}") if len(sources) > 3: - print(f" ...and {len(sources) - 3} more files") - print(f" Canonical fact_ids: {len(self.fact_ids)} defined") + console.event(f" ...and {len(sources) - 3} more files") + console.event(f" Canonical fact_ids: {len(self.fact_ids)} defined") errors += 1 return errors @@ -380,12 +384,12 @@ class ContentIndex: if not isinstance(slug, str): continue if slug not in available: - print(f'XREF ERROR: triangle "{slug}" not found') - print(f" In: {_rel(path)}") + console.event(f'XREF ERROR: triangle "{slug}" not found') + console.event(f" In: {_rel(path)}") if available: - print(f" Available triangles: {', '.join(sorted(available))}") + console.event(f" Available triangles: {', '.join(sorted(available))}") else: - print(f" No triangles found in {_rel(district_dir / 'triangles')}") + console.event(f" No triangles found in {_rel(district_dir / 'triangles')}") errors += 1 return errors @@ -401,10 +405,10 @@ class ContentIndex: continue actual = len(list(npcs_dir.glob("*.yaml"))) if declared != actual: - print(f"XREF WARNING: npc_count mismatch") - print(f" Declared: {declared}") - print(f" Actual NPC files: {actual}") - print(f" In: {_rel(path)}") + console.event("XREF WARNING: npc_count mismatch") + console.event(f" Declared: {declared}") + console.event(f" Actual NPC files: {actual}") + console.event(f" In: {_rel(path)}") warnings += 1 return warnings @@ -426,18 +430,18 @@ class ContentIndex: continue # Per-file duplicate if line_id in local_seen: - print(f'XREF ERROR: duplicate dialogue line id "{line_id}"') - print(f" In: {_rel(path)}") - print(f" First: line {local_seen[line_id]}") - print(f" Duplicate: line {i}") + console.event(f'XREF ERROR: duplicate dialogue line id "{line_id}"') + console.event(f" In: {_rel(path)}") + console.event(f" First: line {local_seen[line_id]}") + console.event(f" Duplicate: line {i}") errors += 1 else: local_seen[line_id] = i # Cross-file duplicate if line_id in global_seen and global_seen[line_id] != path: - print(f'XREF ERROR: dialogue line id "{line_id}" used in multiple files') - print(f" First: {_rel(global_seen[line_id])}") - print(f" Also in: {_rel(path)}") + console.event(f'XREF ERROR: dialogue line id "{line_id}" used in multiple files') + console.event(f" First: {_rel(global_seen[line_id])}") + console.event(f" Also in: {_rel(path)}") errors += 1 elif line_id not in global_seen: global_seen[line_id] = path @@ -473,7 +477,7 @@ class ContentIndex: continue checked.add(pair) if target in self.npcs and cid not in npc_rels.get(target, set()): - print(f"XREF WARNING: {cid} has relationship to {target} but {target} has no reciprocal entry") + console.event(f"XREF WARNING: {cid} has relationship to {target} but {target} has no reciprocal entry") warnings += 1 return warnings @@ -487,7 +491,7 @@ def schema_validate(campaigns_dir: Path) -> tuple[int, int, int]: yaml_files = sorted(campaigns_dir.rglob("*.yaml")) if not yaml_files: - print("No YAML files found under campaigns/", file=sys.stderr) + console.event("No YAML files found under campaigns/") return 0, 0, 1 for yaml_path in yaml_files: @@ -497,7 +501,7 @@ def schema_validate(campaigns_dir: Path) -> tuple[int, int, int]: continue if not schema_path.exists(): - print(f"MISSING SCHEMA: {schema_path.name} for {yaml_path.relative_to(CONTENT_DIR)}") + console.event(f"MISSING SCHEMA: {schema_path.name} for {yaml_path.relative_to(CONTENT_DIR)}") errors += 1 continue @@ -511,7 +515,7 @@ def schema_validate(campaigns_dir: Path) -> tuple[int, int, int]: with open(yaml_path) as f: data = yaml.safe_load(f) except yaml.YAMLError as e: - print(f"YAML ERROR: {yaml_path.relative_to(CONTENT_DIR)}: {e}") + console.event(f"YAML ERROR: {yaml_path.relative_to(CONTENT_DIR)}: {e}") errors += 1 continue @@ -524,40 +528,43 @@ def schema_validate(campaigns_dir: Path) -> tuple[int, int, int]: validated += 1 except jsonschema.ValidationError as e: rel = yaml_path.relative_to(CONTENT_DIR) - print(f"INVALID: {rel}") - print(f" Schema: {schema_path.name}") - print(f" Error: {e.message}") + console.event(f"INVALID: {rel}") + console.event(f" Schema: {schema_path.name}") + console.event(f" Error: {e.message}") if e.absolute_path: - print(f" Path: {'.'.join(str(p) for p in e.absolute_path)}") + console.event(f" Path: {'.'.join(str(p) for p in e.absolute_path)}") errors += 1 return validated, skipped, errors -def main() -> int: +def validate() -> int: + """Run both passes. Returns the exit code. + + Replaces the old main(). Same logic, same messages, in the same order — but + they go out as EVENTS through core/console rather than through print, so + they stream as the validation runs, carry the invocation's job id, and are + rendered by the sink. A service does not decide to print (D-263). + """ campaigns_dir = CONTENT_DIR / "campaigns" if not campaigns_dir.exists(): - print(f"No campaigns directory at {campaigns_dir}", file=sys.stderr) + console.event(f"No campaigns directory at {campaigns_dir}") return 1 # Pass 1: Schema validation validated, skipped, schema_errors = schema_validate(campaigns_dir) - print(f"\nPass 1 (schema): {validated} validated, {skipped} skipped, {schema_errors} errors") + console.event(f"\nPass 1 (schema): {validated} validated, {skipped} skipped, {schema_errors} errors") if schema_errors > 0: - print(f"\nSchema validation failed ({schema_errors} errors) -- skipping cross-references") + console.event(f"\nSchema validation failed ({schema_errors} errors) -- skipping cross-references") return 1 # Pass 2: Cross-reference validation - print("\nPass 2 (cross-references):") + console.event("\nPass 2 (cross-references):") index = ContentIndex(campaigns_dir) index.build() xref_errors, xref_warnings = index.validate_references() total_errors = schema_errors + xref_errors - print(f"\nValidated {validated} files: {total_errors} errors, {xref_warnings} warnings") - return 1 if total_errors > 0 else 0 - - -if __name__ == "__main__": - sys.exit(main()) + console.event(f"\nValidated {validated} files: {total_errors} errors, {xref_warnings} warnings") + return (1 if total_errors > 0 else 0) diff --git a/tooling/domains/validate/ron.py b/tooling/domains/validate/ron.py new file mode 100644 index 000000000..d436d31c6 --- /dev/null +++ b/tooling/domains/validate/ron.py @@ -0,0 +1,112 @@ +"""RON validation — ported from the bash `validate-ron` (D-263). + +The old script was three languages deep: bash dispatching on a flag, a Python +heredoc doing collision detection, and `cargo run` for schema validation. The +heredoc is the reason this is a rewrite rather than a move — logic embedded in +a shell string cannot be imported, cannot be tested, and cannot be read by +anything that indexes Python. + +What stayed shell-shaped is the one genuine OS interaction: running the Rust +validator. That goes through `core.process.run`, which is the sanctioned exec. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +from tooling.core import config, console, process +from tooling.core.errors import ReachError + +SCHEMAS = ("zone", "zone_type", "culture") + + +def name_collisions(directory: Path) -> int: + """Report names shared between culture name pools. Returns an exit code. + + A name appearing in two cultures' pools makes generated NPCs ambiguous + about where they are from, which is invisible until someone notices two + cultures producing the same surnames. + """ + if not directory.is_dir(): + raise ReachError( + f"directory not found: {directory}", + fix="pass a directory containing culture-*.ron files", + ) + + files = sorted(directory.glob("culture-*.ron")) + if not files: + console.event(f"No culture-*.ron files found in: {directory}") + return 0 + + given: dict[str, set[str]] = {} + family: dict[str, set[str]] = {} + for path in files: + text = path.read_text(encoding="utf-8") + match = re.search(r'\bid\s*:\s*"([^"]+)"', text) + culture = match.group(1) if match else path.name + given[culture] = set(_names(text, "given_names")) + family[culture] = set(_names(text, "family_names")) + + collisions = False + for field, pools in (("given_names", given), ("family_names", family)): + for name, cultures in sorted(_shared(pools).items()): + collisions = True + console.event( + f'COLLISION {field}: "{name}" in {", ".join(sorted(cultures))}', + level="error", + ) + + if collisions: + return 1 + + console.verdict( + f"OK: no name collisions across {len(given)} culture(s): " + f"{', '.join(sorted(given))}" + ) + return 0 + + +def ron_file(path: Path, schema: str) -> int: + """Validate one .ron file against a Rust struct schema.""" + if schema not in SCHEMAS: + raise ReachError( + f"unknown schema {schema!r}", + fix=f"choose one of: {', '.join(SCHEMAS)}", + exit_code=2, + ) + if not path.is_file(): + # Checked before resolving, because realpath on a missing file gives an + # error about the path rather than about the file — the old script made + # the same distinction and it is worth keeping. + raise ReachError( + f"file not found: {path}", + fix="check the path, or pass a directory to `reach validate name-collisions`", + ) + + result = process.run( + ["cargo", "run", "--quiet", "--bin", "validate_ron", "--", str(path.resolve()), schema], + cwd=config.path("server"), + check=False, + capture=False, + missing_fix="install Rust — make setup-rust", + ) + return result.returncode + + +def _names(text: str, field: str) -> list[str]: + """Quoted strings from a named RON array field, comments stripped.""" + match = re.search(rf"\b{re.escape(field)}\s*:\s*\[([^\]]*)\]", text, re.DOTALL) + if not match: + return [] + block = re.sub(r"//[^\n]*", "", match.group(1)) + return re.findall(r'"([^"]+)"', block) + + +def _shared(pools: dict[str, set[str]]) -> dict[str, list[str]]: + """name -> the cultures claiming it, for names claimed more than once.""" + owners: dict[str, list[str]] = {} + for culture, names in pools.items(): + for name in names: + owners.setdefault(name, []).append(culture) + return {name: cultures for name, cultures in owners.items() if len(cultures) > 1} diff --git a/tooling/domains/validate/router.py b/tooling/domains/validate/router.py new file mode 100644 index 000000000..fa43b738d --- /dev/null +++ b/tooling/domains/validate/router.py @@ -0,0 +1,100 @@ +"""Transport for the `validate` domain — args in, delegate, format out. + +Zero logic. Note what these commands do NOT do: collect output. The validators +emit their findings as events through `core/console` as they run, so a long +content validation streams rather than going quiet and dumping at the end. The +router's job is the verdict and the exit code. +""" + +from __future__ import annotations + +from pathlib import Path + +import typer + +from tooling.core import cli, console +from tooling.core.command import command +from tooling.core.errors import ReachError +from tooling.domains.validate import checklist as checklist_module +from tooling.domains.validate import content as content_module +from tooling.domains.validate import ron as ron_module + +app = cli.domain("validate", "Content, checklists and RON against their schemas.") + + +@app.callback() +def _domain() -> None: + """Keeps `validate` a group (Typer collapses a single-command app).""" + + +@app.command("content") +@command +def content() -> None: + """Validate content YAML against JSON schemas, then cross-references.""" + code = content_module.validate() + if code != 0: + raise ReachError( + "validate-content: validation failed", + fix="the errors above name each file and what is wrong with it; " + "schemas live in server/content/_schema/", + exit_code=code, + ) + console.verdict("validate-content: OK") + + +@app.command("checklist") +@command +def checklist( + check: bool = typer.Option( + False, "--check", help="Schema validation only, for the pre-PR chain." + ), +) -> None: + """Validate checklist YAML against its schema, and ids for uniqueness.""" + code = checklist_module.validate(check_only=check) + if code != 0: + raise ReachError( + "validate-checklist: validation failed", + fix="the errors above name each checklist and the field at fault", + exit_code=code, + ) + console.verdict("validate-checklist: OK") + + +@app.command("ron") +@command +def ron( + path: Path = typer.Argument(..., help="The .ron file to validate."), + schema: str = typer.Argument(..., help=f"One of: {', '.join(ron_module.SCHEMAS)}"), +) -> None: + """Validate a RON file against its Rust struct schema.""" + code = ron_module.ron_file(path, schema) + if code != 0: + raise ReachError( + f"validate-ron: {path} does not match the {schema} schema", + fix="the validator's output above names the field; the struct is in " + "server/src/ — compare field names and types", + exit_code=code, + ) + console.verdict(f"validate-ron: OK — {path} matches {schema}") + + +@app.command("name-collisions") +@command +def name_collisions( + directory: Path = typer.Argument(..., help="Directory holding culture-*.ron files."), +) -> None: + """Report names shared between two cultures' name pools. + + A separate verb rather than a flag on `ron`, because it answers a different + question: `ron` asks whether ONE file is well-formed, this asks whether the + SET of them is coherent. The old script fused them behind + --check-name-collisions and had to branch on it before doing anything. + """ + code = ron_module.name_collisions(directory) + if code != 0: + raise ReachError( + "validate-ron: name pools collide across cultures", + fix="rename the colliding entries so each name belongs to one culture — " + "a shared name makes a generated NPC's origin ambiguous", + exit_code=code, + ) diff --git a/tooling/main.py b/tooling/main.py index 26beb574e..dbbf123a7 100644 --- a/tooling/main.py +++ b/tooling/main.py @@ -51,6 +51,10 @@ DOMAINS: dict[str, tuple[str, str]] = { "tooling.domains.check.router:app", "Consistency gates — the checks the push hook runs", ), + "validate": ( + "tooling.domains.validate.router:app", + "Content, checklists and RON against their schemas", + ), "jobs": ( "tooling.domains.jobs.router:app", "Detached runs — status, logs and outcomes", diff --git a/tooling/test_validate.py b/tooling/test_validate.py new file mode 100644 index 000000000..b2fb7da50 --- /dev/null +++ b/tooling/test_validate.py @@ -0,0 +1,180 @@ +#!/usr/bin/env python3 +"""Behaviour of the `reach validate` verbs (T-1282). + +Parity against the scripts these replaced was established on the live tree — +`content` reproduces the original's output byte for byte including its counts, +`name-collisions` likewise — and recorded in the ticket. What is pinned here is +the behaviour those runs could not reach: the failure paths. + +That gap matters more than usual for this domain. `validate-content` currently +FAILS on the live tree (13 missing schemas, pre-existing), so its success path +is the one nothing exercises; `name-collisions` currently PASSES, so its +detection path is the one nothing exercises. A live run proves whichever half +the repo happens to be in. + +Run: python3 tooling/test_validate.py +""" + +import os +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +CULTURE = '''( + id: "{cid}", + naming: ( + given_names: [ + // a comment mentioning "decoy" which must not be collected + {given} + ], + family_names: [ + {family} + ], + ), +) +''' + + +def _reach(*args: str, root: Path | None = None) -> subprocess.CompletedProcess[str]: + env = {**os.environ, "SR_OUTPUT_FORMAT": "text"} + if root is not None: + env["SR_REPO_ROOT"] = str(root) + return subprocess.run( + ["reach", "validate", *args], + capture_output=True, + text=True, + cwd=REPO_ROOT, + env=env, + ) + + +def _cultures(directory: Path, pools: dict[str, tuple[list[str], list[str]]]) -> None: + directory.mkdir(parents=True, exist_ok=True) + for cid, (given, family) in pools.items(): + (directory / f"culture-{cid}.ron").write_text( + CULTURE.format( + cid=cid, + given=", ".join(f'"{n}"' for n in given), + family=", ".join(f'"{n}"' for n in family), + ), + encoding="utf-8", + ) + + +def test_collision_detected(failures: list[str]) -> None: + """Two cultures sharing a name is a failure that names both.""" + with tempfile.TemporaryDirectory() as tmp: + directory = Path(tmp) / "global" + _cultures( + directory, + { + "alpha": (["Ada", "Shared"], ["Alpha"]), + "beta": (["Bo", "Shared"], ["Beta"]), + }, + ) + result = _reach("name-collisions", str(directory)) + combined = result.stdout + result.stderr + + if result.returncode == 0: + failures.append( + "[collision] a shared name exited 0 — a collision that reports " + "success makes generated NPCs ambiguous about their origin, " + "invisibly" + ) + if "Shared" not in combined: + failures.append("[collision] the colliding name is not in the output") + for culture in ("alpha", "beta"): + if culture not in combined: + failures.append( + f"[collision] {culture} is not named — a collision report that " + "omits an owner cannot be acted on" + ) + if "decoy" in combined: + failures.append( + "[collision] a name inside a // comment was collected; comments " + "must be stripped before extracting quoted strings" + ) + + +def test_no_collision(failures: list[str]) -> None: + with tempfile.TemporaryDirectory() as tmp: + directory = Path(tmp) / "global" + _cultures(directory, {"alpha": (["Ada"], ["Alpha"]), "beta": (["Bo"], ["Beta"])}) + result = _reach("name-collisions", str(directory)) + if result.returncode != 0: + failures.append( + f"[no-collision] exited {result.returncode} with disjoint pools" + ) + + +def test_empty_directory(failures: list[str]) -> None: + """No culture files is not a failure — there is nothing to contradict.""" + with tempfile.TemporaryDirectory() as tmp: + directory = Path(tmp) / "global" + directory.mkdir(parents=True) + result = _reach("name-collisions", str(directory)) + if result.returncode != 0: + failures.append( + f"[empty] exited {result.returncode} on a directory with no cultures" + ) + + +def test_missing_directory(failures: list[str]) -> None: + result = _reach("name-collisions", "/definitely/not/a/directory") + if result.returncode == 0: + failures.append("[missing-dir] a nonexistent directory exited 0") + + +def test_ron_argument_errors(failures: list[str]) -> None: + """Bad arguments fail before anything is executed. + + Both cases matter because the alternative is invoking cargo to discover + them, which is slow and reports the mistake in the validator's vocabulary + rather than the caller's. + """ + unknown = _reach("ron", "server/content/global/culture-osse.ron", "not_a_schema") + if unknown.returncode != 2: + failures.append( + f"[ron-schema] unknown schema exited {unknown.returncode}, expected 2" + ) + if "zone_type" not in (unknown.stdout + unknown.stderr): + failures.append( + "[ron-schema] the rejection does not list the accepted schemas — the " + "closed-set rule D-263 exists for" + ) + + missing = _reach("ron", "/definitely/not/a/file.ron", "culture") + if missing.returncode == 0: + failures.append("[ron-file] a nonexistent file exited 0") + + +def main() -> int: + if shutil.which("reach") is None: + print( + "test_validate: `reach` is not on PATH.\n Fix: make install-reach", + file=sys.stderr, + ) + return 1 + + failures: list[str] = [] + test_collision_detected(failures) + test_no_collision(failures) + test_empty_directory(failures) + test_missing_directory(failures) + test_ron_argument_errors(failures) + + if failures: + print("test_validate: FAIL", file=sys.stderr) + for failure in failures: + print(f" - {failure}", file=sys.stderr) + return 1 + print("test_validate: OK — collisions detected and named, argument errors caught") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tooling/validate-ron b/tooling/validate-ron deleted file mode 100755 index fa00ea257..000000000 --- a/tooling/validate-ron +++ /dev/null @@ -1,137 +0,0 @@ -#!/usr/bin/env bash -# RON content validator — wrapper for the Rust validate_ron binary (#611). -# -# Usage: -# tooling/validate-ron -# tooling/validate-ron --check-name-collisions -# -# Examples: -# tooling/validate-ron server/content/global/zone-types/rural_agricultural.ron zone_type -# tooling/validate-ron server/content/global/culture-van-maanens-star.example.ron culture -# tooling/validate-ron server/content/global/zone-identity-spec.example.ron zone -# tooling/validate-ron --check-name-collisions server/content/global/ -# -# --check-name-collisions: -# Scans all culture-*.ron files in the given directory. Extracts naming.given_names -# and naming.family_names pools from each file. Reports any name that appears in more -# than one culture's pool. Exits 1 on collision; exits 0 if no collisions found. -# Output is machine-readable (one collision per line). - -set -euo pipefail - -# --- Name collision mode -------------------------------------------------- -if [ "${1:-}" = "--check-name-collisions" ]; then - if [ $# -lt 2 ]; then - echo "Usage: tooling/validate-ron --check-name-collisions " - exit 1 - fi - SCAN_DIR="$2" - if [ ! -d "$SCAN_DIR" ]; then - echo "Error: directory not found: $SCAN_DIR" - exit 1 - fi - python3 - "$SCAN_DIR" <<'PYEOF' -import sys -import re -import os -import glob - -scan_dir = sys.argv[1] -pattern = os.path.join(scan_dir, "culture-*.ron") -files = sorted(glob.glob(pattern)) - -if not files: - print(f"No culture-*.ron files found in: {scan_dir}") - sys.exit(0) - -def extract_names_from_block(text, field): - """Extract quoted string list from a named RON array field.""" - # Match: field: [\n "name", "name", ...\n] - # Handles multi-line arrays with comments inside. - pat = re.compile( - r'\b' + re.escape(field) + r'\s*:\s*\[([^\]]*)\]', - re.DOTALL - ) - m = pat.search(text) - if not m: - return [] - block = m.group(1) - # Strip line comments before extracting quoted strings - block = re.sub(r'//[^\n]*', '', block) - return re.findall(r'"([^"]+)"', block) - -# culture_id -> set of names (given + family, tracked separately for reporting) -given_by_culture = {} -family_by_culture = {} - -for fpath in files: - with open(fpath, 'r', encoding='utf-8') as f: - text = f.read() - # Extract culture id from the id: "..." field - id_m = re.search(r'\bid\s*:\s*"([^"]+)"', text) - culture_id = id_m.group(1) if id_m else os.path.basename(fpath) - - given_by_culture[culture_id] = set(extract_names_from_block(text, 'given_names')) - family_by_culture[culture_id] = set(extract_names_from_block(text, 'family_names')) - -cultures = list(given_by_culture.keys()) -collisions_found = False - -# Check given_names collisions -all_given_names = {} # name -> list of cultures -for culture, names in given_by_culture.items(): - for name in names: - all_given_names.setdefault(name, []).append(culture) - -for name, cultures_with_name in sorted(all_given_names.items()): - if len(cultures_with_name) > 1: - collisions_found = True - print(f"COLLISION given_names: \"{name}\" in {', '.join(sorted(cultures_with_name))}") - -# Check family_names collisions -all_family_names = {} # name -> list of cultures -for culture, names in family_by_culture.items(): - for name in names: - all_family_names.setdefault(name, []).append(culture) - -for name, cultures_with_name in sorted(all_family_names.items()): - if len(cultures_with_name) > 1: - collisions_found = True - print(f"COLLISION family_names: \"{name}\" in {', '.join(sorted(cultures_with_name))}") - -if not collisions_found: - print(f"OK: no name collisions across {len(cultures)} culture(s): {', '.join(sorted(cultures))}") - sys.exit(0) -else: - sys.exit(1) -PYEOF - exit $? -fi -# --- End name collision mode ---------------------------------------------- - -if [ $# -lt 2 ]; then - echo "Usage: tooling/validate-ron " - echo " tooling/validate-ron --check-name-collisions " - echo "" - echo "Validates a RON file against the Rust struct schema." - echo "Schema types:" - echo " zone_type — ZoneTypeTemplate (D-142 zone-type template)" - echo " zone — ZoneSpec (legacy zone identity spec)" - echo " culture — CultureProfile (culture profile)" - echo "" - echo "Name collision check:" - echo " --check-name-collisions Scan culture-*.ron files for shared name pool entries" - exit 1 -fi - -# Check file exists before resolving — realpath gives unhelpful errors otherwise -if [ ! -f "$1" ]; then - echo "Error: file not found: $1" - exit 1 -fi - -FILE="$(realpath "$1")" -SCHEMA="$2" - -cd "$(dirname "$0")/../server" -exec cargo run --quiet --bin validate_ron -- "$FILE" "$SCHEMA"