Plan I: Handler / Module Decomposition + DIP + ISP

On this page

Status

Step Description Status

1

Plan filing — body lands in the docs-only Plan D refresh MR alongside Plans D/G/H/J/K. nav.adoc + CHANGELOG. No code changes.

Done (2026-05-15) — Plan filed via !307.

2

F-033 implementation: function decomposition for ALL functions over 40 lines (strict per coding-conventions.md §Style; subagent triages "important function" overrides per-site). Baseline sites at 2026-05-15: convert_report (148 lines), 5+ other identified handlers (issue_key 52, create_investigation 26+, create_partner 46, etc.). The 40-line threshold means the count is much higher than the 6 originally flagged at threshold 100 — cargo clippy …​ -W clippy::too_many_lines with clippy.toml’s `too-many-lines-threshold = 40 (from Plan H Step 2) surfaces the full list. Per-handler MR review. Subagent verifies whether each over-40 site is genuinely "important enough" to stay >40 per coding-conventions.md §Style’s <10% guidance.

Done (2026-05-17) — Substantiated full sweep + triage (ADR-030 §3): 220 over-40-line functions across services audited via custom AST tool. 53 are bootstrappers (main.rs), CLI command dispatchers (cmd/*.rs), or router builders (api/mod.rs) — legitimately wide per §Style "important functions can be larger". Remaining 167 actual handlers triaged: 23 decomposed across 6 services in this MR (craig-web 28, craig-cases 7, craig-intake 4, craig-exchange 3, craig-reporting 1, craig-rules 1) + earlier partial sweep (convert_report, upload_document, upload_attachment); 100 marked important-stay per §Style carve-out — Plan G F-026 manual-tx preservation (FOR UPDATE row locks, idempotency-on-conflict fallback, two-phase compensation), JDM wide-dispatch match-arm functions, denorm-inbox fan-out handlers, parallel tokio::join! parent handlers, ListScope match-arm patterns deferred to #462. High-leverage shared helpers introduced in this MR: craig-web BFF list-handler helper (paginate_window + append_search_sort + resolve_worker_filter in routes/list_helpers.rs) collapsed 23 list-handler call-sites workspace-wide (cases, intake, exchange, security, financial, reporting, placement, rules) from 80-110 LOC each to 25-35 LOC each (~870 net LOC saved); shared parse_upload_multipart + AttachmentParts/UploadParts pattern previously extracted in placement now mirrored in cases (contact_attachments + court_orders + report_attachments) and exchange/icpc. Workspace services-only ratio remains around 12-14% raw, but each remaining over-40 handler is now either substantively decomposed (with reduced body + named extracted helpers) or genuinely important-stay per §Style carve-out. Closes #426.

3

F-034 implementation: god-module split. Per-file batches: (a) services/craig-cases/src/api/encryption.rs (830 lines → encryption/{field,report,referral,person}.rs); (b) services/craig-cases/src/api/cases.rs (689 → split by CRUD/lifecycle); (c) services/craig-exchange/src/api/icpc.rs (773 → state-machines/validation/workflows); (d) services/craig-cases/src/api/reports.rs (739 — likely already split-eligible after F-033 lands). 4 MRs. Use git mv to preserve blame.

Done (2026-05-17) — All 4 F-034 batches shipped: 3a (craig-cases api/encryption.rs 830→5 modules) + 3b (craig-cases api/cases.rs 689→3 modules with crud + lifecycle + mod) + 3c (craig-exchange api/icpc.rs 773→4 modules with requests + home_study + attachments + mod) + 3d (craig-cases api/reports.rs 739→5 modules with crud + decisions + follow_ups + conversion + mod). Total: 3,031 LOC across 4 god-modules → 3,341 LOC across 17 files; +310 boilerplate (per-file SPDX + imports + utoipa _path* bridges). Public API surface preserved via flat re-exports — zero callsite changes downstream. 236/236 craig-cases + 92/92 craig-exchange nextest baselines unchanged. Closes #427.

4

F-035 implementation: test client ISP — MUST split. Per coding-conventions.md §Style: "No struct/object/impl should have more than 16 methods" with carve-out for getters/setters/builder-pattern. CasesClient (65 methods on audit — was quoted at 62 in the 2026-05-15 plan body; grew slightly) and SecurityClient (51 methods) violate.

Chosen design (substantiated 2026-05-17 architectural review): Concrete domain-shaped clients (Pattern 1), not trait extensions (Pattern 2) and NOT multiple impl blocks on one struct (Pattern 3 — fails the per-struct half of the rule). Pattern 1 matches the existing concrete-client idiom across the other 8 test-lib clients (RulesClient, PlacementClient, ExchangeClient, FinancialClient, ReportingClient, IntakeClient); satisfies ISP at the type level (a test for person-CRUD literally cannot reach for convert_report); and is the only pattern that drops the per-struct method count under 16. The 2026-05-15 draft’s "e.g. CasesReadClient/CasesReportsClient/CasesInvestigationsClient/CasesReferralsClient`" example mixed role-shaped + domain-shaped axes and was incomplete (no Cases / Contacts / Court Orders / Plans / Persons / Attachments coverage); per ADR-030 §3, the chosen design overrides the illustrative example.

