refactor(serve): split edda-serve/src/lib.rs into sub-modules

[fagemx]

Problem

edda-serve/src/lib.rs is 8,722 lines with 12 distinct concerns mixed into a single file. It is both the largest file and one of the most frequently changed (churn hotspot), creating merge conflict risk and poor navigability.

Mixed concerns include: auth/pairing, decisions, drafts, events, telemetry, snapshots, analytics, metrics, policy, sync, dashboard/UI, ingestion, SSE streaming.

Proposed Structure

src/
├── lib.rs              (~100 lines: re-exports, serve() entrypoint)
├── error.rs            (~50 lines: AppError enum)
├── state.rs            (~50 lines: AppState, config)
├── middleware.rs        (~100 lines: auth middleware)
├── api/
│   ├── mod.rs          (router setup)
│   ├── decisions.rs    (~350 lines)
│   ├── drafts.rs       (~500 lines)
│   ├── events.rs       (~220 lines)
│   ├── telemetry.rs    (~310 lines)
│   ├── snapshots.rs    (~315 lines)
│   ├── analytics.rs    (~275 lines)
│   ├── metrics.rs      (~335 lines)
│   ├── sync.rs         (~110 lines)
│   ├── briefs.rs       (~405 lines)
│   ├── ingestion.rs    (~280 lines)
│   └── stream.rs       (~120 lines)

Notes

• No sub-modules currently exist — this would be the first split
• handle_draft_action (245 lines) is the largest single function
• Tests (~4,300 lines) should mirror the module split
• Pure refactor — no behavior change

[fagemx]

Research: Split edda-serve/src/lib.rs into Sub-Modules (GH-375)

Current State

File: crates/edda-serve/src/lib.rs — 8,781 lines, single file, no sub-modules.

External consumers: Only edda-cli/src/cmd_serve.rs imports from this crate, using edda_serve::ServeConfig and edda_serve::serve().

