Skip to content

feat(cli): add --resolution flag to hyperframes init for 4k scaffolding#661

Open
jrusso1020 wants to merge 2 commits intomainfrom
05-07-feat_cli_add_--resolution_flag_to_hyperframes_init_for_4k_scaffolding
Open

feat(cli): add --resolution flag to hyperframes init for 4k scaffolding#661
jrusso1020 wants to merge 2 commits intomainfrom
05-07-feat_cli_add_--resolution_flag_to_hyperframes_init_for_4k_scaffolding

Conversation

@jrusso1020
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 commented May 7, 2026

What

Adds a --resolution flag to hyperframes init that scaffolds projects at the chosen canvas preset: landscape (1920×1080), portrait (1080×1920), landscape-4k (3840×2160), or portrait-4k (2160×3840). Aliases: 1080p, 4k, uhd.

hyperframes init my-video --resolution 4k --example blank --non-interactive

Why

PR #660 added 4K to the CanvasResolution type system but every bundled and remote template still ships data-width="1920" data-height="1080". Without this flag, the only way to author at 4K is to hand-edit the scaffolded HTML in five places (data-width, data-height, data-resolution, the inline html, body { width/height } CSS, and the <meta name="viewport"> tag). This is PR 2 of the 4K stack.

Stacked on #660.

How

New applyResolutionPreset(destDir, resolution) exported from init.ts walks every .html file in the scaffold and rewrites the dimension fingerprint by regex. Regex (not DOM-parsing) preserves template comments and indentation byte-for-byte — the scaffolded HTML is hand-authored content that templates author for review-friendly diffs.

The function is invoked from scaffoldProject() after patchVideoSrc() and writeTailwindSupport() so it composes cleanly with existing flags (--video, --tailwind).

Flag wiring:

  • Citty arg accepts --resolution <preset> with the alias map applied via normalizeResolutionFlag(). Unknown values exit non-zero with a clear error before any directory is created.
  • Both interactive and non-interactive scaffold paths thread the preset into scaffoldProject().

Test plan

  • Unit tests added/updated — 4 new tests in init.test.ts:
    • applyResolutionPreset rewrites every dimension fingerprint for landscape-4k
    • applyResolutionPreset swaps to portrait dimensions for portrait-4k
    • End-to-end hyperframes init --resolution 4k --example blank --non-interactive produces a project with data-width="3840", data-height="2160", data-resolution="landscape-4k", and 4K CSS
    • Unknown --resolution 8k exits non-zero, no directory created
  • Manual testing performed — bun run --cwd packages/cli test src/commands/init.test.ts (11/11 pass)
  • Documentation updated — docs/packages/cli.mdx has the new flag in the init table

Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approve. Reads cleanly, the regex-based rewriter is the right call for templates (preserves comments and indentation), tests cover both unit and end-to-end paths, and the unknown-flag rejection happens before any directory is created. Validation-up-front + bail-cleanly is the correct UX.

