diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 5c4be5c..3903024 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -256,6 +256,22 @@ Scope: `internal/engine/compose.go` and focused composition tests. Validation: composition tests and `./scripts/check.sh`. Completion: the private composer returns the correct tree and range projection without aliasing inputs. +Completion evidence: added a policy-aware private composer while retaining the +unregistered adapter. Registered sequences validate every actual contribution, +including singleton and empty lists; distinct records append in source/list +order, duplicate exact string identities fail, and other overlap kinds retain +strict-conflict behavior. The result carries per-source sequence ranges for +later projection. Tests cover malformed identity records, within/across-list +duplicates including empty IDs and differing bodies, absent/non-sequence paths, +empty lists, unregistered conflicts, and input/result ownership. These checks +passed with Go `go1.26.9 linux/amd64`: + +- `GOTOOLCHAIN=local GOPROXY=off go test ./internal/engine -run 'Compose' -count=1` +- `GOTOOLCHAIN=local ./scripts/check.sh` +- `git diff --check` + +No environmental limitation affected validation; checks ran on Linux/amd64. + ## Stage 3 — Project composition origins and claims onto appended indexes Scope: composition evidence, not import acquisition. diff --git a/internal/engine/compose.go b/internal/engine/compose.go index be534b4..056f4f5 100644 --- a/internal/engine/compose.go +++ b/internal/engine/compose.go @@ -6,13 +6,14 @@ import "gitea.maximumdirect.net/eric/ConfigKit/internal/document" // precedence layers. Claims remain ordered by source and declaration traversal; // origins retain every source that contributes to a composite mapping. type composedDocument struct { - root *document.Node - claims []compositionClaim - origins map[string]Origin - overlays []overlayRecord - applications []historyApplication - inputBytes int64 - inputNodes int + root *document.Node + claims []compositionClaim + sequenceRanges []sequenceRange + origins map[string]Origin + overlays []overlayRecord + applications []historyApplication + inputBytes int64 + inputNodes int } // assemblyAccumulator owns one mutable working document. Acquired trees held by @@ -31,16 +32,18 @@ func newAssemblyAccumulator(owned *composedDocument) *assemblyAccumulator { func cloneComposedDocument(base *composedDocument) *composedDocument { return &composedDocument{ root: document.Clone(base.root), claims: cloneCompositionClaims(base.claims), - origins: cloneOriginIndex(base.origins), overlays: cloneOverlayRecords(base.overlays), + sequenceRanges: cloneSequenceRanges(base.sequenceRanges), + origins: cloneOriginIndex(base.origins), overlays: cloneOverlayRecords(base.overlays), applications: cloneHistoryApplications(base.applications), inputBytes: base.inputBytes, inputNodes: base.inputNodes, } } type compositionClaim struct { - path Path - kind document.Kind - location Location + path Path + kind document.Kind + location Location + sourceIndex int } type compositionClaims struct { @@ -48,16 +51,32 @@ type compositionClaims struct { byPath map[string][]compositionClaim } +// sequenceRange records where one source's sequence items were assigned in the +// composed sequence at path. End is exclusive. +type sequenceRange struct { + sourceIndex int + path Path + start int + end int +} + // composeDocuments recursively joins disjoint mapping entries. Source order // controls only output traversal and diagnostics: every non-mapping overlap is // rejected before a result is assembled. func composeDocuments(inputs ...*document.Node) (*composedDocument, []Diagnostic) { + return composeDocumentsWithPolicy(sequencePolicy{}, inputs...) +} + +// composeDocumentsWithPolicy composes one strict tier using the registered +// sequence rules when policy contains registrations. The ordinary adapter above +// deliberately preserves the existing unregistered behavior for current callers. +func composeDocumentsWithPolicy(policy sequencePolicy, inputs ...*document.Node) (*composedDocument, []Diagnostic) { if len(inputs) == 0 { return nil, []Diagnostic{compositionInvalidDiagnostic("strict composition requires at least one document", nil)} } collected := compositionClaims{byPath: make(map[string][]compositionClaim)} - for _, input := range inputs { + for sourceIndex, input := range inputs { if input == nil || input.Kind != document.Mapping { var location *Location if input != nil { @@ -66,7 +85,7 @@ func composeDocuments(inputs ...*document.Node) (*composedDocument, []Diagnostic } return nil, []Diagnostic{compositionInvalidDiagnostic("strict composition requires mapping document roots", location)} } - collected.append(input, Path{}) + collected.append(input, Path{}, sourceIndex) } checked := make(map[string]struct{}, len(collected.byPath)) @@ -77,16 +96,21 @@ func composeDocuments(inputs ...*document.Node) (*composedDocument, []Diagnostic } checked[pointer] = struct{}{} claims := collected.byPath[pointer] - if len(claims) < 2 || allMappingClaims(claims) { + if len(claims) < 2 || allMappingClaims(claims) || (allSequenceClaims(claims) && policy.registered(claim.path)) { continue } return nil, []Diagnostic{compositionConflictDiagnostic(claim.path, claims)} } + validated, ranges, diagnostics := validateRegisteredSequences(inputs, collected.ordered, policy) + if len(diagnostics) != 0 { + return nil, diagnostics + } + root := document.Clone(inputs[0]) index := new(mappingIndex) - for _, input := range inputs[1:] { - mergeDisjointMappings(root, input, index) + for sourceIndex, input := range inputs[1:] { + mergeComposedMappings(root, input, Path{}, index, sourceIndex+1, policy, validated) } applications := make([]historyApplication, 0, len(inputs)) for _, input := range inputs { @@ -95,10 +119,11 @@ func composeDocuments(inputs ...*document.Node) (*composedDocument, []Diagnostic origins := compositionOrigins(collected.ordered) completeCompositionOrigins(root, Path{}, origins) return &composedDocument{ - root: root, - claims: cloneCompositionClaims(collected.ordered), - origins: origins, - applications: applications, + root: root, + claims: cloneCompositionClaims(collected.ordered), + sequenceRanges: cloneSequenceRanges(ranges), + origins: origins, + applications: applications, }, nil } @@ -121,11 +146,12 @@ func completeCompositionOrigins(node *document.Node, path Path, origins map[stri } } -func (c *compositionClaims) append(node *document.Node, path Path) { +func (c *compositionClaims) append(node *document.Node, path Path, sourceIndex int) { claim := compositionClaim{ - path: path.clone(), - kind: node.Kind, - location: publicLocation(node.Location), + path: path.clone(), + kind: node.Kind, + location: publicLocation(node.Location), + sourceIndex: sourceIndex, } c.ordered = append(c.ordered, claim) pointer := path.String() @@ -135,10 +161,149 @@ func (c *compositionClaims) append(node *document.Node, path Path) { return } for _, entry := range node.Mapping { - c.append(entry.Value, path.Key(entry.Key)) + c.append(entry.Value, path.Key(entry.Key), sourceIndex) } } +func allSequenceClaims(claims []compositionClaim) bool { + for _, claim := range claims { + if claim.kind != document.Sequence { + return false + } + } + return true +} + +type validatedSequence struct { + items []*document.Node +} + +func validateRegisteredSequences(inputs []*document.Node, claims []compositionClaim, policy sequencePolicy) (map[string]map[int]validatedSequence, []sequenceRange, []Diagnostic) { + itemsByPath := make(map[string]map[int]validatedSequence) + identitiesByPath := make(map[string]map[string]Location) + lengthsByPath := make(map[string]int) + var ranges []sequenceRange + for _, claim := range claims { + if claim.kind != document.Sequence { + continue + } + identityField, registered := policy.identityFields[claim.path.String()] + if !registered { + continue + } + node, ok := documentAtPath(inputs[claim.sourceIndex], claim.path) + if !ok || node.Kind != document.Sequence { + continue + } + pathKey := claim.path.String() + identities := identitiesByPath[pathKey] + if identities == nil { + identities = make(map[string]Location, len(node.Sequence)) + identitiesByPath[pathKey] = identities + } + start := lengthsByPath[pathKey] + contribution := validatedSequence{items: node.Sequence} + for itemIndex, item := range node.Sequence { + itemPath := claim.path.Index(itemIndex) + if item == nil || item.Kind != document.Mapping { + location := (*Location)(nil) + if item != nil { + converted := publicLocation(item.Location) + location = &converted + } + return nil, nil, []Diagnostic{{ + Code: CodeComposeInvalid, Severity: SeverityError, + 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) + } + return nil, nil, []Diagnostic{{ + Code: CodeComposeInvalid, Severity: SeverityError, + Path: itemPath.Key(identityField), Message: "registered sequence identity must be a string", + Primary: &location, + }} + } + if first, duplicate := identities[field.Text]; duplicate { + current := publicLocation(field.Location) + 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) + } + if itemsByPath[pathKey] == nil { + itemsByPath[pathKey] = make(map[int]validatedSequence) + } + itemsByPath[pathKey][claim.sourceIndex] = contribution + lengthsByPath[pathKey] = start + len(node.Sequence) + ranges = append(ranges, sequenceRange{ + sourceIndex: claim.sourceIndex, path: claim.path.clone(), start: start, end: start + len(node.Sequence), + }) + } + return itemsByPath, ranges, nil +} + +func mergeComposedMappings(target, incoming *document.Node, path Path, index *mappingIndex, sourceIndex int, policy sequencePolicy, validated map[string]map[int]validatedSequence) { + for _, incomingEntry := range incoming.Mapping { + entryPath := path.Key(incomingEntry.Key) + position := index.find(target, incomingEntry.Key) + if position < 0 { + cloned := incomingEntry + cloned.Value = document.Clone(incomingEntry.Value) + index.append(target, cloned) + continue + } + current := target.Mapping[position].Value + if current.Kind == document.Sequence && incomingEntry.Value.Kind == document.Sequence && policy.registered(entryPath) { + contribution := validated[entryPath.String()][sourceIndex] + if contribution.items != nil { + for _, item := range contribution.items { + current.Sequence = append(current.Sequence, document.Clone(item)) + } + } + continue + } + mergeDisjointMappingsAtPath(current, incomingEntry.Value, entryPath, index, sourceIndex, policy, validated) + } +} + +func mergeDisjointMappingsAtPath(target, incoming *document.Node, path Path, index *mappingIndex, sourceIndex int, policy sequencePolicy, validated map[string]map[int]validatedSequence) { + if target.Kind != document.Mapping || incoming.Kind != document.Mapping { + return + } + 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 { + cloned[i] = assigned + cloned[i].path = assigned.path.clone() + } + return cloned +} + func allMappingClaims(claims []compositionClaim) bool { for _, claim := range claims { if claim.kind != document.Mapping { diff --git a/internal/engine/compose_test.go b/internal/engine/compose_test.go index 0e950f6..4e81eb6 100644 --- a/internal/engine/compose_test.go +++ b/internal/engine/compose_test.go @@ -2,6 +2,7 @@ package engine import ( "reflect" + "strings" "testing" "gitea.maximumdirect.net/eric/ConfigKit/internal/document" @@ -190,6 +191,164 @@ func TestComposeDocumentsRejectsInvalidBoundaries(t *testing.T) { } } +func TestComposeDocumentsAppendsRegisteredSequencesAndReturnsRanges(t *testing.T) { + first := parseCompositionDocument(t, "source-1", "records: [{id: '', left: 1}, {id: ' spaced ', right: 1}]\nother: [one]\n") + second := parseCompositionDocument(t, "source-2", "records: [{id: ' ', text: second}]\n") + firstBefore, secondBefore := document.Clone(first), document.Clone(second) + policy := sequencePolicy{identityFields: map[string]string{"/records": "id"}} + + composed, diagnostics := composeDocumentsWithPolicy(policy, first, second) + if len(diagnostics) != 0 { + t.Fatalf("composeDocumentsWithPolicy() diagnostics = %#v", diagnostics) + } + records, ok := documentAtPath(composed.root, Keys("records")) + if !ok || records.Kind != document.Sequence || len(records.Sequence) != 3 { + t.Fatalf("records = %#v, want three appended records", records) + } + if got := composed.sequenceRanges; !reflect.DeepEqual(got, []sequenceRange{ + {sourceIndex: 0, path: Keys("records"), start: 0, end: 2}, + {sourceIndex: 1, path: Keys("records"), start: 2, end: 3}, + }) { + t.Fatalf("sequence ranges = %#v", got) + } + if !reflect.DeepEqual(first, firstBefore) || !reflect.DeepEqual(second, secondBefore) { + t.Fatal("registered composition mutated an input") + } + composed.root.Mapping[0].Value.Sequence[0].Mapping[1].Value.Text = "changed" + if !reflect.DeepEqual(first, firstBefore) || !reflect.DeepEqual(second, secondBefore) { + t.Fatal("composed sequence aliases an input") + } +} + +func TestComposeDocumentsValidatesRegisteredSequenceSingletons(t *testing.T) { + tests := []struct { + name string + body string + path string + wantCode Code + }{ + {name: "nonmapping item", body: "records: [plain]\n", path: "/records/0", wantCode: CodeComposeInvalid}, + {name: "missing identity", body: "records: [{name: plain}]\n", path: "/records/0", wantCode: CodeComposeInvalid}, + {name: "null identity", body: "records: [{id: null}]\n", path: "/records/0/id", wantCode: CodeComposeInvalid}, + {name: "numeric identity", body: "records: [{id: 12}]\n", path: "/records/0/id", wantCode: CodeComposeInvalid}, + {name: "boolean identity", body: "records: [{id: false}]\n", path: "/records/0/id", wantCode: CodeComposeInvalid}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, diagnostics := composeDocumentsWithPolicy( + sequencePolicy{identityFields: map[string]string{"/records": "id"}}, + parseCompositionDocument(t, "source-1", test.body), + ) + if len(diagnostics) != 1 || diagnostics[0].Code != test.wantCode || diagnostics[0].Path.String() != test.path { + t.Fatalf("diagnostics = %#v, want %s at %s", diagnostics, test.wantCode, test.path) + } + if diagnostics[0].Primary == nil { + t.Fatal("malformed sequence diagnostic has no source location") + } + }) + } +} + +func TestComposeDocumentsRejectsRegisteredDuplicateIdentities(t *testing.T) { + tests := []struct { + name string + first string + second string + within bool + }{ + {name: "within identical bodies", first: "records: [{id: '', value: same}, {id: '', value: same}]\n", within: true}, + {name: "within disjoint bodies", first: "records: [{id: same, left: 1}, {id: same, right: 2}]\n", within: true}, + {name: "across identical bodies", first: "records: [{id: same, value: same}]\n", second: "records: [{id: same, value: same}]\n"}, + {name: "across disjoint bodies", first: "records: [{id: same, left: 1}]\n", second: "records: [{id: same, right: 2}]\n"}, + {name: "empty identity across lists", first: "records: [{id: ''}]\n", second: "records: [{id: ''}]\n"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + inputs := []*document.Node{parseCompositionDocument(t, "source-1", test.first)} + if !test.within { + inputs = append(inputs, parseCompositionDocument(t, "source-2", test.second)) + } + _, diagnostics := composeDocumentsWithPolicy( + sequencePolicy{identityFields: map[string]string{"/records": "id"}}, inputs..., + ) + if len(diagnostics) != 1 || diagnostics[0].Code != CodeComposeConflict || diagnostics[0].Path.String() != "/records" { + t.Fatalf("diagnostics = %#v, want duplicate conflict at /records", diagnostics) + } + if diagnostics[0].Primary == nil || len(diagnostics[0].Related) != 1 { + t.Fatalf("duplicate locations = %#v", diagnostics[0]) + } + for _, secret := range []string{"same", "left", "right", "value"} { + if strings.Contains(diagnostics[0].Message, secret) { + t.Fatalf("diagnostic leaks %q: %q", secret, diagnostics[0].Message) + } + } + }) + } +} + +func TestComposeDocumentsSkipsMissingAndNonSequenceRegistrations(t *testing.T) { + policy := sequencePolicy{identityFields: map[string]string{ + "/absent": "id", "/not-a-sequence": "id", "/catalog": "id", "/catalog/records": "id", + }} + composed, diagnostics := composeDocumentsWithPolicy(policy, parseCompositionDocument(t, "source-1", ` +not-a-sequence: null +catalog: + records: [{id: present}] +`)) + if len(diagnostics) != 0 { + t.Fatalf("composeDocumentsWithPolicy() diagnostics = %#v", diagnostics) + } + if len(composed.sequenceRanges) != 1 || composed.sequenceRanges[0].path.String() != "/catalog/records" { + t.Fatalf("sequence ranges = %#v, want only the reached descendant sequence", composed.sequenceRanges) + } + + composed, diagnostics = composeDocumentsWithPolicy( + sequencePolicy{identityFields: map[string]string{"/records": "id"}}, + parseCompositionDocument(t, "source-1", "records: []\n"), + parseCompositionDocument(t, "source-2", "records: []\n"), + ) + if len(diagnostics) != 0 || len(composed.root.Mapping[0].Value.Sequence) != 0 { + t.Fatalf("empty registered sequences = (%#v, %#v)", composed, diagnostics) + } + if got := composed.sequenceRanges; !reflect.DeepEqual(got, []sequenceRange{ + {sourceIndex: 0, path: Keys("records"), start: 0, end: 0}, + {sourceIndex: 1, path: Keys("records"), start: 0, end: 0}, + }) { + t.Fatalf("empty sequence ranges = %#v", got) + } +} + +func TestComposeDocumentsKeepsUnregisteredSequenceConflicts(t *testing.T) { + _, diagnostics := composeDocumentsWithPolicy( + sequencePolicy{identityFields: map[string]string{"/registered": "id"}}, + parseCompositionDocument(t, "source-1", "unregistered: [one]\n"), + parseCompositionDocument(t, "source-2", "unregistered: [two]\n"), + ) + assertCompositionConflict(t, diagnostics, "/unregistered", []string{"source-1", "source-2"}) +} + +func TestComposeDocumentsKeepsNonSequenceRegisteredOverlapsConflicting(t *testing.T) { + tests := []struct { + name string + first string + second string + }{ + {name: "mapping and sequence", first: "records: {id: one}\n", second: "records: [{id: two}]\n"}, + {name: "null and sequence", first: "records: null\n", second: "records: [{id: two}]\n"}, + {name: "scalar and sequence", first: "records: value\n", second: "records: [{id: two}]\n"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, diagnostics := composeDocumentsWithPolicy( + sequencePolicy{identityFields: map[string]string{"/records": "id"}}, + parseCompositionDocument(t, "source-1", test.first), + parseCompositionDocument(t, "source-2", test.second), + ) + assertCompositionConflict(t, diagnostics, "/records", []string{"source-1", "source-2"}) + }) + } +} + func parseCompositionDocument(t *testing.T, sourceID, input string) *document.Node { t.Helper() parsed, diagnostics := parseSource(Bytes(sourceID, []byte(input)), sourceID, defaultParseLimits())