From 48fee8a0b6c783191995c90062805a2a386ea0a9 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sat, 15 Aug 2026 00:06:32 +0200 Subject: [PATCH] feat(config): make the canvas-generation/version pairing a gate, not a habit (T-1242) project.yaml's version is the Atlas disk cache's only invalidation signal, and nothing enforced that changing canvas GENERATION also moved it. It broke five times -- 0.4.2 lake_margin_q, 0.4.3 coast_warp_px, 0.4.4 the extent inversion, 0.4.5 the Global sentinel, 0.4.6 one-course-per-river -- each bumped only after someone noticed a wrong map. The failure is invisible to its author: it needs a warm cache to reproduce, so a cold checkout looks fine. T-1239 is the last one, and it took eight days. tooling/canvas_sources.py is the path registry; tooling/check-canvas-version rejects a push that touches those paths without moving project.yaml's version line. Wired into the pre-push hook, `make check-canvas-version`, and, for the parsing units, `make test-tooling`. Verified against real history rather than a synthetic branch: run over 4e503c356 -- the commit that actually caused T-1239 -- the gate rejects and names the three files. Run over the commits that DID bump (bdea71953, 39f0fd8c5, and T-1239's own fix), it passes. The registry is globbed, not hand-listed. step_canvas.rs imports ten sibling modules and those import more, so a traced closure would be stale within a month, and stale here is silent. It over-includes on purpose: a false positive costs one bump and one round of cache misses, a false negative costs another week of a wrong map -- the ticket's own ruling. Two deliberate calls worth naming. The registry includes ITSELF, which closes the narrowing hole: remove a path and change that same path in one push, and the gate still fires because the registry file is in the set. And there is no override flag -- it would be reached for exactly when someone is certain their change is harmless, which is the reasoning behind all five regressions. Version bumped 0.4.6 -> 0.4.7 with NO canvas-generation change: self-inclusion means adding the registry trips its own rule. Spent rather than special-cased, because the first exception is how a rule like this dies. The units cover the property no branch run can show -- that editing project.yaml's comment block, which quotes old version NUMBERS directly above the field, is not a bump -- plus a registry-coverage test naming the files each of the five known regressions touched, so a future narrowing past them fails loudly. Co-Authored-By: Claude Opus 5 (1M context) --- .config/hooks/pre-push | 18 +++ .pql/changelog/ticket_history/2026-08.sql | 2 + .pql/changelog/tickets/2026-08.sql | 2 + CLAUDE.md | 2 +- Makefile | 10 +- client/project.godot | 2 +- docs/DEVOPS.md | 30 ++++ project.yaml | 10 +- tooling/canvas_sources.py | 158 ++++++++++++++++++++ tooling/check-canvas-version | 164 +++++++++++++++++++++ tooling/test_canvas_version_check.py | 171 ++++++++++++++++++++++ 11 files changed, 565 insertions(+), 4 deletions(-) create mode 100644 tooling/canvas_sources.py create mode 100755 tooling/check-canvas-version create mode 100644 tooling/test_canvas_version_check.py diff --git a/.config/hooks/pre-push b/.config/hooks/pre-push index 49b913119..a43d5d777 100755 --- a/.config/hooks/pre-push +++ b/.config/hooks/pre-push @@ -298,6 +298,24 @@ if [ -f "$REPO_ROOT/tooling/check-client-version" ]; then fi fi +# --- Canvas-generation / version pairing (T-1242) --- +# A change to how a canvas is GENERATED is half a change; the other half is +# bumping project.yaml's version, or every warm Atlas cache keeps serving +# canvases built by code that no longer exists. That pairing broke five times +# (0.4.2 through 0.4.6), every one bumped after the fact, the last costing the +# eight-day T-1239 diagnosis. The path registry is tooling/canvas_sources.py. +# +# Self-check first: the registry fails closed on an empty glob, and a registry +# that cannot load must not be read as "nothing to enforce". +if [ -f "$REPO_ROOT/tooling/check-canvas-version" ]; then + echo "pre-push: checking canvas-generation version pairing..." + if python3 "$REPO_ROOT/tooling/check-canvas-version"; then + : + else + fail_check "canvas generation changed without a project.yaml version bump" + fi +fi + # --- Clerk review (D-221) --- # DISABLED 2026-05-23 (#965) pending rework. Two problems made it net-negative: # 1. Non-exhaustive — a single run reports ~the first contradiction it finds diff --git a/.pql/changelog/ticket_history/2026-08.sql b/.pql/changelog/ticket_history/2026-08.sql index 2c3e5b006..5e0590a98 100644 --- a/.pql/changelog/ticket_history/2026-08.sql +++ b/.pql/changelog/ticket_history/2026-08.sql @@ -171,3 +171,5 @@ FOLLOW-UPS worth their own tickets, not done here: 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 ('06FXF1VDVQDQ8EFGTXX787M90R', 'status', 'in_progress', 'done', NULL, '2026-08-14 21:20:27', '2026-08-14 21:20:27.835', '2026-08-14 21:20:27.835', NULL, '815c19499dbf40e3b7a79f44c09089c2', 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 ('06G0495WRHF8ADK82VR8CH1J8R', 'status', 'backlog', 'in_progress', NULL, '2026-08-14 21:21:35', '2026-08-14 21:21:35.098', '2026-08-14 21:21:35.098', NULL, 'caa82c705ee32a7582705b2dd5d2cf17', 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 ('06G0495WRHF8ADK82VR8CH1J8R', 'status', 'in_progress', 'done', NULL, '2026-08-14 21:35:35', '2026-08-14 21:35:35.624', '2026-08-14 21:35:35.624', NULL, '1de837ed9e43f37c5ec0f2c33778068e', 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 ('06G04975H3S7GRVQXHKYCR7BKR', 'status', 'backlog', 'in_progress', NULL, '2026-08-14 21:41:02', '2026-08-14 21:41:02.688', '2026-08-14 21:41:02.688', NULL, '11035a92e7d4ab5c58e1f3654ba90fa0', 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 ('06G04975H3S7GRVQXHKYCR7BKR', 'status', 'in_progress', 'in_progress', NULL, '2026-08-14 21:41:17', '2026-08-14 21:41:17.977', '2026-08-14 21:41:17.977', NULL, '96b6f8d466e161a1c87cd8cd349fcea2', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-08.sql b/.pql/changelog/tickets/2026-08.sql index 6c987c55c..dc64b7d6e 100644 --- a/.pql/changelog/tickets/2026-08.sql +++ b/.pql/changelog/tickets/2026-08.sql @@ -304,3 +304,5 @@ FOLLOW-UPS worth their own tickets, not done here: project.yaml version must move) rather than a fifth comment.', 'done', 'high', NULL, 'client', 'D-261', '2026-08-06 14:43:24.765', '2026-08-14 21:20:27.835', NULL, 'fa2402988b35d9f0db6ffcf7cf7a81c2', 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 ('06G0495WRHF8ADK82VR8CH1J8R', 'bug', '06FB0TNSRZXCHGS16BFHSSGSV4', 'Atlas disk cache never invalidates in an exported build — current_schema_version() falls back to ''?.?.?''', 'step_canvas_disk_cache.gd::current_schema_version() reads ProjectSettings.globalize_path(''res://'') + ''/../project.yaml''. That resolves to the repo-root file in a dev run (res:// = client/), but an exported build has no project.yaml one level above res://, so the function returns its ''?.?.?'' fallback. Every exported build therefore stamps and compares the SAME sentinel version, which means the schema-version invalidation path — the cache''s only invalidation signal — is inert in a shipped game: a canvas cached by one build is served forever by every later build. Found while diagnosing T-1239, where the same mechanism failed in dev for a different reason (the version simply was not bumped). Fix direction: bake the version into the client at export time (a generated const, ProjectSettings application/config/version, or an exported resource) rather than reading a repo file at runtime. Note test_current_schema_version_reads_project_yaml passes in dev and would not catch this — it asserts the non-fallback path, in the only environment where that path works.', 'in_progress', 'medium', NULL, 'client', 'D-255', '2026-08-14 21:19:17.188', '2026-08-14 21:21:35.097', NULL, '0e36862b9cbd420d1f0d8114f7673cc9', 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 ('06G0495WRHF8ADK82VR8CH1J8R', 'bug', '06FB0TNSRZXCHGS16BFHSSGSV4', 'Atlas disk cache never invalidates in an exported build — current_schema_version() falls back to ''?.?.?''', 'step_canvas_disk_cache.gd::current_schema_version() reads ProjectSettings.globalize_path(''res://'') + ''/../project.yaml''. That resolves to the repo-root file in a dev run (res:// = client/), but an exported build has no project.yaml one level above res://, so the function returns its ''?.?.?'' fallback. Every exported build therefore stamps and compares the SAME sentinel version, which means the schema-version invalidation path — the cache''s only invalidation signal — is inert in a shipped game: a canvas cached by one build is served forever by every later build. Found while diagnosing T-1239, where the same mechanism failed in dev for a different reason (the version simply was not bumped). Fix direction: bake the version into the client at export time (a generated const, ProjectSettings application/config/version, or an exported resource) rather than reading a repo file at runtime. Note test_current_schema_version_reads_project_yaml passes in dev and would not catch this — it asserts the non-fallback path, in the only environment where that path works.', 'done', 'medium', NULL, 'client', 'D-255', '2026-08-14 21:19:17.188', '2026-08-14 21:35:35.623', NULL, '3bd4d5eaacf6a4792e00a644a3333535', 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 ('06G04975H3S7GRVQXHKYCR7BKR', 'task', '06FB0TNSRZXCHGS16BFHSSGSV4', 'Enforce the canvas-generation/project.yaml-version pairing — four silent stale-cache regressions and counting', 'project.yaml''s version is the Atlas disk cache''s only invalidation signal, and nothing enforces that a change to canvas GENERATION also moves it. The file''s own comment block now records four bumps forced after the fact by exactly this failure: 0.4.2 (lake_margin_q semantics), 0.4.3 (coast_warp_px at orbital sampling), 0.4.4 (D-255 extent inversion), 0.4.5 (Global sentinel), and now 0.4.6 (T-1237 one-course-per-river, diagnosed as T-1239 eight days after it shipped). The failure is silent and machine-dependent: it reproduces only where a warm cache exists, so the author with a cold checkout sees nothing wrong. Direction: a pre-push check in .config/hooks/pre-push — if the push touches the canvas-generation paths (server/src/atlas/step_canvas.rs, river_course.rs, layer1.rs, district_profile.rs, the client step_canvas layers) and project.yaml''s version line is unchanged in the same range, reject with the reason. Registry-driven like tooling/generator_sources.py rather than a hand-kept path list in the hook. A false positive is cheap (bump the version, entries miss once); a false negative is another week of a wrong map.', 'in_progress', 'medium', NULL, 'client', 'D-255', '2026-08-14 21:19:27.624', '2026-08-14 21:41:02.683', NULL, 'efd83e7e6e689653b411d77629c19985', 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 ('06G04975H3S7GRVQXHKYCR7BKR', 'task', '06FB0TNSRZXCHGS16BFHSSGSV4', 'Enforce the canvas-generation/project.yaml-version pairing — four silent stale-cache regressions and counting', 'project.yaml''s version is the Atlas disk cache''s only invalidation signal, and nothing enforces that a change to canvas GENERATION also moves it. The file''s own comment block now records four bumps forced after the fact by exactly this failure: 0.4.2 (lake_margin_q semantics), 0.4.3 (coast_warp_px at orbital sampling), 0.4.4 (D-255 extent inversion), 0.4.5 (Global sentinel), and now 0.4.6 (T-1237 one-course-per-river, diagnosed as T-1239 eight days after it shipped). The failure is silent and machine-dependent: it reproduces only where a warm cache exists, so the author with a cold checkout sees nothing wrong. Direction: a pre-push check in .config/hooks/pre-push — if the push touches the canvas-generation paths (server/src/atlas/step_canvas.rs, river_course.rs, layer1.rs, district_profile.rs, the client step_canvas layers) and project.yaml''s version line is unchanged in the same range, reject with the reason. Registry-driven like tooling/generator_sources.py rather than a hand-kept path list in the hook. A false positive is cheap (bump the version, entries miss once); a false negative is another week of a wrong map.', 'in_progress', 'medium', NULL, 'client', 'D-255', '2026-08-14 21:19:27.624', '2026-08-14 21:41:17.977', NULL, 'cb2da99199993e9c60322845ceaf22b8', 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/CLAUDE.md b/CLAUDE.md index 3c07054c6..51b024a42 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ A top-down life-sim — asymmetric information, occlusion-based perception, sing **Official Title:** The Settled Reach **Repository name:** settled-reach -**Version source of truth:** `project.yaml` (root `version` field, scheme: `0.{phase}.{n}`), mirrored into `client/project.godot`'s `application/config/version` — the client can't read `project.yaml` at runtime (an exported build has no repo root), and the Atlas disk cache keys its only invalidation signal on that value. Bump both; `make check-client-version` (run by the pre-push hook) fails on drift. See `client/scripts/build_version.gd` (T-1241). +**Version source of truth:** `project.yaml` (root `version` field, scheme: `0.{phase}.{n}`), mirrored into `client/project.godot`'s `application/config/version` — the client can't read `project.yaml` at runtime (an exported build has no repo root), and the Atlas disk cache keys its only invalidation signal on that value. Bump both; `make check-client-version` (run by the pre-push hook) fails on drift. See `client/scripts/build_version.gd` (T-1241). **Changing canvas generation requires bumping this version** — otherwise warm caches keep serving canvases built by code that no longer exists (five regressions, most recently T-1239). The pre-push hook enforces it via `make check-canvas-version` against the path registry in `tooling/canvas_sources.py` (T-1242); there is no override, and bumping when unsure costs one cache miss. ## Project Structure diff --git a/Makefile b/Makefile index 263a0568e..3a4249d97 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ 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 \ decisions-sync decisions-active decisions-validate \ validate-content check-fact-ids setup-hooks install-hooks \ - audit deny atlas-verify economy-db regen-db check-systems-db check-client-version \ + audit deny atlas-verify economy-db regen-db check-systems-db check-client-version check-canvas-version \ 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 \ @@ -57,6 +57,7 @@ help: @echo " make regen-db Regenerate systems.db from all sources + stamp meta table (#855)" @echo " make check-systems-db Verify systems.db meta stamp matches current generator sources" @echo " make check-client-version Verify client/project.godot version mirrors project.yaml" + @echo " make check-canvas-version Verify canvas-generation changes carry a version bump" @echo " make install-hooks Install pre-push + pre-commit git hooks (once per clone)" @echo " make fixtures-client Generate GDScript->Rust cross-encoder fixtures (#475)" @echo " make golden-diff Show diff if golden file output has changed" @@ -263,6 +264,10 @@ test-tooling: @echo " [test-tooling] oasis ring-scaling pin (T-964, PR #210 review)..." @$(VENV_PY) tooling/planet-gen/test_oasis_ring_scaling.py 2> .cache/test-tooling-oasis-ring.log || \ { echo " FAIL: oasis ring scaling — log follows:"; cat .cache/test-tooling-oasis-ring.log; exit 1; } + @echo " [test-tooling] canvas-generation version gate units (T-1242)..." + @mkdir -p .cache + @python3 tooling/test_canvas_version_check.py 2> .cache/test-tooling-canvas-version.log || \ + { echo " FAIL: canvas version gate units — log follows:"; cat .cache/test-tooling-canvas-version.log; exit 1; } @echo " [test-tooling] economy_import.traits validation units (T-995/PR #173 H2)..." @mkdir -p .cache @python3 tooling/economy-db/test_traits.py 2> .cache/test-tooling-traits.log || \ @@ -410,6 +415,9 @@ check-systems-db: ## Verify systems.db meta stamp matches current generator sou check-client-version: ## Verify client/project.godot's baked version matches project.yaml (T-1241) @python3 tooling/check-client-version +check-canvas-version: ## Verify canvas-generation changes carry a project.yaml version bump (T-1242) + @python3 tooling/check-canvas-version + econ-sim: ## Build the economics simulation binary (Layer 1+2: Leontief + tâtonnement trade) @cargo build --manifest-path tooling/econ-sim/Cargo.toml --release @echo "Built: tooling/econ-sim/target/release/econ-sim" diff --git a/client/project.godot b/client/project.godot index 80b3c377e..8e80a7d17 100644 --- a/client/project.godot +++ b/client/project.godot @@ -17,7 +17,7 @@ config/name="The Settled Reach" ; in the editor and in a shipped build, where res://../project.yaml does not ; exist at all (T-1241). Kept honest by `make check-client-version`, which the ; pre-push hook runs — do not edit this by hand without moving project.yaml too. -config/version="0.4.6" +config/version="0.4.7" run/main_scene="res://scenes/main_menu.tscn" config/features=PackedStringArray("4.6", "GL Compatibility") config/icon="res://icon.svg" diff --git a/docs/DEVOPS.md b/docs/DEVOPS.md index 2ff54adbc..05c2344db 100644 --- a/docs/DEVOPS.md +++ b/docs/DEVOPS.md @@ -273,6 +273,36 @@ The pre-push hook runs this unconditionally (drift persists on `main` once introduced, so gating it on "were those files touched in this push" would let an existing drift ride along). **Bump both files together.** +## Canvas-generation version pairing + +Changing *how* a canvas is generated is only half a change. The other half is bumping +`project.yaml`'s version — otherwise every warm Atlas cache keeps serving canvases +built by code that no longer exists. That pairing was never enforced and broke five +times (0.4.2 lake_margin_q, 0.4.3 coast_warp_px, 0.4.4 the extent inversion, 0.4.5 the +Global sentinel, 0.4.6 one-course-per-river), each bumped only after the fact. The last +one took eight days to find (T-1239) because the failure is invisible to its author: it +reproduces only where a warm cache exists. + +```bash +make check-canvas-version # exit 1 = generation changed, version didn't +``` + +The path registry is `tooling/canvas_sources.py` — globbed, not hand-listed, so a +module added in a future split is covered the moment it exists. It **deliberately +over-includes**: a false positive costs one version bump and one round of cache misses, +a false negative costs another week of a wrong map. + +The registry includes itself, which closes the narrowing hole — removing a path *and* +changing that path in one push still trips the gate, because the registry file is in the +set. The cost is that editing the registry requires a version bump. + +There is **no override flag**, on purpose. It would be reached for exactly when someone +is certain their change is harmless, which is the state of mind that produced all five +regressions. If you are sure, bump anyway — it costs one cache miss. + +Units for the gate (including "a comment edit that quotes version numbers is not a +bump") run in `make test-tooling`. + ## Pre-commit and Pre-push Hooks Git hooks are stored in `.config/hooks/` (version-controlled). Activate them with: diff --git a/project.yaml b/project.yaml index e812ba63f..518b37c04 100644 --- a/project.yaml +++ b/project.yaml @@ -34,7 +34,15 @@ name: The Settled Reach # silent regression that reproduces only on machines with a warm cache. The # capture harness no longer hides it (tests/run-visual isolates user:// per run, # T-1239), but a real player's cache is still governed by this number alone. -version: 0.4.6 +# 0.4.7 carries NO canvas-generation change — it is the gate that enforces the +# pairing described above, arriving (T-1242). tooling/canvas_sources.py lists the +# paths, tooling/check-canvas-version rejects a push that touches them without +# moving this line, and the registry includes ITSELF so that narrowing it and +# changing a canvas file in one push still demands a bump. That self-inclusion is +# why this bump exists: adding the registry trips its own rule. The version is +# spent rather than special-cased, because "this change is obviously harmless" is +# the reasoning that produced all five regressions above. +version: 0.4.7 repository: settled-reach diff --git a/tooling/canvas_sources.py b/tooling/canvas_sources.py new file mode 100644 index 000000000..3d0840292 --- /dev/null +++ b/tooling/canvas_sources.py @@ -0,0 +1,158 @@ +#!/usr/bin/env python3 +""" +canvas_sources — single source of truth for the Atlas canvas-generation path set. + +`project.yaml`'s `version:` is the Atlas disk cache's ONLY invalidation signal +(client/scripts/build_version.gd, D-255). A change to how a canvas is GENERATED +is therefore only half a change; the other half is bumping that version, or every +warm cache keeps serving canvases built by code that no longer exists. + +Nothing enforced that pairing, and it broke five times: project.yaml's own +comments record 0.4.2 (lake_margin_q semantics), 0.4.3 (coast_warp_px at orbital +sampling), 0.4.4 (the D-255 extent inversion), 0.4.5 (the Global sentinel), and +0.4.6 (T-1237 one-course-per-river) — every one of them bumped *after the fact*, +the last only after T-1239 spent eight days diagnosing a map drawn from a canvas +whose generating code had been replaced. The failure is invisible to its author: +it reproduces only where a warm cache exists, so a cold checkout looks fine. + +This module is the registry `tooling/check-canvas-version` intersects against, +kept here rather than inline in the hook for the same reason +`tooling/generator_sources.py` exists (T-1067): one list, one place, imported by +everything that needs it. + +WHY THIS DELIBERATELY OVER-INCLUDES +----------------------------------- +The set is collected by GLOB, not hand-listed, and covers the whole atlas module +rather than a traced dependency closure. + +`step_canvas.rs` directly imports ten sibling modules and those pull in more +(district_profile -> domain_warp/detail_scatter/coast_invention/..., layer1 -> +drainage/hydrology_equilibrium/features). A hand-maintained closure of that would +be wrong within a month, and being wrong here is silent — exactly the failure +this registry exists to stop. Globbing is self-maintaining: a module added in a +future split is covered the moment it exists. + +The cost asymmetry is the whole argument, and it is the ticket's own ruling +(T-1242): a false positive costs one version bump and one round of cache misses; +a false negative costs another week of a wrong map. So when the choice is +"include a file that might not change canvas bytes" versus "risk missing one that +does", this includes it. + +Fail closed on an empty glob, per generator_sources.py's precedent: an empty set +would silently pass every push. + +Usage: + python3 tooling/canvas_sources.py --list +""" + +import argparse +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# --------------------------------------------------------------------------- +# Server: the code that PRODUCES canvas bytes +# --------------------------------------------------------------------------- + +# The whole atlas module. Canvas bytes are produced by step_canvas.rs out of +# layer1/TerrainAnalysis/district_profile/river_course/hydrology and a long tail +# of invention modules; see this file's header for why the boundary is the +# module rather than a traced import closure. +ATLAS_DIR: Path = REPO_ROOT / "server" / "src" / "atlas" + +# The seed chain feeds every deterministic decision the cascade makes +# (step_canvas.rs: `use crate::seed::SeedChain`), so a change to seed derivation +# changes canvas bytes without touching atlas/ at all. +SEED_RS: Path = REPO_ROOT / "server" / "src" / "seed.rs" + +# --------------------------------------------------------------------------- +# Client: the code that KEYS, STORES and INTERPRETS those bytes +# --------------------------------------------------------------------------- + +# The step_canvas cluster: the disk cache and its index format, the in-memory +# cache, the request/transport layer that derives extents and spacing (and +# therefore cache KEYS), and the layers that decode the payload. +# +# The render-only members (terrain/annotation layers) are included on purpose. +# A pure draw change cannot make cached bytes wrong, so including them can force +# an unnecessary re-derive — but drawing the line *inside* this directory means +# hand-judging which file is "really" wire-shaped, and T-1237 changed the server +# course shape and the client annotation layer in the same commit. That judgement +# is precisely where a false negative would come from. +CLIENT_STEP_CANVAS_DIR: Path = ( + REPO_ROOT / "client" / "ui" / "implant" / "apps" / "atlas" / "step_canvas" +) + +# The accessor the cache reads its invalidation tag through. +BUILD_VERSION_GD: Path = REPO_ROOT / "client" / "scripts" / "build_version.gd" + + +def _rust_sources(directory: Path, label: str) -> tuple[Path, ...]: + """Every .rs file in `directory`, collected by glob and fail-closed.""" + sources = tuple(sorted(directory.rglob("*.rs"))) + if not sources: + raise RuntimeError( + f"{label} sources not found at {directory} — the canvas-generation " + "path set would be incomplete, and this check would pass every push" + ) + return sources + + +def _gdscript_sources(directory: Path, label: str) -> tuple[Path, ...]: + """Every .gd file in `directory`, collected by glob and fail-closed. + + `.uid` sidecars are Godot bookkeeping and carry no behaviour, so they are + excluded — a uid churn should not demand a version bump. + """ + sources = tuple(sorted(directory.rglob("*.gd"))) + if not sources: + raise RuntimeError( + f"{label} sources not found at {directory} — the canvas-generation " + "path set would be incomplete, and this check would pass every push" + ) + return sources + + +def canvas_sources() -> tuple[Path, ...]: + """Every file whose change may alter canvas bytes or their interpretation. + + This registry itself is a member: loosening the set must be as visible as + any other canvas-generation change (generator_sources.py makes the same + call for the same reason). + """ + return ( + Path(__file__).resolve(), + *_rust_sources(ATLAS_DIR, "server atlas"), + SEED_RS, + *_gdscript_sources(CLIENT_STEP_CANVAS_DIR, "client step_canvas"), + BUILD_VERSION_GD, + ) + + +def relative_paths() -> tuple[str, ...]: + """The registry as repo-relative POSIX paths, for matching git output.""" + return tuple(p.relative_to(REPO_ROOT).as_posix() for p in canvas_sources()) + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Single source of truth for the Atlas canvas-generation path set" + ) + parser.add_argument( + "--list", + action="store_true", + help="Print the canvas-generation paths, one repo-relative path per line", + ) + args = parser.parse_args() + + if not args.list: + parser.print_help() + sys.exit(2) + + for path in relative_paths(): + print(path) + + +if __name__ == "__main__": + main() diff --git a/tooling/check-canvas-version b/tooling/check-canvas-version new file mode 100755 index 000000000..b7f8d1f98 --- /dev/null +++ b/tooling/check-canvas-version @@ -0,0 +1,164 @@ +#!/usr/bin/env python3 +"""Fail if a push changes canvas generation without moving project.yaml's version. + +`project.yaml`'s `version:` is the Atlas disk cache's only invalidation signal. +Change how a canvas is generated without moving it and every warm cache keeps +serving canvases built by code that no longer exists — silently, and only on +machines that have a warm cache, so the author never sees it. That has happened +five times (see tooling/canvas_sources.py for the roll-call); T-1239 is what the +last one cost. + +The rule: if the push touches anything in the canvas-generation registry, the +`version:` line in project.yaml must change in the SAME range. + +Deliberately no override flag. The ticket's ruling (T-1242) is that a false +positive is cheap — one version bump, one round of cache misses — and a false +negative is another week of a wrong map. An escape hatch would be reached for +exactly when someone is sure their change is harmless, which is the state of mind +that produced all five regressions. + +Exit: 0 = fine (or nothing relevant in range), 1 = version bump required. +""" + +import argparse +import subprocess +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from canvas_sources import relative_paths # noqa: E402 + +REPO_ROOT = Path(__file__).resolve().parent.parent +DEFAULT_BASE = "origin/main" + + +def git(*args: str) -> str | None: + """Run a git command, returning stdout, or None if it failed.""" + result = subprocess.run( + ["git", "-C", str(REPO_ROOT), *args], + capture_output=True, + text=True, + ) + if result.returncode != 0: + return None + return result.stdout + + +def changed_files(commit_range: str) -> list[str] | None: + out = git("diff", "--name-only", commit_range) + if out is None: + return None + return [line for line in out.splitlines() if line] + + +def diff_has_version_bump(diff_text: str) -> bool: + """Does this project.yaml diff actually move the `version:` field? + + Pure, so the property is testable without constructing git history + (tooling/test_canvas_version_check.py). + + Matched on the diff body rather than on the file appearing in --name-only: + project.yaml carries a long comment block documenting past bumps — including + lines that quote old version NUMBERS — so editing that commentary, or any + other field in the file, must NOT count as bumping the version. + + Requires the ADDED side: a lone deletion means the field was removed, not + moved. Diff context/metadata lines such as `+++ b/project.yaml` must not + match either, which is why this anchors on `+version:` exactly. + """ + for line in diff_text.splitlines(): + if line.startswith("+++"): + continue # diff header, not content + if line.startswith("+version:"): + return True + return False + + +def version_line_changed(commit_range: str) -> bool: + """Did project.yaml's `version:` line itself change in this range?""" + out = git("diff", "-U0", commit_range, "--", "project.yaml") + if out is None: + return False + return diff_has_version_bump(out) + + +def main() -> int: + parser = argparse.ArgumentParser( + description="Require a project.yaml version bump alongside canvas-generation changes" + ) + parser.add_argument( + "--base", + default=DEFAULT_BASE, + help=f"Base ref to compare against (default: {DEFAULT_BASE})", + ) + parser.add_argument( + "--head", + default="HEAD", + help="Head ref to compare (default: HEAD)", + ) + args = parser.parse_args() + + # Three-dot: what HEAD added since the merge base, matching the systems.db + # stamp check's own convention in .config/hooks/pre-push. + commit_range = f"{args.base}...{args.head}" + + if git("rev-parse", "--verify", args.base) is None: + # No base to compare against (fresh clone, no remote yet). Skipping is + # correct rather than failing: there is no "range" to judge. + print( + f"check-canvas-version: {args.base} not found — skipping (nothing to compare)" + ) + return 0 + + changed = changed_files(commit_range) + if changed is None: + print( + f"check-canvas-version: could not diff {commit_range} — skipping", + file=sys.stderr, + ) + return 0 + + registry = set(relative_paths()) + touched = sorted(set(changed) & registry) + + if not touched: + print("check-canvas-version: no canvas-generation changes in range — OK") + return 0 + + if version_line_changed(commit_range): + print( + f"check-canvas-version: OK — {len(touched)} canvas-generation file(s) " + "changed and project.yaml's version moved with them" + ) + return 0 + + shown = touched[:10] + remainder = len(touched) - len(shown) + print( + "check-canvas-version: canvas generation changed without a version bump\n" + "\n" + f" Range: {commit_range}\n" + " Changed canvas-generation files:\n" + + "".join(f" {p}\n" for p in shown) + + (f" ... and {remainder} more\n" if remainder else "") + + "\n" + "project.yaml's `version:` is the Atlas disk cache's ONLY invalidation\n" + "signal. Without a bump, every warm cache keeps serving canvases built by\n" + "the code you just changed — silently, and only on machines that have a\n" + "warm cache, so you will not see it on a cold checkout.\n" + "\n" + "Fix: bump `version:` in project.yaml (scheme 0.{phase}.{n}), add a line to\n" + "the comment block above it saying what the old entries carried, and mirror\n" + "the new value into client/project.godot's config/version.\n" + "\n" + "If you are certain this change cannot alter canvas bytes, bump it anyway:\n" + "the cost is one round of cache misses. That trade is the point — this has\n" + "shipped broken five times, most recently T-1239, which took eight days to\n" + "find.", + file=sys.stderr, + ) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tooling/test_canvas_version_check.py b/tooling/test_canvas_version_check.py new file mode 100644 index 000000000..d0b45ce51 --- /dev/null +++ b/tooling/test_canvas_version_check.py @@ -0,0 +1,171 @@ +#!/usr/bin/env python3 +"""Units for the canvas-generation/version pairing gate (T-1242). + +The gate's whole value is that it fires on the cases that shipped broken and +stays quiet otherwise. Two properties carry that, and neither is observable from +"the check passed on this branch": + + 1. Editing project.yaml WITHOUT moving `version:` is not a bump. The file + carries a comment block that quotes old version numbers (0.4.2 … 0.4.6), so + a naive "did project.yaml change" or "does the diff mention a version" + test would count commentary edits as a bump and wave through exactly the + regressions this exists to catch. + 2. The registry is non-empty and covers the files the five known regressions + actually touched. An empty or narrowed registry passes every push silently. + +Run: python3 tooling/test_canvas_version_check.py +""" + +import importlib.util +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +TOOLING = REPO_ROOT / "tooling" + +sys.path.insert(0, str(TOOLING)) +from canvas_sources import relative_paths # noqa: E402 + + +def _load_check_module(): + """Import the extensionless check script as a module.""" + path = TOOLING / "check-canvas-version" + spec = importlib.util.spec_from_loader( + "check_canvas_version", + importlib.machinery.SourceFileLoader("check_canvas_version", str(path)), + ) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +check = _load_check_module() + +FAILURES: list[str] = [] + + +def expect(condition: bool, label: str) -> None: + if condition: + print(f" ok {label}") + else: + print(f" FAIL {label}") + FAILURES.append(label) + + +def test_real_bump_counts() -> None: + diff = ( + "--- a/project.yaml\n" + "+++ b/project.yaml\n" + "@@ -22 +22 @@\n" + "-version: 0.4.5\n" + "+version: 0.4.6\n" + ) + expect(check.diff_has_version_bump(diff), "a real -version/+version pair is a bump") + + +def test_comment_edit_is_not_a_bump() -> None: + # The exact shape that must NOT count: adding a line to the comment block + # that documents what earlier versions carried. It mentions version numbers + # and sits directly above the field. + diff = ( + "--- a/project.yaml\n" + "+++ b/project.yaml\n" + "@@ -20,0 +21,2 @@\n" + "+# 0.4.5-tagged canvases carry pre-T-1237 river courses: one course per\n" + "+# D8 hop rather than one per river. 0.4.6 forces them to miss.\n" + ) + expect( + not check.diff_has_version_bump(diff), + "a comment-block edit quoting version numbers is NOT a bump", + ) + + +def test_other_field_edit_is_not_a_bump() -> None: + diff = ( + "--- a/project.yaml\n" + "+++ b/project.yaml\n" + "@@ -23 +23 @@\n" + "-repository: settled-reach\n" + "+repository: settled-reach-renamed\n" + ) + expect( + not check.diff_has_version_bump(diff), + "editing another field is NOT a bump", + ) + + +def test_diff_header_is_not_a_bump() -> None: + # `+++ b/project.yaml` starts with '+' and must not be mistaken for content. + diff = "--- a/project.yaml\n+++ b/project.yaml\n@@ -1 +1 @@\n-name: x\n+name: y\n" + expect( + not check.diff_has_version_bump(diff), + "the +++ diff header is NOT a bump", + ) + + +def test_deletion_alone_is_not_a_bump() -> None: + diff = "--- a/project.yaml\n+++ b/project.yaml\n@@ -22 +21,0 @@\n-version: 0.4.6\n" + expect( + not check.diff_has_version_bump(diff), + "removing the version field is NOT a bump", + ) + + +def test_empty_diff_is_not_a_bump() -> None: + expect(not check.diff_has_version_bump(""), "an empty diff is NOT a bump") + + +def test_registry_is_populated() -> None: + paths = relative_paths() + expect(len(paths) > 10, f"registry is populated ({len(paths)} paths)") + + +def test_registry_covers_the_known_regressions() -> None: + """The files the five documented stale-cache regressions actually touched. + + If a future edit narrows the registry past any of these, the gate stops + catching the exact class of bug it was built for — silently. This is the + test that makes such a narrowing loud. + """ + paths = set(relative_paths()) + required = [ + # T-1237 / T-1239: one course per river (0.4.6) + "server/src/atlas/step_canvas.rs", + "server/src/atlas/river_course.rs", + "client/ui/implant/apps/atlas/step_canvas/step_canvas_annotation_layer.gd", + # D-255 extent inversion (0.4.4) + Global sentinel (0.4.5) + "client/ui/implant/apps/atlas/step_canvas/step_canvas_transport.gd", + # coast_warp_px at orbital sampling (0.4.3) + "server/src/atlas/coast_invention.rs", + # lake_margin_q semantics (0.4.2) + "server/src/atlas/hydrology_equilibrium.rs", + # the layer every rung samples + "server/src/atlas/layer1.rs", + "server/src/atlas/district_profile.rs", + # the cache that the version tag actually governs + "client/ui/implant/apps/atlas/step_canvas/step_canvas_disk_cache.gd", + ] + for path in required: + expect(path in paths, f"registry covers {path}") + + +def main() -> int: + print("test_canvas_version_check:") + test_real_bump_counts() + test_comment_edit_is_not_a_bump() + test_other_field_edit_is_not_a_bump() + test_diff_header_is_not_a_bump() + test_deletion_alone_is_not_a_bump() + test_empty_diff_is_not_a_bump() + test_registry_is_populated() + test_registry_covers_the_known_regressions() + + if FAILURES: + print(f"\nFAILED ({len(FAILURES)}): " + "; ".join(FAILURES), file=sys.stderr) + return 1 + print("test_canvas_version_check: PASS") + return 0 + + +if __name__ == "__main__": + sys.exit(main())