From a852301801c839777a965a044183c946bd3cd665 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 9 Oct 2026 01:35:44 +0000 Subject: [PATCH] Add invocation-owned Cobra sessions --- docs/roadmap/implementation.md | 29 +++++- internal/cobra/parse.go | 139 +++++++++++++++++------------ internal/cobra/parse_test.go | 45 ++++++++++ internal/cobra/performance_test.go | 24 +++++ 4 files changed, 179 insertions(+), 58 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 7fe234c..dc17b82 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Performance implementation plan -Status: implementation in progress; Stages 1–7 are complete, Stages 8–11 have not started. +Status: implementation in progress; Stages 1–8 are complete, Stages 9–11 have not started. Date: 2026-10-09. @@ -615,7 +615,7 @@ with one tree allocation retained per app metadata generation. ## Stage 8: introduce an invocation-owned adapter session -**Status:** not started. **Prerequisite:** Stage 7. +**Status:** complete. **Prerequisite:** Stage 7. **Files:** the adapter, its tests/benchmarks, and evidence. Root execution remains functional through the existing package-level adapter helpers during this stage. @@ -646,7 +646,30 @@ functional through the existing package-level adapter helpers during this stage. **Completion:** the adapter can perform parsing and metadata presentation on one prepared invocation tree while legacy root entry points still compile. -**Evidence:** pending. +**Evidence:** `GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none +GOSUMDB=off go test -mod=readonly -count=1 ./internal/cobra .` passed. Added +coverage for parse followed by help and usage on one session, fresh mutable +values and streams across separately prepared sessions, and neutral failures +for missing trees. Help output is captured and the selected command's original +streams are restored. Existing exact-path, parse-failure, and flag-state tests +continue to pass. The package-level helpers now each prepare one session and +delegate to its method. + +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 `3ad3559` with this stage's +changes in the working tree. The five-sample adapter command passed: +`GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none GOSUMDB=off go test -mod=readonly -run '^$' -bench '^BenchmarkPerformance(Parse|SessionParseAndHelp)$' -benchmem -benchtime=200ms -count=5 ./internal/cobra`. +Raw output is in `/tmp/commandkit-performance-adapter-after-stage8.txt`. + +The new parse-plus-help session benchmark, with an 8-sibling tree prepared +outside the timed loop, measured 33,456–50,352 ns/op, 10,658 B/op, and 226 +allocations. The existing shallow/wide Parse wrapper measured 242,952–242,956 +B/op and 1,706 allocations; deep measured 119,552–119,553 B/op and 827 +allocations. Compared with the Stage 4 adapter baseline, each adds 16 bytes and +one allocation for the one-session wrapper. Timings ranged widely in this run, +so they are inconclusive; tree construction remains included in these wrapper +benchmarks. The session benchmark excludes its one-time tree preparation and +measures repeated parse plus help on the same fresh-per-session backend state. ## Stage 9: reuse the adapter session throughout root execution diff --git a/internal/cobra/parse.go b/internal/cobra/parse.go index cb1b274..4d7afac 100644 --- a/internal/cobra/parse.go +++ b/internal/cobra/parse.go @@ -58,52 +58,6 @@ type Result struct { Failure *Failure } -// RenderHelp renders registered metadata for the exact command path. It uses -// Cobra's usage metadata and returns backend failures as neutral message data. -func RenderHelp(root *Command, path []string) (string, *Failure) { - if root == nil { - return "", &Failure{Message: "command tree is nil"} - } - buffer := new(bytes.Buffer) - options := normalizeOptions(ParseOptions{Context: context.Background(), In: strings.NewReader(""), Out: buffer, Err: buffer}) - backendRoot, nodes := buildTree(root, options) - selected, remaining, err := backendRoot.Find(path) - if err != nil { - return "", &Failure{Message: err.Error()} - } - 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) - selected.SetErr(buffer) - if err := selected.Usage(); err != nil { - return "", &Failure{Message: err.Error()} - } - return buffer.String(), nil -} - -// UsageLine returns Cobra's usage line for an exact registered command path. -func UsageLine(root *Command, path []string) (string, *Failure) { - if root == nil { - return "", &Failure{Message: "command tree is nil"} - } - 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 := 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() - if selected.HasSubCommands() { - line += " [command]" - } - return line, nil -} - // ParseOptions makes the parser's context and streams explicit. Nil writers // are replaced with io.Discard. type ParseOptions struct { @@ -113,29 +67,43 @@ type ParseOptions struct { Err io.Writer } -// Parse constructs a fresh Cobra tree and parses one argv. It routes only the -// leading command words, stopping permanently at the first option or --. -func Parse(root *Command, argv []string, options ParseOptions) Result { - result := Result{} +// Session owns one mutable backend tree for a single command invocation. +type Session struct { + root *spfcobra.Command + declarations map[*spfcobra.Command]*Command +} + +// Prepare constructs a fresh backend tree for one invocation. +func Prepare(root *Command, options ParseOptions) *Session { + session := &Session{} if root == nil { - return failure(result, "command tree is nil") + return session } options = normalizeOptions(options) - backendRoot, nodes := buildTree(root, options) + session.root, session.declarations = buildTree(root, options) + return session +} + +// Parse routes leading command words and parses flags on this session's tree. +func (session *Session) Parse(argv []string) Result { + result := Result{} + if session == nil || session.root == nil { + return failure(result, "command tree is nil") + } leading, tail := splitRoutingPrefix(argv) - selected, unconsumed, err := backendRoot.Find(leading) + selected, unconsumed, err := session.root.Find(leading) if err != nil { return failure(result, err.Error()) } if selected == nil { return failure(result, "no command was selected") } - selectedNode := nodes[selected] + selectedNode := session.declarations[selected] if selectedNode == nil { return failure(result, "selected command is not part of the declared tree") } var pathOK bool - result.Path, pathOK = pathFor(selected, nodes) + result.Path, pathOK = pathFor(selected, session.declarations) if !pathOK { return failure(result, "selected command is not part of the declared tree") } @@ -170,6 +138,67 @@ func Parse(root *Command, argv []string, options ParseOptions) Result { return result } +// RenderHelp renders registered metadata for an exact path on this session. +func (session *Session) RenderHelp(path []string) (string, *Failure) { + if session == nil || session.root == nil { + return "", &Failure{Message: "command tree is nil"} + } + selected, remaining, err := session.root.Find(path) + if err != nil { + return "", &Failure{Message: err.Error()} + } + declaration := session.declarations[selected] + selectedPath, pathOK := pathFor(selected, session.declarations) + if declaration == nil || !pathOK || len(remaining) != 0 || !equalPath(path, selectedPath) { + return "", &Failure{Message: "help path is not registered"} + } + buffer := new(bytes.Buffer) + previousOut, previousErr := selected.OutOrStdout(), selected.ErrOrStderr() + selected.SetOut(buffer) + selected.SetErr(buffer) + defer func() { + selected.SetOut(previousOut) + selected.SetErr(previousErr) + }() + if err := selected.Usage(); err != nil { + return "", &Failure{Message: err.Error()} + } + return buffer.String(), nil +} + +// UsageLine returns Cobra's usage line for an exact path on this session. +func (session *Session) UsageLine(path []string) (string, *Failure) { + if session == nil || session.root == nil { + return "", &Failure{Message: "command tree is nil"} + } + selected, remaining, err := session.root.Find(path) + declaration := session.declarations[selected] + selectedPath, pathOK := pathFor(selected, session.declarations) + if err != nil || declaration == nil || !pathOK || len(remaining) != 0 || !equalPath(path, selectedPath) { + return "", &Failure{Message: "usage path is not registered"} + } + line := selected.UseLine() + if selected.HasSubCommands() { + line += " [command]" + } + return line, nil +} + +// RenderHelp renders a fresh command tree for one help request. +func RenderHelp(root *Command, path []string) (string, *Failure) { + return Prepare(root, ParseOptions{}).RenderHelp(path) +} + +// UsageLine resolves a path on a fresh command tree. +func UsageLine(root *Command, path []string) (string, *Failure) { + return Prepare(root, ParseOptions{}).UsageLine(path) +} + +// Parse constructs a fresh command tree and parses one argv. +func Parse(root *Command, argv []string, options ParseOptions) Result { + return Prepare(root, options).Parse(argv) +} + func normalizeOptions(options ParseOptions) ParseOptions { if options.Context == nil { options.Context = context.Background() diff --git a/internal/cobra/parse_test.go b/internal/cobra/parse_test.go index fa15ade..ffb6c28 100644 --- a/internal/cobra/parse_test.go +++ b/internal/cobra/parse_test.go @@ -1,6 +1,7 @@ package cobra import ( + "bytes" "context" "fmt" "io" @@ -179,3 +180,47 @@ func TestSelectedPathsResolveThroughBackendParents(t *testing.T) { t.Fatalf("root usage failed: %v", failure) } } + +func TestSessionReusesTreeForParseHelpAndUsageWithFreshState(t *testing.T) { + tree := &Command{Name: "app", Children: []*Command{{Name: "run", Runnable: true, Flags: []Flag{{Name: "value"}}}}} + var output, errors bytes.Buffer + session := Prepare(tree, ParseOptions{Context: context.Background(), Out: &output, Err: &errors}) + first := session.Parse([]string{"run", "--value=old"}) + if first.Failure != nil || !reflect.DeepEqual(first.FlagValues, [][]string{{"old"}}) { + t.Fatalf("first session parse = %#v", first) + } + help, failure := session.RenderHelp([]string{"run"}) + if failure != nil || !strings.Contains(help, "run") { + t.Fatalf("session help = %q, %v", help, failure) + } + line, failure := session.UsageLine([]string{"run"}) + if failure != nil || !strings.Contains(line, "run") { + t.Fatalf("session usage = %q, %v", line, failure) + } + if output.Len() != 0 || errors.Len() != 0 { + t.Fatalf("help escaped session delivery control: out=%q err=%q", output.String(), errors.String()) + } + + var secondOutput, secondErrors bytes.Buffer + second := Prepare(tree, ParseOptions{Context: context.Background(), Out: &secondOutput, Err: &secondErrors}) + secondResult := second.Parse([]string{"run"}) + if secondResult.Failure != nil || !reflect.DeepEqual(secondResult.FlagValues, [][]string{nil}) { + t.Fatalf("fresh session state = %#v", secondResult) + } + if output.Len() != 0 || errors.Len() != 0 || secondOutput.Len() != 0 || secondErrors.Len() != 0 { + t.Fatal("prepared session streams were unexpectedly used") + } +} + +func TestMissingSessionTreeReturnsNeutralFailures(t *testing.T) { + session := Prepare(nil, ParseOptions{}) + if result := session.Parse(nil); result.Failure == nil || result.Failure.Message != "command tree is nil" { + t.Fatalf("missing-tree parse = %#v", result) + } + if _, failure := session.RenderHelp(nil); failure == nil || failure.Message != "command tree is nil" { + t.Fatalf("missing-tree help failure = %#v", failure) + } + if _, failure := session.UsageLine(nil); failure == nil || failure.Message != "command tree is nil" { + t.Fatalf("missing-tree usage failure = %#v", failure) + } +} diff --git a/internal/cobra/performance_test.go b/internal/cobra/performance_test.go index 84ead94..8ff709c 100644 --- a/internal/cobra/performance_test.go +++ b/internal/cobra/performance_test.go @@ -38,6 +38,30 @@ func BenchmarkPerformanceParse(b *testing.B) { } } +func BenchmarkPerformanceSessionParseAndHelp(b *testing.B) { + root := performanceFlatTree(8) + args := []string{"wide007"} + session := Prepare(root, ParseOptions{Context: context.Background(), Out: io.Discard, Err: io.Discard}) + result := session.Parse(args) + if result.Failure != nil { + b.Fatalf("fixture parse failed: %s", result.Failure.Message) + } + if help, failure := session.RenderHelp(result.Path); failure != nil || help == "" { + b.Fatalf("fixture help failed: %q %v", help, failure) + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + result := session.Parse(args) + if result.Failure != nil { + b.Fatalf("parse failed: %s", result.Failure.Message) + } + if help, failure := session.RenderHelp(result.Path); failure != nil || help == "" { + b.Fatalf("help failed: %q %v", help, failure) + } + } +} + func performanceFlatTree(count int) *Command { root := &Command{Name: "app", Children: make([]*Command, count)} for index := range root.Children {