Repository navigation
fix(core): embed the weight-list font when the page's Google link gives none - #5186
Conversation
…es none Since #4569 the renderer fetches the Google Fonts URL the page wrote. When that link is malformed (e.g. Bricolage Grotesque with wght@100..900, past the family's 200..800), Google answers 400, the family stays unresolved, and a distributed render fails closed. Before #4569 the renderer's own weight-list request resolved it. If the page's links yield no usable faces, fall back to that weight-list request and warn with each link's status. Fail-closed transient errors still throw before the fallback, and a working link is used unchanged. Preview and render share this resolver, so both embed the same faces. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mily Production pages link Bricolage Grotesque with opsz@7..96 alongside a second family; Google answers 200 and returns only the second family. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An authored link can carry page text in `text=`, so the warning now records only the family, each link's outcome (http_<status> or no_faces) and how many faces the weight-list fallback embedded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jrusso1020
left a comment
There was a problem hiding this comment.
Approved at 476a6dc4.
The root cause holds against live Google Fonts. I sent the request shapes from the body to fonts.googleapis.com with a woff2 user agent:
Bricolage+Grotesque:opsz,wght@7..96,500&family=Hanken+Grotesk:wght@400;500;600returns 200 with 12 faces, all Hanken and none Bricolage. That is the silent drop.Bricolage+Grotesque:opsz,wght@7..96,500alone returns 400, and with12..96it returns 200 with 3 faces.- The fallback,
ital,wght@0,100;…;0,900;1,400;1,700, returns 200 with Bricolage at 200 to 800, with and withouttext=. Google tolerates the out-of-range 100/900 and the missing italics, so the fallback does resolve the production family.
The change is right.
fetchFontResourcethrows under fail-closed on a retryable status or network error once retries run out. So a transient miss on the page's link still fails the render before any fallback, as the body says. Only a 4xx or a 200 without faces reaches the weight list.- Faces still go through
parseGoogleFontSource'sfont-familycheck, so a multi-family link can't lend one family's faces to another. A link that yields any faces returns them unchanged, which keeps the #4569 behavior. - The fallback goes through the same per-mode
googleFontCssCachekey, so repeat compiles of a dropped family make one extra request per process. - Families in
FONT_ALIASESand families with no page link take the weight-list path directly, as they did before.
Local runs. vitest run src/fonts passes 225 of 226. The one failure is aliasSupplement "has a canonical display name for every alias target", which also fails at the merge-base 21d14b25b. When I replaced if (faces.length > 0) return faces; with return faces;, 9 tests in googleSubsets/declaredAlias failed, including the multi-family drop case.
Non-blocking
Commentsis red because this adds 2 comment lines todeterministicFonts.ts, raising its comment share from 192/1949 to 194/1969. It isn't required, but cutting one comment line keeps it green.link_outcomes=no_facescovers three different cases: a 200 that drops the family, a fail-open network error ({ css: null }with no status), and a woff2 that wouldn't download. If the Datadog count needs to tell those apart, the network case could reportnetwork.- Under fail-open (Studio preview), a page whose link drops a family now embeds the weight-list faces instead of falling back to a system font. That is the pre-#4569 result and matches the render. The faces are static instances at default optical size rather than the
opszrange the page asked for.
CI. At this head, 35 checks had passed and the required regression shards, Windows render and tests, and Studio edit-accuracy were still running when I posted.
— Rames
Edit accuracy: accurate 2059 (base branch 2059), smooth 1629 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
At 476a6dc4, the fallback is functionally sound in the reviewed paths: a 200 stylesheet that omits Bricolage triggers the family-specific weight list without replacing Hanken's linked faces; transient authored-link failures still throw before fallback in fail-closed mode. The new tests cover both boundary cases, and required hosted checks are green. I did not run local tests or a live render.
One change before my approval: the added test cases use function-local dynamic imports in deterministicFonts-googleSubsets.test.ts. Please hoist the imported symbol to the existing module-level import (inline comment). Our top-level-import-only rule includes tests. The non-required Comments ratchet and the broad no_faces warning label are cleanup/observability notes, not functional blockers. No merge or rollout action from this review.
— Jerrai
…bset tests Also drops the two comments the fallback added, so the file's comment share does not rise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Re-approving at 3e64e33f. The delta since my approval at 476a6dc4 is the fix for the change request on function-local imports, plus two deleted comments.
injectDeterministicFontFacesis now imported at module scope alongside_clearGoogleFontCssCacheForTests, and all 20await import("./deterministicFonts.js")calls are gone, including the pre-existing ones. This doesn't change behavior. The test file already imported./deterministicFonts.jsstatically, so the module was loaded before any test ran.HYPERFRAMES_FONT_CACHE_DIR, whichbeforeAllsets, is read lazily insideresolveFontCacheRoot()/defaultCacheDir()at call time, never at module load.deterministicFonts.tsloses only the two comments above the fallback request and the warn line. No code changes, and this should clear the comment ratchet.
Required checks were still running when I posted; GitHub won't merge until they're green.
— Rames
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
The import-hygiene hold is fixed at 3e64e33f: the test file now imports injectDeterministicFontFaces at module scope, removes the function-local dynamic imports, and leaves the fallback/error behavior unchanged. The two new production comments were removed; Comments is green. My previous functional review still applies to this one-commit cleanup. Required Windows render and runtime-contract checks were still pending at approval; code approval is not CI completion or deployment evidence. The broader no_faces label can be refined separately if the dashboard needs that distinction.
— Jerrai
…0072026 Conflict in packages/core/src/fonts/deterministicFonts.ts: kept #5186's authored-zero-faces default fallback and its stylesheet result/status shapes unchanged; threaded the diagnostics attempt through every stylesheet fetch (authored and default fallback each get their own). - Jerrai
What
Since #4569 the renderer fetches the Google Fonts stylesheet the page itself links, so the film draws the same cut as the preview. When that link gives no usable faces for a family, the family stays unresolved and a distributed render (fail-closed) throws
Unresolved fonts in fail-closed mode. Before #4569 the renderer's own weight-list request (ital,wght@0,100;…;0,900;1,400;1,700) resolved those families.Production root cause (confirmed)
Datadog, last 14 days,
Unresolved fonts in fail-closed mode: 1,031 of ~1,125 errors name Bricolage Grotesque, allHyperframeRenderStreamingWorkflow, first seen 2026-10-06 23:48 UTC — right after hyperframe-internal moved producer from 0.8.70 to 0.8.138 (#2852), the first producer build containing #4569.The three failing compositions I pulled from S3 link:
Bricolage's
opszaxis starts at 12. Requested alone,opsz,wght@7..96,500is a 400; inside a multi-family link Google answers 200 and silently omits Bricolage, returning only the other family. So the failure is "200 but no faces for this family", not an HTTP 400.Running
injectDeterministicFontFaces(html, { failClosedFontFetch: true, allowSystemFontCapture: false })on those three realindex.htmlfiles against live Google Fonts:main: all three throwUnresolved fonts in fail-closed mode: Bricolage Grotesque(matches production).Other single-family links Google rejects outright (400), e.g.
Bricolage+Grotesque:wght@100..900orwght@900; LLM-written pages produce such links often (experiment-frameworkVA-1582 repairswght@400700-style URLs on the GSAP path).Change
In
fetchGoogleFont, if the page's links yield no usable faces for a family, fall back to the weight-list request and warn with fields only —google_font_link_fallback family="…" link_outcomes=http_400|no_faces fallback_faces=N— so Datadog can count real 400s vs silently-dropped families and whether the fallback resolved them. No URL, CSS or page text is logged (an authored link can carry page text intext=). Previously the status was dropped silently.hyperframes-localize-fontsCLI (Video Agent streamed preview) and producer all callinjectDeterministicFontFaces, so they embed the same faces.Falling back on any no-faces outcome (not only 400) is deliberate: the production case is a 200. The fallback is the request the renderer sent before #4569, so font resolution is never worse than it was then.
Tests
deterministicFonts-googleSubsets.test.ts:wght@100..900(400) renders under fail-closed.deterministicFonts-declaredAlias.test.ts: a verbatim-declared unknown family is now queried twice by the same spelling (link + weight list) and still fails closed.Mutation-checked: the previous resolver fails the new cases; narrowing the fallback to 400 only fails the 403/404/503/200 cases.
src/fonts: 227/228 pass; the remaining one (local system-font capture timeout) fails onmaintoo in this environment.🤖 Generated with Claude Code