From 53b79528d90bb20363cb8ed6e21568b7733c0a02 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 9 Oct 2026 04:14:45 +0000 Subject: [PATCH] Keep reference algorithms in tests --- docs/roadmap/remediation.md | 11 ++- internal/engine/diagnostic.go | 4 - internal/engine/path.go | 12 --- internal/engine/path_test.go | 40 ---------- internal/engine/resolution.go | 27 ------- internal/engine/resolution_index_test.go | 93 ++++++++++++++++++++---- 6 files changed, 87 insertions(+), 100 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index b7ec80f..612080b 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -8,8 +8,8 @@ membership maps broadly match the selected architecture. Whole-tree replay and the expressly deferred ownership/trace changes remain unchanged. The review found no reproduced functional regression or unintentionally stronger -public contract. The following bounded implementation and test-quality findings -remain to be addressed. They do not authorize changes to public semantics, +public contract. All three bounded implementation and test-quality findings +below are now addressed. They do not authorize changes to public semantics, dependencies, resource limits, release gates, or credential handling. The older, completed ReleaseKit review record remains in Git history at @@ -78,7 +78,12 @@ is missing regression protection, not an observed sensitivity leak. ## 3. Finish moving reference-only machinery out of production (P3) -Status: Open. +Status: Complete. Reference containment/deduplication is String-based and +test-only. The unused constructor/comparator/path helpers are removed; coverage +tests exercise both live record/snapshot adapters, and dependency membership +tests exercise the actual resolution record. Required live helpers and immutable +inspection evidence are unchanged. Focused tests, `./scripts/check.sh`, and +`go test -race ./...` passed; code search found no remaining obsolete symbols. Affected: [resolution.go](../../internal/engine/resolution.go), `newResolutionCoverageIndex` (lines 373–382), `appendUniqueString`, diff --git a/internal/engine/diagnostic.go b/internal/engine/diagnostic.go index 37e37cd..a77d846 100644 --- a/internal/engine/diagnostic.go +++ b/internal/engine/diagnostic.go @@ -102,10 +102,6 @@ func canonicalDiagnostics(diagnostics []Diagnostic) []Diagnostic { return canonical } -func compareDiagnostic(left, right Diagnostic) int { - return compareDiagnosticWithPaths(left, right, left.Path.String(), right.Path.String()) -} - func compareDiagnosticWithPaths(left, right Diagnostic, leftPath, rightPath string) int { if order := compareOptionalLocation(left.Primary, right.Primary); order != 0 { return order diff --git a/internal/engine/path.go b/internal/engine/path.go index a4bef1b..ba21e0e 100644 --- a/internal/engine/path.go +++ b/internal/engine/path.go @@ -155,18 +155,6 @@ func pathIndexSpellingEqual(index int, token string) bool { return true } -func pathSpellingEqual(left, right Path) bool { - if len(left.segments) != len(right.segments) { - return false - } - for i, segment := range left.segments { - if !pathTokenSpellingEqual(segment, right.segments[i]) { - return false - } - } - return true -} - func escapePathToken(token string) string { token = strings.ReplaceAll(token, "~", "~0") return strings.ReplaceAll(token, "/", "~1") diff --git a/internal/engine/path_test.go b/internal/engine/path_test.go index 9d4ab88..c91c984 100644 --- a/internal/engine/path_test.go +++ b/internal/engine/path_test.go @@ -96,37 +96,6 @@ func TestPathContainmentMatchesPointerSpelling(t *testing.T) { } } -func TestAppendUniquePathMatchesPointerSpelling(t *testing.T) { - parse := func(pointer string) Path { - path, err := ParsePath(pointer) - if err != nil { - t.Fatalf("ParsePath(%q) error = %v", pointer, err) - } - return path - } - paths := []Path{ - Keys("a/b"), parse("/a~1b"), - Keys("a~1b"), parse("/a~01b"), - Keys("items").Index(0), parse("/items/0"), Keys("items", "0"), - Keys("items", "01"), parse("/items/01"), - Path{}.Index(-1), Keys("-1"), - Keys("a", "b"), Keys("a/b"), - } - var got, want []Path - for _, path := range paths { - got = appendUniquePath(got, path) - want = appendUniquePathByPointer(want, path) - } - if len(got) != len(want) { - t.Fatalf("unique path counts = %d, want %d", len(got), len(want)) - } - for i := range got { - if got[i].String() != want[i].String() { - t.Fatalf("unique path %d = %q, want %q", i, got[i].String(), want[i].String()) - } - } -} - func pathContainsByPointer(container, descendant Path) bool { if !container.valid() || !descendant.valid() || len(container.segments) > len(descendant.segments) { return false @@ -139,15 +108,6 @@ func pathContainsByPointer(container, descendant Path) bool { return value == prefix || len(value) > len(prefix) && value[:len(prefix)] == prefix && value[len(prefix)] == '/' } -func appendUniquePathByPointer(paths []Path, path Path) []Path { - for _, existing := range paths { - if existing.String() == path.String() { - return paths - } - } - return append(paths, path.clone()) -} - func TestPathPreservesSegmentIntentAndImmutability(t *testing.T) { t.Parallel() diff --git a/internal/engine/resolution.go b/internal/engine/resolution.go index b11c520..e00546d 100644 --- a/internal/engine/resolution.go +++ b/internal/engine/resolution.go @@ -370,17 +370,6 @@ type resolutionCoverageIndex struct { depths []int } -func newResolutionCoverageIndex(paths []Path) *resolutionCoverageIndex { - index := &resolutionCoverageIndex{depths: make([]int, len(paths))} - for id, path := range paths { - index.depths[id] = len(path.segments) - if path.valid() { - index.prefix.insert(path.String(), id) - } - } - return index -} - func newResolutionCoverageIndexFromRecords(records []*resolutionRecord) *resolutionCoverageIndex { index := &resolutionCoverageIndex{depths: make([]int, len(records))} for id, record := range records { @@ -436,22 +425,6 @@ func pathContains(container, descendant Path) bool { return true } -func appendUniqueString(values []string, value string) []string { - if !slices.Contains(values, value) { - return append(values, value) - } - return values -} - -func appendUniquePath(paths []Path, path Path) []Path { - for _, existing := range paths { - if pathSpellingEqual(existing, path) { - return paths - } - } - return append(paths, path.clone()) -} - func clonePaths(paths []Path) []Path { if paths == nil { return nil diff --git a/internal/engine/resolution_index_test.go b/internal/engine/resolution_index_test.go index 701c1f8..d331b40 100644 --- a/internal/engine/resolution_index_test.go +++ b/internal/engine/resolution_index_test.go @@ -2,6 +2,7 @@ package engine import ( "reflect" + "slices" "testing" "gitea.maximumdirect.net/eric/ConfigKit/internal/document" @@ -36,14 +37,14 @@ func coalescedResolutionSnapshotsByScan(snapshots []resolutionSnapshot) []resolu for _, snapshot := range snapshots { merged := false for i := range groups { - if !pathContains(groups[i].change.Path, snapshot.change.Path) { + if !pathContainsByPointer(groups[i].change.Path, snapshot.change.Path) { continue } for _, reason := range snapshot.change.Reasons { - groups[i].change.Reasons = appendUniqueString(groups[i].change.Reasons, reason) + groups[i].change.Reasons = appendUniqueStringByScan(groups[i].change.Reasons, reason) } for _, dependency := range snapshot.change.Dependencies { - groups[i].change.Dependencies = appendUniquePath(groups[i].change.Dependencies, dependency) + groups[i].change.Dependencies = appendUniquePathByPointer(groups[i].change.Dependencies, dependency) } if snapshot.change.Origin.Operation == ChangeNormalization { groups[i].change.Origin.Operation = ChangeNormalization @@ -59,6 +60,49 @@ func coalescedResolutionSnapshotsByScan(snapshots []resolutionSnapshot) []resolu return groups } +func appendUniqueStringByScan(values []string, value string) []string { + if !slices.Contains(values, value) { + return append(values, value) + } + return values +} + +func appendUniquePathByPointer(paths []Path, path Path) []Path { + for _, existing := range paths { + if existing.String() == path.String() { + return paths + } + } + return append(paths, path.clone()) +} + +func TestResolutionDependencyMembershipMatchesPointerSpelling(t *testing.T) { + parse := func(pointer string) Path { + path, err := ParsePath(pointer) + if err != nil { + t.Fatalf("ParsePath(%q) error = %v", pointer, err) + } + return path + } + paths := []Path{ + Keys("a/b"), parse("/a~1b"), + Keys("a~1b"), parse("/a~01b"), + Keys("items").Index(0), parse("/items/0"), Keys("items", "0"), + Keys("items", "01"), parse("/items/01"), + Keys("-1"), parse("/-1"), + Keys("a", "b"), Keys("a/b"), + } + var record resolutionRecord + var want []Path + for _, path := range paths { + record.addDependency(path) + want = appendUniquePathByPointer(want, path) + } + if !reflect.DeepEqual(record.dependencies, want) { + t.Fatalf("resolution dependencies = %#v, want %#v", record.dependencies, want) + } +} + func TestResolutionCoverageIndexMatchesOrderedScan(t *testing.T) { paths := []Path{ Keys("group"), @@ -67,7 +111,18 @@ func TestResolutionCoverageIndexMatchesOrderedScan(t *testing.T) { Keys("group", "field", "nested"), Keys("other"), } - index := newResolutionCoverageIndex(paths) + records := make([]*resolutionRecord, len(paths)) + snapshots := make([]resolutionSnapshot, len(paths)) + for id, path := range paths { + records[id] = &resolutionRecord{path: path} + snapshots[id] = resolutionSnapshot{change: Change{Path: path}} + } + snapshotIndex, pointers := newResolutionCoverageIndexFromSnapshots(snapshots) + for id, pointer := range pointers { + if pointer != paths[id].String() { + t.Errorf("snapshot pointer %d = %q, want %q", id, pointer, paths[id].String()) + } + } queries := []Path{ Path{}, Keys("group"), @@ -77,17 +132,27 @@ func TestResolutionCoverageIndexMatchesOrderedScan(t *testing.T) { Keys("other", "child"), Keys("group").Index(-1), } - for _, query := range queries { - wantID, wantDepth, wantFound := 0, -1, false - for id, candidate := range paths { - if pathContains(candidate, query) && (!wantFound || len(candidate.segments) > wantDepth) { - wantID, wantDepth, wantFound = id, len(candidate.segments), true + for _, test := range []struct { + name string + index *resolutionCoverageIndex + }{ + {"records", newResolutionCoverageIndexFromRecords(records)}, + {"snapshots", snapshotIndex}, + } { + t.Run(test.name, func(t *testing.T) { + for _, query := range queries { + wantID, wantDepth, wantFound := 0, -1, false + for id, candidate := range paths { + if pathContainsByPointer(candidate, query) && (!wantFound || len(candidate.segments) > wantDepth) { + wantID, wantDepth, wantFound = id, len(candidate.segments), true + } + } + gotID, gotFound := test.index.deepestCoveringID(query) + if gotID != wantID || gotFound != wantFound { + t.Errorf("deepestCoveringID(%s) = %d, %v; want %d, %v", query.String(), gotID, gotFound, wantID, wantFound) + } } - } - gotID, gotFound := index.deepestCoveringID(query) - if gotID != wantID || gotFound != wantFound { - t.Errorf("deepestCoveringID(%s) = %d, %v; want %d, %v", query.String(), gotID, gotFound, wantID, wantFound) - } + }) } }