mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-08-31 02:41:44 +00:00
Vance IngallsandClaude Opus 5
f0cc9b1a34
* fix(skills): make the carve CLI work against the published core, and honour its own group invariant Two defects found by using the shipped feature end to end on a real project rather than inside this repo. **It could not load core at all.** `loadCore` resolved `./audio-carve` and `./audio-fx` with `require.resolve`. The workspace manifest declares a `node` condition, so that resolved fine here — but the PUBLISHED manifest (`publishConfig.exports`) carries only `import` + `types`, so every consumer of the released package got ERR_PACKAGE_PATH_NOT_EXPORTED for a package that ships those files perfectly well. The script was broken everywhere except where it was developed, and its error text blamed a missing/outdated package, which no install can fix. It now keeps the project anchor and falls back to the manifest's declared `import` target. **It violated the invariant its own SKILL.md sets.** SKILL.md is explicit: "A carve against more than one clip id is wrong. Group the clips and carve against the group. This is an invariant, not a tip." The script wrote `sources: voices.map((v) => v.id)` unconditionally, so every run against grouped voices produced output that tripped the repo's own `audio_carve_ungrouped_sources` lint rule, and a voice added to the group later would silently play outside the carve's awareness. When every voice shares one group it now records the group; mixed, partially grouped or ungrouped voices keep their ids so the lint rule still fires on the case it is meant to catch. `main()` moves behind an entry guard so the pure helper can be imported and tested; `node carve.mjs` is unaffected (verified against a real composition). Six tests, and the manifest hash is regenerated for the changed skill. * fix(skills): run the carve CLI through symlinks, and keep the bed out of its own sources Two blockers from review, both of the class this PR's first fix was about: correct where it was developed, broken for the audience it ships to. **The entry guard silently skipped `main()` through any symlinked path.** `process.argv[1]` keeps the spelling the caller typed while `import.meta.url` is derived from the realpath, because node resolves the main module's symlinks. So the raw compare added to make the helpers importable turned the CLI into a no-op that wrote nothing and exited 0. Reachable with no symlink of one's own: on macOS `/tmp` is a link to `/private/tmp`, and `SKILL.md` documents the entry point as `node <SKILL_DIR>/scripts/carve.mjs`, so any install placed behind a link breaks too. Reproduced against the published core by a reviewer, not only inferred. Fixed by realpathing the left side. This repo already documents and solves the same trap in three scripts (`frame-packets-core.mjs`, `preflight.mjs`, `project-dir.mjs`); the canonical comment is carried over verbatim. A local copy rather than an import, because skills install independently — `hyperframes-audio` has no dependency on `hyperframes-core` being present. **`carveSources` could make the bed its own carve source.** It decided from the voices alone, so a bed sharing their group (`mix`) got `sources: ["mix"]` written onto it. `resolveCarveSourceIds` expands a group id to every current member and takes no host element to exclude, so the next analysis in Studio hands the bed to itself and the duck envelope fights the bed's own content instead of speech — the "never carve a track against itself" invariant, arriving one re-analysis after a first pass that was genuinely correct (`main()` sums the detected voices directly and never round-trips through group resolution, which is why the PR's own end-to-end check could not catch it). The fix is at the call site, not in the resolver: neither `resolveCarveSourceIds` nor `resolveCarveVoices` receives the host, so "make the resolver skip the target" would be a signature change on shared core. `carveSources(voices, bed)` declines the group form when the bed is a member and records clip ids, which is exactly what `audio_carve_ungrouped_sources` exists to raise — plus a stderr note saying why, so the lint message does not read as "group clips you already grouped". Scoped to `<audio>` beds: group membership is audio-only, so a `<video>` bed cannot be pulled in by an expansion and declining there would be a false positive. SKILL.md now states the constraint next to the group invariant it belongs to. Tests: six added, closing both gaps review named. The bed-in-group regression and a symlinked CLI invocation both fail on the previous commit (silent exit 0 vs the usage error) and pass now; three more pin the cases that must NOT decline (different group, ungrouped bed, video bed). `loadCore` is now exported and covered by a fixture package carrying an import-only export map — the published manifest's shape — so this PR's first fix is pinned without depending on npm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): refuse the carve group when a non-voice member would widen it Closes the second branch of the original blocker, which the bed fix did not cover: detected voices sharing `voiceover` with an existing SFX or music member. Detection correctly leaves that member out, but the persisted `sources: ["voiceover"]` resolves wider on the next Studio analysis — `resolveCarveSourceIds` expands the group to every current member and `resolveCarveVoices` keeps any audio with a src — so the extra clip enters the sidechain and the bed starts ducking under a whoosh. Same shape as the bed case: the first pass is genuinely correct because `main()` sums the voice list `detectTracks` returned and never round-trips through group resolution. Taking the first of the two suggested fixes (membership + classification in the collapse decision) rather than deriving the first pass from the resolved group: analysing whatever the group happens to hold would make the CLI measure clips it classified as non-voice, which is the arrangement problem rather than a licence to sidechain them. `groupSourceRefusal(voices, bed, members)` replaces `bedInVoiceGroup` and returns `{group, reason, ids}` or null, so the decision and the stderr note come from one place. `members` is every `<audio>` in the composition as `{id, group, nameKind}` with `nameKind` from core's `classifyAudioName`, so this and Studio's picker classify identically. `detectTracks` now returns the media list it already built. Classification, not membership, is what makes this safe. A member classified `music` or `sfx` blocks the group; a member classified `voice` or `unknown` does not. That distinction is load-bearing: `detectTracks` only analyses voices that overlap the bed, so an outro line that starts after the bed ends is routinely a group member this run did not measure — and covering it on a later analysis without editing `sources` is the entire reason SKILL.md says to name the group. Refusing on "any member the run did not analyse" would collapse the group form into clip ids for every ordinary narration sequence. `unknown` follows detection's own loose-in-the-safe-direction rule, since detection treats an unknown name as a possible voice. The note now names the blocking member, for either reason, since "sources are clip ids" plus `audio_carve_ungrouped_sources` reads as nonsense to an author who did group their clips. Tests: six added, 18 in the file. The two regressions (sfx member, music member) and the refusal shape fail with the mixed branch ablated and pass with it; three more pin the cases that must NOT refuse — a non-overlapping voice member, an `unknown` member, and an sfx member of a different group. SKILL.md states both refusals and the voice-member exemption next to the group invariant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): make `members` required so dropping it cannot undo the widening fix Review finding, and the one link no test covered. `carveSources` and `groupSourceRefusal` defaulted `members = []`, and with an empty list the `mixed` refusal cannot fire — so a refactor that dropped the third argument at the call site would return the group form again with the entire suite green. That is the same signature as the bug the argument exists to prevent: `main()` sums the detected voice list directly, so the first CLI pass is correct either way and only a later Studio re-analysis reads the widened attribute. Nothing goes red. `main()` is also the only code that BUILDS `members`, and no test runs it — the symlink test stops at the usage error and a real run needs ffmpeg. Both defaults are gone, so a missing argument throws on `members.filter`. The nine cases that predate the membership check now pass `[]` explicitly, which also documents that they are about the bed and the group attributes alone, and a new test asserts both functions throw when the argument is omitted. Verified it fails when the defaults are restored. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>