Skip to content

Commit b2fc18b

Browse files
fix(skills,lint): correct composition-contract claims the code contradicts (#3468)
The runtime absorbed a series of authoring mistakes over time and `runtime/init.ts` says so in its own comments, but the skills kept teaching the old rules. Four of them actively cost an agent a failing run: add `crossorigin` (lint rejects it unconditionally), never build a timeline inside `async` (lint calls that the documented contract), never `gsap.set` later-scene clips (two fixHints instruct exactly that), and 12 copyable media snippets with no `id`, which render silent. Corrected in every place each claim appeared, including `hyperframes-animation`, three workflow scripts, the scaffolded project instructions, the CLI `docs` command, and the public docs site: `data-track-index` is a Studio display lane the render never reads, `class="clip"` is a layout convention rather than a visibility requirement, timed elements may nest, the visibility window is half-open, sub-composition host dimensions are backfilled, and the root-fill rule applies only to the layered-composite path. Behaviour changes, each backed by a render rather than by reading code: - `timeline_registry_missing_init` deleted. The runtime creates the registry before any inline script; a composition without the guard line renders and animates correctly. - `video_nested_in_timed_element` kept, message corrected. A rendered repro shows the nested-with-local-start case really does break, so the rule guards a real defect, but nothing is "FROZEN": the extractor ignores the wrapper's offset while visibility uses it, so the clip shows wrong frames and then vanishes. - `mediaRenderIds` now stamps media whose source is a `<source>` child, closing a duplicate-id gap the old `[src]`-only selector left open. - Stale messages fixed on `subcomposition_root_styled_by_class` and `deprecated_data_layer`. `coreSkillContent.test.ts` pinned the literal sentence that made root `data-start` look required, so it is narrowed to structure plus the regression it genuinely catches. Not covered, and flagged in the PR: the media global-vs-local start heuristic in `runtime/init.ts` is the root cause behind the nested-video defect. Removing it changes the meaning of existing compositions and needs its own deprecation.
1 parent 3e17ddc commit b2fc18b

31 files changed

Lines changed: 288 additions & 150 deletions

docs/concepts/data-attributes.mdx

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,20 @@ a start, a duration, and a track:
1717
| ------------------ | ------------------------------------------------ |
1818
| `data-start` | When the element enters the composition timeline |
1919
| `data-duration` | How long its timeline slot lasts |
20-
| `data-track-index` | Which timeline lane owns that slot |
20+
| `data-track-index` | Which Studio lane displays it (optional; the render ignores it) |
2121

22-
Add `class="clip"` to timed DOM and image elements so the runtime can control
23-
their visibility. Video visibility is managed by the media runtime; audio has
24-
no visual lifecycle.
22+
Add `class="clip"` to timed DOM and image elements. The runtime keys visibility
23+
off `data-start` rather than the class, but the shared `.clip` rule is what gives
24+
a scene its full-frame box. Video visibility is managed by the media runtime;
25+
audio has no visual lifecycle.
2526

2627
## Tracks are not layers
2728

28-
Tracks prevent time ranges from colliding. They do not decide which element is
29-
in front. Use CSS `z-index` for paint order.
29+
Tracks are the rows Studio draws. They do not decide which element is in front,
30+
and they do not schedule anything. Use CSS `z-index` for paint order.
3031

31-
Two clips on one track cannot overlap. Put an intentional overlap, such as a
32-
crossfade, on separate tracks:
32+
Two clips on one track may overlap; the render composites both. Separate tracks
33+
keep an intentional overlap, such as a crossfade, readable in Studio:
3334

3435
```html
3536
<video

docs/reference/html-schema.mdx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,12 @@ unbounded animation, and timeline-free compositions need an explicit duration.
9797
| `id` | Yes | Stable identifier for timing, editing, and animation |
9898
| `data-start` | Yes | Start in seconds or a relative timing expression |
9999
| `data-duration` | Yes for DOM, image, and nested-composition clips | Visible slot length in seconds |
100-
| `data-track-index` | Yes | Timeline lane used to prevent temporal overlap |
101-
| `class="clip"` | Yes for authored timed DOM and image elements | Lets the runtime own their visibility window |
100+
| `data-track-index` | No | Studio timeline lane, display only. The render never reads it and it does not prevent overlap |
101+
| `class="clip"` | Recommended for authored timed DOM and image elements | Layout and tooling convention. Visibility is keyed off `data-start`, but the shared `.clip` rule supplies the full-frame box |
102102

103103
`data-track-index` does not control paint order. Use CSS `z-index` for
104-
front-to-back layering. Two clips on the same track must not overlap in time.
104+
front-to-back layering. Two clips on the same track may overlap in time; the
105+
lane is a Studio display row, not a scheduling constraint.
105106

106107
Video visibility is managed as media and does not require `class="clip"`.
107108
Audio has no visual lifecycle.
@@ -124,7 +125,7 @@ Audio has no visual lifecycle.
124125
| --- | --- | --- |
125126
| `data-media-start` / `data-playback-start` | Video, audio, nested composition | Offset into the source file, used by trim and split. Two groups of readers disagree, so the right name depends on the element. **Read only `data-media-start`:** the timing compiler, the HTML parser, `hyperframes validate` (which only inspects `<audio>`), and the engine's audio mixer (which feeds ffmpeg `-ss`). **Read `data-playback-start` first, falling back to `data-media-start`:** the runtime player, Studio (which also writes it), and `hyperframes snapshot`. Because the audio mixer reads only `data-media-start`, a `<video>` authored with just `data-playback-start` renders a trimmed picture over untrimmed audio. Set the name by kind: **`<video>` / `<audio>` → `data-media-start`**; **nested composition → `data-playback-start`** — composition hosts are inspected only by the playback-start-first readers (the media-start-only ones are all `<video>`/`<audio>`-scoped). `data-media-start` still works there as a fallback, but `data-playback-start` is the canonical name Studio writes and normalises to for new composition hosts, so the other name works until an edit rewrites it (it is the [child-timeline offset](/concepts/compositions)). |
126127
| `data-playback-rate` | Video, audio, nested composition | Playback multiplier from `0.1` to `5` |
127-
| `data-volume` | Video and audio | Static volume from `0` to `1` |
128+
| `data-volume` | Video and audio | Static gain. `1` is 0 dB, `0` is silence, and values above `1` boost up to `3.98` (+12 dB) |
128129
| `data-has-audio="true"` | Video | Declares that the video contributes audio |
129130

130131
Video and audio may omit `data-duration` when their intrinsic duration is known

packages/cli/src/commands/coreSkillContent.test.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,17 @@ const REPO_ROOT = join(fileURLToPath(new URL(".", import.meta.url)), "..", "..",
88
const read = (...parts: string[]): string => readFileSync(join(REPO_ROOT, ...parts), "utf8");
99

1010
describe("hyperframes-core contract docs", () => {
11-
it("keeps root data-start in the minimal composition skeleton", () => {
11+
it("keeps a runnable root in the minimal composition skeleton", () => {
1212
const minimal = read("skills", "hyperframes-core", "references", "minimal-composition.md");
1313

14-
expect(minimal).toMatch(/data-composition-id="main"[\s\S]{0,300}data-start="0"/);
15-
expect(minimal).toContain('Root `<div>` with `data-composition-id`, `data-start="0"`');
14+
// Structural pin: the skeleton must still declare a root the runtime can find
15+
// and size. The prose around it is deliberately not pinned: asserting exact
16+
// sentences here made every docs correction a CI failure, and the sentence this
17+
// replaces ("Root <div> with data-composition-id, data-start=\"0\"") listed
18+
// data-start as required when the runtime stamps it (runtime/init.ts).
19+
expect(minimal).toMatch(/data-composition-id="main"/);
20+
expect(minimal).toMatch(/data-width="1920"[\s\S]{0,120}data-height="1080"/);
21+
expect(minimal).toMatch(/window\.__timelines\["main"\]/);
1622
});
1723

1824
it("teaches check as the canonical quality gate", () => {

packages/cli/src/docs/data-attributes.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ Core attributes for controlling element timing and behavior.
66

77
- `data-start="0"` — Start time in seconds
88
- `data-duration="5"` — Duration in seconds
9-
- `data-track-index="0"`Timeline track number (controls z-ordering)
9+
- `data-track-index="0"`Studio timeline lane, display only. The render never reads it, and it does not control paint order (use CSS `z-index`) or prevent overlap. Optional.
1010

1111
## Media
1212

1313
- `data-media-start="2"` — Media playback offset / trim point (seconds)
14-
- `data-volume="0.8"` — Audio/video volume, 0 to 1
14+
- `data-volume="0.8"` — Audio/video gain. `1` is 0 dB, `0` is silence, and values above `1` boost up to `3.98` (+12 dB)
1515
- `data-has-audio="true"` — Indicates video has an audio track
1616

1717
## Composition
@@ -23,4 +23,4 @@ Core attributes for controlling element timing and behavior.
2323

2424
## Element Visibility
2525

26-
Add `class="clip"` to timed elements so the runtime can manage their visibility lifecycle.
26+
Add `class="clip"` to timed elements. The runtime keys visibility off `data-start`, not this class, but the shared `.clip` rule is what gives a scene its full-frame box and Studio treats it as an edit hint.

packages/cli/src/templates/_shared/AGENTS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ Fix all errors before presenting the result. Warnings should be reviewed before
8888

8989
## Key Rules
9090

91-
1. Every timed element needs `data-start`, `data-duration`, and `data-track-index`
92-
2. Elements with timing **MUST** have `class="clip"`the framework uses this for visibility control
91+
1. Every timed element needs `data-start` and a duration. `data-start` is what marks it as timed; `data-track-index` is an optional Studio display lane the render never reads
92+
2. Give timed visual elements `class="clip"`. The framework keys visibility off `data-start`, not the class, but the shared `.clip` CSS is what gives a scene its full-frame box, and `lint` warns without it
9393
3. Timelines must be paused and registered on `window.__timelines`:
9494
```js
9595
window.__timelines = window.__timelines || {};

packages/cli/src/templates/_shared/CLAUDE.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ Fix all errors before presenting the result. Warnings should be reviewed before
8888

8989
## Key Rules
9090

91-
1. Every timed element needs `data-start`, `data-duration`, and `data-track-index`
92-
2. Elements with timing **MUST** have `class="clip"`the framework uses this for visibility control
91+
1. Every timed element needs `data-start` and a duration. `data-start` is what marks it as timed; `data-track-index` is an optional Studio display lane the render never reads
92+
2. Give timed visual elements `class="clip"`. The framework keys visibility off `data-start`, not the class, but the shared `.clip` CSS is what gives a scene its full-frame box, and `lint` warns without it
9393
3. Timelines must be paused and registered on `window.__timelines`:
9494
```js
9595
window.__timelines = window.__timelines || {};

packages/core/src/compiler/mediaRenderIds.test.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,39 @@ describe("assignMediaRenderIds", () => {
7575
expect(ids[1]).toBe("clip__hf2");
7676
});
7777

78-
it("leaves media without a src alone", () => {
78+
it("leaves media with no source at all alone", () => {
7979
const { document } = parseHTML('<video id="no-src"></video>');
8080
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
8181
expect(document.querySelector("video")?.hasAttribute(MEDIA_RENDER_ID_ATTR)).toBe(false);
8282
});
83+
84+
it("stamps media whose source is a <source> child rather than a src attribute", () => {
85+
// The selector used to be `video[src], audio[src], img[src]`, so this shape
86+
// was never stamped and two inlined scenes kept colliding ids in the render
87+
// document, which is the exact failure this module exists to prevent.
88+
const { document } = parseHTML(
89+
'<video id="clip"><source src="a.mp4" type="video/mp4"></video>' +
90+
'<video id="clip"><source src="a.mp4" type="video/mp4"></video>',
91+
);
92+
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
93+
expect(
94+
Array.from(document.querySelectorAll("video")).map((el) =>
95+
el.getAttribute(MEDIA_RENDER_ID_ATTR),
96+
),
97+
).toEqual(["clip", "clip__hf2"]);
98+
});
99+
100+
it("stamps <audio> with a <source> child too", () => {
101+
const { document } = parseHTML('<audio id="bed"><source src="bed.mp3"></audio>');
102+
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
103+
expect(document.querySelector("audio")?.getAttribute(MEDIA_RENDER_ID_ATTR)).toBe("bed");
104+
});
105+
106+
it("ignores a <source> child that carries no src", () => {
107+
const { document } = parseHTML('<video id="empty"><source type="video/mp4"></video>');
108+
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
109+
expect(document.querySelector("video")?.hasAttribute(MEDIA_RENDER_ID_ATTR)).toBe(false);
110+
});
83111
});
84112

85113
describe("audio group render ids", () => {

packages/core/src/compiler/mediaRenderIds.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,35 @@ export const MEDIA_RENDER_ID_ATTR = "data-hf-render-id";
4040
*/
4141
export const AUDIO_GROUP_RENDER_ID_ATTR = "data-hf-group-render-id";
4242

43-
/** Elements the render pipeline addresses by id. */
44-
const MEDIA_SELECTOR = "video[src], audio[src], img[src]";
43+
/**
44+
* Elements the render pipeline addresses by id.
45+
*
46+
* `<video>`/`<audio>` are matched whether the source is a `src` attribute or a
47+
* `<source>` child. Matching only `[src]` left the `<source>`-child shape
48+
* unstamped, so two scenes each declaring `<video id="clip"><source …></video>`
49+
* kept colliding ids in the render document, which is exactly the failure this
50+
* module exists to prevent.
51+
*/
52+
const MEDIA_SELECTOR = "video, audio, img[src]";
53+
4554
/** Buses, which are addressed by id in exactly the same way and collide the
4655
* same way. Only an id'd bus can be joined at all. */
4756
const AUDIO_GROUP_SELECTOR = "hf-audio-group[id]";
4857

58+
/** A `<source>`-bearing media element is addressable even without its own `src`. */
59+
function hasPlayableSource(el: MediaElementLike): boolean {
60+
if (el.getAttribute("src")) return true;
61+
const sources = el.querySelectorAll?.("source[src]");
62+
if (!sources) return false;
63+
for (const _ of sources) return true;
64+
return false;
65+
}
66+
4967
interface MediaElementLike {
68+
readonly tagName?: string;
5069
getAttribute(name: string): string | null;
5170
setAttribute(name: string, value: string): void;
71+
querySelectorAll?(selector: string): Iterable<unknown>;
5272
}
5373

5474
/** A bus or member, which additionally needs subtree scoping to be paired up. */
@@ -88,6 +108,7 @@ export function assignMediaRenderIds(document: DocumentLike): void {
88108
const pending: MediaElementLike[] = [];
89109

90110
for (const el of document.querySelectorAll(MEDIA_SELECTOR)) {
111+
if (!hasPlayableSource(el)) continue;
91112
const existing = el.getAttribute(MEDIA_RENDER_ID_ATTR);
92113
if (existing) {
93114
taken.add(existing);

packages/lint/src/rules/composition.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,8 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding
432432
severity: "error",
433433
message: `<${tag.name}${elementId ? ` id="${elementId}"` : ""}> uses data-layer instead of data-track-index.`,
434434
elementId,
435-
fixHint: "Replace data-layer with data-track-index. The runtime reads data-track-index.",
435+
fixHint:
436+
"Replace data-layer with data-track-index, which is the canonical name Studio and the linter read. Neither name is read by the render.",
436437
snippet: truncateSnippet(tag.raw),
437438
});
438439
}
@@ -935,10 +936,13 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding
935936
// can't leak styles into each other. A rule whose LEFTMOST selector is the ROOT
936937
// element's own class (e.g. `.frame { ... }` on the same element that carries
937938
// data-composition-id) therefore becomes a DESCENDANT selector that can never
938-
// match the root — the whole scene renders unstyled (tiny text top-left, images
939-
// at natural size). lint/validate/inspect evaluate the file in isolation (no
940-
// scoping) and Studio previews each scene in its own iframe (no scoping), so the
941-
// break is invisible until the composited MP4 render. Style the root via `#root`
939+
// match the SCOPED element itself. NOTE on the symptom: since #1886 the producer
940+
// preserves the authored root as a `data-hf-inner-root` wrapper INSIDE the scoped
941+
// element (regression fixture packages/producer/tests/sub-comp-class-selector),
942+
// so the class still matches as a descendant and the scene no longer renders
943+
// unstyled. This rule is now a consistency constraint, not a render-bug guard:
944+
// `#root` is the shape the registry blocks model and the one the scoper
945+
// special-cases. Style the root via `#root`
942946
// (the scoper special-cases the root id) and descendants via plain selectors,
943947
// like the registry blocks — the runtime already scopes each scene by id, so a
944948
// class namespace on the root is redundant.
@@ -960,10 +964,10 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding
960964
severity: "error",
961965
message:
962966
`Root element has class="${rootClasses.join(" ")}" and is styled by ${offenders.length} rule(s) keyed off that class (e.g. ${example}). ` +
963-
`At render, every sub-composition rule is scoped to [data-composition-id="${rootCompositionId}"] <selector>, so a selector whose leftmost part is the ROOT's own class becomes a descendant selector that cannot match the root — the scene renders unstyled (tiny text top-left, full-size images). ` +
964-
`lint/validate/inspect and Studio's per-frame iframe preview do not scope, so this passes every static check and looks correct in preview.`,
967+
`At render, every sub-composition rule is scoped to [data-composition-id="${rootCompositionId}"] <selector>, so a selector whose leftmost part is the ROOT's own class becomes a descendant selector that cannot match the scoped element itself. ` +
968+
`Since #1886 the producer preserves the authored root as an inner wrapper, so this no longer renders the scene unstyled, but #root is the shape the scoper special-cases and the registry blocks model. Use it so preview, render, and Studio agree.`,
965969
selector: example,
966-
fixHint: `Give the root id="root" and style it with \`#root { ... }\` plus plain descendant selectors (\`.kicker\`, \`#hero\`) — the runtime already scopes each sub-composition by data-composition-id, so a class namespace on the root is redundant and breaks under scoping.`,
970+
fixHint: `Give the root id="root" and style it with \`#root { ... }\` plus plain descendant selectors (\`.kicker\`, \`#hero\`) — the runtime already scopes each sub-composition by data-composition-id, so a class namespace on the root is redundant.`,
967971
snippet: truncateSnippet(rootTag.raw),
968972
},
969973
];

