feat(tool): a beacon sound the Mission Maker chooses (FEAT-CUSTOM-BEACON-SOUNDS) - #112
Merged
Conversation
Reviewer's GuideImplements custom beacon sound support end-to-end: schema metadata for a sound picker and hidden labels, backend session support for storing sound bytes and reading them from missions, validation and install logic to ensure custom sounds are present and written under reserved names, new REST APIs and Svelte UI components for choosing/resetting sounds, plus documentation and tests. Sequence diagram for reopening a mission to recover custom beacon soundssequenceDiagram
actor MissionMaker
participant WebBackend as WebAppBackend
participant Session
participant Install as InstallModule
MissionMaker->>WebBackend: GET /api/dialog/miz (pick_miz)
WebBackend->>Dialogs: pick_miz()
Dialogs-->>WebBackend: miz_path
MissionMaker->>WebBackend: POST /api/load_path (loadPath)
WebBackend->>Session: load_path(miz_path)
Session->>Install: read_config(miz_path)
Install-->>Session: FoundConfig(catalog)
Session->>Session: _catalog = FoundConfig.catalog
Session->>Install: read_sounds_from_miz(miz_path, _catalog)
Install-->>Session: {setting: bytes} for customised sounds
Session->>Session: _sounds = read_sounds_from_miz(...)
MissionMaker->>WebBackend: GET /api/sounds (get_sounds)
WebBackend->>Session: session.catalog
WebBackend->>WebBackend: _sound_state()
WebBackend->>Session: session.sound(setting)
WebBackend->>WebBackend: _sounds_available(session.mission_path)
WebBackend-->>MissionMaker: {sounds: SoundState[]}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
davidp57
added a commit
that referenced
this pull request
Aug 9, 2026
Set in the lot's own PR, per the convention — not left for a post-merge commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
davidp57
force-pushed
the
feature/custom-beacon-sounds
branch
from
August 9, 2026 08:57
ed6e2f6 to
56cdba9
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The validation error for missing custom sounds (
validate.sound.missing) always reports the reserved in-mission filename; consider including the original disk name from the label when available so the Mission Maker can more easily identify which file to reselect. - Both
_sounds_availableand_target_soundsreopen and scan the target.mizindependently during inject; you could refactor to share a single zip read per request to avoid duplicated I/O on large missions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The validation error for missing custom sounds (`validate.sound.missing`) always reports the reserved in-mission filename; consider including the original disk name from the label when available so the Mission Maker can more easily identify which file to reselect.
- Both `_sounds_available` and `_target_sounds` reopen and scan the target `.miz` independently during inject; you could refactor to share a single zip read per request to avoid duplicated I/O on large missions.
## Individual Comments
### Comment 1
<location path="tools/ctld-tools/web/src/lib/model.ts" line_range="71-72" />
<code_context>
+ * `schema` is optional only so old call sites keep working; pass it, or search will offer the
+ * hidden tool-written keys that `classify` deliberately leaves out of the families.
+ */
+export function settingKeys(snap: Snapshot, schema?: SchemaInfo): string[] {
+ return snap.keys.filter((k) => !isStructured(snap.values[k]) && !schema?.keys[k]?.hidden)
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Making `schema` optional for `settingKeys` can lead to inconsistencies with family classification
Because hidden-key filtering depends on `schema`, omitting it means search will surface hidden tool-maintained keys that `classify` intentionally excludes, reintroducing the inconsistency you just fixed. To avoid this, consider making `schema` required and updating call sites, or at least adding a dev-time assertion that there are no hidden keys when `schema` is not provided.
Suggested implementation:
```typescript
export function settingKeys(snap: Snapshot, schema?: SchemaInfo): string[] {
if (!schema) {
// Dev-time assertion: callers should always pass schema so hidden tool-maintained keys
// stay consistent with family classification.
if (typeof process !== 'undefined' && process.env && process.env.NODE_ENV !== 'production') {
throw new Error(
'settingKeys: schema is required to avoid exposing hidden tool-maintained keys. ' +
'Update call sites to pass SchemaInfo.',
)
}
// In production, fall back to the legacy behavior (no hidden-key filtering) to avoid
// breaking older call sites, at the cost of possible inconsistency.
return snap.keys.filter((k) => !isStructured(snap.values[k]))
}
return snap.keys.filter(
(k) => !isStructured(snap.values[k]) && !schema.keys[k]?.hidden,
)
}
```
If your tooling allows it, you may also want to:
1. Tighten the TypeScript signature to `schema: SchemaInfo` once all call sites are updated and the dev-time error stops firing.
2. Optionally, add a TODO above the function indicating that the optional `schema` parameter is temporary and will be made required after legacy call sites are migrated.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+71
to
+72
| export function settingKeys(snap: Snapshot, schema?: SchemaInfo): string[] { | ||
| return snap.keys.filter((k) => !isStructured(snap.values[k]) && !schema?.keys[k]?.hidden) |
There was a problem hiding this comment.
suggestion (bug_risk): Making schema optional for settingKeys can lead to inconsistencies with family classification
Because hidden-key filtering depends on schema, omitting it means search will surface hidden tool-maintained keys that classify intentionally excludes, reintroducing the inconsistency you just fixed. To avoid this, consider making schema required and updating call sites, or at least adding a dev-time assertion that there are no hidden keys when schema is not provided.
Suggested implementation:
export function settingKeys(snap: Snapshot, schema?: SchemaInfo): string[] {
if (!schema) {
// Dev-time assertion: callers should always pass schema so hidden tool-maintained keys
// stay consistent with family classification.
if (typeof process !== 'undefined' && process.env && process.env.NODE_ENV !== 'production') {
throw new Error(
'settingKeys: schema is required to avoid exposing hidden tool-maintained keys. ' +
'Update call sites to pass SchemaInfo.',
)
}
// In production, fall back to the legacy behavior (no hidden-key filtering) to avoid
// breaking older call sites, at the cost of possible inconsistency.
return snap.keys.filter((k) => !isStructured(snap.values[k]))
}
return snap.keys.filter(
(k) => !isStructured(snap.values[k]) && !schema.keys[k]?.hidden,
)
}If your tooling allows it, you may also want to:
- Tighten the TypeScript signature to
schema: SchemaInfoonce all call sites are updated and the dev-time error stops firing. - Optionally, add a TODO above the function indicating that the optional
schemaparameter is temporary and will be made required after legacy call sites are migrated.
Carried here rather than into a one-line PR of its own, per Zip. The convention is to set the index line inside the lot's own PR; #110 shipped without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…CON-SOUNDS) radioSound and radioSoundFC3 were text boxes: typing a name changed what the engine plays without putting any such file in the mission, so using your own beacon tone meant editing the .miz by hand — the manual step FEAT-ONE-CLICK-INSTALL exists to remove. Each now gets a Default / Custom picker. A chosen .ogg is read at selection, kept in the session, and written into the archive with its resource key and preload trigger like the bundled ones. The model, per ADR 0012: a chosen file enters the mission under a reserved name (CTLD_beacon_custom.ogg), so the configuration value itself says the sound is customised — no second key that could contradict the engine, and no misreading of a Mission Maker whose own file is called beacon.ogg. The name it had on disk survives as a schema-only label, never catalogued: a catalogue entry would be a parameter under ADR 0011 Addendum 1, so completeness would demand it and every pre-lot configuration would report a missing setting at mission start (FIX-TOOL-I18N-LANG's wall). Reading the bytes at selection rather than at install is what makes an installed mission reconfigurable: reopening the .miz recovers the sound, so it reinstalls on another machine with the original file deleted. A .yaml cannot carry a binary, so reopening one blocks the install with a validation error naming the file to pick again — unless the target mission already holds it. An OggS signature check catches the renamed .mp3 that would give silent beacons discovered in flight; no size cap, the size is reported instead. The picker is bound to `editor: sound` in the schema, never to a setting name in a component (FEAT-EDITOR-COVERAGE), and `hidden: true` keeps the labels out of the families and out of search. Typing a file name by hand still works for a sound added through the Mission Editor. Documented EN + FR, in the tool guide and the configuration reference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Set in the lot's own PR, per the convention — not left for a post-merge commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
davidp57
force-pushed
the
feature/custom-beacon-sounds
branch
from
August 9, 2026 09:00
56cdba9 to
c2f1d8f
Compare
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.
Why
radioSound/radioSoundFC3were free-text boxes. Typing a name changed what the engine plays without putting any such file in the mission, so using your own beacon tone meant editing the.mizby hand — the manual stepFEAT-ONE-CLICK-INSTALLexists to remove. Zip's verdict on the text box ("ça n'a aucun sens") was fair.Each setting now gets a Default / Custom picker. A chosen
.oggis read at selection, held in the session, and written into the archive with its resource key and preload trigger exactly like the bundled ones.The model (ADR 0012, grilled before any code)
CTLD_beacon_custom.ogg), so the configuration value itself says the sound is customised. No second key that could contradict the engine — and no misreading of the ordinary case of a Mission Maker whose own file is calledbeacon.ogg, which "differs from the default" would silently overwrite.radioSoundOriginalName). Catalogued it would be a parameter under ADR 0011 Addendum 1, so completeness would demand it and every pre-lot configuration would report a missing setting at mission start —FIX-TOOL-I18N-LANG's wall, paid once already..mizrecovers the sound, so it reinstalls on another machine, with the file the Mission Maker chose long deleted. A path would rot; a.mizhas no path to offer at all..yamlcannot carry a binary, so reopening one blocks the install with a validation error naming the file to pick again — unless the target mission already holds it (a reinstall, or a file added through the Mission Editor).OggSis checked; a renamed.mp3would give silent beacons discovered in flight. No size cap — the size is reported instead..ogguntil the Mission Editor drops it. Deliberate.Tests
tests/test_sounds.py— 13 tests, the load-bearing one being the full round trip: install with a custom sound →unlink()the source file →session.reset()→ reopen the.miz→ the bytes and the label come back → reinstall into a different mission and assert the file lands byte for byte.SoundPicker.test.ts— 7 component tests: both sources offered, the original name shown (not the reserved one), the warning when the file is no longer held, cancellation, and a backend refusal surfaced rather than swallowed.278 Python tests, 128 frontend tests, ruff + mypy clean.
UI binding
editor: soundin the schema drives the picker — no setting name in a component (FEAT-EDITOR-COVERAGEbanned that).hidden: truekeeps the labels out of the families and out of search, so nobody hand-edits a label into disagreeing with the sound it describes.Typing a file name by hand is unchanged and still supported. Documented EN + FR in the tool guide and the configuration reference.
🤖 Generated with Claude Code
Summary by Sourcery
Add support for mission-maker-selected beacon sound files, with backend, validation, install, and UI changes to treat them as first-class, reconfigurable resources.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: