From 5c5f2af6cc46ad2f03178866076face77e2924e4 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 24 Oct 2026 14:10:44 +0000 Subject: [PATCH] Limit inheritance policy work to the relevant scope --- docs/roadmap/remediation.md | 11 +- internal/engine/inheritance.go | 21 +-- internal/engine/inheritance_policy_index.go | 123 ++++++++++++++++++ .../engine/inheritance_policy_index_test.go | 111 ++++++++++++++++ internal/engine/inheritance_schedule.go | 8 +- internal/engine/inheritance_scope.go | 3 +- internal/engine/inheritance_walk.go | 24 +--- 7 files changed, 258 insertions(+), 43 deletions(-) create mode 100644 internal/engine/inheritance_policy_index.go create mode 100644 internal/engine/inheritance_policy_index_test.go diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index b0bc9f3..40da092 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -847,7 +847,16 @@ do not change override semantics or impose batch limits. ## 21. Localize Inheritance Policy Work To The Relevant Scope — P2 -Status: [ ] Open. +Status: [x] Complete. + +Completion: inheritance now keeps a per-load semantic path index, including +separate indexes for record-owned scopes. Walker handoff and descendant +retirement touch the changed scope; mapping and identified-record preparation +reuse indexes across sibling queries. Only relevant policies are cloned into +independent templates. Added wide independent YAML groups, pending-policy +replacement, literal-key/index distinctions, and ownership regressions. Existing +nested item/inheritance, mode-lifetime, and application-constraint coverage, +`./scripts/check.sh`, and `go test -race ./internal/engine` pass. Owners: `inheritancePolicyScopeAtPath` in [`inheritance.go`](../../internal/engine/inheritance.go), graph preparation in diff --git a/internal/engine/inheritance.go b/internal/engine/inheritance.go index 4951999..8436193 100644 --- a/internal/engine/inheritance.go +++ b/internal/engine/inheritance.go @@ -114,26 +114,7 @@ func cloneInheritanceOperations(operations []inheritanceOperation) []inheritance } func inheritancePolicyScopeAtPath(scope collectionPolicyScope, prefix Path) collectionPolicyScope { - localized := collectionPolicyScope{} - for _, entry := range scope.collections { - if relative, inside := relativeHistoryPath(prefix, entry.destination); inside { - localized.set(relative, cloneCollectionPolicyForInheritance(entry.policy)) - continue - } - relative, inside := relativeHistoryPath(entry.destination, prefix) - if !inside || len(relative.segments) == 0 || relative.segments[0].kind != pathSegmentIndex { - continue - } - record, found := entry.policy.recordPolicyAt(relative.segments[0].index) - if !found { - continue - } - remainder := Path{segments: append([]pathSegment(nil), relative.segments[1:]...)} - for _, nested := range inheritancePolicyScopeAtPath(record.scope, remainder).collections { - localized.set(nested.destination, cloneCollectionPolicyForInheritance(nested.policy)) - } - } - return localized + return newInheritancePolicyIndex(scope).localize(prefix) } func projectInheritancePolicyTemplates(templates []collectionPolicyScope, prefix Path) []projectedCollectionPolicy { diff --git a/internal/engine/inheritance_policy_index.go b/internal/engine/inheritance_policy_index.go new file mode 100644 index 0000000..d80a0d4 --- /dev/null +++ b/internal/engine/inheritance_policy_index.go @@ -0,0 +1,123 @@ +package engine + +// inheritancePolicyIndex borrows immutable policy entries. Semantic path +// segments keep mapping keys distinct from sequence indexes. Record-owned +// scopes have their own indexes, just as they have their own policy tables. +type inheritancePolicyIndex struct { + entry *collectionPolicyEntry + children map[pathSegment]*inheritancePolicyIndex + records map[int]*inheritancePolicyIndex +} + +func newInheritancePolicyIndex(scope collectionPolicyScope) *inheritancePolicyIndex { + index := &inheritancePolicyIndex{} + for _, entry := range scope.collections { + index.set(entry) + } + return index +} + +func (index *inheritancePolicyIndex) set(entry collectionPolicyEntry) { + node := index + for _, segment := range entry.destination.segments { + if node.children == nil { + node.children = make(map[pathSegment]*inheritancePolicyIndex) + } + if node.children[segment] == nil { + node.children[segment] = &inheritancePolicyIndex{} + } + node = node.children[segment] + } + node.entry = &entry + node.records = nil + for position, owner := range entry.policy.recordOrder { + scope := entry.policy.recordPolicies[owner] + if len(scope.collections) == 0 { + continue + } + if node.records == nil { + node.records = make(map[int]*inheritancePolicyIndex) + } + node.records[position] = newInheritancePolicyIndex(scope) + } +} + +// localize copies only policies in the requested scope. Earlier record-owned +// policies are considered before direct entries added by destination expansion. +func (index *inheritancePolicyIndex) localize(prefix Path) collectionPolicyScope { + localized := collectionPolicyScope{} + node := index + for offset, segment := range prefix.segments { + if node == nil { + return localized + } + if segment.kind == pathSegmentIndex { + if record := node.records[segment.index]; record != nil { + remainder := Path{segments: prefix.segments[offset+1:]} + for _, entry := range record.localize(remainder).collections { + localized.set(entry.destination, entry.policy) + } + } + } + node = node.children[segment] + } + var collect func(*inheritancePolicyIndex) + collect = func(node *inheritancePolicyIndex) { + if node == nil { + return + } + if node.entry != nil { + relative, _ := relativeHistoryPath(prefix, node.entry.destination) + localized.set(relative, cloneCollectionPolicyForInheritance(node.entry.policy)) + } + for _, child := range node.children { + collect(child) + } + } + collect(node) + return localized +} + +func (index *inheritancePolicyIndex) absoluteScope(prefix Path) collectionPolicyScope { + scope := collectionPolicyScope{} + for _, entry := range index.localize(prefix).collections { + scope.set(appendDestination(prefix, entry.destination), entry.policy) + } + return scope +} + +// replace updates the walker's owned table and its borrowed index together. +// Retirement visits only direct entries below the changed destination. +func (index *inheritancePolicyIndex) replace(scope *collectionPolicyScope, prefix Path, replacement collectionPolicyScope) { + node := index + var parent *inheritancePolicyIndex + var last pathSegment + for _, segment := range prefix.segments { + if node == nil { + break + } + parent, last, node = node, segment, node.children[segment] + } + var remove func(*inheritancePolicyIndex) + remove = func(node *inheritancePolicyIndex) { + if node == nil { + return + } + if node.entry != nil { + delete(scope.collections, node.entry.destination.String()) + } + for _, child := range node.children { + remove(child) + } + } + remove(node) + if parent == nil { + *index = inheritancePolicyIndex{} + } else { + delete(parent.children, last) + } + for _, entry := range replacement.collections { + scope.set(entry.destination, entry.policy) + index.set(scope.collections[entry.destination.String()]) + } +} diff --git a/internal/engine/inheritance_policy_index_test.go b/internal/engine/inheritance_policy_index_test.go new file mode 100644 index 0000000..da6fd57 --- /dev/null +++ b/internal/engine/inheritance_policy_index_test.go @@ -0,0 +1,111 @@ +package engine + +import ( + "fmt" + "strings" + "testing" +) + +func TestInheritancePolicyIndexKeepsSemanticScopesAndOwnedTemplates(t *testing.T) { + policy := collectionPolicy{identityFields: []string{"id"}, identityPresent: true} + nested := collectionPolicyScope{} + nested.set(Keys("group", "base", "items"), policy) + outer := policy + record := newCollectionRecordPolicy() + record.scope = nested + outer.setRecordPolicyAt(0, record) + scope := collectionPolicyScope{} + scope.set(Keys("records"), outer) + scope.set(Keys("numeric", "0", "items"), policy) + index := newInheritancePolicyIndex(scope) + + first := index.localize(Keys("records").Index(0).Key("group")) + second := index.localize(Keys("records").Index(0).Key("group")) + if len(first.collections) != 1 || len(second.collections) != 1 { + t.Fatal("record-owned scope was not localized") + } + entry := first.collections["/base/items"] + entry.policy.identityFields[0] = "changed" + if second.collections["/base/items"].policy.identityFields[0] != "id" || nested.collections["/group/base/items"].policy.identityFields[0] != "id" { + t.Fatal("localized policy shares mutable template state") + } + if len(index.localize(Keys("records", "0")).collections) != 0 || len(index.localize(Keys("numeric").Index(0)).collections) != 0 { + t.Fatal("mapping keys and sequence indexes were conflated") + } + if len(index.localize(Keys("numeric", "0")).collections) != 1 { + t.Fatal("numeric mapping key did not retain its policy") + } + left := index.localize(Keys("records")) + right := index.localize(Keys("records")) + leftOwner := left.collections[""].policy.recordOrder[0] + rightOwner := right.collections[""].policy.recordOrder[0] + if leftOwner == rightOwner || leftOwner == record.owner || rightOwner == record.owner { + t.Fatal("independent templates share a record owner") + } +} + +func TestInheritancePolicyIndexReplacesOnlyTheChangedScope(t *testing.T) { + scope := collectionPolicyScope{} + policy := collectionPolicy{explicitMode: sequenceModeAppend, explicitModePresent: true} + for index := range 128 { + scope.set(Keys(fmt.Sprintf("group-%03d", index), "pending"), policy) + } + index := newInheritancePolicyIndex(scope) + changed := Keys("group-064") + if len(index.absoluteScope(changed).collections) != 1 { + t.Fatal("scope includes unrelated groups") + } + replacement := collectionPolicyScope{} + replacement.set(changed.Key("introduced"), policy) + index.replace(&scope, changed, replacement) + if len(scope.collections) != 128 { + t.Fatal("replacement lost unrelated pending declarations") + } + local := index.localize(changed) + if _, found := local.collections["/introduced"]; !found || len(local.collections) != 1 { + t.Fatal("retired policy survived or introduced policy was not indexed") + } + index.replace(&scope, changed, collectionPolicyScope{}) + if len(index.localize(changed).collections) != 0 || len(scope.collections) != 127 { + t.Fatal("descendant retirement did not update the table and index together") + } + if len(index.localize(Keys("group-065")).collections) != 1 { + t.Fatal("retirement changed another scope") + } +} + +func TestInheritanceWideIndependentYamlCollectionScopes(t *testing.T) { + var body strings.Builder + body.WriteString("configkit:\n sequences:\n") + for index := range 64 { + fmt.Fprintf(&body, " - {destination: [groups, group-%03d, base, items], key: id, mode: append}\n", index) + } + body.WriteString("groups:\n") + for index := range 64 { + fmt.Fprintf(&body, " group-%03d:\n base: {items: [{id: shared, value: parent}]}\n child: {extends: base, items: [{id: shared, value: child}]}\n", index) + } + type item struct { + ID string `yaml:"id"` + Value string `yaml:"value"` + } + type definition struct { + Items []item `yaml:"items"` + } + type config struct { + Groups map[string]map[string]definition `yaml:"groups"` + } + loaded, err := testLoad[config](Bytes("wide-inheritance", []byte(body.String()))) + if err != nil { + t.Fatal(err) + } + metadata := loaded.Metadata() + for name, group := range loaded.Config.Groups { + if len(group["base"].Items) != 1 || group["base"].Items[0].Value != "parent" || len(group["child"].Items) != 1 || group["child"].Items[0].Value != "child" { + t.Fatalf("independent scope %s composed incorrectly: %#v", name, group) + } + path := Keys("groups", name, "child", "items").Index(0).Key("value") + if info, exists := metadata.Lookup(path); !exists || !info.Explicit || info.Origin.Layer != LayerBase { + t.Fatalf("inherited destination lost source metadata at %s", path) + } + } +} diff --git a/internal/engine/inheritance_schedule.go b/internal/engine/inheritance_schedule.go index a7460ac..a8f3c18 100644 --- a/internal/engine/inheritance_schedule.go +++ b/internal/engine/inheritance_schedule.go @@ -47,9 +47,10 @@ func analyzeInheritanceGraphWithCollectionRules(scope *document.Node, scopePath }} } + policyIndex := newInheritancePolicyIndex(policies) graph := &inheritanceGraph{ scope: scope, scopePath: scopePath, - collectionPolicies: cloneCollectionPolicyScopeForInheritance(policies), applicationPolicy: applicationPolicy, + collectionPolicies: policyIndex.absoluteScope(scopePath), applicationPolicy: applicationPolicy, byName: make(map[string]int, len(scope.Mapping)), } entries := make(map[string]document.Entry, len(scope.Mapping)) @@ -69,7 +70,7 @@ func analyzeInheritanceGraphWithCollectionRules(scope *document.Node, scopePath graph.byName[name] = index graph.nodes = append(graph.nodes, inheritanceTemplate{ name: name, entry: entries[name], path: scopePath.Key(name), - collectionPolicies: inheritancePolicyScopeAtPath(policies, scopePath.Key(name)), + collectionPolicies: policyIndex.localize(scopePath.Key(name)), }) } @@ -355,6 +356,7 @@ func applyInheritanceScopeTemplates(adapter inheritanceScopeAdapter, order []int root := adapter.rebuild(resolved) result.collectionPolicies = collectionPolicyScope{} result.origins = directInheritanceOrigins(&document.Node{Kind: root.Kind, Location: root.Location}, adapter.scopePath(), inputOrigins) + policyIndex := newInheritancePolicyIndex(adapter.collectionPolicies()) for _, slot := range adapter.slots() { if slot.hasTemplate { index := slot.template @@ -364,7 +366,7 @@ func applyInheritanceScopeTemplates(adapter inheritanceScopeAdapter, order []int } continue } - localPolicies := inheritancePolicyScopeAtPath(adapter.collectionPolicies(), slot.path) + localPolicies := policyIndex.localize(slot.path) projectInheritancePolicyRoots(&result.collectionPolicies, localPolicies, slot.path) for pointer, origin := range directInheritanceOrigins(slot.value, slot.path, inputOrigins) { result.origins[pointer] = origin diff --git a/internal/engine/inheritance_scope.go b/internal/engine/inheritance_scope.go index 3d535c5..2597925 100644 --- a/internal/engine/inheritance_scope.go +++ b/internal/engine/inheritance_scope.go @@ -109,10 +109,11 @@ func (scope identifiedSequenceInheritanceScope) applicationPolicy() sequencePoli func (scope identifiedSequenceInheritanceScope) slotOverhead() int { return 0 } func (scope identifiedSequenceInheritanceScope) templates() []inheritanceExecutionTemplate { result := make([]inheritanceExecutionTemplate, len(scope.graph.nodes)) + policyIndex := newInheritancePolicyIndex(scope.policies) for index, node := range scope.graph.nodes { result[index] = inheritanceExecutionTemplate{ entry: document.Entry{Value: node.item}, path: node.path, - collectionPolicies: inheritancePolicyScopeAtPath(scope.policies, node.path), + collectionPolicies: policyIndex.localize(node.path), parent: node.parent, hasParent: node.hasParent, hasDirective: node.directive != nil, cancelled: node.cancelled, directive: document.Entry{Value: node.directive}, directivePath: node.directivePath, diff --git a/internal/engine/inheritance_walk.go b/internal/engine/inheritance_walk.go index 312e56b..cf7fa13 100644 --- a/internal/engine/inheritance_walk.go +++ b/internal/engine/inheritance_walk.go @@ -11,6 +11,7 @@ type inheritanceWalker struct { nodes int // logical non-directive nodes in the entire working tree origins map[string]Origin collectionPolicies collectionPolicyScope + policyIndex *inheritancePolicyIndex applicationPolicy sequencePolicy limits projectionLimits retained *retainedNodeCounter @@ -66,6 +67,7 @@ func expandInheritanceWithCollectionRuleBudgets(input *document.Node, inputOrigi applicationPolicy: applicationPolicy, retained: newRetainedNodeCounter(historyLimit), } + walker.policyIndex = newInheritancePolicyIndex(walker.collectionPolicies) if problem := walker.visit(&root, Path{}, true); problem != nil { return nil, []Diagnostic{*problem} } @@ -113,20 +115,13 @@ func (w *inheritanceWalker) visit(node **document.Node, path Path, unscoped bool limits.nodes -= w.nodes - before } resolved, diagnostics := expandSiblingDefinitionsWithCollectionRules( - *node, path, limits, w.origins, w.collectionPolicies, w.applicationPolicy, w.retained, + *node, path, limits, w.origins, w.policyIndex.absoluteScope(path), w.applicationPolicy, w.retained, ) if len(diagnostics) != 0 { return &diagnostics[0] } *node = resolved.root - for pointer, entry := range w.collectionPolicies.collections { - if pathHasPrefix(entry.destination, path) { - delete(w.collectionPolicies.collections, pointer) - } - } - for _, entry := range resolved.collectionPolicies.collections { - w.collectionPolicies.set(entry.destination, entry.policy) - } + w.policyIndex.replace(&w.collectionPolicies, path, resolved.collectionPolicies) deleteOriginSubtree(w.origins, path, original) for pointer, origin := range resolved.origins { w.origins[pointer] = origin @@ -203,20 +198,13 @@ func (w *inheritanceWalker) expandIdentifiedSequence(node **document.Node, path limits.nodes -= w.nodes - before } resolved, diagnostics := applyIdentifiedInheritanceTemplates( - graph, limits, w.origins, w.collectionPolicies, w.applicationPolicy, w.retained, + graph, limits, w.origins, w.policyIndex.absoluteScope(path), w.applicationPolicy, w.retained, ) if len(diagnostics) != 0 { return &diagnostics[0] } *node = resolved.root - for pointer, entry := range w.collectionPolicies.collections { - if pathHasPrefix(entry.destination, path) { - delete(w.collectionPolicies.collections, pointer) - } - } - for _, entry := range resolved.collectionPolicies.collections { - w.collectionPolicies.set(entry.destination, entry.policy) - } + w.policyIndex.replace(&w.collectionPolicies, path, resolved.collectionPolicies) deleteOriginSubtree(w.origins, path, original) for pointer, origin := range resolved.origins { w.origins[pointer] = origin