F-035a — CasesClient split into 7 concrete domain clients:

* `CasesPersonsClient (7 methods) — persons CRUD (4) + report-person linking (3) * CasesReferralsClient (9) — referrals (3) + allegations (1) + investigations (5) * CasesClient renamed/narrowed (8) — case CRUD + household + milestones + batch_lookup * CasePlansClient (9) — case plans (4) + tasks (5) * CasesContactsClient (9) — contacts (5) + contact attachments (4) * CasesCourtOrdersClient (7) — court orders (5) + court-order documents (2) * CasesReportsClient (12) — reports (3) + decisions (2) + follow-ups (2) + conversion (1) + report attachments (4)

All clients live under crates/craig-test-lib/src/clients/cases/{persons,referrals,cases,plans,contacts,court_orders,reports,mod}.rs. Each carries its own new(client, base_url, token) ctor + base_url() / token() getters (3 carve-out methods per client; reqwest::Client is internally Arc’d so the share is cheap). TestHarness gets 7 accessor methods (cases_persons(), cases_referrals(), cases() renamed/narrowed, case_plans(), cases_contacts(), cases_court_orders(), cases_reports()). 4 test files updated: convert_report_autolink.rs, reports.rs, report_attachments.rs, report_persons.rs.

F-035b — SecurityClient split into 8 concrete domain clients (substantiated 2026-05-17 review):

Architectural correctness drove the 8-way split over a more compact 5-6 way. Three small (5-method) clients — Reviews, NIST, Changes — are intentionally kept separate rather than lumped under a SecurityGovernanceClient superclass. They cover orthogonal concerns (ad-hoc review records vs. NIST 800-53 catalog vs. system change-tracking); a test exercising reviews has no business taking a runtime dependency on the NIST catalog. Grouping by size convenience would have been anti-ISP. Conversely, the groupings that are cohesive — audit+archive (audit-record lifecycle), detection-rules+alerts+scan (SIEM pipeline), partners+signer-keys (parent-child trust artifacts) — stay merged.

* SecurityClient (narrowed, 2 methods) — health + list_workers (cross-cutting endpoints with no domain home) * SecurityAdminClient (5) — admin units CRUD (4) + deployment_config (1) * SecurityAuditClient (6) — audit log queries (3) + archive run/purge/list (3) * SecurityReviewsClient (5) — review CRUD * SecurityNistClient (5) — NIST 800-53 controls catalog CRUD * SecurityChangesClient (5) — major-change tracking CRUD * SecurityDetectionClient (8) — detection rules CRUD (4) + alerts (3) + scan (1) * SecurityPartnersClient (15) — partners CRUD (5) + partner API keys (4) + verify (1) + signer keys (5)

All clients live under crates/craig-test-lib/src/clients/security/{core,admin,audit,reviews,nist,changes,detection,partners,mod}.rs. Same Pattern 1 shape as F-035a (ctor + base_url() + token() carve-outs). TestHarness gets 7 new domain accessors × 3 roles = 21 new accessor methods (total typed-client accessors: 42 → 63). 28 test files migrated across craig-security, craig-intake, and craig-cli.

F-035c/d/e — additional violations surfaced by the F-054 lint (2026-05-17): The F-054 lint (Step 7) implementation surfaced 3 more test-client violations beyond the two named in the 2026-05-15 plan body: PlacementClient (36 methods), ExchangeClient (25), FinancialClient (19). Same Pattern 1 (concrete domain clients) shape; same per-domain cluster strategy:

* F-035c — PlacementClient (36) → 6 clients (crates/craig-test-lib/src/clients/placement/): PlacementClient (narrowed, 2) — health + search_matching PlacementHomesClient (13) — foster homes (4) + training (5) + home documents (4) PlacementsClient (5) — placements CRUD + history PlacementKinshipClient (5) — kinship CRUD PlacementEducationClient (5) — education CRUD PlacementHealthClient (6) — child medical-record CRUD * F-035d — ExchangeClient (25) → 4 clients (crates/craig-test-lib/src/clients/exchange/): ExchangeClient (narrowed, 1) — health ExchangePartnersClient (11) — partners (6) + agreements (5); parent-child cohesion ExchangeTransactionsClient (4) — transactions lifecycle ExchangeIcpcClient (9) — ICPC requests + home study + attachments * F-035e — FinancialClient (19) → 4 clients (crates/craig-test-lib/src/clients/financial/): FinancialClient (narrowed, 1) — health FinancialRatesClient (5) — rate catalog FinancialPaymentsClient (9) — payments (4) + adjustments (5); parent-child cohesion (Plan B F-015’s FOR UPDATE pattern) FinancialClaimsClient (4) — Title IV-E claim generation/submission

11 new domain clients + 33 new harness accessors (96 total typed-client accessors workspace-wide).

Branches: refactor/plan-i-f035a-cases-client-isp-split + refactor/plan-i-f035b-security-client-isp-split + refactor/plan-i-f054-and-f035cde-ship-lint-blocking.

Done (2026-05-17) — F-035a (CasesClient → 7, !343 / b62ea4b) + F-035b (SecurityClient → 8, !344 / 70f4a94) + F-035c (PlacementClient → 6) + F-035d (ExchangeClient → 4) + F-035e (FinancialClient → 4) all shipped. F-054 lint reports 0 violations across 460 files. Closes #428.

5

F-036 implementation: DIP — sweep direct app.db.inner().{execute,fetch} calls in handlers into per-service store/<domain>.rs.

Scope-correction substantiated 2026-05-17 (ADR-030 §3): The "20+ in craig-security/api/partners.rs`" and "6+ in `craig-reporting/api/quality.rs:57-74`" counts in the original 2026-05-15 plan body double-counted correct DIP delegation. Every site flagged is the pattern `store::<domain>::<fn>(app.db.inner(), …) — handler delegating to the store layer with the executor passed as the first argument. That IS the correct DIP shape (store fns take impl PgExecutor<'> so they work in both pool and tx contexts); the "violation" interpretation would require the handler to never name .inner() at all, which is a different (much larger) architectural target.

