fix(watch): keep the audio decoder across a numeric delay change

[kixelated]

Closes #4777

Problem

Changing @moq/watch's numeric delay (100 to 120 ms, or "auto" to a number) resubscribed audio, rebuilt the decoder, and truncated the ring: an audible gap. The audio decoder read effect.get(sync.in.delay) === "instant", which tracks the whole signal, so every numeric step reran #runDecoder and #runEnabled, and the replacement subscription's handover truncated the ring.

Approach

• Sync derives out.instant (one owner for the predicate). The audio decoder and Sync's own #runMaxAge and #runDelay read it, so only a switch to or from "instant" reruns them. wait and its #sleep still check and wake on in.delay directly, since out.instant lags a same-turn change until effects flush (regression test included). Numeric changes still reach the ring through #runLatency's setLatency.
• Fixed the stale Delay doc, which said the owner must turn audio off; the decoder already does.
• New js/watch/src/audio/decoder.test.ts drives a real Decoder with fake WebAudio/WebCodecs globals and a fed container consumer: a numeric change and "auto" to a number keep one subscription, one codec, and no ring truncate/reset (both fail on main with 2 subscriptions); a switch to "instant" resets the ring, and back rebuilds with one handover truncate.
• Deleted the quest and its references.

Impact

• @moq/watch: adds Sync.out.instant, a read-only Getter<boolean>. Additive, no breaking change. The maintainer decided it stays public.
• Wire: none.

Alternatives

• A private predicate inside the decoder would duplicate the "instant" check across Sync and the decoder; out.instant keeps one owner.

Follow-ups

• Audio graph lifetime remains the other needless audio teardown in this decoder.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[kixelated]

Quest outcome: implemented as planned; just check passes locally. The new decoder test fails on main (2 subscriptions after a numeric step) and passes here.

Open decision for the maintainer: whether Sync.out.instant should be public. Recommendation: keep it public, since out is the component convention and the audio decoder is an in-package consumer; the planned SyncInput reshape in /quest/m1/av-clock.md can revisit it.

Left as a draft for review.

(Written by Claude Opus 5.5)

[kixelated]

Maintainer decision: Sync.out.instant stays public, following the out component convention. The SyncInput reshape in /quest/m1/av-clock.md can revisit it.

(Written by Claude Opus 5.5)

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 5 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 09d2a6b7-896c-4d07-a4b2-b98b1e1c4ef6

📥 Commits

Reviewing files that changed from the base of the PR and between cc1079c and aa40391.

📒 Files selected for processing (8)

• js/watch/src/audio/decoder.test.ts
• js/watch/src/audio/decoder.ts
• js/watch/src/sync.test.ts
• js/watch/src/sync.ts
• quest/m0/README.md
• quest/m0/audio-jitter-target/watch.md
• quest/m0/watch-delay-rebuild.md
• quest/m1/watch-audio-graph.md

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 7279c57.

[P2] Preserve the immediate instant-mode behavior of Sync.wait (js/watch/src/sync.ts:255). out.instant starts false and #runInstant runs on a microtask, so const sync = new Sync({ delay: "instant" }); await sync.wait(Time.Milli(0)); now rejects with "reference not set; call received() first"; the base resolves. A same-turn switch into instant after reset has the same stale-predicate problem. Keep synchronous input peeks in wait (they do not subscribe or trigger decoder rebuilds), or otherwise make its mode check immediately current, and add a regression test without a preliminary flush.

Direction: deriving a deduplicated mode signal for the decoder effects is a focused fix, and numeric latency still flows through #runLatency. Retain that approach while preserving wait's synchronous mode semantics. No wire change; the public Getter addition matches the recorded maintainer decision.

Verification: inspected all eight changed files and the relevant decoder, Sync, and signal scheduling code. An isolated Node reproduction using base/head Sync and the repository's Signal/Time implementations, with import paths adapted, resolves on base and rejects on head. Full Bun tests, just check, and real-browser audio playback were not run.

(Written by OpenAI)

[kixelated]

Automated review — head aa4039141f66ebbab41055edaf878dc3b4381b84

Small, well-targeted fix for #4777. Deriving Sync.out.instant means #runEnabled and #runDecoder in js/watch/src/audio/decoder.ts only rerun when the mode flips, and numeric steps still reach the ring through #runLatency and out.delay. I traced the signal scheduling in @moq/signals and it holds up. Sync is always built before the decoder, so #runInstant's first microtask runs before the decoder's first effects, and a player constructed with delay: "instant" doesn't open a throwaway subscription. On a numeric↔instant switch, #runDelay sees out.instant and out.jitter settle together, so there's no transient out.delay. The last push, which moved wait() back to peeking in.delay, is right too, because out.instant lags a same-turn change by one flush.

