From df18e897af4b1b5bf8452b4dc95adbd13f6bc3e1 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 23 Aug 2026 21:27:27 -0400 Subject: [PATCH] feat(ledger): the consumer map reads transition names and concatenated prefixes (#2970) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Paying down #2962 measured `flag="unused-css"` against a hand audit and found it had a permanent false-positive floor: roughly sixty classes it called unused are alive and always would be, because two ordinary authoring forms produce names no reader of `class=` attributes can see. A flag whose list you cannot act on line by line is worse than no flag — act on it and you delete live UI. - A transition `name=` IS a class reference. `` makes Vue apply `.toast-enter-active` and its siblings at runtime, and React's `` does the same with a different suffix set. Every spelling of the tag is read (`Transition`, `TransitionGroup`, `transition-group`), and the emitted suffix set is the union of Vue 3, Vue 2 and React: naming a class no rule defines costs nothing, since it resolves to no row. A bound `:name` stays unknowable. - A concatenated name contributes its static head as a PREFIX reference. `` `status-${s}` ``, `'pri-' + p` and `class="card-{{ v }}"` all leave a head behind once the hole is blanked — and `_CLASS_TOKEN_RE` accepts a trailing hyphen, so until now the extractor emitted a junk token `"status-"` that matched nothing. It is now `status-*`, and resolve_consumers credits every row whose symbol starts with that head, each under the same own-file-else-fan-out rule as an exact token. `*` cannot occur in a class token, so the marker rides the existing dict[str, int] with no schema change. A head shorter than two characters says nothing and is dropped. Crediting every candidate row is the honest reading: the template genuinely does not say which one it built, and the alternative is reporting live rules as dead. What the map still cannot see is a name assembled in a script — `classList.add` — which stays deliberately out of scope; the skill, the `flag=` docstring and the refresh payload docs all say so now. Co-Authored-By: Claude Fable 5 --- plugin/skills/shape-accounting/SKILL.md | 6 ++- src/scribe/mcp/tools/shapes.py | 10 +++-- src/scribe/services/coverage.py | 57 +++++++++++++++++++++++-- src/scribe/services/shape_ledger.py | 33 +++++++++++--- tests/test_pattern_coverage.py | 39 +++++++++++++++++ tests/test_shape_ledger.py | 34 +++++++++++++++ 6 files changed, 164 insertions(+), 15 deletions(-) diff --git a/plugin/skills/shape-accounting/SKILL.md b/plugin/skills/shape-accounting/SKILL.md index e9196a3..23a2ccf 100644 --- a/plugin/skills/shape-accounting/SKILL.md +++ b/plugin/skills/shape-accounting/SKILL.md @@ -125,7 +125,11 @@ the last sweep left it. Three surfaces say so without anyone running an audit templates, one recipe → derive; one template each, different purposes → dismiss. `list_shapes(flag="unused-css")` is the map's negative space — css rules no template names, a deletion candidate to look at, never - auto-deleted (a class built at runtime is invisible to the map). + auto-deleted. The map reads the two class forms templates don't spell out + — a ``'s generated classes, and the prefix of a + concatenated name (`` `status-${s}` `` credits every `status-…` rule) — + so what it flags is worth reading. What it still cannot see is a name + assembled in a script (`classList.add`), so confirm before deleting. After the one-time pay-down the derive queue reads empty; anything in it afterwards is drift of the moment, and the hint already said so at the write. diff --git a/src/scribe/mcp/tools/shapes.py b/src/scribe/mcp/tools/shapes.py index 3ba6e6d..c1d5dd3 100644 --- a/src/scribe/mcp/tools/shapes.py +++ b/src/scribe/mcp/tools/shapes.py @@ -118,8 +118,9 @@ async def list_shapes( whose body changed since judged (the judgment stands; confirm it again with classify_shapes, or re-judge); "unused-css" (milestone 302): live css rules no file's markup names — a - deletion candidate to look at, never auto-deleted (the map reads - templates only; a class built at runtime is invisible to it). + deletion candidate to look at, never auto-deleted. Transition + classes and concatenated names are read (#2970), so the list is + worth acting on; a name assembled in a script still is not. Returns {"shapes": [...], "total": N} — total counts every match, not just this page. Every css row carries `used_by` {count, paths} — the @@ -305,8 +306,9 @@ async def refresh_pattern_coverage(project_id: int) -> dict: unclassified, repos, largest_gaps, `proposed` (canon proposals awaiting confirmation), `derive_groups` (the biggest repeats-with-no-canon families, each css one with `consumers` — the files whose markup - render it, milestone 302), `unused_css` (css rules no template names; - None where the map has no evidence of templates), `derive_new` (copies + render it, milestone 302), `unused_css` (css rules no template names — + counting a ``'s generated classes and concatenated + names as named, #2970; None where the map has no evidence of templates), `derive_new` (copies that joined a family since the previous refresh — the drift to act on now: derive the canon, don't queue an audit), `proposer` (what this refresh examined) — plus diff --git a/src/scribe/services/coverage.py b/src/scribe/services/coverage.py index dcb31ae..a16ff7a 100644 --- a/src/scribe/services/coverage.py +++ b/src/scribe/services/coverage.py @@ -294,6 +294,34 @@ _DYNAMIC_CLASS_RE = re.compile( ) # Svelte's directive form: class:active={cond}. _SVELTE_CLASS_RE = re.compile(r"(? makes Vue add .toast-enter-active et al at +# runtime, and React's does the same. A +# reader of `class=` attributes alone therefore calls every one of those +# rules unused, which is a false positive no amount of care in the +# stylesheet can avoid (#2970). A dynamic `:name="…"` stays unknowable. +_TRANSITION_NAME_RE = re.compile( + r"""<\s*[Tt]ransition(?:-[Gg]roup|Group)?\b[^>]*?(? list[str]: """The class tokens of a static attribute value: whitespace-split, only - well-formed names (an interpolation like `{{ cls }}` contributes none).""" - return [t for t in _MUSTACHE_RE.sub(" ", value).split() if _CLASS_TOKEN_RE.match(t)] + well-formed names. An interpolation (`{{ cls }}`, `${cls}`) is blanked + before the split, so a name built around one leaves its static head — + `status-` from `status-{{ s }}` — which is emitted as the prefix + reference `status-*` rather than as a class nothing is called.""" + out: list[str] = [] + for t in _MUSTACHE_RE.sub(" ", value).split(): + if not _CLASS_TOKEN_RE.match(t): + continue + if t.endswith(("-", "_")): + if len(t.rstrip("-_")) >= _PREFIX_MIN_STEM: + out.append(t + PREFIX_MARK) + continue + out.append(t) + return out def _dynamic_class_tokens(expr: str) -> list[str]: @@ -336,7 +376,13 @@ def class_references(path: str, text: str) -> dict[str, int]: files that carry no markup (by suffix). Reads the static `class=` / `className=` attributes, the Vue and React dynamic forms and Svelte's `class:x` directive; never a CSS selector (`.x {` is a definition, read - by extract_definitions) and never a script's `querySelector('.x')`.""" + by extract_definitions) and never a script's `querySelector('.x')`. + + Two forms name classes without spelling them out, and both are read + (#2970): a transition `name=` stands for every class the framework + generates from it, and a concatenated name contributes the prefix + reference `head-*` — which resolve_consumers matches against every row + whose symbol starts with `head-`.""" if not (path or "").lower().endswith(_TEMPLATE_SUFFIXES): return {} counts: dict[str, int] = {} @@ -351,6 +397,11 @@ def class_references(path: str, text: str) -> dict[str, int]: expr = next((g for g in m.groups() if g is not None), "") bump(_dynamic_class_tokens(expr)) bump([m.group(1) for m in _SVELTE_CLASS_RE.finditer(text)]) + for m in _TRANSITION_NAME_RE.finditer(text): + name = next((g for g in m.groups() if g is not None), "").strip() + if not _CLASS_TOKEN_RE.match(name): + continue + bump([name + suffix for suffix in _TRANSITION_SUFFIXES]) return counts diff --git a/src/scribe/services/shape_ledger.py b/src/scribe/services/shape_ledger.py index 3d155bc..02c9d33 100644 --- a/src/scribe/services/shape_ledger.py +++ b/src/scribe/services/shape_ledger.py @@ -279,20 +279,39 @@ def resolve_consumers( of that name when F defines it (a scoped rule is consumed by its own template); otherwise to every other file's row of that name — a shared sheet, or, when several files define it, all of them: the map says - "ambiguous" by fanning out rather than guessing one.""" + "ambiguous" by fanning out rather than guessing one. + + A token ending in ``PREFIX_MARK`` is a PREFIX reference (#2970) — the + static head of a name the template concatenates, `status-*` from + `` `status-${s}` ``. It stands for every row whose symbol starts with + that head, each resolved by the same own-file-else-fan-out rule. The + template cannot tell us WHICH of them it built, so the map credits all + of them rather than calling live rules unused.""" + # Lazy, like the extract_definitions import below: coverage reaches into + # this module during a refresh, so neither may import the other at load. + from scribe.services.coverage import PREFIX_MARK + by_symbol: dict[str, list[tuple[int, str]]] = {} for sid, path, symbol in css_rows: by_symbol.setdefault(symbol, []).append((sid, path)) out: dict[tuple[int, str], int] = {} + + def credit(rows: list[tuple[int, str]], consumer: str, count: int) -> None: + own = [sid for sid, path in rows if path == consumer] + for sid in own or [sid for sid, _path in rows]: + out[(sid, consumer)] = out.get((sid, consumer), 0) + int(count) + for consumer, tokens in references.items(): for token, count in tokens.items(): - rows = by_symbol.get(token) - if not rows: + if token.endswith(PREFIX_MARK): + head = token[: -len(PREFIX_MARK)] + for symbol, rows in by_symbol.items(): + if symbol.startswith(head): + credit(rows, consumer, count) continue - own = [sid for sid, path in rows if path == consumer] - targets = own or [sid for sid, _path in rows] - for sid in targets: - out[(sid, consumer)] = out.get((sid, consumer), 0) + int(count) + rows = by_symbol.get(token) + if rows: + credit(rows, consumer, count) return out diff --git a/tests/test_pattern_coverage.py b/tests/test_pattern_coverage.py index f94a15c..d1cec30 100644 --- a/tests/test_pattern_coverage.py +++ b/tests/test_pattern_coverage.py @@ -152,6 +152,45 @@ def test_class_references_reads_vue_static_and_dynamic_forms_only(): } +def test_class_references_reads_framework_transition_names(): + """A transition `name=` is a class reference: the framework applies + `.toast-enter-active` and friends at runtime, so a stylesheet that + defines them is consumed even though no template spells one out (#2970). + Every spelling of the tag counts; a bound `:name` stays unknowable.""" + refs = class_references( + "a/T.vue", + '
', + ) + for suffix in ("-enter-from", "-enter-active", "-leave-to", "-move"): + assert refs["toast" + suffix] == 1, suffix + assert refs["toast-item"] == 1 # the static attribute still counts + + assert "peek-enter-active" in class_references("a/T.vue", '') + assert "g-move" in class_references("a/T.vue", '') + # React's CSSTransition names the same idea with a different suffix set + react = class_references("a/T.jsx", '') + assert {"fade-enter-active", "fade-exit-active", "fade-exit-done"} <= set(react) + # A bound name is a variable, not a name we can read + assert class_references("a/T.vue", '') == {} + + +def test_class_references_reads_a_concatenated_name_as_a_prefix(): + """`status-${s}` cannot be resolved to one class, but its static head is + real information: it is emitted as the prefix reference `status-*` so + the rows it could have built are not reported unused (#2970). A head too + short to mean anything, or a bare separator, says nothing.""" + vue = ( + '
' + "" + '' + ) + assert class_references("a/P.vue", vue) == {"status-*": 1, "pri-*": 1} + # a server template interpolating into the middle of a name, same reading + assert class_references("t/p.html", '') == {"card-*": 1} + # an ordinary name never picks up the marker + assert class_references("a/P.vue", '
') == {"page-header": 1} + + def test_class_references_reads_react_svelte_and_server_templates(): tsx = ( "export function X({ on }: { on: boolean }) {\n" diff --git a/tests/test_shape_ledger.py b/tests/test_shape_ledger.py index 6ab47a9..70fc648 100644 --- a/tests/test_shape_ledger.py +++ b/tests/test_shape_ledger.py @@ -473,6 +473,40 @@ def test_resolve_consumers_prefers_the_own_file_and_fans_out_for_shared_names(): assert all(sid != 6 for sid, _ in resolve_consumers(css_rows, refs)) +def test_resolve_consumers_credits_every_row_a_prefix_could_have_built(): + """#2970: `status-${s}` names a class the map cannot pin down, so the + prefix reference `status-*` credits every row whose symbol starts with + the head — each under the same own-file-else-fan-out rule. Crediting all + of them is the honest reading: the alternative is calling live rules + unused, which is what the flag existed to avoid.""" + from scribe.services.shape_ledger import resolve_consumers + css_rows = [ + (1, "assets/app.css", "status-done"), + (2, "assets/app.css", "status-todo"), + (3, "v/Board.vue", "status-done"), # a scoped copy of one of them + (4, "assets/app.css", "btn-primary"), + (5, "assets/anim.css", "toast-enter-active"), + ] + # A file that defines none of them fans out across every match. + assert resolve_consumers(css_rows, {"v/List.vue": {"status-*": 2}}) == { + (1, "v/List.vue"): 2, (2, "v/List.vue"): 2, (3, "v/List.vue"): 2, + } + # A file that DOES define one keeps the own-file rule, per symbol: its own + # status-done row, and the shared status-todo it does not define. + assert resolve_consumers(css_rows, {"v/Board.vue": {"status-*": 1}}) == { + (3, "v/Board.vue"): 1, (2, "v/Board.vue"): 1, + } + # A prefix that matches nothing is silent, and exact tokens are untouched. + assert resolve_consumers(css_rows, {"v/X.vue": {"zz-*": 1}}) == {} + assert resolve_consumers(css_rows, {"v/X.vue": {"btn-primary": 3}}) == { + (4, "v/X.vue"): 3, + } + # A transition class resolves exactly, like any other name. + assert resolve_consumers(css_rows, {"c/Toast.vue": {"toast-enter-active": 1}}) == { + (5, "c/Toast.vue"): 1, + } + + def test_consumer_edges_table_cascades_with_the_shape(): from scribe.models import Base from scribe.models.code_shape import CONSUMER_BASES, CodeShapeConsumer