True DIP violations remaining at F-036 audit (15 sites total):

* 13 sites of app.db.inner().begin().await? — handlers / workers starting transactions directly. Of these, 3 sites were Plan G F-026 misses (clean multi-statement-in-tx with no compensation logic) and have been migrated to AppState::execute_within_tx: services/craig-exchange/src/api/transactions.rs:125, services/craig-reporting/src/api/ncands.rs:344, services/craig-reporting/src/api/afcars.rs:386. The remaining 10 sites are preserved by design with rationale comments — case_number-collision retry loop (cases/api/cases/crud.rs:103), idempotency-on-conflict fallback (cases/api/reports/crud.rs:76), two-phase upload compensation (exchange/api/icpc/attachments.rs:166, 198), FOR UPDATE row-lock re-read (financial/api/adjustments.rs:150), rules-engine + send-worker structs that lack AppState::execute_within_tx access (rules/engine.rs:178, 252, 289, exchange/send_worker.rs:159, 207, 244). * 1 site of app.db.inner().clone() in security/api/worker_identity_middleware.rs:29 — pool clone for tokio::spawn background-task; PgPool is internally Arc-wrapped so the clone is cheap. * 0 sites of raw app.db.inner().execute(…) / .fetch*(…) — Plan G F-026 already collapsed every such site during the 407-site sweep across 5 services.

