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

packages/engine/src/config.test.ts

@@ -4,6 +4,7 @@ import { tmpdir } from "node:os";
 import {
   resolveConfig,
   resolveDefaultDrawElement,
+  explainDrawElementDisabled,
   DEFAULT_CONFIG,
   scaleProtocolTimeoutForComposition,
   shouldClampToScreenshotForConcreteGpu,
@@ -233,6 +234,85 @@ describe("resolveConfig", () => {
     });
   });
 
+  // Every resolveDefaultDrawElement branch returns a bare `false`, so a render
+  // that never became a DE candidate reached telemetry with no reason at all
+  // and landed in the dashboard's `other` bucket. These pin that each silent
+  // refusal now has a name, and that the names stay in the same ORDER as the
+  // resolver's branches — if the two drift, the reason is a plausible lie,
+  // which is worse than no reason.
+  describe("explainDrawElementDisabled (names the silent refusals)", () => {
+    const base = { browserGpuMode: "hardware" as const, workerEncode: true };
+
+    it("names each refusal", () => {
+      expect(explainDrawElementDisabled({ ...base, platform: "linux" })).toBe(
+        "unsupported_platform",
+      );
+      expect(
+        explainDrawElementDisabled({ ...base, platform: "darwin", browserGpuMode: "software" }),
+      ).toBe("software_gpu");
+      expect(explainDrawElementDisabled({ ...base, platform: "win32", workerEncode: false })).toBe(
+        "worker_encode_off",
+      );
+    });
+
+    // The Windows case this shipped for: hardware GPU, supported platform,
+    // worker-encode on — nothing environmental explains it, so it was an
+    // explicit opt-out. Must NOT masquerade as one of the other three.
+    // The caller-side path: resolveDefaultDrawElement never even runs when the
+    // feature is off at a higher config level, so the orchestrator seeds from
+    // the environment alone and must land on `disabled` rather than inventing
+    // an environmental cause.
+    it("reports `disabled` for a config-time refusal on a healthy host", () => {
+      expect(
+        resolveDefaultDrawElement({ ...base, useDrawElement: false, platform: "darwin" }),
+      ).toBe(false);
+      expect(explainDrawElementDisabled({ ...base, platform: "darwin" })).toBe("disabled");
+    });
+
+    it("falls back to `disabled` when nothing environmental explains it", () => {
+      expect(explainDrawElementDisabled({ ...base, platform: "win32" })).toBe("disabled");
+      expect(explainDrawElementDisabled({ ...base, platform: "darwin" })).toBe("disabled");
+    });
+
+    // Platform is checked BEFORE gpu mode, matching the resolver. A linux
+    // software host is reported as unsupported_platform, not software_gpu:
+    // fixing the GPU would not help.
+    it("orders platform ahead of gpu mode, like the resolver", () => {
+      expect(
+        explainDrawElementDisabled({
+          platform: "linux",
+          browserGpuMode: "software",
+          workerEncode: false,
+        }),
+      ).toBe("unsupported_platform");
+    });
+
+    // The contract that keeps the two functions honest: whenever the resolver
+    // says false, the explainer must produce a reason, and whenever it says
+    // true the caller must not ask.
+    it("covers every input where the resolver refuses", () => {
+      const platforms: NodeJS.Platform[] = ["darwin", "win32", "linux"];
+      const gpuModes = ["hardware", "software", "auto"] as const;
+      for (const platform of platforms) {
+        for (const browserGpuMode of gpuModes) {
+          for (const workerEncode of [true, false]) {
+            const on = resolveDefaultDrawElement({
+              useDrawElement: true,
+              explicitOptIn: false,
+              platform,
+              browserGpuMode,
+              workerEncode,
+            });
+            if (on) continue;
+            expect(explainDrawElementDisabled({ platform, browserGpuMode, workerEncode })).not.toBe(
+              "disabled",
+            );
+          }
+        }
+      }
+    });
+  });
+
   describe("resolveDefaultDrawElement (pure host clamp)", () => {
     const base = {
       useDrawElement: true,

packages/engine/src/config.ts

@@ -741,6 +741,39 @@ export function resolveDefaultDrawElement(args: {
   return args.workerEncode;
 }
 
+/**
+ * Why {@link resolveDefaultDrawElement} said no. Call ONLY when the resolved
+ * `useDrawElement` is false — the branches mirror that resolver's, in order.
+ *
+ * Every branch there returns a bare `false` and records nothing, so a render
+ * that never became a drawElement candidate reaches telemetry with no
+ * `de_compile_gate`, no `de_clamp_reason` and no `de_gate_reason`. Those land
+ * in the "Why not drawElement" dashboard's catch-all `other` bucket, which
+ * measured 56,507 renders over 14 days — its second-largest bar, explaining
+ * nothing. The orchestrator's own clamp only fires while `useDrawElement` is
+ * still true, so it cannot cover a config-time refusal by construction.
+ *
+ * Takes only the environmental inputs on purpose: the caller holds the
+ * POST-resolution `useDrawElement`, from which the pre-resolution request is
+ * no longer recoverable. So "none of these three explain it" is itself the
+ * answer — the feature was switched off explicitly.
+ *
+ * Kept separate from the resolver rather than widening its return type: it sits
+ * on the config hot path and several call sites want a plain boolean. Mirror
+ * any branch change in both.
+ */
+export function explainDrawElementDisabled(args: {
+  platform: NodeJS.Platform;
+  browserGpuMode: EngineConfig["browserGpuMode"];
+  workerEncode: boolean;
+}): "unsupported_platform" | "software_gpu" | "worker_encode_off" | "disabled" {
+  // Platform first: on an unsupported host the GPU mode is beside the point.
+  if (!isDrawElementPlatform(args.platform)) return "unsupported_platform";
+  if (args.browserGpuMode === "software") return "software_gpu";
+  if (!args.workerEncode) return "worker_encode_off";
+  return "disabled";
+}
+
 export function resolveConfig(overrides?: Partial<EngineConfig>): EngineConfig {
   const env = (key: string): string | undefined => process.env[key];
   const envNum = (key: string, fallback: number): number => {

packages/engine/src/index.ts

@@ -53,6 +53,7 @@ export {
   scaleProtocolTimeoutForComposition,
   shouldClampToScreenshotForConcreteGpu,
   applyConcreteGpuScreenshotClamp,
+  explainDrawElementDisabled,
   resolveExtractCacheDir,
   defaultExtractCacheDir,
   EXTRACT_CACHE_DIR_DISABLED_ALIASES,

packages/producer/src/services/renderOrchestrator.test.ts

@@ -44,6 +44,8 @@ import {
   shouldPreferSingleWorkerDrawElement,
   shouldStreamParallelCapture,
   shouldUseStreamingEncode,
+  resolveObservedCaptureMode,
+  createCaptureObservabilityUpdater,
 } from "./renderOrchestrator.js";
 import { probeRequiresBrowser } from "./render/stages/probeStage.js";
 import { ensureFrameWritten } from "./render/stages/captureHdrFrameShared.js";
@@ -2660,3 +2662,71 @@ describe("closeOrphanedProbeForRetry (probe cleanup before verify-triggered retr
     expect((log.warn.mock.calls[0][1] as { error: string }).error).toBe("string-only rejection");
   });
 });
+
+// BeginFrame is Linux-only in both real entry points, but the observability
+// field derived its mode from `forceScreenshot` alone. That mislabelled 30,625
+// Windows renders as `beginframe` over 14 days — a fifth of the fast-capture
+// dashboard's capture-mode data — for captures that were really screenshot.
+describe("resolveObservedCaptureMode", () => {
+  it("only ever reports beginframe on linux", () => {
+    expect(resolveObservedCaptureMode(false, "linux")).toBe("beginframe");
+    expect(resolveObservedCaptureMode(false, "win32")).toBe("screenshot");
+    expect(resolveObservedCaptureMode(false, "darwin")).toBe("screenshot");
+  });
+
+  // The case the old code got right, kept so a future simplification back to
+  // a bare boolean fails here rather than in production telemetry.
+  it("reports screenshot whenever screenshot was forced, linux included", () => {
+    expect(resolveObservedCaptureMode(true, "linux")).toBe("screenshot");
+    expect(resolveObservedCaptureMode(true, "win32")).toBe("screenshot");
+  });
+});
+
+// The blocker found in review: seeding `captureMode` at construction is not
+// enough. `updateCaptureObservability` fires at 23 sites, and the post-compile
+// `{ forceScreenshot }` patch runs on EVERY render — the old closure re-derived
+// from `forceScreenshot` alone and put `beginframe` back before capture began,
+// so both telemetry emits read the reverted value. These go through the closure
+// rather than the helper, which is the only way to catch that.
+describe("createCaptureObservabilityUpdater", () => {
+  const seed = (platform: NodeJS.Platform, forceScreenshot: boolean) => {
+    const observability = {
+      forceScreenshot,
+      captureMode: resolveObservedCaptureMode(forceScreenshot, platform),
+      browserGpuMode: "hardware" as const,
+    } as Parameters<typeof createCaptureObservabilityUpdater>[0];
+    return { observability, update: createCaptureObservabilityUpdater(observability, platform) };
+  };
+
+  // The exact production case: Windows, hardware GPU, screenshot not forced.
+  it("keeps screenshot on win32 across an unrelated patch", () => {
+    const { observability, update } = seed("win32", false);
+    expect(observability.captureMode).toBe("screenshot");
+    update({ transientRetries: 1 });
+    expect(observability.captureMode).toBe("screenshot");
+  });
+
+  // The unconditional post-compile patch — the one that guaranteed the revert.
+  it("keeps screenshot on win32 when forceScreenshot is re-patched false", () => {
+    const { observability, update } = seed("win32", false);
+    update({ forceScreenshot: false });
+    expect(observability.captureMode).toBe("screenshot");
+  });
+
+  // Linux must still be able to report beginframe, or the fix would have
+  // silently disabled the field everywhere instead of correcting it.
+  it("still reports beginframe on linux, and honours a later force", () => {
+    const { observability, update } = seed("linux", false);
+    expect(observability.captureMode).toBe("beginframe");
+    update({ transientRetries: 2 });
+    expect(observability.captureMode).toBe("beginframe");
+    update({ forceScreenshot: true });
+    expect(observability.captureMode).toBe("screenshot");
+  });
+
+  it("applies the patch itself, not only the derived mode", () => {
+    const { observability, update } = seed("win32", false);
+    update({ workerCount: 4 });
+    expect(observability.workerCount).toBe(4);
+  });
+});

packages/producer/src/services/renderOrchestrator.ts

@@ -80,6 +80,7 @@ import {
   resolveBrowserGpuMode,
   resolveHeadlessShellPath,
   applyConcreteGpuScreenshotClamp,
+  explainDrawElementDisabled,
   scaleProtocolTimeoutForComposition,
   classifyCaptureFailure,
   cloneCaptureWarning,
@@ -807,6 +808,72 @@ export function buildMissingFrameRetryBatches(
   return batches;
 }
 
+/**
+ * The capture mode this render will REPORT, pre-capture.
+ *
+ * BeginFrame is Linux-only. Both real entry points enforce that —
+ * `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. Every non-Linux render that did not force screenshot
+ * therefore reported `beginframe` for a capture that was really screenshot:
+ * 30,625 Windows renders over 14 days, a fifth of the fast-capture dashboard's
+ * capture-mode data.
+ *
+ * `config.ts` documents this same 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).
+ *
+ * NECESSARY, NOT SUFFICIENT — read this before trusting the value on Linux.
+ * The platform test is the only condition modelled here. Linux BeginFrame
+ * additionally requires a headless-shell binary, no supersampling, no
+ * transparent drawElement route (`frameCapture.ts` preMode) and the
+ * `--enable-begin-frame-control` flag (`browserManager.ts`). Any of those can
+ * make the ACTUAL mode screenshot while this still reports `beginframe`, so a
+ * Linux `beginframe` reading is an upper bound, not a fact. The authoritative
+ * value is the session's own `launchCaptureMode` — the same field the runtime
+ * video gate already falls back to. Deriving this field from the resolved
+ * session instead of from config is the real fix and is deliberately NOT in
+ * this change: it closes the Windows mislabel, which is platform-only and
+ * needs no session plumbing.
+ *
+ * Pure; exported for tests.
+ */
+export function resolveObservedCaptureMode(
+  forceScreenshot: boolean,
+  platform: NodeJS.Platform = process.platform,
+): "screenshot" | "beginframe" {
+  return forceScreenshot || platform !== "linux" ? "screenshot" : "beginframe";
+}
+
+/**
+ * Build the observability patcher, re-deriving `captureMode` on every patch.
+ *
+ * Extracted and exported because the previous inline closure was where the
+ * Windows mislabel actually lived. Seeding `captureMode` correctly at
+ * construction is NOT sufficient: this updater is invoked at 23 sites through
+ * the pipeline, and one of them —
+ * `updateCaptureObservability({ forceScreenshot: captureForceScreenshot })`
+ * straight after compile — runs unconditionally on every render. The old body
+ * re-derived from `forceScreenshot` alone, so the seeded value was overwritten
+ * with `beginframe` again before capture began, and both the success and error
+ * telemetry emits read the reverted object. A helper-only test cannot catch
+ * that: it never round-trips through this closure. Hence the export.
+ */
+export function createCaptureObservabilityUpdater(
+  observability: RenderCaptureObservability,
+  platform: NodeJS.Platform = process.platform,
+): (patch: Partial<RenderCaptureObservability>) => void {
+  return (patch: Partial<RenderCaptureObservability>): void => {
+    Object.assign(observability, patch);
+    observability.captureMode = resolveObservedCaptureMode(
+      Boolean(observability.forceScreenshot),
+      platform,
+    );
+  };
+}
+
 export function getNextRetryWorkerCount(currentWorkers: number): number {
   return Math.max(1, Math.floor(currentWorkers / 2));
 }
@@ -2017,20 +2084,15 @@ async function executeRenderPipeline(input: {
   const chunkedEncodeSize = cfg.chunkSizeFrames;
   const captureObservability: RenderCaptureObservability = {
     forceScreenshot: Boolean(cfg.forceScreenshot),
-    captureMode: cfg.forceScreenshot ? "screenshot" : "beginframe",
+    captureMode: resolveObservedCaptureMode(Boolean(cfg.forceScreenshot)),
     browserGpuMode: cfg.browserGpuMode,
     protocolTimeoutMs: cfg.protocolTimeout,
     pageNavigationTimeoutMs: cfg.pageNavigationTimeout,
     playerReadyTimeoutMs: cfg.playerReadyTimeout,
   };
   let extractionObservability: RenderExtractionObservability | undefined;
   let compositionHash: string | undefined;
-  const updateCaptureObservability = (patch: Partial<RenderCaptureObservability>): void => {
-    Object.assign(captureObservability, patch);
-    captureObservability.captureMode = captureObservability.forceScreenshot
-      ? "screenshot"
-      : "beginframe";
-  };
+  const updateCaptureObservability = createCaptureObservabilityUpdater(captureObservability);
   // Function-scoped (not inside the try) so both the success path AND the catch
   // can read it — the catch records transient-retry burn on renders that still
   // failed, which is the more actionable signal for tuning the retry cap.
@@ -2187,7 +2249,21 @@ async function executeRenderPipeline(input: {
     // drawElement release telemetry: why default DE disengaged (if it did),
     // whether self-verify fell back, and the drain-side counters.
     const deCompileGate = compileResult.deCompileGate;
-    let deClampReason: string | undefined;
+    // Seed with the CONFIG-TIME refusal, if there was one. The clamp further
+    // down only runs `if (cfg.useDrawElement && ...)`, so a render that never
+    // became a drawElement candidate at all could never acquire a reason —
+    // it reached telemetry with every DE field empty and landed in the
+    // dashboard's `other` bucket (56,507 renders / 14d, second-largest bar on
+    // "Why not drawElement", explaining nothing). Re-derived from the same
+    // inputs `resolveConfig` used, so it cannot disagree with the decision.
+    // Later clamps overwrite this: a more specific reason always wins.
+    let deClampReason: string | undefined = cfg.useDrawElement
+      ? undefined
+      : explainDrawElementDisabled({
+          platform: process.platform,
+          browserGpuMode: cfg.browserGpuMode,
+          workerEncode: cfg.enableDrawElementWorkerEncode,
+        });
     // "inverted" = fired and held; "reverted" = fired but the self-verify
     // retry rolled back to the parallel path; undefined = never fired.
     let deWorkerInversion: "inverted" | "reverted" | undefined;
@@ -3052,8 +3128,9 @@ async function executeRenderPipeline(input: {
       // Which mode will stream: the engine picks beginframe only on Linux with
       // headless-shell and no forced screenshot (frameCapture.ts preMode);
       // everything else is screenshot. Recorded for telemetry cohorting.
-      const captureParallelStream =
-        process.platform === "linux" && !captureForceScreenshot ? "beginframe" : "screenshot";
+      // Same predicate as the observability field — use the one helper so the
+      // two cannot drift if the router's modes ever change.
+      const captureParallelStream = resolveObservedCaptureMode(captureForceScreenshot);
       log.info(
         `[Render] Parallel ${captureParallelStream} capture will stream to the encoder ` +
           `(interleaved, ${workerCount} workers) instead of the disk path. ` +