Retire the completed clone traversal review
This commit is contained in:
@@ -250,7 +250,7 @@ code changes. Use the component overview to find the corresponding implementatio
|
||||
| Layered loading and source equivalence | [Grouped lifecycle](../internal/engine/load_request_test.go), [layered facade](../facade_layered_test.go), [equivalence](../facade_equivalence_test.go) | `go test ./internal/engine -run '^TestGrouped'`, `go test . -run '^TestFacadeLayered'`, and `go test . -run '^TestEquivalentSourceForms'` |
|
||||
| Input and relative-import destinations, mounted sources, and source ownership | [Destination engine tests](../internal/engine/destination_test.go), [tier tests](../internal/engine/destination_tiers_test.go), [import traversal and version tests](../internal/engine/destination_import_test.go), [accounting tests](../internal/engine/destination_accounting_test.go), [lifecycle inspection tests](../internal/engine/destination_import_lifecycle_test.go), [public source/error tests](../facade_destination_test.go), [resource contexts](../facade_resource_context_test.go), and [consumer examples](../examples/consumers/destinations_test.go) | `go test ./internal/engine -run 'Destination|Import|Glob|Mounted'`, `go test . -run 'Destination|Import|SourceContext|FacadeMounted'`, and `go test ./examples/consumers` |
|
||||
| Shared source-version modes and evidence | [Engine source-version tests](../internal/engine/source_version_test.go), [public facade tests](../facade_source_version_mode_test.go), and [consumer example tests](../examples/consumers/layered_test.go) | `go test ./internal/engine -run 'SourceVersion'`, `go test . -run 'SourceVersion'`, and `go test ./examples/consumers` |
|
||||
| Schema, defaults, projection, cloning | [Implementation flow](internal/overview.md#implementation-flow), [schema](../internal/engine/schema_test.go), [decoding](../internal/engine/decode_test.go), [projection and typed cloning](../internal/engine/projection_test.go), [deferred clone review](roadmap/future.md#4-clone-traversal-review) | `go test ./internal/engine -run 'Schema\|Decode\|Projection\|Clone\|Inline\|CompareDocument'` |
|
||||
| Schema, defaults, projection, cloning | [Implementation flow](internal/overview.md#implementation-flow), [schema](../internal/engine/schema_test.go), [decoding](../internal/engine/decode_test.go), [projection and typed cloning](../internal/engine/projection_test.go) | `go test ./internal/engine -run 'Schema\|Decode\|Projection\|Clone\|Inline\|CompareDocument'` |
|
||||
| Lifecycle, resolution, validation, callback ownership | [Loading](../internal/engine/load_test.go), [resolution](../internal/engine/resolution_test.go), [validation](../internal/engine/validation_test.go), [ownership](../internal/engine/callback_ownership_test.go), [independence](../internal/engine/independence_test.go) | `go test ./internal/engine -run 'Load\|Resolution\|Validation\|Validator\|Hook\|Callback\|Parallel'` |
|
||||
| Paths, metadata, provenance | [Paths](../internal/engine/path_test.go), [metadata](../internal/engine/metadata_test.go), [provenance](../internal/engine/provenance_test.go) | `go test ./internal/engine -run 'Path\|Metadata\|Provenance\|Queries'` |
|
||||
| Passive resource contexts | [Engine contexts](../internal/engine/resource_context_test.go), [public contexts](../facade_resource_context_test.go) | `go test ./internal/engine -run 'Resource'` and `go test . -run 'SourceContext'` |
|
||||
|
||||
@@ -142,5 +142,5 @@ covered by `imports_test.go`, `load_request_test.go`, and focused source,
|
||||
discovery, and destination tests; inheritance by the inheritance test files;
|
||||
reflection behavior by `schema_test.go`, `projection_test.go`, and
|
||||
`decode_test.go`. The [development test map](../development.md#task-to-test-map)
|
||||
links focused commands. Typed clone extraction remains deferred in
|
||||
[`future.md`](../roadmap/future.md#4-clone-traversal-review).
|
||||
links focused commands. [`clone.go`](../../internal/engine/clone.go) owns typed
|
||||
copying, using the shared descriptors and per-operation traversal helpers.
|
||||
|
||||
@@ -81,23 +81,24 @@ conversions maintenance defects.
|
||||
|
||||
### Findings And Recommended Treatment
|
||||
|
||||
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.
|
||||
The finding numbers below preserve the audit's identifiers. The implementation
|
||||
plan records progress on the selected work. The cloning portion of finding 3
|
||||
was reviewed separately and closed without a refactor; see
|
||||
[OQ-2](#oq-2-how-much-clone-cleanup-should-this-effort-commit-to).
|
||||
|
||||
| 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; cloning deferred | Main refactor for projection. Clone traversal review is tracked separately in [future.md](future.md#4-clone-traversal-review). |
|
||||
| 3. Projection and cloning | Higher for projection; clone review complete | Main refactor for projection. The separate clone review concluded that the existing traversal should be retained; see [OQ-2](#oq-2-how-much-clone-cleanup-should-this-effort-commit-to). |
|
||||
| 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 completion of the selected work;
|
||||
the deferred clone review retains its separate disposition.
|
||||
The implementation plan records validation of the selected work; the completed
|
||||
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`
|
||||
@@ -336,9 +337,10 @@ 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.
|
||||
|
||||
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.
|
||||
Clone cleanup remains outside this effort; the separate
|
||||
[review](#oq-2-how-much-clone-cleanup-should-this-effort-commit-to) concluded that
|
||||
no refactor was warranted. 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
|
||||
@@ -451,8 +453,8 @@ Completion requires reviewable evidence that:
|
||||
resource lifetime, and algorithmic corrections remain protected by appropriate
|
||||
tests and source review.
|
||||
- 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.
|
||||
traversal review closed separately without a refactor. 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.
|
||||
|
||||
@@ -473,9 +475,16 @@ not establish a perpetual restriction on future API design.
|
||||
|
||||
### OQ-2: How Much Clone Cleanup Should This Effort Commit To?
|
||||
|
||||
**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.
|
||||
**Settled:** clone review was excluded from the consolidation sprint and completed
|
||||
separately on 2026-10-25 against `8dfaf44`. Retain the existing clone traversal;
|
||||
no cleanup is planned.
|
||||
|
||||
The review examined the traversal, shared helpers, schema-member construction,
|
||||
lifecycle callers, and relevant tests. Its branches already keep allocation,
|
||||
recursive copying, and cycle cleanup together. Shared depth, cycle, dynamic-type,
|
||||
and text-marshaling operations already have common owners. Further extraction,
|
||||
including the similar slice/array loops, did not offer enough maintenance benefit
|
||||
to justify the additional indirection. Focused engine tests for projection,
|
||||
cloning, inline fields, callback ownership, and secret-copy behavior passed.
|
||||
No code changes or benchmarks were needed. This is a maintenance assessment,
|
||||
not a restriction on future changes justified by new needs.
|
||||
|
||||
@@ -83,36 +83,3 @@ 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?
|
||||
|
||||
@@ -21,8 +21,9 @@ add public obligations.
|
||||
Implement only the selected consolidation. Retain the public `Load`,
|
||||
`LoadLayered`, options, inputs, callbacks, source/search descriptions, inspection,
|
||||
and sequence-registration APIs. Keep the root-to-engine-to-document/YAML
|
||||
dependency direction. Clone traversal review/refactoring remains deferred under
|
||||
[future.md](future.md#4-clone-traversal-review). Changes to shared descriptors must
|
||||
dependency direction. Clone traversal review/refactoring was excluded from this
|
||||
sprint; the subsequent [review outcome](consolidation.md#oq-2-how-much-clone-cleanup-should-this-effort-commit-to)
|
||||
retains the existing traversal. Changes to shared descriptors must
|
||||
preserve existing cloning behavior. This sprint does not publish a release,
|
||||
update dependencies/toolchain requirements, change configuration semantics, or
|
||||
introduce complexity/performance gates.
|
||||
@@ -729,15 +730,16 @@ without a new registration framework. Run R for both Stages 17–18.
|
||||
policy/state/tier lifetimes, resolved context versus live handles, import file
|
||||
owners, and explicit inheritance/projection/schema handoffs. Fix moved-symbol
|
||||
and behavioral-test navigation and stale comments affected by the refactor.
|
||||
4. Preserve public contract definitions and accepted ADR decisions. Keep deferred
|
||||
clone work in `future.md`. No policy change or new ADR is needed for the
|
||||
selected private refactor.
|
||||
4. Preserve public contract definitions and accepted ADR decisions. Keep clone
|
||||
work outside this sprint; its subsequent disposition is recorded under
|
||||
[OQ-2](consolidation.md#oq-2-how-much-clone-cleanup-should-this-effort-commit-to).
|
||||
No policy change or new ADR is needed for the selected private refactor.
|
||||
|
||||
Progress notes (2026-10-25): added a consumer entry-point and ownership guide,
|
||||
linked the existing adoption examples, and documented normalized requests,
|
||||
acquisition/context lifetimes, import owners, and inheritance/projection/schema
|
||||
handoffs. Updated test navigation and retained clone review as deferred.
|
||||
Documentation links and repository boundaries passed.
|
||||
handoffs. Updated test navigation and retained clone review as deferred at this
|
||||
stage. Documentation links and repository boundaries passed.
|
||||
|
||||
**Finish:** current documentation describes implemented structure, with the
|
||||
roadmap/plan retaining future work and progress until release cleanup.
|
||||
@@ -773,7 +775,7 @@ was needed. The final development check and race-enabled full test suite passed.
|
||||
| --- | --- |
|
||||
| 1: imports | Stages 3–6 and 10–13 remove options reconstruction/default coupling and forwarding ladders; acquisition/scope/evidence owners and admission operations are clear. A and final checks pass. |
|
||||
| 2: inheritance | Stages 14–16 return explicit template results, keep one executor, and preserve accounting and value/history/identity agreement. I and final checks pass. |
|
||||
| 3: projection | Stage 17 extracts substantial handlers sharing operation state. R and final checks pass. Clone review remains deferred, not implicitly completed. |
|
||||
| 3: projection | Stage 17 extracts substantial handlers sharing operation state. R and final checks pass. Clone review was excluded from the sprint and subsequently closed without a refactor; see [OQ-2](consolidation.md#oq-2-how-much-clone-cleanup-should-this-effort-commit-to). |
|
||||
| 4: schema | Stage 18 clarifies ordinary/inline assembly while preserving both descriptor views. R and final checks pass. |
|
||||
| 5: contexts | Stages 7–9 share resolved facts across input/import/overlay/passive contexts without retaining handles or changing validation timing. A and final checks pass. |
|
||||
| 6: facade | Stage 2 removes unused inverse conversions/adapters and shares real conversion. F and final boundary/consumer checks pass. |
|
||||
|
||||
@@ -104,10 +104,11 @@ time of that audit; completion is recorded with the finding above.
|
||||
|
||||
The selected stable Kahn scheduler, deterministic sorting, composition rules,
|
||||
scope boundaries, and required ownership copies are not reconsidered here.
|
||||
Previously deferred experiments in [future.md](future.md#3-performance-experiments)
|
||||
and the [clone review](future.md#4-clone-traversal-review) retain their separate
|
||||
disposition. These findings do not establish new public complexity guarantees,
|
||||
input restrictions, performance thresholds, or release gates.
|
||||
Previously deferred experiments remain in [future.md](future.md#3-performance-experiments).
|
||||
The separately completed [clone review](consolidation.md#oq-2-how-much-clone-cleanup-should-this-effort-commit-to)
|
||||
concluded that no refactor was warranted. These findings do not establish new
|
||||
public complexity guarantees, input restrictions, performance thresholds, or
|
||||
release gates.
|
||||
|
||||
## 2. Avoid Searching All Sources For Each Empty Import Selection
|
||||
|
||||
|
||||
Reference in New Issue
Block a user