Skip to content

Commit e9de2fa

Browse files
vanceingallsclaude
andcommitted
refactor(producer): make the short-comp band baseline-first and its attribution decisive
Recut after pre-registering the read exposed two flaws in the first cut: 1. Attribution was wrong. de_short_band keyed on frame count + element ceiling alone, so a webm render, a compile-gated comp, or a forced screenshot at 400f reported "applied" while its routing was untouched — poisoning the measurement cohort with unaffected renders and diluting any effect toward zero. Now the predicate is evaluated twice (900 floor vs band floor) and the band is DECISIVE only when the calls disagree: every other eligibility condition passed and only the floor differed. The cohort contains exactly the renders whose routing the band decides. 2. A same-release flip is unfalsifiable. composition_element_count ships WITH the routing change, so the before-period cannot be filtered to the same cohort as the after-period — the comparison would show a speedup even if the change did nothing (the after-cohort excludes big comps by construction; the before-cohort includes them). Routing is therefore gated behind HF_DE_SHORT_BAND_ROUTE, default OFF: this release computes and emits the full band decision on every render ("applied" is the counterfactual "would have inverted"), a follow-up flips the default. Identical cohort selector on both sides of the boundary, and the skipped/oversize renders in the same frame band form a concurrent control — a difference-in-differences that absorbs secular drift (content mix, version-correlated populations, hardware), which a plain before/after cannot. Also: countElementTags now counts HTML void elements. Counting only closers read an image gallery as a tiny comp and opened the band on exactly the content most likely to lose it (images skew expensive to paint). Opening tags stay uncounted — inline scripts' `a < b` would false-positive. Counter semantics are frozen while the baseline is read: the distribution the baseline release records must be measured by the same counter that later gates. Revert-rate baseline for the pre-registered read, measured over 14d fleet-wide: the 900+ inversion runs 31,756 inverted / 1,705 reverted = 5.1%. At the benched 1.16-1.24x win and ~1.8x revert cost, expected net for the band is ~12%. Kill criteria for the flip release: DiD <= 0, in-band revert rate > 5.1% baseline, or band fallback rate > DE baseline. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
1 parent 0749cd9 commit e9de2fa

4 files changed

Lines changed: 126 additions & 84 deletions

File tree

‎packages/producer/src/services/render/observability.ts‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,17 @@ export interface RenderCaptureObservability {
8181
*/
8282
compositionElementCount?: number;
8383
/**
84-
* Why the short-comp band did or did not apply to this render:
85-
* "applied" (frames landed in 250-899 and the element count cleared the
86-
* ceiling), "skipped_elements" (band was open by frame count but the comp
87-
* was too large), or undefined when the frame count made the band
88-
* irrelevant either way. Distinguishes "the new routing chose this" from
89-
* "the pre-existing 900 floor chose this" — otherwise a fleet perf shift is
90-
* unattributable.
84+
* Short-comp band decision, emitted only when the band is DECISIVE — every
85+
* other inversion-eligibility condition passed and only the floor (250 vs
86+
* 900) differed. "applied": the element count cleared the ceiling too, so
87+
* with routing enabled (HF_DE_SHORT_BAND_ROUTE) this render inverts; in the
88+
* baseline release the same value is the COUNTERFACTUAL "would have
89+
* inverted". "skipped_elements": the element ceiling was the only blocker.
90+
* Unset: the band could not have affected this render (ineligible for some
91+
* other reason, or already inverting at 900+). The selector is computed
92+
* identically before and after the routing flip, and the skipped/oversize
93+
* renders form the concurrent control for the difference-in-differences
94+
* read — that is the entire point of the field.
9195
*/
9296
deShortBand?: "applied" | "skipped_elements";
9397
/** DE parallel-router outcome: "routed" (fired, held) | "reverted" (fired, self-verify retry rolled back). */

‎packages/producer/src/services/render/perfSummary.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export interface DrawElementPerfInput {
8282
preInversionWorkers?: number;
8383
/** Rough compiled-composition element count — gate variable for the short-comp inversion band. */
8484
compositionElementCount?: number;
85-
/** Short-comp band attribution: "applied" | "skipped_elements"; unset when the band was irrelevant. */
85+
/** 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. */
8686
shortBand?: string;
8787
parallelRouter?: "routed" | "reverted";
8888
/** Auto-resolved worker count before the router pinned it to 3 (set only when the router fired). */

‎packages/producer/src/services/renderOrchestrator.test.ts‎

Lines changed: 63 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1695,63 +1695,67 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
16951695
expect(shouldPreferSingleWorkerDrawElement(eligible)).toBe(true);
16961696
});
16971697

