diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index e978df8..6fc211f 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -135,6 +135,8 @@ invalid entry identities remain repairable and no new load failures are added. ## Stage 3 — Compare ordinary lists recursively during resolution +**Status:** Complete. Focused resolution and public diff checks passed. + **Scope:** resolution-only delta production and its initial integration. 1. Add a private resolution comparator, preferably in `resolution_compare.go`. diff --git a/internal/engine/resolution.go b/internal/engine/resolution.go index b3c154b..9a3436d 100644 --- a/internal/engine/resolution.go +++ b/internal/engine/resolution.go @@ -313,7 +313,7 @@ func runResolution[T any](configuration *T, resolver func(*T, *Resolution) error state.addDiagnostic(CodeResolveInvalidValue, issue.path, "application resolution produced an invalid configuration value") return resolutionResult{diagnostics: canonicalDiagnostics(state.diagnostics)} } - deltas := compareConfigurationValues(decoded.final, after) + deltas := compareResolutionValues(decoded.final, after) ordered := orderedResolutionRecords(state.records) coverage := newResolutionCoverageIndexFromRecords(ordered) beforeLookup := buildDocumentLookup(decoded.final) diff --git a/internal/engine/resolution_compare.go b/internal/engine/resolution_compare.go new file mode 100644 index 0000000..06cdc1e --- /dev/null +++ b/internal/engine/resolution_compare.go @@ -0,0 +1,52 @@ +package engine + +import ( + "cmp" + "slices" + + "go.maximumdirect.net/configkit/internal/document" +) + +// compareResolutionValues reports field-level changes at stable list positions. +func compareResolutionValues(before, after *document.Node) []documentDelta { + var deltas []documentDelta + var compare func(*document.Node, *document.Node, Path) + compare = func(left, right *document.Node, path Path) { + if left == nil || right == nil || left.Kind != right.Kind { + compareConfigurationValue(left, right, path, &deltas) + return + } + switch left.Kind { + case document.Mapping: + leftEntries := make(map[string]*document.Node, len(left.Mapping)) + for _, entry := range left.Mapping { + leftEntries[entry.Key] = entry.Value + } + for _, entry := range right.Mapping { + compare(leftEntries[entry.Key], entry.Value, path.Key(entry.Key)) + delete(leftEntries, entry.Key) + } + for key, value := range leftEntries { + compare(value, nil, path.Key(key)) + } + case document.Sequence: + if len(left.Sequence) != len(right.Sequence) { + deltas = append(deltas, documentDelta{path: path.clone(), kind: documentReplaced}) + return + } + for index := range left.Sequence { + compare(left.Sequence[index], right.Sequence[index], path.Index(index)) + } + default: + compareConfigurationValue(left, right, path, &deltas) + } + } + compare(before, after, Path{}) + slices.SortFunc(deltas, func(a, b documentDelta) int { + if order := cmp.Compare(a.path.String(), b.path.String()); order != 0 { + return order + } + return int(a.kind) - int(b.kind) + }) + return deltas +} diff --git a/internal/engine/resolution_compare_test.go b/internal/engine/resolution_compare_test.go new file mode 100644 index 0000000..04d5c31 --- /dev/null +++ b/internal/engine/resolution_compare_test.go @@ -0,0 +1,74 @@ +package engine + +import ( + "errors" + "testing" +) + +func TestResolutionAuditsOrdinaryListFields(t *testing.T) { + type item struct { + Count int `yaml:"count"` + Enabled bool `yaml:"enabled"` + Nested []*int `yaml:"nested"` + } + type config struct { + Items []item `yaml:"items"` + } + for _, test := range []struct { + name string + other, resize bool + wantPath string + }{ + {name: "covered field"}, + {name: "uncovered field", other: true, wantPath: "/items/0/enabled"}, + {name: "length change", resize: true, wantPath: "/items"}, + } { + t.Run(test.name, func(t *testing.T) { + result, err := testLoad(Bytes("items", []byte("items: [{count: 3, enabled: true, nested: [null]}]\n")), Options[config]{Definition: Definition[config]{ + Resolve: func(value *config, r *Resolution) error { + value.Items[0].Count = 0 + r.RecordNormalization(Keys("items").Index(0).Key("count"), "clear count") + if test.other { + value.Items[0].Enabled = false + } + if test.resize { + value.Items = append(value.Items, item{}) + } + return nil + }, + }}) + if test.wantPath == "" { + if err != nil || result.Config.Items[0].Count != 0 { + t.Fatalf("result = %#v, error = %v", result, err) + } + return + } + var failure *LoadError + if !errors.As(err, &failure) { + t.Fatalf("error = %v", err) + } + found := false + for _, diagnostic := range failure.Diagnostics() { + if diagnostic.Code == CodeResolveUnrecordedChange && diagnostic.Path.String() == test.wantPath { + found = true + } + } + if !found { + t.Fatalf("diagnostics = %#v", failure.Diagnostics()) + } + }) + } +} + +func TestResolutionComparisonKeepsDiffAtomic(t *testing.T) { + before := parseCompositionDocument(t, "before", "items: [{values: [true, null]}]") + after := parseCompositionDocument(t, "after", "items: [{values: [false, 0]}]") + deltas := compareResolutionValues(before, after) + if len(deltas) != 2 || deltas[0].path.String() != "/items/0/values/0" || deltas[1].path.String() != "/items/0/values/1" { + t.Fatalf("resolution deltas = %#v", deltas) + } + atomic := compareConfigurationValues(before, after) + if len(atomic) != 1 || atomic[0].path.String() != "/items" { + t.Fatalf("atomic deltas = %#v", atomic) + } +}