pmacs/docs/lua-bindings-split-framing.md

Splitting lua_bindings.rs — framing (F-016)

src/lua_bindings.rs is 15,202 lines — the repository's largest file by 2×. It concentrates the entire Rust↔Lua surface: a shared core (the registry alias, BindingError, the BufferIdLua userdata, intercept views, ~15 state-holder types, the install() spine) followed by ~20 independent pmacs.<domain> API surfaces (packages, lsp, mcp, completion, project, index, window, minibuffer, parse, theme, process, diag, async, ansi, …) and ~2,800 lines of tests. Audit F-016.

This is exactly the file the audit calls out ("combines many Lua API domains, package management glue, LSP stores, theme bindings, attachment bindings, and tests in one file") and its guidance is explicit: "Split only along stable boundaries, not as a drive-by refactor." The pmacs.<domain> sections, already fenced by // --- banners and each wired through its own install_<domain>_module seam, are those stable boundaries.

Non-negotiable: behavior-preserving code motion

This arc moves code; it does not change it. Every tranche is:

• Pure relocation. A domain section's items move verbatim into lua_bindings/<domain>.rs. No logic edits, no signature changes, no renames beyond what visibility requires.
• Minimal visibility widening. Items in the shared core that a moved domain references become pub(crate) (or pub(super)); a domain's own install_<domain>_module becomes pub(super) so the parent install() still calls it. Nothing gains wider visibility than the move demands — the compiler names each one (E0603/E0433), so the set is exact, not guessed.
• Green per tranche. cargo build + cargo clippy --all-targets + the full test suite pass under both Lua flavors after every tranche, before it's committed. A split that changes a test outcome is a bug in the split.

The Lua-visible API (pmacs.buffer.*, pmacs.lsp.*, …) is byte-for-byte unchanged — the same install() builds the same tables; only the Rust file layout moves. No .lua code and no test behavior changes.

Target structure

src/lua_bindings.rs → src/lua_bindings/:

• mod.rs — the shared core (registry alias, BindingError, BufferIdLua + its method-adders, LuaInterceptView, the state-holder types, require_init_phase, shared helpers) and the top-level pub fn install() spine that calls each domain's installer. This is the stable hub every domain depends on; it stays put.
• lua_bindings/<domain>.rs — one file per pmacs.<domain> surface, exposing pub(super) fn install_<domain>_module(...). Each #[cfg(test)] mod tests moves with its domain where the tests only touch that domain's (now pub(super)-reachable) surface; shared/cross-cutting tests stay in mod.rs.

The dependency shape is a hub-and-spoke: mod.rs is the hub; domains are spokes that depend on the hub and (ideally) not on each other. Where a genuine domain→domain edge exists (e.g. completion→lsp), the depended-on domain is extracted first and its needed items widened to pub(crate).

Incremental, not a big bang

A single 15k-line move is unreviewable and risks silent breakage. Instead, each tranche is its own PR: convert the file to a directory module + extract one dependency-ordered group of domains, prove green, merge, next. Leaf domains (no outgoing cross-domain edges) go first to establish the pattern; coupled domains follow their dependencies.

What the coupling recon established

• Two misplaced helper clusters cause almost every cross-domain edge. The generic JSON converters (lua_to_json/lua_table_to_json/ json_to_lua) sit inside the lsp section but are used by async, mcp, completion, and the completion framework — they belong in shared core. The ANSI converters (event_to_lua_table/style_to_lua_table) sit in the packages line-range but belong to ansi (process uses them too). Hoisting these dissolves the async→lsp, mcp→lsp, completion→lsp, and ansi→packages edges.
• The banners are not clean cut-lines. The packages line-range is a grab-bag: it physically contains the core module installers (command, menu, help, hook, describe, keymap) that install() calls and that belong to shared core, plus editor sub-installers. Extraction must move items to their logical home, not by line-range.
• Rust privacy is on our side. A child module can read its ancestors' private items, so a domain moved to lua_bindings/<d>.rs reaches shared-core internals via super:: with no widening. Widening is needed only for (a) a domain's install fn so the parent can call it (pub(super)), (b) sibling→sibling edges, and (c) external callers — the last handled by pub(crate) use <d>::<item>; re-exports in mod.rs so crate::lua_bindings::<item> paths in editor.rs/lua.rs/etc. never change.
• Real edges that survive the hoist: process → ansi, project → lsp, completion_framework → index/lsp (type-alias only). These extract after their dependency.
• Tests are one flat #[cfg(test)] mod tests over super::* touching private items (BufferIdLua, require_init_phase, install, handles). They stay in mod.rs and move to lua_bindings/tests.rs last, once the handful of items they name are reachable.

Tranche plan (one PR each)

0. This PR — directory conversion + the truest leaf (diag). lua_bindings.rs → lua_bindings/mod.rs; extract the smallest zero-outgoing-edge leaf, diag, as lua_bindings/diag.rs. Deliberately minimal: a large mechanical refactor's first PR should validate the directory module, the super::-access discipline, and the new-file CI path on the simplest real case before moving code in bulk. Subsequent tranches batch multiple domains now that the mechanics are proven.
1. index (the one genuinely clean remaining leaf). Zero shared-core coupling; establishes the pub(crate) use re-export for external callers (editor.rs, completion_framework.rs) and super:: access to a stranded helper (lua_to_json). Correction after tranche 1: the recon under-counted the misplaced helpers. Beyond the JSON/ANSI clusters, the style/color converters (lua_to_style/style_to_lua/ color_to_lua/…) sit in the theme section, and caller_source + the command/menu builders (build_command_from_spec, build_menu_item_from_spec, BindArgs, register, …) sit at the tail of the minibuffer range — all shared, used across sections. So parse/theme, window, and minibuffer are not clean line-range extractions until the hoist below runs.
2. Extract mcp — and the corrected mental model. A helper-hoist is not a prerequisite for most domains. The contamination that stopped parse/theme is narrow: it bites only when a shared helper is defined inside the range being extracted (the style helpers live in the theme range and are used elsewhere, so extracting that range removes them from mod.rs). A domain that merely uses a cross-section helper reaches it via super:: (parent-private access) — no hoist needed. So mcp extracts cleanly today: all its items are self-contained, and it reaches the JSON converters (still in the lsp section) via super::json_to_lua/lua_to_json, exactly like index. The JSON-helper hoist is deferred to the tranche that extracts lsp itself (where they are defined).
3. Hoist-then-extract the genuinely contaminated sections. Only the sections that define shared helpers used elsewhere need a hoist first: theme (style/color converters), minibuffer/window (caller_ source, command/menu builders), lsp (JSON converters), packages (the core module installers). Hoist those helpers to shared-core position (a within-mod.rs reposition, no behavior change), then extract the now-clean ranges.
4. The lsp hub + its JSON consumers. lsp, then async, mcp, completion (edge-free once the JSON helpers are hoisted).
5. The coupled tail. process (→ansi), project (→lsp), completion_framework (→index/lsp), and the editor umbrella (gathering its sub-installers scattered across the window/packages ranges).
6. Tests. Move the flat mod tests into lua_bindings/tests.rs.

Each tranche is independently green and mergeable; the order is dependency-correct so no tranche introduces an unresolved sibling edge.

Scope of this arc

This arc splits src/lua_bindings.rs only. The other large files the audit lists — src/editor.rs (7,093) and pmacs-gpu/src/main.rs (6,761) — are separate future arcs with their own stable boundaries (editor command groups; GPU input / render-pipeline / layout). Bundling them here would violate the "one stable boundary at a time" discipline. Named as follow-ups, not started.

Categorical bets

• The compiler is the safety net. Because the split is pure motion, every real breakage is a compile error (missing item, private item) or a failing test — there is no silent behavioral drift to hunt for. That is what makes a mechanical refactor of this size tractable.
• Widen visibility exactly as far as the move forces, no further. pub(crate)/pub(super) over pub; the goal is the same encapsulation in more files, not a newly-public surface.
• Stable boundaries only. The pmacs.<domain> seams are API-shaped and long-lived; splitting along them ages well. Splitting by line-count or incidental adjacency would not.

Validation

Per tranche: cargo fmt clean; cargo clippy --all-targets clean under luajit and lua54; full cargo test green under both flavors (the 2,800-line test suite is the behavioral oracle — same tests, same outcomes, new locations). No Lua-side change, so the .lua fixtures and acceptance suites are untouched and must stay green.

As-built

Tranche 0 (this PR). git mv src/lua_bindings.rs src/lua_bindings/mod.rs (the pub mod lua_bindings; in lib.rs resolves to mod.rs unchanged), then extracted the pmacs.diag surface — diagnostic_to_lua + install_diag, moved verbatim — into src/lua_bindings/diag.rs (229 lines). mod.rs declares mod diag; and its one internal call site became diag::install_diag(...); diag.rs reaches shared-core items (BufferIdLua, SharedCore) via super:: and imports externals (SharedLspManager, crate::diag::*) directly. No visibility widening was needed — the child module sees the parent's items, and install_diag's only caller is mod.rs itself. No re-export needed (nothing external references it). mod.rs: 15,202 → 14,986 lines.

Validated: cargo fmt clean; clippy --lib clean under luajit and lua54; full lib suite 1437 passed / 0 failed under luajit (the tests, which drive pmacs.diag.* through the Lua VM, are the behavioral oracle — unchanged outcomes, code merely relocated).

Tranche 1 (this PR). Extracted pmacs.index (the project symbol-index surface) into src/lua_bindings/index.rs (390 lines) — the one remaining clean leaf (its 3 private helpers are used only within its own range). mod.rs declares mod index; and pub use index::{SharedProjectIndexer, make_project_indexer};, which keeps the crate::lua_bindings::… paths in editor.rs + completion_framework.rs (and an in-file completion-framework use) valid. index.rs has zero shared-core coupling — it depends only on crate::project_index, mlua, std, and reaches one stranded helper (lua_to_json, still in the lsp section) via super::. Verbatim move, no logic change. mod.rs: 14,986 → 14,603 lines.

While vetting the next leaves, discovered the recon under-counted the misplaced helpers (see the corrected tranche plan above): parse/theme, window, and minibuffer trail off into shared style/color, caller_ source, and command/menu helpers, so they need the helper-hoist tranche (now #2) before they can be extracted cleanly. This tranche stops at index rather than force those.

Validated: cargo fmt clean; clippy --lib clean under both flavors; full lib suite 1437 passed / 0 failed under both luajit and lua54.

Tranche 2 (this PR). Extracted pmacs.mcp (the MCP client surface) into src/lua_bindings/mcp.rs (602 lines), verbatim. All mcp items are self-contained; it reaches the JSON converters via super::json_to_lua/ lua_to_json and SharedProcessSupervisor via super:: — no hoist needed (see the corrected model in the tranche plan). mod.rs re-exports make_mcp_manager (external caller editor.rs) and McpServerIdLua — the latter to preserve the public-API path crate::lua_bindings:: McpServerIdLua (moving it into a private module had dropped it from the crate surface, surfacing as dead-code on id(); the split must not shrink the public API). mod.rs: 14,603 → 14,020 lines.

Review follow-up: the pub install_* wiring fns (install_diag, install_project_index, install_mcp) were pub before the split but weren't re-exported, silently dropping their crate::lua_bindings::* paths. They take crate-internal handle types (so no external caller can invoke them, and none does), but to keep the split strictly API-preserving they're now re-exported alongside the factories/handles — restoring the paths for the two already-merged tranches too. Deliberately narrowing them to pub(crate) is left as a separate, intentional change.

Validated: cargo fmt clean; clippy --lib clean under both flavors; full lib suite 1437 passed / 0 failed under both luajit and lua54.