ConfigKit/docs/roadmap/remediation.md

# Pre-release Maintainability Remediation

## Status and scope

Status: in progress; completed stages and their validation are recorded below.
Audit baseline: `9b147bfd96aec25e0125dcae66e136e56dc7a670`.

This roadmap owns the agreed maintainability cleanup before v0.1.0. It replaces
the completed implementation-defect remediation record, whose findings, fixes,
and validation evidence remain available in Git history. It does not reopen
those defects or replace the original [implementation plan](implementation.md).

The audit found worthwhile consolidation and ownership improvements, rather than
a need to redesign ConfigKit. The offline default suite and vet passed at the
audit baseline. That evidence is not validation of future changes or a release
candidate.

Read the [development guide](../development.md),
[architecture policy](../policy/architecture.md),
[testing policy](../policy/testing.md), and
[documentation policy](../policy/documentation.md) before implementation.
The [configuration contract](../config.md) owns current semantics; the
[component overview](../internal/overview.md) owns implementation inventory.
Release preparation remains governed by the [release policy](../policy/release.md)
and [maintainer procedure](../release.md).

## Execution rules and invariants

Implement the numbered stages in order. Each stage is one reviewable unit of
work: inspect its current owners and tests, make the change, validate it, update
its status and evidence here, and commit the nonempty unit with a short plain
English message. Do not create empty commits. Record any discovered behavioral
defect separately with a reproduction rather than silently changing semantics
under a refactoring stage.

Keep the root public package and the existing `internal/document` and
`internal/yaml` packages. Do not introduce dependencies, public API changes,
provider interfaces, configurable merge policies, or a generic visitor framework.
Private file splits do not require new packages or ADRs. Parser-specific types
must remain behind the existing YAML adapter boundary.

Preserve the implemented contracts, particularly:

- Strict composition remains distinct from intentional profile/CLI overlay.
- Input precedence, final-only typed decoding, retained defaults, canonical
  decoding explanation events, and recorded domain resolution remain unchanged.
- Source identity, containment, optional-import rules, limits, diagnostic codes,
  source coordinates, ordering, and early callback termination remain intact.
- Semantic paths retain explicit key/index distinctions and historical shapes.
- Returned values and inspection snapshots remain detached; secret values remain
  absent from history, rendering, and library-generated diagnostics.
- Resolution runs before secret lookup and validation; ordinary loading never
  bootstraps environment variables implicitly.

Use focused existing behavioral tests during each stage and the baseline checks
from the development guide before committing production changes. Add tests only
for meaningful gaps exposed by extraction, not to enforce file locations or
private call choreography. Update current owning documentation alongside each
move. Planned filenames below are private implementation destinations, not new
consumer contracts.

## Stage 1 — Remove unused overlay value snapshots

Status: complete.

Completion: removed unread overlay before/after trees and their construction and
cloning, including the now-unused CLI position lookup. Field-read review confirmed
that resolution snapshots and history applications have separate live consumers;
those and displaced origins remain intact. The full offline test suite, vet,
build, and whitespace checks passed with Go 1.26.7 on Linux/amd64.

Evidence: `overlayRecord` in [overlay.go](../../overlay.go) retains `before` and
`after` trees; profile and CLI processing construct them and
`cloneOverlayRecords` clones them. Production consumers use the record's change
and displaced origins, while explanation uses `historyApplication` instead.

Work:

1. Reconfirm all field reads before removal. Remove only the unused overlay
   snapshot fields and their construction/cloning in `overlay.go` and
   [override.go](../../override.go).
2. Preserve `change`, `displaced`, input applications, and their ownership rules.
   Do not remove resolution snapshots or retained decoded trees: those have
   separate consumers and budget semantics.
3. Remove locals and imports made unnecessary by the deletion. Do not replace
   the removed state with a new cache or history representation.

Acceptance: profile/CLI provenance, explanation replay, composition independence,
and redaction tests pass unchanged in observable behavior. Source inspection
confirms no unused overlay snapshot construction remains. An allocation benchmark
is not required to justify removing provably unread clones.

