Skip to content

Review WS5: Split god-files (mcp.rs, main.rs, build.rs), add lib.rs, error-handling consistency #64

Description

@willem445

From the July 2026 codebase review (see docs/plan-codebase-review-recommendations.md). Priority: MEDIUM (structural; best done after the workspace + perf issues to avoid churn).

Tasks

  • Split src/sempkg/src/mcp.rs (3,291 lines) into mcp/{jsonrpc,schema,format,query,context}.rs — JSON-RPC transport loop, tool schemas, formatters, the UnifiedHit query pipeline (RRF/dedup/KWIC/merge), and McpContext + simple tool methods respectively.
  • Split src/sempkg/src/main.rs (2,535 lines) into commands/{add,sync,query,...}.rs. Dedupe first: BuildOptions construction is copy-pasted between add_from_github (main.rs:1662-1716) and add_from_local (main.rs:2298-2348) with an identical resolve_dirs closure repeated 6×; extract resolve_dirs() + a single build_and_install_bundle().
  • Split src/sembundle/src/build.rs (1,498 lines) into build/{codegraph,lance_docs,code_index,chunk,tools}.rs. While there: remove the dead _cwd param and never-used passthrough=false branch in invoke (build.rs:1383,1407-1417) — and note CodeGraph subprocess failures currently capture no stderr; fix that. Factor the near-identical chunk-emission tails of the two symbol extractors (build.rs:703-734 vs :900-929).
  • Add src/lib.rs to sempkg (currently bin-only) re-exporting modules, making main.rs a thin wrapper — unlocks tests/ integration tests for the currently untested store.rs, tool dispatch, and install/sync orchestration.
  • Error-handling consistency: delete 7 never-constructed SempkgError variants (error.rs: ManifestNotFound, PackageNotFound, InvalidBundle, NotIndexed, RerankerModelNotFound, Db, Reranker); pick one convention per layer (typed only where callers match on variants, else anyhow + .context()). In sembundle, add PackError::Lance/Db variants — InvalidField is currently abused for LanceDB/DB failures (build.rs:400,410,419,591,608,625,1070,1105,1121,1131), producing misleading "invalid field" errors. Consider consolidating the per-subcommand error enums (they're erased to Box<dyn Error> in main.rs anyway).

Verification

Pure refactor — full Rust test suite green, MCP functional suite green, no CLI output changes (CLAUDE.md requires output stability).

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestrustPull requests that update rust code

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions