From a893c7716c0336abe677a7dda6a5db5093691bf9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 26 Oct 2026 14:36:15 +0000 Subject: [PATCH] Preserve explicitness across resolution shape changes --- docs/roadmap/implementation.md | 6 +- internal/engine/load_metadata.go | 21 +++-- internal/engine/load_result.go | 2 +- internal/engine/metadata_test.go | 2 +- internal/engine/override_context_test.go | 2 +- internal/engine/resolution_index_test.go | 6 +- internal/engine/resolution_test.go | 99 ++++++++++++++++++++++++ 7 files changed, 125 insertions(+), 13 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 8e22c50..7fc38e9 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Interface Contract Remediation Implementation Plan -Status: in progress. Stage 1 is complete; Stages 2–18 remain pending. +Status: in progress. Stages 1–2 are complete; Stages 3–18 remain pending. Baseline: `f6d54a094a2b5ada5748686620659399dce7ebf0`. Audience: a Luna coding agent executing the stages sequentially. @@ -315,7 +315,9 @@ and the selected root regression. ## Stage 2 — Correct Final Explicitness Across Shape Changes Finding 2. Owners: `applyResolutionMetadata`, `refreshResolutionFields`, -`resolveLoadInput`, and resolution/metadata tests. Status: pending. +`resolveLoadInput`, and resolution/metadata tests. Status: complete. Validation: +`GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Resolution|Metadata|Provenance' -count=1` +and `GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`. 1. Pass the frozen `resolutionInput` to effective metadata refresh rather than attempting to infer entry shape from a final pointer bucket. diff --git a/internal/engine/load_metadata.go b/internal/engine/load_metadata.go index 8bc936c..6e7b1d2 100644 --- a/internal/engine/load_metadata.go +++ b/internal/engine/load_metadata.go @@ -274,7 +274,7 @@ func cloneMetadata(metadata Metadata) Metadata { return cloned } -func applyResolutionMetadata(metadata *Metadata, resolved resolutionResult) { +func applyResolutionMetadata(metadata *Metadata, resolved resolutionResult, entry resolutionInput) { finalPaths := make(map[string]*document.Node) indexDocument(resolved.final, Path{}, finalPaths) snapshotCoverage, snapshotPointers := newResolutionCoverageIndexFromSnapshots(resolved.snapshots) @@ -303,7 +303,7 @@ func applyResolutionMetadata(metadata *Metadata, resolved resolutionResult) { appendResolutionDescendantHistory(metadata, resolved.snapshots, snapshotPointers, finalPaths, &deltaCoverage, snapshotIDs) applyResolutionFieldOrigins(metadata.fields, resolved.snapshots, &deltaCoverage, snapshotIDs) - refreshResolutionFields(metadata.fields, finalPaths, resolved.snapshots, snapshotCoverage) + refreshResolutionFields(metadata.fields, finalPaths, resolved.final, entry, resolved.snapshots, snapshotCoverage) } func appendResolutionDescendantHistory(metadata *Metadata, snapshots []resolutionSnapshot, snapshotPointers []string, finalPaths map[string]*document.Node, deltaIndex *pathPrefixIndex, snapshotIDs []int) { @@ -368,22 +368,29 @@ func applyResolutionFieldOrigins(fields map[string]FieldInfo, snapshots []resolu } } -func refreshResolutionFields(fields map[string]FieldInfo, finalPaths map[string]*document.Node, snapshots []resolutionSnapshot, snapshotCoverage *resolutionCoverageIndex) { +func refreshResolutionFields(fields map[string]FieldInfo, finalPaths map[string]*document.Node, final *document.Node, entry resolutionInput, snapshots []resolutionSnapshot, snapshotCoverage *resolutionCoverageIndex) { for pointer := range fields { if _, ok := finalPaths[pointer]; !ok { delete(fields, pointer) } } - for pointer, node := range finalPaths { + walkDocument(final, Path{}, func(path Path, node *document.Node) { + pointer := path.String() + entryInfo, entryFound := entry.lookup(path) if info, ok := fields[pointer]; ok { + info.Explicit = entryFound && entryInfo.Explicit info.Null = node.Kind == document.Null fields[pointer] = info - continue + return } origin := Origin{Layer: LayerResolution} if snapshotID, ok := snapshotCoverage.deepestCoveringPointerID(pointer); ok { origin = cloneOrigin(snapshots[snapshotID].change.Origin) } - fields[pointer] = FieldInfo{Null: node.Kind == document.Null, Origin: origin} - } + fields[pointer] = FieldInfo{ + Explicit: entryFound && entryInfo.Explicit, + Null: node.Kind == document.Null, + Origin: origin, + } + }) } diff --git a/internal/engine/load_result.go b/internal/engine/load_result.go index 8956b41..01e38d0 100644 --- a/internal/engine/load_result.go +++ b/internal/engine/load_result.go @@ -67,7 +67,7 @@ func resolveLoadInput[T any](schema *schemaDescriptor, definition Definition[T], } final = resolved.final resolutionSnapshots = resolved.snapshots - applyResolutionMetadata(&metadata, resolved) + applyResolutionMetadata(&metadata, resolved, prepared.resolution) referenceInput = documentWithMetadataLocations(final, metadata, input.document) } applications := []historyApplication(nil) diff --git a/internal/engine/metadata_test.go b/internal/engine/metadata_test.go index 923d463..a798315 100644 --- a/internal/engine/metadata_test.go +++ b/internal/engine/metadata_test.go @@ -269,7 +269,7 @@ func TestResolutionInputKeepsDetachedEntryMetadata(t *testing.T) { if len(resolved.diagnostics) != 0 { t.Fatalf("resolution diagnostics = %#v", resolved.diagnostics) } - applyResolutionMetadata(&metadata, resolved) + applyResolutionMetadata(&metadata, resolved, input) metadata.effectivePaths = effectivePathIndex(resolved.final) again, ok := input.lookup(parsedItem) diff --git a/internal/engine/override_context_test.go b/internal/engine/override_context_test.go index f4ef99f..002a849 100644 --- a/internal/engine/override_context_test.go +++ b/internal/engine/override_context_test.go @@ -120,7 +120,7 @@ func TestPostDecodeOverrideContextOrdersDecodeCLIAndResolutionEvidence(t *testin if len(resolved.diagnostics) != 0 || configuration.Count != 5 || configuration.Derived != 42 { t.Fatalf("resolution = %#v, configuration = %#v", resolved, configuration) } - applyResolutionMetadata(&prepared.metadata, resolved) + applyResolutionMetadata(&prepared.metadata, resolved, prepared.resolution) effectiveReferenceInput := documentWithMetadataLocations(resolved.final, prepared.metadata) if node, ok := documentAtPath(effectiveReferenceInput, Keys("api_key")); !ok || node.Text != "RESOLVED_SECRET" || node.Location.SourceID != cliSource { t.Fatalf("effective reference/location = %#v", node) diff --git a/internal/engine/resolution_index_test.go b/internal/engine/resolution_index_test.go index d331b40..c67490c 100644 --- a/internal/engine/resolution_index_test.go +++ b/internal/engine/resolution_index_test.go @@ -217,6 +217,10 @@ func TestApplyResolutionMetadataPreservesOverlappingDeltaOrder(t *testing.T) { fields: map[string]FieldInfo{"/group/item/leaf": {Origin: oldOrigin}}, history: make(map[string][]Change), } + entry := resolutionInput{metadata: Metadata{ + fields: map[string]FieldInfo{"/group/item/leaf": {Explicit: true, Origin: oldOrigin}}, + effectivePaths: effectivePathIndex(final), + }} resolved := resolutionResult{ final: final, deltas: []documentDelta{{path: Keys("group")}, {path: Keys("group", "item")}}, @@ -226,7 +230,7 @@ func TestApplyResolutionMetadataPreservesOverlappingDeltaOrder(t *testing.T) { }, } - applyResolutionMetadata(metadata, resolved) + applyResolutionMetadata(metadata, resolved, entry) field := metadata.fields["/group/item/leaf"] if field.Origin.Operation != childOrigin.Operation { diff --git a/internal/engine/resolution_test.go b/internal/engine/resolution_test.go index a4d8e21..82c1f5e 100644 --- a/internal/engine/resolution_test.go +++ b/internal/engine/resolution_test.go @@ -390,6 +390,105 @@ func TestResolutionPresenceUsesEntryShapeAcrossDynamicChanges(t *testing.T) { } } +func TestResolutionMetadataExplicitnessUsesEntryShape(t *testing.T) { + t.Parallel() + + type config struct { + Shape any `yaml:"shape"` + } + tests := []struct { + name string + input string + entryPath Path + finalPath Path + shapeChanges bool + mutate func(*config) + wantNull bool + wantOrigin ChangeKind + wantOriginLayer Layer + wantExplicit bool + }{ + { + name: "mapping key becomes sequence index beneath changed ancestor", + input: "shape: {'0': {leaf: old}}\n", + entryPath: Keys("shape", "0", "leaf"), + finalPath: Keys("shape").Index(0).Key("leaf"), + shapeChanges: true, + mutate: func(configuration *config) { configuration.Shape = []any{map[string]any{"leaf": "new"}} }, + wantOrigin: ChangeNormalization, + wantOriginLayer: LayerResolution, + }, + { + name: "sequence index becomes mapping key beneath changed ancestor", + input: "shape: [{leaf: old}]\n", + entryPath: Keys("shape").Index(0).Key("leaf"), + finalPath: Keys("shape", "0", "leaf"), + shapeChanges: true, + mutate: func(configuration *config) { configuration.Shape = map[string]any{"0": map[string]any{"leaf": "new"}} }, + wantOrigin: ChangeNormalization, + wantOriginLayer: LayerResolution, + }, + { + name: "same-shape ancestor normalization retains presence and null", + input: "shape: {nested: {leaf: old}}\n", + entryPath: Keys("shape", "nested", "leaf"), + finalPath: Keys("shape", "nested", "leaf"), + mutate: func(configuration *config) { + configuration.Shape = map[string]any{"nested": map[string]any{"leaf": nil}} + }, + wantNull: true, + wantOrigin: ChangeNormalization, + wantOriginLayer: LayerResolution, + wantExplicit: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var callbackErr error + result, err := testLoad(Bytes("shape-change.yml", []byte(test.input)), Options[config]{Definition: Definition[config]{ + Resolve: func(configuration *config, resolution *Resolution) error { + checkEntry := func() error { + info, found := resolution.InputInfo(test.entryPath) + if !found || !info.Explicit || !resolution.IsExplicit(test.entryPath) { + return errors.New("resolution entry view lost its original shape") + } + if test.shapeChanges { + if _, found := resolution.InputInfo(test.finalPath); found || resolution.IsExplicit(test.finalPath) { + return errors.New("resolution entry view adopted the final shape") + } + } + return nil + } + if err := checkEntry(); err != nil { + callbackErr = err + return err + } + resolution.Record(Keys("shape"), ChangeNormalization, "normalize shape") + test.mutate(configuration) + if err := checkEntry(); err != nil { + callbackErr = err + return err + } + return nil + }, + }}) + if callbackErr != nil { + t.Fatal(callbackErr) + } + if err != nil { + t.Fatal(err) + } + info, found := result.Metadata().Lookup(test.finalPath) + if !found || info.Explicit != test.wantExplicit || result.Metadata().IsExplicit(test.finalPath) != test.wantExplicit { + t.Fatalf("final metadata = %#v, found %v, IsExplicit %v; want explicit %v", info, found, result.Metadata().IsExplicit(test.finalPath), test.wantExplicit) + } + if info.Null != test.wantNull || info.Origin.Layer != test.wantOriginLayer || info.Origin.Operation != test.wantOrigin { + t.Fatalf("final metadata null/origin = %v/%#v", info.Null, info.Origin) + } + }) + } +} + func TestResolutionEntryQueriesExposeDetachedLoadInput(t *testing.T) { t.Parallel()