## Stage 2 — Centralize parser issue translation

Status: complete.

Completion: `parserDiagnostics` in `parse.go` now owns detached, canonical
translation for selected, imported, and profile sources. Existing tests cover
safe selected-source errors and imported/profile source attribution, so no
duplicate regression was added. Full offline tests, vet, build, and whitespace
checks passed with Go 1.26.7 on Linux/amd64.

Evidence: [parse.go](../../parse.go) translates adapter issues inline, while
`importParseDiagnostics` in [imports.go](../../imports.go) repeats the translation
for imports and profiles.

Work:

1. Define one private `parserDiagnostics` helper in `parse.go` that maps adapter
   issues to detached public diagnostics, including related locations.
2. Route selected-source, import, and profile parsing through it. Remove the
   import-specific translator and keep upstream parser errors private.
3. Return canonically ordered diagnostics from the shared translation boundary.
   Preserve existing codes, messages, primary/related coordinates, and source IDs.
   Keep per-source parsing budgets and graph accounting at their current owners.

Acceptance: malformed selected/imported/profile input retains safe, correctly
attributed diagnostics. Use existing parser and import diagnostic tests; add a
small cross-entry regression only if a source path lacks equivalent coverage.

## Stage 3 — Share bounded regular-file read mechanics

Status: complete.

Completion: `source_file.go` owns regular-file preflight/open/recheck/close and
bounded reads, with private failure kinds and stat/open-only optional-missing
classification. Callers retain containment, metadata, budgets, accounting, and
limit diagnostics. Existing filesystem, FIFO, budget, and reader tests pass;
a private regression covers nondeterministic filesystem failure phases. Full
offline baseline and race suites passed with Go 1.26.7 on Linux/amd64.

Evidence: `readSource`, `importTraversal.readImport`, and `readProfileFile` repeat
preflight stat, open, post-open stat, regular-file checks, size checks, and bounded
reads in [source.go](../../source.go), [imports.go](../../imports.go), and
[profile.go](../../profile.go).

Work:

1. Introduce private helpers in `source_file.go` for regular-file opening and
   bounded reading. Use narrow function parameters for stat/open operations so
   callers can supply `os.Stat`/`os.Open` or root-bound methods. Do not introduce
   an exported filesystem interface or substitute unrestricted opening for
   `os.Root` access.
2. Keep preflight before open and post-open verification. Close every opened
   file on all paths. Preserve the existing treatment of read-only file close
   errors; root-directory close diagnostics are a separate responsibility.
3. Return private classified failures with enough distinction for callers to
   map errors safely. Optional imports may suppress only the currently allowed
   missing-target failures at stat/open, not later read failures. Do not expose
   filesystem errors downstream through wrapping.
4. Keep source metadata, canonicalization, containment, per-source/aggregate
   budgets, byte accounting, and caller-specific limit diagnostics outside the
   shared mechanics. Pass the computed byte limit into the reader and retain
   both the size precheck and limit-plus-one read enforcement.
5. Reuse bounded read mechanics for reader/byte sources only where it simplifies
   existing code without changing single-use or ownership behavior. Keep secrets
   bootstrap independent: its symlink, environment, and reporting policies differ.

Acceptance: source, import, profile, FIFO, containment, symlink, optional-missing,
exact-budget, and reader-ownership tests pass. Retain the deadline-isolated FIFO
regression. Cover private failure classification only where public tests cannot
reliably induce an important filesystem failure. Run race tests for this unit.

## Stage 4 — Establish shared document traversal ownership

Status: complete.

Completion: `document_walk.go` owns nil-safe preorder value walking, borrowed
indexing, and semantic lookup. Metadata shapes and projected sensitivity use the
walker with defensive path copies. Specialized origins and budget traversals
remain explicit. Full offline tests (including historical path shapes, metadata
detachment, and dynamic secrets), vet, build, and whitespace checks passed with
Go 1.26.7 on Linux/amd64.

