diff --git a/packages/cli/src/commands/render.ts b/packages/cli/src/commands/render.ts index 5c8fb75310..d832746369 100644 --- a/packages/cli/src/commands/render.ts +++ b/packages/cli/src/commands/render.ts @@ -1109,7 +1109,6 @@ let deParallelRouterTrialFiredThisProcess = false; * resetting outside a test process where many independent test cases share * one imported module instance. */ -// fallow-ignore-next-line unused-export export function __resetDeParallelRouterTrialStateForTests(): void { deParallelRouterTrialManagedByUs = false; deParallelRouterTrialFiredThisProcess = false; @@ -1433,6 +1432,9 @@ function trackRenderMetrics( deClampReason: perf?.drawElement?.clampReason, deWorkerInversion: perf?.drawElement?.workerInversion, dePreInversionWorkers: perf?.drawElement?.preInversionWorkers, + compositionElementCount: perf?.drawElement?.compositionElementCount, + compositionElementCountSource: perf?.drawElement?.compositionElementCountSource, + deShortBand: perf?.drawElement?.shortBand, deParallelRouter: perf?.drawElement?.parallelRouter, dePreRouterWorkers: perf?.drawElement?.preRouterWorkers, deGateReason: perf?.drawElement?.gateReason, diff --git a/packages/cli/src/telemetry/events.ts b/packages/cli/src/telemetry/events.ts index cd68747569..3bc3db5e84 100644 --- a/packages/cli/src/telemetry/events.ts +++ b/packages/cli/src/telemetry/events.ts @@ -72,6 +72,9 @@ export interface RenderObservabilityTelemetryPayload { // the more authoritative perfSummary value wins when both are present. captureDeWorkerInversion?: string; captureDePreInversionWorkers?: number; + captureCompositionElementCount?: number; + captureCompositionElementCountSource?: string; + captureDeShortBand?: string; captureDeParallelRouter?: string; captureDeGpuRenderer?: string; captureDePreRouterWorkers?: number; @@ -130,6 +133,9 @@ function renderObservabilityEventProperties(props: RenderObservabilityTelemetryP capture_memory_exhaustion_detected: props.captureMemoryExhaustionDetected, de_worker_inversion: props.captureDeWorkerInversion, de_pre_inversion_workers: props.captureDePreInversionWorkers, + composition_element_count: props.captureCompositionElementCount, + composition_element_count_source: props.captureCompositionElementCountSource, + de_short_band: props.captureDeShortBand, de_parallel_router: props.captureDeParallelRouter, gpu_renderer: props.captureDeGpuRenderer, de_pre_router_workers: props.captureDePreRouterWorkers, @@ -204,6 +210,9 @@ export function trackRenderComplete( deClampReason?: string; deWorkerInversion?: string; dePreInversionWorkers?: number; + compositionElementCount?: number; + compositionElementCountSource?: string; + deShortBand?: string; deParallelRouter?: string; dePreRouterWorkers?: number; deGateReason?: string; @@ -303,6 +312,9 @@ export function trackRenderComplete( de_clamp_reason: props.deClampReason, de_worker_inversion: props.deWorkerInversion, de_pre_inversion_workers: props.dePreInversionWorkers, + composition_element_count: props.compositionElementCount, + composition_element_count_source: props.compositionElementCountSource, + de_short_band: props.deShortBand, de_parallel_router: props.deParallelRouter, de_pre_router_workers: props.dePreRouterWorkers, de_gate_reason: props.deGateReason, diff --git a/packages/cli/src/telemetry/renderObservability.ts b/packages/cli/src/telemetry/renderObservability.ts index 87554d6939..362837ae8a 100644 --- a/packages/cli/src/telemetry/renderObservability.ts +++ b/packages/cli/src/telemetry/renderObservability.ts @@ -42,6 +42,9 @@ export function renderObservabilityTelemetryPayload( captureMemoryExhaustionDetected: capture.memoryExhaustionDetected, captureDeWorkerInversion: capture.deWorkerInversion, captureDePreInversionWorkers: capture.dePreInversionWorkers, + captureCompositionElementCount: capture.compositionElementCount, + captureCompositionElementCountSource: capture.compositionElementCountSource, + captureDeShortBand: capture.deShortBand, captureDeParallelRouter: capture.deParallelRouter, captureDeGpuRenderer: capture.deGpuRenderer, captureDePreRouterWorkers: capture.dePreRouterWorkers, diff --git a/packages/engine/src/services/frameCapture.ts b/packages/engine/src/services/frameCapture.ts index 4751f7a053..0a8e9bc55c 100644 --- a/packages/engine/src/services/frameCapture.ts +++ b/packages/engine/src/services/frameCapture.ts @@ -3786,6 +3786,8 @@ export function getCapturePerfSummary(session: CaptureSession): CapturePerfSumma p95TotalMs: percentileOf(session.capturePerf.frameMs, 0.95), p99TotalMs: percentileOf(session.capturePerf.frameMs, 0.99), subTimelineWaitOutcome: session.subTimelineWaitOutcome, + initDurationMs: session.initTelemetry?.initDurationMs, + initTweenCount: session.initTelemetry?.tweenCount, warnings: cloneCaptureWarnings(session.warnings), staticDedupReused: session.staticDedupCount ?? 0, staticDedupEnabled: session.staticDedupEnabled ?? false, diff --git a/packages/engine/src/types.ts b/packages/engine/src/types.ts index 4f30c1efba..dd5faa5e6b 100644 --- a/packages/engine/src/types.ts +++ b/packages/engine/src/types.ts @@ -241,6 +241,18 @@ export interface CapturePerfSummary { p99TotalMs: number; /** Sub-composition timeline wait outcome (absent pre-init). */ subTimelineWaitOutcome?: SubTimelineWaitOutcome; + /** + * Session init telemetry, mirrored from the `[FrameCapture:INIT]` console + * line so PARALLEL workers report it too: worker sessions' console buffers + * only propagate to the orchestrator on failure, which left the + * multi-worker path — the short-comp band's entire population — with 0% + * coverage of the motion axis (`observability_init_tween_count`) in fleet + * telemetry. Riding the perf summary reuses the one channel that already + * flows back per worker on success. + */ + initDurationMs?: number; + /** GSAP tween count at init — the motion-axis signal for capture routing analysis. */ + initTweenCount?: number; /** Correctness warnings observed before or during capture. */ warnings?: CaptureWarning[]; /** diff --git a/packages/producer/src/services/render/observability.test.ts b/packages/producer/src/services/render/observability.test.ts index 7e52898609..3e9070c63d 100644 --- a/packages/producer/src/services/render/observability.test.ts +++ b/packages/producer/src/services/render/observability.test.ts @@ -407,3 +407,34 @@ describe("RenderObservabilityRecorder", () => { ); }); }); + +describe("init observability fallback (parallel workers)", () => { + const makeRecorder = () => + new RenderObservabilityRecorder({ renderJobId: "render-par", pipelineStartMs: Date.now() }); + + it("uses the structured fallback when the console has no INIT line — the parallel success path", () => { + const summary = makeRecorder().summary({ + lastBrowserConsole: ["[FrameCapture:NAV] page.goto start"], + capture: { forceScreenshot: false, captureMode: "screenshot" }, + initFallback: { initDurationMs: 850, tweenCount: 1200 }, + }); + expect(summary.init).toEqual({ initDurationMs: 850, tweenCount: 1200 }); + }); + + it("max-merges console INIT lines over the fallback, matching multi-session semantics", () => { + const summary = makeRecorder().summary({ + lastBrowserConsole: ["[FrameCapture:INIT] complete initDurationMs=1234 tweenCount=42"], + capture: { forceScreenshot: false, captureMode: "screenshot" }, + initFallback: { initDurationMs: 850, tweenCount: 1200 }, + }); + expect(summary.init).toEqual({ initDurationMs: 1234, tweenCount: 1200 }); + }); + + it("stays undefined when neither source has anything", () => { + const summary = makeRecorder().summary({ + lastBrowserConsole: [], + capture: { forceScreenshot: false, captureMode: "screenshot" }, + }); + expect(summary.init).toBeUndefined(); + }); +}); diff --git a/packages/producer/src/services/render/observability.ts b/packages/producer/src/services/render/observability.ts index 39111960cd..925f9fcfb0 100644 --- a/packages/producer/src/services/render/observability.ts +++ b/packages/producer/src/services/render/observability.ts @@ -70,6 +70,43 @@ export interface RenderCaptureObservability { deWorkerInversion?: "inverted" | "reverted"; /** Worker count the resolver would have used absent the inversion; undefined if it never fired. */ dePreInversionWorkers?: number; + /** + * Element count for the short-comp band gate (`resolveCompositionElementCount`): + * the LIVE DOM size from the already-running probe session when one is + * initialized, falling back to a static scan of the compiled HTML + * (`countElementTags`) otherwise. Live is authoritative — a static scan + * cannot see elements a composition's own script creates at runtime. + * + * Emitted on every render, not just inverted ones — this is the variable the + * short-comp inversion band is gated on, and the fleet distribution of it is + * unknown. Without it there is no way to tell whether the 2500 ceiling opens + * the band for most short comps or almost none, and no way to re-derive the + * threshold from real content instead of synthetic sweeps. + */ + compositionElementCount?: number; + /** + * Provenance of `compositionElementCount`: "live" (measured from the probe + * session's real DOM — sees runtime-generated elements) or "static" (source + * markup scan, which does not). The probe is CONDITIONAL, so this is not a + * detail: only a `live` count may open the short band, and the fleet rate of + * "static" sizes the population a future conditional-probe-launch would + * unlock for the band. + */ + compositionElementCountSource?: "live" | "static"; + /** + * 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" | "unmeasured"; /** DE parallel-router outcome: "routed" (fired, held) | "reverted" (fired, self-verify retry rolled back). */ deParallelRouter?: "routed" | "reverted"; /** @@ -246,20 +283,26 @@ function readUnsignedIntAfter(line: string, prefix: string): number | undefined return digits > 0 ? value : undefined; } -function summarizeInitObservability(lines: string[]): RenderInitObservability | undefined { - let initDurationMs: number | undefined; - let tweenCount: number | undefined; +/** Max of two optional readings — multiple worker/session INIT records can appear; keep the worst. */ +function maxReading(current: number | undefined, next: number | undefined): number | undefined { + if (next === undefined) return current; + return current === undefined ? next : Math.max(current, next); +} + +function summarizeInitObservability( + lines: string[], + fallback?: RenderInitObservability, +): RenderInitObservability | undefined { + // Console parsing only sees THIS process's session buffer, so parallel + // workers' INIT lines never reach it — their init telemetry arrives + // structured via the per-worker perf summaries instead. Seed with that and + // let the console parse (same max semantics) refine it. + let initDurationMs: number | undefined = fallback?.initDurationMs; + let tweenCount: number | undefined = fallback?.tweenCount; for (const line of lines) { if (!line.includes("[FrameCapture:INIT]")) continue; - const duration = readUnsignedIntAfter(line, "initDurationMs="); - const tweens = readUnsignedIntAfter(line, "tweenCount="); - // Multiple worker/session INIT records can appear; keep the worst observed startup cost. - if (duration !== undefined) { - initDurationMs = initDurationMs === undefined ? duration : Math.max(initDurationMs, duration); - } - if (tweens !== undefined) { - tweenCount = tweenCount === undefined ? tweens : Math.max(tweenCount, tweens); - } + initDurationMs = maxReading(initDurationMs, readUnsignedIntAfter(line, "initDurationMs=")); + tweenCount = maxReading(tweenCount, readUnsignedIntAfter(line, "tweenCount=")); } if (initDurationMs === undefined && tweenCount === undefined) return undefined; return { initDurationMs, tweenCount }; @@ -378,6 +421,8 @@ export class RenderObservabilityRecorder { summary(input: { lastBrowserConsole: string[]; capture: RenderCaptureObservability; + /** Structured init telemetry from per-worker perf summaries — the only success-path channel parallel workers have (their console buffers propagate on failure only). */ + initFallback?: RenderInitObservability; extraction?: RenderExtractionObservability; compositionHash?: string; }): RenderObservabilitySummary { @@ -392,7 +437,7 @@ export class RenderObservabilityRecorder { browserDiagnostics: summarizeBrowserDiagnostics(input.lastBrowserConsole), capture: { ...input.capture }, extraction: input.extraction ? { ...input.extraction } : undefined, - init: summarizeInitObservability(input.lastBrowserConsole), + init: summarizeInitObservability(input.lastBrowserConsole, input.initFallback), }; } diff --git a/packages/producer/src/services/render/perfSummary.ts b/packages/producer/src/services/render/perfSummary.ts index fe72e1e1c0..e4f4db3fc8 100644 --- a/packages/producer/src/services/render/perfSummary.ts +++ b/packages/producer/src/services/render/perfSummary.ts @@ -80,6 +80,12 @@ export interface DrawElementPerfInput { workerInversion?: "inverted" | "reverted"; /** Auto-resolved worker count before the inversion pinned it to 1 (set only when the inversion fired). */ preInversionWorkers?: number; + /** Rough compiled-composition element count — gate variable for the short-comp inversion band. */ + compositionElementCount?: number; + /** Provenance of the element count: "live" (probe DOM, trusted to gate) | "static" (source scan, not). */ + compositionElementCountSource?: "live" | "static"; + /** 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?: "applied" | "skipped_elements" | "unmeasured"; parallelRouter?: "routed" | "reverted"; /** Auto-resolved worker count before the router pinned it to 3 (set only when the router fired). */ preRouterWorkers?: number; @@ -121,6 +127,9 @@ function aggregateDrawElement( clampReason: de.clampReason, workerInversion: de.workerInversion ?? "none", preInversionWorkers: de.preInversionWorkers, + compositionElementCount: de.compositionElementCount, + compositionElementCountSource: de.compositionElementCountSource, + shortBand: de.shortBand, parallelRouter: de.parallelRouter ?? "none", preRouterWorkers: de.preRouterWorkers, gateReason: gateReasons.length > 0 ? gateReasons.join("|") : undefined, diff --git a/packages/producer/src/services/render/stages/probeStage.ts b/packages/producer/src/services/render/stages/probeStage.ts index 58142a30f0..8d19667509 100644 --- a/packages/producer/src/services/render/stages/probeStage.ts +++ b/packages/producer/src/services/render/stages/probeStage.ts @@ -184,6 +184,37 @@ function hasRuntimeInsertedMedia(html: string): boolean { ); } +/** + * Does this render need a browser probe at all? + * + * Extracted as a pure predicate because whether a probe runs decides whether + * a LIVE DOM element count is available downstream, and the short-comp + * inversion band fails closed without one (see + * `resolveCompositionElementCount` / `resolveDeShortBand`). Notably NONE of + * these conditions fire for a known-duration, media-free composition that + * builds thousands of `div`/`span` nodes in its own init script — + * `hasRuntimeInsertedMedia` matches only `createElement("video"|"audio")` — + * so that shape is measured statically and must never reach the band's + * `applied` cohort (review finding, R4). + */ +export function probeRequiresBrowser(args: { + durationSeconds: number; + unresolvedCompositionCount: number; + hasAutoStart: boolean; + hasScriptedAudio: boolean; + hasVariableMedia: boolean; + hasInsertedMedia: boolean; +}): boolean { + return ( + args.durationSeconds <= 0 || + args.unresolvedCompositionCount > 0 || + args.hasAutoStart || + args.hasScriptedAudio || + args.hasVariableMedia || + args.hasInsertedMedia + ); +} + export async function runProbeStage(input: ProbeStageInput): Promise { const { projectDir, @@ -217,13 +248,14 @@ export async function runProbeStage(input: ProbeStageInput): Promise 0 || - hasAutoStart || - hasScriptedAudio || - hasVariableMedia || - hasInsertedMedia; + const needsBrowser = probeRequiresBrowser({ + durationSeconds: composition.duration, + unresolvedCompositionCount: compiled.unresolvedCompositions.length, + hasAutoStart, + hasScriptedAudio, + hasVariableMedia, + hasInsertedMedia, + }); if (needsBrowser) { const reasons = []; diff --git a/packages/producer/src/services/renderOrchestrator.test.ts b/packages/producer/src/services/renderOrchestrator.test.ts index 791a17389b..752e65d95d 100644 --- a/packages/producer/src/services/renderOrchestrator.test.ts +++ b/packages/producer/src/services/renderOrchestrator.test.ts @@ -34,11 +34,17 @@ import { resolveParallelRouterRetryPlan, resetCaptureAttemptProgress, shouldRetryViaPinnedFallback, + countElementTags, + envInt, + mergeWorkerInitObservability, + resolveCompositionElementCount, + resolveDeShortBand, shouldPreferParallelDrawElement, shouldPreferSingleWorkerDrawElement, shouldStreamParallelCapture, shouldUseStreamingEncode, } from "./renderOrchestrator.js"; +import { probeRequiresBrowser } from "./render/stages/probeStage.js"; import { ensureFrameWritten } from "./render/stages/captureHdrFrameShared.js"; import { resolveCompositeTransfer, shouldUseLayeredComposite } from "./hdrCompositor.js"; import { @@ -1693,6 +1699,390 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => { expect(shouldPreferSingleWorkerDrawElement(eligible)).toBe(true); }); + // ── 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, 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("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: Math.min(900, 0), + }), + ).toBe(false); + }); + }); + + describe("resolveDeShortBand", () => { + const live = { elementCountSource: "live" as const }; + + it("reports applied only when decisive, live-measured, and under the ceiling", () => { + expect( + resolveDeShortBand({ + ...live, + invertAtBaseFloor: false, + invertAtBandFloor: true, + bandEnabled: true, + bandOpen: true, + }), + ).toBe("applied"); + }); + + it("reports skipped_elements when live-measured and over the ceiling", () => { + expect( + resolveDeShortBand({ + ...live, + invertAtBaseFloor: false, + invertAtBandFloor: true, + bandEnabled: true, + bandOpen: false, + }), + ).toBe("skipped_elements"); + }); + + it("is undefined when the base floor already inverts — the band changed nothing", () => { + expect( + resolveDeShortBand({ + ...live, + invertAtBaseFloor: true, + invertAtBandFloor: true, + bandEnabled: true, + bandOpen: true, + }), + ).toBeUndefined(); + }); + + it("is undefined when neither floor inverts — the render was ineligible for some other reason", () => { + expect( + resolveDeShortBand({ + ...live, + invertAtBaseFloor: false, + invertAtBandFloor: false, + bandEnabled: true, + bandOpen: true, + }), + ).toBeUndefined(); + }); + + // Review finding (R1 + R2, both reviewers): HF_DE_SHORT_MAX_ELEMENTS=0 + // must read as "band disabled" (undefined), never "comp too large" + // (skipped_elements) — the latter would poison the DiD control cohort by + // mislabeling a kill-switch event as a real oversize measurement. + it("HF_DE_SHORT_MAX_ELEMENTS=0 (bandEnabled=false) reports undefined even when the render would otherwise be decisive", () => { + expect( + resolveDeShortBand({ + ...live, + invertAtBaseFloor: false, + invertAtBandFloor: true, // an otherwise-eligible in-band render + bandEnabled: false, // the kill switch + bandOpen: false, // deShortBandOpen also false when the switch is off + }), + ).toBeUndefined(); + }); + + // Review finding (R4): the probe supplying the live count is conditional, + // so a static count is an UNBOUNDED undercount on runtime-generated DOM. + // It must never produce "applied" (would route a 40k-node comp) and must + // never produce "skipped_elements" either (would contaminate the DiD + // control cohort with a number that isn't a real oversize observation). + it("fails closed to unmeasured when the count came from the static scan, never applied", () => { + expect( + resolveDeShortBand({ + elementCountSource: "static", + invertAtBaseFloor: false, + invertAtBandFloor: true, + bandEnabled: true, + bandOpen: true, // static scan said "small" — must NOT be believed + }), + ).toBe("unmeasured"); + }); + + it("reports unmeasured (not skipped_elements) for a static count over the ceiling — it is not a real observation either", () => { + expect( + resolveDeShortBand({ + elementCountSource: "static", + invertAtBaseFloor: false, + invertAtBandFloor: true, + bandEnabled: true, + bandOpen: false, + }), + ).toBe("unmeasured"); + }); + + it("stays undefined for a static count when the band was not decisive anyway", () => { + expect( + resolveDeShortBand({ + elementCountSource: "static", + invertAtBaseFloor: true, + invertAtBandFloor: true, + bandEnabled: true, + bandOpen: true, + }), + ).toBeUndefined(); + }); + }); + + // Review finding (R4), end-to-end: the review asked for a regression with a + // known duration, no media / unresolved compositions, and >2500 + // script-created nodes, asserting it cannot enter `applied` without a live + // count. This walks the real decision chain rather than a full render — + // the probe gate, the count resolver, and the band attribution are each the + // production function, wired in the same order the pipeline wires them. + describe("no-probe dynamic-DOM composition cannot enter the applied cohort (R4 regression)", () => { + // A caption-style comp: known duration, no media, builds 4000 spans in + // its own init script. Mirrors packages/producer/tests/style-10-prod. + const DYNAMIC_DOM_HTML = [ + '
', + "", + ].join("\n"); + + it("gets no browser probe — none of the probe conditions fire for this shape", () => { + expect( + probeRequiresBrowser({ + durationSeconds: 13.3, // known + unresolvedCompositionCount: 0, // resolved + hasAutoStart: false, // no media + hasScriptedAudio: false, + hasVariableMedia: false, + // createElement("span") is not createElement("video"|"audio") + hasInsertedMedia: false, + }), + ).toBe(false); + }); + + it("therefore measures statically, and the static count wildly understates the live DOM", async () => { + const resolved = await resolveCompositionElementCount(null, DYNAMIC_DOM_HTML); + expect(resolved.source).toBe("static"); + // Source markup has a handful of tags; the live DOM would have 4000+. + expect(resolved.count).toBeLessThan(2500); + }); + + it("and therefore reports unmeasured — never applied — so it cannot route or join either cohort", async () => { + const { count, source } = await resolveCompositionElementCount(null, DYNAMIC_DOM_HTML); + const bandOpen = source === "live" && count <= 2500; + const band = resolveDeShortBand({ + // A 400-frame render that would otherwise be perfectly eligible. + invertAtBaseFloor: false, + invertAtBandFloor: true, + bandEnabled: true, + bandOpen, + elementCountSource: source, + }); + expect(band).toBe("unmeasured"); + expect(band).not.toBe("applied"); + }); + }); + + describe("mergeWorkerInitObservability", () => { + it("max-merges across workers and ignores workers that reported nothing", () => { + expect( + mergeWorkerInitObservability([ + { initDurationMs: 400, initTweenCount: 900 }, + {}, + { initDurationMs: 1250, initTweenCount: 880 }, + ]), + ).toEqual({ initDurationMs: 1250, tweenCount: 900 }); + }); + + it("returns undefined when no worker reported — summary.init must stay absent, not zeroed", () => { + expect(mergeWorkerInitObservability([])).toBeUndefined(); + expect(mergeWorkerInitObservability([{}, {}])).toBeUndefined(); + }); + }); + + describe("countElementTags", () => { + it("counts closing tags", () => { + expect(countElementTags("
a
")).toBe(2); + }); + + it("counts void elements — an image gallery must not read as a tiny comp", () => { + expect(countElementTags("
")).toBe(3); + expect(countElementTags('')).toBe(2); + }); + + // Review-flagged blocker (v1): SVG elements are neither closing-tag-shaped + // nor in the HTML void list, so a self-closing-SVG-heavy comp read as + // element count 0 — an UNBOUNDED undercount, the same failure class as + // the original counterexample, and the exact shape of comp the + // measured 1.8x regression case is made of. The ceiling cannot bound an + // error that has no bound of its own. + it("counts self-closing SVG elements — the 40k-node regression case must not read as empty", () => { + expect(countElementTags("".repeat(40000))).toBe(40000); + expect(countElementTags('')).toBe(1); + expect(countElementTags("")).toBe(1); + }); + + it("does not double-count a self-closed void element (still just 1)", () => { + expect(countElementTags('')).toBe(1); + expect(countElementTags('')).toBe(1); + }); + + it("does not false-positive on minified JS division-after-comparison (the self-closing alt's real risk)", () => { + // Unspaced "" is the adversarial case: "<" IS immediately + // followed by a letter, so the generic self-closing alt gets as far as + // starting a match — but it still requires the literal two-char "/>" + // sequence, and here a "c" sits between the "/" and the ">", so + // backtracking never finds one and it correctly fails to match. + expect(countElementTags("if(ad){}")).toBe(0); + }); + + 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", () => { + expect(countElementTags("")).toBe(0); + expect(countElementTags("<<<>>>")).toBe(0); + }); + + it("scales to a large document without a full parse", () => { + expect(countElementTags("

x

".repeat(40000))).toBe(40000); + }); + }); + + describe("resolveCompositionElementCount", () => { + // Review finding (R3): a static scan of SOURCE markup cannot see DOM a + // composition's own script creates at runtime (document.createElement) — + // an unbounded undercount no regex can close. style-10-prod's real + // per-transcript-word caption generator is exactly this shape: 2 source + // tags, thousands of live nodes after init. + // + // Review finding (R4): the probe that provides the live count is + // CONDITIONAL, so the fallback cases below are not merely "less precise" + // — they are UNSAFE to gate on, and every one of them must report + // provenance "static" so the caller can fail closed. + it("uses the live DOM count from an initialized probe session, ignoring the (much smaller) source scan", async () => { + const session = { isInitialized: true, page: { evaluate: async () => 40001 } }; + expect(await resolveCompositionElementCount(session, "
")).toEqual({ + count: 40001, + source: "live", + }); + }); + + it("reports static provenance when there is no probe session", async () => { + expect(await resolveCompositionElementCount(null, "
")).toEqual({ + count: 2, + source: "static", + }); + }); + + it("reports static provenance when the probe session is not yet initialized", async () => { + const session = { isInitialized: false, page: { evaluate: async () => 999 } }; + expect(await resolveCompositionElementCount(session, "
")).toEqual({ + count: 1, + source: "static", + }); + }); + + it("reports static provenance when page.evaluate throws (detached frame, mid-navigation)", async () => { + const session = { + isInitialized: true, + page: { + evaluate: async () => { + throw new Error("Execution context was destroyed"); + }, + }, + }; + expect(await resolveCompositionElementCount(session, "
")).toEqual({ + count: 2, + source: "static", + }); + }); + + it("reports static provenance when evaluate resolves a non-finite value", async () => { + const session = { isInitialized: true, page: { evaluate: async () => Number.NaN } }; + expect(await resolveCompositionElementCount(session, "
")).toEqual({ + count: 1, + source: "static", + }); + }); + }); + + describe("envInt", () => { + afterEach(() => { + delete process.env.HF_TEST_ENV_INT; + }); + + it("falls back when unset, empty, or non-numeric — a typo must not disable a guard", () => { + expect(envInt("HF_TEST_ENV_INT", 2500)).toBe(2500); + process.env.HF_TEST_ENV_INT = ""; + expect(envInt("HF_TEST_ENV_INT", 2500)).toBe(2500); + process.env.HF_TEST_ENV_INT = "lots"; + expect(envInt("HF_TEST_ENV_INT", 2500)).toBe(2500); + }); + + it("reads an explicit value, including 0 as a real disable", () => { + process.env.HF_TEST_ENV_INT = "700"; + expect(envInt("HF_TEST_ENV_INT", 2500)).toBe(700); + process.env.HF_TEST_ENV_INT = "0"; + expect(envInt("HF_TEST_ENV_INT", 2500)).toBe(0); + }); + }); + it("honors explicitly requested workers", () => { expect(shouldPreferSingleWorkerDrawElement({ ...eligible, requestedWorkers: 3 })).toBe(false); }); diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts index 232bc29acc..a24e36eda8 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -492,6 +492,12 @@ export interface RenderPerfSummary { workerInversion?: string; /** Worker count the auto-resolution chose BEFORE the inversion pinned it to 1 — the parallel counterfactual for speedup math. Only set when the inversion fired. */ preInversionWorkers?: number; + /** Rough compiled-composition element count — the variable the short-comp inversion band is gated on. Always set. */ + compositionElementCount?: number; + /** Rough compiled-composition element-count provenance: "live" (probe DOM) | "static" (source scan, not trusted to open the band). */ + compositionElementCountSource?: "live" | "static"; + /** Short-comp band attribution: "applied" | "skipped_elements" | "unmeasured"; unset when the frame count made the band irrelevant. */ + shortBand?: "applied" | "skipped_elements" | "unmeasured"; /** DE parallel-router outcome: "routed" (fired, held), "reverted" (fired, self-verify retry rolled back), "none". Mutually exclusive with workerInversion. */ parallelRouter?: string; /** Worker count the auto-resolution chose BEFORE the router pinned it to 3 — the single-worker-inversion counterfactual. Only set when the router fired. */ @@ -1222,6 +1228,183 @@ export function shouldUseStreamingEncode( return workerCount === 1; } +/** + * Integer tuning knob from the environment. Matches the convention the + * surrounding DE thresholds already use: unset OR set-but-empty falls back to + * the default (a blank var is not a kill switch), and so does anything + * non-numeric — a typo must never silently disable a routing guard. + */ +export function envInt(name: string, fallback: number): number { + const raw = process.env[name]; + if (raw === undefined || raw.trim() === "") return fallback; + const parsed = Number(raw); + return Number.isFinite(parsed) ? parsed : fallback; +} + +/** + * Rough element count for compiled composition HTML. + * + * Deliberately a string scan and not a `parseHTML` + `querySelectorAll` (the + * `countAuthoredTimedClips` approach): this runs on EVERY render before the + * 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 tag counting is comfortably inside the + * margin — PROVIDED the count is not unboundedly low for some real content + * shape. Three sources are counted, each catching a case the others miss: + * + * 1. Closing tags (``) — the base count for ordinary HTML. + * 2. Named HTML void elements (``, `
`, …), bare or self-closed — + * 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. + * 3. Any self-closing tag (``, ``) — SVG's own + * elements are neither closing-tag-shaped nor in the void list, so + * without this a self-closing-SVG-heavy composition (`` x 40k) + * counted as ZERO — an unbounded undercount, not a rounding error, and + * exactly the shape of comp the 1.8x regression case is made of + * (review finding: the ceiling cannot compensate for an error with no + * bound). + * + * Opening (non-self-closing, non-void) tags are deliberately NOT counted: + * compiled comps embed inline scripts, and `a < b` or `x `), so + * ordinary JS comparisons and divisions don't qualify — verified by test. + * + * FALLBACK ONLY as of the live-DOM fix below — a string scan of the SOURCE + * markup cannot see elements a composition's own script creates at runtime + * (`document.createElement`), which is an unbounded undercount no regex can + * close: `style-10-prod`'s per-transcript-word caption generator measures 2 + * source tags against thousands of live nodes after init (review finding). + * `resolveCompositionElementCount` prefers the initialized probe session's + * live count and uses this only when no such session exists. + */ +export function countElementTags(html: string): number { + const matches = html.match( + /<\/[a-zA-Z]|<(?:img|br|hr|input|source|track|area|base|col|embed|link|meta|param|wbr)\b|<[a-zA-Z][-a-zA-Z0-9]*\b[^>]*\/>/gi, + ); + return matches === null ? 0 : matches.length; +} + +/** + * Element count for the short-comp band's gate, WITH PROVENANCE. + * + * `live` — measured from the initialized probe session's real DOM. This is + * the only trustworthy source: it sees elements a composition's own script + * created after load, which no scan of the source markup can (the + * caption-word-span pattern above builds thousands of nodes from two source + * tags). + * + * `static` — the `countElementTags` fallback. Emitted for diagnostics, but + * NOT trusted to open the band: the probe is conditional (see + * `probeStage.ts`'s `needsBrowser` — only unknown duration, unresolved + * compositions, or specific media cases launch one), so a known-duration, + * media-free composition that builds 40k nodes in script gets no probe, and + * a static count that says "2". Treating that as measured would admit + * exactly the regression case the ceiling exists to exclude (review finding, + * R4). The caller fails closed on anything but `live`. + * + * 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 resolver that later gates routing, or the baseline is invalid. + */ +export async function resolveCompositionElementCount( + probeSession: Pick | null, + html: string, +): Promise<{ count: number; source: "live" | "static" }> { + if (probeSession?.isInitialized) { + try { + const liveCount = await probeSession.page.evaluate( + () => document.querySelectorAll("*").length, + ); + if (typeof liveCount === "number" && Number.isFinite(liveCount)) { + return { count: liveCount, source: "live" }; + } + } catch { + // Probe page evaluate can fail (navigation mid-flight, detached frame, + // page crash) — fall through to the static scan rather than block the + // render on a routing-gate measurement. + } + } + return { count: countElementTags(html), source: "static" }; +} + +/** + * Max-merge init telemetry across per-worker capture perf summaries — the + * success-path channel for PARALLEL renders, whose worker console buffers + * (and so the `[FrameCapture:INIT]` line) only propagate on failure. Max + * matches summarizeInitObservability's own multi-session semantics: keep the + * worst observed startup cost for duration. Tween count is per-composition, + * so workers should agree — max is a defensive read against a worker that + * initializes before the timeline is fully wired, not an expected disagreement. + */ +export function mergeWorkerInitObservability( + perfs: ReadonlyArray<{ initDurationMs?: number; initTweenCount?: number }>, +): { initDurationMs?: number; tweenCount?: number } | undefined { + let initDurationMs: number | undefined; + let tweenCount: number | undefined; + for (const perf of perfs) { + if (perf.initDurationMs !== undefined) { + initDurationMs = + initDurationMs === undefined + ? perf.initDurationMs + : Math.max(initDurationMs, perf.initDurationMs); + } + if (perf.initTweenCount !== undefined) { + tweenCount = + tweenCount === undefined ? perf.initTweenCount : Math.max(tweenCount, perf.initTweenCount); + } + } + if (initDurationMs === undefined && tweenCount === undefined) return undefined; + return { initDurationMs, tweenCount }; +} + +/** + * The short-comp band's attribution decision, extracted as a pure function so + * the gating fixes below are independently testable rather than living inline + * where only a full render pipeline run could exercise them. + * + * A value is emitted ONLY when the band is DECISIVE — every other + * inversion-eligibility condition already passed (both floor evaluations + * agree on everything except which floor they used) and the band floor alone + * flipped the answer. `bandEnabled` gates that decisiveness itself: + * `HF_DE_SHORT_MAX_ELEMENTS=0` is a documented kill switch (symmetric with + * `HF_DE_SHORT_MIN_FRAMES=0`, which already disables via the predicate's own + * `minFrames > 0` guard), and without this gate a fired kill switch left + * every in-band render decisive against a real floor comparison — reporting + * "skipped_elements" (comp too large) instead of undefined (band disabled) + * and corrupting the DiD control cohort with kill-switched renders (review + * finding). + * + * Three decisive outcomes, and the distinction between the last two is the + * point: + * "applied" — measured LIVE and under the ceiling. Only this + * routes (once HF_DE_SHORT_BAND_ROUTE is on) and only + * this joins the treatment cohort. + * "skipped_elements" — measured live, over the ceiling. A real oversize + * observation; the DiD control group. + * "unmeasured" — no live DOM count available (no probe session ran; + * see `resolveCompositionElementCount`). FAILS CLOSED: + * never routes, and kept out of BOTH cohorts so a + * static undercount cannot masquerade as a small comp + * (review finding, R4). Emitted rather than dropped + * because its fleet rate sizes the population a + * future conditional-probe-launch would unlock. + */ +export function resolveDeShortBand(args: { + invertAtBaseFloor: boolean; + invertAtBandFloor: boolean; + bandEnabled: boolean; + bandOpen: boolean; + elementCountSource: "live" | "static"; +}): "applied" | "skipped_elements" | "unmeasured" | undefined { + const decisive = args.bandEnabled && args.invertAtBandFloor && !args.invertAtBaseFloor; + if (!decisive) return undefined; + if (args.elementCountSource !== "live") return "unmeasured"; + return args.bandOpen ? "applied" : "skipped_elements"; +} + /** * DE priority inversion predicate: should an AUTO-resolved multi-worker render * drop to single-worker verified drawElement streaming? @@ -2405,10 +2588,58 @@ async function executeRenderPipeline(input: { ? 900 : Number(deSingleMinFramesRaw); const deSingleMinFrames = Number.isFinite(deSingleMinFramesNum) ? deSingleMinFramesNum : 900; + // Short-comp band: 31% of fleet renders (24h, 0.7.78+) are DE-eligible + // comps clamped to parallel screenshot purely because they sit under this + // floor. A controlled sweep (fixed synthetic content, {250,400,600,900}f, + // single-DE vs parallel-screenshot-W4, 3 reps, capture modes verified per + // run) showed single-DE winning 1.16-1.24x at EVERY size — but only for + // content in constant motion. A follow-up 2x2 (movers x static DOM nodes) + // found the two variables pull in opposite directions: motion favours DE, + // DOM size punishes it, and DE's wall-clock scales ~0.50ms/node against + // parallel screenshot's ~0.22ms (drawElement repaints the whole tree per + // frame; fan-out amortizes it). At 24 movers / 400f the measured curve is + // +5% for DE at 0 nodes, -4% at 7k, -41% at 20k, -80% at 40k — crossover + // near ~3.9k. Since motion only ever helps DE, a node ceiling calibrated + // at the LOWEST-motion case is safe for every motion level, so the short + // band opens only below `deShortBandMaxElements`. Above it the original + // 900 floor stands, unchanged. + const deShortBandMinFrames = envInt("HF_DE_SHORT_MIN_FRAMES", 250); + const deShortBandMaxElements = envInt("HF_DE_SHORT_MAX_ELEMENTS", 2500); + // `source` is load-bearing, not diagnostic: the probe is CONDITIONAL + // (probeStage's `needsBrowser` — unknown duration, unresolved + // compositions, or specific media cases), so a known-duration media-free + // comp that builds its DOM in script has no live count available and the + // static scan reads it as tiny. Only a `live` count may open the band. + const { count: compositionElementCount, source: compositionElementCountSource } = + await resolveCompositionElementCount(probeSession, compiled.html); + // HF_DE_SHORT_MAX_ELEMENTS=0 is the documented kill switch (symmetric + // with HF_DE_SHORT_MIN_FRAMES=0, which disables via the predicate's own + // minFrames > 0 guard). Gated explicitly here too — without it, a fired + // max-elements kill switch left every in-band render decisive against a + // real floor comparison, so it reported "skipped_elements" (comp too + // large) instead of undefined (band disabled), corrupting the DiD + // control cohort with kill-switched renders (review finding). + const deShortBandEnabled = deShortBandMaxElements > 0; + const deShortBandOpen = + deShortBandEnabled && + deShortBandMinFrames > 0 && + compositionElementCountSource === "live" && + compositionElementCount <= deShortBandMaxElements; + // 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, @@ -2428,7 +2659,32 @@ 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), + }); + const deShortBand = resolveDeShortBand({ + invertAtBaseFloor, + invertAtBandFloor, + bandEnabled: deShortBandEnabled, + bandOpen: deShortBandOpen, + elementCountSource: compositionElementCountSource, }); + const deInversionEligible = + deShortBandRoute && deShortBand === "applied" ? invertAtBandFloor : invertAtBaseFloor; + // The floor that actually decided this render — for the human-facing log + // below, so it never claims e.g. "400 frames >= 900" for a band-routed + // inversion (review finding). + const deInversionEffectiveMinFrames = + deShortBandRoute && deShortBand === "applied" + ? Math.min(deSingleMinFrames, deShortBandMinFrames) + : deSingleMinFrames; // 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 @@ -2662,7 +2918,7 @@ async function executeRenderPipeline(input: { log.info( "[Render] Fast capture: single-worker drawElement streaming preferred over " + `${workerCount}-worker screenshot capture (${totalFrames} frames >= ` + - `${deSingleMinFrames}; verified path, measured faster at every worker count). ` + + `${deInversionEffectiveMinFrames}; verified path, measured faster at every worker count). ` + "Set HF_DE_SINGLE_MIN_FRAMES=0 or --workers N to override.", ); workerCount = 1; @@ -2678,6 +2934,12 @@ async function executeRenderPipeline(input: { // any resource-pressure failure unique to this cohort. dePreInversionWorkers: deWorkerInversion ? preRoutingWorkerCount : undefined, dePreRouterWorkers: deParallelRouter ? preRoutingWorkerCount : undefined, + // Short-comp band attribution — see the field docs. Emitted on every + // 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, + compositionElementCountSource, + 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 @@ -3496,6 +3758,7 @@ async function executeRenderPipeline(input: { const observabilitySummary = observability.summary({ lastBrowserConsole, capture: captureObservability, + initFallback: mergeWorkerInitObservability(dedupPerfs), extraction: extractionObservability, compositionHash, }); @@ -3524,6 +3787,9 @@ async function executeRenderPipeline(input: { clampReason: deClampReason, workerInversion: deWorkerInversion, preInversionWorkers: deWorkerInversion ? preRoutingWorkerCount : undefined, + compositionElementCount, + compositionElementCountSource, + shortBand: deShortBand, parallelRouter: deParallelRouter, preRouterWorkers: deParallelRouter ? preRoutingWorkerCount : undefined, selfVerifyFallback: deSelfVerifyFallback, @@ -3655,6 +3921,7 @@ async function executeRenderPipeline(input: { const observabilitySummary = observability.summary({ lastBrowserConsole, capture: captureObservability, + initFallback: mergeWorkerInitObservability(dedupPerfs), extraction: extractionObservability, compositionHash, });