diff --git a/docs/roadmap/deadcode.md b/docs/roadmap/deadcode.md new file mode 100644 index 0000000..cee891d --- /dev/null +++ b/docs/roadmap/deadcode.md @@ -0,0 +1,307 @@ +# Dead Code And Obsolete Functionality Audit + +Reviewed revision: `b1c278b38bead1fceeb80b99d2f336b0cb6fe2fd`. +Audit date: 2026-10-25. + +## Scope And Method + +This audit covers the public facade, engine, document and YAML packages, +consumer examples, and development/release entry points. It identifies private +implementation cleanup, not public features to retire or new release gates. +All findings below are open opportunities for further consideration. + +Knowledge-graph discovery and caller tracing were supplemented by a temporary, +type-aware reference inventory built outside the repository with the locally +cached `golang.org/x/tools` v0.49.0. It distinguishes declarations, production +references, test references, and direct field writes. Static inventories were +run for Linux, Darwin, and Windows; only Linux tests were executed. A separate +SSA/Rapid Type Analysis pass over test executables supplied additional leads. +Neither absent graph edges nor an unreachable result from the selected test +programs was treated as proof of dead library code. In particular, uninstantiated +generic declarations and unexercised public APIs require caller/source review. + +The standalone `deadcode`, `unused`, and `staticcheck` commands were not installed. +Building the cached standalone `deadcode` command offline encountered an uncached +telemetry dependency; the temporary analysis instead used cached packages/SSA/RTA +libraries directly. Repository dependencies and tooling were not changed. + +Candidates were checked for interface dispatch, reflection, whole-record use, +map-key equality, build constraints, public consumers, and documented retention. +The existing Linux suite and `go vet ./...` pass with Go 1.26.10. No benchmarks, +new tests, or production changes were made. + +## Architectural Assessment + +The [implemented architecture](../internal/overview.md) remains coherent. The +root owns the supported API and explicit conversions; `internal/engine` owns +synchronous acquisition, composition, inheritance, decoding, CLI application, +resolution, validation, and frozen inspection. `internal/document` owns the +ordered tree, and `internal/yaml` owns parser/dumper integration. Ordinary and +layered loading share one lifecycle. + +Most findings are remnants beside that shared lifecycle: unused snapshots, +private clone families disconnected from the facade, obsolete helper entry +points, and bookkeeping whose results no longer reach an active consumer. +No supported public feature or deliberate compatibility mechanism was established +as obsolete. Existing correctness and resource findings remain owned by their +separate audit documents. + +## 1. Remove The Unconsumed Override-Result Pre-CLI Snapshot + +Priority: medium. Scope: engine loading and override handoff. + +**Relevant code.** `decodedOverrideResult.preCLI` and `applyDecodedOverrides` +in [override_decode.go](../../internal/engine/override_decode.go), lines 9–18, +33, and 79; `preparePostDecodeOverrideContext` in +[override_context.go](../../internal/engine/override_context.go), lines 20–84. + +**Evidence.** `applyDecodedOverrides` clones the complete `decoded.final` tree +and returns it in `preCLI`. Type-aware references identify no production read +of that particular field. Its readers are assertions in `override_test.go` and +`override_context_test.go`. The production recipient instead independently clones +`before.final` into `postDecodeOverrideContext.preCLI`. + +This performs an extra traversal and allocation proportional to the decoded +tree on successful ordinary and layered loads, including loads without CLI +assignments. It does not supply the pre-CLI snapshot used by explanation. + +**Potential cleanup.** Remove the unused result member and its clone, and +update assertions tied to that member. Keep the independently owned pre-CLI +state that actually feeds resolution/publication and explanation. This is not a +proposal to share mutable trees or redesign the remaining ownership handoffs. + +## 2. Retire The Unqueried Published Structural-Input Clone + +Priority: medium. Scope: engine publication and immutable lookup. + +**Relevant code.** `Inspection.structuralInput` in +[load.go](../../internal/engine/load.go), line 96, and `publishLoadResult` in +[load_result.go](../../internal/engine/load_result.go), lines 112, 119, and 131. +Lookup ownership is in +[document_lookup.go](../../internal/engine/document_lookup.go); active replay is +in [explain.go](../../internal/engine/explain.go). + +**Evidence.** Publication clones `resolved.structuralInput`, stores the clone +on `Inspection`, and indexes it as an additional immutable tree. No production +reader accesses `Inspection.structuralInput`; one private test populates it. +Explanation replays source applications and inheritance operations, then uses +`preCLI` and `input`. It does not query this separately cloned structural root. +The lookup is addressed by a supplied node pointer, not by searching all indexed +roots, so indexing the otherwise unreachable clone does not make it useful. +Its mapping-node keys also keep the tree alive. + +**Potential cleanup.** Remove the final publication clone, unused inspection +member, and that clone's inclusion in `retainedTrees`. Preserve +`resolved.structuralInput` where it is still used for path finalization and +sensitivity classification before publication. Do not indiscriminately remove +earlier structural evidence or active source/application history. + +Coordinate this bounded removal with finding 2 in the +[resource audit](resources.md): removing an unused tree does not by itself fix +accounting for the other retained replay trees. + +## 3. Remove The Facade's Disconnected Record-Clone Family + +Priority: low. Scope: root facade, across the root/engine conversion boundary. + +**Relevant code.** The private helper block in +[records.go](../../records.go), lines 317–430: +`cloneLocation`, `cloneLocations`, `cloneSourceInfo`, `cloneImportSelections`, +`cloneSourceVersionCheck`, `cloneSourceInfos`, `cloneOrigin`, `cloneFieldInfo`, +`cloneChange`, `cloneDiagnostic`, and `cloneDiagnostics`. + +**Evidence.** None is reachable from a production facade entry point. +`cloneSourceInfos`, `cloneFieldInfo`, `cloneChange`, and `cloneDiagnostics` +have no references at all. Their supporting helpers refer to one another; +the only external callers of this family are private `records_test.go` calls +to `cloneSourceInfo`. Public results already acquire root-owned records through +`fromEngineSourceInfo`, `fromEngineOrigin`, `fromEngineFieldInfo`, +`fromEngineChange`, and diagnostic conversions in +[conversion.go](../../conversion.go). The engine's separate clone helpers are +active and are not removal candidates. + +**Potential cleanup.** Delete the disconnected root helper family. Keep relevant +conversion checks, and express ownership checks through repeated public record +queries and mutation of returned nested data rather than testing an unused +clone API. This complements the facade-test observations in the +[testing audit](testing.md); it does not eliminate explicit conversions, +root-owned types, or active `Path.clone` callers. + +## 4. Remove Uncalled Engine Helper Remnants + +Priority: low. Scope: individual engine helpers and facade transport. + +The following definitions have no callers or function-value references in +production or tests, across the inspected build configurations: + +| Definition | Relevant code | Evidence and active replacement | +| --- | --- | --- | +| `mergeDisjointMappings` | [compose.go](../../internal/engine/compose.go), line 486 | Its only reference is its own recursion. Strict composition uses `mergeDisjointMappingsAtPath` and the shared recursive merge machinery. | +| `mergeHistoryDocument` | [history.go](../../internal/engine/history.go), line 240 | No caller remains. Replay calls `mergeHistoryDocumentAtPath` directly with sequence evidence. | +| `inheritanceDirectiveLocation` | [inheritance.go](../../internal/engine/inheritance.go), line 176 | No caller remains. Active preparation uses the directive entry and its location directly. | +| `addSecretEvidencePath` | [secret.go](../../internal/engine/secret.go), line 137 | No caller remains. Classification obtains projected sensitivity paths from version evidence and feeds the active path accumulator. | +| `PlacementModeFromFacade`, `PlacementModeForFacade` | [destination.go](../../internal/engine/destination.go), lines 23 and 26 | Neither is used by the facade. Input/options decorators perform the incoming conversion; `SourceInfoPlacementModeForFacade` supplies outgoing evidence. | + +**Potential cleanup.** Delete these definitions and any imports/comments left +solely for them, keeping their active counterparts. These are private functions +or internal-package exports, not supported root API. The unused placement pair +was already identified in finding 5 of the [complexity audit](complexity.md); +this audit confirms that bounded removal without expanding its broader transport +refactor. + +## 5. Stop Constructing Unconsumed Combination Evidence + +Priority: medium. Scope: sequence combination and inheritance bookkeeping. + +**Relevant code and evidence.** + +- `sequenceCombinationResult.discardedLower` in + [sequence_combine.go](../../internal/engine/sequence_combine.go), lines 26 + and 72, is populated with a newly allocated list of every lower sequence + index during replacement. It has no production reader; three test assertions + inspect it. Overlay and inheritance owners already handle whole replacement + from the selected operation and lower value. The result therefore allocates + an unused `O(n)` index slice for an `n`-item lower sequence. +- `inheritanceReplacement.path`, `.previous`, and `.effective` in + [inheritance.go](../../internal/engine/inheritance.go), lines 135–141, are + written by [inheritance_combine.go](../../internal/engine/inheritance_combine.go) + and [inheritance_collection.go](../../internal/engine/inheritance_collection.go) + but never read by production. Populating them performs path/origin copies + and `displacedOrigins` subtree walks whose results are discarded. The active + history path uses `inheritanceOperation`, its origin maps, and + `projectedInheritanceChanges` in + [inheritance_history.go](../../internal/engine/inheritance_history.go). + The replacement's `before` and `after` trees, however, are read by retained-node + accounting in [inheritance_finalize.go](../../internal/engine/inheritance_finalize.go) + and [history.go](../../internal/engine/history.go). + +**Potential cleanup.** Remove the unread members and computations exclusively +feeding them. Preserve higher/lower item projections, replacement mappings, +`appendedHigher`, active provenance/history, and replacement node accounting. +Keep `sequenceIndexes` where append still uses it, and keep `displacedOrigins` +where overlay processing needs it. Adjust private evidence assertions while +retaining behavior-level replacement, removed-descendant, and history tests. +This consolidates duplicate evidence ownership rather than removing inspection +functionality or changing collection semantics. + +## 6. Trim Write-Only Private Result And Transport Metadata + +Priority: low. Scope: engine phase handoffs and private result shapes. + +The following members are populated but have no production reader. Some are +inspected by private tests; none is exposed as a root record or callback field. +They add apparent responsibilities to result shapes that active consumers do not +actually use: + +| Private record and members | Relevant code and evidence | +| --- | --- | +| `collectionPolicyResolution.declarations`, `declarationsPresent`, `identityDeclarationPresent`, `applicationIdentityFields` | [collection_policy.go](../../internal/engine/collection_policy.go), lines 14–24. The resolver uses local normalized declarations/fields to compute the active policy; these returned copies/flags are not consumed. Preserve `modeDeclarationPresent` and `applicationIdentityPresent`, which are consumed. | +| `inputPresence.null` | [decode.go](../../internal/engine/decode.go), line 25. Presence collection writes it, but current metadata null reporting uses the effective node's kind. The presence entry and its location remain active. | +| `rootDirectiveExtraction.importsPresent`, `sequencesPresent` | [directives.go](../../internal/engine/directives.go), lines 25–26. Parsing sets them, but callers use the parsed declaration slices and diagnostics. | +| `importSelection.outcome` | [import_selection.go](../../internal/engine/import_selection.go), line 29, and [glob.go](../../internal/engine/glob.go). Selectors set it, but traversal uses candidate lists and admitted-source counts. The separate `importSelectionEvidence.outcome` is consumed by public source-record conversion and must remain. | +| `importDeclarationSpec.excludeSet` | [import_spec.go](../../internal/engine/import_spec.go), line 38. Parsing sets it; active policy resolution consumes `exclude`, not this flag. | +| `schemaPath.field`, `dynamic` | [schema.go](../../internal/engine/schema.go), lines 335–339. Path resolution tracks and returns them, but callers consume only the descriptor and success result. | +| `selectedOverlayRequest.filesystem` | [selected_overlay.go](../../internal/engine/selected_overlay.go), line 33. Callers populate it, but the loader uses its explicit filesystem-access argument. | +| `sequenceDeclarationSpec.destinationLocation` | [sequence_declaration.go](../../internal/engine/sequence_declaration.go), line 17. The parser stores it, but later diagnostics use other retained declaration/field locations. | +| `sourceVersionEvidence.absence`, `checked`; `sourceVersionAncestor.kind` | [source_version.go](../../internal/engine/source_version.go), lines 27–43. These annotations have no production consumers. Actual checking, version/presence/location evidence, and ancestor paths are consumed separately. | +| `postDecodeOverrideContext.sources` | [override_context.go](../../internal/engine/override_context.go), line 14. The local source list is used to build resolution input, but returning it again in this member serves only tests. | + +**Potential cleanup.** Remove these unused result slots and any bookkeeping +whose sole purpose is filling them. Keep computations that validate or produce +the active result, and keep public presence, null, selection, version, ownership, +and diagnostic behavior unchanged. In particular, do not remove the source list +used for callback input, public empty-selection evidence, declaration validation, +or filesystem access. Rework assertions on inactive annotations to check the +actual supported outcome where they protect a meaningful risk. + +This is a bounded handoff cleanup, not a requirement to remove every field with +no explicit selector read: map keys, embedded records, and interface contracts +need separate treatment. + +## 7. Consolidate The Test-Only Import Directive Parser + +Priority: low. Scope: engine import parsing and test fixtures. + +**Relevant code.** `parseImportSpecificationDirective` in +[import_spec.go](../../internal/engine/import_spec.go), lines 183–201; +`parseImportSpecsYAML` in +[import_spec_test.go](../../internal/engine/import_spec_test.go); and +`parseRootDirectiveBlock` in [directives.go](../../internal/engine/directives.go). + +**Evidence.** The older parser has exactly one direct caller, the test fixture +helper. That helper feeds many selector/declaration tests, but no supported load +uses this parser. The active root parser owns the same mapping/field dispatch +and delegates import declarations to the same `parseImportDeclarations` function, +while also supporting the appropriate sequence-declaration context. This is a +parallel directive-validation implementation, not merely a thin test adapter. + +**Potential cleanup.** Have the fixture helper use the active root-directive +parser with the appropriate context, or parse the import declaration list +directly when that is the test's actual boundary. Remove the obsolete production +parser after preserving relevant malformed-declaration and selector coverage. +Do not retire the active imports-only context wholesale: layered version +processing still uses it after extracting the outer directives. + +## 8. Remove The Test-Only Eager Acquisition Mode + +Priority: medium. Scope: engine acquisition. Previously recorded in finding 1 +of the [complexity audit](complexity.md); not a second independent defect. + +**Relevant code.** `inputAcquisitionRequest.deferAssembly` in +[acquisition.go](../../internal/engine/acquisition.go), and the final branch of +`acquireImportedWithAccess` in +[import_acquisition.go](../../internal/engine/import_acquisition.go), lines 206–224. + +**Evidence.** Both production request construction sites, `executeLoadRequest` +in [load.go](../../internal/engine/load.go) and `acquireUnderlayGroupScopes` +in [load_request.go](../../internal/engine/load_request.go), set the flag to +true. Only private test entry helpers choose the eager path. Its assembly call +and alternate return protocol are therefore disconnected from supported loading. + +**Potential cleanup.** Make acquisition consistently return retained scopes. +Tests needing composition can explicitly invoke the active composition phase +through a test helper. Preserve pending-item reservation, resource closure, +accounting, diagnostics, and composition coverage. Thin test conveniences that +delegate to active implementations are not generally dead functionality; the +problem here is the alternate production phase protocol maintained solely for +tests. + +## Retained Functionality And Review Limits + +- Public exports remain supported even when current in-repository programs do + not call them. Discovery presets, `Metadata.HistoricalPaths`, generic APIs, + diagnostic codes, and optional controls are not removal recommendations. +- Heap methods, filesystem adapters, error/string/format methods, writer + methods, reflection descriptors, callback bindings, and platform-specific + implementations may be live without direct calls. They were not classified + as dead on reference counts alone. Nonzero-size identity tokens and + whole-struct map keys also have meaningful behavior without field reads. +- `decodeInlineMapEntry` and import cycle diagnostics have real production + callers despite gaps in current execution coverage. They remain necessary. +- `Inspection.resolution` has no production selector reader, but the + [recorded-resolution contract](../config.md#recorded-application-resolution) + documents retaining each recorded before/after snapshot. This audit does not + propose dropping that retention merely because explanation uses a coalesced + replay view. Any separate retention redesign must respect that contract. +- The retained inclusion graph, historical paths, sensitivity evidence, and + passive contexts serve supported behavior. Deferred experiments in + [future work](future.md#3-performance-experiments) are not obsolete features + to remove. Neither are deliberate overlay/default/version semantics. +- This audit establishes cleanup candidates, not measured performance gains or + an exhaustive proof that all unsupported input states are unreachable. + +## Validation + +The following checks passed against unchanged production code: + +```sh +env PATH=/snap/go/current/bin:$PATH GOTOOLCHAIN=local GOTELEMETRY=off \ + GOPROXY=off GOCACHE=/tmp/configkit-review-cache-shared \ + go test ./... -count=1 +env PATH=/snap/go/current/bin:$PATH GOTOOLCHAIN=local GOTELEMETRY=off \ + GOPROXY=off GOCACHE=/tmp/configkit-review-cache-shared go vet ./... +``` + +The audit document is additionally checked by the existing local documentation +link test and whitespace checks. No production code, tests, dependencies, +policies, public contracts, or release machinery are changed by this audit.