Repository navigation
Composition audio beds are dropped in head-only composition modes #147
Description
Activity
Picked up by /implement - driver Codex, branch
147-preserve-audio-beds, 2026-05-23T06:51:43Z.gc_codex_review — cycle 1 of 1 (pre-push) on issue #147 (branch
147-preserve-audio-beds)Core review
Verdict:
shipThis change is shaped correctly: the design seam is the composition-to-head-slice projection, and the fix keeps composition-level audio-bed metadata attached when that projection truncates manifest and scene slices. It updates both truncation sites, so scene loading and scene navigation keep the same boundary contract instead of solving audio playback lower in the stack. The test pins the user-visible behavior across the two head-slice modes that were likely losing the bed. No obvious next variation is foreclosed.
No blocking findings.
Security review
Verdict:
shipThis change is shaped correctly: it keeps audio-bed propagation at the existing head-slice truncation seam in both scene loading and scene navigation, without adding a new trust boundary, repository access path, request parser, or dynamic execution surface. The cross-cutting concern is runtime media behavior across loop and scrub modes; carrying an already-resolved composition field through the truncated composition object does not weaken validation or scoping, and it leaves the obvious next variation, more composition-level metadata surviving head slicing, at the same seam.
No blocking findings.
gc_codex_review pre-push cycle 1 of 1 complete for issue #147 on branch '147-preserve-audio-beds'. Posted by the MCP server to enforce the pre-push hard-cap-1 contract (issues #796, #804, #906). Do not edit or delete — used by the next
gc_codex_review(uncommitted) invocation to count cycles.Review decision record — codex cycle 1 (issue #147)
Reviewer: codex
Cycle: 1Architectural read:
Core reviewer: This change is shaped correctly: the design seam is the composition-to-head-slice projection, and the fix keeps composition-level audio-bed metadata attached when that projection truncates manifest and scene slices. It updates both truncation sites, so scene loading and scene navigation keep the same boundary contract instead of solving audio playback lower in the stack. The test pins the user-visible behavior across the two head-slice modes that were likely losing the bed. No obvious next variation is foreclosed.
Security reviewer: This change is shaped correctly: it keeps audio-bed propagation at the existing head-slice truncation seam in both scene loading and scene navigation, without adding a new trust boundary, repository access path, request parser, or dynamic execution surface. The cross-cutting concern is runtime media behavior across loop and scrub modes; carrying an already-resolved composition field through the truncated composition object does not weaken validation or scoping, and it leaves the obvious next variation, more composition-level metadata surviving head slicing, at the same seam.
Blocking findings: 0 (clean run)
gc_test_quality_review cycle 1 of 1 — issue #147
Reviewer: test-quality (claude-sonnet-4-6 via gc_test_quality_review)
Branch:147-preserve-audio-beds
Cycle: 1 / 1
Findings: 1Finding 1 — [critical]
tests/runtime/scene-loader-audio.test.ts::scene loader — audio service wiring (PUL-F024 / ADR-004)::does not build an audio service under mode=prompter (no scene mounts):379Problem: The
headscene is declared withassets: ['/audio/head.mp3']but withoutaudio: ['/audio/head.mp3'], soscene.audiodefaults to[]. If a regression causedmode=prompterto take the non-prompter branch and callbuildLoad,createAudioServicewould be called withallowedSources: [], the scene'screate()would callaudio.load('head-bed', { src: '/audio/head.mp3' }), the allowlist check would reject it, no sound would be created, andexpect(audio.created).toHaveLength(0)would still pass. The test cannot distinguish between 'lifecycle was suppressed by the prompter path' and 'lifecycle ran but every audio load was blocked by the empty allowlist'.
Why it matters: A regression removing theeffectiveMode(target) === 'prompter'branch guard at line 1711 of scene-loader.ts — causing the scene resolver lifecycle to mount under mode=prompter — would go entirely undetected. The scene'screate()hook would execute (and the test comment 'Would register a sound — but prompter never mounts the scene' is incorrect: even if it ran, no sound would be created becausescene.audiois empty).
Fix: Addaudio: ['/audio/head.mp3']to thebuildScenecall (alongside the existingassetsentry). ThencollectAudioSourceswould include/audio/head.mp3inallowedSources; if the lifecycle ran,audio.load('head-bed', { src: '/audio/head.mp3' })would succeed,audio.createdwould have length 1, and the assertion would correctly fail — detecting the regression.gc_test_quality_review cycle 1 of 1 complete for issue #147 on branch '147-preserve-audio-beds'. Posted by the MCP server to enforce the gc_test_quality_review hard-cap-1 contract (issue #884 follow-up, default lowered in #906). Do not edit or delete — used by the next
gc_test_quality_reviewinvocation to count cycles.Review decision record — test-quality cycle 1 (issue #147)
Reviewer: test-quality
Cycle: 1Architectural read:
This file pins the scene loader's audio-wiring seam — the four behavioral contracts PUL-F024/ADR-004 record — through a recording audio engine that is a real in-process implementation, not a mock; it captures every
createSoundcall and every handle method call so assertions can be made on both engine-level state (muted flag, created count) and the AudioService's own state machine (isDisposed, isMuted). The three describe blocks (core wiring, rehearsal, unlock-gate bypass matrix, composition bed) map cleanly onto the PUL-Fxxx clause groups being pinned. The recording unlock adapter and recording timeline adapter follow the same pattern and are correctly factored. The issue #99 regression nail (single stop-group for a repeated scene id) uses the cue log as the observable signal — the right observable for that specific invariant. The shape is correct and the overall coverage is strong. The one seam gap is in the prompter-bypass test, where the scene's emptyscene.audioallowlist accidentally masks the regression signal: the assertionaudio.created.toHaveLength(0)passes whether the lifecycle was suppressed (expected) or accidentally ran but the load was rejected by the allowlist (regression).Blocking findings: 1
Finding 1 —
one-off- ID:
F1 - Title: (no title)
- Decision: fix
- Rationale: Addressed by next cycle
- ID:
Proceeding without an over-cap test-quality review cycle per user instruction after fixing the cycle-1 finding and passing local verification (
@keplerops/pulsar@0.1.0 lint /home/atomik/src/pulsar
biome check .Checked 191 files in 196ms. No fixes applied.
@keplerops/pulsar@0.1.0 typecheck /home/atomik/src/pulsar
tsc --noEmit@keplerops/pulsar@0.1.0 test /home/atomik/src/pulsar
vitest runRUN v3.2.4 /home/atomik/src/pulsar
✓ tests/runtime/rng.test.ts (8 tests) 559ms
✓ createSeededRng > yields every draw in the half-open interval [0, 1) 551ms
✓ tests/runtime/timeline.test.ts (98 tests) 382ms
✓ tests/runtime/policy-q004-resource-cleanup.test.ts (145 tests) 304ms
✓ tests/system/helpers.test.ts (20 tests) 492ms
✓ tests/runtime/policy-a008-mode-dispatch.test.ts (80 tests) 396ms
✓ tests/runtime/scrub-cue-gating.test.ts (1 test) 1067ms
✓ PUL-F017 — monotonic-forward audio cue gating > fires a cue on a forward crossing of its master time and suppresses it on a reverse crossing 1066ms
✓ tests/system/chrome-slots.test.ts (10 tests) 330ms
✓ tests/runtime/policy-q008-dom-css-accessibility.test.ts (85 tests) 1040ms
✓ PUL-Q008 — DOM/CSS accessibility (source scan) > runtime tree (current code revision) > contains no Q008 violations acrosssrc/runtime/**/*.ts779ms
✓ tests/runtime/policy-a004-export-pipeline.test.ts (12 tests) 1396ms
✓ PUL-A004 — live runtime independent of export pipeline (source scan) > runtime tree (current code revision) > contains no A004 violations across the runtime-core file set 1362ms
✓ tests/runtime/policy-a003-rendering-libraries.test.ts (15 tests) 1250ms
✓ PUL-A003 — optional rendering libraries are scene-local (source scan) > runtime tree (current code revision) > contains no A003 violations across the runtime-core file set 1178ms
✓ tests/runtime/policy-a006-slide-frameworks.test.ts (14 tests) 1444ms
✓ PUL-A006 — live runtime independent of slide frameworks (source scan) > runtime tree (current code revision) > contains no A006 violations across the runtime-core file set 1390ms
✓ tests/system/chrome-extras.test.ts (5 tests) 310ms
✓ tests/runtime/policy-a010-export-metadata-share.test.ts (117 tests) 1678ms
✓ PUL-A010 — live and export share scene metadata (source scan) > runtime tree (current code revision) > rule 2: zero forbidden parallel scene/composition declarations acrosssrc/**/*.ts1232ms
✓ tests/runtime/navigation.test.ts (111 tests) 163ms
✓ tests/runtime/policy-a009-captions-single-source.test.ts (145 tests) 1714ms
✓ PUL-A009 — captions / prompter single source (source scan) > runtime tree (current code revision) > rule 2: zero forbidden parallel caption-schema declarations acrosssrc/**/*.ts1367ms
✓ tests/runtime/screenshot-determinism-source.test.ts (80 tests) 1976ms
✓ PUL-Q001 — screenshot determinism source scan > runtime tree (current code revision) > contains no non-deterministic primitives acrosssrc/**/*.ts1861ms
✓ tests/runtime/audio.test.ts (114 tests) 107ms
✓ tests/runtime/policy-a002-audio-encapsulation.test.ts (47 tests) 224ms
✓ tests/runtime/policy-q007-remote-code-execution.test.ts (68 tests) 2336ms
✓ PUL-Q007 — no remote code execution (source scan) > runtime tree (current code revision) > contains no Q007 violations acrosssrc/**/*.ts2197ms
✓ tests/runtime/policy-a001-timeline-encapsulation.test.ts (27 tests) 140ms
✓ tests/system/presenter-bridge.test.ts (10 tests) 126ms
✓ tests/runtime/composition.test.ts (116 tests) 33ms
✓ tests/runtime/policy-q003-url-state-determinism.test.ts (130 tests) 2322ms
✓ PUL-Q003 — URL state determinism (source scan) > runtime tree (current code revision) > contains no Q003 violations acrosssrc/**/*.ts2093ms
✓ tests/runtime/audio-engine.test.ts (15 tests) 98ms
✓ tests/runtime/scene-loader-present.test.ts (47 tests) 54ms
✓ tests/runtime/policy-a005-declarative-composition.test.ts (38 tests) 87ms
✓ tests/runtime/scene-loader.test.ts (29 tests) 49ms
✓ tests/runtime/scene-loader-audio.test.ts (45 tests) 71ms
✓ tests/runtime/composition-resolver.test.ts (42 tests) 76ms
✓ tests/runtime/scene-loader-screenshot-prompter.test.ts (33 tests) 88ms
✓ tests/runtime/policy-scene-trust-model-doc.test.ts (2 tests) 164ms
✓ tests/runtime/policy-q002-browser-support.test.ts (21 tests) 83ms
✓ tests/runtime/asset-preloader.test.ts (34 tests) 148ms
✓ tests/runtime/scene.test.ts (125 tests) 38ms
✓ tests/runtime/scene-loader-beat-mode.test.ts (28 tests) 39ms
✓ tests/runtime/scene-loader-paused-scrub.test.ts (34 tests) 68ms
✓ tests/runtime/id-registry.test.ts (5 tests) 10ms
✓ tests/system/audio-helpers.test.ts (13 tests) 23ms
✓ tests/system/scrub-controls.test.ts (13 tests) 38ms
✓ tests/runtime/validation.test.ts (52 tests) 38ms
✓ tests/runtime/scene-navigation.test.ts (63 tests) 41ms
✓ tests/runtime/pul-q006-error-context.test.ts (16 tests) 44ms
✓ tests/runtime/scene-loader-standalone-loop.test.ts (26 tests) 59ms
✓ tests/system/standalone-audio-bed.test.ts (3 tests) 21ms
✓ tests/system/presenter-driven.test.ts (11 tests) 39ms
✓ tests/runtime/error.test.ts (16 tests) 10ms
✓ tests/runtime/composition-registry.test.ts (24 tests) 39ms
✓ tests/runtime/scene-loader-chrome.test.ts (19 tests) 37ms
✓ tests/runtime/policy-biome-complexity-gate.test.ts (5 tests) 46ms
✓ tests/runtime/registry.test.ts (35 tests) 23ms
✓ tests/runtime/presenter.test.ts (26 tests) 34ms
✓ tests/scenes/browser-support-fixture.test.ts (19 tests) 47ms
✓ tests/scenes/loop-fixture.test.ts (20 tests) 23ms
✓ tests/scenes/screenshot-rng-fixture.test.ts (8 tests) 16ms
✓ tests/scenes/paused-fixture.test.ts (19 tests) 26ms
✓ tests/runtime/scene-loader-screenshot-seed.test.ts (9 tests) 21ms
✓ tests/system/presenter-keyboard.test.ts (11 tests) 55ms
✓ tests/scenes/dom-css-accessibility-fixture.test.ts (22 tests) 55ms
✓ tests/runtime/version.test.ts (2 tests) 7ms
✓ tests/runtime/prompter.test.ts (12 tests) 26ms
✓ tests/scenes/placeholder.test.ts (15 tests) 28ms
✓ tests/runtime/workbench-chrome.test.ts (22 tests) 29ms
✓ tests/runtime/audio-unlock-dom.test.ts (7 tests) 12ms
✓ tests/system/register-tokens.test.ts (2 tests) 5ms
✓ tests/scenes/scrub-fixture.test.ts (18 tests) 38ms
✓ tests/system/counter.test.ts (12 tests) 11ms
✓ tests/system/transitions.test.ts (13 tests) 20ms
✓ tests/system/presenter-renderers.test.ts (6 tests) 10ms
✓ tests/system/prompter-window-url.test.ts (4 tests) 13ms
✓ tests/system/terminal-audio.test.ts (8 tests) 11ms
✓ tests/system/templates-options.test.ts (20 tests) 37ms
✓ tests/system/templates.test.ts (58 tests) 27ms
✓ tests/system/pulsar-intro-deck.test.ts (7 tests) 9ms
✓ tests/runtime/workbench-graph.test.ts (6 tests) 9msTest Files 74 passed (74)
Tests 2598 passed (2598)
Start at 09:05:50
Duration 7.67s (transform 9.40s, setup 1.27s, collect 44.26s, tests 21.88s, environment 19ms, prepare 11.10s), plus pre-commit).Final report — issue #147 complete
PR: #154
PR #154 is open and green. The Ground Control Sonar watcher could not query REST because SONAR_TOKEN is unset on the MCP host; GitHub reported both SonarCloud checks passing.
Files changed
Added:
changelog.d/147.fixed.md
Modified:
src/runtime/scene-loader.tssrc/runtime/scene-navigation.tstests/runtime/scene-loader-audio.test.ts
Reviews
- codex: Cycle 1 clean; 0 findings.
- test-quality: Cycle 1 reported one test weakness; fixed and locally verified, then user authorized continuing.
- sonarcloud: GitHub SonarCloud checks passed.
Traceability reconciliation
- IMPLEMENTS / TESTS / DOCUMENTS added: 0
- Links updated: 0
- Stale links removed: 0
No in-scope requirement UIDs; no traceability links changed.
Status
- CI: ✅ green
- SonarCloud: ✅ passed
- PR ready for user review and merge.
Review finding
Composition navigation modes that truncate a composition to its head scene rebuild the composition context without carrying
audioBedforward.mode=standaloneintentionally suppresses beds, butmode=loopandmode=scrubare audible modes and should preserve the surrounding composition bed.Evidence
src/runtime/scene-loader.ts:1561applySingleSceneSlice()creates a newcompositionobject withid,manifestSlice,sceneSlice, andstartIndex, but omitsaudioBed.src/runtime/scene-navigation.ts:492truncateToHead()has the same omission for direct bridge callers.src/runtime/scene-loader.ts:1315passes a bed tocreateAudioService()only fromresolved.composition?.audioBed, so once the slice transform drops it the bed cannot play.Impact
A bed-carrying composition addressed with
mode=loopormode=scrubsilently loses its bed. This makes composition playback differ from the declared composition and from present-mode behavior. Direct callers ofloadSceneNavigationTarget()also get a truncated head slice without the bed.Recommended fix
Preserve
audioBedwhen head-only composition slices are rebuilt, and keep the existingbedSuppressed: mode === 'standalone'behavior as the explicit standalone suppression point.Acceptance checks
applySingleSceneSlice()preservesresolved.composition.audioBedwhen rebuilding the head slice.truncateToHead()preservestarget.composition.audioBedfor direct callers.audioBedinmode=loopandmode=scruband prove the bed is constructed/started.mode=standalonesuppresses the bed.