Record the complexity and maintainability audit

docs/roadmap/audit/03-complexity.md

@@ -0,0 +1,259 @@
+# Complexity and maintainability audit
+
+Status: seven open P3 refactoring opportunities. Reviewed source:
+`1e4fb73d159e2fa3ea0a3591eb56624138e613f6`.
+
+## Architecture and review method
+
+The [internal overview](../../internal/overview.md) inventories the implemented
+owners. Core owns captured declarations, installation, routing, typed input
+preparation, completion, reporting, and outcomes. Neutral metadata separates
+those responsibilities from the private Cobra/pflag adapter. ckconfig owns
+configuration command assembly, scope/input coordination, and ConfigKit report
+adaptation. ckprompt owns PromptKit command assembly and native operation/report
+adaptation and depends on public core and ckconfig contracts. Applications own
+loaders, builders, runtime dependencies, and executable lifecycle; upstream
+libraries own domain algorithms. Release tooling delegates to ReleaseKit.
+
+The review used codebase-memory-mcp's graph complexity/cognitive metrics, symbol
+search, source snippets, and call traces, followed by direct source review.
+Production and test symbols were considered separately; excluded examples and
+launcher tests were inspected directly. The indexed Go source matches the
+reviewed revision. Scores below are the extractor's `complexity` and `cognitive`
+values, not portable thresholds or proof of a defect. Nested callbacks and
+branch-heavy type handling can raise a score without justifying a refactor.
+
+Representative candidates from the metric pass:
+
+| Function | Function span | Complexity | Cognitive |
+| --- | --- | ---: | ---: |
+| ckconfig.captureSuite | family.go:86–211 | 31 | 78 |
+| walkCompletionFlags | completion_flags.go:43–98 | 16 | 61 |
+| cobra.interpretFlagTokens | internal/cobra/flags.go:10–103 | 16 | 51 |
+| prepareInputWithOccurrences | binding.go:252–338 | 21 | 45 |
+| ckconfig.prepareConfigOperation | input.go:196–317 | 25 | 38 |
+
+The findings below identify responsibilities or policies that could be made
+easier to maintain, rather than asking for functions to meet a numeric limit.
+The [architecture policy](../../policy/architecture.md) and accepted
+[ADRs](../../adr/README.md) remain authoritative: retain separate optional
+modules, typed public contracts, backend isolation, upstream domain ownership,
+and operation/presentation error separation. Avoid a universal operation engine,
+new declaration language, or generic provider framework merely to reduce lines.
+
+No production code or repository tests were changed. No runtime tests,
+benchmarks, or performance measurements were run for this static audit.
+Documentation links, source locations, whitespace, and the final diff were
+checked. No additional ordinary correctness defect was established. The open
+findings in [01-correctness.md](01-correctness.md) and
+[02-efficiency.md](02-efficiency.md) remain separate; these refactors do not
+constitute fixes for those findings.
+
+## M1 — P3: Scope capture combines several compilation phases
+
+**Code:** [family.go](../../../integrations/ckconfig/family.go), captureSuite
+lines 86–211.
+
+**Complexity:** the highest reported production cognitive score corresponds to
+126 lines that copy selectors, rename comparison bindings, normalize and validate
+scopes, collect retained errors, validate provider wiring, establish scope order
+and indexes, deduplicate selector scope references, attach bindings to scopes,
+derive operation-wide binding unions, and build discovery inventory. The nested
+selector/scope/operation branches require tracking several partially populated
+collections at once. A new operation or selector rule can affect multiple
+regions of the same function.
+
+**Potential refactoring:** make captureSuite an explicit sequence of private
+phases: capture/validate scope declarations; establish ordered scope metadata;
+capture/validate selectors; attach scope and comparison bindings; derive the
+operation unions and inventory. Extract complete responsibilities with clear
+inputs/results, reusing preparedSuite and preparedScope where appropriate.
+Keep small related loops together instead of creating a helper for every branch.
+
+Preserve declaration ownership, retained error order, empty-suite behavior,
+scope order, selector/binding identity, comparison requiredness, and metadata-only
+construction. This opportunity does not require selector-membership indexes or
+reopening the deferred efficiency work.
+
+## M2 — P3: Configuration preflight mixes shared and operation-specific rules
+
+**Code:** [input.go](../../../integrations/ckconfig/input.go),
+newConfigOperationCommand lines 61–170, configOperationPlan lines 40–59, and
+prepareConfigOperation lines 196–317.
+
+**Complexity:** one command builder and one preparation function cover five
+operations, with fields for several distinct submodes in the same private input
+and plan. Preparation interleaves flag-combination rules, explain-help selection,
+scope selection, capability checks, pointer parsing, plan construction, ordinary
+scope input preparation, and independent comparison-side preparation. Understanding
+one path requires determining which fields and branches are irrelevant to it.
+Changing show, explain, or comparison behavior requires reasoning about the same
+large control flow and its factory/check ordering.
+
+**Potential refactoring:** separate operation-specific option preparation from
+shared scope/capability selection, and isolate comparison input preparation from
+ordinary selected-scope preparation. Give meaningful groups of operation flags
+small private constructors where that makes the declarations easier to scan.
+Start with functions over the existing types; split private option records only
+where that materially clarifies which data a handler consumes. A generic rule
+registry or elaborate plan hierarchy is unnecessary.
+
+Keep the current ordering of preflight, input factories, binding application,
+input checks, state creation, and acquisition. Preserve explain's help path,
+supplied-state distinctions, full aggregate capability checks, independent
+comparison selections, and existing failure classifications.
+
+## M3 — P3: Parsing and completion maintain parallel flag-consumption logic
+
+**Code:** [flags.go](../../../internal/cobra/flags.go), interpretFlagTokens lines
+10–103; [completion_flags.go](../../../completion_flags.go), walkCompletionFlags
+lines 43–98 and completionOptionTarget lines 130–175;
+[shared flag helpers](../../../internal/commandmeta/flags.go).
+
+**Complexity:** token classification, long-flag splitting, and spelling lookup
+are already shared, but the consumers separately implement the remaining grammar:
+short-cluster traversal, boolean `=value`, attached nonboolean values, consumption
+of the next token, option-looking values, unknown spellings, and terminators.
+The completion functions add partial-token handling to substantially the same
+branch structure. Grammar maintenance therefore crosses the public core package
+and the internal backend adapter, despite their deliberate neutral boundary.
+
+**Potential refactoring:** extend the existing internal neutral helpers with a
+small token-consumption primitive that reports the reached flag, attached value
+or remaining suffix, and whether another token is needed. Keep normalization,
+occurrence/spelling recording, backend failure handling, and completion choices
+in their current owners. Share the consumption facts rather than forcing both
+callers through one policy-heavy parser.
+
+Completion must continue to accept an unfinished current word or a pending value
+where execution would fail. Preserve the backend's consumed failure prefix,
+entered spelling provenance, short-cluster boundaries, explicitly empty values,
+and `--` behavior. This can remain entirely internal; no public parser API or
+backend type exposure is needed.
+
+## M4 — P3: Typed preparation combines conversion, provenance, and replay assembly
+
+**Code:** [binding.go](../../../binding.go), prepareInputWithOccurrences lines
+252–338; [flag_values.go](../../../flag_values.go), CaptureFlags and applyTo.
+
+**Complexity:** prepareInputWithOccurrences validates parallel input shapes,
+converts flags, translates indexed conversion errors into entered spellings,
+assigns occurrence ordinals, reconstructs dense application order, tracks required
+flags, allocates operands, converts arguments, and assembles prepared callbacks.
+Deeply nested error/provenance branches sit inside the conversion loop. Captured
+bindings retain shared ordinal references for later subset replay, so changing
+this code also requires understanding the separate FlagValues path.
+
+**Potential refactoring:** separate entered-spelling error translation,
+flag/occurrence preparation, and argument preparation into cohesive private
+functions. Leave a short orchestration function that makes conversion-before-input
+construction and the application order visible. Keep ordinal assignment next to
+the preparation that owns its identity, with a short explanation of how captured
+streams reuse it.
+
+Retain the selected dense reconstruction and heap merge, parser-log versus
+fallback ordering, original binding identity, deferred required checks, and
+conversion error identity. Do not introduce a generic event-processing framework
+or replace the already remediated algorithms to lower the complexity score.
+
+## M5 — P3: Configuration diagnostic adaptation is maintained in both integrations
+
+**Code:** [configkit_adapter.go](../../../integrations/ckconfig/configkit_adapter.go),
+configKitDiagnosticsWithHeading and its cloning/rendering helpers, lines 137–209;
+[adapters.go](../../../integrations/ckprompt/adapters.go), promptKitDiagnostics
+and writePromptKitDiagnostics, lines 513–572. Related scheduling copies are
+queueConfigDiagnostics in [operations.go](../../../integrations/ckconfig/operations.go)
+and queuePromptDiagnostics in [command.go](../../../integrations/ckprompt/command.go).
+
+**Complexity:** both native adapters snapshot ConfigKit sources and diagnostic
+locations, partition error records from ordinary messages, create deferred detail
+callbacks, print a source legend, and delegate record formatting to ConfigKit.
+These are the same upstream data types and closely matching presentation rules.
+The two integrations also have matching severity-to-Reporter scheduling switches.
+Changes to evidence capture, source labeling, or diagnostic scheduling need
+coordinated maintenance across separate modules.
+
+**Potential refactoring:** consider one narrow, supported ConfigKit-diagnostic
+adaptation function owned by ckconfig, returning the existing core Diagnostic
+and Explanation values. ckprompt already depends on public ckconfig and could
+reuse it while retaining its operation-specific fallback explanations and supplied
+heading/side labels. This would consolidate the substantial native adaptation
+without making core depend on ConfigKit or exposing integration internals.
+
+The smaller scheduling duplication could separately use a narrow core reporting
+helper if the public API benefit justifies it; it does not justify a new shared
+module or universal diagnostics framework. Treat any new exported helper as an
+API decision, not an automatic consequence of deduplication. Preserve snapshot
+ownership, record order, error identity, independent explanation/message evidence,
+and immediate progress versus deferred ordinary diagnostic timing.
+
+## M6 — P3: Explanation providers must implement an unused rendering capability
+
+**Code:** [contracts.go](../../../integrations/ckconfig/contracts.go),
+ExplanationReader lines 76–80 and View.Explanation; consumers in
+[explain.go](../../../integrations/ckconfig/explain.go) and
+[show.go](../../../integrations/ckconfig/show.go). The direct writer invocation
+in [contracts_adapter_test.go](../../../integrations/ckconfig/contracts_adapter_test.go)
+is at line 121.
+
+**Complexity:** ExplanationReader requires both Explain and WriteExplanation.
+After the completed acquired-report remediation, production handlers use only
+Explain: core renders the selected report and its text template. Graph tracing
+and a repository-wide source search found no production WriteExplanation call;
+the remaining call is an adapter test, with additional test implementations.
+Custom providers must still supply a renderer that standard commands never use,
+and the contract suggests a parallel presentation responsibility.
+
+**Potential refactoring:** consider narrowing the provider requirement to native
+explanation acquisition and retiring the unused writer requirement from the
+CommandKit-facing interface. Native ConfigKit/PromptKit objects can retain their
+own rendering methods. Update provider examples, the direct adapter assertion,
+and the interface's GoDoc consistently if that change is selected.
+
+This is a public contract opportunity, not dead code that can be removed without
+review. External consumers may call WriteExplanation through View.Explanation;
+repository searches cannot establish otherwise. Account for that source impact
+under the [compatibility policy](../../policy/release.md#versioning-and-compatibility).
+Do not reinstate provider-owned text rendering merely to give the method a caller.
+
+## M7 — P3: Terminal and ordinary messages duplicate detail-rendering mechanics
+
+**Code:** [reporter.go](../../../reporter.go), renderTerminalLocked lines 393–423
+and submitMessageLocked lines 439–491; recordRendererErrorLocked and
+combineOperationErrors supply the common error classification.
+
+**Complexity:** both paths skip nil detail callbacks, create a coordinated writer,
+invoke each callback, distinguish its rendering error from its writer error,
+retain the first of each, and combine the results. Ordinary messages add the
+hidden-detail/io.Discard branch. This duplicates delicate bookkeeping inside a
+reporter that already coordinates visibility, locking, deferred delivery, and
+outcome finalization. The concern is duplicated policy, rather than file length
+or a large score for either individual function.
+
+**Potential refactoring:** extract a private detail-rendering helper that runs
+under the existing reporting lock and returns separate rendering and writing
+errors. Let the callers retain headings, visibility selection, leading-message
+failure handling, and destination delivery-state updates. Keep the visible versus
+discarded writer choice explicit so hidden details still run as selected.
+
+Preserve nil-detail handling, callback order/lifetime, sticky writer failures,
+joined error evidence, non-reentrancy, and terminal explanation timing. Keep one
+coordination boundary; additional locks or an interchangeable rendering pipeline
+would add complexity. This extraction is independent of the deferred-finalization
+correctness finding in the earlier audit.
+
+## Assessment and suggested order
+
+M1, M2, and M4 are local extractions with clear phase boundaries. M7 consolidates
+one existing internal policy. M3 has broader grammar impact and should retain
+distinct execution and completion policies. M5 and M6 need deliberate public
+contract decisions because the integrations are independently released modules.
+
+Other high-scoring areas were examined without additional findings. The finite
+alias termination proof has inherent algorithmic complexity and an expressly
+selected design. YAML conversion's type/tag and alias checks protect selected
+representation rules; replacing them with a generalized conversion framework
+would not be a demonstrated simplification. The large interactive completion test
+combines several necessary shell cases, while already sharing its terminal driver.
+The composed example's larger functions mostly contain explicit wiring and fixture
+data. None warrants a rewrite solely because of a score or line count.