Evidence: `indexDocument`, `walkDocumentPaths`, and `projectedSensitivity` repeat
semantic tree traversal in [load_metadata.go](../../load_metadata.go),
[metadata.go](../../metadata.go), and [secret.go](../../secret.go). General document
lookup currently lives in [load.go](../../load.go).

Work:

1. Create root-private `document_walk.go` with a nil-safe preorder walker taking
   a starting `Path` and a callback receiving both path and node. Visit mappings
   in stored order and sequences in index order; construct explicit Key/Index
   segments. It visits value nodes, not synthetic mapping-key nodes.
2. Move `indexDocument` and `documentAtPath` to this owner. Implement indexing,
   metadata shape collection, and projected sensitivity collection using the
   walker where their traversal semantics match.
3. Document callback borrowing: callers must clone paths or nodes when retaining
   mutable state. Preserve existing defensive copies and avoid sharing mutable
   metadata across results.
4. Keep path-bearing helpers in the root package so `internal/document` does not
   depend on the public package. Pure node representation and cloning stay in
   `internal/document`.
5. Leave specialized traversals explicit where they need inherited origins,
   pruning, node identity deduplication, or different counting. In particular,
   input limits count mapping keys and retained-history limits count shared nodes
   once; do not implement either using the new walker without those semantics.

Acceptance: numeric map keys versus sequence indices, escaped paths, unknown
paths, historical shape changes, deleted descendants, dynamic secret tags, and
metadata detachment remain protected by existing tests. No public path behavior
or predicate coverage changes. Do not add tests that assert walker call counts.

## Stage 5 — Separate typed projection, cloning, and comparison

Status: complete.

Completion: projection, detached cloning, shared reflection state, and value-only
comparison now have separate owners. Named secret clone modes preserve the two
ownership barriers. Validation combines value and sensitivity comparison in a
named helper and separately audits resolved secret state. Standard `cmp.Compare`
replaces the custom comparator; tests changed only for private symbol renames.
Full offline baseline and race suites passed with Go 1.26.7 on Linux/amd64,
including codec/cycle/limit, callback ownership, sensitivity, and secret-state
regressions.

Evidence: [projection.go](../../projection.go) combines roughly 622 lines of
projection, detached cloning, dynamic schema handling, cycle/limit enforcement,
and structural comparison. Value equality deliberately ignores sensitivity,
which validation must check separately.

Work:

1. Keep canonical typed projection in `projection.go`; move cloning into
   `clone.go`, shared reflection operation state and cycle helpers into
   `value_operation.go`, and value-tree comparison into `document_compare.go`.
   Preserve the descriptor-driven approach and per-operation schema cache.
2. Rename the private equality/comparison entry points to make configuration-value
   semantics explicit. Document that location and sensitivity metadata are
   excluded, mapping order is immaterial, and sequences are compared atomically
   for change reporting. Update all callers without changing their semantics.
3. Give validator mutation comparison a named private owner in `validation.go`.
   It must combine projected value changes and sensitivity changes, while
   retaining private resolved-secret-state auditing as an additional check.
   Do not make resolver value comparison start treating sensitivity as a value.
4. Preserve the distinction between cloning unresolved public secret references
   and publication cloning of already validated private state. Name/document
   the clone mode explicitly rather than leaving an unexplained boolean at
   sensitive call sites.
5. Replace custom `cmpString` calls with the standard library comparator and
   remove that helper. Do not perform unrelated test-style rewrites.

Acceptance: projection/cloning tests cover nil versus empty, custom text codecs,
inline fields, dynamic types, cycles, and limits. Validator mutation regressions
must still reject equal-text sensitivity changes and secret-state changes.
Callback ownership and secret lookup tests must prove publication does not share
borrowed values or reread the environment. Run race tests for this unit.

## Stage 6 — Clarify source assembly and override ownership

Status: complete.

