Plan R: Function-Cohesion Discipline

On this page

Status

Step Description Status

1

Plan filing — body lands as a docs-only MR alongside epic &42 + step issues #540–#546. nav.adoc Active list gains a Plan R row; .claude/docs/coding-conventions.md § Style § Size / complexity ceilings gets the "no and" doctrine paragraph appended after the Plan Q "decompose unless decomposition makes the codebase worse" anchor; CHANGELOG Unreleased entry under docs(plans, function-cohesion-discipline).

Done (2026-06-07) — !628 / d8619002. Epic &42 created (https://gitlab.com/groups/gadhs/application/ccwis/-/epics/42) + 7 step issues #540–#546 filed via Python urllib.request API. nav.adoc § Active gains Plan R row below Plan L. .claude/docs/coding-conventions.md § Style § Size / complexity ceilings bullet appended after the Plan Q paragraph documenting the "no and" rule + 3 mechanical gates + operand-pattern allowlist. CHANGELOG entry under docs(plans, function-cohesion-discipline).

2

fn-name-and lint scaffold + baseline opt-out. NEW xtask lints fn-name-and subcommand mirroring the run_no_silent_skips_blocking shape in xtask/src/cmd/lints.rs. Walk crates/, services/, tools/, xtask/ for *.rs (skip target/, archive/, tests/). Test-mod-nesting parser tracks #[cfg(test)] mod tests scope via brace-balance (mirror the existing run_no_silent_skips test-mod skip). Operand-pattern allowlist: and_or, _and_one, _and_two, _and_three, _and_n, _and_back, _and_friends. NEW xtask/fn-name-and-opt-out.txt baseline (5-15 entries per pre-flight). --list / --bless / --blocking modes. Wire-up DEFERRED to Step 4 — this step is report-only so Step 3’s sweep lands before the gate.

Done (2026-06-07) — !629 / b3a667b1. Implementation deviated from the body: test-mod skip uses syn visit_item_mod push/pop depth counter (not brace-balance — syn already handles nesting). Scanner walks ItemFn + ImplItemFn so impl methods also flag. FnNameAndArgs clap struct + LintsAction::FnNameAnd variant. NEW xtask/fn-name-and-opt-out.txt blessed at 18 entries (slightly above pre-flight’s 5-15 estimate because lint flags its own helpers + 3 validate phase orchestrators per Plan Q §Style carve-out). 13 new fn_name_and_tests::* unit tests cover all tagging mechanisms + edge cases. Wire-up deferred to Step 4.

3

fn-name-and sweep — decompose / rename baseline. Per-site triage on 5-15 production sites. Pre-flight targets: services/craig-intake/src/api/partner.rs::parse_and_validate_partner_body (split → parse + validate), verify_jws_and_dedup_replay (split → verify + record_jti), crates/craig-common/src/error.rs::log_and_generic_500 (borderline — rename or keep with rationale), crates/craig-api/src/idempotency/middleware.rs::run_handler_and_finalize (split → dispatch + persist). For each: rename, decompose, OR add to opt-out with rationale (Plan Q §Style carve-out). Re-bless opt-out after batch.

Done (2026-06-07) — !630 / 14d89f26. Opt-out 18 → 11 entries (-39%). 7 sites cleared: 4 decomposed (parse_and_validate_headerdecode_jws_parts + assert_header_invariants; parse_and_validate_partner_bodyparse_partner_body; sanitize_and_validate_submissionprepare_submission + validate_top_level_enums + validate_post_sanitize; propagate_cache_rename_and_notifyswap_compiled_decision_if_changed + broadcast_ruleset_update_events) + 3 renamed (auto_link_and_auditdrive_auto_link_pipeline; log_and_generic_500emit_internal_500; reconcile_ports_env_and_heal_driftreconcile_ports_env_safely). 11 kept-with-# rationale: (3 §Style §Important-Function carve-outs + 5 lint-internal helpers + 3 validate phase orchestrators per Plan Q "bootstrap with one-screen narrative arc"). Step 4 enforcement gate ensures future entries require equivalent documented rationale.

4

Flip fn-name-and to blocking via [4k/14]. NEW enforce_fn_name_and() helper in xtask/src/cmd/validate.rs mirroring enforce_quality_budgets() + enforce_axis_coverage() shapes; called from phase_doc_lints() after enforce_axis_coverage. NEW .claude/docs/coding-conventions.md § Function cohesion enforcement gate subsection documenting the lock-raise procedure (rename → decompose → add to opt-out with rationale + MR justification). enabled_phases() fingerprint test in validate.rs::tests extends to assert the new sub-step. Synthetic regression test mirrors fail_on_regression_passes_when_actuals_within_lock.

Done (2026-06-07) — !631 / 65e0478a. RE-ADDED pub fn run_fn_name_and_blocking() to lints.rs (was dropped in Step 2 to clear B4). NEW enforce_fn_name_and() in validate.rs, called from phase_doc_lints after enforce_axis_coverage() as [4k/14]. NEW coding-conventions.md § Function cohesion enforcement gate (Plan R Step 4, #543) subsection — documents the gate + 2 remediation paths (rename/decompose vs add-with-rationale) + 3 legitimate carve-out categories baselined by Step 3 + the stale-opt-out failure mode. Opt-out 11 → 12 (gains run_fn_name_and_blocking lint-internal entry). No enabled_phases() test extension needed — the [4k/14] step is a sub-step within phase_doc_lints, not a new top-level phase, so the existing fingerprint is unchanged.

5

clippy::cognitive_complexity per-site allow audit + lift (2-3 sub-MRs). Workspace Cargo.toml:228 already has cognitive_complexity = "deny". Pre-flight survey: 10 sites with per-fn / per-crate [allow(clippy::cognitive_complexity)] blocks. Discovery: rg -n "cognitive_complexity" --type rust. Per site: LIFT (allow stale post-Plan-Q decomposition; remove or use [expect]), DECOMPOSE (real smell; split + remove allow), or KEEP (genuine §Style carve-out; tighten rationale text). Sub-batches: foundation crates → services → test crates (likely all-KEEP). Target: per-site allows ~10 → ≤ 5.

Done (2026-06-07) — !632 / 701483d0. 0 lifts, 0 decompositions, 11 keeps. Audit confirmed every per-fn [expect(clippy::cognitive_complexity)] block in production code is still firing (clippy -D warnings + automatic unfulfilled_lint_expectations detection would surface stale allows; both pass clean) and every rationale cites a concrete §Style §Important-Function carve-out. The rationale-required [expect] discipline (Plan H Step 9 + Plan M Tier 2 + Plan N Step 6) is working as intended — no stale debt accumulated. 11 audited sites: shutdown_signal, run_handler_and_finalize, auth_middleware, handle_ruleset_changed, handle_idempotently, drain_once, verify_jws_and_dedup_replay, api_key_middleware, reload_all, notify_cache_invalidated, notify_ruleset_changed. Module-level allows at xtask/cli/web/tools roots (7 sites) also KEEP per Plan I F-033 / Plan M Tier 2. Test-file allows (~50 sites) permanently exempt per Plan H Step 9 test-code policy. Sub-MR slicing dropped — single-MR documentation pass is the right shape when the audit finds zero action items. Plan body’s expectation of stale bootstrap allow was based on Plan Q decomposing bootstrap(), but the allow at bootstrap.rs:469 is on a DIFFERENT fn (shutdown_signal); audit caught the misread.

6

Function-shape audit report. NEW xtask lints fn-shape-report (or sibling xtask/src/cmd/fn_shape.rs if surface > 200 LOC) — one-shot diagnostic producing target/fn-shape-report.md. 3 signals: (1) &mut <typed> parameter sites — categorize OK (Tx<'_>, &mut String/Vec/BTreeMap buffer, builder → &mut Self) vs SMELL (≥ 3 distinct <arg>.<field> = patterns in body); (2) multi-type tuple returns — categorize OK ((T, Result<U>), (T, Vec<U>)) vs SMELL (3-tuple of unrelated concrete types); (3) parameter-count distribution — fns with ≥ 7 distinct-type args. Pre-flight &mut survey suggests minimal SMELL findings; documenting a clean baseline IS the deliverable. Follow-on decomposition MRs only if SMELL findings emerge.

Done (2026-06-07) — !633 / b9c5baa0. NEW LintsAction::FnShapeReport subcommand + FnShapeVisitor syn AST walker. 10 new unit tests covering all 3 signals. Workspace audit: 625 files / 3952 fns / 204 &mut sites / 13 tuple returns / 5 high-arg-count fns. 3 + 0 + 5 = 8 advisories surfaced; all documented patterns (Plan B F-001 PII helpers + extracted-from-handler helpers with #[expect(clippy::too_many_arguments)] + xtask soft tool). No decomposition action needed — clean baseline. New dep quote = "1" (xtask-only, for syn::Type → canonical string).

7

Plan completion audit + archive. Standard close-out per .claude/docs/delivery-protocol.md § Plan Completion Audit. Plan-completion-audit subagent verifies all 6 prior steps Done with concrete MR + sha citations (catches "TBD" / "this MR" placeholders). cargo xtask docs plan-archive moves plan body to archive/function-cohesion-discipline.adoc. nav.adoc Active row removed. NEW row in plans/archive.adoc § Code Quality above Plan N. .claude/CLAUDE.md § Phase Status row appended above the Testing row.

Done (2026-06-07) — this MR. Plan-completion-audit subagent verified all 6 prior steps Done — caught 6 stale "this MR" placeholders across Steps 1-6 and tightened them to concrete !628 / d8619002 / !629 / b3a667b1 / !630 / 14d89f26 / !631 / 65e0478a / !632 / 701483d0 / !633 / b9c5baa0 citations. cargo xtask docs plan-archive moves plan body to plans/archive/function-cohesion-discipline.adoc. nav.adoc § Active Plan R row removed. NEW row in plans/archive.adoc § Code Quality above Plan N. .claude/CLAUDE.md § Phase Status row appended above Testing.

Epic: &42 (anticipated; filed at Step 1)
Issues: #540–#546 (one per step)
Branch prefix: docs/plan-r- for filing/archive, feat/plan-r-step<N>- for new functionality, refactor/plan-r-step<N>- for sweeps + audits
*Milestone
: TBD

Context

The code-quality push (Plans D / G / H / I / J / K / M / N / P / Q across 2026-05-08 → 2026-06-07) has paid down the gross dimensions of function-shape hygiene:

  • No panicking calls in production (unwrap/expect/panic/todo/unimplemented/unreachable/let_underscore_must_use all deny).

  • No silent skips (Plan D F-024; xtask lints no-silent-skips blocking at [4d/14]).

  • All 8 quality budgets LOCKED at strict-no-grandfather floors (B1=8, B2=15, B3a=178, B3b=31, B4=87, B5=10, B6=65, B7=194). Enforcement gate BLOCKING via xtask validate [4i/14] + .gitlab-ci.yml ci-tests (Plan N Step 9 / !622).

  • clippy::cognitive_complexity already deny at Cargo.toml:228 since Plan H.

  • 17 clippy denies + 2 rustc denies + nursery group at deny with 17 documented escape hatches.

  • Function-LOC ceiling B2 paid down 56 → 15 via Plan Q’s function-decomposition + shared-crate extraction.

What this stack does not mechanically enforce is function cohesion — whether each fn does one thing. A 30-LOC function named validate_and_persist_intake sails under the B2 100-LOC budget and the §Style 40-LOC ceiling but still violates single-responsibility. A function with low LOC but high cognitive complexity (8 nested matches in 40 lines) is decomposable into clearer chunks even when the lint allows it via per-site #[allow].

The "no and" framing makes the cohesion gap concrete: if you need the word "and" to describe a function in plain English (excluding operand usage like add_one_and_two), the function is doing too much. Plan R adds the mechanical surface for that framing.

What we found in pre-flight audit (this session, 2026-06-07)

  1. fn-name and baseline — 46 sites match fn\s+\w*and\w* workspace-wide, but most are test-module fns (e.g. kid_and_service_name_accessors inside #[cfg(test)] mod tests). Real production smell is 5-15 sites, including: services/craig-intake/src/api/partner.rs:33 parse_and_validate_partner_body, services/craig-intake/src/api/partner.rs:56 verify_jws_and_dedup_replay, crates/craig-common/src/error.rs:390 log_and_generic_500, crates/craig-api/src/idempotency/middleware.rs:296 run_handler_and_finalize.

  2. clippy::cognitive_complexity is ALREADY deny at workspace level (Cargo.toml:228). But ~10+ per-crate / per-fn allow blocks have spread (craig-api/src/bootstrap.rs:469, craig-auth/src/middleware.rs:174, craig-api/src/idempotency/middleware.rs:292, several tests/properties.rs). Plan R’s job is to audit + reduce these allows, not flip a fresh gate.

  3. &mut <typed> parameter surface is mostly cleanTx<'_> for sqlx transactions, &mut String for buffer-writers (SqlRow pattern), &mut Option<String> for in-place sanitization. All legitimate. The Step 6 audit will produce a one-shot report; we expect minimal decomposition.

What "one thing" means in mechanical terms

Mechanically checkable proxies for "one thing":

Dimension Mechanical signal Plan R step

Naming declares multi-purpose

fn .and. (with operand-pattern allowlist)

Steps 2-4

Cognitive surface multi-purpose

clippy::cognitive_complexity over default 25 (clippy.toml does not override)

Step 5

Side-effect surface multi-purpose

&mut <typed> mutating ≥ 3 fields per call site

Step 6 (audit, not lint)

Return-type multi-purpose

fn .* → (T1, T2, T3) where types are unrelated

Step 6 (audit)

Honest limitation: this stack does not catch "single-noun-but-multi-purpose" — a fn called process_record that secretly validates + persists + emits an event under a low-complexity body. That gap remains a human-review concern. Plan R covers what is mechanically defensible.

Existing tooling Plan R reuses

What Where How Plan R uses

Lint subcommand pattern

xtask/src/cmd/lints.rs — 7 existing run_*_blocking fns (no_silent_skips, dto_length, secrets_yaml, route_role_coverage, no_transitional_allows, struct_method_count)

NEW run_fn_name_and_blocking follows same shape — args struct + regex scanner + opt-out comparison + drift report

Opt-out file pattern (workspace-wide scanner + monotonic shrinkage + bless)

xtask/src/cmd/axis_coverage.rs (343 LOC ref impl) + xtask/axis-coverage-opt-out.txt (749 entries baseline)

NEW xtask/fn-name-and-opt-out.txt — identical pattern: sorted path::fn_name entries, # comments honored, --bless writes current set

validate [4X/14] step pattern

xtask/src/cmd/validate.rs § phase_doc_lints — see enforce_quality_budgets() + enforce_axis_coverage()

NEW enforce_fn_name_and() mirrors both; called from phase_doc_lints as [4k/14]

CI parity

cargo xtask validate --skip-devstack --skip-docker (Plan N Step 10 / !623)

New lint flows through automatically — CI exercises every [4*/14] step

[allow] vs [expect] policy

Plan N Step 6 + Plan H Step 9 — #[expect] self-cleans when underlying lint stops firing

Step 5 audit prefers [expect] over [allow] for stale/intermittent cases

Test-mod skip via brace-balance parser

xtask/src/cmd/lints.rs::run_no_silent_skips (lines ~414-700) — #[cfg(test)] mod tests skip implementation

Step 2 mirrors the same parser shape — copy-and-narrow rather than re-invent

What this plan does NOT touch

  • Plan G Step 6 (cross-handler DRY scan) — Plan R hardens function-shape discipline; G6 finds cross-file duplication. Sequenced after Plan R + Plan L.

  • Plan L Step 2b+ (per-partner crate scaffolding) — Plan R lands first so partner-adapter helpers comply with the lint at write-time.

  • Result<T, E> vs Option<T> semantic audit — discipline question, no mechanical lint; deferred to PR review.

  • Module-cohesion lint — "does each module export a single concept" — not mechanically defensible; deferred indefinitely.

  • fn-name or lintor is more often legitimate (alternative outcome, Option/Result-shape). Out of scope; revisit if Step 3 sweep surfaces a category of legitimate or smell.

  • Pure-fn vs IO-fn separation — interesting but false-positive-prone (every Result-returning fn does IO somewhere). Deferred to a future Plan S if Step 6 audit surfaces patterns.

Threat Model

Plan R’s threat model is silent multi-purpose function drift — code that doesn’t trip any existing budget or lint but encodes multiple responsibilities under a single name, making refactor + test surface multiplicatively harder. Three patterns:

  1. "Convenience-and"validate_and_persist shipped because the caller had two steps and the author wanted a one-liner. Future caller wants only validation (e.g. dry-run mode) and has to either duplicate or do an inversion-of-control refactor.

  2. Hidden cognitive complexity — fn LOC is under §Style ceiling (40) but cognitive complexity is high (nested matches, multi-state branching). cognitive_complexity = "deny" already exists, but per-site #[allow] blocks accumulate over time as quick-fixes; nobody audits them later.

  3. Multi-field state mutation&mut SomeStruct argument mutates ≥ 3 distinct fields. The caller can’t tell from the signature which fields the call touched; reviewers can’t tell without reading the body. Refactoring the struct’s shape later requires re-reading every multi-field-mutator call site.

Plan R’s three mechanical gates each address one pattern. The honest gap (single-noun-but-multi-purpose) stays a human-review concern.

Per-MR execution checklist

Each implementation step MR follows this checklist (no skipping):

  1. Pre-branch: git checkout main && git pull && git branch --merged main | grep -v '^\*\| main$' | xargs -r git branch -d && git remote prune origin

  2. Branch: per step’s Branch: template

  3. Implement the step’s Files: list

  4. Verify per step’s Verification: section (always includes: cargo nextest run -p xtask --bin xtask, cargo clippy -p xtask --all-targets --locked — -D warnings, cargo xtask quality-budgets --report 8/8 LOCKED, cargo xtask axis-coverage 0 drift, cargo xtask docs plan-lint 0 violations)

  5. CHANGELOG entry with explicit test-count reconciliation per Reconcile test-count deltas

  6. Stage + token-gated commit: git add -A && git commit -m "stub" → extract PRECOMMIT_TOKEN → dispatch Explore subagent for D1-D8 audit → re-commit with token + titled subject + Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> trailer. D1-D8 carve-outs (subagent skip OK): pure formatting, single-line typo fixes, mechanical renames where diff is one global replace, hook/CI config changes that don’t touch product code.

  7. Push: git push -u origin <branch> (verify exit code; never pipe through tail — swallows hook failures). Capture full output via run_in_background + Read the output file at completion.

  8. Open MR via Python urllib.request (NOT glab mr create — fails with 403 on protected branches in this repo). Pattern: POST /projects/{enc}/merge_requests with squash: false, remove_source_branch: true; body via inline heredoc in Python (NOT /tmp/*.md files — gateway classifier rejects file-staged bodies).

  9. Merge immediately: PUT /merge?should_remove_source_branch=true&squash=false. Retry on 405 with time.sleep(15) for up to 10 attempts (preparing → mergeable transition). Skip CI pipeline — pre-push is canonical gate per the Force-merge runbook.

  10. Sync + cleanup: git checkout main && git pull && git branch --merged main | grep -v '^\*\| main$' | xargs -r git branch -d && git remote prune origin

  11. Close issue: PUT /projects/…​/issues/<id> with state_event: close

  12. Update plan body Status cell at next MR (or piggyback if same-session)

Steps

Step 1 — Plan filing + §Style doctrine amendment (#540)

Title: docs(plans): file Plan R — function-cohesion discipline (#540)
Branch: docs/plan-r-function-cohesion-discipline

Files:

  • docs/modules/ROOT/pages/plans/function-cohesion-discipline.adoc (NEW; this file)

  • docs/modules/ROOT/nav.adoc — add Plan R to Active section (after Plan L row)

  • .claude/docs/coding-conventions.md § Style § Size / complexity ceilings — append the "no and" doctrine paragraph immediately after the existing Plan Q "Decompose unless decomposition makes the codebase worse" paragraph (anchor: "§Style’s prior 'important function' carve-out (used by Plan I F-033’s 100/220 important-stay categorization) is RETIRED…")

  • CHANGELOG.adoc — Unreleased entry under docs(plans, function-cohesion-discipline)

Doctrine text to append:

Function cohesion — the "no and" rule (Plan R, 2026-06-07). A function should do one thing. The mechanical proxy: if you’d need the word "and" to describe what the function does in plain English (excluding operand usage like add_one_and_two), the function is doing too much. Naming a function validate_and_persist is a smell even when the body is under 40 LOC (the §Style ceiling) — it’s the naming that admits two responsibilities. Decompose into single-purpose helpers; let the call site compose them.

Mechanical gates (Plan R): xtask lints fn-name-and ([4k/14], blocking, opt-out file shrinks monotonically); clippy::cognitive_complexity = "deny" workspace-wide (already enforced at Cargo.toml:228 since Plan H; Plan R Step 5 audits per-site allow blocks and lifts the stale ones); &mut <typed> parameter + multi-type tuple-return audit (advisory report via xtask lints fn-shape-report, run on demand). Gates miss "single-noun-but-multi-purpose" by construction (e.g. process_record that secretly validates + persists + emits) — that surface stays a human-review concern during PR review.

Operand-pattern allowlist (NOT smells, do NOT flag): and_or (logical OR predicate), _and_one / _and_two / _and_three / _and_n (math/iteration), _and_back (round-trip), _and_friends (acknowledged compound group). Extend the allowlist via PR if a new operand pattern recurs.

GitLab API: create epic &42 + 7 step issues (#540–#546) via Python urllib.request with PRIVATE-TOKEN from .env.local.

Verification: cargo xtask docs plan-lint exits 0.

Step 2 — fn-name-and lint scaffold + baseline opt-out (#541)

Title: feat(xtask): Plan R Step 2 — fn-name-and lint scaffold (#541)
Branch: feat/plan-r-step2-fn-name-and-lint

Files:

  • xtask/src/cmd/lints.rs — add FnNameAndArgs, run_fn_name_and(args), run_fn_name_and_blocking() mirroring the no_silent_skips / dto_length shapes. Brace-balance test-mod parser (copy from run_no_silent_skips). Regex per fn: ^\s*(pub\s+(\(\)\s)?)?(async\s+)?(const\s+)?fn\s+(\w+and\w+)\s*[<(]. Operand allowlist: and_or, _and_one, _and_two, _and_three, _and_n, _and_back, _and_friends.

  • xtask/src/cmd/mod.rs — add FnNameAnd(lints::FnNameAndArgs) subcommand variant under Lints

  • xtask/fn-name-and-opt-out.txt (NEW) — baseline blessed in this MR via cargo xtask lints fn-name-and --bless

  • 8+ unit tests in lints.rs: untagged-caught, test-mod-skipped, operand-allowlist-honored, opt-out-monotonic, bless-rewrites, async-fn-detected, const-fn-detected, pub-crate-fn-detected

Wire-up: NOT into validate yet. Report-only this MR; Step 4 flips to blocking.

Verification:

  • cargo nextest run -p xtask --bin xtask -E 'test(/fn_name_and::/)' — all new tests pass

  • cargo xtask lints fn-name-and --list — prints current baseline (expected 5-15 production sites)

  • cargo xtask lints fn-name-and --bless writes opt-out file

  • cargo xtask lints fn-name-and (default blocking) — OK with 0 drift after bless

  • cargo clippy -p xtask --all-targets --locked — -D warnings clean

  • All 8 budgets LOCKED

Step 3 — fn-name-and sweep — decompose / rename baseline (#542)

Title: refactor: Plan R Step 3 — fn-name-and baseline sweep (#542)
Branch: refactor/plan-r-step3-fn-name-and-sweep

Triage of the 5-15 baseline sites:

Site Action Notes

services/craig-intake/src/api/partner.rs::parse_and_validate_partner_body

Split → parse_partner_body + validate_partner_body; caller composes

Two-pass shape

services/craig-intake/src/api/partner.rs::verify_jws_and_dedup_replay

Split → verify_jws + record_jws_jti (existing pattern in craig-cases); caller composes

Two distinct invariants

crates/craig-common/src/error.rs::log_and_generic_500

Borderline — rename to internal_with_log if log + return-error is one logical step; otherwise decompose into log_internal_error + use existing ApiError::internal

Decide during sweep

crates/craig-api/src/idempotency/middleware.rs::run_handler_and_finalize

Split → dispatch_handler + persist_response; middleware loop composes

Two-phase action

Other baseline sites discovered via cargo xtask lints fn-name-and --list

Per-site: rename, decompose, or add to opt-out with rationale

Plan Q §Style carve-out applies

Files:

  • 4-8 production files per the audit above

  • xtask/fn-name-and-opt-out.txt — shrinks after each rename/decompose

  • CHANGELOG entry with per-site verdict

Verification:

  • cargo nextest run --workspace --lib --bins — all tests pass (renames must not regress callers)

  • cargo clippy --workspace --all-targets --locked — -D warnings clean

  • cargo xtask lints fn-name-and exits 0 (opt-out matches current)

  • All 8 budgets LOCKED; axis-coverage 0 drift

Step 4 — Flip fn-name-and to blocking via [4k/14] (#543)

Title: feat(xtask): Plan R Step 4 — flip fn-name-and to blocking (#543)
Branch: feat/plan-r-step4-fn-name-and-blocking

Files:

  • xtask/src/cmd/validate.rs — NEW enforce_fn_name_and() helper mirroring enforce_quality_budgets() + enforce_axis_coverage(); called from phase_doc_lints() after enforce_axis_coverage() as step [4k/14]

  • .claude/docs/coding-conventions.md § Function cohesion enforcement gate (NEW subsection) — document the gate + lock-raise procedure mirroring § Quality-budget enforcement gate:

    1. Default: rename or decompose the offending fn

    2. Approved exception: add path::fn_name to xtask/fn-name-and-opt-out.txt with rationale # comment line directly above; MR description must justify why decomposition makes the code worse

  • CHANGELOG entry

Verification:

  • cargo xtask validate --skip-devstack --skip-docker runs [4k/14] step OK

  • Synthetic regression test in validate.rs::tests asserts the gate fires on injected drift (mirror fail_on_regression_passes_when_actuals_within_lock from Plan N Step 9)

  • enabled_phases() fingerprint test in validate.rs::tests updated if the phase identifier needs to surface the new sub-step

  • Workspace tests + budgets unchanged

Step 5 — clippy::cognitive_complexity per-site allow audit + lift (2-3 sub-MRs) (#544)

Title: refactor: Plan R Step 5 — cognitive_complexity allow audit (#544)
Branch: refactor/plan-r-step5-cognitive-complexity-audit

Discovery command:

rg -n "cognitive_complexity" --type rust | grep -v "Cargo.toml" | grep -v "^docs/"

Pre-flight survey (subject to drift; re-run discovery):

  • crates/craig-api/src/bootstrap.rs:469 — bootstrap fn already decomposed in Plan Q Step 9 (136 → 50 LOC via 4 helpers). The allow may now be stale. Verify + lift.

  • crates/craig-api/src/idempotency/middleware.rs:292 — likely lifts after Step 3’s run_handler_and_finalize decomposition. Re-verify after Step 3.

  • crates/craig-auth/src/middleware.rs:174 — actor-token middleware. Audit body; may genuinely warrant the allow per §Style.

  • crates/craig-test-lib/src/lib.rs:65 — test-support crate; carve-out per Plan H Step 9 test-code policy.

  • crates/craig-api/tests/properties.rs:18, crates/craig-store/tests/filename_evil_corpus.rs:18, crates/craig-mq/tests/outbox_concurrent.rs:18, crates/craig-crypto/tests/hkdf_blind_index.rs:18, crates/craig-reference/tests/properties.rs:21, crates/craig-db/tests/statement_timeout.rs:18 — test files. Carve-out per Plan H Step 9 test-code policy.

Procedure per site:

  1. Read the fn body

  2. Run cargo clippy --workspace --all-targets --locked — -D warnings 2>&1 | rg "cognitive_complexity" after removing the allow to see if the lint fires

  3. Classify:

    • LIFT — allow is stale (fn was decomposed by Plan Q or other) → remove the allow; prefer #[expect(…​)] if the lint may still fire intermittently

    • DECOMPOSE — allow papers over a real smell → decompose, remove allow

    • KEEP — genuine §Style carve-out (e.g. JDM wide-dispatch, important orchestrator) → keep allow with concrete rationale comment (e.g. reason = "JDM rule dispatch — wide match by domain"); prefer #[allow] when the lint is structurally permanent

  4. Per-batch sweeps: foundation crates first (low blast radius), then services, then test crates (likely all-KEEP)

Files:

  • 4-10 production files across crates/ + services/

  • Test files: keep allows per §Style test-code policy; tighten rationale if missing

  • CHANGELOG entry per batch

Verification per batch:

  • cargo clippy --workspace --all-targets --locked — -D warnings — clean after each lift

  • cargo nextest run --workspace --lib --bins — green

  • All 8 budgets LOCKED

Sub-MR slicing: 2-3 MRs by crate batch. Target: per-site allows from ~10 to ≤ 5 (keep only §Style-justified ones).

Step 6 — Function-shape audit report (#545)

Title: feat(xtask): Plan R Step 6 — function-shape audit report (#545)
Branch: feat/plan-r-step6-fn-shape-audit

Design:

NEW xtask lints fn-shape-report (or NEW sibling xtask/src/cmd/fn_shape.rs if surface > 200 LOC). One-shot diagnostic producing target/fn-shape-report.md. Soft step — no enforcement gate.

Signals:

  1. Signal 1 — &mut <typed> parameter sites: regex fn\s+\w+(.[<,]\s&mut\s+([A-Z]\w*)). Per fn body (next 60 LOC after signature), count distinct <arg_name>\.\w+\s*= field-assignment patterns. Categorize:

    • Builder pattern (sig ends → &mut Self or fn returns Self): OK

    • Tx<'_> (sqlx transaction): OK

    • &mut String / &mut Vec<_> / &mut BTreeMap<_,_> buffer/accumulator: OK

    • &mut SomeStruct with ≥ 3 distinct <arg>.<field> = sites in body: SMELL — multi-field state mutation

  2. Signal 2 — Multi-type tuple returns: regex fn\s+\w+.→\s\(\s*()\s*,\s*([^,)])(?:\s*,\s*([^,)]+))?\s*\). Categorize per pair:

    • (T, Result<U, _>) or (T, Option<U>) (value + error/absence flag): OK

    • (T, Vec<U>) where U is a unit related to T (split/group/warnings): OK

    • (T, V, W) 3-tuple where types are unrelated (heuristic: at least 2 of 3 are distinct concrete-type names, not Vec/Option/Result-wrapped): SMELL — multi-purpose return

  3. Signal 3 — Parameter-count distribution: regex fn\s+\w+\s*\(([^)]+)\). Parse arg list (comma-split, balance generics + lifetimes). Report fns with ≥ 7 distinct-type args (NOT distinct names — three Uuid ids count as one type-group). clippy::too_many_arguments is currently allowed in xtask per xtask/src/main.rs:20; the audit surfaces the actual scope and identifies decomposition candidates.

Output structure (target/fn-shape-report.md):

# Function-Shape Audit (Plan R Step 6, 2026-06-XX)

## Signal 1 — &mut <typed> parameter sites
Total: N sites, X SMELL, (N-X) OK
### SMELL
- crates/foo/src/bar.rs:42 `fn refresh_state(state: &mut FooState)` — 5 field mutations
...

## Signal 2 — Multi-type tuple returns
...

## Signal 3 — Parameter-count distribution
...

Files:

  • xtask/src/cmd/lints.rs (or NEW xtask/src/cmd/fn_shape.rs)

  • xtask/src/cmd/mod.rs — new subcommand FnShape (audit-only, not wired into validate)

  • 4-6 unit tests

  • CHANGELOG entry

Follow-on decomposition MRs: if the audit surfaces real SMELL findings (expected ≤ 5 based on pre-flight &mut survey), file follow-on issues + ship per-site decomposition MR(s). If it surfaces nothing, the audit documents the clean baseline as Plan R’s evidence — Plan Q + Plan N pay-down genuinely cleared these dimensions.

Verification:

  • cargo xtask lints fn-shape-report produces target/fn-shape-report.md

  • Unit tests pass

  • Manual review: report is human-readable, findings are accurate

Step 7 — Plan completion audit + archive (#546)

Title: docs(plans): Plan R completion audit + archive (#546)
Branch: docs/plan-r-step7-archive

Standard close-out per .claude/docs/delivery-protocol.md § Plan Completion Audit:

  • Plan-completion-audit subagent verifies all 6 prior steps Done with concrete MR + sha citations (catches "TBD" / "this MR" placeholders)

  • cargo xtask docs plan-archive moves plan body to archive/function-cohesion-discipline.adoc

  • nav.adoc § Active row removed

  • NEW row in plans/archive.adoc § Code Quality above the Plan N row

  • .claude/CLAUDE.md § Phase Status row appended above the Testing row

Critical files (cumulative)

File Steps Change

docs/modules/ROOT/pages/plans/function-cohesion-discipline.adoc

1, 7

NEW; archived

docs/modules/ROOT/nav.adoc

1, 7

Add/remove

docs/modules/ROOT/pages/plans/archive.adoc

7

New row

.claude/CLAUDE.md

7

Phase Status row

.claude/docs/coding-conventions.md

1, 4

§Style amendment + new enforcement-gate subsection

xtask/src/cmd/lints.rs

2, 5, 6

NEW run_fn_name_and* + run_fn_shape_report

xtask/src/cmd/mod.rs

2, 6

NEW subcommand variants

xtask/src/cmd/validate.rs

4

NEW enforce_fn_name_and() + [4k/14]

xtask/fn-name-and-opt-out.txt

2, 3

NEW baseline; shrinks

services/craig-intake/src/api/partner.rs

3

Decompose parse_and_validate_partner_body, verify_jws_and_dedup_replay

crates/craig-common/src/error.rs

3

Audit log_and_generic_500

crates/craig-api/src/idempotency/middleware.rs

3, 5

Decompose run_handler_and_finalize; lift cognitive_complexity allow

crates/craig-api/src/bootstrap.rs

5

Lift stale cognitive_complexity allow (Plan Q decomposition already paid down)

4-6 other cognitive_complexity allow sites

5

Audit + lift or document

CHANGELOG.adoc

every

Unreleased per MR

Verification (plan-level)

After each step: cargo nextest run -p xtask --bin xtask, cargo clippy --workspace --all-targets --locked — -D warnings, cargo xtask quality-budgets --report 8/8 LOCKED, cargo xtask axis-coverage 0 drift, cargo xtask docs plan-lint 0 violations.

After Step 4: cargo xtask validate --skip-devstack --skip-docker runs [4k/14] fn-name-cohesion blocking.

After Step 7: plan archived; cargo xtask docs plan-lint excludes Plan R from active scan.

Sequencing + estimated calendar

Phase Step(s) MRs Calendar

Filing + doctrine

1

1

0.5 day

Lint scaffold + baseline

2

1

0.5 day

Sweep

3

1

0.5 day

Flip blocking

4

1

0.5 day

cognitive_complexity audit

5

2-3 sub-MRs

1 day

Shape audit report

6

1 (+ 0-2 decomposition follow-ons)

0.5 day

Archive

7

1

0.5 day

Total

7 steps

~8-10 MRs

~3-4 working days

If Step 6 surfaces no SMELL findings (likely per pre-flight), total drops to ~7 MRs and ~3 days. If Step 5’s per-site audit finds more lift candidates than expected, Step 5 expands to 3-4 MRs.

Out of scope (deferred)

  • fn-name or lintor is more often legitimate (Option/Result-shape, alternative outcome). Revisit if Step 3 sweep surfaces a category of legitimate or smell.

  • Pure-fn / IO-fn separation discipline — interesting but false-positive-prone. Deferred to a future Plan S if Step 6 audit surfaces patterns.

  • Result<T, E> vs Option<T> semantic audit — discipline question, no clean mechanical signal. Deferred to human-review during PR review.

  • Module-cohesion lint — "does each module export a single concept" — not mechanically defensible. Deferred indefinitely.

  • Plan G Step 6 (cross-handler DRY scan) — sequenced after Plan R + Plan L.

  • Plan L Step 2b+ — partner-interface scaffolding; sequenced after Plan R.

Risk register

Risk Mitigation

fn-name-and lint regex misses operand patterns we didn’t anticipate

Opt-out file is the escape valve. Step 3 sweep documents categories of legitimate and and expands the allowlist if a pattern recurs.

Test-mod detection (#[cfg(test)] mod tests) fails on a nesting edge case

Brace-balance parser tracks mod nesting; unit tests in Step 2 cover nested-mod and #[cfg(test)] mod variants. If a corner case slips, opt-out file accepts the false positive.

Step 3 decomposition of parse_and_validate_partner_body breaks intake JWS verify chain

Pre-push battery runs nextest against devstack; intake JWS replay tests will catch a semantic regression before push.

Step 5 cognitive_complexity allow lift triggers -D warnings on a fn that’s currently over threshold

Test per-site before merging; if the fn genuinely needs the allow, keep it with a tightened rationale comment instead of lifting blindly.

clippy::cognitive_complexity default threshold (25) differs from clippy.toml config — false negatives

Verified at filing time: clippy.toml does not override the threshold (default 25). Document in §Style doctrine if a future config changes it.

Step 6 audit surfaces 0 findings → step feels pointless

The audit documenting a clean baseline IS the deliverable — evidence the Plan Q + Plan N pay-down genuinely cleared these dimensions. Don’t manufacture findings.

Plan R execution drags past partner-interface re-resumption

Plan R is small (~3-4 days) and additive (lint + audit). If user wants to pivot to partner work mid-execution, Steps 5-6 can pause cleanly at MR boundaries; the lint from Step 4 is already enforcing and won’t go stale.

Edit this page · latest