From 8ba3c339156c9d96dd08407019b550191851e5e5 Mon Sep 17 00:00:00 2001
From: Vance Ingalls <vance@heygen.com>
Date: Wed, 8 Jul 2026 15:21:05 -0700
Subject: [PATCH] fix(engine,producer,cli): close review gaps in sub-timeline
 fail-fast
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address PR #2045 review feedback:
- Share a SubTimelineWaitOutcome type (engine) end-to-end instead of
  widening to string across CapturePerfSummary / RenderPerfSummary /
  telemetry, so the three layers can't drift.
- Dedupe scriptLoadFailures on push — a 4xx response and its trailing
  requestfailed both recorded the same URL, doubling the failed-URL
  list in the fail-fast warning.
- Thread the sub-timeline-wait outcome into render_error (not just
  render_complete): a render that fail-fasts and then fails downstream
  (pollVideosReady, extract, encode) previously dropped this signal on
  the floor. dedupPerfs is now function-scoped so the catch path can
  read it, same treatment as the existing captureAttempts array.
---
 packages/cli/src/telemetry/events.ts          |  6 ++--
 .../cli/src/telemetry/renderObservability.ts  |  9 ++++--
 packages/engine/src/index.ts                  |  1 +
 packages/engine/src/services/frameCapture.ts  | 19 ++++++++---
 packages/engine/src/types.ts                  | 11 +++++--
 .../producer/src/services/render/cleanup.ts   |  8 ++++-
 .../src/services/render/perfSummary.ts        | 32 ++++++++++++-------
 .../src/services/renderOrchestrator.ts        | 24 +++++++++-----
 8 files changed, 79 insertions(+), 31 deletions(-)

diff --git a/packages/cli/src/telemetry/events.ts b/packages/cli/src/telemetry/events.ts
index db4f30feb..b138778f1 100644
--- a/packages/cli/src/telemetry/events.ts
+++ b/packages/cli/src/telemetry/events.ts
@@ -1,8 +1,11 @@
 import { redactTelemetryString, type OutputResolutionIssueKind } from "@hyperframes/core";