Important

  • importantpackages/cli/src/commands/init.ts:51RESOLUTION_ALIASES has "portrait-4k": "portrait-4k" — that's a self-map that does nothing because VALID_RESOLUTIONS already matches the canonical name first. (#665 removes it during dedup.) Harmless today, but it's symptomatic — see the cross-PR comment below.

  • importantpackages/cli/src/commands/init.ts:489-510applyResolutionPreset regex-walks every HTML file and rewrites dimension fingerprints, but a template that only declares dimensions in an external .css file will silently keep 1080p #stage styles. Templates today seem to inline their CSS (sample fixture in the test confirms), but that's a template-author convention, not a guaranteed invariant. Either narrow the doc comment ("inline-CSS templates only — external stylesheets are not patched") or also walk .css siblings. Add a test that exercises a fixture with no dimension fingerprint at all (the function should be a no-op, not error).

  • importantpackages/cli/src/commands/init.ts:476-486bodyCssRe requires width: to appear before height: and within the same html, body { ... } block. Hand-authored CSS where the order is reversed (height: 1080px; width: 1920px;) will silently not be rewritten. The htmlParser (PR #660) already learned both orderings via stageMatchReverse; this should too. Add a fixture test with reversed-order CSS — it'll demonstrate the gap.

Nits

  • nitpackages/cli/src/commands/init.ts:485viewportRe only matches when width= precedes height= in the content attribute. Same ordering caveat as above; mostly cosmetic since <meta viewport> ordering is conventional.

  • nitpackages/cli/src/commands/init.ts:445 — JSDoc lists 5 fields rewritten; if any future template adds a sixth (e.g. an aria-label with dims, an OG image dim), this comment goes stale. Worth a short "this list is enforced by tests, not by code" note.

  • nitpackages/cli/src/commands/init.ts:495-498dataWidthRe has /g and uses .test() followed by .replace(). With /g, .test() advances lastIndex and the subsequent .replace() is fine because it's a fresh call, but the pattern is fragile if anyone refactors to reuse the same regex object. Either drop the /g from the test (and use .match()) or drop the test and just rely on .replace() (which is a no-op when nothing matches).

  • nitpackages/cli/src/commands/init.ts:64normalizeResolutionFlag lowercases and looks up — fine. Worth confirming no Windows-y casing issue with --resolution 4K (uppercase K). Looks like .toLowerCase() covers it, but a one-line test would pin it.

Cross-PR

The duplication of VALID_RESOLUTIONS + RESOLUTION_ALIASES + normalizeResolutionFlag between init.ts (this PR) and render.ts (#663) is exactly what #665 has to undo. The right shape would have been to land #660 with normalizeResolutionFlag already exported from @hyperframes/core, then both CLI files consume it. The drift you found ("portrait-4k": "portrait-4k" in init only) is the canonical signal. Stack hygiene nit, not a blocker for this PR — just calling it out so the team's next foundation PR ships the helpers.

Praise

The applyResolutionPreset → scaffoldProject ordering (after patchVideoSrc, after writeTailwindSupport) composes cleanly with existing flags. The "exit before any work on bad input" pattern is correct. End-to-end test with --example blank --resolution 4k is exactly the right level of coverage.

— Vai

Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes because --resolution scaffolding can produce internally inconsistent projects for real examples.

applyResolutionPreset() rewrites data-width / data-height, data-resolution, one very specific html, body { width; height; } CSS shape, and a width=..., height=... viewport. Registry examples are not all shaped like the blank template. For example, warm-grain uses:

  • body, html { width: 1920px; height: 1080px; }
  • #main-composition { width: 1920px; height: 1080px; }
  • <meta name="viewport" content="width=device-width, initial-scale=1.0" />

I reproduced this on the stacked head by applying applyResolutionPreset(..., "landscape-4k") to registry/examples/warm-grain: the data-width / data-height became 3840x2160, but the body and main composition CSS stayed 1920x1080. A generated project then declares 4K metadata while still laying out at 1080p.

Please broaden the rewrite to handle the actual template shapes we ship, and add a regression test against at least one registry-style example such as warm-grain.

I rechecked the live head before posting: dbbef868187a04562a96d862719e333631275df9.

@jrusso1020 jrusso1020 force-pushed the 05-07-feat_cli_add_--resolution_flag_to_hyperframes_init_for_4k_scaffolding branch from dbbef86 to cb0abed Compare May 7, 2026 05:48
vanceingalls
vanceingalls previously approved these changes May 7, 2026
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review — APPROVE

Verdict shift: COMMENT (2 important) → APPROVE.

Reference: prior review #661 (review)

Addressed since prior review (commit cb0abedf)

  • Important — reverse-order CSS: packages/cli/src/commands/init.ts:497 — added bodyCssReverseRe to handle height:Xpx … width:Ypx ordering in addition to width-first. Both are tried in sequence.
  • Test for it: packages/cli/src/commands/init.test.ts:281rewrites height-before-width inline CSS explicitly covers the reversed-order path.
  • External .css walking: deliberately scoped out and now documented in the function-level JSDoc (init.ts:419-424) — "Templates whose #stage dimensions live in an external .css stylesheet are not patched … the bundled blank template inlines its CSS." Acknowledged + documented is acceptable here; the docs warn template authors who might break the contract.
  • Test breadth: also added uppercase --resolution 4K acceptance, no-op-on-fragment safety, and unknown-value rejection (init.test.ts).

Still open

  • Dead self-map alias "portrait-4k": "portrait-4k": this PR still has the local RESOLUTION_ALIASES table with the self-map. PR #665 deletes the local copy entirely and routes through normalizeResolutionFlag exported from @hyperframes/core — so this gets cleaned up by the next PR in the stack as predicted in the prior review. Not a blocker on this PR.

New findings

None.

— Vai

miguel-heygen
miguel-heygen previously approved these changes May 7, 2026
Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All issues from round 1 addressed. Verified in re-review. — Magi

@jrusso1020 jrusso1020 changed the base branch from 05-07-feat_core_add_4k_canvas_resolution_presets to graphite-base/661 May 7, 2026 06:09
jrusso1020 added a commit that referenced this pull request May 7, 2026
## What

Adds `landscape-4k` (3840×2160) and `portrait-4k` (2160×3840) presets to `CanvasResolution` and `CANVAS_DIMENSIONS` in `@hyperframes/core`. Foundation for end-to-end 4K rendering support.

## Why

There is no codified way today to mark a composition as 4K. The string union `CanvasResolution = "landscape" | "portrait"` is the only enum used by templates, generators, and stage-zoom math; without 4K members, scaffolds and helpers always emit 1080p dimensions even when the underlying engine + encoder pipeline can already handle larger viewports (Chrome `setViewport`, ffmpeg `libx264`/`libvpx-vp9`/`prores_ks` all scale fine).

This is PR 1 of a 3-PR stack making 4K a first-class option:

1. **PR #660 (this)** — core types/constants + parser detection.
2. **PR #661** — `hyperframes init --resolution 4k` flag to scaffold 4K projects.
3. **PR #662** — byte-budget the frame data-URI cache so 4K renders don't OOM.

## How

- `CanvasResolution` extended additively (no breaking change).
- `CANVAS_DIMENSIONS` gains `landscape-4k` / `portrait-4k` entries.
- `parseResolutionFromHtml` accepts `data-resolution="landscape-4k|portrait-4k"` and infers 4K from `data-composition-width`/`data-composition-height` (long side ≥ 2560 → UHD variant).
- `parseResolutionFromCss` reuses the same dimension-aware classifier so inline `#stage { width/height }` styles are detected too.
- Helper extracted: `resolveResolutionFromDimensions(w, h)`.
- `cli/info.ts` cleanup: replaces a nested `parsed.resolution === "portrait" ? 1080 : 1920` ternary with a `CANVAS_DIMENSIONS[parsed.resolution]` lookup so it stays correct for 4K.

Stage-CSS generators (`templates/base.ts`, `generators/hyperframes.ts`) already index `CANVAS_DIMENSIONS[resolution]`, so they pick up the new presets automatically.

## Test plan

- [x] Unit tests added/updated — 4 new parser tests, 1 expanded constants test
- [x] Manual testing performed — `bun run --cwd packages/core test` (679 pass), `bun run --cwd packages/cli test` (277 pass)
- [ ] Documentation updated (deferred to PR #661 where the user-facing flag lands)
@jrusso1020 jrusso1020 force-pushed the 05-07-feat_cli_add_--resolution_flag_to_hyperframes_init_for_4k_scaffolding branch from cb0abed to bced546 Compare May 7, 2026 06:09
@graphite-app graphite-app Bot changed the base branch from graphite-base/661 to main May 7, 2026 06:10
@graphite-app graphite-app Bot dismissed stale reviews from miguel-heygen and vanceingalls May 7, 2026 06:10

The base branch was changed.

@jrusso1020 jrusso1020 force-pushed the 05-07-feat_cli_add_--resolution_flag_to_hyperframes_init_for_4k_scaffolding branch from bced546 to 9f0074e Compare May 7, 2026 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants