526 lines
33 KiB
Markdown
526 lines
33 KiB
Markdown
# 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 and settling
|
||
the [open questions](#open-questions).
|
||
|
||
## 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 Baseline
|
||
|
||
Reviewed worktree: `c9574ef`, immediately after the `v0.10.0` release at `5959537`.
|
||
This document replaces the earlier `complexity.md` roadmap and `remediation.md`
|
||
audit record. Their recommendations and relevant evidence are incorporated here;
|
||
the original documents remain available in Git history:
|
||
|
||
```sh
|
||
git show fbaaaf1:docs/roadmap/complexity.md
|
||
git show fbaaaf1:docs/roadmap/remediation.md
|
||
```
|
||
|
||
The earlier roadmap reviewed `7a83704f43ef9870db33661231cdff5d6ee502f7`, before
|
||
discovery. The audit reviewed `5959537d1adc2516e7f7e8881df72b707ec48c3f`, including
|
||
implemented discovery. The consolidation review checked their recommendations
|
||
against that newer baseline and also considered the public API.
|
||
|
||
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.
|
||
Neither this review nor the audit established a runtime correctness defect.
|
||
|
||
### Audit Measurements
|
||
|
||
The audit refreshed the codebase-memory graph and used function size, cognitive
|
||
complexity, branch counts, and parameter counts to rank candidates for source
|
||
inspection. It indexed 901 non-test functions/methods and 928 test-file
|
||
declarations. Its fast index excludes examples, documentation, and scripts; a
|
||
tracked-file inventory covered those separately. Standalone `gocyclo`, `gocognit`,
|
||
`scc`, and `cloc` tools were unavailable.
|
||
|
||
These are the graph tool's reported measurements at the audited revision.
|
||
Function spans include comments and blank lines. The scores support relative
|
||
triage; they are not standardized acceptance thresholds or evidence of a defect.
|
||
|
||
| Function | Source | Lines | Cognitive score |
|
||
| --- | --- | ---: | ---: |
|
||
| `valueOperation.project` | [projection.go](../../internal/engine/projection.go) | 195 | 174 |
|
||
| `parseImportDeclarationSpec` | [import_spec.go](../../internal/engine/import_spec.go) | 127 | 129 |
|
||
| `valueOperation.clone` | [clone.go](../../internal/engine/clone.go) | 130 | 104 |
|
||
| `schemaBuilder.describeFields` | [schema.go](../../internal/engine/schema.go) | 99 | 100 |
|
||
| `assembleBaseInputTrackedForRole` | [load_input.go](../../internal/engine/load_input.go) | 188 | 96 |
|
||
| `selectDirectoryImportCandidates` | [import_selection.go](../../internal/engine/import_selection.go) | 154 | 96 |
|
||
| `valueBuilder.build` | [value.go](../../internal/engine/value.go) | 104 | 93 |
|
||
| `decodeValue` | [decode.go](../../internal/engine/decode.go) | 115 | 85 |
|
||
| `applyInheritanceScopeTemplates` | [inheritance_schedule.go](../../internal/engine/inheritance_schedule.go) | 219 | 75 |
|
||
| `contextsForAcquiredSources` | [source_context.go](../../internal/engine/source_context.go) | 102 | 72 |
|
||
|
||
At that revision, the engine contained 88 production Go files and approximately
|
||
20,144 physical lines; the root facade contained 16 files and 1,861 lines.
|
||
`imports.go` was the largest production file at 1,195 lines. These measurements
|
||
locate concentrated responsibilities. They do not establish a need for more
|
||
packages or make the smaller document/YAML visitors and explicit facade
|
||
conversions maintenance defects.
|
||
|
||
### Findings And Recommended Treatment
|
||
|
||
The finding numbers below preserve the audit's identifiers. All six remain open
|
||
work; retiring their original documents does not mark them implemented.
|
||
|
||
| Audit finding | Maintenance priority | Assessment and recommended treatment |
|
||
| --- | --- | --- |
|
||
| 1. Import entry points and responsibilities | Higher | Main refactor. Include shared acquisition normalization, import-default ownership, declaration parsing, and directory admission. |
|
||
| 2. Shared inheritance executor | Higher | Main refactor. Expose meaningful phases and explicit combination results while retaining one executor. |
|
||
| 3. Projection and cloning | Higher for projection; secondary for cloning | Main refactor for projection; conditional, bounded follow-up for substantial clone handlers, preserving the distinction between the operations. |
|
||
| 4. Schema field discovery and inline assembly | Medium | Include a focused refactor coordinated with projection. Keep one schema builder and its two descriptor views. |
|
||
| 5. Resolved filesystem context | Medium | Include alongside acquisition work. Reuse resolved facts without merging resource lifetimes or changing validation timing. |
|
||
| 6. Unused reverse inspection conversions | Small, independently reviewable cleanup | Include in the root facade and engine. Retain useful forward-conversion and detached-result coverage. |
|
||
|
||
Findings 1–3 confirm the earlier roadmap's main opportunities; findings 4–6 add
|
||
focused work. The following sections own their rationale and intended changes.
|
||
Implementation and validation will establish each finding's disposition.
|
||
|
||
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`.
|
||
Item-scope application matching and inheritance policy localization are examples
|
||
of completed corrections that overlap this work and whose regressions should
|
||
remain protected.
|
||
|
||
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.
|
||
|
||
In [imports.go](../../internal/engine/imports.go), the
|
||
`loadImportedDocumentsTrackedWithVersion`,
|
||
`loadImportedDocumentsTrackedWithProcessing`, and
|
||
`loadImportedDocumentsTrackedWithProcessingDeferred` variants have corresponding
|
||
`loadImportedWithAccess...` variants. Both final signatures carry 13 parameters.
|
||
Placement, processing results, sequence policy, accounting, repetition tracking,
|
||
namespace identity, and deferred assembly must stay aligned through the chain.
|
||
The same file's `importTraversal.visitPath` combines source-admission decisions
|
||
in 174 lines, alongside acquisition, pending-item assembly, and evidence work.
|
||
|
||
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. |
|
||
|
||
Keep repetition tracking tier-scoped. 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 `parseImportDeclarationSpec`, 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 `selectDirectoryImportCandidates` and
|
||
`importTraversal.visitPath`, separate admission and descent decisions from
|
||
traversal orchestration where they represent distinct work. 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.
|
||
Context construction is used by ordinary acquisition and both selected-overlay
|
||
paths, so those callers need to use the same applicable derivation rules.
|
||
|
||
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. Scalar projection retains the existing number, duration,
|
||
text-codec, and secret-reference representations. 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.
|
||
Keep pointer-chain and recursive-inline decisions explicit, preserve supported
|
||
field/tag forms and ambiguity paths, and retain cache/building-state behavior.
|
||
|
||
Provisionally treat clone cleanup as a conditional follow-up, as recommended in
|
||
[OQ-2](#oq-2-how-much-clone-cleanup-should-this-effort-commit-to). Extract substantial
|
||
handlers where they make ownership behavior easier to read after the projection
|
||
and schema work; completing this scope need not require clone edits if the
|
||
remaining flow is already clear. 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`, `toEngineSourceVersionCheck`, and
|
||
`toEngineImportSelections` from [conversion.go](../../conversion.go), along with
|
||
the engine adapters used only by that chain.
|
||
At the reviewed revision, the three callers of `toEngineSourceInfo` are tests;
|
||
`toEngineOrigin` has none. `SourceInfoWithPlacement`,
|
||
`SourceInfoWithImportSelections`, `importSelectorFromString`, and
|
||
`importSelectionOutcomeFromString` in [records.go](../../internal/engine/records.go)
|
||
have no production role beyond that reverse chain. Consumers receive inspection
|
||
records; production facade operations do not send those records back to the
|
||
engine. The unused inverse path adds another ownership mapping to maintain as
|
||
inspection evidence grows. Recheck callers at the implementation baseline before
|
||
removal.
|
||
|
||
Rewrite the affected [record tests](../../records_test.go) 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. Under the
|
||
recommended answer to [OQ-1](#oq-1-should-the-public-loading-apis-change-in-this-effort),
|
||
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 implementation baseline and answers to the open questions; map
|
||
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;
|
||
assess and, where useful, complete bounded clone cleanup under the scope
|
||
selected in OQ-2.
|
||
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 questions below concern the public direction and
|
||
scope of the work. No additional feature semantics are needed to carry out the
|
||
recommended refactors.
|
||
|
||
## Open Questions
|
||
|
||
The target above follows the recommended answers below, pending discussion.
|
||
Existing behavior and accepted architectural boundaries remain the baseline;
|
||
routine private design choices can be settled in the implementation plan.
|
||
|
||
### OQ-1: Should The Public Loading APIs Change In This Effort?
|
||
|
||
**Recommendation:** retain `Load`/`Options` and `LoadLayered`/`LayeredOptions`,
|
||
and consolidate their private inputs, conversions, and acquisition machinery.
|
||
The current simple call needs little setup, while layered loading makes input
|
||
groups and independent overlay context explicit. The maintained consumer examples
|
||
demonstrate useful roles for both. Better guidance and shared private machinery
|
||
address the observed duplication without adding public concepts.
|
||
|
||
**Alternative:** introduce one public request model for main input, underlays,
|
||
and load-wide options, with a convenience entry for simple loading. This could
|
||
reduce repeated option declarations and make a consumer's move to layered loading
|
||
more uniform. It is viable before 1.0, but needs concrete simple and advanced
|
||
examples to show an improvement, plus a deliberate choice about ordinary versus
|
||
layered overlay-context behavior. It would expand this effort into public API
|
||
design and consumer migration. If selected, revise the public target before
|
||
preparing the internal implementation plan.
|
||
|
||
### OQ-2: How Much Clone Cleanup Should This Effort Commit To?
|
||
|
||
**Recommendation:** commit to projection and schema refactoring, then assess
|
||
clone handlers as a bounded follow-up. Include extractions that clarify typed
|
||
copy ownership, secret-state handling, or codec behavior; allow the review to
|
||
conclude that no clone changes are needed. Projection and schema have identified
|
||
mixed responsibilities, while part of cloning's size is ordinary type dispatch.
|
||
This keeps the effort focused on demonstrated maintenance benefit. Record the
|
||
assessment under audit finding 3 so conditional work is not silently omitted.
|
||
|
||
**Alternative:** include a focused clone-handler refactor as committed scope
|
||
alongside projection and schema. Reviewing the related traversals together could
|
||
give maintainers more consistent places to find struct, map, and sequence handling.
|
||
The tradeoff is additional ownership-sensitive code movement when extraction may
|
||
not simplify the flow. Cloning and projection would still retain their distinct
|
||
semantics and operation state; a common visitor is not needed for either choice.
|