Skip to content

fix(producer,engine): stop mislabelling capture mode, and name the silent drawElement refusals - #3151

Open
vanceingalls wants to merge 1 commit into
mainfrom
fix/de-observability-capture-mode-and-clamp-reason
Open

fix(producer,engine): stop mislabelling capture mode, and name the silent drawElement refusals#3151
vanceingalls wants to merge 1 commit into
mainfrom
fix/de-observability-capture-mode-and-clamp-reason

Conversation

@vanceingalls

Copy link
Copy Markdown
Collaborator

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.

…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant