feat(#325): Phase 3 — config, capture toggles, action/gesture events#372
feat(#325): Phase 3 — config, capture toggles, action/gesture events#372amiable-dev merged 6 commits intomainfrom
Conversation
…filters Config (conductor-core): - New [event_console] TOML section: buffer_size (R925), max_events_per_second (R924), capture_midi/capture_processed/ capture_actions toggles (R926-R928) - Named filters: [event_console.filters.<name>] presets with event_type, channel, note_min, note_max, device_id (R911-R913) - EventConsoleConfig + NamedEventFilter structs Daemon: - Buffer size reads from config, falls back to 1000 default - event_monitor_max field for runtime buffer cap CLI: - --filter/-F loads named filter from config (R914) - Named filter merges with CLI flags (CLI takes precedence) - load_config_for_filter() reads ~/.config/conductor/config.toml Tests: 3 new config tests + all existing tests updated
Action events (R897-R898): - emit_action_event() pushes mode_change, action_executed, action_error events to monitor buffer on dispatch result - Errors include error message in device_id field for console display Processed events (R893): - emit_processed_event() maps all ProcessedEvent variants to monitor: pad_pressed, pad_released, short_press, medium_press, long_press, hold_detected, double_tap, chord_detected, encoder_turn, cc_received, aftertouch, pitch_bend - LongPress now visible in Event Console for duration debugging - Raw passthrough events skipped (no useful display)
Frontend: - getEventColor covers pad_pressed/released, cc_received, encoder_turn, short_press, medium_press, long_press, hold_detected, double_tap, chord_detected with semantic coloring (R893, R901-904) - Green=quick, yellow=medium, red=long, pink=hold, purple=double, cyan=chord for intuitive gesture debugging Tests: +1 new test group (gesture events), expanded daemon type tests Frontend: 365 tests passing
… field Council findings addressed (score 7/10 → fixes): 1. Wire capture toggles: capture_processed and capture_actions now actually gate emit calls (were config-only, not enforced) 2. Fix device_id misuse: added MonitorEvent.detail field for action messages instead of stuffing them into device_id (R897-R898) 3. Extract push_monitor_event() DRY helper — consolidates buffer push logic from 4 sites into one method 4. Clamp buffer_size to min 1 to prevent 0-size edge case Tests: - test_monitor_event_detail_field: verifies detail serialization - test_monitor_event_detail_skipped_when_none: skip_serializing_if - test_buffer_size_min_clamp: edge case verification
… while loop Council findings addressed: 1. emit_processed_event now populates detail field with duration, interval, chord notes, encoder direction — no more data loss 2. Multi-device path now emits processed events with device_id 3. push_monitor_event uses while loop (future-proof for buffer resize) 4. CC/encoder values use proper cc/value fields instead of note/velocity 5. Document capture toggles require restart (not runtime-reloadable) 6. PadReleased includes hold duration in detail
There was a problem hiding this comment.
Pull request overview
Implements Phase 3 of Issue #325 by expanding the event console pipeline to be config-driven, adding richer processed/action event emission in the daemon, and updating the GUI color mapping/tests for new gesture and processed event types.
Changes:
- Added
[event_console]configuration (buffer sizing, capture toggles, named filters) toconductor-coreconfig types/loader. - Extended daemon monitoring with a DRY buffer push helper, new
MonitorEvent.detail, plus processed/action event emission. - Updated GUI event color mapping and tests to recognize additional processed/gesture event types.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| conductor-gui/ui/src/lib/utils/midi-helpers.ts | Extends getEventColor mappings for new processed/gesture event types. |
| conductor-gui/ui/src/lib/components/LiveEventConsole.test.ts | Adds test coverage for new color mappings. |
| conductor-daemon/src/daemon/types.rs | Adds detail field to MonitorEvent for richer monitoring payloads. |
| conductor-daemon/src/daemon/mcp_tools.rs | Updates test config construction to include event_console. |
| conductor-daemon/src/daemon/mcp.rs | Updates test config construction to include event_console. |
| conductor-daemon/src/daemon/llm/plan.rs | Updates test config construction to include event_console. |
| conductor-daemon/src/daemon/llm/history.rs | Updates test config construction to include event_console. |
| conductor-daemon/src/daemon/llm/executor.rs | Updates test config construction to include event_console. |
| conductor-daemon/src/daemon/engine_manager.rs | Makes monitor buffer size configurable, adds capture toggles, and emits processed/action monitor events via a shared push helper. |
| conductor-daemon/src/bin/conductorctl.rs | Adds --filter/-F named filter support and config loading helper for filter lookup. |
| conductor-core/tests/proptest_multidevice.rs | Updates test config construction to include event_console. |
| conductor-core/tests/multi_device_rule_routing_test.rs | Updates test config construction to include event_console. |
| conductor-core/tests/mode_management_integration_test.rs | Updates test config construction to include event_console. |
| conductor-core/tests/mapping_device_filter_test.rs | Updates test config construction to include event_console. |
| conductor-core/tests/e2e_multidevice_test.rs | Updates test config construction to include event_console. |
| conductor-core/tests/config_validation_test.rs | Updates test config construction to include event_console. |
| conductor-core/src/config/types.rs | Introduces EventConsoleConfig + NamedEventFilter and wires it into the top-level Config. |
| conductor-core/src/config/loader.rs | Extends Config::default() to set event_console: None for backward compatibility. |
| case 'note_off': | ||
| case 'pad_released': return '#6b7280'; | ||
| case 'cc': | ||
| case 'cc_received': |
There was a problem hiding this comment.
getEventColor no longer handles the raw daemon event type encoder, but the daemon still emits event_type: "encoder" for InputEvent::EncoderTurned (see create_monitor_event). This will fall through to the default grey color and makes encoder events inconsistent with the rest of the console. Consider adding encoder as an alias (and keeping the existing encoder_turn mapping for processed events).
| case 'cc_received': | |
| case 'cc_received': | |
| case 'encoder': |
There was a problem hiding this comment.
Already handled — encoder case exists at line 44 alongside encoder_turn (added in commit 0aad339).
|
|
||
| // Event monitor capture (Issue #326) — behind atomic bool, zero cost when disabled | ||
| // Uses try_lock(): if IPC is draining events, skip capture (best-effort monitoring) | ||
| if self.event_monitor_active.load(Ordering::Relaxed) |
There was a problem hiding this comment.
The raw event-monitor capture path isn’t gated by the new [event_console] capture_midi toggle. As written, enabling the monitor will still push note_on/note_off/cc/encoder/... events even when capture_midi = false in config. To make R926 effective, plumb capture_midi into EngineManager (similar to capture_processed / capture_actions) and include it in this if guard.
| if self.event_monitor_active.load(Ordering::Relaxed) | |
| if self.event_monitor_active.load(Ordering::Relaxed) | |
| && self.event_monitor_capture_midi |
There was a problem hiding this comment.
Fixed in commit 2ce30c7 — capture_midi is now gated on both single-device (line 2762) and multi-device (line 2957) raw event capture paths.
|
|
||
| // Event monitor capture with device_id (Issue #326) | ||
| // Uses try_lock(): if IPC is draining events, skip capture (best-effort monitoring) | ||
| if self.event_monitor_active.load(Ordering::Relaxed) |
There was a problem hiding this comment.
Same issue in the multi-device path: raw monitor capture with device_id isn’t gated by [event_console] capture_midi, so users can’t disable raw MIDI capture in multi-device mode either. Add the capture_midi check here as well once it’s stored on EngineManager.
| if self.event_monitor_active.load(Ordering::Relaxed) | |
| if self.event_monitor_active.load(Ordering::Relaxed) | |
| && self.capture_midi.load(Ordering::Relaxed) |
There was a problem hiding this comment.
Fixed alongside the single-device path in commit 2ce30c7 — see line 2957.
conductor-core/src/config/types.rs
Outdated
| /// Event console configuration (Issue #325 — R924, R925, R926-R928) | ||
| /// | ||
| /// Controls event monitoring buffer, rate limiting, and capture toggles. | ||
| #[derive(Debug, Clone, Deserialize, Serialize)] | ||
| pub struct EventConsoleConfig { | ||
| /// Maximum events per second displayed in follow mode (0 = unlimited, R924) | ||
| #[serde(default)] | ||
| pub max_events_per_second: u32, |
There was a problem hiding this comment.
max_events_per_second is introduced as a config knob but is currently unused across the repo (no daemon/CLI/GUI code reads or enforces it). Either wire it into the follow-mode display rate limiting (R924) or remove/rename it until it has an effect, to avoid a confusing no-op config setting.
| /// Event console configuration (Issue #325 — R924, R925, R926-R928) | |
| /// | |
| /// Controls event monitoring buffer, rate limiting, and capture toggles. | |
| #[derive(Debug, Clone, Deserialize, Serialize)] | |
| pub struct EventConsoleConfig { | |
| /// Maximum events per second displayed in follow mode (0 = unlimited, R924) | |
| #[serde(default)] | |
| pub max_events_per_second: u32, | |
| /// Event console configuration (Issue #325 — R925, R926-R928) | |
| /// | |
| /// Controls event monitoring buffer and capture toggles. | |
| #[derive(Debug, Clone, Deserialize, Serialize)] | |
| pub struct EventConsoleConfig { |
There was a problem hiding this comment.
Agreed — max_events_per_second was removed. It'll come back when R924 (rate limiting) is implemented in a future phase.
| /// Human-readable detail/message (action results, errors, etc.) | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub detail: Option<String>, |
There was a problem hiding this comment.
The MonitorEvent rustdoc lists a fixed set of event_type strings, but this PR adds new emitted types (e.g. pad_pressed, pad_released, short_press, action_executed, etc.) and a new detail field for action/processed metadata. Update the documentation near MonitorEvent so downstream consumers (CLI/GUI) have an accurate contract for the wire format.
There was a problem hiding this comment.
Updated in commit a332737 — the event_type rustdoc now lists all raw MIDI, processed gesture, and action event types, plus documents the new detail field.
…, cleanup CI fixes: - map_or → is_none_or/is_some_and (clippy::unnecessary_map_or) - Add event_console: None to integration tests, benches, workspace tests Copilot review (5 comments): 1. Add 'encoder' alias in getEventColor (raw MIDI event type) 2. Gate raw MIDI capture with capture_midi toggle (R926) — both single-device and multi-device paths 3. Remove unused max_events_per_second config (was no-op) 4. Update MonitorEvent doc with all event type strings 5. Add capture_midi field to EngineManager
Summary
Phase 3 of Issue #325 — config-driven event console, action event monitoring, and gesture event visibility.
Changes
EventConsoleConfig (R911-R914, R924-R928):
[event_console]TOML config sectionbuffer_size(R925) — configurable event buffer (default 1000, min 1)max_events_per_second(R924) — rate limit configcapture_midi/capture_processed/capture_actionstoggles (R926-R928)[event_console.filters.<name>]presets (R911-R913)--filter/-Floads named filter from config (R914)Action Event Emission (R897-R898):
emit_action_event()pushes mode_change, action_executed, action_errordetailfield on MonitorEvent (not device_id)capture_actionstoggleProcessed Event Emission (R893):
emit_processed_event()maps all ProcessedEvent variantscapture_processedtoggleFrontend:
Infrastructure:
push_monitor_event()DRY helper (consolidates 4 buffer push sites)whileloop for buffer cap (future-proof)Council Review
Two rounds of council review (7/10 initial):
Requirements Addressed
R893, R897-R898, R911-R914, R924-R928
Tests