Skip to content

Commit 9bb4b4c

Browse files
miguel-heygenmiguel-heygen
authored
fix(lint): break two fix-loops and drop two rules the runtime owns (#3400)
An eval of time-to-finished-video against Remotion found the whole gap sits after authoring, in the lint/check -> fix -> recheck loop. Lint execution is 3-5% of wall; the cost is the model turn each finding triggers. So the expensive rule is the one an agent cannot satisfy, and the next most expensive is the one that fires on correct code. Two rules could not be satisfied at all. `gsap_fullscreen_overlay_starts_visible` on a from() reveal was a closed cycle. It errored on `tl.from("#flash", { opacity: 0 })`, which is not a defect: from() seats its start values immediately, so on a paused timeline the overlay already measures opacity 0 at t=0. Both of its fixHints (authored CSS `opacity: 0`, or an immediate `gsap.set`) turn that working composition into a real defect, which `gsap_from_opacity_noop` correctly errors on -- and that rule's fixHint says to remove the very thing we just asked for. Applying either hint bounced between the two errors forever. The root cause was not the reporting condition but `laterHidden`, which counted the reveal itself: a from-tween records its START values, so `from({opacity: 0})` read as its own later hide. Excluding the reveal, and excluding from-tweens (which end visible), is what actually fixes it. The later-hidden shape still reports and still converges. `caption_text_overflow_risk` told authors to add `overflow: hidden`, which is exactly what `caption_overflow_clips_scaled_words` errors on. Following the warning produced an error. The hint now says to keep overflow visible. Two rules asserted a failure that cannot happen. `root_composition_missing_data_start` errored because "the runtime needs data-start=0 on the root element to begin playback". The runtime sets it itself -- init.ts:286-292, whose comment reads "Agents sometimes omit data-start on the root composition element ... Default to 0 for the root." The rule demanded the fix the runtime had already applied. 16 of the 643 shipped registry files. `overlapping_clips_same_track` claimed overlapping clips "cause rendering conflicts". Nothing reads the track index at render: timeline.ts:586 states "Track index is display-only; render never reads it", and grepping `trackIndex|track-index` across engine and producer source returns zero hits. Two clips overlapping on one track is a crossfade. Two false positives fixed rather than removed, because the invariants are real. `timeline_id_mismatch` fired on the legal one-liner `window.__timelines = { main: gsap.timeline({ paused: true }) }`. The body regex was non-greedy, so it stopped at the brace of the inlined OPTIONS object, and the entry scanner harvested `paused` as a composition id. The resulting fixHint named a registration that does not exist, so it could never be applied; hoisting to a variable was the only escape and nothing said so. It now walks brace depth and reads top-level keys only. `non_deterministic_code` fired on `new Date("2026-01-01T00:00:00Z")`, which is deterministic, and on `Math.random()` inside a string literal that a code-display composition renders but never executes. Date is now zero-arg only, and patterns run against string-stripped source -- except the GSAP `"random(...)"` tween value, where the string IS the executed value and must still be scanned. That exception is what the first attempt got wrong; the existing tests caught it. Measured over the 643 shipped registry files: 386 -> 370 errors, no new codes. The corpus does not exercise the loop cases, so it understates the change -- the point is the turns those loops cost, not the finding count.
1 parent d476551 commit 9bb4b4c

9 files changed

Lines changed: 270 additions & 189 deletions

File tree

‎packages/lint/src/rules/captions.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,4 +163,33 @@ describe("caption rules", () => {
163163
expect(finding).toBeDefined();
164164
expect(finding?.severity).toBe("error");
165165
});
166+
167+
describe("caption_text_overflow_risk — its fix must not create an error", () => {
168+
const cap = (css: string) => `
169+
<html><body>
170+
<div data-composition-id="captions" data-width="1920" data-height="1080">
171+
<style>.caption-group{${css}}</style><div class="caption-group"></div>
172+
</div>
173+
<script>
174+
const tl = gsap.timeline({ paused: true });
175+
words.forEach((w) => tl.to(w, { scale: 1.3 }));
176+
window.__timelines = { captions: tl };
177+
</script>
178+
</body></html>`;
179+
180+
it("clears when the fixHint is applied as written", async () => {
181+
// The hint used to say "and overflow: hidden", which is exactly what
182+
// caption_overflow_clips_scaled_words errors on. Following the warning
183+
// produced an error.
184+
const before = await lintHyperframeHtml(cap("position:absolute;white-space:nowrap"));
185+
expect(before.findings.find((f) => f.code === "caption_text_overflow_risk")).toBeDefined();
186+
187+
const after = await lintHyperframeHtml(
188+
cap("position:absolute;white-space:nowrap;max-width:1600px;overflow:visible"),
189+
);
190+
const blocking = after.findings.filter((f) => f.severity !== "info");
191+
expect(blocking.map((f) => f.code)).not.toContain("caption_text_overflow_risk");
192+
expect(blocking.map((f) => f.code)).not.toContain("caption_overflow_clips_scaled_words");
193+
});
194+
});
166195
});

