From 7b2dc2f2c975abe2708945097f5dd4802313215e Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 7 Nov 2026 12:14:11 +0000 Subject: [PATCH] Prepare subtree filter prefixes once per discovery --- docs/roadmap/remediation.md | 13 ++++++- integrations/ckconfig/discovery_test.go | 52 ++++++++++++++++++++++--- integrations/ckconfig/explain.go | 24 +++++++----- 3 files changed, 73 insertions(+), 16 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 93db99c..75fd8ee 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Shared reporting and PromptKit adoption remediation -Status: one open finding; R1–R6 complete. R1–R3 are from the architectural review of +Status: all seven findings complete. R1–R3 are from the architectural review of `f83866e44a8a9066db52cbd2aa5cccde0d69a839`. R4–R7 are from the algorithmic audit of `f5b2d1f80c4720b762f3301c29d5050710c210d0`; R4 is a correctness issue and R5–R7 are efficiency opportunities for consideration. @@ -284,6 +284,13 @@ and forwarding, not the choice of sorting or merging mechanism. ## R7 — P3: Path discovery reparses its invariant subtree prefix +**Complete.** Discovery serializes and tokenizes the requested prefix once, +and serializes each child once. Filtering now takes O(U+L) conversion/matching +work and O(U) retained prefix storage, excluding native metadata and report work. +Command checks cover root, escaped slash/tilde, exact paths, descendants, sibling +prefixes, long unmatched prefixes, and historical/explicitness composition. The +independent ckconfig and complete aligned workspace suites pass with Go 1.27.0. + **Location:** [explain.go](../../integrations/ckconfig/explain.go), runConfigExplanation's discovery loop and pathWithin lines 177–194. @@ -309,6 +316,10 @@ not the planned native pointer semantics or discovery algorithm. ## Applying the fixes +All completion markers describe fixes in the current source. Publication and +released dependency alignment remain separate release work. The original +observations above retain the evidence from the audited baseline. + Keep fixes and regression coverage at the CommandKit adaptation/reporting boundaries under the [testing policy](../policy/testing.md). These findings do not call for reimplementation or independent verification of upstream domain diff --git a/integrations/ckconfig/discovery_test.go b/integrations/ckconfig/discovery_test.go index 15fe861..a0eb7f4 100644 --- a/integrations/ckconfig/discovery_test.go +++ b/integrations/ckconfig/discovery_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "io" + "reflect" "strings" "testing" @@ -16,7 +17,7 @@ func TestExplainDiscoveryModesAndMetadata(t *testing.T) { loads, inputs, states := 0, 0, 0 scope := FromConfigKit("app", "", func(_ context.Context, selection Selection, _, _ struct{}) (*configkit.Result[map[string]any], error) { loads++ - return configkit.Load(configkit.Mapping("fixture", map[string]any{"a/b": map[string]any{"x": 1}, "ab": 2, "abacus": 3, "gone": map[string]any{"x": 1}}), configkit.Options[map[string]any]{Overrides: selection.Overrides}) + return configkit.Load(configkit.Mapping("fixture", map[string]any{"a/b": map[string]any{"x": 1}, "a~b": 4, "ab": 2, "abacus": 3, "gone": map[string]any{"x": 1}}), configkit.Options[map[string]any]{Overrides: selection.Overrides}) }) scope.Features.Overrides = true app := newConfigTestApp(t, Command(Suite[struct{}, struct{}]{NewInput: func() struct{} { inputs++; return struct{}{} }, NewState: func() struct{} { states++; return struct{}{} }, Scopes: []Scope[struct{}, struct{}]{scope}})) @@ -37,7 +38,11 @@ func TestExplainDiscoveryModesAndMetadata(t *testing.T) { } result, out = run("--list", "--under=/a~1b", "--format=json") var report pathDiscoveryReport - if result.Kind != commandkit.OutcomeSuccess || json.Unmarshal([]byte(out), &report) != nil || len(report.Paths) != 2 { + decode := func(output string) error { + report = pathDiscoveryReport{} + return json.Unmarshal([]byte(output), &report) + } + if result.Kind != commandkit.OutcomeSuccess || decode(out) != nil || len(report.Paths) != 2 { t.Fatalf("escaped subtree: %#v %q", result, out) } seen := map[string]bool{} @@ -48,11 +53,46 @@ func TestExplainDiscoveryModesAndMetadata(t *testing.T) { t.Fatalf("escaped paths: %#v", report.Paths) } result, out = run("--list", "--under=/ab", "--format=json") - if result.Kind != commandkit.OutcomeSuccess || json.Unmarshal([]byte(out), &report) != nil || len(report.Paths) != 1 || report.Paths[0].Path != "/ab" { + if result.Kind != commandkit.OutcomeSuccess || decode(out) != nil || len(report.Paths) != 1 || report.Paths[0].Path != "/ab" { t.Fatalf("token boundary: %#v %q", result, out) } + + for _, tc := range []struct { + prefix string + paths []string + }{ + {"/a~1b/x", []string{"/a~1b/x"}}, + {"/a~0b", []string{"/a~0b"}}, + {"/a", nil}, + {"/a~1b/x/unknown", nil}, + {"/ab/" + strings.Repeat("too-deep/", 20) + "unknown", nil}, + } { + result, out = run("--list", "--under="+tc.prefix, "--format=json") + if result.Status != 0 || decode(out) != nil { + t.Fatalf("prefix %q: %#v %s", tc.prefix, result, out) + } + var paths []string + for _, entry := range report.Paths { + paths = append(paths, entry.Path) + } + if !reflect.DeepEqual(paths, tc.paths) { + t.Fatalf("prefix %q: paths=%v want=%v", tc.prefix, paths, tc.paths) + } + } + result, baseline := run("--list", "--format=json") + if result.Status != 0 { + t.Fatalf("unfiltered discovery=%#v", result) + } + var all pathDiscoveryReport + if err := json.Unmarshal([]byte(baseline), &all); err != nil { + t.Fatal(err) + } + result, out = run("--list", "--under=", "--format=json") + if result.Status != 0 || decode(out) != nil || !reflect.DeepEqual(report, all) { + t.Fatalf("root subtree changed discovery: %#v %s", result, out) + } result, out = run("--list", "--historical", "--set=/gone=null", "--format=json") - if result.Kind != commandkit.OutcomeSuccess || json.Unmarshal([]byte(out), &report) != nil { + if result.Kind != commandkit.OutcomeSuccess || decode(out) != nil { t.Fatalf("historical: %#v %q", result, out) } removed := false @@ -64,8 +104,8 @@ func TestExplainDiscoveryModesAndMetadata(t *testing.T) { if !removed { t.Fatalf("missing historical entry: %q", out) } - result, out = run("--list", "--historical", "--explicit=false", "--set=/gone=null", "--format=json") - if result.Kind != commandkit.OutcomeSuccess || json.Unmarshal([]byte(out), &report) != nil { + result, out = run("--list", "--under=/gone", "--historical", "--explicit=false", "--set=/gone=null", "--format=json") + if result.Kind != commandkit.OutcomeSuccess || decode(out) != nil { t.Fatalf("explicit filter: %#v %q", result, out) } for _, entry := range report.Paths { diff --git a/integrations/ckconfig/explain.go b/integrations/ckconfig/explain.go index 27dfa52..0661683 100644 --- a/integrations/ckconfig/explain.go +++ b/integrations/ckconfig/explain.go @@ -130,8 +130,13 @@ func runConfigExplanation[I, S any](invocation commandkit.Invocation, plan *conf return configFailure("Configuration scope did not provide path metadata.", acquired.Explanation, fmt.Errorf("configuration scope %q declared path discovery without metadata", name)) } report := pathDiscoveryReport{Scope: name, Paths: []pathDiscoveryEntry{}} + var prefix []string + if plan.under != nil { + prefix = pointerTokens(plan.under.String()) + } for _, path := range acquired.Value.Metadata.HistoricalPaths() { - if plan.under != nil && !pathWithin(path, *plan.under) { + pointer := path.String() + if plan.under != nil && !pathWithin(pointer, prefix) { continue } info, present := acquired.Value.Metadata.Lookup(path) @@ -141,7 +146,7 @@ func runConfigExplanation[I, S any](invocation commandkit.Invocation, plan *conf if present && plan.explicit.Set && info.Explicit != plan.explicit.Value { continue } - entry := pathDiscoveryEntry{Path: path.String(), Present: present} + entry := pathDiscoveryEntry{Path: pointer, Present: present} if present { entry.Metadata = &pathMetadata{Explicit: info.Explicit} } @@ -192,14 +197,15 @@ func runConfigExplanation[I, S any](invocation commandkit.Invocation, plan *conf return commandkit.Success() } -func pathWithin(path, parent configkit.Path) bool { - tokens := func(value string) []string { - if value == "" { - return nil - } - return strings.Split(strings.TrimPrefix(value, "/"), "/") +func pointerTokens(value string) []string { + if value == "" { + return nil } - child, prefix := tokens(path.String()), tokens(parent.String()) + return strings.Split(strings.TrimPrefix(value, "/"), "/") +} + +func pathWithin(path string, prefix []string) bool { + child := pointerTokens(path) if len(prefix) > len(child) { return false }