Defer clone traversal review to future work
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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?
|
||||
|
||||
Reference in New Issue
Block a user