Revert "Feat/channel tool feedback animation (#2569)" (#2596)

This reverts commit e556a816e4.

pkg/agent/loop.go

@@ -112,7 +112,6 @@ const (
 	pendingTurnPrefix          = "pending-"
 	metadataKeyMessageKind     = "message_kind"
 	messageKindThought         = "thought"
-	messageKindToolFeedback    = "tool_feedback"
 	metadataKeyAccountID       = "account_id"
 	metadataKeyGuildID         = "guild_id"
 	metadataKeyTeamID          = "team_id"

pkg/agent/loop_test.go

@@ -24,7 +24,6 @@ import (
 	"github.com/sipeed/picoclaw/pkg/routing"
 	"github.com/sipeed/picoclaw/pkg/session"
 	"github.com/sipeed/picoclaw/pkg/tools"
-	"github.com/sipeed/picoclaw/pkg/utils"
 )
 
 type fakeChannel struct{ id string }
@@ -1759,157 +1758,6 @@ func (m *toolFeedbackProvider) GetDefaultModel() string {
 	return "heartbeat-tool-feedback-model"
 }
 
-type toolFeedbackReasoningProvider struct {
-	filePath string
-	calls    int
-}
-
-func (m *toolFeedbackReasoningProvider) Chat(
-	ctx context.Context,
-	messages []providers.Message,
-	tools []providers.ToolDefinition,
-	model string,
-	opts map[string]any,
-) (*providers.LLMResponse, error) {
-	m.calls++
-	if m.calls == 1 {
-		return &providers.LLMResponse{
-			ReasoningContent: "Read README.md first to confirm the context that needs to be changed.",
-			ToolCalls: []providers.ToolCall{{
-				ID:        "call_reasoning_read_file",
-				Type:      "function",
-				Name:      "read_file",
-				Arguments: map[string]any{"path": m.filePath},
-			}},
-		}, nil
-	}
-
-	return &providers.LLMResponse{
-		Content:   "DONE",
-		ToolCalls: []providers.ToolCall{},
-	}, nil
-}
-
-func (m *toolFeedbackReasoningProvider) GetDefaultModel() string {
-	return "tool-feedback-reasoning-model"
-}
-
-func TestToolFeedbackExplanationFromResponse_UsesCurrentContentFirst(t *testing.T) {
-	response := &providers.LLMResponse{
-		Content:          "Read README.md first",
-		ReasoningContent: "current reasoning fallback",
-	}
-	messages := []providers.Message{
-		{Role: "user", Content: "check file"},
-		{Role: "assistant", Content: "Previous turn explanation"},
-		{Role: "tool", Content: "tool output", ToolCallID: "call_1"},
-	}
-
-	got := toolFeedbackExplanationFromResponse(response, messages, 300)
-	if got != "Read README.md first" {
-		t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want current content", got)
-	}
-}
-
-func TestToolFeedbackExplanationFromResponse_UsesExplicitToolCallExtraContent(t *testing.T) {
-	response := &providers.LLMResponse{
-		ToolCalls: []providers.ToolCall{{
-			ID:   "call_1",
-			Name: "read_file",
-			ExtraContent: &providers.ExtraContent{
-				ToolFeedbackExplanation: "Read README.md first to confirm the current project structure.",
-			},
-		}},
-	}
-	messages := []providers.Message{
-		{Role: "user", Content: "check file"},
-		{Role: "assistant", Content: ""},
-		{Role: "tool", Content: "tool output", ToolCallID: "call_1"},
-	}
-
-	got := toolFeedbackExplanationFromResponse(response, messages, 300)
-	if got != "Read README.md first to confirm the current project structure." {
-		t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want explicit tool feedback explanation", got)
-	}
-}
-
-func TestToolFeedbackExplanationForToolCall_PrefersToolSpecificExtraContent(t *testing.T) {
-	response := &providers.LLMResponse{
-		Content: "Shared explanation",
-		ToolCalls: []providers.ToolCall{
-			{
-				ID:   "call_1",
-				Name: "read_file",
-				ExtraContent: &providers.ExtraContent{
-					ToolFeedbackExplanation: "Read README.md first.",
-				},
-			},
-			{
-				ID:   "call_2",
-				Name: "edit_file",
-				ExtraContent: &providers.ExtraContent{
-					ToolFeedbackExplanation: "Update config example after reading it.",
-				},
-			},
-		},
-	}
-
-	got1 := toolFeedbackExplanationForToolCall(response, response.ToolCalls[0], nil, 300)
-	got2 := toolFeedbackExplanationForToolCall(response, response.ToolCalls[1], nil, 300)
-	if got1 != "Read README.md first." {
-		t.Fatalf("toolFeedbackExplanationForToolCall() first = %q, want tool-specific explanation", got1)
-	}
-	if got2 != "Update config example after reading it." {
-		t.Fatalf("toolFeedbackExplanationForToolCall() second = %q, want tool-specific explanation", got2)
-	}
-}
-
-func TestToolFeedbackExplanationForToolCall_DoesNotReuseAnotherToolCallExplanation(t *testing.T) {
-	response := &providers.LLMResponse{
-		ToolCalls: []providers.ToolCall{
-			{
-				ID:   "call_1",
-				Name: "read_file",
-			},
-			{
-				ID:   "call_2",
-				Name: "edit_file",
-				ExtraContent: &providers.ExtraContent{
-					ToolFeedbackExplanation: "Update config example after reading it.",
-				},
-			},
-		},
-	}
-	messages := []providers.Message{
-		{Role: "user", Content: "inspect the config and update the example"},
-	}
-
-	got := toolFeedbackExplanationForToolCall(response, response.ToolCalls[0], messages, 300)
-	want := utils.ToolFeedbackContinuationHint + ": inspect the config and update the example"
-	if got != want {
-		t.Fatalf("toolFeedbackExplanationForToolCall() = %q, want %q", got, want)
-	}
-}
-
-func TestToolFeedbackExplanationFromResponse_DoesNotUseReasoningContent(t *testing.T) {
-	response := &providers.LLMResponse{
-		Content:          "",
-		ReasoningContent: "hidden reasoning should not be shown",
-	}
-	messages := []providers.Message{
-		{Role: "user", Content: "check file"},
-		{Role: "assistant", Content: "Previous turn explanation"},
-		{Role: "user", Content: "Inspect README.md and update the config example."},
-		{Role: "tool", Content: "tool output", ToolCallID: "call_1"},
-	}
-
-	got := toolFeedbackExplanationFromResponse(response, messages, 300)
-	want := utils.ToolFeedbackContinuationHint + ": Inspect README.md and update the config example."
-	if got != want {
-		t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want latest user content fallback", got)
-	}
-}
-
 type picoInterleavedContentProvider struct {
 	calls int
 }
@@ -3808,16 +3656,7 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) {
 			t.Fatalf("unexpected tool feedback context: %+v", outbound.Context)
 		}
 		if !strings.Contains(outbound.Content, "`read_file`") {
-			t.Fatalf("tool feedback content = %q, want read_file summary", outbound.Content)
-		}
-		if !strings.Contains(outbound.Content, utils.ToolFeedbackContinuationHint) {
-			t.Fatalf("tool feedback content = %q, want continuation hint fallback", outbound.Content)
-		}
-		if !strings.Contains(outbound.Content, "check tool feedback") {
-			t.Fatalf("tool feedback content = %q, want current user intent fallback", outbound.Content)
-		}
-		if strings.Contains(outbound.Content, "Previous turn explanation") {
-			t.Fatalf("tool feedback content = %q, want no previous assistant fallback", outbound.Content)
+			t.Fatalf("tool feedback content = %q, want read_file preview", outbound.Content)
 		}
 		if outbound.AgentID != "main" {
 			t.Fatalf("tool feedback agent_id = %q, want main", outbound.AgentID)
@@ -3833,130 +3672,6 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) {
 	}
 }
 
