Skip to content

refactor: architecture pass — split large modules into topic-grouped files#147

Merged
yogthos merged 7 commits into
mainfrom
arch/split-large-modules
May 27, 2026
Merged

refactor: architecture pass — split large modules into topic-grouped files#147
yogthos merged 7 commits into
mainfrom
arch/split-large-modules

Conversation

@yogthos
Copy link
Copy Markdown
Collaborator

@yogthos yogthos commented May 27, 2026

Summary

Architecture review identified a handful of files in the >2000 LOC
range that were impeding navigation. This branch reorganizes them
without changing behavior — every commit is a mechanical split,
verified clean against the full test suite (1718 pass / 0 fail).

Before / after (largest source files)

File Before After Change
src/ui/mod.rs 5905 3476 −41%
src/ui/slash.rs 2537 1048 −59%
src/ui/renderer.rs 2328 1604 −31%
src/agent/agent_loop/tools.rs 1960 848 −57%
src/session/mod.rs 1815 854 −53%
src/agent/agent_loop/run.rs 1799 818 −55%
src/permission/checker.rs 1741 892 −49%
src/extras/session_db.rs 1674 992 −41%
src/provider/mod.rs 1578 1164 −26%
src/agent/agent_loop/bridge.rs 1088 475 −56%
src/plugin/mod.rs 3818 1389 −64%

Commits

  1. Extract inline test modules (5eef045) — every #[cfg(test)] mod tests over ~400 lines moves to a sibling <stem>_tests.rs, included via #[cfg(test)] #[path] mod tests;. Nine files affected. ~9000 lines moved.

  2. Split ui/mod.rs helpers (0df56de) — top-of-file and trailing helper functions move into single-topic modules:

    • ui/colors.rs — themed color accessors
    • ui/text_output.rs — user-input rendering, sanitization, strip_leading_system_reminder + tests
    • ui/chat_state.rs — per-chat UI-state snapshot/restore (dirge-ov2 phase C)
    • ui/agent_io.rs — agent-stream rendering, partial-on-abort capture, persist-turn-to-db
    • ui/panel_render.rs — modified-files cache + panel data builder
    • ui/search_rewind.rs — chat search + rewind picker
    • ui/shell_exec.rs!cmd shell escape handler
  3. Split ui/slash.rs by command group (d4b3c6c) — slash.rs becomes slash/ directory:

    • slash/mod.rs — dispatcher + handle_compress + shared helpers
    • slash/cmd_model.rs/model, /mode, /prompt, /reasoning, /regen-prompts, /toggle
    • slash/cmd_session.rs/sessions, /tree, /fork, /clone, /undo, /retry, /tasks, /clear
    • slash/cmd_worktree.rs/worktree, /wt-merge, /wt-exit
    • slash/cmd_misc.rs/mcp, /btw, /cd, /panel, /quit, /help, /allow, /loop

    Introduces SlashCtx<'a> to bundle the references each handler needs.

  4. Extract run_interactive event handlers (767fdcf) — the four largest tokio::select! arms move into ui/run_handlers/:

    • tool_result.rs (312 lines) — was inline ~300 LOC
    • done.rs (598 lines) — was inline ~470 LOC; the hardest arm
    • interjected.rs (157 lines) — was inline ~110 LOC
    • context_overflow.rs (290 lines) — was inline ~250 LOC

    Bundled state via RunCtx<'a> (18 fields) + per-handler explicit params. make_run_ctx! macro keeps call sites one-liners.

  5. Storyboard path updates (44200dc) — fix file:line references in docs/storyboards/ after the moves.

Why these splits in particular

Each handler/file group reflects a single concern a developer touches when fixing a bug or adding a feature:

  • "How is user input echoed?" → ui/text_output.rs
  • "Why does ToolResult render this way?" → ui/run_handlers/tool_result.rs
  • "/compress focus topic?" → ui/slash/mod.rs for the slash dispatch, provider/mod.rs::compress_messages for the prompt, agent/compression.rs for the auto-trigger
  • "Permission rule order?" → permission/checker.rs (production-only now, tests in sibling)

Compatibility

  • All public functions keep their existing signatures and paths
  • Storyboard cross-references updated to new locations
  • git mv used where possible to preserve file history (slash.rs → slash/mod.rs)
  • #[cfg(feature = "plugin")] gates preserved across moves
  • The strip_system_reminder_tests and other inline test sub-modules move with their owning helpers

