Files
ConfigKit/docs/roadmap/remediation.md

15 KiB
Raw Blame History

Complexity Audit Findings

Status: open recommendations for consideration. This is an audit record, not an implementation plan or a new release gate.

The consolidation roadmap evaluates all six findings alongside the earlier internal roadmap and the public API. It proposes one coordinated scope; the findings remain open until implementation and validation establish their disposition.

Reviewed revision: 5959537d1adc2516e7f7e8881df72b707ec48c3f.

Scope And Method

The review considered production Go code, package boundaries, private interfaces, and supporting tests. The local codebase-memory graph was refreshed, and its function-size, cognitive-complexity, branch, and parameter metrics were used to rank candidates. Source inspection and inbound call traces then distinguished mixed responsibilities from ordinary dispatch and deliberate boundary code. Text-reference searches checked the apparently unused conversions in finding 6. No benchmark campaign or runtime correctness testing was part of this audit.

The graph indexed 901 non-test functions/methods and 928 test-file declarations. Its fast index excludes examples, documentation, and scripts; the tracked-file inventory covered those separately. Standalone gocyclo, gocognit, scc, and cloc executables were not available. The following scores are the graph tool's reported cognitive scores, used for relative triage rather than as limits or standardized acceptance criteria. Function spans include comments and blank lines within the declaration.

Function Source Lines Cognitive score
valueOperation.project projection.go 195 174
parseImportDeclarationSpec import_spec.go 127 129
valueOperation.clone clone.go 130 104
schemaBuilder.describeFields schema.go 99 100
assembleBaseInputTrackedForRole load_input.go 188 96
selectDirectoryImportCandidates import_selection.go 154 96
valueBuilder.build value.go 104 93
decodeValue decode.go 115 85
applyInheritanceScopeTemplates inheritance_schedule.go 219 75
contextsForAcquiredSources source_context.go 102 72

The private engine contains 88 production Go files and approximately 20,144 physical lines, compared with 16 files and 1,861 lines in the root facade. imports.go is the largest production file at 1,195 lines. Size identifies where to inspect; it does not establish that the engine needs to be split into packages.

Findings 1–3 confirm opportunities already selected in the pending complexity roadmap. Findings 4–6 add narrower opportunities. They should be coordinated with that roadmap rather than implemented as an independent overlapping refactor. No ordinary correctness defect was established by this static review.

1. Consolidate Import Entry Points And Separate Import Responsibilities

Status: open. Priority: higher maintenance benefit. Scope: internal/engine.

Evidence and complexity. imports.go contains parallel forwarding ladders such as loadImportedDocumentsTrackedWithVersion, loadImportedDocumentsTrackedWithProcessing, and loadImportedDocumentsTrackedWithProcessingDeferred, plus corresponding loadImportedWithAccess... variants. Both final signatures carry 13 declared parameters, counting names in grouped declarations. Related values must remain aligned across these calls: placement, version state, processing results, sequence policy, accounting, repetition tracking, namespace identity, and deferred assembly.

The same file also owns source admission, filesystem access lifetime, recursive acquisition, pending-item assembly, and evidence projection. Source admission in importTraversal.visitPath is 174 lines. The 188-line input assembler in load_input.go adds discovery, source-role, directive, version, placement, and resource-context decisions around that flow. Import defaults are carried through sourceVersionState, although import defaulting and version checking are independent concerns. Readers have to follow that incidental state relationship to understand ordinary import behavior.

Potential refactoring. Follow the roadmap's cohesive request/context design, with named inputs and explicit ownership of load-wide state. Remove forwarding variants that only add another optional argument; retain helpers that express distinct operations. Give acquisition, already-acquired scope composition, and evidence projection focused homes. Keep load-wide import defaults with an appropriate acquisition/load context rather than requiring version state to be their transport.

Within that work, extract coherent declaration-field parsing and directory-entry admission operations from parseImportDeclarationSpec and selectDirectoryImportCandidates. Keep their selector-policy checks and deterministic traversal visible. Moving code into smaller functions without clarifying state handoffs would not resolve the main maintenance problem.

Preserve acquisition order, strict scope scheduling, item-local composition, source/version evidence, existing limits, and explicit resource closure. The existing import/discovery and ordinary/layered integration tests are the relevant behavioral protection.

2. Expose The Phases Of The Shared Inheritance Executor

Status: open. Priority: higher maintenance benefit. Scope: internal/engine.

Evidence and complexity. applyInheritanceScopeTemplates in inheritance_schedule.go coordinates template preparation, directive controls, retained-node accounting, parent identity exclusion, inherited collection-policy preparation, recursive mapping/sequence combination, origin transfer, replacement history, replay operations, final policy projection, and scope rebuilding in one 219-line body. Its merge callbacks mutate several surrounding results, so the evidence produced by one combination is difficult to assess independently of the whole executor.

Potential refactoring. Keep the useful mapping/identified-sequence adapters and one shared executor. Extract parent/child preparation, combination, evidence construction, and accounting into meaningful operations with small, explicit result records where appropriate. Make it clear which values are borrowed templates and which are owned resolved results. Keep one sequence-combination result as the source of value, policy, origin, and historical-position evidence.

This is the inheritance refactor already described in the complexity roadmap. Preserve dependency-ready ordering, enclosing-before-nested evaluation, child identity, policy ownership, history budgets, and sensitivity. Reuse the mapping and identified-record inheritance, history, and ownership tests.

3. Decompose Reflective Projection And Review Clone Handlers Separately

Status: open. Priority: higher maintenance benefit for projection; secondary for cloning. Scope: internal/engine.