Test section: Lines 4398–8781 (~4,383 lines, 112 #[tokio::test] functions).

Handler section: Lines 1–4395 (~4,395 lines of production code).

Identified Concerns (12 Domains)

1. Config + State + Error (lines 41–131)

• ServeConfig (pub), AppState, PairingRequest, ChronicleContext
• AppError enum + From impls + IntoResponse
• ~90 lines

2. Auth/Pairing (lines 359–722)

• auth_middleware, is_localhost, generate_pairing_token
• Pairing endpoints: create_pairing, complete_pairing, list_paired_devices, revoke_device, revoke_all_devices
• Request/response structs: CreatePairingRequest, CompletePairingResponse, DeviceInfo, RevokeDeviceRequest
• ~365 lines

3. Decisions (lines 787–1078)

• get_decisions, post_decisions_batch, get_decision_outcomes, get_decision_chain
• validate_iso8601 helper (shared with snapshots at line 2398)
• Structs: DecisionsQuery, BatchQuery, BatchSubQuery, BatchResponse, ChainQuery, ChainResponse, ChainNodeResponse, ChainMeta
• ~290 lines

4. Drafts (lines 1158–1655)

• get_drafts, post_draft_approve, post_draft_deny, handle_draft_action (245 lines, largest fn)
• Helpers: load_agent_phase_states, recent_decision_summaries, recent_commit_summaries
• Structs: DraftItem, DraftsResponse, MinimalDraft, MinimalStage, ApproveRequest, ApprovalResponse
• ~500 lines

5. Events / Notes / Decisions Write (lines 724–786, 1657–1880)

• health, get_status, get_context, post_note, post_decide, post_karvi_event
• get_log
• Structs: StatusResponse, LastCommit, ContextQuery, ContextResponse, NoteBody, EventResponse, DecideBody, DecideResponse, KarviEventBody, KarviEventResponse, LogQuery, LogEntry, LogResponse
• ~380 lines

6. Telemetry (lines 1881–2192)

• post_telemetry, get_telemetry, get_telemetry_stats, compute_telemetry_stats
• Structs: TelemetryBody, TelemetryOp, TelemetryTokenUsage, TelemetryCost, TelemetryResponse, TelemetryQuery, TelemetryStatsQuery
• ~310 lines

7. Snapshots (lines 2193–2508)

• post_snapshot, get_snapshots, get_snapshots_by_hash, reconstruct_snapshot
• Uses validate_iso8601 from decisions
• Structs: SnapshotBody, SnapshotResponse, SnapshotsQuery
• ~315 lines

8. Analytics / Recap / Overview (lines 2509–2805)

• get_recap, get_recap_cached, get_overview, get_projects
• Structs: RecapQuery, RecapAnchor, NeedsYouItem, DecisionItem, RelatedItem, RecapLayers, RecapMeta, RecapResponse, OverviewRedItem, OverviewYellowItem, OverviewGreenItem, OverviewResponse, ProjectStatus, ProjectsResponse
• OverviewResponse is shared with Dashboard
• ~300 lines

9. Actors + Briefs (lines 2806–2868, 3591–3658)

• get_actors, get_actor, get_briefs, get_brief
• Structs: ActorResponse, ActorsListResponse, BriefsQuery
• ~120 lines

10. Metrics + Dashboard (lines 2869–3202, 3660–3988)

• Quality: get_quality_metrics (QualityQuery)
• Controls: get_controls_suggestions, get_controls_patches, post_approve_controls_patch
• Metrics: get_metrics_overview, get_metrics_trends
• Dashboard: get_dashboard, serve_dashboard, compute_attention
• Structs: many (DashboardResponse, DashboardPeriod, DashboardSummary, TimelineEntry, MetricsOverviewResponse, MetricsTotals, TrendsQuery, TrendsResponse, TrendPoint, etc.)
• compute_attention shared between get_overview and get_dashboard
• ~650 lines

11. Policy (Scope/Authz/Approval) (lines 3203–3475)

• post_scope_check, get_scope_whitelist, post_authz_check, get_tool_tier, post_approval_check
• Structs: ScopeCheckBody, ScopeCheckResult, ScopeCheckResponse, WhitelistQuery, WhitelistClaim, WhitelistResponse, ApprovalCheckRequest
• ~275 lines

12. SSE Streaming (lines 3990–4112)

• get_event_stream, sse_event_name
• Struct: StreamParams
• ~120 lines

13. Ingestion (lines 4114–4394)

• post_ingestion_evaluate, post_ingestion_record, get_ingestion_records, get_ingestion_suggestions, post_suggestion_accept, post_suggestion_reject
• Helper: time_now_rfc3339
• Structs: EvaluateBody, EvaluateResponse, ManualIngestBody, IngestionRecordsQuery
• ~280 lines

14. Sync (test-only, lines 3478–3584)

• post_sync, sources_from_group, sources_from_name
• All gated behind #[cfg(test)]
• ~105 lines

Cross-Cutting Dependencies

Shared Item	Used By
AppState + open_ledger()	ALL handlers
AppError enum	ALL handlers
validate_iso8601()	decisions, snapshots, village_stats
compute_attention()	overview, dashboard
OverviewResponse + Red/Yellow/Green items	overview, dashboard
time_now_rfc3339()	ingestion only
reconstruct_snapshot()	snapshots only
load_agent_phase_states()	drafts only
recent_decision_summaries() / recent_commit_summaries()	drafts only
EventResponse	note, decide

Test-Handler Mapping

Tests follow no special grouping — they test endpoints sequentially. Key groupings by what they test:

• Health/status/context: ~3 tests
• Decisions: ~2 tests (+ batch/chain in later block)
• Drafts: ~1 test
• Overview/projects: ~2 tests
• Recap: ~2 tests
• Karvi events: ~6 tests
• Scope/whitelist: ~6 tests
• Authz: ~6 tests
• Dashboard/metrics: ~6 tests
• Actors: ~2 tests
• SSE stream: ~5 tests
• Snapshot: ~10+ tests
• Approval: ~5+ tests
• Telemetry: ~5+ tests
• Patterns/villages: ~3+ tests
• Ingestion: ~8 tests
• Decision chain/batch: ~5+ tests

Churn Analysis

Last 20 commits touching this file show continuous feature additions (telemetry, ingestion, patterns, village stats, error context). Each new feature adds ~200-400 lines. This is the highest-churn file in the crate.

Key Risks

1. Route registration duplication: Routes are registered in TWO places — serve() (line 155-231) and router() (line 271-357). Any split must preserve both.
2. Shared types: OverviewResponse, AppError, AppState are used across module boundaries.
3. Test infrastructure: setup_workspace, STORE_LOCK, StoreRootGuard, and the router() function are shared across all tests.
4. #[cfg(test)] sync code: Lines 3478-3584 are test-only production handlers — need to move with tests or stay gated.

[fagemx]

Innovation: Split edda-serve/src/lib.rs into Sub-Modules (GH-375)

Solution Approaches

Approach A: Issue's Proposed Structure (Flat api/ Module)

The issue proposes a flat api/ module with one file per domain. This is the most common axum pattern.

src/
├── lib.rs              (~150 lines: re-exports, serve(), router())
├── error.rs            (~80 lines: AppError enum + impls)
├── state.rs            (~40 lines: AppState, ServeConfig, ChronicleContext)
├── middleware.rs        (~100 lines: auth middleware, is_localhost, pairing auth)
├── helpers.rs           (~20 lines: validate_iso8601, time_now_rfc3339)
├── api/
│   ├── mod.rs          (~10 lines: pub mod declarations)
│   ├── auth.rs         (~365 lines: pairing endpoints)
│   ├── decisions.rs    (~290 lines)
│   ├── drafts.rs       (~500 lines)
│   ├── events.rs       (~380 lines: health, status, context, note, decide, karvi, log)
│   ├── telemetry.rs    (~310 lines)
│   ├── snapshots.rs    (~315 lines)
│   ├── analytics.rs    (~300 lines: recap, overview, projects)
│   ├── metrics.rs      (~650 lines: quality, controls, dashboard, trends, compute_attention)
│   ├── policy.rs       (~275 lines: scope, authz, approval, tool-tier)
│   ├── briefs.rs       (~120 lines: briefs + actors)
│   ├── stream.rs       (~120 lines: SSE)
│   └── ingestion.rs    (~280 lines)

Pros: Simple, flat, easy to navigate. Matches issue description closely.
Cons: metrics.rs at 650 lines is still large. events.rs is a grab-bag of unrelated handlers.

Approach B: Deeper Split (Separate Dashboard/Controls)

Split the oversized modules further:

src/
├── lib.rs
├── error.rs
├── state.rs
├── middleware.rs
├── helpers.rs
├── api/
│   ├── mod.rs
│   ├── auth.rs
│   ├── decisions.rs
│   ├── drafts.rs
│   ├── events.rs       (health, status, context, note, decide, log)
│   ├── karvi.rs         (karvi event handler, separated from events)
│   ├── telemetry.rs
│   ├── snapshots.rs
│   ├── analytics.rs     (recap, overview, projects)
│   ├── dashboard.rs     (dashboard + compute_attention)
│   ├── metrics.rs       (quality, trends, overview metrics)
│   ├── controls.rs      (controls suggestions, patches)
│   ├── policy.rs
│   ├── briefs.rs
│   ├── stream.rs
│   └── ingestion.rs

Pros: No module exceeds ~400 lines. Fine-grained ownership.
Cons: More files to navigate. compute_attention and OverviewResponse must be shared between analytics.rs and dashboard.rs.

Approach C: Router-Per-Module Pattern

Each module exports its own Router sub-tree, lib.rs merges them:

// api/decisions.rs
pub fn router() -> Router<Arc<AppState>> {
    Router::new()
        .route("/api/decisions", get(get_decisions))
        .route("/api/decisions/batch", post(post_decisions_batch))
        // ...
}

// lib.rs
let app = Router::new()
    .merge(api::decisions::router())
    .merge(api::telemetry::router())
    // ...

Pros: Each module owns its routes — no duplication. Eliminates the serve() vs router() duplication.
Cons: More structural change. Requires each module to import Arc<AppState>.

Evaluation

Criterion	A (Flat)	B (Deeper)	C (Router-Per-Module)
Complexity	Low	Medium	Medium
Merge conflict reduction	High	High	Highest
Route duplication fix	No	No	Yes
Max file size	650 lines	~400 lines	~400 lines
Diff readability	Best	Good	Good
Risk of regression	Lowest	Low	Low-Medium

Recommended Approach: A + C Hybrid

Use Approach A's module structure (matching the issue proposal) combined with Approach C's router-per-module pattern to fix the route duplication problem.

Each api/*.rs module:

1. Contains its handler functions and request/response types (private to module)
2. Exports a pub(crate) fn routes() -> Router<Arc<AppState>> function
3. Tests for that module live in a #[cfg(test)] mod tests inside that file

lib.rs becomes:

1. mod declarations
2. Re-exports of ServeConfig and serve()
3. The serve() function that merges all sub-routers
4. The test router() function that merges all sub-routers (identical logic, no auth middleware)

Handling Shared Types

Strategy: Put shared types in dedicated files at the src/ level, not inside api/:

• error.rs — AppError (used everywhere)
• state.rs — AppState, ServeConfig, ChronicleContext (used everywhere)
• helpers.rs — validate_iso8601, time_now_rfc3339 (used by 2-3 modules)

Types used by only one module stay in that module's file.

Cross-module types like OverviewResponse (used by both analytics and dashboard): keep in analytics.rs and have dashboard.rs import use super::analytics::OverviewResponse.

Handling Tests

Strategy: Tests move with their handlers.

• Each api/*.rs file gets its own #[cfg(test)] mod tests { ... } block
• Shared test infrastructure (setup_workspace, STORE_LOCK, StoreRootGuard) stays in a shared tests_common.rs or test_helpers.rs at src/ level, gated behind #[cfg(test)]
• The router() function moves to a #[cfg(test)] helper that all test modules share

Handling #[cfg(test)] Sync Code

The sync endpoints (lines 3478-3584) are test-only handlers registered in router(). They move into a sync.rs module that is entirely #[cfg(test)].

Key Design Decisions

1. pub(crate) visibility: Handler functions and types are pub(crate), not pub. Only ServeConfig and serve() are pub.
2. No new external dependencies: Pure file reorganization.
3. Router-per-module: Each module exports routes(), eliminating the dual-registration problem.
4. Tests co-located: Tests live next to the code they test, not in a giant monolithic test module.

[fagemx]

Plan: Split edda-serve/src/lib.rs into Sub-Modules (GH-375)

Overview

Pure refactor: split the 8,781-line edda-serve/src/lib.rs into sub-modules. No behavior change. Router-per-module pattern to eliminate route duplication.

Target Structure

crates/edda-serve/src/
  lib.rs              (~150 lines: mod declarations, serve(), #[cfg(test)] router())
  error.rs            (~80 lines: AppError enum, From impls, IntoResponse)
  state.rs            (~50 lines: ServeConfig, AppState, ChronicleContext, PairingRequest)
  helpers.rs           (~25 lines: validate_iso8601, time_now_rfc3339)
  middleware.rs        (~70 lines: auth_middleware, is_localhost)
  api/
    mod.rs          (~15 lines: pub mod declarations)
    auth.rs         (~365 lines: pairing endpoints + tests)
    decisions.rs    (~290 lines + tests)
    drafts.rs       (~500 lines + tests)
    events.rs       (~380 lines: health/status/context/note/decide/karvi/log + tests)
    telemetry.rs    (~310 lines + tests)
    snapshots.rs    (~315 lines + tests)
    analytics.rs    (~300 lines: recap/overview/projects + tests)
    metrics.rs      (~335 lines: quality/controls/trends/overview-metrics + tests)
    dashboard.rs    (~330 lines: dashboard/compute_attention + tests)
    policy.rs       (~275 lines: scope/authz/approval/tool-tier + tests)
    briefs.rs       (~120 lines: briefs + actors + tests)
    stream.rs       (~120 lines: SSE + tests)
    ingestion.rs    (~280 lines + tests)
  sync.rs             (~105 lines, #[cfg(test)] only: sync handler + helpers)

Execution Steps

The refactoring is done in a specific order to ensure the codebase compiles at every step.

Step 1: Create error.rs

Extract from lib.rs:

• AppError enum (lines 73-89)
• From<serde_json::Error>, From<serde_yaml::Error>, From<globset::Error>, From<std::io::Error> impls (lines 91-113)
• IntoResponse for AppError impl (lines 115-130)

Make AppError and its variants pub(crate).

In lib.rs: add mod error; and pub(crate) use error::AppError;

Verify: cargo check -p edda-serve

Step 2: Create state.rs

Extract from lib.rs:

• ServeConfig struct (lines 43-46) -- keep pub
• AppState struct (lines 50-54) -- pub(crate)
• PairingRequest struct (lines 56-59) -- pub(crate)
• ChronicleContext struct (lines 61-63) -- pub(crate)
• impl AppState block (lines 65-69) -- pub(crate)

In lib.rs: add mod state; and pub use state::ServeConfig; pub(crate) use state::AppState;

Verify: cargo check -p edda-serve

Step 3: Create helpers.rs

Extract from lib.rs:

• validate_iso8601 (line 807-811)
• time_now_rfc3339 (lines 4169-4173)

Both pub(crate).

In lib.rs: add mod helpers;

Verify: cargo check -p edda-serve

Step 4: Create middleware.rs

Extract from lib.rs:

• is_localhost (lines 362-376)
• generate_pairing_token (lines 379-385)
• auth_middleware (lines 388-424)

All pub(crate).

In lib.rs: add mod middleware;

Verify: cargo check -p edda-serve

Step 5: Create api/mod.rs and first handler module api/auth.rs

Create api/mod.rs with pub(crate) mod declarations.

Create api/auth.rs with:

• All pairing structs and handlers (lines 426-722)
• A pub(crate) fn routes() -> Router<Arc<AppState>> function that registers auth routes
• Move relevant tests from the test module

In lib.rs:

• Add mod api;
• Replace auth routes in serve() with .merge(api::auth::routes())
• Same for test router()

Verify: cargo check -p edda-serve and cargo test -p edda-serve

Steps 6-17: Extract remaining handler modules one at a time

Same pattern for each domain. Order chosen to minimize cross-module dependencies:

Step	Module	Routes	Key complexity
6	api/events.rs	health, status, context, note, decide, karvi, log	EventResponse shared between note and decide
7	api/decisions.rs	decisions, batch, outcomes, chain	validate_iso8601 imported from helpers
8	api/drafts.rs	drafts, approve, deny	Largest handler handle_draft_action (245 lines)
9	api/telemetry.rs	telemetry POST/GET, stats	compute_telemetry_stats helper
10	api/snapshots.rs	snapshot POST, snapshots GET, by-hash	reconstruct_snapshot helper; uses validate_iso8601
11	api/analytics.rs	recap, recap/cached, overview, projects	Defines OverviewResponse + Red/Yellow/Green types (shared with dashboard)
12	api/dashboard.rs	dashboard JSON, dashboard HTML	compute_attention helper; imports OverviewResponse from analytics
13	api/metrics.rs	quality, controls, overview-metrics, trends	Controls patches + suggestions
14	api/policy.rs	scope/check, whitelist, authz, approval, tool-tier	No internal deps
15	api/briefs.rs	briefs, brief by ID, actors	Simple delegations
16	api/stream.rs	events/stream SSE	Uses async_stream, tokio_stream
17	api/ingestion.rs	ingestion evaluate/records/suggestions	Uses time_now_rfc3339 from helpers

After each step: cargo check -p edda-serve + cargo test -p edda-serve

Step 18: Create sync.rs (test-only)

Extract the #[cfg(test)] sync handlers and helpers (lines 3478-3584). Gate the entire module behind #[cfg(test)].

Step 19: Extract test infrastructure

Create test_helpers.rs (gated behind #[cfg(test)]):

#[cfg(test)]
pub(crate) mod test_helpers {
    pub(crate) static STORE_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
    pub(crate) struct StoreRootGuard;
    // impl Drop for StoreRootGuard { ... }
    pub(crate) fn setup_workspace(dir: &Path) { ... }
    pub(crate) fn router(repo_root: &Path) -> Router { ... }
}

The router() function merges all api::*::routes() sub-routers plus sync test routes.

Step 20: Move tests to their modules

For each api/*.rs module, move the relevant tests from the monolithic mod tests into a #[cfg(test)] mod tests block inside that file.

Each test module imports from crate::test_helpers.

Step 21: Final cleanup

• Remove the now-empty monolithic mod tests from lib.rs
• Verify lib.rs is ~150 lines
• Run full test suite: cargo test -p edda-serve
• Run clippy: cargo clippy -p edda-serve
• Run fmt: cargo fmt -p edda-serve

Test Strategy

During refactoring

• cargo check -p edda-serve after every file extraction (compile check)
• cargo test -p edda-serve after every handler module extraction (behavior check)
• No test should be modified in logic -- only moved and imports updated

After refactoring

• cargo test -p edda-serve -- all 112 tests must pass
• cargo test --workspace -- no cross-crate regression
• cargo clippy --workspace -- zero warnings
• cargo fmt --check -- formatting clean

Verification criteria

• Exact same public API: edda_serve::serve() and edda_serve::ServeConfig
• Same number of routes registered
• Same test count (112 tests)
• No behavior change in any endpoint

Risk Mitigations

1. Route duplication eliminated: Both serve() and router() call api::*::routes(), single source of truth
2. Compile-at-every-step: Extract one module, verify, commit. If any step breaks, revert just that step
3. Import errors: The main risk is missing use statements. cargo check catches these instantly
4. Shared types: Centralized in error.rs, state.rs, helpers.rs, analytics.rs (for OverviewResponse)
5. Merge conflict window: This is a large refactor touching one file. Coordinate to avoid parallel edits to lib.rs during implementation

Estimated Effort

• ~21 atomic steps, each taking 5-15 minutes
• Total: ~3-4 hours of focused work
• Can be done in 2-3 commits:
  1. Foundation (error, state, helpers, middleware)
  2. Handler modules (api/*.rs) -- could be one large commit or split per-module
  3. Test migration

[fagemx]

Work completed. PR created: #383

Split the 8,781-line lib.rs into 16 focused files (4 foundation + 12 api handlers). All 106 tests pass, zero clippy warnings.