From 4f53dd4f2cd607de17b2d2329746fa5da0cadeaa Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 21 Jul 2026 00:23:24 -0700 Subject: [PATCH] fix(engine): carry frameStride onto WorkerResult (fixes interleaved worker false-positive) --- .../src/services/parallelCoordinator.test.ts | 54 +++++++++++++++++++ .../src/services/parallelCoordinator.ts | 39 ++++++++++---- 2 files changed, 83 insertions(+), 10 deletions(-) diff --git a/packages/engine/src/services/parallelCoordinator.test.ts b/packages/engine/src/services/parallelCoordinator.test.ts index dbe35da2f..91d294bad 100644 --- a/packages/engine/src/services/parallelCoordinator.test.ts +++ b/packages/engine/src/services/parallelCoordinator.test.ts @@ -3,12 +3,14 @@ import { calculateOptimalWorkers, distributeFrames, expectedFramesForTask, + flagSilentWorkerExits, formatWorkerFailure, selectWorkerDiagnostics, shouldDisableBrowserPoolForParallelWorker, shouldVerifyWorkerGpu, synthesizeSilentWorkerExitError, resolveParallelDeVerifySamples, + type WorkerResult, } from "./parallelCoordinator.js"; import type { EngineConfig } from "../config.js"; @@ -250,6 +252,58 @@ describe("synthesizeSilentWorkerExitError", () => { }); }); +describe("flagSilentWorkerExits", () => { + it("does not flag a fully-successful interleaved worker (regression: PRINFRA-300)", () => { + // 3-way interleave over 150 frames: worker 0 captures 0, 3, ..., 147 → 50 + // frames, which is its FULL expected count at stride=3. Without + // `frameStride` on the result, expectedFramesForTask would fall back to + // stride=1 (150) and false-positive this as a silent death. + const results: WorkerResult[] = [ + { + workerId: 0, + framesCaptured: 50, + startFrame: 0, + endFrame: 150, + frameStride: 3, + durationMs: 1000, + }, + ]; + flagSilentWorkerExits(results); + expect(results[0]?.error).toBeUndefined(); + }); + + it("still flags a genuinely under-captured interleaved worker", () => { + const results: WorkerResult[] = [ + { + workerId: 1, + framesCaptured: 20, // expected 50 at stride=3 + startFrame: 1, + endFrame: 150, + frameStride: 3, + durationMs: 1000, + }, + ]; + flagSilentWorkerExits(results); + expect(results[0]?.error).toContain("worker 1 exited without terminal error string"); + expect(results[0]?.error).toContain("expected=50"); + }); + + it("does not overwrite an existing error", () => { + const results: WorkerResult[] = [ + { + workerId: 2, + framesCaptured: 0, + startFrame: 0, + endFrame: 10, + durationMs: 1000, + error: "Protocol error (Page.captureScreenshot): Target closed", + }, + ]; + flagSilentWorkerExits(results); + expect(results[0]?.error).toBe("Protocol error (Page.captureScreenshot): Target closed"); + }); +}); + describe("shouldVerifyWorkerGpu", () => { const softwareConfig: Partial = { browserGpuMode: "software" }; diff --git a/packages/engine/src/services/parallelCoordinator.ts b/packages/engine/src/services/parallelCoordinator.ts index 50e26e51a..b52308886 100644 --- a/packages/engine/src/services/parallelCoordinator.ts +++ b/packages/engine/src/services/parallelCoordinator.ts @@ -63,6 +63,14 @@ export interface WorkerResult { framesCaptured: number; startFrame: number; endFrame: number; + /** + * Mirrors the originating `WorkerTask.frameStride` (default 1). Required by + * `expectedFramesForTask` — without it, every interleaved (stride > 1) + * worker's expected count is computed as the full contiguous range instead + * of range/stride, so a fully-successful worker looks like it under-captured + * and gets misclassified as a silent death (see `synthesizeSilentWorkerExitError`). + */ + frameStride?: number; durationMs: number; perf?: CapturePerfSummary; error?: string; @@ -191,6 +199,24 @@ export function synthesizeSilentWorkerExitError( ); } +/** + * A worker may return without an error string yet with `framesCaptured` + * below its task's expected count — the silent-exit shape field signal + * ts=1784042064 called out. Synthesize a terminal error string in-place so + * the caller's failure filter treats it as a failure (and so the caller's + * failure message actually names what went wrong). Requires each result to + * carry `frameStride` (see `WorkerResult`) — without it, an interleaved + * worker's true `framesCaptured` (range/stride) is compared against the full + * contiguous range and every successful interleaved worker false-positives. + */ +export function flagSilentWorkerExits(results: WorkerResult[]): void { + for (const r of results) { + if (!r.error && r.framesCaptured < expectedFramesForTask(r)) { + r.error = synthesizeSilentWorkerExitError(r, expectedFramesForTask(r)); + } + } +} + export function formatWorkerFailure(result: WorkerResult): string { const errorText = result.error && result.error.length > 0 @@ -487,6 +513,7 @@ async function executeWorkerTask( framesCaptured, startFrame: task.startFrame, endFrame: task.endFrame, + frameStride: task.frameStride, durationMs: Date.now() - startTime, perf, }; @@ -509,6 +536,7 @@ async function executeWorkerTask( framesCaptured, startFrame: task.startFrame, endFrame: task.endFrame, + frameStride: task.frameStride, durationMs: Date.now() - startTime, perf, error: failure.message, @@ -613,16 +641,7 @@ export async function executeParallelCapture( ), ); - // A worker may return without an error string yet with framesCaptured - // below the task's expected count — that's the silent-exit shape field - // signal ts=1784042064 called out. Synthesize a terminal error string - // in-place so the filter below treats it as a failure (and so the - // caller's failure message actually names what went wrong). - for (const r of results) { - if (!r.error && r.framesCaptured < expectedFramesForTask(r)) { - r.error = synthesizeSilentWorkerExitError(r, expectedFramesForTask(r)); - } - } + flagSilentWorkerExits(results); const errors = results.filter((r) => r.failure || r.error); if (errors.length > 0) {