Verification

  • cargo check --features "semantic,semantic-ts,semantic-python,semantic-bash,semantic-clojure,semantic-go,semantic-ruby,semantic-rust,semantic-java,semantic-c,semantic-cpp,mcp,plugin": clean
  • cargo test --bin dirge (same features): 1718 pass / 0 fail / 0 ignored
  • cargo check --no-default-features: clean
  • cargo check --features "git-worktree,loop": clean

Test plan

  • Full test suite passes on the branch
  • Build with multiple feature subsets (full, none, partial)
  • Storyboard paths re-verified
  • CI on PR
  • Smoke-test interactive (dirge -p "hello") before merge

Yogthos added 7 commits May 27, 2026 11:10
Mechanical move: every `#[cfg(test)] mod tests { … }` block over
~400 lines is now pulled into a sibling file via `#[path]`
include. Parents stay at ~production-only sizes; tests keep
`use super::*` access to private items.

Files affected (source line counts before → after):
- src/plugin/mod.rs              3818 → 1389  (+2434 in mod_tests.rs)
- src/agent/agent_loop/run.rs    1799 →  818  (+984  in run_tests.rs)
- src/agent/agent_loop/tools.rs  1960 →  848  (+1115 in tools_tests.rs)
- src/agent/agent_loop/bridge.rs 1088 →  475  (+616  in bridge_tests.rs)
- src/session/mod.rs             1815 →  854  (+961  in mod_tests.rs)
- src/permission/checker.rs      1741 →  892  (+849  in checker_tests.rs)
- src/provider/mod.rs            1578 → 1164  (+414  in mod_tests.rs)
- src/extras/session_db.rs       1674 →  992  (+682  in session_db_tests.rs)
- src/ui/renderer.rs             2328 → 1604  (+724  in renderer_tests.rs)

Net: ~9000 lines moved from production-mixed files to test-only
siblings. Each parent now has a single 3-line incantation:

  #[cfg(test)]
  #[path = "<stem>_tests.rs"]
  mod tests;

(plugin/mod.rs keeps its `#[cfg(all(test, feature = "plugin"))]`
gate so cargo test without the plugin feature still works.)

Pure mechanical move — zero behavioral changes, zero test logic
touched. Build clean; full test suite 1718 pass / 0 fail / 0
ignored.

Follow-ups (still to do this branch):
- ui/mod.rs at 5905 lines — `run_interactive` is the bulk;
  needs its own surgical split.
- plugin/mod.rs at 1389 lines — `PluginManager` impl block
  could split into trait-grouped impl blocks.
ui/mod.rs was 5905 lines. Most of the bulk is `run_interactive` (a
4300-line tokio::select! loop) which stays in place — but the
top-of-file and trailing helper functions are extracted into
single-topic modules so:

  "fix how user input is echoed" → src/ui/text_output.rs
  "color tweak in panel render" → src/ui/colors.rs
  "panel data shape" → src/ui/panel_render.rs
  "search/rewind picker bug" → src/ui/search_rewind.rs

New modules (lines):
- src/ui/colors.rs        67   c_agent/c_error/c_tool/c_perm, parse_plugin_color, resolve_color
- src/ui/text_output.rs   136  with_queue, write_user_lines, sanitize_single_line,
                                strip_leading_system_reminder + 5 tests
- src/ui/chat_state.rs    100  ChatUiState, save_chat_ui_state, load_chat_ui_state
- src/ui/agent_io.rs      293  apply_subagent_panel_event, render_agent_stream,
                                capture_partial_on_abort, persist_turn_to_db,
                                render_plugin_entry
- src/ui/panel_render.rs  148  PANEL_MODIFIED_CACHE, panel_modified_cached,
                                build_panel_data
- src/ui/search_rewind.rs 253  is_placeholder_pattern, suggest_pattern,
                                update_search, open_rewind_picker, rewind_session
- src/ui/shell_exec.rs    33   run_shell_command

ui/mod.rs: 5905 → 5001 (−904 lines).

run_interactive's call sites are unchanged — a single `use
crate::ui::{colors, text_output, …}::{...}` block at the top of
mod.rs re-imports the names so existing call sites still read
`c_agent()`, `with_queue(...)`, `rewind_session(...)` etc.

Plugin-gated items (parse_plugin_color, render_plugin_entry)
preserve their `#[cfg(feature = "plugin")]` gates across the
move. The strip_system_reminder_tests inline module moves with
strip_leading_system_reminder into text_output.rs.

Build clean. Full test suite: 1718 pass / 0 fail / 0 ignored.

Follow-ups:
- ui/mod.rs still 5001 lines — `run_interactive`'s big event-
  handler arms (ToolResult ~300 LOC, Done ~470 LOC) could be
  extracted into per-event handler fns next.
- ui/slash.rs at 2537 — the 30-arm /cmd match could be split
  into topic-grouped handler files.
- permission/checker.rs still has a large PermissionChecker::
  check function that could be broken up by rule-class.
`src/ui/slash.rs` (2537 lines) had one giant `handle_slash`
function whose match statement spanned ~25 slash-command arms.
Splitting by command family so a developer fixing a `/sessions`
bug doesn't scroll past `/worktree`, `/mcp`, `/btw`, etc.

Conversion:
- `src/ui/slash.rs` → `src/ui/slash/mod.rs` (via git mv —
  preserves history; the `crate::ui::slash` path stays valid)
- New `src/ui/slash/cmd_model.rs` (395 lines) —
  /model, /mode, /prompt, /reasoning, /regen-prompts, /toggle
- New `src/ui/slash/cmd_session.rs` (363 lines) —
  /sessions, /tree, /fork, /clone, /undo, /retry, /tasks, /clear
- New `src/ui/slash/cmd_worktree.rs` (175 lines) —
  /worktree, /wt-merge, /wt-exit
- New `src/ui/slash/cmd_misc.rs` (651 lines) —
  /mcp, /btw, /cd, /panel, /quit, /help, /allow, /loop

mod.rs is now 1048 lines (down from 2537, −59%). The dispatcher
is a clean ~75-line match where each arm is a one-liner call
into the topic file.

Key design choice: introduced `pub(super) struct SlashCtx<'a>`
that bundles the mutable references `handle_slash` was juggling
(renderer, session, cli, cfg, context, agent, client, plus
feature-gated loop_state / mcp_manager / semantic_manager /
lsp_manager). Each handler takes `&mut SlashCtx<'_>` so
signatures stay uniform.

Preserved invariants:
- `handle_slash`, `handle_compress`, `undo_last`, `CompressOutcome`,
  `UndoOutcome`, `align_cut_to_user_boundary`, `try_complete`,
  `slash_command_names`, `is_known_slash_command`,
  `builtin_commands`, `format_completion_preview` — all kept in
  `mod.rs` (the callers from `src/ui/mod.rs` are unchanged).
- `/compress` | `/compact` arm kept in mod.rs since it just
  emits the `DEFER_COMPRESS:` sentinel for the outer loop.
- All `DEFER_*` sentinel errors (`DEFER_COMPRESS`, `DEFER_WT_MERGE`,
  `DEFER_WT_EXIT`) preserved verbatim.

Build clean with full features, --no-default-features, and the
git-worktree+loop subset. Full test suite: 1718 pass / 0 fail
/ 0 ignored.
…n_handlers/

`run_interactive`'s tokio::select! had ~470-line `Done`, ~300-line
`ToolResult`, ~250-line `ContextOverflow`, ~110-line `Interjected`
arms inlined. Each is now a dedicated handler module in
`src/ui/run_handlers/`. The dispatcher arms in run_interactive
collapse to a single function call against a `RunCtx<'a>` bundle.

New module: `src/ui/run_handlers/`
- mod.rs (RunCtx + macro_rules! make_run_ctx! shorthand, 95 lines)
- tool_result.rs (handle_tool_result, 312 lines) — pairs to call,
  renders inside chamber / as `↳` trailer / as fresh chamber for
  parallel-execution races, with colorized edit-diff path
- done.rs (handle_done, 598 lines) — closes chamber, runs plugin
  on-response / on-complete / prepare-next-run (with optional
  agent rebuild on model swap), finalizes streamed text, auto-
  compacts, dispatches decide_post_done_action, spawns background
  review + curator, handles git-worktree return, drains
  interjections
- interjected.rs (handle_interjected, 157 lines) — finalizes
  partial response, persists with Interrupted tool-call entries,
  drains queued interjections
