From 8e3a0892d49d93edf94be6e232c48eef37291875 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 17 Oct 2026 04:54:01 +0000 Subject: [PATCH] Project item conflict diagnostics through enclosing scopes --- docs/roadmap/remediation.md | 9 ++++- facade_sequence_test.go | 58 ++++++++++++++++++++++++++++ internal/engine/imports.go | 68 +++++++++++++++++++++++++++++---- internal/engine/load_request.go | 2 +- 4 files changed, 127 insertions(+), 10 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 64e2ac8..80f7282 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -266,7 +266,14 @@ timing-threshold test requirement. ## 8. Do not fabricate index zero for item-local conflict diagnostics — P2 -- [ ] Addressed and regression-tested. +- [x] Addressed and regression-tested. + +Completion: failed item composition is projected through each enclosing scope +using preceding contributions, including prior grouped inputs. Only composition +diagnostics are rebased; physical locations and source-local access/syntax +diagnostics are retained. Regressions cover authored/imported predecessors, +nested items, item mains, grouped item sources, and failure evidence. Focused +facade/engine tests and the everyday check passed. **Owner:** `visitItemSource` in [imports.go](../../internal/engine/imports.go), lines 519–525. diff --git a/facade_sequence_test.go b/facade_sequence_test.go index 1491ab7..1825f76 100644 --- a/facade_sequence_test.go +++ b/facade_sequence_test.go @@ -142,6 +142,64 @@ func TestFacadeRegisteredReplacementExplainsRemovedDescendants(t *testing.T) { } } +func TestFacadeItemConflictDiagnosticsFollowEnclosingIndexes(t *testing.T) { + t.Parallel() + filesystem := configkit.NewFilesystem("conflicts", fstest.MapFS{ + "main.yml": {Data: []byte("records: [{id: authored}]\nconfigkit: {imports: [{path: good.yml, destination: [records], placement: sequence-item}, {path: bad.yml, destination: [records], placement: sequence-item}]}\n")}, + "group.yml": {Data: []byte("records: [{id: authored}]\nconfigkit: {imports: [{path: bad.yml, destination: [records], placement: sequence-item}]}\n")}, + "nested.yml": {Data: []byte("records: [{id: authored}]\nconfigkit: {imports: [{path: parent.yml, destination: [records], placement: sequence-item}]}\n")}, + "parent.yml": {Data: []byte("id: parent\nchildren: [{id: authored-child}]\nconfigkit: {imports: [{path: bad.yml, destination: [children], placement: sequence-item}]}\n")}, + "good.yml": {Data: []byte("id: good\n")}, + "bad.yml": {Data: []byte("id: bad\nconfigkit: {imports: [{path: helper.yml}]}\n")}, + "helper.yml": {Data: []byte("id: duplicate-field\n")}, + }) + definition := configkit.Definition[map[string]any]{SequenceCompositions: []configkit.SequenceComposition{{Path: configkit.Keys("records"), IdentityField: "id"}}} + for _, test := range []struct { + name, file, path string + grouped, itemMain bool + }{ + {"preceding authored and imported items", "main.yml", "/records/2/id", false, false}, + {"nested item", "nested.yml", "/records/1/children/1/id", false, false}, + {"preceding grouped input", "group.yml", "/records/2/id", true, false}, + {"item main source", "bad.yml", "/records/0/id", false, true}, + {"grouped item source", "bad.yml", "/records/1/id", true, true}, + } { + t.Run(test.name, func(t *testing.T) { + var err error + input := configkit.Input{Source: filesystem.File(test.file), Imports: true} + if test.itemMain { + input.Destination = configkit.Keys("records") + input.PlacementMode = configkit.PlacementModeSequenceItem + } + if test.grouped { + _, err = configkit.LoadLayered[map[string]any](configkit.Input{Source: configkit.NoSource()}, configkit.LayeredOptions[map[string]any]{ + Definition: definition, Underlays: []configkit.Underlay{{Inputs: []configkit.Input{ + {Source: configkit.Mapping("prior", map[string]any{"records": []any{map[string]any{"id": "prior"}}})}, + input, + }}}, + }) + } else { + _, err = configkit.Load(input.Source, configkit.Options[map[string]any]{Definition: definition, Imports: true, Destination: input.Destination, PlacementMode: input.PlacementMode}) + } + loadError, ok := err.(*configkit.LoadError) + if !ok || len(loadError.Diagnostics()) != 1 { + t.Fatalf("expected helper conflict: %v", err) + } + diagnostic := loadError.Diagnostics()[0] + if diagnostic.Code != configkit.CodeComposeConflict || diagnostic.Path.String() != test.path { + t.Fatalf("diagnostic = %s %s, want %s", diagnostic.Code, diagnostic.Path.String(), test.path) + } + files := make(map[string]string) + for _, source := range loadError.Sources() { + files[source.ID] = source.Path + } + if diagnostic.Primary == nil || files[diagnostic.Primary.SourceID] != "bad.yml" || len(diagnostic.Related) != 1 || files[diagnostic.Related[0].SourceID] != "helper.yml" { + t.Fatal("conflict lost physical source locations") + } + }) + } +} + func TestFacadeSequencePlacementControlsAcrossLoadEntryPoints(t *testing.T) { definition := configkit.Definition[map[string]any]{ SequenceCompositions: []configkit.SequenceComposition{{Path: configkit.Keys("entries"), IdentityField: "id"}}, diff --git a/internal/engine/imports.go b/internal/engine/imports.go index 43fd597..e2e70ea 100644 --- a/internal/engine/imports.go +++ b/internal/engine/imports.go @@ -214,6 +214,9 @@ func loadImportedWithAccessTrackedAndProcessingPolicy(access *filesystemAccess, closeErr := access.close() if len(diagnostics) != 0 { + if mainIsItem { + diagnostics = prependCompositionDiagnosticPaths(diagnostics, destination.Index(0)) + } return nil, cloneSourceInfos(state.sources), diagnostics } if closeErr != nil { @@ -227,7 +230,7 @@ func loadImportedWithAccessTrackedAndProcessingPolicy(access *filesystemAccess, } completed, failures := composeDocuments(documents...) if len(failures) != 0 { - return nil, cloneSourceInfos(state.sources), prependDiagnosticPaths(failures, destination.Index(0)) + return nil, cloneSourceInfos(state.sources), prependCompositionDiagnosticPaths(failures, destination.Index(0)) } if versionState != nil { protectionRule := versionState.localRule @@ -525,7 +528,7 @@ func (s *importTraversal) visitItemSource(canonical string, declaration importDe itemScope := s.scope s.scope, s.itemScope, s.itemPrefix, s.itemFallback = parentScope, parentItemScope, parentPrefix, parentFallback if len(diagnostics) != 0 { - return diagnostics + return projectItemDiagnosticPaths(diagnostics, parentScope, destination) } documents := make([]*document.Node, 0, len(itemScope.contributions)) for _, contribution := range itemScope.contributions { @@ -533,11 +536,7 @@ func (s *importTraversal) visitItemSource(canonical string, declaration importDe } completed, diagnostics := composeDocuments(documents...) if len(diagnostics) != 0 { - prefix := destination.Index(0) - if parentItemScope { - prefix = appendDestination(parentPrefix, prefix) - } - return prependDiagnosticPaths(diagnostics, prefix) + return projectItemDiagnosticPaths(diagnostics, parentScope, destination) } itemScope.destination = Path{} completed.acquisition = []acquisitionScope{cloneAcquisitionScope(itemScope)} @@ -580,9 +579,12 @@ func importedVersionClaim(documents []*document.Node, composed *composedDocument return nil } -func prependDiagnosticPaths(diagnostics []Diagnostic, prefix Path) []Diagnostic { +func prependCompositionDiagnosticPaths(diagnostics []Diagnostic, prefix Path) []Diagnostic { cloned := append([]Diagnostic(nil), diagnostics...) for index := range cloned { + if !isCompositionDiagnostic(cloned[index]) { + continue + } cloned[index].Path = appendDestination(prefix, cloned[index].Path) cloned[index].Primary = cloneLocation(cloned[index].Primary) cloned[index].Related = cloneLocations(cloned[index].Related) @@ -590,6 +592,56 @@ func prependDiagnosticPaths(diagnostics []Diagnostic, prefix Path) []Diagnostic return cloned } +func isCompositionDiagnostic(diagnostic Diagnostic) bool { + return diagnostic.Code == CodeComposeConflict || diagnostic.Code == CodeComposeInvalid +} + +// On a composition failure only the earlier contributions are needed to locate +// this item. Project one enclosing scope at a time as traversal unwinds; syntax +// and access diagnostics keep their original source-local coordinates. +func projectItemDiagnosticPaths(diagnostics []Diagnostic, parent acquisitionScope, destination Path) []Diagnostic { + for _, diagnostic := range diagnostics { + if isCompositionDiagnostic(diagnostic) { + index := priorSequenceLength([]acquisitionScope{parent}, destination) + return prependCompositionDiagnosticPaths(diagnostics, destination.Index(index)) + } + } + return diagnostics +} + +// This scan is confined to failed loads, not the successful construction path. +func priorSequenceLength(scopes []acquisitionScope, path Path) int { + length := 0 + for _, scope := range scopes { + for _, contribution := range scope.contributions { + if node, present := documentAtPath(contribution.document, path); present && node.Kind == document.Sequence { + length += len(node.Sequence) + } + } + } + return length +} + +func projectPriorInputDiagnosticPaths(diagnostics []Diagnostic, scopes []acquisitionScope) []Diagnostic { + cloned := cloneDiagnostics(diagnostics) + for index := range cloned { + if !isCompositionDiagnostic(cloned[index]) { + continue + } + for position, segment := range cloned[index].Path.segments { + if segment.kind != pathSegmentIndex { + continue + } + container := Path{segments: cloned[index].Path.segments[:position]} + cloned[index].Path.segments[position].index += priorSequenceLength(scopes, container) + // Registration does not address paths through item indexes. A prior + // input shifts the outer collection, not its item-local sequences. + break + } + } + return cloned +} + func projectNestedEvidence(composed *composedDocument, scope acquisitionScope) { if composed == nil { return diff --git a/internal/engine/load_request.go b/internal/engine/load_request.go index 469ea82..31f91bf 100644 --- a/internal/engine/load_request.go +++ b/internal/engine/load_request.go @@ -175,7 +175,7 @@ func acquireUnderlayGroupWithPolicyVersion(group loadGroup, index int, rule *Ver } mergeResourceContexts(&contexts, acquired.contexts) if len(diagnostics) != 0 { - return nil, sources, contexts, diagnostics + return nil, sources, contexts, projectPriorInputDiagnosticPaths(diagnostics, scopes) } if acquired.document == nil { continue