diff --git a/config/config.example.json b/config/config.example.json index 910c4fbd38..ac20b4385a 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -16,7 +16,9 @@ "tool_feedback": { "enabled": false, "max_args_length": 300, - "separate_messages": false + "separate_messages": false, + "animation_interval_secs": 3, + "edit_min_interval_seconds": 0 } } }, diff --git a/docs/operations/debug.md b/docs/operations/debug.md index eacd723808..c19e930fc0 100644 --- a/docs/operations/debug.md +++ b/docs/operations/debug.md @@ -66,7 +66,9 @@ Debug logs are server-side only. If you want the agent to send a visible notific "tool_feedback": { "enabled": true, "max_args_length": 300, - "separate_messages": true + "separate_messages": true, + "animation_interval_secs": 3, + "edit_min_interval_seconds": 0 } } } @@ -88,14 +90,19 @@ When `enabled` is `true`, every tool call sends a short message to the chat befo | `enabled` | bool | `false` | Send a chat notification for each tool call | | `separate_messages` | bool | `false` | Keep every tool feedback update as a separate chat message instead of reusing a single placeholder/progress message | | `max_args_length` | int | `300` | Maximum characters of the serialised arguments included in the notification | +| `animation_interval_secs` | int | `3` | Seconds between progress animation edits for channels that support editable tool feedback | +| `edit_min_interval_seconds` | int | `0` | Minimum seconds between edits of the same tracked progress message. `0` preserves legacy behavior with no edit throttle | ### Environment variables -Both fields can also be set via environment variables: +These fields can also be set via environment variables: ```bash PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_ENABLED=true PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_MAX_ARGS_LENGTH=300 +PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_SEPARATE_MESSAGES=false +PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_ANIMATION_INTERVAL_SECS=3 +PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_EDIT_MIN_INTERVAL_SECONDS=10 ``` > **Note:** `tool_feedback` is independent of `--debug` mode. It works in production and does not require the gateway to be started with any special flag. diff --git a/pkg/channels/discord/discord.go b/pkg/channels/discord/discord.go index 514b9b3b1c..4ba1f30cad 100644 --- a/pkg/channels/discord/discord.go +++ b/pkg/channels/discord/discord.go @@ -133,6 +133,12 @@ func (c *DiscordChannel) Start(ctx context.Context) error { return nil } +func (c *DiscordChannel) ConfigureToolFeedbackAnimator(cfg channels.ToolFeedbackAnimatorConfig) { + if c.progress != nil { + c.progress.Configure(cfg) + } +} + func (c *DiscordChannel) Stop(ctx context.Context) error { logger.InfoC("discord", "Stopping Discord bot") c.SetRunning(false) diff --git a/pkg/channels/feishu/feishu_64.go b/pkg/channels/feishu/feishu_64.go index d09c021c78..a98d1b7103 100644 --- a/pkg/channels/feishu/feishu_64.go +++ b/pkg/channels/feishu/feishu_64.go @@ -129,6 +129,12 @@ func (c *FeishuChannel) Start(ctx context.Context) error { return nil } +func (c *FeishuChannel) ConfigureToolFeedbackAnimator(cfg channels.ToolFeedbackAnimatorConfig) { + if c.progress != nil { + c.progress.Configure(cfg) + } +} + func (c *FeishuChannel) Stop(ctx context.Context) error { c.mu.Lock() if c.cancel != nil { diff --git a/pkg/channels/manager.go b/pkg/channels/manager.go index 9d6ca543fe..4036817029 100644 --- a/pkg/channels/manager.go +++ b/pkg/channels/manager.go @@ -143,6 +143,10 @@ type toolFeedbackMessageContentPreparer interface { PrepareToolFeedbackMessageContent(content string) string } +type toolFeedbackAnimatorConfigurer interface { + ConfigureToolFeedbackAnimator(cfg ToolFeedbackAnimatorConfig) +} + type asyncTask struct { cancel context.CancelFunc } @@ -594,6 +598,12 @@ func (m *Manager) initChannel(typeName, channelName string) { if setter, ok := ch.(interface{ SetOwner(ch Channel) }); ok { setter.SetOwner(ch) } + if setter, ok := ch.(toolFeedbackAnimatorConfigurer); ok && m.config != nil { + setter.ConfigureToolFeedbackAnimator(ToolFeedbackAnimatorConfig{ + AnimationInterval: m.config.Agents.Defaults.GetToolFeedbackAnimationInterval(), + MinEditInterval: m.config.Agents.Defaults.GetToolFeedbackEditMinInterval(), + }) + } m.channels[channelName] = ch m.publishChannelEvent( runtimeevents.KindChannelLifecycleInitialized, diff --git a/pkg/channels/matrix/matrix.go b/pkg/channels/matrix/matrix.go index 04599d6d21..79323451ee 100644 --- a/pkg/channels/matrix/matrix.go +++ b/pkg/channels/matrix/matrix.go @@ -299,6 +299,12 @@ func (c *MatrixChannel) Start(ctx context.Context) error { return nil } +func (c *MatrixChannel) ConfigureToolFeedbackAnimator(cfg channels.ToolFeedbackAnimatorConfig) { + if c.progress != nil { + c.progress.Configure(cfg) + } +} + func (c *MatrixChannel) Stop(ctx context.Context) error { logger.InfoC("matrix", "Stopping Matrix channel") c.SetRunning(false) diff --git a/pkg/channels/pico/pico.go b/pkg/channels/pico/pico.go index d1de8f4d53..33e6270497 100644 --- a/pkg/channels/pico/pico.go +++ b/pkg/channels/pico/pico.go @@ -252,6 +252,12 @@ func (c *PicoChannel) Start(ctx context.Context) error { return nil } +func (c *PicoChannel) ConfigureToolFeedbackAnimator(cfg channels.ToolFeedbackAnimatorConfig) { + if c.progress != nil { + c.progress.Configure(cfg) + } +} + // Stop implements Channel. func (c *PicoChannel) Stop(ctx context.Context) error { logger.InfoC("pico", "Stopping Pico Protocol channel") diff --git a/pkg/channels/telegram/telegram.go b/pkg/channels/telegram/telegram.go index cebebfed64..9837e18bda 100644 --- a/pkg/channels/telegram/telegram.go +++ b/pkg/channels/telegram/telegram.go @@ -159,6 +159,12 @@ func (c *TelegramChannel) Start(ctx context.Context) error { return nil } +func (c *TelegramChannel) ConfigureToolFeedbackAnimator(cfg channels.ToolFeedbackAnimatorConfig) { + if c.progress != nil { + c.progress.Configure(cfg) + } +} + func (c *TelegramChannel) Stop(ctx context.Context) error { logger.InfoC("telegram", "Stopping Telegram bot...") c.SetRunning(false) diff --git a/pkg/channels/tool_feedback_animator.go b/pkg/channels/tool_feedback_animator.go index b424612bfe..6ed353f3d0 100644 --- a/pkg/channels/tool_feedback_animator.go +++ b/pkg/channels/tool_feedback_animator.go @@ -2,17 +2,22 @@ package channels import ( "context" + "errors" + "regexp" + "strconv" "strings" "sync" "time" ) -const toolFeedbackAnimationInterval = 3 * time.Second +const defaultToolFeedbackAnimationInterval = 3 * time.Second const initialToolFeedbackAnimationFrame = "" var toolFeedbackAnimationFrames = []string{"..", "."} +var retryAfterPattern = regexp.MustCompile(`retry after:? (\d+)`) + // MaxToolFeedbackAnimationFrameLength returns the largest frame suffix length // so callers can reserve room before sending messages to length-limited APIs. func MaxToolFeedbackAnimationFrameLength() int { @@ -32,18 +37,51 @@ type toolFeedbackAnimationState struct { done chan struct{} } +// ToolFeedbackAnimatorConfig controls how often editable progress messages are +// updated. Zero values preserve the legacy behavior: animation edits every +// three seconds and no minimum interval between content edits. +type ToolFeedbackAnimatorConfig struct { + AnimationInterval time.Duration + MinEditInterval time.Duration +} + type ToolFeedbackAnimator struct { - mu sync.Mutex - editFn func(ctx context.Context, chatID, messageID, content string) error - entries map[string]*toolFeedbackAnimationState + mu sync.Mutex + editFn func(ctx context.Context, chatID, messageID, content string) error + entries map[string]*toolFeedbackAnimationState + animationInterval time.Duration + minEditInterval time.Duration + lastEditAt map[string]time.Time + editPausedTil map[string]time.Time } func NewToolFeedbackAnimator( editFn func(ctx context.Context, chatID, messageID, content string) error, ) *ToolFeedbackAnimator { return &ToolFeedbackAnimator{ - editFn: editFn, - entries: make(map[string]*toolFeedbackAnimationState), + editFn: editFn, + entries: make(map[string]*toolFeedbackAnimationState), + animationInterval: defaultToolFeedbackAnimationInterval, + lastEditAt: make(map[string]time.Time), + editPausedTil: make(map[string]time.Time), + } +} + +func (a *ToolFeedbackAnimator) Configure(cfg ToolFeedbackAnimatorConfig) { + if a == nil { + return + } + a.mu.Lock() + defer a.mu.Unlock() + if cfg.AnimationInterval > 0 { + a.animationInterval = cfg.AnimationInterval + } else { + a.animationInterval = defaultToolFeedbackAnimationInterval + } + if cfg.MinEditInterval > 0 { + a.minEditInterval = cfg.MinEditInterval + } else { + a.minEditInterval = 0 } } @@ -122,12 +160,24 @@ func (a *ToolFeedbackAnimator) Update(ctx context.Context, chatID, content strin return "", false, nil } + if a.shouldSkipEdit(chatID) { + a.Record(chatID, msgID, content) + return msgID, true, nil + } animatedContent := InitialAnimatedToolFeedbackContent(content) if err := a.editFn(ctx, strings.TrimSpace(chatID), msgID, animatedContent); err != nil { + if isMessageNotModifiedError(err) { + a.Record(chatID, msgID, content) + return msgID, true, nil + } + if delay, ok := a.retryAfterDelay(err); ok { + a.pauseEdits(chatID, delay) + } a.Record(chatID, msgID, baseContent) return "", true, err } + a.markEdit(chatID) a.Record(chatID, msgID, content) return msgID, true, nil } @@ -163,7 +213,7 @@ func (a *ToolFeedbackAnimator) detach(chatID string) *toolFeedbackAnimationState func (a *ToolFeedbackAnimator) run(chatID string, entry *toolFeedbackAnimationState) { defer close(entry.done) - ticker := time.NewTicker(toolFeedbackAnimationInterval) + ticker := time.NewTicker(a.getAnimationInterval()) defer ticker.Stop() frameIdx := 1 @@ -176,16 +226,108 @@ func (a *ToolFeedbackAnimator) run(chatID string, entry *toolFeedbackAnimationSt if a.editFn == nil { continue } + if a.shouldSkipEdit(chatID) { + continue + } frame := toolFeedbackAnimationFrames[frameIdx%len(toolFeedbackAnimationFrames)] content := formatAnimatedToolFeedbackContent(entry.baseContent, frame) ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - _ = a.editFn(ctx, chatID, entry.messageID, content) + if err := a.editFn(ctx, chatID, entry.messageID, content); err != nil { + if delay, ok := a.retryAfterDelay(err); ok { + a.pauseEdits(chatID, delay) + } + } else { + a.markEdit(chatID) + } cancel() frameIdx++ } } } +func (a *ToolFeedbackAnimator) getAnimationInterval() time.Duration { + if a == nil { + return defaultToolFeedbackAnimationInterval + } + a.mu.Lock() + defer a.mu.Unlock() + if a.animationInterval > 0 { + return a.animationInterval + } + return defaultToolFeedbackAnimationInterval +} + +func (a *ToolFeedbackAnimator) shouldSkipEdit(chatID string) bool { + if a == nil { + return true + } + now := time.Now() + a.mu.Lock() + defer a.mu.Unlock() + if until := a.editPausedTil[chatID]; until.After(now) { + return true + } + if a.minEditInterval <= 0 { + return false + } + if last := a.lastEditAt[chatID]; !last.IsZero() && now.Sub(last) < a.minEditInterval { + return true + } + return false +} + +func (a *ToolFeedbackAnimator) markEdit(chatID string) { + if a == nil { + return + } + a.mu.Lock() + a.lastEditAt[chatID] = time.Now() + a.mu.Unlock() +} + +func (a *ToolFeedbackAnimator) pauseEdits(chatID string, delay time.Duration) { + if a == nil || delay <= 0 { + return + } + a.mu.Lock() + a.editPausedTil[chatID] = time.Now().Add(delay) + a.mu.Unlock() +} + +func (a *ToolFeedbackAnimator) retryAfterDelay(err error) (time.Duration, bool) { + if err == nil || a == nil { + return 0, false + } + a.mu.Lock() + minInterval := a.minEditInterval + a.mu.Unlock() + if minInterval <= 0 { + return 0, false + } + errText := strings.ToLower(err.Error()) + if !errors.Is(err, ErrRateLimit) && + !strings.Contains(errText, "too many requests") && + !strings.Contains(errText, "429") { + return 0, false + } + match := retryAfterPattern.FindStringSubmatch(errText) + if len(match) != 2 { + return minInterval, true + } + seconds, parseErr := strconv.Atoi(match[1]) + if parseErr != nil || seconds <= 0 { + return minInterval, true + } + return time.Duration(seconds) * time.Second, true +} + +func isMessageNotModifiedError(err error) bool { + if err == nil { + return false + } + return strings.Contains(strings.ToLower(err.Error()), "message is not modified") +} + func InitialAnimatedToolFeedbackContent(baseContent string) string { return formatAnimatedToolFeedbackContent(baseContent, initialToolFeedbackAnimationFrame) } diff --git a/pkg/channels/tool_feedback_animator_test.go b/pkg/channels/tool_feedback_animator_test.go index a232845482..ceae22fed4 100644 --- a/pkg/channels/tool_feedback_animator_test.go +++ b/pkg/channels/tool_feedback_animator_test.go @@ -3,7 +3,9 @@ package channels import ( "context" "errors" + "fmt" "testing" + "time" ) func TestFormatAnimatedToolFeedbackContent(t *testing.T) { @@ -119,3 +121,79 @@ func TestToolFeedbackAnimator_UpdateFailureRestoresTracking(t *testing.T) { t.Fatalf("Current() after failed Update = (%q, %v), want (msg-1, true)", currentID, ok) } } + +func TestToolFeedbackAnimator_UpdateDoesNotDebounceByDefault(t *testing.T) { + editCalls := 0 + animator := NewToolFeedbackAnimator(func(context.Context, string, string, string) error { + editCalls++ + return nil + }) + defer animator.StopAll() + + animator.Record("chat-1", "msg-1", "🔧 `read_file`") + animator.markEdit("chat-1") + + msgID, handled, err := animator.Update(context.Background(), "chat-1", "🔧 `write_file`") + if err != nil { + t.Fatalf("Update() error = %v", err) + } + if !handled || msgID != "msg-1" { + t.Fatalf("Update() = (%q, %v), want (msg-1, true)", msgID, handled) + } + if editCalls != 1 { + t.Fatalf("edit calls = %d, want 1 when min edit interval is unset", editCalls) + } +} + +func TestToolFeedbackAnimator_UpdateDebouncesRecentEditWhenConfigured(t *testing.T) { + editCalls := 0 + animator := NewToolFeedbackAnimator(func(context.Context, string, string, string) error { + editCalls++ + return nil + }) + defer animator.StopAll() + animator.Configure(ToolFeedbackAnimatorConfig{MinEditInterval: 10 * time.Second}) + + animator.Record("chat-1", "msg-1", "🔧 `read_file`") + animator.markEdit("chat-1") + + msgID, handled, err := animator.Update(context.Background(), "chat-1", "🔧 `write_file`") + if err != nil { + t.Fatalf("Update() error = %v", err) + } + if !handled || msgID != "msg-1" { + t.Fatalf("Update() = (%q, %v), want (msg-1, true)", msgID, handled) + } + if editCalls != 0 { + t.Fatalf("edit calls = %d, want 0 while debounced", editCalls) + } + _, baseContent, ok := animator.Take("chat-1") + if !ok { + t.Fatal("expected debounced update to keep tracking") + } + if baseContent != "🔧 `write_file`" { + t.Fatalf("tracked content = %q, want latest content", baseContent) + } +} + +func TestToolFeedbackAnimator_RetryAfterDelayParsesTelegramErrorWhenConfigured(t *testing.T) { + animator := NewToolFeedbackAnimator(nil) + animator.Configure(ToolFeedbackAnimatorConfig{MinEditInterval: 10 * time.Second}) + + err := fmt.Errorf("telego: editMessageText: api: 429 %q: %w", "Too Many Requests: retry after 14", ErrRateLimit) + delay, ok := animator.retryAfterDelay(err) + if !ok { + t.Fatal("retryAfterDelay() ok = false, want true") + } + if delay != 14*time.Second { + t.Fatalf("retryAfterDelay() = %v, want 14s", delay) + } +} + +func TestToolFeedbackAnimator_RetryAfterDisabledByDefault(t *testing.T) { + animator := NewToolFeedbackAnimator(nil) + err := fmt.Errorf("telego: editMessageText: api: 429 %q: %w", "Too Many Requests: retry after 14", ErrRateLimit) + if delay, ok := animator.retryAfterDelay(err); ok { + t.Fatalf("retryAfterDelay() = (%v, true), want disabled by default", delay) + } +} diff --git a/pkg/config/config.go b/pkg/config/config.go index acceee4d5a..a43fa9b15b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -249,9 +249,11 @@ type SubTurnConfig struct { } type ToolFeedbackConfig struct { - Enabled bool `json:"enabled" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_ENABLED"` - MaxArgsLength int `json:"max_args_length" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_MAX_ARGS_LENGTH"` - SeparateMessages bool `json:"separate_messages" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_SEPARATE_MESSAGES"` + Enabled bool `json:"enabled" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_ENABLED"` + MaxArgsLength int `json:"max_args_length" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_MAX_ARGS_LENGTH"` + SeparateMessages bool `json:"separate_messages" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_SEPARATE_MESSAGES"` + AnimationIntervalSecs int `json:"animation_interval_secs" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_ANIMATION_INTERVAL_SECS"` + EditMinIntervalSeconds int `json:"edit_min_interval_seconds" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_EDIT_MIN_INTERVAL_SECONDS"` } type AgentDefaults struct { @@ -311,6 +313,20 @@ func (d *AgentDefaults) IsToolFeedbackSeparateMessagesEnabled() bool { return d.ToolFeedback.SeparateMessages } +func (d *AgentDefaults) GetToolFeedbackAnimationInterval() time.Duration { + if d.ToolFeedback.AnimationIntervalSecs > 0 { + return time.Duration(d.ToolFeedback.AnimationIntervalSecs) * time.Second + } + return 3 * time.Second +} + +func (d *AgentDefaults) GetToolFeedbackEditMinInterval() time.Duration { + if d.ToolFeedback.EditMinIntervalSeconds > 0 { + return time.Duration(d.ToolFeedback.EditMinIntervalSeconds) * time.Second + } + return 0 +} + // GetModelName returns the effective model name for the agent defaults. // It prefers the new "model_name" field but falls back to "model" for backward compatibility. func (d *AgentDefaults) GetModelName() string { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 4f1c5c5e84..baaa938bd1 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -7,6 +7,7 @@ import ( "runtime" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v3" @@ -790,6 +791,12 @@ func TestDefaultConfig_ToolFeedbackDisabled(t *testing.T) { if cfg.Agents.Defaults.ToolFeedback.SeparateMessages { t.Fatal("DefaultConfig().Agents.Defaults.ToolFeedback.SeparateMessages should be false") } + if got := cfg.Agents.Defaults.GetToolFeedbackAnimationInterval(); got != 3*time.Second { + t.Fatalf("DefaultConfig().Agents.Defaults.GetToolFeedbackAnimationInterval() = %v, want 3s", got) + } + if got := cfg.Agents.Defaults.GetToolFeedbackEditMinInterval(); got != 0 { + t.Fatalf("DefaultConfig().Agents.Defaults.GetToolFeedbackEditMinInterval() = %v, want 0", got) + } } func TestLoadConfig_ToolFeedbackDefaultsFalseWhenUnset(t *testing.T) { @@ -813,6 +820,37 @@ func TestLoadConfig_ToolFeedbackDefaultsFalseWhenUnset(t *testing.T) { if cfg.Agents.Defaults.ToolFeedback.SeparateMessages { t.Fatal("agents.defaults.tool_feedback.separate_messages should remain false when unset in config file") } + if got := cfg.Agents.Defaults.GetToolFeedbackAnimationInterval(); got != 3*time.Second { + t.Fatalf("agents.defaults.tool_feedback.animation_interval_secs = %v, want default 3s", got) + } + if got := cfg.Agents.Defaults.GetToolFeedbackEditMinInterval(); got != 0 { + t.Fatalf("agents.defaults.tool_feedback.edit_min_interval_seconds = %v, want default 0", got) + } +} + +func TestLoadConfig_ToolFeedbackThrottleIntervals(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + if err := os.WriteFile( + configPath, + []byte( + `{"version":1,"agents":{"defaults":{"tool_feedback":{"enabled":true,"animation_interval_secs":5,"edit_min_interval_seconds":10}}}}`, + ), + 0o600, + ); err != nil { + t.Fatalf("WriteFile() error: %v", err) + } + + cfg, err := LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error: %v", err) + } + if got := cfg.Agents.Defaults.GetToolFeedbackAnimationInterval(); got != 5*time.Second { + t.Fatalf("agents.defaults.tool_feedback.animation_interval_secs = %v, want 5s", got) + } + if got := cfg.Agents.Defaults.GetToolFeedbackEditMinInterval(); got != 10*time.Second { + t.Fatalf("agents.defaults.tool_feedback.edit_min_interval_seconds = %v, want 10s", got) + } } func TestLoadConfig_WebPreferNativeDefaultsTrueWhenUnset(t *testing.T) { diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 8e2494ae52..ad49357063 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -35,9 +35,11 @@ func DefaultConfig() *Config { SummarizeTokenPercent: 75, SteeringMode: "one-at-a-time", ToolFeedback: ToolFeedbackConfig{ - Enabled: false, - MaxArgsLength: 300, - SeparateMessages: false, + Enabled: false, + MaxArgsLength: 300, + SeparateMessages: false, + AnimationIntervalSecs: 3, + EditMinIntervalSeconds: 0, }, SplitOnMarker: false, MaxLLMRetries: 2,