Precompute canonical paths before sorting

docs/roadmap/implementation.md

@@ -156,7 +156,7 @@ Roadmap candidate 2. Owners: engine `path.go`, `resolution.go`, `metadata.go`.
 Validate: `go test ./internal/engine -run 'Path|Metadata|Resolution|Override|Inheritance'`
 and the everyday check. Public `Path.String` stays unchanged.
 
-## Stage 5: Format sorting keys once per item
+## Stage 5: Format sorting keys once per item — Complete
 
 Roadmap candidate 2. Owners: engine resolution, overlay, override application,
 document comparison, and diagnostics.

internal/engine/diagnostic.go

@@ -85,15 +85,32 @@ func canonicalDiagnostics(diagnostics []Diagnostic) []Diagnostic {
 	for i := range canonical {
 		slices.SortFunc(canonical[i].Related, compareLocation)
 	}
-	slices.SortFunc(canonical, compareDiagnostic)
+	type orderedDiagnostic struct {
+		diagnostic Diagnostic
+		pointer    string
+	}
+	ordered := make([]orderedDiagnostic, len(canonical))
+	for i, diagnostic := range canonical {
+		ordered[i] = orderedDiagnostic{diagnostic: diagnostic, pointer: diagnostic.Path.String()}
+	}
+	slices.SortFunc(ordered, func(left, right orderedDiagnostic) int {
+		return compareDiagnosticWithPaths(left.diagnostic, right.diagnostic, left.pointer, right.pointer)
+	})
+	for i, item := range ordered {
+		canonical[i] = item.diagnostic
+	}
 	return canonical
 }
 
 func compareDiagnostic(left, right Diagnostic) int {
+	return compareDiagnosticWithPaths(left, right, left.Path.String(), right.Path.String())
+}
+
+func compareDiagnosticWithPaths(left, right Diagnostic, leftPath, rightPath string) int {
 	if order := compareOptionalLocation(left.Primary, right.Primary); order != 0 {
 		return order
 	}
-	if order := cmp.Compare(left.Path.String(), right.Path.String()); order != 0 {
+	if order := cmp.Compare(leftPath, rightPath); order != 0 {
 		return order
 	}
 	if order := cmp.Compare(left.Code, right.Code); order != 0 {

internal/engine/diagnostic_test.go

@@ -90,6 +90,26 @@ func TestDiagnosticsAreCanonicalAndDetached(t *testing.T) {
 	}
 }
 
+func TestCanonicalDiagnosticsSortEscapedPaths(t *testing.T) {
+	primary := &Location{SourceID: "source-1", Line: 1, Column: 1}
+	diagnostics := []Diagnostic{
+		{Code: "same", Severity: SeverityError, Path: Keys("a/b"), Message: "same", Primary: primary},
+		{Code: "same", Severity: SeverityError, Path: Keys("a~b"), Message: "same", Primary: primary},
+		{Code: "z", Severity: SeverityError, Path: Keys("same"), Message: "same", Primary: primary},
+		{Code: "a", Severity: SeverityError, Path: Keys("same"), Message: "same", Primary: primary},
+	}
+	canonical := canonicalDiagnostics(diagnostics)
+	want := []string{"/a~0b", "/a~1b", "/same", "/same"}
+	for i, diagnostic := range canonical {
+		if got := diagnostic.Path.String(); got != want[i] {
+			t.Fatalf("diagnostic[%d] path = %q, want %q", i, got, want[i])
+		}
+	}
+	if canonical[2].Code != "a" || canonical[3].Code != "z" {
+		t.Fatalf("same-path code order = %q, %q", canonical[2].Code, canonical[3].Code)
+	}
+}
+
 func TestWriteDiagnosticsEscapesControls(t *testing.T) {
 	t.Parallel()
 

internal/engine/document_compare.go

@@ -24,12 +24,23 @@ type documentDelta struct {
 func compareConfigurationValues(before, after *document.Node) []documentDelta {
 	var deltas []documentDelta
 	compareConfigurationValue(before, after, Path{}, &deltas)
-	slices.SortFunc(deltas, func(left, right documentDelta) int {
+	type orderedDelta struct {
-		if order := cmp.Compare(left.path.String(), right.path.String()); order != 0 {
+		delta   documentDelta
+		pointer string
+	}
+	ordered := make([]orderedDelta, len(deltas))
+	for i, delta := range deltas {
+		ordered[i] = orderedDelta{delta: delta, pointer: delta.path.String()}
+	}
+	slices.SortFunc(ordered, func(left, right orderedDelta) int {
+		if order := cmp.Compare(left.pointer, right.pointer); order != 0 {
 			return order
 		}
-		return int(left.kind) - int(right.kind)
+		return int(left.delta.kind) - int(right.delta.kind)
 	})
+	for i, item := range ordered {
+		deltas[i] = item.delta
+	}
 	return deltas
 }
 

internal/engine/overlay.go

@@ -131,9 +131,24 @@ func previousOrigins(origin Origin) []Origin {
 func displacedOrigins(node *document.Node, origins map[string]Origin, path Path) []displacedDefinition {
 	var displaced []displacedDefinition
 	collectDisplacedOrigins(&displaced, node, origins, path, Origin{})
-	slices.SortFunc(displaced, func(left, right displacedDefinition) int {
+	return sortDisplacedDefinitions(displaced)
-		return strings.Compare(left.path.String(), right.path.String())
+}
+
+func sortDisplacedDefinitions(displaced []displacedDefinition) []displacedDefinition {
+	type orderedDefinition struct {
+		definition displacedDefinition
+		pointer    string
+	}
+	ordered := make([]orderedDefinition, len(displaced))
+	for i, definition := range displaced {
+		ordered[i] = orderedDefinition{definition: definition, pointer: definition.path.String()}
+	}
+	slices.SortFunc(ordered, func(left, right orderedDefinition) int {
+		return strings.Compare(left.pointer, right.pointer)
 	})
+	for i, item := range ordered {
+		displaced[i] = item.definition
+	}
 	return displaced
 }
 

internal/engine/overlay_test.go

@@ -117,3 +117,26 @@ func TestOwnedOverlayKeepsCompositeContributorsAndAcquiredInputs(t *testing.T) {
 		t.Fatal("working tree mutation changed an acquired input")
 	}
 }
+
+func TestDisplacedOriginsSortEscapedPointers(t *testing.T) {
+	input := parseCompositionDocument(t, "source-1", `values:
+  z: old
+  a/b: slash
+  a~b: tilde
+  nested: {x: old}
+`)
+	composed, diagnostics := composeDocuments(input)
+	if len(diagnostics) != 0 {
+		t.Fatal(diagnostics)
+	}
+	displaced := displacedOrigins(composed.root, composed.origins, Path{})
+	want := []string{"", "/values", "/values/a~0b", "/values/a~1b", "/values/nested", "/values/nested/x", "/values/z"}
+	if len(displaced) != len(want) {
+		t.Fatalf("displaced definitions = %d, want %d", len(displaced), len(want))
+	}
+	for i, definition := range displaced {
+		if got := definition.path.String(); got != want[i] {
+			t.Fatalf("displaced[%d] path = %q, want %q", i, got, want[i])
+		}
+	}
+}

internal/engine/override_apply.go

@@ -1,9 +1,6 @@
 package engine
 
 import (
-	"slices"
-	"strings"
-
 	"gitea.maximumdirect.net/eric/ConfigKit/internal/document"
 )
 
@@ -86,26 +83,28 @@ func applyPreparedOverride(result *composedDocument, override preparedOverride,
 }
 
 func affectedOverrideOrigins(existing, incoming *document.Node, origins map[string]Origin, path Path, index *mappingIndex) []displacedDefinition {
-	displaced := []displacedDefinition{{path: path.clone(), origin: cloneOrigin(origins[path.String()])}}
+	var displaced []displacedDefinition
-	if existing.Kind == document.Mapping && incoming.Kind == document.Mapping {
+	collectAffectedOverrideOrigins(&displaced, existing, incoming, origins, path, index)
-		for _, entry := range incoming.Mapping {
+	return sortDisplacedDefinitions(displaced)
-			position := index.find(existing, entry.Key)
+}
-			if position < 0 {
+
-				continue
+func collectAffectedOverrideOrigins(displaced *[]displacedDefinition, existing, incoming *document.Node, origins map[string]Origin, path Path, index *mappingIndex) {
-			}
+	if existing.Kind != document.Mapping || incoming.Kind != document.Mapping {
-			childPath := path.Key(entry.Key)
+		collectDisplacedOrigins(displaced, existing, origins, path, Origin{})
-			childExisting := existing.Mapping[position].Value
+		return
-			if childExisting.Kind == document.Mapping && entry.Value.Kind == document.Mapping {
+	}
-				displaced = append(displaced, affectedOverrideOrigins(childExisting, entry.Value, origins, childPath, index)...)
+	*displaced = append(*displaced, displacedDefinition{path: path.clone(), origin: cloneOrigin(origins[path.String()])})
-			} else {
+	for _, entry := range incoming.Mapping {
-				displaced = append(displaced, displacedOrigins(childExisting, origins, childPath)...)
+		position := index.find(existing, entry.Key)
-			}
+		if position < 0 {
-		}
+			continue
-	} else {
+		}
-		displaced = displacedOrigins(existing, origins, path)
+		childPath := path.Key(entry.Key)
-	}
+		childExisting := existing.Mapping[position].Value
-	slices.SortFunc(displaced, func(left, right displacedDefinition) int {
+		if childExisting.Kind == document.Mapping && entry.Value.Kind == document.Mapping {
-		return strings.Compare(left.path.String(), right.path.String())
+			collectAffectedOverrideOrigins(displaced, childExisting, entry.Value, origins, childPath, index)
-	})
+		} else {
-	return displaced
+			collectDisplacedOrigins(displaced, childExisting, origins, childPath, Origin{})
+		}
+	}
 }

internal/engine/override_test.go

@@ -271,6 +271,43 @@ func TestOverrideEqualReplacementStillAppearsInHistory(t *testing.T) {
 	}
 }
 
+func TestAffectedOverrideOriginsSortEscapedPointers(t *testing.T) {
+	existingInput := parseCompositionDocument(t, "source-1", `values:
+  z: old
+  a/b: slash
+  a~b: tilde
+  nested: {x: old}
+`)
+	existing, diagnostics := composeDocuments(existingInput)
+	if len(diagnostics) != 0 {
+		t.Fatal(diagnostics)
+	}
+	incoming := parseCompositionDocument(t, "source-2", `values:
+  z: new
+  a/b: new
+  a~b: new
+  nested: {x: new}
+`)
+	existingValues, ok := documentAtPath(existing.root, Keys("values"))
+	if !ok {
+		t.Fatal("existing values mapping is missing")
+	}
+	incomingValues, ok := documentAtPath(incoming, Keys("values"))
+	if !ok {
+		t.Fatal("incoming values mapping is missing")
+	}
+	displaced := affectedOverrideOrigins(existingValues, incomingValues, existing.origins, Keys("values"), &mappingIndex{})
+	want := []string{"/values", "/values/a~0b", "/values/a~1b", "/values/nested", "/values/nested/x", "/values/z"}
+	if len(displaced) != len(want) {
+		t.Fatalf("displaced definitions = %d, want %d", len(displaced), len(want))
+	}
+	for i, definition := range displaced {
+		if got := definition.path.String(); got != want[i] {
+			t.Fatalf("displaced[%d] path = %q, want %q", i, got, want[i])
+		}
+	}
+}
+
 func TestOverrideAggregateLimitsFailBeforeApplication(t *testing.T) {
 	base, diagnostics := composeDocuments(&document.Node{Kind: document.Mapping})
 	if len(diagnostics) != 0 {

internal/engine/projection_test.go

@@ -357,6 +357,21 @@ func TestCompareDocumentTreesTracksSemanticChanges(t *testing.T) {
 	}
 }
 
+func TestCompareConfigurationValuesSortEscapedPointers(t *testing.T) {
+	before := parseCompositionDocument(t, "before", `values: {z: old, 'a/b': old, 'a~b': old, nested: {x: old}}`)
+	after := parseCompositionDocument(t, "after", `values: {z: new, 'a/b': new, 'a~b': new, nested: {x: new}}`)
+	deltas := compareConfigurationValues(before, after)
+	want := []string{"/values/a~0b", "/values/a~1b", "/values/nested/x", "/values/z"}
+	if len(deltas) != len(want) {
+		t.Fatalf("deltas = %#v", deltas)
+	}
+	for i, delta := range deltas {
+		if got := delta.path.String(); got != want[i] {
+			t.Fatalf("delta[%d] path = %q, want %q", i, got, want[i])
+		}
+	}
+}
+
 func mustSchemaFor[T any](t *testing.T) *schemaDescriptor {
 	t.Helper()
 	schema, issue := testBuildSchema(reflect.TypeFor[T]())

internal/engine/resolution.go

@@ -313,14 +313,22 @@ func runResolution[T any](configuration *T, resolver func(*T, *Resolution) error
 }
 
 func orderedResolutionRecords(records map[string]*resolutionRecord) []*resolutionRecord {
-	ordered := make([]*resolutionRecord, 0, len(records))
+	type orderedRecord struct {
-	for _, record := range records {
+		record  *resolutionRecord
-		ordered = append(ordered, record)
+		pointer string
 	}
-	slices.SortFunc(ordered, func(left, right *resolutionRecord) int {
+	ordered := make([]orderedRecord, 0, len(records))
-		return strings.Compare(left.path.String(), right.path.String())
+	for _, record := range records {
+		ordered = append(ordered, orderedRecord{record: record, pointer: record.path.String()})
+	}
+	slices.SortFunc(ordered, func(left, right orderedRecord) int {
+		return strings.Compare(left.pointer, right.pointer)
 	})
-	return ordered
+	result := make([]*resolutionRecord, len(ordered))
+	for i, item := range ordered {
+		result[i] = item.record
+	}
+	return result
 }
 
 func coveringResolutionRecord(records []*resolutionRecord, path Path) *resolutionRecord {

internal/engine/resolution_test.go

@@ -206,6 +206,31 @@ func TestResolutionSequenceReplacementUpdatesDescendantOrigins(t *testing.T) {
 	}
 }
 
+func TestOrderedResolutionRecordsSortEscapedPointers(t *testing.T) {
+	parsedIndex, err := ParsePath("/items/0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	records := map[string]*resolutionRecord{
+		"tilde":  {path: Keys("a~b")},
+		"slash":  {path: Keys("a/b")},
+		"nested": {path: Keys("a", "b")},
+		"index":  {path: Keys("items").Index(0)},
+		"parsed": {path: parsedIndex},
+		"last":   {path: Keys("z")},
+	}
+	ordered := orderedResolutionRecords(records)
+	want := []string{"/a/b", "/a~0b", "/a~1b", "/items/0", "/items/0", "/z"}
+	if len(ordered) != len(want) {
+		t.Fatalf("ordered records = %d, want %d", len(ordered), len(want))
+	}
+	for i, record := range ordered {
+		if got := record.path.String(); got != want[i] {
+			t.Fatalf("record[%d] path = %q, want %q", i, got, want[i])
+		}
+	}
+}
+
 func TestResolutionRejectsUnrecordedAndMismatchedMutations(t *testing.T) {
 	t.Parallel()