- context_overflow.rs (handle_context_overflow, 290 lines) — auto-
  compact + respawn with tool-side-effect safety + interjection-
  queue draining on each failure branch

RunCtx<'a> bundles 18 fields of mutable borrows + 2 non-mut config
refs the inline handlers were juggling: renderer, session,
response/reasoning buffers + start lines, agent_line_started,
last_tool_name / last_tool_call_id, chamber state, tool_calls
buf + counter, last_collapsed, last_user_prompt, cli, cfg.
Per-handler extras (runner state, channels, feature-gated
managers) are explicit params. The `make_run_ctx!` macro
collapses each call site's RunCtx literal from ~20 lines to one.

Arms that stayed inline (under the 100-line threshold or
control-flow-entangled): Reasoning, Token, ToolCall, ToolStarted,
CustomMessage, Error, TurnStart, TurnEnd, ContextCompacted,
UserMessage, RetryNotice.

Final stats:
- src/ui/mod.rs:   5001 → 3476 lines (−1525, total this branch
  5905 → 3476, −41%)
- run_handlers/:   1414 lines across 5 files
- mod_tests.rs:    571 lines (already extracted prior round)

Build clean, 1718 pass / 0 fail / 0 ignored.

Bonus: removed an unused `use compact_str::CompactString` import
in mod.rs that the handler extraction left behind.
slash.rs is now slash/mod.rs (the file became a directory module).
The interjection-queue drain paths moved from src/ui/mod.rs into
the dedicated run_handlers/{done,interjected}.rs files. Updated
storyboards 03 and 04 with the new locations.
Mechanical rustfmt pass on the files touched during this branch
plus a handful of others that had drift. No logic changes.

33 files, +5692/-5736 (net −44; formatting rebalances long lines
in the new test sibling files, the run_handlers/* modules, and
the cmd_*.rs slash-command modules).

Build clean, 1718 pass / 0 fail / 0 ignored.
Addresses every warning surfaced after the architecture split:

**Wired in** (had a real production purpose but the path was
incomplete):

- `AbortSignal::cancel()` — production cooperative-cancel path
  finally wired through the UI. Added `cancel_tx: mpsc::Sender<()>`
  to `AgentRunner` paired with the existing `interject_tx`.
  `into_agent_runner` spawns a second bridge task that translates
  the first received `()` into `signal.cancel()` on the inner
  signal. UI's Ctrl+C / Ctrl+D / Esc handlers (three sites in
  ui/mod.rs) now fire `agent_cancel.try_send(())` BEFORE
  `JoinHandle::abort()` so the retry loop + rig stream see
  `is_cancelled()` and exit through their clean error paths
  before the task is killed. The previously-dead `cancel()`
  setter, `is_cancelled()` checks in `retry.rs`/`rig_stream.rs`
  are all now live.

**Removed** (truly dead, never called):

- `compression::CompressionResult` — defined for a richer return
  shape but `run_compaction_pass` emits the loop event directly.
  No consumer.
- `compression::approx_tokens` — superseded by
  `estimate_messages_tokens`. Never called.
- `webfetch::is_private_ipv4` final `_ => false` arm — the
  preceding `[a, _, _, _] => a >= 240` pattern is the exhaustive
  catch-all (matches every first octet, returns false when <240).
  Comment added explaining the arm's dual role.

**Renamed**:

- `run_agent_loop_with_summarizer` → `run_agent_loop` (the wrapper
  that forwarded with `None` was the dead duplicate). Same for
  `run_loop_with_summarizer` → `run_loop`. All ~12 test sites in
  `run_tests.rs`/`steering.rs` updated to pass `None` as the new
  trailing `summarize_fn` arg. Single canonical name, no wrappers,
  no `_with_summarizer` suffix in the codebase anymore.

Also pairs `agent_cancel: &mut Option<mpsc::Sender<()>>` parameter
into the three run_handlers/ extracted functions (`done`,
`interjected`, `context_overflow`) so they thread the new channel
back into the UI state struct just like `agent_interject`.

Build clean, **zero** warnings (was 6). Full test suite:
1718 pass / 0 fail / 0 ignored.

cargo fmt pass included.
@yogthos yogthos merged commit e43dee4 into main May 27, 2026
1 check passed
@yogthos yogthos deleted the arch/split-large-modules branch May 27, 2026 17:38
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.

1 participant