Keep import selection evidence with its declaring source

This commit is contained in:
2026-10-25 14:36:39 +00:00
parent e585decdb2
commit 64be8b4df5
5 changed files with 79 additions and 28 deletions

View File

@@ -111,7 +111,7 @@ input restrictions, performance thresholds, or release gates.
## 2. Avoid Searching All Sources For Each Empty Import Selection ## 2. Avoid Searching All Sources For Each Empty Import Selection
Status: open. Priority: low. Category: algorithmic efficiency. Status: complete (2026-10-25). Priority: low. Category: algorithmic efficiency.
**Evidence.** `walkDeclarations` calls `recordEmptyImportSelection` for each **Evidence.** `walkDeclarations` calls `recordEmptyImportSelection` for each
optional declaration that admits no source. In optional declaration that admits no source. In
@@ -141,6 +141,15 @@ A broad-fragment behavioral case is useful if existing coverage does not protect
the chosen index handoff. Verify the removed scan by source review rather than the chosen index handoff. Verify the removed scan by source review rather than
adding timing thresholds or assertions about private lookup counts. adding timing thresholds or assertions about private lookup counts.
**Completion.** Declaration traversal now receives the declaring source's stable
integer position, captured at admission and carried through mapping and item
imports. Empty selections append evidence directly to that record without
searching the source slice or retaining pointers into it. The broad-fragment
test covers source-slice growth, missing-source rollback, and parent evidence
after child traversal; existing detached evidence, nested item, underlay, and
partial-failure tests remain effective. Focused tests, the development check,
and race-enabled import/destination tests passed with Go 1.26.10 on Linux.
## 3. Restrict Import-Scope Edge Preparation To Relevant Nodes ## 3. Restrict Import-Scope Edge Preparation To Relevant Nodes
Status: open. Priority: medium. Category: algorithmic efficiency and allocation Status: open. Priority: medium. Category: algorithmic efficiency and allocation

View File

