mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-11 23:00:03 +00:00
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:
co-authored by
Claude Opus 5
parent
d74afc7b7d
commit
c61a24b510
@@ -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}`),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user