Merge pull request #2529 from heygen-com/via/resolution-portrait-fix

fix(cli): accept portrait aspects for --resolution alias flag

Author: vanceingalls

.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
@@ -743,6 +766,36 @@
       // work on this same branch (commits 444639d75, b57b31beb, 6f2e9848c,
       // eba8a0fa2), unrelated to the Grade group (Plan 5) currently landing.
       "packages/studio/src/components/editor/propertyPanelSections.tsx",
+      // Portrait --resolution alias fix (via/resolution-portrait-fix):
+      // server.ts `render` (cyclo 10 / CRAP 31.6) and distributed/plan.ts
+      // `plan` (cyclo 33 / CRAP 36.7) are both pre-existing complexity —
+      // the PR only threads `outputResolutionAspectAgnostic` through the
+      // parseRenderOverrides/RenderInput/DistributedRenderConfig shape and
+      // adds one field spread inside `plan`. Neither function body gained
+      // branches, but the line-shift fingerprint re-flags the inherited
+      // complexity. The new re-target logic itself is extracted into
+      // `adaptAspectAgnosticResolution` in compileStage.ts to keep that
+      // 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",
     ],
   },
 }

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<string, unknown> = {
+    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\]/);
+  });
+});

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<string, unknown>): 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<string, unknown>,
   fps: number,
   width: number,
   height: number,
   variables: Record<string, unknown> | undefined,
 ): Record<string, unknown> {
+  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 ─────────────────────────────────────────────────────────

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,14 +304,16 @@ export default defineCommand({
           process.exit(1);
         }
         const { runRender } = await import("./lambda/render.js");
+        const renderResolution = parseOutputResolution(args["output-resolution"]);
         await runRender({
           projectDir,
           stackName,
           siteId: args["site-id"] as string | undefined,
           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",
+  });
 }

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();
+  });
+});