fix(design-sync): font fallback stacks by category (CE-7)

[linardsb]

Summary

Clients without a design font rendered the converter's text in the browser default serif: the override path wrote the bare family (font-family:Geist Mono), and the inline paths appended ,sans-serif whatever the font, so mono and serif design fonts fell back to sans. Every converter font write now carries a fallback stack of the design font's own category, so a client without the font falls back to the right kind of face.

What changed

• fix(design-sync): font fallback stacks by category (CE-7) (468be5e7)
• New app/design_sync/font_stacks.py: font_category() infers mono / sans / serif from the family name (Figma exposes no generic family); font_stack() returns the family plus 'Courier New', Courier, monospace / Helvetica, Arial, sans-serif / Georgia, 'Times New Roman', serif, deduped, idempotent, and sanitised to \w plus .- so the value is attribute-safe without html.escape (an escaped ' becomes ', whose ; breaks the renderer's second font write).
• component_matcher.py: the six inline font sites, the VML button spec and the three override emissions call font_stack.
• component_renderer.py: _replace_heading_font, _replace_body_font and _apply_text_node_style (font-family only) re-apply font_stack as a guard instead of html.escape.
• data/debug/content_check_allowlist.yaml: the six CE-2 font_generic entries owned by Font fallback stacks by category #425 are deleted.
• data/debug/{5,6,7,8,9,10,reframe}/expected.html regenerated: font values only (masked audit masked_equal=True × 7, observed).
• data/debug/fidelity_baseline.json re-stamped for cases 6, 7, 10, note in docs/fidelity-gate.md §Re-stamping.
• Tests: new app/design_sync/tests/test_font_stacks.py (80 tests, observed: uv run pytest app/design_sync/tests/test_font_stacks.py -q → 80 passed at 468be5e7): builder, each matcher site, a second font write on a real per-node anchor, a minimal Figma tree through convert_document, and a category invariant plus no escaped font outside MSO over all 7 corpus cases. 15 existing tests updated to the stacks (derived: 7 + 1 + 1 + 1 + 5 across test_column_text_styling, test_card_composite, test_component_matcher, test_component_renderer, test_content_checks), plus the two pill constants in test_cta_fidelity.py.

git diff --stat origin/main..HEAD → 21 files changed, 1159 insertions(+), 285 deletions(-) (observed).

Validation

observed — make check-full, at 468be5e7, exit 0:

pytest:  9123 passed, 0 failed, 123 skipped
vitest:  780 passed, 0 failed
window:  2026-10-04T17:37:19Z -> 2026-10-04T17:42:40Z

full log: .claude/state/gate/20261004T173719Z-468be5e7.log

Converter evidence. Re-run at 468be5e7 (observed): corpus invariant uv run pytest app/design_sync/tests/test_font_stacks.py -q -k corpus → 7 passed; ladder uv run pytest app/design_sync/tests/test_converter_data_regression.py -q -k ladder → 7 passed; the pinned-image test_fidelity_gate_holds_baseline PASSED inside the make check-full run above (log line). The remaining figures are from the implementation runs on the code committed as branch commit 6c5c6f18 (observed there; RED and mutation runs reverted parts of it in the working tree); they hold at this head because git diff --stat 6c5c6f18 468be5e7 -- app/ data/debug/ shows only the baseline re-stamp and one pyright comment line in test_font_stacks.py (derived):

• RED on base: the category invariant fails on all 7 cases with 7/3/6/10/3/21/6 violations (5/6/7/8/9/10/reframe); the site, override, double-write and minimal-tree tests fail on assertions.
• Mutations: reverting the renderer guard fails the double-write test and corpus cases 5 and 10; reverting the _text_node_overrides emission fails only its emission test (the guard covers the rest).
• python -m app.design_sync.tests.content_checks → "No problems.", font_generic pass on all 6 cases.
• Ladder: data/debug/ladder_snapshot.json unchanged (git diff origin/main -- data/debug/ladder_snapshot.json empty).
• Fidelity gate before the stamp: only c7 2833:1942 −0.0059 beyond the 0.005 margin; in margin: 6 2833:1430 +0.0004; 7 2833:1870 −0.0024, 2833:1882 −0.0034, 2833:1898 −0.0011; 10 2833:1141 −0.0007, 2833:1176 +0.0001, 2833:1197 +0.0003, 2833:1227 −0.0004; every other section +0.0000. After the stamp: passes (see above).
• A3 full_image (local, 7 rows each side): c5 0.8660 → 0.8672, c6 0.8131 → 0.8117, c10 0.7361 → 0.7368; c7, c8, c9, reframe unchanged. Section drops: c5 idx 8 −0.007 (a 1 px shift below the "Discover →" CTA, the known scorer artefact), c6 idx 1 −0.006 (Roboto cells now Helvetica instead of serif).
• Mammut "BUILD YOUR LAYERS" at 600 px, local Chromium (Geist Mono not installed): rendered face before Times, after Courier New.
• Manual check: c6 and c10 before / after / manual-build side-by-sides viewed at 600 px (c10 CTAs now mono; c6 heading and body now sans, like the manual build).

Notes for the reviewer

• Ratified at planning (Q3): the c7 2833:1942 gate drop (−0.0059; the 30 px Noto Sans heading now renders in Liberation Sans instead of Liberation Serif in the pinned image, and the design reference is a sans) and the A3 drops on c5 idx 8 and c6 idx 1.
• Out of scope (Q1): the shell / <body> font from tokens (token_transforms._font_stack), owned by ce-3-file-wide-tokens-body-font.
• Intended exception: the VML <center> font inside <!--[if mso]> stays HTML-escaped by vml_button._attr; nothing rewrites it later.
• Deviations from the plan (all amended into the plan):
  1. The minimal-tree test has a lead and a tail frame: with two frames the first classified as email-header and dropped the fonts under test.
  2. The corpus test also asserts no font-family:' outside MSO blocks: lxml decodes entities, so the category invariant alone stayed green when the renderer guard was reverted.
  3. _apply_image_corner_radius is unchanged (the planning spike touched it; it only receives border-*-radius props).
  4. _card_text_row uses font_stack(text.font_family or "Arial") (ruff SIM108; same output).
  5. The content-check reader helper rebuilds the first Geist Mono stack outside a conditional block: the first occurrence is now in a CE-11 <!--[if !mso]><!--> twin, which the reader counts twice.
  6. The mammut "before" render used the base snapshot HTML instead of stashed call sites.
  7. The T3 RED run covered the whole test file, not the plan's -k filter.
• The fidelity stamp names branch commit 6c5c6f18, which is gone after the WIP squash (convention in docs/fidelity-gate.md §Re-stamping).

Linked

Closes #425 · Epic #439 · Plan .agents/plans/ce-7-font-fallback-stacks.md

Opened as a draft; CI's ready job flips it when every ci.yml job (backend, frontend, sdk-check, trivy, migrations, integration, migration-lint, commit-lint, e2e-smoke) and codeql are green on this head. A red job leaves it here with the failing check on the PR.

🤖 Generated with Claude Code

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[linardsb]

Verdict: APPROVE (with one Medium to fix or ratify before merge). Recorded as a comment because GitHub blocks a formal self-approval.

PR #472 review: fix(design-sync): font fallback stacks by category (CE-7)

Head 468be5e7 · Base main @ 9a19d74ccdad00e6bfa6ab0e18af35ded392a8c3 (equals the live origin/main tip; round 1, so the guarantees and fix-mechanism passes don't apply)

Summary

Every converter font-family write now goes through font_stack(), which appends a fallback stack of the font's own type: mono, sans or serif. A client without the design font now falls back to the same kind of face, instead of the default serif or a sans for every font.

The code-reviewer agent read all 21 files in full, and I re-ran validation myself. There are no Critical or High issues. One Medium: a few sans and display fonts are classed as monospace, which is worse than the old blanket sans-serif. The rest are Low.

Issues

Medium

F1 · app/design_sync/font_stacks.py:57,100 · Some sans fonts are classed as monospace.

• Why: _MONO_PREFIXES matches by "starts with" on mono and code, and the mono check runs before the serif check.
• Observed: font_stack('Codec Pro') returns 'Codec Pro', 'Courier New', Courier, monospace, and 'Monotype Corsiva' also gets the mono stack.
• Same mechanism, not run: Monotype Garamond, Monoton and Monofett.
• Effect: before this PR, every font fell back to sans-serif. A Codec Pro body font now renders as Courier New in clients without it. A font wrongly put in sans is no worse than before; a font wrongly put in mono or serif is a regression.
• Fix: match mono and code as whole words. Keep "starts with" matching for the brand stems (consol, courier, menlo, monaco). Add Codec Pro and Monotype Corsiva to the classifier tests.

Low

• F2 · font_stacks.py:57-81 · Some real mono fonts fall to the sans stack: Inconsolata (observed), Cousine (observed), Anonymous Pro and Iosevka. That is no worse than before. Optionally add them as whole-word tokens.
• F3 · font_stacks.py:113 · Names containing . stay unquoted. _clean keeps ., but _render only quotes names with a space or a leading digit. Gill.Sans is not a valid unquoted CSS name, so the browser drops the whole declaration. The old path did the same, so this is not a regression. Fix: quote any name that is not a CSS identifier.
• F4 · font_stacks.py:83,89 · Some non-Latin characters are deleted rather than turned into spaces: combining marks (Thai, Indic), NBSP and U+3000. The deletion glues words together. Plain CJK survives (observed: 思源黑体). Figma family names are Latin in practice.
• F5 · component_renderer.py:1669 · No unit test fails if _replace_heading_font goes back to html.escape. The existing heading tests use Georgia, serif and Helvetica, where the two functions give the same output. Only the corpus ' check on case 7 would catch it, and that mutation was not in the reported mutation runs. Add a heading-override test with Noto Sans.
• F6 · test_font_stacks.py (test_minimal_tree_every_font_matches_category, last assert) · v.endswith("serif") also passes for sans-serif. The category_violations == [] line above does the real check. Use endswith(", serif").
• F7 · Test names are stale: test_column_text_styling.py:148,231 (*_escapes_font_family) and test_card_composite.py:213. They now check that characters are stripped, not escaped. Rename.

Code scanning

gh api …/code-scanning/alerts?ref=refs/pull/472/merge&state=open returned 0 open alerts from Semgrep and CodeQL (observed).

Validation

Gate	Result
make check-full at 468be5e7 (local, this review)	exit 0: pytest 9123 passed / 0 failed / 123 skipped, vitest 780 passed, mypy and pyright clean, .env.example no drift; lint rewrote nothing (observed)
CI on 468be5e7	all 14 checks green, Ready for review passed (observed, gh pr checks 472)
test_font_stacks.py count	80 collected (observed: --collect-only), matches the PR body

Numbers pass

• Fidelity baseline deltas: all 9 deltas in data/debug/fidelity_baseline.json match the PR body and docs/fidelity-gate.md exactly (derived from the JSON diff). Examples: c7 2833:1942 0.8611 → 0.8552 = −0.0059, and c6 2833:1430 0.9178 → 0.9182 = +0.0004.
• Diff stat: "21 files, 1159+/285−" is correct (observed).
• Allowlist: "six font_generic entries deleted" is correct (observed, 6 removed).
• Updated tests: "15 existing tests updated" is labelled derived, and 7 + 1 + 1 + 1 + 5 = 15.
• Older runs: RED-on-base, mutation and A3 figures are attributed to branch commit 6c5c6f18. The reviewer confirmed that 6c5c6f18..468be5e7 changes only a pyright pragma line plus the baseline JSON, so the attribution holds.
• I found no figure presented as observed without a run behind it.

Deferred items

There is no ledger entry for #425 or CE-7. ce-3-file-wide-tokens-body-font overlaps the topic and is correctly left out of scope (Q1). Nothing needs closing or stamping.

Done well

• Escaping: the cleaned output cannot contain ", ;, &, <, > or \, so it cannot break style="…" or end the renderer's font-family:[^;"]+ rewrites early. It also removes the old risk of a backslash in a re.sub replacement.
• Idempotent: font_stack returns the same stack when given its own output, so re-applying it in the renderer adds nothing. Dedupe ignores case and quotes.
• Tests:
  • RED-on-base is recorded, and the guard-revert mutation fails the corpus check.
  • The double-write test runs on real per-node anchors and has a guard so it cannot pass on empty output.
  • Fixtures are the real corpus and a real Figma tree run through convert_document. No synthetic email HTML was added.
• Scope: a masked diff of all 7 expected.html files shows only font-family values changed, and MSO blocks are untouched. The c7 drop and the out-of-scope body font are documented and ratified.

Recommendation

Approve. Before merge, fix F1 (a small change to the classifier plus two test cases) or ratify it as accepted. The Lows can ship as they are or be picked up together in a follow-up. A human makes the merge call.