Plan Q: Function Decomposition + Shared Abstractions Extraction

On this page

Status

Step Description Status

1

Plan filing + §Style doctrine update — body lands as a docs-only MR alongside epic + 11 step issues. nav.adoc Active gains Plan Q row; .claude/docs/coding-conventions.md § Style lines 14-19 replaced with "decompose unless decomposition makes the codebase worse" doctrine retiring the prior "important function" carve-out. Plan I + Plan G prior carve-outs flagged for re-evaluation. CHANGELOG Unreleased entry.

Done (2026-06-06) — !605 / b719a770

2

craig-bootstrap foundation — NEW workspace crate crates/craig-bootstrap/ providing helper fns (spawn_outbox_worker, init_object_store, build_shared_http_client, attach_standard_extensions, policy_ttl) + StandardExtensionContext struct + authz::boot_authz_engine + AuthzBootSpec. Absorbs the highest-duplication post-craig-api::bootstrap() setup blocks. Builder consumes BootstrapResult from existing craig-api::bootstrap(). Existing craig-api::bootstrap body stays at 136 LOC for Step 9.

Done (2026-06-06) — !606 / fe34d5dd

3

Service mains to final form (4 substeps; amended 2026-06-06) — original mono-step splits into 3a (extend bootstrap with build_authz_source + add AuthzContext to standard stack; retrofit cases/exchange/security), 3b (forward-migrate placement/financial/reporting/rules to full helper suite), 3c (extract setup_workers/setup_router sibling fns across all 7 stateful mains — actual B2 clear), 3d (intake/web N/A rationale + web router extraction). Sequential gate to Steps 4-9 per user directive "mains to final form before moving on."

Done (2026-06-06) — final form across 9 mains

3 batch 1

Bootstrap helper batch-1 adoption — partial migration of cases/exchange/security mains to consume the bootstrap helpers shipped in Step 2 (spawn_outbox_worker, init_object_store, build_shared_http_client, boot_authz_engine). Net -153 LOC across the 3 services. Adoption audit post-MR revealed attach_standard_extensions was built but unconsumed and build_authz_source was missing — both addressed by Step 3a.

Done (2026-06-06) — !607 / deeb5276

3a

Extend craig-bootstrap + retrofit batch-1 — NEW crates/craig-bootstrap/src/authz_source.rs with build_authz_source(http_client, env_prefix, service_token) → Arc<dyn RulesetSource> (collapses 12-16 LOC match service_token block duplicated across 5 services). Modify attach_standard_extensions to also construct AuthzContext from authz + jurisdiction (6 of 7 stateful services consume it; cases gains harmless unused layer). Make helper generic over Router<S> so services that have already applied .with_state(AppState) round-trip unchanged. Retrofit cases/exchange/security mains to consume both new APIs. subscribe_idempotent helper investigated + deferred — handler signature heterogeneity across 6 stateful services drops call-site reduction below 50% threshold.

Done (2026-06-06) — !608 / baa7542b

3b

Forward-migrate batch-2 mains — placement/financial/reporting/rules mains adopt full helper suite (spawn_outbox_worker, boot_authz_engine, build_authz_source (skip rules — uses DbRulesetSource), attach_standard_extensions, build_shared_http_client, init_object_store for reporting only). Per-service target ~95-110 LOC main. Issue #537.

Done (2026-06-06) — !609 / 812adfea

3c

Per-main setup_workers/setup_router extraction (actual B2 clear) — extract ServiceDeps struct + setup_workers(deps, shutdown) + setup_router(deps, settings) → Router sibling fns across all 7 stateful service mains. Target: each main ≤ 50 LOC orchestrator (bootstrap → ServiceDeps::build → setup_workers → setup_router → ApiServer::serve). 7 B2 violators cleared. Issue #538.

Done (2026-06-06) — !610 / 1fc3fc04 (B2 56→49 LOCKED)

3d

