feat(lint): font loading + invalid capture path composition rules#1004
Conversation
Two new composition lint rules catching failure modes that recurred across the 11-round website-to-video eval. Both ship with vitest coverage; total lint suite goes from 148 to 151 tests. **`fonts.ts` (new) — two warnings** - `google_fonts_import`: composition loads fonts from `fonts.googleapis.com` via `<link>` or `@import url(...)`. External font requests fail in sandboxed/offline renders and add latency. Fix hint points to root-relative `capture/assets/fonts/...woff2` with a local `@font-face` declaration. - `font_family_without_font_face`: CSS uses a font-family that isn't declared with `@font-face` and isn't in the auto-bundled font set (Inter, JetBrains Mono, etc.). Text would silently fall back to system-ui — the visual fidelity loss the eval kept hitting. Fix hint points to the captured woff2 files. **`composition.ts` invalid_capture_path (new) — one error** Sub-compositions live in `compositions/` but get served with the project root as their base URL. `<img src="../capture/...">` works on disk but 404s in Studio and renders. Errors with a fix hint saying replace `../capture/` with root-relative `capture/`. Three vitest cases: `<img>` triggers, multi-occurrence url()s are counted, root-relative paths stay clean. Registry source files and installed blocks are exempted. **Wiring** `hyperframeLinter.ts` runs the new fonts rules alongside the existing rule set; the composition rule was added inline so it picks up automatically.
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Content matches previously approved #989 (exact same diff stats +339/-0). Re-approved.
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Re-approving the v2 re-cut.
Content equivalence verified — pulled the patch payloads via API for both #989 and #1004 (composition.ts, composition.test.ts, fonts.ts, fonts.test.ts, hyperframeLinter.ts) and the patches are character-identical: same +/- per file, same hunks, same lines. The PR is byte-equivalent to what was already approved on #989.
Merge via GitHub UI individually per Ular's commitment.
— Rames Jusso
ukimsanov
dismissed stale reviews from jrusso1020 and miguel-heygen
The base branch was changed.
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Re-approved after rebase onto main (now includes #1003). Same single commit, content unchanged.
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Re-approving post-rebase onto main.
Content unchanged from prior approval: head SHA is still 2c9544f6d4... — same as my earlier approval. GitHub updated the base pointer to main (now at eb3c9d7e7f... after #1003 merged) without producing a rebase commit, because #1003's file set (packages/cli/src/capture/) is disjoint from #1004's (packages/core/src/lint/rules/). No content drift, no rebase conflict.
One CI artifact worth flagging before merge
regression-shards (shard-7, sub-composition-video style-18-prod raf-ball-render-compat font-varia...) is showing failure, but the run was at 18:20:08 UTC — before #1003 merged. The umbrella regression job re-ran at 18:48:13 UTC (post-rebase) and collected the stale pre-rebase shard result, surfacing as a top-line failure.
Sanity check: main's regression workflow at 18:28:56 UTC (right after #1003 merged) succeeded — so #1003's changes are passing on main. And this PR's content (new lint rules in packages/core/src/lint/rules/) doesn't touch any render path that regression tests would exercise.
Suggested action: trigger a re-run of regression-shards (shard-7) before merging. Very likely a stale-pre-rebase artifact or a flake (sub-composition-video has had Chrome frame-timing drift recently — 3a23542a regenerated that baseline yesterday). If the re-run passes, merge is safe.
Per Ular's commitment + James's global rule: still merge via GitHub UI individually, no Graphite stack-merge.
— Rames Jusso
|
@miguel-heygen My AI is saying that those test are not passing and your #1001 has a fix for it...should I wait? |
I'll admin-merge this btw |
3 participants
Re-stacked version of #989 (silently lost in yesterday's Graphite stack-merge).
Content identical to what was approved on #989. Stacked on #1003 (capture pipeline).
What's in this PR
Three new composition lint rules:
google_fonts_import— flags@importof Google Fonts URLs (use captured fonts instead)font_family_without_font_face— flagsfont-family: 'X'when there's no matching@font-facedeclarationinvalid_capture_path— flags../capture/paths insrc/hrefattributes and inline scriptsAll fix-hints use root-relative
capture/assets/fonts/...woff2paths.Original review history
graphite-base/988)Layer 2 of 4. Once #1003 merges, GitHub will auto-update this PR's base to
main.