From 298e70b552594982d77466e537a642a550e446b6 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 23 Oct 2026 13:55:25 +0000 Subject: [PATCH] Add dynamic sequence identity selectors --- docs/roadmap/implementation.md | 5 +- internal/engine/compose.go | 86 ++++-- internal/engine/sequence_registration.go | 247 +++++++++++++++++- internal/engine/sequence_registration_test.go | 154 +++++++++++ 4 files changed, 451 insertions(+), 41 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 4c68024..c9ea2da 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Sequence Identity And Combination Implementation Plan -Status: implementation in progress; Stage 1 complete. +Status: implementation in progress; Stages 1–2 complete. Audience: a Luna coding agent implementing one numbered stage per prompt, in order. This plan starts at commit @@ -199,6 +199,9 @@ Scope: private registration model, selector matching, overlap validation. Validation: focused registration/selector tests and the everyday check. This stage does not export the replacement API or add final-result validation yet. +**Completion:** Implemented and verified with focused registration/selector +tests and `./scripts/check.sh`. + ## Stage 3 — Parse Collection Declarations Without Filesystem Coupling Scope: private parser and directive extraction helpers, initially unit-tested diff --git a/internal/engine/compose.go b/internal/engine/compose.go index b229281..d2dc45f 100644 --- a/internal/engine/compose.go +++ b/internal/engine/compose.go @@ -228,7 +228,7 @@ func validateRegisteredSequences(claims []compositionClaim, policy sequencePolic if claim.kind != document.Sequence { continue } - identityField, registered := policy.identityFields[claim.path.String()] + identityFields, registered := policy.identityFieldsAt(claim.path) if !registered { continue } @@ -254,36 +254,71 @@ func validateRegisteredSequences(claims []compositionClaim, policy sequencePolic Path: itemPath, Message: "registered sequence items must be mappings", Primary: location, }} } - fieldIndex := documentEntryIndex(item.Mapping, identityField) - if fieldIndex < 0 { - location := publicLocation(item.Location) - return nil, nil, []Diagnostic{{ - Code: CodeComposeInvalid, Severity: SeverityError, - Path: itemPath, Message: "registered sequence item is missing its identity field", - Primary: &location, - }} - } - field := item.Mapping[fieldIndex].Value - if field == nil || field.Kind != document.String { - location := publicLocation(item.Mapping[fieldIndex].ValueLocation) - if field != nil { - location = publicLocation(field.Location) + var identity string + var identityLocation document.Location + if legacyField, legacy := policy.identityFields[claim.path.String()]; legacy { + fieldIndex := documentEntryIndex(item.Mapping, legacyField) + if fieldIndex < 0 { + location := publicLocation(item.Location) + return nil, nil, []Diagnostic{{ + Code: CodeComposeInvalid, Severity: SeverityError, + Path: itemPath, Message: "registered sequence item is missing its identity field", + Primary: &location, + }} } - return nil, nil, []Diagnostic{{ - Code: CodeComposeInvalid, Severity: SeverityError, - Path: itemPath.Key(identityField), Message: "registered sequence identity must be a string", - Primary: &location, - }} + field := item.Mapping[fieldIndex].Value + if field == nil || field.Kind != document.String { + location := publicLocation(item.Mapping[fieldIndex].ValueLocation) + if field != nil { + location = publicLocation(field.Location) + } + return nil, nil, []Diagnostic{{ + Code: CodeComposeInvalid, Severity: SeverityError, + Path: itemPath.Key(legacyField), Message: "registered sequence identity must be a string", + Primary: &location, + }} + } + identity, identityLocation = field.Text, field.Location + } else { + mapping := newSequenceMappingIndex(item) + var locations []document.Location + var fieldName string + var fieldIndex int + identity, locations, fieldName, fieldIndex = mapping.identity(identityFields) + if fieldIndex == -1 { + location := publicLocation(item.Location) + return nil, nil, []Diagnostic{{ + Code: CodeComposeInvalid, Severity: SeverityError, + Path: itemPath.Key(fieldName), Message: "registered sequence item is missing an identity field", + Primary: &location, + }} + } + if fieldIndex == -2 || fieldIndex >= 0 { + location := publicLocation(item.Location) + fieldPath := itemPath.Key(fieldName) + if fieldIndex >= 0 { + location = publicLocation(item.Mapping[fieldIndex].ValueLocation) + if field := item.Mapping[fieldIndex].Value; field != nil { + location = publicLocation(field.Location) + } + } + return nil, nil, []Diagnostic{{ + Code: CodeComposeInvalid, Severity: SeverityError, + Path: fieldPath, Message: "registered sequence identity fields must be scalar values", + Primary: &location, + }} + } + identityLocation = locations[0] } - if first, duplicate := identities[field.Text]; duplicate { - current := publicLocation(field.Location) + if first, duplicate := identities[identity]; duplicate { + current := publicLocation(identityLocation) return nil, nil, []Diagnostic{{ Code: CodeComposeConflict, Severity: SeverityError, Path: claim.path.clone(), Message: "registered sequence contains duplicate identities", Primary: &first, Related: []Location{current}, }} } - identities[field.Text] = publicLocation(field.Location) + identities[identity] = publicLocation(identityLocation) } if itemsByPath[pathKey] == nil { itemsByPath[pathKey] = make(map[int]validatedSequence) @@ -328,11 +363,6 @@ func mergeDisjointMappingsAtPath(target, incoming *document.Node, path Path, ind mergeComposedMappings(target, incoming, path, index, sourceIndex, policy, validated) } -func (p sequencePolicy) registered(path Path) bool { - _, ok := p.identityFields[path.String()] - return ok -} - func cloneSequenceRanges(ranges []sequenceRange) []sequenceRange { cloned := make([]sequenceRange, len(ranges)) for i, assigned := range ranges { diff --git a/internal/engine/sequence_registration.go b/internal/engine/sequence_registration.go index 142b145..ba88377 100644 --- a/internal/engine/sequence_registration.go +++ b/internal/engine/sequence_registration.go @@ -5,8 +5,37 @@ type sequenceRegistration struct { identityField string } +type sequenceSelectorSegmentKind uint8 + +const ( + sequenceSelectorLiteral sequenceSelectorSegmentKind = iota + 1 + sequenceSelectorAnyMappingKey +) + +type sequenceSelectorSegment struct { + kind sequenceSelectorSegmentKind + key string +} + +type sequenceIdentitySelector []sequenceSelectorSegment + +type sequenceIdentityRegistration struct { + selector sequenceIdentitySelector + identityFields []string +} + +type sequenceSelectorTrie struct { + literals map[string]*sequenceSelectorTrie + wildcard *sequenceSelectorTrie + fields []string + terminal bool +} + type sequencePolicy struct { + // identityFields preserves the old exact-path view for existing internal + // callers while selector matching is introduced behind the same policy. identityFields map[string]string + selectorTrie *sequenceSelectorTrie } // DefinitionWithSequenceRegistrations attaches facade-owned registrations to a @@ -26,8 +55,9 @@ func DefinitionWithSequenceRegistrations[T any](definition Definition[T], paths } func compileSequencePolicy(registrations []sequenceRegistration) (sequencePolicy, []Diagnostic) { - policy := sequencePolicy{identityFields: make(map[string]string, len(registrations))} + common := make([]sequenceIdentityRegistration, 0, len(registrations)) var diagnostics []Diagnostic + legacyPaths := make(map[string]string, len(registrations)) for _, registration := range registrations { path, err := normalizeDestination(registration.path) if err != nil { @@ -37,18 +67,211 @@ func compileSequencePolicy(registrations []sequenceRegistration) (sequencePolicy }) continue } - - key := path.String() - if current, ok := policy.identityFields[key]; ok { - if current != registration.identityField { - diagnostics = append(diagnostics, Diagnostic{ - Code: CodeOptionsInvalid, Severity: SeverityError, - Path: path, Message: "sequence registrations at one path must use the same identity field", - }) - } - continue + selector := make(sequenceIdentitySelector, len(path.segments)) + for index, segment := range path.segments { + selector[index] = sequenceSelectorSegment{kind: sequenceSelectorLiteral, key: segment.key} } - policy.identityFields[key] = registration.identityField + common = append(common, sequenceIdentityRegistration{ + selector: selector, identityFields: []string{registration.identityField}, + }) + legacyPaths[path.String()] = registration.identityField } + policy, compiledDiagnostics := compileSequenceIdentityPolicy(common) + policy.identityFields = legacyPaths + diagnostics = append(diagnostics, compiledDiagnostics...) return policy, canonicalDiagnostics(diagnostics) } + +func compileSequenceIdentityPolicy(registrations []sequenceIdentityRegistration) (sequencePolicy, []Diagnostic) { + policy := sequencePolicy{} + compiled := make([]sequenceIdentityRegistration, 0, len(registrations)) + var diagnostics []Diagnostic + for _, registration := range registrations { + selector := cloneSequenceIdentitySelector(registration.selector) + fields, err := normalizeSequenceIdentityFields(registration.identityFields) + if err != nil { + diagnostics = append(diagnostics, Diagnostic{ + Code: CodeOptionsInvalid, Severity: SeverityError, Path: selectorDiagnosticPath(selector), + Message: "sequence registration requires at least one identity field", + }) + continue + } + if err := validateSequenceIdentitySelector(selector); err != nil { + diagnostics = append(diagnostics, Diagnostic{ + Code: CodeOptionsInvalid, Severity: SeverityError, Path: selectorDiagnosticPath(selector), + Message: "sequence registration selector must contain only valid mapping segments", + }) + continue + } + compiled = append(compiled, sequenceIdentityRegistration{selector: selector, identityFields: fields}) + } + + for leftIndex := 0; leftIndex < len(compiled); leftIndex++ { + for rightIndex := leftIndex + 1; rightIndex < len(compiled); rightIndex++ { + left, right := compiled[leftIndex], compiled[rightIndex] + if !sequenceIdentitySelectorsOverlap(left.selector, right.selector) || equalSequenceIdentityFields(left.identityFields, right.identityFields) { + continue + } + diagnostics = append(diagnostics, Diagnostic{ + Code: CodeOptionsInvalid, Severity: SeverityError, Path: selectorDiagnosticPath(left.selector), + Message: "overlapping sequence registrations must use the same identity fields", + }) + } + } + if len(diagnostics) != 0 { + return policy, canonicalDiagnostics(diagnostics) + } + + unique := make([]sequenceIdentityRegistration, 0, len(compiled)) + for _, registration := range compiled { + duplicate := false + for _, existing := range unique { + if sequenceIdentitySelectorsEqual(existing.selector, registration.selector) && equalSequenceIdentityFields(existing.identityFields, registration.identityFields) { + duplicate = true + break + } + } + if duplicate { + continue + } + unique = append(unique, registration) + } + for _, registration := range unique { + policy.addRegistration(registration) + } + return policy, nil +} + +func (p *sequencePolicy) addRegistration(registration sequenceIdentityRegistration) { + if p.selectorTrie == nil { + p.selectorTrie = &sequenceSelectorTrie{} + } + node := p.selectorTrie + for _, segment := range registration.selector { + if segment.kind == sequenceSelectorAnyMappingKey { + if node.wildcard == nil { + node.wildcard = &sequenceSelectorTrie{} + } + node = node.wildcard + continue + } + if node.literals == nil { + node.literals = make(map[string]*sequenceSelectorTrie) + } + if node.literals[segment.key] == nil { + node.literals[segment.key] = &sequenceSelectorTrie{} + } + node = node.literals[segment.key] + } + node.fields = append([]string(nil), registration.identityFields...) + node.terminal = true +} + +func (p sequencePolicy) identityFieldsAt(path Path) ([]string, bool) { + if !path.valid() { + return nil, false + } + if p.selectorTrie == nil { + field, ok := p.identityFields[path.String()] + if !ok { + return nil, false + } + return []string{field}, true + } + current := []*sequenceSelectorTrie{p.selectorTrie} + for _, segment := range path.segments { + if segment.kind != pathSegmentKey && segment.kind != pathSegmentParsed { + return nil, false + } + next := make([]*sequenceSelectorTrie, 0, len(current)*2) + for _, node := range current { + if literal := node.literals[segment.key]; literal != nil { + next = append(next, literal) + } + if node.wildcard != nil { + next = append(next, node.wildcard) + } + } + if len(next) == 0 { + return nil, false + } + current = next + } + for _, node := range current { + if node.terminal { + return append([]string(nil), node.fields...), true + } + } + return nil, false +} + +func (p sequencePolicy) registered(path Path) bool { + _, ok := p.identityFieldsAt(path) + return ok +} + +func validateSequenceIdentitySelector(selector sequenceIdentitySelector) error { + for _, segment := range selector { + switch segment.kind { + case sequenceSelectorLiteral: + if _, err := normalizeDestination(Keys(segment.key)); err != nil { + return err + } + case sequenceSelectorAnyMappingKey: + default: + return errInvalidDestination + } + } + return nil +} + +func sequenceIdentitySelectorsEqual(left, right sequenceIdentitySelector) bool { + if len(left) != len(right) { + return false + } + for index := range left { + if left[index] != right[index] { + return false + } + } + return true +} + +func sequenceIdentitySelectorsOverlap(left, right sequenceIdentitySelector) bool { + if len(left) != len(right) { + return false + } + for index := range left { + if left[index].kind == sequenceSelectorLiteral && right[index].kind == sequenceSelectorLiteral && left[index].key != right[index].key { + return false + } + } + return true +} + +func equalSequenceIdentityFields(left, right []string) bool { + if len(left) != len(right) { + return false + } + for index := range left { + if left[index] != right[index] { + return false + } + } + return true +} + +func cloneSequenceIdentitySelector(selector sequenceIdentitySelector) sequenceIdentitySelector { + return append(sequenceIdentitySelector(nil), selector...) +} + +func selectorDiagnosticPath(selector sequenceIdentitySelector) Path { + path := Path{} + for _, segment := range selector { + if segment.kind != sequenceSelectorLiteral { + return Path{} + } + path = path.Key(segment.key) + } + return path +} diff --git a/internal/engine/sequence_registration_test.go b/internal/engine/sequence_registration_test.go index 6fc2aaf..dab4986 100644 --- a/internal/engine/sequence_registration_test.go +++ b/internal/engine/sequence_registration_test.go @@ -85,6 +85,160 @@ func TestCompileSequencePolicyNormalizesRegistrations(t *testing.T) { } } +func TestCompileSequenceIdentityPolicyMatchesExactAndDynamicSelectors(t *testing.T) { + policy, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{ + {selector: sequenceIdentitySelector{literalSequenceSelector("prompts")}, identityFields: []string{"id"}}, + {selector: sequenceIdentitySelector{literalSequenceSelector("collections"), anyMappingSequenceSelector(), literalSequenceSelector("prompts")}, identityFields: []string{"id"}}, + {selector: sequenceIdentitySelector{literalSequenceSelector("collections"), literalSequenceSelector("weather"), literalSequenceSelector("prompts")}, identityFields: []string{"id"}}, + }) + if len(diagnostics) != 0 { + t.Fatalf("diagnostics = %#v", diagnostics) + } + for _, path := range []Path{Keys("prompts"), Keys("collections", "weather", "prompts")} { + fields, matched := policy.identityFieldsAt(path) + if !matched || !equalSequenceIdentityFields(fields, []string{"id"}) { + t.Errorf("identityFieldsAt(%q) = %#v, %t", path.String(), fields, matched) + } + } + if _, matched := policy.identityFieldsAt(Keys("collections", "weather", "other")); matched { + t.Fatal("selector matched a different destination") + } +} + +func TestDynamicSequenceIdentityScopesRemainIndependent(t *testing.T) { + policy, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{{ + selector: sequenceIdentitySelector{literalSequenceSelector("collections"), anyMappingSequenceSelector(), literalSequenceSelector("prompts")}, + identityFields: []string{"id"}, + }}) + if len(diagnostics) != 0 { + t.Fatalf("diagnostics = %#v", diagnostics) + } + input := parseCompositionDocument(t, "source", "collections:\n weatherreporter:\n prompts: [{id: shared}]\n homewatch:\n prompts: [{id: shared}]\n") + if _, diagnostics := composeDocumentsWithPolicy(policy, input); len(diagnostics) != 0 { + t.Fatalf("same identity in distinct matched destinations conflicted: %#v", diagnostics) + } +} + +func TestCompileSequenceIdentityPolicyMatchesMultipleWildcardLevelsAndScopes(t *testing.T) { + policy, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{ + {selector: sequenceIdentitySelector{ + literalSequenceSelector("collections"), anyMappingSequenceSelector(), + literalSequenceSelector("groups"), anyMappingSequenceSelector(), literalSequenceSelector("prompts"), + }, identityFields: []string{"namespace", "id"}}, + }) + if len(diagnostics) != 0 { + t.Fatalf("diagnostics = %#v", diagnostics) + } + for _, path := range []Path{ + Keys("collections", "weather", "groups", "one", "prompts"), + Keys("collections", "weather", "groups", "two", "prompts"), + } { + fields, matched := policy.identityFieldsAt(path) + if !matched || !equalSequenceIdentityFields(fields, []string{"id", "namespace"}) { + t.Errorf("identityFieldsAt(%q) = %#v, %t", path.String(), fields, matched) + } + } + if !policy.registered(Keys("collections", "weather", "groups", "two", "prompts")) { + t.Fatal("dynamic registration did not match a separate uniqueness scope") + } +} + +func TestCompileSequenceIdentityPolicyPreservesLiteralKeysAndRejectsIndexPaths(t *testing.T) { + policy, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{ + {selector: sequenceIdentitySelector{literalSequenceSelector(""), literalSequenceSelector("a/b~c.%"), literalSequenceSelector("prompts")}, identityFields: []string{"id"}}, + }) + if len(diagnostics) != 0 { + t.Fatalf("diagnostics = %#v", diagnostics) + } + if _, matched := policy.identityFieldsAt(Keys("", "a/b~c.%", "prompts")); !matched { + t.Fatal("selector did not preserve empty and punctuation keys") + } + if _, matched := policy.identityFieldsAt(Keys("", "a/b~c.%", "prompts").Index(0)); matched { + t.Fatal("selector matched a sequence-index destination") + } +} + +func TestCompileSequenceIdentityPolicyCoalescesEquivalentFieldSets(t *testing.T) { + selector := sequenceIdentitySelector{literalSequenceSelector("items")} + policy, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{ + {selector: selector, identityFields: []string{"namespace", "id", "id"}}, + {selector: cloneSequenceIdentitySelector(selector), identityFields: []string{"id", "namespace"}}, + }) + if len(diagnostics) != 0 { + t.Fatalf("diagnostics = %#v", diagnostics) + } + if leaf := policy.selectorTrie.literals["items"]; leaf == nil || !leaf.terminal { + t.Fatal("equivalent selector did not compile to one terminal trie entry") + } + fields, matched := policy.identityFieldsAt(Keys("items")) + if !matched || !equalSequenceIdentityFields(fields, []string{"id", "namespace"}) { + t.Fatalf("identity fields = %#v, matched = %t", fields, matched) + } +} + +func TestCompileSequenceIdentityPolicyRejectsConflictingOverlapsWithoutInput(t *testing.T) { + _, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{ + {selector: sequenceIdentitySelector{literalSequenceSelector("collections"), anyMappingSequenceSelector(), literalSequenceSelector("prompts")}, identityFields: []string{"id"}}, + {selector: sequenceIdentitySelector{literalSequenceSelector("collections"), literalSequenceSelector("weather"), literalSequenceSelector("prompts")}, identityFields: []string{"name"}}, + }) + if len(diagnostics) != 1 || diagnostics[0].Code != CodeOptionsInvalid { + t.Fatalf("diagnostics = %#v, want overlap definition error", diagnostics) + } +} + +func TestCompileSequenceIdentityPolicyAllowsDisjointSelectorsAndRoot(t *testing.T) { + policy, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{ + {selector: sequenceIdentitySelector{}, identityFields: []string{"id"}}, + {selector: sequenceIdentitySelector{literalSequenceSelector("items")}, identityFields: []string{"name"}}, + {selector: sequenceIdentitySelector{literalSequenceSelector("collections"), anyMappingSequenceSelector()}, identityFields: []string{"key"}}, + {selector: sequenceIdentitySelector{literalSequenceSelector("collections"), literalSequenceSelector("static"), literalSequenceSelector("prompts")}, identityFields: []string{"id"}}, + }) + if len(diagnostics) != 0 { + t.Fatalf("diagnostics = %#v", diagnostics) + } + if fields, matched := policy.identityFieldsAt(Path{}); !matched || !equalSequenceIdentityFields(fields, []string{"id"}) { + t.Fatalf("root selector = %#v, %t", fields, matched) + } + for _, path := range []Path{Keys("items"), Keys("collections", "dynamic"), Keys("collections", "static", "prompts")} { + if _, matched := policy.identityFieldsAt(path); !matched { + t.Errorf("disjoint selector failed to match %q", path.String()) + } + } +} + +func TestCompileSequenceIdentityPolicySnapshotsSelectorAndFields(t *testing.T) { + selector := sequenceIdentitySelector{literalSequenceSelector("items")} + fields := []string{"id", "namespace"} + policy, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{{selector: selector, identityFields: fields}}) + if len(diagnostics) != 0 { + t.Fatalf("diagnostics = %#v", diagnostics) + } + selector[0].key = "changed" + fields[0] = "changed" + if got, matched := policy.identityFieldsAt(Keys("items")); !matched || !equalSequenceIdentityFields(got, []string{"id", "namespace"}) { + t.Fatalf("compiled registration changed after input mutation: %#v, %t", got, matched) + } +} + +func TestCompileSequenceIdentityPolicyRejectsInvalidSelectorsAndEmptyFields(t *testing.T) { + _, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{ + {selector: sequenceIdentitySelector{literalSequenceSelector("configkit")}, identityFields: []string{"id"}}, + {selector: sequenceIdentitySelector{{kind: sequenceSelectorSegmentKind(99)}}, identityFields: []string{"id"}}, + {selector: sequenceIdentitySelector{literalSequenceSelector("unused")}, identityFields: nil}, + }) + if len(diagnostics) != 3 { + t.Fatalf("diagnostics = %#v, want all three definition errors", diagnostics) + } +} + +func literalSequenceSelector(key string) sequenceSelectorSegment { + return sequenceSelectorSegment{kind: sequenceSelectorLiteral, key: key} +} + +func anyMappingSequenceSelector() sequenceSelectorSegment { + return sequenceSelectorSegment{kind: sequenceSelectorAnyMappingKey} +} + func TestSequenceRegistrationPreflightDoesNotReadSources(t *testing.T) { t.Parallel() type config struct {