fix(studio,core,engine): close the defects a max-effort review found in the fixes

A review of the five fix commits found eleven real defects, including a regression one of them introduced. Each was verified against the code before being acted on; the ALTITUDE-only items are not touched here. REGRESSION, from "group rows survive a collapse". Skipping member rows for a collapsed group also removed them from `tracks`, and every group consumer recovered its member ELEMENTS by looking them up there. Since collapsed is the default and nothing seeds the expansion set, that meant: half-lit solo silently off for every group (undoing c0b7bafd9 one commit later), the automation-lane count always 0, and the bus strip labelling its members "track 1", "track 2". Membership is not a display concern, so it no longer travels through the display list: `TimelineTrackGroupInfo` carries `memberElements` directly. Group bus. `reanchor` wrote `fader.gain.value` BEFORE cancelling the booked automation — an AudioParam value write inside a live curve throws, and this runs inside `schedulePlayback`, whose catch turns a throw into `return null`: the MEMBER would have silently dropped out of the pass. Worse, the generation was stamped before the attempt, so no sibling retried and the bus kept the previous pass's envelopes — finding 11 unfixed on exactly the pass that failed. Now: clear first, stamp only on success, and isolate the call. The mock's gain node had no `cancelScheduledValues` at all, so the whole scheduling surface was unexercised; it is stubbed now, which is what surfaced this. `reanchor` also could not clear a lane that no longer EXISTS — `scheduleVolumeLane` returns early with no lane, and a surviving envelope outranks a `.value` write, so deleting a group's automation mid-session left the old ramps owning the fader for the rest of the session. The preview fader applied `data-volume` unclamped while the render clamps to [0,1]: an authored `data-volume="2"` previewed +6 dB and rendered at unity, `-1` previewed with inverted polarity and rendered silent. A preview/render divergence inside the commit whose purpose was removing one. Pitch shift. The `everShifted` latch was the wrong mechanism: it was set before the bypass check (so a node at `mix: 0` burned the bypass without shifting anything), it made the FIRST step off zero a hard dry-to-wet splice 50 ms wide — an audible click on a slider drag — and once latched it kept preview permanently delayed while the render, building a fresh node from the attribute, bypassed. Replaced with a ramped wet amount: no click in either direction, and a node set back to zero reaches true bypass, so preview and render agree again. Silent no-ops. The throw added inside `createAudioGroupAndAssignMembers` was caught one frame up and not rethrown, so the carve's auto-group still saw success and persisted `sources: [groupId]` for a group that was never written — the exact failure the throw was added to prevent. The group-pointer button dropped clips with no DOM id and grouped the REMAINDER, leaving them outside the bus while the UI showed the track as grouped; the button is withheld now instead. The creation rollback stripped `data-audio-group` outright rather than restoring each member's prior value, so a failed save could un-group clips that were already in another group. `insertGroupElement` treated ANY element already holding the id as "ours", which would have aimed every later group write at an unrelated element. `setAudioMuteHidden` rescheduled Web Audio mid-play without `stopAll()`. Bumping the generation only rejects future stale schedules; it does not stop running sources and there is no per-element dedup, so flipping the canary during playback would have started a second buffer source for every in-window clip. `invalidateGroupInfoCache` was missed by the DOM-edit path: the rack reaches `<hf-audio-group>` through the DOM editor, not through the timeline's writers. Hooked at `setOrRemovePreviewAttribute` — the one chokepoint every attribute write passes — so this does not stay a per-caller obligation. Both defects in the ffmpeg-header test are mine: it early-returned instead of skipping when ffmpeg is absent (reporting green having asserted nothing), and pinned this build's 18-byte fmt / offset-92 layout as a requirement, which would fail on a legal canonical header the parser also handles. Also: the group-degradation note is no longer dropped when the outer mix degrades too, and a malformed doc comment (two stacked openers) is fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-03 12:54:29 +00:00 · 2026-08-20 02:15:39 -07:00
parent ae9e680542
commit c4ebda22ec
19 changed files with 325 additions and 66 deletions
@@ -168,23 +168,77 @@ describe("the worklet processors themselves", () => {
      expect(maxErr).toBeLessThan(1e-6);
    });

