diff --git a/docs/roadmap/complexity.md b/docs/roadmap/complexity.md index 3b167e0..ebe65be 100644 --- a/docs/roadmap/complexity.md +++ b/docs/roadmap/complexity.md @@ -4,6 +4,11 @@ Status: proposed target state. This roadmap defines a behavior-preserving refactor; a later implementation plan will define stages and exact private signatures and file names. +The combined proposal for the next scope is +[ConfigKit Consolidation Toward 1.0](consolidation.md). It incorporates this +internal target, the later audit, and a review of consumer-facing simplification. +Use that proposal for scope review; this document retains the earlier analysis. + ## Purpose Make ConfigKit's import, inheritance, and projection implementation easier to diff --git a/docs/roadmap/consolidation.md b/docs/roadmap/consolidation.md new file mode 100644 index 0000000..8241137 --- /dev/null +++ b/docs/roadmap/consolidation.md @@ -0,0 +1,407 @@ +# ConfigKit Consolidation Toward 1.0 + +Status: proposed scope and target state for review. This document recommends +refactors; it does not record implementation progress or authorize a release. +Prepare the staged implementation plan after reviewing this roadmap. + +## Purpose + +Consolidate the machinery introduced by recent import, collection, inheritance, +and discovery work so ConfigKit is easier to understand, maintain, and adopt +before its public contracts become stable. Success means fewer independently +maintained paths through the same behavior, clearer state ownership, and concise +consumer workflows. Reducing a function's line count is useful only when it +serves those outcomes. + +The review considered public API consolidation as well as internal extraction. +The recommended scope retains the existing simple and layered loading APIs, +consolidates their private translation and acquisition machinery, and removes +unused reverse inspection conversions. No public signature or configuration +semantic change is currently recommended. This is an assessment of the reviewed +consumer workflows, not a requirement to preserve every beta API in perpetuity. + +## Evidence And Relationship To Existing Work + +Reviewed worktree: `c9574ef`, immediately after the `v0.10.0` release at `5959537`. +The inputs are the earlier [complexity roadmap](complexity.md), whose reviewed +baseline predates discovery, and the new [complexity audit](remediation.md). +The older roadmap already acknowledges implemented discovery, but its selected +work is narrower than this proposal. + +This review inspected the current public load/input/discovery declarations and +maintained consumer examples, the shared request executor and acquisition flow, +import parsing and selection, inheritance execution, reflective projection and +cloning, schema field construction, and filesystem-context derivation. Graph +caller traces and text-reference checks confirmed the unused reverse conversion +chain. This was source review, not a new runtime correctness or performance audit. +The audit's complexity scores locate candidates; they are not standardized +acceptance thresholds or evidence of a runtime defect. + +| Audit finding | Assessment and recommended treatment | +| --- | --- | +| 1. Import entry points and responsibilities | Main refactor. Include shared acquisition normalization, import-default ownership, declaration parsing, and directory admission. | +| 2. Shared inheritance executor | Main refactor. Expose meaningful phases and explicit combination results while retaining one executor. | +| 3. Projection and cloning | Main refactor for projection; bounded follow-up for substantial clone handlers, preserving the distinction between the operations. | +| 4. Schema field discovery and inline assembly | Include a focused refactor coordinated with projection. Keep one schema builder and its two descriptor views. | +| 5. Resolved filesystem context | Include alongside acquisition work. Reuse resolved facts without merging resource lifetimes or changing validation timing. | +| 6. Unused reverse inspection conversions | Include as an independent cleanup in the root facade and engine. Retain useful forward-conversion and detached-result coverage. | + +This is the combined proposal for scope review, rather than a second independent +implementation of the earlier roadmap. Keep the audit as evidence; its findings +are not closed by adopting this document. Completed correctness and algorithmic +remediations remain part of the baseline, including sequence records preserved +at `ea46001:docs/roadmap/remediation.md` and discovery records preserved at +`0369111:docs/roadmap/remediation.md`. + +The [architecture policy](../policy/architecture.md), +[configuration contract](../config.md), and exported declaration comments own +existing constraints and behavior. The +[component overview](../internal/overview.md) and +[import internals](../internal/imports.md) locate implementation responsibilities. + +## Consumer-Facing Target + +### Keep Simple Loading Simple And Advanced Inputs Explicit + +The current APIs already provide progressive setup: + +- A simple consumer calls `Load[Config](File(path))`. +- A consumer with defaults, imports, or resolution supplies `Options[Config]`. +- Explicit discovery produces an ordinary `Source`, so it uses the same loading + call and lifecycle. +- A layered consumer supplies an `Input` and `LayeredOptions[Config]`, with + `Underlay.Inputs` distinguishing strict peers from precedence tiers. + +The two loading functions already converge on `loadRequest` and +`executeLoadRequest`. Their public differences have meaning: ordinary options +include the one input's controls; layered loading has per-input controls and a +separate overlay filesystem. Ordinary overlay context follows its selected +source, whereas layered overlay context is independently specified. These are +observable conventions, not interchangeable forwarding spellings. + +**Recommendation:** retain `Load`, `LoadLayered`, `Options`, `LayeredOptions`, +`Input`, and `Underlay`. Remove repeated translation and option reconstruction +behind them. Keep common application rules in `Definition`; do not require +applications to build a runtime loader or manually coordinate phases. + +A single public request type could place main input, underlays, and common +options together. It would reduce duplicated option fields and make promotion +from ordinary to layered loading more uniform. However, a sole request-based +entry would add setup to the simplest call; retaining a convenience entry would +still leave two call forms. Either approach must explicitly resolve the existing +overlay-context differences. The maintained examples do not yet show enough +consumer benefit to justify that redesign. This recommendation is based on the +resulting API, rather than the amount of refactoring needed to achieve it. + +### Preserve Useful Convenience And Ownership Boundaries + +| Surface reviewed | Recommended public end state | +| --- | --- | +| `Source`, `Filesystem`, `Search`, filename groups and presets | Retain typed, reusable source/search descriptions. Discovery selects a concrete source during loading; applications do not loop over complete loads. | +| `FilesystemContext` | Retain the public description and current host/native rules. Consolidate its resolved private representation; do not expose acquisition handles or engine context records. | +| `SequenceIdentityAt` and `SequenceIdentityMatching` | Retain the exact-path convenience and dynamic selector form over the common registration model. Both serve distinct consumer shapes. | +| `Definition`, resolution and validation callbacks | Retain application-owned domain rules inside one recorded lifecycle. Preserve the distinction between mutable recorded resolution and read-only validation. | +| Results, metadata, explanations, source context and diff | Retain detached, safe inspection with consumer-owned presentation. Remove unused inverse conversions without removing supported queries. | +| Root wrappers and private conversions | Retain root-owned public types and explicit ownership conversion under ADR 0006. Consolidate repeated real conversions and delete unused ones. | + +The consumer-facing work is to make the ordinary path easier to find and show +how additional features compose, using the existing maintained examples. Update +the adoption guide with a concise choice between simple loading, discovery, and +layered loading, and show which options belong to an input versus a whole load. +Keep exact behavior in its canonical owners. Examples should make application +rules recognizable and avoid requiring consumers to recreate generic merging, +presence, or provenance machinery. + +Use the current simple README call, bundled-file and layered catalog examples, +the import-focused prompt catalog, and the discovered sequence catalog to assess +the end state. The latter exercises imports, underlays, independent overlay +context, CLI assignment, and recorded resolution in one workflow. A high number +of options in that example does not by itself justify a new abstraction. + +## Internal Target + +Keep the root facade, cohesive `internal/engine`, ordered document model, and +private YAML adapter. Use ordinary private records, functions, and focused files +to express responsibilities. Existing mapping combination, sequence combination, +dependency scheduling, schema descriptors, and traversal accounting remain shared. + +`executeLoadRequest` stays the visible lifecycle coordinator. It should show +acquisition, assembly, inheritance, decoding/CLI assignment, resolution, secret +processing, validation, and publication in their established order. Extract a +phase only when it gives that work a clear input, output, or ownership boundary. +Do not replace the visible lifecycle with a registry of configurable stages. + +### 1. Normalize Loading Inputs And Consolidate Import Acquisition + +The current executor constructs an engine `Options[T]` from `loadRequest`, and +underlay acquisition constructs `Options[struct{}]` to call the same input +assembler. `assembleBaseInputTrackedForRole` then combines discovery, directive +processing, version rules, imports, placement, accounting, and contexts. Import +entry points carry long positional lists through several forwarding variants. +This repeats the representation of information already normalized for a load. + +Introduce cohesive private acquisition inputs and explicit state ownership, +reusing `loadRequest`, `loadInput`, `acquisitionScope`, and `importTraversal` +where they fit. Translate ordinary and layered public declarations into the +shared loading model once. Acquisition should receive the source, input role, +placement, context, and policies it needs without a fabricated application type +or a reconstructed broad options object. Small adapters may preserve meaningful +entry operations while callers migrate; remove obsolete forwarding variants +within the completed refactor. + +Separate immutable policy, mutable processing state, and results. Move import +defaults out of `sourceVersionState` and into the load/acquisition policy owner. +Version state continues to own version processing, projected effective version +location, evidence, and diagnostics. Local and effective version rules have +different meanings and must remain distinguishable. + +The state boundaries are especially important: + +| Lifetime or scope | State and responsibility | +| --- | --- | +| Whole load | Aggregate resource accounting, application definition/registrations, import defaults, shared version evidence and diagnostics. | +| One strict tier | Repetition tracking shared by that tier's inputs, namespace scheduling anchors, and its inclusion/composition graph relationship. A later tier may reuse a file. | +| Active import traversal | Active routes and inclusion edges, filesystem access ownership, and cycle detection, including edges crossing item-scope boundaries. | +| One composition scope | Peer contributions, pending collection declarations, item-local helpers, and deferred item placement. Nested scopes do not become separate precedence tiers. | +| Retained result | Source records, effective destinations, origins, history, and passive resource-context descriptions. | + +The older roadmap's broad description of load-wide repetition state should not +be implemented as a global visited-file restriction. In the current code, +`acquireUnderlayGroupScopes` creates a fresh visited map per group. Preserve valid +reuse across tiers while keeping shared aggregate limits. Namespace equality, +candidate spelling, and stable scheduling keys also serve different purposes. + +Give acquisition, already-acquired scope assembly, and evidence projection +focused homes. Keep discovery at the acquisition boundary: resolve a search once +per input consumption, then process the concrete file or absence. Preserve +source IDs and source-local version processing in acquisition order, even when +composition uses a different dependency-ready order. The layered main is acquired +before underlays; lower-tier identity can still inform its later strict assembly. + +Within import parsing, extract boolean/collection/placement field handling and +selector-value parsing where it clarifies responsibilities. Keep field presence, +authored error locations, selector applicability, and the order of consequential +validation visible. Within directory selection, separate entry admission and +descent decisions from traversal orchestration. Retain the existing root +deduplication, candidate union, exclusions, sorting, and active-path cycle rules. +No shared abstraction should conflate literal first-match main discovery with +multi-file import selection. + +### 2. Share Resolved Filesystem Facts Without Sharing Live Handles + +`importFilesystem`, `nativeFilesystemAccess`, and `contextsForAcquiredSources` +derive overlapping root/base/namespace facts. The latter reconstructs context +from combinations of source kind and optional fields, including absent input. +Discovery and independent layered overlays make these combinations consequential. + +Use a small engine-owned resolved-context description for the facts actually +needed by acquisition and passive views: namespace identity/kind, resolved root, +source base, and selected source path when present. Preserve the distinction +between host paths and native slash paths. Reuse already resolved facts where +available instead of repeatedly deriving them from the original options. + +Resolution and validation can occur at different existing lifecycle points. +The record must not force every source to perform all filesystem checks early. +Context descriptions, library-owned acquisition handles, and returned passive +resource views have different lifetimes. Closing acquisition must remain +explicit on success and failure; retaining context must not retain an open root +handle or assume ownership of a borrowed `fs.FS`. + +Cover selected and absent discovery outcomes, root-only absent host context, +native sources, in-memory inputs with explicit context, and independently rooted +overlays. Passive context remains associated with the source that contributed +the value, including retained underlay and inherited values. Use the acquired +source's canonical identity where available. Preserve containment failures, +source associations, deferred search bases, and terminal selected-file errors. + +Implement this work alongside acquisition consolidation so the same callers are +not redesigned twice. The description is private; it does not replace the public +`FilesystemContext` or `ResourceContext` contracts. + +### 3. Make Inheritance Combination Results Explicit + +Keep the mapping and identified-record adapters and one shared executor. +`applyInheritanceScopeTemplates` currently prepares templates, mutates values +through merge callbacks, projects policy and origins, records replacements and +replay evidence, and enforces budgets in one body. The callbacks mutate several +surrounding result collections, making a combination difficult to review alone. + +Give the executor named operations for child/control preparation, parent payload +and policy preparation, combination, evidence construction, and retained-node +accounting. Use a small private result for outputs of one combination where it +replaces unrelated surrounding mutations. Value, policy, origin, replacement, +and sequence-position evidence should come from the same combination outcome; +do not rerun a merge to reconstruct its history. + +Keep borrowed authored templates separate from owned resolved payloads and from +retained history snapshots. Parent identity exclusion and child identity/origin +preservation must apply consistently to values and history. Preserve nested +unresolved instructions for destination-local evaluation, policy retirement on +actual replacement, cancellation, and source-version movement through sequences. + +The shared Kahn scheduler with stable keys remains unchanged. Resolve enclosing +relationships before nested relationships and retain assembled record order. +Preserve expansion limits and retained-history accounting for shared nodes; +extraction must not charge shared evidence twice or make an unbounded snapshot. + +### 4. Separate Projection Handlers And Clarify Schema Assembly + +Keep `projectConfiguration` as the schema-checked entry and `valueOperation` as +the operation-state owner. Move substantial struct, map, and sequence projection +into focused handlers. Keep pointer/dynamic dispatch and scalar representation +choices visible. All handlers share the same cycle, depth, and node state. + +Struct projection needs particular care: physical members drive traversal, +flattened fields drive name lookup and sensitivity, and an inline container is +not retained as an extra output node. Preserve declaration order, deterministic +map order, nil/empty distinctions, collision diagnostics, codec failures, and +secret classification. Keep cycle entries active through child traversal and +balance their release on every exit. + +In `schemaBuilder.describeFields`, separate field discovery and tag parsing from +inline struct assembly, inline map assembly, and ordinary field registration. +Retain one builder/cache and its recursion detection. Make the paired updates to +`members`, flattened `fields`, `fieldByName`, and `inlineMap` easy to follow. +Keep index-path construction and inherited secret flags intact. These descriptor +views are both needed; combining them into one collection would shift complexity +into decoding and projection. + +Follow with a bounded cleanup of substantial clone handlers where it makes their +ownership behavior easier to read. Keep cloning distinct from projection: +cloning reconstructs typed values, preserves physical fields, round-trips text +codecs, and distinguishes unresolved from already validated secret state. +Projection builds a canonical document and charges output nodes. Extract common +helpers only where the semantics actually agree. The mostly explicit dispatch +in `decodeValue` and `valueBuilder.build` does not warrant a generic reflection +visitor solely because it has a high complexity score. + +### 5. Remove Unused Inspection Conversions And Consolidate Real Adapters + +Remove `toEngineSourceInfo`, `toEngineOrigin`, the exclusive reverse version and +import-selection converters, and the engine adapters used only by that chain. +At the reviewed revision, the three callers of `toEngineSourceInfo` are tests; +`toEngineOrigin` has none. `SourceInfoWithPlacement`, +`SourceInfoWithImportSelections`, and their exclusive string-to-enum helpers +have no production role beyond that reverse chain. Recheck callers at the +implementation baseline before removal. + +Rewrite the affected record tests around real engine-to-public conversion and +detached public inspection. Preserve useful placement, version evidence, +import-selection, and copy-isolation assertions. Avoid manufacturing replacement +production setters solely to keep an old round-trip fixture working. Existing +facade loads already provide genuine sources and inspection records. + +Retain reverse conversions that serve real inputs: `toEngineDiagnostics`, for +example, supports `WriteDiagnostics`. Consolidate repeated override/input +conversion in `toEngineOptions` and `toEngineLayeredOptions` where semantics +match. Keep explicit root-to-engine ownership conversion and the public-boundary +checks. A reflective transport framework or public engine aliases would obscure +the boundary rather than simplify it. + +## Scope, Compatibility, And Policy Consequences + +The proposed work spans private engine code, private root-facade conversions, +affected tests, consumer guidance, and implementation documentation. This is a +deliberate expansion beyond the older engine-only roadmap. Preserve public +types/signatures and the behavior owned by the current contracts, including: + +- Acquisition versus composition ordering, strict scopes versus precedence + tiers, identity ownership, placement, and source-version protection. +- Discovery exhaustion and error behavior, source lifetime, passive resource + contexts, and host/native namespace distinctions. +- Defaults, post-decode CLI assignment, recorded resolution, secret handling, + final registered-identity enforcement, and semantic validation. +- Explicit presence, provenance/history, sensitivity, safe diagnostics and diff, + frozen inspection, and detached publication/query results. +- Contracted diagnostic codes, paths, coordinates, ordering, and resource limits. + +Do not turn incidental helper order, exact private representations, numerical +complexity scores, or allocation counts into new guarantees. Keep the existing +algorithmic remediations effective; watch for repeated whole-tree traversals, +policy scans, or copies introduced by helper extraction. + +The recommendation fits the existing architecture policy and ADRs 0006, +0015, 0017, 0018, and 0019. It needs no new architectural ownership decision or +policy revision merely to introduce private helpers. Update the component +overview and import internals when code moves. Accepted ADR decision content +remains unchanged. If review selects a materially different public loading model, +assess its behavioral consequences explicitly before implementation. + +Pre-1.0 public changes are possible under the [release policy](../policy/release.md), +but they must have a consumer benefit and documented impact. This draft does not +propose a parallel compatibility API or temporary migration scaffold. It also +does not make refactoring completion a new release gate or establish that 1.0 +is otherwise ready. Future performance experiments remain in +[future.md](future.md#3-performance-experiments); no benchmark campaign was run +or is required to claim readability improvements. + +## Recommended Work Order + +This is dependency guidance for the later plan, not a commit-by-commit schedule: + +1. Confirm the reviewed revision and public-surface direction; map the retained + behavior to existing tests and record concrete coverage gaps. +2. Remove unused reverse inspection conversions and adjust their tests. This is + small and independently reviewable. +3. Normalize acquisition inputs and separate import-default ownership. Develop + the resolved-context handoff with those callers, preserving validation and + closure timing; then separate acquisition, item assembly, and evidence work. +4. Refactor inheritance around explicit preparation and combination results, + retaining completed correctness and algorithmic regressions. +5. Refactor projection and schema field assembly in separate reviewable changes; + complete the bounded clone cleanup where it improves the remaining flow. +6. Consolidate real facade adapters, finish consumer/internal navigation, and run + the applicable integration and ownership checks on the combined result. + +Steps 4 and 5 have no necessary implementation dependency on each other. Keep +correctness fixes discovered during refactoring identifiable with their own +regression evidence rather than hiding behavior changes inside code movement. + +## Validation And Completion + +Use the [development guide](../development.md) and +[testing policy](../policy/testing.md) for focused checks and the ordinary full +development check. Preserve existing behavior tests; rewrite tests coupled only +to removed plumbing. Add a test when a consequential handoff risk lacks coverage, +not for every new helper. + +| Work area | Existing protection and review focus | +| --- | --- | +| Acquisition and imports | Selection, defaults, scheduling, relative/item placement, source-version evidence, aggregate limits, repetition within versus across tiers, and ordinary/layered equivalence. | +| Resolved contexts | Host/native access, discovery selected/absent outcomes, terminal context failures, source-relative resources, independent overlay context, and handle closure. | +| Inheritance | Mapping and record inheritance, nested destination-local evaluation, child identity, policy lifetime, version movement, history ordering, sensitivity, and retained-node budgets. | +| Projection/schema/clone | Inline fields, recursive schemas, dynamic values, nil/empty values, cycles and limits, text codecs, secret states, callback ownership, and independent loads. | +| Facade and consumers | Forward conversion, detached records, supported API boundaries, source-form equivalence, maintained simple/catalog/discovery examples, and safe explanation/diff. | + +Examples of established boundaries include +[source-form equivalence](../../facade_equivalence_test.go), +[discovery integration](../../facade_discovery_test.go), +[final identity validation](../../facade_final_sequence_identity_test.go), +[inheritance history](../../internal/engine/inheritance_history_test.go), and +[projection and cloning](../../internal/engine/projection_test.go). +Use applicable race checks when changing ownership or shared state. Reuse checks +for unchanged content and keep benchmark work separate unless new evidence +justifies it and it is authorized. + +Completion requires reviewable evidence that: + +- Simple and advanced consumer examples remain concise and valid, with no new + public concepts required for existing workflows. +- Both load entry points reach one normalized internal model; acquisition no + longer reconstructs broad options or uses version state to transport defaults. +- Import, context, inheritance, and reflection responsibilities have identifiable + owners and explicit handoffs without duplicate semantic implementations. +- Unused reverse conversions and their exclusive adapters are removed, with + useful inspection ownership coverage retained. +- Contracted values, presence, provenance/history, diagnostics, sensitivity, + resource lifetime, and algorithmic corrections remain protected by appropriate + tests and source review. +- The reviewed scope is accounted for against all six audit findings; completed + findings have validation evidence rather than being closed because code moved. +- Consumer guidance, contributor references, and internal documentation describe + the resulting implementation. Current contracts continue to own behavior. + +Private type/helper names, final file splits, and exact stage boundaries belong +in the implementation plan. The public-surface recommendation and scope above +are ready for review; no additional feature semantics are needed to carry out +the recommended refactors. diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index e4d1191..9b878c3 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -3,6 +3,11 @@ Status: open recommendations for consideration. This is an audit record, not an implementation plan or a new release gate. +The [consolidation roadmap](consolidation.md) 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