Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .agents/plans/ce-7-font-fallback-stacks.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ The following plan should be complete, but validate documentation, codebase patt
|---|---|---|
| D1 | New module `app/design_sync/font_stacks.py`: `FontCategory` (`StrEnum` SANS/SERIF/MONO), `_CATEGORY_STACK: dict[FontCategory, tuple[str, ...]]`, `font_category(family) -> FontCategory`, `font_stack(family) -> str`. | Issue scope: "one stack builder … new builder module". |
| D2 | Stacks exactly as the issue: SANS `Helvetica, Arial, sans-serif`; SERIF `Georgia, 'Times New Roman', serif`; MONO `'Courier New', Courier, monospace`. Output = the design family first, then the category stack, deduped ignoring case and quotes, joined with `", "`. | Issue scope. Dedupe so `Arial` → `Arial, Helvetica, sans-serif` and `Courier New` → `'Courier New', Courier, monospace`. |
| D3 | Category is inferred from the family **name**. Figma exposes no generic family: `raw_figma.json` text styles carry only `fontFamily`, `fontPostScriptName`, `fontSize`, `fontStyle`, `fontWeight` (observed: `grep -o '"font[A-Za-z]*"' data/debug/10/raw_figma.json`, 29 each), and `structure.json` carries `font_family` only. Tokenise the lower-cased name on spaces/hyphens. Order: MONO (any token **starting with** `mono`, `code`, `courier`, `consol`, `menlo`, `monaco` — prefix so `Consolas` matches) → SANS (token starting with `sans`, `grotesk`, `grotesque`) → SERIF (any token equal to `serif`, `slab`, or a name in a short known-serif list — any token, so `EB Garamond` matches: Georgia, Times, Garamond, Baskerville, Bodoni, Didot, Caslon, Playfair, Merriweather, Lora, Cambria, Palatino, Cormorant, Crimson, Spectral, Domine) → default SANS. | Mono first so `Roboto Mono`/`Noto Sans Mono` stay mono; sans before serif so `Merriweather Sans` is sans. The name list is font data, not fixture data (generality rule holds: keyed on a Figma text property, never a design/node/colour). |
| D3 | Category is inferred from the family **name**. Figma exposes no generic family: `raw_figma.json` text styles carry only `fontFamily`, `fontPostScriptName`, `fontSize`, `fontStyle`, `fontWeight` (observed: `grep -o '"font[A-Za-z]*"' data/debug/10/raw_figma.json`, 29 each), and `structure.json` carries `font_family` only. Tokenise the lower-cased name on spaces/hyphens. Order: MONO (any token **equal to** `mono`, `code` or `monospace`, or **starting with** `courier`, `consol`, `menlo`, `monaco`, `monolisa`, `monoid`, `mononoki`, `monofur`, `codenewroman` — prefix so `Consolas` matches; whole-word `mono`/`code` since PR #472 review F1, so `Codec Pro` and `Monotype Corsiva` are not mono; one-word mono brand stems and the `monospace` token since PR #473 review F1/F2, so `MonoLisa` and a leading `ui-monospace` stay mono) → SANS (token starting with `sans`, `grotesk`, `grotesque`) → SERIF (any token equal to `serif`, `slab`, or a name in a short known-serif list — any token, so `EB Garamond` matches: Georgia, Times, Garamond, Baskerville, Bodoni, Didot, Caslon, Playfair, Merriweather, Lora, Cambria, Palatino, Cormorant, Crimson, Spectral, Domine) → default SANS. | Mono first so `Roboto Mono`/`Noto Sans Mono` stay mono; sans before serif so `Merriweather Sans` is sans. The name list is font data, not fixture data (generality rule holds: keyed on a Figma text property, never a design/node/colour). |
| D4 | Input that is already a list: sanitise each family; if the last is a CSS generic family, return the normalised list unchanged (idempotent); otherwise keep every authored family and append the first family's category stack, deduped. | Providers other than Figma (penpot, html import, tests at `test_component_matcher.py:310`, `test_vml_button.py:161`) already pass lists. Idempotence lets the renderer re-apply the builder as a guard. |
| D5 | Sanitising lives in the builder: strip `'`/`"`, keep only `\w` (Unicode, so `_` stays) plus ` .-` per family (regex `[^\w .-]` removed), collapse spaces; a family emptied by this is dropped; an empty result gives the SANS stack alone. Multi-word names are wrapped in single quotes. Call sites **do not** `html.escape` the builder's output. | `html.escape(quote=True)` turns `'` into `&#x27;`, whose `;` ends the renderer's `font-family:\s*[^;"]+` match (`component_renderer.py:440-447`, `:466-473`) and `_upsert_style_decl` (`:2051`) on a second write, leaving a junk declaration. The builder's charset contains no `"`, `<`, `>`, `&`, so it is attribute-safe without escaping. No template uses single-quoted `style='…'` attributes (observed: `grep -rln "style='" email-templates/components` → none). Exception kept: `vml_button._attr` (`vml_button.py:56-57`) still escapes the spec font into `&#x27;` in the VML `<center>`; safe because it sits inside an `<!--[if mso]>` block that no later regex rewrites, and lxml readers decode it. Families are any `\w` (Unicode letters/digits) plus ` _.-`, so non-Latin names survive; a name is quoted when it has a space or a token starting with a digit (an unquoted `3Dumb` drops the whole declaration). |
| D6 | Canonical write point = the matcher's override emission (`_text_node_overrides` `component_matcher.py:3059-3060`, `_build_token_overrides` `:3233-3239`) emits `font_stack(...)`. The renderer's three appliers call `font_stack(value)` again as a guard (no-op on a stack by D4). | Every bare value in today's output comes from this override path (observed probe, Evidence E2). The renderer guard covers any other `TokenOverride("font-family", …)` producer. |
Expand Down Expand Up @@ -169,7 +169,7 @@ Phase 1 (builder, T1–T2) → Phase 2 (call sites, T3–T6) → Phase 3 (fallou

### T2 CREATE `app/design_sync/font_stacks.py`

- **IMPLEMENT**: D1–D5. Public: `FontCategory`, `font_category(family: str) -> FontCategory`, `font_stack(family: str) -> str`. Private: `_CATEGORY_STACK`, `_GENERIC` (`sans-serif`, `serif`, `monospace`, plus `cursive`, `fantasy`, `system-ui`, `ui-serif`, `ui-sans-serif`, `ui-monospace`, `ui-rounded` — same set as `content_checks.GENERIC_FAMILIES`), `_split(value) -> list[str]` (split on `,`, strip quotes, sanitise per D5), `_render(name) -> str` (generic names lower-cased and never quoted; single-quote when the name has a space or a digit-leading token). Classifier per D3 (any token; prefix match for MONO/SANS keywords). The spike's ~45-line version passed S2/S4; it is a reference, not the deliverable (no docstrings, no tests).
- **IMPLEMENT**: D1–D5. Public: `FontCategory`, `font_category(family: str) -> FontCategory`, `font_stack(family: str) -> str`. Private: `_CATEGORY_STACK`, `_GENERIC` (`sans-serif`, `serif`, `monospace`, plus `cursive`, `fantasy`, `system-ui`, `ui-serif`, `ui-sans-serif`, `ui-monospace`, `ui-rounded` — same set as `content_checks.GENERIC_FAMILIES`), `_split(value) -> list[str]` (split on `,`, strip quotes, sanitise per D5), `_render(name) -> str` (generic names lower-cased and never quoted; single-quote when the name has a space or a digit-leading token). Classifier per D3 (any token; whole-word `mono`/`code`/`monospace`, prefix match for the mono brand stems and the SANS keywords). The spike's ~45-line version passed S2/S4; it is a reference, not the deliverable (no docstrings, no tests).
- **IMPORTS**: `re`, `enum.StrEnum`.
- **GOTCHA**: generic family names must never be quoted (`'serif'` is a family called "serif"). Keep the module free of `html` escaping (D5).
- **VALIDATE**: T1 green; `uv run mypy app/design_sync/font_stacks.py && uv run pyright app/design_sync/font_stacks.py`; `uv run ruff check --no-fix app/design_sync/font_stacks.py app/design_sync/tests/test_font_stacks.py`.
Expand Down
20 changes: 17 additions & 3 deletions app/design_sync/font_stacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,22 @@ class FontCategory(StrEnum):
}
)