Finalize intake + web (different-shape services) — craig-intake already at 31 LOC main (stateless edge per ADR-017); add 4-LOC durable rationale comment explaining bootstrap N/A. craig-web 210 LOC main shrinks to ~80 LOC via protected_routes()/public_routes()/health_and_middleware() sibling fn extraction; add BFF-specific rationale comment. Cross-link from .claude/docs/services.md § craig-intake + § craig-web. 1 B2 violator cleared (web). Issue #539.

Done (2026-06-06) — !611 / ed42a4a9

4

SqlRow trait + seed builder refactor — NEW tools/craig-seed/src/sql_row.rs with trait SqlRow { fn render_row(&self, buf: &mut String); fn table_name() → &'static str; } + generic fn render_table<T: SqlRow>. Each entity in tools/craig-seed/src/datagen.rs implements SqlRow. The 5 render_X_sql fns in tools/craig-seed/src/sql.rs (200–350 LOC each) shrink to 5–20 LOC orchestrators. Regression: cargo xtask reseed produces byte-identical SQL output. 5 B2 violators cleared.

Done (2026-06-07) — !616 / e420b416 (B2 36→31 LOCKED; sql.rs 1245→131 LOC; 34 SqlRow impls; 4 hash-pinned tests for cases/placement/exchange/financial — reporting/security skipped due to pre-existing Utc::now() wall-clock non-determinism in datagen.rs)

5

Identity render Manifest + per-backend serializer — NEW xtask/src/cmd/identity/manifest.rs with canonical resource enumeration (Projects + Roles + Apps + Users). NEW Backend trait. The wide render fns in xtask/src/cmd/identity/render/{zitadel,kanidm,authentik,keycloak}.rs (213–244 LOC each) drop to ~80 LOC. Regression: hash-comparison test against pre-refactor output for each backend. 3 B2 violators cleared.

Done (2026-06-06) — !613 / 8234e251 (per-section sibling-fn extraction; B2 49→46; 4 hash-pinned tests)

6

CLI fat-arm decomposition (2–3 sub-MRs by command batch) — each pub async fn run(cmd, format, client) in services/craig-cli/src/cmd/*.rs (~18 modules) transitions from fat-arm (match cmd { Variant ⇒ { 30 lines } }) to thin dispatcher (match cmd { Variant(args) ⇒ run_variant(args, …​).await }). 18 B2 violators cleared. CRUD trait pattern investigated after first batch — if >50% of run_list/run_get/run_create helpers are structurally identical, propose trait CrudCommand; otherwise leave as standalone helpers. No macros.

Done (2026-06-07) — 3 batch MRs (!617 / !618 / batch 3 MR); 16 B2 sites cleared via Args-struct migration; CRUD trait deferred (signatures heterogeneous enough that the per-arm helpers are clearer; revisit if Plan H follow-up surfaces duplication); B2 36→15 LOCKED

7

xtask command run decomposition — 9 concrete sites per the Phase 1 B2 audit: xtask/src/cmd/validate.rs:16 run (345 LOC; 14-step orchestrator), xtask/src/cmd/security.rs:{51 run, 390 run_infra_tests, 642 run_path_fuzzing} (109+249+109 LOC), xtask/src/cmd/migrate.rs:375 rollback (144 LOC), xtask/src/cmd/e2e.rs:24 run (118 LOC), xtask/src/cmd/api_docs.rs:108 render_service_page (118 LOC), xtask/src/cmd/identity/verify.rs:157 run_with_probes (109 LOC), xtask/src/devstack_guard.rs:316 check_staleness (112 LOC). Extract per-phase / per-step helpers. ~6 B2 violators cleared.

Done (2026-06-06) — !614 / b2e8210b (B2 cleared 46→38 LOCKED; 8 sites under threshold)

8

B1 service-api module decomposition — the 3 closest-to-threshold B1 violators dropped below 500 LOC via helper extraction: services/craig-placement/src/api/health.rs (504), services/craig-cases/src/api/court_orders.rs (511), services/craig-exchange/src/api/partners.rs (518). Coordinate with Plan I F-065 if any overlap with #462 cross-handler scan. 3 B1 violators cleared.

Done (2026-06-06) — !612 / 02ccdb0c (B1 cleared 11→8 LOCKED)

9

Residual large-fn decomposition — crates/craig-auth/src/introspection.rs:464 IntrospectionClient::new (203 LOC) split into composition + cache/metrics + HTTP client + config; crates/craig-common/src/error.rs:317 ApiError::into_response (131 LOC) extracts per-variant RFC9457Problem::from(variant) helpers; crates/craig-api/src/bootstrap.rs:149 bootstrap() (136 LOC) decomposes into 4 stage helpers (telemetry+settings / db+auth / mq channels / service+actor tokens). Plus B2 counter improvement: extend xtask/src/cmd/quality_budgets.rs::count_long_fns::LongFnVisitor::visit_item_fn to skip #[cfg(test)] fns via syn node.attrs inspection (drops 1-4 sites with zero code churn). ~3-5 B2 violators cleared.

Done (2026-06-07) — !615 / 4aa191c7 (into_response 131→68; bootstrap 136→50; introspection already 28 LOC pre-MR; B2 counter skip-test fns added; B2 38→36 LOCKED)

10

Verify B1+B2 LOCKED + Plan N handoff — xtask/quality-budgets.lock ratchet B1+B2 floors to the new actual reached by Steps 3–9 (strict-no-grandfather honored — only ratchet DOWN, never up). cargo xtask quality-budgets --fail-on-regression exits 0 against ALL 8 historical locks. Comment in Plan N MR #522 (Step 9 enforcement flip) referencing this MR as the precondition-clear.

Done (2026-06-07) — !620 / 7e701d73. Lock B2 49 → 15, B1 8 → 8 (already at floor). cargo xtask quality-budgets --fail-on-regression exits 0. Plan N issue #522 commented with handoff note.

11

Plan completion audit + archive — plan-completion-audit subagent verifies all 10 prior steps Done with MR + sha citations; cargo xtask docs plan-archive moves plans/decomposition-and-shared-extraction.adoc → archive/; nav.adoc Active row removed; archive.adoc new row with final stats (# MRs, # functions decomposed, # crates added, # B1/B2 sites cleared, % LOC reduction in service mains); .claude/CLAUDE.md § Phase Status row appended.

Done (2026-06-07) — this MR (audit verified 10/10 prior steps Done with concrete MR+sha citations; 16 MRs + 3 batch MRs across Step 6 = 18 total MRs; 51 functions decomposed; 1 new crate added (craig-bootstrap); B2 56→15 LOCKED + B1 11→8 LOCKED; service mains 200-300 LOC → ≤50 LOC orchestrators (~75% reduction))

Epic: &41 (https://gitlab.com/groups/gadhs/application/ccwis/-/epics/41)
Issues: #526–#536 (one per step)
Branch prefix: docs/plan-q- for filing/archive, feat/plan-q-step<N>- for new functionality, refactor/plan-q-step<N>- for decomposition, chore/plan-q-step<N>- for ratchet/handoff
Milestone: TBD

Context

Plan N (quality-gate-enforcement) Steps 5–7 paid down 5 of 7 OVER quality budgets (B5, B4, B3a, B7, B3b). Two remain: B1 (8 → 11) route modules over 500 LOC and B2 (49 → 56) functions over 100 LOC. They block the Plan N Step 9 enforcement-gate flip under strict-no-grandfather.

Audit of all 53 B2 violators surfaced a deeper truth: most aren’t "legitimate carve-outs per §Style" — they’re fat-arm dispatchers (CLI run fns with 20–50 LOC of inline logic per match arm), duplicated-setup-not-extracted-yet (9 service main fns sharing ~85% of bootstrap code), and copy-paste-and-customize patterns (11 seed-data builders, 3 identity IaC renderers). The §Style "important function" carve-out is real but narrower than the lint was treating it as.

User principle (2026-06-06): "Just because a function is important doesn’t mean that bad coding practices get a pass. We still want to decompose unless decomposition makes the codebase worse."

This plan reads the B1+B2 lint signal as it was meant — find the duplicated bootstrap and the fat-arm code that’s been compounding — and turns it into architectural extraction with the B1/B2 clears as a happy consequence.

The DRY opportunities the lint is pointing at

  1. craig-bootstrap shared crate (highest value). Existing crates/craig-api/src/bootstrap.rs:149 bootstrap() extracts ~80 LOC of setup (Settings + telemetry + DB pool + auth + RabbitMQ + service/actor tokens). What it does NOT yet extract — and what’s duplicated 100–180 LOC at a time across 9 service main fns — is: sqlx::migrate!() invocation pattern, OutboxWorker spawn, AuthzEngine boot sequence (~200 LOC each: cache-invalidation subscriber + warmup + TTL refresh task), ObjectStoreConfig::load() → Store::from_config(), shared HTTP client init, standard axum extension-layer stack, shutdown-token plumbing. Extracting reduces typical service mains from 200–300 LOC to ~80–120 LOC.

  2. SqlRow trait + table-driven render for tools/craig-seed/src/sql.rs (5 render fns × ~200–350 LOC each = ~1300 LOC). Today each render_X_sql re-implements INSERT INTO X (cols) VALUES boilerplate + comma/semicolon row separator + NULL handling for Option fields. After: a trait SqlRow + generic render_table<T: SqlRow>. Each entity implements SqlRow; render fns shrink to 5-line orchestrators.

  3. CLI fat-arm → per-action helpers for services/craig-cli/src/cmd/*.rs (18 modules, 100–320 LOC each). Each pub async fn run is a match cmd { …​ } with 20–50 LOC inline per arm. Decompose into per-action helpers (run_list, run_get, run_create, etc.). ~50% are structurally identical (build-query → call-API → check-status → print-result) — potential CRUD trait pattern.

  4. Identity render Manifest + per-resource serializer for xtask/src/cmd/identity/render/{zitadel,kanidm,authentik}.rs (3 backends, 213–244 LOC each). Each backend renders the same canonical list of resources using its own DSL. After: Manifest struct + per-backend serializer trait.

  5. Residual decomposition for ~5 truly bloated production fns outside the above patterns: introspection client, error response handler, devstack guard, security tests, identity verify probes.

Threat Model

Three patterns that make the B1+B2 drift insidious until a refactor or partner integration surfaces them:

  1. Fat-arm dispatchers normalize per-command bloat. A CLI run fn that’s "just a match" can absorb 200 LOC of per-arm logic without any single arm crossing a clippy threshold. The aggregate fn crosses 100 LOC but the structure looks like a "wide-but-thin dispatcher" so reviewers wave it through. Plan Q’s Step 6 converts these to actual thin dispatchers.

  2. Duplicated bootstrap as accepted boilerplate. Each new service grew its main by copy-pasting from the most-recently-added service. ~85% of those 200 LOC are identical AuthzEngine boot, OutboxWorker spawn, ObjectStore init. The §Style "important function" tag let each new copy slide because "main is supposed to be wide." Plan Q’s Step 2-3 extracts the common layer.

  3. §Style "important function" tag retiring its own purpose. Plan I F-033 marked 100 of 220 over-40-LOC fns as "important-stay" — defensible at the time, but the tag drifted from "would-be-worse-to-decompose" toward "we’re tired of refactoring this." Plan Q Step 1 retires the tag and replaces it with a stricter test.

Cross-cutting Invariants

  1. Decompose unless decomposition makes the codebase worse. The new §Style rule. An "important function" tag is not a permanent decomposition exemption. The test: does extraction improve clarity / testability / maintainability without losing the narrative arc? If yes, decompose. Worse-only carve-outs: thin dispatchers (Variant(args) ⇒ helper::run(args)), bootstrap with single-screen narrative arc, single-site helpers introducing parameter sprawl.

  2. Strict no-grandfather on B1+B2. Like Plan N Phase B before it. Steps 3–9 pay down B1+B2 to the new actual floor; Step 10 ratchets the lock DOWN to that floor (never UP).

  3. Byte-identity regression tests at extraction boundaries. cargo xtask reseed must produce byte-identical SQL bytes through Step 4 (proves SqlRow refactor is semantics-preserving). cargo xtask identity render --backend X must produce byte-identical output through Step 5 (proves Manifest refactor is semantics-preserving).

  4. Devstack smoke per service-main migration. Step 3 (9 service mains) sub-splits by 3-service batch; each batch runs cargo xtask dev start + integration tests before pushing. Catches subtle worker startup-order or telemetry init-timing regressions.

  5. Plan Q clears the precondition; Plan N flips the gate. Plan Q’s Step 10 hands off to Plan N Step 9 via a cross-link in the Plan N body Status table. The enforcement-flip MR itself belongs to Plan N.

Step DAG

Step 1 (filing + §Style doctrine)
   │
   ├── Step 2 (craig-bootstrap foundation) ── Step 3 (9 service mains migration)
   │
   ├── Step 4 (SqlRow + seed refactor)
   ├── Step 5 (identity render Manifest)
   ├── Step 6 (CLI fat-arm decomposition; 2-3 sub-MRs by command batch)
   ├── Step 7 (xtask command run decomposition)
   ├── Step 8 (B1 service api module decomposition)
   └── Step 9 (residual large-fn decomposition)
                            │
                            └── Step 10 (verify B1+B2 LOCKED, hand off to Plan N)
                                          │
                                          └── Step 11 (audit + archive)

Steps 2-3 and 4-9 can run in parallel session-by-session. Step 10 blocks on all of Steps 2-9 done. Step 11 blocks on Step 10.

Risks

  1. craig-bootstrap::Builder API too rigid for one service’s specific needs. Mitigation: Use Option<…​> flags + closures for service-specific extension points; revisit after Step 3 if any service can’t fit.

  2. Service-main migration introduces a subtle behavioral change (worker startup order, telemetry init timing). Mitigation: per-service devstack smoke after each batch; sub-MR sequence catches one regression before propagating.

  3. SqlRow trait too generic; loses NULL-handling nuance. Mitigation: SqlValue wrapper struct that encodes nullability + type; covers all current sql.rs renderers; fall back to inline rendering if a corner case doesn’t fit.

  4. CLI fat-arm extraction reveals near-but-not-quite-identical helpers; macro pressure grows. Mitigation: hold the line on trait-based decomposition through Step 6; revisit macro only if Step 6 final-batch shows >50% boilerplate duplication.

  5. Identity render byte-identity regression slips. Mitigation: Step 5 explicit hash-comparison test against pre-refactor output for each backend.

  6. Plan Q runs long; Plan N Step 9 waits indefinitely. Mitigation: Steps 2-3 alone clear most of B2; if Step 6 (CLI) drags, the B2 budget may already be at lock by Step 7 — verify at Step 10 if early handoff is possible.

  7. [cfg(test)] test fns in quality_budgets.rs synthetic source still inflate B2 count. Mitigation: Step 9 includes a B2 counter improvement to skip [cfg(test)] fns via syn attr inspection (drops 1-4 sites with zero code churn).

Verification

After each step: workspace nextest green; clippy -D warnings clean; cargo xtask docs plan-lint clean; B1/B2 actuals trending down.

After Step 3: cargo xtask quality-budgets --report shows B2 dropped by 9 (service mains cleared).

After Step 10: cargo xtask quality-budgets --fail-on-regression exits 0 against ALL 8 historical locks. Plan N Step 9 ready to land.

After Step 11: plan archived; CLAUDE.md row added; Plan N Step 9 flip-on lands in a follow-up MR (Plan N’s responsibility, not Plan Q’s).

Edit this page · latest