diff --git a/packages/producer/src/services/render/observability.ts b/packages/producer/src/services/render/observability.ts index dfe0bc2a4..55b96961f 100644 --- a/packages/producer/src/services/render/observability.ts +++ b/packages/producer/src/services/render/observability.ts @@ -81,13 +81,17 @@ export interface RenderCaptureObservability { */ compositionElementCount?: number; /** - * Why the short-comp band did or did not apply to this render: - * "applied" (frames landed in 250-899 and the element count cleared the - * ceiling), "skipped_elements" (band was open by frame count but the comp - * was too large), or undefined when the frame count made the band - * irrelevant either way. Distinguishes "the new routing chose this" from - * "the pre-existing 900 floor chose this" — otherwise a fleet perf shift is - * unattributable. + * Short-comp band decision, emitted only when the band is DECISIVE — every + * other inversion-eligibility condition passed and only the floor (250 vs + * 900) differed. "applied": the element count cleared the ceiling too, so + * with routing enabled (HF_DE_SHORT_BAND_ROUTE) this render inverts; in the + * baseline release the same value is the COUNTERFACTUAL "would have + * inverted". "skipped_elements": the element ceiling was the only blocker. + * Unset: the band could not have affected this render (ineligible for some + * other reason, or already inverting at 900+). The selector is computed + * identically before and after the routing flip, and the skipped/oversize + * renders form the concurrent control for the difference-in-differences + * read — that is the entire point of the field. */ deShortBand?: "applied" | "skipped_elements"; /** DE parallel-router outcome: "routed" (fired, held) | "reverted" (fired, self-verify retry rolled back). */ diff --git a/packages/producer/src/services/render/perfSummary.ts b/packages/producer/src/services/render/perfSummary.ts index 0770d58ee..273532930 100644 --- a/packages/producer/src/services/render/perfSummary.ts +++ b/packages/producer/src/services/render/perfSummary.ts @@ -82,7 +82,7 @@ export interface DrawElementPerfInput { preInversionWorkers?: number; /** Rough compiled-composition element count — gate variable for the short-comp inversion band. */ compositionElementCount?: number; - /** Short-comp band attribution: "applied" | "skipped_elements"; unset when the band was irrelevant. */ + /** Short-comp band decision when the band was DECISIVE: "applied" (inverts once HF_DE_SHORT_BAND_ROUTE is on; counterfactual in the baseline release) | "skipped_elements" (element ceiling was the only blocker); unset when the band could not have affected this render. */ shortBand?: string; parallelRouter?: "routed" | "reverted"; /** Auto-resolved worker count before the router pinned it to 3 (set only when the router fired). */ diff --git a/packages/producer/src/services/renderOrchestrator.test.ts b/packages/producer/src/services/renderOrchestrator.test.ts index 16536f213..2b25e5e4a 100644 --- a/packages/producer/src/services/renderOrchestrator.test.ts +++ b/packages/producer/src/services/renderOrchestrator.test.ts @@ -1695,63 +1695,67 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => { expect(shouldPreferSingleWorkerDrawElement(eligible)).toBe(true); }); - // ── Short-comp band ──────────────────────────────────────────────────── - // The band lowers the effective floor from 900 to 250 for SMALL comps only. - // The predicate itself is unchanged — the call site picks the floor — so - // these pin the arithmetic the call site performs. + // ── Short-comp band ────────────────────────────────────────────────────────────────────── + // The call site evaluates the predicate TWICE — once at the 900 floor, once + // at the 250 band floor — and the band is DECISIVE only when the calls + // disagree. These pin that arithmetic, including the property the design + // depends on: the band can only ADD inversions, never remove one. // - // Measured basis (400f, single-DE vs parallel-screenshot-W4, 2 reps, ratio - // = ss4/de1 so >1 means DE wins): - // 24 movers / 0 nodes -> 1.05 DE wins - // 320 movers / 0 nodes -> 1.24 DE wins - // 320 movers / 7000 nodes -> 1.09 DE wins - // 24 movers / 7000 nodes -> 0.96 DE LOSES - // 24 movers / 20000 nodes -> 0.71 DE loses badly - // 24 movers / 40000 nodes -> 0.55 DE loses very badly - // Motion helps DE, DOM size punishes it; the ceiling is calibrated at the - // lowest-motion case so every higher-motion comp is covered too. - describe("short-comp band floor arithmetic", () => { - const shortBandFloor = (elementCount: number, maxElements = 2500): number => - elementCount <= maxElements ? Math.min(900, 250) : 900; + // Measured basis (400f, single-DE vs parallel-screenshot-W4, ratio = ss4/de1 + // so >1 means DE wins): + // 24 movers / 0 nodes -> 1.05 | 24 movers / 7000 nodes -> 0.96 + // 320 movers / 0 nodes -> 1.24 | 24 movers / 20000 nodes -> 0.71 + // 320 movers / 7000 nodes -> 1.09 | 24 movers / 40000 nodes -> 0.55 + // Motion helps DE, DOM size punishes it; the element ceiling is calibrated + // at the lowest-motion case so every higher-motion comp is covered too. + describe("short-comp band decisiveness (two-floor evaluation)", () => { + const BAND_FLOOR = Math.min(900, 250); + const atBase = (totalFrames: number, over?: Partial) => + shouldPreferSingleWorkerDrawElement({ ...eligible, ...over, totalFrames, minFrames: 900 }); + const atBand = (totalFrames: number, over?: Partial) => + shouldPreferSingleWorkerDrawElement({ + ...eligible, + ...over, + totalFrames, + minFrames: BAND_FLOOR, + }); - it("opens the 250-frame floor for a small comp", () => { - expect(shortBandFloor(800)).toBe(250); + it("is decisive exactly in the 250-899 window for an otherwise-eligible render", () => { + expect(atBand(400) && !atBase(400)).toBe(true); + expect(atBand(250) && !atBase(250)).toBe(true); + expect(atBand(899) && !atBase(899)).toBe(true); + }); + + it("is NOT decisive below the band floor — nothing fires either way", () => { + expect(atBand(200)).toBe(false); + expect(atBase(200)).toBe(false); + }); + + it("is NOT decisive at 900+ — the pre-existing floor already inverts, unchanged", () => { + expect(atBase(2380)).toBe(true); + expect(atBand(2380) && !atBase(2380)).toBe(false); + }); + + it("is NOT decisive when the render is ineligible for any other reason — the attribution cohort must exclude renders the band cannot affect", () => { + for (const over of [ + { requestedWorkers: 3 as const }, + { useDrawElement: false }, + { deCompileGate: "css_effect:filter" }, + { forceScreenshot: true }, + { outputFormat: "webm" as const }, + { singleWorkerStreamingOk: false }, + { probeDeGated: true }, + ]) { + expect(atBand(400, over)).toBe(false); + } + }); + + it("HF_DE_SHORT_MIN_FRAMES=0 disables via the predicate's own minFrames guard", () => { expect( shouldPreferSingleWorkerDrawElement({ ...eligible, totalFrames: 400, - minFrames: shortBandFloor(800), - }), - ).toBe(true); - }); - - it("keeps the 900-frame floor for a large comp — the measured 1.8x regression case", () => { - expect(shortBandFloor(40000)).toBe(900); - expect( - shouldPreferSingleWorkerDrawElement({ - ...eligible, - totalFrames: 400, - minFrames: shortBandFloor(40000), - }), - ).toBe(false); - }); - - it("never RAISES the floor: a large comp at 900+ frames still inverts as it did before", () => { - expect( - shouldPreferSingleWorkerDrawElement({ - ...eligible, - totalFrames: 2380, - minFrames: shortBandFloor(40000), - }), - ).toBe(true); - }); - - it("leaves comps below the band floor alone", () => { - expect( - shouldPreferSingleWorkerDrawElement({ - ...eligible, - totalFrames: 200, - minFrames: shortBandFloor(800), + minFrames: Math.min(900, 0), }), ).toBe(false); }); @@ -1762,8 +1766,18 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => { expect(countElementTags("
a
")).toBe(2); }); - it("undercounts void elements — biases the count DOWN, so the ceiling must stay conservative", () => { - expect(countElementTags("
")).toBe(0); + it("counts void elements — an image gallery must not read as a tiny comp", () => { + expect(countElementTags("
")).toBe(3); + expect(countElementTags('')).toBe(2); + }); + + it("does not false-positive on inline-script comparisons or void-prefixed words", () => { + // Only the closer counts: "if (a < b && x ")).toBe( + 1, + ); }); it("is stable on empty and malformed input rather than throwing", () => { diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts index 2369bf2c0..acb8cd74a 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -1247,13 +1247,22 @@ export function envInt(name: string, fallback: number): number { * routing decision, and a full linkedom parse of the exact documents that * matter here — the 20k-40k node ones — is the most expensive case. Precision * is not needed. It feeds a threshold whose measured crossover is ~3.9k and - * whose default sits at 2500, so counting closing tags is comfortably inside - * the margin. Undercounts void elements (``, `
`) and self-closing - * SVG nodes, which biases the count DOWN — the direction that opens the band - * — so the ceiling is the thing to keep conservative. + * whose default sits at 2500, so tag counting is comfortably inside the + * margin. Closing tags plus HTML void elements (``, `
`, …) — voids + * matter because they skew EXPENSIVE to paint (images), and counting only + * closers would read an image gallery as a tiny comp and open the band on + * exactly the content most likely to lose it. Opening tags are deliberately + * NOT counted: compiled comps embed inline scripts where `a < b` would + * false-positive. Self-closing SVG children still undercount; acceptable. + * + * These semantics are FROZEN while the short-band baseline is being read — + * the fleet distribution recorded by the baseline release must be measured + * by the same counter that later gates routing, or the baseline is invalid. */ export function countElementTags(html: string): number { - const matches = html.match(/<\/[a-zA-Z]/g); + const matches = html.match( + /<\/[a-zA-Z]|<(?:img|br|hr|input|source|track|area|base|col|embed|link|meta|param|wbr)\b/gi, + ); return matches === null ? 0 : matches.length; } @@ -2462,20 +2471,21 @@ async function executeRenderPipeline(input: { deShortBandMinFrames > 0 && deShortBandMaxElements > 0 && compositionElementCount <= deShortBandMaxElements; - const deEffectiveMinFrames = deShortBandOpen - ? Math.min(deSingleMinFrames, deShortBandMinFrames) - : deSingleMinFrames; - // Does the band even matter for this render? Only when the frame count - // falls in the newly-opened window — at or above the original floor the - // inversion fires regardless, and below `deShortBandMinFrames` nothing - // fires either way. Keeps the telemetry reason from claiming credit (or - // blame) on renders the change could not have affected. - const deShortBandApplies = - totalFrames >= deShortBandMinFrames && totalFrames < deSingleMinFrames; + // Baseline-first sequencing: this release EVALUATES the band on every + // render and emits the decision, but only routes on it when + // HF_DE_SHORT_BAND_ROUTE=true (flipped by default in a follow-up release). + // The point is a difference-in-differences read: the cohort selector + // (`de_short_band`) is computed identically before and after the flip — + // "applied" is counterfactual in the baseline release and factual after — + // and the skipped/oversize renders in the same frame band form a + // concurrent control that absorbs secular drift (content mix, version- + // correlated populations, hardware). A plain before/after cannot + // attribute a fleet perf shift to this change; this can. + const deShortBandRoute = process.env.HF_DE_SHORT_BAND_ROUTE === "true"; // "Would ANY multi-worker resolution be inverted?" — if workers resolve // to 1 naturally the outcome is identical either way. const WOULD_RESOLVE_MULTI_WORKER = 2; - const deInversionEligible = shouldPreferSingleWorkerDrawElement({ + const deInversionArgs = { workerCount: WOULD_RESOLVE_MULTI_WORKER, requestedWorkers: job.config.workers, useDrawElement: cfg.useDrawElement, @@ -2483,7 +2493,7 @@ async function executeRenderPipeline(input: { forceScreenshot: captureForceScreenshot, outputFormat, totalFrames, - minFrames: deEffectiveMinFrames, + minFrames: deSingleMinFrames, singleWorkerStreamingOk: shouldUseStreamingEncode(cfg, outputFormat, 1, job.duration), layeredOrEffectRoute: hasHdrContent || compiled.hasShaderTransitions, supersampling: deviceScaleFactor > 1, @@ -2495,7 +2505,29 @@ async function executeRenderPipeline(input: { process.env.PRODUCER_EXPERIMENTAL_FAST_CAPTURE === "true" || // Verified parallel DE streaming (opt-in) wants its parallelism kept. process.env.HF_DE_PARALLEL_STREAM === "true", + }; + const invertAtBaseFloor = shouldPreferSingleWorkerDrawElement(deInversionArgs); + // Same render, same eligibility, band floor instead of 900. Math.min so a + // user override of HF_DE_SINGLE_MIN_FRAMES below the band floor keeps + // winning; HF_DE_SHORT_MIN_FRAMES=0 disables via the predicate's own + // minFrames > 0 check. + const invertAtBandFloor = shouldPreferSingleWorkerDrawElement({ + ...deInversionArgs, + minFrames: Math.min(deSingleMinFrames, deShortBandMinFrames), }); + // The band is DECISIVE only when it alone flips the decision — every + // other eligibility condition already passed and only the floor differed. + // Renders the band could not have affected (ineligible for any other + // reason, or already inverting at 900+) stay unset, so the telemetry + // cohort contains exactly the renders whose routing this change decides. + const deShortBandDecisive = invertAtBandFloor && !invertAtBaseFloor; + const deShortBand = deShortBandDecisive + ? deShortBandOpen + ? ("applied" as const) + : ("skipped_elements" as const) + : undefined; + const deInversionEligible = + deShortBandRoute && deShortBand === "applied" ? invertAtBandFloor : invertAtBaseFloor; // DE parallel-router eligibility — see shouldPreferParallelDrawElement. // Default-off (HF_DE_PARALLEL_ROUTER); HF_DE_PARALLEL_MIN_FRAMES default // 700, re-calibrated 2026-07-27 from the original safe-high 2000. A @@ -2749,11 +2781,7 @@ async function executeRenderPipeline(input: { // render so the fleet element-count distribution is readable, and so a // perf shift can be split into "the new band did it" vs "unchanged". compositionElementCount, - deShortBand: deShortBandApplies - ? deShortBandOpen - ? "applied" - : "skipped_elements" - : undefined, + deShortBand, // Same rationale as the counters above: carried on live capture // observability, not only the success-path perfSummary, so a crash / // OOM / timeout still reports which GPU backend it happened on. That @@ -3601,11 +3629,7 @@ async function executeRenderPipeline(input: { workerInversion: deWorkerInversion, preInversionWorkers: deWorkerInversion ? preRoutingWorkerCount : undefined, compositionElementCount, - shortBand: deShortBandApplies - ? deShortBandOpen - ? "applied" - : "skipped_elements" - : undefined, + shortBand: deShortBand, parallelRouter: deParallelRouter, preRouterWorkers: deParallelRouter ? preRoutingWorkerCount : undefined, selfVerifyFallback: deSelfVerifyFallback,