From 1954052631e5647c9d130d56cc925109dca656bb Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 26 Oct 2026 18:17:04 +0000 Subject: [PATCH] Preserve composition conflict claimant order --- docs/roadmap/implementation.md | 7 ++- facade_diagnostic_order_test.go | 85 ++++++++++++++++++++++++++++++ internal/engine/diagnostic.go | 4 +- internal/engine/diagnostic_test.go | 39 ++++++++++++++ 4 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 facade_diagnostic_order_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index d89d7d2..2886e03 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Interface Contract Remediation Implementation Plan -Status: in progress. Stages 1–16 are complete; Stages 17–18 remain pending. +Status: in progress. Stages 1–17 are complete; Stage 18 remains pending. Baseline: `f6d54a094a2b5ada5748686620659399dce7ebf0`. Audience: a Luna coding agent executing the stages sequentially. @@ -719,7 +719,10 @@ ordinary suite when buildable: `go test ./...`. ## Stage 17 — Preserve Composition Conflict Ordering End To End Finding 4. Owners: `canonicalDiagnostics`, composition diagnostic producers, -root diagnostic conversion/rendering tests. Status: pending. +root diagnostic conversion/rendering tests. Status: complete. Validation: +`GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Diagnostic|Compose|Conflict' -count=1`, +`GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test . -run 'TestFacadeCompositionConflictKeepsClaimantOrder' -count=1`, +and `git diff --check` passed. 1. Implement the code-based ordering rule specified above. Verify every `CodeComposeConflict` producer; keep claimant order and leave single-related diff --git a/facade_diagnostic_order_test.go b/facade_diagnostic_order_test.go new file mode 100644 index 0000000..c4cea7c --- /dev/null +++ b/facade_diagnostic_order_test.go @@ -0,0 +1,85 @@ +package configkit_test + +import ( + "bytes" + "errors" + "strings" + "testing" + "testing/fstest" + + configkit "gitea.maximumdirect.net/eric/ConfigKit" +) + +func TestFacadeCompositionConflictKeepsClaimantOrder(t *testing.T) { + filesystem := configkit.NewFilesystem("bundle", fstest.MapFS{ + "a.yml": &fstest.MapFile{Data: []byte(`configkit: + imports: + - path: z.yml +`)}, + "b.yml": &fstest.MapFile{Data: []byte("x: b\n")}, + "c.yml": &fstest.MapFile{Data: []byte("x: c\n")}, + "z.yml": &fstest.MapFile{Data: []byte("x: z\n")}, + }) + main := configkit.Mapping("main", map[string]any{ + "configkit": map[string]any{"imports": []any{ + map[string]any{"path": "a.yml"}, + map[string]any{"path": "b.yml"}, + map[string]any{"path": "c.yml"}, + }}, + }) + _, err := configkit.Load[map[string]any](main, configkit.Options[map[string]any]{ + Imports: true, + Filesystem: &configkit.FilesystemContext{ + FS: &filesystem, Root: ".", BaseDir: ".", + }, + }) + var loadError *configkit.LoadError + if !errors.As(err, &loadError) { + t.Fatalf("Load error = %v, want composition LoadError", err) + } + diagnostics := loadError.Diagnostics() + if len(diagnostics) != 1 { + t.Fatalf("diagnostics = %#v, want one composition conflict", diagnostics) + } + diagnostic := diagnostics[0] + if diagnostic.Code != configkit.CodeComposeConflict || diagnostic.Path.String() != "/x" || diagnostic.Primary == nil { + t.Fatalf("composition diagnostic = %#v", diagnostic) + } + + pathsByID := make(map[string]string) + for _, source := range loadError.Sources() { + pathsByID[source.ID] = source.Path + } + claimant := func(location configkit.Location) string { + for _, name := range []string{"b.yml", "c.yml", "z.yml"} { + if pathsByID[location.SourceID] == name || strings.HasSuffix(pathsByID[location.SourceID], "/"+name) { + return strings.TrimSuffix(name, ".yml") + } + } + return "" + } + if got := claimant(*diagnostic.Primary); got != "b" { + t.Fatalf("primary claimant = %q, want b (sources: %#v)", got, pathsByID) + } + if len(diagnostic.Related) != 2 || claimant(diagnostic.Related[0]) != "c" || claimant(diagnostic.Related[1]) != "z" { + t.Fatalf("related claimants = %#v, want c then z", diagnostic.Related) + } + + var output bytes.Buffer + if err := configkit.WriteDiagnostics(&output, diagnostics); err != nil { + t.Fatal(err) + } + positions := []int{ + strings.Index(output.String(), diagnostic.Primary.SourceID), + strings.Index(output.String(), diagnostic.Related[0].SourceID), + strings.Index(output.String(), diagnostic.Related[1].SourceID), + } + if positions[0] < 0 || positions[1] <= positions[0] || positions[2] <= positions[1] { + t.Fatalf("rendered claimant order = %v in %q", positions, output.String()) + } + + diagnostics[0].Related[0].SourceID = "mutated" + if got := claimant(loadError.Diagnostics()[0].Related[0]); got != "c" { + t.Fatalf("LoadError.Diagnostics retained caller mutation; first claimant = %q", got) + } +} diff --git a/internal/engine/diagnostic.go b/internal/engine/diagnostic.go index 4e78a02..27e0675 100644 --- a/internal/engine/diagnostic.go +++ b/internal/engine/diagnostic.go @@ -88,7 +88,9 @@ func (a *diagnosticAccumulator) loadError() *LoadError { func canonicalDiagnostics(diagnostics []Diagnostic) []Diagnostic { canonical := cloneDiagnostics(diagnostics) for i := range canonical { - slices.SortFunc(canonical[i].Related, compareLocation) + if canonical[i].Code != CodeComposeConflict { + slices.SortFunc(canonical[i].Related, compareLocation) + } } type orderedDiagnostic struct { diagnostic Diagnostic diff --git a/internal/engine/diagnostic_test.go b/internal/engine/diagnostic_test.go index 518071e..c6726f6 100644 --- a/internal/engine/diagnostic_test.go +++ b/internal/engine/diagnostic_test.go @@ -4,6 +4,7 @@ import ( "bytes" "errors" "io" + "slices" "strings" "testing" ) @@ -110,6 +111,44 @@ func TestCanonicalDiagnosticsSortEscapedPaths(t *testing.T) { } } +func TestCanonicalDiagnosticsPreservesCompositionClaimantOrder(t *testing.T) { + composition := Diagnostic{ + Code: CodeComposeConflict, + Severity: SeverityError, + Path: Keys("value"), + Primary: &Location{SourceID: "source-4"}, + Related: []Location{ + {SourceID: "source-5"}, + {SourceID: "source-3"}, + }, + } + other := Diagnostic{ + Code: "decode.type", + Severity: SeverityError, + Path: Keys("other"), + Related: []Location{ + {SourceID: "source-5"}, + {SourceID: "source-3"}, + }, + } + + canonical := canonicalDiagnostics([]Diagnostic{composition, other}) + var compositionResult, genericResult Diagnostic + for _, diagnostic := range canonical { + if diagnostic.Code == CodeComposeConflict { + compositionResult = diagnostic + } else { + genericResult = diagnostic + } + } + if got := []string{compositionResult.Related[0].SourceID, compositionResult.Related[1].SourceID}; !slices.Equal(got, []string{"source-5", "source-3"}) { + t.Fatalf("composition related locations = %v, want claimant order", got) + } + if got := []string{genericResult.Related[0].SourceID, genericResult.Related[1].SourceID}; !slices.Equal(got, []string{"source-3", "source-5"}) { + t.Fatalf("generic related locations = %v, want sorted order", got) + } +} + func TestWriteDiagnosticsEscapesControls(t *testing.T) { t.Parallel()