fix(producer): unbias the static element count and stop zeroing failures

Review findings on #2891. Two of them bite directly on this PR's own
purpose — making the fleet element-count distribution readable — so they
are fixed rather than noted.

countElementTags counted `</` + letter anywhere, including inside inline
JS. A compiled comp containing `const h = "</div>"` or a template literal
building `</span>` inflated the count once per occurrence. Compiled comps
embed large inline scripts, so the bias is systematic, not noise, and it
lands entirely on the ~83% of renders with no probe session — precisely
the cohort this PR exists to characterize. Script and style bodies are
now stripped before matching; losing their own closing tags costs 1-2
counts against a threshold in the thousands.

The new elementCount fell back to 0 when its page.evaluate threw,
following the tweenCount pattern beside it. For this field that pattern
is wrong: evaluate failures concentrate on the huge-DOM compositions the
field is meant to observe, and a 0 there is indistinguishable from a
legitimately empty comp, so the fleet p50/p99 would absorb both silently.
It is now undefined on failure, the INIT console line omits the token
entirely rather than emitting a zero, and the parser reports absent —
mirroring the live/static provenance split the routing resolver already
uses.

Also documented: the "every render reaches this path" claim holds only
for renders that survive to end of init, so the tail is survivor-biased
and should be read as a lower bound; and the two element-count fields now
say plainly which is which — composition_element_count gates routing,
observability_init_element_count is the observational counterpart — so
the follow-up analysis can't query the wrong one.

Nits: envInt is integer-only per its name, both live-DOM reads use
getElementsByTagName (live collection length, no NodeList materialized on
the 40k-node tail), and the attribution block notes that it runs with
routing off by design.

