Reuse adapter sessions during execution

2026-10-09 01:42:03 +00:00
parent a852301801
commit a7960e5e94
6 changed files with 328 additions and 32 deletions
--- a/builtins.go
+++ b/builtins.go
@@ -38,11 +38,7 @@ func (a *App) installBuiltins() error {
 			func(value string) (string, error) { return value, nil },
 			func(input *helpInput, value Supplied[[]string]) { input.Targets = value })},
 		Run: func(invocation Invocation, input helpInput) Outcome {
-			target, err := a.helpCommandTarget(input.Targets.Value)
-			if err != nil {
-				return Usage(err)
-			}
-			return a.renderHelpOutcome(target, invocation.Streams)
+			return a.renderHelpTargets(input.Targets.Value, invocation.Streams)
 		},
 	})
 	if err != nil {
@@ -92,12 +88,30 @@ func parseVersionFormat(value string) (string, error) {
 }

 func (a *App) renderHelpOutcome(path []string, streams Streams) Outcome {
+	session := internalcobra.Prepare(a.bridgeTree(), internalcobra.ParseOptions{})
+	return a.renderHelpOutcomeWithSession(path, streams, session)
+}
+
+func (a *App) renderHelpTargets(targets []string, streams Streams) Outcome {
+	session := internalcobra.Prepare(a.bridgeTree(), internalcobra.ParseOptions{})
+	return a.renderHelpTargetsWithSession(targets, streams, session)
+}
+
+func (a *App) renderHelpTargetsWithSession(targets []string, streams Streams, session *internalcobra.Session) Outcome {
+	target, err := a.helpCommandTarget(targets)
+	if err != nil {
+		return Usage(err)
+	}
+	return a.renderHelpOutcomeWithSession(target, streams, session)
+}
+
+func (a *App) renderHelpOutcomeWithSession(path []string, streams Streams, session *internalcobra.Session) Outcome {
 	if len(path) > 0 {
 		if _, ok := a.commandAt(path); !ok {
 			return Usage(fmt.Errorf("unknown help path %q", strings.Join(path, " ")))
 		}
 	}
-	content, failure := internalcobra.RenderHelp(a.bridgeTree(), path)
+	content, failure := session.RenderHelp(path)
 	if failure != nil {
 		return Failure(errors.New(failure.Message))
 	}
--- a/docs/roadmap/implementation.md
+++ b/docs/roadmap/implementation.md
@@ -673,7 +673,7 @@ measures repeated parse plus help on the same fresh-per-session backend state.

 ## Stage 9: reuse the adapter session throughout root execution

-**Status:** not started. **Prerequisite:** Stage 8.
+**Status:** complete. **Prerequisite:** Stage 8.

 **Files:** [execute.go](../../execute.go), [builtins.go](../../builtins.go),
 private parser coordination, [execute_external_test.go](../../execute_external_test.go),
@@ -719,7 +719,63 @@ to confirm that signatures and backend isolation are unchanged.
 **Completion:** every normal `Execute` path uses the selected OD-1 lifecycle and
 all existing public handler and presentation semantics remain intact.

-**Evidence:** pending.
+**Evidence:** `GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none
+GOSUMDB=off go test -mod=readonly -count=1 ./internal/cobra .` passed. Existing
+help, parser, outcome, and cancellation coverage continues to pass. Added checks
+for a successful flag-bearing invocation followed by local help with an empty
+value, explicit false and omitted values, distinct contexts/streams across
+executions, handler-returned usage after successful registration, and help plus
+usage against metadata registered after a session was prepared. The built-in
+help handler and top-level dispatcher share target resolution/rendering. Source
+review confirms `Execute` prepares one session from the cached neutral tree;
+presentation reuses it when the tree pointer is unchanged and prepares against
+new metadata only after successful registration. Parsing, input factories,
+checks, and handlers are not rerun for presentation. Public signatures and the
+Cobra facade boundary are unchanged.
+
+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 `a852301` with this stage's
+changes in the working tree. The five-sample command passed:
+`GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none GOSUMDB=off go test -mod=readonly -run '^$' -bench '^BenchmarkPerformanceDispatch$' -benchmem -benchtime=200ms -count=5 .`.
+Raw output is in `/tmp/commandkit-performance-dispatch-after-stage9.txt`;
+focused wide and cold repeats are in
+`/tmp/commandkit-performance-dispatch-stage9-repeat.txt` and
+`/tmp/commandkit-performance-dispatch-stage9-cold-repeat.txt`.
+
+Compared with the Stage 1 baseline, small repeated success measured
+38,671–44,179 ns/op, 29,576–29,578 B/op, and 269 allocations (baseline:
+31,066–41,041 ns/op, 33,209–33,210 B/op, 287 allocations). The timing ranges
+overlap; bytes fell by about 3.6 KB and allocations by 18. Local help measured
+85,120–91,091 ns/op, 41,428–41,430 B/op, and 509 allocations (baseline:
+103,022–116,183 ns/op, 78,176–78,177 B/op, 810 allocations). Explicit help
+measured 89,431–116,591 ns/op, 41,917–41,918 B/op, and 516 allocations
+(baseline: 101,998–111,895 ns/op, 78,152–78,153 B/op, 815 allocations).
+Parser usage measured 28,521–46,876 ns/op, 30,128–30,129 B/op, and 292
+allocations (baseline: 62,596–74,475 ns/op, 66,363–66,364 B/op, 591
+allocations). Handler usage measured 29,563–53,319 ns/op, 29,976–29,977
+B/op, and 284 allocations (baseline: 61,878–72,264 ns/op, 66,211–66,212
+B/op, 583 allocations).
+
+The focused wide repeat measured success at 178,004–194,710 ns/op, 247,796–
+247,806 B/op, and 1,758 allocations (baseline: 218,073–253,010 ns/op,
+269,063–269,070 B/op, 1,898 allocations). Wide help measured 227,764–243,610
+ns/op, 259,685–259,689 B/op, and 1,998 allocations (baseline: 491,172–
+622,188 ns/op, 549,923–549,940 B/op, 4,033 allocations). Deep success
+measured 151,780–172,540 ns/op, 124,410–124,412 B/op, and 881 allocations
+(baseline: 119,958–139,625 ns/op, 139,787–139,791 B/op, 1,074 allocations),
+so this run was slower while reducing bytes and allocations. Deep help measured
+253,765–324,263 ns/op, 157,797–157,808 B/op, and 1,238 allocations; its
+focused Stage 1 baseline was 300,155–326,094 ns/op, 294,882–294,896 B/op, and
+2,408 allocations.
+
+Cold small construction plus first execution measured 37,934–42,626 ns/op,
+38,411–38,412 B/op, and 340 allocations (baseline: 38,152–50,235 ns/op,
+38,275–38,276 B/op, 339 allocations). Cold wide measured 267,018–335,726
+ns/op, 305,212–305,220 B/op, and 2,196 allocations (baseline: 251,945–
+315,276 ns/op, 291,822–291,827 B/op, 2,184 allocations). Both timing ranges
+overlap their baselines; retained session coordination adds a small cold
+allocation/byte cost. Results show lower help/usage work and lower repeated
+allocation costs, with timing variation on repeated success and deep dispatch.

 ## Stage 10: prepare configuration scope metadata once

--- a/execute.go
+++ b/execute.go
@@ -19,34 +19,111 @@ func (a *App) Execute(ctx context.Context, args []string, streams Streams) Outco
 		return a.present(Failure(errors.New("invocation context must not be nil")), nil, streams)
 	}
 	streams = normalizeStreams(streams)
-	parsed := a.parse(args, internalcobra.ParseOptions{
-		Context: ctx,
-		Out:     streams.Out,
-		Err:     streams.Err,
-	})
+	current := newExecution(a, ctx, streams)
+	parsed := current.parse(args)
 	if parsed.Failure != nil {
-		return a.present(Usage(errors.New(parsed.Failure.Message)), parsed.Path, streams)
+		return current.present(Usage(errors.New(parsed.Failure.Message)), parsed.Path)
 	}
 	command, ok := a.commandAt(parsed.Path)
 	if !ok || command.payload == nil {
-		return a.present(a.renderHelpOutcome(parsed.Path, streams), parsed.Path, streams)
+		return current.present(current.renderHelp(parsed.Path), parsed.Path)
 	}
 	if parsed.HelpRequested || !command.payload.hasRun() {
 		if _, err := command.payload.prepare(parsed.FlagValues, parsed.Operands, false); err != nil {
-			return a.present(Usage(err), parsed.Path, streams)
+			return current.present(Usage(err), parsed.Path)
 		}
-		return a.present(a.renderHelpOutcome(parsed.Path, streams), parsed.Path, streams)
+		return current.present(current.renderHelp(parsed.Path), parsed.Path)
 	}
 	buildInput, err := command.payload.prepare(parsed.FlagValues, parsed.Operands, true)
 	if err != nil {
-		return a.present(Usage(err), parsed.Path, streams)
+		return current.present(Usage(err), parsed.Path)
 	}
 	input := buildInput()
 	if err := command.payload.checkInput(input); err != nil {
-		return a.present(Usage(err), parsed.Path, streams)
+		return current.present(Usage(err), parsed.Path)
+	}
+	if command.name == "help" {
+		help, ok := input.(*helpInput)
+		if !ok {
+			return current.present(Failure(errors.New("built-in help input has an unexpected type")), parsed.Path)
+		}
+		return current.present(current.renderHelpTargets(help.Targets.Value), parsed.Path)
 	}
 	outcome := command.payload.run(Invocation{Context: ctx, Streams: streams}, input)
-	return a.present(outcome, parsed.Path, streams)
+	return current.present(outcome, parsed.Path)
+}
+
+type execution struct {
+	app      *App
+	ctx      context.Context
+	streams  Streams
+	options  internalcobra.ParseOptions
+	metadata *internalcobra.Command
+	session  *internalcobra.Session
+}
+
+func newExecution(app *App, ctx context.Context, streams Streams) *execution {
+	current := &execution{
+		app:     app,
+		ctx:     ctx,
+		streams: streams,
+		options: internalcobra.ParseOptions{Context: ctx, Out: streams.Out, Err: streams.Err},
+	}
+	if app != nil {
+		current.metadata = app.bridgeTree()
+		current.session = internalcobra.Prepare(current.metadata, current.options)
+	}
+	return current
+}
+
+func (current *execution) parse(args []string) internalcobra.Result {
+	if current.app == nil {
+		return internalcobra.Result{Failure: &internalcobra.Failure{Message: "cannot parse commands on a nil app"}}
+	}
+	return current.session.Parse(args)
+}
+
+func (current *execution) presentationSession() *internalcobra.Session {
+	if current.app == nil {
+		return nil
+	}
+	metadata := current.app.bridgeTree()
+	if current.session == nil || current.session.Metadata() != metadata {
+		current.metadata = metadata
+		current.session = internalcobra.Prepare(metadata, current.options)
+	}
+	return current.session
+}
+
+func (current *execution) renderHelp(path []string) Outcome {
+	if current.app == nil {
+		return Failure(errors.New("cannot render help on a nil app"))
+	}
+	return current.app.renderHelpOutcomeWithSession(path, current.streams, current.presentationSession())
+}
+
+func (current *execution) renderHelpTargets(targets []string) Outcome {
+	if current.app == nil {
+		return Failure(errors.New("cannot render help on a nil app"))
+	}
+	return current.app.renderHelpTargetsWithSession(targets, current.streams, current.presentationSession())
+}
+
+func (current *execution) usageLine(path []string) string {
+	if current.app == nil {
+		return "Usage: command [command]"
+	}
+	session := current.presentationSession()
+	if session != nil {
+		if usage, failure := session.UsageLine(path); failure == nil {
+			return "Usage: " + usage
+		}
+	}
+	return current.app.usageFallback()
+}
+
+func (current *execution) present(outcome Outcome, path []string) Outcome {
+	return current.app.presentWithUsageLine(outcome, path, current.streams, current.usageLine)
 }

 func normalizeStreams(streams Streams) Streams {
@@ -88,6 +165,10 @@ func (a *App) commandAt(path []string) (Command, bool) {
 }

 func (a *App) present(outcome Outcome, path []string, streams Streams) Outcome {
+	return a.presentWithUsageLine(outcome, path, streams, a.usageLine)
+}
+
+func (a *App) presentWithUsageLine(outcome Outcome, path []string, streams Streams, usageLine func([]string) string) Outcome {
 	if outcome.Kind == "" && outcome.Status == 0 && outcome.Domain == "" &&
 		outcome.Cause == nil && outcome.DeliveryError == nil &&
 		!outcome.ReportIncomplete && len(outcome.Diagnostics) == 0 {
@@ -105,7 +186,7 @@ func (a *App) present(outcome Outcome, path []string, streams Streams) Outcome {
 		diagnostics = append(diagnostics, MessageDiagnostic(configkit.SeverityError, outcome.Cause.Error()))
 	}
 	if outcome.Kind == OutcomeUsage {
-		diagnostics = append(diagnostics, MessageDiagnostic(configkit.SeverityError, a.usageLine(path)))
+		diagnostics = append(diagnostics, MessageDiagnostic(configkit.SeverityError, usageLine(path)))
 	}
 	outcome.Diagnostics = diagnostics
 	if len(diagnostics) == 0 {
@@ -137,15 +218,19 @@ func (a *App) present(outcome Outcome, path []string, streams Streams) Outcome {
 }

 func (a *App) usageLine(path []string) string {
-	name := "command"
-	if a != nil && a.identity.Name != "" {
-		name = a.identity.Name
-	}
 	if a != nil {
 		if usage, failure := internalcobra.UsageLine(a.bridgeTree(), path); failure == nil {
 			return "Usage: " + usage
 		}
 	}
+	return a.usageFallback()
+}
+
+func (a *App) usageFallback() string {
+	name := "command"
+	if a != nil && a.identity.Name != "" {
+		name = a.identity.Name
+	}
 	return fmt.Sprintf("Usage: %s [command]", name)
 }

--- a/execute_external_test.go
+++ b/execute_external_test.go
@@ -21,8 +21,9 @@ func TestExecuteRunsTypedCommandsWithFreshInvocationState(t *testing.T) {
 	var inputs []input
 	var contexts []context.Context
 	var streams []commandkit.Streams
-	ctx := context.WithValue(context.Background(), struct{}{}, "invocation")
-	var out, diagnostic bytes.Buffer
+	firstContext := context.WithValue(context.Background(), struct{}{}, "first")
+	secondContext := context.WithValue(context.Background(), struct{}{}, "second")
+	var firstOut, secondOut, firstDiagnostic, secondDiagnostic bytes.Buffer
 	command, err := commandkit.NewCommand(commandkit.CommandSpec[input]{
 		Name: "run",
 		Flags: []commandkit.Flag[input]{
@@ -43,9 +44,9 @@ func TestExecuteRunsTypedCommandsWithFreshInvocationState(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	first := app.Execute(ctx, []string{"run", "--name=first", "--id=a,b", "--id=c"}, commandkit.Streams{Out: &out, Err: &diagnostic})
-	second := app.Execute(ctx, []string{"run"}, commandkit.Streams{Out: &out, Err: &diagnostic})
-	third := app.Execute(ctx, []string{"run"}, commandkit.Streams{})
+	first := app.Execute(firstContext, []string{"run", "--name=first", "--id=a,b", "--id=c"}, commandkit.Streams{Out: &firstOut, Err: &firstDiagnostic})
+	second := app.Execute(secondContext, []string{"run"}, commandkit.Streams{Out: &secondOut, Err: &secondDiagnostic})
+	third := app.Execute(context.Background(), []string{"run"}, commandkit.Streams{})
 	if first.Kind != commandkit.OutcomeSuccess || second.Kind != commandkit.OutcomeSuccess || third.Kind != commandkit.OutcomeSuccess {
 		t.Fatalf("outcomes = %#v, %#v, and %#v", first, second, third)
 	}
@@ -58,8 +59,96 @@ func TestExecuteRunsTypedCommandsWithFreshInvocationState(t *testing.T) {
 	if streams[2].Out != io.Discard || streams[2].Err != io.Discard {
 		t.Fatalf("nil streams were not replaced with io.Discard: %#v", streams[2])
 	}
-	if contexts[0] != ctx || streams[0].Out != &out || streams[0].Err != &diagnostic {
-		t.Fatalf("handler invocation context or streams were not forwarded")
+	if contexts[0] != firstContext || contexts[1] != secondContext || streams[0].Out != &firstOut || streams[0].Err != &firstDiagnostic || streams[1].Out != &secondOut || streams[1].Err != &secondDiagnostic {
+		t.Fatalf("handler invocation contexts or streams were not forwarded independently")
+	}
+}
+
+func TestExecuteUsesFreshSessionForHelpAfterFlagBearingInvocation(t *testing.T) {
+	type input struct {
+		name    commandkit.Supplied[string]
+		enabled commandkit.Supplied[bool]
+	}
+	var received []input
+	command, err := commandkit.NewCommand(commandkit.CommandSpec[input]{
+		Name: "run",
+		Flags: []commandkit.Flag[input]{
+			commandkit.ValueFlag(commandkit.FlagMeta{Name: "name"}, (*string)(nil), func(value string) (string, error) {
+				return value, nil
+			}, func(target *input, value commandkit.Supplied[string]) { target.name = value }),
+			commandkit.BoolFlag(commandkit.FlagMeta{Name: "enabled"}, false, func(target *input, value commandkit.Supplied[bool]) { target.enabled = value }),
+		},
+		Run: func(_ commandkit.Invocation, value input) commandkit.Outcome {
+			received = append(received, value)
+			return commandkit.Success()
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	app, err := commandkit.New(commandkit.Identity{Name: "example"}, command)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var firstOutput, secondOutput, diagnostic bytes.Buffer
+	first := app.Execute(context.Background(), []string{"run", "--name=chosen", "--enabled=false"}, commandkit.Streams{Out: &firstOutput, Err: &diagnostic})
+	if first.Kind != commandkit.OutcomeSuccess {
+		t.Fatalf("flag-bearing invocation = %#v", first)
+	}
+	help := app.Execute(context.Background(), []string{"run", "--name=", "--help"}, commandkit.Streams{Out: &secondOutput, Err: &diagnostic})
+	if help.Kind != commandkit.OutcomeSuccess || !strings.Contains(secondOutput.String(), "--enabled") {
+		t.Fatalf("help invocation = %#v, output %q", help, secondOutput.String())
+	}
+	last := app.Execute(context.Background(), []string{"run"}, commandkit.Streams{})
+	if last.Kind != commandkit.OutcomeSuccess {
+		t.Fatalf("omitted-value invocation = %#v", last)
+	}
+	if len(received) != 2 || received[0].name.Value != "chosen" || !received[0].name.Set || received[0].enabled.Value || !received[0].enabled.Set {
+		t.Fatalf("first input = %#v", received)
+	}
+	if received[1].name.Set || received[1].name.Value != "" || received[1].enabled.Set {
+		t.Fatalf("later invocation inherited flag state: %#v", received[1])
+	}
+}
+
+func TestExecutePresentsHandlerUsageAfterRegisteringCommand(t *testing.T) {
+	usageCause := errors.New("handler requests usage")
+	var app *commandkit.App
+	var calls int
+	late, err := commandkit.NewCommand(commandkit.CommandSpec[struct{}]{
+		Name: "late",
+		Run:  func(commandkit.Invocation, struct{}) commandkit.Outcome { return commandkit.Success() },
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	command, err := commandkit.NewCommand(commandkit.CommandSpec[struct{}]{
+		Name: "run",
+		Run: func(commandkit.Invocation, struct{}) commandkit.Outcome {
+			calls++
+			if err := app.Register(late); err != nil {
+				return commandkit.Failure(err)
+			}
+			return commandkit.Usage(usageCause)
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	app, err = commandkit.New(commandkit.Identity{Name: "example"}, command)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var diagnostic bytes.Buffer
+	result := app.Execute(context.Background(), []string{"run"}, commandkit.Streams{Err: &diagnostic})
+	if result.Kind != commandkit.OutcomeUsage || !errors.Is(result.Cause, usageCause) || calls != 1 {
+		t.Fatalf("handler usage = %#v, calls %d", result, calls)
+	}
+	if !strings.Contains(diagnostic.String(), "Usage: example run") {
+		t.Fatalf("handler usage diagnostic = %q", diagnostic.String())
+	}
+	if result := app.Execute(context.Background(), []string{"late"}, commandkit.Streams{}); result.Kind != commandkit.OutcomeSuccess {
+		t.Fatalf("registered command execution = %#v", result)
 	}
 }

--- a/internal/cobra/parse.go
+++ b/internal/cobra/parse.go
@@ -69,6 +69,7 @@ type ParseOptions struct {

 // Session owns one mutable backend tree for a single command invocation.
 type Session struct {
+	metadata     *Command
 	root         *spfcobra.Command
 	declarations map[*spfcobra.Command]*Command
 }
@@ -79,11 +80,20 @@ func Prepare(root *Command, options ParseOptions) *Session {
 	if root == nil {
 		return session
 	}
+	session.metadata = root
 	options = normalizeOptions(options)
 	session.root, session.declarations = buildTree(root, options)
 	return session
 }

+// Metadata returns the neutral tree used to prepare this session.
+func (session *Session) Metadata() *Command {
+	if session == nil {
+		return nil
+	}
+	return session.metadata
+}
+
 // Parse routes leading command words and parses flags on this session's tree.
 func (session *Session) Parse(argv []string) Result {
 	result := Result{}
--- a/parser_test.go
+++ b/parser_test.go
@@ -53,6 +53,48 @@ func TestAppParseUsesRegisteredBindingsWithTheAdapter(t *testing.T) {
 	}
 }

+func TestExecutionRefreshesHelpMetadataAfterRegistration(t *testing.T) {
+	first, err := NewCommand(CommandSpec[struct{}]{
+		Name: "first",
+		Run:  func(Invocation, struct{}) Outcome { return Success() },
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	app, err := New(Identity{Name: "example"}, first)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var output bytes.Buffer
+	current := newExecution(app, context.Background(), Streams{Out: &output, Err: io.Discard})
+	parsed := current.parse([]string{"first"})
+	if parsed.Failure != nil {
+		t.Fatalf("initial parse failed: %s", parsed.Failure.Message)
+	}
+
+	late, err := NewCommand(CommandSpec[struct{}]{
+		Name:    "late",
+		Summary: "registered during execution",
+		Run:     func(Invocation, struct{}) Outcome { return Success() },
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := app.Register(late); err != nil {
+		t.Fatal(err)
+	}
+
+	if got := current.renderHelp([]string{"late"}); got.Kind != OutcomeSuccess {
+		t.Fatalf("help after registration = %#v", got)
+	}
+	if !strings.Contains(output.String(), "registered during execution") {
+		t.Fatalf("help did not use registered metadata: %q", output.String())
+	}
+	if got := current.usageLine([]string{"late"}); !strings.HasPrefix(got, "Usage: example late") {
+		t.Fatalf("usage after registration = %q", got)
+	}
+}
+
 func TestBridgeTreeRefreshesAfterSuccessfulRegistration(t *testing.T) {
 	app, err := New(Identity{Name: "example"})
 	if err != nil {