From af43d6f4385361a5c0aa038e9ce00cfe726ea328 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 19 Oct 2026 19:39:19 +0000 Subject: [PATCH] Schedule item-local import scopes --- docs/config.md | 13 +-- facade_sequence_test.go | 50 +++++------- internal/engine/import_graph.go | 4 +- internal/engine/imports.go | 25 ++++-- internal/engine/imports_test.go | 135 +++++++++++++++++++++++++++++++- 5 files changed, 181 insertions(+), 46 deletions(-) diff --git a/docs/config.md b/docs/config.md index 26ab4c2..511837c 100644 --- a/docs/config.md +++ b/docs/config.md @@ -936,11 +936,14 @@ version-callback order. Ordinary mapping contributions in a main tier are assemb dependency-first: ConfigKit selects the lexically smallest stable source key among the contributions whose imports are complete. Independent branches may interleave, and the main body may be assembled after its imports. Sequence-item contributions -retain their declaration order and local helper scope; they are not flattened into -this ordinary mapping schedule. Assembly order never permits replacement. Required -missing paths fail; `optional: true` suppresses only target nonexistence, not invalid -paths, containment failures, directories, permissions, parse errors, or composition -errors. +in that tier are nodes in the same schedule; each selected item first assembles its +own strict mapping scope, with helpers before the bodies that import them. Those +local scopes do not use application collection policies and are not flattened into +the outer schedule. Item positions and diagnostic paths follow the resulting +assembly order. +Assembly order never permits replacement. Required missing paths fail; `optional: +true` suppresses only target nonexistence, not invalid paths, containment failures, +directories, permissions, parse errors, or composition errors. Glob declarations use the slash-based Go `path.Match` grammar: `*`, `?`, character classes, and backslash escapes are supported, while recursive `**` syntax is invalid. diff --git a/facade_sequence_test.go b/facade_sequence_test.go index 1825f76..f98bccc 100644 --- a/facade_sequence_test.go +++ b/facade_sequence_test.go @@ -51,7 +51,7 @@ func TestFacadeNoSourcePreservesAbsenceWithSequencePolicy(t *testing.T) { } } -func TestFacadeItemHelperHistoryPreservesTraversalOrder(t *testing.T) { +func TestFacadeItemHelperHistoryPreservesScheduledPlacement(t *testing.T) { t.Parallel() filesystem := configkit.NewFilesystem("history", fstest.MapFS{ "main.yml": {Data: []byte("records: [{id: authored}]\nconfigkit: {imports: [{path: one.yml, destination: [records], placement: sequence-item}, {path: two.yml, destination: [records], placement: sequence-item}]}\n")}, @@ -68,19 +68,17 @@ func TestFacadeItemHelperHistoryPreservesTraversalOrder(t *testing.T) { if err != nil { t.Fatal(err) } - sourceOrder := make(map[string]int) sourceFiles := make(map[string]string) - for index, source := range result.Sources() { - sourceOrder[source.ID] = index + for _, source := range result.Sources() { sourceFiles[source.ID] = source.Path } for _, test := range []struct { path configkit.Path file, text string }{ - {configkit.Keys("records").Index(1).Key("text"), "helper-one.yml", "first helper\n"}, - {configkit.Keys("records").Index(2).Key("text"), "helper-two.yml", "second helper\n"}, - {configkit.Keys("records").Index(1).Key("children").Index(0).Key("text"), "child-helper.yml", "nested helper\n"}, + {configkit.Keys("records").Index(0).Key("text"), "helper-one.yml", "first helper\n"}, + {configkit.Keys("records").Index(1).Key("text"), "helper-two.yml", "second helper\n"}, + {configkit.Keys("records").Index(0).Key("children").Index(0).Key("text"), "child-helper.yml", "nested helper\n"}, } { explanation, ok := result.Explain(test.path) if !ok || explanation.Effective.YAML != test.text || len(explanation.History) != 1 { @@ -95,20 +93,6 @@ func TestFacadeItemHelperHistoryPreservesTraversalOrder(t *testing.T) { t.Fatal("metadata and explanation disagree") } } - explanation, ok := result.Explain(configkit.Keys()) - if !ok { - t.Fatal("root explanation missing") - } - previous := -1 - for _, change := range explanation.History { - for _, location := range change.Origin.Locations { - current := sourceOrder[location.SourceID] - if current < previous { - t.Fatalf("source %s replayed out of traversal order", location.SourceID) - } - previous = current - } - } } func TestFacadeRegisteredReplacementExplainsRemovedDescendants(t *testing.T) { @@ -158,9 +142,9 @@ func TestFacadeItemConflictDiagnosticsFollowEnclosingIndexes(t *testing.T) { 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}, + {"preceding authored and imported items", "main.yml", "/records/0/id", false, false}, + {"nested item", "nested.yml", "/records/0/children/0/id", false, false}, + {"preceding grouped input", "group.yml", "/records/1/id", true, false}, {"item main source", "bad.yml", "/records/0/id", false, true}, {"grouped item source", "bad.yml", "/records/1/id", true, true}, } { @@ -193,8 +177,16 @@ func TestFacadeItemConflictDiagnosticsFollowEnclosingIndexes(t *testing.T) { 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") + locations := append([]configkit.Location(nil), diagnostic.Related...) + if diagnostic.Primary != nil { + locations = append(locations, *diagnostic.Primary) + } + gotFiles := make(map[string]bool, len(locations)) + for _, location := range locations { + gotFiles[files[location.SourceID]] = location.Line == 1 && location.Column == 5 + } + if len(locations) != 2 || !gotFiles["bad.yml"] || !gotFiles["helper.yml"] { + t.Fatalf("conflict lost physical source locations: diagnostic=%#v sources=%#v", diagnostic, files) } }) } @@ -326,7 +318,7 @@ items: t.Fatalf("load item glob: %v", err) } items := result.Config["items"].([]any) - wantIDs := []string{"authored", "imported-a", "imported-b"} + wantIDs := []string{"imported-a", "imported-b", "authored"} for index, item := range items { if got := item.(map[string]any)["id"]; got != wantIDs[index] { t.Errorf("item %d ID = %#v, want %q", index, got, wantIDs[index]) @@ -344,10 +336,10 @@ items: } } metadata := result.Metadata() - for index := 1; index < len(wantIDs); index++ { + for index := 0; index < 2; index++ { path := configkit.Keys("items").Index(index).Key("id") info, ok := metadata.Lookup(path) - if !ok || !info.Explicit || len(info.Origin.Locations) != 1 || info.Origin.Locations[0].SourceID != []string{"source-2", "source-3"}[index-1] { + if !ok || !info.Explicit || len(info.Origin.Locations) != 1 || info.Origin.Locations[0].SourceID != []string{"source-2", "source-3"}[index] { t.Errorf("shifted item origin at %s = %#v, %v", path.String(), info, ok) } } diff --git a/internal/engine/import_graph.go b/internal/engine/import_graph.go index 7e3c0f1..f87a8a1 100644 --- a/internal/engine/import_graph.go +++ b/internal/engine/import_graph.go @@ -148,12 +148,12 @@ func (graph importInclusionGraph) scheduleCycle(closingEdge int, location Locati }, true } -func scheduleOrdinaryContributions(scope acquisitionScope, graph importInclusionGraph) acquisitionScope { +func scheduleAcquisitionScope(scope acquisitionScope, graph importInclusionGraph) acquisitionScope { positions := make(map[importNodeKey]int, len(scope.contributions)) slots := make([]int, 0, len(scope.contributions)) keys := make([]dependencyKey, 0, len(scope.contributions)) for index, contribution := range scope.contributions { - if contribution.scopeOwner != scope.owner || (contribution.role != importRoleMain && contribution.role != importRoleMapping) { + if contribution.scopeOwner != scope.owner { continue } positions[contribution.identity] = len(keys) diff --git a/internal/engine/imports.go b/internal/engine/imports.go index c2221cc..f7c6299 100644 --- a/internal/engine/imports.go +++ b/internal/engine/imports.go @@ -220,9 +220,7 @@ func loadImportedWithAccessTrackedAndProcessingPolicy(access *filesystemAccess, } } if len(diagnostics) == 0 { - if !mainIsItem { - state.scope = scheduleOrdinaryContributions(state.scope, state.importGraph) - } + state.scheduleAcquisitionScopes(&state.scope) diagnostics = state.composePendingItems(&state.scope) } @@ -572,7 +570,7 @@ func (s *importTraversal) visitItemSource(canonical string, identity importNodeK itemScope := s.scope s.scope, s.itemScope, s.itemPrefix, s.itemFallback = parentScope, parentItemScope, parentPrefix, parentFallback if len(diagnostics) != 0 { - return projectItemDiagnosticPaths(diagnostics, parentScope, destination) + return diagnostics } itemScope.destination = Path{} s.scope.contributions = append(s.scope.contributions, acquiredContribution{ @@ -584,6 +582,15 @@ func (s *importTraversal) visitItemSource(canonical string, identity importNodeK return nil } +func (s *importTraversal) scheduleAcquisitionScopes(scope *acquisitionScope) { + *scope = scheduleAcquisitionScope(*scope, s.importGraph) + for index := range scope.contributions { + if pending := scope.contributions[index].pendingItem; pending != nil { + s.scheduleAcquisitionScopes(pending) + } + } +} + func (s *importTraversal) composePendingItems(scope *acquisitionScope) []Diagnostic { for index := range scope.contributions { contribution := &scope.contributions[index] @@ -592,7 +599,7 @@ func (s *importTraversal) composePendingItems(scope *acquisitionScope) []Diagnos } itemScope := contribution.pendingItem if diagnostics := s.composePendingItems(itemScope); len(diagnostics) != 0 { - return projectItemDiagnosticPaths(diagnostics, *scope, contribution.destination) + return projectItemDiagnosticPaths(diagnostics, *scope, contribution.destination, index) } documents := make([]*document.Node, 0, len(itemScope.contributions)) for _, nested := range itemScope.contributions { @@ -600,7 +607,7 @@ func (s *importTraversal) composePendingItems(scope *acquisitionScope) []Diagnos } completed, diagnostics := composeDocuments(documents...) if len(diagnostics) != 0 { - return projectItemDiagnosticPaths(diagnostics, *scope, contribution.destination) + return projectItemDiagnosticPaths(diagnostics, *scope, contribution.destination, index) } completed.acquisition = []acquisitionScope{cloneAcquisitionScope(*itemScope)} projectNestedEvidence(completed, *itemScope) @@ -668,10 +675,12 @@ func isCompositionDiagnostic(diagnostic Diagnostic) bool { // 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 { +func projectItemDiagnosticPaths(diagnostics []Diagnostic, parent acquisitionScope, destination Path, beforeContribution int) []Diagnostic { for _, diagnostic := range diagnostics { if isCompositionDiagnostic(diagnostic) { - index := priorSequenceLength([]acquisitionScope{parent}, destination) + prefix := parent + prefix.contributions = parent.contributions[:beforeContribution] + index := priorSequenceLength([]acquisitionScope{prefix}, destination) return prependCompositionDiagnosticPaths(diagnostics, destination.Index(index)) } } diff --git a/internal/engine/imports_test.go b/internal/engine/imports_test.go index b777e52..b506709 100644 --- a/internal/engine/imports_test.go +++ b/internal/engine/imports_test.go @@ -812,8 +812,14 @@ func TestItemAcquisitionRetainsInclusionGraphAndScopeOwnership(t *testing.T) { if rootScope.owner != graph.nodes[0].key || len(rootScope.contributions) != 2 { t.Fatalf("root acquisition scope = %#v", rootScope) } - item := rootScope.contributions[1] - if item.identity != graph.nodes[1].key || item.role != importRoleItem || item.scopeOwner != rootScope.owner || item.nested == nil { + var item *acquiredContribution + for index := range rootScope.contributions { + if rootScope.contributions[index].role == importRoleItem { + item = &rootScope.contributions[index] + break + } + } + if item == nil || item.identity != graph.nodes[1].key || item.scopeOwner != rootScope.owner || item.nested == nil { t.Fatalf("outer item contribution = %#v", item) } itemScope := item.nested.acquisition[0] @@ -831,6 +837,131 @@ func TestItemAcquisitionRetainsInclusionGraphAndScopeOwnership(t *testing.T) { } } +func TestItemContributionsUseDependencyReadySchedules(t *testing.T) { + t.Run("outer independent items", func(t *testing.T) { + type item struct { + ID string `yaml:"id"` + } + type config struct { + Items []item `yaml:"items"` + } + definition := Definition[config]{sequenceRegistrations: []sequenceRegistration{{path: Keys("items"), identityField: "id"}}} + directory := t.TempDir() + mainPath := writeImportFile(t, directory, "main.yml", `configkit: + imports: + - path: z-item.yml + destination: [items] + placement: sequence-item + - path: a-item.yml + destination: [items] + placement: sequence-item +items: [] +`) + zPath := writeImportFile(t, directory, "z-item.yml", "id: z\n") + aPath := writeImportFile(t, directory, "a-item.yml", "id: a\n") + result, err := testLoad[config](File(mainPath), Options[config]{Imports: true, Definition: definition}) + if err != nil { + t.Fatalf("load item imports: %v", err) + } + if len(result.Config.Items) != 2 || result.Config.Items[0].ID != "a" || result.Config.Items[1].ID != "z" { + t.Fatalf("scheduled items = %#v", result.Config.Items) + } + wantSources := []string{canonicalTestPath(t, mainPath), canonicalTestPath(t, zPath), canonicalTestPath(t, aPath)} + for index, want := range wantSources { + if got := result.Sources()[index].Path; got != want { + t.Errorf("acquired source %d = %q, want %q", index, got, want) + } + } + }) + + t.Run("unequal helper branches", func(t *testing.T) { + directory := t.TempDir() + mainPath := writeImportFile(t, directory, "main.yml", `configkit: + imports: + - path: item.yml + destination: [items] + placement: sequence-item +`) + writeImportFile(t, directory, "item.yml", `configkit: + imports: + - path: b-parent.yml + - path: c-independent.yml +id: item +`) + writeImportFile(t, directory, "b-parent.yml", "configkit: {imports: [{path: a-dependency.yml}]}\nid: b\n") + writeImportFile(t, directory, "a-dependency.yml", "id: a\n") + writeImportFile(t, directory, "c-independent.yml", "id: c\n") + loadError := requireLoadCode(t, File(mainPath), Options[importTestConfig]{Imports: true}, CodeComposeConflict) + diagnostic := loadError.Diagnostics()[0] + if diagnostic.Path.String() != "/items/0/id" || diagnostic.Primary == nil || diagnostic.Primary.SourceID != "source-4" { + t.Fatalf("item helper conflict = %#v", diagnostic) + } + wantRelated := map[string]bool{"source-3": false, "source-5": false, "source-2": false} + if len(diagnostic.Related) != len(wantRelated) { + t.Fatalf("related sites = %#v", diagnostic.Related) + } + for _, site := range diagnostic.Related { + if _, expected := wantRelated[site.SourceID]; !expected { + t.Errorf("unexpected related site source %q", site.SourceID) + } + wantRelated[site.SourceID] = true + } + for sourceID, found := range wantRelated { + if !found { + t.Errorf("related sites do not include %q", sourceID) + } + } + }) + + t.Run("selected main item dependencies", func(t *testing.T) { + directory := t.TempDir() + mainPath := writeImportFile(t, directory, "main.yml", "configkit: {imports: [{path: helper.yml}]}\nid: main\n") + writeImportFile(t, directory, "helper.yml", "id: helper\n") + options := Options[importTestConfig]{ + Imports: true, destination: Keys("items"), placementMode: placementModeSequenceItem, + } + loadError := requireLoadCode(t, File(mainPath), options, CodeComposeConflict) + diagnostic := loadError.Diagnostics()[0] + if diagnostic.Path.String() != "/items/0/id" || diagnostic.Primary == nil || diagnostic.Primary.SourceID != "source-2" || len(diagnostic.Related) != 1 || diagnostic.Related[0].SourceID != "source-1" { + t.Fatalf("main item helper conflict = %#v", diagnostic) + } + }) +} + +func TestItemHelperScopeScheduleInterleavesUnequalBranches(t *testing.T) { + directory := t.TempDir() + mainPath := writeImportFile(t, directory, "main.yml", `configkit: + imports: + - path: item.yml + destination: [items] + placement: sequence-item +`) + writeImportFile(t, directory, "item.yml", "configkit: {imports: [{path: b-parent.yml}, {path: c-independent.yml}]}\nitem: true\n") + writeImportFile(t, directory, "b-parent.yml", "configkit: {imports: [{path: a-dependency.yml}]}\nb: true\n") + writeImportFile(t, directory, "a-dependency.yml", "a: true\n") + writeImportFile(t, directory, "c-independent.yml", "c: true\n") + input, diagnostics := assembleBaseInput[importTestConfig](File(mainPath), Options[importTestConfig]{Imports: true}) + if len(diagnostics) != 0 { + t.Fatalf("assemble item scope: %#v", diagnostics) + } + var nested *composedDocument + for _, contribution := range input.composition.acquisition[0].contributions { + if contribution.role == importRoleItem { + nested = contribution.nested + break + } + } + if nested == nil || len(nested.acquisition) != 1 { + t.Fatalf("item composition = %#v", nested) + } + wantIDs := []string{"source-4", "source-3", "source-5", "source-2"} + for index, want := range wantIDs { + if got := nested.acquisition[0].contributions[index].source.ID; got != want { + t.Errorf("local contribution %d = %q, want %q", index, got, want) + } + } +} + func TestLoadImportsCanonicalSymlinksAndKeepsHardLinksDistinct(t *testing.T) { if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { t.Skip("initial filesystem contract covers Linux and macOS")