‎packages/lint/src/rules/captions.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,11 @@ export const captionRules: Array<(ctx: LintContext) => HyperframeLintFinding[]>
5757
selector: (selector ?? "").trim(),
5858
message: `Caption selector "${(selector ?? "").trim()}" has white-space: nowrap but no max-width. Long phrases will clip off-screen.`,
5959
fixHint:
60-
"Add max-width: 1600px (landscape) or max-width: 900px (portrait) and overflow: hidden.",
60+
// Deliberately does NOT say `overflow: hidden`: caption words are scaled
61+
// above 1.0x, and clipping them is exactly what caption_overflow_clips_scaled_words
62+
// errors on. Recommending it here made this warning's own fix produce an error.
63+
"Add max-width: 1600px (landscape) or max-width: 900px (portrait). Keep " +
64+
"overflow visible so scaled emphasis words are not clipped.",
6165
});
6266
}
6367
}

‎packages/lint/src/rules/composition.test.ts‎

Lines changed: 0 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -528,78 +528,6 @@ describe("composition rules", () => {
528528
});
529529
});
530530

531-
describe("overlapping_clips_same_track", () => {
532-
it("flags overlapping clips on the same track", async () => {
533-
const html = `
534-
<html><body>
535-
<div data-composition-id="c1" data-width="1920" data-height="1080">
536-
<div class="clip" data-start="0" data-duration="3" data-track-index="0">A</div>
537-
<div class="clip" data-start="2" data-duration="3" data-track-index="0">B</div>
538-
</div>
539-
<script>
540-
window.__timelines = window.__timelines || {};
541-
window.__timelines["c1"] = gsap.timeline({ paused: true });
542-
</script>
543-
</body></html>`;
544-
const result = await lintHyperframeHtml(html);
545-
const finding = result.findings.find((f) => f.code === "overlapping_clips_same_track");
546-
expect(finding).toBeDefined();
547-
expect(finding?.severity).toBe("error");
548-
});
549-
550-
it("does not flag clips on different tracks", async () => {
551-
const html = `
552-
<html><body>
553-
<div data-composition-id="c1" data-width="1920" data-height="1080">
554-
<div class="clip" data-start="0" data-duration="3" data-track-index="0">A</div>
555-
<div class="clip" data-start="1" data-duration="3" data-track-index="1">B</div>
556-
</div>
557-
<script>
558-
window.__timelines = window.__timelines || {};
559-
window.__timelines["c1"] = gsap.timeline({ paused: true });
560-
</script>
561-
</body></html>`;
562-
const result = await lintHyperframeHtml(html);
563-
const finding = result.findings.find((f) => f.code === "overlapping_clips_same_track");
564-
expect(finding).toBeUndefined();
565-
});
566-
567-
it("does not flag sequential clips on the same track", async () => {
568-
const html = `
569-
<html><body>
570-
<div data-composition-id="c1" data-width="1920" data-height="1080">
571-
<div class="clip" data-start="0" data-duration="2" data-track-index="0">A</div>
572-
<div class="clip" data-start="2" data-duration="2" data-track-index="0">B</div>
573-
</div>
574-
<script>
575-
window.__timelines = window.__timelines || {};
576-
window.__timelines["c1"] = gsap.timeline({ paused: true });
577-
</script>
578-
</body></html>`;
579-
const result = await lintHyperframeHtml(html);
580-
const finding = result.findings.find((f) => f.code === "overlapping_clips_same_track");
581-
expect(finding).toBeUndefined();
582-
});
583-
584-
it("does not flag adjacencies where parseFloat + add drifts by a few ulps", async () => {
585-
// parseFloat("0.1") + parseFloat("0.2") = 0.30000000000000004
586-
const html = `
587-
<html><body>
588-
<div data-composition-id="c1" data-width="1920" data-height="1080">
589-
<div class="clip" data-start="0.1" data-duration="0.2" data-track-index="0">A</div>
590-
<div class="clip" data-start="0.3" data-duration="0.2" data-track-index="0">B</div>
591-
</div>
592-
<script>
593-
window.__timelines = window.__timelines || {};
594-
window.__timelines["c1"] = gsap.timeline({ paused: true });
595-
</script>
596-
</body></html>`;
597-
const result = await lintHyperframeHtml(html);
598-
const finding = result.findings.find((f) => f.code === "overlapping_clips_same_track");
599-
expect(finding).toBeUndefined();
600-
});
601-
});
602-
603531
describe("root_composition_missing_html_wrapper", () => {
604532
it("flags bare composition div as error", async () => {
605533
// Exact scenario from the screenshot — bare div with composition attributes, no HTML wrapper

‎packages/lint/src/rules/composition.ts‎

Lines changed: 1 addition & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,6 @@ const HEAVY_OVERLAY_CSS_PATTERN =
5959
/(?:filter\s*:[^;}]*\bblur\s*\()|(?:clip-path\s*:(?!\s*(?:none|inherit|initial|unset)\b)\s*[^;}]+)|(?:radial-gradient\s*\()/i;
6060
const INLINE_STYLE_DISPLAY_NONE_PATTERN = /(?:^|;)\s*display\s*:\s*none\b/i;
6161

62-
// `parseFloat("0.1") + parseFloat("0.2") = 0.30000000000000004`. Sub-second
63-
// authored adjacencies survive parse + add as a value a few ulps above the
64-
// next clip's start; a strict `>` fires the overlap rule on adjacencies that
65-
// are exact in the source HTML. 1μs sits ~11 orders of magnitude above the
66-
// observed drift (worst ~2e-16s across every realistic decimal pair) and 4
67-
// below one 60fps frame (~16.67ms), so this only ever swallows float slop.
68-
const OVERLAP_EPSILON_SECONDS = 1e-6;
69-
7062
function readTagTiming(rawTag: string) {
7163
return readClipTiming({ getAttribute: (name) => readAttr(rawTag, name) });
7264
}
@@ -164,6 +156,7 @@ function leftmostCompoundId(selector: string): string | null {
164156
// are scanned — the flat `[^{}]*` body class naturally skips @keyframes
165157
// bodies (which contain nested `{...}` stops) and other @-rules, so keyframe
166158
// selectors like `0%`/`100%` don't leak in.
159+
// fallow-ignore-next-line complexity
167160
function collectHeavyOverlayHooks(styles: ExtractedBlock[]): {
168161
classes: Set<string>;
169162
ids: Set<string>;
@@ -557,75 +550,6 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding
557550
return findings;
558551
},
559552

560-
// overlapping_clips_same_track
561-
// fallow-ignore-next-line complexity
562-
({ tags }) => {
563-
const findings: HyperframeLintFinding[] = [];
564-
565-
type ClipInfo = { start: number; end: number; elementId?: string; snippet: string };
566-
const trackMap = new Map<string, ClipInfo[]>();
567-
568-
for (const tag of tags) {
569-
const trackStr = readAttr(tag.raw, COMPOSITION_ATTRIBUTES.trackIndex);
570-
if (!trackStr) continue;
571-
const timing = readTagTiming(tag.raw);
572-
const { start, duration } = timing;
573-
const track = trackStr;
574-
575-
// Skip non-numeric (relative timing references like "intro-comp")
576-
if (start == null || duration == null) continue;
577-
578-
const clips = trackMap.get(track) || [];
579-
clips.push({
580-
start,
581-
end: start + duration,
582-
elementId: readAttr(tag.raw, "id") || undefined,
583-
snippet: truncateSnippet(tag.raw) || "",
584-
});
585-
trackMap.set(track, clips);
586-
}
587-
588-
for (const [track, clips] of trackMap) {
589-
clips.sort((a, b) => a.start - b.start);
590-
for (let i = 0; i < clips.length - 1; i++) {
591-
const current = clips[i];
592-
const next = clips[i + 1];
593-
if (!current || !next) continue;
594-
if (current.end - next.start > OVERLAP_EPSILON_SECONDS) {
595-
findings.push({
596-
code: "overlapping_clips_same_track",
597-
severity: "error",
598-
message: `Track ${track}: clip ending at ${current.end}s overlaps with clip starting at ${next.start}s. Overlapping clips on the same track cause rendering conflicts.`,
599-
fixHint:
600-
"Adjust data-start or data-duration so clips on the same track do not overlap, or move one clip to a different data-track-index.",
601-
});
602-
}
603-
}
604-
}
605-
606-
return findings;
607-
},
608-
609-
// root_composition_missing_data_start
610-
({ rootTag, options }) => {
611-
const findings: HyperframeLintFinding[] = [];
612-
if (options.isSubComposition) return findings;
613-
if (!rootTag) return findings;
614-
const compId = readDecodedAttr(rootTag.raw, "data-composition-id");
615-
if (!compId) return findings;
616-
const hasStart = readAttr(rootTag.raw, "data-start") !== null;
617-
if (!hasStart) {
618-
findings.push({
619-
code: "root_composition_missing_data_start",
620-
severity: "error",
621-
message: `Root composition "${compId}" is missing data-start. The runtime needs data-start="0" on the root element to begin playback.`,
622-
fixHint: 'Add data-start="0" to the root composition element.',
623-
snippet: truncateSnippet(rootTag.raw),
624-
});
625-
}
626-
return findings;
627-
},
628-
629553
// standalone_composition_wrapped_in_template
630554
({ rawSource, options }) => {
631555
const findings: HyperframeLintFinding[] = [];

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -708,4 +708,64 @@ describe("core rules", () => {
708708
expect(finding).toBeUndefined();
709709
});
710710
});
711+
712+
describe("non_deterministic_code — determinism is about execution, not text", () => {
713+
const comp = (script: string) => `
714+
<html><body>
715+
<div data-composition-id="main" data-width="1920" data-height="1080" data-start="0" data-duration="5"></div>
716+
<script src="gsap.min.js"></script>
717+
<script>const tl = gsap.timeline({ paused: true }); ${script} window.__timelines = { main: tl };</script>
718+
</body></html>`;
719+
720+
it("does not flag new Date() with a fixed timestamp", async () => {
721+
// Deterministic, and the fixHint ("remove time-dependent code") cannot be
722+
// applied without deleting the label the composition renders.
723+
const result = await lintHyperframeHtml(
724+
comp(`const label = new Date("2026-01-01T00:00:00Z").toISOString();`),
725+
);
726+
expect(result.findings.find((f) => f.code === "non_deterministic_code")).toBeUndefined();
727+
});
728+
729+
it("does not flag non-deterministic APIs quoted inside a string literal", async () => {
730+
// Code-display compositions render source they never execute.
731+
const result = await lintHyperframeHtml(comp(`const SNIPPET = "const x = Math.random();";`));
732+
expect(result.findings.find((f) => f.code === "non_deterministic_code")).toBeUndefined();
733+
});
734+
735+
it("still flags a bare new Date()", async () => {
736+
const result = await lintHyperframeHtml(comp(`const now = new Date();`));
737+
expect(result.findings.find((f) => f.code === "non_deterministic_code")).toBeDefined();
738+
});
739+
740+
it("still flags Math.random() in executed code", async () => {
741+
const result = await lintHyperframeHtml(comp(`const r = Math.random();`));
742+
expect(result.findings.find((f) => f.code === "non_deterministic_code")).toBeDefined();
743+
});
744+
});
745+
746+
describe("timeline_id_mismatch — only top-level registry keys are composition ids", () => {
747+
const comp = (script: string) => `
748+
<html><body>
749+
<div data-composition-id="main" data-width="1920" data-height="1080" data-start="0" data-duration="5"></div>
750+
<script src="gsap.min.js"></script>
751+
<script>${script}</script>
752+
</body></html>`;
753+
754+
it("does not flag the one-liner registration form", async () => {
755+
// The inlined options object is not a registration. Reading `paused` as a
756+
// composition id produced an error whose fixHint named a registration that
757+
// did not exist, so it could never be applied.
758+
const result = await lintHyperframeHtml(
759+
comp(`window.__timelines = { main: gsap.timeline({ paused: true }) };`),
760+
);
761+
expect(result.findings.find((f) => f.code === "timeline_id_mismatch")).toBeUndefined();
762+
});
763+
764+
it("still flags a genuinely mismatched id", async () => {
765+
const result = await lintHyperframeHtml(
766+
comp(`window.__timelines = { wrongid: gsap.timeline({ paused: true }) };`),
767+
);
768+
expect(result.findings.find((f) => f.code === "timeline_id_mismatch")).toBeDefined();
769+
});
770+
});
711771
});

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

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
readDecodedAttr,
77
truncateSnippet,
88
stripJsComments,
9+
stripStringLiterals,
910
extractCompositionIdsFromCss,
1011
extractTimelineRegistryKeys,
1112
getInlineScriptSyntaxError,
@@ -204,6 +205,7 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [
204205
},
205206

206207
// root_missing_composition_id + root_missing_dimensions
208+
// fallow-ignore-next-line complexity
207209
({ rootTag }) => {
208210
const findings: HyperframeLintFinding[] = [];
209211
if (!rootTag || !readDecodedAttr(rootTag.raw, "data-composition-id")) {
@@ -247,6 +249,7 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [
247249
},
248250

249251
// missing_timeline_registry + timeline_registry_missing_init
252+
// fallow-ignore-next-line complexity
250253
({ source, rawSource, rootTag, options }) => {
251254
// Sub-compositions inherit window.__timelines from the host composition
252255
if (options.isSubComposition || rawSource.trimStart().toLowerCase().startsWith("<template")) {
@@ -439,7 +442,13 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [
439442
// non_deterministic_code
440443
({ scripts }) => {
441444
const findings: HyperframeLintFinding[] = [];
442-
const patterns: Array<{ pattern: RegExp; label: string; hint: string }> = [
445+
const patterns: Array<{
446+
pattern: RegExp;
447+
label: string;
448+
hint: string;
449+
/** Match against raw source, because the value being matched is a string GSAP parses. */
450+
scansStrings?: boolean;
451+
}> = [
443452
{
444453
pattern: /Math\.random\s*\(/,
445454
label: "Math.random()",
@@ -451,7 +460,10 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [
451460
hint: "Remove time-dependent code. Use GSAP timeline position instead of wall-clock time.",
452461
},
453462
{
454-
pattern: /new\s+Date\s*\(/,
463+
// Zero-arg only. `new Date(<fixed timestamp>)` is fully deterministic and is how
464+
// a composition labels a fixed date on an axis or card; the hint ("remove
465+
// time-dependent code") cannot be applied to it without deleting the label.
466+
pattern: /new\s+Date\s*\(\s*\)/,
455467
label: "new Date()",
456468
hint: "Remove time-dependent code. Use GSAP timeline position instead of wall-clock time.",
457469
},
@@ -472,16 +484,25 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [
472484
},
473485
{
474486
// GSAP string form: "random(...)" / "+=random(...)" — re-rolls at tween init.
487+
// `scansStrings` because here the string IS the executed value: GSAP parses it.
488+
// Every other pattern above matches executable code, so a match inside a string
489+
// literal is inert text and must not be reported.
475490
pattern: /["'`](?:[+-]=)?random\(\s*[-\d[]/,
491+
scansStrings: true,
476492
label: '"random(...)" tween value',
477493
hint: "GSAP random string values re-roll at tween init and each render worker initializes independently. Use fixed values or precompute with a seeded PRNG.",
478494
},
479495
];
480496

481497
for (const script of scripts) {
482-
const stripped = stripJsComments(script.content);
483-
for (const { pattern, label, hint } of patterns) {
484-
if (pattern.test(stripped)) {
498+
const withoutComments = stripJsComments(script.content);
499+
// Strings are content, not code. A composition that DISPLAYS source (the
500+
// code-snippet blocks, /pr-to-video) carries `Math.random()` inside a string
501+
// literal it never executes, and reported itself non-deterministic with no
502+
// way to clear the error while still rendering the snippet.
503+
const executable = stripStringLiterals(withoutComments);
504+
for (const { pattern, label, hint, scansStrings } of patterns) {
505+
if (pattern.test(scansStrings ? withoutComments : executable)) {
485506
findings.push({
486507
code: "non_deterministic_code",
487508
severity: "error",

0 commit comments

Comments
 (0)