fix(engine): preserve source frame identity above 99,999 (#3503)

* fix(engine): preserve extracted frame identity

* fix(producer): order legacy distributed frames numerically

Author: miguel-heygen

packages/engine/src/services/extractedFrameIndex.test.ts

@@ -0,0 +1,81 @@
+import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { basename, join } from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
+
+import {
+  ExtractedFrameSequenceError,
+  extractedFrameIndex,
+  framePathsFromDirectory,
+} from "./extractedFrameIndex.js";
+
+const roots: string[] = [];
+
+function frameDir(): string {
+  const root = mkdtempSync(join(tmpdir(), "hf-extracted-frame-index-"));
+  roots.push(root);
+  return root;
+}
+
+function seed(root: string, ...files: string[]): void {
+  for (const file of files) writeFileSync(join(root, file), file);
+}
+
+afterEach(() => {
+  for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
+});
+
+describe("extractedFrameIndex", () => {
+  it("derives frame identity across the five-to-six-digit boundary", () => {
+    expect(extractedFrameIndex("frame_99999.jpg", "jpg")).toBe(99_998);
+    expect(extractedFrameIndex("frame_100000.jpg", "jpg")).toBe(99_999);
+  });
+
+  it("refuses malformed, zero, and wrong-format frame candidates", () => {
+    expect(() => extractedFrameIndex("frame_bad.jpg", "jpg")).toThrow(ExtractedFrameSequenceError);
+    expect(() => extractedFrameIndex("frame_00000.jpg", "jpg")).toThrow(
+      ExtractedFrameSequenceError,
+    );
+    expect(() => extractedFrameIndex("frame_00001.png", "jpg")).toThrow(
+      ExtractedFrameSequenceError,
+    );
+  });
+});
+
+describe("framePathsFromDirectory", () => {
+  it("maps by the filename ordinal instead of directory or lexical position", () => {
+    const root = frameDir();
+    seed(
+      root,
+      ...Array.from({ length: 10 }, (_, index) => `frame_${index + 1}.jpg`).reverse(),
+      "notes.txt",
+    );
+
+    const paths = framePathsFromDirectory(root, "jpg");
+
+    expect(paths.size).toBe(10);
+    expect(basename(paths.get(8)!)).toBe("frame_9.jpg");
+    expect(basename(paths.get(9)!)).toBe("frame_10.jpg");
+  });
+
+  it("fails loudly when two filenames claim the same numeric frame", () => {
+    const root = frameDir();
+    seed(root, "frame_1.jpg", "frame_00001.jpg");
+
+    expect(() => framePathsFromDirectory(root, "jpg")).toThrow(/duplicate.*frame index 0/i);
+  });
+
+  it("fails loudly instead of shifting later frames across a gap", () => {
+    const root = frameDir();
+    seed(root, "frame_00001.jpg", "frame_00003.jpg");
+
+    expect(() => framePathsFromDirectory(root, "jpg")).toThrow(/missing.*frame index 1/i);
+  });
+
+  it("fails on frame-prefixed malformed candidates but ignores unrelated files", () => {
+    const root = frameDir();
+    seed(root, "frame_00001.jpg", "frame_bad.jpg", "notes.txt");
+
+    expect(() => framePathsFromDirectory(root, "jpg")).toThrow(/invalid.*frame filename/i);
+  });
+});

packages/engine/src/services/extractedFrameIndex.ts

@@ -0,0 +1,55 @@
+import { readdirSync } from "node:fs";
+import { join } from "node:path";
+
+export type ExtractedFrameFormat = "jpg" | "png";
+
+export const FRAME_FILENAME_PREFIX = "frame_";
+
+export class ExtractedFrameSequenceError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = "ExtractedFrameSequenceError";
+  }
+}
+
+export function extractedFrameIndex(file: string, format: ExtractedFrameFormat): number {
+  const match = new RegExp(`^${FRAME_FILENAME_PREFIX}(\\d+)\\.${format}$`).exec(file);
+  if (!match) {
+    throw new ExtractedFrameSequenceError(`Invalid extracted frame filename: ${file}`);
+  }
+  const ordinal = Number.parseInt(match[1]!, 10);
+  if (!Number.isSafeInteger(ordinal) || ordinal < 1) {
+    throw new ExtractedFrameSequenceError(`Invalid extracted frame ordinal: ${file}`);
+  }
+  return ordinal - 1;
+}
+
+export function framePathsFromDirectory(
+  outputDir: string,
+  format: ExtractedFrameFormat,
+): Map<number, string> {
+  const suffix = `.${format}`;
+  const indexed = new Map<number, string>();
+  for (const file of readdirSync(outputDir)) {
+    if (!file.startsWith(FRAME_FILENAME_PREFIX) || !file.endsWith(suffix)) continue;
+    const index = extractedFrameIndex(file, format);
+    if (indexed.has(index)) {
+      throw new ExtractedFrameSequenceError(
+        `Duplicate extracted frame index ${index}: ${indexed.get(index)} and ${file}`,
+      );
+    }
+    indexed.set(index, join(outputDir, file));
+  }
+
+  const ordered = new Map<number, string>();
+  for (let index = 0; index < indexed.size; index += 1) {
+    const path = indexed.get(index);
+    if (!path) {
+      throw new ExtractedFrameSequenceError(
+        `Missing extracted frame index ${index} in ${outputDir}`,
+      );
+    }
+    ordered.set(index, path);
+  }
+  return ordered;
+}

