fix(producer,engine): stop mislabelling capture mode, and name the silent drawElement refusals - #3151
Open
vanceingalls wants to merge 1 commit into
Open
Conversation
…lent drawElement refusals Two observability defects found while auditing the fast-capture dashboard. Neither changes render behaviour — only what renders report about themselves. ## 1. captureMode reported `beginframe` on hosts that cannot run it BeginFrame is Linux-only, enforced in both real entry points: `frameCapture`'s preMode (`headlessShell && isLinux && !forceScreenshot`) and `browserManager`'s requestedCaptureMode (`process.platform === "linux"`). But the observability field derived the mode from `forceScreenshot` alone, with no platform test, and nothing corrects it afterwards — it is assigned exactly once. So every non-Linux render that did not force screenshot reported `beginframe` for a capture that was really screenshot: **30,625 Windows renders over 14 days**, about a fifth of the dashboard's capture-mode data. `config.ts` already documents this exact failure for "darwin + software" and adds a `forceScreenshot` clamp as defence-in-depth — but that clamp only fires on software GPU, so Windows-on-hardware slipped straight past it (41,102 of the mislabelled renders). Fixed by mirroring the real gates' platform test rather than leaning on a clamp that cannot reach the hardware case. Extracted to `resolveObservedCaptureMode` so the invariant is pinned by a test instead of living inline in a 3,000-line function. `distributed/plan.ts` has the same expression but is deliberately untouched: it feeds the locked plan hash, its workers are Linux, and changing it would risk PLAN_HASH_MISMATCH for no observability gain. ## 2. Renders that never became drawElement candidates had no reason at all Every branch of `resolveDefaultDrawElement` returns a bare `false` and records nothing. The orchestrator's clamp only runs `if (cfg.useDrawElement && ...)`, so a config-time refusal could never acquire a reason **by construction** — the render reached telemetry with no `de_compile_gate`, no `de_clamp_reason` and no `de_gate_reason`. Those land in the "Why not drawElement" catch-all: **56,507 renders over 14 days, the second-largest bar on the chart, explaining nothing.** Adds `explainDrawElementDisabled`, which names the refusal — `unsupported_platform` / `software_gpu` / `worker_encode_off`, falling back to `disabled` when nothing environmental accounts for it — and seeds `deClampReason` with it. Later clamps still overwrite: a more specific reason wins. It takes only the environmental inputs deliberately. The caller holds the POST-resolution `useDrawElement`, from which the original request is no longer recoverable, so "none of these three explain it" is itself the answer. ## Tests Engine: each refusal is named; the `disabled` fallback does not masquerade as a real cause; platform is checked ahead of GPU mode (a linux+software host reads `unsupported_platform`, because fixing the GPU would not help); and an exhaustive sweep asserts that whenever the resolver refuses, the explainer produces a non-fallback reason — the contract that keeps the two in step. Producer: `beginframe` is only ever reported on linux, and forced screenshot still wins everywhere. engine 1480 passing, producer 579 passing. oxlint and oxfmt clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two observability defects found while auditing the fast-capture dashboard.
Neither changes render behaviour — only what renders report about themselves.
1. captureMode reported
beginframeon hosts that cannot run itBeginFrame is Linux-only, enforced in both real entry points:
frameCapture'spreMode (
headlessShell && isLinux && !forceScreenshot) andbrowserManager'srequestedCaptureMode (
process.platform === "linux"). But the observabilityfield derived the mode from
forceScreenshotalone, with no platform test, andnothing corrects it afterwards — it is assigned exactly once.
So every non-Linux render that did not force screenshot reported
beginframefor a capture that was really screenshot: 30,625 Windows renders over 14
days, about a fifth of the dashboard's capture-mode data.
config.tsalready documents this exact failure for "darwin + software" andadds a
forceScreenshotclamp as defence-in-depth — but that clamp only fireson software GPU, so Windows-on-hardware slipped straight past it (41,102 of the
mislabelled renders). Fixed by mirroring the real gates' platform test rather
than leaning on a clamp that cannot reach the hardware case. Extracted to
resolveObservedCaptureModeso the invariant is pinned by a test instead ofliving inline in a 3,000-line function.
distributed/plan.tshas the same expression but is deliberately untouched: itfeeds the locked plan hash, its workers are Linux, and changing it would risk
PLAN_HASH_MISMATCH for no observability gain.
2. Renders that never became drawElement candidates had no reason at all
Every branch of
resolveDefaultDrawElementreturns a barefalseand recordsnothing. The orchestrator's clamp only runs
if (cfg.useDrawElement && ...),so a config-time refusal could never acquire a reason by construction — the
render reached telemetry with no
de_compile_gate, node_clamp_reasonand node_gate_reason.Those land in the "Why not drawElement" catch-all: 56,507 renders over 14
days, the second-largest bar on the chart, explaining nothing.
Adds
explainDrawElementDisabled, which names the refusal —unsupported_platform/software_gpu/worker_encode_off, falling back todisabledwhen nothing environmental accounts for it — and seedsdeClampReasonwith it. Later clamps still overwrite: a more specific reasonwins.
It takes only the environmental inputs deliberately. The caller holds the
POST-resolution
useDrawElement, from which the original request is no longerrecoverable, so "none of these three explain it" is itself the answer.
Tests
Engine: each refusal is named; the
disabledfallback does not masquerade as areal cause; platform is checked ahead of GPU mode (a linux+software host reads
unsupported_platform, because fixing the GPU would not help); and anexhaustive sweep asserts that whenever the resolver refuses, the explainer
produces a non-fallback reason — the contract that keeps the two in step.
Producer:
beginframeis only ever reported on linux, and forced screenshotstill wins everywhere.
engine 1480 passing, producer 579 passing. oxlint and oxfmt clean.