From 26720b98f0ade2b61aba6ae7d626b4dfdb92d225 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 17 Oct 2026 04:44:29 +0000 Subject: [PATCH] Preserve item helper history in source order --- docs/roadmap/remediation.md | 8 ++++- facade_sequence_test.go | 61 +++++++++++++++++++++++++++++++++++++ internal/engine/history.go | 17 ++++++----- internal/engine/imports.go | 21 +++++++------ 4 files changed, 89 insertions(+), 18 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index db99c61..f2de426 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -53,7 +53,13 @@ declaration remains inspectable. Existing item/helper evidence tests still pass. ## 2. Preserve source chronology when expanding item history — P2 -- [ ] Addressed and regression-tested. +- [x] Addressed and regression-tested. + +Completion: structural fragments use their local singleton index; item body and +helper events are expanded beside their declaring contribution. Mapping history +queries ignore unrelated sibling fields. A public regression covers preceding +authored records, two item sources, nested helpers, field origins, and root +traversal order. Focused facade/history tests and the everyday check passed. **Owner:** `projectNestedEvidence` in [imports.go](../../internal/engine/imports.go), especially lines 592–608. diff --git a/facade_sequence_test.go b/facade_sequence_test.go index aa3ed3c..526dc3d 100644 --- a/facade_sequence_test.go +++ b/facade_sequence_test.go @@ -6,10 +6,71 @@ import ( "path/filepath" "strings" "testing" + "testing/fstest" configkit "gitea.maximumdirect.net/eric/ConfigKit" ) +func TestFacadeItemHelperHistoryPreservesTraversalOrder(t *testing.T) { + t.Parallel() + filesystem := configkit.NewFilesystem("history", fstest.MapFS{ + "main.yml": {Data: []byte("records: [{id: authored}]\nconfigkit: {imports: [{path: one.yml, destination: [records], placement: sequence-item}, {path: two.yml, destination: [records], placement: sequence-item}]}\n")}, + "one.yml": {Data: []byte("id: one\nconfigkit: {imports: [{path: helper-one.yml}, {path: child.yml, destination: [children], placement: sequence-item}]}\n")}, + "two.yml": {Data: []byte("id: two\nconfigkit: {imports: [{path: helper-two.yml}]}\n")}, + "helper-one.yml": {Data: []byte("text: first helper\n")}, + "helper-two.yml": {Data: []byte("text: second helper\n")}, + "child.yml": {Data: []byte("id: child\nconfigkit: {imports: [{path: child-helper.yml}]}\n")}, + "child-helper.yml": {Data: []byte("text: nested helper\n")}, + }) + result, err := configkit.Load(filesystem.File("main.yml"), configkit.Options[map[string]any]{ + Imports: true, Definition: configkit.Definition[map[string]any]{SequenceCompositions: []configkit.SequenceComposition{{Path: configkit.Keys("records"), IdentityField: "id"}}}, + }) + if err != nil { + t.Fatal(err) + } + sourceOrder := make(map[string]int) + sourceFiles := make(map[string]string) + for index, source := range result.Sources() { + sourceOrder[source.ID] = index + sourceFiles[source.ID] = source.Path + } + for _, test := range []struct { + path configkit.Path + file, text string + }{ + {configkit.Keys("records").Index(1).Key("text"), "helper-one.yml", "first helper\n"}, + {configkit.Keys("records").Index(2).Key("text"), "helper-two.yml", "second helper\n"}, + {configkit.Keys("records").Index(1).Key("children").Index(0).Key("text"), "child-helper.yml", "nested helper\n"}, + } { + explanation, ok := result.Explain(test.path) + if !ok || explanation.Effective.YAML != test.text || len(explanation.History) != 1 { + t.Fatalf("helper history at %s = %#v", test.path.String(), explanation) + } + locations := explanation.History[0].Origin.Locations + if len(locations) != 1 || sourceFiles[locations[0].SourceID] != test.file { + t.Fatalf("helper origin at %s = %#v", test.path.String(), locations) + } + info, ok := result.Metadata().Lookup(test.path) + if !ok || len(info.Origin.Locations) != 1 || info.Origin.Locations[0].SourceID != locations[0].SourceID { + t.Fatal("metadata and explanation disagree") + } + } + explanation, ok := result.Explain(configkit.Keys()) + if !ok { + t.Fatal("root explanation missing") + } + previous := -1 + for _, change := range explanation.History { + for _, location := range change.Origin.Locations { + current := sourceOrder[location.SourceID] + if current < previous { + t.Fatalf("source %s replayed out of traversal order", location.SourceID) + } + previous = current + } + } +} + func TestFacadeSequencePlacementControlsAcrossLoadEntryPoints(t *testing.T) { definition := configkit.Definition[map[string]any]{ SequenceCompositions: []configkit.SequenceComposition{{Path: configkit.Keys("entries"), IdentityField: "id"}}, diff --git a/internal/engine/history.go b/internal/engine/history.go index 1c31353..735b468 100644 --- a/internal/engine/history.go +++ b/internal/engine/history.go @@ -125,8 +125,8 @@ func applicationOperationPath(application historyApplication, query Path, lookup if !application.path.valid() || application.value == nil { return Path{}, false } - if application.path.String() != "" { - if pathsIntersect(application.path, query) { + if !pathContains(application.path, query) { + if pathContains(query, application.path) { return application.path.clone(), true } return Path{}, false @@ -148,17 +148,18 @@ func applicationOperationPath(application historyApplication, query Path, lookup if sequencePathMatched { return Path{}, false } - if len(query.segments) == 0 { - return Path{}, true + if len(query.segments) == len(application.path.segments) || application.value.Kind != document.Mapping { + return application.path.clone(), true } - for length := 1; length <= len(query.segments); length++ { - prefix := Path{segments: clonePathSegments(query.segments[:length], 0)} + relative, _ := relativeHistoryPath(application.path, query) + for length := 1; length <= len(relative.segments); length++ { + prefix := Path{segments: clonePathSegments(relative.segments[:length], 0)} node, present := documentAtPathWithLookup(application.value, prefix, lookup) if !present { return Path{}, false } - if length == len(query.segments) || node.Kind != document.Mapping { - return prefix, true + if length == len(relative.segments) || node.Kind != document.Mapping { + return appendDestination(application.path, prefix), true } } return Path{}, false diff --git a/internal/engine/imports.go b/internal/engine/imports.go index 75f6942..fd7ad17 100644 --- a/internal/engine/imports.go +++ b/internal/engine/imports.go @@ -578,8 +578,11 @@ func projectNestedEvidence(composed *composedDocument, scope acquisitionScope) { if composed == nil { return } + applications := composed.applications + projectedApplications := make([]historyApplication, 0, len(applications)) for sourceIndex, contribution := range scope.contributions { if contribution.nested == nil { + projectedApplications = append(projectedApplications, applications[sourceIndex]) continue } start := 0 @@ -590,26 +593,26 @@ func projectNestedEvidence(composed *composedDocument, scope acquisitionScope) { } } prefix := contribution.destination.Index(start) - if sourceIndex < len(composed.applications) { - outer := composed.applications[sourceIndex] + if sourceIndex < len(applications) { + outer := applications[sourceIndex] structural := document.Clone(outer.value) - if node, found := documentAtPath(structural, contribution.destination); found && node.Kind == document.Sequence && len(node.Sequence) > start { - node.Sequence[start] = &document.Node{Kind: document.Mapping, Location: node.Sequence[start].Location} + if node, found := documentAtPath(structural, contribution.destination); found && node.Kind == document.Sequence && len(node.Sequence) > 0 { + // The retained fragment is still a local singleton. Its assigned + // outer index belongs to the projection, not this source tree. + node.Sequence[0] = &document.Node{Kind: document.Mapping, Location: node.Sequence[0].Location} } outer.value = structural - composed.applications[sourceIndex] = outer + projectedApplications = append(projectedApplications, outer) for _, nested := range contribution.nested.applications { projected := nested projected.path = appendDestination(prefix, nested.path) projected.sequences = cloneSequenceHistoryContributions(nested.sequences) - for index := range projected.sequences { - projected.sequences[index].path = appendDestination(prefix, projected.sequences[index].path) - } - composed.applications = append(composed.applications, projected) + projectedApplications = append(projectedApplications, projected) } } projectNestedOrigins(composed.origins, contribution.nested.root, contribution.nested.origins, prefix) } + composed.applications = projectedApplications } func projectNestedOrigins(target map[string]Origin, root *document.Node, origins map[string]Origin, prefix Path) {