Repository navigation
feat(lint): add rules for non-deterministic code, missing clip class, overlapping tracks, and rAF detection - #193
Conversation
… overlapping tracks, and rAF detection Add 4 new lint rules to close E2E testing gaps: - non_deterministic_code (error): detects Math.random(), Date.now(), new Date(), performance.now(), crypto.getRandomValues() in scripts - timed_element_missing_clip_class (warning): flags timed elements without class="clip" - overlapping_clips_same_track (error): detects overlapping clips on the same track index - requestanimationframe_in_composition (warning): flags rAF usage that won't sync with frame capture All rules strip single-line comments before scanning to avoid false positives. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
@miguel-heygen should we make these warnings? I think there are potential use cases for some simulations to use thse but yes they will not produce the exact same output unless we seed the RNG ? |
|
what does this mean exactly? |
requestAnimationFrame (raf) doesn't work well with the renderer using vivus.js library |
with a seed it's fully deterministic, in my opinion we should make these errors because they can produce unexpected outputs that we can't control with the runtime code in certain cases |
vanceingalls
left a comment
vanceingalls
left a comment
There was a problem hiding this comment.
Review feedback
Important: Non-null assertions
packages/core/src/lint/rules/composition.ts uses clips[i]! and clips[i + 1]! in the overlap rule. Project convention forbids ! assertions — use a guard like if (!current || !next) continue; instead.
Important: Block comment false positives
The comment stripping in non_deterministic_code and requestanimationframe_in_composition only strips // ... single-line comments. Multi-line /* Math.random() */ will still trigger false positives. Fix:
const stripped = script.content
.replace(/\/\/.*$/gm, "")
.replace(/\/\*[\s\S]*?\*\//g, "");Note: Rule overlap
The new timed_element_missing_clip_class (warning) overlaps with the existing timed_element_missing_visibility_hidden (info). A single element could trigger both with slightly contradictory messaging. Consider merging or gating the older rule.
Tests look solid — 3 per rule with positive/negative/edge cases.
Address review feedback: - Strip /* */ block comments in addition to // line comments to prevent false positives in non_deterministic_code and rAF rules - Replace clips[i]! non-null assertions with guard clause per project convention Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
All three review items addressed:
All 56 rule tests still passing. |
… overlapping tracks, and rAF detection (#193) ## Summary Adds 4 new lint rules found missing during comprehensive E2E testing: - **`non_deterministic_code`** (error) — detects `Math.random()`, `Date.now()`, `new Date()`, `performance.now()`, `crypto.getRandomValues()` in scripts. Confirmed: two renders with Math.random() produced different checksums. - **`timed_element_missing_clip_class`** (warning) — flags elements with `data-start`/`data-duration` but no `class="clip"`. Without it, elements are visible forever instead of only during their time window. - **`overlapping_clips_same_track`** (error) — detects clips on the same `data-track-index` with overlapping time ranges. - **`requestanimationframe_in_composition`** (warning) — warns that rAF-based animations don't sync with frame capture. Discovered when Vivus.js (rAF-based) produced incorrect output. 12 new tests, all passing. **Part 1 of 5** in a stacked PR series fixing E2E test findings. ## Test plan - [x] 12 new tests (3 per rule: positive, negative, edge case) - [x] Full suite: 56 pass in rule tests - [x] `npx tsx scripts/lint-skills.ts` — no issues
… overlapping tracks, and rAF detection (heygen-com#193) ## Summary Adds 4 new lint rules found missing during comprehensive E2E testing: - **`non_deterministic_code`** (error) — detects `Math.random()`, `Date.now()`, `new Date()`, `performance.now()`, `crypto.getRandomValues()` in scripts. Confirmed: two renders with Math.random() produced different checksums. - **`timed_element_missing_clip_class`** (warning) — flags elements with `data-start`/`data-duration` but no `class="clip"`. Without it, elements are visible forever instead of only during their time window. - **`overlapping_clips_same_track`** (error) — detects clips on the same `data-track-index` with overlapping time ranges. - **`requestanimationframe_in_composition`** (warning) — warns that rAF-based animations don't sync with frame capture. Discovered when Vivus.js (rAF-based) produced incorrect output. 12 new tests, all passing. **Part 1 of 5** in a stacked PR series fixing E2E test findings. ## Test plan - [x] 12 new tests (3 per rule: positive, negative, edge case) - [x] Full suite: 56 pass in rule tests - [x] `npx tsx scripts/lint-skills.ts` — no issues
happy-dom ^20.9.0 升到 ^21.0.4（修 GHSA-834v-8h66-3h79），bun.lock 解析為 21.0.4。 測試端配合 21 的行為差異，沒有動產品程式：iframe 頁面載入改在 player、sdk、studio 的 vitest 設定開啟；inlineTextStyleRange、Menu、ShortcutsPanel、motion-signature、 manualOffsetDrag、NLEPreview.slowBoot 的測試調整環境設定或等待方式。 移除 dependabot 對 happy-dom 的 semver-major ignore；DECISIONS 記錄理由失效與 CodeQL 警示 heygen-com#186 至 heygen-com#193 以 false positive 駁回。 Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
…branch Replace the stale pinned pointer (2ccbbe2, "draft heygen-com#193 pending adoption") with a short one: the Kernel's one entry guide on the integration branch the Kernel's work board names, the board, the founder Discussion, and a bounded claim that says who the session is, what it was asked and how it plans to do it. Drop the stray blank line the draft left at the end of AGENTS.md, CLAUDE.md, GEMINI.md and .github/copilot-instructions.md. Local rules and workflow pins are unchanged. Co-Authored-By: Claude <[email protected]> Claude-Session: https://claude.ai/code/session_01KucyfnCuSTnjx5Fa6T4A3d
Summary
Adds 4 new lint rules found missing during comprehensive E2E testing:
non_deterministic_code(error) — detectsMath.random(),Date.now(),new Date(),performance.now(),crypto.getRandomValues()in scripts. Confirmed: two renders with Math.random() produced different checksums.timed_element_missing_clip_class(warning) — flags elements withdata-start/data-durationbut noclass="clip". Without it, elements are visible forever instead of only during their time window.overlapping_clips_same_track(error) — detects clips on the samedata-track-indexwith overlapping time ranges.requestanimationframe_in_composition(warning) — warns that rAF-based animations don't sync with frame capture. Discovered when Vivus.js (rAF-based) produced incorrect output.12 new tests, all passing.
Part 1 of 5 in a stacked PR series fixing E2E test findings.
Test plan
npx tsx scripts/lint-skills.ts— no issues