diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 28d8f25..f2751ea 100644 --- a/docs/roadmap/remediation.md +++ b/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 diff --git a/internal/engine/collection_lookup_test.go b/internal/engine/collection_lookup_test.go new file mode 100644 index 0000000..057d969 --- /dev/null +++ b/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) + } + } +} diff --git a/internal/engine/inheritance_collection.go b/internal/engine/inheritance_collection.go index 3c15da5..6d12ed0 100644 --- a/internal/engine/inheritance_collection.go +++ b/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 { diff --git a/internal/engine/overlay.go b/internal/engine/overlay.go index c1c1d61..76ea405 100644 --- a/internal/engine/overlay.go +++ b/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...)