From 2d398ed27429382d327ad3668fa434d8a690cc26 Mon Sep 17 00:00:00 2001 From: Via Date: Thu, 16 Jul 2026 08:01:56 +0000 Subject: [PATCH] fix(cli): wire aspect-agnostic resolution through cloudrun/lambda/batch + preflight recompute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses R2 CHANGES_REQUESTED from Miga + Rames on PR #2529: 1. Sibling-surface gap (blocker): `hyperframes cloudrun render{,-batch}`, `hyperframes lambda render{,-batch}` all advertised the same tier-only aliases (`1080p` / `hd` / `4k` / `uhd`) but normalized them to `landscape` and never set `outputResolutionAspectAgnostic`. The distributed plumbing PR #2529 added received `undefined` from those callers, so portrait `1080p` still hit the original aspect-mismatch on Cloud Run / Lambda. Fix: introduce `resolveResolutionFlagPair` in `@hyperframes/parsers` (the single source of truth for the two-step normalize + aspect-agnostic detect) and route every distributed entrypoint through a shared `parseOutputResolutionFlag` CLI util so the alias signal now reaches `SerializableDistributedRenderConfig`. Studio Server keeps its canonical-only HTTP contract; that intent is now pinned in tests. 2. Preflight recompute (hardening): the earlier "downgrade aspect-mismatch" preflight cleared un-remapped mismatches, so IG 4:5 (non-preset aspect, no sibling) and portrait-4K comp + `--resolution 1080p` (remap + downsample) both slipped through to fail late in `resolveDeviceScaleFactor`. Now `checkRenderResolutionPreflight` computes the effective preset via `suggestMatchingPreset` (mirroring the compile stage's `adaptAspectAgnosticResolution`) and re-checks against that — only genuinely-fixable mismatches clear early. New tests pin both regressed input classes. 3. Docker forwarding boundary test (Miga's important #2): pinned `1080p` survives verbatim as `--resolution 1080p` in the Docker args so the in-container CLI can re-run `isAspectAgnosticResolutionAlias`. 4. Doc-nit (Miga): parsers/src/types.ts no longer references the nonexistent `resolveResolutionForComposition` — points at the actual remap helpers. Fallow: cloudrun.ts / lambda.ts share 390 lines of pre-existing structural symmetry (parallel AWS + GCP dispatchers), and lambda/render.ts + render-batch.ts declare parallel RenderArgs interfaces. Both re-flagged after threading the aspect-agnostic field through each surface; ignored with justification in .fallowrc.jsonc. lambda.ts's `run` and lambda/render.ts's `waitForCompletion` are pre-existing CRAP-score hotspots untouched by this PR — added under health.ignore. Co-Authored-By: Claude — Via --- .fallowrc.jsonc | 41 +++++++ packages/cli/src/commands/cloudrun.test.ts | 66 +++++++++++ packages/cli/src/commands/cloudrun.ts | 43 ++++--- packages/cli/src/commands/lambda.ts | 42 ++++--- .../src/commands/lambda/render-batch.test.ts | 42 ++++++- .../cli/src/commands/lambda/render-batch.ts | 49 +++++--- .../cli/src/commands/lambda/render.test.ts | 57 ++++++++++ packages/cli/src/commands/lambda/render.ts | 57 +++++++--- packages/cli/src/commands/render.test.ts | 41 +++++++ packages/cli/src/commands/render.ts | 30 ++++- packages/cli/src/utils/dockerRunArgs.test.ts | 20 ++++ .../src/utils/parseOutputResolution.test.ts | 105 ++++++++++++++++++ .../cli/src/utils/parseOutputResolution.ts | 67 +++++++++++ packages/core/src/core.types.ts | 1 + packages/core/src/index.test.ts | 36 ++++++ packages/core/src/index.ts | 1 + packages/parsers/src/types.ts | 39 ++++++- .../studio-server/src/routes/render.test.ts | 36 ++++++ 18 files changed, 707 insertions(+), 66 deletions(-) create mode 100644 packages/cli/src/commands/cloudrun.test.ts create mode 100644 packages/cli/src/commands/lambda/render.test.ts create mode 100644 packages/cli/src/utils/parseOutputResolution.test.ts create mode 100644 packages/cli/src/utils/parseOutputResolution.ts diff --git a/.fallowrc.jsonc b/.fallowrc.jsonc index 14efceac6..08f8e67ba 100644 --- a/.fallowrc.jsonc +++ b/.fallowrc.jsonc @@ -469,6 +469,29 @@ // utils/compositionServer.ts; the remaining clone is per-command logging text // (different labels/help lines) — extracting it would over-abstract. "packages/cli/src/commands/present.ts", + // Portrait --resolution alias fix (via/resolution-portrait-fix): + // cloudrun.ts and lambda.ts are intentionally symmetric per-adapter + // dispatchers — same subcommand surface (deploy / render / render-batch / + // progress / destroy), same argument parsers (parseFormat / parseCodec / + // parseQuality / parsePositiveInt), same wire-config shape. The 390-line + // cross-file clone is that pre-existing structural symmetry; the shared + // resolution-flag parse now lives in utils/parseOutputResolution.ts, but + // consolidating the per-adapter dispatcher body further would collapse + // two distinct SDK surfaces (AWS + GCP) into a single verb router that + // future adapters (Azure, etc.) would have to fork back out of. + // Line-shift fingerprint after adding `outputResolutionAspectAgnostic` + // threading re-flags the inherited clones. + "packages/cli/src/commands/cloudrun.ts", + "packages/cli/src/commands/lambda.ts", + // lambda/render.ts and lambda/render-batch.ts declare parallel + // RenderArgs / RenderBatchArgs interfaces (same core render knobs, with + // batch-only extras like maxConcurrent / dryRun). Extracting the shared + // subset into a base interface would force every consumer to spell out + // the intersection at every call site; the current shape is + // intent-preserving. Pre-existing dupe, re-flagged after threading the + // aspect-agnostic field through both interfaces. + "packages/cli/src/commands/lambda/render.ts", + "packages/cli/src/commands/lambda/render-batch.ts", // skillsManifest.test.ts: parallel arrange/act/assert cases for locateInstall // (project vs global scope, per-agent host conventions, claude-code priority). // Each case seeds a dir then asserts the resolved location/agent; collapsing @@ -755,6 +778,24 @@ // stage's runCompileStage under the cyclo/cognitive thresholds. "packages/producer/src/server.ts", "packages/producer/src/services/distributed/plan.ts", + // Sibling-surface fix (PR #2529 R2): lambda.ts's top-level `run` + // (cyclo 39, CRAP 1560) is the big subcommand switch that pre-dates + // this PR. The change threads two additional variables through the + // `render` and `render-batch` branches (parsed resolution + + // aspect-agnostic flag) but adds no new branches. parseIntFlag / + // parseEnum (both cyclo 5, CRAP 30 — right at the threshold) are also + // pre-existing utility parsers; the file-level line shift after + // adding the shared parseOutputResolutionFlag call re-flags them at + // the boundary. All three findings are inherited complexity, not new + // branches introduced by the aspect-agnostic threading. + "packages/cli/src/commands/lambda.ts", + // Sibling-surface fix (PR #2529 R2): lambda/render.ts's + // `waitForCompletion` (cyclo 11, CRAP 37.1) is the pre-existing SFN + // progress-poll loop. This PR only adds `outputResolutionAspectAgnostic` + // to `RenderArgs` + a two-line extraction (`buildLambdaRenderConfig`); + // `waitForCompletion` is untouched. Line-shift fingerprint re-flags + // the inherited complexity. + "packages/cli/src/commands/lambda/render.ts", ], }, } diff --git a/packages/cli/src/commands/cloudrun.test.ts b/packages/cli/src/commands/cloudrun.test.ts new file mode 100644 index 000000000..5922ff52d --- /dev/null +++ b/packages/cli/src/commands/cloudrun.test.ts @@ -0,0 +1,66 @@ +/** + * Boundary test for the `hyperframes cloudrun render{,-batch}` wire config. + * Pins that the aspect-agnostic flag reaches `SerializableDistributedRenderConfig` + * so the Cloud Run worker's compile stage can remap `landscape` → `portrait`. + * + * The parse helper itself is covered at `../utils/parseOutputResolution.test.ts`; + * we only re-check the entrypoint composition here. + */ + +import { describe, expect, it } from "vitest"; +import { buildRenderConfig } from "./cloudrun.js"; + +describe("cloudrun wire config — aspect-agnostic threading", () => { + const baseArgs: Record = { + format: "mp4", + codec: undefined, + quality: undefined, + "chunk-size": undefined, + "max-parallel-chunks": undefined, + "target-chunk-frames": undefined, + }; + + it("threads outputResolutionAspectAgnostic=true through the wire config for portrait 1080p", () => { + // The exact bug shape: portrait comp + `--output-resolution 1080p`. + // Before the fix, the alias signal never reached the wire; Cloud Run + // then hit the same portrait rejection this PR set out to eliminate. + const config = buildRenderConfig( + { ...baseArgs, "output-resolution": "1080p" }, + 30, + 1080, + 1920, + undefined, + ); + expect(config.outputResolution).toBe("landscape"); + expect(config.outputResolutionAspectAgnostic).toBe(true); + }); + + it("keeps the aspect-agnostic key absent when the flag is a canonical preset", () => { + // Sparse-wire invariant: don't broadcast `false` for the common path — + // the compile-stage remap only fires on `true`. + const config = buildRenderConfig( + { ...baseArgs, "output-resolution": "portrait-4k" }, + 30, + 2160, + 3840, + undefined, + ); + expect(config.outputResolution).toBe("portrait-4k"); + expect(config).not.toHaveProperty("outputResolutionAspectAgnostic"); + }); + + it("omits both resolution fields when --output-resolution is unset", () => { + const config = buildRenderConfig(baseArgs, 30, 1920, 1080, undefined); + expect(config).not.toHaveProperty("outputResolution"); + expect(config).not.toHaveProperty("outputResolutionAspectAgnostic"); + }); + + it("preserves the surface-labeled strict-throw contract on unknown values", () => { + // Sanity guard: the local delegate stays wired to the shared helper's + // throw semantics rather than silently downgrading to undefined. Full + // input-space coverage lives at `../utils/parseOutputResolution.test.ts`. + expect(() => + buildRenderConfig({ "output-resolution": "8k" }, 30, 1920, 1080, undefined), + ).toThrow(/\[cloudrun render\]/); + }); +}); diff --git a/packages/cli/src/commands/cloudrun.ts b/packages/cli/src/commands/cloudrun.ts index 65c2bda02..b88c209bb 100644 --- a/packages/cli/src/commands/cloudrun.ts +++ b/packages/cli/src/commands/cloudrun.ts @@ -18,11 +18,8 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { defineCommand } from "citty"; -import { - type CanvasResolution, - normalizeResolutionFlag, - VALID_CANVAS_RESOLUTIONS, -} from "@hyperframes/core"; +import { type CanvasResolution } from "@hyperframes/core"; +import { parseOutputResolutionFlag } from "../utils/parseOutputResolution.js"; import type { Example } from "./_examples.js"; import { c } from "../ui/colors.js"; import { @@ -774,13 +771,21 @@ function runDestroy(args: Record): void { * separately (it differs per batch entry). Mirrors the local `hyperframes * render` flag surface so the two stay consistent. */ -function buildRenderConfig( +/** + * Exported for unit-test coverage of the aspect-agnostic wire shape — the + * portrait-1080p sibling-surface regression that shipped in v0.7.60 landed + * here because this builder dropped the tier-alias signal on the floor. + */ +export function buildRenderConfig( args: Record, fps: number, width: number, height: number, variables: Record | undefined, ): Record { + const { outputResolution, outputResolutionAspectAgnostic } = parseOutputResolution( + args["output-resolution"], + ); return stripUndefined({ fps, width, @@ -791,7 +796,11 @@ function buildRenderConfig( chunkSize: parsePositiveInt(args["chunk-size"], "--chunk-size"), maxParallelChunks: parsePositiveInt(args["max-parallel-chunks"], "--max-parallel-chunks"), targetChunkFrames: parsePositiveInt(args["target-chunk-frames"], "--target-chunk-frames"), - outputResolution: parseOutputResolution(args["output-resolution"]), + outputResolution, + // Set only when true so the wire shape stays sparse for the common + // canonical-preset path (matches how the flag flows through + // `SerializableDistributedRenderConfig` from every other emitter). + outputResolutionAspectAgnostic: outputResolutionAspectAgnostic ? true : undefined, variables, }); } @@ -823,14 +832,18 @@ function resolveAndValidateVariables( return variables; } -function parseOutputResolution(raw: unknown): CanvasResolution | undefined { - if (raw == null || raw === "") return undefined; - const normalized = normalizeResolutionFlag(String(raw)); - if (normalized) return normalized; - throw new Error( - `[cloudrun render] --output-resolution must be one of ${VALID_CANVAS_RESOLUTIONS.join("|")} ` + - `(or an alias: 1080p, 4k, uhd, hd, …); got ${String(raw)}`, - ); +/** + * Cloud Run flavor of the shared {@link parseOutputResolutionFlag} — carries + * the aspect-agnostic signal through so `SerializableDistributedRenderConfig` + * can trigger the compile-stage remap. The runtime work lives in the shared + * util; wire-config-level coverage lives at `cloudrun.test.ts`, and full + * input-space coverage at `../utils/parseOutputResolution.test.ts`. + */ +function parseOutputResolution(raw: unknown): { + outputResolution: CanvasResolution | undefined; + outputResolutionAspectAgnostic: boolean; +} { + return parseOutputResolutionFlag(raw, { surfaceLabel: "[cloudrun render]" }); } // ── parse helpers ───────────────────────────────────────────────────────── diff --git a/packages/cli/src/commands/lambda.ts b/packages/cli/src/commands/lambda.ts index f5ad901a3..513928a3e 100644 --- a/packages/cli/src/commands/lambda.ts +++ b/packages/cli/src/commands/lambda.ts @@ -10,11 +10,8 @@ import { defineCommand } from "citty"; import type { DistributedFormat } from "@hyperframes/aws-lambda/sdk"; -import { - type CanvasResolution, - VALID_CANVAS_RESOLUTIONS, - normalizeResolutionFlag, -} from "@hyperframes/core"; +import { type CanvasResolution } from "@hyperframes/core"; +import { parseOutputResolutionFlag } from "../utils/parseOutputResolution.js"; import type { Example } from "./_examples.js"; import { c } from "../ui/colors.js"; import { readAllowedCompositionFpsFromDir } from "../utils/compositionFps.js"; @@ -307,6 +304,7 @@ export default defineCommand({ process.exit(1); } const { runRender } = await import("./lambda/render.js"); + const renderResolution = parseOutputResolution(args["output-resolution"]); await runRender({ projectDir, stackName, @@ -314,7 +312,8 @@ export default defineCommand({ fps: fpsRaw, width, height, - outputResolution: parseOutputResolution(args["output-resolution"]), + outputResolution: renderResolution.outputResolution, + outputResolutionAspectAgnostic: renderResolution.outputResolutionAspectAgnostic, format: parseFormat(args.format), codec: parseCodec(args.codec), quality: parseQuality(args.quality), @@ -362,6 +361,7 @@ export default defineCommand({ process.exit(1); } const { runRenderBatch } = await import("./lambda/render-batch.js"); + const batchResolution = parseOutputResolution(args["output-resolution"]); await runRenderBatch({ projectDir, stackName, @@ -370,7 +370,8 @@ export default defineCommand({ fps: fpsRaw, width, height, - outputResolution: parseOutputResolution(args["output-resolution"]), + outputResolution: batchResolution.outputResolution, + outputResolutionAspectAgnostic: batchResolution.outputResolutionAspectAgnostic, format: parseFormat(args.format), codec: parseCodec(args.codec), quality: parseQuality(args.quality), @@ -481,12 +482,23 @@ const parseQuality = (raw: unknown): (typeof QUALITIES)[number] | undefined => const parseChromeSource = (raw: unknown): (typeof CHROME_SOURCES)[number] => parseEnum(raw, CHROME_SOURCES, "[lambda deploy] --chrome-source", "sparticuz")!; -function parseOutputResolution(raw: unknown): CanvasResolution | undefined { - if (raw == null || raw === "") return undefined; - const normalized = normalizeResolutionFlag(String(raw)); - if (normalized) return normalized; - throw new Error( - `[lambda render] --output-resolution must be one of ${VALID_CANVAS_RESOLUTIONS.join("|")} ` + - `(or an alias: 1080p, 4k, uhd, hd, 1080p-portrait, portrait-1080p, 4k-portrait, 1080p-square, square-1080p, 4k-square); got ${String(raw)}`, - ); +/** + * Lambda flavor of the shared {@link parseOutputResolutionFlag} — same wire + * contract as the Cloud Run counterpart. Runtime work lives in the shared + * util; wire-config-level coverage lives at `./lambda/render.test.ts` / + * `./lambda/render-batch.test.ts`, and full input-space coverage at + * `../utils/parseOutputResolution.test.ts`. + */ +function parseOutputResolution(raw: unknown): { + outputResolution: CanvasResolution | undefined; + outputResolutionAspectAgnostic: boolean; +} { + return parseOutputResolutionFlag(raw, { + surfaceLabel: "[lambda render]", + // The Lambda `--output-resolution` help text advertises the full alias + // list (tier-only + orientation-suffixed) — keep the error message + // faithful to that surface's docs. + aliasHint: + "1080p, 4k, uhd, hd, 1080p-portrait, portrait-1080p, 4k-portrait, 1080p-square, square-1080p, 4k-square", + }); } diff --git a/packages/cli/src/commands/lambda/render-batch.test.ts b/packages/cli/src/commands/lambda/render-batch.test.ts index 283ca0088..5dfb9ff14 100644 --- a/packages/cli/src/commands/lambda/render-batch.test.ts +++ b/packages/cli/src/commands/lambda/render-batch.test.ts @@ -2,7 +2,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { parseBatchFile, runWithConcurrencyLimit } from "./render-batch.js"; +import { + buildLambdaBatchRenderConfig, + parseBatchFile, + runWithConcurrencyLimit, + type RenderBatchArgs, +} from "./render-batch.js"; let tmpDir: string; @@ -137,3 +142,38 @@ describe("parseBatchFile", () => { expectExitOne('{"outputKey":"renders/a.mp4","variables":[1,2,3]}\n'); }); }); + +// See `../cloudrun.test.ts` / `./render.test.ts` for the sibling wire-config +// coverage. Repeating it at every entrypoint is deliberate: cross-scaffold +// drift is exactly what shipped PR #2529 R2 CHANGES_REQUESTED. +describe("buildLambdaBatchRenderConfig — aspect-agnostic wire threading", () => { + const baseArgs: RenderBatchArgs = { + projectDir: "/tmp/hf-batch", + stackName: "hf-test", + batch: "/tmp/batch.jsonl", + fps: 30, + width: 1080, + height: 1920, + format: "mp4", + json: false, + }; + + it("threads outputResolutionAspectAgnostic=true through for portrait 1080p", () => { + // The batch entrypoint fans out N Step Functions executions from a + // single wire config, so a dropped alias flag multiplies into N broken + // renders — pin it here. + const config = buildLambdaBatchRenderConfig({ + ...baseArgs, + outputResolution: "landscape", + outputResolutionAspectAgnostic: true, + }); + expect(config.outputResolution).toBe("landscape"); + expect(config.outputResolutionAspectAgnostic).toBe(true); + }); + + it("keeps the aspect-agnostic key absent when the flag is a canonical preset", () => { + const config = buildLambdaBatchRenderConfig({ ...baseArgs, outputResolution: "portrait-4k" }); + expect(config.outputResolution).toBe("portrait-4k"); + expect(config.outputResolutionAspectAgnostic).toBeUndefined(); + }); +}); diff --git a/packages/cli/src/commands/lambda/render-batch.ts b/packages/cli/src/commands/lambda/render-batch.ts index 2c5ad177c..340e9ef63 100644 --- a/packages/cli/src/commands/lambda/render-batch.ts +++ b/packages/cli/src/commands/lambda/render-batch.ts @@ -65,6 +65,13 @@ export interface RenderBatchArgs { height: number; /** See {@link RenderArgs.outputResolution}. */ outputResolution?: CanvasResolution; + /** + * See {@link RenderArgs.outputResolutionAspectAgnostic}. Threaded through + * `SerializableDistributedRenderConfig` so the Lambda worker's compile + * stage remaps aspect-agnostic aliases (`1080p` / `hd` / `4k` / `uhd`) to + * the composition's orientation — same regression class as the local CLI. + */ + outputResolutionAspectAgnostic?: boolean; format: DistributedFormat; codec?: "h264" | "h265"; quality?: "draft" | "standard" | "high"; @@ -209,19 +216,7 @@ export async function runRenderBatch(args: RenderBatchArgs): Promise { process.exit(1); } - const config: SerializableDistributedRenderConfig = { - fps: args.fps, - width: args.width, - height: args.height, - outputResolution: args.outputResolution, - format: args.format, - codec: args.codec, - quality: args.quality, - chunkSize: args.chunkSize, - maxParallelChunks: args.maxParallelChunks, - targetChunkFrames: args.targetChunkFrames, - runtimeCap: "lambda", - }; + const config: SerializableDistributedRenderConfig = buildLambdaBatchRenderConfig(args); // Deploy the site once and reuse it across every entry. --site-id and // --dry-run both skip the deploy via a synthesised handle. @@ -350,6 +345,34 @@ function makePlaceholderSiteHandle(siteId: string, bucketName: string): SiteHand }; } +/** + * Build the shared wire {@link SerializableDistributedRenderConfig} used + * for every entry in a `hyperframes lambda render-batch` run. Extracted for + * boundary-test coverage of the aspect-agnostic flag threading (per-entry + * `variables` are still overlayed at dispatch time inside {@link runRenderBatch}). + */ +export function buildLambdaBatchRenderConfig( + args: RenderBatchArgs, +): SerializableDistributedRenderConfig { + const config: SerializableDistributedRenderConfig = { + fps: args.fps, + width: args.width, + height: args.height, + outputResolution: args.outputResolution, + format: args.format, + codec: args.codec, + quality: args.quality, + chunkSize: args.chunkSize, + maxParallelChunks: args.maxParallelChunks, + targetChunkFrames: args.targetChunkFrames, + runtimeCap: "lambda", + }; + if (args.outputResolutionAspectAgnostic) { + config.outputResolutionAspectAgnostic = true; + } + return config; +} + /** * Read the JSONL batch file and return one parsed entry per non-blank * line. Reads the whole file into memory — fine for typical batch sizes, diff --git a/packages/cli/src/commands/lambda/render.test.ts b/packages/cli/src/commands/lambda/render.test.ts new file mode 100644 index 000000000..ef3305f5a --- /dev/null +++ b/packages/cli/src/commands/lambda/render.test.ts @@ -0,0 +1,57 @@ +/** + * Boundary tests for the wire config emitted by `hyperframes lambda render`. + * Pins that the aspect-agnostic resolution flag survives all the way into + * `SerializableDistributedRenderConfig`, which is what the Lambda worker's + * compile stage reads before remapping `landscape` → `portrait` for an + * aspect-agnostic alias like `--output-resolution 1080p`. + * + * See `../cloudrun.test.ts` for the parseOutputResolution counterpart and + * `../render.test.ts` for the local-CLI preflight guardrails. + */ + +import { describe, expect, it } from "vitest"; +import { buildLambdaRenderConfig, type RenderArgs } from "./render.js"; + +const BASE_ARGS: RenderArgs = { + projectDir: "/tmp/hf-project", + stackName: "hf-test", + fps: 30, + width: 1080, + height: 1920, + format: "mp4", + json: false, + wait: false, + waitIntervalMs: 5000, +}; + +describe("buildLambdaRenderConfig — aspect-agnostic wire threading", () => { + it("threads outputResolutionAspectAgnostic=true through for portrait 1080p", () => { + // The exact bug shape: portrait 1080×1920 comp + `--output-resolution 1080p`. + // Before the fix, the aspect-agnostic flag never reached the wire, so + // the Lambda worker saw the explicit `landscape` preset and rejected + // the portrait comp — reproducing the local-CLI regression on the + // distributed path. + const config = buildLambdaRenderConfig( + { ...BASE_ARGS, outputResolution: "landscape", outputResolutionAspectAgnostic: true }, + undefined, + ); + expect(config.outputResolution).toBe("landscape"); + expect(config.outputResolutionAspectAgnostic).toBe(true); + }); + + it("keeps the aspect-agnostic key absent when the flag is a canonical preset", () => { + // Sparse-wire invariant: only forward `true`; canonical presets stay + // strict and don't need the compile-stage remap. + const config = buildLambdaRenderConfig( + { ...BASE_ARGS, outputResolution: "portrait-4k" }, + undefined, + ); + expect(config.outputResolution).toBe("portrait-4k"); + expect(config.outputResolutionAspectAgnostic).toBeUndefined(); + }); + + it("carries variables verbatim through the wire config", () => { + const config = buildLambdaRenderConfig(BASE_ARGS, { alice: "hello" }); + expect(config.variables).toEqual({ alice: "hello" }); + }); +}); diff --git a/packages/cli/src/commands/lambda/render.ts b/packages/cli/src/commands/lambda/render.ts index d9b31e2c9..a416769d4 100644 --- a/packages/cli/src/commands/lambda/render.ts +++ b/packages/cli/src/commands/lambda/render.ts @@ -42,6 +42,15 @@ export interface RenderArgs { * from a 1920×1080 layout uses DPR 2, etc. */ outputResolution?: CanvasResolution; + /** + * True when {@link outputResolution} was normalized from an aspect-agnostic + * alias (`1080p` / `hd` / `4k` / `uhd`). Threaded through + * `SerializableDistributedRenderConfig` so the Lambda worker's compile stage + * remaps `landscape` → `portrait` / `square` when the composition's + * orientation demands it. Dropping this field is what shipped the + * portrait-1080p regression at the sibling entrypoints (see PR #2529). + */ + outputResolutionAspectAgnostic?: boolean; format: DistributedFormat; codec?: "h264" | "h265"; quality?: "draft" | "standard" | "high"; @@ -114,20 +123,7 @@ export async function runRender(args: RenderArgs): Promise { } } - const config: SerializableDistributedRenderConfig = { - fps: args.fps, - width: args.width, - height: args.height, - outputResolution: args.outputResolution, - format: args.format, - codec: args.codec, - quality: args.quality, - chunkSize: args.chunkSize, - maxParallelChunks: args.maxParallelChunks, - targetChunkFrames: args.targetChunkFrames, - runtimeCap: "lambda", - variables, - }; + const config: SerializableDistributedRenderConfig = buildLambdaRenderConfig(args, variables); // When the caller passes only --site-id, synthesise the minimum-shape // SiteHandle pointing at the deterministic content-addressed key. The @@ -233,3 +229,36 @@ async function waitForCompletion( function sleep(ms: number): Promise { return new Promise((res) => setTimeout(res, ms)); } + +/** + * Build the wire {@link SerializableDistributedRenderConfig} for + * `hyperframes lambda render`. Extracted for boundary-test coverage of the + * aspect-agnostic flag threading — the field this helper preserves is what + * lets the Lambda worker's compile stage remap `landscape` → `portrait` on + * an aspect-agnostic alias (`1080p` / `hd` / `4k` / `uhd`). + */ +export function buildLambdaRenderConfig( + args: RenderArgs, + variables: Record | undefined, +): SerializableDistributedRenderConfig { + const config: SerializableDistributedRenderConfig = { + fps: args.fps, + width: args.width, + height: args.height, + outputResolution: args.outputResolution, + format: args.format, + codec: args.codec, + quality: args.quality, + chunkSize: args.chunkSize, + maxParallelChunks: args.maxParallelChunks, + targetChunkFrames: args.targetChunkFrames, + runtimeCap: "lambda", + variables, + }; + // Keep the wire shape sparse when the alias flag wasn't set — matches how + // `SerializableDistributedRenderConfig` is emitted at every other adapter. + if (args.outputResolutionAspectAgnostic) { + config.outputResolutionAspectAgnostic = true; + } + return config; +} diff --git a/packages/cli/src/commands/render.test.ts b/packages/cli/src/commands/render.test.ts index aeea82d04..ebdc31026 100644 --- a/packages/cli/src/commands/render.test.ts +++ b/packages/cli/src/commands/render.test.ts @@ -1244,6 +1244,47 @@ describe("checkRenderResolutionPreflight", () => { expect(result?.kind).toBe("aspect-mismatch"); expect(result?.message).toContain("--resolution portrait"); }); + + // Rames Δ2 on PR #2529: the earlier "downgrade aspect-mismatch to + // undefined" preflight cleared *un-remapped* mismatches, so two input + // classes below regressed from an early actionable error to a late throw + // deep in `resolveDeviceScaleFactor` (browser + ffmpeg already up). + // The fix computes the *effective* preset via `suggestMatchingPreset` + // (mirroring the compile stage) and re-checks against that, so only + // genuinely-fixable mismatches clear early. + + it("blocks IG 4:5 (non-preset aspect) early with an aspect-aware message", async () => { + // 1080×1350 is a 4:5 portrait — no canonical preset shares that aspect, + // so `suggestMatchingPreset` returns undefined and `adaptAspectAgnosticResolution` + // keeps the original preset. Before the fix, aspect-agnostic downgraded + // the mismatch here to undefined; now the preflight surfaces it early. + const result = await checkRenderResolutionPreflight(comp(1080, 1350), "landscape", agnostic); + expect(result?.kind).toBe("aspect-mismatch"); + // No sibling preset to suggest → message falls back to the "pick a preset + // whose orientation matches" hint (see `buildAspectMismatch` in + // `@hyperframes/parsers/outputResolutionCompatibility`). + expect(result?.message).toMatch(/preset whose orientation matches|omit --resolution/i); + }); + + it("blocks a portrait-4K comp + `1080p` downsample early (orientation-flip masks the tier gap)", async () => { + // 2160×3840 (portrait 4K) + `--resolution 1080p` — the compile stage + // remaps `landscape` → `portrait` (1080×1920), and *then* the preset + // is smaller than the composition. The un-remapped preflight let this + // slip through as an aspect-mismatch downgrade; the remap-then-check + // catches the real failure — downsampling — early. + const result = await checkRenderResolutionPreflight(comp(2160, 3840), "landscape", agnostic); + expect(result?.kind).toBe("downsampling"); + }); + + it("blocks a portrait 720p comp + `1080p` non-integer upscale early (orientation-flip masks the fractional DPR)", async () => { + // 720×1280 (portrait 720p) + `--resolution 1080p` — remap `landscape` + // → `portrait` (1080×1920). widthRatio = 1080 / 720 = 1.5, which + // `resolveDeviceScaleFactor` rejects. Surfacing it in preflight beats + // failing after Chrome + ffmpeg spin up. Same class as Miga's + // important note on PR #2529. + const result = await checkRenderResolutionPreflight(comp(720, 1280), "landscape", agnostic); + expect(result?.kind).toBe("non-integer-scale"); + }); }); }); diff --git a/packages/cli/src/commands/render.ts b/packages/cli/src/commands/render.ts index 914b73a4c..bd2faf927 100644 --- a/packages/cli/src/commands/render.ts +++ b/packages/cli/src/commands/render.ts @@ -92,6 +92,7 @@ import { normalizeResolutionFlag, isAspectAgnosticResolutionAlias, checkOutputResolutionCompatibility, + suggestMatchingPreset, parseFps, fpsToNumber, fpsToFfmpegArg, @@ -1216,20 +1217,37 @@ export async function checkRenderResolutionPreflight( // Couldn't determine the composition's actual dimensions — defer to the // pipeline's own defense-in-depth check rather than guess. if (!dims) return undefined; + // Aspect-agnostic aliases (`--resolution 1080p` / `hd` / `4k` / `uhd`) + // don't nail an orientation — the compile stage remaps them to the + // composition's orientation via `adaptAspectAgnosticResolution` + + // `suggestMatchingPreset`. Mirror that remap here BEFORE running the + // compatibility check so the early rejection reflects the *effective* + // preset the pipeline will actually use. + // + // Doing this pre-check (rather than post-hoc "downgrade aspect-mismatch") + // is what makes the following cases fail early with an aspect-aware + // message instead of throwing deep in the compile stage after Chrome + + // ffmpeg have already spun up (Rames Δ2 on PR #2529): + // + // - Non-preset aspect (e.g. IG 4:5 1080×1350): no sibling preset + // matches → `suggestMatchingPreset` returns `undefined` → we keep the + // original preset and surface the aspect-mismatch normally. + // - Orientation-flip + tier-too-small (portrait-4K comp 2160×3840 + + // `--resolution 1080p`): remaps `landscape` → `portrait` (1080×1920), + // re-check catches the downsample early with a clear message. + const effective = + modes.aspectAgnostic === true + ? (suggestMatchingPreset(dims.width, dims.height, outputResolution) ?? outputResolution) + : outputResolution; const compat = checkOutputResolutionCompatibility({ compositionWidth: dims.width, compositionHeight: dims.height, - outputResolution, + outputResolution: effective, alphaRequested: modes.alphaRequested, hdrRequested: modes.hdrRequested, }); // Narrow to the incompatible case; `message`/`kind` are always set there. if (compat.ok || !compat.message || !compat.kind) return undefined; - // Aspect-agnostic aliases delegate orientation to the composition — a - // landscape-vs-portrait mismatch is expected and self-heals in the compile - // stage. Only *aspect-mismatch* is downgraded; other issue kinds still - // block (see the `aspectAgnostic` note above). - if (modes.aspectAgnostic && compat.kind === "aspect-mismatch") return undefined; return { message: compat.message, kind: compat.kind }; } diff --git a/packages/cli/src/utils/dockerRunArgs.test.ts b/packages/cli/src/utils/dockerRunArgs.test.ts index 97b4e4ade..e35886447 100644 --- a/packages/cli/src/utils/dockerRunArgs.test.ts +++ b/packages/cli/src/utils/dockerRunArgs.test.ts @@ -377,6 +377,26 @@ describe("buildDockerRunArgs", () => { expect(args[idx + 1]).toBe("landscape-4k"); }); + it("forwards the RAW aspect-agnostic alias `1080p` verbatim (does not pre-normalize to `landscape`)", () => { + // Miga R2 important note on PR #2529: Docker correctness now depends on + // forwarding the raw alias string, not the canonical preset — the + // in-container CLI re-runs `normalizeResolutionFlag` + + // `isAspectAgnosticResolutionAlias` so aspect-agnostic aliases keep + // their orientation-adaptive behavior. A future refactor that + // silently substitutes the normalized preset here would restore the + // portrait-only Docker failure this test pins against. + const args = buildDockerRunArgs({ + ...FIXED_INPUT, + options: { ...BASE, outputResolution: "1080p" }, + }); + const idx = args.indexOf("--resolution"); + expect(idx).toBeGreaterThan(-1); + expect(args[idx + 1]).toBe("1080p"); + // Belt-and-braces: the normalized preset name must NOT slip in as a + // second value that would confuse citty parsing on the container side. + expect(args).not.toContain("landscape"); + }); + it("omits --resolution when outputResolution is not set", () => { const args = buildDockerRunArgs({ ...FIXED_INPUT, options: BASE }); expect(args).not.toContain("--resolution"); diff --git a/packages/cli/src/utils/parseOutputResolution.test.ts b/packages/cli/src/utils/parseOutputResolution.test.ts new file mode 100644 index 000000000..e099b5d23 --- /dev/null +++ b/packages/cli/src/utils/parseOutputResolution.test.ts @@ -0,0 +1,105 @@ +/** + * Boundary tests for the shared `parseOutputResolutionFlag` helper. Every + * distributed entrypoint (`hyperframes cloudrun render{,-batch}`, + * `hyperframes lambda render{,-batch}`) delegates to this one function — + * covering it here (rather than at each surface) makes the sibling-surface + * regression this PR fixes unreachable by construction: any future surface + * that calls `parseOutputResolutionFlag` inherits the correct alias + * threading. The wire-config-level tests at + * `../commands/cloudrun.test.ts` / `../commands/lambda/render.test.ts` / + * `../commands/lambda/render-batch.test.ts` still exercise the + * per-entrypoint composition so the plumbing stays end-to-end covered. + */ + +import { describe, expect, it } from "vitest"; +import { parseOutputResolutionFlag } from "./parseOutputResolution.js"; + +const CLOUDRUN = { surfaceLabel: "[cloudrun render]" } as const; +const LAMBDA = { + surfaceLabel: "[lambda render]", + aliasHint: + "1080p, 4k, uhd, hd, 1080p-portrait, portrait-1080p, 4k-portrait, 1080p-square, square-1080p, 4k-square", +} as const; + +describe("parseOutputResolutionFlag", () => { + it.each([undefined, "", null])( + "returns undefined + false when the flag is omitted (raw=%s)", + (raw) => { + expect(parseOutputResolutionFlag(raw, CLOUDRUN)).toEqual({ + outputResolution: undefined, + outputResolutionAspectAgnostic: false, + }); + }, + ); + + it.each(["landscape", "portrait-4k", "square", "square-4k"])( + "normalizes canonical preset %s with aspect-agnostic=false", + (preset) => { + const { outputResolution, outputResolutionAspectAgnostic } = parseOutputResolutionFlag( + preset, + CLOUDRUN, + ); + expect(outputResolution).toBe(preset); + expect(outputResolutionAspectAgnostic).toBe(false); + }, + ); + + // The blocker path from Miga's R2 review: without this pair, a portrait + // composition with `--output-resolution 1080p` reaches the compile stage + // as the explicit `landscape` preset and rejects with the original + // aspect-mismatch instead of remapping to `portrait`. + it.each(["1080p", "hd", "4k", "uhd"])( + "flags aspect-agnostic tier alias %s so the compile stage can remap orientation", + (alias) => { + const { outputResolution, outputResolutionAspectAgnostic } = parseOutputResolutionFlag( + alias, + CLOUDRUN, + ); + expect(outputResolutionAspectAgnostic).toBe(true); + expect(outputResolution).toBeDefined(); + }, + ); + + it.each(["1080p-portrait", "portrait-1080p", "1080p-square", "4k-portrait", "4k-square"])( + "does NOT flag orientation-suffixed alias %s as aspect-agnostic", + (alias) => { + // The user picked an orientation — respect it, don't silently swap. + const { outputResolutionAspectAgnostic } = parseOutputResolutionFlag(alias, CLOUDRUN); + expect(outputResolutionAspectAgnostic).toBe(false); + }, + ); + + it("treats input case-insensitively (1080P, UHD, HD, 4K all pass)", () => { + for (const alias of ["1080P", "UHD", "HD", "4K"]) { + expect(parseOutputResolutionFlag(alias, CLOUDRUN).outputResolutionAspectAgnostic).toBe(true); + } + }); + + it("throws with the caller-supplied surface label on unknown values", () => { + // The two surfaces MUST use the same underlying helper (see PR #2529 — + // divergent copies is exactly the cross-scaffold drift class this + // consolidation prevents), but each stakes its own label so debugging + // still points at the right verb. + expect(() => parseOutputResolutionFlag("8k", CLOUDRUN)).toThrow(/\[cloudrun render\]/); + expect(() => parseOutputResolutionFlag("8k", LAMBDA)).toThrow(/\[lambda render\]/); + }); + + it("appends the caller-supplied aliasHint to the error text (so the message stays surface-accurate)", () => { + // Lambda advertises the full orientation-suffixed alias list in help + // text; the error message must match that surface for the user's + // "did you mean?" search to land on real docs. + const err = getThrown(() => parseOutputResolutionFlag("8k", LAMBDA)); + expect(err.message).toContain("1080p-portrait"); + expect(err.message).toContain("4k-portrait"); + }); +}); + +function getThrown(fn: () => void): Error { + try { + fn(); + } catch (e) { + if (e instanceof Error) return e; + throw new Error(`Non-Error thrown: ${String(e)}`); + } + throw new Error("Expected fn to throw, but it did not"); +} diff --git a/packages/cli/src/utils/parseOutputResolution.ts b/packages/cli/src/utils/parseOutputResolution.ts new file mode 100644 index 000000000..3b0a00de3 --- /dev/null +++ b/packages/cli/src/utils/parseOutputResolution.ts @@ -0,0 +1,67 @@ +/** + * Shared `--output-resolution` / `--resolution` normalizer for the distributed + * render entrypoints (`hyperframes cloudrun render{,-batch}`, `hyperframes + * lambda render{,-batch}`) plus the local `hyperframes render` command. + * + * The one field this helper carries that the previous per-surface copies + * were dropping is `outputResolutionAspectAgnostic`: `true` when the raw + * flag was a tier-only alias (`1080p` / `hd` / `4k` / `uhd`). Passing it + * through into `SerializableDistributedRenderConfig` is what lets the + * remote worker's compile stage remap `landscape` → `portrait` when the + * composition demands it. Dropping the flag at any single entrypoint + * reproduces the portrait-1080p regression this helper prevents (see + * PR #2529 R2 CHANGES_REQUESTED and the sibling-surface enumeration in + * Miga + Rames's reviews). + * + * The strict-throw contract (unknown values raise instead of silently + * degrading to `outputResolution: undefined`) is preserved so a typo like + * `--output-resolution 8k` fails fast rather than falling back to + * composition dimensions. + */ + +import { type CanvasResolution, resolveResolutionFlagPair } from "@hyperframes/core"; +import { VALID_CANVAS_RESOLUTIONS } from "@hyperframes/core"; + +/** + * Free-text prefix the thrown error is scoped to (e.g. `"[cloudrun render]"`, + * `"[lambda render]"`). Kept as a caller-supplied string rather than a + * fixed enum so future surfaces (Studio Server, an SDK wrapper, …) can + * opt in without editing this file. + */ +export interface OutputResolutionParseOptions { + surfaceLabel: string; + /** + * Optional per-surface hint appended to the error message. Defaults to a + * generic tier-alias hint; the Lambda surface exposes additional + * orientation-suffixed aliases the CLI accepts (`1080p-portrait`, `4k-portrait`, + * …) — pass a custom hint to keep the error text faithful. + */ + aliasHint?: string; +} + +/** + * Parse the user-supplied resolution flag into the pair the distributed + * wire config needs. Returns `{ outputResolution: undefined, + * outputResolutionAspectAgnostic: false }` when the flag is absent so the + * caller can spread the result unconditionally. + * + * Throws (not exits) on an unknown value — CLI callers wrap that in their + * own errorBox / process.exit; SDK callers surface the error to their own + * user. + */ +export function parseOutputResolutionFlag( + raw: unknown, + options: OutputResolutionParseOptions, +): { outputResolution: CanvasResolution | undefined; outputResolutionAspectAgnostic: boolean } { + if (raw == null || raw === "") { + return { outputResolution: undefined, outputResolutionAspectAgnostic: false }; + } + const asString = String(raw); + const { outputResolution, outputResolutionAspectAgnostic } = resolveResolutionFlagPair(asString); + if (outputResolution) return { outputResolution, outputResolutionAspectAgnostic }; + const aliasHint = options.aliasHint ?? "1080p, 4k, uhd, hd, …"; + throw new Error( + `${options.surfaceLabel} --output-resolution must be one of ${VALID_CANVAS_RESOLUTIONS.join("|")} ` + + `(or an alias: ${aliasHint}); got ${asString}`, + ); +} diff --git a/packages/core/src/core.types.ts b/packages/core/src/core.types.ts index 182ee9250..99d356b55 100644 --- a/packages/core/src/core.types.ts +++ b/packages/core/src/core.types.ts @@ -180,6 +180,7 @@ export { VALID_CANVAS_RESOLUTIONS, normalizeResolutionFlag, isAspectAgnosticResolutionAlias, + resolveResolutionFlagPair, checkOutputResolutionCompatibility, suggestMatchingPreset, COMPOSITION_VARIABLE_TYPES, diff --git a/packages/core/src/index.test.ts b/packages/core/src/index.test.ts index 524d9601d..793951ba4 100644 --- a/packages/core/src/index.test.ts +++ b/packages/core/src/index.test.ts @@ -63,6 +63,42 @@ describe("@hyperframes/core public API exports", () => { expect(core.isAspectAgnosticResolutionAlias(undefined)).toBe(false); }); + it("exports resolveResolutionFlagPair — the pair every distributed entrypoint must forward", () => { + // The single source of truth every distributed adapter reads + // (`hyperframes cloudrun render`, `hyperframes lambda render`, + // `hyperframes lambda render-batch`). Divergent copies across those + // callers is what shipped the portrait-1080p failure this helper + // exists to prevent (PR #2529). Case-insensitive on the raw input. + expect(core.resolveResolutionFlagPair("1080p")).toEqual({ + outputResolution: "landscape", + outputResolutionAspectAgnostic: true, + }); + expect(core.resolveResolutionFlagPair("4K")).toEqual({ + outputResolution: "landscape-4k", + outputResolutionAspectAgnostic: true, + }); + // Canonical preset — aspect-agnostic stays false. + expect(core.resolveResolutionFlagPair("portrait")).toEqual({ + outputResolution: "portrait", + outputResolutionAspectAgnostic: false, + }); + // Orientation-suffixed alias — aspect-agnostic stays false. + expect(core.resolveResolutionFlagPair("1080p-portrait")).toEqual({ + outputResolution: "portrait", + outputResolutionAspectAgnostic: false, + }); + // Unknown / empty / undefined → both fields degrade cleanly so + // callers can produce their own "invalid" UX. + expect(core.resolveResolutionFlagPair("8k")).toEqual({ + outputResolution: undefined, + outputResolutionAspectAgnostic: false, + }); + expect(core.resolveResolutionFlagPair(undefined)).toEqual({ + outputResolution: undefined, + outputResolutionAspectAgnostic: false, + }); + }); + it("exports TIMELINE_COLORS", () => { expect(core.TIMELINE_COLORS).toBeDefined(); expect(core.TIMELINE_COLORS.video).toBeDefined(); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index cc27079cc..3d93e0adf 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -56,6 +56,7 @@ export { VALID_CANVAS_RESOLUTIONS, normalizeResolutionFlag, isAspectAgnosticResolutionAlias, + resolveResolutionFlagPair, checkOutputResolutionCompatibility, suggestMatchingPreset, parseFps, diff --git a/packages/parsers/src/types.ts b/packages/parsers/src/types.ts index 58621d88e..4820ba34f 100644 --- a/packages/parsers/src/types.ts +++ b/packages/parsers/src/types.ts @@ -58,7 +58,8 @@ const RESOLUTION_ALIASES: Record = { * portrait / square compositions with a cryptic "aspect ratio does not match" * error deep inside the render pipeline. Consumers that know the composition's * dimensions can consult this set to decide whether to auto-adapt the preset's - * orientation instead — see `resolveResolutionForComposition`. + * orientation instead — see `adaptAspectAgnosticResolution` in the compile + * stage (`@hyperframes/producer`) and `suggestMatchingPreset` here. * * Orientation-suffixed aliases (`1080p-portrait`, `4k-square`, …) are absent * on purpose: the user *did* pick an orientation, and honoring it is important @@ -97,13 +98,47 @@ export function normalizeResolutionFlag(input: string | undefined): CanvasResolu * * Consumers pair this signal with the composition's dimensions (in a * follow-up pass after HTML parse) to pick the right preset via - * `resolveResolutionForComposition`. + * `suggestMatchingPreset` — see `adaptAspectAgnosticResolution` in the + * compile stage (`@hyperframes/producer`) for the canonical remap. */ export function isAspectAgnosticResolutionAlias(input: string | undefined): boolean { if (!input) return false; return ASPECT_AGNOSTIC_RESOLUTION_ALIASES.has(input.toLowerCase()); } +/** + * Public-boundary helper: given a raw `--resolution` / `--output-resolution` + * flag value, return the pair every distributed render entrypoint needs to + * forward end-to-end so the compile stage can adapt aspect-agnostic aliases + * to the composition's orientation: + * + * - `outputResolution`: normalized {@link CanvasResolution} (or `undefined` + * for unknown values — callers own their invalid-input UX). + * - `outputResolutionAspectAgnostic`: `true` when the raw input was a + * tier-only alias (`1080p` / `hd` / `4k` / `uhd`). Passes through to + * `DistributedRenderConfig.outputResolutionAspectAgnostic` so the compile + * stage remaps `landscape` → `portrait` / `square` when the composition + * dimensions demand it. + * + * Exported to centralize the two-step pattern (`normalizeResolutionFlag` + + * `isAspectAgnosticResolutionAlias`) that would otherwise be duplicated at + * every entrypoint that emits a `DistributedRenderConfig` (`hyperframes + * cloudrun render`, `hyperframes lambda render` / `render-batch`, the local + * CLI). Divergence between those callers is what shipped the portrait-1080p + * regression this helper prevents from recurring. + */ +export interface ResolvedResolutionFlag { + outputResolution: CanvasResolution | undefined; + outputResolutionAspectAgnostic: boolean; +} + +export function resolveResolutionFlagPair(input: string | undefined): ResolvedResolutionFlag { + return { + outputResolution: normalizeResolutionFlag(input), + outputResolutionAspectAgnostic: isAspectAgnosticResolutionAlias(input), + }; +} + export interface TimelineElementBase { id: string; type: TimelineElementType; diff --git a/packages/studio-server/src/routes/render.test.ts b/packages/studio-server/src/routes/render.test.ts index 2b44d4e3d..f3b851d4e 100644 --- a/packages/studio-server/src/routes/render.test.ts +++ b/packages/studio-server/src/routes/render.test.ts @@ -101,6 +101,42 @@ describe("POST /projects/:id/render — outputResolution forwarding", () => { } }); + // Contract pin per PR #2529 R2 review: the HTTP route accepts canonical + // presets only, not the CLI-side tier aliases (`1080p` / `hd` / `4k` / + // `uhd`, `landscape-4k` is already canonical). Miga asked that Studio + // Server's contract stay explicit here rather than silently extending it. + // Aliases fall through to the same `undefined` sink as any unknown value, + // which means the render falls back to composition dimensions — a safe, + // deliberate no-op rather than a portrait-1080p failure mode. + it.each(["1080p", "4k", "hd", "uhd"])( + "does NOT accept the CLI-side tier alias %s (canonical-only contract)", + async (alias) => { + const spy = vi.fn(); + const { app, cleanup } = buildApp(spy); + try { + const res = await app.request("http://localhost/projects/demo/render", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + fps: 30, + quality: "standard", + format: "mp4", + resolution: alias, + }), + }); + expect(res.status).toBe(200); + // Alias falls through to undefined → render uses composition dims. + // If Studio Server ever extends its contract to accept aliases, + // it must also plumb `outputResolutionAspectAgnostic` alongside so + // the compile stage can remap orientation; today the surface is + // deliberately narrow. + expect(spy.mock.calls[0][0].outputResolution).toBeUndefined(); + } finally { + cleanup(); + } + }, + ); + it("accepts each canonical preset value", async () => { for (const preset of VALID_CANVAS_RESOLUTIONS) { const spy = vi.fn();