From 2e17e365a1396c60fd277a6ffb7adf03d6d2211f Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 9 Oct 2026 12:14:47 +0000 Subject: [PATCH] Prepare canonical configuration diff regions --- docs/roadmap/implementation.md | 2 +- internal/engine/diff.go | 36 +++++++++ internal/engine/diff_test.go | 144 +++++++++++++++++++++++++++++++++ 3 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 internal/engine/diff.go create mode 100644 internal/engine/diff_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 6028882..4c42d62 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -156,7 +156,7 @@ lands in Stage 10. ## Stage 2 — Prepare canonical changed regions -Status: planned. Scope: a private helper and structural tests only. +Status: complete. Scope: a private helper and structural tests only. 1. Add `internal/engine/diff.go` with private preparation over two inspections. Reject unavailable inspection/final state with safe errors and no regions. diff --git a/internal/engine/diff.go b/internal/engine/diff.go new file mode 100644 index 0000000..89b7373 --- /dev/null +++ b/internal/engine/diff.go @@ -0,0 +1,36 @@ +package engine + +import ( + "errors" + + "gitea.maximumdirect.net/eric/ConfigKit/internal/document" +) + +var errDiffInspectionUnavailable = errors.New("configkit: diff inspection unavailable") + +type diffRegion struct { + path Path + left *document.Node + right *document.Node +} + +func prepareDiffRegions(left, right *Inspection) ([]diffRegion, error) { + if left == nil || left.final == nil || right == nil || right.final == nil { + return nil, errDiffInspectionUnavailable + } + + deltas := compareConfigurationValues(left.final, right.final) + regions := make([]diffRegion, 0, len(deltas)) + for _, delta := range deltas { + leftNode, leftPresent := documentAtPathWithLookup(left.final, delta.path, left.immutableLookup) + if !leftPresent { + leftNode = nil + } + rightNode, rightPresent := documentAtPathWithLookup(right.final, delta.path, right.immutableLookup) + if !rightPresent { + rightNode = nil + } + regions = append(regions, diffRegion{path: delta.path, left: leftNode, right: rightNode}) + } + return regions, nil +} diff --git a/internal/engine/diff_test.go b/internal/engine/diff_test.go new file mode 100644 index 0000000..6b3cbe4 --- /dev/null +++ b/internal/engine/diff_test.go @@ -0,0 +1,144 @@ +package engine + +import ( + "testing" + + "gitea.maximumdirect.net/eric/ConfigKit/internal/document" +) + +func TestPrepareDiffRegionsUsesCanonicalConfigurationComparison(t *testing.T) { + tests := []struct { + name string + left string + right string + want []expectedDiffRegion + }{ + { + name: "mapping order is ignored for equal values", + left: "first: 1\nsecond: true\n", + right: "second: true\nfirst: 1\n", + }, + { + name: "addition removal null and empty values", + left: "removed: null\n", + right: "added: null\nempty_map: {}\nempty_sequence: []\n", + want: []expectedDiffRegion{ + {path: "/added", rightKind: document.Null, rightPresent: true}, + {path: "/empty_map", rightKind: document.Mapping, rightPresent: true}, + {path: "/empty_sequence", rightKind: document.Sequence, rightPresent: true}, + {path: "/removed", leftKind: document.Null, leftPresent: true}, + }, + }, + { + name: "scalar kinds and atomic sequences", + left: "scalar: 1\nitems: [one, two]\n", + right: "scalar: '1'\nitems: [one, changed]\n", + want: []expectedDiffRegion{ + {path: "/items", leftKind: document.Sequence, rightKind: document.Sequence, leftPresent: true, rightPresent: true}, + {path: "/scalar", leftKind: document.Number, rightKind: document.String, leftPresent: true, rightPresent: true}, + }, + }, + { + name: "escaped empty and numeric mapping keys sort by pointer", + left: `"": old` + "\n'0': old\n'01': old\n'a/b': old\n'a~b': old\n", + right: `"": new` + "\n'0': new\n'01': new\n'a/b': new\n'a~b': new\n", + want: []expectedDiffRegion{ + {path: "/", leftKind: document.String, rightKind: document.String, leftPresent: true, rightPresent: true}, + {path: "/0", leftKind: document.String, rightKind: document.String, leftPresent: true, rightPresent: true}, + {path: "/01", leftKind: document.String, rightKind: document.String, leftPresent: true, rightPresent: true}, + {path: "/a~0b", leftKind: document.String, rightKind: document.String, leftPresent: true, rightPresent: true}, + {path: "/a~1b", leftKind: document.String, rightKind: document.String, leftPresent: true, rightPresent: true}, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + leftRoot := parseCompositionDocument(t, "left", test.left) + rightRoot := parseCompositionDocument(t, "right", test.right) + regions, err := prepareDiffRegions(diffTestInspection(leftRoot), diffTestInspection(rightRoot)) + if err != nil { + t.Fatal(err) + } + if len(regions) != len(test.want) { + t.Fatalf("regions = %#v, want %#v", regions, test.want) + } + for index, want := range test.want { + region := regions[index] + if region.path.String() != want.path { + t.Fatalf("region[%d].path = %q, want %q", index, region.path.String(), want.path) + } + assertDiffRegionNode(t, "left", region.left, want.leftPresent, want.leftKind) + assertDiffRegionNode(t, "right", region.right, want.rightPresent, want.rightKind) + } + }) + } +} + +func TestPrepareDiffRegionsHandlesRootReplacementAndBorrowsNodes(t *testing.T) { + leftRoot := &document.Node{Kind: document.String, Text: "before"} + rightRoot := &document.Node{Kind: document.Mapping, Mapping: []document.Entry{{ + Key: "value", Value: &document.Node{Kind: document.Number, Text: "1"}, + }}} + regions, err := prepareDiffRegions(diffTestInspection(leftRoot), diffTestInspection(rightRoot)) + if err != nil { + t.Fatal(err) + } + if len(regions) != 1 || regions[0].path.String() != "" || regions[0].left != leftRoot || regions[0].right != rightRoot { + t.Fatalf("root region = %#v", regions) + } + + leftRoot = parseCompositionDocument(t, "left-child", "value: before\n") + rightRoot = parseCompositionDocument(t, "right-child", "value: after\n") + regions, err = prepareDiffRegions(diffTestInspection(leftRoot), diffTestInspection(rightRoot)) + if err != nil { + t.Fatal(err) + } + if len(regions) != 1 || regions[0].left != mappingValue(t, leftRoot, "value") || regions[0].right != mappingValue(t, rightRoot, "value") { + t.Fatalf("child nodes were not borrowed from the frozen trees: %#v", regions) + } +} + +func TestPrepareDiffRegionsRejectsUnavailableInspections(t *testing.T) { + valid := diffTestInspection(&document.Node{Kind: document.String, Text: "safe"}) + tests := []struct { + name string + left, right *Inspection + }{ + {name: "nil left", left: nil, right: valid}, + {name: "nil right", left: valid, right: nil}, + {name: "missing left tree", left: &Inspection{}, right: valid}, + {name: "missing right tree", left: valid, right: &Inspection{}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + regions, err := prepareDiffRegions(test.left, test.right) + if err == nil || regions != nil { + t.Fatalf("regions, err = %#v, %v", regions, err) + } + }) + } +} + +type expectedDiffRegion struct { + path string + leftKind, rightKind document.Kind + leftPresent, rightPresent bool +} + +func diffTestInspection(root *document.Node) *Inspection { + return &Inspection{final: root, immutableLookup: buildDocumentLookup(root)} +} + +func assertDiffRegionNode(t *testing.T, side string, node *document.Node, wantPresent bool, wantKind document.Kind) { + t.Helper() + if !wantPresent { + if node != nil { + t.Fatalf("%s node = %#v, want absence", side, node) + } + return + } + if node == nil || node.Kind != wantKind { + t.Fatalf("%s node = %#v, want kind %v", side, node, wantKind) + } +}