feat(config): T-1281 — dataflow-graph and systems-db-stamp join the check domain
Both were already Python, so these are moves rather than rewrites, and both produce byte-identical output to their originals on the live tree with the same exit codes. The E402 debt evaporated on contact, which is the first concrete evidence for T-1274's premise. check-systems-db-stamp reached generator_sources through a sys.path.insert and a noqa suppression, because tooling/ was not a package. It now imports as `from tooling import generator_sources` — no hack, no suppression. The stamp gate's six failure modes are preserved as a StampState enum rather than collapsed into 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. One deliberate behavioural difference, flagged rather than hidden: the old stamp script was silent on success unless given --verbose, and the new one always prints its verdict. No fact is lost, so parity holds, 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 gives way to the global one, which is the consolidation this initiative is for. Also corrects a claim in the ticket itself: check-dataflow-graph.py does not parse git output, it globs the filesystem. Only check-canvas-version parses git, so only that fixture needs a real repo. Still open and recorded as such: check-canvas-version, and parity tests for these two — both were verified side by side on the live tree, which proves the happy path and nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -21,6 +21,7 @@ from tooling.core import cli, console
|
||||
from tooling.core.command import command
|
||||
from tooling.core.errors import ReachError
|
||||
from tooling.domains.check import service
|
||||
from tooling.domains.check.schemas import StampState
|
||||
|
||||
app = cli.domain("check", "Consistency gates — the checks the push hook runs.")
|
||||
|
||||
@@ -66,6 +67,75 @@ def client_version() -> None:
|
||||
console.verdict(f"check-client-version: OK — {result.yaml_version}")
|
||||
|
||||
|
||||
_STAMP_REMEDY = {
|
||||
StampState.UNSTAMPED: "make regen-db — systems.db carries no stamp to verify",
|
||||
StampState.BAD_VERSION: "make regen-db — the recorded schema_version predates semver",
|
||||
StampState.CONFLICT: "make regen-db — the DB was partially regenerated, so nothing "
|
||||
"in it is internally consistent",
|
||||
StampState.UNKNOWN: "register the generator in tooling/generator_sources.py, then "
|
||||
"make regen-db",
|
||||
StampState.BROKEN: "a registered generator source has moved or been deleted — fix "
|
||||
"the path in tooling/generator_sources.py",
|
||||
StampState.STALE: "make regen-db, then stage server/data/systems.db",
|
||||
}
|
||||
|
||||
|
||||
@app.command("systems-db-stamp")
|
||||
@command
|
||||
def systems_db_stamp() -> None:
|
||||
"""Fail if systems.db is older than the generator sources that produced it."""
|
||||
result = service.systems_db_stamp()
|
||||
|
||||
if result.state is StampState.ABSENT:
|
||||
console.verdict("check-systems-db-stamp: no systems.db — nothing to verify")
|
||||
return
|
||||
|
||||
if result.state is StampState.OK:
|
||||
console.verdict(
|
||||
f"check-systems-db-stamp: OK — {result.generators} generator(s) up to date"
|
||||
)
|
||||
return
|
||||
|
||||
detail = "".join(f"\n {line}" for line in result.details)
|
||||
raise ReachError(
|
||||
f"check-systems-db-stamp: {result.state.value.upper()}{detail}",
|
||||
fix=_STAMP_REMEDY[result.state],
|
||||
# UNSTAMPED is 2, everything else 1 — a distinction the pre-push hook
|
||||
# has relied on since T-857 and which parity must preserve.
|
||||
exit_code=result.exit_code,
|
||||
)
|
||||
|
||||
|
||||
@app.command("dataflow-graph")
|
||||
@command
|
||||
def dataflow_graph() -> None:
|
||||
"""Fail if a path named in a hand-authored diagram no longer resolves."""
|
||||
result = service.dataflow_graph()
|
||||
|
||||
if result.missing or result.unresolved:
|
||||
detail = "\n".join(f" {line}" for line in result.missing + result.unresolved)
|
||||
raise ReachError(
|
||||
f"check-dataflow-graph: FAILED\n{detail}",
|
||||
fix=(
|
||||
"either the path moved, in which case update the diagram, or the "
|
||||
"diagram was always wrong — check the source before editing either"
|
||||
),
|
||||
)
|
||||
|
||||
if result.checked == 0:
|
||||
# A checker that found nothing to check reports success, which is the
|
||||
# quietest way for this gate to stop working: the label format changes
|
||||
# and every run stays green while asserting nothing.
|
||||
raise ReachError(
|
||||
"check-dataflow-graph: no path-like tokens found — the checker is "
|
||||
"not actually checking anything",
|
||||
fix="the d2 label format likely changed; update the token pattern in "
|
||||
"domains/check/service.py",
|
||||
)
|
||||
|
||||
console.verdict(f"check-dataflow-graph: OK — {result.diagrams} diagram(s)")
|
||||
|
||||
|
||||
@app.command("fact-ids")
|
||||
@command
|
||||
def fact_ids() -> None:
|
||||
|
||||
@@ -12,6 +12,8 @@ reported.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from enum import Enum
|
||||
|
||||
from pydantic import BaseModel, ConfigDict
|
||||
|
||||
|
||||
@@ -45,6 +47,65 @@ class FactIdCheck(BaseModel):
|
||||
return not self.unknown
|
||||
|
||||
|
||||
class DiagramCheck(BaseModel):
|
||||
"""The outcome of checking that paths named in diagrams still resolve.
|
||||
|
||||
`checked` matters as much as `unresolved`. A diagram whose label format
|
||||
changed yields zero tokens, and a checker that finds nothing to check
|
||||
reports success — so a count of zero is itself a failure, not a clean run.
|
||||
"""
|
||||
|
||||
model_config = ConfigDict(frozen=True)
|
||||
|
||||
diagrams: int
|
||||
checked: int
|
||||
unresolved: list[str] = []
|
||||
missing: list[str] = []
|
||||
|
||||
@property
|
||||
def ok(self) -> bool:
|
||||
return not self.unresolved and not self.missing and self.checked > 0
|
||||
|
||||
|
||||
class StampState(str, Enum):
|
||||
"""Why the systems.db stamp check reached its verdict.
|
||||
|
||||
Distinguished rather than collapsed into pass/fail because they carry
|
||||
different remedies and, in one case, a different exit code: UNSTAMPED exits
|
||||
2 while every other failure exits 1, and the pre-push hook has treated that
|
||||
2 as meaningful since T-857.
|
||||
"""
|
||||
|
||||
OK = "ok"
|
||||
ABSENT = "absent" # no DB at all — not a failure, nothing to verify
|
||||
UNSTAMPED = "unstamped" # no meta table or empty: exit 2
|
||||
BAD_VERSION = "bad_version" # schema_version is not semver
|
||||
CONFLICT = "conflict" # generators disagree on schema_version
|
||||
UNKNOWN = "unknown" # a generator not in the registry
|
||||
BROKEN = "broken" # a registered source file is gone
|
||||
STALE = "stale" # sources changed since the stamp was written
|
||||
|
||||
|
||||
class StampCheck(BaseModel):
|
||||
"""The outcome of comparing systems.db's meta stamp against its sources."""
|
||||
|
||||
model_config = ConfigDict(frozen=True)
|
||||
|
||||
state: StampState
|
||||
generators: int = 0
|
||||
details: list[str] = []
|
||||
|
||||
@property
|
||||
def ok(self) -> bool:
|
||||
return self.state in (StampState.OK, StampState.ABSENT)
|
||||
|
||||
@property
|
||||
def exit_code(self) -> int:
|
||||
if self.ok:
|
||||
return 0
|
||||
return 2 if self.state is StampState.UNSTAMPED else 1
|
||||
|
||||
|
||||
class VersionCheck(BaseModel):
|
||||
"""The outcome of comparing project.yaml against client/project.godot.
|
||||
|
||||
|
||||
@@ -9,10 +9,18 @@ a rewrite.
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import sqlite3
|
||||
from pathlib import Path
|
||||
|
||||
from tooling.core import config
|
||||
from tooling.domains.check.schemas import FactIdCheck, UnknownFact, VersionCheck
|
||||
from tooling.domains.check.schemas import (
|
||||
DiagramCheck,
|
||||
FactIdCheck,
|
||||
StampCheck,
|
||||
StampState,
|
||||
UnknownFact,
|
||||
VersionCheck,
|
||||
)
|
||||
|
||||
# Anchored to line start so the commentary above `version:` (which mentions
|
||||
# earlier versions by number) can never be mistaken for the field itself.
|
||||
@@ -52,6 +60,166 @@ def client_version() -> VersionCheck:
|
||||
)
|
||||
|
||||
|
||||
# --- systems-db stamp -----------------------------------------------------
|
||||
#
|
||||
# server/data/systems.db is a committed build artefact. Its meta table records
|
||||
# the SHA-1 of each generator's sources at generation time; if the sources have
|
||||
# moved on and the DB has not, a push would ship a snapshot nobody can reproduce
|
||||
# (T-855, T-857).
|
||||
|
||||
_SEMVER = re.compile(r"^\d+\.\d+\.\d+$")
|
||||
|
||||
|
||||
def systems_db_stamp() -> StampCheck:
|
||||
"""Compare systems.db's recorded generator SHAs against the sources on disk."""
|
||||
# Imported directly rather than through a sys.path insert — the old script
|
||||
# needed the hack because tooling/ was not a package. That is the E402 debt
|
||||
# T-1274 expects to evaporate, doing so.
|
||||
from tooling import generator_sources
|
||||
|
||||
db_path = config.path("server", "data", "systems.db")
|
||||
if not db_path.exists():
|
||||
return StampCheck(state=StampState.ABSENT)
|
||||
|
||||
try:
|
||||
connection = sqlite3.connect(str(db_path))
|
||||
rows = connection.execute(
|
||||
"SELECT generator_name, schema_version, generator_sha FROM meta"
|
||||
).fetchall()
|
||||
connection.close()
|
||||
except sqlite3.OperationalError:
|
||||
return StampCheck(state=StampState.UNSTAMPED)
|
||||
|
||||
if not rows:
|
||||
return StampCheck(state=StampState.UNSTAMPED)
|
||||
|
||||
bad_version: list[str] = []
|
||||
unknown: list[str] = []
|
||||
stale: list[str] = []
|
||||
versions: dict[str, str] = {}
|
||||
|
||||
for name, schema_version, stored_sha in rows:
|
||||
versions[name] = schema_version
|
||||
# An old DB may still carry a 40-char SHA here rather than a semver.
|
||||
# Flagged rather than silently accepted, so the answer is "regen" and
|
||||
# not a pass that hides a schema from a different era (T-888).
|
||||
if not _SEMVER.match(schema_version or ""):
|
||||
bad_version.append(
|
||||
f"{name}: schema_version={schema_version!r} "
|
||||
"(expected semver like '1.0.0' — run make regen-db)"
|
||||
)
|
||||
|
||||
sources = generator_sources.GENERATOR_SOURCES.get(name)
|
||||
if sources is None:
|
||||
# Fail CLOSED. A branch that adds a generator without registering it
|
||||
# would otherwise pass this gate while checking nothing about it.
|
||||
unknown.append(name)
|
||||
continue
|
||||
try:
|
||||
current = generator_sources.file_sha1(*sources)
|
||||
except FileNotFoundError as exc:
|
||||
return StampCheck(state=StampState.BROKEN, details=[f"{name}: {exc}"])
|
||||
if current != stored_sha:
|
||||
stale.append(name)
|
||||
|
||||
if bad_version:
|
||||
return StampCheck(state=StampState.BAD_VERSION, details=bad_version)
|
||||
|
||||
# Generators disagreeing on schema_version means the DB was partially
|
||||
# regenerated against different source trees — worse than stale, because
|
||||
# nothing about it is internally consistent.
|
||||
if len(set(versions.values())) > 1:
|
||||
return StampCheck(
|
||||
state=StampState.CONFLICT,
|
||||
details=[f"{gen}: {ver}" for gen, ver in sorted(versions.items())],
|
||||
)
|
||||
|
||||
if unknown:
|
||||
return StampCheck(state=StampState.UNKNOWN, details=sorted(unknown))
|
||||
if stale:
|
||||
return StampCheck(state=StampState.STALE, details=sorted(stale))
|
||||
|
||||
return StampCheck(state=StampState.OK, generators=len(rows))
|
||||
|
||||
|
||||
# --- dataflow-graph -------------------------------------------------------
|
||||
#
|
||||
# A diagram that names files goes stale SILENTLY: nothing fails when a path
|
||||
# moves, so the map keeps asserting a layout that is no longer true. This closes
|
||||
# the cheap half of that gap (D-262). It CANNOT check whether an edge still
|
||||
# means what it says — if wiki_sync.py stopped writing body pages tomorrow every
|
||||
# path here would still exist. Edge semantics are a human job.
|
||||
|
||||
# Hand-authored diagrams whose labels name real repo paths. A diagram absent
|
||||
# from this list is not checked; add one when it starts naming files.
|
||||
CHECKED_DIAGRAMS = ["data-flow/wiki-generator-flow.d2"]
|
||||
|
||||
# Generated sources are skipped even if listed: their correctness is the
|
||||
# generator's problem (the source-canonical rule), and they name star-system ids
|
||||
# rather than repo paths.
|
||||
GENERATED_PREFIXES = ("design/star-map-",)
|
||||
|
||||
_LABEL = re.compile(r'"((?:[^"\\]|\\.)*)"')
|
||||
_PATH_TOKEN = re.compile(r"[A-Za-z0-9_.\-*{}/]*/[A-Za-z0-9_.\-*{}/]+")
|
||||
|
||||
|
||||
def dataflow_graph() -> DiagramCheck:
|
||||
"""Assert every repo path named in a hand-authored diagram still resolves."""
|
||||
root = config.repo_root()
|
||||
top_level = {entry.name for entry in root.iterdir()}
|
||||
diagram_root = root / "docs" / "diagrams"
|
||||
|
||||
unresolved: list[str] = []
|
||||
missing: list[str] = []
|
||||
checked = 0
|
||||
considered = [d for d in CHECKED_DIAGRAMS if not d.startswith(GENERATED_PREFIXES)]
|
||||
|
||||
for diagram in considered:
|
||||
path = diagram_root / diagram
|
||||
if not path.exists():
|
||||
missing.append(f"{diagram}: diagram not found")
|
||||
continue
|
||||
for label in _LABEL.findall(path.read_text(encoding="utf-8")):
|
||||
for token in _path_tokens(label, top_level):
|
||||
checked += 1
|
||||
if not _resolves(root, token):
|
||||
unresolved.append(f"{diagram}: path does not resolve: {token}")
|
||||
|
||||
return DiagramCheck(
|
||||
diagrams=len(considered),
|
||||
checked=checked,
|
||||
unresolved=unresolved,
|
||||
missing=missing,
|
||||
)
|
||||
|
||||
|
||||
def _path_tokens(label: str, top_level: set[str]) -> list[str]:
|
||||
# Labels use \n for line breaks and · to separate sibling files.
|
||||
flat = label.replace("\\n", " ").replace("·", " ")
|
||||
tokens = []
|
||||
for raw in _PATH_TOKEN.findall(flat):
|
||||
token = raw.strip(".,;:")
|
||||
# A token only counts if its first segment is a real top-level entry.
|
||||
# Without this, legend prose like "dashed one-time or bootstrap" yields
|
||||
# the token one-time/bootstrap and fails the check on nothing.
|
||||
if token and token.split("/", 1)[0] in top_level:
|
||||
tokens.append(token)
|
||||
return tokens
|
||||
|
||||
|
||||
def _resolves(root: Path, token: str) -> bool:
|
||||
if (root / token).exists():
|
||||
return True
|
||||
# bodies/{id}/index.md -> bodies/*/index.md, then glob it.
|
||||
pattern = re.sub(r"\{[^}]*\}", "*", token)
|
||||
if "*" in pattern:
|
||||
try:
|
||||
return any(root.glob(pattern))
|
||||
except (ValueError, OSError):
|
||||
return False
|
||||
return False
|
||||
|
||||
|
||||
# Ported from the bash `check-fact-ids` (D-263: the shell scripts are rewritten,
|
||||
# not wrapped). Deliberately still LINE-MATCHED rather than YAML-parsed: the
|
||||
# original was grep-based, and parsing YAML properly would change which lines
|
||||
|
||||
Reference in New Issue
Block a user