Skip to content

feat(#325): Phase 3 — config, capture toggles, action/gesture events#372

Merged
amiable-dev merged 6 commits intomainfrom
feat/325-phase3-event-console
Feb 26, 2026
Merged

feat(#325): Phase 3 — config, capture toggles, action/gesture events#372
amiable-dev merged 6 commits intomainfrom
feat/325-phase3-event-console

Conversation

@amiable-dev
Copy link
Owner

Summary

Phase 3 of Issue #325 — config-driven event console, action event monitoring, and gesture event visibility.

Changes

EventConsoleConfig (R911-R914, R924-R928):

  • New [event_console] TOML config section
  • buffer_size (R925) — configurable event buffer (default 1000, min 1)
  • max_events_per_second (R924) — rate limit config
  • capture_midi / capture_processed / capture_actions toggles (R926-R928)
  • Named filters: [event_console.filters.<name>] presets (R911-R913)
  • CLI --filter/-F loads named filter from config (R914)

Action Event Emission (R897-R898):

  • emit_action_event() pushes mode_change, action_executed, action_error
  • Uses new detail field on MonitorEvent (not device_id)
  • Gated by capture_actions toggle

Processed Event Emission (R893):

  • emit_processed_event() maps all ProcessedEvent variants
  • LongPress, MediumPress, ShortPress, HoldDetected visible with duration
  • ChordDetected shows constituent notes, EncoderTurned shows direction
  • Gated by capture_processed toggle
  • Both single-device and multi-device paths emit with device context

Frontend:

  • Gesture event colors: green=quick, yellow=medium, red=long, pink=hold, purple=double, cyan=chord
  • Covers pad_pressed, cc_received, encoder_turn and all gesture types

Infrastructure:

  • push_monitor_event() DRY helper (consolidates 4 buffer push sites)
  • while loop for buffer cap (future-proof)
  • Buffer size clamped to min 1

Council Review

Two rounds of council review (7/10 initial):

  • R1: Capture toggles wired, detail field, DRY helper, buffer clamp
  • R2: Rich event detail, device path emission, while loop, cc/value fields

Requirements Addressed

R893, R897-R898, R911-R914, R924-R928

Tests

  • 469 daemon + 250 GUI + 365 frontend = 1084 total, 0 failures
  • New: 3 config tests, 3 monitor event tests, expanded getEventColor tests

…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
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) to conductor-core config 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':
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
case 'cc_received':
case 'cc_received':
case 'encoder':

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
if self.event_monitor_active.load(Ordering::Relaxed)
if self.event_monitor_active.load(Ordering::Relaxed)
&& self.event_monitor_capture_midi

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 2ce30c7capture_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)
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
if self.event_monitor_active.load(Ordering::Relaxed)
if self.event_monitor_active.load(Ordering::Relaxed)
&& self.capture_midi.load(Ordering::Relaxed)

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed alongside the single-device path in commit 2ce30c7 — see line 2957.

Comment on lines +281 to +288
/// 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,
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
/// 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 {

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — max_events_per_second was removed. It'll come back when R924 (rate limiting) is implemented in a future phase.

Comment on lines +155 to +157
/// Human-readable detail/message (action results, errors, etc.)
#[serde(default, skip_serializing_if = "Option::is_none")]
pub detail: Option<String>,
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.

@amiable-dev amiable-dev merged commit f097357 into main Feb 26, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants