Sub D: Gain effect — end-to-end + canonical effect shape by yuichkun · Pull Request #13 · yuichkun/den

yuichkun · 2026-04-14T10:13:55Z

Closes #5. Parent epic: #1.

Summary

Ships the first real DSP effect (Gain) through every layer — Rust kernel → WASM → AudioWorklet → @denaudio/effects class → catalog page — and refactors the transitional Sub B/C TypeScript API into the canonical effect shape every future effect will copy.

import { Gain } from "@denaudio/effects";

const ctx = new AudioContext();
await Gain.register(ctx);
const gain = new Gain(ctx, { gain: 0.5 });
source.connect(gain).connect(ctx.destination);
gain.gain.setValueAtTime(1.0, ctx.currentTime);

All four test tiers green:

Tier	Result
Tier1 (cargo --lib effects::gain)	3/3
Tier2 (Node golden null)	48 sub-tests (gain 40 + passthrough 8)
Tier3a (Playwright + OAC)	7 (gain 4 + passthrough 3)
Tier3b (Vercel preview)	needs human review ← @yuichkun

Spec divergences (intentional, all recorded in Issue #5 body)

#	What	Why
1	`GainOptions` does NOT extend `RegisterOptions`	URL overrides only matter to async `register()`; passing them to the sync constructor would silently no-op (footgun)
2	`createDenNode` + `WasmReady` interface removed entirely	Pre-1.0; no consumers after Passthrough refactor; smaller public API surface
3	`Passthrough` also gets `dispose()`	Canonical-shape parity — Sub E's add-effect template specifies one shape, not "real effect" vs "test infra" forks
4	`mountEffectPage()` helper extracted into `packages/examples/src/effect-page.ts`	Two pages share probe + deferred A/B + auto-refresh viz; codifies the shape Sub E will reference
5	`mountFilePicker` widget + `mountABPlayer` "user file" entry	Real DSP evaluation needs user audio (drums, vocals); CANONICAL signals validate correctness but not musical usability. 30 s cap, mono→stereo upmix
6	Golden count: 5 × 8 = 40 (not 35 from §6.5)	Python `CANONICAL` has 8 entries incl. `pink`; the 35 in the issue body was off-by-one

Implementation-time fixes worth flagging

Tier2 plus_6db tolerance loosened to -90 dBFS per Issue §8 Fallback Sub A: Repo scaffold — Rust workspace + VitePlus monorepo + CI skeleton #2 (b). Wide-band chirp × f32-vs-f64 trajectory diverges to -91.37 dBFS at +6 dB. The other 7 plus_6db signals all clear -96 with margin; only the chirp is wide-band enough to expose the noise floor. The four other presets (unity, minus_6db, silence, mid_fade) stay tight at -96.
panic_handler gated on cfg(not(test)) with a host-target loop {} fallback, otherwise cargo test --lib fails on host (the test binary brings panic_handler from std and conflicts). The wasm build still traps via core::arch::wasm32::unreachable().
Playwright webServer port moved to 5273 (was 5173). 5173 collided with an unrelated local Vite project on the maintainer's machine and reuseExistingServer: true silently latched onto it, serving the wrong app with no useful error.
mountFilePicker clones the ArrayBuffer via slice(0) before decodeAudioData so re-picking the same file works (Safari historically rejects the second decode of an already-consumed buffer).

Test plan

cargo test -p den-core --lib — 3/3
cargo clippy --workspace --all-targets -- -D warnings — clean
cargo fmt --check — clean
vp check — typecheck + lint + format clean across 34 files
vp run build — all 6 packages built
vp run test:tier2 — gain + passthrough golden null
vp run test:tier3a — Playwright + OfflineAudioContext
Tier3b — Vercel preview (this PR's): navigate to /#/gain, click "Enable A/B player", verify:
- signal selector + Play/Stop work
- gain slider 0–2 changes loudness monotonically without audible clicks
- gain = 0 produces silence within ~300 ms
- waveform/spectrogram auto-refresh on signal AND slider changes
- file picker accepts a small wav (e.g. a 3 s drum loop), file appears as "user file" in the dropdown, plays through Gain at 0.5 = half volume
- /#/passthrough still works identically (regression after mountEffectPage extraction)

🤖 Generated with Claude Code

Implements Issue #5. Ships the first real DSP effect (Gain) through every layer — Rust kernel → WASM → AudioWorklet → @denaudio/effects class → catalog page — and refactors the transitional Sub B/C TypeScript API into the canonical effect shape every future effect will copy. Rust: * Restructure den-core: alloc/dealloc → src/alloc_shim.rs; each kernel under src/effects/<name>.rs (passthrough moved verbatim). * Add libm = "0.2" for no_std expf in the smoothing-coef init. * Implement Gain kernel: GainState (smoothed_l, smoothed_r, smooth_coef), 1-pole exponential smoothing (tau = 20 ms, parent D13), per-sample a-rate / k-rate dispatch. 3 host-target unit tests. * Gate panic_handler on cfg(not(test)) so cargo test --lib compiles on host (test binary brings its own from std); wasm build still traps via core::arch::wasm32::unreachable. Worklet: * registerDenWorklet → Promise<void> with sync getCachedWasmBytes(ctx) accessor; in-flight slot serializes concurrent register() calls (eagerly implements §8 Fallback #4). * Drop createDenNode + WasmReady — the canonical shape extends AudioWorkletNode directly with sync constructors. * processor.ts: parameterDescriptors with gain a-rate, per-kernel state + param scratch alloc, port {__denCmd:"destroy"} handler frees state + I/O and returns false from next process(). Effects: * @denaudio/effects/Gain — extends AudioWorkletNode, readonly gain AudioParam, dispose(). GainOptions = { gain?: number } (NOT extends RegisterOptions — URL overrides apply only to register()). * Passthrough refactored to the same shape (sync construct + dispose), @internal in TSDoc, kept exported for the test harness. * README with the canonical happy-path example. Test pipeline: * scipy reference: gain_process(x, target, sr, tau=0.020, init=1.0) + REGISTRY["gain"] with 5 presets (unity, ±6 dB, silence, mid_fade). Generates 40 goldens (5 × 8 CANONICAL signals). * Tier2: tight 4 presets at -96 dBFS, plus_6db at -90 dBFS per Fallback #2 (b) — wide-band chirp × f32-vs-f64 trajectory diverges to -91 dBFS at +6 dB, well anticipated in §8. * Tier3a Gain: 4 (preset, signal) pairs via setValueAtTime to exercise the AudioParam path. Passthrough spec updated to new sync construct (Passthrough.create() factory removed). Catalog (Tier3b): * mountEffectPage helper extracted: probe + deferred A/B + sliders + auto-refresh viz + bridge. pages/passthrough.ts and pages/gain.ts are now ~15-line declarations through it. * mountFilePicker widget: decode → max 30 s cap → mono→stereo upmix. mountABPlayer gains optional getUserFile + "user file" dropdown entry; offline render uses the user file's length. * Move dev port to 5273 (5173 conflicted with another local Vite app and Playwright's reuseExistingServer silently latched onto it). Issue #5 body amended in-line to record all spec divergences (Issue-as-SSoT). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

vercel · 2026-04-14T10:14:00Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
den	Ready	Preview, Comment	Apr 14, 2026 3:53pm

…fecycle Aggregated findings from 4 parallel review agents (Codex + Rust ABI + TS worklet/effects + tests/catalog) on PR #13. All gates still green. P0 * Rust kernel: smoothing state moves f32 → f64 (per-sample multiply also f64, output cast to f32). Empirically the f32 state hit the steady- state multiplication noise floor at ~-91 dBFS at +6 dB, forcing per- preset Tier2 overrides; f64 matches scipy bit-for-bit and clears -96 dBFS on every preset by > 30 dB. State 12 → 24 bytes (negligible), ALU 2× (still trivial — gain is the simplest effect). Tier2's `plus_6db` -90 override removed; single -96 across all 5 presets. * `den_alloc()` returns 0 on OOM; processor and Rust kernel exports were both blindly dereferencing. Add release-mode null guards in `den_gain_init` / `den_gain_process`, and OOM short-circuit in the processor constructor (sets `alive = false`, next `process()` returns false, host GCs). P1 * `registerDenWorklet`: wrap the async body in try/finally so the in- flight slot is cleared on rejection too. Without this, a single failed register (bad URL, network blip) poisons the context — every later retry returns the same rejected promise forever. * A/B player `stop()` now calls `effect.dispose()` (when available) instead of just `disconnect()`. Each Play / signal-switch was leaking the worklet's WASM-side state for the lifetime of the AudioContext. * `mountFilePicker`: pre-decode size guard at 50 MB. Without it a 5 GB WAV would be fully read + sliced before the post-decode duration check fires (OOM in headless Chromium). * `effect-page.ts` `enableLive`: try/catch around register; on failure restore the Enable button and close the AudioContext so the user can retry. * Goldens: `gen.py` zeroes the libsndfile PEAK chunk Unix-timestamp after every `sf.write`. Without this `vp run gen-golden` flips ~80 bytes across the 40 gain WAVs on every run for no semantic reason (and accidentally pollutes diffs). * Cargo.toml `[lib] doctest = false` + lib.rs comment correction so plain `cargo test -p den-core` (without `--lib`) doesn't trip on the `unwinding panics not supported without std` doctest path. P2 (cleanup; behavior unchanged) * Worklet processor: try/catch around the destroy handler so a `den_dealloc` panic doesn't tear down the entire AudioWorklet- GlobalScope. `disposeIoBuffers` and `alive` move to private. Unknown `__denCmd` messages now `console.warn` instead of silently no-op'ing. * Effects: `Gain.dispose` and `Passthrough.dispose` get an idempotency guard (#disposed flag) and a louder TSDoc warning that they disconnect ALL routing. * Rust: `pub use` is now an explicit list (no `effects::gain::*` glob), keeping `GainState` out of the crate root surface. * Catalog: vite.config.ts drops the dead `port: 5173` (CLI override in package.json was already 5273); main.ts resets `__denReady`/`__denTier3a` on every page navigation so cross-page state can't leak between Tier3a runs. * README: install snippet uses `vp add` (matches repo's vp-only rule in AGENTS.md / global CLAUDE.md). Issue #5 body amended in-line for §5.2 (dispose semantics), §5.3 (Tier2 fixed at 48 kHz), §6.1 (f64 GainState), §6.10.2 (pre-decode guard); 5 new decisions table rows: Smoothing-state precision, Tier2 tolerance, OOM handling, register failure recovery, golden reproducibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

User-visible fixes from the deployed Vercel preview review. 1. `refreshViz skipped: ... call await Effect.register(ctx) first` warning + missing waveform/spectrogram. `mountABPlayer.getLastRendered()` mints a fresh `OfflineAudioContext` per render, but `mountEffectPage` only registered on the realtime ctx. The sync `getCachedWasmBytes(offlineCtx)` then threw, `refreshViz` swallowed the error, and the visualizers never mounted. Fix: in the `makeNode` wrapper passed to `mountABPlayer`, `await opts.register(c, ...)` first — idempotent on the realtime ctx, does the real fetch+addModule on each fresh offline ctx. 2. File picker UI in wrong place. Was a separate `<section id="file">` peer to the A/B player section, so the relationship to the "user file" entry in the signal dropdown was invisible. Move it INSIDE the A/B section as an appended sub-container with a thin divider, and auto-select the user file in the dropdown when the picker emits a load. Now the connection is obvious at a glance. 3. ~100 ms startup burst on Gain page even at slider=0. Caused by the kernel's smoothed state being hard-coded to 1.0 at init (Sub D §6.1 "Initial smoothed state" decision row). With slider=0, `parameterData.gain = 0` set the AudioParam target to 0, but the smoother still ramped from 1.0 → 0 over 5τ ≈ 100 ms, producing audible decay when the user expected silence. Reverse that decision: extend `den_gain_init(state, sr)` to `den_gain_init(state, sr, initial)`, forward `options.gain ?? 1.0` from the `Gain` constructor through a new `__denInitialGain` processorOption to the worklet, which seeds the kernel. First sample is now AT the user-requested gain — no transient. Subsequent automation (`setValueAtTime`, `linearRampToValueAtTime`) still smooths via the per-sample 1-pole as before. Tier1 tests keep init=1.0 (analytic predictions hold); Tier2 reference also keeps init=1.0 (matches scipy `gain_process(init=1.0)`); Tier3a `new Gain(ctx)` followed by `setValueAtTime(target, 0)` still exercises the 1.0→target ramp. Issue #5 §3 decision row updated in-line. 4. Belt-and-braces for any residual startup click on either page: `mountABPlayer.play()` now ramps wet AND dry GainNodes from 0 to their target mix levels over 30 ms at `src.start()`. Masks the AudioBufferSourceNode start click, the chirp's 20 Hz sample-0 boom, and OS audio buffer warm-up. 30 ms is well under the speech- perception threshold so users don't notice the delay. All gates re-green: cargo test --lib (3/3), Tier2 (48 sub-tests at single -96 dBFS), Tier3a (7), vp check, vp run build. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…lyserNode Discard the entire `mountEffectPage` / `mountABPlayer` / `mountFilePicker` / `mountWaveform` / `mountSpectrogram` abstraction and the `getLastRendered` offline-render-on-every-slider-change strategy. Every bug we hit on the deployed preview (OOM under slider drag, gain inconsistency from `currentNode` aliasing, half-mounted pages, audible 100 ms startup transient at gain=0, broken waveform rendering) traced back to that abstraction. Replace with two self-contained pages (~290 + ~225 lines, fully readable top-to-bottom) and a tiny `lib/viz.ts` (drawWaveform + drawSpectrum, AnalyserNode-driven). Architecture * One `AudioContext` per page lifecycle. One source / one effect / one analyser. Slider changes call `setValueAtTime` on the live AudioParam — no offline render, no per-event allocation. * `AnalyserNode` + `requestAnimationFrame` drives the visualizers. OOM under rapid slider drag is now structurally impossible. * Bypass toggle (replaces the wet/dry crossfade slider) — clearer A/B semantics, simpler graph. * File picker is one option in the source dropdown ("Custom file…") instead of a separate widget. Auto-selects the loaded file as the active source on pick. * Tier3a Playwright tests are unaffected — they use OAC directly, not the catalog UI; both spec files still pass without changes. Waveform fix * `fftSize=2048` covers ~42 ms; at 10 kHz that's 427 cycles squeezed into a 962-pixel canvas, which the old min/max-per-pixel renderer collapsed to a solid blue band ("waveform looks broken"). Show only the latest 256 samples (~5.3 ms) as a connected oscilloscope-style line trace. Now sine_1k, sine_10k, and chirp all render as visibly distinct waveforms; gain=0 collapses to a flat center line. Verified locally via `agent-browser` against `vite preview`: - waveform rendering at sine_1k / sine_10k / chirp / gain=0 - linear gain scaling (0 → 0 px, 0.25 → 7860 px, 0.5 → 15374 px, 1.0 → 30488 px — proportional) - 200 rapid slider changes in 1.15 s with 0 errors, 0 OOM - Bypass ON at gain=0 still shows signal (effect routed around) - Source switch mid-playback rebuilds source cleanly - Passthrough page mounts + plays + visualizers work - 0 console errors locally (the Vercel `payload` error is Vercel Live Toolbar / extension noise, not our code — `rg payload` against the bundled JS returns no hits) All gates green: cargo test --lib (3/3), vp check, vp run build, vp run test (Tier2 48 sub-tests + Tier3a 7 tests). Bundle size: 43.7 KB → 37.1 KB (dropped fft.js dep, dropped widgets.ts abstraction layer). Issue #5 §3 amended: "Catalog page common code" + "User-file picker" decision rows marked superseded; new "Catalog page architecture" row documents the AnalyserNode + Bypass + self-contained-pages choice. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b2d7d4c08d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex review on PR #13 (b2d7d4c) flagged: the `Gain` constructor forwarded `options.gain` to both `parameterData.gain` (which the AudioParam descriptor clamps to [0, 10]) AND to `processorOptions.__denInitialGain` (which `den_gain_init` writes straight into the kernel's smoother state). Result: a caller passing `gain: -1` would seed the smoother at -1 and emit inverted-phase audio for the first ~100 ms before converging to the AudioParam's clamped 0; `gain: NaN` would pin the smoother at NaN forever (NaN × anything = NaN, every subsequent automation event also NaN). Clamp to the descriptor's [0, 10] range and substitute 1.0 on non-finite input. Both fields now consume the same sanitized value. Tier1 + Tier2 + Tier3a still green (existing tests use valid values 0 / 0.25 / 0.5011872336272722 / 1.0 / 1.9952623149688795). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

yuichkun · 2026-04-14T12:50:31Z

@codex review

CI's `setup-vp@v1.6.0` ships oxfmt newer than my local vp 0.1.15 — the upgraded formatter wants the `Number.isFinite(requested) ? ... : 1.0` ternary on a single line (still < 100 chars). Local was passing because the older oxfmt accepted both forms. Synced local vp to 0.1.16 and applied `vp check --fix`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

den/packages/examples/src/main.ts

Line 32 in 1e19f66

void page.render(stage);

Guard page render against stale async completion

render() fire-and-forgets page.render(stage) even though each page does async setup before setting window.__denTier3a/__denReady. If the hash changes while an earlier render is still awaiting, that stale render can finish later and overwrite the bridge for the currently displayed page, causing wrong test bridge bindings and route-dependent flakiness. Add a navigation token/cancellation check so only the latest render is allowed to publish globals.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex review on PR #13 caught: when no play ctx exists yet, `onFilePick` mints a `new AudioContext()` for `decodeAudioData` and never closes it. Browsers cap concurrent AudioContexts at ~6 (Chrome) — a few file re-picks before pressing Play would brick the page until reload. Decoded `AudioBuffer` is portable across contexts per Web Audio §1.4, so closing the temp ctx after decode is safe; the buffer stays valid for later playback against the real ctx. Wrap the decode in try/finally and `await tempCtx.close()` only when we actually created the temp ctx (never close the live play ctx). Same fix in both gain.ts and passthrough.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

yuichkun · 2026-04-14T13:00:56Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9c56deeec9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex review on PR #13 caught two P2 bugs. 1. processor.ts:148 — partial WASM allocation leak on OOM. `new DenProcessor()` allocates l_in / r_in / l_out / r_out sequentially. If l_in succeeds but r_in fails (OOM under low memory), the early `return` fired without freeing l_in. Same for the gain branch (paramScratch / state). Since `port.onmessage` teardown wasn't wired yet, those bytes leaked for the lifetime of the AudioContext — repeated `new Gain(ctx)` in low memory would burn through linear memory permanently. Extracted the dealloc logic into `private freeAllAllocations()` (null-safe per pointer) and call it from BOTH the OOM bail paths and the destroy handler. DRY + correct. 2. main.ts:27 + pages — stale async render race. Both pages do `await Effect.register(probeCtx)` before mutating `window.__denTier3a`. Quickly navigating /#/gain → /#/passthrough left the gain probe in flight; when it resumed, it overwrote the passthrough bridge with stale `Gain` symbols. Tier3a / hash-nav consumers would observe the wrong page state intermittently. Worse: the previous page's AudioContext / source / RAF stayed alive after navigation since main.ts only cleared `stage.innerHTML`. `main.ts` now mints an `AbortController` per render, aborts the previous before mounting the new. Page render takes `signal: AbortSignal`: - registers `signal.addEventListener("abort", …)` for synchronous teardown (RAF cancel, source.stop, effect.dispose, ctx.close) - bails (`if (signal.aborted) return`) before each `window.__den*` mutation so a stale render can't clobber the bridge Both gates green: cargo --lib (3/3), vp check, vp run build, vp run test (Tier2 48 + Tier3a 7). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

yuichkun · 2026-04-14T13:15:34Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc303a7e68

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex P2: drawWaveform / drawSpectrum allocated fresh `Float32Array` buffers (8 KB at fftSize=2048) per call. At 60 fps × 2 functions per page that's ~960 KB/s of GC pressure, and contradicts issue #5's "no per-frame allocations" goal. Hoist scratch buffers to module scope, lazy-allocate, resize only when fftSize / frequencyBinCount changes. Safe because main.ts mounts at most one page at a time. Issue #5 "Catalog page architecture" decision row refined: the "no shared helpers" rule was overly broad — the harm was in *stateful / lifecycle* abstractions (managing AudioContexts, effect nodes, refresh timers across closures), not in pure pixel- pushing utilities. Codified the line: a helper qualifies iff (1) takes inputs as args, (2) holds no per-page state, (3) returns a value or paints a canvas without owning a lifecycle. `lib/viz.ts` qualifies; the dropped `mountEffectPage` did not. (Codex round-3 P1 finding for `lib/viz.ts` import is therefore declined — the spec text was the bug, not the code.) Tier1 + Tier2 + Tier3a green. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

yuichkun · 2026-04-14T13:28:12Z

@codex review

yuichkun · 2026-04-14T13:32:03Z

@codex 再レビュー

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 366d8b7e9f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

yuichkun · 2026-04-14T13:42:22Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 254aa4dfa5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex P1 follow-up to 254aa4d's MODULE_KEY fix: that fix only covered the case where addModule had ALREADY succeeded by the time the retry started. If `fetchWasmBytes` rejects fast while `addModule` is still pending, `Promise.all` rejects immediately, `finally` clears the in-flight lock, and a quick retry can launch a SECOND concurrent `addModule` against the same URL — Chrome short-circuits but Firefox / Safari are not specified to, and on those browsers the second worklet evaluation re-runs `registerProcessor("den-processor", …)` which throws `NotSupportedError`. Capture both legs as named promises and `await Promise.allSettled` in `finally` before clearing the lock. `allSettled` is a no-op when both already settled (happy path) and avoids the race when one rejected fast. The original error is preserved via try/finally semantics. This fully covers the addModule retry surface together with the MODULE_KEY flag from 254aa4d: * If addModule SUCCEEDED (flag set): retry skips it. * If addModule still PENDING: retry waits via the lock. * If addModule rejected: retry re-runs (fresh attempt). Tier1 + Tier2 + Tier3a green. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

yuichkun · 2026-04-14T13:56:31Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4d93578836

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4d93578836

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex P1 caught spec-vs-code drift on `pages/gain.ts:6` — the issue body still mandated a shared `mountEffectPage` helper while the code had pivoted to fully self-contained pages. The maintainer correctly called out the pivot itself: with 50+ effects on the v1.0–v3.0 roadmap, inlining ~150 lines of identical scaffolding into every page would mean 7,500+ lines of pure copy-paste (and Sub D's explicit purpose is to ESTABLISH the canonical effect shape that Sub E's add-effect template will reference). Restore a shared helper, but lean: `renderEffectPage` in `packages/examples/src/lib/effect-page.ts` codifies every lesson the self-contained-page detour proved out — and structurally prevents the bugs the original `mountEffectPage` had: - ONE realtime ctx; the helper never feeds an offline ctx into `makeNode` (no offline-render-on-slider strategy at all), so "currentNode aliasing across realtime/offline" cannot happen. - `signal.addEventListener("abort", teardownLive)` registered SYNCHRONOUSLY before the first await — nav-while-probing tears down cleanly (no half-mounted pages, no leaked AudioContexts). - AnalyserNode + requestAnimationFrame for waveform/spectrum (constant memory; no `WebAssembly.Instance` per slider tick). - Inline file picker in the source dropdown (Custom file…), with 50 MB pre-decode size cap and `await tempCtx.close()` in finally so file re-picks before Play don't leak AudioContexts. - Bypass toggle (single boolean) replacing the wet/dry crossfade. - Multichannel handled by Web Audio's `channelInterpretation: "speakers"` ITU-R BS.775 downmix (no JS-side downmix — addresses Codex P2 on `pages/gain.ts:177`). Each page becomes a ~25-line declarative config: await renderEffectPage(root, signal, { title, description, register: (ctx, opts) => Effect.register(ctx, opts), makeNode: (ctx, params) => new Effect(ctx, ...), applyParam, params, bridge, workletUrl, }); Issue #5 amended in-line: §3 "Catalog page architecture" decision row rewritten with the journey honestly recorded; §6.8 + §6.10 + DoD checklist updated to the `renderEffectPage` shape; old `widgets.ts` / `mountFilePicker` / `mountABPlayer` references dropped (file picker is now inline; no separate widget file). Tier1 + Tier2 + Tier3a green; bundle 39.17 KB → 36.91 KB (helper deduplication). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

yuichkun · 2026-04-14T14:25:24Z

@codex review

yuichkun · 2026-04-14T14:29:11Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fefe22b27a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex P1: `Symbol.for("den.worklet.cache")` is global across bundled copies of `@denaudio/worklet` (and the comment claims this as a mixed-version compat feature). An older Sub B build stored a `Promise<{ bytes, workletModuleAdded }>` under the same key. Sub D's `registerDenWorklet` was treating ANY truthy value as "cache hit", so loading alongside such a build would short-circuit register and have `getCachedWasmBytes` return `(promise as Cached).bytes === undefined` — `instantiateSync(undefined)` would then throw inside the worklet. Pre-1.0 there are no Sub B installs in the wild, but the comment documents mixed-version sharing as a design feature. If we claim it, we should support it correctly. Cheap insurance for 1.0+. Validate `raw.bytes instanceof ArrayBuffer` in `readCached`; anything else (a Promise, a partial / corrupted state, a future schema) is treated as no-cache and the regular register path overwrites the slot with the current shape. Tier1 + Tier2 + Tier3a green. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

yuichkun · 2026-04-14T14:59:57Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5f1bc63925

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex round-8: two more independent findings. R8a (P1, effect-page.ts): concurrent Play click race. The `playBtn` handler gated only on `live`, but `start()` is async and `live` stays null until after `await opts.register(...)`. A quick double-click would spawn two parallel `AudioContext`s, and only the last one would be tracked in `live` — earlier contexts kept playing in the background with no UI affordance to stop. Add a `starting` flag (and `playBtn.disabled = true` for visible feedback) that wraps the whole `start()` await chain via a private `startInner()`; the guard in the click handler skips when either `live` or `starting` is truthy. R8b (P2, worklet/main.ts): WeakMap fallback masked by symbol slot. After R7 added `bytes instanceof ArrayBuffer` shape validation, `readCached` still used `aug[CACHE_KEY] ?? FALLBACK_CACHE.get(ctx)` — meaning a truthy-but-incompatible symbol slot value (e.g. a Sub B-style `Promise`) would short-circuit `??` and never read the WeakMap fallback. In the narrow case where the ctx is also frozen (so `writeCached` could only write to the WeakMap), a fresh `registerDenWorklet` would write to WeakMap but `readCached` would keep ignoring it. Check the symbol slot AND the WeakMap fallback INDEPENDENTLY, returning whichever first satisfies the shape check. Both gates green: cargo --lib 3/3, vp check, vp run build, vp run test (Tier2 48 + Tier3a 7). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

yuichkun · 2026-04-14T15:17:20Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3ad21162a2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex round-9 P1 flagged that an older `@denaudio/worklet` (Sub B, Promise<WasmReady> shape) in the same `Symbol.for("den.worklet.cache")` slot would, after R7's shape validation, be treated as no-cache — so the register path runs again and `addModule` may fail with NotSupportedError on browsers that don't dedupe duplicate `registerProcessor` calls. Real chain, but the necessary scenario (Sub B + Sub D loaded on the same ctx) cannot exist: Sub B was internal-only, never publicly released. Same-major Sub D ↔ Sub D copies share the current `{ bytes: ArrayBuffer }` shape and interoperate fine. The only genuinely incompatible peer would be a hypothetical future v2, which would have to ship its own migration path anyway (await any legacy Promise it finds, OR pick a fresh symbol name). Declining the proposed "tolerate addModule rejections" fix: - Pre-1.0 the scenario is impossible (no Sub B in the wild). - Browser error message / DOMException name format is unspecified by W3C, so error-shape detection is fragile across browsers. - A generic "addModule fail = OK" hack creates a real false-positive failure mode: real addModule errors (bad workletUrl, CSP violation, worklet syntax error) get silently converted into "ready" → next `new AudioWorkletNode` throws with an unrelated message, hiding the actual cause. Instead, tighten the comment's claim to match reality: same-major mixed copies interoperate; cross-major needs deliberate migration, deferred until v2 actually exists. No code change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

yuichkun · 2026-04-14T15:33:45Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1ba9f9aee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex P2: `writeModuleAdded(ctx)` fires as soon as addModule resolves, but didn't record WHICH url was loaded. A caller that first passed a loadable-but-wrong workletUrl (typo, mirror mismatch, or a file that simply didn't `registerProcessor("den-processor", …)`) would latch MODULE_KEY = true after the wrong script loaded. On retry with a corrected workletUrl, the flag short-circuited addModule, so the correct script was never loaded for that context — subsequent `new AudioWorkletNode(..., "den-processor")` would keep failing silently (or use the wrong processor). Promote `MODULE_KEY` from `true` to `{ url: string }` and pass the workletUrl through `readModuleAdded(ctx, url)` / `writeModuleAdded( ctx, url)`. A retry with the SAME url still skips addModule (R4 still holds — fetch-failure-but-addModule-succeeded path is safe). A retry with a DIFFERENT url re-attempts addModule, which either succeeds (ctx was clean) or surfaces a clear `NotSupportedError` (some prior url already latched a processor onto the ctx — only real recovery is a fresh AudioContext, but the user gets an explicit signal instead of silent wrong behavior). Pre-1.0 no shipped `@denaudio/worklet` wrote the old `true` shape publicly, so the `true` → `{ url }` transition has no cross-version compat cost. Tier1 + Tier2 + Tier3a green. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

yuichkun · 2026-04-14T15:52:38Z

@codex review

chatgpt-codex-connector · 2026-04-14T16:02:14Z

Codex Review: Didn't find any major issues. Another round soon, please!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

vercel Bot deployed to Preview April 14, 2026 10:14 View deployment

vercel Bot deployed to Preview April 14, 2026 10:39 View deployment

vercel Bot deployed to Preview April 14, 2026 11:02 View deployment

vercel Bot deployed to Preview April 14, 2026 12:39 View deployment

yuichkun marked this pull request as ready for review April 14, 2026 12:40

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/effects/src/gain.ts

vercel Bot deployed to Preview April 14, 2026 12:49 View deployment

vercel Bot deployed to Preview April 14, 2026 12:53 View deployment

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/examples/src/pages/gain.ts Outdated

vercel Bot deployed to Preview April 14, 2026 12:59 View deployment

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/examples/src/main.ts

Comment thread packages/worklet/src/processor.ts

vercel Bot deployed to Preview April 14, 2026 13:15 View deployment

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/examples/src/lib/viz.ts Outdated

Comment thread packages/examples/src/pages/gain.ts Outdated

vercel Bot deployed to Preview April 14, 2026 13:28 View deployment

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/worklet/src/main.ts Outdated

vercel Bot deployed to Preview April 14, 2026 13:43 View deployment

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/worklet/src/main.ts Outdated

vercel Bot deployed to Preview April 14, 2026 13:57 View deployment

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/examples/src/pages/gain.ts Outdated

Comment thread packages/examples/src/pages/gain.ts Outdated

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/examples/src/pages/gain.ts Outdated

Comment thread packages/examples/src/pages/gain.ts Outdated

vercel Bot deployed to Preview April 14, 2026 14:26 View deployment

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/worklet/src/main.ts

vercel Bot deployed to Preview April 14, 2026 15:00 View deployment

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/examples/src/lib/effect-page.ts Outdated

Comment thread packages/worklet/src/main.ts Outdated

vercel Bot deployed to Preview April 14, 2026 15:17 View deployment

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/worklet/src/main.ts

vercel Bot deployed to Preview April 14, 2026 15:34 View deployment

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread packages/worklet/src/main.ts Outdated

vercel Bot deployed to Preview April 14, 2026 15:53 View deployment

yuichkun merged commit c15480a into main Apr 14, 2026
6 checks passed

Conversation

yuichkun commented Apr 14, 2026

Summary

Spec divergences (intentional, all recorded in Issue #5 body)

Implementation-time fixes worth flagging

Test plan

Uh oh!

vercel Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

yuichkun commented Apr 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

yuichkun commented Apr 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

yuichkun commented Apr 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

yuichkun commented Apr 14, 2026

Uh oh!

yuichkun commented Apr 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

yuichkun commented Apr 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

yuichkun commented Apr 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

yuichkun commented Apr 14, 2026

Uh oh!

yuichkun commented Apr 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

yuichkun commented Apr 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

vercel Bot commented Apr 14, 2026 •

edited

Loading