Reuse document indexes for collection policy checks

docs/roadmap/remediation.md

@@ -1,8 +1,7 @@
 # Sequence Implementation Review Remediation
 
-Status: findings 1–17 are complete. The algorithm audit below identifies six
-additional open findings, numbered 18–23. This record introduces no new release
-gate.
+Status: findings 1–23 are complete, including the six additional algorithm-audit
+findings numbered 18–23. This record introduces no new release gate.
 
 Reviewed commit: `0be7cdb00eff576fda6b30c0600b2134d4e2e086`.
 Implementation range: `bc6b977..0be7cdb`.
@@ -40,6 +39,14 @@ ownership regressions; the contract corrections are verified against the
 implemented operations and replay. Each additional finding has its own completion
 record and commit.
 
+Validation after algorithm remediation: `./scripts/check.sh` and
+`go test -race ./...` pass with Go 1.26.10 on Linux. Focused regressions cover the
+selector scope correction, sparse projection storage, broad CLI metadata,
+inheritance policy localization/ownership, effective fallback locations, and
+broad collection-policy lookups. Scaling conclusions follow from the revised
+algorithms and structural coverage; no timing threshold or public performance
+guarantee was added.
+
 P1 denotes the highest-priority correctness issue here; P2 denotes normal-priority
 work needed to complete the selected behavior. Preserve the agreed scope while
 fixing these issues. In particular, do not resolve failures by restricting valid
@@ -919,7 +926,15 @@ or skip needed location attachment to avoid the work.
 
 ## 23. Index Repeated Collection-Policy Document Lookups — P2
 
-Status: [ ] Open.
+Status: [x] Complete.
+
+Completion: overlay policy matching shares an immutable incoming-tree lookup;
+declaration-only checks lazily index the completed result after merging.
+Inheritance policy reconciliation shares one lookup over its stable parent and
+child inputs. Added broad overlay/inheritance collections, retained and pending
+declarations, escaped keys, ordered append, and origin coverage. Existing
+retirement and diagnostic regressions, `./scripts/check.sh`, and the final
+`go test -race ./...` pass.
 
 Owners: `mergeOverlayMappingsWithCollectionPolicy` in
 [`overlay.go`](../../internal/engine/overlay.go) and

internal/engine/collection_lookup_test.go

@@ -0,0 +1,98 @@
+package engine
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"gitea.maximumdirect.net/eric/ConfigKit/internal/document"
+)
+
+func TestOverlayBroadCollectionPoliciesAndRetainedDeclarations(t *testing.T) {
+	var baseBody, overlayBody strings.Builder
+	basePolicies := collectionPolicyScope{}
+	var declarations []sequenceDeclarationSpec
+	for index := range 64 {
+		name := fmt.Sprintf("group/%03d~", index)
+		fmt.Fprintf(&baseBody, "%s: {items: [{id: base}], retained: [{id: retained}], scalar: pending}\n", name)
+		fmt.Fprintf(&overlayBody, "%s: {items: [{id: overlay}], added: true}\n", name)
+		basePolicies.set(Keys(name, "items"), collectionPolicy{
+			identityFields: []string{"id"}, identityPresent: true,
+			explicitMode: sequenceModeAppend, explicitModePresent: true,
+		})
+		for _, key := range []string{"retained", "missing", "scalar"} {
+			declarations = append(declarations, sequenceDeclarationSpec{
+				destination: Keys(name, key), identityFields: []string{"id"}, identityPresent: true,
+				mode: sequenceModeAppend, modePresent: true,
+			})
+		}
+	}
+	base, diagnostics := composeDocuments(parseCompositionDocument(t, "base", baseBody.String()))
+	if len(diagnostics) != 0 {
+		t.Fatal(diagnostics)
+	}
+	base.collectionPolicies = basePolicies
+	assembly := newAssemblyAccumulator(base)
+	overlay := parseCompositionDocument(t, "overlay", overlayBody.String())
+	diagnostics = assembly.applyOverlayWithCollectionPolicy(overlay, nil, collectionPolicyScope{}, nil, LayerOverlay, sequencePolicy{}, declarations)
+	if len(diagnostics) != 0 {
+		t.Fatal(diagnostics)
+	}
+	lookup := buildDocumentLookup(assembly.current.root)
+	for index := range 64 {
+		name := fmt.Sprintf("group/%03d~", index)
+		items, found := documentAtPathWithLookup(assembly.current.root, Keys(name, "items"), lookup)
+		if !found || items.Kind != document.Sequence || len(items.Sequence) != 2 || items.Sequence[0].Mapping[0].Value.Text != "base" || items.Sequence[1].Mapping[0].Value.Text != "overlay" {
+			t.Fatalf("collection %s did not append in order", name)
+		}
+		for _, key := range []string{"retained", "missing", "scalar"} {
+			policy, found := assembly.current.collectionPolicies.at(Keys(name, key))
+			if !found || !policy.identityPresent || !policy.explicitModePresent || policy.explicitMode != sequenceModeAppend {
+				t.Fatalf("retained/pending declaration lost at %s/%s", name, key)
+			}
+		}
+		if _, found := documentAtPathWithLookup(assembly.current.root, Keys(name, "missing"), lookup); found {
+			t.Fatal("pending declaration created a value")
+		}
+		if origin := assembly.current.origins[Keys(name, "items").Index(1).Key("id").String()]; origin.Layer != LayerOverlay {
+			t.Fatal("appended item lost overlay attribution")
+		}
+	}
+}
+
+func TestInheritanceBroadCollectionsInOneDefinition(t *testing.T) {
+	var body strings.Builder
+	policies := collectionPolicyScope{}
+	for _, definition := range []string{"base", "child"} {
+		fmt.Fprintf(&body, "%s:\n", definition)
+		if definition == "child" {
+			body.WriteString("  extends: base\n")
+		}
+		for index := range 64 {
+			name := fmt.Sprintf("group/%03d~", index)
+			fmt.Fprintf(&body, "  %s: {items: [{id: %s}]}\n", name, definition)
+			policy := collectionPolicy{identityFields: []string{"id"}, identityPresent: true}
+			if definition == "child" {
+				policy.explicitMode, policy.explicitModePresent = sequenceModeAppend, true
+			}
+			policies.set(Keys(definition, name, "items"), policy)
+			policies.set(Keys(definition, name, "pending"), collectionPolicy{identityFields: []string{"id"}, identityPresent: true})
+		}
+	}
+	root := parseCompositionDocument(t, "wide-collections", body.String())
+	result, diagnostics := expandInheritanceWithPolicies(root, nil, policies, defaultProjectionLimits())
+	if len(diagnostics) != 0 {
+		t.Fatal(diagnostics)
+	}
+	lookup := buildDocumentLookup(result.root)
+	for index := range 64 {
+		name := fmt.Sprintf("group/%03d~", index)
+		items, found := documentAtPathWithLookup(result.root, Keys("child", name, "items"), lookup)
+		if !found || len(items.Sequence) != 2 || items.Sequence[0].Mapping[0].Value.Text != "base" || items.Sequence[1].Mapping[0].Value.Text != "child" {
+			t.Fatalf("inherited collection %s did not append in order", name)
+		}
+		if policy, found := result.collectionPolicies.at(Keys("child", name, "pending")); !found || !policy.identityPresent {
+			t.Fatalf("pending inherited policy lost at %s", name)
+		}
+	}
+}