Completion: filesystem context/canonicalization/containment moved to
`filesystem.go`; directive parsing and restrictions moved to `directives.go`;
input node counting now shares the document owner with explicit key accounting.
Override preparation, parser-neutral value building, and application are separate;
profile and CLI application share the composition clone. Deferred profile cleanup
preserves primary failures and explicitly checks the success-path close once.
Full offline tests, vet, build, and whitespace checks passed with Go 1.26.7 on
Linux/amd64, including imports/globs, profiles, CLI, limits, and provenance.

Evidence: [imports.go](../../imports.go) mixes filesystem context, directives,
graph traversal, and diagnostics; [override.go](../../override.go) combines value
construction/validation and application. Shared composition cloning is located
inside the override feature, while profile overlay repeats it.

Work:

1. Keep import traversal and route diagnostics in `imports.go`. Move filesystem
   context validation/canonicalization/containment to `filesystem.go`, and
   directive structures/parsing/fragment restrictions to `directives.go`.
   Give source-independent node counting an appropriate shared document owner,
   with its mapping-key accounting documented.
2. Keep override validation and preparation in `override.go`, move parser-neutral
   value building to `override_value.go`, and application/displacement mechanics
   to `override_apply.go`.
3. Move `cloneComposedDocument` next to `composedDocument` in `compose.go`; use it
   for both profile overlay and overrides. Ensure every retained field is copied
   according to its existing ownership contract.
4. Keep strict composition, overlay, and history replay separate. Share small
   mechanics when identical; do not introduce merge strategy flags or one general
   merge engine. Preserve the existing profile-versus-CLI recording behavior.
5. Simplify profile root cleanup using deferred failure-path cleanup plus an
   explicit checked success-path close. Preserve primary failure diagnostics and
   the existing successful-read root-close error diagnostic; avoid double-close
   effects. Import traversal cleanup may remain explicit where already clear.

Acceptance: imports/globs, directive restrictions, source ordering, profiles,
overrides, version restrictions, limits, and composition independence tests pass.
No new cross-package dependencies appear. Root handles close on every path and
profile/CLI provenance remains identical.

## Stage 7 — Extract explicit loader lifecycle phases

Status: complete.

Completion: `load.go` now visibly coordinates ordered phases using source assembly
in `load_input.go`, option selection in `options.go`, and typed phase outputs and
publication in `load_result.go`. Version checking lives in `validation.go`.
One composition initializer covers profiles, CLI, and NoSource overrides without
changing budgets. Diagnostics and source handoffs remain explicit, both ownership
clones remain, and publication adds no history copies. Full offline baseline and
race suites passed with Go 1.26.7 on Linux/amd64; lifecycle, early-stop, warning,
precedence, provenance, callback ownership, and secret tests passed unchanged.

Evidence: `loadWithSecretLookup` in [load.go](../../load.go) coordinates roughly
200 lines of source assembly, decoding, callbacks, history, secrets, and result
publication, including repeated creation of a composition handoff.

Work:

1. Keep the exported entry point and a readable linear coordinator in `load.go`.
   Move option selection to `options.go`, input assembly to `load_input.go`, and
   typed lifecycle/result publication to `load_result.go`. Move version checking
   beside the validation machinery rather than keeping it among document helpers.
2. Use small private handoff structs for assembled input and necessary phase
   outputs, not a mutable all-purpose loader object or a long list of output
   parameters. Reuse one composition initialization helper for profile and CLI
   processing, including NoSource override handling and source byte/node totals.
3. Preserve the exact existing order of option validation, parsing/import checks,
   schema validation, version checking, profile/CLI application, defaults factory,
   typed decoding, resolution, retained-history limit enforcement, secret lookup,
   validation, metadata/classification, and result publication.
4. Keep diagnostic/source accumulation explicit. Warnings survive successful
   phases; errors stop dependent work and never return a usable partial Result.
5. Preserve both ownership barriers: detach resolver-borrowed values before secret
   resolution and validation, then detach equal-valued validator replacements
   before publication while preserving validated secret state. Do not remove
   either clone as apparently redundant work.
6. Group Result's frozen inspection state only if it clarifies this handoff; do
   not add an additional history copy or change caller ownership of Config.

