Skip to content

fix(render): allow explicit entry without index#2307

Merged
miguel-heygen merged 3 commits into
mainfrom
fix/render-composition-no-index-stack
Jul 13, 2026
Merged

fix(render): allow explicit entry without index#2307
miguel-heygen merged 3 commits into
mainfrom
fix/render-composition-no-index-stack

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Summary

  • let render --composition <file> resolve a valid project directory even when root index.html is absent
  • use the explicit entry for lint, variables, batch validation, fps, slideshow, and resolution preflight
  • pass the absolute entry path into the explicit-entry lint contract from fix(render): scope strict lint to composition #2261
  • preserve the existing index.html requirement for every default render

Relationship

Stacked on #2261 because that PR introduces explicit-entry lint scoping. This follow-up fixes the earlier project-discovery gate and its relative-vs-absolute lint call.

Reproduction

npx hyperframes@0.7.55 render -c "Branching Narratives 02 - The Knock.dc.html" ... in a directory containing that file but no index.html exits before resolving -c, despite help saying the flag renders a specific file instead of index.html.

Verification

  • bun run --cwd packages/cli test --run src/utils/project.test.ts src/utils/renderArgs.test.ts src/utils/lintProject.test.ts (102 passed)
  • bun run --cwd packages/cli typecheck
  • source CLI rendered the standalone .dc.html with no index to H.264; ffprobe: 320x180, 24fps, 1.000s
  • pre-commit lint/format/typecheck hooks passed

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed at 2ee0ba79.

Clean follow-up to #2261. The three changes compose well:

  1. resolveProjectOrThrow's requireIndex option — additive, defaults to backwards-compat (require). All other callers (beats.ts:72, benchmark.ts:61, check.ts:121, cloud/render.ts:512,540, compositions.ts:177, info.ts:56, etc.) call resolveProject(args.dir) positionally, so they keep the strict gate. Only render.ts opts into the relaxed mode. Good scoping.
  2. renderTarget hoisted to a single site at render.ts:403 — replaces two inline duplications in the slideshow probe (line 719) and resolution preflight (line 854). Same expression entryFile ? resolve(project.dir, entryFile) : project.indexPath now consumed by batch (indexPath: renderTarget at 701), slideshow, lint (entryFile ? renderTarget : undefined at 818), resolution preflight, and variables (validateVariablesAgainstProject(renderTarget, variables) at 945). One primitive, five consumers — no drift possible.
  3. explicitEntry = entryFile ? renderTarget : undefined — now an absolute path, matching the "absolute" contract Miguel's comment at 815 documents. lintProject in #2261 resolves both cases (resolve(entryFile) is idempotent on absolutes), so the contract change is consistent.

Nits

  • 🟡 The two composition-related guards use slightly different heuristics:

    • hasExplicitComposition at line 393 checks 4 sentinels: != null, !== "", !== ".", !== "./". Feeds resolveProject({ requireIndex: !hasExplicit }).
    • explicitEntry at 818 uses entryFile ? renderTarget : undefined — relies on resolveCompositionEntryArg returning undefined when composition is empty/default.

    If resolveCompositionEntryArg and hasExplicitComposition's inline logic ever diverge (e.g. a whitespace-only --composition " "hasExplicit would trim to "" and treat as false; resolveCompositionEntryArg doesn't trim in the same way), you'd get requireIndex: true but a non-null entryFile, or the reverse. Neither is fatal (fails downstream on readFileSync), just an internal-consistency wart. One shared hasExplicitComposition derived once, used everywhere, would collapse this. Doesn't need to happen in this PR.

  • 🟡 Test coverage: the new resolveProjectOrThrow test proves requireIndex: false accepts a bare directory. But there's no end-to-end test in render.ts (or in project.test.ts) that covers "no index.html + explicit composition → render succeeds." Reasonable to leave for integration coverage; ratchets down testing surface area.

LGTM. Stack looks good end-to-end.

Review by Rames D Jusso

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

unify the explicit-composition sentinel/entry logic and cover no index.html + explicit composition render

Addressed in 9577441c1 (with base update 7bc421357): sentinel normalization now lives in renderArgs.ts and is shared by project gating and entry parsing. Added a command-level regression that creates a project with only standalone.html, runs the render command through the mocked producer, and verifies the render job receives that entry. Evidence: render/renderArgs/project suites 83/83; CLI typecheck; targeted format check.

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

