Layout hygiene + Piano mapping mode#2
Open
myfunc wants to merge 2 commits into
Open
Conversation
…g ghosts
Addresses user report: "too many Knob Panel A/B; old windows appear on reload."
Root causes:
- The App.tsx store subscription opened a floating panel for every entry
in panels store on first handshake, double-stacking them over any
panels already in the loaded layout and over any panels closed by user
in a previous session.
- update_panel on an active panel with bank-change cleared the old slot
but did not reactivate on the new slot, so active_panels[pad:A] ended
up null while pad-A panels existed.
- No cleanup for "ghost" panels created through menu clicks and later
closed in dockview (dockview close != backend DELETE /api/panels).
- Template titles drifted from bank on A/B toggle (title "Knob Panel B"
but bank "A").
Backend (core.py, app.py, mapper.py):
- VALID_BANKS dict gates create_panel/update_panel: {'pad':('A','B'),
'knob':('A','B'),'piano':('play','map')}.
- New reconcile_panels(fix_titles: bool) method: clears stale active-
slot refs, assigns unique live panels to empty slots, optionally
rewrites titles that match the template regex while preserving
custom titles. Called automatically after _migrate_panel_presets in
bootstrap(); exposed as POST /api/panels/reconcile so users can
heal existing broken state without restarting.
- update_panel: when the panel being edited was active on the old
bank, the new bank slot is populated (auto-activate on empty; leave
alone if occupied) so no panel becomes a silent orphan.
- save_config (mapper.py) is now atomic: write tempfile -> fsync ->
os.replace(), preventing TOML corruption on concurrent writers.
Frontend (App.tsx, PanelHeader.tsx, useLayoutPersistence.ts):
- LAYOUT_SCHEMA_VERSION 2 -> 3; saveLayout stamps schemaVersion on
writes and loadLayout discards saved layouts without a matching
version. filterOrphanPanelRefs strips panel ids that aren't in the
current store (keeping utility panels properties/log/settings).
- onReady builds a deterministic 2x2 grid (pad A/B top, knob A/B
below, properties+log right). No floating ever.
- Store subscription no longer open-by-default: uses knownPanelIdsRef
+ layoutReadyRef so first handshake doesn't re-spawn panels that
are already part of the layout (or that the user intentionally
closed). A one-shot reconcile pass right after layoutReady adds any
truly-new panels that arrived during the async gap.
- Menu now exposes explicit A/B choices (Add Pad Panel A/B, Add Knob
Panel A/B) instead of a single "Add Pad Panel" that always created
bank A.
- PanelHeader bank switch rewrites title iff the current title matches
the template regex; custom user titles are preserved.
feat(piano): freeform panel with play/map banks + per-preset mapping
Piano is now a first-class freeform panel with two banks:
- play: original sample playback behavior unchanged.
- map: each key in the MPK keyboard range (36..72) can be bound to
an action (keystroke, plugin action, script, app-open, text-type),
dispatched through the same executor as pad actions. Out-of-range
notes fall through to pad dispatch; in-range notes on the active
map panel are swallowed (no audio fallthrough).
Backend (core.py, mapper.py, config.toml):
- VALID_BANKS['piano'] = ('play', 'map'); create_panel/update_panel
accept type='piano'.
- PIANO_MAP_NOTE_MIN=36 / MAX=72, PianoKeyMapping + PianoPreset
dataclasses, AppConfig.piano_presets round-tripped by load_config /
save_config.
- [[piano_presets]] section in config.toml (Default preset shipped
empty; commented example shows the shape).
- Mapper.lookup_piano_key(preset_name, note) -> mapping | None.
- core.handle_piano_note(note, velocity, on): map wins over play, no-op
if neither is active. Called from _handle_event_locked before the
legacy plugin_manager.on_pad_press fallback.
- activate_panel on a piano panel triggers Piano.stop_all() under
core._lock so notes held on play don't get stuck when the user flips
to map mid-press (and vice versa).
API (app.py):
- GET/POST /api/piano/presets, GET/DELETE /api/piano/presets/{name},
PATCH /api/piano/presets/{name}/keys/{note} (validates [36..72],
strict label type check, early-return on no-op patches, GCs empty
entries). All writes hold core._lock across save_config to prevent
interleaved truncate+rewrite.
- /api/piano/note + /off accept bank in body; bank='map' routes
through handle_piano_note, otherwise the existing play flow.
- piano_presets.changed event published on every mutation.
Frontend (types.ts, stores, PanelHeader.tsx, PianoPanel.tsx,
PropertiesPanel.tsx, WebSocketProvider.tsx):
- PanelType gains 'piano'; PianoBank='play'|'map'; ActivePanelKey
union includes 'piano:play'/'piano:map'; PianoKeyMapping +
PianoPreset types.
- useAppStore: pianoPresets + fetchPianoPresets + updatePianoKey
(optimistic PATCH). createPanelRequest default bank derives from
type (piano -> 'play', else 'A'). selectedPianoNote integrated
with Properties selection.
- PanelHeader: bank options are type-driven ([Play][Map] for piano),
preset dropdown pulls from pianoPresets for piano panels.
- PianoPanel split into guard-wrapper + PianoPanelInner (Rules-of-
Hooks: early-return now precedes no hook calls). Play mode
unchanged. Map mode renders the 36..72 range with label overlays,
click-to-select, and dispatches through /api/piano/note bank='map'
(no audio engine path).
- PropertiesPanel: PianoKeyPropertiesView with the same action
picker the pad editor uses — keystroke keys, plugin_action target,
script process, open-app, type-text — persisted via updatePianoKey.
- WebSocketProvider: handles piano_presets.changed + pulls
piano_presets out of the handshake payload so multi-tab stays in
sync.
- Menu: Add Piano Panel (Play) / (Map) entries alongside Pad/Knob
A/B entries.
Tests: 103 -> 119. New suites:
- tests/test_panel_reconcile.py (6): stale slot, empty-slot
assignment, template-only title rewrite, idempotence, healthy
no-op, ambiguous warning.
- tests/test_piano_presets.py (3): TOML load, lookup, missing.
- tests/test_piano_mapping.py (6+): map -> executor, play -> plugin,
map wins when both active, no-active-no-op, out-of-range
fallthrough, map swallows unmapped in-range.
Review round 1 findings addressed before commit:
- Rules-of-Hooks crash in PianoPanel (split into wrapper + inner).
- Non-atomic save_config (tempfile+fsync+os.replace; piano
endpoints hold core._lock across save).
- Piano action picker could only set action type (now full
per-type editor matching pads).
- Handshake race where panels arriving before layoutReady were
never opened (one-shot reconcile after layoutReady).
- Missing piano_presets.changed WS handler.
- LAYOUT_SCHEMA_VERSION bump was dead code (now stamped in saveLayout).
- Stuck notes on bank switch (Piano.stop_all() on activate).
- createPanelRequest default bank='A' invalid for piano.
- PATCH piano key created-then-GC'd empty entries on no-op patches.
- PATCH accepted non-string label silently.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Applying the 4 "must close before merge" findings and 3 easy wins from the PR #1 review. Added in this branch (PR #2) per user request since PR #1 is the parent — GitHub shows cumulative diff. Must-close - `audio_engine._apply_reconfigure` preserved FX params: `FXChain.apply_state(state)` added; the engine snapshots via `get_state()` before rebuilding the chain and restores both on the success path and the rollback path. `_stream_error` is now also cleared on successful rollback so stale errors don't bleed through. - Shutdown deadlock on pending reconfigure: `_handle_command(_CmdShutdown)` now drains the remaining queue, and any `_CmdReconfigure` gets `{ok: False, error: "engine shutting down"}` + `done.set()` before the producer exits. No more 5 s ghost timeouts. - `_pitch_cache` thread safety: introduced `self._pitch_cache_lock` in `PianoPlugin.__init__`, guards every `.get` / `[]=` / `.clear()` access (MIDI thread, HTTP thread, reconfigure thread). - `update_panel` lock asymmetry: full critical section (settings read/write, mapper routing, legacy mirror, snapshot) now runs under `self._lock`. `plugin_manager.on_mode_changed`, `notify_preset_changed`, and `event_bus.publish` still fire outside the lock to avoid reentrancy into plugin code. Easy wins - Removed `.lower()` from preset-name comparisons in `delete_preset` / `rename_preset` — matches the case-sensitive semantics used everywhere else. - `_migrate_panel_presets` now generates panel IDs with `uuid.uuid4().hex[:8]` suffix (consistent with `create_panel`), eliminating the collision risk from `int(time()*1000)+index`. - Added public `Mapper.iter_active_panels()` returning a defensive copy; `core.py` no longer reaches into `self.mapper._active_panels` / `_panel_presets` directly. Not in this commit (intentional) - Package-relative imports in `plugins/piano/*`, pre-allocated scratch in `Pitch`/`Reverb`, `threading.Event`-signaled producer loop — bigger refactors, tracked as follow-ups in PR #1. - Frontend `.replace('#', '#')` concern — current code already renders bare `{key.name}`; no change needed. Tests (+5, 119 -> 124, all green) - `test_reconfigure_preserves_fx_state`: sets non-default reverb/volume params, reconfigures to 48 kHz, asserts params preserved. - `test_shutdown_acks_pending_reconfigure`: two queued reconfigures during shutdown both get `ok=False, error='engine shutting down'`, no 5 s hang. - `test_pitch_cache_thread_safe`: 4 threads interleave put/get/clear, no `RuntimeError`. - `test_update_panel_holds_lock_for_full_critical_section`: lock-acquire counter around `settings.put` asserts zero out-of-lock writes during `update_panel(bank=, preset=)`. - `test_mapper_iter_active_panels_is_copy`: mutating the returned dict does not leak into live state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4 tasks
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.
Stacked on top of #1 (
feat/freeform-panels-audio-engine).Two halves
1. Layout hygiene (user report)
Root causes: store subscription opened a floating panel for every entry on handshake → stacked on top of the loaded layout;
update_panelcleared old slot without repopulating the new one on bank change; no cleanup for "ghost" panels left by menu clicks that were then closed in dockview. Layout schema bump was dead code.Fixes: idempotent
reconcile_panels()+POST /api/panels/reconcile; deterministic 2x2 grid (no floating);knownPanelIdsRef+layoutReadyRef+ one-shot reconcile; explicitAdd Pad/Knob Panel A/Bmenu; PanelHeader auto-syncs template titles; atomicsave_config;LAYOUT_SCHEMA_VERSION 2 -> 3stamped insaveLayout.2. Piano as freeform panel with play/map banks
Same UX as pad/knob panels — PanelHeader with
[Play] [Map]bank selector, preset dropdown, engineering LED activate button.Play mode — unchanged sample playback. Map mode — keys 36..72 bind to actions (keystroke, plugin action, script, app-open, text-type), dispatched through the same executor pad actions use. Map wins over play; out-of-range falls through to pad dispatch; in-range unmapped on map is swallowed.
activate_panelon a piano panel callsPiano.stop_all()so held notes don't get stuck on bank flip.Config:
[[piano_presets]]inconfig.toml, shape mirrors[[pad_presets]].PR #1 review feedback
Review on PR #1 flagged 4 must-close items and several easy wins. Addressing them in this PR rather than opening a third, per user request — GitHub shows the cumulative diff.
Must-close
_apply_reconfigurepreserves FX params via newFXChain.get_state()/apply_state()(snapshot on success + rollback)._CmdShutdowndrains the queue and ACKs pending_CmdReconfigurewith{ok:False, error:"engine shutting down"}— no more 5 s ghost timeouts._pitch_cache_lockguards every access (MIDI / HTTP / reconfigure threads).update_panelruns the whole critical section underself._lock;on_mode_changed/notify_preset_changed/event_bus.publishremain outside.Easy wins
delete_preset/rename_presetis now strict (matches the rest of the codebase)._migrate_panel_presetsusesuuid.uuid4().hex[:8]suffixes (consistent withcreate_panel).Mapper.iter_active_panels()public accessor;core.pyno longer touches_active_panels/_panel_presets.Deferred to follow-ups (bigger refactors)
plugins/piano/*.Pitch/Reverballpass.threading.Event-signaled producer loop instead of 1 ms busy-sleep.Decisions baked in
config.toml[[piano_presets]].LAYOUT_SCHEMA_VERSIONbumpTest plan
pytest— 124/124 passing (103 -> 124). Suites:test_panel_reconcile.py(6),test_piano_presets.py(3),test_piano_mapping.py(6),test_audio_engine.py(+3 new),test_core_panels.py(+1 new),test_mapper.py(+1 new).npm run build— tsc clean, vite OK.from backend.app import create_app— ok (mandatory after the IndentationError we shipped last round).save_config, full action editor for piano keys, handshake race, WSpiano_presets.changedhandler, schema stamp, stuck notes on bank switch, createPanelRequest default, no-op PATCH waste, strict label type.Stacked PR notes
Targets
feat/freeform-panels-audio-engine(PR #1). When #1 merges, GitHub retargets this tofeature/web-uiautomatically.🤖 Generated with Claude Code