Cagire/refacto.md

Refactoring Plan: Isolation and Lower Connascence

This document outlines practical, incremental refactoring tasks to improve architectural isolation and reduce connascence in this codebase. The goal is to make changes safe and reversible, while reducing coupling between UI, engine, and state.

Principles:

• Preserve behavior. Refactor in small steps with clear boundaries.
• Reduce connascence by making dependencies explicit and localized.
• Make render paths pure views over state.
• Centralize bootstrapping and cross-cutting wiring.

Task 1: Extract Shared Bootstrap (Detailed Checklist)

Problem:

• src/main.rs and src/bin/desktop.rs duplicate long initialization sequences (settings, audio config, MIDI discovery, samples, sequencer startup). This creates drift and implicit coupling.

Refactor:

• Create a bootstrap module (e.g., src/bootstrap.rs) that returns a fully configured App, SequencerHandle, and any shared runtime state.
• Both binaries call into this module with minimal differences (e.g., terminal vs desktop display setup).

Checklist:

1. Inventory duplicated blocks
   • In src/main.rs, locate: settings load, link setup, App::new, audio config assignment, MIDI load, sample scan, sequencer spawn, build_stream.
   • In src/bin/desktop.rs, locate the analogous blocks (very similar ordering).
2. Define a shared config struct
   • Create a BootstrapArgs struct with fields: samples, output, input, channels, buffer.
   • This mirrors CLI args from both binaries.
3. Extract app init
   • Create fn build_app(settings: &Settings, args: &BootstrapArgs) -> App in a new module (e.g., src/bootstrap.rs).
   • Move these from both binaries:
     • App::new()
     • app playback queued changes setup
     • audio config assignments from settings/args
     • UI settings (runtime_highlight, show_scope, show_spectrum, completion, color scheme/hue in terminal path)
     • MIDI device selection (output/input device selection based on stored names)
4. Extract audio/sample init
   • Create fn load_samples(paths: &[PathBuf]) -> (Vec<SampleEntry>, usize) returning the sample list and count.
   • Replace inline scan_samples_dir loops in both binaries.
5. Extract sequencer init
   • Create fn build_sequencer(app: &App, settings: &Settings, link: Arc<LinkState>, ...) -> SequencerHandle that:
     • Builds SequencerConfig
     • Calls spawn_sequencer
     • Returns the handle + receivers needed for MIDI/audio.
6. Extract stream init
   • Create fn build_audio_stream(config: &AudioStreamConfig, ...) -> Result<(Stream, StreamInfo, AnalysisHandle), Error>
   • Use it in both binaries to set app.audio fields.
7. Replace duplicated code
   • Update src/main.rs and src/bin/desktop.rs to call the bootstrap functions.
8. Keep env-specific parts in binary
   • Terminal UI init (crossterm setup) remains in src/main.rs.
   • Desktop font/egui setup remains in src/bin/desktop.rs.

Outcome:

• One place to reason about initialization and one place to fix config drift.

Connascence reduced:

• Reduced connascence of meaning and position across two binary entry points.

Task 2: Remove State-to-View Layer Inversion (Detailed Checklist)

Problem:

• UiState::default depends on view modules (dict_view::category_count(), help_view::topic_count()). This inverts layering and creates brittle dependencies.

Refactor:

• Move those counts into a pure data module, for example src/state/ui_defaults.rs or src/data/help_topics.rs.
• UiState::default should depend only on state/data modules, not views.

Checklist:

1. Identify view-level sources
   • src/views/help_view.rs defines topics using Topic(...) list.
   • src/views/dict_view.rs defines category list and category_count().
2. Create a pure data module
   • Add a new module (e.g., src/data/help_dict.rs) with:
     • pub const HELP_TOPIC_COUNT: usize = ...
     • pub const DICT_CATEGORY_COUNT: usize = ...
   • If the counts are derived from arrays, move the arrays into this module and re-export.
3. Update views to depend on data module
   • help_view::topic_count() should return HELP_TOPIC_COUNT.
   • dict_view::category_count() should return DICT_CATEGORY_COUNT.
4. Update UiState::default
   • Replace calls to view modules with data module constants in src/state/ui.rs.
5. Confirm no other state files import views
   • Use rg "views::" src/state to verify.

Outcome:

• UI state is decoupled from rendering code.

Connascence reduced:

• Reduced connascence of type (views no longer required by state).

Task 3: Split App Dispatch into Subsystem Handlers (Detailed Checklist)

Problem:

• App::dispatch is a very large match that mutates many unrelated parts of state. This makes the App a God object and introduces high connascence across domains.

Refactor:

• Introduce subsystem handlers: ProjectController, PlaybackController, UiController, AudioController, MidiController.
• Dispatch routes to a handler based on command domain.

Checklist:

1. Partition commands by domain
   • Playback: TogglePlaying, TempoUp, TempoDown, staging changes.
   • Project edits: rename/save/load, pattern/bank changes.
   • UI-only: modals, status, help/dict navigation.
   • Audio settings: device list navigation, buffer/channels settings.
   • MIDI: connect/disconnect, CC, etc.
2. Introduce handler modules
   • Create src/controllers/ (or src/handlers/) with playback.rs, project.rs, ui.rs, audio.rs, midi.rs.