The quest cleanup is complete: there are no remaining links to watch-delay-rebuild.md (README, audio-jitter-target/watch.md, watch-audio-graph.md).

Blocking

None.

Non-blocking

1. CI hasn't run yet. Check, Test, Replay, and Release JS Packages are all queued on this head. The new decoder.test.ts swaps AudioContext, AudioDecoder, and the other WebAudio/WebCodecs globals and spies on Container.Consumer.prototype. It does this alongside other watch test files in one Bun process, so it's worth confirming it's green and not order-dependent before merging.
2. wait() checks one predicate but wakes on another (js/watch/src/sync.ts:256/265 vs :297). The loop checks in.delay.peek() === "instant", but #sleep now wakes on out.instant.changed, which fires one signal hop after in.delay. The outcome is right, since parked frames still leave, just a microtask or two later than on main. The comment at :264 ("Switching to "instant" wakes the sleep below") is still accurate. If you want the two to line up exactly, #sleep could listen to in.delay.changed again; a numeric step only re-loops once, which is harmless. This is a nit, not a bug.

Cross-PR

• feat(audio): measure the auto playout target from arrival timing #4162 (audio-jitter-target merge, open and ready) rewrites Sync.#runJitter and #runDelay, and it edits quest/m0/audio-jitter-target/watch.md and decoder.ts. Whichever lands second gets a textual conflict. When resolving it, keep #runDelay and #runMaxAge on out.instant (feat(audio): measure the auto playout target from arrival timing #4162's #runDelay reads effect.get(this.in.delay) for the mode, which is fine since numbers matter there). Also make sure the two decoder gates stay on out.instant, or the rebuild-on-numeric-step bug comes back. feat(audio): measure the auto playout target from arrival timing #4162's new #runSubscribeMaxAge reads sync.in.delay === "auto", which is fine because it only sets a signal and doesn't tear anything down.

Verdict: MERGE (once CI is green)

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Automated follow-up review — head 145ad2c63aec5ba7a9da050dbe440b3c0c84e026 (previous Grok review: aa40391)

This push is a one-line change in js/watch/src/sync.ts:297. #sleep now wakes on this.in.delay.changed instead of #out.instant.changed, so wait() checks and wakes on the same predicate (in.delay.peek() === "instant" at :256/:265). That fixes earlier non-blocking #2: a same-turn switch to "instant" releases parked frames right away instead of one effect flush later. A numeric step also wakes the sleep now, but that just re-loops once with the new out.delay, which is harmless, and it matches the #sleep doc comment ("anything the sleep was computed from"). Getter<Delay> exposes changed(fn): Dispose, so the listener is still released on every wake path. The decoder gates are untouched and stay on out.instant, so the original #4777 fix still holds.

Blocking

None.

Still open (non-blocking)

1. CI is still pending. Check, Test, Replay, and Release JS Packages haven't finished on this head. Confirm decoder.test.ts, which swaps WebAudio/WebCodecs globals in a shared Bun process, is green before merging.
2. Cross-PR: the textual conflict with feat(audio): measure the auto playout target from arrival timing #4162 in sync.ts, decoder.ts, and quest/m0/audio-jitter-target/watch.md still applies. When resolving it, keep the decoder gates and #runDelay/#runMaxAge on out.instant.

Verdict: MERGE (once CI is green)

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Merge summary:

• Decoder gates and Sync's #runDelay/#runMaxAge read the derived out.instant, so a numeric delay step (or "auto" to a number) no longer resubscribes, rebuilds the codec, or truncates the ring. Closes watch: changing a numeric delay re-creates the audio subscription and decoder, so every step, up or down, is an audio gap #4777.
• Addressed the review nit: #sleep wakes on in.delay again, matching the predicate wait() checks, so a same-turn switch to "instant" releases parked frames without waiting for an effect flush.
• The OpenAI finding (instant wait() before effects flush) was fixed in aa40391 with a regression test.
• Decision: Sync.out.instant stays public (maintainer). Additive, no wire change.
• feat(audio): measure the auto playout target from arrival timing #4162 will conflict in sync.ts, decoder.ts, and quest/m0/audio-jitter-target/watch.md; keep the decoder gates and #runDelay/#runMaxAge on out.instant when resolving.

(Written by Claude Opus 5.5)