feat(core): slideshow schema, parser, and lint rule

[vanceingalls]

Slideshow mode — 1/5: core schema, parser & lint

Foundation for slideshow mode: a composition can declare an embedded slideshow manifest that turns its continuous timeline into a discrete, navigable deck. This PR adds the data model, parser, and validation — no runtime/UI yet.

What & why

A slide is just an existing scene (data-composition-id + data-start/data-duration) plus metadata declared in one embedded <script type="application/hyperframes-slideshow+json"> island. Keeping the manifest in the composition means no new file format and no build step — slides are additive metadata over a normal composition.

Key changes

• slideshow/slideshow.types.ts — SlideshowManifest, SlideRef, SlideHotspot, SlideSequence and their resolved counterparts. TTS fields (ttsScript/ttsAudioUrl/ttsDurationMs) are present but reserved (playback not built).
• slideshow/parseSlideshow.ts — parseSlideshowManifest(html) extracts the island; resolveSlideshow(manifest, scenes) resolves each sceneId to a {start,end} range (honouring optional startTime/endTime overrides) and returns validation errors for: unresolved sceneId, fragment outside a slide's range, hotspot targeting an unknown sequence, and overlapping main-line slides.
• lint/rules/slideshow.ts — surfaces those resolve errors under hyperframes lint; derives scenes from data-composition-id (matching the runtime's scene source).
• lint/rules/core.ts — exempts the slideshow island MIME type from the inline-script-syntax check.
• ./slideshow subpath export (dev + publishConfig) so downstream packages import only the lightweight parser, keeping core's Node-only barrel out of their typecheck graph.

Testing

parseSlideshow.test.ts + slideshow.test.ts (vitest) cover parse, resolution, every error path, and the lint rule.

Stack

Bottom of a 5-PR stack: core → player (#1581) → studio (#1582) → skill (#1583) → examples (#1584).

🤖 Generated with Claude Code

[vanceingalls]

• feat(core): slideshow schema, parser, and lint rule #1580 : 2 dependent PRs (#1581 , #1589 ) 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miga-heygen]

Review: feat(core): slideshow schema, parser, and lint rule

Solid foundation PR. The "slides as metadata over existing scenes" design is clean — embedding the manifest as a typed JSON island inside the composition HTML avoids a new file format and keeps slides purely additive. The parser, resolver, and lint rule are well-structured and test coverage is thorough. A few things worth discussing before this merges as the base of a 10-PR stack:

---

1. Schema — types look good, one design question

The types are well-defined and properly constrained. SlideRef → ResolvedSlide progression is clean: optional fields become required, arrays get defaults. The extends SlideRef on ResolvedSlide is the right call — it means downstream consumers always get the superset.

Question on SlideHotspot.region: The { x, y, w, h } comment says "% of slide" — should this be enforced (0-100 range validation) in the resolver, or is that intentionally deferred to the player PR? If deferred, a // TODO: validate bounds in player comment would help, since this type will be the contract for 4 downstream PRs.

TTS fields: Flagging ttsScript/ttsAudioUrl/ttsDurationMs as "reserved — parsed and carried, never consumed" is the right approach. Consider adding @experimental or @reserved JSDoc tags so IDE tooltips surface this intent — right now it's only documented in the PR description, which evaporates from working memory fast.

2. Parser — solid, one edge case

Regex extraction (parseSlideshowManifest): The regex approach is consistent with how the linter extracts scripts elsewhere. The escaping of . and + in the MIME type is correct.

Edge case — multiple islands: RegExp.exec with no g flag returns the first match. If someone accidentally pastes two <script type="application/hyperframes-slideshow+json"> blocks, the second is silently ignored. Worth either (a) erroring on >1 match, or (b) documenting single-island-only as an intentional constraint. Given this is the foundation PR, establishing this invariant now prevents confusion in downstream PRs.

isManifest validator: This only checks typeof === "object" and Array.isArray(slides). It does not validate that each element of slides is a valid SlideRef (e.g., has a sceneId string). A manifest like {"slides": [42, null, "oops"]} passes isManifest and then blows up inside resolveSlide with an unhelpful error. Consider at minimum checking slides.every(s => typeof s?.sceneId === 'string').

3. Resolver — clean logic, two notes

Overlap detection: The adjacent-pair comparison after sorting by start is correct for detecting overlaps. One nuance: if two slides share the exact same start, the sort is not stable on end, so which one is "prev" vs "curr" is arbitrary. This doesn't affect correctness (they overlap either way), but the error message might name them in a surprising order. Not blocking, just noting.

Fragment validation boundary condition: f < start || f > end — a fragment exactly at end is allowed. Is that intentional? Fragments at the exact boundary of a slide's end time are technically at the transition point. If fragments represent "pause points within a slide," having one at the very last moment seems odd. If it's intentional, a comment clarifying the inclusive-end semantics would help.

4. Lint rule — well-integrated, one concern

Scene extraction uses data-composition-id only (not .clip[id]): This is explicitly tested and correct per the PR description — it mirrors the runtime's scene source. The test at line 29-35 of slideshow.test.ts validating that .clip[id] does NOT resolve is a great inclusion.

Concern — isSceneLikeCompositionId is duplicated: The comment says it "mirrors" packages/core/src/runtime/timeline.ts, but it's a full copy. If the runtime's heuristic changes (e.g., a new exclusion pattern), the lint rule silently diverges. Options: (a) import the canonical function, (b) add a // SYNC: comment pointing to the source of truth, (c) extract to a shared util. For a foundation PR, at minimum (b) would be good.

5. API surface

The ./slideshow subpath export is the right call — keeping the parser importable without pulling in the full core barrel is good for downstream bundle size. The re-export from src/index.ts provides convenience for consumers who already depend on core.

6. Security

No injection risk. The manifest is parsed from HTML the author controls, validated through JSON.parse (which is safe), and the regex only extracts content between matched script tags. No eval, no dynamic code execution, no user-supplied HTML interpretation. Clean.

7. Minor nits

• slideshow.ts line 1: // fallow-ignore-file code-duplication — is "fallow" the project's lint-ignore directive? Just making sure this isn't a typo for "eslint-disable" or similar.
• The parseTiming helper in the lint rule re-implements timing extraction that resolveTimeRange also handles. Not a problem now, but as the stack grows, consider whether the lint rule should delegate more to the parser rather than re-deriving scene data.

---

Verdict

This is a well-designed foundation. The schema is minimal and extensible, the parser handles edge cases thoughtfully (partial overrides, missing scenes), the lint rule integrates cleanly with the existing linter architecture, and test coverage is comprehensive (13 test cases covering happy paths and all error variants).

The isManifest shallow validation and the duplicated isSceneLikeCompositionId are the two items I'd want addressed before this merges — everything else is nits or "consider for a follow-up."

Not stamping — leaving for the author to address the above, then happy to re-review.

Review by Miga

[miguel-heygen]

LGTM

[james-russo-rames-d-jusso]

Reviewed at 778e53500acdeeb51fddfbd9f43f514127fede91.

Verdict: clean foundation design — schema/parser/resolver shape is right, test coverage on resolve paths is good. Layering onto Miga's review (who covered isManifest shallow validation, isSceneLikeCompositionId drift, multi-island silent ignore, fragment boundary, region bounds, TTS doc tags). A few things they didn't surface that matter for this being the base of a 10-PR stack.

---

🛑 Blockers

None.

⚠️ Concerns

1. Lint findings carry no location info — observability gap for the rule's whole job.
packages/core/src/lint/rules/slideshow.ts:55-78 — every finding the slideshow rule emits has only code + message + fixHint. No elementId, no selector, no snippet. Every other rule in packages/core/src/lint/rules/ populates at least one of these (composition.ts:128-136, core.ts, media.ts, gsap.ts, textures.ts all do). When a hyperframes lint user gets Slideshow manifest error: slide references unresolved sceneId "ghost", they don't know whether the offender is in the main-line slides array or in slideSequences[2].slides[4]. For a manifest with 20 slides across 3 sequences, this turns a one-line fix into a hunt. Suggest: thread a path-like locator (slides[3] / slideSequences["deep"].slides[1]) through resolveSlide errors, and populate the island's <script> tag location as snippet (extract via ctx.scripts.find(s => /type=["']application\/hyperframes-slideshow\+json["']/.test(s.attrs))).

2. Duplicate sequence id silently overwrites — wire-contract bug for hotspots.
parseSlideshow.ts:117-123 — for (const seq of manifest.slideSequences ?? []) { sequences[seq.id] = ... }. If two sequences share an id, the second wins, and any earlier hotspots that targeted that id now resolve to the wrong branch with no error. Since hotspot target → sequence is the only navigation primitive in this schema, a silent collision corrupts the deck. Three-line fix:

if (sequences[seq.id]) {
  errors.push(`duplicate slideSequence id "${seq.id}"`);
  continue;
}
sequences[seq.id] = { ... };

3. Lint rule re-parses ctx.source instead of using the centralized ctx.scripts extractor.
packages/core/src/lint/rules/slideshow.ts:53 calls parseSlideshowManifest(ctx.source), which builds its own regex against the full HTML source. But ctx.scripts already contains every <script> block extracted by the centralized SCRIPT_BLOCK_PATTERN in packages/core/src/lint/utils.ts:21. Two failure modes from running parallel extractors:

• The two regexes can diverge subtly (the central one uses <script\b([^>]*)>, the new one uses <script[^>]*type=["']…["'][^>]*>). A script tag whose attribute is unquoted (type=application/hyperframes-slideshow+json) matches the central extractor but not the new one.
• When Miga's "multiple islands" concern gets addressed, the right loop body is for (const block of ctx.scripts.filter(s => …)) — that's a free fix if you migrate to ctx.scripts now.
  Suggest: filter ctx.scripts by MIME type in the rule, JSON.parse the content, and drop the duplicate regex from parseSlideshowManifest entirely (or keep it as the non-lint extractor for direct consumers).

4. ISLAND_TYPE MIME constant duplicated across files.
parseSlideshow.ts:10 defines ISLAND_TYPE = "application/hyperframes-slideshow+json". The same string appears verbatim in lint/rules/core.ts:167 (the exemption regex), lint/rules/slideshow.ts:60 (the fix-hint message), all four test files (literal strings in HTML fixtures), and the file-level comment block. With 4 downstream PRs (player, studio, skill, examples) about to import this layer, the chance that someone tweaks the MIME type in one place and not another goes up. Suggest exporting ISLAND_TYPE from slideshow.types.ts (or a new slideshow/constants.ts) and importing it everywhere — including the regex literal in core.ts:167 (use new RegExp against the imported constant).

💡 Nits

5. Empty-slides manifest silently passes.
parseSlideshow.ts:125 — manifest.slides.map(...) = [], no errors. A manifest with {"slides": []} (or one whose only slides got removed) lints clean but is nonsense for a slideshow. Probably fine to leave for the player layer to handle ("0-slide composition = no slideshow surface"), but if resolveSlideshow.errors is meant to be the exhaustive validation gate before player accepts a manifest, worth a slides array is empty finding here.

6. Sequence slides aren't overlap-checked.
parseSlideshow.ts:138-145 overlap check applies only to slides (main-line), not slideSequences[*].slides. Reasonable — sequences are branches that may interleave with main-line time — but worth a one-line comment clarifying intent so the player author doesn't assume sequences are also non-overlapping.

7. // fallow-ignore-file code-duplication at slideshow.ts:1.
Effectively asking the repo's Fallow audit CI (which is passing) to skip flagging the isSceneLikeCompositionId copy. That's the right call short-term, but it means Miga's "SYNC comment" suggestion is doubly important — the fallow-ignore explicitly suppresses the one signal that would catch divergence.

✅ Looks good

• The ./slideshow subpath export design — agreed with Miga, keeps player/studio bundles thin.
• resolveTimeRange partial-override semantics + tests at parseSlideshow.test.ts:80-129 are exemplary; the "no scene + only one bound" error path is exactly the kind of nuance authors hit.
• TTS reservation as parse-and-carry — defers cleanly without polluting the resolver.
• core.ts:167 MIME exemption from inline-script-syntax check — necessary and minimal.
• 13 tests covering happy path + every error variant. Solid.

Stack-coherence note

This is the foundation 4 downstream PRs import from. Concerns 1 (location info), 2 (sequence id collision), and 4 (MIME constant) all get harder to fix once player/studio start importing ResolvedSlideshow.sequences / parseSlideshowManifest. Cheap to address now, expensive later.

Not stamping — Miga's concerns + the above. Happy to re-review.

Review by Rames D Jusso

[miga-heygen]

Re-review: checking prior findings

Re-reviewing against 778e535 — the same (and only) commit. No follow-up pushes since the original reviews from Miga and Rames.

Status of previously flagged items

1. Shallow isManifest() validation — STILL OPEN

parseSlideshow.ts isManifest() still only checks typeof === "object" and Array.isArray(slides). A manifest like {"slides": [42, null, "oops"]} passes validation and then blows up inside resolveSlide with an unhelpful error (trying to read .sceneId off a number). Minimum fix: slides.every(s => typeof s?.sceneId === 'string').

2. Duplicated isSceneLikeCompositionId — STILL OPEN

lint/rules/slideshow.ts still contains a full copy of isSceneLikeCompositionId from runtime/timeline.ts. The comment says "Mirrors" but there's no sync mechanism — no import, no // SYNC: marker, no shared util. The fallow-ignore-file code-duplication directive suppresses the one CI signal that would catch divergence. At minimum, add a // SYNC: runtime/timeline.ts:isSceneLikeCompositionId comment so grep can find both copies. Extracting to a shared util would be better, especially with 4 downstream PRs about to build on this.

3. Sequence-id collision — STILL OPEN (originally flagged by Rames)

parseSlideshow.ts:117-123 still does sequences[seq.id] = { ... } with no collision check. Duplicate sequence IDs silently overwrite earlier entries, which means hotspots targeting that ID resolve to the wrong branch with no error. Three-line fix:

if (sequences[seq.id]) {
  errors.push(`duplicate slideSequence id "${seq.id}"`);
  continue;
}

Summary

All three items from the prior round remain unaddressed — no new commits have been pushed since the original reviews. Waiting on a follow-up push before re-evaluating.

Re-review by Miga