Design-system delivery, the contrast fix, recall scoping, and backup coverage #93
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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"(?<![\w-])(--[A-Za-z0-9_-]+)\s*:")
|
||||
REFERENCE = re.compile(r"var\(\s*(--[A-Za-z0-9_-]+)")
|
||||
SUPERSEDES_LINE = re.compile(r"^\s*\*\s*(\S+)\s*->\s*(--[A-Za-z0-9_-]+)\s*$")
|
||||
HEX_LITERAL = re.compile(r"#[0-9a-fA-F]{3,8}\b")
|
||||
STYLE_BLOCK = re.compile(r"<style[^>]*>(.*?)</style>", 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"(?<![0-9A-Za-z_#-])" + re.escape(literal) + r"(?![0-9A-Za-z_-])",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
|
||||
def style_source(path: pathlib.Path) -> str:
|
||||
"""The CSS in a file — `<style>` blocks for an SFC, the whole of a .css.
|
||||
|
||||
Comments are stripped, and that is load-bearing rather than tidy. A comment
|
||||
EXPLAINING a rule mentions the very literal the rule forbids: this file's own
|
||||
stylesheet documents why it avoids `#fff`, and the first run of this checker
|
||||
reported that explanation as a violation. A checker that flags the
|
||||
documentation of a rule teaches people to stop documenting rules.
|
||||
"""
|
||||
text = path.read_text(encoding="utf-8", errors="replace")
|
||||
css = "\n".join(STYLE_BLOCK.findall(text)) if path.suffix == ".vue" else text
|
||||
return CSS_COMMENT.sub(" ", css)
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--sheet", default="frontend/src/assets/theme.css")
|
||||
parser.add_argument("--root", default="frontend/src")
|
||||
parser.add_argument(
|
||||
"--report-literals", action="store_true",
|
||||
help="also list raw colour literals (advisory, never fails)",
|
||||
)
|
||||
args = parser.parse_args()
|
||||
|
||||
sheet_path = pathlib.Path(args.sheet)
|
||||
if not sheet_path.is_file():
|
||||
print(f"error: stylesheet not found: {sheet_path}", file=sys.stderr)
|
||||
return 2
|
||||
|
||||
sheet = sheet_path.read_text()
|
||||
declared = declared_tokens(sheet)
|
||||
supersedes = superseded_literals(sheet)
|
||||
print(f"{sheet_path}: {len(declared)} tokens declared, "
|
||||
f"{len(supersedes)} superseded literals recorded\n")
|
||||
|
||||
root = pathlib.Path(args.root)
|
||||
sources = sorted(
|
||||
[p for p in root.rglob("*.vue")] + [p for p in root.rglob("*.css")]
|
||||
)
|
||||
|
||||
unresolved: list[tuple[pathlib.Path, str]] = []
|
||||
superseded_hits: list[tuple[pathlib.Path, str, str]] = []
|
||||
literal_count = 0
|
||||
|
||||
for path in sources:
|
||||
if path == sheet_path:
|
||||
continue
|
||||
css = style_source(path)
|
||||
if not css.strip():
|
||||
continue
|
||||
|
||||
# A component may legitimately declare a local custom property; a
|
||||
# reference to it is not unresolved.
|
||||
local = set(DECLARATION.findall(css))
|
||||
for name in sorted(set(REFERENCE.findall(css))):
|
||||
if name not in declared and name not in local:
|
||||
unresolved.append((path, name))
|
||||
|
||||
for literal, token in supersedes.items():
|
||||
if _literal_pattern(literal).search(css):
|
||||
superseded_hits.append((path, literal, token))
|
||||
|
||||
literal_count += len(HEX_LITERAL.findall(css))
|
||||
|
||||
if unresolved:
|
||||
print(f"FAIL — {len(unresolved)} unresolvable var() reference(s).")
|
||||
print(" These render as the fallback if given one, or as nothing at all.")
|
||||
print(" Either way nothing errors, which is why they survive.\n")
|
||||
for path, name in unresolved:
|
||||
print(f" {path}: {name}")
|
||||
print()
|
||||
else:
|
||||
print("OK — every var() reference resolves to a declared token.\n")
|
||||
|
||||
if superseded_hits:
|
||||
print(f"REPORT — {len(superseded_hits)} superseded literal(s). "
|
||||
"The sheet says what to write instead:")
|
||||
seen: set[tuple[str, str]] = set()
|
||||
for path, literal, token in superseded_hits:
|
||||
key = (str(path), literal)
|
||||
if key in seen:
|
||||
continue
|
||||
seen.add(key)
|
||||
print(f" {path}: {literal} -> {token}")
|
||||
print()
|
||||
|
||||
if args.report_literals:
|
||||
print(f"REPORT — {literal_count} raw colour literal(s) in component CSS.")
|
||||
print(" Advisory: a literal is a value stated outside the system, so it "
|
||||
"cannot follow a palette change.\n")
|
||||
|
||||
return 1 if unresolved else 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
@@ -194,6 +194,27 @@ def render_stylesheet(
|
||||
lines.append("}")
|
||||
lines.append("")
|
||||
|
||||
# A trailing, machine-readable record of what this system says to write
|
||||
# INSTEAD of a given literal.
|
||||
#
|
||||
# The sheet carries its own supersedes declarations so that any consumer has
|
||||
# them — notably a CI check, which has the component sources but no database.
|
||||
# Hardcoding the mapping in a checker would bake one install's palette into
|
||||
# the tool; reading it from the sheet keeps the checker instance-agnostic and
|
||||
# keeps this file the single source.
|
||||
replacements = [
|
||||
(literal, getattr(token, "name", ""))
|
||||
for token in tokens
|
||||
for literal in (getattr(token, "supersedes", None) or ())
|
||||
if is_valid_token_name(getattr(token, "name", ""))
|
||||
]
|
||||
if replacements:
|
||||
lines.append("/* SUPERSEDES — write the token, not the literal.")
|
||||
for literal, name in replacements:
|
||||
lines.append(f" * {safe_comment(str(literal))} -> {name}")
|
||||
lines.append(" */")
|
||||
lines.append("")
|
||||
|
||||
return "\n".join(lines).rstrip() + "\n"
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,175 @@
|
||||
"""The CI-side token check (#2277).
|
||||
|
||||
The checker is stdlib-only and lives in scripts/ so CI can run it without an
|
||||
install, so these tests import it by path rather than as a package.
|
||||
|
||||
Two properties matter more than the parsing: it must not fire on a COMMENT that
|
||||
discusses a rule, and it must not fire on a longer literal that merely contains a
|
||||
shorter one. Both would make the report untrustworthy, and an untrustworthy
|
||||
report is worse than none — people stop reading it and the check stops working
|
||||
while still passing.
|
||||
"""
|
||||
import importlib.util
|
||||
import pathlib
|
||||
|
||||
import pytest
|
||||
|
||||
_PATH = pathlib.Path(__file__).resolve().parents[1] / "scripts" / "check_design_tokens.py"
|
||||
_spec = importlib.util.spec_from_file_location("check_design_tokens", _PATH)
|
||||
check = importlib.util.module_from_spec(_spec)
|
||||
_spec.loader.exec_module(check)
|
||||
|
||||
|
||||
SHEET = """
|
||||
:root {
|
||||
/* surface */
|
||||
--fs-surface-page: #14171A; /* page bg */
|
||||
--fs-text-primary: #E8E4D8;
|
||||
}
|
||||
|
||||
/* SUPERSEDES — write the token, not the literal.
|
||||
* #fff -> --fs-text-primary
|
||||
* #ffffff -> --fs-text-primary
|
||||
* bold -> --fs-weight-medium
|
||||
*/
|
||||
"""
|
||||
|
||||
|
||||
# --- reading the sheet ------------------------------------------------------
|
||||
|
||||
def test_declarations_after_a_comment_are_not_lost():
|
||||
"""Anchored on the colon alone. Anchoring on `{` or `;` silently drops every
|
||||
declaration that follows a comment — a mistake made once already in this
|
||||
codebase, which lost three tokens without erroring."""
|
||||
assert check.declared_tokens(SHEET) == {"--fs-surface-page", "--fs-text-primary"}
|
||||
|
||||
|
||||
def test_the_supersedes_block_is_read_from_the_sheet_not_hardcoded():
|
||||
"""Rule #115: the checker must know nothing about any install's palette.
|
||||
Everything it enforces comes out of the stylesheet it is pointed at."""
|
||||
assert check.superseded_literals(SHEET) == {
|
||||
"#fff": "--fs-text-primary",
|
||||
"#ffffff": "--fs-text-primary",
|
||||
"bold": "--fs-weight-medium",
|
||||
}
|
||||
|
||||
|
||||
def test_a_sheet_with_no_supersedes_block_yields_nothing():
|
||||
assert check.superseded_literals(":root { --a: 1px; }") == {}
|
||||
|
||||
|
||||
# --- the literal matcher ----------------------------------------------------
|
||||
|
||||
def test_a_short_hex_does_not_match_inside_a_longer_one():
|
||||
"""`#fff` firing on `#ffffff` would send someone to change correct code."""
|
||||
assert check._literal_pattern("#fff").search("color: #ffffff;") is None
|
||||
assert check._literal_pattern("#fff").search("color: #fff;") is not None
|
||||
|
||||
|
||||
def test_the_match_is_case_insensitive():
|
||||
assert check._literal_pattern("#ffffff").search("color: #FFFFFF;") is not None
|
||||
|
||||
|
||||
def test_a_keyword_does_not_match_inside_a_longer_word():
|
||||
"""`bold` must not fire on `font-weight: bolder` or a class named
|
||||
`.bold-label` — the boundary is what keeps the report readable."""
|
||||
assert check._literal_pattern("bold").search("font-weight: bolder;") is None
|
||||
assert check._literal_pattern("bold").search(".bold-label { }") is None
|
||||
assert check._literal_pattern("bold").search("font-weight: bold;") is not None
|
||||
|
||||
|
||||
# --- reading a component ----------------------------------------------------
|
||||
|
||||
def test_only_the_style_block_of_an_sfc_is_read(tmp_path):
|
||||
"""A hex in a template attribute or a script string is not a stylesheet
|
||||
violation, and reporting it would bury the ones that are."""
|
||||
sfc = tmp_path / "X.vue"
|
||||
sfc.write_text(
|
||||
'<template><div data-x="#fff">white</div></template>\n'
|
||||
'<script setup>const c = "#fff";</script>\n'
|
||||
'<style scoped>.a { color: var(--fs-text-primary); }</style>\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(
|
||||
"<style scoped>\n"
|
||||
"/* Deliberately NOT #fff — pure white is never text. */\n"
|
||||
".a { color: var(--fs-text-primary); }\n"
|
||||
"</style>\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"<style>{component}</style>")
|
||||
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
|
||||
Reference in New Issue
Block a user