mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
refactor(producer): tighten resolveChunkPlan assertion + trim comments
Address self-review findings: - assertPositiveInteger now only runs on the caller-supplied path so the error message names `configChunkSize` only when the caller actually passed one. Previously, the assertion fired against `resolvedChunkSize` on both paths and would have lied about the offending input. - Drop the call-site comment that narrated the diff/history; the function docstring already covers the contract. - Drop the internal-track name and date from the MIN_CHUNK_SIZE rationale and the test block header. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -96,12 +96,9 @@ describe("resolveChunkPlan", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// ── Auto-size when configChunkSize is undefined ───────────────────────
|
// ── Auto-size when configChunkSize is undefined ───────────────────────
|
||||||
// Pre-fix, `plan()` defaulted `chunkSize` to 240 on a `?? DEFAULT_CHUNK_SIZE`
|
// The auto-sizer picks `max(MIN_CHUNK_SIZE, ceil(totalFrames /
|
||||||
// line, so a 660-frame composition with `maxParallelChunks=16` ended up at
|
// maxParallelChunks))` whenever the caller leaves `chunkSize` undefined,
|
||||||
// 3 chunks (ceil(660/240)) regardless of the caller's fan-out intent.
|
// honoring `maxParallelChunks` instead of clamping at a 240-frame default.
|
||||||
// Surfaced by the lever-1 chunk-scaling benchmark on 2026-05-17. The
|
|
||||||
// auto-sizer now picks `max(MIN_CHUNK_SIZE, ceil(totalFrames /
|
|
||||||
// maxParallelChunks))` whenever the caller leaves `chunkSize` undefined.
|
|
||||||
|
|
||||||
it("explicit chunkSize wins: 660 frames + chunkSize=240 + maxParallelChunks=16 → 3 chunks", () => {
|
it("explicit chunkSize wins: 660 frames + chunkSize=240 + maxParallelChunks=16 → 3 chunks", () => {
|
||||||
// Regression guard for the "explicit number still works" half of the
|
// Regression guard for the "explicit number still works" half of the
|
||||||
|
|||||||
@@ -198,9 +198,8 @@ export const DEFAULT_MAX_PARALLEL_CHUNKS = 16;
|
|||||||
/**
|
/**
|
||||||
* Floor for the auto-sized `chunkSize` when the caller leaves it
|
* Floor for the auto-sized `chunkSize` when the caller leaves it
|
||||||
* `undefined`. Anything smaller hits a per-chunk fixed-overhead wall
|
* `undefined`. Anything smaller hits a per-chunk fixed-overhead wall
|
||||||
* (worker boot + plan download + ffmpeg init) that outweighs the
|
* (worker boot + plan download + planHash recompute + ffmpeg init) that
|
||||||
* parallelism gain, per the lever-1 chunk-scaling benchmark on
|
* outweighs the parallelism gain on tiny renders.
|
||||||
* 2026-05-17.
|
|
||||||
*/
|
*/
|
||||||
export const MIN_CHUNK_SIZE = 10;
|
export const MIN_CHUNK_SIZE = 10;
|
||||||
/**
|
/**
|
||||||
@@ -379,11 +378,16 @@ export function resolveChunkPlan(
|
|||||||
// silently truncate.
|
// silently truncate.
|
||||||
assertPositiveInteger("totalFrames", totalFrames);
|
assertPositiveInteger("totalFrames", totalFrames);
|
||||||
assertPositiveInteger("maxParallelChunks", maxParallelChunks);
|
assertPositiveInteger("maxParallelChunks", maxParallelChunks);
|
||||||
|
// Validate the caller-supplied value with its real name so the error
|
||||||
|
// message points at the actual bad input. The auto-sized branch is
|
||||||
|
// provably a positive integer (totalFrames and maxParallelChunks are
|
||||||
|
// already validated above, MIN_CHUNK_SIZE is a positive integer
|
||||||
|
// constant), so it doesn't need re-checking.
|
||||||
|
if (configChunkSize !== undefined) {
|
||||||
|
assertPositiveInteger("configChunkSize", configChunkSize);
|
||||||
|
}
|
||||||
const resolvedChunkSize =
|
const resolvedChunkSize =
|
||||||
configChunkSize !== undefined
|
configChunkSize ?? Math.max(MIN_CHUNK_SIZE, Math.ceil(totalFrames / maxParallelChunks));
|
||||||
? configChunkSize
|
|
||||||
: Math.max(MIN_CHUNK_SIZE, Math.ceil(totalFrames / maxParallelChunks));
|
|
||||||
assertPositiveInteger("configChunkSize", resolvedChunkSize);
|
|
||||||
const naiveCount = Math.ceil(totalFrames / resolvedChunkSize);
|
const naiveCount = Math.ceil(totalFrames / resolvedChunkSize);
|
||||||
const chunkCount = Math.min(maxParallelChunks, Math.max(1, naiveCount));
|
const chunkCount = Math.min(maxParallelChunks, Math.max(1, naiveCount));
|
||||||
const effectiveChunkSize = Math.max(resolvedChunkSize, Math.ceil(totalFrames / chunkCount));
|
const effectiveChunkSize = Math.max(resolvedChunkSize, Math.ceil(totalFrames / chunkCount));
|
||||||
@@ -779,11 +783,6 @@ export async function plan(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ── Chunking decisions + locked config ──
|
// ── Chunking decisions + locked config ──
|
||||||
// Pass `config.chunkSize` through verbatim — `resolveChunkPlan` handles
|
|
||||||
// the `undefined` case by auto-sizing from `maxParallelChunks`, so a
|
|
||||||
// caller that bumps `maxParallelChunks` to 16 without setting
|
|
||||||
// `chunkSize` actually gets 16 chunks instead of silently clamping at
|
|
||||||
// the old 240-frame default.
|
|
||||||
const maxParallel = config.maxParallelChunks ?? DEFAULT_MAX_PARALLEL_CHUNKS;
|
const maxParallel = config.maxParallelChunks ?? DEFAULT_MAX_PARALLEL_CHUNKS;
|
||||||
const { chunkCount, effectiveChunkSize } = resolveChunkPlan(
|
const { chunkCount, effectiveChunkSize } = resolveChunkPlan(
|
||||||
totalFrames,
|
totalFrames,
|
||||||
|
|||||||
Reference in New Issue
Block a user