feat(design): offer starter token ROLES at creation, never values
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 16s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Failing after 46s
CI & Build / Build & push image (push) Skipped
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 16s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Failing after 46s
CI & Build / Build & push image (push) Skipped
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs
This commit is contained in:
@@ -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
|
||||
@@ -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}"
|
||||
|
||||
Reference in New Issue
Block a user