diff --git a/docs/internal/imports.md b/docs/internal/imports.md index e4fe464..f09681c 100644 --- a/docs/internal/imports.md +++ b/docs/internal/imports.md @@ -72,6 +72,12 @@ tuple comparisons. On a stalled graph, deterministic DFS over the residual graph returns a closed cycle rotated to its smallest key; blocked dependents are not reported as cycle members. +Import edge preparation builds an importer-to-target adjacency index once for +an acquisition's outer and item-local scheduling, and once for grouped +scheduling. Each scope gathers edges from its participating identities and +retains only edges with both endpoints in its local slots. The retained +inclusion graph still owns cycle diagnostics and source relationships. + Import composition schedules a main tier or one underlay group as a single strict-scope graph. Applicable collection declarations are gathered for the scope before peer composition, including identity inherited from lower tiers. diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 3d773a9..6b7b514 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -152,7 +152,7 @@ and race-enabled import/destination tests passed with Go 1.26.10 on Linux. ## 3. Restrict Import-Scope Edge Preparation To Relevant Nodes -Status: open. Priority: medium. Category: algorithmic efficiency and allocation +Status: complete (2026-10-25). Priority: medium. Category: algorithmic efficiency and allocation scaling. **Evidence.** `scheduleAcquisitionScopes` in @@ -190,3 +190,16 @@ coverage in [imports_test.go](../../internal/engine/imports_test.go) and existing destination/provenance evidence tests. Cover broad independent item scopes if a consequential gap remains; verify index construction and edge visitation bounds by source review without new performance gates. + +**Completion.** Scheduling now builds an importer-to-target adjacency index once +for an acquisition and shares it across outer and pending item scopes. Grouped +scheduling builds its own index once. Local preparation visits only adjacency +lists for participating identities, includes only edges with both endpoints in +the local slot map, and allocates edge storage as needed rather than reserving +the whole graph per scope. For the broad sibling family, preparation is O(E + P) +before the unchanged stable Kahn scheduling. The inclusion graph and cycle +diagnostics remain unchanged. Tests cover excluded endpoints/unselected slots +and broad main/underlay item scopes with retained helper origins and projected +destinations. Focused scheduling tests, the development check, documentation +links, and `go test -race ./...` passed with Go 1.26.10 on Linux. No timing or +allocation thresholds were introduced. diff --git a/internal/engine/dependency_integration_test.go b/internal/engine/dependency_integration_test.go index 4e38946..a1e699a 100644 --- a/internal/engine/dependency_integration_test.go +++ b/internal/engine/dependency_integration_test.go @@ -5,6 +5,29 @@ import ( "testing" ) +func TestImportSchedulingKeepsUnselectedSlotsAndScopeBoundaries(t *testing.T) { + dependent := importNodeKey{resourcePath: "a-dependent"} + external := importNodeKey{resourcePath: "external"} + prerequisite := importNodeKey{resourcePath: "z-prerequisite"} + graph := newImportInclusionGraph() + for _, key := range []importNodeKey{dependent, external, prerequisite} { + graph.addNode(importGraphNode{key: key}) + } + graph.addEdge(dependent, prerequisite, Location{}) + graph.addEdge(dependent, external, Location{}) + graph.addEdge(external, prerequisite, Location{}) + contributions := []acquiredContribution{ + {identity: dependent}, {identity: external}, {identity: prerequisite}, + } + scheduled := scheduleContributionSlots(contributions, []int{0, 2}, indexImportDependencies(graph)) + if scheduled[0].identity != prerequisite || scheduled[1].identity != external || scheduled[2].identity != dependent { + t.Fatalf("scope schedule = %#v", scheduled) + } + if contributions[0].identity != dependent || contributions[2].identity != prerequisite { + t.Fatal("scheduling changed the input slots") + } +} + func TestImportAndInheritanceIntegrationsUseEquivalentReadySchedules(t *testing.T) { names := []string{"a-base", "b-independent", "z-dependent"} keys := make(map[string]importNodeKey, len(names)) @@ -33,7 +56,7 @@ func TestImportAndInheritanceIntegrationsUseEquivalentReadySchedules(t *testing. } graph.addEdge(keys["z-dependent"], keys["a-base"], Location{}) - scheduled := scheduleContributionSlots(contributions, []int{0, 1, 2}, graph) + scheduled := scheduleContributionSlots(contributions, []int{0, 1, 2}, indexImportDependencies(graph)) got := make([]string, len(scheduled)) for index, contribution := range scheduled { got[index] = contribution.identity.resourcePath[1:] diff --git a/internal/engine/import_acquisition.go b/internal/engine/import_acquisition.go index c6a6965..eaab38b 100644 --- a/internal/engine/import_acquisition.go +++ b/internal/engine/import_acquisition.go @@ -178,7 +178,7 @@ func acquireImportedWithAccess(request importAcquisitionRequest, policy *acquisi } } if len(diagnostics) == 0 { - state.scheduleAcquisitionScopes(&state.scope) + scheduleAcquisitionScopes(&state.scope, indexImportDependencies(state.importGraph)) } closeErr := access.close() diff --git a/internal/engine/import_graph.go b/internal/engine/import_graph.go index 9922d0f..590e355 100644 --- a/internal/engine/import_graph.go +++ b/internal/engine/import_graph.go @@ -157,7 +157,20 @@ func (graph importInclusionGraph) scheduleCycle(closingEdge int, location Locati }, true } -func scheduleAcquisitionScope(scope acquisitionScope, graph importInclusionGraph) acquisitionScope { +// importDependencyIndex is built once per scheduling operation and shared by +// its scopes. Inclusion targets are grouped by importer, so local scheduling +// need not scan or reserve storage for unrelated graph edges. +type importDependencyIndex map[importNodeKey][]importNodeKey + +func indexImportDependencies(graph importInclusionGraph) importDependencyIndex { + index := make(importDependencyIndex) + for _, edge := range graph.edges { + index[edge.importer] = append(index[edge.importer], edge.imported) + } + return index +} + +func scheduleAcquisitionScope(scope acquisitionScope, dependencies importDependencyIndex) acquisitionScope { slots := make([]int, 0, len(scope.contributions)) for index, contribution := range scope.contributions { if contribution.scopeOwner != scope.owner { @@ -166,7 +179,7 @@ func scheduleAcquisitionScope(scope acquisitionScope, graph importInclusionGraph slots = append(slots, index) } scheduled := scope - scheduled.contributions = scheduleContributionSlots(scope.contributions, slots, graph) + scheduled.contributions = scheduleContributionSlots(scope.contributions, slots, dependencies) return scheduled } @@ -179,10 +192,10 @@ func scheduleAcquisitionGroup(scopes []acquisitionScope, graph importInclusionGr for index := range slots { slots[index] = index } - return acquisitionScope{contributions: scheduleContributionSlots(contributions, slots, graph)} + return acquisitionScope{contributions: scheduleContributionSlots(contributions, slots, indexImportDependencies(graph))} } -func scheduleContributionSlots(contributions []acquiredContribution, slots []int, graph importInclusionGraph) []acquiredContribution { +func scheduleContributionSlots(contributions []acquiredContribution, slots []int, dependencies importDependencyIndex) []acquiredContribution { if len(slots) < 2 { return append([]acquiredContribution(nil), contributions...) } @@ -193,12 +206,12 @@ func scheduleContributionSlots(contributions []acquiredContribution, slots []int positions[contribution.identity] = position keys[position] = contribution.identity.dependencyKey() } - edges := make([]dependencyEdge, 0, len(graph.edges)) - for _, edge := range graph.edges { - importer, importerOK := positions[edge.importer] - imported, importedOK := positions[edge.imported] - if importerOK && importedOK { - edges = append(edges, dependencyEdge{prerequisite: imported, dependent: importer}) + var edges []dependencyEdge + for importerKey, importer := range positions { + for _, importedKey := range dependencies[importerKey] { + if imported, exists := positions[importedKey]; exists { + edges = append(edges, dependencyEdge{prerequisite: imported, dependent: importer}) + } } } order, _ := scheduleDependencies(keys, edges) diff --git a/internal/engine/imports.go b/internal/engine/imports.go index bc10df2..6edfab4 100644 --- a/internal/engine/imports.go +++ b/internal/engine/imports.go @@ -53,11 +53,11 @@ type importTraversal struct { importGraph importInclusionGraph } -func (s *importTraversal) scheduleAcquisitionScopes(scope *acquisitionScope) { - *scope = scheduleAcquisitionScope(*scope, s.importGraph) +func scheduleAcquisitionScopes(scope *acquisitionScope, dependencies importDependencyIndex) { + *scope = scheduleAcquisitionScope(*scope, dependencies) for index := range scope.contributions { if pending := scope.contributions[index].pendingItem; pending != nil { - s.scheduleAcquisitionScopes(pending) + scheduleAcquisitionScopes(pending, dependencies) } } } diff --git a/internal/engine/imports_test.go b/internal/engine/imports_test.go index 9e08fe5..69710bd 100644 --- a/internal/engine/imports_test.go +++ b/internal/engine/imports_test.go @@ -1107,6 +1107,70 @@ id: item }) } +func TestBroadItemScopesKeepHelperOriginsAndDestinations(t *testing.T) { + t.Parallel() + + type item struct { + ID string `yaml:"id"` + Value string `yaml:"value"` + } + type config struct { + Items []item `yaml:"items"` + } + directory := t.TempDir() + const count = 32 + var main strings.Builder + main.WriteString("configkit:\n imports:\n") + for index := count - 1; index >= 0; index-- { + name := strconv.Itoa(100 + index) + main.WriteString(" - path: item-" + name + ".yml\n destination: [items]\n placement: sequence-item\n") + writeImportFile(t, directory, "item-"+name+".yml", "configkit: {imports: [{path: helper-"+name+".yml}]}\nid: item-"+name+"\n") + writeImportFile(t, directory, "helper-"+name+".yml", "value: helper-"+name+"\n") + } + mainPath := writeImportFile(t, directory, "main.yml", main.String()) + definition := Definition[config]{sequenceIdentityRegistrations: []sequenceIdentityRegistration{{ + selector: sequenceIdentitySelector{literalSequenceSelector("items")}, identityFields: []string{"id"}, + }}} + for _, layered := range []bool{false, true} { + t.Run(strconv.FormatBool(layered), func(t *testing.T) { + var loaded *Loaded[config] + var err error + if layered { + loaded, err = LoadLayered(Input{Source: NoSource()}, testSecretBinding(), LayeredOptions[config]{ + Definition: definition, + Underlays: []Underlay{{Inputs: []Input{{Source: File(mainPath), Imports: true}}}}, + }) + } else { + loaded, err = Load(File(mainPath), testSecretBinding(), Options[config]{Imports: true, Definition: definition}) + } + if err != nil { + t.Fatal(err) + } + result := testLoaded(loaded) + sources := result.Sources() + if len(result.Config.Items) != count || len(sources) != 2*count+1 { + t.Fatalf("item/source counts = %d/%d", len(result.Config.Items), len(sources)) + } + metadata := result.Metadata() + for index, record := range result.Config.Items { + name := strconv.Itoa(100 + index) + if record.ID != "item-"+name || record.Value != "helper-"+name { + t.Fatalf("item %d = %#v", index, record) + } + // Acquisition remains authored (descending); final item order is + // dependency-ready (ascending), with helper evidence reprojected. + helper := sources[2*(count-index)] + field, found := metadata.Lookup(Keys("items").Index(index).Key("value")) + if !found || len(field.Origin.Locations) != 1 || field.Origin.Locations[0].SourceID != helper.ID || + helper.Path != canonicalTestPath(t, filepath.Join(directory, "helper-"+name+".yml")) || + helper.Destination.String() != Keys("items").Index(index).String() { + t.Fatalf("item %d helper evidence = %#v, %#v", index, field, helper) + } + } + }) + } +} + func TestItemHelperScopeScheduleInterleavesUnequalBranches(t *testing.T) { directory := t.TempDir() mainPath := writeImportFile(t, directory, "main.yml", `configkit: