diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index a054911..57ab828 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -96,6 +96,13 @@ composition, decode, and budget codes retain their meanings. ## Stage 1 — Prepare shared recursive combination +**Completed 2026-09-29.** Recursive mapping overlay now uses a private +value-combination walk with path-aware add, combine, and replacement callbacks; +the existing overlay adapter continues to own source origins and history, while +strict composition remains separate. A focused table covers recursive and +atomic replacements, empty mappings, event paths, and detached inputs. Focused +composition/overlay/override/underlay tests and `./scripts/check.sh` passed. + Inspect `internal/engine/overlay.go`, `compose.go`, `mapping_index.go`, `history.go`, and `internal/document/document.go`. `applyOverlay` already shares `mergeOverlayMappings` across underlays, main-over-underlay assembly, and selected diff --git a/internal/engine/overlay.go b/internal/engine/overlay.go index 55280e0..6689ed9 100644 --- a/internal/engine/overlay.go +++ b/internal/engine/overlay.go @@ -53,40 +53,29 @@ func (a *assemblyAccumulator) applyOverlay(overlay *document.Node, incomingOrigi } func mergeOverlayMappings(result *composedDocument, target, overlay *document.Node, current Path, layer Layer, recordReplacements bool, incomingOrigins map[string]Origin, index *mappingIndex) { - for _, incoming := range overlay.Mapping { - path := current.Key(incoming.Key) - position := index.find(target, incoming.Key) - if position < 0 { - cloned := incoming - cloned.Value = document.Clone(incoming.Value) - index.append(target, cloned) - collectOverlayOriginsFrom(result.origins, incomingOrigins, incoming.Value, path, layer) - continue - } - - existing := target.Mapping[position].Value - if existing.Kind == document.Mapping && incoming.Value.Kind == document.Mapping { - mergeOriginContribution(result.origins, path, incomingOrigin(incomingOrigins, path, incoming.Value, layer)) - mergeOverlayMappings(result, existing, incoming.Value, path, layer, recordReplacements, incomingOrigins, index) - continue - } - - displaced := displacedOrigins(existing, result.origins, path) - previous := cloneOrigin(result.origins[path.String()]) - deleteOriginSubtree(result.origins, path, existing) - target.Mapping[position] = incoming - target.Mapping[position].Value = document.Clone(incoming.Value) - collectOverlayOriginsFrom(result.origins, incomingOrigins, incoming.Value, path, layer) - if recordReplacements { - result.overlays = append(result.overlays, overlayRecord{ - change: Change{ - Path: path.clone(), Origin: incomingOrigin(incomingOrigins, path, incoming.Value, layer), - Previous: previousOrigins(previous), - }, - displaced: displaced, - }) - } - } + mergeRecursiveMappings(target, overlay, current, 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) { + displaced := displacedOrigins(existing, result.origins, path) + previous := cloneOrigin(result.origins[path.String()]) + deleteOriginSubtree(result.origins, path, existing) + collectOverlayOriginsFrom(result.origins, incomingOrigins, incoming, path, layer) + if recordReplacements { + result.overlays = append(result.overlays, overlayRecord{ + change: Change{ + Path: path.clone(), Origin: incomingOrigin(incomingOrigins, path, incoming, layer), + Previous: previousOrigins(previous), + }, + displaced: displaced, + }) + } + }, + }) } func collectOverlayOrigins(origins map[string]Origin, node *document.Node, path Path, layer Layer) { diff --git a/internal/engine/recursive_merge.go b/internal/engine/recursive_merge.go new file mode 100644 index 0000000..a03819e --- /dev/null +++ b/internal/engine/recursive_merge.go @@ -0,0 +1,45 @@ +package engine + +import "gitea.maximumdirect.net/eric/ConfigKit/internal/document" + +// recursiveMergeHooks lets an assembly owner account for the values changed by +// a recursive mapping overlay without making source roles part of the merge. +// Hooks run before replacement and receive the still-owned incoming subtree. +type recursiveMergeHooks struct { + added func(Path, *document.Node) + combined func(Path, *document.Node) + replaced func(Path, *document.Node, *document.Node) +} + +// mergeRecursiveMappings changes an owned mapping and never aliases incoming +// values into it. Entry order follows the target, then new incoming entries. +func mergeRecursiveMappings(target, incoming *document.Node, current Path, index *mappingIndex, hooks recursiveMergeHooks) { + for _, entry := range incoming.Mapping { + path := current.Key(entry.Key) + position := index.find(target, entry.Key) + if position < 0 { + cloned := entry + cloned.Value = document.Clone(entry.Value) + index.append(target, cloned) + if hooks.added != nil { + hooks.added(path, entry.Value) + } + continue + } + + existing := target.Mapping[position].Value + if existing.Kind == document.Mapping && entry.Value.Kind == document.Mapping { + if hooks.combined != nil { + hooks.combined(path, entry.Value) + } + mergeRecursiveMappings(existing, entry.Value, path, index, hooks) + continue + } + + if hooks.replaced != nil { + hooks.replaced(path, existing, entry.Value) + } + target.Mapping[position] = entry + target.Mapping[position].Value = document.Clone(entry.Value) + } +} diff --git a/internal/engine/recursive_merge_test.go b/internal/engine/recursive_merge_test.go new file mode 100644 index 0000000..47592c7 --- /dev/null +++ b/internal/engine/recursive_merge_test.go @@ -0,0 +1,90 @@ +package engine + +import ( + "reflect" + "testing" + + "gitea.maximumdirect.net/eric/ConfigKit/internal/document" +) + +func TestRecursiveMappingMergePreservesValuesPathsAndInputs(t *testing.T) { + type expectedNode struct { + path Path + kind document.Kind + text string + entries int + } + tests := []struct { + name, base, incoming string + want []expectedNode + events []string + }{ + { + name: "nested mappings", base: "shared: {kept: base, changed: old}\n", incoming: "shared: {changed: new, added: child}\n", + want: []expectedNode{ + {path: Keys("shared"), kind: document.Mapping, entries: 3}, + {path: Keys("shared", "kept"), kind: document.String, text: "base"}, + {path: Keys("shared", "changed"), kind: document.String, text: "new"}, + {path: Keys("shared", "added"), kind: document.String, text: "child"}, + }, + events: []string{"combine:/shared", "replace:/shared/changed", "add:/shared/added"}, + }, + { + name: "sequence replacement", base: "value: [old]\n", incoming: "value: []\n", + want: []expectedNode{{path: Keys("value"), kind: document.Sequence}}, + events: []string{"replace:/value"}, + }, + { + name: "scalar replacement", base: "value: old\n", incoming: "value: new\n", + want: []expectedNode{{path: Keys("value"), kind: document.String, text: "new"}}, + events: []string{"replace:/value"}, + }, + { + name: "null replacement", base: "value: old\n", incoming: "value: null\n", + want: []expectedNode{{path: Keys("value"), kind: document.Null}}, + events: []string{"replace:/value"}, + }, + { + name: "empty mapping retains children", base: "shared: {kept: base}\n", incoming: "shared: {}\n", + want: []expectedNode{ + {path: Keys("shared"), kind: document.Mapping, entries: 1}, + {path: Keys("shared", "kept"), kind: document.String, text: "base"}, + }, + events: []string{"combine:/shared"}, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + base := parseCompositionDocument(t, "source-1", test.base) + incoming := parseCompositionDocument(t, "source-2", test.incoming) + baseBefore, incomingBefore := document.Clone(base), document.Clone(incoming) + owned := document.Clone(base) + var events []string + mergeRecursiveMappings(owned, incoming, Path{}, new(mappingIndex), recursiveMergeHooks{ + added: func(path Path, _ *document.Node) { events = append(events, "add:"+path.String()) }, + combined: func(path Path, _ *document.Node) { + events = append(events, "combine:"+path.String()) + }, + replaced: func(path Path, _, _ *document.Node) { + events = append(events, "replace:"+path.String()) + }, + }) + if !reflect.DeepEqual(events, test.events) { + t.Fatalf("merge events = %#v, want %#v", events, test.events) + } + for _, expected := range test.want { + node, found := documentAtPath(owned, expected.path) + if !found || node.Kind != expected.kind || node.Text != expected.text || len(node.Mapping) != expected.entries { + t.Errorf("merged %s = %#v, found %v", expected.path.String(), node, found) + } + } + if !reflect.DeepEqual(base, baseBefore) || !reflect.DeepEqual(incoming, incomingBefore) { + t.Fatal("merge changed an input") + } + owned.Mapping[0].Value.Location.SourceID = "changed" + if !reflect.DeepEqual(base, baseBefore) || !reflect.DeepEqual(incoming, incomingBefore) { + t.Fatal("merged result aliases an input") + } + }) + } +}