From 22f907c44d99cb29383cf35231dde8c74f74862e Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 3 Aug 2026 11:37:20 -0400 Subject: [PATCH] feat(design): offer starter token ROLES at creation, never values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A literal gets written into a stylesheet when there is no role to reach for. This codebase demonstrated it: the house style had no "text on a filled colour" role, so 76 call sites wrote a pure-white literal — not out of defiance, but because nothing existed to write instead. The correction was not a better ban list; it was declaring the missing role (#2275, #2349). So the useful moment is creation. A system whose roles are named on day one never presents the occasion. Ten groups, ~40 roles: surface, text, action, semantic, border, accent, radius, space, motion, state. Operator's call was one flat list, every group individually skippable — presets keyed to app shape (web / CLI / docs) were rejected because they need the product to hold opinions about app categories, and a wrong category is worse than a list someone prunes once. TWO BOUNDARIES THIS HAS TO HOLD, both rule #115: - The ROLES ship; the VALUES never do. Every seeded token has an empty value_by_mode, so a fresh system is a set of named, deliberately-unanswered questions. A test asserts no hex appears anywhere in the module — not just that tokens are blank, but that no palette hides in a comment waiting to be pasted in. - The PREFIX is the install's. `--fs-` is FabledSword's convention, not the product's; the default is a neutral `--ds-` and callers pass their own. Valueless roles are already legible downstream — render_stylesheet emits them as commented-out declarations and stylesheet_for_system reports them under `valueless` (#2299) — so "declared but undecided" reads correctly with nothing new built. Both surfaces, per rule #33: MCP gains starter_role_groups/token_prefix plus list_starter_role_groups(); REST gains the same on POST plus GET /api/design-systems/starter-roles. The parity enumeration is extended rather than loosened. Note create_design_system treats None and [] alike (seed nothing), while starter_tokens treats None as "all". Deliberate: creation must never write 40 rows into a system whose caller never asked, and the everything-checked default belongs in the UI where the operator can see it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs --- src/scribe/mcp/tools/design_systems.py | 35 ++++ src/scribe/routes/design_systems.py | 19 ++ src/scribe/services/design_starter_roles.py | 185 ++++++++++++++++++++ src/scribe/services/design_systems.py | 21 +++ tests/test_design_starter_roles.py | 136 ++++++++++++++ tests/test_routes_design_systems.py | 2 +- 6 files changed, 397 insertions(+), 1 deletion(-) create mode 100644 src/scribe/services/design_starter_roles.py create mode 100644 tests/test_design_starter_roles.py diff --git a/src/scribe/mcp/tools/design_systems.py b/src/scribe/mcp/tools/design_systems.py index c8af5db..63bb7e3 100644 --- a/src/scribe/mcp/tools/design_systems.py +++ b/src/scribe/mcp/tools/design_systems.py @@ -22,6 +22,11 @@ from __future__ import annotations from scribe.mcp._context import current_user_id from scribe.services import design_systems as ds_svc from scribe.services.design_systems import DesignSystemCycle +from scribe.services.design_starter_roles import ( + ALL_GROUPS, + DEFAULT_TOKEN_PREFIX, + describe_groups, +) async def create_design_system( @@ -29,6 +34,8 @@ async def create_design_system( description: str = "", guidance: str = "", parent_id: int = 0, + starter_role_groups: list[str] | None = None, + token_prefix: str = "", ) -> dict: """Create a design system, optionally inheriting from another. @@ -41,20 +48,47 @@ async def create_design_system( parent_id: Inherit from this system — it holds the defaults this one overrides. Omit (0) for a top-level "family" system, which is what a first design system usually is. + starter_role_groups: Seed the system with named but VALUELESS token + roles, so there is something to reach for before a literal gets + written instead. Call list_starter_role_groups() for the catalogue. + Pass ["all"] for every group. Omit for none — a system with three + hand-written tokens is a legitimate design system. + token_prefix: Naming convention for the seeded roles, e.g. "--fs-". + Defaults to a neutral "--ds-"; pass the install's own if it has one. + Ignored when no starter groups are requested. """ uid = current_user_id() + groups = starter_role_groups + if groups and len(groups) == 1 and groups[0] == "all": + groups = list(ALL_GROUPS) system = await ds_svc.create_design_system( uid, title=title, description=description or None, guidance=guidance or None, parent_id=parent_id or None, + starter_role_groups=groups, + token_prefix=token_prefix or DEFAULT_TOKEN_PREFIX, ) if system is None: raise ValueError(f"parent design system {parent_id} not found or not writable") return system.to_dict() +async def list_starter_role_groups() -> dict: + """The starter token ROLES offered at design-system creation. + + Roles, not values. Every group is a set of named questions — "page + background, the deepest surface" — that the operator answers with their own + palette. Nothing here carries a colour, because a default palette would be + one install's taste shipped as product. + + Reach for this before create_design_system so the choice is informed, and + pass the group names you want as `starter_role_groups`. + """ + return {"groups": describe_groups(), "default_prefix": DEFAULT_TOKEN_PREFIX} + + async def list_design_systems() -> dict: """List your design systems. An empty list is normal — most installs have none.""" uid = current_user_id() @@ -350,6 +384,7 @@ async def set_project_design_system(project_id: int, design_system_id: int = 0) def register(mcp) -> None: for fn in ( create_design_system, + list_starter_role_groups, list_design_systems, get_design_system, resolve_design_system, diff --git a/src/scribe/routes/design_systems.py b/src/scribe/routes/design_systems.py index 6487ed5..1bb3628 100644 --- a/src/scribe/routes/design_systems.py +++ b/src/scribe/routes/design_systems.py @@ -19,6 +19,10 @@ from quart import Blueprint, g, jsonify, request from scribe.auth import login_required from scribe.services import design_systems as ds_svc +from scribe.services.design_starter_roles import ( + DEFAULT_TOKEN_PREFIX, + describe_groups, +) from scribe.services.design_systems import DesignSystemCycle design_systems_bp = Blueprint("design_systems", __name__, url_prefix="/api") @@ -56,12 +60,27 @@ async def create_design_system(): description=data.get("description") or None, guidance=data.get("guidance") or None, parent_id=data.get("parent_id"), + starter_role_groups=data.get("starter_role_groups"), + token_prefix=data.get("token_prefix") or DEFAULT_TOKEN_PREFIX, ) if system is None: return jsonify({"error": "parent design system not found"}), 404 return jsonify(system.to_dict()), 201 +@design_systems_bp.get("/design-systems/starter-roles") +@login_required +async def list_starter_role_groups(): + """The starter role catalogue, for the creation form's checklist. + + Roles and purposes only — no values, ever. See services/design_starter_roles. + """ + return jsonify({ + "groups": describe_groups(), + "default_prefix": DEFAULT_TOKEN_PREFIX, + }) + + @design_systems_bp.get("/design-systems/") @login_required async def get_design_system(design_system_id: int): diff --git a/src/scribe/services/design_starter_roles.py b/src/scribe/services/design_starter_roles.py new file mode 100644 index 0000000..ab9c456 --- /dev/null +++ b/src/scribe/services/design_starter_roles.py @@ -0,0 +1,185 @@ +"""A starter set of token ROLES, offered when a design system is created. + +WHY THIS EXISTS +--------------- +A literal gets written into a stylesheet when there is no role to reach for. +That is the mechanism, and this codebase produced a clean demonstration of it: +the house style had no "text on a filled colour" role, so 76 call sites wrote +a pure-white literal — not out of defiance, but because nothing existed to write +instead (#2275). The correction was not a better ban list. It was declaring the +missing role. + +So the useful moment is CREATION. A system whose roles are named on day one +never presents the occasion for a literal, and never needs a list of values it +forbids. + +WHAT SHIPS AND WHAT DOES NOT (rule #115) +---------------------------------------- +The ROLES ship: `surface-page`, `text-primary`, `action-destructive` are +generic CSS-design vocabulary, not one operator's kit. Every install that has a +page has a page background. + +The VALUES never ship. Each token is created with an empty `value_by_mode`, so +a fresh system is a set of named, deliberately-unanswered questions. No hex +appears anywhere in this file, and none should ever be added to it — a default +palette would be this operator's palette wearing product clothes. + +A valueless token is already legible downstream: `render_stylesheet` emits it as +a commented-out declaration in its group (#2299), and `stylesheet_for_system` +reports it under `valueless`. So a blank role reads as "to be decided" rather +than as breakage, without anything new. + +THE PREFIX IS THE INSTALL'S +--------------------------- +`--fs-` is FabledSword's convention, not the product's. The prefix is a +parameter with a neutral default; a caller that has a house convention passes +it. Baking `--fs-` in would put one family's naming into every install. + +FLAT, NOT PRESET +---------------- +One list, every group individually skippable, all on by default (operator's +call, 2026-08-03). Presets keyed to app shape — web / CLI / docs — were +considered and rejected: they would require the product to hold opinions about +app categories, and a wrong category is worse than a generic list someone +prunes once. +""" +from __future__ import annotations + +DEFAULT_TOKEN_PREFIX = "--ds-" + +# group -> (what the group is for, ((role suffix, purpose), ...)) +# +# Purposes are written as the QUESTION the operator is answering, because that +# is what an unfilled role is. "Page background, the deepest surface" tells you +# what to put there; "Colour 1" does not. +STARTER_ROLE_GROUPS: dict[str, tuple[str, tuple[tuple[str, str], ...]]] = { + "surface": ( + "Backgrounds, by elevation", + ( + ("surface-page", "Page background, the deepest surface"), + ("surface-raised", "Cards and raised elements"), + ("surface-hover", "Hovered surfaces, secondary elevation"), + ), + ), + "text": ( + "Foreground colours, by emphasis", + ( + ("text-primary", "Primary text on a page or raised surface"), + ("text-secondary", "Secondary text and captions"), + ("text-tertiary", "Hints and metadata"), + # The role whose absence caused 76 literals. It is in the starter + # set deliberately: text on a filled colour is NOT the page text + # colour, because the surface under it does not change with the + # mode while the page does. + ("text-on-action", "Text on a filled colour — buttons, badges"), + ), + ), + "action": ( + "What the user can do — kept separate from the accent, which is identity", + ( + ("action-primary", "The confirming action: Save, Submit"), + ("action-secondary", "Non-destructive alternates"), + ("action-destructive", "Irreversible actions — delete, revoke"), + ), + ), + "semantic": ( + "What the system is telling you", + ( + ("success", "Something worked"), + ("warning", "Something needs attention"), + ("error", "Something failed — distinct from destructive"), + ("info", "Neutral information"), + ), + ), + "border": ( + "Boundaries and dividers", + ( + ("border-color", "The line colour itself"), + ("border", "The default structural border, as a shorthand"), + ("border-hover", "Border on hover or emphasis"), + ("border-active", "Selected or current — the one border that may carry the accent"), + ), + ), + "accent": ( + "This install's identity — not its actions", + ( + ("accent", "The single signature colour"), + ("accent-soft", "Tinted backgrounds — pills, tags"), + ("accent-faint", "The faintest wash"), + ), + ), + "radius": ( + "Corner rounding", + ( + ("radius-sm", "Pills, tags, code spans"), + ("radius-md", "Buttons, inputs, small cards"), + ("radius-lg", "Cards, panels, modals"), + ), + ), + "space": ( + "The spacing scale — a gap not on the scale is a decision to justify", + tuple((f"space-{i}", f"Spacing step {i}") for i in range(1, 11)), + ), + "motion": ( + "Transition timing — motion supports the interaction, never performs", + ( + ("ease", "The one easing curve, used by every transition"), + ("dur-fast", "Hovers, colour and border changes"), + ("dur-base", "Most state changes"), + ("dur-slow", "Larger surface or layout shifts"), + ), + ), + "state": ( + "Cross-cutting states that are otherwise improvised per view", + ( + ("disabled-opacity", "Opacity for disabled controls"), + ("overlay", "Scrim behind modals and dialogs"), + ), + ), +} + +ALL_GROUPS: tuple[str, ...] = tuple(STARTER_ROLE_GROUPS) + + +def starter_tokens( + groups: list[str] | tuple[str, ...] | None = None, + prefix: str = DEFAULT_TOKEN_PREFIX, +) -> list[dict]: + """Token rows for the chosen groups — names and purposes only, no values. + + `groups` of None means every group; an empty list means none, which is a + real answer and not the same as None. An operator who wants three tokens + should be able to get three. + + Unknown group names are ignored rather than raising: this feeds a + checkbox list, and a stale name from an older client should not fail a + creation that is otherwise fine. + """ + chosen = ALL_GROUPS if groups is None else [g for g in groups if g in STARTER_ROLE_GROUPS] + rows: list[dict] = [] + for group in chosen: + _, roles = STARTER_ROLE_GROUPS[group] + for index, (suffix, purpose) in enumerate(roles, start=1): + rows.append({ + "name": f"{prefix}{suffix}", + "group_name": group, + "purpose": purpose, + # Empty, not absent: the column is NOT NULL with a {} default, + # so absence has exactly one spelling here as it does there. + "value_by_mode": {}, + "order_index": index, + }) + return rows + + +def describe_groups() -> list[dict]: + """The catalogue, for a UI to render as a checklist.""" + return [ + { + "group": group, + "description": description, + "token_count": len(roles), + "names": [suffix for suffix, _ in roles], + } + for group, (description, roles) in STARTER_ROLE_GROUPS.items() + ] diff --git a/src/scribe/services/design_systems.py b/src/scribe/services/design_systems.py index e4bb63b..522829d 100644 --- a/src/scribe/services/design_systems.py +++ b/src/scribe/services/design_systems.py @@ -27,6 +27,10 @@ from scribe.services.design_stylesheet import ( duplicate_values, render_stylesheet, ) +from scribe.services.design_starter_roles import ( + DEFAULT_TOKEN_PREFIX, + starter_tokens, +) from scribe.services.design_cascade import ( ResolvedToken, ancestry, @@ -79,11 +83,23 @@ async def create_design_system( description: str | None = None, guidance: str | None = None, parent_id: int | None = None, + starter_role_groups: list[str] | None = None, + token_prefix: str = DEFAULT_TOKEN_PREFIX, ) -> DesignSystem | None: """Create a system, with or without a parent. Returns None when `parent_id` names a system the caller may not write — which, per the ACL, means one they do not own. + + `starter_role_groups` seeds the system with named, VALUELESS token roles + (#2349) — the moment a role is missing is the moment a literal gets written + instead, so the cheapest time to name them is now. Pass a list of group + names to choose, `[]` for none, or None for none. + + None and `[]` deliberately mean the same thing here, unlike in + `starter_tokens` where None means "all": creation must not seed 40 rows + into a system whose caller never asked. Opting in is the caller's job, and + the UI's default of everything-checked lives in the UI. """ if parent_id is not None and not await access.can_write_design_system( user_id, parent_id @@ -100,6 +116,11 @@ async def create_design_system( session.add(system) await session.commit() await session.refresh(system) + + if starter_role_groups: + for row in starter_tokens(starter_role_groups, prefix=token_prefix): + session.add(DesignToken(design_system_id=system.id, **row)) + await session.commit() return system diff --git a/tests/test_design_starter_roles.py b/tests/test_design_starter_roles.py new file mode 100644 index 0000000..7f9e79c --- /dev/null +++ b/tests/test_design_starter_roles.py @@ -0,0 +1,136 @@ +"""The starter role set (#2349). + +The premise: a literal gets written when there is no role to reach for. This +codebase demonstrated it — the house style had no "text on a filled colour" +role, so 76 call sites wrote `color: #fff` (#2275). Naming the roles at +creation removes the occasion. + +The tests that matter here are about the BOUNDARY, not the content. Roles ship +with the product; values never do. A default palette would be one operator's +taste shipped as product code (rule #115), and it would be very easy to add by +accident while "being helpful". +""" +import re + +import pytest + +from scribe.services import design_starter_roles as roles + + +def test_no_role_carries_a_VALUE(): + """THE rule-115 guard. Every seeded token is a named question with no + answer. The moment one ships a hex, the product is prescribing an install's + palette.""" + for row in roles.starter_tokens(): + assert row["value_by_mode"] == {}, f"{row['name']} shipped a value" + + +def _colour_literals(text: str) -> list[str]: + """Hex colours in `text`, NOT counting issue references. + + `#2275` is four hex-valid digits and also how this codebase cites an issue — + so the naive pattern flags its own documentation, which is the third time + that has happened here (#2353). A real colour either contains a letter + a–f or is a full 6/8-digit value; an all-decimal 3- or 4-digit match is an + issue number. + """ + out = [] + for m in re.findall(r"#[0-9a-fA-F]{3,8}\b", text): + digits = m[1:] + if len(digits) not in (3, 4, 6, 8): + continue + if len(digits) in (6, 8) or any(c in "abcdefABCDEF" for c in digits): + out.append(m) + return out + + +def test_the_module_contains_no_colour_literals_at_all(): + """Belt and braces on the above, and the stronger claim: not just that + tokens are blank, but that no palette hides in a comment or a docstring + waiting to be pasted in. Checks the SOURCE, not the output.""" + import pathlib + src = pathlib.Path(roles.__file__).read_text() + hexes = _colour_literals(src) + assert not hexes, f"colour literals in product code: {hexes}" + + +def test_the_colour_check_does_not_flag_issue_references(): + """Pins the exclusion above, because without it this file fails on its own + citations and the obvious 'fix' is to delete the check.""" + assert _colour_literals("see #2275 and #2349") == [] + assert _colour_literals("color: #fff") == ["#fff"] + assert _colour_literals("#E8E4D8 on #14171A") == ["#E8E4D8", "#14171A"] + assert _colour_literals("#000000") == ["#000000"] + + +def test_every_group_is_individually_selectable(): + """Operator's call: one flat list, all skippable. An install that wants + three tokens must be able to get three.""" + only_text = roles.starter_tokens(["text"]) + assert {r["group_name"] for r in only_text} == {"text"} + assert len(only_text) == 4 + + +def test_empty_selection_yields_nothing_and_is_not_the_same_as_None(): + """`[]` is a real answer — "none of them" — and must not be read as + "unspecified, so give me everything". Getting this backwards would seed 40 + rows into a system whose creator explicitly declined.""" + assert roles.starter_tokens([]) == [] + assert len(roles.starter_tokens(None)) > 30 + + +def test_unknown_group_names_are_ignored_not_fatal(): + """This feeds a checkbox list. A stale name from an older client should not + fail an otherwise-fine creation.""" + out = roles.starter_tokens(["text", "not-a-real-group"]) + assert {r["group_name"] for r in out} == {"text"} + + +def test_the_prefix_is_the_installs_choice(): + """`--fs-` is FabledSword's convention, not the product's. Baking it in + would put one family's naming into every install.""" + assert all(r["name"].startswith("--ds-") for r in roles.starter_tokens(["text"])) + custom = roles.starter_tokens(["text"], prefix="--acme-") + assert all(r["name"].startswith("--acme-") for r in custom) + assert "--acme-text-primary" in {r["name"] for r in custom} + + +def test_text_on_action_is_in_the_starter_set(): + """The specific role whose absence produced 76 literals. It is separate + from text-primary on purpose: the surfaces it sits on do not change with + the mode, while the page does — so reusing text-primary there passes in + dark and fails contrast in light (#2275).""" + names = {r["name"] for r in roles.starter_tokens(["text"])} + assert "--ds-text-on-action" in names + assert "--ds-text-primary" in names + + +def test_names_are_valid_custom_properties(): + """They go straight into a stylesheet; an invalid name is a silent no-op + rather than an error, which is the worst failure mode available.""" + valid = re.compile(r"^--[A-Za-z0-9_-]+$") + for row in roles.starter_tokens(): + assert valid.match(row["name"]), row["name"] + + +def test_no_duplicate_names_across_the_whole_set(): + """A design system has a partial-unique index on (system, name); a + duplicate in the starter set would make creation fail at the DB with a + constraint error rather than anything legible.""" + names = [r["name"] for r in roles.starter_tokens()] + assert len(names) == len(set(names)) + + +def test_every_role_states_a_purpose(): + """An unfilled role is only useful if it says what belongs there. "Colour + 1" is a blank with extra steps.""" + for row in roles.starter_tokens(): + assert row["purpose"].strip(), row["name"] + + +def test_describe_groups_matches_what_starter_tokens_produces(): + """The catalogue a UI renders and the rows creation writes must not drift — + a checklist offering a group that seeds nothing is a lie in the UI.""" + described = {g["group"]: g["token_count"] for g in roles.describe_groups()} + for group, count in described.items(): + assert len(roles.starter_tokens([group])) == count diff --git a/tests/test_routes_design_systems.py b/tests/test_routes_design_systems.py index 8b0b54a..eca7396 100644 --- a/tests/test_routes_design_systems.py +++ b/tests/test_routes_design_systems.py @@ -85,7 +85,7 @@ def test_agent_and_web_surfaces_stay_at_parity(): "resolve_design_system", "update_design_system", "delete_design_system", "create_design_token", "list_design_tokens", "update_design_token", "delete_design_token", "set_project_design_system", - "get_design_system_stylesheet", + "get_design_system_stylesheet", "list_starter_role_groups", ): assert callable(getattr(tools, name)), f"MCP tool missing: {name}" assert callable(getattr(routes, name)), f"REST route missing: {name}"