+import type { SubTimelineWaitOutcome } from "@hyperframes/engine";
 import { trackEvent } from "./client.js";
 import { readConfig } from "./config.js";
 
 export interface RenderObservabilityTelemetryPayload {
+  /** Worst sub-composition timeline wait outcome across sessions. */
+  subTimelineWait?: SubTimelineWaitOutcome;
   observabilityRenderJobId?: string;
   observabilityCompositionHash?: string;
   observabilityEventCount?: number;
@@ -47,6 +50,7 @@ export interface RenderObservabilityTelemetryPayload {
 
 function renderObservabilityEventProperties(props: RenderObservabilityTelemetryPayload) {
   return {
+    sub_timeline_wait: props.subTimelineWait,
     observability_render_job_id: props.observabilityRenderJobId,
     observability_composition_hash: props.observabilityCompositionHash,
     observability_event_count: props.observabilityEventCount,
@@ -148,7 +152,6 @@ export function trackRenderComplete(
     captureAvgMs?: number;
     /** Warmup-robust per-frame capture median (basis for speedup estimates). */
     captureP50Ms?: number;
-    subTimelineWait?: string;
     /** <video> element count (speedup segmentation: injection comps read lower). */
     videoCount?: number;
     capturePeakMs?: number;
@@ -222,7 +225,6 @@ export function trackRenderComplete(
       speed_ratio: props.speedRatio,
       capture_avg_ms: props.captureAvgMs,
       capture_p50_ms: props.captureP50Ms,
-      sub_timeline_wait: props.subTimelineWait,
       video_count: props.videoCount,
       capture_peak_ms: props.capturePeakMs,
       peak_memory_mb: props.peakMemoryMb,
diff --git a/packages/cli/src/telemetry/renderObservability.ts b/packages/cli/src/telemetry/renderObservability.ts
index 41b96329d..748d112cd 100644
--- a/packages/cli/src/telemetry/renderObservability.ts
+++ b/packages/cli/src/telemetry/renderObservability.ts
@@ -58,7 +58,10 @@ export function renderObservabilityTelemetryPayload(
 export function renderJobObservabilityTelemetryPayload(
   job: RenderJob | undefined,
 ): RenderObservabilityTelemetryPayload {
-  return renderObservabilityTelemetryPayload(
-    job?.errorDetails?.observability ?? job?.perfSummary?.observability,
-  );
+  return {
+    ...renderObservabilityTelemetryPayload(
+      job?.errorDetails?.observability ?? job?.perfSummary?.observability,
+    ),
+    subTimelineWait: job?.errorDetails?.subTimelineWait ?? job?.perfSummary?.subTimelineWait,
+  };
 }
diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts
index 659843165..b174e8c22 100644
--- a/packages/engine/src/index.ts
+++ b/packages/engine/src/index.ts
@@ -40,6 +40,7 @@ export type {
   CaptureResult,
   CaptureBufferResult,
   CapturePerfSummary,
+  SubTimelineWaitOutcome,
 } from "./types.js";
 
 // ── Configuration ──────────────────────────────────────────────────────────────
diff --git a/packages/engine/src/services/frameCapture.ts b/packages/engine/src/services/frameCapture.ts
index 8b01313bb..72d1ec030 100644
--- a/packages/engine/src/services/frameCapture.ts
+++ b/packages/engine/src/services/frameCapture.ts
@@ -50,6 +50,7 @@ import type {
   CaptureResult,
   CaptureBufferResult,
   CapturePerfSummary,
+  SubTimelineWaitOutcome,
 } from "../types.js";
 
 export type { CaptureOptions, CaptureResult, CaptureBufferResult, CapturePerfSummary };
@@ -104,7 +105,7 @@ export interface CaptureSession {
    */
   scriptLoadFailures: string[];
   /** Outcome of the sub-composition timeline wait: ready | timeout | script_failure. */
-  subTimelineWaitOutcome?: "ready" | "timeout" | "script_failure";
+  subTimelineWaitOutcome?: SubTimelineWaitOutcome;
   initTelemetry?: {
     initDurationMs: number;
     tweenCount: number;
@@ -1159,7 +1160,7 @@ export async function pollSubCompositionTimelines(
   // is cut to `scriptFailureGraceMs` from its start.
   getScriptLoadFailures?: () => readonly string[],
   scriptFailureGraceMs: number = 2_000,
-): Promise<"ready" | "timeout" | "script_failure"> {
+): Promise<SubTimelineWaitOutcome> {
   // Hosts may opt out of the timeline wait with `data-no-timeline` —
   // compositions driven purely by CSS animations / rAF (the render-compat
   // contract) never register window.__timelines[id], and without the opt-out
@@ -1408,6 +1409,16 @@ async function waitForOptionalTailwindReady(page: Page, timeoutMs: number): Prom
   }
 }
 
+// A 4xx `response` and a `requestfailed` can both fire for the same script
+// (e.g. a `requestfailed` following the 4xx), and repeated <script> tags for
+// the same URL duplicate it further — dedupe so the fail-fast warning names
+// each failed URL once.
+function recordScriptLoadFailure(session: CaptureSession, url: string): void {
+  if (!session.scriptLoadFailures.includes(url)) {
+    session.scriptLoadFailures.push(url);
+  }
+}
+
 // fallow-ignore-next-line unit-size
 export async function initializeSession(session: CaptureSession): Promise<void> {
   const { page, serverUrl } = session;
@@ -1439,7 +1450,7 @@ export async function initializeSession(session: CaptureSession): Promise<void>
 
   page.on("requestfailed", (request) => {
     if (request.resourceType() === "script") {
-      session.scriptLoadFailures.push(request.url());
+      recordScriptLoadFailure(session, request.url());
     }
     appendBrowserDiagnostic(
       session,
@@ -1458,7 +1469,7 @@ export async function initializeSession(session: CaptureSession): Promise<void>
 
     const request = response.request();
     if (request.resourceType() === "script") {
-      session.scriptLoadFailures.push(response.url());
+      recordScriptLoadFailure(session, response.url());
     }
     appendBrowserDiagnostic(
       session,
diff --git a/packages/engine/src/types.ts b/packages/engine/src/types.ts
index 204ea9bb1..fc267821f 100644
--- a/packages/engine/src/types.ts
+++ b/packages/engine/src/types.ts
@@ -6,6 +6,13 @@
  */
 import type { Fps } from "@hyperframes/core";
 
+/**
+ * Outcome of waiting for a sub-composition's GSAP timelines to register.
+ * Threaded string-typed through `CapturePerfSummary` / `RenderPerfSummary` /
+ * telemetry so a single alias keeps the values in sync end-to-end.
+ */
+export type SubTimelineWaitOutcome = "ready" | "timeout" | "script_failure";
+
 // ── Seek Protocol ──────────────────────────────────────────────────────────────
 
 /**
@@ -183,8 +190,8 @@ export interface CapturePerfSummary {
    * averages). Basis for in-the-wild speedup estimates. 0 when no frames.
    */
   p50TotalMs: number;
-  /** Sub-composition timeline wait outcome: ready | timeout | script_failure (absent pre-init). */
-  subTimelineWaitOutcome?: string;
+  /** Sub-composition timeline wait outcome (absent pre-init). */
+  subTimelineWaitOutcome?: SubTimelineWaitOutcome;
   /**
    * Frames served from the static-dedup cache instead of a real seek+screenshot
    * (opt-out HF_STATIC_DEDUP=false). 0 when dedup was off or never armed. NOT counted
diff --git a/packages/producer/src/services/render/cleanup.ts b/packages/producer/src/services/render/cleanup.ts
index ce3f7aa67..9db8ac083 100644
--- a/packages/producer/src/services/render/cleanup.ts
+++ b/packages/producer/src/services/render/cleanup.ts
@@ -5,7 +5,11 @@
 
 import { rmSync } from "node:fs";
 import { freemem } from "node:os";
-import { type CaptureSession, closeCaptureSession } from "@hyperframes/engine";
+import {
+  type CaptureSession,
+  type SubTimelineWaitOutcome,
+  closeCaptureSession,
+} from "@hyperframes/engine";
 import type { FileServerHandle } from "../fileServer.js";
 import { defaultLogger, type ProducerLogger } from "../../logger.js";
 import type { HdrDiagnostics, RenderJob } from "../renderOrchestrator.js";
@@ -82,6 +86,7 @@ export function buildRenderErrorDetails(input: {
   perfStages: Record<string, number>;
   hdrDiagnostics: HdrDiagnostics;
   observability?: RenderObservabilitySummary;
+  subTimelineWait?: SubTimelineWaitOutcome;
 }): NonNullable<RenderJob["errorDetails"]> {
   const errorMessage = normalizeErrorMessage(input.error);
   const errorStack = input.error instanceof Error ? input.error.stack : undefined;
@@ -99,5 +104,6 @@ export function buildRenderErrorDetails(input: {
         ? { ...input.hdrDiagnostics }
         : undefined,
     observability: input.observability,
+    subTimelineWait: input.subTimelineWait,
   };
 }
diff --git a/packages/producer/src/services/render/perfSummary.ts b/packages/producer/src/services/render/perfSummary.ts
index 1791129a9..b449c4c4b 100644
--- a/packages/producer/src/services/render/perfSummary.ts
+++ b/packages/producer/src/services/render/perfSummary.ts
@@ -4,7 +4,7 @@
  */
 
 import { fpsToNumber } from "@hyperframes/core";
-import type { CapturePerfSummary } from "@hyperframes/engine";
+import type { CapturePerfSummary, SubTimelineWaitOutcome } from "@hyperframes/engine";
 import type { CaptureCalibrationSample, CaptureCostEstimate } from "./captureCost.js";
 import type {
   CaptureAttemptSummary,
@@ -15,6 +15,25 @@ import type {
 import { type HdrPerfCollector, finalizeHdrPerf } from "./hdrPerf.js";
 import type { RenderObservabilitySummary } from "./observability.js";
 
+/**
+ * Worst sub-composition timeline wait outcome across sessions: script_failure
+ * > timeout > ready. Shared by the success-path perf summary and the error
+ * path (a fail-fast can still be followed by an unrelated downstream
+ * failure, e.g. in `pollVideosReady` or encode — that render fires
+ * `render_error`, not `render_complete`, and should carry this too).
+ */
+export function worstSubTimelineWaitOutcome(
+  perfs: CapturePerfSummary[],
+): SubTimelineWaitOutcome | undefined {
+  const outcomes = perfs
+    .map((p) => p.subTimelineWaitOutcome)
+    .filter((o): o is SubTimelineWaitOutcome => !!o);
+  if (outcomes.length === 0) return undefined;
+  if (outcomes.includes("script_failure")) return "script_failure";
+  if (outcomes.includes("timeout")) return "timeout";
+  return "ready";
+}
+
 /**
  * Append each parallel worker's static-dedup perf into the render-level sink
  * (skipping workers that reported none). Shared by the disk + streaming parallel
@@ -189,16 +208,7 @@ export function buildRenderPerfSummary(input: {
               input.totalFrames,
           )
         : undefined,
-    subTimelineWait: (() => {
-      const outcomes = input.dedupPerfs
-        .map((p) => p.subTimelineWaitOutcome)
-        .filter((o): o is string => !!o);
-      if (outcomes.length === 0) return undefined;
-      // Worst outcome wins: script_failure > timeout > ready.
-      if (outcomes.includes("script_failure")) return "script_failure";
-      if (outcomes.includes("timeout")) return "timeout";
-      return "ready";
-    })(),
+    subTimelineWait: worstSubTimelineWaitOutcome(input.dedupPerfs),
     captureP50Ms: (() => {
       // Per-frame median from the engine's samples; when parallel workers
       // report separately, take the busiest session's median.
diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts
index deee72239..45f4dfebf 100644
--- a/packages/producer/src/services/renderOrchestrator.ts
+++ b/packages/producer/src/services/renderOrchestrator.ts
@@ -67,6 +67,7 @@ import {
   LOW_MEMORY_TOTAL_MB_THRESHOLD,
   assertConfiguredFfmpegBinariesExist,
   type CapturePerfSummary,
+  type SubTimelineWaitOutcome,
   resolveBrowserGpuMode,
   resolveHeadlessShellPath,
   scaleProtocolTimeoutForComposition,
@@ -90,7 +91,11 @@ import { buildRenderErrorDetails, cleanupRenderResources, safeCleanup } from "./
 import { normalizeErrorMessage } from "../utils/errorMessage.js";
 import { formatCaptureFrameName } from "../utils/paths.js";
 import { resolveEffectiveHdrMode } from "./render/hdrMode.js";
-import { buildRenderPerfSummary, pushWorkerDedupPerfs } from "./render/perfSummary.js";
+import {
+  buildRenderPerfSummary,
+  pushWorkerDedupPerfs,
+  worstSubTimelineWaitOutcome,
+} from "./render/perfSummary.js";
 import { getCaptureStageBrowserConsole } from "./render/captureStageError.js";
 import { resolveVideoCaptureBeyondViewport } from "./render/captureBeyondViewport.js";
 import {
@@ -322,8 +327,8 @@ export interface RenderPerfSummary {
    * captured the most frames when parallel workers report separately.
    */
   captureP50Ms?: number;
-  /** Worst sub-composition timeline wait outcome across sessions: ready | timeout | script_failure. */
-  subTimelineWait?: string;
+  /** Worst sub-composition timeline wait outcome across sessions. */
+  subTimelineWait?: SubTimelineWaitOutcome;
   capturePeakMs?: number;
   captureCalibration?: {
     sampledFrames: number[];
@@ -457,6 +462,8 @@ export interface RenderJob {
     perfStages?: Record<string, number>;
     hdrDiagnostics?: HdrDiagnostics;
     observability?: RenderObservabilitySummary;
+    /** Worst sub-composition timeline wait outcome across sessions captured before the failure. */
+    subTimelineWait?: SubTimelineWaitOutcome;
   };
 }
 
@@ -1189,6 +1196,11 @@ export async function executeRenderJob(
   // 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.
   const captureAttempts: CaptureAttemptSummary[] = [];
+  // Static-dedup perf, appended per sequential session / per parallel worker
+  // by the capture stage. Also function-scoped so the catch block can read
+  // the sub-timeline-wait outcome for a render that fails downstream of a
+  // fail-fast (aggregated into the success-path perf summary below too).
+  const dedupPerfs: CapturePerfSummary[] = [];
   const recordTransientRetryObservability = (): void => {
     const count = captureAttempts.filter((a) => a.reason === "transient-retry").length;
     if (count > 0) updateCaptureObservability({ transientRetries: count });
@@ -1824,11 +1836,6 @@ export async function executeRenderJob(
       }
     }
 
-    // `captureAttempts` is declared at function scope above (shared with the
-    // catch block). Static-dedup perf, appended per sequential session / per
-    // parallel worker by the capture stage, aggregated into the perf summary below.
-    const dedupPerfs: CapturePerfSummary[] = [];
-
     // png-sequence is "no container" — outputPath is treated as a directory and
     // the encode/mux/faststart stages are skipped entirely. The empty extension
     // keeps `videoOnlyPath` (which is constructed below) sensible even though
@@ -2434,6 +2441,7 @@ export async function executeRenderJob(
       perfStages,
       hdrDiagnostics,
       observability: observabilitySummary,
+      subTimelineWait: worstSubTimelineWaitOutcome(dedupPerfs),
     });
 
     log.info("[Render] Failure summary", {