R2 reviewed at 9577441c, delta from R1 head 2ee0ba79 (base #2261 also refreshed in this stack).

R1 nit on hasExplicitComposition vs parseCompositionEntryArg divergence cleanly addressed:

  • One classifier, no whitespace divergence — extracted normalizeCompositionEntryArg + hasExplicitCompositionArg at packages/cli/src/utils/renderArgs.ts:131 and reused from render.ts:392. Both call sites now go through the same .trim().replace(/^\.\//, "") + "." → undefined normalization, so pathological inputs (" ", "./", ".") can't split behavior between "gate the strict lint" and "resolve the entry file" anymore.
  • Test locks the invarianthasExplicitCompositionArg test explicitly enumerates [undefined, "", " ", ".", "./"] all → false and "./compositions/intro.html"true. Plus the new render-command integration test at render.test.ts:1071 renders an explicit composition from a project with no index.html, so the strict-mode bypass is end-to-end verified.

LGTM from my side.

Review by Rames D Jusso

jrusso1020
jrusso1020 previously approved these changes Jul 13, 2026

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving on Rames-D's go (R2 delta clean, nit round addressed) + Magi's CI-green confirmation. Verified at final head 9577441c: CI green, no changes-requested, mergeable.

Base automatically changed from fix/strict-composition-lint-scope to main July 13, 2026 02:18
@miguel-heygen miguel-heygen dismissed jrusso1020’s stale review July 13, 2026 02:18

The base branch was changed.

@miguel-heygen miguel-heygen force-pushed the fix/render-composition-no-index-stack branch from 9577441 to 716adc6 Compare July 13, 2026 02:21

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

R3 reviewed at 716adc60, post-rebase onto merged main. Delta from R2 head 9577441c.

Most of the child diff is unchanged from R2, but the rebase does introduce one substantive delta at the pre-render lint call site that's worth calling out because it silently fixes a hidden interaction with the just-merged #2261 guard:

Pre-render lint now threads the absolute renderTarget instead of the project-relative entryFile:

// lintProject's explicit-entry contract is an absolute source path;
// entryFile is project-relative for the producer.
const explicitEntry = entryFile ? renderTarget : undefined;

Traced through: lintProject(projectDir, entryFile) does indexPath = resolve(entryFile) — for a relative entry that resolves against CWD, not projectDir. Combined with #2261's newly-landed isWithinProjectRoot(projectDir, indexPath) guard, the pre-render lint would have thrown Explicit lint entry is outside the project directory on every explicit-composition render invoked from any working directory other than the project root. R3's absolute-path threading avoids that regression entirely.

Bonus: the same rebase collapses the two inline renderTarget recomputations under slideshow-probe and resolution-preflight so the hoisted renderTarget at render.ts:397 is now the single source of truth (which also fixed variables validation at line 941 — validateVariablesAgainstProject(project.indexPath, ...)validateVariablesAgainstProject(renderTarget, ...); the old code was silently validating variables against index.html even when rendering an explicit entry).

Everything else — hasExplicitCompositionArg classifier, normalizeCompositionEntryArg shared normalization, resolveProjectOrThrow(dirArg, { requireIndex }), both new tests — is byte-identical to R2.

LGTM from my side.

Review by Rames D Jusso

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

R4 reviewed at e3e847c1, delta from R3 head 716adc60.

One-line change: raises the per-test timeout on the new renders an explicit composition from a project with no index.html test to 60s. Scoped to that one integration test — every other assertion in the file keeps Vitest's default, so this doesn't broadly loosen the timeout budget. Matches the failure signature Magi described (test-only timeout under full parallel monorepo load, no assertion failure).

LGTM from my side.

Review by Rames D Jusso

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-approving on Rames-D's R4 go. Final head e3e847c1 fully green (earlier reds were GHA infra + an unrelated Windows-load test timeout, all rerun clean per Magi); the only delta from R3 is the one-line 60s test-timeout bump. No changes-requested, mergeable.

@miguel-heygen miguel-heygen merged commit 796d5df into main Jul 13, 2026
62 of 71 checks passed
@miguel-heygen miguel-heygen deleted the fix/render-composition-no-index-stack branch July 13, 2026 03:01
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