3. Move code in thin slices
   • Start with the smallest domain (UI-only commands).
   • Move those match arms into UiController::handle and call from App::dispatch.
   • Repeat for each domain.
4. Define minimal interfaces
   • Each controller takes only the state it needs (e.g., &mut UiState, not &mut App).
5. Keep App::dispatch as a router
   • Single match that forwards to controller or returns early.
6. Reduce App field exposure
   • Make fields private where possible and provide accessors for controllers.
7. Update tests (if/when added)
   • New controllers can be unit-tested without UI/engine wiring.

Outcome:

• Clear separation of concerns. Smaller, testable units.

Connascence reduced:

• Reduced connascence of execution and meaning inside one massive switch.

Task 4: Make Rendering Pure (No Side Effects / Heavy Work) (Detailed Checklist)

Problem:

• Rendering does real work (compute_stack_display runs a Forth evaluation each frame). This couples rendering to runtime behavior and risks UI jank.

Refactor:

• Compute stack previews in a controller step when editor state changes, store result in UiState or a new cache state.
• Renderers become pure read-only views of state.

Checklist:

1. Locate render-time work
   • src/views/render.rs → compute_stack_display builds a Forth engine and evaluates script.
2. Introduce cache state
   • Add a StackPreview struct to src/state/editor.rs or src/state/ui.rs:
     • last_cursor_line, lines_hash, result.
3. Define update points
   • Update cache when editor content changes or cursor moves.
   • Likely locations:
     • input.rs when editor receives input.
     • app.rs functions that mutate editor state.
4. Move compute logic to controller/service
   • Create src/services/stack_preview.rs that computes preview from editor state.
   • Call it from update points, store in state.
5. Make render pure
   • Replace compute_stack_display usage with cached state from UiState/EditorContext.
6. Verify consistency
   • Ensure preview updates on paste and modal editor inputs (see Event::Paste handling in src/main.rs).

Outcome:

• Rendering becomes deterministic and cheap.

Connascence reduced:

• Reduced connascence of timing (render cycle no longer coupled to evaluation).

Task 5: Concurrency Boundary for Script Runtime (Detailed Checklist)

Problem:

• variables, dict, rng are shared between UI and sequencer threads via Arc<Mutex> and ArcSwap. This is a hidden concurrency coupling and risks contention in RT threads.

Refactor:

• Introduce a message-based interface: UI posts commands to a dedicated script runtime in the sequencer thread.
• UI no longer directly mutates shared dict/vars.

Checklist:

1. Identify shared state
   • App::new constructs variables, dict, rng, script_engine.
   • These are passed into spawn_sequencer in src/main.rs.
2. Define a runtime command channel
   • New enum ScriptRuntimeCommand (e.g., Reset, UpdateVar, ClearDict, etc.).
   • Add channel to sequencer config and main loop.
3. Move ownership into sequencer thread
   • spawn_sequencer creates its own variables/dict/rng and ScriptEngine.
   • Sequencer thread consumes ScriptRuntimeCommands and applies changes.
4. Update UI interaction points
   • Anywhere UI currently mutates variables or dict should send a runtime command instead.
   • Example: App::dispatch branch that does self.variables.store(...) / self.dict.lock().clear().
5. Replace direct shared references
   • Remove ArcSwap and Mutex from App for script runtime.
6. Verify audio thread safety
   • Ensure no locks are taken in RT critical sections.

Outcome:

• Clear ownership and fewer concurrency hazards.

Connascence reduced:

• Reduced connascence of timing and execution across threads.

Task 6: Isolate Model from Re-Exports (Detailed Checklist)

Problem:

• src/model/mod.rs is mostly re-exports. This blurs boundaries and encourages wide imports.

Refactor:

• Introduce a narrower domain API layer (e.g., model::ProjectFacade) that exposes only the subset the app needs.
• Limit broad re-exports.

Checklist:

1. Identify heavy imports
   • use crate::model::{...} in src/app.rs, src/input.rs, src/views/*.
2. Create small facade modules
   • e.g., src/model/project_facade.rs exporting only Project, Pattern, Bank, etc.
   • e.g., src/model/script_facade.rs for script-related types.
3. Replace pub use in model/mod.rs
   • Narrow to explicit re-exports or remove when not needed.
4. Update call sites
   • Replace crate::model::* with focused imports from facade modules.
5. Document boundary
   • Add brief module docs explaining what is safe to use externally.

Outcome:

• Clearer ownership and lower coupling to external crate internals.

Connascence reduced:

• Reduced connascence of meaning across module boundaries.

Suggested Order

• Task 1 (bootstrap) and Task 2 (state/view decoupling) first. These are low risk and improve clarity.
• Task 3 (split dispatch) next to make the codebase easier to change.
• Task 4 (pure rendering) and Task 5 (runtime boundary) after the above to avoid broad ripple effects.
• Task 6 (model isolation) last, because it benefits from clearer boundaries established earlier.

Success Criteria

• Startup is defined in one module and reused by both binaries.
• UiState compiles without importing anything from views.
• App::dispatch reduced by at least 50% and sub-handlers exist.
• Render path is a pure function of state (no evaluation, no locks).
• Script runtime state is owned by the sequencer thread.