# Token prefixes, so "Consolas" matches "consol".
_MONO_PREFIXES = ("mono", "code", "courier", "consol", "menlo", "monaco")
# Whole tokens, so "Codec Pro" and "Monotype Corsiva" are not mono.
# "monospace" also catches a leading generic ("ui-monospace" splits on "-").
_MONO_TOKENS = frozenset({"mono", "code", "monospace"})
# Brand-stem prefixes, so "Consolas" matches "consol" and one-word mono
# brands ("MonoLisa", "CodeNewRoman") stay mono.
_MONO_PREFIXES = (
"courier",
"consol",
"menlo",
"monaco",
"monolisa",
"monoid",
"mononoki",
"monofur",
"codenewroman",
)
_SANS_PREFIXES = ("sans", "grotesk", "grotesque")
# Whole tokens, so "EB Garamond" matches "garamond".
_SERIF_TOKENS = frozenset(
Expand Down Expand Up @@ -97,7 +111,7 @@ def _split(value: str) -> list[str]:
def font_category(family: str) -> FontCategory:
"""Category of one family name, inferred from its tokens."""
tokens = _TOKEN_SPLIT_RE.split(_clean(family).lower())
if any(t.startswith(p) for t in tokens for p in _MONO_PREFIXES):
if any(t in _MONO_TOKENS or t.startswith(_MONO_PREFIXES) for t in tokens):
return FontCategory.MONO
if any(t.startswith(p) for t in tokens for p in _SANS_PREFIXES):
return FontCategory.SANS
Expand Down
2 changes: 1 addition & 1 deletion app/design_sync/tests/test_card_composite.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def test_card_text_row_carries_mso_and_font_props() -> None:
assert prop in value, f"missing {prop} on the card text cell"


def test_card_font_family_escaped_with_fallback() -> None:
def test_card_font_family_stripped_with_fallback() -> None:
"""H1 regression: a design font name can't break out of the style attr, and the
category fallback stack is appended when the value has no comma."""
from app.design_sync.component_matcher import _card_text_row
Expand Down
4 changes: 2 additions & 2 deletions app/design_sync/tests/test_column_text_styling.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def test_column_row_skips_zero_letter_spacing() -> None:
assert "letter-spacing" not in row # 0.0 is the no-op default


def test_column_row_escapes_font_family() -> None:
def test_column_row_strips_font_family_breakout() -> None:
"""A font name must not break out of the style attribute (CSS injection)."""
evil = _styled_text(font_family='Arial" onmouseover="x')
row = _column_text_row(evil, is_heading=True)
Expand Down Expand Up @@ -228,7 +228,7 @@ def test_cta_label_typography_falls_back_to_legacy_defaults() -> None:
assert "font-weight:bold" in css # pre-52.4b default


def test_cta_label_typography_escapes_font_family() -> None:
def test_cta_label_typography_strips_font_family_breakout() -> None:
"""A CTA font name must not break out of the style attribute."""
css = _cta_label_typography(_styled_button(font_family='Arial" onmouseover="x'))
assert '" onmouseover' not in css
Expand Down
11 changes: 11 additions & 0 deletions app/design_sync/tests/test_component_renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,17 @@ def test_heading_font_override(self, renderer: ComponentRenderer) -> None:
result = renderer.render_section(match)
assert "Georgia, serif" in result.html

def test_heading_font_override_quotes_unescaped(self, renderer: ComponentRenderer) -> None:
"""A multi-word family keeps its bare quotes; ``html.escape`` would emit ``&#x27;``."""
match = _make_match(
"text-block",
fills=[SlotFill("heading", "Title")],
overrides=[TokenOverride("font-family", "_heading", "Noto Sans")],
)
result = renderer.render_section(match)
assert "font-family:'Noto Sans', Helvetica, Arial, sans-serif" in result.html
assert "&#x27;" not in result.html

def test_heading_color_override(self, renderer: ComponentRenderer) -> None:
match = _make_match(
"text-block",
Expand Down
28 changes: 27 additions & 1 deletion app/design_sync/tests/test_font_stacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,19 @@ def test_category_stack_covers_enum() -> None:
*[(f, FontCategory.SERIF) for f in _SERIF],
("Brandname Display", FontCategory.SANS), # unknown → sans
("Consolas", FontCategory.MONO), # prefix match
("Source Code Pro", FontCategory.MONO), # whole token "code"
# "mono"/"code" are whole tokens, not prefixes (PR #472 F1)
("Codec Pro", FontCategory.SANS),
("Monotype Corsiva", FontCategory.SANS),
("Monoton", FontCategory.SANS),
("Monofett", FontCategory.SANS),
("Monotype Garamond", FontCategory.SERIF),
# one-word mono brands match by stem (PR #473 F1)
("MonoLisa", FontCategory.MONO),
("Monoid", FontCategory.MONO),
("Mononoki", FontCategory.MONO),
("Monofur", FontCategory.MONO),
("CodeNewRoman", FontCategory.MONO),
],
)
def test_font_category(family: str, category: FontCategory) -> None:
Expand All @@ -117,6 +130,19 @@ def test_generic_terminated_list_is_unchanged() -> None:
assert font_stack("Georgia, serif") == "Georgia, serif"


@pytest.mark.parametrize(
("family", "expected"),
[
("ui-monospace, Menlo", "ui-monospace, Menlo, 'Courier New', Courier, monospace"),
# the stack's "monospace" is already present, so dedup drops it
("monospace, Foo", "monospace, Foo, 'Courier New', Courier"),
],
)
def test_leading_generic_monospace_gets_mono_stack(family: str, expected: str) -> None:
# PR #473 F2: a generic first family with no trailing generic
assert font_stack(family) == expected


def test_list_without_generic_gets_first_familys_stack() -> None:
assert font_stack("'Helvetica Neue', Arial") == "'Helvetica Neue', Arial, Helvetica, sans-serif"

Expand Down Expand Up @@ -356,7 +382,7 @@ def test_minimal_tree_every_font_matches_category() -> None:
assert category_violations(html) == []
values = font_family_values(html)
assert any(v.startswith("'JetBrains Mono'") and v.endswith("monospace") for v in values)
assert any(v.startswith("Lora") and v.endswith("serif") for v in values)
assert any(v.startswith("Lora") and v.endswith(", serif") for v in values)


# ── Corpus ───────────────────────────────────────────────────────
Expand Down
Loading