From 549fca239d445df27b89eb43cb0523deca96b806 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 25 Oct 2026 22:42:21 +0000 Subject: [PATCH] Separate collection overlay responsibilities --- docs/roadmap/implementation.md | 10 ++ internal/engine/overlay.go | 218 +++++++++++++++++++++++---------- 2 files changed, 162 insertions(+), 66 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 67aa8d8..3ba4380 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -816,6 +816,16 @@ combination diagnostics remain protected. Newly separated inputs/results make lower versus incoming versus retained ownership explicit. Focused check: `go test ./internal/engine -run 'Collection|Sequence|Overlay|Inheritance|History'`. +Status: **complete** (preparation/ownership part of E03). Prior completed commits: +`2736ddf`, `deb9aa5`, `ae3408a`, `ea27f17`, `08a8c25`, `877c497`, `8aba43d`, +`7bf2f0c`, `0f9ecf1`, `119b42c`, `247a12b`, `4fcff5f`, `12d350a`, `08f2a8f`, +`826062a`, `e4a5ecb`, `3122e2f`, `6f6130f`, `4fd3bee`, `4bf940c`, `d8bf6ab`, +`f185821`, `a8ae2e9`, and `8832449`. Validation passed: +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Collection|Sequence|Overlay|Inheritance|History' -count=1`, +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`, +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./... -count=1`, +and `git diff --check`. + ## Stage 26 — Index Assembly Policy Retirement By Semantic Path Finding: **E01**. diff --git a/internal/engine/overlay.go b/internal/engine/overlay.go index 76ea405..b1b2ab5 100644 --- a/internal/engine/overlay.go +++ b/internal/engine/overlay.go @@ -83,7 +83,6 @@ func (a *assemblyAccumulator) applyOverlayWithCollectionProjection( if a == nil || a.current == nil || a.current.root == nil || a.current.root.Kind != document.Mapping || overlay == nil || overlay.Kind != document.Mapping { return nil, invalidOverlayDocument() } - mergeOriginContribution(a.current.origins, Path{}, incomingOrigin(incomingOrigins, Path{}, overlay, layer)) sequences, diagnostics := mergeOverlayMappingsWithCollectionPolicy(a, overlay, incomingOrigins, incomingPolicies, layer, sequencePolicy, declarations) if len(diagnostics) != 0 { return nil, diagnostics @@ -110,11 +109,39 @@ func mergeOverlayMappingsWithCollectionPolicy( sequencePolicy sequencePolicy, declarations []sequenceDeclarationSpec, ) ([]sequenceHistoryContribution, []Diagnostic) { - result := assembly.current + prepared, diagnostics := prepareOverlayCollection( + assembly.current.collectionPolicies, overlay, sequencePolicy, declarations, + ) + if len(diagnostics) != 0 { + return nil, diagnostics + } + operation := newOverlayMergeOperation(assembly, overlay, incomingOrigins, incomingPolicies, layer, prepared) + operation.apply() + if len(operation.diagnostics) != 0 { + return nil, canonicalDiagnostics(operation.diagnostics) + } + return finalizeOverlayCollection(assembly, prepared, operation) +} + +// preparedOverlayCollection contains decisions made against the read-only +// lower policy scope. Incoming declarations remain grouped by destination for +// declaration-only finalization after value application. +type preparedOverlayCollection struct { + declarationsByPath map[string][]sequenceDeclarationSpec + resolutions map[string]collectionPolicyResolution + sequencePolicy sequencePolicy +} + +func prepareOverlayCollection( + lowerPolicies collectionPolicyScope, + overlay *document.Node, + sequencePolicy sequencePolicy, + declarations []sequenceDeclarationSpec, +) (*preparedOverlayCollection, []Diagnostic) { claims := compositionClaims{byPath: make(map[string][]compositionClaim)} claims.append(overlay, Path{}, 0) identityFieldsByPath, resolvedPolicies, diagnostics := resolveStrictCollectionPolicies( - claims.ordered, sequencePolicy, result.collectionPolicies, declarations, + claims.ordered, sequencePolicy, lowerPolicies, declarations, ) if len(diagnostics) != 0 { return nil, diagnostics @@ -139,13 +166,13 @@ func mergeOverlayMappingsWithCollectionPolicy( registered[key] = true } var overlayLookup *documentLookup - if len(result.collectionPolicies.collections) != 0 { + if len(lowerPolicies.collections) != 0 { overlayLookup = buildDocumentLookup(overlay) } - for key := range result.collectionPolicies.collections { - if node, found := documentAtPathWithLookup(overlay, result.collectionPolicies.collections[key].destination, overlayLookup); found && node != nil { + for key := range lowerPolicies.collections { + if node, found := documentAtPathWithLookup(overlay, lowerPolicies.collections[key].destination, overlayLookup); found && node != nil { registered[key] = true - registeredPaths[key] = result.collectionPolicies.collections[key].destination.clone() + registeredPaths[key] = lowerPolicies.collections[key].destination.clone() } } @@ -167,65 +194,124 @@ func mergeOverlayMappingsWithCollectionPolicy( effectiveIdentityFields: cloneStrings(identityFieldsByPath[key]), } } + return &preparedOverlayCollection{ + declarationsByPath: declarationsByPath, + resolutions: resolutions, + sequencePolicy: sequencePolicy, + }, nil +} - sequenceApplications := make(map[string]sequenceHistoryContribution) - policyValues := make(map[string]bool) - var combinationDiagnostics []Diagnostic - mergeOverlayMappingsWithHooks(result.root, overlay, Path{}, &assembly.index, - recursiveMergeHooks{ - added: func(path Path, incoming *document.Node) { - collectOverlayOriginsFrom(result.origins, incomingOrigins, incoming, path, layer) - }, - combined: func(path Path, incoming *document.Node) { - mergeOriginContribution(result.origins, path, incomingOrigin(incomingOrigins, path, incoming, layer)) - }, - replaced: func(path Path, existing, incoming *document.Node) { - assembly.current.collectionPolicies.retireCollectionPoliciesAtOrBelow(path) - recordOverlayReplacement(result, path, existing, incoming, layer, true, incomingOrigins) - }, - merge: func(path Path, existing, incoming *document.Node) (*document.Node, bool) { - resolution, active := resolutions[path.String()] - if !active { - return nil, false - } - combined, issues := combineSequence(existing, incoming, resolution, sequenceCombinationContext{destination: path}) - if len(issues) != 0 { - combinationDiagnostics = append(combinationDiagnostics, issues...) - return existing, false - } - applyOverlaySequenceEvidence(result, path, existing, incoming, combined, resolution, incomingOrigins, incomingPolicies, layer) - policyValues[path.String()] = true - history := sequenceHistoryContribution{path: path.clone(), mode: sequenceHistoryMapped} - if resolution.operation == sequenceModeReplace { - history.mode = sequenceHistoryReplace - if incoming != nil && incoming.Kind == document.Sequence { - history.length = len(incoming.Sequence) - } - } else { - history.items = append([]sequenceItemProjection(nil), combined.higherItems...) - history.replacements = append([]sequenceItemReplacement(nil), combined.replacements...) - } - sequenceApplications[path.String()] = history - return combined.value, true - }, - }, - ) - if len(combinationDiagnostics) != 0 { - return nil, canonicalDiagnostics(combinationDiagnostics) +// overlayMergeOperation owns recursive value application and its associated +// origin, policy, diagnostic, and sequence-history evidence. +type overlayMergeOperation struct { + assembly *assemblyAccumulator + result *composedDocument + index *mappingIndex + overlay *document.Node + incomingOrigins map[string]Origin + incomingPolicies collectionPolicyScope + layer Layer + prepared *preparedOverlayCollection + diagnostics []Diagnostic + sequenceApplications map[string]sequenceHistoryContribution + valueDestinations map[string]bool +} + +func newOverlayMergeOperation( + assembly *assemblyAccumulator, + overlay *document.Node, + incomingOrigins map[string]Origin, + incomingPolicies collectionPolicyScope, + layer Layer, + prepared *preparedOverlayCollection, +) *overlayMergeOperation { + return &overlayMergeOperation{ + assembly: assembly, result: assembly.current, index: &assembly.index, overlay: overlay, + incomingOrigins: incomingOrigins, incomingPolicies: incomingPolicies, + layer: layer, prepared: prepared, + sequenceApplications: make(map[string]sequenceHistoryContribution), + valueDestinations: make(map[string]bool), } +} - // Values are now complete. Index lazily for declaration-only checks; - // this lookup is never retained across the value-changing merge above. +func (operation *overlayMergeOperation) apply() { + mergeOriginContribution(operation.result.origins, Path{}, incomingOrigin(operation.incomingOrigins, Path{}, operation.overlay, operation.layer)) + mergeOverlayMappingsWithHooks( + operation.result.root, operation.overlay, Path{}, operation.index, + operation.callbacks(), + ) +} + +func (operation *overlayMergeOperation) callbacks() recursiveMergeHooks { + return recursiveMergeHooks{ + added: operation.added, + combined: operation.combined, + replaced: operation.replaced, + merge: operation.merge, + } +} + +func (operation *overlayMergeOperation) added(path Path, incoming *document.Node) { + collectOverlayOriginsFrom(operation.result.origins, operation.incomingOrigins, incoming, path, operation.layer) +} + +func (operation *overlayMergeOperation) combined(path Path, incoming *document.Node) { + mergeOriginContribution(operation.result.origins, path, incomingOrigin(operation.incomingOrigins, path, incoming, operation.layer)) +} + +func (operation *overlayMergeOperation) replaced(path Path, existing, incoming *document.Node) { + operation.result.collectionPolicies.retireCollectionPoliciesAtOrBelow(path) + recordOverlayReplacement(operation.result, path, existing, incoming, operation.layer, true, operation.incomingOrigins) +} + +func (operation *overlayMergeOperation) merge(path Path, existing, incoming *document.Node) (*document.Node, bool) { + resolution, active := operation.prepared.resolutions[path.String()] + if !active { + return nil, false + } + combined, issues := combineSequence(existing, incoming, resolution, sequenceCombinationContext{destination: path}) + if len(issues) != 0 { + operation.diagnostics = append(operation.diagnostics, issues...) + return existing, false + } + applyOverlaySequenceEvidence( + operation.result, path, existing, incoming, combined, resolution, + operation.incomingOrigins, operation.incomingPolicies, operation.layer, + ) + operation.valueDestinations[path.String()] = true + history := sequenceHistoryContribution{path: path.clone(), mode: sequenceHistoryMapped} + if resolution.operation == sequenceModeReplace { + history.mode = sequenceHistoryReplace + if incoming != nil && incoming.Kind == document.Sequence { + history.length = len(incoming.Sequence) + } + } else { + history.items = append([]sequenceItemProjection(nil), combined.higherItems...) + history.replacements = append([]sequenceItemReplacement(nil), combined.replacements...) + } + operation.sequenceApplications[path.String()] = history + return combined.value, true +} + +func finalizeOverlayCollection( + assembly *assemblyAccumulator, + prepared *preparedOverlayCollection, + operation *overlayMergeOperation, +) ([]sequenceHistoryContribution, []Diagnostic) { + var diagnostics []Diagnostic + + // Values are complete. Build the retained lookup lazily only if a + // declaration-only destination needs to be checked. var retainedLookup *documentLookup - for key, grouped := range declarationsByPath { - if policyValues[key] { + for key, grouped := range prepared.declarationsByPath { + if operation.valueDestinations[key] { continue } path := grouped[0].destination - applicationIdentity, _ := sequencePolicy.identityFieldsAt(path) - current, diagnostics := resolveCollectionPolicy(path, grouped, applicationIdentity, nil) - if len(diagnostics) != 0 { - combinationDiagnostics = append(combinationDiagnostics, diagnostics...) + applicationIdentity, _ := prepared.sequencePolicy.identityFieldsAt(path) + current, issues := resolveCollectionPolicy(path, grouped, applicationIdentity, nil) + if len(issues) != 0 { + diagnostics = append(diagnostics, issues...) continue } lower, found := assembly.current.collectionPolicies.at(path) @@ -239,28 +325,28 @@ func mergeOverlayMappingsWithCollectionPolicy( retained, _ := documentAtPathWithLookup(assembly.current.root, path, retainedLookup) if retained != nil && retained.Kind == document.Sequence && len(current.effectiveIdentityFields) != 0 { if _, issues := validateSequenceIdentity(retained, path, current.effectiveIdentityFields, sequenceIdentityDiagnosticCodes{invalid: CodeSequenceInvalid, conflict: CodeSequenceConflict}); len(issues) != 0 { - combinationDiagnostics = append(combinationDiagnostics, issues...) + diagnostics = append(diagnostics, issues...) continue } } policy, issues := collectionPolicyWithoutValue(lowerPolicy, current.policy, retained, path) if len(issues) != 0 { - combinationDiagnostics = append(combinationDiagnostics, issues...) + diagnostics = append(diagnostics, issues...) continue } assembly.current.collectionPolicies.set(path, policy) } - if len(combinationDiagnostics) != 0 { - return nil, canonicalDiagnostics(combinationDiagnostics) + if len(diagnostics) != 0 { + return nil, canonicalDiagnostics(diagnostics) } - keys := make([]string, 0, len(sequenceApplications)) - for key := range sequenceApplications { + keys := make([]string, 0, len(operation.sequenceApplications)) + for key := range operation.sequenceApplications { keys = append(keys, key) } slices.Sort(keys) sequences := make([]sequenceHistoryContribution, 0, len(keys)) for _, key := range keys { - sequences = append(sequences, sequenceApplications[key]) + sequences = append(sequences, operation.sequenceApplications[key]) } return sequences, nil }