packages/engine/src/services/extractionCache.test.ts

@@ -25,6 +25,7 @@ import {
   partialCacheEntryDir,
   publishCacheEntry,
   readKeyStat,
+  rehydrateCacheEntry,
   type CacheKeyInput,
 } from "./extractionCache.js";
 
@@ -76,6 +77,42 @@ describe("extractionCache constants", () => {
   });
 });
 
+describe("rehydrateCacheEntry frame identity", () => {
+  it("fails loudly when a complete cache has a frame-number gap", () => {
+    const { tmpRoot } = makeCacheRoot();
+    try {
+      writeFileSync(join(tmpRoot, "frame_00001.jpg"), "one");
+      writeFileSync(join(tmpRoot, "frame_00003.jpg"), "three");
+
+      expect(() =>
+        rehydrateCacheEntry(
+          { dir: tmpRoot, keyHash: "a".repeat(64) },
+          {
+            videoId: "video-gap",
+            srcPath: "/video.mp4",
+            fps: 30,
+            format: "jpg",
+            metadata: {
+              durationSeconds: 1,
+              videoStreamDurationSeconds: 1,
+              width: 1920,
+              height: 1080,
+              fps: 30,
+              videoCodec: "h264",
+              hasAudio: false,
+              isVFR: false,
+              hasAlpha: false,
+              colorSpace: null,
+            },
+          },
+        ),
+      ).toThrow(/missing.*frame index 1/i);
+    } finally {
+      removeCacheRoot(tmpRoot);
+    }
+  });
+});
+
 describe("computeCacheKey", () => {
   let tmpRoot: string;
   let sourceFile: string;

packages/engine/src/services/extractionCache.ts

@@ -46,9 +46,10 @@ import {
 } from "node:fs";
 import { join } from "node:path";
 import type { VideoMetadata } from "../utils/ffprobe.js";
+import { FRAME_FILENAME_PREFIX, framePathsFromDirectory } from "./extractedFrameIndex.js";
 
 /** Filename prefix for extracted frames. Shared with the extractor. */
-export const FRAME_FILENAME_PREFIX = "frame_";
+export { FRAME_FILENAME_PREFIX } from "./extractedFrameIndex.js";
 
 /** Sentinel filename written after a cache entry is fully populated. */
 export const COMPLETE_SENTINEL = ".hf-complete";
@@ -508,14 +509,7 @@ export function rehydrateCacheEntry(
   options: RehydrateOptions,
 ): RehydratedFrames {
   const framePattern = `${FRAME_FILENAME_PREFIX}%05d.${options.format}`;
-  const framePaths = new Map<number, string>();
-  const suffix = `.${options.format}`;
-  const files = readdirSync(entry.dir)
-    .filter((f) => f.startsWith(FRAME_FILENAME_PREFIX) && f.endsWith(suffix))
-    .sort();
-  files.forEach((file, idx) => {
-    framePaths.set(idx, join(entry.dir, file));
-  });
+  const framePaths = framePathsFromDirectory(entry.dir, options.format);
   return {
     videoId: options.videoId,
     srcPath: options.srcPath,

packages/engine/src/services/videoFrameExtractor.ts

@@ -6,7 +6,7 @@
  * Videos are replaced with <img> elements during capture.
  */
 
-import { copyFileSync, existsSync, linkSync, mkdirSync, readdirSync, rmSync } from "fs";
+import { copyFileSync, existsSync, linkSync, mkdirSync, rmSync } from "fs";
 import { isAbsolute, join, posix, resolve, sep } from "path";
 import { parseHTML } from "linkedom";
 import {
@@ -55,6 +55,7 @@ import {
   type CacheEntry,
   type CacheFrameFormat,
 } from "./extractionCache.js";
+import { framePathsFromDirectory } from "./extractedFrameIndex.js";
 
 export interface VideoElement {
   id: string;
@@ -802,13 +803,7 @@ export async function extractVideoFramesRange(
     );
   }
 
-  const framePaths = new Map<number, string>();
-  const files = readdirSync(videoOutputDir)
-    .filter((f) => f.startsWith(FRAME_FILENAME_PREFIX) && f.endsWith(`.${format}`))
-    .sort();
-  files.forEach((file, index) => {
-    framePaths.set(index, join(videoOutputDir, file));
-  });
+  const framePaths = framePathsFromDirectory(videoOutputDir, format);
   if (framePaths.size === 0 && duration > 0) {
     throw new VideoSourceExtractionError(
       "zero_output",
@@ -1180,24 +1175,14 @@ type SupersetGroupPlan = {
   members: SupersetMemberPlan[];
 };
 
-function extractedFrameFileNames(outputDir: string, format: CacheFrameFormat): string[] {
-  const suffix = `.${format}`;
-  return readdirSync(outputDir)
-    .filter((file) => file.startsWith(FRAME_FILENAME_PREFIX) && file.endsWith(suffix))
-    .sort();
-}
-
 function extractedFramesFromDirectory(
   work: PreparedExtraction,
   outputDir: string,
   srcPath: string,
   fps: number,
 ): ExtractedFrames {
   const framePattern = `${FRAME_FILENAME_PREFIX}%05d.${work.format}`;
-  const framePaths = new Map<number, string>();
-  extractedFrameFileNames(outputDir, work.format).forEach((file, index) => {
-    framePaths.set(index, join(outputDir, file));
-  });
+  const framePaths = framePathsFromDirectory(outputDir, work.format);
   return {
     videoId: work.video.id,
     srcPath,

packages/producer/src/services/distributed/rebuildExtractedFrames.test.ts

@@ -178,6 +178,34 @@ describe("rebuildExtractedFramesFromPlanDir", () => {
     }
   });
 
+  it("orders mixed-width dense-v1 filenames by numeric ordinal", () => {
+    const planDir = mkdtempSync(join(tmpdir(), "hf-rebuild-frames-v1-mixed-width-"));
+    try {
+      const frameNames = Array.from({ length: 10 }, (_, index) => `frame_${index + 1}.jpg`);
+      makeFramesDir(planDir, "vid-v1-mixed-width", frameNames.toReversed());
+
+      const [extracted] = rebuildExtractedFramesFromPlanDir(planDir, [
+        {
+          videoId: "vid-v1-mixed-width",
+          srcPath: "/v1-mixed-width.mp4",
+          framePattern: "frame_%05d.jpg",
+          fps: 30,
+          totalFrames: frameNames.length,
+          metadata: VIDEO_METADATA_STUB,
+        },
+      ]);
+
+      expect(extracted!.framePaths.get(8)).toBe(
+        join(planDir, "video-frames", "vid-v1-mixed-width", "frame_9.jpg"),
+      );
+      expect(extracted!.framePaths.get(9)).toBe(
+        join(planDir, "video-frames", "vid-v1-mixed-width", "frame_10.jpg"),
+      );
+    } finally {
+      rmSync(planDir, { recursive: true, force: true });
+    }
+  });
+
   it("preserves original indexes for a sparse v2 chunk materialization", () => {
     const planDir = mkdtempSync(join(tmpdir(), "hf-rebuild-frames-sparse-"));
     try {

packages/producer/src/services/distributed/renderChunk.ts

@@ -324,6 +324,13 @@ export async function beginFrameSessionNeedsScreenshotFallback(
   return !(await probe(session.page, timeoutMs, probeTick, session.beginFrameIntervalMs));
 }
 
+function frameNumberFromFileName(name: string): number | null {
+  const match = /(\d+)(?=\.[^.]+$)/.exec(name);
+  if (!match) return null;
+  const frameNumber = Number(match[1]);
+  return Number.isSafeInteger(frameNumber) ? frameNumber : null;
+}
+
 /**
  * Rebuild the engine's in-memory `ExtractedFrames[]` from the on-disk
  * planDir layout. `<planDir>/video-frames/<videoId>/` holds the numbered
@@ -355,22 +362,27 @@ export function rebuildExtractedFramesFromPlanDir(
       );
     }
     // framePattern looks like `frame_%05d.jpg`; sprintf isn't available at
-    // runtime so list-and-sort the directory. Sorted-by-name matches
-    // sorted-by-frame-index because the extractor writes zero-padded
-    // monotonic indices.
+    // runtime so list the directory and order numeric names by their ordinal.
+    // Width changes once FFmpeg passes the padding minimum, so lexical order
+    // would interleave frame_100000 before frame_10001.
     const ext = (extname(v.framePattern) || ".jpg").toLowerCase();
     const frames = readdirSync(outputDir)
       .filter((name) => name.toLowerCase().endsWith(ext))
-      .sort();
+      .sort((left, right) => {
+        const leftNumber = frameNumberFromFileName(left);
+        const rightNumber = frameNumberFromFileName(right);
+        if (leftNumber === null || rightNumber === null) return left.localeCompare(right);
+        return leftNumber - rightNumber || left.localeCompare(right);
+      });
     const framePaths = new Map<number, string>();
     for (let i = 0; i < frames.length; i++) {
       const frameName = frames[i];
       if (!frameName) continue;
       // V1 plans preserve the historical sorted-position behavior even for
       // unusual zero-based filenames. V2 materialization is sparse, so only
       // that mode derives the original index from ffmpeg's 1-based filename.
-      const numbered = indexMode === "sparse-v2" ? /(\d+)(?=\.[^.]+$)/.exec(frameName) : null;
-      const frameIndex = numbered ? Number(numbered[1]) - 1 : i;
+      const frameNumber = indexMode === "sparse-v2" ? frameNumberFromFileName(frameName) : null;
+      const frameIndex = frameNumber === null ? i : frameNumber - 1;
       framePaths.set(frameIndex, join(outputDir, frameName));
     }
     result.push({