-func TestProcessMessage_DoesNotLeakReasoningContentInToolFeedback(t *testing.T) {
-	tmpDir := t.TempDir()
-	heartbeatFile := filepath.Join(tmpDir, "tool-feedback-reasoning.txt")
-	if err := os.WriteFile(heartbeatFile, []byte("tool feedback task"), 0o644); err != nil {
-		t.Fatalf("WriteFile() error = %v", err)
-	}
-
-	cfg := &config.Config{
-		Agents: config.AgentsConfig{
-			Defaults: config.AgentDefaults{
-				Workspace:         tmpDir,
-				ModelName:         "test-model",
-				MaxTokens:         4096,
-				MaxToolIterations: 10,
-				ToolFeedback: config.ToolFeedbackConfig{
-					Enabled:       true,
-					MaxArgsLength: 300,
-				},
-			},
-		},
-		Tools: config.ToolsConfig{
-			ReadFile: config.ReadFileToolConfig{
-				Enabled: true,
-			},
-		},
-	}
-
-	msgBus := bus.NewMessageBus()
-	provider := &toolFeedbackReasoningProvider{filePath: heartbeatFile}
-	al := NewAgentLoop(cfg, msgBus, provider)
-
-	response, err := al.processMessage(context.Background(), testInboundMessage(bus.InboundMessage{
-		Channel:  "telegram",
-		SenderID: "user-1",
-		ChatID:   "chat-1",
-		Content:  "check reasoning fallback",
-	}))
-	if err != nil {
-		t.Fatalf("processMessage() error = %v", err)
-	}
-	if response != "DONE" {
-		t.Fatalf("processMessage() response = %q, want %q", response, "DONE")
-	}
-
-	select {
-	case outbound := <-msgBus.OutboundChan():
-		if !strings.Contains(outbound.Content, "`read_file`") {
-			t.Fatalf("tool feedback content = %q, want read_file summary", outbound.Content)
-		}
-		if !strings.Contains(outbound.Content, utils.ToolFeedbackContinuationHint) {
-			t.Fatalf("tool feedback content = %q, want continuation hint fallback", outbound.Content)
-		}
-		if !strings.Contains(outbound.Content, "check reasoning fallback") {
-			t.Fatalf("tool feedback content = %q, want current user intent fallback", outbound.Content)
-		}
-		if strings.Contains(outbound.Content, "Read README.md first") {
-			t.Fatalf("tool feedback content = %q, should not leak hidden reasoning", outbound.Content)
-		}
-	case <-time.After(2 * time.Second):
-		t.Fatal("expected outbound tool feedback without leaking reasoning")
-	}
-}
-
-func TestProcessMessage_DoesNotPublishToolFeedbackForDiscordWhenDisabled(t *testing.T) {
-	assertToolFeedbackNotPublishedWhenDisabled(t, "discord")
-}
-
-func assertToolFeedbackNotPublishedWhenDisabled(t *testing.T, channel string) {
-	t.Helper()
-
-	tmpDir := t.TempDir()
-	heartbeatFile := filepath.Join(tmpDir, "tool-feedback-"+channel+".txt")
-	if err := os.WriteFile(heartbeatFile, []byte("tool feedback task"), 0o644); err != nil {
-		t.Fatalf("WriteFile() error = %v", err)
-	}
-
-	cfg := &config.Config{
-		Agents: config.AgentsConfig{
-			Defaults: config.AgentDefaults{
-				Workspace:         tmpDir,
-				ModelName:         "test-model",
-				MaxTokens:         4096,
-				MaxToolIterations: 10,
-			},
-		},
-		Tools: config.ToolsConfig{
-			ReadFile: config.ReadFileToolConfig{
-				Enabled: true,
-			},
-		},
-	}
-
-	msgBus := bus.NewMessageBus()
-	provider := &toolFeedbackProvider{filePath: heartbeatFile}
-	al := NewAgentLoop(cfg, msgBus, provider)
-
-	response, err := al.processMessage(context.Background(), testInboundMessage(bus.InboundMessage{
-		Channel:  channel,
-		SenderID: "user-1",
-		ChatID:   "chat-1",
-		Content:  "check tool feedback",
-	}))
-	if err != nil {
-		t.Fatalf("processMessage() error = %v", err)
-	}
-	if response != "HEARTBEAT_OK" {
-		t.Fatalf("processMessage() response = %q, want %q", response, "HEARTBEAT_OK")
-	}
-
-	select {
-	case outbound := <-msgBus.OutboundChan():
-		t.Fatalf("expected no outbound tool feedback for %s when disabled, got %+v", channel, outbound)
-	case <-time.After(200 * time.Millisecond):
-	}
-}
-
-func TestProcessMessage_DoesNotPublishToolFeedbackForTelegramWhenDisabled(t *testing.T) {
-	assertToolFeedbackNotPublishedWhenDisabled(t, "telegram")
-}
-
-func TestProcessMessage_DoesNotPublishToolFeedbackForFeishuWhenDisabled(t *testing.T) {
-	assertToolFeedbackNotPublishedWhenDisabled(t, "feishu")
-}
-
 func TestProcessMessage_MessageToolPublishesOutboundWithTurnMetadata(t *testing.T) {
 	cfg := config.DefaultConfig()
 	cfg.Agents.Defaults.Workspace = t.TempDir()
@@ -4131,85 +3846,6 @@ func TestRunAgentLoop_PicoSkipsInterimPublishWhenNotAllowed(t *testing.T) {
 	}
 }
 
