fix(cli): update dev command for consolidated studio - #5
Closed
miguel-heygen wants to merge 1 commit into
Closed
Conversation
- hyperframes dev: use pnpm exec vite for the new studio package - Symlink projects into studio/data/projects - Clean embedded mode stub (not yet available for published CLI) - Remove internal-only scripts from root package.json - Strip archive/producer-internal/ngrok references
This was referenced Apr 7, 2026
miguel-heygen
added a commit
that referenced
this pull request
Apr 7, 2026
## Summary Adds critical rendering constraints to the `hyperframes` skill discovered from eval analysis of 27 agent-generated compositions. These guardrails prevent agents from producing compositions that technically work but render poorly. ## What it fixes | Rule Added | Eval Prompts Affected | Issue | | --- | --- | --- | | Ban `repeat: -1` | #20 loading-spinner (2.0/5) | Infinite timeline broke capture engine | | Ban async timeline construction | #16 particle-logo (2.6/5) | Timeline empty at capture time | | Min font size 16px (labels), 20px (body) | #7, #8, #13, #14, #15, #19 | Illegible text after encoding | | Ban full-screen dark linear gradients | #3, #5, #10, #14 | H.264 color banding | | `<link>` fonts over CSS `@import` | #7, #24 | Font loading race conditions | ## Changes - **Rules section**: Added `repeat: -1` ban, async timeline ban, items 8-9 to "Never do" list - **Typography section**: Expanded font size guidance with specific minimums per text role (headlines, body, labels) - **New "Backgrounds and Color" section**: Guidance on avoiding gradient banding - **Output Checklist**: 5 new items covering all new constraints ## Test plan - [ ] Run eval with updated skill and compare avg quality scores - [x] Skill renders correctly in `/hyperframes` invocation
vanceingalls
added a commit
that referenced
this pull request
Apr 16, 2026
Blockers: - #2: late_init_set false positive on fractional opacity (0.5 matched as 0) Fixed: /opacity\s*:\s*0(?![.\d])/ negative lookahead - #3: scene-1 prefix skip matches scene 10+ (s1- matches s10-) Fixed: extract full number and compare exactly High severity: - #4: autoAlpha not covered by late_init_set Fixed: checks both opacity and autoAlpha - #5: al() crashes on non-hex colors (#fff shorthand, rgb(), null) Fixed: guard + shorthand expansion + NaN fallback - #6: "Full palette" with null bg crashes isDark Fixed: null guard defaults to dark - #7: template literals missed by tl_from_in_multiscene Fixed: regex includes backtick quotes Medium: - #9: no retry limit on eval failures → infinite loop Fixed: max 2 retries, then escalate to user - #10: vague ID convention Fixed: explicit s{N}- prefix rule in multi-scene.md - #11: visual-style.md backward compat Fixed: Step 0b checks both filenames - #13: preview_html script injection Fixed: documented prohibition in design-picker.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 tasks
This was referenced Apr 27, 2026
8 tasks
This was referenced May 12, 2026
Merged
This was referenced May 16, 2026
This was referenced May 17, 2026
3 tasks
ukimsanov
added a commit
that referenced
this pull request
May 20, 2026
Three concrete bugs found while auditing PR #991: 1. html-in-canvas-patterns.md (#1 in catalog, 3D Rotation with Bloom): The code example used `new THREE.EffectComposer(renderer)` UMD-style namespace access while the ESM imports right below pull them in as bare named imports. Three.js r150+ removed the UMD `examples/js/` globals, so as written the example throws `TypeError: THREE.EffectComposer is not a constructor`. Switched to the bare names matching the imports. THREE.Vector2 stays as-is — Vector2 is on the THREE namespace. 2. techniques.md (#5, Lottie Animation): The CDN path `@lottiefiles/dotlottie-web/dist/dotlottie-player.js` returns 404. `@lottiefiles/dotlottie-web` is the JavaScript SDK, not a web component — its `main` is `dist/index.cjs`. The web-component package is `@lottiefiles/dotlottie-wc` and the custom element is `<dotlottie-wc>`, not `<dotlottie-player>`. Updated both. 3. techniques.md (5 occurrences across Lottie / lottie-web / Video / @font-face examples): asset paths used the `../capture/` pattern that PR #989's `invalid_capture_path` lint rule emits an error for. Replaced all with root-relative `capture/...`. PRs #989 and #991 are no longer self-contradictory.
ukimsanov
added a commit
that referenced
this pull request
May 20, 2026
Three concrete bugs found while auditing PR #991: 1. html-in-canvas-patterns.md (#1 in catalog, 3D Rotation with Bloom): The code example used `new THREE.EffectComposer(renderer)` UMD-style namespace access while the ESM imports right below pull them in as bare named imports. Three.js r150+ removed the UMD `examples/js/` globals, so as written the example throws `TypeError: THREE.EffectComposer is not a constructor`. Switched to the bare names matching the imports. THREE.Vector2 stays as-is — Vector2 is on the THREE namespace. 2. techniques.md (#5, Lottie Animation): The CDN path `@lottiefiles/dotlottie-web/dist/dotlottie-player.js` returns 404. `@lottiefiles/dotlottie-web` is the JavaScript SDK, not a web component — its `main` is `dist/index.cjs`. The web-component package is `@lottiefiles/dotlottie-wc` and the custom element is `<dotlottie-wc>`, not `<dotlottie-player>`. Updated both. 3. techniques.md (5 occurrences across Lottie / lottie-web / Video / @font-face examples): asset paths used the `../capture/` pattern that PR #989's `invalid_capture_path` lint rule emits an error for. Replaced all with root-relative `capture/...`. PRs #989 and #991 are no longer self-contradictory.
ukimsanov
added a commit
that referenced
this pull request
May 20, 2026
Three concrete bugs found while auditing PR #991: 1. html-in-canvas-patterns.md (#1 in catalog, 3D Rotation with Bloom): The code example used `new THREE.EffectComposer(renderer)` UMD-style namespace access while the ESM imports right below pull them in as bare named imports. Three.js r150+ removed the UMD `examples/js/` globals, so as written the example throws `TypeError: THREE.EffectComposer is not a constructor`. Switched to the bare names matching the imports. THREE.Vector2 stays as-is — Vector2 is on the THREE namespace. 2. techniques.md (#5, Lottie Animation): The CDN path `@lottiefiles/dotlottie-web/dist/dotlottie-player.js` returns 404. `@lottiefiles/dotlottie-web` is the JavaScript SDK, not a web component — its `main` is `dist/index.cjs`. The web-component package is `@lottiefiles/dotlottie-wc` and the custom element is `<dotlottie-wc>`, not `<dotlottie-player>`. Updated both. 3. techniques.md (5 occurrences across Lottie / lottie-web / Video / @font-face examples): asset paths used the `../capture/` pattern that PR #989's `invalid_capture_path` lint rule emits an error for. Replaced all with root-relative `capture/...`. PRs #989 and #991 are no longer self-contradictory.
ukimsanov
added a commit
that referenced
this pull request
May 20, 2026
Three concrete bugs found while auditing PR #991: 1. html-in-canvas-patterns.md (#1 in catalog, 3D Rotation with Bloom): The code example used `new THREE.EffectComposer(renderer)` UMD-style namespace access while the ESM imports right below pull them in as bare named imports. Three.js r150+ removed the UMD `examples/js/` globals, so as written the example throws `TypeError: THREE.EffectComposer is not a constructor`. Switched to the bare names matching the imports. THREE.Vector2 stays as-is — Vector2 is on the THREE namespace. 2. techniques.md (#5, Lottie Animation): The CDN path `@lottiefiles/dotlottie-web/dist/dotlottie-player.js` returns 404. `@lottiefiles/dotlottie-web` is the JavaScript SDK, not a web component — its `main` is `dist/index.cjs`. The web-component package is `@lottiefiles/dotlottie-wc` and the custom element is `<dotlottie-wc>`, not `<dotlottie-player>`. Updated both. 3. techniques.md (5 occurrences across Lottie / lottie-web / Video / @font-face examples): asset paths used the `../capture/` pattern that PR #989's `invalid_capture_path` lint rule emits an error for. Replaced all with root-relative `capture/...`. PRs #989 and #991 are no longer self-contradictory.
miguel-heygen
added a commit
that referenced
this pull request
Jul 9, 2026
## What Fixes five reported false-positive/false-negative patterns in the WCAG contrast audit (`hyperframes validate --contrast`): 1. **SVG fill vs. text color** — foreground read from CSS `color` instead of SVG `fill`. 2. **Cross-component color bleed** — background estimate bleeds into a neighboring panel/layer. 3. **Backdrop-filter glass text** — background estimate misses the blur/tint and reads the raw backdrop. 4. **Partially-overlapping translucent decoration** — a decorative shape inside or partly touching the text's bbox goes undetected. 5. **Solid-fill pill/button** — investigated, did **not** reproduce; already handled correctly by the existing own-background ancestor walk. Not touched. ## Why The audit estimated an element's background two ways: - foreground: always `getComputedStyle(el).color` — wrong for SVG `<text>`/`<tspan>`, which is painted via `fill`, an independent CSS property. - background: a 4px pixel ring sampled just **outside** the text's bounding box, with a fallback to an ancestor's opaque `background-color` for solid pills/buttons. The ring is a proximity heuristic. It's wrong whenever what's immediately outside the text differs from what's actually behind it: - text near the edge of its own panel, with a differently-colored sibling panel/layer just past the bbox — the ring samples the neighbor. - a `backdrop-filter: blur()` glass panel sized only a couple pixels larger than the text — the ring exits the panel into the raw, unblurred, untinted backdrop. - a translucent decoration that only partially overlaps the ring, or sits entirely **inside** the bbox — invisible to the ring regardless of size. ## How **SVG fill (#1):** elements inside an `<svg>` (`el.ownerSVGElement`) now prefer the computed `fill` when it resolves to a solid `rgb()`/`rgba()` color, falling back to `color` for paint values that aren't a plain color (`none`, `context-fill`, gradient/pattern refs). **Cross-comp bleed / glass blur / partial decoration (#2–#4):** replaced the ring-sampling + own-background-ancestor-walk heuristic with a two-phase capture: 1. `__contrastAuditPrepare()` walks the DOM, computes each candidate's foreground (unchanged logic from #1), and **hides that element's own text paint** (`color`/`fill` → `transparent`, layout-neutral — no reflow). 2. The caller takes **one** screenshot with the glyphs invisible (same number of screenshots as before — just moved after the hide instead of before it). 3. `__contrastAuditFinish(imgBase64, time, candidates)` restores the original paint immediately, then samples the **real composited pixels directly inside each element's own bbox** — no proximity heuristic needed, since these are the exact pixels that were behind the glyphs. This is a real architectural change to `contrast-audit.browser.js`'s calling contract (single `__contrastAudit` → `__contrastAuditPrepare`/`__contrastAuditFinish`), with `validate.ts`'s `runContrastAudit` updated to match, including a try/finally restore-safety-net so a mid-loop screenshot/decode failure can't leave a later sample auditing a page with stale hidden text. Mirrored the identical change in `skills/hyperframes-creative/scripts/contrast-report.mjs`, which duplicates the same DOM-walk/sampling logic (not just the WCAG math). There, the **visible** frame for the human-facing overlay image still comes from the producer's normal `captureFrameToBuffer` path (unchanged); only the **background-sampling** capture is a plain `session.page.screenshot()` taken after hiding text — deliberately bypassing `captureFrameToBuffer`, whose static-frame dedup cache knows nothing about the DOM mutation and would hand back a stale pre-mutation buffer. **Solid-fill pill (#5):** reproduced a rounded pill/button with a busy page background outside it. The existing own-background ancestor walk already resolves the pill's declared `background-color` correctly regardless of the rounded corners — confirmed via repro, both before and after this change report the identical (correct) result. No fix needed; left untouched, and this case is covered by the new architecture too (would give the same right answer even without the ancestor-walk fallback). Added `packages/cli/src/commands/contrast-sample.ts` (mirroring the existing `contrast-bg.ts`/`contrast-fg.ts` pattern) hosting the pure sample-rect/grid-point computation, unit tested — the browser-injected scripts can't import it directly, so it's kept in sync by hand, same convention as the rest of this file. ## Test plan - [x] Unit tests: `contrast-fg.test.ts` (SVG fill resolution), `contrast-sample.test.ts` (sample-rect clamping/degenerate cases), plus the full `packages/cli` suite (1424 tests) passes, including an updated `layout-audit.browser.test.ts` case that called the old single-function `__contrastAudit` API directly. - [x] Manual verification — standalone `puppeteer-core` harness against real `chrome-headless-shell`, one minimal HTML fixture per pattern, comparing the audit's reported ratio/verdict against a hand-constructed ground truth: - **SVG fill**: `fill:white` / no `color` on black bg → before: `fg=rgb(0,0,0)` ratio `1:1` (false FAIL); after: `fg=rgb(255,255,255)` ratio `21:1` (correct PASS). - **Cross-comp bleed**: text on a black sibling highlight box 2px larger than the text, white page bg outside it → before: `bg=rgb(255,255,255)` ratio `1.23:1` (false FAIL); after: `bg=rgb(0,0,0)` ratio `17.14:1` (correct PASS). - **Glass blur**: black text on an 18%-white-tinted `backdrop-filter: blur(14px)` panel over a yellow/blue gradient, panel only ~2px larger than the text → before: `bg=rgb(0,64,255)` (raw gradient color, blur/tint completely missed) ratio `3.18:1` (false FAIL); after: `bg=rgb(159,160,165)` (correct blurred/tinted blend) ratio `8.05:1` (correct PASS). - **Partial decoration**: text 92%-covered by a translucent white badge on a dark bg → before: `bg=rgb(16,16,16)` (ring never touches the badge, which sits entirely inside the bbox) ratio `17.45:1` (false PASS); after: `bg=rgb(171,171,171)` (correctly detects the badge) ratio `2.11:1` (correct FAIL). - **Solid pill sanity**: unaffected — `bg=rgb(10,10,10)` ratio `19.8:1` before and after. - [x] End-to-end: ran the actual `hyperframes validate --contrast` CLI command (via `tsx src/cli.ts`) against a real scaffolded project containing all 4 patterns simultaneously — only the genuinely-failing case (the 92%-covered decoration) is reported (`1.09:1`, need `3:1`); the cross-comp-bleed, glass-blur, and solid-pill cases are correctly silent. A second vanilla scaffold with plain white-on-dark text produces zero false positives. - [x] `oxlint`, `oxfmt --check`, and `tsc --noEmit` all pass on the changed files.
3 tasks
3 tasks
meefs
pushed a commit
to meefs/hyperframes
that referenced
this pull request
Jul 16, 2026
Field signal ts=1784040753 (#hyperframes-cli-feedback): a composition with ~40 heavy overlay DOM elements — `filter:blur`, oversized `radial-gradient`, and `clip-path` animations — captures solid-black for the first ~half of the render, recovering near the end. Reproduces identically via drawElement AND forced --no-browser-gpu screenshot capture AND `snapshot`, so the capture layer itself is the offender, not encoder/mux. Independent of duration (padding the timeline grows the bad zone proportionally, doesn't shift it). Presence alone matters — even opacity:0 / visibility:hidden / unused overlays contribute. Reporter's workaround was splitting into per-transition mini-compositions + FFmpeg concat. Add compositionCheck rule `composition_heavy_overlay_count_high` (warning). Counts DOM elements that carry any of: inline `style` filter:blur / clip-path (non-none) / radial-gradient, or a class/id whose top-level CSS rule body sets one of those. `display:none` elements are counted-out (removed from render tree); opacity:0 / visibility:hidden overlays are counted-in per the field-signal repro shape. Warns at 25 to give lead time before the observed 40-element bad zone. Skips registry source and installed-block files, mirroring `composition_file_too_large`. Includes a `ts=1784040753` reference in fixHint so authors can trace the risk shape. Stack: PR heygen-com#5 of 9 (base via/parity-telemetry-gate). Signed-off-by: Via Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
3 tasks
3 tasks
This was referenced Jul 27, 2026
ukimsanov
added a commit
that referenced
this pull request
Aug 5, 2026
…omplete
HoverVideo now: (1) exposes a focusable button that toggles/plays sound by
keyboard, touch and pointer — hover is an enhancement, not the only path;
(2) gates the source behind an IntersectionObserver so offscreen cards no longer
download/decode, releasing the buffer on exit; (3) still blocks autoplay under
reduced motion while allowing voluntary playback via the button. Routed all 30
Thirty-Days cards through it (hasAudio={false} for the 12 silent ones) so no raw
<video autoPlay> bypasses the motion guard. Flagged by Magi (P1 #4/#5/#6).
ukimsanov
added a commit
that referenced
this pull request
Aug 5, 2026
ReplicaCompare now assigns/decodes its reference+replica only near the viewport and releases both (pause + removeAttribute + load) on exit, closing the last eager-load surface from Magi's #5. Sync + reduced-motion guard preserved.
ukimsanov
added a commit
that referenced
this pull request
Aug 5, 2026
carriedSectionsFrom() decided whether a ## Usage section was generated by matching its first line against a list of historical opener phrases — so a hand-written Usage section that happened to open that way was classified as generated and silently deleted on regeneration. Ownership is now purely set membership: a section is generated iff its heading is one the template emits, and ambiguous 'usage' is no longer in that set (the template never emits it), so any ## Usage is carried. Exported carriedSectionsFrom behind an entrypoint guard and added two executable preservation fixtures. Flagged by Magi (#5).
dahans-msft2
referenced
this pull request
in dahans-msft2/hyperframes
Aug 6, 2026
…m#217) ## Summary Adds critical rendering constraints to the `hyperframes` skill discovered from eval analysis of 27 agent-generated compositions. These guardrails prevent agents from producing compositions that technically work but render poorly. ## What it fixes | Rule Added | Eval Prompts Affected | Issue | | --- | --- | --- | | Ban `repeat: -1` | #20 loading-spinner (2.0/5) | Infinite timeline broke capture engine | | Ban async timeline construction | #16 particle-logo (2.6/5) | Timeline empty at capture time | | Min font size 16px (labels), 20px (body) | #7, #8, #13, #14, #15, #19 | Illegible text after encoding | | Ban full-screen dark linear gradients | #3, #5, #10, #14 | H.264 color banding | | `<link>` fonts over CSS `@import` | #7, #24 | Font loading race conditions | ## Changes - **Rules section**: Added `repeat: -1` ban, async timeline ban, items 8-9 to "Never do" list - **Typography section**: Expanded font size guidance with specific minimums per text role (headlines, body, labels) - **New "Backgrounds and Color" section**: Guidance on avoiding gradient banding - **Output Checklist**: 5 new items covering all new constraints ## Test plan - [ ] Run eval with updated skill and compare avg quality scores - [x] Skill renders correctly in `/hyperframes` invocation
dahans-msft2
referenced
this pull request
in dahans-msft2/hyperframes
Aug 6, 2026
Three concrete bugs found while auditing PR heygen-com#991: 1. html-in-canvas-patterns.md (#1 in catalog, 3D Rotation with Bloom): The code example used `new THREE.EffectComposer(renderer)` UMD-style namespace access while the ESM imports right below pull them in as bare named imports. Three.js r150+ removed the UMD `examples/js/` globals, so as written the example throws `TypeError: THREE.EffectComposer is not a constructor`. Switched to the bare names matching the imports. THREE.Vector2 stays as-is — Vector2 is on the THREE namespace. 2. techniques.md (#5, Lottie Animation): The CDN path `@lottiefiles/dotlottie-web/dist/dotlottie-player.js` returns 404. `@lottiefiles/dotlottie-web` is the JavaScript SDK, not a web component — its `main` is `dist/index.cjs`. The web-component package is `@lottiefiles/dotlottie-wc` and the custom element is `<dotlottie-wc>`, not `<dotlottie-player>`. Updated both. 3. techniques.md (5 occurrences across Lottie / lottie-web / Video / @font-face examples): asset paths used the `../capture/` pattern that PR heygen-com#989's `invalid_capture_path` lint rule emits an error for. Replaced all with root-relative `capture/...`. PRs heygen-com#989 and heygen-com#991 are no longer self-contradictory.
dahans-msft2
referenced
this pull request
in dahans-msft2/hyperframes
Aug 6, 2026
…gen-com#1466) (heygen-com#1539) * fix(studio): restore timeline move/resize fallback parity (review heygen-com#1466) The §3.2 sdkTimingPersist rewrite regressed the non-SDK fallback path vs the pre-cutover behavior. Restored, on both fallback entry points (no-session and sdkTimingPersist-returned-unhandled): - Resize live DOM patch dropped the conditional data-playback-start/media-start attr — restored so a start-trim updates the preview's in-point immediately. - Move/resize fallback dropped the GSAP-position sync (shift/scaleGsapPositions) + reloadPreview — restored so server-path edits keep GSAP tweens in sync and refresh the preview (the SDK path folds both into setTiming). - Undo-coalesce drift: fallback enqueueEdit carried no coalesceKey while the SDK branch did — plumbed coalesceKey through persistTimelineEdit so undo granularity is identical on either path. - Documented the hasPbsAdjustment second clause + sdkTimingPersist before-capture transition limitation. Flag-off (dark launch) so this lands as one fix PR at the stack tip rather than restacking the mid-stack §3.2 commit. heygen-com#1500 review items: parity-harness gap already closed at the tip (arc/unroll recast-vs-acorn parity added); blockRemoveRange flagged 'potential' but verified correct (no comma residue on any block position). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk): retire duplicate removeGsapKeyframe keyframeIndex variant (review heygen-com#1498) EditOp had two removeGsapKeyframe members with the same discriminant but different shapes (keyframeIndex vs percentage) — TS can't discriminate them and a handler could get the wrong shape. Per both reviewers (option 2): retire the keyframeIndex variant. It had no production caller (Studio dispatches percentage only); removed the dead by-index handleRemoveGsapKeyframe + simplified the dispatcher. resolveKeyframe stays (setGsapKeyframe still uses keyframeIndex). Converted the one by-index test to the percentage API. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio): gate ALL cutover persist paths on the flag — true dark launch (review heygen-com#1469 finding #6) Only sdkCutoverPersist (style/text/attr) checked STUDIO_SDK_CUTOVER_ENABLED. sdkTimingPersist, dispatchGsapOpAndPersist (every GSAP op) and sdkDeletePersist guarded only on `!sdkSession` — and useSdkSession opens a session by default for shadow/selection, so timing/GSAP/keyframe/delete cutover was ALWAYS live regardless of the flag. Flipping the flag OFF could not disable it, so the data-loss bugs in those paths (single-prop wipe, wrong-keyframe match, tween collapse, arc strip) ship LIVE on merge instead of being dark-launched. Added the flag guard at all three chokepoints → flag OFF returns false → callers fall back to the legacy server path. Makes the stack genuinely dark-launchable: merge is now a no-op in prod, and the remaining cutover correctness bugs become flip-prerequisites rather than merge-blockers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(core,sdk): correct 8 GSAP write-path review findings (heygen-com#1539) Eight correctness bugs from the SDK-cutover review. Several were cases where BOTH writers were identically wrong, so the recast-vs-acorn parity suite stayed green; the new tests assert the real-world-correct result, not agreement. - #2 findKfPropByPct: match the CLOSEST keyframe within tolerance, not the first within 2% — removing/updating 50% on 0/49/50/100 no longer hits 49%. - #3 handleSetTiming: shift each tween by the start DELTA and scale duration by the clip-duration RATIO per-tween, instead of writing absolute newStart/ newDuration onto every tween (which collapsed staggers and blew durations). - #4 enableArcPath: insert motionPath via appendRight at the object start so the insertion can't collide with the x/y remove-range end (which made MagicString discard the append and emit '{}'). - #5 splitAnimationsInScript: compute the inherited baseline in a forward pre-pass so the split-spanning midpoint sees earlier tweens (the reverse write loop is kept for stable count-suffixed ids). - #9 unrollDynamicAnimations: preserve non-target loop-body statements (e.g. tl.set initial-state) per iteration instead of overwriting the whole loop. - #10 buildMotionPathObjectCode (both writers): emit the cubic form when segment curviness varies so per-segment curviness survives, not just segments[0]. - #11 readLastWaypointXY: handle UnaryExpression so negative destination coords are recovered when disabling an arc path. - #15 no-bang: removed every `!` non-null assertion in the touched files, replaced with guards/fallbacks. Tests: gsapWriter.reviewFixes.test.ts (#2/#4/#5/#9/#10/#11) and mutate.gsap.test.ts setTiming GSAP-sync block (#3). All fail on the base and pass after the fix; tsc + full core/sdk suites + parity stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio): SDK cutover review fixes — merge tween props, stabilize debounce, serialize gsap writes, on-disk undo baseline, self-write identity Addresses 5 SDK-cutover review findings (studio-only): - #1 useGsapPropertyDebounce: editing one GSAP tween property no longer drops the tween's other animated props. setGsapTween REPLACES the property set, so merge the single edit into the tween's CURRENT properties (read from the SDK doc) before dispatching, mirroring the legacy server merge. - #7 useGsapPropertyDebounce: stabilize the flush callback by reading sdk deps from a ref instead of an unmemoized literal, so a parent re-render mid-edit no longer tears down + flushes the debounce (one commit/undo entry per render). - #8 sdkCutover/useGsapScriptCommits: route SDK gsap-write persists through the same per-file keyed serializer the legacy commitMutation uses, so concurrent same-file read-modify-writes can't interleave and lose an edit. - #12 sdkCutover/useTimelineEditing: capture the exact on-disk bytes as the undo 'before' for timing/GSAP persists (matching the style/delete paths) instead of a normalized SDK serialize() re-emit that reformatted the whole file on undo. - #14 useSdkSession/sdkSelfWriteRegistry: discriminate a cutover echo from an undo write by CONTENT identity (registered self-write hash), not just the 2 s timestamp window — an undo write always reloads the SDK session. Tests: useGsapPropertyDebounce(.test), useGsapPropertyDebounceFlush.test, sdkSelfWriteRegistry.test, and new sdkCutover.test cases; each reproduces the review scenario and asserts the corrected behavior (verified red before fix). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(core): extract split/collapse helpers to satisfy no-fallow-ignore rule The #5 (split) and #15 (no-bang guards) fixes pushed splitAnimationsInScript and removeAllKeyframesFromScript over fallow's complexity threshold, and a fallow-ignore had been added to splitAnimationsInScript. Per the hard rule (never ignore — fix), extracted buildSpanningSplit + applyTweenSplit (split) and buildCollapsedFlatVars (collapse), and removed the ignore. Both functions now under threshold; fallow new-only gate reports 0 new findings. Behavior unchanged — core 1811 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(studio): pin dark-launch flag-gate contract (review heygen-com#1539, Rames/Via) flag OFF ⇒ sdkTimingPersist / sdkGsapTweenPersist (GSAP-op chokepoint) / sdkDeletePersist all return false even with a valid session → legacy fallback. The prod flag-flip rests on this contract; sdkCutover.test.ts only mocks the flag TRUE, so a future gate refactor could silently re-enable cutover on flag-off without failing CI. This sibling file mocks it FALSE and locks the three guards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio): leading flag-gate on sdkGsapTweenPersist (review heygen-com#1539 nit, Via) The add-op getElement existence check ran before the inner gate, so flag-off did an SDK touch before falling back. Lead with the flag guard to match the other three chokepoints — flag-off is now a clean no-op at every entry point. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(core): unroll-preservation regressions — non-for loops + AST index substitution (review R2) The #9 unroll-preservation fix had two confirmed regressions: - Non-for loops (forEach/for-of/for-in/while): loopIndexVarName returns null, so substitution no-op'd and preserved siblings kept a now-undefined loop variable (e.g. `item`) → ReferenceError at render. Now returns null for those forms → caller falls back to the blanket loop overwrite (drops siblings, valid code). The #9 fixture only used `for(let i…)` so it never caught this. - substituteLoopIndex did a \bvar\b regex over raw source including string literals, corrupting selectors like ".row-i" → ".row-0". Now AST-based: substitutes only real Identifier uses, skipping string literals and non-computed member/key positions (extracted isIndexBindingPosition helper to stay under the fallow complexity threshold — no ignore added). Two regression tests added (forEach no-dangling-var; for-loop string-literal intact). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk,core): unrollDynamicAnimations rejects empty element list (R1 #1501b) An empty `elements` array has no unrolled form — the writer would overwrite the loop/statement with zero tween calls, silently deleting the animation. - gsapWriterAcorn: unrollDynamicAnimations returns the script verbatim on an empty list (no-op instead of a destructive overwrite). - validateOp: reject unrollDynamicAnimations with empty elements as E_INVALID_ARGS so callers get a clean error rather than silent corruption. - Tests: writer no-op on []; validateOp E_INVALID_ARGS on []. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(sdk): cache draft element in applyDraft, drop HTMLElement casts (R1 #1490a) applyDraft runs at 60fps during a drag but re-ran doc.querySelector on every call — the _draftEl/_draftId fields were only consumed by commit/cancel, never to skip the query. Reuse the tracked element when the id matches and the node is still connected; re-query only on id change or detach (iframe reload). Retypes _draftEl to HTMLElement | null (only ever set from querySelector<HTMLElement>), which removes the `as HTMLElement` casts in commitPreview / _clearDraft. Test asserts a repeated same-id drag queries once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk,core): round-3 correctness — unroll AST safety, single-dispatch undo, empty-arg guards, persist decouple Addresses the highest-severity round-3 review findings: - gsapWriterAcorn unroll (R3 #1/#2/#9): the round-2 AST-substitution fix emitted invalid GSAP for object shorthand `{ i }` (→ `{ 0 }`) and shadowed inner bindings (→ `for(let i=0;0<3;0++)`), and silently dropped sibling statements on non-`for` loops (forEach/for-of). The unroll now REFUSES (no-ops, leaving the dynamic loop intact) whenever siblings can't be safely reproduced — a non-`for` loop, an unmodeled statement, or an unsafe index use — instead of dropping or corrupting. Plain `for` loops with safe siblings still unroll. - session single-dispatch undo (R3 #5/#11): _dispatch now reverses the inverse patch list (parity with batch()). A single op emitting order-dependent inverse patches — a nested parent+child removeElement, an aliased multi-target — undid forward and dropped the child subtree / landed on an intermediate value. - materializeKeyframes empty-array (R3 #10): the unguarded twin of the just-fixed unrollDynamicAnimations. Writer no-ops on an empty keyframe list; validateOp rejects it as E_INVALID_ARGS (shared gsapScriptMissing helper). - history:false persist decouple (R3 #4): persist (auto-save) no longer lives inside the history-enable block, so opting out of SDK undo no longer silently disables all disk writes (data-loss trap for heygen-com#1496's flag consumers). Tests: unroll refuse cases (shorthand/shadow/forEach) + safe-for-loop regression; nested removeElement undo; materializeKeyframes writer no-op + validateOp reject; history:false-still-persists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(core): stripGsapForId re-parses per removal so all tweens for a deleted element are stripped (R3 #3) Animation ids are count-based (positional), so removing one tween renumbers the survivors. stripGsapForId captured every matching id from a single up-front parse then removed against the mutating script — after the first removal the later ids were stale and silently no-op'd, leaving an orphaned tl.to() referencing the just-deleted element. Now re-parse after each removal and strip the first still-matching animation until none remain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(core): gsap writer — keyframe ease routing, convert preserves delay, addLabel dedup (R3 #7/#8/#12) - #7: updateAnimationInScript routes an ease update on a keyframe tween to keyframes.easeEach (per-keyframe), not a top-level ease that GSAP ignores — the user's keyframe-easing edit was silently a no-op. - #8: convertToKeyframesFromScript now preserves every non-editable vars key (delay/callbacks/stagger/yoyo/…) verbatim via preservedVarsEntries instead of rebuilding from the GsapAnimation object, which had no `delay` field and dropped it — shifting the tween's start time. - #12: addLabelToScript moves an existing same-named label (overwrites its position) instead of appending a duplicate; duplicates made removeLabel over-remove (it deletes every match, including a pre-existing label). Tests: easeEach routing, delay preservation, addLabel move-not-duplicate + hand-authored-dup removal. Updated the old "no dedup contract" corpus test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk): handleSetTiming #domId + data-duration sync; validateOp resolves ids + arc/selector (R3 #6/#13, CF2 #15/#16) CF2 #15: handleSetTiming re-synced GSAP tweens only when the selector matched the element's hf-id. The common #domId-targeted tween (authored by the Studio panel) never matched, so moving/resizing a clip via the SDK timing path left its animations unsynced. Now match the tween selector against the DOM id too. CF2 #16: handleSetTiming read/wrote only data-end. Clips authored with data-duration (what the runtime prefers) got a fresh data-end beside a stale data-duration (no playback change) and oldDuration=null collapsed the GSAP duration-scale ratio to 1. Now read duration preferring data-duration, and write back to whichever attribute the clip uses (timingPath gains a "duration" field). R3 #13b: deleteAllForSelector compared selectors with strict === and missed the alternate quote style ([data-hf-id='x'] vs "x"); now quote-insensitive. R3 #6/#13a: validateOp now resolves the animationId for id-bearing GSAP ops (E_TARGET_NOT_FOUND instead of a misleading ok that no-ops at apply), and updateArcSegment validates the arc is enabled + the segment index is in range. Tests: #domId move sync, data-duration resize + scale, quote-insensitive delete, unresolved-id rejection, arc-segment preconditions. Updated the loose-can() test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(core,sdk): name the acorn-node type alias; keyToPath round-trips timing.duration (R3 #14) - gsapWriterAcorn: replace the bare `: any` AST-node annotations with the named `type Node = any` alias, matching the established convention in gsapParserAcorn.ts / gsapInline.ts ("acorn ESTree nodes are structurally untyped"). Documents intent and is greppable; type-identical (zero runtime change). A full ESTree typing is a deliberate architecture decision the codebase has not taken and is out of scope here. - patches: keyToPath/timingPath now include the "duration" timing field added for the data-duration resize fix, so a timing.duration override round-trips on T3 replay instead of being dropped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk): cascadeRemoveAnimations re-parses per removal (R4 — SDK twin of #3) cascadeRemoveAnimations captured every matching animation id from a single up-front parse, then removed against the mutating script — the SDK-side twin of the stripGsapForId bug (R3 #3). Animation ids are positional, so removing the first tween for an element renumbered the survivors and the stale later ids no-op'd, orphaning those tweens on the just-removed element. Now re-parse after each removal and strip the first still-matching animation until none remain. Also adds the reviewer's defense-in-depth test: an aliased multi-target setStyle (same id twice) undoes to the original, not the intermediate (exercises the single-dispatch inverse reversal from R3 #5/#11). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dahans-msft2
referenced
this pull request
in dahans-msft2/hyperframes
Aug 6, 2026
…st default (WS-3F) (heygen-com#1573) * refactor(core): retire recast/babel, route all GSAP mutations to acorn (WS-E/3.F) - Delete gsapParser.ts (2595-line recast-based parser/writer) - Delete gsapParser.test.ts, gsapParser.stress.test.ts, gsapParser.test-helpers.ts - Add gsapParserExports.ts: re-export umbrella for gsap-parser subpath - Move SplitAnimationsOptions/SplitAnimationsResult to gsapSerialize.ts - executeGsapMutation: async->sync, static acorn imports replace loadGsapParser() - Fix 3 function name mismatches in files.ts switch cases - generators/hyperframes.ts: imports from gsapSerialize (blocker resolved) - gsapWriterAcorn.ts: SplitAnimationsOptions from gsapSerialize - Parity tests: recast oracle removed; acorn-only regression (14 pass) - Remove recast and @babel/parser from core/package.json Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sdk): harden mutation handlers + widen variable API (code-review) Self-contained review fixes for the SDK-hotspot stack (heygen-com#1569–heygen-com#1573). The dispatch path (_dispatch → applyOp) never runs validateOp, so the new WS-D/WS-3.C guards were advisory-only; re-enforce them in the handlers. - addElement: null-guard the resolved parent (no more `as Element` masking a null → crash on unknown parent id); reject <script> and multi-root fragments via parseInsertableFragment instead of inserting raw markup / silently dropping extra roots. - addWithKeyframes / replaceWithKeyframes: bail on empty keyframes (no degenerate `keyframes: {}` tween) and when the animationId resolves to nothing (no silent degrade-to-add leaving a duplicate tween). - isObjectVariableValue: exclude arrays so an array override value can't be misclassified as a font/image object and written into the variable model. - Composition.setVariableValue: widen the public interface signature to `… | FontValue | ImageValue` to match the impl + EditOp (B2 object-valued variables were unreachable via the typed API). - mutate.gsap.test.ts: import addKeyframeToScript from gsap-writer-acorn — the gsap-parser subpath no longer re-exports write fns after recast retire, so the test threw at runtime (red suite). - Dedup: export EXCLUDED_TAGS from hfIds.ts and drop the verbatim HF_EXCLUDED_TAGS copy in mutate.ts. Adds guard regression tests. SDK 340/340, core hfIds 13/13, build green, fallow --gate new-only clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk): variable-model dedup + undo/scoped-parent correctness; test honesty (code-review) Second batch of review fixes for the SDK-hotspot stack. - Variable model (#7, #13): extract readVariableDefault/writeVariableDefault into a shared engine/variableModel.ts used by both mutate.ts (forward) and apply-patches.ts (replay), so the model shape can't diverge. Add clearVariableDefault and make a `variable` remove patch DELETE the decl's `default` key — the exact inverse of a first-set on a default-less variable. Previously undo of such a set no-op'd and stranded the value. - addElement scoped parent (#8): record the caller's id verbatim (scoped "hf-host/hf-leaf" path or composition id) as the patch parentId instead of the bare data-hf-id, so redo/replay re-resolves the SAME parent via resolveScoped rather than the canonical top-level dup (or document.body). - resolveTimings honesty (#5): correct the header + test that claimed a live "preview == render" parity — neither path consumes the resolver yet (anchor inputs are Pacific/backend-deferred). It's a pure-function property, not a current guarantee. - GSAP writer parity (#12): the recast oracle was deleted in WS-3.F, leaving the WS-3.C keyframe ops comparing acorn output to itself. Pin them as golden inline snapshots and drop the now-dead recast scaffolding (replaceWithKfRecast, removeAnimRecast alias). Remaining pre-WS-3.C parity blocks noted as follow-up. Adds regression tests (undo of default-less variable; scoped-parent redo). SDK 342/342, core timingResolver+parity green, build + fallow --gate new-only clean. Not changed (need design / out of scope): #9 pre-heygen-com#1569 persisted-override CSS replay (moot for unreleased data; proper fix is render-time CSS derivation), #11 replaceWithKeyframes stale positional id (mitigated by the missing-id no-op guard + type doc; full fix needs non-positional ids). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk): replay CSS-prop derivation for legacy var overrides; stale-id selector guard (code-review) Final review-fix batch — the two items deferred from the prior pass. - #9 legacy variable-override CSS: applyOverrideSet now derives the `--{id}` CSS custom prop from any scalar `var.{id}` override on replay (and removes it for a null override). Sets written before the model/CSS split carried only `var.{id}`; without this, replaying them updated the JSON model but left `var(--{id})` bindings rendering the schema default. Replay-path only — the undo path (applyOne) is untouched, so heygen-com#1569's separate-patch undo correctness is preserved. Object (font/image) values are never CSS, so they are skipped. - #11 stale positional id: replaceWithKeyframes now requires the located animation to still target the caller's `targetSelector`. Position-derived ids re-point after structural edits; a stale id resolving to a DIFFERENT element's tween previously got silently replaced. It now bails (no-op) unless the id still points at the expected selector. Adds regression tests (legacy var.{id}-only override restores CSS; object override writes no CSS; stale-id-wrong-selector replace is a no-op). SDK 345/345, build + fallow --gate new-only clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(core): gate acorn GSAP writer behind cutover flag; keep recast default (WS-3F) Product decision pivot: acorn no longer replaces recast as the GSAP writer. Recast remains the default server writer; acorn runs only when STUDIO_SDK_CUTOVER_ENABLED=true (or =1) is set server-side — the same env flag name as the client Vite var, so a single switch flips both sides. Changes: - Restore gsapParser.ts (recast writer) + test/stress/helper files deleted by 3F - Restore @babel/parser + recast deps in packages/core/package.json - Add isAcornGsapWriterEnabled() + loadGsapParser() to files.ts (lines 59-82) - Split executeGsapMutation into async dispatcher + executeGsapMutationRecast (recast, async via loadGsapParser) + executeGsapMutationAcorn (acorn, sync) - Dispatcher defaults to recast; acorn branch taken only when flag is on - Restore gsapWriter.parity.test.ts, gsapWriterParity.acorn.test.ts, and gsapWriterParity.corpus.test.ts to true recast-vs-acorn differential suites (not acorn-vs-itself) - Exempt gsapParser.ts in .fallowrc.jsonc health.ignore + ignoreExports (pre-existing complexity + barrel re-exports consumed outside diff scope) - Add fallow-ignore-file code-duplication to files.ts (intentional parallel switch bodies for two writers) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
dahans-msft2
referenced
this pull request
in dahans-msft2/hyperframes
Aug 6, 2026
…eygen-com#1738) * feat(cli): add skills version check, update, and freshness manifest Give the HyperFrames skill bundle a content fingerprint so agents and users can tell whether installed skills are the latest version, on any platform that can run the CLI. - skills-manifest.json (repo root): per-skill sha256 over the whole skill directory; minimal {source, skills}, no version/timestamp so it is fully deterministic. Generated by scripts/gen-skills-manifest.ts. - `hyperframes skills check` [--json]: compares installed skills to the manifest; exits non-zero when something is outdated (agent/CI gate). - `hyperframes skills update`: thin wrapper over `npx skills update`. - Passive nudge on render/lint/validate when skills are stale (24h cache, same opt-out as the CLI self-update notice). - "latest" resolved via `git ls-remote` + SHA-pinned raw URL to dodge GitHub raw-CDN lag, falling back to the main branch URL. - CI job + lefthook hook keep skills-manifest.json in sync with skills/. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): add execFile to child_process mock in skills test skills.test.ts mocks node:child_process but only declared execFileSync and spawn. Loading skills.js transitively loads skillsManifest.ts, which runs promisify(execFile) at module load, so vitest threw on the missing execFile named export. Add a bare stub — these tests never invoke it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): init installs all skills; skills update pulls the full set Make `hyperframes init` the single place skills are pulled in full, and make "update" mean "get everything" rather than "refresh what's there". - init now always installs/refreshes ALL skills (incl. ones not yet present) instead of prompting "Install AI coding skills?" — opt out with `init --skip-skills`. Both the interactive and non-interactive paths pass `--all --yes` so the complete set is fetched. - `hyperframes skills update` switches from `npx skills update` (which only refreshes already-installed skills) to `skills add --all`, so it installs missing skills too — the same install step init runs. - SKILL.md documents init-installs-all and the new update semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): skills check treats missing skills as needing an update The full skill set is now the goal (init and `skills update` both pull all, including ones not installed), so a partial install is no longer "a choice" — it's something to fix. - diffSkills: updateAvailable is now true when anything is outdated OR missing (local-only still doesn't count). So `skills check` exits non-zero — and renders "Update:" instead of "up to date" — whenever a skill is missing, not just when one is stale. - The passive render/lint/validate nudge follows suit: it now counts missing alongside outdated ("N skills out of date or missing"), tracked via a new skillsMissingCount cache field. - SKILL.md documents the stricter check. Note: platforms that intentionally vendor only a subset of skills (e.g. a Codex snapshot) will now see check report non-zero. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): install/update skills straight from the GitHub repo `skills add owner/repo` can resolve through the skills.sh registry, which lags behind the repo — so `update` could install a stale version while `check` (which resolves latest directly from GitHub) keeps reporting "outdated", an endless loop. Switch the install source to the full GitHub URL (https://github.com/heygen-com/hyperframes), which makes `skills add` git-clone the repo directly at latest main, bypassing the registry. This covers `hyperframes skills`, `hyperframes skills update`, and `init`'s skill install — all of which go through SOURCES. Now install/update and check agree on what "latest" means. The init "install skills" hint now points at `npx hyperframes skills update` so the manual path uses the same GitHub-direct fetch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): init checks skills against GitHub, installs only when stale `hyperframes init` now runs the skills version check first and only (re)installs when something is outdated or missing — instead of unconditionally re-pulling every time. Re-running init on an already-current project is now a no-op ("skills are already up to date"). - New ensureSkillsCurrent() helper, shared by both the interactive and non-interactive init paths (no duplicated install logic). - The check resolves "latest" straight from GitHub (same source the install uses); best-effort — if it can't reach GitHub it installs anyway. - SKILL.md updated to describe the check-then-install behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): address skills manifest review feedback From the PR review (points 1, 2, 4, 5): 1. Remove the `local-only` skill status. checkSkills only ever hashes manifest-listed skills, so a local-only status could never appear in the end-to-end output — and making it appear would wrongly flag unrelated skills (the `.../skills` dir is shared across sources). diffSkills now reports only on manifest skills; skills on disk that aren't in the manifest are ignored. 2. Drop the redundant per-directory sort in listFilesSorted — the single final out.sort() is what guarantees a deterministic hash (verified: manifest unchanged). 4. resolveLatestManifest local-path detection now uses path.isAbsolute, so Windows absolute paths (C:\...) are treated as local instead of falling through to a remote fetch. 5. fetchManifest validates the response shape (asSkillsManifest) instead of a blind `as` cast, so a CDN error page served as 200 fails with a clear error rather than a cryptic crash later in diffSkills. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): strict skills update + auto-discover any agent host Address PR review (Magi blocker + James/Rames robustness): - Blocker (Magi): `skills update` is the documented recovery path for `skills check || skills update`, but it delegated to installAllSkills() which swallowed missing-npx and failed `skills add` as "skipped", exiting 0 even when nothing changed. Add a strict mode that throws on failure; update sets a non-zero exit (init stays best-effort). New tests simulate a non-zero `skills add` (exit 1) and the success path. - Robustness (James/Rames #2): the upstream `skills` CLI installs into ~72 agent conventions; a hard-coded list (4, or even 11) can't track that. Replace defaultSkillRoots with discoverSkillRoots — it scans cwd + $HOME for any `<host>/skills/<manifest-skill>/SKILL.md` (plus the XDG `.config/<host>/skills`), so detection is structural and future-proof, no closed list. agentFromDir infers the host from the path. - Tests (Rames #3): temp-fixture detection tests for every convention × {project, global}, scope priority, claude-code preference, the no-install case, the --dir override, and an unknown/new host (proving the no-closed-list property). - Docs (Rames #4/#5): SKILL.md notes init's best-effort GitHub round-trip; findRepoManifest climbs 16 levels (was 8) for deep monorepos. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): resolve CodeQL file-system race + de-flake Windows npx test Two CI fixes: - CodeQL (high, js/file-system-race) at gen-skills-manifest.ts: the existsSync(outPath) precheck followed by writeFileSync(outPath) is a check-then-write race. Read the committed manifest directly in a try/catch instead (missing/unreadable ⇒ "no committed manifest"), so there's no precheck to race against. Behavior is unchanged. - Windows Tests: npxCommand.test.ts's real `npx --version` smoke test cold-starts slower than vitest's 5s default on Windows runners and timed out. Give the test 60s headroom (and a 30s exec timeout). Kept as a real execution check — mocking would reduce it to a tautology. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): repair garbled npx smoke-test timeout comment The explanatory comment for the 60s timeout was scrambled across the callback/timeout arguments, failing oxfmt --check (and thus preflight, which in turn skipped preview-parity and failed the regression gate). Move it above the it() call so it no longer sits between call arguments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dahans-msft2
referenced
this pull request
in dahans-msft2/hyperframes
Aug 6, 2026
… + multi-agent mirror (heygen-com#1753) * feat(cli): add skills version check, update, and freshness manifest Give the HyperFrames skill bundle a content fingerprint so agents and users can tell whether installed skills are the latest version, on any platform that can run the CLI. - skills-manifest.json (repo root): per-skill sha256 over the whole skill directory; minimal {source, skills}, no version/timestamp so it is fully deterministic. Generated by scripts/gen-skills-manifest.ts. - `hyperframes skills check` [--json]: compares installed skills to the manifest; exits non-zero when something is outdated (agent/CI gate). - `hyperframes skills update`: thin wrapper over `npx skills update`. - Passive nudge on render/lint/validate when skills are stale (24h cache, same opt-out as the CLI self-update notice). - "latest" resolved via `git ls-remote` + SHA-pinned raw URL to dodge GitHub raw-CDN lag, falling back to the main branch URL. - CI job + lefthook hook keep skills-manifest.json in sync with skills/. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): add execFile to child_process mock in skills test skills.test.ts mocks node:child_process but only declared execFileSync and spawn. Loading skills.js transitively loads skillsManifest.ts, which runs promisify(execFile) at module load, so vitest threw on the missing execFile named export. Add a bare stub — these tests never invoke it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): init installs all skills; skills update pulls the full set Make `hyperframes init` the single place skills are pulled in full, and make "update" mean "get everything" rather than "refresh what's there". - init now always installs/refreshes ALL skills (incl. ones not yet present) instead of prompting "Install AI coding skills?" — opt out with `init --skip-skills`. Both the interactive and non-interactive paths pass `--all --yes` so the complete set is fetched. - `hyperframes skills update` switches from `npx skills update` (which only refreshes already-installed skills) to `skills add --all`, so it installs missing skills too — the same install step init runs. - SKILL.md documents init-installs-all and the new update semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): skills check treats missing skills as needing an update The full skill set is now the goal (init and `skills update` both pull all, including ones not installed), so a partial install is no longer "a choice" — it's something to fix. - diffSkills: updateAvailable is now true when anything is outdated OR missing (local-only still doesn't count). So `skills check` exits non-zero — and renders "Update:" instead of "up to date" — whenever a skill is missing, not just when one is stale. - The passive render/lint/validate nudge follows suit: it now counts missing alongside outdated ("N skills out of date or missing"), tracked via a new skillsMissingCount cache field. - SKILL.md documents the stricter check. Note: platforms that intentionally vendor only a subset of skills (e.g. a Codex snapshot) will now see check report non-zero. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): install/update skills straight from the GitHub repo `skills add owner/repo` can resolve through the skills.sh registry, which lags behind the repo — so `update` could install a stale version while `check` (which resolves latest directly from GitHub) keeps reporting "outdated", an endless loop. Switch the install source to the full GitHub URL (https://github.com/heygen-com/hyperframes), which makes `skills add` git-clone the repo directly at latest main, bypassing the registry. This covers `hyperframes skills`, `hyperframes skills update`, and `init`'s skill install — all of which go through SOURCES. Now install/update and check agree on what "latest" means. The init "install skills" hint now points at `npx hyperframes skills update` so the manual path uses the same GitHub-direct fetch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): init checks skills against GitHub, installs only when stale `hyperframes init` now runs the skills version check first and only (re)installs when something is outdated or missing — instead of unconditionally re-pulling every time. Re-running init on an already-current project is now a no-op ("skills are already up to date"). - New ensureSkillsCurrent() helper, shared by both the interactive and non-interactive init paths (no duplicated install logic). - The check resolves "latest" straight from GitHub (same source the install uses); best-effort — if it can't reach GitHub it installs anyway. - SKILL.md updated to describe the check-then-install behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): address skills manifest review feedback From the PR review (points 1, 2, 4, 5): 1. Remove the `local-only` skill status. checkSkills only ever hashes manifest-listed skills, so a local-only status could never appear in the end-to-end output — and making it appear would wrongly flag unrelated skills (the `.../skills` dir is shared across sources). diffSkills now reports only on manifest skills; skills on disk that aren't in the manifest are ignored. 2. Drop the redundant per-directory sort in listFilesSorted — the single final out.sort() is what guarantees a deterministic hash (verified: manifest unchanged). 4. resolveLatestManifest local-path detection now uses path.isAbsolute, so Windows absolute paths (C:\...) are treated as local instead of falling through to a remote fetch. 5. fetchManifest validates the response shape (asSkillsManifest) instead of a blind `as` cast, so a CDN error page served as 200 fails with a clear error rather than a cryptic crash later in diffSkills. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): strict skills update + auto-discover any agent host Address PR review (Magi blocker + James/Rames robustness): - Blocker (Magi): `skills update` is the documented recovery path for `skills check || skills update`, but it delegated to installAllSkills() which swallowed missing-npx and failed `skills add` as "skipped", exiting 0 even when nothing changed. Add a strict mode that throws on failure; update sets a non-zero exit (init stays best-effort). New tests simulate a non-zero `skills add` (exit 1) and the success path. - Robustness (James/Rames #2): the upstream `skills` CLI installs into ~72 agent conventions; a hard-coded list (4, or even 11) can't track that. Replace defaultSkillRoots with discoverSkillRoots — it scans cwd + $HOME for any `<host>/skills/<manifest-skill>/SKILL.md` (plus the XDG `.config/<host>/skills`), so detection is structural and future-proof, no closed list. agentFromDir infers the host from the path. - Tests (Rames #3): temp-fixture detection tests for every convention × {project, global}, scope priority, claude-code preference, the no-install case, the --dir override, and an unknown/new host (proving the no-closed-list property). - Docs (Rames #4/#5): SKILL.md notes init's best-effort GitHub round-trip; findRepoManifest climbs 16 levels (was 8) for deep monorepos. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): resolve CodeQL file-system race + de-flake Windows npx test Two CI fixes: - CodeQL (high, js/file-system-race) at gen-skills-manifest.ts: the existsSync(outPath) precheck followed by writeFileSync(outPath) is a check-then-write race. Read the committed manifest directly in a try/catch instead (missing/unreadable ⇒ "no committed manifest"), so there's no precheck to race against. Behavior is unchanged. - Windows Tests: npxCommand.test.ts's real `npx --version` smoke test cold-starts slower than vitest's 5s default on Windows runners and timed out. Give the test 60s headroom (and a 30s exec timeout). Kept as a real execution check — mocking would reduce it to a tautology. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): repair garbled npx smoke-test timeout comment The explanatory comment for the 60s timeout was scrambled across the callback/timeout arguments, failing oxfmt --check (and thus preflight, which in turn skipped preview-parity and failed the regression gate). Move it above the it() call so it no longer sits between call arguments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): install skills once globally + symlink-mirror to every agent The previous install path sprayed a full ~6.7MB skill copy into each of the ~70 agent conventions `skills add --all` knows (a fresh init produced 40+ dirs / 341MB, incl. a stray dotless `agent/` from the Eve convention). Install ONCE, globally, as one faithful copy, then symlink it everywhere: - `skills add <url> --skill '*' --global --agent claude-code universal --copy` lands real files in ~/.claude/skills (Claude Code reads this at global priority) and ~/.agents/skills (the shared universal store). - mirrorGlobalSkills() fans that store out to every OTHER installed agent's GLOBAL dir (~/.cursor/skills, goose -> ~/.config/goose/skills, ...) — but only for agents present on the machine (marker dir exists), so nothing is sprayed. Unix: per-skill relative symlink into the store (one source of truth, auto-fresh on update); Windows: copy (symlinks need admin / Developer Mode there — the same fallback upstream and gstack make). Why global: skills are framework-general knowledge, not project content; Claude Code (and most agents) prioritize the personal/global scope, so the global copy is the one actually loaded — and it installs once instead of multiplying per project. The per-agent dir list is GENERATED from upstream's src/agents.ts at a pinned tag (the `skills` package exports nothing importable), committed as agentDirs.generated.ts and resolved env-faithfully at runtime (XDG_CONFIG_HOME / CODEX_HOME / CLAUDE_CONFIG_DIR honored). Regenerate with `bun run --cwd packages/cli gen:agent-dirs` when the pin moves. Covers all 70 agents that define a global dir (eve/promptscript define none); the bare project-dir agents (openclaw, astrbot) are namespaced globally, so the stray-`agent/` footgun is gone. `skills check` now scans global ($HOME) before project (cwd) to match the runtime load order — so it reports on the copy the agent will really use, not a stale project copy a newer global install silently overrides. Test plan: - skills.test.ts: install spawns the global --copy args, never --all; update stays strict + exits non-zero on failure. - skillsMirror.test.ts: Unix relative symlinks, Windows copy, XDG_CONFIG_HOME honored, install-owned stores skipped, marker-gating, idempotent refresh, generated-table shape. - skillsManifest.test.ts: check is global-first. - Full CLI suite green (981); oxlint / oxfmt / tsc clean; gen:agent-dirs --check clean (offline + network produce byte-identical output). - Benchmark (isolated HOME, local CLI): claude+hermes and all 70 agents — ~/.claude + ~/.agents real (19 each), every installed agent's global dir = 19 symlinks into the store, zero spray into unseeded agents, check global-first. (The 9 "outdated" check reports are the separate skills.sh registry lag, not this change.) - .fallowrc.jsonc: exempt the codegen script's inherent parser complexity and the parallel-case duplication in skillsManifest.test.ts (same rationale the config already uses for SlideshowPanel.test.ts / hyperframes-player.test.ts). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): install skills with --full-depth so a fresh install reads as current `skills add <url>` without --full-depth fetches from the skills.sh registry blob ("Fetching skills"), which lags GitHub main by hours — so a freshly installed/updated set read as ~9 skills "outdated" right after install, and `skills update` couldn't fix it (it re-fetched the same stale blob → death loop). --full-depth switches it to a real `git clone` of HEAD ("Cloning repository"), the only path that yields the genuine latest. - Add --full-depth to the global install args. Verified (isolated HOME): blob path → 10 current / 9 outdated; --full-depth → 19 current / 0 outdated. - The clone is heavier than the blob fetch, so set GIT_LFS_SKIP_SMUDGE=1 (skills are text; the repo's LFS objects are unrelated binaries the install doesn't need) and raise the spawn timeout 120s → 300s. - Correct the stale comment that claimed a full URL already bypasses skills.sh — it doesn't; only --full-depth does. Benchmark (skills-bench, local CLI): B.death-loop and J1.init-detect-and-refresh flip FAIL → PASS (install/update/init now 19/0); mirror smoke reports 19 current / 0 outdated. (spine still reflects the raw documented `skills add <slug>` command — the upstream skills.sh path, not this CLI.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(skills): drop --skip-skills from workflow init so new projects refresh skills The creation workflows scaffolded with `hyperframes init … --skip-skills`, which skipped the skills currency check. Now that init installs globally, is a no-op when already current, and pulls the genuine latest (via --full-depth), there's no reason to skip it: removing --skip-skills means every new project runs the check and refreshes the global skill set from GitHub when it's stale. Add a one-line note to each workflow (embedded-captions, faceless-explainer, motion-graphics, music-to-video, pr-to-video, product-launch-video) and the hyperframes-cli + /hyperframes router explaining what init does. skills-manifest.json regenerated by the pre-commit hook to match the edited skill bundles. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): scope agent mirror to HyperFrames' own skills, not the whole store mirrorGlobalSkills listed every */SKILL.md in ~/.claude/skills and fanned them out — but that store is shared, so a user's gstack / personal / company Claude skills would get symlinked (and, since linkOrCopy removes the target first, could overwrite a same-named skill) into Cursor / Codex / Goose / etc. Scope the mirror to HyperFrames' own skills via the upstream lock's source attribution — the same definition the prune already uses (skillsAttributedToSource) — never a directory listing. New hyperframesSkillNames() reads the global lock and returns only skills attributed to heygen-com/hyperframes; the mirror intersects that allow-list with what's in the store. Empty (no lock / nothing attributed) → mirror nothing, never everything. Also fixes the cosmetic "director(ies)" log typo (now singular/plural-aware) and extracts the fan-out into mirrorToInstalledAgents() to keep installAllSkills under the complexity gate. Regression: skillsMirror.test.ts asserts a foreign gstack skill in the store is neither mirrored out nor allowed to replace another agent's same-named skill; the skills-bench harness seeds ~/.claude/skills/gstack and asserts it never leaks to any agent. 1045 CLI tests + lint/types/fallow green. Addresses Magi's request-changes on heygen-com#1753. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dahans-msft2
referenced
this pull request
in dahans-msft2/hyperframes
Aug 6, 2026
…gen-com#1829) * feat(telemetry): unify CLI and Studio PostHog identity (Layer 1) Seed the CLI's anonymous distinct_id into Studio at launch so a developer's CLI and their Studio browser session resolve to the same PostHog person. Also unifies Studio's two previously-independent anonymous ids into one source of truth. Uses only the existing anonymous machine id (no new PII). - cli: inject window.__HF_CLI_DISTINCT_ID into the served index.html <head> (mirrors the existing __HF_STUDIO_ENV__ injection) + add a fallback GET /api/telemetry-identity endpoint. Only seeds when CLI telemetry is enabled; empty/no-op otherwise. - studio: new telemetry/distinctId.ts single source of truth; adopts the CLI-seeded id when present, else falls back to the existing per-browser localStorage id. Both Studio clients (studio:* and studio_*/render) now share this one id. * fix(telemetry): keep Studio distinct_id resolver fail-silent on getItem resolveStudioDistinctId read localStorage.getItem() outside a try/catch while every other external access in the module is guarded. In a storage-restricted context where the localStorage reference resolves but getItem throws, the resolver threw — breaking the module's fail-silent contract (telemetry must never break Studio). Guard the reads and treat a throw as "no id". Also drop an unnecessary `as` cast in the test per the repo CLAUDE.md convention (the optional global is already declared). * refactor(telemetry): address review feedback on identity unification - dedup safeLocalStorage/safeSessionStorage into utils/safeStorage.ts, used by both telemetry/config.ts and telemetry/distinctId.ts (Miga #6) - replace redundant `??=` with `=` in the no-storage branch; cachedId is guaranteed null there (Miga #2) - extract buildStudioHeadScripts() so the "identity script before env script" head-injection ordering is a pure, tested invariant (Miga #5) - add tests: head-script ordering + telemetry-off passthrough, and a Studio memoization test proving an adopted CLI id survives a later window.__HF_CLI_DISTINCT_ID reassignment (Rames) - clarify the XSS-escaping comment (both < and / escaped so no </script> sequence can form) (Miga #1)
dahans-msft2
referenced
this pull request
in dahans-msft2/hyperframes
Aug 6, 2026
…reaming over auto-parallel (heygen-com#2026) * feat(producer,cli): drawElement priority inversion — single-worker streaming over auto-parallel clamp:parallel eats 50% of local renders (1,326/fortnight; DE engagement stuck at 3.8%) by routing multi-worker renders to unverified screenshot capture. Benchmarks (2026-07-08, 4 comps x W1/W2/W3/W5) show that above the ~900-frame amortization crossover, single-worker VERIFIED drawElement streaming beats screenshot-parallel at EVERY worker count (2,380f: 66s vs 109-127s; 3,600f: 33s vs 39-56s; parallel scaling flattens past W2), while below it DE's fixed init cost loses by <=2.2s. - shouldPreferSingleWorkerDrawElement (exported predicate + 7 unit tests): inverts an AUTO-resolved multi-worker render to workerCount=1 when the comp matches the benchmarked configuration — default-on DE (darwin hardware clamp upstream), no compile gate, no forced-screenshot hint, mp4 output, single-worker streaming eligible, and totalFrames >= HF_DE_SINGLE_MIN_FRAMES (default 900; 0 disables). Explicit --workers N is always honored. - Inverted renders keep the probe session and land on the worker-encode streaming drain — the ONLY path with runtime self-verification, so this moves ~40% of previously-clamped renders onto the verified fast path. Comps that later hit an init-time gate (~1.5% of local renders) render single-worker screenshot streaming; accepted trade. - Telemetry: de_worker_inversion on render_complete (orchestrator -> perfSummary.workerInversion -> CLI), plus the worker_resolution observability checkpoint now records deWorkerInversion. Validation: e2e matrix on 2,381f comp — auto->5 workers inverted to 1, DE verified 4x inf PSNR, RENDER_OK; short comp (360f) auto stays 5-worker; explicit WORKERS=3 honored; HF_DE_SINGLE_MIN_FRAMES=0 disables. Canary suite 7/7 (PSNRs identical). renderOrchestrator tests 86/86. tsc/oxlint/oxfmt clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(producer): review fixes — inversion routing guards, calibration skip, retry revert Max code-review round on the inversion (13 confirmed findings): - Streaming spawn-failure disk fallback now clamps default-on drawElement (deClampReason=disk_path, DE-mode probe closed) exactly like the pre-capture clamp — previously it carried useDrawElement=true onto the unverified disk path, the hole the verified-path confinement exists to close, newly reachable for every inverted render. - Predicate gained the routing knowledge it was blind to: layered/HDR and shader-transition comps (drawElement never runs there), supersampling (deviceScaleFactor>1 init gate), a probe session whose init gates already disengaged DE, and the PRODUCER_EXPERIMENTAL_FAST_CAPTURE=true explicit parallel-DE opt-in (honored like --workers N). - Eligibility is evaluated BEFORE capture calibration and skips it when the inversion pins workers to 1 regardless of the estimate — the throwaway calibration browser + sample captures cost ~41s on the 2,381-frame benchmark comp (auto render: 111.6s -> 70.1s total). - Self-verify retry reverts the inversion: the re-render returns to the pre-inversion parallel screenshot path (disk) instead of single-worker screenshot streaming, the slowest shape for exactly the comps drawElement damages. - HF_DE_SINGLE_MIN_FRAMES="" (set-but-empty) now falls back to the 900 default instead of aliasing the 0 kill switch. - Timeout advisory uses the RESOLVED worker count — an inverted render that times out no longer prints "Retry with --workers 1" (the configuration that just failed). - Telemetry: deWorkerInversion recorded in capture observability (failed renders are attributable), emitted as literal false when not fired (queryable denominator), and the drawElement perf input shape is one exported DrawElementPerfInput type instead of three copies. - Tests: requestedWorkers undefined (the value production actually passes) + the four new predicate guards; 91/91. Validation: e2e auto render — calibration skipped (deInversionEligible), inversion fires, DE verified 4x inf, total 70.1s (was 111.6s); HF_DE_SINGLE_MIN_FRAMES=0 restores calibration + parallel; canary suite 7/7 (PSNRs identical); tsc/oxlint/oxfmt clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(producer,cli): review round 2 — loss-cohort telemetry, retry-plan helper, boundary tests - de_worker_inversion is now a tri-state string ("inverted" | "reverted" | "none") instead of a boolean: the self-verify retry marks the render "reverted" rather than resetting to false, so the dashboard can segment the lost-inversion cohort first-class instead of inferring it from deSelfVerifyFallback + frame-count joins (james-russo #1). - The retry rollback is extracted to resolveInversionRetryPlan (pure, exported) with unit coverage: pre-inversion worker-count restore, streaming re-resolution (multi-worker retry -> disk), "reverted" state, null when never inverted (james-russo #2). - WOULD_RESOLVE_MULTI_WORKER named constant replaces the bare sentinel 2 (james-russo #5); minFrames: -1 boundary case added (miga #3). 94/94 renderOrchestrator tests; tsc/oxlint/oxfmt clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(producer,cli): emit de_pre_inversion_workers for the parallel counterfactual The ramp-down decision needs "did DE beat the parallel render it displaced", not just "did DE beat single-worker screenshot". Emitting the worker count the auto-resolution chose BEFORE the inversion pinned it to 1 makes the parallel counterfactual computable per render (screenshot ms/frame from the verify samples / W x the measured parallel-efficiency curve). Set only when the inversion fired. Smoke: 2,381f auto render -> de_worker_inversion="inverted", de_pre_inversion_workers=5, mode=drawelement, verify armed 4. 99/99 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
dahans-msft2
referenced
this pull request
in dahans-msft2/hyperframes
Aug 6, 2026
## What Fixes five reported false-positive/false-negative patterns in the WCAG contrast audit (`hyperframes validate --contrast`): 1. **SVG fill vs. text color** — foreground read from CSS `color` instead of SVG `fill`. 2. **Cross-component color bleed** — background estimate bleeds into a neighboring panel/layer. 3. **Backdrop-filter glass text** — background estimate misses the blur/tint and reads the raw backdrop. 4. **Partially-overlapping translucent decoration** — a decorative shape inside or partly touching the text's bbox goes undetected. 5. **Solid-fill pill/button** — investigated, did **not** reproduce; already handled correctly by the existing own-background ancestor walk. Not touched. ## Why The audit estimated an element's background two ways: - foreground: always `getComputedStyle(el).color` — wrong for SVG `<text>`/`<tspan>`, which is painted via `fill`, an independent CSS property. - background: a 4px pixel ring sampled just **outside** the text's bounding box, with a fallback to an ancestor's opaque `background-color` for solid pills/buttons. The ring is a proximity heuristic. It's wrong whenever what's immediately outside the text differs from what's actually behind it: - text near the edge of its own panel, with a differently-colored sibling panel/layer just past the bbox — the ring samples the neighbor. - a `backdrop-filter: blur()` glass panel sized only a couple pixels larger than the text — the ring exits the panel into the raw, unblurred, untinted backdrop. - a translucent decoration that only partially overlaps the ring, or sits entirely **inside** the bbox — invisible to the ring regardless of size. ## How **SVG fill (#1):** elements inside an `<svg>` (`el.ownerSVGElement`) now prefer the computed `fill` when it resolves to a solid `rgb()`/`rgba()` color, falling back to `color` for paint values that aren't a plain color (`none`, `context-fill`, gradient/pattern refs). **Cross-comp bleed / glass blur / partial decoration (#2–#4):** replaced the ring-sampling + own-background-ancestor-walk heuristic with a two-phase capture: 1. `__contrastAuditPrepare()` walks the DOM, computes each candidate's foreground (unchanged logic from #1), and **hides that element's own text paint** (`color`/`fill` → `transparent`, layout-neutral — no reflow). 2. The caller takes **one** screenshot with the glyphs invisible (same number of screenshots as before — just moved after the hide instead of before it). 3. `__contrastAuditFinish(imgBase64, time, candidates)` restores the original paint immediately, then samples the **real composited pixels directly inside each element's own bbox** — no proximity heuristic needed, since these are the exact pixels that were behind the glyphs. This is a real architectural change to `contrast-audit.browser.js`'s calling contract (single `__contrastAudit` → `__contrastAuditPrepare`/`__contrastAuditFinish`), with `validate.ts`'s `runContrastAudit` updated to match, including a try/finally restore-safety-net so a mid-loop screenshot/decode failure can't leave a later sample auditing a page with stale hidden text. Mirrored the identical change in `skills/hyperframes-creative/scripts/contrast-report.mjs`, which duplicates the same DOM-walk/sampling logic (not just the WCAG math). There, the **visible** frame for the human-facing overlay image still comes from the producer's normal `captureFrameToBuffer` path (unchanged); only the **background-sampling** capture is a plain `session.page.screenshot()` taken after hiding text — deliberately bypassing `captureFrameToBuffer`, whose static-frame dedup cache knows nothing about the DOM mutation and would hand back a stale pre-mutation buffer. **Solid-fill pill (#5):** reproduced a rounded pill/button with a busy page background outside it. The existing own-background ancestor walk already resolves the pill's declared `background-color` correctly regardless of the rounded corners — confirmed via repro, both before and after this change report the identical (correct) result. No fix needed; left untouched, and this case is covered by the new architecture too (would give the same right answer even without the ancestor-walk fallback). Added `packages/cli/src/commands/contrast-sample.ts` (mirroring the existing `contrast-bg.ts`/`contrast-fg.ts` pattern) hosting the pure sample-rect/grid-point computation, unit tested — the browser-injected scripts can't import it directly, so it's kept in sync by hand, same convention as the rest of this file. ## Test plan - [x] Unit tests: `contrast-fg.test.ts` (SVG fill resolution), `contrast-sample.test.ts` (sample-rect clamping/degenerate cases), plus the full `packages/cli` suite (1424 tests) passes, including an updated `layout-audit.browser.test.ts` case that called the old single-function `__contrastAudit` API directly. - [x] Manual verification — standalone `puppeteer-core` harness against real `chrome-headless-shell`, one minimal HTML fixture per pattern, comparing the audit's reported ratio/verdict against a hand-constructed ground truth: - **SVG fill**: `fill:white` / no `color` on black bg → before: `fg=rgb(0,0,0)` ratio `1:1` (false FAIL); after: `fg=rgb(255,255,255)` ratio `21:1` (correct PASS). - **Cross-comp bleed**: text on a black sibling highlight box 2px larger than the text, white page bg outside it → before: `bg=rgb(255,255,255)` ratio `1.23:1` (false FAIL); after: `bg=rgb(0,0,0)` ratio `17.14:1` (correct PASS). - **Glass blur**: black text on an 18%-white-tinted `backdrop-filter: blur(14px)` panel over a yellow/blue gradient, panel only ~2px larger than the text → before: `bg=rgb(0,64,255)` (raw gradient color, blur/tint completely missed) ratio `3.18:1` (false FAIL); after: `bg=rgb(159,160,165)` (correct blurred/tinted blend) ratio `8.05:1` (correct PASS). - **Partial decoration**: text 92%-covered by a translucent white badge on a dark bg → before: `bg=rgb(16,16,16)` (ring never touches the badge, which sits entirely inside the bbox) ratio `17.45:1` (false PASS); after: `bg=rgb(171,171,171)` (correctly detects the badge) ratio `2.11:1` (correct FAIL). - **Solid pill sanity**: unaffected — `bg=rgb(10,10,10)` ratio `19.8:1` before and after. - [x] End-to-end: ran the actual `hyperframes validate --contrast` CLI command (via `tsx src/cli.ts`) against a real scaffolded project containing all 4 patterns simultaneously — only the genuinely-failing case (the 92%-covered decoration) is reported (`1.09:1`, need `3:1`); the cross-comp-bleed, glass-blur, and solid-pill cases are correctly silent. A second vanilla scaffold with plain white-on-dark text produces zero false positives. - [x] `oxlint`, `oxfmt --check`, and `tsc --noEmit` all pass on the changed files.
dahans-msft2
referenced
this pull request
in dahans-msft2/hyperframes
Aug 6, 2026
Field signal ts=1784040753 (#hyperframes-cli-feedback): a composition with ~40 heavy overlay DOM elements — `filter:blur`, oversized `radial-gradient`, and `clip-path` animations — captures solid-black for the first ~half of the render, recovering near the end. Reproduces identically via drawElement AND forced --no-browser-gpu screenshot capture AND `snapshot`, so the capture layer itself is the offender, not encoder/mux. Independent of duration (padding the timeline grows the bad zone proportionally, doesn't shift it). Presence alone matters — even opacity:0 / visibility:hidden / unused overlays contribute. Reporter's workaround was splitting into per-transition mini-compositions + FFmpeg concat. Add compositionCheck rule `composition_heavy_overlay_count_high` (warning). Counts DOM elements that carry any of: inline `style` filter:blur / clip-path (non-none) / radial-gradient, or a class/id whose top-level CSS rule body sets one of those. `display:none` elements are counted-out (removed from render tree); opacity:0 / visibility:hidden overlays are counted-in per the field-signal repro shape. Warns at 25 to give lead time before the observed 40-element bad zone. Skips registry source and installed-block files, mirroring `composition_file_too_large`. Includes a `ts=1784040753` reference in fixHint so authors can trace the risk shape. Stack: PR #5 of 9 (base via/parity-telemetry-gate). Signed-off-by: Via Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced Aug 11, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hyperframes dev: usepnpm exec vitefor the new studio packagestudio/data/projectspackage.jsonTest plan
npx tsx packages/cli/src/cli.ts dev /path/to/projectstarts studiopnpm devfrom repo root works