BuboBubo/Cagire
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Ungoing refactoring

Browse Source
This commit is contained in:
Raphaël Forment
2026-02-04 18:47:40 +01:00
parent 2097997372
commit 6cf9d2eec1
19 changed files with 749 additions and 451 deletions
Download Patch File Download Diff File Expand all files Collapse all files

227
refacto.md Normal file
View File

@@ -0,0 +1,227 @@
# 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 `ScriptRuntimeCommand`s 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.