feat(player): AHX/THX Sound pane + download polish (Chunk β)#38
Merged
Conversation
…ownload extension
Two threads from Chunk β of add-ahx-playback, tightly coupled through
Player.tsx so committed together:
1. Per-control Sound pane gating (D9 amendment, post-Phase-0 spike).
The Sound pane's two controls now gate independently by the
AudioPlayer facade's activeEngine:
- Amiga emulation toggle: enabled only for libopenmpt.
- Stereo separation slider: enabled for libopenmpt AND ahx
(ahx2play accepts stereo separation at the same 0..100 scale);
disabled for tfmx only.
The all-disabled banner ("Sound settings only affect classic MOD
files") now appears only when both controls are disabled, i.e.
when TFMX is active. For AHX the slider is live, so the banner
would be misleading.
Implementation:
- SoundPane reads a new `activeEngine?: EngineKind` prop.
- SourceDrawer's SoundProps mirrors the new prop.
- Player.tsx tracks activeEngine in React state, set
synchronously after each player.play() call so the gating
updates immediately rather than waiting for the new worklet's
meta postback.
- Player.tsx's `m`-key handler migrates from
`metaData.type.toLowerCase() !== "mod"` to the
activeEngine-based predicate. The two sites must move
together — otherwise the shortcut and the UI disagree.
2. Mod Archive download extension preserved for AHX/THX.
downloadTrack and downloadFavoriteMods both fetch a Blob from
api.modarchive.org/downloads.php. Both used to write `<title>.mod`
regardless of the underlying format — AHX/THX files downloaded
from Mod Archive would mislabel as `.mod`, and a user sharing
such a file would propagate the wrong extension to recipients.
New helper `sniffDownloadExtension(blob)` reads the first 4 bytes
and returns `.ahx` for AHX/THX content (per the same 4-byte gate
as `looksLikeAhx` — 3-letter prefix + version byte ∈ {0x00, 0x01}),
else null. Both download paths fall back to `.mod` on null,
preserving today's behaviour for all libopenmpt formats.
This deviates from D13's "use ModItem.filename" decision in favour
of byte sniffing because the filename isn't always available at
download time (random walks, permalink loads, and favorites all
bypass the chart context). Byte sniffing is universal, costs ~4
bytes per download, and keeps the FavoriteTrack shape unchanged
(no storage migration). The bytes are the canonical source of
truth anyway — a `.mod`-named file with AHX magic is still AHX.
make verify green.
Assisted-by: ClaudeCode:claude-opus-4-7
…overage
Moves the magic-byte sniff from lib/audio-player.ts into a tiny pure-
utility module so it can be unit-tested in isolation. audio-player.ts
depends on the ChiptuneJsPlayer ambient global (registered via the
public/chiptune3.js script tag) and can't be imported in a Node test
runtime.
The 24 new test cases mirror the spec scenarios in
openspec/changes/add-ahx-playback/specs/ahx-playback/spec.md:
- Canonical AHX magic (AHX\0, AHX\1)
- Legacy THX magic (THX\0, THX\1) — the bulk of modarchive's `.ahx`
corpus carries this magic, validating D4's dual-prefix design
- Mismatched version bytes (0x02, 0xFF, 0x10, 0x20) fall through
- Unrelated prefixes (IT IMPM, XM "Exte", MED MMD0, HTML "<!DO")
fall through — the WAF/HTML scenario from design.md's false-
positive analysis
- Undersized buffers (0, 3 bytes) return false; exactly 4 bytes
matching AHX v1 is the inclusive boundary
- Non-ArrayBuffer inputs (Uint8Array view, plain object,
TfmxPair-shaped, null/undefined/string/number) all return false
Tests run via `npm test` (vitest). Not gated by `make verify` today
but covered by `npm run test:ci` in the CI workflow.
Assisted-by: ClaudeCode:claude-opus-4-7
… hint, npm test gate Three fixes from the adversarial review of PR #38 (Chunk β). 1. THX-magic files download as `.thx`, not `.ahx` (A2). sniffDownloadExtension now distinguishes the two prefixes: AHX-magic files return ".ahx", THX-magic files return ".thx". Honours the spec scenario "Legacy THX modarchive track downloads as `.thx`" — the bytes ARE the upstream identity, so a THX-magic file round-trips with the legacy extension that matches what the user would see on Modarchive. The format-engine routing is unchanged (both still play via ahx2play through looksLikeAhx); the distinction is purely cosmetic round-trip fidelity. 2. AHX-active Amiga-toggle hint in the Sound pane (B5). When an AHX file is playing only the Amiga emulation toggle is disabled (the stereo separation slider is live per the D9 matrix), so the all-disabled "Sound settings only affect classic MOD files" banner doesn't fire. Without an explanation the greyed-out toggle was silent — users had no hint why it wouldn't respond. New one-line note next to the Amiga heading when activeEngine === "ahx": "Amiga emulation has no effect for AHX tracks — they render through ahx2play's built-in Paula model." 3. `npm test` added to `make verify` (E1). The previous Makefile target ran lint + typecheck + audit + build and explicitly omitted `npm test`. The 24 new unit tests for looksLikeAhx (and the 9 pre-existing resolveSafe tests) would not block a CI regression. Now `make verify` runs `npm test` between typecheck and audit. Pre-existing gap, but added in this PR because the same PR adds 145 lines of test scaffold that depend on something CI never invoked. The remaining review findings (sticky activeEngine after stop, m-key useEffect deps closure-staleness, banner string hardcoded "TFMX", test label "AHX v2" ambiguity, HVL silent-reject) are recorded in _bmad-output/implementation-artifacts/deferred-work.md for joint follow-up. make verify green (now including npm test — 33/33 tests pass). Assisted-by: ClaudeCode:claude-opus-4-7
The Local tab's drop-zone label read "Drop MOD or TFMX files here" and listed only MOD-family + TFMX extensions. AHX/THX support landed in Chunk α and accepts .ahx and .thx files, but the hint text wasn't updated — users dropping AHX files had no visual confirmation the catalogue would accept them. Updated to "Drop MOD, AHX, or TFMX files here" with the extension hint extended to include `.ahx .thx` between the MOD-family list and the TFMX pair patterns. The file input itself has no `accept=` filter (it uses webkitdirectory for folder selection), so the OS-level picker already accepts AHX files — only the UI hint was stale. Assisted-by: ClaudeCode:claude-opus-4-7
indigo423
added a commit
that referenced
this pull request
May 17, 2026
… hint, npm test gate Three fixes from the adversarial review of PR #38 (Chunk β). 1. THX-magic files download as `.thx`, not `.ahx` (A2). sniffDownloadExtension now distinguishes the two prefixes: AHX-magic files return ".ahx", THX-magic files return ".thx". Honours the spec scenario "Legacy THX modarchive track downloads as `.thx`" — the bytes ARE the upstream identity, so a THX-magic file round-trips with the legacy extension that matches what the user would see on Modarchive. The format-engine routing is unchanged (both still play via ahx2play through looksLikeAhx); the distinction is purely cosmetic round-trip fidelity. 2. AHX-active Amiga-toggle hint in the Sound pane (B5). When an AHX file is playing only the Amiga emulation toggle is disabled (the stereo separation slider is live per the D9 matrix), so the all-disabled "Sound settings only affect classic MOD files" banner doesn't fire. Without an explanation the greyed-out toggle was silent — users had no hint why it wouldn't respond. New one-line note next to the Amiga heading when activeEngine === "ahx": "Amiga emulation has no effect for AHX tracks — they render through ahx2play's built-in Paula model." 3. `npm test` added to `make verify` (E1). The previous Makefile target ran lint + typecheck + audit + build and explicitly omitted `npm test`. The 24 new unit tests for looksLikeAhx (and the 9 pre-existing resolveSafe tests) would not block a CI regression. Now `make verify` runs `npm test` between typecheck and audit. Pre-existing gap, but added in this PR because the same PR adds 145 lines of test scaffold that depend on something CI never invoked. The remaining review findings (sticky activeEngine after stop, m-key useEffect deps closure-staleness, banner string hardcoded "TFMX", test label "AHX v2" ambiguity, HVL silent-reject) are recorded in _bmad-output/implementation-artifacts/deferred-work.md for joint follow-up. make verify green (now including npm test — 33/33 tests pass). Assisted-by: ClaudeCode:claude-opus-4-7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Chunk β of the add-ahx-playback OpenSpec change. Follows
#37 (Chunk α — engine
Summary
Two threads, both flowing from the Phase 0 spike's findings.
1. Per-control Sound pane gating (D9 amendment)
The Sound pane's two controls now gate independently per the design.md
D9 matrix:
The "Sound settings only affect classic MOD files" banner now only
shows when ALL controls are disabled — i.e. TFMX is active. For AHX
the slider is live, so the all-disabled banner would mislead.
Two string-equality sites migrated together to the facade's new
activeEngine: EngineKindgetter:components/SoundPane.tsx— gating predicate.components/Player.tsx—m-key keyboard handler for Amiga emulation.Plus a new
activeEngineReact state inPlayer.tsx, setsynchronously after each
player.play()call so the Sound panere-renders immediately on engine switch (without waiting for the new
worklet's meta postback).
2. Mod Archive download extension preserved
downloadTrackanddownloadFavoriteModsno longer hardcode.modfor Mod Archive downloads. Both now sniff the first 4 bytes of the
fetched Blob and pick
.ahxfor AHX/THX content, falling back to.modotherwise. The shared sniff helper mirrorslooksLikeAhx'sgate (3-letter prefix + version byte ∈ {0x00, 0x01}).
This deviates from D13's "use ModItem.filename" decision in favour
of byte sniffing because the filename isn't always available at
download time (random walks, permalink loads, and favorites all bypass
the chart context). Byte sniffing is universal, costs ~4 bytes per
download, and keeps the
FavoriteTracklocalStorage shape unchanged(no migration needed).
3. Refactor:
looksLikeAhxextracted + unit testedlib/audio-player.tsdepends on the ambientChiptuneJsPlayerglobal(registered via
public/chiptune3.jsscript tag) and can't beimported in a Node test runtime. The 4-byte sniff lived inside that
file in Chunk α; moved to a pure-utility module at
lib/ahx-magic.tsso vitest can exercise it.
24 new test cases cover the spec scenarios: canonical AHX magic,
legacy THX magic (the bulk of modarchive's
.ahxcorpus), mismatchedversion bytes, unrelated prefixes (XM, IT, MED, HTML/WAF response),
undersized buffers, and non-ArrayBuffer inputs. 33 / 33 tests pass.
Test plan
CI:
make verifygreen locally (lint + typecheck + audit + build)npm testgreen locally (33/33 — 9 existing resolveSafe + 24 new looksLikeAhx)Manual smoke (pending human — needs browser):
slider stays enabled at 30%, AHX renders with stereo separation applied.
render block.
stereo slider enabled, NO "Sound settings only affect classic MOD
files" banner.
mkey during AHX: toast "Amiga emulation only affectsMOD tracks", no cycle. During MOD: cycles A1200 → A500 → Off.
.ahx, not.mod. Regression check: MOD downloads still.mod.that entry inside the zip ends in
.ahx. MOD entries still.mod.What's NOT in this PR
Chunk γ (final follow-up):
modarchive papercut fix.
package.jsonbump to0.5.0.The deferred-work items from the Chunk α code review (cross-engine
handshake gap, ackslot reuse, worklet-Set leak, generation counter
lockstep) remain pre-existing TFMX behaviour and are recorded for
joint follow-up — out of scope for this PR.
Refs
openspec/changes/add-ahx-playback/(local artifact;openspec/is gitignored by convention)design.mdD9, D12, D13, D14 + Phase 0 Resolved memospecs/ahx-playback/spec.mdrequirements"Sound pane gating is per-control during AHX playback",
"AHX worklet reports a stable meta.type value",
"Mod Archive AHX downloads preserve the upstream extension",
"AudioPlayer.load(url) is libopenmpt-only by design".