diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 1af761a..7495acf 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Performance implementation plan -Status: implementation in progress; Stages 1–3 are complete, Stages 4–11 have not started. +Status: implementation in progress; Stages 1–4 are complete, Stages 5–11 have not started. Date: 2026-10-09. @@ -376,7 +376,7 @@ construction; text requests retain their report semantics and formatting errors. ## Stage 4: index backend declarations and traverse parents -**Status:** not started. **Prerequisite:** Stage 3. +**Status:** complete. **Prerequisite:** Stage 3. **Files:** [adapter](../../internal/cobra/parse.go), [adapter tests](../../internal/cobra/parse_test.go), relevant benchmarks, and @@ -402,7 +402,30 @@ parse and dispatch benchmarks. **Completion:** declaration lookup uses the index and ancestry traversal is proportional to selected depth after backend construction. -**Evidence:** pending. +**Evidence:** `go test -mod=readonly -count=1 ./internal/cobra .` passed with +the common offline environment settings. The focused test covers a 24-level +path, 128 siblings, root selection, unknown help/usage paths, and exact help and +usage resolution. Existing exact-path and parse failure tests remain in place. + +Measurements used Go 1.26.8, linux/amd64, Linux 6.17.0-41-generic, Intel Core +i5-7260U CPU @ 2.20 GHz, based on source revision `628dc78` with this stage's +changes in the working tree. The five-sample adapter command was +`GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none GOSUMDB=off go test -mod=readonly -run '^$' -bench '^BenchmarkPerformanceParse$' -benchmem -benchtime=200ms -count=5 ./internal/cobra`; +the dispatch command used the same flags with `-bench +'^BenchmarkPerformanceDispatch$' .`. Both passed. Raw outputs are in +`/tmp/commandkit-performance-parse-after-stage4.txt` and +`/tmp/commandkit-performance-dispatch-after-stage4.txt`. + +Adapter shallow/wide Parse measured 149,426–171,199 ns/op, +242,936 B/op, and 1,705 allocs/op, compared with baseline 150,918–171,598 +ns/op, 242,936 B/op, and 1,705 allocs/op. Deep Parse measured 90,608–104,770 +ns/op, 119,536 B/op, and 826 allocs/op, compared with baseline 86,282–109,697 +ns/op, 119,920 B/op, and 827 allocs/op. Timings overlap; deep parsing saves +384 B and one allocation. End-to-end dispatch allocations remain effectively +unchanged across the measured cases. Timing variation, including deep-help +outliers, is substantial, so these runs do not support an end-to-end speedup +claim. The comparison isolates the selected-path lookup change while backend +tree construction remains in each parse. ## Stage 5: scan YAML preambles without splitting documents diff --git a/internal/cobra/parse.go b/internal/cobra/parse.go index 0e140e6..cb1b274 100644 --- a/internal/cobra/parse.go +++ b/internal/cobra/parse.go @@ -71,8 +71,9 @@ func RenderHelp(root *Command, path []string) (string, *Failure) { if err != nil { return "", &Failure{Message: err.Error()} } - declaration := declarationFor(nodes, selected) - if declaration == nil || len(remaining) != 0 || !equalPath(path, pathFor(root, declaration)) { + declaration := nodes[selected] + selectedPath, pathOK := pathFor(selected, nodes) + if declaration == nil || !pathOK || len(remaining) != 0 || !equalPath(path, selectedPath) { return "", &Failure{Message: "help path is not registered"} } selected.SetOut(buffer) @@ -91,8 +92,9 @@ func UsageLine(root *Command, path []string) (string, *Failure) { options := normalizeOptions(ParseOptions{Context: context.Background(), In: strings.NewReader(""), Out: io.Discard, Err: io.Discard}) backendRoot, nodes := buildTree(root, options) selected, remaining, err := backendRoot.Find(path) - declaration := declarationFor(nodes, selected) - if err != nil || declaration == nil || len(remaining) != 0 || !equalPath(path, pathFor(root, declaration)) { + declaration := nodes[selected] + selectedPath, pathOK := pathFor(selected, nodes) + if err != nil || declaration == nil || !pathOK || len(remaining) != 0 || !equalPath(path, selectedPath) { return "", &Failure{Message: "usage path is not registered"} } line := selected.UseLine() @@ -128,11 +130,15 @@ func Parse(root *Command, argv []string, options ParseOptions) Result { if selected == nil { return failure(result, "no command was selected") } - selectedNode := declarationFor(nodes, selected) + selectedNode := nodes[selected] if selectedNode == nil { return failure(result, "selected command is not part of the declared tree") } - result.Path = pathFor(root, selectedNode) + var pathOK bool + result.Path, pathOK = pathFor(selected, nodes) + if !pathOK { + return failure(result, "selected command is not part of the declared tree") + } consumed := len(leading) - len(unconsumed) if consumed != len(result.Path) || !equalPath(leading[:consumed], result.Path) { return failure(result, "command path must match a registered command exactly") @@ -180,8 +186,8 @@ func normalizeOptions(options ParseOptions) ParseOptions { return options } -func buildTree(root *Command, options ParseOptions) (*spfcobra.Command, map[*Command]*spfcobra.Command) { - nodes := make(map[*Command]*spfcobra.Command) +func buildTree(root *Command, options ParseOptions) (*spfcobra.Command, map[*spfcobra.Command]*Command) { + nodes := make(map[*spfcobra.Command]*Command) var build func(*Command) *spfcobra.Command build = func(declaration *Command) *spfcobra.Command { backend := &spfcobra.Command{ @@ -223,7 +229,7 @@ func buildTree(root *Command, options ParseOptions) (*spfcobra.Command, map[*Com // Cobra's automatic help flag is suppressed because CommandKit declares // and interprets the local help flag itself. backend.SetHelpCommand(nil) - nodes[declaration] = backend + nodes[backend] = declaration for _, child := range declaration.Children { backend.AddCommand(build(child)) } @@ -320,15 +326,6 @@ func equalPath(left, right []string) bool { return true } -func declarationFor(nodes map[*Command]*spfcobra.Command, selected *spfcobra.Command) *Command { - for declaration, backend := range nodes { - if backend == selected { - return declaration - } - } - return nil -} - func splitRoutingPrefix(argv []string) ([]string, []string) { for index, argument := range argv { if argument == "--" || (strings.HasPrefix(argument, "-") && argument != "-") { @@ -338,29 +335,21 @@ func splitRoutingPrefix(argv []string) ([]string, []string) { return append([]string(nil), argv...), nil } -func pathFor(root, selected *Command) []string { +func pathFor(selected *spfcobra.Command, declarations map[*spfcobra.Command]*Command) ([]string, bool) { var reverse []string - for current := selected; current != nil && current != root; { - reverse = append(reverse, current.Name) - current = parentOf(root, current) - } - path := make([]string, len(reverse)) - for index := range reverse { - path[len(reverse)-index-1] = reverse[index] - } - return path -} - -func parentOf(root, target *Command) *Command { - for _, child := range root.Children { - if child == target { - return root + for current := selected; current != nil; current = current.Parent() { + declaration := declarations[current] + if declaration == nil { + return nil, false } - if parent := parentOf(child, target); parent != nil { - return parent + if current.Parent() != nil { + reverse = append(reverse, declaration.Name) } } - return nil + for left, right := 0, len(reverse)-1; left < right; left, right = left+1, right-1 { + reverse[left], reverse[right] = reverse[right], reverse[left] + } + return reverse, true } func failure(result Result, message string) Result { diff --git a/internal/cobra/parse_test.go b/internal/cobra/parse_test.go index 0ff9b6d..fa15ade 100644 --- a/internal/cobra/parse_test.go +++ b/internal/cobra/parse_test.go @@ -2,6 +2,7 @@ package cobra import ( "context" + "fmt" "io" "reflect" "strings" @@ -130,3 +131,51 @@ func TestParseReturnsNeutralFailuresAndFreshState(t *testing.T) { t.Fatalf("unknown flag failure = %#v", failed.Failure) } } + +func TestSelectedPathsResolveThroughBackendParents(t *testing.T) { + root := &Command{Name: "app"} + for index := 0; index < 128; index++ { + root.Children = append(root.Children, &Command{Name: fmt.Sprintf("wide%03d", index), Runnable: true}) + } + path := make([]string, 24) + current := root + for index := range path { + path[index] = fmt.Sprintf("deep%02d", index) + child := &Command{Name: path[index]} + if index == len(path)-1 { + child.Runnable = true + } + current.Children = append(current.Children, child) + current = child + } + + result := Parse(root, path, ParseOptions{}) + if result.Failure != nil || !reflect.DeepEqual(result.Path, path) { + t.Fatalf("deep parse path = %#v", result) + } + rootResult := Parse(root, nil, ParseOptions{}) + if rootResult.Failure != nil || len(rootResult.Path) != 0 { + t.Fatalf("root parse path = %#v", rootResult) + } + + help, failure := RenderHelp(root, path) + if failure != nil || !strings.Contains(help, path[len(path)-1]) { + t.Fatalf("deep help = %q, %v", help, failure) + } + if _, failure := RenderHelp(root, []string{"wide128"}); failure == nil { + t.Fatal("help accepted an unknown sibling") + } + line, failure := UsageLine(root, path) + if failure != nil || !strings.Contains(line, path[len(path)-1]) { + t.Fatalf("deep usage line = %q, %v", line, failure) + } + if _, failure := UsageLine(root, []string{"wide128"}); failure == nil { + t.Fatal("usage accepted an unknown sibling") + } + if _, failure := RenderHelp(root, nil); failure != nil { + t.Fatalf("root help failed: %v", failure) + } + if _, failure := UsageLine(root, nil); failure != nil { + t.Fatalf("root usage failed: %v", failure) + } +}