internal/engine/inheritance_collection.go

@@ -103,15 +103,19 @@ func mergeInheritanceCollectionPolicies(
 	for key, entry := range child.collections {
 		paths[key] = entry.destination.clone()
 	}
+	var lookup *documentLookup
+	if len(paths) != 0 {
+		lookup = buildDocumentLookup(parentValue, childValue)
+	}
 	var diagnostics []Diagnostic
 	for _, path := range paths {
 		relative, inside := relativeHistoryPath(parentRootPath, path)
 		var retained, incoming *document.Node
 		if inside {
 			if !replaced.contains(path, false) {
-				retained, _ = documentAtPath(parentValue, relative)
+				retained, _ = documentAtPathWithLookup(parentValue, relative, lookup)
 			}
-			incoming, _ = documentAtPath(childValue, relative)
+			incoming, _ = documentAtPathWithLookup(childValue, relative, lookup)
 		}
 		resolution, lower, _, active, issues := inheritanceCollectionResolution(path, parent, child, applicationPolicy, incoming)
 		if len(issues) != 0 {

internal/engine/overlay.go

@@ -138,8 +138,12 @@ func mergeOverlayMappingsWithCollectionPolicy(
 	for key := range declarationsByPath {
 		registered[key] = true
 	}
+	var overlayLookup *documentLookup
+	if len(result.collectionPolicies.collections) != 0 {
+		overlayLookup = buildDocumentLookup(overlay)
+	}
 	for key := range result.collectionPolicies.collections {
-		if node, found := documentAtPath(overlay, result.collectionPolicies.collections[key].destination); found && node != nil {
+		if node, found := documentAtPathWithLookup(overlay, result.collectionPolicies.collections[key].destination, overlayLookup); found && node != nil {
 			registered[key] = true
 			registeredPaths[key] = result.collectionPolicies.collections[key].destination.clone()
 		}
@@ -210,6 +214,9 @@ func mergeOverlayMappingsWithCollectionPolicy(
 		return nil, canonicalDiagnostics(combinationDiagnostics)
 	}
 
+	// Values are now complete. Index lazily for declaration-only checks;
+	// this lookup is never retained across the value-changing merge above.
+	var retainedLookup *documentLookup
 	for key, grouped := range declarationsByPath {
 		if policyValues[key] {
 			continue
@@ -226,7 +233,10 @@ func mergeOverlayMappingsWithCollectionPolicy(
 		if found {
 			lowerPolicy = &lower
 		}
-		retained, _ := documentAtPath(assembly.current.root, path)
+		if retainedLookup == nil {
+			retainedLookup = buildDocumentLookup(assembly.current.root)
+		}
+		retained, _ := documentAtPathWithLookup(assembly.current.root, path, retainedLookup)
 		if retained != nil && retained.Kind == document.Sequence && len(current.effectiveIdentityFields) != 0 {
 			if _, issues := validateSequenceIdentity(retained, path, current.effectiveIdentityFields, sequenceIdentityDiagnosticCodes{invalid: CodeSequenceInvalid, conflict: CodeSequenceConflict}); len(issues) != 0 {
 				combinationDiagnostics = append(combinationDiagnostics, issues...)