From 8a99e6107001c69117a41dc37bc6ea09d2910af9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 25 Oct 2026 11:54:46 +0000 Subject: [PATCH] Defer clone traversal review to future work --- docs/roadmap/consolidation.md | 77 +++++++++++++---------------------- docs/roadmap/future.md | 33 +++++++++++++++ 2 files changed, 62 insertions(+), 48 deletions(-) diff --git a/docs/roadmap/consolidation.md b/docs/roadmap/consolidation.md index 5613109..f74a6d9 100644 --- a/docs/roadmap/consolidation.md +++ b/docs/roadmap/consolidation.md @@ -2,8 +2,8 @@ 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). +The scope questions are settled. Prepare the staged implementation plan after +reviewing this roadmap. ## Purpose @@ -81,21 +81,23 @@ 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. +The finding numbers below preserve the audit's identifiers. The selected work +remains unimplemented; the cloning portion of finding 3 is deferred to future +work. Retiring the original documents does not mark their findings 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. | +| 3. Projection and cloning | Higher for projection; cloning deferred | Main refactor for projection. Clone traversal review is tracked separately in [future.md](future.md#4-clone-traversal-review). | | 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. +Implementation and validation will establish completion of the selected work; +the deferred clone review retains its separate disposition. Completed correctness and algorithmic remediations remain part of the baseline, including sequence records preserved at `ea46001:docs/roadmap/remediation.md` @@ -334,17 +336,13 @@ 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. +Clone traversal review and cleanup are deferred to +[future.md](future.md#4-clone-traversal-review). This effort preserves cloning's +existing behavior while changing the shared schema and projection machinery. +Keep projection's document-building semantics distinct from typed copying. +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 @@ -417,7 +415,7 @@ or is required to claim readability improvements. 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 +1. Confirm the implementation baseline and settled scope; 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. @@ -426,9 +424,7 @@ This is dependency guidance for the later plan, not a commit-by-commit schedule: 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. +5. Refactor projection and schema field assembly in separate reviewable changes. 6. Consolidate real facade adapters, finish consumer/internal navigation, and run the applicable integration and ownership checks on the combined result. @@ -449,7 +445,7 @@ not for every new helper. | 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. | +| Projection/schema | Inline fields, recursive schemas, dynamic values, nil/empty values, cycles and limits, text codecs, secret states, callback ownership, and independent loads; preserve existing cloning behavior where shared descriptors/state are affected. | | 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 @@ -475,15 +471,15 @@ Completion requires reviewable evidence that: - 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. +- The reviewed scope is accounted for against all six audit findings, with clone + traversal review deferred to future work. Completed portions 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 remaining question concerns the scope of clone -cleanup. No additional feature semantics are needed to carry out the recommended -refactors. +in the implementation plan. No open scope questions remain, and no additional +feature semantics are needed to carry out the recommended refactors. ## Settled Decisions @@ -496,26 +492,11 @@ guidance. The [consumer-facing target](#consumer-facing-target) records the rationale and tradeoffs. This decision governs the consolidation effort; it does not establish a perpetual restriction on future API design. -## Open Questions - -OQ-2 remains open. The target above follows its recommended answer pending -discussion. Existing behavior and accepted architectural boundaries remain the -baseline; routine private design choices can be settled in the 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. +**Settled:** defer clone traversal review and any resulting cleanup to +[future.md](future.md#4-clone-traversal-review). Neither the review nor clone +refactoring is part of this consolidation effort. Projection and schema +refactoring remain in scope, including protection of existing cloning behavior +where they share descriptors or operation state. The future review can conclude +that no clone edits are warranted; its goal is demonstrated maintenance benefit. diff --git a/docs/roadmap/future.md b/docs/roadmap/future.md index bc37848..fad4f3d 100644 --- a/docs/roadmap/future.md +++ b/docs/roadmap/future.md @@ -83,3 +83,36 @@ evidence are preserved in Git history at `8e91a2b` under `docs/roadmap/`. across loads without introducing aliasing? - What cache lifetime and concurrency design would make schema reuse worthwhile without excessive retained memory? + +## 4. Clone traversal review + +Status: deferred. Review typed configuration cloning after the projection and +schema consolidation work, when the resulting structure can inform the review. +This is future maintenance work, not a prerequisite for completing consolidation. + +The complexity audit identified `valueOperation.clone` in +[clone.go](../../internal/engine/clone.go) as a candidate for cleanup. Much of its +size is straightforward type dispatch. Focused handlers may clarify allocation, +recursive copying, cycle tracking, secret-state handling, or text-codec +reconstruction, but extraction may also add indirection without making the code +easier to understand. + +When this work is taken up, assess the traversal and record whether changes are +worthwhile; a conclusion that no refactor is needed is valid. Make focused +changes only where the maintenance benefit justifies the code movement. Keep +cloning's typed-copy semantics distinct from projection's document-building +semantics. Preserve detached values, nil/empty distinctions, physical inline +fields, cycle/depth checks, and unresolved versus validated secret handling. +Reuse existing ownership, codec, secret, and traversal tests, adding coverage for +consequential gaps. No performance improvement is assumed. + +### Questions to resolve before implementation + +- After projection and schema consolidation, which clone branches still mix + responsibilities enough to benefit from extraction? +- Would a proposed helper keep allocation, recursive copying, and cycle cleanup + easier to follow together, or merely move a short loop out of sight? +- Are any small shared operations semantically identical across traversals, or + would sharing require special cases that obscure their different purposes? +- Which ownership or special-value risks, if any, lack sufficient existing test + coverage for the proposed changes?