feat(ledger): the consumer map reads transition names and concatenated prefixes (#2970)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Failing after 10s
CI & Build / integration (push) Successful in 26s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 23s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Failing after 10s
CI & Build / integration (push) Successful in 26s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 23s
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. `<Transition name="toast">` makes Vue apply `.toast-enter-active` and its siblings at runtime, and React's `<CSSTransition classNames="fade">` 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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 →
|
templates, one recipe → derive; one template each, different purposes →
|
||||||
dismiss. `list_shapes(flag="unused-css")` is the map's negative space —
|
dismiss. `list_shapes(flag="unused-css")` is the map's negative space —
|
||||||
css rules no template names, a deletion candidate to look at, never
|
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 `<Transition name="x">`'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
|
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.
|
afterwards is drift of the moment, and the hint already said so at the write.
|
||||||
|
|||||||
@@ -118,8 +118,9 @@ async def list_shapes(
|
|||||||
whose body changed since judged (the judgment stands; confirm
|
whose body changed since judged (the judgment stands; confirm
|
||||||
it again with classify_shapes, or re-judge); "unused-css"
|
it again with classify_shapes, or re-judge); "unused-css"
|
||||||
(milestone 302): live css rules no file's markup names — a
|
(milestone 302): live css rules no file's markup names — a
|
||||||
deletion candidate to look at, never auto-deleted (the map reads
|
deletion candidate to look at, never auto-deleted. Transition
|
||||||
templates only; a class built at runtime is invisible to it).
|
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
|
Returns {"shapes": [...], "total": N} — total counts every match, not
|
||||||
just this page. Every css row carries `used_by` {count, paths} — the
|
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
|
unclassified, repos, largest_gaps, `proposed` (canon proposals awaiting
|
||||||
confirmation), `derive_groups` (the biggest repeats-with-no-canon
|
confirmation), `derive_groups` (the biggest repeats-with-no-canon
|
||||||
families, each css one with `consumers` — the files whose markup
|
families, each css one with `consumers` — the files whose markup
|
||||||
render it, milestone 302), `unused_css` (css rules no template names;
|
render it, milestone 302), `unused_css` (css rules no template names —
|
||||||
None where the map has no evidence of templates), `derive_new` (copies
|
counting a `<Transition name=>`'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
|
that joined a family since the previous
|
||||||
refresh — the drift to act on now: derive the canon, don't queue an
|
refresh — the drift to act on now: derive the canon, don't queue an
|
||||||
audit), `proposer` (what this refresh examined) — plus
|
audit), `proposer` (what this refresh examined) — plus
|
||||||
|
|||||||
@@ -294,6 +294,34 @@ _DYNAMIC_CLASS_RE = re.compile(
|
|||||||
)
|
)
|
||||||
# Svelte's directive form: class:active={cond}.
|
# Svelte's directive form: class:active={cond}.
|
||||||
_SVELTE_CLASS_RE = re.compile(r"(?<![:\w.-])class:([A-Za-z_][A-Za-z0-9_-]*)\s*=")
|
_SVELTE_CLASS_RE = re.compile(r"(?<![:\w.-])class:([A-Za-z_][A-Za-z0-9_-]*)\s*=")
|
||||||
|
# Transition classes are applied by the FRAMEWORK, never written in markup:
|
||||||
|
# <Transition name="toast"> makes Vue add .toast-enter-active et al at
|
||||||
|
# runtime, and React's <CSSTransition classNames="fade"> 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[^>]*?(?<![:\w.-])name\s*=\s*"""
|
||||||
|
r"""(?:"([^"]*)"|'([^']*)')"""
|
||||||
|
r"""|(?<![:\w.-])classNames\s*=\s*(?:"([^"]*)"|'([^']*)')"""
|
||||||
|
)
|
||||||
|
# The union of what Vue 3, Vue 2 and React CSSTransition generate. Naming a
|
||||||
|
# class that no rule defines costs nothing — it resolves to no row — so the
|
||||||
|
# union is safer than guessing the framework from the file.
|
||||||
|
_TRANSITION_SUFFIXES = (
|
||||||
|
"-enter", "-enter-from", "-enter-active", "-enter-to", "-enter-done",
|
||||||
|
"-leave", "-leave-from", "-leave-active", "-leave-to",
|
||||||
|
"-exit", "-exit-active", "-exit-done",
|
||||||
|
"-appear", "-appear-from", "-appear-active", "-appear-to", "-appear-done",
|
||||||
|
"-move",
|
||||||
|
)
|
||||||
|
# A name built by concatenation — `status-${s}`, 'pri-' + p, class="c-{{ v }}"
|
||||||
|
# — leaves its static head behind once the hole is blanked. That head is a
|
||||||
|
# PREFIX reference, spelled `status-*`: "*" cannot occur in a class token, so
|
||||||
|
# the marker rides the plain token dict without a schema change. Needs a real
|
||||||
|
# name before the separator; `a-` or a bare `-` says nothing worth matching.
|
||||||
|
_PREFIX_MIN_STEM = 2
|
||||||
|
PREFIX_MARK = "*"
|
||||||
# Inside a dynamic expression: string literals (ternary arms, array items,
|
# Inside a dynamic expression: string literals (ternary arms, array items,
|
||||||
# quoted object keys) and the bare keys of object literals.
|
# quoted object keys) and the bare keys of object literals.
|
||||||
_STR_LIT_RE = re.compile(r"""'([^'\\]*)'|"([^"\\]*)"|`([^`]*)`""")
|
_STR_LIT_RE = re.compile(r"""'([^'\\]*)'|"([^"\\]*)"|`([^`]*)`""")
|
||||||
@@ -307,8 +335,20 @@ _MUSTACHE_RE = re.compile(r"\{[{%][^}]*[}%]\}")
|
|||||||
|
|
||||||
def _class_tokens(value: str) -> list[str]:
|
def _class_tokens(value: str) -> list[str]:
|
||||||
"""The class tokens of a static attribute value: whitespace-split, only
|
"""The class tokens of a static attribute value: whitespace-split, only
|
||||||
well-formed names (an interpolation like `{{ cls }}` contributes none)."""
|
well-formed names. An interpolation (`{{ cls }}`, `${cls}`) is blanked
|
||||||
return [t for t in _MUSTACHE_RE.sub(" ", value).split() if _CLASS_TOKEN_RE.match(t)]
|
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]:
|
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=` /
|
files that carry no markup (by suffix). Reads the static `class=` /
|
||||||
`className=` attributes, the Vue and React dynamic forms and Svelte's
|
`className=` attributes, the Vue and React dynamic forms and Svelte's
|
||||||
`class:x` directive; never a CSS selector (`.x {` is a definition, read
|
`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):
|
if not (path or "").lower().endswith(_TEMPLATE_SUFFIXES):
|
||||||
return {}
|
return {}
|
||||||
counts: dict[str, int] = {}
|
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), "")
|
expr = next((g for g in m.groups() if g is not None), "")
|
||||||
bump(_dynamic_class_tokens(expr))
|
bump(_dynamic_class_tokens(expr))
|
||||||
bump([m.group(1) for m in _SVELTE_CLASS_RE.finditer(text)])
|
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
|
return counts
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -279,20 +279,39 @@ def resolve_consumers(
|
|||||||
of that name when F defines it (a scoped rule is consumed by its own
|
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
|
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
|
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]]] = {}
|
by_symbol: dict[str, list[tuple[int, str]]] = {}
|
||||||
for sid, path, symbol in css_rows:
|
for sid, path, symbol in css_rows:
|
||||||
by_symbol.setdefault(symbol, []).append((sid, path))
|
by_symbol.setdefault(symbol, []).append((sid, path))
|
||||||
out: dict[tuple[int, str], int] = {}
|
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 consumer, tokens in references.items():
|
||||||
for token, count in tokens.items():
|
for token, count in tokens.items():
|
||||||
rows = by_symbol.get(token)
|
if token.endswith(PREFIX_MARK):
|
||||||
if not rows:
|
head = token[: -len(PREFIX_MARK)]
|
||||||
|
for symbol, rows in by_symbol.items():
|
||||||
|
if symbol.startswith(head):
|
||||||
|
credit(rows, consumer, count)
|
||||||
continue
|
continue
|
||||||
own = [sid for sid, path in rows if path == consumer]
|
rows = by_symbol.get(token)
|
||||||
targets = own or [sid for sid, _path in rows]
|
if rows:
|
||||||
for sid in targets:
|
credit(rows, consumer, count)
|
||||||
out[(sid, consumer)] = out.get((sid, consumer), 0) + int(count)
|
|
||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -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",
|
||||||
|
'<transition-group name="toast"><div class="toast-item" /></transition-group>',
|
||||||
|
)
|
||||||
|
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", '<Transition name="peek">')
|
||||||
|
assert "g-move" in class_references("a/T.vue", '<TransitionGroup name="g">')
|
||||||
|
# React's CSSTransition names the same idea with a different suffix set
|
||||||
|
react = class_references("a/T.jsx", '<CSSTransition classNames="fade">')
|
||||||
|
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", '<Transition :name="dyn">') == {}
|
||||||
|
|
||||||
|
|
||||||
|
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 = (
|
||||||
|
'<div :class="`status-${task.status}`" />'
|
||||||
|
"<span :class=\"['pri-' + p]\" />"
|
||||||
|
'<b class="a-" /><u class="-" />'
|
||||||
|
)
|
||||||
|
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", '<i class="card-{{ v }}" />') == {"card-*": 1}
|
||||||
|
# an ordinary name never picks up the marker
|
||||||
|
assert class_references("a/P.vue", '<div class="page-header" />') == {"page-header": 1}
|
||||||
|
|
||||||
|
|
||||||
def test_class_references_reads_react_svelte_and_server_templates():
|
def test_class_references_reads_react_svelte_and_server_templates():
|
||||||
tsx = (
|
tsx = (
|
||||||
"export function X({ on }: { on: boolean }) {\n"
|
"export function X({ on }: { on: boolean }) {\n"
|
||||||
|
|||||||
@@ -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))
|
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():
|
def test_consumer_edges_table_cascades_with_the_shape():
|
||||||
from scribe.models import Base
|
from scribe.models import Base
|
||||||
from scribe.models.code_shape import CONSUMER_BASES, CodeShapeConsumer
|
from scribe.models.code_shape import CONSUMER_BASES, CodeShapeConsumer
|
||||||
|
|||||||
Reference in New Issue
Block a user