diff --git a/frontend/src/api/designSystems.ts b/frontend/src/api/designSystems.ts index 0576ce1..c857d73 100644 --- a/frontend/src/api/designSystems.ts +++ b/frontend/src/api/designSystems.ts @@ -178,3 +178,27 @@ export interface StylesheetResult { * rather than restated per element. */ export const fetchStylesheet = (id: number) => apiGet(`/api/design-systems/${id}/stylesheet`); + +export interface SnippetFinding { + snippet_id: number; + title: string; + /** References that resolve against the sheet. */ + used: string[]; + /** `var(--x)` where the system has no `--x` — renders as nothing at all. */ + unknown: string[]; + /** Literals the sheet says to stop writing, paired with what to write. */ + superseded_literals: { literal: string; use_instead: string }[]; + /** Custom properties the snippet mints for itself instead of reusing. */ + local_definitions: string[]; +} + +export interface SnippetCheck { + design_system_id: number; + checked: number; + /** Only snippets with something to act on; clean ones are omitted. */ + findings: SnippetFinding[]; +} + +/** Which recorded snippets disagree with this design system's sheet. */ +export const checkSnippets = (id: number) => + apiGet(`/api/design-systems/${id}/snippet-check`); diff --git a/frontend/src/views/DesignSystemsView.vue b/frontend/src/views/DesignSystemsView.vue index 85c491f..9736eca 100644 --- a/frontend/src/views/DesignSystemsView.vue +++ b/frontend/src/views/DesignSystemsView.vue @@ -29,6 +29,7 @@ import { fetchDesignSystems, fetchDesignTokens, fetchResolvedTokens, + checkSnippets, fetchStylesheet, importFromRulebook, updateDesignSystem, @@ -37,6 +38,7 @@ import { type DesignToken, type ImportReport, type ResolvedToken, + type SnippetCheck, type StylesheetResult, } from "@/api/designSystems"; import { listRulebooks, type Rulebook } from "@/api/rulebooks"; @@ -433,6 +435,33 @@ watch([selectedId, ownTokens], () => { const duplicateEntries = computed(() => Object.entries(sheet.value?.duplicates ?? {})); +// --- do the snippets use the sheet? ----------------------------------------- + +const snippetCheck = ref(null); +const checkingSnippets = ref(false); + +/** The component layer checked against the sheet. + * + * Run on demand rather than on open: it reads every snippet's full body, which + * is a real cost, and the answer only changes when the sheet or the snippets + * do. */ +async function runSnippetCheck() { + if (selectedId.value === null || checkingSnippets.value) return; + checkingSnippets.value = true; + try { + snippetCheck.value = await checkSnippets(selectedId.value); + } catch { + snippetCheck.value = null; + toast.show("Couldn't check the snippets", "error"); + } finally { + checkingSnippets.value = false; + } +} + +watch(selectedId, () => { + snippetCheck.value = null; +}); + // --- import from a rulebook ------------------------------------------------- const rulebooks = ref([]); @@ -693,6 +722,66 @@ function isColourish(value: string): boolean { + +
+
+

Components using this sheet

+ +
+

+ Snippets are the component layer — buttons, tables, input schemes — + and they are meant to use the tags this sheet declares. This finds + the three ways that goes wrong silently. +

+ + +
+
@@ -1330,6 +1419,51 @@ function isColourish(value: string): boolean { gap: 0.35rem; } +.finding-list { + list-style: none; + margin: 0; + padding: 0; +} + +.finding-list li { + padding: 0.6rem 0; + border-bottom: 1px solid var(--color-border); +} + +.finding-title { + font-weight: 500; + color: var(--color-text); +} + +.finding-line { + margin: 0.3rem 0 0; + font-size: 0.85rem; + color: var(--color-text-secondary); + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 0.35rem; +} + +.spec-status { + font-size: 0.7rem; + text-transform: uppercase; + letter-spacing: 0.06em; + padding: 0.1rem 0.45rem; + border-radius: var(--radius-sm); + flex: none; +} + +.spec-status.violated { + background: var(--color-priority-high-bg); + color: var(--color-priority-high); +} + +.spec-status.missing { + background: var(--color-priority-medium-bg); + color: var(--color-priority-medium); +} + .sheet { background: var(--color-bg); border: 1px solid var(--color-border); diff --git a/src/scribe/mcp/tools/design_systems.py b/src/scribe/mcp/tools/design_systems.py index b6a1122..3620a0f 100644 --- a/src/scribe/mcp/tools/design_systems.py +++ b/src/scribe/mcp/tools/design_systems.py @@ -167,6 +167,41 @@ async def get_design_system_stylesheet( return result +async def check_snippets_against_design_system( + design_system_id: int, + project_id: int = 0, +) -> dict: + """Which recorded snippets disagree with a design system's sheet. + + Snippets are the component layer — buttons, tables, input schemes — and they + are supposed to use the tags the sheet declares. Three findings per snippet, + each currently silent in the codebase: + + unknown `var(--x)` where the system has no `--x`. Renders as + NOTHING: no error, no failing test, just an element + that quietly isn't styled. + superseded_literals a literal the sheet says to stop writing, paired with + the token to write instead. + local_definitions custom properties the snippet mints for itself rather + than using shared ones — the bloat a shared sheet + exists to prevent. + + Snippets with nothing to report are omitted. Reach for this before writing + or reviewing component CSS. + + Args: + design_system_id: The system whose sheet is authoritative. + project_id: Narrow to one project, or 0 for every project. + """ + uid = current_user_id() + result = await ds_svc.check_snippets_against_system( + uid, design_system_id, project_id + ) + if result is None: + raise ValueError(f"design system {design_system_id} not found") + return result + + async def import_design_system_from_rulebook( design_system_id: int, rulebook_id: int, @@ -336,6 +371,7 @@ def register(mcp) -> None: update_design_system, delete_design_system, get_design_system_stylesheet, + check_snippets_against_design_system, import_design_system_from_rulebook, create_design_token, list_design_tokens, diff --git a/src/scribe/routes/design_systems.py b/src/scribe/routes/design_systems.py index 48706ea..8a672a8 100644 --- a/src/scribe/routes/design_systems.py +++ b/src/scribe/routes/design_systems.py @@ -135,6 +135,24 @@ async def get_design_system_stylesheet(design_system_id: int): return jsonify(result) +@design_systems_bp.get("/design-systems//snippet-check") +@login_required +async def check_snippets_against_system(design_system_id: int): + """Which recorded snippets disagree with this system's sheet. + + `?project_id=` narrows to one project; omit it to check every project, which + is usually right — a component recorded elsewhere still has to use the same + tags. + """ + project_id = request.args.get("project_id", type=int) or 0 + result = await ds_svc.check_snippets_against_system( + _uid(), design_system_id, project_id + ) + if result is None: + return _not_found() + return jsonify(result) + + @design_systems_bp.post("/design-systems//import") @login_required async def import_design_system(design_system_id: int): diff --git a/src/scribe/services/design_stylesheet.py b/src/scribe/services/design_stylesheet.py index b6f4e5a..c734ec9 100644 --- a/src/scribe/services/design_stylesheet.py +++ b/src/scribe/services/design_stylesheet.py @@ -211,3 +211,84 @@ def duplicate_values(tokens: Sequence) -> dict[str, list[str]]: continue by_value.setdefault(str(base).strip().lower(), []).append(name) return {value: names for value, names in by_value.items() if len(names) > 1} + + +# --------------------------------------------------------------------------- +# Reading a sheet from the other side: does this code use it correctly? +# --------------------------------------------------------------------------- +# +# "The snippets use the tags from the sheet" is a verifiable relation, and +# nothing checked it before. Three questions, each a different failure: +# +# var(--x) where no --x exists -> renders as NOTHING. No error, no test +# failure, no visual clue beyond the thing +# silently not being styled. +# a superseded literal in code -> the value the sheet said to stop writing, +# and the sheet knows what to write instead. +# --x: declared inside a snippet -> a component minting its own token is the +# bloat a shared sheet exists to prevent. +# +# The first is not hypothetical: `--color-accent` was used throughout a new view +# in this codebase and does not exist. + +_VAR_REFERENCE = re.compile(r"var\(\s*(--[A-Za-z0-9_-]+)") +_LOCAL_DEFINITION = re.compile(r"(? set[str]: + """Every custom property the code reads through `var()`.""" + return set(_VAR_REFERENCE.findall(code or "")) + + +def defined_tokens(code: str) -> set[str]: + """Every custom property the code declares itself. + + Excludes names it also reads: `--x: var(--x, fallback)` is a redeclaration + of something the sheet owns, which the unknown-reference check already + covers more precisely. + """ + return set(_LOCAL_DEFINITION.findall(code or "")) - referenced_tokens(code or "") + + +def _literal_pattern(literal: str) -> re.Pattern: + """Match a literal value without matching a longer one that contains it. + + `#fff` must not match inside `#ffffff` — they are different colours, and a + finding that fired on the wrong one would send someone to change code that + was already correct. + """ + escaped = re.escape(literal) + lead = r"(? dict: + """What this code gets wrong about that token set. + + `tokens` is any sequence with `.name`, `.value_by_mode` and `.supersedes` — + resolved tokens, or stored ones. + + Reports rather than scores. Every finding here has a legitimate exception: + a snippet may target a system it isn't being checked against, and a literal + may be deliberate in a context the token doesn't cover. What it removes is + the SILENCE — all three currently fail with no signal at all. + """ + known = {getattr(t, "name", "") for t in tokens} + referenced = referenced_tokens(code) + + superseded: list[dict] = [] + for token in tokens: + for literal in getattr(token, "supersedes", None) or (): + if _literal_pattern(str(literal)).search(code or ""): + superseded.append({ + "literal": literal, + "use_instead": getattr(token, "name", ""), + }) + + return { + "used": sorted(referenced & known), + "unknown": sorted(referenced - known), + "superseded_literals": superseded, + "local_definitions": sorted(defined_tokens(code)), + } diff --git a/src/scribe/services/design_systems.py b/src/scribe/services/design_systems.py index bcb2757..e985380 100644 --- a/src/scribe/services/design_systems.py +++ b/src/scribe/services/design_systems.py @@ -21,7 +21,11 @@ from scribe.models import async_session from scribe.models.design_system import DesignSystem, DesignToken from scribe.models.project import Project from scribe.services import access -from scribe.services.design_stylesheet import duplicate_values, render_stylesheet +from scribe.services.design_stylesheet import ( + check_code_against_tokens, + duplicate_values, + render_stylesheet, +) from scribe.services.design_cascade import ( ResolvedToken, ancestry, @@ -437,3 +441,57 @@ async def stylesheet_for_system( "valueless": [t.name for t in resolved if not t.value_by_mode], "duplicates": duplicate_values(resolved), } + + +async def check_snippets_against_system( + user_id: int, design_system_id: int, project_id: int = 0 +) -> dict | None: + """Which recorded snippets disagree with this design system's sheet. + + The relation the operator named — "the snippets use the tags from the sheet" + — turned into a check. For each snippet: `var(--x)` references with no such + token, literals the sheet says to stop writing, and custom properties the + snippet mints for itself instead of using shared ones. + + Snippets with nothing to report are omitted entirely. A list of everything + that is fine is a list nobody reads twice. + """ + resolved = await resolve_design_system(user_id, design_system_id) + if resolved is None: + return None + + from scribe.services import snippets as snippets_svc + + # `list_snippets` returns (rows, total) and caps limit at 100; `project_id` + # must be None — not 0 — to reach across every project, since 0 would filter + # to a project with that id. + rows, _total = await snippets_svc.list_snippets( + user_id=user_id, project_id=project_id or None, limit=100, + ) + + findings: list[dict] = [] + for row in rows: + snippet_id = row.get("id") + if snippet_id is None: + continue + # The list rows carry a preview, not the code. The check has to read the + # whole body or it would report on a truncation. + note = await snippets_svc.get_snippet(user_id=user_id, snippet_id=int(snippet_id)) + if note is None: + continue + report = check_code_against_tokens(note.body or "", resolved) + if not ( + report["unknown"] or report["superseded_literals"] or report["local_definitions"] + ): + continue + findings.append({ + "snippet_id": int(snippet_id), + "title": note.title or "", + **report, + }) + + return { + "design_system_id": design_system_id, + "checked": len(rows), + "findings": findings, + } diff --git a/tests/test_design_stylesheet.py b/tests/test_design_stylesheet.py index df18171..a9bb674 100644 --- a/tests/test_design_stylesheet.py +++ b/tests/test_design_stylesheet.py @@ -8,6 +8,7 @@ declaration. from types import SimpleNamespace from scribe.services.design_stylesheet import ( + check_code_against_tokens, duplicate_values, is_valid_token_name, render_stylesheet, @@ -228,3 +229,93 @@ def test_valueless_tokens_never_count_as_duplicates_of_each_other(): """Otherwise every unfilled token would collide with every other one and the report would be nothing but noise on a fresh import.""" assert duplicate_values([_token("--fs-a", {}), _token("--fs-b", {})]) == {} + + +# --- reading the sheet from the other side ---------------------------------- +# +# "The snippets use the tags from the sheet" made checkable. All three findings +# below are currently SILENT in this codebase — no error, no failing test. + +def _tok(name, base=None, supersedes=()): + return SimpleNamespace( + name=name, + value_by_mode={"base": base} if base else {}, + supersedes=list(supersedes), + group_name=None, purpose=None, + ) + + +SHEET = [ + _tok("--fs-obsidian", "#14171a"), + _tok("--fs-parchment", "#e8e4d8", supersedes=["#fff", "#ffffff"]), +] + + +def test_a_reference_to_a_token_that_does_not_exist_is_reported(): + """THE bug this exists for. `var(--color-accent)` where no such token exists + renders as nothing at all — invisible focus rings, unstyled elements, no + error and no failing test. It happened in this codebase this session.""" + report = check_code_against_tokens("outline: 2px solid var(--color-accent);", SHEET) + assert report["unknown"] == ["--color-accent"] + assert report["used"] == [] + + +def test_a_reference_that_resolves_is_reported_as_used_not_as_a_finding(): + report = check_code_against_tokens("background: var(--fs-obsidian);", SHEET) + assert report["used"] == ["--fs-obsidian"] + assert report["unknown"] == [] + + +def test_a_superseded_literal_is_found_and_names_its_replacement(): + """The reframe paying off end to end: the finding says what to WRITE, not + merely what is wrong. That is only possible because the token declared it.""" + report = check_code_against_tokens("color: #fff;", SHEET) + assert report["superseded_literals"] == [ + {"literal": "#fff", "use_instead": "--fs-parchment"} + ] + + +def test_a_shorter_hex_does_not_match_inside_a_longer_one(): + """`#fff` must not fire on `#ffffff` — different colours, and a finding on + the wrong one sends someone to change code that was already correct.""" + report = check_code_against_tokens("color: #ffffffee;", SHEET) + assert [f["literal"] for f in report["superseded_literals"]] == [] + + +def test_the_literal_match_is_case_insensitive(): + """Rulebooks write `#FFFFFF` and code writes `#ffffff`. A case-sensitive + check would silently find nothing — the same trap normalize_hex exists for.""" + report = check_code_against_tokens("color: #FFFFFF;", SHEET) + assert report["superseded_literals"] == [ + {"literal": "#ffffff", "use_instead": "--fs-parchment"} + ] + + +def test_a_token_defined_and_read_locally_is_reported_once_not_twice(): + """A snippet minting its own custom property is the bloat a shared sheet + exists to prevent — but when it also reads it back, that is ONE fact. The + unknown-reference check owns it, because "--btn-bg does not exist in the + sheet" is the more precise statement of the same problem.""" + report = check_code_against_tokens(".btn { --btn-bg: #333; background: var(--btn-bg); }", SHEET) + assert report["unknown"] == ["--btn-bg"] + assert report["local_definitions"] == [] + + +def test_a_local_definition_never_read_back_is_still_reported(): + report = check_code_against_tokens(".btn { --btn-bg: #333; }", SHEET) + assert report["local_definitions"] == ["--btn-bg"] + + +def test_a_declaration_that_is_not_a_custom_property_is_not_mistaken_for_one(): + report = check_code_against_tokens(".btn { color: red; background: blue; }", SHEET) + assert report["local_definitions"] == [] + + +def test_clean_code_reports_nothing_to_act_on(): + report = check_code_against_tokens( + ".btn { background: var(--fs-obsidian); color: var(--fs-parchment); }", SHEET + ) + assert report["unknown"] == [] + assert report["superseded_literals"] == [] + assert report["local_definitions"] == [] + assert report["used"] == ["--fs-obsidian", "--fs-parchment"] diff --git a/tests/test_routes_design_systems.py b/tests/test_routes_design_systems.py index f53be22..190dd4e 100644 --- a/tests/test_routes_design_systems.py +++ b/tests/test_routes_design_systems.py @@ -25,6 +25,7 @@ def test_route_handlers_callable(): "list_design_systems", "create_design_system", "get_design_system", "update_design_system", "delete_design_system", "resolve_design_system", "import_design_system", "get_design_system_stylesheet", + "check_snippets_against_system", "list_design_tokens", "create_design_token", "update_design_token", "delete_design_token", "set_project_design_system", ): @@ -47,6 +48,7 @@ def test_every_endpoint_is_reachable_on_the_app(): "/api/design-systems//resolved", "/api/design-systems//import", "/api/design-systems//stylesheet", + "/api/design-systems//snippet-check", "/api/design-systems//tokens", "/api/design-tokens/", "/api/projects//design-system", @@ -61,7 +63,7 @@ def test_service_functions_take_user_id(): "update_design_system", "delete_design_system", "resolve_design_system", "create_token", "list_tokens", "update_token", "delete_token", "set_project_design_system", "import_from_rulebook", - "stylesheet_for_system", + "stylesheet_for_system", "check_snippets_against_system", ): fn = getattr(svc, fn_name) assert callable(fn) @@ -94,6 +96,8 @@ def test_agent_and_web_surfaces_stay_at_parity(): # so the loop above can't pair it. It still has to exist on both. assert callable(tools.import_design_system_from_rulebook) assert callable(routes.import_design_system) + assert callable(tools.check_snippets_against_design_system) + assert callable(routes.check_snippets_against_system) def test_every_mcp_tool_in_the_module_is_registered():