Fault injection confirms the new tests bite: disabling script stripping
fails 4, and the zero-vs-undefined case is pinned separately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-07-29 23:51:14 -07:00
co-authored by Claude Opus 5
parent d74afc7b7d
commit c61a24b510
6 changed files with 122 additions and 27 deletions
+29 -10
View File
@@ -119,8 +119,8 @@ export interface CaptureSession {
initTelemetry?: {
initDurationMs: number;
tweenCount: number;
/** Live DOM element count at end of init — observational; see collectSessionInitTelemetry. */
elementCount: number;
/** Live DOM element count at end of init; undefined when the measurement itself failed. Observational see collectSessionInitTelemetry. */
elementCount?: number;
};
capturePerf: {
frames: number;
@@ -408,7 +408,7 @@ function appendBrowserDiagnostic(session: CaptureSession, text: string): void {
async function collectSessionInitTelemetry(
page: Page,
initStart: number,
): Promise<{ initDurationMs: number; tweenCount: number; elementCount: number }> {
): Promise<{ initDurationMs: number; tweenCount: number; elementCount?: number }> {
const initDurationMs = Date.now() - initStart;
// Live DOM size, measured once the init sequence has completed so
// script-generated elements are present. This is the SAME quantity the
@@ -417,14 +417,30 @@ async function collectSessionInitTelemetry(
// Its job is coverage: the routing gate can only read a live count on the
// ~17% of renders that get a probe session, which leaves the fleet
// element-count distribution unknowable for the rest (and hides exactly
// the dangerous shape — small source markup, huge runtime DOM). Every
// render reaches this path, so the distribution becomes readable even
// where the gate stays blind.
let elementCount = 0;
// the dangerous shape — small source markup, huge runtime DOM).
//
// Coverage caveat, since the "every render reaches this" claim is easy to
// over-read: every render that SURVIVES TO END OF INIT reaches it. Renders
// that die in browser launch, navigation, or OOM before init completes
// never emit — and on the huge-DOM tail this field exists to characterize,
// those are disproportionately the ones that fail early. The distribution
// is therefore survivor-biased toward smaller comps; treat the tail as a
// lower bound (review finding).
// Deliberately `undefined` (not 0) when the measurement fails, breaking
// from the tweenCount fallback pattern directly above. A page.evaluate
// crash during init is most likely on exactly the huge-DOM compositions
// this field exists to observe, and a 0 there is indistinguishable from a
// legitimately empty comp — the fleet p50/p99 would silently absorb both
// (review finding). Mirrors the live/static provenance split the routing
// resolver already uses: "not measured" must not read as "measured small".
// getElementsByTagName over querySelectorAll: a live HTMLCollection's
// length avoids materializing a 40k-node static NodeList on the tail this
// is here to characterize.
let elementCount: number | undefined;
try {
elementCount = await page.evaluate(() => document.querySelectorAll("*").length);
elementCount = await page.evaluate(() => document.getElementsByTagName("*").length);
} catch {
elementCount = 0;
elementCount = undefined;
}
let tweenCount = 0;
try {
@@ -460,7 +476,10 @@ async function recordSessionInitTelemetry(
session.initTelemetry = telemetry;
appendBrowserDiagnostic(
session,
`[FrameCapture:INIT] complete initDurationMs=${telemetry.initDurationMs} tweenCount=${telemetry.tweenCount} elementCount=${telemetry.elementCount}`,
`[FrameCapture:INIT] complete initDurationMs=${telemetry.initDurationMs} tweenCount=${telemetry.tweenCount}` +
// Omitted rather than zeroed when unmeasured, so the parser reports
// absent instead of inventing an empty DOM.
(telemetry.elementCount === undefined ? "" : ` elementCount=${telemetry.elementCount}`),
);
}
+17 -5
View File
@@ -254,11 +254,23 @@ export interface CapturePerfSummary {
/** GSAP tween count at init — the motion-axis signal for capture routing analysis. */
initTweenCount?: number;
/**
* Live DOM element count at end of init. Observational counterpart to the
* short-comp routing gate's own count: the gate can only measure the ~17%
* of renders that get a probe session, so without this the fleet
* element-count distribution — and any large-runtime-DOM tail — stays
* invisible for the rest.
* Live DOM element count at end of init; undefined when the measurement
* failed (never 0 — see collectSessionInitTelemetry).
*
* WHICH FIELD TO QUERY — two element counts exist and they answer
* different questions:
* • `composition_element_count` (+ `_source`) is the ROUTING-RELEVANT
* one. Measured from the PROBE session before the routing decision,
* falling back to a static source scan. That is what the short-comp
* band actually gates on.
* • `observability_init_element_count` (this field) is the
* OBSERVATIONAL counterpart. Measured from the capture session's own
* DOM at end of init, on every surviving render — including the ~83%
* with no probe, where the routing signal is a blind static scan.
* They agree for most comps and diverge for one that mutates its DOM
* between probe launch and capture init. Use this for distribution and
* tail questions; use `composition_element_count` for anything about what
* the router did (review finding).
*/
initElementCount?: number;
/** Correctness warnings observed before or during capture. */
@@ -435,6 +435,19 @@ describe("init observability fallback (parallel workers)", () => {
// The single-session path has no structured fallback — it parses the console
// line only. This is the path that covers renders the routing gate cannot
// measure, so the element count must survive it.
// The collector omits `elementCount=` entirely when the page.evaluate that
// measures it failed, rather than emitting 0 — a 0 there is
// indistinguishable from a legitimately empty comp, and evaluate failures
// concentrate on exactly the huge-DOM tail this field exists to observe.
it("leaves elementCount absent when the INIT line omits it (measurement failed)", () => {
const summary = makeRecorder().summary({
lastBrowserConsole: ["[FrameCapture:INIT] complete initDurationMs=90 tweenCount=7"],
capture: { forceScreenshot: false, captureMode: "screenshot" },
});
expect(summary.init).toEqual({ initDurationMs: 90, tweenCount: 7, elementCount: undefined });
expect(summary.init?.elementCount).not.toBe(0);
});
it("parses elementCount from the console INIT line with no fallback at all", () => {
const summary = makeRecorder().summary({
lastBrowserConsole: [
@@ -183,12 +183,18 @@ export interface RenderInitObservability {
initDurationMs?: number;
tweenCount?: number;
/**
* Live DOM element count at end of capture-session init. Observational:
* measured after routing has already been decided, so it cannot gate — it
* exists because the routing gate's own count is only available on the
* ~17% of renders that get a probe session, leaving the fleet
* element-count distribution (and any large-runtime-DOM tail) unreadable
* for the rest.
* Live DOM element count at end of capture-session init; undefined when
* the measurement failed (never 0). Observational: measured after routing
* has already been decided, so it cannot gate — it exists because the
* routing gate's own count is only available on the ~17% of renders that
* get a probe session, leaving the fleet element-count distribution (and
* any large-runtime-DOM tail) unreadable for the rest.
*
* Not interchangeable with `RenderCaptureObservability.compositionElementCount`:
* that one is measured pre-routing from the probe session (or a static
* scan) and is what the band gates on. This one is measured post-routing
* from the capture session and covers renders the gate cannot see. Query
* the former for router behaviour, this for distribution/tail analysis.
*/
elementCount?: number;
}
@@ -1994,12 +1994,38 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => {
});
it("does not false-positive on inline-script comparisons or void-prefixed words", () => {
// Only the </script> closer counts: "<breadth" and "<imgWidth" hit the
// br/img alternatives but fail the \b word boundary (next char is a
// word char), and bare "a < b" comparisons match nothing.
// Script bodies are stripped wholesale (with their own closing tag), so
// nothing inside can match — including "<breadth" / "<imgWidth", which
// would anyway fail the \b word boundary.
expect(countElementTags("<script>if (a < b && x <breadth && y <imgWidth) {}</script>")).toBe(
0,
);
});
// Review finding: the `</[a-zA-Z]` alternation matches ANY "</" + letter,
// including inside JS strings and template literals. Compiled comps embed
// large inline scripts, so this bias is systematic — and it lands entirely
// on the ~83% of renders with no probe, for which this scan is the only
// element signal.
it("does not count closing tags written inside inline script strings", () => {
expect(countElementTags('<div></div><script>const h = "</div></div></div>";</script>')).toBe(
1,
);
expect(
countElementTags("<p></p><script>const t = words.map(w => `</span>`).join('');</script>"),
).toBe(1);
});
it("strips <style> bodies too — CSS content strings can carry the same shapes", () => {
expect(countElementTags('<div></div><style>a::after{content:"</div>"}</style>')).toBe(1);
});
it("strips multiple and attributed script blocks, not just the first", () => {
expect(
countElementTags(
'<div></div><script type="module">"</span>"</script><script>"</span>"</script>',
),
).toBe(1);
});
it("is stable on empty and malformed input rather than throwing", () => {
@@ -1243,7 +1243,10 @@ 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;
// Integer-only, per the name: a fractional threshold would compare
// sensibly against integer counts but silently means something the knob
// never promised, so treat it as a typo and fall back (review nit).
return Number.isInteger(parsed) ? parsed : fallback;
}
/**
@@ -1286,7 +1289,16 @@ export function envInt(name: string, fallback: number): number {
* live count and uses this only when no such session exists.
*/
export function countElementTags(html: string): number {
const matches = html.match(
// Strip inline <script>/<style> bodies BEFORE matching. Every alternation
// below can fire on ordinary JS text — `const html = "</div>"` or a
// template literal building `</span>` inflates the count once per
// occurrence — and compiled comps embed large inline scripts. That bias is
// systematic, not noise, and it lands entirely on the ~83% of renders with
// no probe session, for which this scan is the only element signal (review
// finding). Removing the bodies also drops their own closing tags, which
// costs 1-2 counts against a threshold in the thousands.
const markup = html.replace(/<(script|style)\b[^>]*>[\s\S]*?<\/\1>/gi, "");
const matches = markup.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;
@@ -1321,7 +1333,9 @@ export async function resolveCompositionElementCount(
if (probeSession?.isInitialized) {
try {
const liveCount = await probeSession.page.evaluate(
() => document.querySelectorAll("*").length,
// Live HTMLCollection length — avoids materializing a static NodeList
// on the large-DOM comps this gate exists to catch (review nit).
() => document.getElementsByTagName("*").length,
);
if (typeof liveCount === "number" && Number.isFinite(liveCount)) {
return { count: liveCount, source: "live" };
@@ -2688,6 +2702,11 @@ async function executeRenderPipeline(input: {
...deInversionArgs,
minFrames: Math.min(deSingleMinFrames, deShortBandMinFrames),
});
// Attribution runs even when routing is OFF — that is the whole point of
// the baseline release: "applied" is the counterfactual "would have
// inverted", and emitting it now is what establishes the DiD cohort
// before the flip. Do not short-circuit this block behind
// `deShortBandRoute` (review nit).
const deShortBand = resolveDeShortBand({
invertAtBaseFloor,
invertAtBandFloor,