Evidence and complexity. valueOperation.project in projection.go has the highest reported cognitive score. Its struct branch alone handles member traversal, inline flattening, key collisions, flattened-container accounting, and secret-field classification. The enclosing dispatcher also handles pointer/cycle lifetime, dynamic descriptors, maps, sequences, and scalar codecs. valueOperation.clone in clone.go has a similar large dispatcher, including distinct secret-copy modes and text-codec round trips.

Potential refactoring. Implement the roadmap's focused struct/map/sequence projection handlers while retaining valueOperation as the traversal-state owner. Keep the dispatch and scalar representation decisions easy to locate. Apply the same readability review to the substantial clone branches as a separate small follow-up if it remains beneficial; keep cloning and projection as distinct operations.

The high-scoring valueBuilder.build and decodeValue are predominantly explicit type dispatch; decoding already delegates collection handling. Their scores alone do not justify a generic reflection visitor. Share only operations whose semantics actually match. Preserve balanced cycle tracking, depth/node accounting, inline order, nil/empty distinctions, codecs, and secret behavior. Existing projection, clone, schema, decoding, callback-ownership, and independent-load tests cover the important risks.

4. Separate Schema Field Discovery From Inline Field Assembly

Status: open. Priority: medium maintenance benefit. Scope: internal/engine.

Evidence and complexity. schemaBuilder.describeFields in schema.go nests field enumeration and tag handling around pointer-chain inspection, recursive-inline detection, inline struct flattening, index-path construction, inline-map admission, duplicate checks, secret propagation, and construction of both members and flattened fields. Those two descriptor views serve different traversal needs, and their consistency currently depends on understanding the full nested routine.

Potential refactoring. Retain one schema builder/cache. Extract named inline struct and inline-map assembly operations and make ordinary-field registration clear. Small helpers can centralize the paired updates to members, fields, fieldByName, and inlineMap, including inherited secret classification, where the same registration semantics apply. Keep pointer and recursive-type decisions explicit rather than moving them into a configurable registration framework.

Preserve accepted field/tag forms, cache/building-state behavior, declaration order, ambiguity diagnostics and paths, and schema/projection/decode agreement. Existing inline, recursive-schema, text-codec, secret, and ambiguity tests are the appropriate protection.

5. Give Resolved Filesystem Context A Clear Shared Representation

Status: open. Priority: medium maintenance benefit. Scope: acquisition and passive context handling within internal/engine.

Evidence and complexity. filesystem.go derives host/native roots, selected-file bases, and containment during import and context validation. contextsForAcquiredSources in source_context.go independently derives much of the same information, then computes each source's root-relative base. It branches among native sources, explicit native contexts, selected host files, absent host input, and in-memory sources before branching again for relative-path calculation. The lifecycle calls it from ordinary acquisition and both selected overlay paths, so a change in context derivation has several integration points.

Potential refactoring. Consider a small engine-owned resolved-context record that names the namespace, root, selected-source path, and base directory. Share the overlapping derivation/relative-base operations across acquisition and passive-context construction, while keeping host and fs.FS path rules explicit. Pass already-resolved facts where they are available instead of reconstructing them from source-kind and optional-context combinations.

Validation, acquisition access, and returned passive views have different jobs; a refactor should preserve their existing timing and ownership. In particular, reuse of a context description need not retain an acquisition handle or make previously deferred checks eager. Preserve selected-file identity, source-ID association, containment diagnostics, absent-input behavior, and independent overlay contexts. Existing native/host filesystem, discovery integration, resource-context, and facade tests provide the behavioral baseline.

6. Remove Unused Reverse Inspection Conversions Across The Facade Boundary

Status: open. Priority: smaller, well-bounded cleanup. Scope: root configkit and internal/engine together.

Evidence and complexity. In conversion.go, toEngineSourceInfo is called only by three tests in records_test.go. Its helpers toEngineSourceVersionCheck and toEngineImportSelections have no callers outside that reverse-conversion chain. toEngineOrigin has no callers. The corresponding engine setters SourceInfoWithImportSelections and SourceInfoWithPlacement in records.go exist only for that chain, as do importSelectorFromString and importSelectionOutcomeFromString.

Consumers receive these inspection records; the production facade does not send them back into the engine. Maintaining an unused reverse path adds a second representation/ownership mapping whenever inspection evidence grows, and the round-trip tests make that machinery appear necessary despite its lack of a runtime caller.

Potential refactoring. Remove the unused inverse conversions and their exclusive engine-side adapters together. Rewrite the affected tests around engine-to-public conversion and detached public query records, preserving useful placement, version-evidence, and import-selection ownership checks. Continue to exercise real source creation/loading through the existing facade integration tests. This can reduce both production interface surface and test scaffolding without adding a transport framework.

Retain the forward inspection conversions and root-owned public types. Other reverse conversions have real users: for example, toEngineDiagnostics supports the public diagnostic renderer. Apply caller-based removal rather than deleting conversions merely because they look symmetrical.

Architectural Assessment And Follow-up

The package direction remains coherent: root-owned public wrappers delegate to the engine; the engine uses the ordered document model and the private YAML adapter. Most complexity is concentrated in multi-responsibility engine routines, rather than distributed through unnecessary packages. The smaller document and YAML packages have clear responsibilities; their tree visitors and the facade's explicit conversions are not inherently maintenance defects.

The strongest next investment is the pending complexity roadmap's import, inheritance, and projection work. Finding 6 is an independently reviewable cleanup; findings 4 and 5 can be assessed as focused follow-ups. Package extraction or public API redesign is not needed to pursue these findings. Keep the current contracts as the behavioral baseline and use the development guide to select proportionate validation during implementation. This audit introduces no new complexity thresholds, configuration restrictions, ownership guarantees, or release requirements.