-func TestRun_PicoToolFeedbackSuppressesDuplicateInterimAssistantContent(t *testing.T) {
-	tmpDir := t.TempDir()
-
-	cfg := &config.Config{
-		Agents: config.AgentsConfig{
-			Defaults: config.AgentDefaults{
-				Workspace:         tmpDir,
-				ModelName:         "test-model",
-				MaxTokens:         4096,
-				MaxToolIterations: 10,
-				ToolFeedback: config.ToolFeedbackConfig{
-					Enabled: true,
-				},
-			},
-		},
-	}
-
-	msgBus := bus.NewMessageBus()
-	provider := &picoInterleavedContentProvider{}
-	al := NewAgentLoop(cfg, msgBus, provider)
-
-	agent := al.GetRegistry().GetDefaultAgent()
-	if agent == nil {
-		t.Fatal("expected default agent")
-	}
-	agent.Tools.Register(&toolLimitTestTool{})
-
-	runCtx, runCancel := context.WithCancel(context.Background())
-	defer runCancel()
-
-	runDone := make(chan error, 1)
-	go func() {
-		runDone <- al.Run(runCtx)
-	}()
-
-	if err := msgBus.PublishInbound(context.Background(), bus.InboundMessage{
-		Channel:  "pico",
-		SenderID: "user-1",
-		ChatID:   "session-1",
-		Content:  "run with tools",
-	}); err != nil {
-		t.Fatalf("PublishInbound() error = %v", err)
-	}
-
-	outputs := make([]string, 0, 2)
-	deadline := time.After(2 * time.Second)
-	for len(outputs) < 2 {
-		select {
-		case outbound := <-msgBus.OutboundChan():
-			outputs = append(outputs, outbound.Content)
-		case <-deadline:
-			t.Fatalf("timed out waiting for pico outputs, got %v", outputs)
-		}
-	}
-
-	if outputs[0] != "🔧 `tool_limit_test_tool`\nintermediate model text" {
-		t.Fatalf("first outbound content = %q, want tool feedback summary", outputs[0])
-	}
-	if outputs[1] != "final model text" {
-		t.Fatalf("second outbound content = %q, want %q", outputs[1], "final model text")
-	}
-
-	runCancel()
-	select {
-	case err := <-runDone:
-		if err != nil {
-			t.Fatalf("Run() error = %v", err)
-		}
-	case <-time.After(2 * time.Second):
-		t.Fatal("timed out waiting for Run() to exit")
-	}
-
-	select {
-	case outbound := <-msgBus.OutboundChan():
-		t.Fatalf("unexpected extra pico output after tool feedback + final reply: %+v", outbound)
-	case <-time.After(200 * time.Millisecond):
-	}
-}
-
 func TestResolveMediaRefs_ResolvesToBase64(t *testing.T) {
 	store := media.NewFileMediaStore()
 	dir := t.TempDir()

pkg/agent/loop_turn.go

@@ -635,11 +635,7 @@ turnLoop:
 		}
 		logger.DebugCF("agent", "LLM response", llmResponseFields)
 
