diff --git a/compiler/src/validate/gates.ts b/compiler/src/validate/gates.ts index d191378..9f9ed4f 100644 --- a/compiler/src/validate/gates.ts +++ b/compiler/src/validate/gates.ts @@ -222,7 +222,7 @@ export function gate2Assets(assetGraph: AssetGraph, fontGraph: FontGraph, gen: { if (zeroByte > 0) issues.push(`${zeroByte} zero-byte downloaded assets`); if (skippedNoReason > 0) issues.push(`${skippedNoReason} skipped assets without reason`); if (gen.remoteRefs.length > 0) issues.push(`${gen.remoteRefs.length} generated refs point to remote origin`); - if (gen.failed404.length > 0) issues.push(`${gen.failed404.length} generated asset refs 404`); + if (gen.failed404.length > 0) issues.push(`${gen.failed404.length} generated asset refs missing (HTTP >= 400 or file absent from export)`); const fontsResolvedOrFallback = fontGraph.entries.every((f) => f.status === "resolved" || (f.status === "fallback" && f.reason)); if (!fontsResolvedOrFallback) issues.push("font declarations not resolved/fallback-recorded"); return { diff --git a/compiler/src/validate/render.ts b/compiler/src/validate/render.ts index 784ec36..8c0d8fd 100644 --- a/compiler/src/validate/render.ts +++ b/compiler/src/validate/render.ts @@ -270,37 +270,47 @@ export type RenderResult = { export type FontFaceStatus = { family: string; weight: string; style: string; status: string }; /** Pure decision for the font-load wait: given the current `document.fonts` entries, return the - * families that are DECLARED (an @font-face exists for them) but not yet `"loaded"`. A family is - * considered pending while ANY of its faces is still `"unloaded"` or `"loading"`; a face that - * `"error"`ed is treated as terminal (it will never load, so waiting longer is pointless — it is - * not reported as pending). Empty return ⇒ every declared face has reached a terminal state and - * the DOM may be measured. The check is state-based (not time-based), so the caller's poll is - * deterministic: it ends the instant this returns empty, regardless of wall-clock timing. */ + * families that are DECLARED (an @font-face exists for them) and still actively `"loading"`. Only + * `"loading"` is pending: browsers lazy-load faces, so after `document.fonts.ready` a face the + * rendered text actually needs is already fetching (`"loading"`), while a face left `"unloaded"` + * is by definition unreferenced by any rendered text and will never load on its own — waiting on + * it just burns the cap. `"loaded"`/`"error"` are terminal. Empty return ⇒ no face is still + * fetching and the DOM may be measured. The check is state-based (not time-based), so the caller's + * poll is deterministic: it ends the instant this returns empty, regardless of wall-clock timing. */ export function pendingFontFamilies(faces: FontFaceStatus[]): string[] { const pending = new Set(); - const terminalByFamily = new Map(); for (const f of faces) { const fam = f.family.replace(/^["']|["']$/g, ""); - if (f.status === "loaded" || f.status === "error") { - if (!terminalByFamily.has(fam)) terminalByFamily.set(fam, true); - } else { - // "unloaded" | "loading" (or any non-terminal state) ⇒ this family is still pending. - pending.add(fam); - terminalByFamily.set(fam, false); - } + // Only a face still actively fetching keeps its family pending; "unloaded" (unreferenced), + // "loaded", and "error" are all non-blocking. + if (f.status === "loading") pending.add(fam); } return [...pending].sort(); } -/** In-page: await `document.fonts.ready`, then poll (bounded) until every declared @font-face has - * reached a terminal state (`loaded`/`error`). Returns the still-pending families when the bound - * expired, or an empty array once all faces settled. Runs entirely inside the browser context so - * it reads the live FontFaceSet. Bounded + state-based ⇒ deterministic (ends on state, never on a - * fixed sleep). `capMs` is a hard ceiling so a hung/erroring font request can't stall the render. */ +/** Declared @font-face faces left `"unloaded"` after the wait bound: not fetched because no + * rendered text resolves to them (unreferenced weight/style/unicode-subset siblings). Reported + * per-face (family+weight+style) for an informational-only log — these are benign, never a + * fidelity problem, since the text that IS measured renders in the face that actually loaded. */ +export function unreferencedFontFaces(faces: FontFaceStatus[]): FontFaceStatus[] { + return faces + .filter((f) => f.status === "unloaded") + .map((f) => ({ family: f.family.replace(/^["']|["']$/g, ""), weight: f.weight, style: f.style, status: f.status })) + .sort((a, b) => `${a.family} ${a.weight} ${a.style}`.localeCompare(`${b.family} ${b.weight} ${b.style}`)); +} + +/** In-page: await `document.fonts.ready`, then poll (bounded) until no declared @font-face is still + * `"loading"`. Browsers lazy-load faces, so once ready has fired the faces the rendered text needs + * are already fetching; the poll only waits on those. Returns `{ pending, unreferenced }`: `pending` + * = families whose face was STILL `"loading"` when the bound expired (a genuinely stuck fetch, kept + * as a warning); `unreferenced` = faces left `"unloaded"` (declared but no rendered text resolves to + * them — benign, surfaced per-face for an informational log only). Runs entirely inside the browser + * context so it reads the live FontFaceSet. Bounded + state-based ⇒ deterministic (ends on state, + * never on a fixed sleep). `capMs` is a hard ceiling so a hung font request can't stall the render. */ async function awaitFontsLoaded( page: import("playwright").Page, opts?: { capMs?: number; pollMs?: number }, -): Promise { +): Promise<{ pending: string[]; unreferenced: FontFaceStatus[] }> { const capMs = opts?.capMs ?? 3000; const pollMs = opts?.pollMs ?? 50; try { @@ -308,36 +318,42 @@ async function awaitFontsLoaded( async ({ capMs, pollMs }) => { const doc = document as Document; const set = doc.fonts as unknown as { ready?: Promise; forEach?: (cb: (f: FontFaceStatus) => void) => void } | undefined; - if (!set) return []; + if (!set) return { pending: [], unreferenced: [] }; const snapshot = (): { family: string; weight: string; style: string; status: string }[] => { const out: { family: string; weight: string; style: string; status: string }[] = []; set.forEach?.((f) => out.push({ family: f.family, weight: f.weight, style: f.style, status: f.status })); return out; }; + // Only a face still actively fetching is pending; "unloaded" is unreferenced (never fetched). const pending = (faces: { family: string; weight: string; style: string; status: string }[]): string[] => { const p = new Set(); for (const f of faces) { - const fam = f.family.replace(/^["']|["']$/g, ""); - if (f.status !== "loaded" && f.status !== "error") p.add(fam); + if (f.status === "loading") p.add(f.family.replace(/^["']|["']$/g, "")); } return [...p].sort(); }; // First give the browser's own aggregate signal a chance (bounded — a hung request must not - // hold ready forever). Then poll the per-face states until all terminal or the cap expires. + // hold ready forever). Then poll the per-face states until none are loading or the cap expires. await Promise.race([set.ready ?? Promise.resolve(), new Promise((r) => setTimeout(r, capMs))]); const deadline = Date.now() + capMs; // eslint-disable-next-line no-constant-condition while (true) { - const left = pending(snapshot()); - if (left.length === 0) return []; - if (Date.now() >= deadline) return left; + const faces = snapshot(); + const left = pending(faces); + const done = left.length === 0 || Date.now() >= deadline; + if (done) { + const unreferenced = faces + .filter((f) => f.status === "unloaded") + .map((f) => ({ family: f.family.replace(/^["']|["']$/g, ""), weight: f.weight, style: f.style, status: f.status })); + return { pending: left, unreferenced }; + } await new Promise((r) => setTimeout(r, pollMs)); } }, { capMs, pollMs }, - ) as string[]; + ) as { pending: string[]; unreferenced: FontFaceStatus[] }; } catch { - return []; // a page without a FontFaceSet (or an evaluate fault) never blocks the walk + return { pending: [], unreferenced: [] }; // no FontFaceSet (or an evaluate fault) never blocks the walk } } @@ -430,12 +446,19 @@ export async function renderApp(opts: { // fallback face (systematically narrower/wider glyphs → a bogus size delta attributed to the // clone). Await document.fonts.ready AND poll (bounded, state-based) until every declared face // is terminal, so this holds for both the walk below and the screenshot further down. - const pendingFonts = await awaitFontsLoaded(page); + const { pending: pendingFonts, unreferenced: unrefFaces } = await awaitFontsLoaded(page); if (pendingFonts.length) { - const msg = `font-wait: ${pendingFonts.length} @font-face families unloaded after wait bound at ${vw}px: ${pendingFonts.join(", ")}`; + const msg = `font-wait: ${pendingFonts.length} @font-face families still loading after wait bound at ${vw}px: ${pendingFonts.join(", ")}`; fontWarnings.push(msg); console.warn(`[render] ${msg}`); } + // Declared-but-unreferenced faces (no rendered text resolves to them) are benign — the + // browser never fetched them by design. Report them per-face (family+weight+style) as an + // informational log only; they are NOT a fidelity warning and never enter fontWarnings. + if (unrefFaces.length) { + const detail = unrefFaces.map((f) => `${f.family} ${f.weight} ${f.style}`).join(", "); + console.log(`[render] font-info: ${unrefFaces.length} declared-but-unreferenced faces at ${vw}px: ${detail}`); + } // Scroll to settle any lazy effects, then back to top. await page.evaluate(async () => { const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); diff --git a/compiler/src/validate/validate.ts b/compiler/src/validate/validate.ts index 070df00..8f4c577 100644 --- a/compiler/src/validate/validate.ts +++ b/compiler/src/validate/validate.ts @@ -28,6 +28,21 @@ export function probeWidthsFor(viewports: number[]): number[] { const widest = sorted[sorted.length - 1] ?? 1920; return [...mids, Math.round(widest * 4 / 3)]; } +/** True if the URL of an aborted `/assets/` fetch resolves to a real file under the served export. + * Mirrors `serveStatic`'s path mapping (path segment joined onto the served root, `..` stripped) so + * an aborted srcset candidate whose file is present on disk is not mistaken for a 404. A URL we + * cannot parse, or one whose file is absent, is treated as a genuine miss (returns false). */ +export function servedAssetExists(servedRoot: string, url: string): boolean { + let pathname: string; + try { + pathname = new URL(url).pathname; + } catch { + return false; + } + const rel = decodeURIComponent(pathname).replace(/^(\.\.[/\\])+/, "").replace(/^\/+/, ""); + return existsSync(join(servedRoot, rel)); +} + import { buildReport, reportToMarkdown, type Report } from "./report.js"; import { readJSON, writeJSON, writeText, ensureDir, fileExists } from "../util/fsx.js"; import type { CaptureResult } from "../capture/capture.js"; @@ -136,12 +151,21 @@ export async function validateRun(runDir: string, opts?: { harnessDir?: string; await server.close(); } } - // Video/audio elements stream and their requests are aborted ("failed") when the - // render snapshot is taken before the stream finishes — the file is materialized, - // so this is not an asset failure. Only count non-streaming assets. - const assetFailed = failedResources.filter( - (f) => f.includes("/assets/") && !/\.(mp4|webm|mov|m4v|ogv|ogg|mp3|wav|m3u8)(\?|$)/i.test(f), - ); + // A generated asset ref is only a real failure if the file it points to is genuinely + // missing from the served export. Two aborts are NOT failures even though Chromium logs + // them as "failed": (a) video/audio streams aborted when the render snapshot is taken + // before the stream finishes; (b) responsive-image candidates whose + // in-flight low-priority fetch is aborted when the page closes or the candidate is + // re-chosen — the image analogue of (a). So count an /assets/ entry only when it is a + // real HTTP >= 400 response, OR a "failed " (aborted) entry whose target file is actually + // absent from the served out/ dir. Genuine 404s (file missing) still fail. + const assetFailed = failedResources.filter((f) => { + if (!f.includes("/assets/")) return false; + if (/\.(mp4|webm|mov|m4v|ogv|ogg|mp3|wav|m3u8)(\?|$)/i.test(f)) return false; // streaming media (a) + if (!f.startsWith("failed ")) return true; // " " — a real >= 400 response, always count + // Aborted fetch (b): excuse it if the file exists under the served export, else it is a real miss. + return build.outDir ? !servedAssetExists(build.outDir, f.slice("failed ".length)) : true; + }); const artifactsPresent = ["manifest.json", "sections.json", "tokens.json", "assets.json", "fonts.json"].every((f) => fileExists(join(generatedDir, f))); const gate0Issues: string[] = []; if (!build.ok) gate0Issues.push("build failed: " + build.stderr.split("\n").filter(Boolean).slice(-3).join(" | ")); diff --git a/compiler/test/gates.test.ts b/compiler/test/gates.test.ts index ef70c2e..616b844 100644 --- a/compiler/test/gates.test.ts +++ b/compiler/test/gates.test.ts @@ -1,6 +1,12 @@ import { describe, it } from "node:test"; import assert from "node:assert/strict"; -import { letterSpacingEquivalent, normHref, countVisibleInCaptureHiddenInClone } from "../src/validate/gates.js"; +import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { letterSpacingEquivalent, normHref, countVisibleInCaptureHiddenInClone, gate2Assets } from "../src/validate/gates.js"; +import { servedAssetExists } from "../src/validate/validate.js"; +import type { AssetGraph } from "../src/infer/assets.js"; +import type { FontGraph } from "../src/infer/fonts.js"; import type { IR, IRNode } from "../src/normalize/ir.js"; import type { PageSnapshot } from "../src/capture/walker.js"; @@ -132,3 +138,51 @@ describe("countVisibleInCaptureHiddenInClone diagnostic", () => { assert.equal(countVisibleInCaptureHiddenInClone(ir, gen, [375]), 0); }); }); + +// FIX 3 — an aborted srcset-candidate image fetch (requestfailed, file present on disk) must not be +// counted as a "404" asset failure. servedAssetExists maps a failed URL to a file under the served +// export exactly as serveStatic does; the assetFailed filter in validate.ts uses it to excuse +// aborted fetches whose file exists, while genuine misses (>= 400, or file absent) still fail. +describe("servedAssetExists (FIX 3 aborted-image excuse)", () => { + const root = mkdtempSync(join(tmpdir(), "served-out-")); + mkdirSync(join(root, "assets", "cloned", "images"), { recursive: true }); + writeFileSync(join(root, "assets", "cloned", "images", "present.webp"), "RIFFxxxxWEBP"); + + it("resolves a URL to a present file under the served root", () => { + assert.equal(servedAssetExists(root, "http://127.0.0.1:5000/assets/cloned/images/present.webp"), true); + }); + + it("returns false when the file is absent from the served root (a genuine miss)", () => { + assert.equal(servedAssetExists(root, "http://127.0.0.1:5000/assets/cloned/images/absent.webp"), false); + }); + + it("ignores query strings when resolving to disk", () => { + assert.equal(servedAssetExists(root, "http://127.0.0.1:5000/assets/cloned/images/present.webp?206w"), true); + }); + + it("does not escape the served root via .. traversal", () => { + assert.equal(servedAssetExists(root, "http://127.0.0.1:5000/../../../etc/passwd"), false); + }); + + it("an unparseable URL is treated as a genuine miss", () => { + assert.equal(servedAssetExists(root, "not-a-url"), false); + }); +}); + +describe("gate2Assets failed-asset message (FIX 3)", () => { + const emptyAssets: AssetGraph = { entries: [], byUrl: new Map() }; + const emptyFonts: FontGraph = { entries: [], css: "" }; + + it("passes when no generated asset refs are missing", () => { + const r = gate2Assets(emptyAssets, emptyFonts, { remoteRefs: [], failed404: [] }); + assert.equal(r.pass, true); + assert.equal(r.metrics.failed404, 0); + }); + + it("fails and reports missing refs (not '404') when a genuine miss is passed", () => { + const r = gate2Assets(emptyAssets, emptyFonts, { remoteRefs: [], failed404: ["failed http://127.0.0.1:5000/assets/cloned/images/absent.webp"] }); + assert.equal(r.pass, false); + assert.equal(r.metrics.failed404, 1); + assert.ok(r.issues.some((i) => i.includes("generated asset refs missing")), `expected 'missing' wording, got: ${r.issues.join(" | ")}`); + }); +}); diff --git a/compiler/test/serveRange.test.ts b/compiler/test/serveRange.test.ts index fdcc6d9..c5eb342 100644 --- a/compiler/test/serveRange.test.ts +++ b/compiler/test/serveRange.test.ts @@ -3,7 +3,7 @@ import assert from "node:assert/strict"; import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { serveStatic, parseRangeHeader, pendingFontFamilies, type FontFaceStatus } from "../src/validate/render.js"; +import { serveStatic, parseRangeHeader, pendingFontFamilies, unreferencedFontFaces, type FontFaceStatus } from "../src/validate/render.js"; // ---- Pure helper: parseRangeHeader ---- describe("parseRangeHeader", () => { @@ -55,7 +55,10 @@ describe("parseRangeHeader", () => { // ---- Pure helper: pendingFontFamilies (the font-load wait decision) ---- // The render walk must run after webfonts are APPLIED, else text boxes measure the fallback face // (a systematic width delta wrongly attributed to the clone). This helper is the state-based (not -// time-based) predicate the in-page poll ends on: it returns the DECLARED families not yet terminal. +// time-based) predicate the in-page poll ends on: it returns the DECLARED families still actively +// fetching. Browsers lazy-load faces, so after document.fonts.ready a face the rendered text needs +// is already "loading"; a face left "unloaded" is unreferenced and will never load on its own — +// waiting on it only burns the cap, so ONLY "loading" is pending. describe("pendingFontFamilies", () => { const face = (family: string, status: string, weight = "400", style = "normal"): FontFaceStatus => ({ family, weight, style, status }); @@ -67,8 +70,8 @@ describe("pendingFontFamilies", () => { assert.deepEqual(pendingFontFamilies([face("Avenir", "loaded"), face("Inter", "loaded")]), []); }); - it("an unloaded face makes its family pending", () => { - assert.deepEqual(pendingFontFamilies([face("Avenir", "unloaded")]), ["Avenir"]); + it("an unloaded face is unreferenced (lazy-load) → NOT pending", () => { + assert.deepEqual(pendingFontFamilies([face("Avenir", "unloaded")]), []); }); it("a loading face makes its family pending", () => { @@ -79,27 +82,63 @@ describe("pendingFontFamilies", () => { assert.deepEqual(pendingFontFamilies([face("Avenir", "error")]), []); }); - it("a family with mixed weights is pending if ANY weight is not terminal", () => { + it("a family is pending if ANY weight is still loading", () => { const faces = [face("Avenir", "loaded", "400"), face("Avenir", "loading", "700")]; assert.deepEqual(pendingFontFamilies(faces), ["Avenir"]); }); + it("a partially-used family (used face loaded, sibling faces unloaded) is NOT pending", () => { + // The real-world false positive this fix removes: a matched face loaded, unreferenced + // weight/style siblings stay "unloaded" — the walk may proceed without waiting on them. + const faces = [face("Avenir", "loaded", "400"), face("Avenir", "unloaded", "700"), face("Avenir", "unloaded", "400", "italic")]; + assert.deepEqual(pendingFontFamilies(faces), []); + }); + it("a family whose faces are all terminal (loaded or errored) is not pending", () => { const faces = [face("Avenir", "loaded", "400"), face("Avenir", "error", "700")]; assert.deepEqual(pendingFontFamilies(faces), []); }); - it("strips surrounding quotes from the reported family name and dedupes+sorts", () => { - const faces = [face('"Source Serif"', "unloaded"), face("Avenir", "loading"), face("Avenir", "unloaded")]; + it("strips surrounding quotes from the reported family name and dedupes+sorts (loading only)", () => { + const faces = [face('"Source Serif"', "loading"), face("Avenir", "loading"), face("Avenir", "unloaded")]; assert.deepEqual(pendingFontFamilies(faces), ["Avenir", "Source Serif"]); }); - it("only the unloaded families are returned when some are loaded", () => { - const faces = [face("Inter", "loaded"), face("Avenir", "unloaded"), face("JetBrains", "loaded")]; + it("only the still-loading families are returned when some are loaded/unloaded", () => { + const faces = [face("Inter", "loaded"), face("Avenir", "loading"), face("JetBrains", "unloaded")]; assert.deepEqual(pendingFontFamilies(faces), ["Avenir"]); }); }); +// ---- Pure helper: unreferencedFontFaces (informational-only per-face report) ---- +// Faces left "unloaded" after the wait bound: declared but no rendered text resolves to them. +// Reported per-face (family+weight+style) for an informational log; benign, never a fidelity issue. +describe("unreferencedFontFaces", () => { + const face = (family: string, status: string, weight = "400", style = "normal"): FontFaceStatus => ({ family, weight, style, status }); + + it("returns only the unloaded faces (not loaded/loading/error)", () => { + const faces = [face("Avenir", "loaded"), face("Merriweather", "unloaded", "700"), face("Inter", "loading"), face("Gotham", "error")]; + assert.deepEqual(unreferencedFontFaces(faces), [{ family: "Merriweather", weight: "700", style: "normal", status: "unloaded" }]); + }); + + it("reports per-face (family+weight+style), not collapsed per-family, and strips quotes + sorts", () => { + const faces = [ + face('"Merriweather"', "unloaded", "700", "normal"), + face("Merriweather", "unloaded", "300", "italic"), + face("Avenir", "unloaded", "400", "normal"), + ]; + assert.deepEqual(unreferencedFontFaces(faces), [ + { family: "Avenir", weight: "400", style: "normal", status: "unloaded" }, + { family: "Merriweather", weight: "300", style: "italic", status: "unloaded" }, + { family: "Merriweather", weight: "700", style: "normal", status: "unloaded" }, + ]); + }); + + it("no unloaded faces → empty", () => { + assert.deepEqual(unreferencedFontFaces([face("Avenir", "loaded"), face("Inter", "loading")]), []); + }); +}); + // ---- Integration: serveStatic answers real Range requests with 206/416 ---- describe("serveStatic Range support (integration)", () => { let rootDir = "";