diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 56171dc..014fb93 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -414,7 +414,7 @@ Roadmap candidate 8. Owner: engine `inheritance_origin.go`. Validate: `go test ./internal/engine -run 'Inheritance|Inherited|Metadata|Provenance|History'` and the everyday check. Complex contributor-trace equality stays unchanged. -## Stage 17: Index simple resolution and origin membership +## Stage 17: Index simple resolution and origin membership — Complete Roadmap candidate 8. Owners: engine `resolution.go` and `load_metadata.go`. diff --git a/internal/engine/inheritance_origin_test.go b/internal/engine/inheritance_origin_test.go index 2f872c4..bec8a98 100644 --- a/internal/engine/inheritance_origin_test.go +++ b/internal/engine/inheritance_origin_test.go @@ -87,6 +87,51 @@ func TestInheritedOriginsKeepSourcePathsPresenceAndIndependentReplacement(t *tes } } +func TestOriginMembershipPreservesLocationAndSourceCoverageRules(t *testing.T) { + location := Location{SourceID: "source", Line: 4, Column: 2} + otherCoordinates := location + otherCoordinates.Line++ + + container := Origin{ + Layer: LayerOverlay, + Operation: ChangeNormalization, + Locations: []Location{location}, + } + if !originContainedBy(container, container) { + t.Fatal("identical location evidence was not contained") + } + coordinateDifference := container + coordinateDifference.Locations = []Location{otherCoordinates} + if originContainedBy(coordinateDifference, container) { + t.Fatal("location containment ignored differing coordinates") + } + + parent := Keys("parent") + child := parent.Key("child") + change := Change{Path: parent, Origin: container} + descendantOrigin := Origin{ + Layer: container.Layer, + Operation: container.Operation, + Locations: []Location{otherCoordinates}, + } + if !originRepresentedByChange(child, descendantOrigin, change) { + t.Fatal("descendant source coverage rejected matching SourceID with different coordinates") + } + descendantOrigin.Locations[0].SourceID = "another-source" + if originRepresentedByChange(child, descendantOrigin, change) { + t.Fatal("descendant source coverage accepted an unrecorded SourceID") + } + + withContributor := container + withContributor.contributors = []inheritanceContributor{{source: parent, location: location}} + if originContainedBy(withContributor, container) { + t.Fatal("exact location containment ignored the contributor-empty mismatch") + } + if !originRepresentedByChange(child, withContributor, change) { + t.Fatal("descendant source coverage was conflated with contributor containment") + } +} + func TestProjectedInheritanceStepsMatchScanAndRetainOwnership(t *testing.T) { sourcePath := Keys("source", "a/b", "01") firstLocations := []Location{{SourceID: "first;:source", Line: -1, Column: 2}} diff --git a/internal/engine/load_metadata.go b/internal/engine/load_metadata.go index 2a8bf8a..8bc936c 100644 --- a/internal/engine/load_metadata.go +++ b/internal/engine/load_metadata.go @@ -157,15 +157,12 @@ func originRepresentedByChange(path Path, origin Origin, change Change) bool { // A subtree replacement already records its descendants' contributions. // Their coordinates differ from the container's, but their sources must be // covered by the operation; matching only the precedence role is insufficient. + recordedSources := make(map[string]struct{}, len(change.Origin.Locations)) + for _, recorded := range change.Origin.Locations { + recordedSources[recorded.SourceID] = struct{}{} + } for _, location := range origin.Locations { - found := false - for _, recorded := range change.Origin.Locations { - if location.SourceID == recorded.SourceID { - found = true - break - } - } - if !found { + if _, found := recordedSources[location.SourceID]; !found { return false } } @@ -189,15 +186,12 @@ func originContainedBy(candidate, container Origin) bool { if candidate.Layer != container.Layer || candidate.Operation != container.Operation { return false } + availableLocations := make(map[Location]struct{}, len(container.Locations)) + for _, location := range container.Locations { + availableLocations[location] = struct{}{} + } for _, location := range candidate.Locations { - found := false - for _, available := range container.Locations { - if location == available { - found = true - break - } - } - if !found { + if _, found := availableLocations[location]; !found { return false } } diff --git a/internal/engine/resolution.go b/internal/engine/resolution.go index 9075916..b11c520 100644 --- a/internal/engine/resolution.go +++ b/internal/engine/resolution.go @@ -26,6 +26,37 @@ type resolutionRecord struct { reasons []string dependencies []Path helperExpected *document.Node + reasonSet map[string]struct{} + dependencySet map[string]struct{} +} + +func (record *resolutionRecord) addReason(reason string) { + if record.reasonSet == nil { + record.reasonSet = make(map[string]struct{}, len(record.reasons)+1) + for _, existing := range record.reasons { + record.reasonSet[existing] = struct{}{} + } + } + if _, found := record.reasonSet[reason]; found { + return + } + record.reasonSet[reason] = struct{}{} + record.reasons = append(record.reasons, reason) +} + +func (record *resolutionRecord) addDependency(path Path) { + pointer := path.String() + if record.dependencySet == nil { + record.dependencySet = make(map[string]struct{}, len(record.dependencies)+1) + for _, existing := range record.dependencies { + record.dependencySet[existing.String()] = struct{}{} + } + } + if _, found := record.dependencySet[pointer]; found { + return + } + record.dependencySet[pointer] = struct{}{} + record.dependencies = append(record.dependencies, path.clone()) } type resolutionSnapshot struct { @@ -207,9 +238,9 @@ func (state *resolutionState) addRecord(path Path, kind ChangeKind, reason strin if kind == ChangeNormalization { record.kind = ChangeNormalization } - record.reasons = appendUniqueString(record.reasons, reason) + record.addReason(reason) for _, dependency := range dependencies { - record.dependencies = appendUniquePath(record.dependencies, dependency) + record.addDependency(dependency) } if expected != nil { record.helperExpected = document.Clone(expected) diff --git a/internal/engine/resolution_test.go b/internal/engine/resolution_test.go index 19bef35..bb60576 100644 --- a/internal/engine/resolution_test.go +++ b/internal/engine/resolution_test.go @@ -122,6 +122,11 @@ func TestResolutionPreservesExplicitNullAndZeroWhileReplacingAbsentBaseline(t *t func TestResolutionCoalescesEqualAndRepeatedHelperIntent(t *testing.T) { t.Parallel() + parsedOther, err := ParsePath("/other") + if err != nil { + t.Fatal(err) + } + type config struct { Value int `yaml:"value"` Other int `yaml:"other"` @@ -130,8 +135,9 @@ func TestResolutionCoalescesEqualAndRepeatedHelperIntent(t *testing.T) { Defaults: func() config { return config{Value: 5} }, Resolve: func(configuration *config, resolution *Resolution) error { Default(resolution, Keys("value"), &configuration.Value, 5, "equal derived intent", Keys("other")) - Default(resolution, Keys("value"), &configuration.Value, 7, "updated derived intent", Keys("other")) + Default(resolution, Keys("value"), &configuration.Value, 7, "updated derived intent", parsedOther) Normalize(resolution, Keys("value"), &configuration.Value, 7, "canonicalized value", Keys("other")) + Normalize(resolution, Keys("value"), &configuration.Value, 7, "updated derived intent", parsedOther) return nil }, }})