From fb3e06f520a8ac4f4eedcdcdaff439b3b9ca7731 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Fri, 17 Jul 2026 03:22:33 +0000 Subject: [PATCH 1/2] fix(producer): credit held video tails in coverage gate --- .../render/videoFrameCoverage.test.ts | 41 +++++++++++++++++++ .../src/services/render/videoFrameCoverage.ts | 13 +++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/packages/producer/src/services/render/videoFrameCoverage.test.ts b/packages/producer/src/services/render/videoFrameCoverage.test.ts index d281eb61dc..c7db92d782 100644 --- a/packages/producer/src/services/render/videoFrameCoverage.test.ts +++ b/packages/producer/src/services/render/videoFrameCoverage.test.ts @@ -128,10 +128,51 @@ describe("computeVideoFrameCoverage", () => { // landed in framePaths (mid-extraction crash, partial cache read, …). const partial = makeExtracted("a", 5); partial.totalFrames = 30; + partial.metadata.durationSeconds = 1; const reports = computeVideoFrameCoverage(videos, [partial], 30); expect(reports[0]!.capturedFrames).toBe(5); expect(reports[0]!.ratio).toBeCloseTo(5 / 30, 5); }); + + it("credits a non-looping held tail against the source portion only", () => { + const videos = [makeVideo({ id: "held", start: 0, end: 10 })]; + const extracted = [makeExtracted("held", 90)]; + extracted[0]!.metadata.durationSeconds = 3; + const reports = computeVideoFrameCoverage(videos, extracted, 30); + expect(reports[0]).toMatchObject({ expectedFrames: 90, capturedFrames: 90, ratio: 1 }); + }); + + it("still requires the full authored slot for looping clips", () => { + const videos = [makeVideo({ id: "loop", start: 0, end: 10, loop: true })]; + const extracted = [makeExtracted("loop", 90)]; + extracted[0]!.metadata.durationSeconds = 3; + const reports = computeVideoFrameCoverage(videos, extracted, 30); + expect(reports[0]).toMatchObject({ expectedFrames: 300, capturedFrames: 90 }); + expect(reports[0]!.ratio).toBeCloseTo(0.3, 5); + }); + + it("still fails when extraction is truncated before the held-tail source", () => { + const videos = [makeVideo({ id: "truncated", start: 0, end: 10 })]; + const extracted = [makeExtracted("truncated", 60)]; + extracted[0]!.metadata.durationSeconds = 3; + const reports = computeVideoFrameCoverage(videos, extracted, 30); + expect(reports[0]).toMatchObject({ expectedFrames: 90, capturedFrames: 60 }); + expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError); + }); + + it("fails closed for invalid or exhausted source durations", () => { + const videos = [ + makeVideo({ id: "zero", start: 0, end: 10 }), + makeVideo({ id: "trimmed-away", start: 0, end: 10, mediaStart: 4 }), + ]; + const extracted = [makeExtracted("zero", 0), makeExtracted("trimmed-away", 30)]; + extracted[0]!.metadata.durationSeconds = 0; + extracted[1]!.metadata.durationSeconds = 3; + const reports = computeVideoFrameCoverage(videos, extracted, 30); + expect(reports[0]).toMatchObject({ expectedFrames: 300, capturedFrames: 0 }); + expect(reports[1]).toMatchObject({ expectedFrames: 300, capturedFrames: 30 }); + expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError); + }); }); describe("assertVideoFrameCoverage", () => { diff --git a/packages/producer/src/services/render/videoFrameCoverage.ts b/packages/producer/src/services/render/videoFrameCoverage.ts index ee41b52c0a..0d75a05474 100644 --- a/packages/producer/src/services/render/videoFrameCoverage.ts +++ b/packages/producer/src/services/render/videoFrameCoverage.ts @@ -140,7 +140,18 @@ export function computeVideoFrameCoverage( const reports: VideoFrameCoverageReport[] = []; for (const video of videos) { const entry = byId.get(video.id); - const expectedFrames = expectedFramesForClip(video.start, video.end, fps); + const slotFrames = expectedFramesForClip(video.start, video.end, fps); + // Non-looping clips intentionally hold their final decoded frame when the + // authored slot outlasts the source (#2516). Coverage must therefore + // measure the source portion, while still requiring the full slot for + // looping clips and for missing extractions (where no hold is possible). + const sourceDuration = entry ? entry.metadata.durationSeconds - video.mediaStart : NaN; + const hasUsableSourceDuration = Number.isFinite(sourceDuration) && sourceDuration > 0; + const sourceFrames = + entry && !video.loop && hasUsableSourceDuration + ? expectedFramesForClip(0, sourceDuration, fps) + : slotFrames; + const expectedFrames = entry && !video.loop ? Math.min(slotFrames, sourceFrames) : slotFrames; // framePaths is a Map — `size` is the number of distinct captured frames // delivered to the runtime injector, which is the load-bearing count // (some extractors report a total that includes cache-hit-skipped frames From 7382ef7a4fb7556ea78dd754f1ddfb806f4dc08d Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 17 Jul 2026 13:27:16 -0400 Subject: [PATCH 2/2] test(producer): decouple makeExtracted's durationSeconds default from delivered frame count Defaulting durationSeconds to delivered/fps made any test modeling a delivery shortfall silently report full coverage unless it remembered to override durationSeconds afterward. Default to Infinity instead so callers fall into the 'no usable source duration' branch (full slot required) unless they explicitly pass a duration. --- .../render/videoFrameCoverage.test.ts | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/packages/producer/src/services/render/videoFrameCoverage.test.ts b/packages/producer/src/services/render/videoFrameCoverage.test.ts index c7db92d782..7ba284e760 100644 --- a/packages/producer/src/services/render/videoFrameCoverage.test.ts +++ b/packages/producer/src/services/render/videoFrameCoverage.test.ts @@ -22,12 +22,27 @@ function makeVideo(overrides: Partial & { id: string }): VideoElem }; } -function makeExtracted(videoId: string, delivered: number, fps = 30): ExtractedFrames { +// `durationSeconds` defaults to Infinity, not `delivered / fps` — the two are +// unrelated in production (source duration comes from ffprobe; `delivered` +// is how many frames the extractor happened to deliver) and coupling them +// here made any test modeling a delivery shortfall silently report full +// coverage unless it remembered to override durationSeconds afterward. An +// unbounded default instead falls into computeVideoFrameCoverage's "no +// usable source duration" branch, which requires the full authored slot — +// the same behavior every test had before source-duration crediting +// existed. Tests that care about a specific source duration (the held-tail +// cases below) pass it explicitly. +function makeExtracted( + videoId: string, + delivered: number, + options: { fps?: number; durationSeconds?: number } = {}, +): ExtractedFrames { + const { fps = 30, durationSeconds = Number.POSITIVE_INFINITY } = options; const framePaths = new Map(); for (let i = 0; i < delivered; i += 1) framePaths.set(i, `/tmp/${videoId}/${i}.jpg`); const metadata: VideoMetadata = { - durationSeconds: delivered / fps, - videoStreamDurationSeconds: delivered / fps, + durationSeconds, + videoStreamDurationSeconds: durationSeconds, width: 1280, height: 720, fps, @@ -128,7 +143,6 @@ describe("computeVideoFrameCoverage", () => { // landed in framePaths (mid-extraction crash, partial cache read, …). const partial = makeExtracted("a", 5); partial.totalFrames = 30; - partial.metadata.durationSeconds = 1; const reports = computeVideoFrameCoverage(videos, [partial], 30); expect(reports[0]!.capturedFrames).toBe(5); expect(reports[0]!.ratio).toBeCloseTo(5 / 30, 5); @@ -136,16 +150,14 @@ describe("computeVideoFrameCoverage", () => { it("credits a non-looping held tail against the source portion only", () => { const videos = [makeVideo({ id: "held", start: 0, end: 10 })]; - const extracted = [makeExtracted("held", 90)]; - extracted[0]!.metadata.durationSeconds = 3; + const extracted = [makeExtracted("held", 90, { durationSeconds: 3 })]; const reports = computeVideoFrameCoverage(videos, extracted, 30); expect(reports[0]).toMatchObject({ expectedFrames: 90, capturedFrames: 90, ratio: 1 }); }); it("still requires the full authored slot for looping clips", () => { const videos = [makeVideo({ id: "loop", start: 0, end: 10, loop: true })]; - const extracted = [makeExtracted("loop", 90)]; - extracted[0]!.metadata.durationSeconds = 3; + const extracted = [makeExtracted("loop", 90, { durationSeconds: 3 })]; const reports = computeVideoFrameCoverage(videos, extracted, 30); expect(reports[0]).toMatchObject({ expectedFrames: 300, capturedFrames: 90 }); expect(reports[0]!.ratio).toBeCloseTo(0.3, 5); @@ -153,8 +165,7 @@ describe("computeVideoFrameCoverage", () => { it("still fails when extraction is truncated before the held-tail source", () => { const videos = [makeVideo({ id: "truncated", start: 0, end: 10 })]; - const extracted = [makeExtracted("truncated", 60)]; - extracted[0]!.metadata.durationSeconds = 3; + const extracted = [makeExtracted("truncated", 60, { durationSeconds: 3 })]; const reports = computeVideoFrameCoverage(videos, extracted, 30); expect(reports[0]).toMatchObject({ expectedFrames: 90, capturedFrames: 60 }); expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError); @@ -165,9 +176,10 @@ describe("computeVideoFrameCoverage", () => { makeVideo({ id: "zero", start: 0, end: 10 }), makeVideo({ id: "trimmed-away", start: 0, end: 10, mediaStart: 4 }), ]; - const extracted = [makeExtracted("zero", 0), makeExtracted("trimmed-away", 30)]; - extracted[0]!.metadata.durationSeconds = 0; - extracted[1]!.metadata.durationSeconds = 3; + const extracted = [ + makeExtracted("zero", 0, { durationSeconds: 0 }), + makeExtracted("trimmed-away", 30, { durationSeconds: 3 }), + ]; const reports = computeVideoFrameCoverage(videos, extracted, 30); expect(reports[0]).toMatchObject({ expectedFrames: 300, capturedFrames: 0 }); expect(reports[1]).toMatchObject({ expectedFrames: 300, capturedFrames: 30 });