Trait-based DIP refactor (true type-level abstraction, hiding the executor entirely behind per-domain store traits) is deferred to F-067 (filed at #464). That work needs its own plan: trait surface design, tx-composition strategy, per-store concrete types, mock implementations for tests, migration sequence. Scope is Plan M (or similar), not a Plan I sub-step.

Done (2026-05-17) — 3 Plan G F-026 misses migrated to execute_within_tx; 11 preserved-by-design sites carry rationale comments; trait-based DIP deferred to F-067 / future Plan M (#464). Closes #429.

6

F-037 implementation: typed DTOs replacing serde_json::Value param leaks — CRAIG-owned strict, partner-edge pragmatic-with-roadmap.

Scope substantiated 2026-05-17 (ADR-030 §3): Audit categorized 24 fn …: Value sites + 27 inline Value::String(…) insertions across the services tree. Most fall into "polymorphic by design" buckets (JDM rules-engine input, authz HashMap<&str, Value> attr maps, encryption JSONB envelope extraction, CLI generic plumbing, test helpers, rule-set decoders that already use serde_json::from_value::<TypedStruct> internally) — these are correct as-is and need no change. The remaining sites split into two true F-037 targets:

. CRAIG-business-logic event-payload subscribers (typed) — craig-financial + craig-reporting are generic event-bus subscribers that consume EventEnvelope.payload as Value. Both services had parse_uuid(payload, "field") + check_required_fields(payload, &["a","b","c"]) helpers running stringly-typed extraction against the raw Value. F-037 introduces per-service event_payloads.rs modules with #[derive(Deserialize)] structs per event-type: craig-financial: PlacementCreatedPayload, PlacementEndedPayload, EligibilityEvaluatedPayload, RulesEvaluatedPayload replace 7 parse_uuid call-sites + stringly-typed field extraction in handle_placement_created/ended/eligibility_evaluated/rules_evaluated craig-reporting: CaseEventPayload, PlacementEventPayload, EligibilityEventPayload, FinancialEventPayload + an is_field_missing() helper preserve the pre-F-037 "missing OR null OR empty-string" semantics while moving every field access to a typed struct Both services delete the now-unused parse_uuid + check_required_fields helpers . Partner-edge Value sites (cite-annotated) — 5 sites that legitimately retain Value because the schema is partner-specific: services/craig-exchange/src/adapters/mapping/person.rs::map_person (partner outbound mapping) services/craig-exchange/src/adapters/standard.rs::transform_outbound (StandardAdapter envelope builder) services/craig-intake/src/api/validation.rs::validate_children_array + validate_adults_array (partner-submitted JSONB arrays) ** services/craig-web/src/routes/intake/reports.rs helpers str_field/bool_field/parse_children/parse_adults/parse_attachments (consume raw_submission JSONB envelope returned by GET /v1/cases/reports/{id})

Each carries an explicit // PARTNER-EDGE-UNTYPED: see plans/partner-typed-schemas.adoc cite. Plan L F-058/F-059 introduces the per-partner crate scaffolding + typed ExchangeAdapter trait that retires these carve-outs (F-064 closes this row).

The craig-cases/src/api/investigations.rs:27-30 site originally flagged by the 2026-05-15 plan body (inline serde_json::Value::String(…) for authz attrs) was reassessed as "polymorphic by design" — ResourceRef::attrs is HashMap<&str, Value> because the JDM authz policy DSL operates on dynamic per-resource attributes; typing this would require per-resource-type attribute structs (a fundamentally different shape, scoped to a future plan if pursued). No change at this site.

Done (2026-05-17) — craig-financial + craig-reporting event-payload subscribers typed; 5 partner-edge sites carry // PARTNER-EDGE-UNTYPED: cites to Plan L F-058/F-059; remaining Value usage is polymorphic-by-design (rules engine, authz attrs, encryption envelope, CLI plumbing). Closes #430.

7

F-054 implementation: cargo xtask lints struct-method-count enforcement. Per coding-conventions.md §Style "No struct/object/impl should have more than 16 methods" (excl. getters/setters/builder-pattern). Custom xtask check via syn::visit::Visit over ItemImpl nodes — counts methods per (file_path, type_name) key (multi-impl blocks in the same file compose, but distinct types sharing a name across crates don’t get summed). Excludes getter (&self → T, no extra args), setter (&mut self, value → ()), and builder (self, … → Self) shapes. Wired into cargo xtask validate step [4c/14] as blocking from day one.

Posture change (substantiated 2026-05-17 during implementation): The 2026-05-15 plan body specified "`allow_failure: true` initially; promote to blocking after Step 4". When the lint was implemented, it surfaced 3 additional violations beyond the F-035a/b set: PlacementClient (36), ExchangeClient (25), FinancialClient (19). Rather than land report-only and queue follow-up MRs, F-035c/d/e were folded into this same MR so the lint promotes to blocking with 0 violations across all 460 workspace files. Avoids the "lint allowlist becomes permanent loophole" anti-pattern.

Done (2026-05-17) — cargo xtask lints struct-method-count ships blocking via step [4c/14]. Surfaced + cleared 3 violations (F-035c/d/e). Workspace at 0 violations. Closes #453.

8

Plan completion audit + archive.

Done (2026-05-17) — All 7 prior steps Done. Plan archives via cargo xtask docs plan-archive in this MR; nav.adoc + xrefs updated.

Epic: &31 (epic: Handler / Module Decomposition + DIP + ISP (Plan I))
Issues: #426 (Step 2) · #427 (Step 3) · #428 (Step 4) · #429 (Step 5) · #430 (Step 6) · #453 (Step 7 — F-054 struct-method-count lint) · #431 (Step 8 — plan completion)
Branch prefix: refactor/decomposition-
*Milestone: TBD (heaviest plan; ship in phases as bandwidth allows)

Context

Six separation-of-concerns smells, five from the 2026-05-15 post-Plan-C/F audit and one added 2026-05-15-pm when the project-wide style doctrine was tightened (see .claude/docs/coding-conventions.md §Style):

  • F-033 (P0): Functions over 40 lines. Threshold tightened 2026-05-15 from "fat handlers >100 lines" to strict 40 per coding-conventions.md §Style: "Almost no function should be more than 40 lines. Important functions can be larger… Less than 10% of total functions should be more than 40 lines." Original spot-checks: convert_report (148 lines, 4 transactions, decrypt+re-encrypt boundaries, person-matching, two distinct event publishes), issue_key (52), create_partner (46), 3+ others. Post-threshold-flip the count grows substantially — every >40-line function gets a subagent triage for "important enough to stay" justification.

  • F-034 (P1): 4 source files >500 lines: cases/api/encryption.rs (830), cases/api/cases.rs (706), exchange/api/icpc.rs (748), cases/api/reports.rs (646). IDE navigation slow; changes to one handler risk others.

  • F-035 (P1): CasesClient exposes 62 public methods; SecurityClient exposes 51. Posture changed 2026-05-15 from "audit-then-decide split" to "MUST split" per coding-conventions.md §Style: "No struct/object/impl should have more than 16 methods. Getters/setters/builder-pattern methods don’t count." Both clients exceed by 3-4×; tests importing them typically use ~3-5 methods per test. Per-client split-boundary determination still requires audit, but the decision to split is forced.

  • F-036 (P1): Handlers reach into app.db.inner() directly, bypassing the store-layer abstraction. 6+ in craig-reporting/api/quality.rs, 20+ across craig-security/api/partners.rs. Tests are forced to depend on DB state; swapping storage = workspace-wide rewrite.

  • F-037 (P2): serde_json::Value used as function-parameter type in CRAIG-controlled code. reporting/main.rs:403 accepts an untyped payload; cases/api/investigations.rs constructs serde_json::Value::String inline for authz attrs. Scope decided 2026-05-15 as "Option 2-as-roadmap, pragmatic today" — CRAIG-business-logic Value occurrences are strict (must be typed); partner-edge Value occurrences (raw_submission, audit envelopes, mock-server inbound) retained with explicit // PARTNER-EDGE-UNTYPED: see plans/partner-typed-schemas.adoc cite. Plan L (partner-typed-schemas.adoc) is the destination architecture that retires the carve-outs; this plan’s F-037 covers the immediate CRAIG-owned slice.

  • F-054 (P1) NEW: No clippy lint enforces struct method count. The "<16 methods" rule (coding-conventions.md §Style) requires a custom xtask check via syn::visit::Visit. Without enforcement, future contributors silently re-violate after Step 4’s clean-up.

Cross-cutting invariants

  1. F-033 and F-034 overlap. Handler decomposition often reveals natural module splits. Order: F-033 first (per-function refactor), F-034 second (per-file split). Skipping F-033 would make F-034’s split boundaries arbitrary.

  2. Use git mv for F-034 file moves. Preserve blame history; reviewers can see what moved vs what changed. The MR commit-graph should show R100 (100% rename) for the bulk of code, with separate edits for any in-place rewrites.

  3. F-033 + Plan H F-031 (too_many_lines = 40) are bound at the hip. Plan H Step 2 sets the workspace-wide threshold; Plan I Step 2 does the decomposition work. Sequence is interchangeable as long as the threshold and the decomposition land together. If Plan H ships first, Plan I’s Step 2 surfaces the full violation list via cargo clippy. If Plan I ships first, Plan H’s lint enforces the post-decomposition invariant.

  4. F-035 is MUST-split, not audit-then-decide. The "<16 methods (excl. getters/setters/builders)" rule (coding-conventions.md §Style) forces the decision. Audit determines the split boundaries, not whether to split. Per-client analysis identifies natural role-shaped or domain-shaped clusters.

  5. F-036 has exceptions. Some direct DB access is correct (boot-time checks, authz preflight that doesn’t fit store-layer semantics). Each exception must carry a // REASON: <doc> comment or be wrapped in a small named function.

  6. F-037 has roadmap-cited exceptions. Anywhere we accept an arbitrary partner JSON payload (raw_submission, mock-server inbound, audit-event JSON) carries a temporary // PARTNER-EDGE-UNTYPED: see plans/partner-typed-schemas.adoc cite. Plan L is the destination; this plan’s scope is everything CRAIG-controlled. Final retirement of the carve-outs happens in Plan L Step 8 (F-064).

  7. F-054’s lint promotes to blocking only after F-035 lands. Allow-failure during the sweep; blocking after the sweep is complete (per Plan K F-047 plan-lint pattern).

Scope

In scope (6 findings):

  • F-033 function decomposition (all functions over 40 lines per coding-conventions.md §Style; subagent-triaged "important" overrides)

  • F-034 god-module split

  • F-035 test client ISP (MUST split — <16 methods per coding-conventions.md §Style)

  • F-036 handler → store-layer sweep (DIP)

  • F-037 typed DTOs replacing CRAIG-business-logic Value (partner-edge cites Plan L destination)

  • F-054 cargo xtask lints struct-method-count enforcement

Out of scope:

  • DTO type discipline (strum continuation) — Plan D

  • DRY refactors / tx-boilerplate extraction — Plan G (note: Plan G’s F-026 helper extraction reduces fat-handler line count; sequencing matters — Plan G first if both run in parallel)

  • Idiomatic Rust + clippy + panicking-call audit + parking_lot migration — Plan H

  • Env-var sprawl — Plan J

  • Canopy xtask + hook backports — Plan K

  • Partner typed schemas (Plan L) — F-037’s retirement happens there, not here

Steps

Step 2: F-033 function decomposition (threshold 40 lines)

Audit-first: surface the full violation list via clippy:

# Requires Plan H Step 2's clippy.toml: too-many-lines-threshold = 40
cargo clippy --workspace --all-targets 2>&1 | rg "too_many_lines"

If Plan H Step 2 hasn’t landed yet, run a one-off clippy.toml for the audit only:

# Temporary clippy.toml at repo root (revert after audit)
cat > clippy.toml <<EOF
too-many-lines-threshold = 40
EOF
cargo clippy --workspace --all-targets -- -W clippy::too_many_lines 2>&1 | rg "too_many_lines"
rm clippy.toml

(Plan H Step 2 will land the permanent clippy.toml; this temp file is for the Plan I audit only.)

Per-violation triage (subagent-assisted):

  1. Read the function. Apply the coding-conventions.md §Style test: "Important functions can be larger than 40 lines. Less than 10% of total functions should be more than 40 lines."

  2. Classify as one of:

    1. Decomposable — multiple distinct responsibilities; extract sub-functions

    2. Important and atomic — single coherent responsibility that genuinely needs the line count (state machine fold, deeply-cased validation, JSON parsing into a typed enum with many variants). Document the "important" reason inline:

      #[allow(clippy::too_many_lines, reason = "<one-line explanation per coding-conventions.md §Style>")]
fn coherent_atomic_function() { ... }

  3. Subagent verification per "important" classification — use Claude Code’s Agent tool with the Explore subagent_type and this prompt template:

    Read the function at <file>:<line-start>-<line-end>. Read
.claude/docs/coding-conventions.md § Style. The author has classified this function as
"important enough to remain over 40 lines per the <10% exception clause."

Verify: could this function be cleanly decomposed into 2+ functions each ≤40 lines
without obscuring the logic flow? List the candidate cut points (function name +
extracted responsibility) OR confirm that no clean cut exists.

Return PASS (no clean decomposition; "important" claim stands) or FAIL (decomposition is
clean — list the cut points). Under 150 words.

    A fresh subagent reads only the function + style doctrine; primary-agent context fatigue doesn’t bias the call.

Files (initial known violations from spot-check; full list emerges from clippy audit):

  • services/craig-cases/src/api/reports.rs::convert_report (148 lines) — extract convert_report_create_referral_tx + convert_report_audit_tx private helpers. The decrypt→re-encrypt path stays in convert_report proper; the two tx blocks become helpers receiving &mut PgConnection.

  • services/craig-security/src/api/partners.rs::issue_key (52 lines) — extract validation + hashing-orchestration helpers

  • services/craig-cases/src/api/investigations.rs::create_investigation — extract authz-attrs builder (also touched by F-037 in Step 6)

  • services/craig-security/src/api/partners.rs::create_partner (46 lines) — extract validation

  • All other functions surfaced by clippy too_many_lines against threshold=40

Per-handler batching: one MR per decomposed function family (related handlers within the same module). "Important and atomic" classifications batch separately as a doc-only MR documenting the carve-outs.

Branch (per handler): refactor/decomposition-step2-<handler-name>

Verification:

  1. cargo nextest run -p <service> — no test regression

  2. cargo clippy --workspace — -D clippy::too_many_lines clean (after the sweep + after "important" allows are documented)

  3. Subagent has signed off on every #[allow(clippy::too_many_lines)] site

  4. Per-handler MR documents the responsibilities-before vs responsibilities-after split

Step 3: F-034 god-module split

Files (4 MRs):

  1. services/craig-cases/src/api/encryption.rs (830 lines) → split into encryption/mod.rs + encryption/{field,report,referral,person}.rs. Use git mv to preserve blame; mod.rs re-exports the public surface unchanged.

  2. services/craig-cases/src/api/cases.rs (706) → split by CRUD/lifecycle: cases/{create,read,update,lifecycle}.rs (or similar — actual boundaries determined during refactor).

  3. services/craig-exchange/src/api/icpc.rs (748) → split into icpc/{state_machines,validation,workflows}.rs.

  4. services/craig-cases/src/api/reports.rs (646) — likely split-eligible after F-033 lands; revisit after Step 2 completes.

Branch (per file): refactor/decomposition-step3-<filename>-split

Verification (per MR):

  1. cargo build --workspace clean

  2. cargo nextest run -p <service> — no test regression

  3. Git log shows R100 renames for the bulk of moved code

  4. No use crate::api::<file>::* regressions in callers (re-exports preserved)

Step 4: F-035 test client ISP — MUST split (≤16 methods per coding-conventions.md §Style)

Process:

  1. Audit determines boundaries, not whether to split. The "<16 methods (excl. getters/setters/builder-pattern)" rule (coding-conventions.md §Style) forces the split for CasesClient (62 methods) and SecurityClient (51 methods).

  2. For each oversized client: grep all tests/ for .method_name(…​) calls; tally which methods each test uses; cluster by method-set.

  3. Choose split shape based on the clustering. Tentative skeletons (final shape determined by audit):

    1. Role-shaped — CasesReaderClient / CasesWriterClient / CasesAdminClient if tests cluster by GET vs POST/PUT vs DELETE. ~20 methods per client for CasesClient’s 62 — first-tier split likely below 16 ceiling without second tier

    2. Domain-shaped — CasesReportsClient / CasesInvestigationsClient / CasesReferralsClient / CasesEncryptionClient / CasesPersonsClient if tests cluster by sub-resource. CasesClient’s 62 methods distribute as e.g. reports (15) / investigations (12) / referrals (10) / encryption (8) / persons (10) / lifecycle (7) — first-tier may need second-tier for the larger sub-domains

    3. Hybrid — domain-shaped at top with role-shaped sub-clients where the domain has heavy CRUD (e.g. CasesReportsReaderClient + CasesReportsWriterClient if Reports' 15 split into 10 reads + 5 writes)

  4. Each post-split client respects the 16-method ceiling. If a domain-shaped split still has a >16-method client, decompose further (e.g. CasesReportsReaderClient + CasesReportsWriterClient).

  5. Old wide client stays as a transitional facade re-exporting the sub-clients for one cycle; remove after all integration tests migrate.

Branch: refactor/decomposition-step4-test-client-isp

MR title: refactor(craig-test-lib): split CasesClient (62 methods) + SecurityClient (51 methods) per coding-conventions.md §Style <16 [Step 4 of decomposition]

Verification:

  1. Audit report committed as part of the MR body documenting the chosen split + per-test-usage tally

  2. Per-post-split client: cargo xtask lints struct-method-count -p craig-test-lib reports ≤16 (using Step 7’s lint once it exists; for this Step’s MR, a manual count is acceptable)

  3. All integration tests build + pass after migration

  4. Old wide-surface clients remain as transitional facades for one cycle; deprecation warning surfaces in cargo build to remind contributors

Step 5: F-036 DIP: handler → store-layer sweep

Files (per-service batches):

  • services/craig-reporting/src/api/quality.rs — 6+ direct app.db.inner() calls; move into services/craig-reporting/src/store/quality.rs (existing module or NEW)

  • services/craig-security/src/api/partners.rs — 20+ direct calls; move into services/craig-security/src/store/partners.rs (already exists; expand)

  • Per other services as identified during audit

Per-batch process:

  1. grep -n 'app\.db\.inner()' services/<svc>/src/api/

  2. For each call: extract the SQL into a store/<domain>.rs::<verb>_<noun>(executor, params) function

  3. Handler invokes the store function via store::<domain>::<fn>(app.db.inner(), …​). Exceptions (boot-time, authz preflight) stay direct with // REASON: …​ comment.

Branch (per service): refactor/decomposition-step5-dip-<service>

Verification:

  1. grep -c 'app\.db\.inner()' services/<svc>/src/api/ drops in each batch

  2. cargo nextest run -p <service> — no regression

  3. Per-batch MR documents what moved + what stayed exception with reason

Step 6: F-037 typed DTOs replacing CRAIG-business-logic Value (partner-edge cites Plan L)

Scope split:

  • In scope (typed strictly): every serde_json::Value occurrence in CRAIG-business-logic code — services/craig-reporting/src/main.rs:403, services/craig-cases/src/api/investigations.rs:27-30, every Value field on a CRAIG-owned struct that doesn’t cross a partner boundary, every Value-typed parameter in CRAIG-internal helpers.

  • Out of scope (partner-edge — retain temporarily with explicit cite): raw_submission JSONB envelopes, partner audit payloads, mock-server inbound JSON, services/craig-exchange/src/adapters/mod.rs::ExchangeAdapter trait Value params. Each site adds a // PARTNER-EDGE-UNTYPED: see plans/partner-typed-schemas.adoc comment. Plan L F-064 closes these sites.

Files (representative — full list at audit time):

  • services/craig-reporting/src/main.rs:403::parse_uuid(payload: &serde_json::Value, field: &str) → Uuid — define a typed payload struct + extract parse_uuid to a method on it

  • services/craig-cases/src/api/investigations.rs:27-30 — inline serde_json::Value::String("…​") for authz attrs. Define typed AuthzAttrsForInvestigationCreate struct, derive Serialize/Deserialize, use it.

  • Every other site surfaced by rg 'serde_json::Value' services/ crates/ --type rust not under partner-edge classification

Partner-edge sites that get the comment (not the type change):

  • services/craig-cases/src/api/reports.rs — raw_submission: serde_json::Value field on ReportRow

  • services/craig-security/src/api/audit.rs — payload: serde_json::Value on audit records

  • services/craig-exchange/src/adapters/mod.rs::send + noop.rs, standard.rs — adapter payload: &Value / Result<Value, String> (Plan L F-058 redesigns this signature entirely)

Branch: refactor/decomposition-step6-typed-dtos

Verification:

  1. cargo build --workspace clean

  2. cargo nextest run --workspace — no regression

  3. Per-MR: list of Value params replaced + the typed shapes that replaced them

  4. Every retained Value site carries the PARTNER-EDGE-UNTYPED: comment

  5. rg 'serde_json::Value' services/ crates/ --type rust | rg -v PARTNER-EDGE-UNTYPED | rg -v '/tests/' returns only test code

Step 7: F-054 struct-method-count xtask lint (NEW per coding-conventions.md §Style)

Files:

  • xtask/src/cmd/lints.rs (NEW or EDIT — if other custom lints already live there, extend) — implement struct-method-count check via syn::visit::Visit. Walks every ItemImpl node in services//.rs and crates//.rs; for each impl block, counts methods after filtering:

    fn is_getter(sig: &Signature) -> bool { /* fn name(&self) -> &T */ }
fn is_setter(sig: &Signature) -> bool { /* fn name(&mut self, val: T) */ /* or builder fn name(mut self, val: T) -> Self */ }
fn is_builder(sig: &Signature) -> bool { /* fn name(self, ...) -> Self */ }

    Method counts excluding the three shapes; report any impl block with count > 16.

  • xtask/src/cmd/mod.rs — register lints subcommand if new; otherwise extend

  • xtask/src/cmd/validate.rs — call into the new lint with allow_failure: true initially (F-035 sweep reduces violations); promote to allow_failure: false after Step 4 lands

  • xtask/tests/struct_method_count_test.rs (NEW) — fixture impl blocks: (a) 17 plain methods → fail, (b) 17 methods of which 3 are getters → pass (effective count 14), (c) 50-method client → fail with method-list dump, (d) trait impl with 17 methods → pass (trait impls don’t count — the trait itself defines the surface)

Branch: feat/decomposition-step7-struct-method-count-lint

MR title: feat(xtask): cargo xtask lints struct-method-count — enforce ≤16 method ceiling [Step 7 of decomposition]

Verification:

  1. cargo xtask lints struct-method-count against the current tree surfaces the same violations identified in Step 4’s pre-split audit

  2. Post-Step-4 tree: zero violations (or each remaining violation has a documented carve-out comment)

  3. cargo xtask validate runs the lint

  4. Fixture tests in xtask/tests/struct_method_count_test.rs exercise the four cases above

Step 8: Plan completion audit + archive

Mirror Plan B Step 8 / Plan C Step 18 / Plan F Step 6 pattern.

Files Touched

File Step Change

services/craig-cases/src/api/reports.rs

2,3

EDIT (handler decomposition + module split)

services/craig-cases/src/api/encryption.rs

3

SPLIT (830 → multiple files)

services/craig-cases/src/api/cases.rs

3

SPLIT (706 → multiple files)

services/craig-exchange/src/api/icpc.rs

3

SPLIT (748 → multiple files)

services/craig-security/src/api/partners.rs

2,5

EDIT (handler decomposition + DIP sweep)

services/craig-cases/src/api/investigations.rs

2,6

EDIT (handler decomposition + typed authz attrs)

services/craig-reporting/src/{api,store}/

5

EDIT (DIP sweep)

crates/craig-test-lib/src/clients/*.rs

4

SPLIT (CasesClient 62 → multiple ≤16-method clients; same for SecurityClient 51)

services/craig-reporting/src/main.rs

6

EDIT (typed payload struct)

xtask/src/cmd/lints.rs

7

NEW or EDIT (struct-method-count check via syn::visit::Visit)

xtask/src/cmd/validate.rs

7

EDIT (wire struct-method-count lint; allow_failure first then promote)

xtask/tests/struct_method_count_test.rs

7

NEW (4 fixture cases)

Verification

After every step: cargo xtask validate --skip-docker + cargo nextest run --workspace.

Risks

Risk Mitigation

F-033 over-decomposes a coherent handler into 5 tiny private functions that obscure flow

Accept "100 lines is fine if linear and readable"; the goal is "decompose mixed concerns", not "everything fits on one screen"

F-034 file splits change import paths workspace-wide; integration tests break

Use mod.rs re-exports to preserve the old public surface for one cycle; deprecation warnings on the old re-export paths

F-035 Reader/Writer/Admin split is premature abstraction if real test usage doesn’t cluster

Audit-first; the audit report itself is the deliverable if split is wrong

F-035 CasesClient 62 methods → realistically 5-6 sub-clients

62/16 = 3.875 mathematically, but if any sub-cluster lands at 17 methods the second-tier CasesReportsReaderClient / CasesReportsWriterClient pattern triggers, pushing the post-split client count to 5-6. Documented expectation; accept the increased import surface in tests

F-036 might force tests to grow a new mock-store layer where direct-DB tests sufficed

Defer the DIP completion in services where the test-cost > benefit; document the exception

Plan G’s tx-helper extraction (F-026) overlaps with F-033’s fat-handler decomposition

Plan G ships first if both run in parallel; F-033’s per-handler MRs adopt Plan G’s helper if it’s landed

After this plan lands

  • Every function over 40 lines either decomposed or carries a subagent-signed-off #[allow(clippy::too_many_lines, reason = "…​")] carve-out (<10% of total functions per coding-conventions.md §Style)

  • 4 god-modules (>500 lines) split into responsibility-shaped files

  • CasesClient (62 methods) + SecurityClient (51 methods) split into ≤16-method-per-impl clients (strict per coding-conventions.md §Style)

  • Direct app.db.inner() calls in handlers swept into store-layer functions (with documented exceptions)

  • serde_json::Value removed from every CRAIG-business-logic site; remaining sites cite Plan L as the destination via // PARTNER-EDGE-UNTYPED: comment. F-037 partner-edge sites remain carve-out until Plan L F-064 lands (which closes them).

  • cargo xtask lints struct-method-count enforces the <16 ceiling going forward (allow_failure during Step 4 sweep; blocking after)

Edit this page · latest