mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-10 22:20:14 +00:00
fix(producer): fail closed when no live element count is available
R4 review finding, and the comment I wrote in R3 was simply wrong: the
probe session is NOT running for every render. probeStage's needsBrowser
gate launches one only for unknown duration, unresolved compositions, or
specific media cases — and hasRuntimeInsertedMedia matches only
createElement("video"|"audio"), never createElement("span"). So the exact
shape that motivated the live-DOM fix (a known-duration, media-free
caption comp building thousands of nodes in script) gets NO probe, falls
back to the static source scan, reads as ~2 elements, and could enter the
applied cohort at 40k live nodes. The R3 fix measured the right thing but
only for the population that already had a probe.
Now the count carries provenance and the band fails closed:
- resolveCompositionElementCount returns { count, source: "live" |
"static" }. Only "live" — an actual DOM measurement — may open the band.
- resolveDeShortBand gains a third decisive outcome, "unmeasured", for
the static case. It deliberately does NOT report skipped_elements: a
static undercount is not a real oversize observation, and putting it in
the control arm would contaminate the DiD just as putting it in the
treatment arm would. Neither cohort; never routes.
- composition_element_count_source ships alongside the count, so the
fleet rate of "static" sizes the population a future
conditional-probe-launch would unlock — which is the data PR B needs to
decide whether that launch cost is worth paying.
Regression coverage walks the real chain rather than a full render, using
the production functions in pipeline order: probeRequiresBrowser (newly
extracted from the inline needsBrowser expression, so the gate is
testable at all) returns false for the caption-comp shape → the resolver
reports static and a count under the ceiling → the band reports
unmeasured, not applied. Fault injection confirms it bites: removing the
one guard line fails exactly these three tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
def98f79c3
commit
4dbf0d90b0
@@ -84,6 +84,15 @@ export interface RenderCaptureObservability {
|
||||
* 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
|
||||
@@ -97,7 +106,7 @@ export interface RenderCaptureObservability {
|
||||
* renders form the concurrent control for the difference-in-differences
|
||||
* read — that is the entire point of the field.
|
||||
*/
|
||||
deShortBand?: "applied" | "skipped_elements";
|
||||
deShortBand?: "applied" | "skipped_elements" | "unmeasured";
|
||||
/** DE parallel-router outcome: "routed" (fired, held) | "reverted" (fired, self-verify retry rolled back). */
|
||||
deParallelRouter?: "routed" | "reverted";
|
||||
/**
|
||||
|
||||
@@ -82,8 +82,10 @@ export interface DrawElementPerfInput {
|
||||
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";
|
||||
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;
|
||||
@@ -126,6 +128,7 @@ function aggregateDrawElement(
|
||||
workerInversion: de.workerInversion ?? "none",
|
||||
preInversionWorkers: de.preInversionWorkers,
|
||||
compositionElementCount: de.compositionElementCount,
|
||||
compositionElementCountSource: de.compositionElementCountSource,
|
||||
shortBand: de.shortBand,
|
||||
parallelRouter: de.parallelRouter ?? "none",
|
||||
preRouterWorkers: de.preRouterWorkers,
|
||||
|
||||
@@ -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<ProbeStageResult> {
|
||||
const {
|
||||
projectDir,
|
||||
@@ -217,13 +248,14 @@ export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageR
|
||||
);
|
||||
const hasVariableMedia = hasVariableBoundMedia(compiled.html, job.config.variables);
|
||||
const hasInsertedMedia = hasRuntimeInsertedMedia(compiled.html);
|
||||
const needsBrowser =
|
||||
composition.duration <= 0 ||
|
||||
compiled.unresolvedCompositions.length > 0 ||
|
||||
hasAutoStart ||
|
||||
hasScriptedAudio ||
|
||||
hasVariableMedia ||
|
||||
hasInsertedMedia;
|
||||
const needsBrowser = probeRequiresBrowser({
|
||||
durationSeconds: composition.duration,
|
||||
unresolvedCompositionCount: compiled.unresolvedCompositions.length,
|
||||
hasAutoStart,
|
||||
hasScriptedAudio,
|
||||
hasVariableMedia,
|
||||
hasInsertedMedia,
|
||||
});
|
||||
|
||||
if (needsBrowser) {
|
||||
const reasons = [];
|
||||
|
||||
@@ -44,6 +44,7 @@ import {
|
||||
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 {
|
||||
@@ -1765,9 +1766,12 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
|
||||
});
|
||||
|
||||
describe("resolveDeShortBand", () => {
|
||||
it("reports applied only when decisive and the element ceiling cleared", () => {
|
||||
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,
|
||||
@@ -1776,9 +1780,10 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
|
||||
).toBe("applied");
|
||||
});
|
||||
|
||||
it("reports skipped_elements when decisive but the comp is oversized", () => {
|
||||
it("reports skipped_elements when live-measured and over the ceiling", () => {
|
||||
expect(
|
||||
resolveDeShortBand({
|
||||
...live,
|
||||
invertAtBaseFloor: false,
|
||||
invertAtBandFloor: true,
|
||||
bandEnabled: true,
|
||||
@@ -1790,6 +1795,7 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
|
||||
it("is undefined when the base floor already inverts — the band changed nothing", () => {
|
||||
expect(
|
||||
resolveDeShortBand({
|
||||
...live,
|
||||
invertAtBaseFloor: true,
|
||||
invertAtBandFloor: true,
|
||||
bandEnabled: true,
|
||||
@@ -1801,6 +1807,7 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
|
||||
it("is undefined when neither floor inverts — the render was ineligible for some other reason", () => {
|
||||
expect(
|
||||
resolveDeShortBand({
|
||||
...live,
|
||||
invertAtBaseFloor: false,
|
||||
invertAtBandFloor: false,
|
||||
bandEnabled: true,
|
||||
@@ -1816,6 +1823,7 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
|
||||
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
|
||||
@@ -1823,6 +1831,105 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
|
||||
}),
|
||||
).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 = [
|
||||
'<div id="root"><div id="captions"></div></div>',
|
||||
"<script>",
|
||||
' const c = document.getElementById("captions");',
|
||||
" for (let i = 0; i < 4000; i++) {",
|
||||
' const el = document.createElement("span");',
|
||||
" el.textContent = String(i);",
|
||||
" c.appendChild(el);",
|
||||
" }",
|
||||
"</script>",
|
||||
].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", () => {
|
||||
@@ -1902,24 +2009,36 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
|
||||
// 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. These pin the fix: prefer the
|
||||
// live count from an initialized probe session, static scan only as
|
||||
// fallback.
|
||||
// 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, "<div><span></span></div>")).toBe(40001);
|
||||
expect(await resolveCompositionElementCount(session, "<div><span></span></div>")).toEqual({
|
||||
count: 40001,
|
||||
source: "live",
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to the static scan when there is no probe session", async () => {
|
||||
expect(await resolveCompositionElementCount(null, "<div><span></span></div>")).toBe(2);
|
||||
it("reports static provenance when there is no probe session", async () => {
|
||||
expect(await resolveCompositionElementCount(null, "<div><span></span></div>")).toEqual({
|
||||
count: 2,
|
||||
source: "static",
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to the static scan when the probe session is not yet initialized", async () => {
|
||||
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, "<div></div>")).toBe(1);
|
||||
expect(await resolveCompositionElementCount(session, "<div></div>")).toEqual({
|
||||
count: 1,
|
||||
source: "static",
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to the static scan when page.evaluate throws (detached frame, mid-navigation)", async () => {
|
||||
it("reports static provenance when page.evaluate throws (detached frame, mid-navigation)", async () => {
|
||||
const session = {
|
||||
isInitialized: true,
|
||||
page: {
|
||||
@@ -1928,12 +2047,18 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
|
||||
},
|
||||
},
|
||||
};
|
||||
expect(await resolveCompositionElementCount(session, "<div><span></span></div>")).toBe(2);
|
||||
expect(await resolveCompositionElementCount(session, "<div><span></span></div>")).toEqual({
|
||||
count: 2,
|
||||
source: "static",
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to the static scan when evaluate resolves a non-finite value", async () => {
|
||||
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, "<div></div>")).toBe(1);
|
||||
expect(await resolveCompositionElementCount(session, "<div></div>")).toEqual({
|
||||
count: 1,
|
||||
source: "static",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -494,8 +494,10 @@ export interface RenderPerfSummary {
|
||||
preInversionWorkers?: number;
|
||||
/** Rough compiled-composition element count — the variable the short-comp inversion band is gated on. Always set. */
|
||||
compositionElementCount?: number;
|
||||
/** Short-comp band attribution: "applied" | "skipped_elements"; unset when the frame count made the band irrelevant. */
|
||||
shortBand?: "applied" | "skipped_elements";
|
||||
/** 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. */
|
||||
@@ -1286,16 +1288,22 @@ export function countElementTags(html: string): number {
|
||||
}
|
||||
|
||||
/**
|
||||
* Element count for the short-comp band's gate — prefers the LIVE DOM of the
|
||||
* probe session that's already running for every render at this point in the
|
||||
* pipeline (its Chrome is reused for capture on the common single-worker
|
||||
* path, so this costs one extra CDP round-trip, not a browser launch) over
|
||||
* the static `countElementTags` scan of `compiled.html`. The live count is
|
||||
* the only one that sees runtime-generated DOM — a composition whose script
|
||||
* builds its own elements after load (the caption-word-span pattern above)
|
||||
* is otherwise measured as near-empty regardless of how the static scanner
|
||||
* is tuned, admitting an arbitrarily large live DOM below the routing
|
||||
* ceiling (review finding, R3).
|
||||
* 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
|
||||
@@ -1304,20 +1312,22 @@ export function countElementTags(html: string): number {
|
||||
export async function resolveCompositionElementCount(
|
||||
probeSession: Pick<CaptureSession, "isInitialized" | "page"> | null,
|
||||
html: string,
|
||||
): Promise<number> {
|
||||
): 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 liveCount;
|
||||
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 countElementTags(html);
|
||||
return { count: countElementTags(html), source: "static" };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1355,26 +1365,43 @@ export function mergeWorkerInitObservability(
|
||||
* the gating fixes below are independently testable rather than living inline
|
||||
* where only a full render pipeline run could exercise them.
|
||||
*
|
||||
* "applied" / "skipped_elements" are 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
|
||||
* 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;
|
||||
}): "applied" | "skipped_elements" | undefined {
|
||||
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";
|
||||
}
|
||||
|
||||
@@ -2578,10 +2605,13 @@ async function executeRenderPipeline(input: {
|
||||
// 900 floor stands, unchanged.
|
||||
const deShortBandMinFrames = envInt("HF_DE_SHORT_MIN_FRAMES", 250);
|
||||
const deShortBandMaxElements = envInt("HF_DE_SHORT_MAX_ELEMENTS", 2500);
|
||||
const compositionElementCount = await resolveCompositionElementCount(
|
||||
probeSession,
|
||||
compiled.html,
|
||||
);
|
||||
// `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
|
||||
@@ -2593,6 +2623,7 @@ async function executeRenderPipeline(input: {
|
||||
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
|
||||
@@ -2643,6 +2674,7 @@ async function executeRenderPipeline(input: {
|
||||
invertAtBandFloor,
|
||||
bandEnabled: deShortBandEnabled,
|
||||
bandOpen: deShortBandOpen,
|
||||
elementCountSource: compositionElementCountSource,
|
||||
});
|
||||
const deInversionEligible =
|
||||
deShortBandRoute && deShortBand === "applied" ? invertAtBandFloor : invertAtBaseFloor;
|
||||
@@ -2906,6 +2938,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,
|
||||
compositionElementCountSource,
|
||||
deShortBand,
|
||||
// Same rationale as the counters above: carried on live capture
|
||||
// observability, not only the success-path perfSummary, so a crash /
|
||||
@@ -3755,6 +3788,7 @@ async function executeRenderPipeline(input: {
|
||||
workerInversion: deWorkerInversion,
|
||||
preInversionWorkers: deWorkerInversion ? preRoutingWorkerCount : undefined,
|
||||
compositionElementCount,
|
||||
compositionElementCountSource,
|
||||
shortBand: deShortBand,
|
||||
parallelRouter: deParallelRouter,
|
||||
preRouterWorkers: deParallelRouter ? preRoutingWorkerCount : undefined,
|
||||
|
||||
Reference in New Issue
Block a user