diff --git a/skills-manifest.json b/skills-manifest.json index 8dda3e5a5..84ceb7df0 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -6,7 +6,7 @@ "files": 138 }, "faceless-explainer": { - "hash": "c70b904aa68cf7e5", + "hash": "1eb3772e62dd71bb", "files": 24 }, "figma": { @@ -62,11 +62,11 @@ "files": 132 }, "pr-to-video": { - "hash": "7769801640dca521", + "hash": "01f46da1e17577ea", "files": 30 }, "product-launch-video": { - "hash": "81953f054fcb9d91", + "hash": "d562efe00647c14b", "files": 28 }, "remotion-to-hyperframes": { diff --git a/skills/faceless-explainer/scripts/build-frame.mjs b/skills/faceless-explainer/scripts/build-frame.mjs index e16f2a328..ec08e112a 100644 --- a/skills/faceless-explainer/scripts/build-frame.mjs +++ b/skills/faceless-explainer/scripts/build-frame.mjs @@ -431,8 +431,15 @@ if (brandFonts.length || (brandColors.length && presetColors.length)) { // ── stage brand font files + emit @font-face ────────────────────────────────── // A brand font is rarely a Google font, so renaming the family in frame.md is not enough: // nothing loads the actual face. If the capture downloaded font files, copy them to -// assets/fonts/ under CLEAN, weight-named names (so captions.mjs' family-prefix matcher +// assets/fonts/ under CLEAN, face-named names (so captions.mjs' family-prefix matcher // finds them too) and append a ready-to-paste, ROOT-RELATIVE @font-face block to frame.md. +// +// The staged NAME is a contract, not cosmetics: captions.mjs derives each face's weight and +// style back out of it. So the name has to carry every axis that distinguishes one face from +// another, and the dedup key has to be the whole face. Naming on weight alone made Google's +// two-file Newsreader download (upright + italic, both scoring "Regular") collide on one +// slot: the italic sorts first, took the name, the upright was never staged, and the block +// below then asserted font-style:normal over italic bytes. if (brandFonts.length) { const norm = (s) => String(s) @@ -442,6 +449,16 @@ if (brandFonts.length) { const FMT = { woff2: "woff2", woff: "woff", ttf: "truetype", otf: "opentype" }; const weightInfo = (name) => { const s = name.toLowerCase(); + // A numeric axis is the font's own answer, so it beats the word heuristic. Fontsource + // names every face that way and carries no weight WORD at all, so word-only parsing + // scored a whole family "Regular" and staged exactly one of its faces. + // + // A weight token must not be buried inside a longer run: this reads capture files, + // which are commonly hash-named, and "Newsreader-a1b200c3.woff2" is not a 200-weight + // face. Hence a non-digit before (which also stops "2100" reading as 100) and no + // alphanumeric after. "Roboto900.ttf" still parses. + const numeric = /(?:^|[^0-9])([1-9]00)(?![0-9a-z])/.exec(s); + if (numeric) return { n: Number(numeric[1]), w: numeric[1] }; if (/black|heavy|ultra|extrabold/.test(s)) return { n: 800, w: "ExtraBold" }; if (/semibold|demibold/.test(s)) return { n: 600, w: "SemiBold" }; if (/bold/.test(s)) return { n: 700, w: "Bold" }; @@ -449,6 +466,7 @@ if (brandFonts.length) { if (/light|thin/.test(s)) return { n: 300, w: "Light" }; return { n: 400, w: "Regular" }; }; + const styleOf = (name) => (/italic|oblique/i.test(name) ? "italic" : "normal"); const fams = [...new Set(brandFonts)]; const srcDirs = [ join(hyperframesDir, "capture/assets/fonts"), @@ -469,13 +487,14 @@ if (brandFonts.length) { const fam = famOf(f); if (!fam) continue; const { n, w } = weightInfo(f); - const clean = `${fam.replace(/[^A-Za-z0-9]/g, "")}-${w}.${extOf(f)}`; + const style = styleOf(f); + const clean = `${fam.replace(/[^A-Za-z0-9]/g, "")}-${w}${style === "italic" ? "-Italic" : ""}.${extOf(f)}`; if (stagedNames.has(clean)) continue; mkdirSync(outDir, { recursive: true }); if (!existsSync(join(outDir, clean))) copyFileSync(join(d, f), join(outDir, clean)); stagedNames.add(clean); faces.push( - `@font-face{font-family:"${fam}";font-weight:${n};font-style:normal;font-display:block;src:url("assets/fonts/${clean}") format("${FMT[extOf(f)]}");}`, + `@font-face{font-family:"${fam}";font-weight:${n};font-style:${style};font-display:block;src:url("assets/fonts/${clean}") format("${FMT[extOf(f)]}");}`, ); } if (faces.length) { diff --git a/skills/faceless-explainer/scripts/captions.mjs b/skills/faceless-explainer/scripts/captions.mjs index 1203b57c6..7e04affe6 100644 --- a/skills/faceless-explainer/scripts/captions.mjs +++ b/skills/faceless-explainer/scripts/captions.mjs @@ -302,6 +302,17 @@ function brandFontFaces(framePath, hyperframesDir) { ].filter((d) => existsSync(d.abs)); const weightOf = (n) => { const s = n.toLowerCase(); + // A numeric axis is the font's own answer, so it beats the word heuristic. Fontsource + // names every face this way ("inter-latin-500-normal.woff2") and carries no weight + // WORD at all, so word-only parsing collapsed a whole family onto 400 and shipped + // exactly one of its faces. + // + // A weight token must not be buried inside a longer run: capture/assets/fonts holds + // hash-named files, and "Newsreader-a1b200c3.woff2" is not a 200-weight face. Hence a + // non-digit before (which also stops "2100" reading as 100) and no alphanumeric after. + // "Roboto900.ttf" still parses — requiring separators on both sides would have lost it. + const numeric = /(?:^|[^0-9])([1-9]00)(?![0-9a-z])/.exec(s); + if (numeric) return Number(numeric[1]); if (/black|heavy|ultra|extrabold/.test(s)) return 800; if (/semibold|demibold/.test(s)) return 600; // before /bold/ — "demibold" contains "bold" if (/bold/.test(s)) return 700; @@ -309,6 +320,14 @@ function brandFontFaces(framePath, hyperframesDir) { if (/light|thin/.test(s)) return 300; return 400; // book / regular / roman }; + // Weight is not the only axis in a filename. Google Fonts ships Newsreader as + // "Newsreader-Italic-VariableFont_opsz,wght.ttf" + "Newsreader-VariableFont_opsz,wght.ttf", + // and the italic sorts first — so without a style axis the italic file claimed the + // family's ONLY 400 slot, the upright file was dropped as a duplicate, and the face + // was declared with no `font-style`. @font-face is deliberately global (the composition + // CSS scoper exempts it, and it has to be), so the whole document then rendered that + // family in italics — captions italicizing every sibling composition. + const styleOf = (n) => (/italic|oblique/i.test(n) ? "italic" : "normal"); const fmtOf = (f) => /\.woff2$/i.test(f) ? "woff2" @@ -345,12 +364,13 @@ function brandFontFaces(framePath, hyperframesDir) { if (claimed.has(f)) continue; // a more specific family already took this file if (!norm(f.replace(/\.(woff2|woff|ttf|otf)$/i, "")).startsWith(key)) continue; const w = weightOf(f); - const dedup = `${fam}-${w}`; - if (seen.has(dedup)) continue; // one src per weight; assets/fonts wins over capture + const style = styleOf(f); + const dedup = `${fam}-${w}-${style}`; + if (seen.has(dedup)) continue; // one src per face; assets/fonts wins over capture seen.add(dedup); claimed.add(f); faces.push( - ` @font-face { font-family: '${fam}'; src: url('${d.rel}/${f}') format('${fmtOf(f)}'); font-weight: ${w}; font-display: block; }`, + ` @font-face { font-family: '${fam}'; src: url('${d.rel}/${f}') format('${fmtOf(f)}'); font-weight: ${w}; font-style: ${style}; font-display: block; }`, ); } } @@ -373,6 +393,8 @@ function brandFontFaces(framePath, hyperframesDir) { return faces.join("\n"); } +export { brandFontFaces }; // exported as a seam for unit testing + // frame.md colors:/typography: → a :root token block, mapped to the fixed semantic // vocab every preset skin references. Robust to per-preset key names: colors are // matched by name, then by luminance. Brand-token overlay (Step 2) flows through diff --git a/skills/faceless-explainer/scripts/captions.test.mjs b/skills/faceless-explainer/scripts/captions.test.mjs index f2724ea42..097623bd0 100644 --- a/skills/faceless-explainer/scripts/captions.test.mjs +++ b/skills/faceless-explainer/scripts/captions.test.mjs @@ -1,8 +1,10 @@ import assert from "node:assert/strict"; -import { readdirSync, readFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; -import { buildFromSkin } from "./captions.mjs"; +import { brandFontFaces, buildFromSkin } from "./captions.mjs"; const presetsDir = fileURLToPath( new URL("../../hyperframes-creative/frame-presets/", import.meta.url), @@ -48,3 +50,170 @@ for (const canvas of ["#f7f3e8", "#111827"]) { }); } } + +// @font-face is document-global on purpose — the composition CSS scoper exempts it, and it +// has to, since a face declaration cannot be scoped. That makes brandFontFaces the one part +// of the captions sub-composition whose output reaches every sibling composition, so it has +// to describe each face exactly: get an axis wrong and the whole document renders the brand +// family wrong. +function withFontProject(files, run) { + const dir = mkdtempSync(join(tmpdir(), "hf-captions-fonts-")); + try { + mkdirSync(join(dir, "assets/fonts"), { recursive: true }); + for (const name of files) writeFileSync(join(dir, "assets/fonts", name), ""); + writeFileSync( + join(dir, "frame.md"), + 'typography:\n display: { fontFamily: "Newsreader", weight: 400 }\n body: { fontFamily: "Inter", weight: 400 }\n', + ); + return run(join(dir, "frame.md"), dir); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +test("an italic file is declared italic, and never squats the family's upright slot", () => { + // Google Fonts' own Newsreader download. The italic sorts first, so before the style axis + // existed it claimed the family's only 400 slot, the upright was dropped as a duplicate, + // and the face shipped with no font-style — italicizing every sibling composition that + // used Newsreader. + const faces = withFontProject( + ["Newsreader-Italic-VariableFont_opsz,wght.ttf", "Newsreader-VariableFont_opsz,wght.ttf"], + brandFontFaces, + ); + const lines = faces.split("\n").filter((line) => line.includes("Newsreader")); + assert.equal(lines.length, 2, "both the upright and the italic file must be declared"); + + const upright = lines.find((line) => line.includes("Newsreader-VariableFont")); + const italic = lines.find((line) => line.includes("Newsreader-Italic-VariableFont")); + assert.ok(upright, "the upright file must survive"); + assert.match(upright, /font-style: normal/); + assert.ok(italic, "the italic file must survive"); + assert.match(italic, /font-style: italic/); +}); + +test("a numeric weight in the filename is read as the weight", () => { + // Fontsource names every face numerically and carries no weight WORD, so word-only + // parsing scored the whole family 400 and shipped exactly one of its four faces. + const faces = withFontProject( + [ + "inter-latin-400-normal.woff2", + "inter-latin-500-normal.woff2", + "inter-latin-600-italic.woff2", + "inter-latin-700-normal.woff2", + ], + brandFontFaces, + ); + const lines = faces.split("\n").filter((line) => line.includes("Inter")); + assert.equal(lines.length, 4, "each face is a distinct weight/style pair"); + for (const [file, weight, style] of [ + ["inter-latin-400-normal", 400, "normal"], + ["inter-latin-500-normal", 500, "normal"], + ["inter-latin-600-italic", 600, "italic"], + ["inter-latin-700-normal", 700, "normal"], + ]) { + const line = lines.find((candidate) => candidate.includes(file)); + assert.ok(line, `${file} must be declared`); + assert.match(line, new RegExp(`font-weight: ${weight};`)); + assert.match(line, new RegExp(`font-style: ${style};`)); + } +}); + +test("word-named weights still parse when the filename carries no numeric axis", () => { + const faces = withFontProject( + ["Newsreader_Bold.woff2", "Newsreader_Regular.woff2"], + brandFontFaces, + ); + const lines = faces.split("\n").filter((line) => line.includes("Newsreader")); + assert.equal(lines.length, 2); + assert.match( + lines.find((line) => line.includes("Bold")), + /font-weight: 700; font-style: normal/, + ); + assert.match( + lines.find((line) => line.includes("Regular")), + /font-weight: 400; font-style: normal/, + ); +}); + +// build-frame.mjs stages captured brand fonts under a REWRITTEN name, and brandFontFaces +// derives the face's axes back out of that name. The two are a contract, and it is easy to +// break silently from either side: build-frame used to drop the style token while renaming, +// so an italic file arrived as "Newsreader-Regular.ttf" and was declared upright — leaving +// the document-global normal slot pointing at italic bytes even once brandFontFaces learned +// about styles. These two tests pin both ends of that contract. +test("the names build-frame.mjs stages round-trip back to the right face", () => { + const faces = withFontProject( + [ + "Newsreader-Regular.ttf", + "Newsreader-Regular-Italic.ttf", + "Inter-400.woff2", + "Inter-600-Italic.woff2", + ], + brandFontFaces, + ); + for (const [file, weight, style] of [ + ["Newsreader-Regular.ttf", 400, "normal"], + ["Newsreader-Regular-Italic.ttf", 400, "italic"], + ["Inter-400.woff2", 400, "normal"], + ["Inter-600-Italic.woff2", 600, "italic"], + ]) { + const line = faces.split("\n").find((candidate) => candidate.includes(`/${file}'`)); + assert.ok(line, `${file} must be declared`); + assert.match(line, new RegExp(`font-weight: ${weight};`)); + assert.match(line, new RegExp(`font-style: ${style};`)); + } +}); + +test("a weight token buried in a longer run is not read as a weight", () => { + // capture/assets/fonts commonly holds hash-named files, and a hash is not a weight. + const faces = withFontProject( + ["Newsreader-a1b200c3.woff2", "Inter-2100.woff2", "Inter900.woff2"], + brandFontFaces, + ); + const weightOf = (file) => + Number(/font-weight: (\d+);/.exec(faces.split("\n").find((l) => l.includes(file)))?.[1]); + + // "200" sits mid-run (…b200c3), so the word path decides: Regular. + assert.equal(weightOf("Newsreader-a1b200c3.woff2"), 400); + // 4-digit guard: "2100" must not read as 100. + assert.equal(weightOf("Inter-2100.woff2"), 400); + // ...but a trailing weight with no separator is still a weight. + assert.equal(weightOf("Inter900.woff2"), 900); +}); + +// captions.mjs ships once per creation workflow because each skill installs standalone, +// and the three copies are meant to be byte-identical. This PR alone had to land the same +// two-axis fix in all three; a future one that lands in only one drifts silently. +test("captions.mjs is byte-identical across the three workflows that ship it", () => { + const [first, ...rest] = ["product-launch-video", "faceless-explainer", "pr-to-video"].map( + (skill) => ({ + skill, + source: readFileSync(new URL(`../../${skill}/scripts/captions.mjs`, import.meta.url), "utf8"), + }), + ); + for (const other of rest) { + assert.equal(other.source, first.source, `${other.skill} drifted from ${first.skill}`); + } +}); + +test("every build-frame.mjs copy stages the style axis it promises", () => { + for (const skill of ["product-launch-video", "faceless-explainer", "pr-to-video"]) { + const source = readFileSync( + new URL(`../../${skill}/scripts/build-frame.mjs`, import.meta.url), + "utf8", + ); + // The staged filename must carry the style, or the italic and upright faces of one + // weight collide on a single name and only whichever sorts first survives. + assert.match( + source, + /const clean = `\$\{fam\.replace\(\/\[\^A-Za-z0-9\]\/g, ""\)\}-\$\{w\}\$\{style === "italic" \? "-Italic" : ""\}\./, + `${skill}/build-frame.mjs must keep the style token in the staged name`, + ); + // ...and the emitted descriptor must report the real style, not a hardcoded normal. + assert.doesNotMatch( + source, + /font-weight:\$\{n\};font-style:normal/, + `${skill}/build-frame.mjs must not assert font-style:normal over captured bytes`, + ); + } +}); diff --git a/skills/pr-to-video/scripts/build-frame.mjs b/skills/pr-to-video/scripts/build-frame.mjs index 259c0e4ba..b5f685598 100644 --- a/skills/pr-to-video/scripts/build-frame.mjs +++ b/skills/pr-to-video/scripts/build-frame.mjs @@ -466,8 +466,15 @@ if (brandFonts.length || (brandColors.length && presetColors.length)) { // ── stage brand font files + emit @font-face ────────────────────────────────── // A brand font is rarely a Google font, so renaming the family in frame.md is not enough: // nothing loads the actual face. If the capture downloaded font files, copy them to -// assets/fonts/ under CLEAN, weight-named names (so captions.mjs' family-prefix matcher +// assets/fonts/ under CLEAN, face-named names (so captions.mjs' family-prefix matcher // finds them too) and append a ready-to-paste, ROOT-RELATIVE @font-face block to frame.md. +// +// The staged NAME is a contract, not cosmetics: captions.mjs derives each face's weight and +// style back out of it. So the name has to carry every axis that distinguishes one face from +// another, and the dedup key has to be the whole face. Naming on weight alone made Google's +// two-file Newsreader download (upright + italic, both scoring "Regular") collide on one +// slot: the italic sorts first, took the name, the upright was never staged, and the block +// below then asserted font-style:normal over italic bytes. if (brandFonts.length) { const norm = (s) => String(s) @@ -477,6 +484,16 @@ if (brandFonts.length) { const FMT = { woff2: "woff2", woff: "woff", ttf: "truetype", otf: "opentype" }; const weightInfo = (name) => { const s = name.toLowerCase(); + // A numeric axis is the font's own answer, so it beats the word heuristic. Fontsource + // names every face that way and carries no weight WORD at all, so word-only parsing + // scored a whole family "Regular" and staged exactly one of its faces. + // + // A weight token must not be buried inside a longer run: this reads capture files, + // which are commonly hash-named, and "Newsreader-a1b200c3.woff2" is not a 200-weight + // face. Hence a non-digit before (which also stops "2100" reading as 100) and no + // alphanumeric after. "Roboto900.ttf" still parses. + const numeric = /(?:^|[^0-9])([1-9]00)(?![0-9a-z])/.exec(s); + if (numeric) return { n: Number(numeric[1]), w: numeric[1] }; if (/black|heavy|ultra|extrabold/.test(s)) return { n: 800, w: "ExtraBold" }; if (/semibold|demibold/.test(s)) return { n: 600, w: "SemiBold" }; if (/bold/.test(s)) return { n: 700, w: "Bold" }; @@ -484,6 +501,7 @@ if (brandFonts.length) { if (/light|thin/.test(s)) return { n: 300, w: "Light" }; return { n: 400, w: "Regular" }; }; + const styleOf = (name) => (/italic|oblique/i.test(name) ? "italic" : "normal"); const fams = [...new Set(brandFonts)]; const srcDirs = [ join(hyperframesDir, "capture/assets/fonts"), @@ -504,13 +522,14 @@ if (brandFonts.length) { const fam = famOf(f); if (!fam) continue; const { n, w } = weightInfo(f); - const clean = `${fam.replace(/[^A-Za-z0-9]/g, "")}-${w}.${extOf(f)}`; + const style = styleOf(f); + const clean = `${fam.replace(/[^A-Za-z0-9]/g, "")}-${w}${style === "italic" ? "-Italic" : ""}.${extOf(f)}`; if (stagedNames.has(clean)) continue; mkdirSync(outDir, { recursive: true }); if (!existsSync(join(outDir, clean))) copyFileSync(join(d, f), join(outDir, clean)); stagedNames.add(clean); faces.push( - `@font-face{font-family:"${fam}";font-weight:${n};font-style:normal;font-display:block;src:url("assets/fonts/${clean}") format("${FMT[extOf(f)]}");}`, + `@font-face{font-family:"${fam}";font-weight:${n};font-style:${style};font-display:block;src:url("assets/fonts/${clean}") format("${FMT[extOf(f)]}");}`, ); } if (faces.length) { diff --git a/skills/pr-to-video/scripts/captions.mjs b/skills/pr-to-video/scripts/captions.mjs index 1203b57c6..7e04affe6 100644 --- a/skills/pr-to-video/scripts/captions.mjs +++ b/skills/pr-to-video/scripts/captions.mjs @@ -302,6 +302,17 @@ function brandFontFaces(framePath, hyperframesDir) { ].filter((d) => existsSync(d.abs)); const weightOf = (n) => { const s = n.toLowerCase(); + // A numeric axis is the font's own answer, so it beats the word heuristic. Fontsource + // names every face this way ("inter-latin-500-normal.woff2") and carries no weight + // WORD at all, so word-only parsing collapsed a whole family onto 400 and shipped + // exactly one of its faces. + // + // A weight token must not be buried inside a longer run: capture/assets/fonts holds + // hash-named files, and "Newsreader-a1b200c3.woff2" is not a 200-weight face. Hence a + // non-digit before (which also stops "2100" reading as 100) and no alphanumeric after. + // "Roboto900.ttf" still parses — requiring separators on both sides would have lost it. + const numeric = /(?:^|[^0-9])([1-9]00)(?![0-9a-z])/.exec(s); + if (numeric) return Number(numeric[1]); if (/black|heavy|ultra|extrabold/.test(s)) return 800; if (/semibold|demibold/.test(s)) return 600; // before /bold/ — "demibold" contains "bold" if (/bold/.test(s)) return 700; @@ -309,6 +320,14 @@ function brandFontFaces(framePath, hyperframesDir) { if (/light|thin/.test(s)) return 300; return 400; // book / regular / roman }; + // Weight is not the only axis in a filename. Google Fonts ships Newsreader as + // "Newsreader-Italic-VariableFont_opsz,wght.ttf" + "Newsreader-VariableFont_opsz,wght.ttf", + // and the italic sorts first — so without a style axis the italic file claimed the + // family's ONLY 400 slot, the upright file was dropped as a duplicate, and the face + // was declared with no `font-style`. @font-face is deliberately global (the composition + // CSS scoper exempts it, and it has to be), so the whole document then rendered that + // family in italics — captions italicizing every sibling composition. + const styleOf = (n) => (/italic|oblique/i.test(n) ? "italic" : "normal"); const fmtOf = (f) => /\.woff2$/i.test(f) ? "woff2" @@ -345,12 +364,13 @@ function brandFontFaces(framePath, hyperframesDir) { if (claimed.has(f)) continue; // a more specific family already took this file if (!norm(f.replace(/\.(woff2|woff|ttf|otf)$/i, "")).startsWith(key)) continue; const w = weightOf(f); - const dedup = `${fam}-${w}`; - if (seen.has(dedup)) continue; // one src per weight; assets/fonts wins over capture + const style = styleOf(f); + const dedup = `${fam}-${w}-${style}`; + if (seen.has(dedup)) continue; // one src per face; assets/fonts wins over capture seen.add(dedup); claimed.add(f); faces.push( - ` @font-face { font-family: '${fam}'; src: url('${d.rel}/${f}') format('${fmtOf(f)}'); font-weight: ${w}; font-display: block; }`, + ` @font-face { font-family: '${fam}'; src: url('${d.rel}/${f}') format('${fmtOf(f)}'); font-weight: ${w}; font-style: ${style}; font-display: block; }`, ); } } @@ -373,6 +393,8 @@ function brandFontFaces(framePath, hyperframesDir) { return faces.join("\n"); } +export { brandFontFaces }; // exported as a seam for unit testing + // frame.md colors:/typography: → a :root token block, mapped to the fixed semantic // vocab every preset skin references. Robust to per-preset key names: colors are // matched by name, then by luminance. Brand-token overlay (Step 2) flows through diff --git a/skills/pr-to-video/scripts/captions.test.mjs b/skills/pr-to-video/scripts/captions.test.mjs index f2724ea42..097623bd0 100644 --- a/skills/pr-to-video/scripts/captions.test.mjs +++ b/skills/pr-to-video/scripts/captions.test.mjs @@ -1,8 +1,10 @@ import assert from "node:assert/strict"; -import { readdirSync, readFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; -import { buildFromSkin } from "./captions.mjs"; +import { brandFontFaces, buildFromSkin } from "./captions.mjs"; const presetsDir = fileURLToPath( new URL("../../hyperframes-creative/frame-presets/", import.meta.url), @@ -48,3 +50,170 @@ for (const canvas of ["#f7f3e8", "#111827"]) { }); } } + +// @font-face is document-global on purpose — the composition CSS scoper exempts it, and it +// has to, since a face declaration cannot be scoped. That makes brandFontFaces the one part +// of the captions sub-composition whose output reaches every sibling composition, so it has +// to describe each face exactly: get an axis wrong and the whole document renders the brand +// family wrong. +function withFontProject(files, run) { + const dir = mkdtempSync(join(tmpdir(), "hf-captions-fonts-")); + try { + mkdirSync(join(dir, "assets/fonts"), { recursive: true }); + for (const name of files) writeFileSync(join(dir, "assets/fonts", name), ""); + writeFileSync( + join(dir, "frame.md"), + 'typography:\n display: { fontFamily: "Newsreader", weight: 400 }\n body: { fontFamily: "Inter", weight: 400 }\n', + ); + return run(join(dir, "frame.md"), dir); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +test("an italic file is declared italic, and never squats the family's upright slot", () => { + // Google Fonts' own Newsreader download. The italic sorts first, so before the style axis + // existed it claimed the family's only 400 slot, the upright was dropped as a duplicate, + // and the face shipped with no font-style — italicizing every sibling composition that + // used Newsreader. + const faces = withFontProject( + ["Newsreader-Italic-VariableFont_opsz,wght.ttf", "Newsreader-VariableFont_opsz,wght.ttf"], + brandFontFaces, + ); + const lines = faces.split("\n").filter((line) => line.includes("Newsreader")); + assert.equal(lines.length, 2, "both the upright and the italic file must be declared"); + + const upright = lines.find((line) => line.includes("Newsreader-VariableFont")); + const italic = lines.find((line) => line.includes("Newsreader-Italic-VariableFont")); + assert.ok(upright, "the upright file must survive"); + assert.match(upright, /font-style: normal/); + assert.ok(italic, "the italic file must survive"); + assert.match(italic, /font-style: italic/); +}); + +test("a numeric weight in the filename is read as the weight", () => { + // Fontsource names every face numerically and carries no weight WORD, so word-only + // parsing scored the whole family 400 and shipped exactly one of its four faces. + const faces = withFontProject( + [ + "inter-latin-400-normal.woff2", + "inter-latin-500-normal.woff2", + "inter-latin-600-italic.woff2", + "inter-latin-700-normal.woff2", + ], + brandFontFaces, + ); + const lines = faces.split("\n").filter((line) => line.includes("Inter")); + assert.equal(lines.length, 4, "each face is a distinct weight/style pair"); + for (const [file, weight, style] of [ + ["inter-latin-400-normal", 400, "normal"], + ["inter-latin-500-normal", 500, "normal"], + ["inter-latin-600-italic", 600, "italic"], + ["inter-latin-700-normal", 700, "normal"], + ]) { + const line = lines.find((candidate) => candidate.includes(file)); + assert.ok(line, `${file} must be declared`); + assert.match(line, new RegExp(`font-weight: ${weight};`)); + assert.match(line, new RegExp(`font-style: ${style};`)); + } +}); + +test("word-named weights still parse when the filename carries no numeric axis", () => { + const faces = withFontProject( + ["Newsreader_Bold.woff2", "Newsreader_Regular.woff2"], + brandFontFaces, + ); + const lines = faces.split("\n").filter((line) => line.includes("Newsreader")); + assert.equal(lines.length, 2); + assert.match( + lines.find((line) => line.includes("Bold")), + /font-weight: 700; font-style: normal/, + ); + assert.match( + lines.find((line) => line.includes("Regular")), + /font-weight: 400; font-style: normal/, + ); +}); + +// build-frame.mjs stages captured brand fonts under a REWRITTEN name, and brandFontFaces +// derives the face's axes back out of that name. The two are a contract, and it is easy to +// break silently from either side: build-frame used to drop the style token while renaming, +// so an italic file arrived as "Newsreader-Regular.ttf" and was declared upright — leaving +// the document-global normal slot pointing at italic bytes even once brandFontFaces learned +// about styles. These two tests pin both ends of that contract. +test("the names build-frame.mjs stages round-trip back to the right face", () => { + const faces = withFontProject( + [ + "Newsreader-Regular.ttf", + "Newsreader-Regular-Italic.ttf", + "Inter-400.woff2", + "Inter-600-Italic.woff2", + ], + brandFontFaces, + ); + for (const [file, weight, style] of [ + ["Newsreader-Regular.ttf", 400, "normal"], + ["Newsreader-Regular-Italic.ttf", 400, "italic"], + ["Inter-400.woff2", 400, "normal"], + ["Inter-600-Italic.woff2", 600, "italic"], + ]) { + const line = faces.split("\n").find((candidate) => candidate.includes(`/${file}'`)); + assert.ok(line, `${file} must be declared`); + assert.match(line, new RegExp(`font-weight: ${weight};`)); + assert.match(line, new RegExp(`font-style: ${style};`)); + } +}); + +test("a weight token buried in a longer run is not read as a weight", () => { + // capture/assets/fonts commonly holds hash-named files, and a hash is not a weight. + const faces = withFontProject( + ["Newsreader-a1b200c3.woff2", "Inter-2100.woff2", "Inter900.woff2"], + brandFontFaces, + ); + const weightOf = (file) => + Number(/font-weight: (\d+);/.exec(faces.split("\n").find((l) => l.includes(file)))?.[1]); + + // "200" sits mid-run (…b200c3), so the word path decides: Regular. + assert.equal(weightOf("Newsreader-a1b200c3.woff2"), 400); + // 4-digit guard: "2100" must not read as 100. + assert.equal(weightOf("Inter-2100.woff2"), 400); + // ...but a trailing weight with no separator is still a weight. + assert.equal(weightOf("Inter900.woff2"), 900); +}); + +// captions.mjs ships once per creation workflow because each skill installs standalone, +// and the three copies are meant to be byte-identical. This PR alone had to land the same +// two-axis fix in all three; a future one that lands in only one drifts silently. +test("captions.mjs is byte-identical across the three workflows that ship it", () => { + const [first, ...rest] = ["product-launch-video", "faceless-explainer", "pr-to-video"].map( + (skill) => ({ + skill, + source: readFileSync(new URL(`../../${skill}/scripts/captions.mjs`, import.meta.url), "utf8"), + }), + ); + for (const other of rest) { + assert.equal(other.source, first.source, `${other.skill} drifted from ${first.skill}`); + } +}); + +test("every build-frame.mjs copy stages the style axis it promises", () => { + for (const skill of ["product-launch-video", "faceless-explainer", "pr-to-video"]) { + const source = readFileSync( + new URL(`../../${skill}/scripts/build-frame.mjs`, import.meta.url), + "utf8", + ); + // The staged filename must carry the style, or the italic and upright faces of one + // weight collide on a single name and only whichever sorts first survives. + assert.match( + source, + /const clean = `\$\{fam\.replace\(\/\[\^A-Za-z0-9\]\/g, ""\)\}-\$\{w\}\$\{style === "italic" \? "-Italic" : ""\}\./, + `${skill}/build-frame.mjs must keep the style token in the staged name`, + ); + // ...and the emitted descriptor must report the real style, not a hardcoded normal. + assert.doesNotMatch( + source, + /font-weight:\$\{n\};font-style:normal/, + `${skill}/build-frame.mjs must not assert font-style:normal over captured bytes`, + ); + } +}); diff --git a/skills/product-launch-video/scripts/build-frame.mjs b/skills/product-launch-video/scripts/build-frame.mjs index 269b60529..7f3bf0144 100644 --- a/skills/product-launch-video/scripts/build-frame.mjs +++ b/skills/product-launch-video/scripts/build-frame.mjs @@ -429,8 +429,15 @@ if (brandFonts.length || (brandColors.length && presetColors.length)) { // ── stage brand font files + emit @font-face ────────────────────────────────── // A brand font is rarely a Google font, so renaming the family in frame.md is not enough: // nothing loads the actual face. If the capture downloaded font files, copy them to -// assets/fonts/ under CLEAN, weight-named names (so captions.mjs' family-prefix matcher +// assets/fonts/ under CLEAN, face-named names (so captions.mjs' family-prefix matcher // finds them too) and append a ready-to-paste, ROOT-RELATIVE @font-face block to frame.md. +// +// The staged NAME is a contract, not cosmetics: captions.mjs derives each face's weight and +// style back out of it. So the name has to carry every axis that distinguishes one face from +// another, and the dedup key has to be the whole face. Naming on weight alone made Google's +// two-file Newsreader download (upright + italic, both scoring "Regular") collide on one +// slot: the italic sorts first, took the name, the upright was never staged, and the block +// below then asserted font-style:normal over italic bytes. if (brandFonts.length) { const norm = (s) => String(s) @@ -440,6 +447,16 @@ if (brandFonts.length) { const FMT = { woff2: "woff2", woff: "woff", ttf: "truetype", otf: "opentype" }; const weightInfo = (name) => { const s = name.toLowerCase(); + // A numeric axis is the font's own answer, so it beats the word heuristic. Fontsource + // names every face that way and carries no weight WORD at all, so word-only parsing + // scored a whole family "Regular" and staged exactly one of its faces. + // + // A weight token must not be buried inside a longer run: this reads capture files, + // which are commonly hash-named, and "Newsreader-a1b200c3.woff2" is not a 200-weight + // face. Hence a non-digit before (which also stops "2100" reading as 100) and no + // alphanumeric after. "Roboto900.ttf" still parses. + const numeric = /(?:^|[^0-9])([1-9]00)(?![0-9a-z])/.exec(s); + if (numeric) return { n: Number(numeric[1]), w: numeric[1] }; if (/black|heavy|ultra|extrabold/.test(s)) return { n: 800, w: "ExtraBold" }; if (/semibold|demibold/.test(s)) return { n: 600, w: "SemiBold" }; if (/bold/.test(s)) return { n: 700, w: "Bold" }; @@ -447,6 +464,7 @@ if (brandFonts.length) { if (/light|thin/.test(s)) return { n: 300, w: "Light" }; return { n: 400, w: "Regular" }; }; + const styleOf = (name) => (/italic|oblique/i.test(name) ? "italic" : "normal"); const fams = [...new Set(brandFonts)]; const srcDirs = [ join(hyperframesDir, "capture/assets/fonts"), @@ -467,13 +485,14 @@ if (brandFonts.length) { const fam = famOf(f); if (!fam) continue; const { n, w } = weightInfo(f); - const clean = `${fam.replace(/[^A-Za-z0-9]/g, "")}-${w}.${extOf(f)}`; + const style = styleOf(f); + const clean = `${fam.replace(/[^A-Za-z0-9]/g, "")}-${w}${style === "italic" ? "-Italic" : ""}.${extOf(f)}`; if (stagedNames.has(clean)) continue; mkdirSync(outDir, { recursive: true }); if (!existsSync(join(outDir, clean))) copyFileSync(join(d, f), join(outDir, clean)); stagedNames.add(clean); faces.push( - `@font-face{font-family:"${fam}";font-weight:${n};font-style:normal;font-display:block;src:url("assets/fonts/${clean}") format("${FMT[extOf(f)]}");}`, + `@font-face{font-family:"${fam}";font-weight:${n};font-style:${style};font-display:block;src:url("assets/fonts/${clean}") format("${FMT[extOf(f)]}");}`, ); } if (faces.length) { diff --git a/skills/product-launch-video/scripts/captions.mjs b/skills/product-launch-video/scripts/captions.mjs index 1203b57c6..7e04affe6 100644 --- a/skills/product-launch-video/scripts/captions.mjs +++ b/skills/product-launch-video/scripts/captions.mjs @@ -302,6 +302,17 @@ function brandFontFaces(framePath, hyperframesDir) { ].filter((d) => existsSync(d.abs)); const weightOf = (n) => { const s = n.toLowerCase(); + // A numeric axis is the font's own answer, so it beats the word heuristic. Fontsource + // names every face this way ("inter-latin-500-normal.woff2") and carries no weight + // WORD at all, so word-only parsing collapsed a whole family onto 400 and shipped + // exactly one of its faces. + // + // A weight token must not be buried inside a longer run: capture/assets/fonts holds + // hash-named files, and "Newsreader-a1b200c3.woff2" is not a 200-weight face. Hence a + // non-digit before (which also stops "2100" reading as 100) and no alphanumeric after. + // "Roboto900.ttf" still parses — requiring separators on both sides would have lost it. + const numeric = /(?:^|[^0-9])([1-9]00)(?![0-9a-z])/.exec(s); + if (numeric) return Number(numeric[1]); if (/black|heavy|ultra|extrabold/.test(s)) return 800; if (/semibold|demibold/.test(s)) return 600; // before /bold/ — "demibold" contains "bold" if (/bold/.test(s)) return 700; @@ -309,6 +320,14 @@ function brandFontFaces(framePath, hyperframesDir) { if (/light|thin/.test(s)) return 300; return 400; // book / regular / roman }; + // Weight is not the only axis in a filename. Google Fonts ships Newsreader as + // "Newsreader-Italic-VariableFont_opsz,wght.ttf" + "Newsreader-VariableFont_opsz,wght.ttf", + // and the italic sorts first — so without a style axis the italic file claimed the + // family's ONLY 400 slot, the upright file was dropped as a duplicate, and the face + // was declared with no `font-style`. @font-face is deliberately global (the composition + // CSS scoper exempts it, and it has to be), so the whole document then rendered that + // family in italics — captions italicizing every sibling composition. + const styleOf = (n) => (/italic|oblique/i.test(n) ? "italic" : "normal"); const fmtOf = (f) => /\.woff2$/i.test(f) ? "woff2" @@ -345,12 +364,13 @@ function brandFontFaces(framePath, hyperframesDir) { if (claimed.has(f)) continue; // a more specific family already took this file if (!norm(f.replace(/\.(woff2|woff|ttf|otf)$/i, "")).startsWith(key)) continue; const w = weightOf(f); - const dedup = `${fam}-${w}`; - if (seen.has(dedup)) continue; // one src per weight; assets/fonts wins over capture + const style = styleOf(f); + const dedup = `${fam}-${w}-${style}`; + if (seen.has(dedup)) continue; // one src per face; assets/fonts wins over capture seen.add(dedup); claimed.add(f); faces.push( - ` @font-face { font-family: '${fam}'; src: url('${d.rel}/${f}') format('${fmtOf(f)}'); font-weight: ${w}; font-display: block; }`, + ` @font-face { font-family: '${fam}'; src: url('${d.rel}/${f}') format('${fmtOf(f)}'); font-weight: ${w}; font-style: ${style}; font-display: block; }`, ); } } @@ -373,6 +393,8 @@ function brandFontFaces(framePath, hyperframesDir) { return faces.join("\n"); } +export { brandFontFaces }; // exported as a seam for unit testing + // frame.md colors:/typography: → a :root token block, mapped to the fixed semantic // vocab every preset skin references. Robust to per-preset key names: colors are // matched by name, then by luminance. Brand-token overlay (Step 2) flows through diff --git a/skills/product-launch-video/scripts/captions.test.mjs b/skills/product-launch-video/scripts/captions.test.mjs index f2724ea42..097623bd0 100644 --- a/skills/product-launch-video/scripts/captions.test.mjs +++ b/skills/product-launch-video/scripts/captions.test.mjs @@ -1,8 +1,10 @@ import assert from "node:assert/strict"; -import { readdirSync, readFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; -import { buildFromSkin } from "./captions.mjs"; +import { brandFontFaces, buildFromSkin } from "./captions.mjs"; const presetsDir = fileURLToPath( new URL("../../hyperframes-creative/frame-presets/", import.meta.url), @@ -48,3 +50,170 @@ for (const canvas of ["#f7f3e8", "#111827"]) { }); } } + +// @font-face is document-global on purpose — the composition CSS scoper exempts it, and it +// has to, since a face declaration cannot be scoped. That makes brandFontFaces the one part +// of the captions sub-composition whose output reaches every sibling composition, so it has +// to describe each face exactly: get an axis wrong and the whole document renders the brand +// family wrong. +function withFontProject(files, run) { + const dir = mkdtempSync(join(tmpdir(), "hf-captions-fonts-")); + try { + mkdirSync(join(dir, "assets/fonts"), { recursive: true }); + for (const name of files) writeFileSync(join(dir, "assets/fonts", name), ""); + writeFileSync( + join(dir, "frame.md"), + 'typography:\n display: { fontFamily: "Newsreader", weight: 400 }\n body: { fontFamily: "Inter", weight: 400 }\n', + ); + return run(join(dir, "frame.md"), dir); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +test("an italic file is declared italic, and never squats the family's upright slot", () => { + // Google Fonts' own Newsreader download. The italic sorts first, so before the style axis + // existed it claimed the family's only 400 slot, the upright was dropped as a duplicate, + // and the face shipped with no font-style — italicizing every sibling composition that + // used Newsreader. + const faces = withFontProject( + ["Newsreader-Italic-VariableFont_opsz,wght.ttf", "Newsreader-VariableFont_opsz,wght.ttf"], + brandFontFaces, + ); + const lines = faces.split("\n").filter((line) => line.includes("Newsreader")); + assert.equal(lines.length, 2, "both the upright and the italic file must be declared"); + + const upright = lines.find((line) => line.includes("Newsreader-VariableFont")); + const italic = lines.find((line) => line.includes("Newsreader-Italic-VariableFont")); + assert.ok(upright, "the upright file must survive"); + assert.match(upright, /font-style: normal/); + assert.ok(italic, "the italic file must survive"); + assert.match(italic, /font-style: italic/); +}); + +test("a numeric weight in the filename is read as the weight", () => { + // Fontsource names every face numerically and carries no weight WORD, so word-only + // parsing scored the whole family 400 and shipped exactly one of its four faces. + const faces = withFontProject( + [ + "inter-latin-400-normal.woff2", + "inter-latin-500-normal.woff2", + "inter-latin-600-italic.woff2", + "inter-latin-700-normal.woff2", + ], + brandFontFaces, + ); + const lines = faces.split("\n").filter((line) => line.includes("Inter")); + assert.equal(lines.length, 4, "each face is a distinct weight/style pair"); + for (const [file, weight, style] of [ + ["inter-latin-400-normal", 400, "normal"], + ["inter-latin-500-normal", 500, "normal"], + ["inter-latin-600-italic", 600, "italic"], + ["inter-latin-700-normal", 700, "normal"], + ]) { + const line = lines.find((candidate) => candidate.includes(file)); + assert.ok(line, `${file} must be declared`); + assert.match(line, new RegExp(`font-weight: ${weight};`)); + assert.match(line, new RegExp(`font-style: ${style};`)); + } +}); + +test("word-named weights still parse when the filename carries no numeric axis", () => { + const faces = withFontProject( + ["Newsreader_Bold.woff2", "Newsreader_Regular.woff2"], + brandFontFaces, + ); + const lines = faces.split("\n").filter((line) => line.includes("Newsreader")); + assert.equal(lines.length, 2); + assert.match( + lines.find((line) => line.includes("Bold")), + /font-weight: 700; font-style: normal/, + ); + assert.match( + lines.find((line) => line.includes("Regular")), + /font-weight: 400; font-style: normal/, + ); +}); + +// build-frame.mjs stages captured brand fonts under a REWRITTEN name, and brandFontFaces +// derives the face's axes back out of that name. The two are a contract, and it is easy to +// break silently from either side: build-frame used to drop the style token while renaming, +// so an italic file arrived as "Newsreader-Regular.ttf" and was declared upright — leaving +// the document-global normal slot pointing at italic bytes even once brandFontFaces learned +// about styles. These two tests pin both ends of that contract. +test("the names build-frame.mjs stages round-trip back to the right face", () => { + const faces = withFontProject( + [ + "Newsreader-Regular.ttf", + "Newsreader-Regular-Italic.ttf", + "Inter-400.woff2", + "Inter-600-Italic.woff2", + ], + brandFontFaces, + ); + for (const [file, weight, style] of [ + ["Newsreader-Regular.ttf", 400, "normal"], + ["Newsreader-Regular-Italic.ttf", 400, "italic"], + ["Inter-400.woff2", 400, "normal"], + ["Inter-600-Italic.woff2", 600, "italic"], + ]) { + const line = faces.split("\n").find((candidate) => candidate.includes(`/${file}'`)); + assert.ok(line, `${file} must be declared`); + assert.match(line, new RegExp(`font-weight: ${weight};`)); + assert.match(line, new RegExp(`font-style: ${style};`)); + } +}); + +test("a weight token buried in a longer run is not read as a weight", () => { + // capture/assets/fonts commonly holds hash-named files, and a hash is not a weight. + const faces = withFontProject( + ["Newsreader-a1b200c3.woff2", "Inter-2100.woff2", "Inter900.woff2"], + brandFontFaces, + ); + const weightOf = (file) => + Number(/font-weight: (\d+);/.exec(faces.split("\n").find((l) => l.includes(file)))?.[1]); + + // "200" sits mid-run (…b200c3), so the word path decides: Regular. + assert.equal(weightOf("Newsreader-a1b200c3.woff2"), 400); + // 4-digit guard: "2100" must not read as 100. + assert.equal(weightOf("Inter-2100.woff2"), 400); + // ...but a trailing weight with no separator is still a weight. + assert.equal(weightOf("Inter900.woff2"), 900); +}); + +// captions.mjs ships once per creation workflow because each skill installs standalone, +// and the three copies are meant to be byte-identical. This PR alone had to land the same +// two-axis fix in all three; a future one that lands in only one drifts silently. +test("captions.mjs is byte-identical across the three workflows that ship it", () => { + const [first, ...rest] = ["product-launch-video", "faceless-explainer", "pr-to-video"].map( + (skill) => ({ + skill, + source: readFileSync(new URL(`../../${skill}/scripts/captions.mjs`, import.meta.url), "utf8"), + }), + ); + for (const other of rest) { + assert.equal(other.source, first.source, `${other.skill} drifted from ${first.skill}`); + } +}); + +test("every build-frame.mjs copy stages the style axis it promises", () => { + for (const skill of ["product-launch-video", "faceless-explainer", "pr-to-video"]) { + const source = readFileSync( + new URL(`../../${skill}/scripts/build-frame.mjs`, import.meta.url), + "utf8", + ); + // The staged filename must carry the style, or the italic and upright faces of one + // weight collide on a single name and only whichever sorts first survives. + assert.match( + source, + /const clean = `\$\{fam\.replace\(\/\[\^A-Za-z0-9\]\/g, ""\)\}-\$\{w\}\$\{style === "italic" \? "-Italic" : ""\}\./, + `${skill}/build-frame.mjs must keep the style token in the staged name`, + ); + // ...and the emitted descriptor must report the real style, not a hardcoded normal. + assert.doesNotMatch( + source, + /font-weight:\$\{n\};font-style:normal/, + `${skill}/build-frame.mjs must not assert font-style:normal over captured bytes`, + ); + } +});