@@ -108,13 +108,14 @@ parent: true
} }
defer func() { _ = access.close() }() defer func() { _ = access.close() }()
state := importTraversal{ state := importTraversal{
access: access, access: access,
policy: &acquisitionPolicy{importLimits: defaultImportLimits()}, policy: &acquisitionPolicy{importLimits: defaultImportLimits()},
state: &acquisitionState{accounting: &importAccounting{}}, state: &acquisitionState{accounting: &importAccounting{sources: 1}},
tier: &tierAcquisitionState{visited: make(map[resourceIdentity]importRoute)}, tier: &tierAcquisitionState{visited: make(map[resourceIdentity]importRoute)},
active: make(map[resourceIdentity]int), sources: sourceInfos(main.source),
active: make(map[resourceIdentity]int),
} }
if diagnostics := state.walkDeclarations(directory, declarations, 0, Keys("entry")); len(diagnostics) != 0 { if diagnostics := state.walkDeclarations(directory, declarations, 0, Keys("entry"), 0); len(diagnostics) != 0 {
t.Fatalf("walk diagnostics = %#v", diagnostics) t.Fatalf("walk diagnostics = %#v", diagnostics)
} }
@@ -128,11 +129,11 @@ parent: true
{path: alphaPath, destination: "/entry/collection"}, {path: alphaPath, destination: "/entry/collection"},
{path: betaPath, destination: "/entry/collection"}, {path: betaPath, destination: "/entry/collection"},
} }
if len(state.sources) != len(wantSources) { if len(state.sources) != len(wantSources)+1 {
t.Fatalf("import sources = %#v, want %d", state.sources, len(wantSources)) t.Fatalf("sources = %#v, want %d including main", state.sources, len(wantSources)+1)
} }
for index, want := range wantSources { for index, want := range wantSources {
got := state.sources[index] got := state.sources[index+1]
if got.Path != canonicalTestPath(t, want.path) || got.Destination.String() != want.destination { if got.Path != canonicalTestPath(t, want.path) || got.Destination.String() != want.destination {
t.Errorf("source %d = (%q, %q), want (%q, %q)", index, got.Path, got.Destination.String(), canonicalTestPath(t, want.path), want.destination) t.Errorf("source %d = (%q, %q), want (%q, %q)", index, got.Path, got.Destination.String(), canonicalTestPath(t, want.path), want.destination)
} }

View File

@@ -22,6 +22,7 @@ type admittedImport struct {
identity resourceIdentity identity resourceIdentity
nodeKey importNodeKey nodeKey importNodeKey
source SourceInfo source SourceInfo
sourceIndex int
payload *document.Node payload *document.Node
declarations []importDeclarationSpec declarations []importDeclarationSpec
sequenceDeclarations []sequenceDeclarationSpec sequenceDeclarations []sequenceDeclarationSpec
@@ -156,7 +157,7 @@ func acquireImportedWithAccess(request importAcquisitionRequest, policy *acquisi
scopeOwner: mainNodeKey, payload: body, document: body, processing: processing, scopeOwner: mainNodeKey, payload: body, document: body, processing: processing,
sequenceDeclarations: cloneSequenceDeclarations(sequenceDeclarations), sequenceDeclarations: cloneSequenceDeclarations(sequenceDeclarations),
}) })
diagnostics = state.walkDeclarations(basePath, declarations, 0, Path{}) diagnostics = state.walkDeclarations(basePath, declarations, 0, Path{}, 0)
} else { } else {
_, placed, err := placeDocument(body, destination, body.Location, maxDocumentDepth, maxDocumentNodes) _, placed, err := placeDocument(body, destination, body.Location, maxDocumentDepth, maxDocumentNodes)
placementNodes := destinationWrapperNodeCount(destination) placementNodes := destinationWrapperNodeCount(destination)
@@ -172,7 +173,7 @@ func acquireImportedWithAccess(request importAcquisitionRequest, policy *acquisi
destination: destination.clone(), processing: processing, destination: destination.clone(), processing: processing,
sequenceDeclarations: cloneSequenceDeclarations(sequenceDeclarations), sequenceDeclarations: cloneSequenceDeclarations(sequenceDeclarations),
}) })
diagnostics = state.walkDeclarations(basePath, declarations, 0, destination) diagnostics = state.walkDeclarations(basePath, declarations, 0, destination, 0)
} }
} }
} }
@@ -239,7 +240,7 @@ func (s *importTraversal) checkInitialLimits() *Diagnostic {
return nil return nil
} }
func (s *importTraversal) walkDeclarations(basePath string, declarations []importDeclarationSpec, depth int, parentDestination Path) []Diagnostic { func (s *importTraversal) walkDeclarations(basePath string, declarations []importDeclarationSpec, depth int, parentDestination Path, sourceIndex int) []Diagnostic {
policies := make([]resolvedImportPolicy, len(declarations)) policies := make([]resolvedImportPolicy, len(declarations))
for index, spec := range declarations { for index, spec := range declarations {
policy, diagnostics := resolveImportPolicy(spec, s.policy.importDefaults) policy, diagnostics := resolveImportPolicy(spec, s.policy.importDefaults)
@@ -280,23 +281,19 @@ func (s *importTraversal) walkDeclarations(basePath string, declarations []impor
} }
} }
if !policy.required && len(s.sources) == sourcesBefore { if !policy.required && len(s.sources) == sourcesBefore {
s.recordEmptyImportSelection(spec) s.recordEmptyImportSelection(sourceIndex, spec)
} }
} }
return nil return nil
} }
func (s *importTraversal) recordEmptyImportSelection(spec importDeclarationSpec) { func (s *importTraversal) recordEmptyImportSelection(sourceIndex int, spec importDeclarationSpec) {
for index := range s.sources { // The declaring source's position survives descendant appends and optional
if s.sources[index].ID != spec.location.SourceID { // missing-source rollback, unlike a pointer into the source slice.
continue s.sources[sourceIndex].importSelections = append(s.sources[sourceIndex].importSelections, importSelectionEvidence{
} path: spec.path.clone(), location: spec.location,
s.sources[index].importSelections = append(s.sources[index].importSelections, importSelectionEvidence{ selector: spec.selector, outcome: importSelectionNone,
path: spec.path.clone(), location: spec.location, })
selector: spec.selector, outcome: importSelectionNone,
})
return
}
} }
func (s *importTraversal) admitImportSource(canonical string, identity resourceIdentity, nodeKey importNodeKey, candidate selectedImportCandidate, policy resolvedImportPolicy, destination Path, location Location) (importAdmissionResult, []Diagnostic) { func (s *importTraversal) admitImportSource(canonical string, identity resourceIdentity, nodeKey importNodeKey, candidate selectedImportCandidate, policy resolvedImportPolicy, destination Path, location Location) (importAdmissionResult, []Diagnostic) {
@@ -321,6 +318,7 @@ func (s *importTraversal) admitImportSource(canonical string, identity resourceI
if kind == SourceKindFilesystem { if kind == SourceKindFilesystem {
info.namespace = s.access.token info.namespace = s.access.token
} }
sourceIndex := len(s.sources)
s.sources = append(s.sources, info) s.sources = append(s.sources, info)
data, skipped, diagnostic := s.readImport(canonical, sourceID, policy.required) data, skipped, diagnostic := s.readImport(canonical, sourceID, policy.required)
if diagnostic != nil { if diagnostic != nil {
@@ -369,7 +367,7 @@ func (s *importTraversal) admitImportSource(canonical string, identity resourceI
} }
return importAdmissionResult{source: admittedImport{ return importAdmissionResult{source: admittedImport{
canonical: canonical, identity: identity, nodeKey: nodeKey, canonical: canonical, identity: identity, nodeKey: nodeKey,
source: cloneSourceInfo(info), payload: processing.payload, source: cloneSourceInfo(info), sourceIndex: sourceIndex, payload: processing.payload,
declarations: processing.imports, declarations: processing.imports,
sequenceDeclarations: cloneSequenceDeclarations(processing.sequences), sequenceDeclarations: cloneSequenceDeclarations(processing.sequences),
processing: &processing, processing: &processing,
@@ -477,7 +475,7 @@ func (s *importTraversal) visitPath(candidate selectedImportCandidate, spec impo
s.active[admitted.identity] = len(s.stack) s.active[admitted.identity] = len(s.stack)
s.tier.visited[admitted.identity] = route s.tier.visited[admitted.identity] = route
s.stack = append(s.stack, route) s.stack = append(s.stack, route)
diagnostics = s.walkDeclarations(s.access.dir(admitted.canonical), admitted.declarations, depth, destination) diagnostics = s.walkDeclarations(s.access.dir(admitted.canonical), admitted.declarations, depth, destination, admitted.sourceIndex)
delete(s.active, admitted.identity) delete(s.active, admitted.identity)
s.stack = s.stack[:len(s.stack)-1] s.stack = s.stack[:len(s.stack)-1]
return diagnostics return diagnostics

View File

@@ -2,11 +2,54 @@ package engine
import ( import (
"errors" "errors"
"fmt"
"path/filepath" "path/filepath"
"reflect" "reflect"
"testing" "testing"
) )
func TestBroadEmptyImportSelectionsKeepDeclaringSourceOwnership(t *testing.T) {
t.Parallel()
directory := t.TempDir()
mainPath := writeImportFile(t, directory, "main.yml", `configkit:
imports:
- glob: fragments/*.yml
- path: []
`)
const fragments = 32
for index := 0; index < fragments; index++ {
writeImportFile(t, directory, fmt.Sprintf("fragments/fragment-%02d.yml", index), `configkit:
imports:
- path: missing.yml
- path: []
`)
}
loaded, err := testLoad[map[string]any](File(mainPath), Options[map[string]any]{Imports: true})
if err != nil {
t.Fatal(err)
}
sources := loaded.Sources()
if len(sources) != fragments+1 {
t.Fatalf("source count = %d, want %d", len(sources), fragments+1)
}
for index, source := range sources {
wantPaths := []string{"/configkit/imports/0", "/configkit/imports/1"}
if index == 0 {
wantPaths = wantPaths[1:]
}
if len(source.importSelections) != len(wantPaths) {
t.Fatalf("source %s selections = %#v", source.ID, source.importSelections)
}
for selectionIndex, selection := range source.importSelections {
if selection.path.String() != wantPaths[selectionIndex] || selection.location.SourceID != source.ID ||
selection.selector != importSelectorPath || selection.outcome != importSelectionNone {
t.Errorf("source %s selection %d = %#v", source.ID, selectionIndex, selection)
}
}
}
}
func TestOptionalEmptyImportSelectionsRemainSourceLocalAndDetached(t *testing.T) { func TestOptionalEmptyImportSelectionsRemainSourceLocalAndDetached(t *testing.T) {
t.Parallel() t.Parallel()

View File

@@ -184,7 +184,7 @@ func (s *importTraversal) visitItemSource(admitted admittedImport, destination,
identity: admitted.nodeKey, role: importRoleItem, scopeOwner: admitted.nodeKey, identity: admitted.nodeKey, role: importRoleItem, scopeOwner: admitted.nodeKey,
processing: admitted.processing, sequenceDeclarations: cloneSequenceDeclarations(admitted.sequenceDeclarations), processing: admitted.processing, sequenceDeclarations: cloneSequenceDeclarations(admitted.sequenceDeclarations),
}) })
diagnostics := s.walkDeclarations(s.access.dir(admitted.canonical), admitted.declarations, depth, Path{}) diagnostics := s.walkDeclarations(s.access.dir(admitted.canonical), admitted.declarations, depth, Path{}, admitted.sourceIndex)
itemScope := s.scope itemScope := s.scope
s.scope, s.itemScope, s.itemPrefix, s.itemFallback = parentScope, parentItemScope, parentPrefix, parentFallback s.scope, s.itemScope, s.itemPrefix, s.itemFallback = parentScope, parentItemScope, parentPrefix, parentFallback
if len(diagnostics) != 0 { if len(diagnostics) != 0 {