fix(cli): wire aspect-agnostic resolution through cloudrun/lambda/batch + preflight recompute

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 <noreply@anthropic.com>

— Via
This commit is contained in:
Via
2026-07-16 08:01:56 +00:00
co-authored by Claude
parent 7e58d050f8
commit 2d398ed274
18 changed files with 707 additions and 66 deletions
@@ -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");
@@ -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");
}
@@ -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}`,
);
}