From 5f37fbe716cb13c78377ba14312d560b0e89ab2d Mon Sep 17 00:00:00 2001 From: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com> Date: Sat, 30 May 2026 01:58:49 -0300 Subject: [PATCH] fix(producer): drop empty trailing chunk slice in distributed render plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveChunkPlan caps chunkCount at maxParallelChunks from the naive count, then rounds effectiveChunkSize up to ceil(totalFrames / chunkCount). When that ceil rounds up, the first (chunkCount - 1) chunks can already cover every frame, so buildChunkSlices emits a final slice with startFrame >= totalFrames — an empty [n, n) or inverted range. renderChunk rejects it (framesInChunk <= 0) and, under Step Functions retries, fails the whole distributed render even though [0, totalFrames) is fully covered. This is reachable from the user-facing CLI: `hyperframes lambda render --chunk-size 10 --max-parallel-chunks 12` on a ~4s/30fps (121-frame) composition yields chunkCount=12, effectiveChunkSize=11, and a 12th slice of [121, 121). Tighten chunkCount to ceil(totalFrames / effectiveChunkSize) after the size is finalized, so the union stays exactly [0, totalFrames) with no empty tail. This only lowers chunkCount in the explicit-small-chunkSize case; the auto-sized and large-chunkSize paths already satisfy ceil(totalFrames / effectiveChunkSize) >= chunkCount, so it's a no-op there (existing tests' chunkCount values are unchanged). Adds a regression test for the 121/10/12 case plus a grid property test asserting contiguous, non-empty, exact coverage across explicit sizes. --- .../src/services/distributed/plan.test.ts | 37 +++++++++++++++++++ .../producer/src/services/distributed/plan.ts | 13 ++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/packages/producer/src/services/distributed/plan.test.ts b/packages/producer/src/services/distributed/plan.test.ts index 1f05048b4..42e333d5d 100644 --- a/packages/producer/src/services/distributed/plan.test.ts +++ b/packages/producer/src/services/distributed/plan.test.ts @@ -125,6 +125,43 @@ describe("resolveChunkPlan", () => { expect(result.chunkCount).toBe(5); expect(result.effectiveChunkSize).toBe(MIN_CHUNK_SIZE); }); + + it("tightens chunkCount so an explicit small chunkSize leaves no empty trailing slice", () => { + // 121 frames, chunkSize=10, maxParallelChunks=12: ceil(121/10)=13 naive → + // capped at 12, but effectiveChunkSize rounds up to ceil(121/12)=11. Eleven + // 11-frame chunks already cover [0,121), so a 12th would be the empty slice + // [121,121) that renderChunk rejects (framesInChunk <= 0). chunkCount must + // tighten to 11. + const result = resolveChunkPlan(121, 10, 12); + expect(result.effectiveChunkSize).toBe(11); + expect(result.chunkCount).toBe(11); + const slices = buildChunkSlices(121, result.chunkCount, result.effectiveChunkSize); + expect(slices).toHaveLength(11); + expect(slices[slices.length - 1]).toEqual({ index: 10, startFrame: 110, endFrame: 121 }); + }); + + it("never emits an empty or inverted slice across a grid of explicit chunk sizes", () => { + for (const totalFrames of [37, 50, 121, 333, 1000, 4001]) { + for (const maxParallel of [2, 4, 12, 16, 40]) { + for (const chunkSize of [10, 11, 15, 24, 100]) { + const { chunkCount, effectiveChunkSize } = resolveChunkPlan( + totalFrames, + chunkSize, + maxParallel, + ); + expect(chunkCount).toBeLessThanOrEqual(maxParallel); + const slices = buildChunkSlices(totalFrames, chunkCount, effectiveChunkSize); + let cursor = 0; + for (const s of slices) { + expect(s.startFrame).toBe(cursor); // contiguous from 0 + expect(s.endFrame).toBeGreaterThan(s.startFrame); // non-empty + cursor = s.endFrame; + } + expect(cursor).toBe(totalFrames); // union is exactly [0, totalFrames) + } + } + } + }); }); describe("buildChunkSlices", () => { diff --git a/packages/producer/src/services/distributed/plan.ts b/packages/producer/src/services/distributed/plan.ts index a5a1c0d0e..02e0a8519 100644 --- a/packages/producer/src/services/distributed/plan.ts +++ b/packages/producer/src/services/distributed/plan.ts @@ -400,6 +400,7 @@ export function measurePlanDirBytes(planDir: string): number { * resolvedChunkSize = configChunkSize ?? max(MIN_CHUNK_SIZE, ceil(totalFrames / maxParallelChunks)) * chunkCount = min(maxParallelChunks, ceil(totalFrames / resolvedChunkSize)) * effectiveChunkSize = max(resolvedChunkSize, ceil(totalFrames / chunkCount)) + * chunkCount = min(chunkCount, ceil(totalFrames / effectiveChunkSize)) // drop empty trailing slice * * Long renders auto-rescale to fewer-but-longer chunks rather than * fragmenting infinitely. Returned `chunkCount >= 1` (`totalFrames === 0` @@ -434,7 +435,17 @@ export function resolveChunkPlan( const naiveCount = Math.ceil(totalFrames / resolvedChunkSize); const chunkCount = Math.min(maxParallelChunks, Math.max(1, naiveCount)); const effectiveChunkSize = Math.max(resolvedChunkSize, Math.ceil(totalFrames / chunkCount)); - return { chunkCount, effectiveChunkSize }; + // Rounding effectiveChunkSize up can let the first (chunkCount - 1) chunks + // already cover every frame, leaving an empty/inverted trailing slice that + // buildChunkSlices would still emit and renderChunk would then reject + // (framesInChunk <= 0), failing the whole distributed render. Tighten + // chunkCount to the number of chunks effectiveChunkSize actually needs so the + // union stays exactly [0, totalFrames) with no empty tail. This only ever + // lowers chunkCount in the explicit-small-chunkSize case; the auto-sized and + // large-chunkSize paths already satisfy ceil(totalFrames / effectiveChunkSize) + // >= chunkCount, so it's a no-op there. + const tightChunkCount = Math.min(chunkCount, Math.ceil(totalFrames / effectiveChunkSize)); + return { chunkCount: tightChunkCount, effectiveChunkSize }; } function assertPositiveInteger(name: string, value: number): void {