Acceptance: lifecycle ordering/early-stop tests, defaults-only loading, all input
combinations, callback ownership, secrets, warning retention, and complete
provenance agreement pass. Keep orchestration visible enough to verify phase
ordering by reading the coordinator. Run the complete race suite.

## Stage 8 — Separate explanation replay, snapshots, and rendering

Status: complete.

Completion: public records and Explain coordination remain in `explain.go`;
immutable applications, replay/coalescing, and shared-node budgeting live in
`history.go`; shared effective/explanation redaction lives in `snapshot.go`;
human output lives in `explain_render.go`. Metadata versus replay ownership is
documented beside history. Existing reconciliation, historical sensitivity,
container/leaf, retained-budget, detachment, and writer-failure tests passed
without expectation changes. Full offline tests, vet, build, and whitespace
checks passed with Go 1.26.7 on Linux/amd64.

Evidence: [explain.go](../../explain.go) combines approximately 576 lines of public
records, replay algorithms, snapshot redaction, retained-history accounting, and
human-readable output.

Work:

1. Keep public explanation records and Explain coordination in `explain.go`.
   Move application/snapshot replay and history budgeting to `history.go`, safe
   snapshot projection to `snapshot.go`, and WriteExplanation with its formatting
   helpers to `explain_render.go`.
2. Make effective rendering and explanation snapshots use the existing shared
   redaction implementation; do not create separate sensitivity rules.
3. Retain the canonical decoding reconciliation event between input replay and
   domain resolution, overlapping resolution intent coalescing, added/deleted
   paths, effective source attribution, and historical secret classification.
4. Document the differences between metadata origin records and value replay:
   they serve different query contracts but must agree on effective results.
   Do not replace them with a new event-store abstraction in this cleanup.
5. Preserve writer failure behavior and ownership of retained trees. Do not cache
   explanations or optimize full-tree replay without measured need.

Acceptance: explanation, effective rendering, history budget, provenance, secret
redaction, and writer-failure tests pass without output changes. Container and
leaf histories still reconcile with the frozen effective typed configuration.

## Stage 9 — Complete ownership documentation and integration validation

Status: pending.

Work:

1. Review the final code for obsolete helpers, duplicate traversal/translation,
   unread retained state, stale comments, and imports left by earlier stages.
   Confirm file splits reflect discrete responsibilities rather than line quotas.
2. Update the component overview to describe the actual owners and fix its stale
   claim that metadata queries are empty. Keep detailed consumer semantics in
   their existing contract; repair navigation and moved-file links throughout
   documentation, including this roadmap's evidence links if necessary.
3. State the retained dependency direction clearly: public lifecycle coordination
   uses private source/document/schema/inspection helpers; the owned document
   representation has no dependency on the root package; only the private YAML
   adapter imports the upstream YAML package.
4. Run the full baseline and race suites, local documentation and dependency
   boundary checks, and the complete source-candidate checker described in the
   release procedure with available qualifying toolchains. Record exact versions,
   host platform, results, and any unavailable platform evidence. Cross-builds
   are not native execution evidence.
5. Review the accumulated diff for public API/configuration/diagnostic changes.
   Expected outcome: no consumer migration or dependency change. If any contract
   change is discovered, resolve it explicitly before closing this roadmap.
6. Record completion evidence and commit this final nonempty unit. Release
   preparation must subsequently retire completed roadmap material and validate
   the exact committed candidate under release policy. Completing this roadmap
   does not create a tag, push refs, or authorize publication.

Acceptance: all stages are complete with recorded validation, existing behavioral
regressions remain protected, documentation matches the new ownership layout,
and the worktree is clean after commits. Required release gates remain visible
in the release procedure rather than being declared satisfied by this refactor.

## Open Questions

None required to begin implementation. The package boundaries, compatibility
constraints, ordered units, and acceptance criteria above are decided. Routine
private naming adjustments are acceptable when they preserve those decisions;
new behavioral discoveries require explicit evidence and disposition.