mirror of
https://github.com/mark3labs/kit.git
synced 2026-06-14 03:30:26 +00:00
fix(kit): make Options.Streaming a *bool to honor unset
- Change Options.Streaming from bool to *bool so a zero-valued Options no longer forces stream=false; New only sets the key when non-nil, letting streaming resolve through the precedence chain (env -> config -> default true). This also fixes the CLI path, which never set the field - Mirror the existing sampling-parameter pointer pattern instead of adding a separate StreamingSet sentinel, keeping Options internally consistent - Update WithStreaming/NewAgent, subagent, and ACP callers to the pointer form; add regression tests for the nil-default and explicit opt-out paths - Update SDK docs (README, pkg/kit/README, options page) with the ptrBool helper and *bool semantics
This commit is contained in:
@@ -556,7 +556,7 @@ host, err := kit.New(ctx, &kit.Options{
|
||||
SystemPrompt: "You are a helpful bot",
|
||||
ConfigFile: "/path/to/config.yml",
|
||||
MaxSteps: 10,
|
||||
Streaming: true,
|
||||
Streaming: ptr(true), // *bool: nil = unset (default true), &false = off
|
||||
Quiet: true,
|
||||
|
||||
// Generation parameters (override env/config/per-model defaults)
|
||||
|
||||
@@ -45,10 +45,11 @@ func (r *sessionRegistry) create(ctx context.Context, cwd string) (*acpSession,
|
||||
// the process-global store (which cobra populated from flags) so launching
|
||||
// `kit acp -m <model> [--thinking-level ...] [--provider-url ...]` is still
|
||||
// honored; .kit.yml and KIT_* env vars are loaded per session by kit.New.
|
||||
streamOn := true
|
||||
kitInstance, err := kit.New(ctx, &kit.Options{
|
||||
SessionDir: cwd,
|
||||
Quiet: true,
|
||||
Streaming: true,
|
||||
Streaming: &streamOn,
|
||||
Model: viper.GetString("model"),
|
||||
ThinkingLevel: viper.GetString("thinking-level"),
|
||||
ProviderURL: viper.GetString("provider-url"),
|
||||
|
||||
+1
-1
@@ -89,7 +89,7 @@ host, err := kit.New(ctx, &kit.Options{
|
||||
SystemPrompt: "You are a helpful bot", // Override system prompt
|
||||
ConfigFile: "/path/to/config.yml", // Use specific config file
|
||||
MaxSteps: 10, // Override max steps
|
||||
Streaming: true, // Enable streaming
|
||||
Streaming: ptrBool(true), // *bool: nil = unset (default true), &false = off
|
||||
Quiet: true, // Suppress debug output
|
||||
|
||||
// Session options
|
||||
|
||||
+22
-11
@@ -890,10 +890,17 @@ type Options struct {
|
||||
SystemPrompt string // Override system prompt
|
||||
ConfigFile string // Override config file path
|
||||
MaxSteps int // Override max steps (0 = use default)
|
||||
Streaming bool // Enable streaming (default from config)
|
||||
Quiet bool // Suppress debug output
|
||||
Tools []Tool // Custom tool set. If empty, AllTools() is used.
|
||||
ExtraTools []Tool // Additional tools added alongside core/MCP/extension tools.
|
||||
|
||||
// Streaming enables or disables streaming output. It is a pointer so the
|
||||
// SDK can distinguish "unset" (nil) from an explicit choice, mirroring the
|
||||
// sampling-parameter fields below. nil leaves streaming to the precedence
|
||||
// chain (env → .kit.yml → default true); a non-nil value forces it. Prefer
|
||||
// [WithStreaming] for the functional-options API.
|
||||
Streaming *bool
|
||||
|
||||
Quiet bool // Suppress debug output
|
||||
Tools []Tool // Custom tool set. If empty, AllTools() is used.
|
||||
ExtraTools []Tool // Additional tools added alongside core/MCP/extension tools.
|
||||
|
||||
// Generation parameters. These override the corresponding values from
|
||||
// .kit.yml / KIT_* environment variables. Leaving a field at its
|
||||
@@ -1250,7 +1257,12 @@ func New(ctx context.Context, opts *Options) (*Kit, error) {
|
||||
if opts.MaxSteps > 0 {
|
||||
v.Set("max-steps", opts.MaxSteps)
|
||||
}
|
||||
v.Set("stream", opts.Streaming)
|
||||
// Only override streaming when the caller explicitly set it. Otherwise
|
||||
// leave the precedence chain (env → config → default true) untouched so a
|
||||
// zero-valued Options does not silently force stream=false.
|
||||
if opts.Streaming != nil {
|
||||
v.Set("stream", *opts.Streaming)
|
||||
}
|
||||
|
||||
// Generation parameter overrides. Each Options field, when set,
|
||||
// is pushed into the instance store here so the existing downstream
|
||||
@@ -1905,19 +1917,18 @@ func (m *Kit) Subagent(ctx context.Context, cfg SubagentConfig) (*SubagentResult
|
||||
|
||||
// Create child Kit instance. Pass the parent's loaded MCP config to
|
||||
// avoid re-loading and re-validating config for the child.
|
||||
// Streaming must be explicitly enabled — Options.Streaming defaults to
|
||||
// false, and New() writes Set("stream", opts.Streaming) on the child's
|
||||
// isolated store. Without this, the subagent would potentially hit
|
||||
// Streaming is enabled explicitly — without it, non-streaming can hit
|
||||
// provider-level differences (e.g. Anthropic non-streaming timeouts with
|
||||
// extended thinking). The child gets its own config store, so this no
|
||||
// longer affects any other concurrent caller.
|
||||
// extended thinking). The child gets its own config store, so this does not
|
||||
// affect any other concurrent caller.
|
||||
streamOn := true
|
||||
childOpts := &Options{
|
||||
Model: model,
|
||||
SystemPrompt: systemPrompt,
|
||||
Tools: tools,
|
||||
NoSession: cfg.NoSession,
|
||||
Quiet: true,
|
||||
Streaming: true,
|
||||
Streaming: &streamOn,
|
||||
MCPConfig: m.mcpConfig,
|
||||
}
|
||||
// Propagate the parent's MCP task configuration so a child subagent
|
||||
|
||||
+6
-4
@@ -27,9 +27,9 @@ type Option func(*Options)
|
||||
// )
|
||||
func NewAgent(ctx context.Context, opts ...Option) (*Kit, error) {
|
||||
// Streaming defaults to true for the ergonomic constructor — this is the
|
||||
// natural expectation for interactive agents and avoids the latent
|
||||
// "false always wins" edge case of the raw Options.Streaming zero value.
|
||||
o := &Options{Streaming: true}
|
||||
// natural expectation for interactive agents. WithStreaming(false) overrides it.
|
||||
streamOn := true
|
||||
o := &Options{Streaming: &streamOn}
|
||||
for _, fn := range opts {
|
||||
fn(o)
|
||||
}
|
||||
@@ -46,7 +46,9 @@ func WithSystemPrompt(p string) Option { return func(o *Options) { o.SystemPromp
|
||||
|
||||
// WithStreaming enables or disables streaming responses. [NewAgent] enables
|
||||
// streaming by default, so pass WithStreaming(false) to opt out.
|
||||
func WithStreaming(b bool) Option { return func(o *Options) { o.Streaming = b } }
|
||||
func WithStreaming(b bool) Option {
|
||||
return func(o *Options) { o.Streaming = &b }
|
||||
}
|
||||
|
||||
// WithMaxTokens sets the maximum output tokens per LLM response. A value of 0
|
||||
// lets the precedence chain (env → config → per-model → SDK floor) resolve a
|
||||
|
||||
@@ -50,8 +50,10 @@ func TestOptionFunctionsPlumbing(t *testing.T) {
|
||||
if o.ConfigFile != "/tmp/.kit.yml" {
|
||||
t.Errorf("WithConfigFile: got %q", o.ConfigFile)
|
||||
}
|
||||
if o.Streaming {
|
||||
t.Error("WithStreaming(false): expected Streaming=false")
|
||||
if o.Streaming == nil {
|
||||
t.Error("WithStreaming: expected Streaming to be set (non-nil)")
|
||||
} else if *o.Streaming {
|
||||
t.Error("WithStreaming(false): expected *Streaming=false")
|
||||
}
|
||||
if !o.Debug {
|
||||
t.Error("WithDebug: expected Debug=true")
|
||||
@@ -176,3 +178,55 @@ func TestNewAgentStreamingOptOut(t *testing.T) {
|
||||
t.Error("WithStreaming(false) should disable streaming")
|
||||
}
|
||||
}
|
||||
|
||||
// TestNewZeroOptionsKeepsStreamingDefault is the regression test for the
|
||||
// unconditional `v.Set("stream", opts.Streaming)` bug: a zero-valued Options
|
||||
// (Streaming == nil) must NOT force stream=false. With Streaming unset,
|
||||
// streaming resolves through the precedence chain, whose SDK default is true.
|
||||
func TestNewZeroOptionsKeepsStreamingDefault(t *testing.T) {
|
||||
if os.Getenv("ANTHROPIC_API_KEY") == "" {
|
||||
t.Skip("Skipping test: ANTHROPIC_API_KEY not set")
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
k, err := kit.New(ctx, &kit.Options{
|
||||
Model: "anthropic/claude-sonnet-4-5-20250929",
|
||||
Quiet: true,
|
||||
NoSession: true,
|
||||
SkipConfig: true, // isolate from any ~/.kit.yml / env stream setting
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("New failed: %v", err)
|
||||
}
|
||||
defer func() { _ = k.Close() }()
|
||||
|
||||
if !k.ConfigBoolForTest("stream") {
|
||||
t.Error("zero-valued Options must not force stream=false; expected the default (true)")
|
||||
}
|
||||
}
|
||||
|
||||
// TestNewStreamingExplicitOptOut verifies that a raw Options can still disable
|
||||
// streaming by setting Streaming to a pointer to false.
|
||||
func TestNewStreamingExplicitOptOut(t *testing.T) {
|
||||
if os.Getenv("ANTHROPIC_API_KEY") == "" {
|
||||
t.Skip("Skipping test: ANTHROPIC_API_KEY not set")
|
||||
}
|
||||
|
||||
streamOff := false
|
||||
ctx := context.Background()
|
||||
k, err := kit.New(ctx, &kit.Options{
|
||||
Model: "anthropic/claude-sonnet-4-5-20250929",
|
||||
Quiet: true,
|
||||
NoSession: true,
|
||||
SkipConfig: true,
|
||||
Streaming: &streamOff,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("New failed: %v", err)
|
||||
}
|
||||
defer func() { _ = k.Close() }()
|
||||
|
||||
if k.ConfigBoolForTest("stream") {
|
||||
t.Error("Streaming=&false should disable streaming")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -28,7 +28,7 @@ host, err := kit.New(ctx, &kit.Options{
|
||||
|
||||
// Behavior
|
||||
MaxSteps: 10,
|
||||
Streaming: true,
|
||||
Streaming: ptrBool(true), // *bool: nil = unset (default true), &false = off
|
||||
Quiet: true,
|
||||
Debug: true,
|
||||
|
||||
@@ -101,7 +101,7 @@ host, err := kit.New(ctx, &kit.Options{
|
||||
| `SystemPrompt` | `string` | — | System prompt text or file path |
|
||||
| `ConfigFile` | `string` | `~/.kit.yml` | Path to config file |
|
||||
| `MaxSteps` | `int` | `0` | Max agent steps (0 = unlimited) |
|
||||
| `Streaming` | `bool` | `true` | Enable streaming output |
|
||||
| `Streaming` | `*bool` | `nil` | Enable streaming output. `nil` leaves it to the precedence chain (env → config → default `true`); `&true`/`&false` forces it. Pointer so unset is distinct from explicit `false`. |
|
||||
| `Quiet` | `bool` | `false` | Suppress output |
|
||||
| `Debug` | `bool` | `false` | Enable debug logging |
|
||||
|
||||
@@ -124,9 +124,10 @@ defaults for samplers).
|
||||
| `FrequencyPenalty` | `*float32` | — | OpenAI-family frequency penalty. `nil` leaves provider default. |
|
||||
| `PresencePenalty` | `*float32` | — | OpenAI-family presence penalty. `nil` leaves provider default. |
|
||||
|
||||
Pointer-typed samplers are populated via a tiny helper:
|
||||
Pointer-typed fields (`Streaming` and the samplers) are populated via tiny helpers:
|
||||
|
||||
```go
|
||||
func ptrBool(v bool) *bool { return &v }
|
||||
func ptrFloat32(v float32) *float32 { return &v }
|
||||
```
|
||||
|
||||
|
||||
Reference in New Issue
Block a user