Prepare subtree filter prefixes once per discovery
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# Shared reporting and PromptKit adoption remediation
|
# 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
|
`f83866e44a8a9066db52cbd2aa5cccde0d69a839`. R4–R7 are from the algorithmic
|
||||||
audit of `f5b2d1f80c4720b762f3301c29d5050710c210d0`; R4 is a correctness
|
audit of `f5b2d1f80c4720b762f3301c29d5050710c210d0`; R4 is a correctness
|
||||||
issue and R5–R7 are efficiency opportunities for consideration.
|
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
|
## 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),
|
**Location:** [explain.go](../../integrations/ckconfig/explain.go),
|
||||||
runConfigExplanation's discovery loop and pathWithin lines 177–194.
|
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
|
## 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
|
Keep fixes and regression coverage at the CommandKit adaptation/reporting
|
||||||
boundaries under the [testing policy](../policy/testing.md). These findings do
|
boundaries under the [testing policy](../policy/testing.md). These findings do
|
||||||
not call for reimplementation or independent verification of upstream domain
|
not call for reimplementation or independent verification of upstream domain
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"io"
|
"io"
|
||||||
|
"reflect"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
@@ -16,7 +17,7 @@ func TestExplainDiscoveryModesAndMetadata(t *testing.T) {
|
|||||||
loads, inputs, states := 0, 0, 0
|
loads, inputs, states := 0, 0, 0
|
||||||
scope := FromConfigKit("app", "", func(_ context.Context, selection Selection, _, _ struct{}) (*configkit.Result[map[string]any], error) {
|
scope := FromConfigKit("app", "", func(_ context.Context, selection Selection, _, _ struct{}) (*configkit.Result[map[string]any], error) {
|
||||||
loads++
|
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
|
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}}))
|
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")
|
result, out = run("--list", "--under=/a~1b", "--format=json")
|
||||||
var report pathDiscoveryReport
|
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)
|
t.Fatalf("escaped subtree: %#v %q", result, out)
|
||||||
}
|
}
|
||||||
seen := map[string]bool{}
|
seen := map[string]bool{}
|
||||||
@@ -48,11 +53,46 @@ func TestExplainDiscoveryModesAndMetadata(t *testing.T) {
|
|||||||
t.Fatalf("escaped paths: %#v", report.Paths)
|
t.Fatalf("escaped paths: %#v", report.Paths)
|
||||||
}
|
}
|
||||||
result, out = run("--list", "--under=/ab", "--format=json")
|
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)
|
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")
|
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)
|
t.Fatalf("historical: %#v %q", result, out)
|
||||||
}
|
}
|
||||||
removed := false
|
removed := false
|
||||||
@@ -64,8 +104,8 @@ func TestExplainDiscoveryModesAndMetadata(t *testing.T) {
|
|||||||
if !removed {
|
if !removed {
|
||||||
t.Fatalf("missing historical entry: %q", out)
|
t.Fatalf("missing historical entry: %q", out)
|
||||||
}
|
}
|
||||||
result, out = run("--list", "--historical", "--explicit=false", "--set=/gone=null", "--format=json")
|
result, out = run("--list", "--under=/gone", "--historical", "--explicit=false", "--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("explicit filter: %#v %q", result, out)
|
t.Fatalf("explicit filter: %#v %q", result, out)
|
||||||
}
|
}
|
||||||
for _, entry := range report.Paths {
|
for _, entry := range report.Paths {
|
||||||
|
|||||||
@@ -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))
|
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{}}
|
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() {
|
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
|
continue
|
||||||
}
|
}
|
||||||
info, present := acquired.Value.Metadata.Lookup(path)
|
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 {
|
if present && plan.explicit.Set && info.Explicit != plan.explicit.Value {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
entry := pathDiscoveryEntry{Path: path.String(), Present: present}
|
entry := pathDiscoveryEntry{Path: pointer, Present: present}
|
||||||
if present {
|
if present {
|
||||||
entry.Metadata = &pathMetadata{Explicit: info.Explicit}
|
entry.Metadata = &pathMetadata{Explicit: info.Explicit}
|
||||||
}
|
}
|
||||||
@@ -192,14 +197,15 @@ func runConfigExplanation[I, S any](invocation commandkit.Invocation, plan *conf
|
|||||||
return commandkit.Success()
|
return commandkit.Success()
|
||||||
}
|
}
|
||||||
|
|
||||||
func pathWithin(path, parent configkit.Path) bool {
|
func pointerTokens(value string) []string {
|
||||||
tokens := func(value string) []string {
|
|
||||||
if value == "" {
|
if value == "" {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
return strings.Split(strings.TrimPrefix(value, "/"), "/")
|
return strings.Split(strings.TrimPrefix(value, "/"), "/")
|
||||||
}
|
}
|
||||||
child, prefix := tokens(path.String()), tokens(parent.String())
|
|
||||||
|
func pathWithin(path string, prefix []string) bool {
|
||||||
|
child := pointerTokens(path)
|
||||||
if len(prefix) > len(child) {
|
if len(prefix) > len(child) {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user