1698-
// ── Short-comp band ────────────────────────────────────────────────────
1699-
// The band lowers the effective floor from 900 to 250 for SMALL comps only.
1700-
// The predicate itself is unchanged — the call site picks the floor — so
1701-
// these pin the arithmetic the call site performs.
1698+
// ── Short-comp band ──────────────────────────────────────────────────────────────────────
1699+
// The call site evaluates the predicate TWICE — once at the 900 floor, once
1700+
// at the 250 band floor — and the band is DECISIVE only when the calls
1701+
// disagree. These pin that arithmetic, including the property the design
1702+
// depends on: the band can only ADD inversions, never remove one.
17021703
//
1703-
// Measured basis (400f, single-DE vs parallel-screenshot-W4, 2 reps, ratio
1704-
// = ss4/de1 so >1 means DE wins):
1705-
// 24 movers / 0 nodes -> 1.05 DE wins
1706-
// 320 movers / 0 nodes -> 1.24 DE wins
1707-
// 320 movers / 7000 nodes -> 1.09 DE wins
1708-
// 24 movers / 7000 nodes -> 0.96 DE LOSES
1709-
// 24 movers / 20000 nodes -> 0.71 DE loses badly
1710-
// 24 movers / 40000 nodes -> 0.55 DE loses very badly
1711-
// Motion helps DE, DOM size punishes it; the ceiling is calibrated at the
1712-
// lowest-motion case so every higher-motion comp is covered too.
1713-
describe("short-comp band floor arithmetic", () => {
1714-
const shortBandFloor = (elementCount: number, maxElements = 2500): number =>
1715-
elementCount <= maxElements ? Math.min(900, 250) : 900;
1716-
1717-
it("opens the 250-frame floor for a small comp", () => {
1718-
expect(shortBandFloor(800)).toBe(250);
1719-
expect(
1720-
shouldPreferSingleWorkerDrawElement({
1721-
...eligible,
1722-
totalFrames: 400,
1723-
minFrames: shortBandFloor(800),
1724-
}),
1725-
).toBe(true);
1704+
// Measured basis (400f, single-DE vs parallel-screenshot-W4, ratio = ss4/de1
1705+
// so >1 means DE wins):
1706+
// 24 movers / 0 nodes -> 1.05 | 24 movers / 7000 nodes -> 0.96
1707+
// 320 movers / 0 nodes -> 1.24 | 24 movers / 20000 nodes -> 0.71
1708+
// 320 movers / 7000 nodes -> 1.09 | 24 movers / 40000 nodes -> 0.55
1709+
// Motion helps DE, DOM size punishes it; the element ceiling is calibrated
1710+
// at the lowest-motion case so every higher-motion comp is covered too.
1711+
describe("short-comp band decisiveness (two-floor evaluation)", () => {
1712+
const BAND_FLOOR = Math.min(900, 250);
1713+
const atBase = (totalFrames: number, over?: Partial<typeof eligible>) =>
1714+
shouldPreferSingleWorkerDrawElement({ ...eligible, ...over, totalFrames, minFrames: 900 });
1715+
const atBand = (totalFrames: number, over?: Partial<typeof eligible>) =>
1716+
shouldPreferSingleWorkerDrawElement({
1717+
...eligible,
1718+
...over,
1719+
totalFrames,
1720+
minFrames: BAND_FLOOR,
1721+
});
1722+
1723+
it("is decisive exactly in the 250-899 window for an otherwise-eligible render", () => {
1724+
expect(atBand(400) && !atBase(400)).toBe(true);
1725+
expect(atBand(250) && !atBase(250)).toBe(true);
1726+
expect(atBand(899) && !atBase(899)).toBe(true);
17261727
});
17271728

1728-
it("keeps the 900-frame floor for a large comp — the measured 1.8x regression case", () => {
1729-
expect(shortBandFloor(40000)).toBe(900);
1730-
expect(
1731-
shouldPreferSingleWorkerDrawElement({
1732-
...eligible,
1733-
totalFrames: 400,
1734-
minFrames: shortBandFloor(40000),
1735-
}),
1736-
).toBe(false);
1729+
it("is NOT decisive below the band floor — nothing fires either way", () => {
1730+
expect(atBand(200)).toBe(false);
1731+
expect(atBase(200)).toBe(false);
17371732
});
17381733

1739-
it("never RAISES the floor: a large comp at 900+ frames still inverts as it did before", () => {
1740-
expect(
1741-
shouldPreferSingleWorkerDrawElement({
1742-
...eligible,
1743-
totalFrames: 2380,
1744-
minFrames: shortBandFloor(40000),
1745-
}),
1746-
).toBe(true);
1734+
it("is NOT decisive at 900+ — the pre-existing floor already inverts, unchanged", () => {
1735+
expect(atBase(2380)).toBe(true);
1736+
expect(atBand(2380) && !atBase(2380)).toBe(false);
1737+
});
1738+
1739+
it("is NOT decisive when the render is ineligible for any other reason — the attribution cohort must exclude renders the band cannot affect", () => {
1740+
for (const over of [
1741+
{ requestedWorkers: 3 as const },
1742+
{ useDrawElement: false },
1743+
{ deCompileGate: "css_effect:filter" },
1744+
{ forceScreenshot: true },
1745+
{ outputFormat: "webm" as const },
1746+
{ singleWorkerStreamingOk: false },
1747+
{ probeDeGated: true },
1748+
]) {
1749+
expect(atBand(400, over)).toBe(false);
1750+
}
17471751
});
17481752

1749-
it("leaves comps below the band floor alone", () => {
1753+
it("HF_DE_SHORT_MIN_FRAMES=0 disables via the predicate's own minFrames guard", () => {
17501754
expect(
17511755
shouldPreferSingleWorkerDrawElement({
17521756
...eligible,
1753-
totalFrames: 200,
1754-
minFrames: shortBandFloor(800),
1757+
totalFrames: 400,
1758+
minFrames: Math.min(900, 0),
17551759
}),
17561760
).toBe(false);
17571761
});
@@ -1762,8 +1766,18 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
17621766
expect(countElementTags("<div><span>a</span></div>")).toBe(2);
17631767
});
17641768

1765-
it("undercounts void elements — biases the count DOWN, so the ceiling must stay conservative", () => {
1766-
expect(countElementTags("<img><br><hr>")).toBe(0);
1769+
it("counts void elements — an image gallery must not read as a tiny comp", () => {
1770+
expect(countElementTags("<img><br><hr>")).toBe(3);
1771+
expect(countElementTags('<img src="a.png"><IMG SRC="b.png">')).toBe(2);
1772+
});
1773+
1774+
it("does not false-positive on inline-script comparisons or void-prefixed words", () => {
1775+
// Only the </script> closer counts: "<breadth" and "<imgWidth" hit the
1776+
// br/img alternatives but fail the \b word boundary (next char is a
1777+
// word char), and bare "a < b" comparisons match nothing.
1778+
expect(countElementTags("<script>if (a < b && x <breadth && y <imgWidth) {}</script>")).toBe(
1779+
1,
1780+
);
17671781
});
17681782

17691783
it("is stable on empty and malformed input rather than throwing", () => {

‎packages/producer/src/services/renderOrchestrator.ts‎

Lines changed: 51 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,13 +1247,22 @@ export function envInt(name: string, fallback: number): number {
12471247
* routing decision, and a full linkedom parse of the exact documents that
12481248
* matter here — the 20k-40k node ones — is the most expensive case. Precision
12491249
* is not needed. It feeds a threshold whose measured crossover is ~3.9k and
1250-
* whose default sits at 2500, so counting closing tags is comfortably inside
1251-
* the margin. Undercounts void elements (`<img>`, `<br>`) and self-closing
1252-
* SVG nodes, which biases the count DOWN — the direction that opens the band
1253-
* — so the ceiling is the thing to keep conservative.
1250+
* whose default sits at 2500, so tag counting is comfortably inside the
1251+
* margin. Closing tags plus HTML void elements (`<img>`, `<br>`, …) — voids
1252+
* matter because they skew EXPENSIVE to paint (images), and counting only
1253+
* closers would read an image gallery as a tiny comp and open the band on
1254+
* exactly the content most likely to lose it. Opening tags are deliberately
1255+
* NOT counted: compiled comps embed inline scripts where `a < b` would
1256+
* false-positive. Self-closing SVG children still undercount; acceptable.
1257+
*
1258+
* These semantics are FROZEN while the short-band baseline is being read —
1259+
* the fleet distribution recorded by the baseline release must be measured
1260+
* by the same counter that later gates routing, or the baseline is invalid.
12541261
*/
12551262
export function countElementTags(html: string): number {
1256-
const matches = html.match(/<\/[a-zA-Z]/g);
1263+
const matches = html.match(
1264+
/<\/[a-zA-Z]|<(?:img|br|hr|input|source|track|area|base|col|embed|link|meta|param|wbr)\b/gi,
1265+
);
12571266
return matches === null ? 0 : matches.length;
12581267
}
12591268

@@ -2462,28 +2471,29 @@ async function executeRenderPipeline(input: {
24622471
deShortBandMinFrames > 0 &&
24632472
deShortBandMaxElements > 0 &&
24642473
compositionElementCount <= deShortBandMaxElements;
2465-
const deEffectiveMinFrames = deShortBandOpen
2466-
? Math.min(deSingleMinFrames, deShortBandMinFrames)
2467-
: deSingleMinFrames;
2468-
// Does the band even matter for this render? Only when the frame count
2469-
// falls in the newly-opened window — at or above the original floor the
2470-
// inversion fires regardless, and below `deShortBandMinFrames` nothing
2471-
// fires either way. Keeps the telemetry reason from claiming credit (or
2472-
// blame) on renders the change could not have affected.
2473-
const deShortBandApplies =
2474-
totalFrames >= deShortBandMinFrames && totalFrames < deSingleMinFrames;
2474+
// Baseline-first sequencing: this release EVALUATES the band on every
2475+
// render and emits the decision, but only routes on it when
2476+
// HF_DE_SHORT_BAND_ROUTE=true (flipped by default in a follow-up release).
2477+
// The point is a difference-in-differences read: the cohort selector
2478+
// (`de_short_band`) is computed identically before and after the flip —
2479+
// "applied" is counterfactual in the baseline release and factual after —
2480+
// and the skipped/oversize renders in the same frame band form a
2481+
// concurrent control that absorbs secular drift (content mix, version-
2482+
// correlated populations, hardware). A plain before/after cannot
2483+
// attribute a fleet perf shift to this change; this can.
2484+
const deShortBandRoute = process.env.HF_DE_SHORT_BAND_ROUTE === "true";
24752485
// "Would ANY multi-worker resolution be inverted?" — if workers resolve
24762486
// to 1 naturally the outcome is identical either way.
24772487
const WOULD_RESOLVE_MULTI_WORKER = 2;
2478-
const deInversionEligible = shouldPreferSingleWorkerDrawElement({
2488+
const deInversionArgs = {
24792489
workerCount: WOULD_RESOLVE_MULTI_WORKER,
24802490
requestedWorkers: job.config.workers,
24812491
useDrawElement: cfg.useDrawElement,
24822492
deCompileGate,
24832493
forceScreenshot: captureForceScreenshot,
24842494
outputFormat,
24852495
totalFrames,
2486-
minFrames: deEffectiveMinFrames,
2496+
minFrames: deSingleMinFrames,
24872497
singleWorkerStreamingOk: shouldUseStreamingEncode(cfg, outputFormat, 1, job.duration),
24882498
layeredOrEffectRoute: hasHdrContent || compiled.hasShaderTransitions,
24892499
supersampling: deviceScaleFactor > 1,
@@ -2495,7 +2505,29 @@ async function executeRenderPipeline(input: {
24952505
process.env.PRODUCER_EXPERIMENTAL_FAST_CAPTURE === "true" ||
24962506
// Verified parallel DE streaming (opt-in) wants its parallelism kept.
24972507
process.env.HF_DE_PARALLEL_STREAM === "true",
2508+
};
2509+
const invertAtBaseFloor = shouldPreferSingleWorkerDrawElement(deInversionArgs);
2510+
// Same render, same eligibility, band floor instead of 900. Math.min so a
2511+
// user override of HF_DE_SINGLE_MIN_FRAMES below the band floor keeps
2512+
// winning; HF_DE_SHORT_MIN_FRAMES=0 disables via the predicate's own
2513+
// minFrames > 0 check.
2514+
const invertAtBandFloor = shouldPreferSingleWorkerDrawElement({
2515+
...deInversionArgs,
2516+
minFrames: Math.min(deSingleMinFrames, deShortBandMinFrames),
24982517
});
2518+
// The band is DECISIVE only when it alone flips the decision — every
2519+
// other eligibility condition already passed and only the floor differed.
2520+
// Renders the band could not have affected (ineligible for any other
2521+
// reason, or already inverting at 900+) stay unset, so the telemetry
2522+
// cohort contains exactly the renders whose routing this change decides.
2523+
const deShortBandDecisive = invertAtBandFloor && !invertAtBaseFloor;
2524+
const deShortBand = deShortBandDecisive
2525+
? deShortBandOpen
2526+
? ("applied" as const)
2527+
: ("skipped_elements" as const)
2528+
: undefined;
2529+
const deInversionEligible =
2530+
deShortBandRoute && deShortBand === "applied" ? invertAtBandFloor : invertAtBaseFloor;
24992531
// DE parallel-router eligibility — see shouldPreferParallelDrawElement.
25002532
// Default-off (HF_DE_PARALLEL_ROUTER); HF_DE_PARALLEL_MIN_FRAMES default
25012533
// 700, re-calibrated 2026-07-27 from the original safe-high 2000. A
@@ -2749,11 +2781,7 @@ async function executeRenderPipeline(input: {
27492781
// render so the fleet element-count distribution is readable, and so a
27502782
// perf shift can be split into "the new band did it" vs "unchanged".
27512783
compositionElementCount,
2752-
deShortBand: deShortBandApplies
2753-
? deShortBandOpen
2754-
? "applied"
2755-
: "skipped_elements"
2756-
: undefined,
2784+
deShortBand,
27572785
// Same rationale as the counters above: carried on live capture
27582786
// observability, not only the success-path perfSummary, so a crash /
27592787
// OOM / timeout still reports which GPU backend it happened on. That
@@ -3601,11 +3629,7 @@ async function executeRenderPipeline(input: {
36013629
workerInversion: deWorkerInversion,
36023630
preInversionWorkers: deWorkerInversion ? preRoutingWorkerCount : undefined,
36033631
compositionElementCount,
3604-
shortBand: deShortBandApplies
3605-
? deShortBandOpen
3606-
? "applied"
3607-
: "skipped_elements"
3608-
: undefined,
3632+
shortBand: deShortBand,
36093633
parallelRouter: deParallelRouter,
36103634
preRouterWorkers: deParallelRouter ? preRoutingWorkerCount : undefined,
36113635
selfVerifyFallback: deSelfVerifyFallback,

0 commit comments

Comments
 (0)