-		if al.bus != nil &&
-			ts.channel == "pico" &&
-			len(response.ToolCalls) > 0 &&
-			ts.opts.AllowInterimPicoPublish &&
-			!shouldPublishToolFeedback(al.cfg, ts) {
+		if al.bus != nil && ts.channel == "pico" && len(response.ToolCalls) > 0 && ts.opts.AllowInterimPicoPublish {
 			if strings.TrimSpace(response.Content) != "" {
 				outCtx, outCancel := context.WithTimeout(turnCtx, 3*time.Second)
 				err := al.bus.PublishOutbound(outCtx, bus.OutboundMessage{
@@ -709,19 +705,7 @@ turnLoop:
 		}
 		for _, tc := range normalizedToolCalls {
 			argumentsJSON, _ := json.Marshal(tc.Arguments)
-			toolFeedbackExplanation := toolFeedbackExplanationForToolCall(
-				response,
-				tc,
-				messages,
-				al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
-			)
 			extraContent := tc.ExtraContent
-			if strings.TrimSpace(toolFeedbackExplanation) != "" {
-				if extraContent == nil {
-					extraContent = &providers.ExtraContent{}
-				}
-				extraContent.ToolFeedbackExplanation = toolFeedbackExplanation
-			}
 			thoughtSignature := ""
 			if tc.Function != nil {
 				thoughtSignature = tc.Function.ThoughtSignature
@@ -799,16 +783,21 @@ turnLoop:
 						)
 
 						// Send tool feedback to chat channel if enabled (same as normal tool execution)
-						if shouldPublishToolFeedback(al.cfg, ts) {
-							toolFeedbackExplanation := toolFeedbackExplanationForToolCall(
-								response,
-								tc,
-								messages,
+						if al.cfg.Agents.Defaults.IsToolFeedbackEnabled() &&
+							ts.channel != "" &&
+							!ts.opts.SuppressToolFeedback {
+							argsJSON, _ := json.Marshal(toolArgs)
+							feedbackPreview := utils.Truncate(
+								string(argsJSON),
 								al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
 							)
-							feedbackMsg := utils.FormatToolFeedbackMessage(toolName, toolFeedbackExplanation)
+							feedbackMsg := utils.FormatToolFeedbackMessage(toolName, feedbackPreview)
 							fbCtx, fbCancel := context.WithTimeout(turnCtx, 3*time.Second)
-							_ = al.bus.PublishOutbound(fbCtx, outboundMessageForTurnWithKind(ts, feedbackMsg, messageKindToolFeedback))
+							_ = al.bus.PublishOutbound(fbCtx, bus.OutboundMessage{
+								Channel: ts.channel,
+								ChatID:  ts.chatID,
+								Content: feedbackMsg,
+							})
 							fbCancel()
 						}
 
@@ -1078,16 +1067,16 @@ turnLoop:
 			)
 
 			// Send tool feedback to chat channel if enabled (from HEAD)
-			if shouldPublishToolFeedback(al.cfg, ts) {
-				toolFeedbackExplanation := toolFeedbackExplanationForToolCall(
-					response,
-					tc,
-					messages,
+			if al.cfg.Agents.Defaults.IsToolFeedbackEnabled() &&
+				ts.channel != "" &&
+				!ts.opts.SuppressToolFeedback {
+				feedbackPreview := utils.Truncate(
+					string(argsJSON),
 					al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
 				)
-				feedbackMsg := utils.FormatToolFeedbackMessage(tc.Name, toolFeedbackExplanation)
+				feedbackMsg := utils.FormatToolFeedbackMessage(tc.Name, feedbackPreview)
 				fbCtx, fbCancel := context.WithTimeout(turnCtx, 3*time.Second)
-				_ = al.bus.PublishOutbound(fbCtx, outboundMessageForTurnWithKind(ts, feedbackMsg, messageKindToolFeedback))
+				_ = al.bus.PublishOutbound(fbCtx, outboundMessageForTurn(ts, feedbackMsg))
 				fbCancel()
 			}
 

pkg/agent/loop_utils.go

@@ -11,7 +11,6 @@ import (
 
 	"github.com/sipeed/picoclaw/pkg/bus"
 	"github.com/sipeed/picoclaw/pkg/commands"
-	"github.com/sipeed/picoclaw/pkg/config"
 	"github.com/sipeed/picoclaw/pkg/providers"
 	"github.com/sipeed/picoclaw/pkg/session"
 	"github.com/sipeed/picoclaw/pkg/utils"
@@ -85,98 +84,6 @@ func outboundMessageForTurn(ts *turnState, content string) bus.OutboundMessage {
 	}
 }
 
-func outboundMessageForTurnWithKind(ts *turnState, content, kind string) bus.OutboundMessage {
-	msg := outboundMessageForTurn(ts, content)
-	if strings.TrimSpace(kind) == "" {
-		return msg
-	}
-	if msg.Context.Raw == nil {
-		msg.Context.Raw = make(map[string]string, 1)
-	}
-	msg.Context.Raw[metadataKeyMessageKind] = kind
-	return msg
-}
-
-func latestUserContent(messages []providers.Message) string {
-	for i := len(messages) - 1; i >= 0; i-- {
-		msg := messages[i]
-		if msg.Role != "user" {
-			continue
-		}
-		if content := strings.TrimSpace(msg.Content); content != "" {
-			return content
-		}
-	}
-	return ""
-}
-
-func toolFeedbackExplanationFromResponse(
-	response *providers.LLMResponse,
-	messages []providers.Message,
-	maxLen int,
-) string {
-	if response == nil {
-		return ""
-	}
-	explanation := strings.TrimSpace(response.Content)
-	if explanation == "" {
-		explanation = toolFeedbackExplanationFromToolCalls(response.ToolCalls)
-	}
-	if explanation == "" {
-		explanation = toolFeedbackExplanationFromMessages(messages)
-	}
-	return utils.Truncate(explanation, maxLen)
-}
-
-func toolFeedbackExplanationFromToolCalls(toolCalls []providers.ToolCall) string {
-	for _, tc := range toolCalls {
-		if tc.ExtraContent == nil {
-			continue
-		}
-		if explanation := strings.TrimSpace(tc.ExtraContent.ToolFeedbackExplanation); explanation != "" {
-			return explanation
-		}
-	}
-	return ""
-}
-
-func toolFeedbackExplanationForToolCall(
-	response *providers.LLMResponse,
-	toolCall providers.ToolCall,
-	messages []providers.Message,
-	maxLen int,
-) string {
-	if toolCall.ExtraContent != nil {
-		if explanation := strings.TrimSpace(toolCall.ExtraContent.ToolFeedbackExplanation); explanation != "" {
-			return utils.Truncate(explanation, maxLen)
-		}
-	}
-	if response == nil {
-		return utils.Truncate(toolFeedbackExplanationFromMessages(messages), maxLen)
-	}
-
-	explanation := strings.TrimSpace(response.Content)
-	if explanation == "" {
-		explanation = toolFeedbackExplanationFromMessages(messages)
-	}
-	return utils.Truncate(explanation, maxLen)
-}
-
-func toolFeedbackExplanationFromMessages(messages []providers.Message) string {
-	explanation := latestUserContent(messages)
-	if explanation != "" {
-		return utils.ToolFeedbackContinuationHint + ": " + explanation
-	}
-	return ""
-}
-
-func shouldPublishToolFeedback(cfg *config.Config, ts *turnState) bool {
-	if ts == nil || ts.channel == "" || ts.opts.SuppressToolFeedback {
-		return false
-	}
-	return cfg != nil && cfg.Agents.Defaults.IsToolFeedbackEnabled()
-}
-
 func cloneEventArguments(args map[string]any) map[string]any {
 	if len(args) == 0 {
 		return nil