From 3257b2124c49f94f7cf8f15ed7850c109477d833 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 6 Sep 2026 20:26:57 +0000 Subject: [PATCH] Implement strict document composition --- compose.go | 162 +++++++++++++++++++++++++++ compose_test.go | 194 +++++++++++++++++++++++++++++++++ docs/config.md | 30 +++++ docs/internal/extraction.md | 9 ++ docs/internal/overview.md | 3 + docs/roadmap/implementation.md | 22 +++- docs/roadmap/initial.md | 36 +++--- records.go | 3 + 8 files changed, 438 insertions(+), 21 deletions(-) create mode 100644 compose.go create mode 100644 compose_test.go diff --git a/compose.go b/compose.go new file mode 100644 index 0000000..a591cdc --- /dev/null +++ b/compose.go @@ -0,0 +1,162 @@ +package configkit + +import "gitea.maximumdirect.net/eric/ConfigKit/internal/document" + +// composedDocument is the private handoff between source assembly and later +// 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 +} + +type compositionClaim struct { + path Path + kind document.Kind + location Location +} + +type compositionClaims struct { + ordered []compositionClaim + byPath map[string][]compositionClaim +} + +// 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) { + 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 { + if input == nil || input.Kind != document.Mapping { + var location *Location + if input != nil { + converted := publicLocation(input.Location) + location = &converted + } + return nil, []Diagnostic{compositionInvalidDiagnostic("strict composition requires mapping document roots", location)} + } + collected.append(input, Path{}) + } + + for _, claim := range collected.ordered { + claims := collected.byPath[claim.path.String()] + if len(claims) < 2 || allMappingClaims(claims) { + continue + } + return nil, []Diagnostic{compositionConflictDiagnostic(claim.path, claims)} + } + + root := document.Clone(inputs[0]) + for _, input := range inputs[1:] { + mergeDisjointMappings(root, input) + } + return &composedDocument{ + root: root, + claims: cloneCompositionClaims(collected.ordered), + origins: compositionOrigins(collected.ordered), + }, nil +} + +func (c *compositionClaims) append(node *document.Node, path Path) { + claim := compositionClaim{ + path: path.clone(), + kind: node.Kind, + location: publicLocation(node.Location), + } + c.ordered = append(c.ordered, claim) + pointer := path.String() + c.byPath[pointer] = append(c.byPath[pointer], claim) + + if node.Kind != document.Mapping { + return + } + for _, entry := range node.Mapping { + c.append(entry.Value, path.Key(entry.Key)) + } +} + +func allMappingClaims(claims []compositionClaim) bool { + for _, claim := range claims { + if claim.kind != document.Mapping { + return false + } + } + return true +} + +func mergeDisjointMappings(target, incoming *document.Node) { + for _, incomingEntry := range incoming.Mapping { + index := documentEntryIndex(target.Mapping, incomingEntry.Key) + if index < 0 { + cloned := incomingEntry + cloned.Value = document.Clone(incomingEntry.Value) + target.Mapping = append(target.Mapping, cloned) + continue + } + // Claim validation guarantees that the only remaining overlap is a + // mapping pair. Empty mappings naturally contribute no child entries. + mergeDisjointMappings(target.Mapping[index].Value, incomingEntry.Value) + } +} + +func documentEntryIndex(entries []document.Entry, key string) int { + for index := range entries { + if entries[index].Key == key { + return index + } + } + return -1 +} + +func compositionOrigins(claims []compositionClaim) map[string]Origin { + origins := make(map[string]Origin, len(claims)) + for _, claim := range claims { + pointer := claim.path.String() + origin := origins[pointer] + origin.Layer = LayerBase + origin.Locations = append(origin.Locations, claim.location) + origins[pointer] = origin + } + return origins +} + +func cloneCompositionClaims(claims []compositionClaim) []compositionClaim { + cloned := make([]compositionClaim, len(claims)) + for index, claim := range claims { + cloned[index] = claim + cloned[index].path = claim.path.clone() + } + return cloned +} + +func compositionInvalidDiagnostic(message string, location *Location) Diagnostic { + return Diagnostic{ + Code: CodeComposeInvalid, Severity: SeverityError, Message: message, + Primary: cloneLocation(location), + } +} + +func compositionConflictDiagnostic(path Path, claims []compositionClaim) Diagnostic { + primary := claims[0].location + related := make([]Location, 0, len(claims)-1) + for _, claim := range claims[1:] { + related = append(related, claim.location) + } + return Diagnostic{ + Code: CodeComposeConflict, + Severity: SeverityError, + Path: path.clone(), + Message: "configuration path is defined by multiple sources", + Primary: &primary, + Related: related, + } +} + +func publicLocation(location document.Location) Location { + return Location{SourceID: location.SourceID, Line: location.Line, Column: location.Column} +} diff --git a/compose_test.go b/compose_test.go new file mode 100644 index 0000000..a806eb3 --- /dev/null +++ b/compose_test.go @@ -0,0 +1,194 @@ +package configkit + +import ( + "reflect" + "testing" + + "gitea.maximumdirect.net/eric/ConfigKit/internal/document" +) + +func TestComposeDocumentsJoinsDisjointMappingsAndTracksClaims(t *testing.T) { + base := parseCompositionDocument(t, "source-1", `services: + api: + enabled: true +empty: {} +`) + fragment := parseCompositionDocument(t, "source-2", `services: + worker: + enabled: false +values: [one, two] +`) + baseBefore := document.Clone(base) + fragmentBefore := document.Clone(fragment) + + composed, diagnostics := composeDocuments(base, fragment) + if len(diagnostics) != 0 { + t.Fatalf("composeDocuments() diagnostics = %#v", diagnostics) + } + for _, path := range []Path{ + Keys("services", "api", "enabled"), + Keys("services", "worker", "enabled"), + Keys("empty"), + Keys("values"), + } { + if _, ok := documentAtPath(composed.root, path); !ok { + t.Errorf("composed document is missing %s", path.String()) + } + } + + serviceOrigin := composed.origins[Keys("services").String()] + wantLocations := []Location{ + {SourceID: "source-1", Line: 2, Column: 3}, + {SourceID: "source-2", Line: 2, Column: 3}, + } + if !reflect.DeepEqual(serviceOrigin, Origin{Layer: LayerBase, Locations: wantLocations}) { + t.Fatalf("services origin = %#v, want composite origin", serviceOrigin) + } + wantClaimPaths := []string{"", "/services", "/services/api", "/services/api/enabled", "/empty", "", "/services", "/services/worker", "/services/worker/enabled", "/values"} + gotClaimPaths := make([]string, len(composed.claims)) + for index, claim := range composed.claims { + gotClaimPaths[index] = claim.path.String() + } + if !reflect.DeepEqual(gotClaimPaths, wantClaimPaths) { + t.Fatalf("claim paths = %#v, want %#v", gotClaimPaths, wantClaimPaths) + } + reversed, reversedDiagnostics := composeDocuments(fragment, base) + if len(reversedDiagnostics) != 0 { + t.Fatalf("reversed composeDocuments() diagnostics = %#v", reversedDiagnostics) + } + if deltas := compareDocumentTrees(composed.root, reversed.root); len(deltas) != 0 { + t.Fatalf("input order changed effective values: %#v", deltas) + } + + if !reflect.DeepEqual(base, baseBefore) || !reflect.DeepEqual(fragment, fragmentBefore) { + t.Fatal("composition mutated an input document") + } + composed.root.Mapping[0].Value.Mapping[0].Value.Mapping[0].Value.Text = "changed" + if !reflect.DeepEqual(base, baseBefore) || !reflect.DeepEqual(fragment, fragmentBefore) { + t.Fatal("composed document aliases an input document") + } +} + +func TestComposeDocumentsRejectsEveryPresentNonMappingOverlap(t *testing.T) { + tests := []struct { + name string + left string + right string + path string + }{ + {name: "equal scalar", left: "value: true\n", right: "value: true\n", path: "/value"}, + {name: "different scalar", left: "value: true\n", right: "value: false\n", path: "/value"}, + {name: "null", left: "value: null\n", right: "value: null\n", path: "/value"}, + {name: "sequence", left: "value: [one]\n", right: "value: [two]\n", path: "/value"}, + {name: "mapping and scalar", left: "value:\n nested: true\n", right: "value: text\n", path: "/value"}, + {name: "scalar and mapping", left: "value: text\n", right: "value:\n nested: true\n", path: "/value"}, + {name: "nested keyed entry", left: "items:\n shared:\n left: 1\n", right: "items:\n shared:\n left: 2\n", path: "/items/shared/left"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + left := parseCompositionDocument(t, "source-1", test.left) + right := parseCompositionDocument(t, "source-2", test.right) + _, diagnostics := composeDocuments(left, right) + assertCompositionConflict(t, diagnostics, test.path, []string{"source-1", "source-2"}) + }) + } +} + +func TestComposeDocumentsReportsAllClaimantsAtExactLocations(t *testing.T) { + first := parseCompositionDocument(t, "source-1", "group:\n value: 1\n") + second := parseCompositionDocument(t, "source-2", "group:\n value: 2\n") + third := parseCompositionDocument(t, "source-3", "group:\n value: 3\n") + + _, diagnostics := composeDocuments(first, second, third) + assertCompositionConflict(t, diagnostics, "/group/value", []string{"source-1", "source-2", "source-3"}) + locations := append([]Location{*diagnostics[0].Primary}, diagnostics[0].Related...) + for _, location := range locations { + if location.Line != 2 || location.Column != 10 { + t.Fatalf("claim location = %#v, want line 2 column 10", location) + } + } + + _, diagnostics = composeDocuments( + parseCompositionDocument(t, "source-1", "group:\n left: 1\n"), + parseCompositionDocument(t, "source-2", "group:\n right: 2\n"), + parseCompositionDocument(t, "source-3", "group: blocked\n"), + ) + assertCompositionConflict(t, diagnostics, "/group", []string{"source-1", "source-2", "source-3"}) +} + +func TestComposeDocumentsTreatsEmptyMappingsAsChildlessContributions(t *testing.T) { + empty, diagnostics := composeDocuments( + parseCompositionDocument(t, "source-1", "{}\n"), + parseCompositionDocument(t, "source-2", "{}\n"), + ) + if len(diagnostics) != 0 || len(empty.root.Mapping) != 0 || len(empty.origins[""].Locations) != 2 { + t.Fatalf("empty root composition = (%#v, %#v)", empty, diagnostics) + } + + tests := []struct { + name string + left string + right string + }{ + {name: "two empty mappings", left: "value: {}\n", right: "value: {}\n"}, + {name: "empty then populated", left: "value: {}\n", right: "value: {nested: true}\n"}, + {name: "populated then empty", left: "value: {nested: true}\n", right: "value: {}\n"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + composed, diagnostics := composeDocuments( + parseCompositionDocument(t, "source-1", test.left), + parseCompositionDocument(t, "source-2", test.right), + ) + if len(diagnostics) != 0 { + t.Fatalf("composeDocuments() diagnostics = %#v", diagnostics) + } + if origin := composed.origins["/value"]; len(origin.Locations) != 2 { + t.Fatalf("value origin = %#v, want both mapping locations", origin) + } + if test.name != "two empty mappings" { + if _, ok := documentAtPath(composed.root, Keys("value", "nested")); !ok { + t.Fatal("populated mapping contribution was not retained") + } + } + }) + } +} + +func TestComposeDocumentsRejectsInvalidBoundaries(t *testing.T) { + if composed, diagnostics := composeDocuments(); composed != nil || len(diagnostics) != 1 || diagnostics[0].Code != CodeComposeInvalid { + t.Fatalf("empty composition = (%#v, %#v)", composed, diagnostics) + } + nonMapping := &document.Node{Kind: document.String, Location: document.Location{SourceID: "source-1", Line: 4, Column: 2}} + if composed, diagnostics := composeDocuments(nonMapping); composed != nil || len(diagnostics) != 1 || diagnostics[0].Primary == nil || diagnostics[0].Primary.Line != 4 { + t.Fatalf("non-mapping composition = (%#v, %#v)", composed, diagnostics) + } +} + +func parseCompositionDocument(t *testing.T, sourceID, input string) *document.Node { + t.Helper() + parsed, diagnostics := parseSource(Bytes(sourceID, []byte(input)), sourceID, defaultParseLimits()) + if len(diagnostics) != 0 { + t.Fatalf("parse diagnostics = %#v", diagnostics) + } + return parsed.document +} + +func assertCompositionConflict(t *testing.T, diagnostics []Diagnostic, path string, sources []string) { + t.Helper() + if len(diagnostics) != 1 { + t.Fatalf("diagnostics = %#v, want one conflict", diagnostics) + } + diagnostic := diagnostics[0] + if diagnostic.Code != CodeComposeConflict || diagnostic.Path.String() != path || diagnostic.Primary == nil { + t.Fatalf("diagnostic = %#v, want compose conflict at %s", diagnostic, path) + } + locations := append([]Location{*diagnostic.Primary}, diagnostic.Related...) + gotSources := make([]string, len(locations)) + for index, location := range locations { + gotSources[index] = location.SourceID + } + if !reflect.DeepEqual(gotSources, sources) { + t.Fatalf("claim sources = %#v, want %#v", gotSources, sources) + } +} diff --git a/docs/config.md b/docs/config.md index 85197af..457826c 100644 --- a/docs/config.md +++ b/docs/config.md @@ -150,6 +150,36 @@ zero in returned detached values. Applications retain responsibility for domain rules and runtime initialization; validators should not start clients or depend on live services as a substitute for configuration validation. +## Strict document composition + +ConfigKit's private document layer can strictly compose parsed base documents. The +operation is available for file-graph integration but is not yet reachable through +the public single-source `Load` lifecycle. Application defaults do not participate +in this operation. + +| Existing node | Incoming node | Result | +| --- | --- | --- | +| Absent | Absent | Absent | +| Absent | Present | Retain the incoming node | +| Present | Absent | Retain the existing node | +| Mapping | Mapping | Recurse by mapping key | +| Any other pair of present nodes | Any other pair of present nodes | `compose.conflict` | + +Every non-mapping overlap is a conflict, even when values are equal. Nulls and +sequences are present atomic values, so they also conflict when repeated; sequences +are never joined by index. Disjoint entries under shared mappings compose, making +stable keyed mappings the natural representation for independently contributed +entries. + +An empty mapping contributes no child definitions. Two empty mappings compose, and +an empty mapping neither clears nor conflicts with another mapping's children. +Shared mapping containers retain all contributing value locations as their composite +base origin. A conflict reports the first claim as its primary location and every +remaining claim as related locations, in deterministic source and declaration +traversal order. Traversal order never authorizes replacement. Composition clones +the resulting tree and its ordered claim records rather than mutating or aliasing an +input document. + ## YAML documents The private YAML boundary currently accepts exactly one nonempty document whose root diff --git a/docs/internal/extraction.md b/docs/internal/extraction.md index 741f310..e0c1c20 100644 --- a/docs/internal/extraction.md +++ b/docs/internal/extraction.md @@ -79,6 +79,15 @@ Assigned to document composition: ConfigKit semantic path model; both precise locations and composite origins are retained. Default values never participate in strict composition. +Implementation record (2026-09-06): `composeDocuments`, its pre-order claim +collection, conflict construction, and recursive mapping join in `compose.go` adapt +the assigned production symbols. `TestComposeDocumentsJoinsDisjointMappingsAndTracksClaims`, +`TestComposeDocumentsRejectsEveryPresentNonMappingOverlap`, and +`TestComposeDocumentsReportsAllClaimantsAtExactLocations` adapt the assigned cases. +The planned empty-mapping difference is covered separately: empty mappings retain +container provenance but contribute no child claims. No unplanned semantic deviation +from the extracted additive composer was required. + ### Imports Assigned to explicit import traversal and glob handling: diff --git a/docs/internal/overview.md b/docs/internal/overview.md index adccef7..bbbd679 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -10,6 +10,9 @@ module and package documentation boundary. Its implemented public primitives are - a parser-independent ordered tree in `internal/document`; - the sole upstream YAML integration in `internal/yaml`, which translates validated input into that owned tree and exposes no parser types outside its boundary; +- a private strict composer in `compose.go` that joins only disjoint document-tree + definitions, retains ordered claims and composite mapping origins, reports every + conflicting location, and never aliases an input tree; - a per-operation private reflection descriptor graph in `schema.go` for supported types, YAML field binding, inlining, secret tags, recursion, and semantic paths; - descriptor-driven canonical projection, detached typed cloning, cycle/limit diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 02b8e7a..bed9687 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -802,7 +802,27 @@ Acceptance: ## Stage 10 — Implement strict document composition -Status: pending. +Status: complete. + +Completion evidence (2026-09-06): + +- Adapted Narratio's additive claim validation and recursive mapping join into the + private `composeDocuments` boundary over ConfigKit-owned document trees. Present + equal or unequal scalars, nulls, sequences, mapping/value kind changes, and nested + duplicate entries return `compose.conflict`; mapping pairs alone recurse. +- Changed the extracted empty-map behavior to ConfigKit's operation table: empty + mappings add no children, compose with other mappings, and retain their container + location in composite base origins. Application defaults never enter the composer. +- Preserved source/declaration traversal in detached ordered claim records and used + it only for assembled order and exact primary/related diagnostics. Conflicts across + three or more inputs report every claimant, while shared containers aggregate all + contributing locations instead of selecting an arbitrary owner. +- Cloned every retained branch and verified mutations of a composed result cannot + affect any input. Focused tests cover disjoint keyed joins, operation categories, + ancestor and nested conflicts, equal values, nulls, sequences, empty mappings, + exact locations, all claimants, invalid boundaries, and nonmutation. The + [current composition contract](../config.md#strict-document-composition) owns the + implemented semantics; public import wiring remains deferred to subsequent work. Read: feature [composition table](initial.md#composition-and-overlay). diff --git a/docs/roadmap/initial.md b/docs/roadmap/initial.md index ed69e23..bc47786 100644 --- a/docs/roadmap/initial.md +++ b/docs/roadmap/initial.md @@ -450,28 +450,24 @@ origin is a resolution operation, with input origins retained separately. ## Composition and overlay -Composition combines the main file and explicitly imported fragments as peers. -Repeated semantic definitions are errors, even when their values are equal. -Both conflicting locations must be reported. Prefer mappings keyed by stable -identifiers when entries need to be contributed independently. +Strict base composition is implemented and now owned by the +[current configuration contract](../config.md#strict-document-composition). +Intentional profile overlay remains target behavior. Its exhaustive operation table +is below; “value” includes scalar, sequence, mapping, and null, while absent means no +node rather than null. -The following table is exhaustive by node category. “Value” includes scalar, -sequence, mapping, and null. Absent means no node, not null. +| Existing/lower node | Incoming/higher node | Overlay | +| --- | --- | --- | +| Absent | Absent | Absent | +| Absent | Value | Incoming | +| Value | Absent | Existing | +| Mapping | Mapping | Recurse by key | +| Any other pair of present nodes | Any other pair of present nodes | Replace entire existing node | -| Existing/lower node | Incoming/higher node | Strict composition | Overlay | -| --- | --- | --- | --- | -| Absent | Absent | Absent | Absent | -| Absent | Value | Incoming | Incoming | -| Value | Absent | Existing | Existing | -| Mapping | Mapping | Recurse by key | Recurse by key | -| Any other pair of present nodes | Any other pair of present nodes | Collision | Replace entire existing node | - -Two empty mappings compose successfully. An empty mapping contributes no child -changes to an existing mapping; it does not clear that mapping. A mapping can -replace a lower scalar, sequence, or null during overlay, subject to the final -schema. Sequences are atomic: two definitions collide in composition and the -higher sequence replaces the entire lower sequence in an overlay. `[]` clears a -sequence. There is no append, union, index merge, or mapping-clear operator. +A mapping can replace a lower scalar, sequence, or null during overlay, subject to +the final schema. Sequences are atomic and the higher sequence replaces the entire +lower sequence in an overlay. `[]` clears a sequence. There is no append, union, +index merge, or mapping-clear operator. Replacing a parent must not leave effective metadata for removed children. Sequence histories describe whole-sequence replacement; index origins refer to diff --git a/records.go b/records.go index f79b2a6..e4e24a7 100644 --- a/records.go +++ b/records.go @@ -78,6 +78,9 @@ const ( CodeImportDisabled Code = "import.disabled" + CodeComposeInvalid Code = "compose.invalid" + CodeComposeConflict Code = "compose.conflict" + CodeVersionType Code = "version.type" CodeVersionError Code = "version.error" CodeVersionWarning Code = "version.warning"