From 25382a112bca3ce7bfe9fee0d5599fda9e80725d Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 2 Nov 2026 17:24:45 +0000 Subject: [PATCH] Preserve matched history for removed fields --- docs/roadmap/remediation.md | 9 +++- internal/engine/metadata.go | 2 +- .../engine/resolution_remediation_test.go | 42 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 internal/engine/resolution_remediation_test.go diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 3ccbcf8..7254d47 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -24,7 +24,14 @@ is not duplicated below. ## Finding 1 — Metadata history rejects a valid matched historical field -**Priority:** P2. **Status:** Open. +**Priority:** P2. **Status:** Complete. + +Completion: metadata history now delegates correspondence-aware eligibility to +the shared selector. A regression verifies agreement with explanation for a +removed field absent from the previous final-slot occupant, while effective +lookup, unknown paths, and explicit key/index distinctions remain correct. +The regression failed before the fix; focused tests and `./scripts/check.sh` +passed after it. **Code:** [engine metadata](../../internal/engine/metadata.go), `Metadata.History` at lines 50–55; compare diff --git a/internal/engine/metadata.go b/internal/engine/metadata.go index d42a43a..d48ad07 100644 --- a/internal/engine/metadata.go +++ b/internal/engine/metadata.go @@ -49,7 +49,7 @@ func (m Metadata) effectiveField(path Path) (FieldInfo, bool) { // Unknown and invalid paths return nil. func (m Metadata) History(path Path) []Change { model := m.historyEvidence() - if model == nil || !model.hasObservedShape(path) { + if model == nil { return nil } selections := model.selectHistory(path) diff --git a/internal/engine/resolution_remediation_test.go b/internal/engine/resolution_remediation_test.go new file mode 100644 index 0000000..29c72e5 --- /dev/null +++ b/internal/engine/resolution_remediation_test.go @@ -0,0 +1,42 @@ +package engine + +import "testing" + +func TestMovedRemovedFieldHasConsistentHistory(t *testing.T) { + type config struct { + Items []map[string]any `yaml:"items"` + } + definition := DefinitionWithSequenceIdentityRegistrations(Definition[config]{ + Resolve: func(value *config, r *Resolution) error { + delete(value.Items[0], "extra") + value.Items[0], value.Items[1] = value.Items[1], value.Items[0] + r.RecordNormalization(Keys("items"), "remove field and reorder") + return nil + }, + }, []SequenceIdentityRegistrationInput{{ExactPath: pathPointer(Keys("items")), IdentityFields: []string{"id"}}}) + result, err := testLoad(Bytes("records", []byte("items: [{id: a, extra: own}, {id: b}]\n")), Options[config]{Definition: definition}) + if err != nil { + t.Fatal(err) + } + path := Keys("items").Index(1).Key("extra") + if _, found := result.Metadata().Lookup(path); found { + t.Fatal("removed field is effective") + } + explanation, found := result.Inspection.Explain(path) + if !found || len(explanation.History) != 2 { + t.Fatalf("explanation = %#v, %t", explanation, found) + } + history := result.Metadata().History(path) + if len(history) != 2 || history[0].Origin.Layer != LayerMain || history[1].Origin.Operation != ChangeNormalization { + t.Fatalf("metadata history = %#v", history) + } + last := explanation.History[1] + if last.Before.YAML != "own\n" || last.After.Present { + t.Fatalf("removal = %#v", last) + } + for _, unknown := range []Path{Keys("items", "1", "extra"), Keys("items").Index(1).Key("unknown")} { + if history := result.Metadata().History(unknown); len(history) != 0 { + t.Fatalf("unexpected history for %s: %#v", unknown.String(), history) + } + } +}