From 378a4b8f99e8022f2670819d87266e602880c878 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 31 Jul 2026 14:52:06 -0400 Subject: [PATCH] feat(ci): check the app's own components against the tokens, not just snippets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the gap the theme.css repoint exposed (#2319, part of #2277). `check_code_against_tokens` could always answer "does this code use the sheet correctly?" — it was only ever fed recorded SNIPPETS. The app's own components, where sixteen unresolvable references were living quietly, were checked by nothing at all. That was structural rather than an oversight: the drift panel runs in the browser and cannot read source files, and the server has no repo access. CI is the only place holding both the sources and the ability to run the check — and it only became cheap once theme.css became a generated artifact, so the source of truth is a committed file with no network and no credentials. **The sheet now carries its own SUPERSEDES block.** That is what keeps the checker instance-agnostic (rule #115): it knows nothing about any palette, and reads both the declarations and the discouraged literals out of whatever stylesheet it is pointed at. Hardcoding "#fff means use the text token" would have baked one install's kit into the tool. Two severities, split on whether the count is already zero: FAIL an unresolvable var() reference — zero today, so this is a ratchet holding a line already reached. It cannot false-positive either: the name is declared or it is not. REPORT superseded literals (32 files) and raw colour literals (246). Gating those means a permanently-red job, and a check nobody reads is worse than no check. **Comments are stripped before scanning, and that fired on the first real run.** A comment explaining why a literal is avoided necessarily contains that literal — DesignSystemsView's stylesheet documents exactly that about `#fff`, and the checker reported the explanation as a violation. A checker that flags the documentation of a rule teaches people to stop documenting rules. Also narrowed `--fs-weight-medium`'s supersedes to the keywords. `600` and `700` are real violations of the two-weight rule, but a bare number matches too much to find by literal scan — `z-index: 600` is not a font weight. That needs a property-aware check, which is a different tool. Verified end to end: the generator's output parses back through the checker's reader, so the two halves cannot drift into disagreeing about the format. --- .forgejo/workflows/ci.yml | 13 +- frontend/src/assets/theme.css | 8 ++ scripts/check_design_tokens.py | 174 ++++++++++++++++++++++ src/scribe/services/design_stylesheet.py | 21 +++ tests/test_check_design_tokens.py | 175 +++++++++++++++++++++++ 5 files changed, 390 insertions(+), 1 deletion(-) create mode 100644 scripts/check_design_tokens.py create mode 100644 tests/test_check_design_tokens.py diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index fc4986c..ad6a9d6 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -165,7 +165,18 @@ jobs: # ruff is pre-installed in the ci-python image — no install # step needed, lint runs in ~2s. - name: Lint - run: ruff check src/ + run: ruff check src/ scripts/ + + # Design tokens: does the frontend's CSS agree with the stylesheet the + # design system generates? Fails only on an unresolvable var() reference — + # that count is at zero, so this is a ratchet rather than a backlog. The + # literal findings are printed, not gated; hundreds exist and a + # permanently-red job is one nobody reads. + # + # Stdlib only, no install, no network: the source of truth is theme.css, + # which is generated from the design system and committed. + - name: Design token check + run: python3 scripts/check_design_tokens.py --report-literals test: name: Python tests diff --git a/frontend/src/assets/theme.css b/frontend/src/assets/theme.css index 1b26bfb..0484367 100644 --- a/frontend/src/assets/theme.css +++ b/frontend/src/assets/theme.css @@ -178,6 +178,14 @@ --fs-text-tertiary: #9A9890; } +/* SUPERSEDES — write the token, not the literal. + * #fff -> --fs-text-primary + * #ffffff -> --fs-text-primary + * white -> --fs-text-primary + * bold -> --fs-weight-medium + * bolder -> --fs-weight-medium + */ + /* ========================================================================== COMPATIBILITY ALIASES — the app's historical names, pointing at the system. diff --git a/scripts/check_design_tokens.py b/scripts/check_design_tokens.py new file mode 100644 index 0000000..558f2b1 --- /dev/null +++ b/scripts/check_design_tokens.py @@ -0,0 +1,174 @@ +#!/usr/bin/env python3 +"""Check the frontend's CSS against the tokens its stylesheet declares. + +The gap this closes: `services/design_stylesheet.check_code_against_tokens` has +always been able to answer "does this code use the sheet correctly?", but the +only thing ever fed to it was recorded SNIPPETS. The app's own components — where +sixteen unresolvable references were found living quietly (#2319) — were checked +by nothing at all. + +That was structural rather than an oversight. The drift panel runs in the browser +and cannot read source files, and the server has no repo access. CI is the only +place holding both the component sources and the ability to run the check, and it +only became cheap once `theme.css` became a generated artifact — so the source of +truth is a local file, with no network and no credentials. + +INSTANCE-AGNOSTIC ON PURPOSE (rule #115). Nothing here knows what a token should +be called or which literals are discouraged. Both come from the stylesheet: the +declarations, and the `SUPERSEDES` block the generator emits. Point it at a +different install's sheet and it checks that install's rules. + +Two severities, and the split is deliberate: + + FAIL an unresolvable `var()` reference. Currently zero, so this is a ratchet + that holds a line already reached rather than a backlog that keeps CI + red. It also cannot false-positive: either the name is declared or it + is not. + REPORT superseded literals and raw colour literals. Hundreds today, so gating + on them would mean a permanently failing job that everyone learns to + ignore — which is worse than no check. +""" +from __future__ import annotations + +import argparse +import pathlib +import re +import sys + +# A declaration is `--name:`; a reference is `var(--name)` or `var(--name, …)`. +DECLARATION = re.compile(r"(?\s*(--[A-Za-z0-9_-]+)\s*$") +HEX_LITERAL = re.compile(r"#[0-9a-fA-F]{3,8}\b") +STYLE_BLOCK = re.compile(r"]*>(.*?)", re.S) +CSS_COMMENT = re.compile(r"/\*.*?\*/", re.S) + + +def declared_tokens(sheet: str) -> set[str]: + """Every custom property the stylesheet declares. + + Anchored on the colon alone. Anchoring on `{` or `;` instead silently drops + every declaration that follows a comment — a mistake made once already, which + lost `--color-bg` and 2 others without erroring. + """ + return set(DECLARATION.findall(sheet)) + + +def superseded_literals(sheet: str) -> dict[str, str]: + """`{literal: token}` from the generator's SUPERSEDES block, lowercased.""" + out: dict[str, str] = {} + for line in sheet.splitlines(): + match = SUPERSEDES_LINE.match(line) + if match: + out[match.group(1).lower()] = match.group(2) + return out + + +def _literal_pattern(literal: str) -> re.Pattern: + """Match a literal without matching a longer one containing it. + + `#fff` must not fire inside `#ffffff`: different colours, and a finding on + the wrong one sends someone to change correct code. + """ + return re.compile( + r"(? str: + """The CSS in a file — `\n' + ) + css = check.style_source(sfc) + assert "--fs-text-primary" in css + assert "#fff" not in css + + +def test_a_comment_explaining_a_rule_is_not_a_violation(tmp_path): + """LOAD-BEARING, and it fired on the first real run. A comment documenting + why a literal is avoided necessarily contains that literal — this codebase's + own stylesheet says so about `#fff`. A checker that flags the documentation + of a rule teaches people to stop documenting rules.""" + sfc = tmp_path / "Y.vue" + sfc.write_text( + "\n" + ) + css = check.style_source(sfc) + assert check._literal_pattern("#fff").search(css) is None + + +def test_a_plain_css_file_is_read_whole(tmp_path): + css_file = tmp_path / "shared.css" + css_file.write_text(".a { color: #fff; }") + assert "#fff" in check.style_source(css_file) + + +# --- the gate --------------------------------------------------------------- + +def _run(tmp_path, sheet: str, component: str, monkeypatch, capsys): + (tmp_path / "assets").mkdir(parents=True, exist_ok=True) + sheet_path = tmp_path / "assets" / "theme.css" + sheet_path.write_text(sheet) + (tmp_path / "C.vue").write_text(f"") + monkeypatch.setattr( + "sys.argv", + ["check", "--sheet", str(sheet_path), "--root", str(tmp_path)], + ) + code = check.main() + return code, capsys.readouterr().out + + +def test_an_unresolvable_reference_fails_the_build(tmp_path, monkeypatch, capsys): + """The one hard gate. It is safe to gate on because the count is zero today — + a ratchet holding a line already reached, not a backlog that keeps CI red.""" + code, out = _run(tmp_path, SHEET, ".a { color: var(--nope); }", monkeypatch, capsys) + assert code == 1 + assert "--nope" in out + + +def test_a_resolvable_reference_passes(tmp_path, monkeypatch, capsys): + code, out = _run( + tmp_path, SHEET, ".a { color: var(--fs-text-primary); }", monkeypatch, capsys + ) + assert code == 0 + assert "every var() reference resolves" in out + + +def test_a_locally_declared_property_is_not_unresolved(tmp_path, monkeypatch, capsys): + """A component may legitimately define its own custom property for local use — + a keyframe variable, a per-instance override. Only a reference to a name that + exists NOWHERE is broken.""" + code, _ = _run( + tmp_path, SHEET, ".a { --local: 4px; padding: var(--local); }", monkeypatch, capsys + ) + assert code == 0 + + +def test_a_superseded_literal_reports_but_does_not_fail(tmp_path, monkeypatch, capsys): + """Hundreds exist. Gating would make a permanently-red job, which is a check + nobody reads — worse than no check at all.""" + code, out = _run(tmp_path, SHEET, ".a { color: #fff; }", monkeypatch, capsys) + assert code == 0 + assert "#fff -> --fs-text-primary" in out + + +def test_a_missing_stylesheet_is_an_error_not_a_pass(tmp_path, monkeypatch, capsys): + """If the sheet moves, the check must fail loudly rather than silently + passing with zero tokens to compare against — which would look identical to + a clean run.""" + monkeypatch.setattr( + "sys.argv", ["check", "--sheet", str(tmp_path / "gone.css"), "--root", str(tmp_path)] + ) + assert check.main() == 2