-    // A track whose semitones are automated THROUGH zero must not jump between
-    // the delayed and the undelayed path — that discontinuity is a click, which
-    // is worse than the delay the bypass would save.
-    it("keeps processing at zero once it has shifted, rather than clicking to dry", async () => {
+    /** Largest sample-to-sample step — a splice between the dry and the
+     *  ~50 ms-delayed wet path shows up here as a discontinuity. */
+    function maxStep(s: Float32Array, from: number, to: number): number {
+      let worst = 0;
+      for (let i = from + 1; i < to; i++) {
+        worst = Math.max(worst, Math.abs((s[i] ?? 0) - (s[i - 1] ?? 0)));
+      }
+      return worst;
+    }
+
+    // Dragging the semitones slider off zero mid-playback swaps the output from
+    // x[t] to x[t-50ms]. Switched hard that is an audible click; the wet amount
+    // is ramped instead. A 440 Hz sine steps ~0.057 per sample at its steepest,
+    // so anything near the signal's own peak is a splice, not the waveform.
+    it("does not click when the shift moves off zero mid-signal", async () => {
+      const HfPitchshift = (await loadProcessors()).get("hf-pitchshift");
+      if (!HfPitchshift) throw new Error("hf-pitchshift not registered");
+      const p = new HfPitchshift({ processorOptions: { semitones: 0, mix: 1 } });
+      run(p, sine(440, 0.3)); // settled dry, ring warm
+      p.p = { ...p.p, semitones: 7 };
+      const output = run(p, sine(440, 0.3));
+      expect(maxStep(output, 0, output.length)).toBeLessThan(0.2);
+    });
+
+    it("does not click on the way back to zero either", async () => {
      const HfPitchshift = (await loadProcessors()).get("hf-pitchshift");
      if (!HfPitchshift) throw new Error("hf-pitchshift not registered");
      const p = new HfPitchshift({ processorOptions: { semitones: 7, mix: 1 } });
-      const input = sine(440, 0.4);
-      run(p, input);
+      run(p, sine(440, 0.3));
      p.p = { ...p.p, semitones: 0 };
-      const output = run(p, sine(440, 0.4));
-      // Still the wet path (grain-delayed), so it does NOT equal the input.
+      const output = run(p, sine(440, 0.3));
+      expect(maxStep(output, 0, output.length)).toBeLessThan(0.2);
+    });
+
+    // ...and having ramped back down it must reach TRUE bypass, not sit on a
+    // permanently latched wet path. The render builds a fresh node from the
+    // saved attribute and bypasses at semitones 0; a preview that stayed wet
+    // would carry a 50 ms delay the export does not have.
+    it("returns to true bypass after being shifted and set back to zero", async () => {
+      const HfPitchshift = (await loadProcessors()).get("hf-pitchshift");
+      if (!HfPitchshift) throw new Error("hf-pitchshift not registered");
+      const p = new HfPitchshift({ processorOptions: { semitones: 7, mix: 1 } });
+      run(p, sine(440, 0.3));
+      p.p = { ...p.p, semitones: 0 };
+      run(p, sine(440, 0.3)); // ramp down settles here
+
+      const input = sine(440, 0.3);
+      const output = run(p, input);
      let maxErr = 0;
-      for (let i = 0; i < 4000; i++) {
+      for (let i = 0; i < input.length; i++) {
        maxErr = Math.max(maxErr, Math.abs((output[i] ?? 0) - (input[i] ?? 0)));
      }
-      expect(maxErr).toBeGreaterThan(1e-3);
+      expect(maxErr).toBeLessThan(1e-6);
+    });
+
+    // A node parked at mix 0 has shifted nothing, so it must not have spent
+    // anything that stops the zero-shift bypass engaging later.
+    it("is transparent at zero after sitting mixed fully out", async () => {
+      const HfPitchshift = (await loadProcessors()).get("hf-pitchshift");
+      if (!HfPitchshift) throw new Error("hf-pitchshift not registered");
+      const p = new HfPitchshift({ processorOptions: { semitones: 7, mix: 0 } });
+      run(p, sine(440, 0.3));
+
+      p.p = { ...p.p, semitones: 0, mix: 1 };
+      const input = sine(440, 0.3);
+      const output = run(p, input);
+      let maxErr = 0;
+      for (let i = 0; i < input.length; i++) {
+        maxErr = Math.max(maxErr, Math.abs((output[i] ?? 0) - (input[i] ?? 0)));
+      }
+      expect(maxErr).toBeLessThan(1e-6);
    });

    // The ring starts empty, so the taps read zeros for the first grain. That
@@ -257,11 +257,17 @@ class HfPitchshift extends AudioWorkletProcessor {
    // behind the write head, so until this fills they would read the ring's
    // zeros — the head of every clip came out attenuated or silent.
    this.filled = 0;
-    // Latched the first time a non-zero shift is asked for. The no-op bypass
-    // below must not engage for a track whose semitones are AUTOMATED through
-    // zero: switching between a delayed and an undelayed path mid-signal is a
-    // click, which is worse than the delay it would save.
-    this.everShifted = false;
+    // How much of the wet (pitch-shifted) path is currently in the output, and
+    // where it is heading. Crossing between dry and wet is a ~50 ms jump in the
+    // signal, so it is RAMPED rather than switched: a hard swap either way is a
+    // click. Ramping in both directions is also what lets a node return to true
+    // bypass at semitones 0 — a one-way latch left preview stuck with the delay
+    // that the render, building a fresh node from the attribute, does not have.
+    this.wet = 0;
+    this.wetTarget = 0;
+    // ~15 ms one-pole, short enough to feel immediate on a slider drag and long
+    // enough that the splice is inaudible.
+    this.wetCoef = Math.exp(-1 / (sampleRate * 0.015));
    this.port.onmessage = (e) => {
      if (e.data && e.data.__hfDispose) { this.dead = true; return; }
      this.p = { ...this.p, ...e.data };
@@ -274,7 +280,6 @@ class HfPitchshift extends AudioWorkletProcessor {
    const p = this.p;
    const semitones = Math.max(-12, Math.min(12, p.semitones ?? 0));
    const mix = Math.max(0, Math.min(1, p.mix ?? 1));
-    if (semitones !== 0) this.everShifted = true;
    const grain = this.grain;
    const ringLen = grain * 2;
    const n = i[0] ? i[0].length : 0;
@@ -282,12 +287,16 @@ class HfPitchshift extends AudioWorkletProcessor {
      if (!this.buf[ch]) this.buf[ch] = new Float32Array(ringLen);
    }

-    // A node asking for no shift at all, or mixed fully out, is transparent.
-    // The grain delay is ~grain/2 whatever the ratio, so at semitones=0 this
-    // used to degrade into a pure 50 ms delay of the signal — while the copy
-    // for that exact setting reads "Unchanged pitch". The ring keeps filling
-    // so a later shift does not start cold.
-    if (mix === 0 || (semitones === 0 && !this.everShifted)) {
+    // Nothing to shift, or mixed fully out. The grain delay is ~grain/2
+    // whatever the ratio, so at semitones=0 this degenerated into a pure 50 ms
+    // delay of the signal — while the copy for that exact setting reads
+    // "Unchanged pitch".
+    this.wetTarget = semitones === 0 ? 0 : mix;
+
+    // Fully dry AND settled: take the cheap transparent path. The ring keeps
+    // filling, so a later shift does not start cold.
+    if (this.wetTarget === 0 && this.wet < 1e-4) {
+      this.wet = 0;
      let w = this.write;
      for (let s = 0; s < n; s++) {
        for (let ch = 0; ch < i.length; ch++) {
@@ -304,7 +313,8 @@ class HfPitchshift extends AudioWorkletProcessor {

    const ratio = Math.pow(2, semitones / 12);
    const inc = (1 - ratio) / grain;
-    let write = this.write, phase = this.phase, filled = this.filled;
+    let write = this.write, phase = this.phase, filled = this.filled, wetNow = this.wet;
+    const target = this.wetTarget, coef = this.wetCoef;
    for (let s = 0; s < n; s++) {
      phase += inc;
      phase -= Math.floor(phase);
@@ -313,7 +323,8 @@ class HfPitchshift extends AudioWorkletProcessor {
      // Ramp the wet path in as the ring fills rather than reading zeros:
      // 100 ms of unshifted audio at the head of a clip beats 50 ms of silence.
      const warm = filled >= grain ? 1 : filled / grain;
-      const wetMix = mix * warm;
+      wetNow = target + coef * (wetNow - target);
+      const wetMix = wetNow * warm;
      for (let ch = 0; ch < i.length; ch++) {
        const ring = this.buf[ch];
        const inp = i[ch], out = o[ch];
@@ -329,6 +340,7 @@ class HfPitchshift extends AudioWorkletProcessor {
    this.write = write;
    this.phase = phase;
    this.filled = filled;
+    this.wet = wetNow;
    return true;
  }
 }
@@ -205,8 +205,15 @@ export function initSandboxRuntimeModular(): void {
    if (silenceHiddenAudio === enabled) return;
    silenceHiddenAudio = enabled;
    // The active-clip set is built with this predicate baked in, so a flip
-    // mid-session has to rebuild it — same reason a `data-hidden` toggle does.
-    if (clock.isPlaying()) scheduleWebAudioForActiveClips();
+    // mid-session has to rebuild it. `stopAll()` first: bumping the generation
+    // only rejects future STALE schedules, it does not stop sources already
+    // started, and there is no per-element dedup — so rescheduling on its own
+    // starts a second buffer source for every in-window clip on top of the ones
+    // still playing. `applyWebAudioRate` pairs the two for the same reason.
+    if (clock.isPlaying()) {
+      webAudio.stopAll();
+      scheduleWebAudioForActiveClips();
+    }
  };
  // `_auto` is a Studio-internal keyframe marker (an auto-tracked endpoint the
  // parser reads back), NOT an animatable property. Register it as a no-op GSAP
@@ -647,7 +647,24 @@ describe("WebAudioTransport", () => {
          addEventListener: vi.fn(),
        })),
        createGain: vi.fn(() => {
-          const node = { gain: { value: 1 }, connect: vi.fn(), disconnect: vi.fn() };
+          // The AudioParam scheduling surface is part of the contract the group
+          // bus uses (`clearParamLane` cancels before re-seeding a reused bus).
+          // A bare `{ value }` made any such call throw, and `schedulePlayback`
+          // swallows throws into `return null` — so the mock's own gap read as
+          // "the member did not play" rather than as a missing stub.
+          const node = {
+            gain: {
+              value: 1,
+              cancelScheduledValues: vi.fn(),
+              cancelAndHoldAtTime: vi.fn(),
+              setValueAtTime: vi.fn(),
+              linearRampToValueAtTime: vi.fn(),
+              exponentialRampToValueAtTime: vi.fn(),
+              setValueCurveAtTime: vi.fn(),
+            },
+            connect: vi.fn(),
+            disconnect: vi.fn(),
+          };
          gainNodes.push(node);
          return node;
        }),
@@ -857,6 +874,29 @@ describe("WebAudioTransport", () => {
      expect(fader.gain.value).toBe(0);
    });

+    // reanchor runs inside schedulePlayback, whose catch turns any throw into
+    // `return null` — so a bus that fails to re-anchor would silently take the
+    // MEMBER out of the pass, and a generation stamped before the attempt would
+    // stop every later member retrying.
+    it("keeps the member playing when re-anchoring the bus throws", async () => {
+      document.body.innerHTML = `<hf-audio-group id="vo" data-volume="0.5"></hf-audio-group>`;
+      const { transport, mock, gen } = setupGroupTransport();
+      await scheduleGrouped(transport, gen, "a", "vo");
+      const fader = mock.gainNodes[5]!;
+      fader.gain.cancelScheduledValues = vi.fn(() => {
+        throw new Error("param is not schedulable");
+      });
+
+      transport.stopAll();
+      const gen2 = transport.startGeneration();
+
+      await expect(scheduleGrouped(transport, gen2, "a", "vo")).resolves.not.toBeNull();
+      // Generation not consumed by the failed attempt, so a sibling still tries.
+      fader.gain.cancelScheduledValues = vi.fn();
+      await scheduleGrouped(transport, gen2, "b", "vo");
+      expect(fader.gain.value).toBeCloseTo(0.5, 6);
+    });
+
    describe('solo — "Hear only this" (B5)', () => {
      it("silences a non-soloed member via its own solo gain, without touching the group bus", async () => {
        const { transport, mock, gen } = setupGroupTransport();
@@ -1,5 +1,6 @@
 import { attachElementFxChain, readElementAutomation, type ElementFxHandle } from "./audioFx.js";
 import {
+  clearParamLane,
  scheduleParamLane,
  volumeLane,
  type AutomationTiming,
@@ -16,6 +17,18 @@ function normalizeRate(rate: number): number {
  return rate;
 }

+/**
+ * The render puts every track volume through its own `clampVolume` before
+ * building the filter, so an authored `<hf-audio-group data-volume="2">`
+ * renders at unity. Preview has to agree or the two diverge on exactly the
+ * attribute this bus exists to honour — and a negative value would invert
+ * polarity in preview while rendering silent. Compositions are hand-authorable,
+ * so out-of-range values do not need the studio slider to be reachable.
+ */
+function clampGroupVolume(volume: number): number {
+  return Math.max(0, Math.min(1, volume));
+}
+
 /**
 * Breadcrumb for the per-element-mute handoff: the transport just claimed a track
 * that was audibly playing through the HTMLMedia fallback. Quiet unless
@@ -323,8 +336,18 @@ export class WebAudioTransport {
      // which for a fade-out is silence for the rest of the session. Re-anchor
      // once per play generation, not once per member scheduled.
      if (existing.generation !== this._playGeneration) {
-        existing.generation = this._playGeneration;
-        existing.reanchor(timing);
+        // Stamped only on success, and isolated: this runs inside
+        // `schedulePlayback`, whose catch turns any throw into `return null` —
+        // i.e. a bus problem would silently drop the MEMBER from the pass. And
+        // stamping first would consume the generation, so no later member of
+        // the same group would retry and the bus would keep the previous pass's
+        // envelopes: finding 11 unfixed on exactly the pass that failed.
+        try {
+          existing.reanchor(timing);
+          existing.generation = this._playGeneration;
+        } catch (err) {
+          swallow("webAudioTransport.groupReanchor", err);
+        }
      }
      return existing.input;
    }
@@ -355,7 +378,7 @@ export class WebAudioTransport {
    // group effect — a compressor, the Giant preset — previewed differently
    // than it rendered.
    const fader = this._ctx.createGain();
-    fader.gain.value = readAudioGroupVolume(groupEl);
+    fader.gain.value = clampGroupVolume(readAudioGroupVolume(groupEl));
    fader.connect(muteGain);
    const fx = attachElementFxChain(
      this._ctx,
@@ -375,7 +398,14 @@ export class WebAudioTransport {
      fx,
      generation: this._playGeneration,
      reanchor: (at: AutomationTiming) => {
-        fader.gain.value = readAudioGroupVolume(groupEl);
+        // Cleared BEFORE the value write, and unconditionally. `scheduleVolumeLane`
+        // clears as part of scheduling, but returns early when the group no
+        // longer has a lane — and a scheduled envelope outranks a `.value`
+        // write, so deleting a group's automation mid-session otherwise left
+        // the previous pass's ramps still owning the param (for a fade-out,
+        // silence) for the rest of the session.
+        clearParamLane([{ param: fader.gain }]);
+        fader.gain.value = clampGroupVolume(readAudioGroupVolume(groupEl));
        fx?.reanchor(at);
        if (groupEl) scheduleVolumeLane(groupEl, fader, at);
      },
@@ -1436,15 +1436,18 @@ export async function processCompositionAudio(
  // A group whose sub-mix had to drop member automation reports it the same
  // way mixAudioTracks reports its own degradation: on a SUCCESSFUL result, so
  // the render ships and the caller can still say what was lost.
-  const degradedGroups = [...groupsDegradedAutomation];
  const degradedNote =
-    degradedGroups.length > 0
-      ? `Volume automation exceeded this ffmpeg build's expression limits in group(s) ${degradedGroups.join(", ")}; rendered at base volume`
+    groupsDegradedAutomation.length > 0
+      ? `Volume automation exceeded this ffmpeg build's expression limits in group(s) ${groupsDegradedAutomation.join(", ")}; rendered at base volume`
      : undefined;

  return {
    ...mixResult,
    durationMs: Date.now() - startMs,
-    error: mixResult.error ?? degradedNote,
+    // Both, when both degraded. `mixResult.error ?? degradedNote` reported only
+    // the outer mix and dropped the one that names which GROUPS lost their
+    // members' automation — two different losses, and the operator needs to
+    // hear about the one they can act on.
+    error: [mixResult.error, degradedNote].filter(Boolean).join("; ") || undefined,
  };
 }
@@ -8,6 +8,7 @@ import { applyVolumeEnvelopeToWav } from "./audioVolumeEnvelope.js";

 const SAMPLE_RATE = 48000;
 const CHANNELS = 2;
+const HAS_FFMPEG = spawnSync(getFfmpegBinary(), ["-version"], { encoding: "utf-8" }).status === 0;

 /** Build a PCM s16le stereo WAV whose every sample equals `value`. */
 function writeConstantWav(path: string, frames: number, value: number): void {
@@ -255,7 +256,7 @@ describe("applyVolumeEnvelopeToWav", () => {
     * not read a real one — and an unreadable file returns false, which the
     * caller reads as "no automation here" and drops the group's envelope.
     */
-    it("reads what ffmpeg actually writes, not just a canonical header", () => {
+    it.skipIf(!HAS_FFMPEG)("reads what ffmpeg actually writes, not just a canonical header", () => {
      const path = join(tmp(), "ffmpeg-f32.wav");
      const made = spawnSync(
        getFfmpegBinary(),
@@ -275,11 +276,13 @@ describe("applyVolumeEnvelopeToWav", () => {
        ],
        { encoding: "utf-8" },
      );
-      if (made.status !== 0) return; // no usable ffmpeg here
+      expect(made.status).toBe(0);

      const before = readFileSync(path);
-      // Non-canonical by construction: prove the fixture is the awkward shape.
-      expect(before.readUInt32LE(16)).toBe(18); // fmt chunk size
+      // The format tag is the load-bearing part; the chunk LAYOUT is this
+      // build's quirk, so it is logged as context rather than required — a
+      // build emitting a canonical 16-byte fmt with data at 44 is legal and
+      // handled, and pinning 18/92 would fail on the good case.
      expect(before.readUInt16LE(20)).toBe(3); // WAVE_FORMAT_IEEE_FLOAT

      expect(
@@ -307,8 +310,10 @@ describe("applyVolumeEnvelopeToWav", () => {
        }
        at += 8 + size + (size % 2);
      }
-      expect(dataOffset).toBeGreaterThan(44);
-      // Faded to silence by the end (stereo float = 8 bytes per frame).
+      expect(dataOffset).toBeGreaterThan(0);
+      // Faded to silence by the end (stereo float = 8 bytes per frame). This is
+      // the assertion that matters: the parser read a real file and the bake
+      // landed, whatever chunk layout the build chose.
      expect(Math.abs(after.readFloatLE(dataOffset + (SAMPLE_RATE - 2) * 8))).toBeLessThan(0.02);
    });
  });
@@ -122,7 +122,6 @@ export function createEnvelopeWalker(
  };
 }

-/**
 /** Every sample scaled by the envelope, in place, in whichever of the two
 *  formats the layout reports. Float is NOT clamped: it is the format the group
 *  sub-mix writes precisely so an over-unity sum keeps its headroom until
@@ -32,6 +32,7 @@ import {
 import { resolveAudioGroups, resolveCarveSourceIds } from "@hyperframes/core/audio-groups";
 import {
  collectCarveCandidates,
+  CARVE_ABORTED,
  isPromiseLike,
  resolveNextCarveSettings,
 } from "./useFxCarveGrouping.js";
@@ -425,6 +426,7 @@ export function useFxCarve(
    // awaited: see resolveNextCarveSettings's own contract.
    const resolved = resolveNextCarveSettings(nextRaw, doc, onAutoGroupCarveSources);
    const next = isPromiseLike(resolved) ? await resolved : resolved;
+    if (next === CARVE_ABORTED) return;
    // Which of the carve's settings moved. One event per change with the action
    // named, rather than a single "carve touched" — enabling a carve and nudging
    // its strength are different decisions and the interesting question (do
@@ -30,6 +30,12 @@ export function isPromiseLike<T>(value: T | Promise<T>): value is Promise<T> {
  return typeof (value as { then?: unknown })?.then === "function";
 }

+/**
+ * "The auto-group failed, do not persist this carve" — distinct from a
+ * legitimate `null`, which means the carve was deliberately cleared.
+ */
+export const CARVE_ABORTED = Symbol("carve-aborted");
+
 /**
 * Plural voiceover carve, always against a group — normative, not a
 * suggestion (groups doc §1.6). Picking a second ungrouped voice clip mints a
@@ -71,8 +77,16 @@ export function resolveNextCarveSettings(
  nextRaw: HfCarveSettings | null,
  doc: Document | undefined,
  assignGroup: ((clipIds: readonly string[], groupId: string) => Promise<void>) | undefined,
-): HfCarveSettings | Promise<HfCarveSettings> | null {
-  return nextRaw && doc ? withAutoGroupedSources(doc, nextRaw, assignGroup) : nextRaw;
+): HfCarveSettings | Promise<HfCarveSettings | typeof CARVE_ABORTED> | null {
+  const resolved = nextRaw && doc ? withAutoGroupedSources(doc, nextRaw, assignGroup) : nextRaw;
+  // A failed auto-group resolves to the sentinel rather than rejecting: the
+  // write has already toasted, and the caller's job is simply not to persist a
+  // carve whose `sources` name a group that was never written — which reads, at
+  // playback, as a carve that silently stops ducking. Caught here rather than
+  // in the caller so the synchronous branch above stays synchronous.
+  return isPromiseLike(resolved)
+    ? resolved.catch((): typeof CARVE_ABORTED => CARVE_ABORTED)
+    : resolved;
 }

 export interface CarveCandidate {
@@ -28,21 +28,47 @@ import {
  type UseTimelineElementVisibilityEditingInput,
 } from "./timelineTrackVisibility";

+/**
+ * Assign (or restore) `data-audio-group` across a set of members.
+ *
+ * `restore` carries each member's PRIOR value so the unwind can put back a
+ * membership that already existed, rather than removing the attribute outright.
+ * `setElementsHidden`, which this mirrors, gets away with a plain `!hidden`
+ * because hidden is boolean; group membership is an arbitrary id, and the carve
+ * path does not check whether a clip is already grouped — so a failed save
+ * could silently un-group clips that belonged to another group before it.
+ */
 function patchLiveAudioGroupState(
  iframe: HTMLIFrameElement | null,
  elements: readonly TimelineElement[],
  groupId: string | null,
  activeCompPath: string | null,
+  restore?: ReadonlyMap<TimelineElement, string | null>,
 ): void {
  for (const element of elements) {
    const target = findTimelineElementInIframe(iframe, element, activeCompPath);
    if (!target) continue;
-    if (groupId) target.setAttribute(HF_AUDIO_GROUP_ATTR, groupId);
+    const next = restore ? (restore.get(element) ?? null) : groupId;
+    if (next) target.setAttribute(HF_AUDIO_GROUP_ATTR, next);
    else target.removeAttribute(HF_AUDIO_GROUP_ATTR);
  }
  invalidateGroupInfoCache(iframe?.contentDocument);
 }

+/** Each member's `data-audio-group` before this write, for the unwind. */
+function captureAudioGroupState(
+  iframe: HTMLIFrameElement | null,
+  elements: readonly TimelineElement[],
+  activeCompPath: string | null,
+): Map<TimelineElement, string | null> {
+  const prior = new Map<TimelineElement, string | null>();
+  for (const element of elements) {
+    const target = findTimelineElementInIframe(iframe, element, activeCompPath);
+    prior.set(element, target?.getAttribute(HF_AUDIO_GROUP_ATTR) ?? null);
+  }
+  return prior;
+}
+
 /** Group ids are interpolated into markup and into a render-side filename, so
 *  they stay in the character set an HTML id and a path can both carry. */
 const GROUP_ID_PATTERN = /^[A-Za-z0-9_-]+$/;
@@ -63,7 +89,17 @@ const GROUP_ID_PATTERN = /^[A-Za-z0-9_-]+$/;
 * buys nothing.
 */
 function insertGroupElement(html: string, groupId: string): string {
-  if (readTagSnippetByTarget(html, { id: groupId }) !== undefined) return html;
+  const existing = readTagSnippetByTarget(html, { id: groupId });
+  if (existing !== undefined) {
+    // Only OUR tag counts as "already there". The id was minted against the
+    // live preview document, which does not contain markup that is on disk but
+    // not rendered (inside a `<template>`, or an unloaded sub-composition) — so
+    // an unrelated element can already own it. Writing nothing there would aim
+    // every later group write (`buildPatchTarget({ domId })`) at that element,
+    // stamping data-volume / data-hidden / data-fx-chain onto it.
+    if (new RegExp(`^<\\s*${HF_AUDIO_GROUP_TAG}\\b`, "i").test(existing)) return html;
+    throw new Error(`Cannot create audio group: id ${groupId} is already used in this file`);
+  }
  const tag = `<${HF_AUDIO_GROUP_TAG} id="${groupId}"></${HF_AUDIO_GROUP_TAG}>`;
  const closeBody = html.lastIndexOf("</body>");
  if (closeBody < 0) return `${html}\n${tag}\n`;
@@ -124,6 +160,7 @@ export async function createAudioGroupAndAssignMembers({
    throw new Error(`Invalid audio group id ${JSON.stringify(groupId)}`);
  }

+  const priorGroups = captureAudioGroupState(previewIframe, elements, activeCompPath);
  patchLiveAudioGroupState(previewIframe, elements, groupId, activeCompPath);
  const createdLiveGroupElement = patchLiveGroupElement(previewIframe, groupId);
  reseekPreviewRuntime(previewIframe);
@@ -191,7 +228,7 @@ export async function createAudioGroupAndAssignMembers({
    // Mirrors setElementsHidden's failure path: the optimistic live patch
    // already ran, so a save failure has to be unwound or the preview shows a
    // grouping that never made it to disk.
-    patchLiveAudioGroupState(previewIframe, elements, null, activeCompPath);
+    patchLiveAudioGroupState(previewIframe, elements, null, activeCompPath, priorGroups);
    if (createdLiveGroupElement) {
      previewIframe?.contentDocument?.getElementById(groupId)?.remove();
    }
@@ -263,6 +300,12 @@ export function useAudioGroupCarveAssignment({
        console.error("[Timeline] Failed to group voice clips", error);
        const message = error instanceof Error ? error.message : "Failed to group voice clips";
        showToast(message);
+        // Rethrown, not just reported: the carve's auto-group chains
+        // `.then(() => ({ ...next, sources: [groupId] }))` off this promise, so
+        // swallowing here let it persist a carve pointing at a group that was
+        // never written — the exact silent no-op the throw inside
+        // `createAudioGroupAndAssignMembers` exists to prevent.
+        throw error;
      }
    },
    [
@@ -115,8 +115,11 @@ describe("useAudioGroupCarveAssignment", () => {
    act(() => root.unmount());
  });

-  // Loud, not silent: the caller persists the group id once this resolves.
-  it("toasts instead of silently writing nothing when an id resolves to no clip", async () => {
+  // Loud AND rejecting: the carve chains `.then(() => ({...next, sources:
+  // [groupId]}))` off this promise, so a resolved-but-failed call let it
+  // persist a carve aimed at a group that was never written. Toasting alone
+  // was not enough — the promise has to carry the failure too.
+  it("rejects, and toasts, when an id resolves to no clip", async () => {
    stubProjectFiles(new Map([["index.html", FILE]]));
    usePlayerStore.getState().setElements([audio({ domId: "voice-1" })]);

@@ -126,7 +129,7 @@ describe("useAudioGroupCarveAssignment", () => {
    });

    await act(async () => {
-      await assign(["voice-1", "voice-gone"], "voiceover");
+      await expect(assign(["voice-1", "voice-gone"], "voiceover")).rejects.toThrow("voice-gone");
    });

    expect(writes.size).toBe(0);
@@ -9,6 +9,8 @@ import type { PersistDomEditOperations } from "./domEditCommitTypes";
 import { reportDomEditPersistFailure } from "./domEditPersistFailure";
 import { bumpDomEditCommitMapVersion, runDomEditCommit } from "./domEditCommitRunner";
 import { syncStoredAutomationFromPreview } from "../player/lib/automationStoreSync";
+import { HF_AUDIO_GROUP_TAG } from "@hyperframes/core/audio-groups";
+import { invalidateGroupInfoCache } from "../player/lib/timelineDOM";

 // ── Types ──

@@ -63,6 +65,17 @@ function setOrRemovePreviewAttribute(
  } else {
    el.setAttribute(fullAttr, value);
  }
+  // Every DOM-edit attribute write funnels through here, which is the only
+  // place that can catch a group edit made from the rack rather than from the
+  // group header — `openGroupFxRack` hands the `<hf-audio-group>` to the DOM
+  // editor, and that path never went near the timeline's own writers. The group
+  // scan is cached against the preview Document, and group edits are live
+  // patches so that document is never replaced; a stale entry is re-read on
+  // every manifest tick, so the header's preset button then builds on the old
+  // chain and discards the rack's edit.
+  if (el.tagName.toLowerCase() === HF_AUDIO_GROUP_TAG) {
+    invalidateGroupInfoCache(el.ownerDocument);
+  }
 }

 function findPreviewAttributeElement(
@@ -1,4 +1,3 @@
-import type { TimelineElement } from "../store/playerStore";
 import { usePlayerStore } from "../store/playerStore";
 import { isGroupHalfLitUnderSolo } from "../store/audioSoloSlice";
 import { runtimeAudioId } from "../lib/timelineElementHelpers";
@@ -23,7 +22,6 @@ interface TimelineGroupRowProps {
  rowKey: number;
  group: TimelineTrackGroupInfo;
  logicalRow: TimelineLogicalRow;
-  tracks: readonly (readonly [number, readonly TimelineElement[]])[];
  top: number;
  height: number;
  virtualized: boolean;
@@ -42,7 +40,6 @@ export function TimelineGroupRow({
  rowKey,
  group,
  logicalRow,
-  tracks,
  top,
  height,
  virtualized,
@@ -54,11 +51,13 @@ export function TimelineGroupRow({
  toggleGroupExpanded,
  toggleLaneOwnerExpanded,
 }: TimelineGroupRowProps) {
-  const memberElements = group.memberTracks.flatMap(
-    (track) => tracks.find(([t]) => t === track)?.[1] ?? [],
-  );
+  // From the group, NOT from `tracks`: a collapsed group emits no member rows
+  // into the display list, and every one of these reads silently degraded to
+  // empty in that (default) state — half-lit solo went dark, the lane count
+  // read 0, and the bus strip fell back to "track 1", "track 2".
+  const memberElements = group.memberElements;
  const memberLabels = group.memberTracks.map((track, i) => {
-    const owner = tracks.find(([t]) => t === track)?.[1]?.find((el) => el.audioGroup);
+    const owner = memberElements.find((el) => el.track === track && el.audioGroup);
    return owner?.label ?? owner?.id ?? `track ${i + 1}`;
  });
  const isLaneOpen = expandedLaneOwnerIds.has(group.id);
@@ -163,7 +163,6 @@ export function TimelineLanes({
                rowKey={rowKey}
                group={group}
                logicalRow={groupLogicalRow}
-                tracks={tracks}
                top={rowGeometry.getRowTop(row)}
                height={rowGeometry.getRowHeight(row)}
                virtualized={rowsVirtualized}
@@ -413,14 +413,22 @@ export function TimelineTrackHeader({
  const openClipFxRack = (clip: TimelineElement) => {
    void domEditActions?.handleTimelineElementSelect(clip);
  };
+  // DOM ids, matching the carve picker's other caller — membership is read back
+  // by `resolveAudioGroups`, which only ever sees the document. A clip with no
+  // DOM id cannot be a member (resolveAudioGroups skips it), so a track holding
+  // one cannot be grouped WHOLE — and grouping the rest would quietly leave
+  // those clips outside the bus, past every fader, mute and effect, while the
+  // UI showed the track as grouped. The button is withheld instead of acting on
+  // a subset, which is also why the carve path's loud guard cannot catch this:
+  // the unresolvable ids were filtered out before the call.
+  const groupableClipIds = trackElements.map(runtimeAudioId);
+  const canGroupWholeTrack =
+    groupableClipIds.length >= 2 && groupableClipIds.every((id) => id !== null);
  const groupUngroupedClips = () => {
    const doc = domEditActions?.previewIframeRef.current?.contentDocument;
    if (!doc || !onGroupClips) return;
-    // DOM ids, matching the carve picker's other caller — membership is read
-    // back by `resolveAudioGroups`, which only ever sees the document.
-    const clipIds = trackElements.map(runtimeAudioId).filter((id): id is string => id !== null);
-    if (clipIds.length < 2) return;
-    void onGroupClips(clipIds, mintGroupId(doc));
+    if (!canGroupWholeTrack) return;
+    void onGroupClips(groupableClipIds as string[], mintGroupId(doc));
  };

  return (
@@ -471,6 +479,7 @@ export function TimelineTrackHeader({
          {isAudioTrack &&
            clipCount > 1 &&
            !isTrackGrouped &&
+            canGroupWholeTrack &&
            isCanaryEnabled("audio-fx-rack") &&
            isCanaryEnabled("audio-groups") && (
              <TimelineFxButton variant="group-pointer" onGroupClips={groupUngroupedClips} />
@@ -315,8 +315,10 @@ export function buildTimelineLogicalRows({
      items: [],
    });
    if (expandedLaneOwnerIds.has(group.id)) {
-      const memberElements = group.memberTracks.flatMap((track) => trackMap.get(track) ?? []);
-      for (const laneGroup of groupAutomationLanes(memberElements)) {
+      // The group's own member list, not `trackMap`: a COLLAPSED group can have
+      // its lane shelf open, and its members are absent from the display list —
+      // so looking them up there emitted zero lane rows for exactly that case.
+      for (const laneGroup of groupAutomationLanes(group.memberElements)) {
        rows.push({
          id: `${groupRowId}::${laneGroup.key}`,
          kind: "row",
@@ -16,6 +16,17 @@ export interface TimelineTrackGroupInfo {
  anchorKey: number;
  /** Member track numbers, ascending. */
  memberTracks: number[];
+  /**
+   * Every clip under this group, in member-track order — INDEPENDENT of whether
+   * the group is expanded.
+   *
+   * Collapsing a group stops emitting its member rows into `tracks`, so anything
+   * that recovered member elements by looking them up there got an empty list in
+   * the default (collapsed) state — silently disabling half-lit solo, the
+   * automation-lane count, and the bus strip's member labels. Membership is not
+   * a display concern, so it does not travel through the display list.
+   */
+  memberElements: TimelineElement[];
  /** The group element's `data-volume`, mirrored from a member's parse (B7's slider). */
  volume: number;
  /** The group element's `data-hidden`, mirrored from a member's parse (B5's group mute). */
@@ -70,6 +81,7 @@ function buildGroupInfo(
  groupId: string,
  fallbackTrackNum: number,
  membership: GroupMembership,
+  rawByTrack: ReadonlyMap<number, TimelineElement[]>,
 ): TimelineTrackGroupInfo {
  const memberTracks = [...(membership.memberTracksByGroup.get(groupId) ?? [])].sort(
    (a, b) => a - b,
@@ -80,6 +92,7 @@ function buildGroupInfo(
    label: membership.labelByGroup.get(groupId) ?? groupId,
    anchorKey: (memberTracks[0] ?? fallbackTrackNum) - 0.5,
    memberTracks,
+    memberElements: memberTracks.flatMap((track) => rawByTrack.get(track) ?? []),
    volume: membership.volumeByGroup.get(groupId) ?? 1,
    hidden: membership.hiddenByGroup.get(groupId) ?? false,
    ...(fxChain ? { fxChain } : {}),
@@ -139,7 +152,7 @@ function groupTimelineTracks(
    }
    if (emitted.has(groupId)) continue;
    emitted.add(groupId);
-    const info = buildGroupInfo(groupId, trackNum, membership);
+    const info = buildGroupInfo(groupId, trackNum, membership, rawByTrack);
    groups.push(info);
    emitGroupRows(info, rawByTrack, trackGroupOf, tracks, expandedGroupIds.has(groupId));
  }
@@ -88,6 +88,18 @@ describe("collapsed audio groups", () => {
    unmount();
  });

+  // Membership is not a display concern. Half-lit solo, the automation-lane
+  // count and the bus strip's member labels all read the group's members, and
+  // all three silently degraded to empty when those were recovered from the
+  // display list — which a collapsed group does not appear in. Collapsed is the
+  // default, so that was every group until someone opened it.
+  it("carries its member elements even while collapsed", () => {
+    const { layout, unmount } = renderGrouped();
+    expect(layout.trackOrder).toEqual([-0.5]); // collapsed: no member rows
+    expect(layout.groups[0]!.memberElements.map((el) => el.id)).toEqual(["voice-1", "voice-2"]);
+    unmount();
+  });
+
  it("emits the member rows once the group is expanded", () => {
    usePlayerStore.setState({ expandedGroupIds: new Set(["voiceover"]) });
    const { layout, unmount } = renderGrouped();