packages/lint/src/rules/core.test.ts

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,15 @@ describe("core rules", () => {
227227
expect(finding).toBeDefined();
228228
});
229229

230-
it("reports error when timeline registry is assigned without initializing", async () => {
230+
// The runtime creates `window.__timelines` at script-evaluation time
231+
// (runtime/entry.ts), before any inline composition script runs, so a bare
232+
// assignment needs no `window.__timelines = window.__timelines || {}` guard.
233+
// Verified by rendering a composition whose only registration is the bare
234+
// assignment: it renders and animates correctly. The old
235+
// `timeline_registry_missing_init` error therefore failed a working file, and
236+
// because a lint ERROR also suppresses the layout and contrast audits in
237+
// `check`, it cost far more than the line it asked for.
238+
it("accepts a bracket registry assignment with no init guard", async () => {
231239
const html = `
232240
<html><body>
233241
<div id="root" data-composition-id="c1" data-width="1920" data-height="1080">
@@ -240,13 +248,13 @@ describe("core rules", () => {
240248
</script>
241249
</body></html>`;
242250
const result = await lintHyperframeHtml(html);
243-
const finding = result.findings.find((f) => f.code === "timeline_registry_missing_init");
244-
expect(finding).toBeDefined();
245-
expect(finding?.severity).toBe("error");
246-
expect(finding?.message).toContain("without initializing");
251+
expect(
252+
result.findings.find((f) => f.code === "timeline_registry_missing_init"),
253+
).toBeUndefined();
254+
expect(result.findings.find((f) => f.code === "missing_timeline_registry")).toBeUndefined();
247255
});
248256

249-
it("reports error when dot timeline registry is assigned without initializing", async () => {
257+
it("accepts a dot registry assignment with no init guard", async () => {
250258
const html = `
251259
<html><body>
252260
<div id="root" data-composition-id="c1" data-width="1920" data-height="1080">
@@ -259,9 +267,10 @@ describe("core rules", () => {
259267
</script>
260268
</body></html>`;
261269
const result = await lintHyperframeHtml(html);
262-
const finding = result.findings.find((f) => f.code === "timeline_registry_missing_init");
263-
expect(finding).toBeDefined();
264-
expect(finding?.severity).toBe("error");
270+
expect(
271+
result.findings.find((f) => f.code === "timeline_registry_missing_init"),
272+
).toBeUndefined();
273+
expect(result.findings.find((f) => f.code === "missing_timeline_registry")).toBeUndefined();
265274
});
266275

267276
it("does not flag timeline assignment when init guard is present", async () => {

0 commit comments

Comments
 (0)