fix(chat): keep tool summaries and assistant output together

2026-06-12 18:08:54 +00:00 · 2026-04-09 22:15:46 +08:00
parent 20d3522069
commit 5b596ed2f0
3 changed files with 141 additions and 14 deletions
@@ -1409,7 +1409,7 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage)
 		Media:             msg.Media,
 		DefaultResponse:   defaultResponse,
 		EnableSummary:     true,
-		SendResponse:      false,
+		SendResponse:      msg.Channel == "pico",
 	}

 	// context-dependent commands check their own Runtime fields and report
@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -14,6 +15,7 @@ import (

 	"github.com/sipeed/picoclaw/pkg/config"
 	"github.com/sipeed/picoclaw/pkg/providers"
+	"github.com/sipeed/picoclaw/pkg/utils"
 )

 // registerSessionRoutes binds session list and detail endpoints to the ServeMux.
@@ -72,6 +74,8 @@ const (
 	// pkg/memory/jsonl.go so oversized lines fail consistently everywhere.
 	maxSessionJSONLLineSize = 10 * 1024 * 1024
 	maxSessionTitleRunes    = 60
+	// Keep session reconstruction aligned with tool_feedback max args preview.
+	sessionToolFeedbackMaxArgsLength = 300

 	handledToolResponseSummaryText = "Requested output delivered via tool attachment."
 )
@@ -275,6 +279,11 @@ func visibleSessionMessages(messages []providers.Message) []sessionChatMessage {
 			}

 		case "assistant":
+			toolSummaryMessages := visibleAssistantToolSummaryMessages(msg.ToolCalls)
+			if len(toolSummaryMessages) > 0 {
+				transcript = append(transcript, toolSummaryMessages...)
+			}
+
 			visibleToolMessages := visibleAssistantToolMessages(msg.ToolCalls)
 			if len(visibleToolMessages) > 0 {
 				transcript = append(transcript, visibleToolMessages...)
@@ -283,7 +292,7 @@ func visibleSessionMessages(messages []providers.Message) []sessionChatMessage {
 			// Pico web chat can persist both visible `message` tool output and a
 			// later plain assistant reply in the same turn. Hide only the fixed
 			// internal summary that marks handled tool delivery.
-			if len(visibleToolMessages) > 0 || !sessionMessageVisible(msg) || assistantMessageInternalOnly(msg) {
+			if !sessionMessageVisible(msg) || assistantMessageInternalOnly(msg) {
 				continue
 			}

@@ -302,6 +311,50 @@ func assistantMessageInternalOnly(msg providers.Message) bool {
 	return strings.TrimSpace(msg.Content) == handledToolResponseSummaryText
 }

+func visibleAssistantToolSummaryMessages(toolCalls []providers.ToolCall) []sessionChatMessage {
+	if len(toolCalls) == 0 {
+		return nil
+	}
+
+	messages := make([]sessionChatMessage, 0, len(toolCalls))
+	for _, tc := range toolCalls {
+		name := tc.Name
+		argsJSON := ""
+		if tc.Function != nil {
+			if name == "" {
+				name = tc.Function.Name
+			}
+			argsJSON = tc.Function.Arguments
+		}
+
+		if strings.TrimSpace(name) == "" {
+			continue
+		}
+
+		if strings.TrimSpace(argsJSON) == "" && len(tc.Arguments) > 0 {
+			if encodedArgs, err := json.Marshal(tc.Arguments); err == nil {
+				argsJSON = string(encodedArgs)
+			}
+		}
+
+		argsPreview := strings.TrimSpace(argsJSON)
+		if argsPreview == "" {
+			argsPreview = "{}"
+		}
+
+		messages = append(messages, sessionChatMessage{
+			Role:    "assistant",
+			Content: formatToolCallSummary(name, utils.Truncate(argsPreview, sessionToolFeedbackMaxArgsLength)),
+		})
+	}
+
+	return messages
+}
+
+func formatToolCallSummary(name, argsPreview string) string {
+	return fmt.Sprintf("\U0001f527 `%s`\n```\n%s\n```", name, argsPreview)
+}
+
 func visibleAssistantToolMessages(toolCalls []providers.ToolCall) []sessionChatMessage {
 	if len(toolCalls) == 0 {
 		return nil
@@ -273,11 +273,14 @@ func TestHandleGetSession_ReconstructsVisibleMessageToolOutput(t *testing.T) {
 	if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
 		t.Fatalf("Unmarshal() error = %v", err)
 	}
-	if len(resp.Messages) != 2 {
-		t.Fatalf("len(resp.Messages) = %d, want 2", len(resp.Messages))
+	if len(resp.Messages) != 3 {
+		t.Fatalf("len(resp.Messages) = %d, want 3", len(resp.Messages))
 	}
-	if resp.Messages[1].Role != "assistant" || resp.Messages[1].Content != "visible tool output" {
-		t.Fatalf("assistant message = %#v, want visible tool output", resp.Messages[1])
+	if !strings.Contains(resp.Messages[1].Content, "`message`") {
+		t.Fatalf("tool summary message = %#v, want message tool summary", resp.Messages[1])
+	}
+	if resp.Messages[2].Role != "assistant" || resp.Messages[2].Content != "visible tool output" {
+		t.Fatalf("assistant message = %#v, want visible tool output", resp.Messages[2])
 	}
 }

@@ -336,14 +339,17 @@ func TestHandleGetSession_PreservesFinalAssistantReplyAfterMessageToolOutput(t *
 	if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
 		t.Fatalf("Unmarshal() error = %v", err)
 	}
-	if len(resp.Messages) != 3 {
-		t.Fatalf("len(resp.Messages) = %d, want 3", len(resp.Messages))
+	if len(resp.Messages) != 4 {
+		t.Fatalf("len(resp.Messages) = %d, want 4", len(resp.Messages))
 	}
-	if resp.Messages[1].Role != "assistant" || resp.Messages[1].Content != "visible tool output" {
-		t.Fatalf("interim assistant message = %#v, want visible tool output", resp.Messages[1])
+	if !strings.Contains(resp.Messages[1].Content, "`message`") {
+		t.Fatalf("tool summary message = %#v, want message tool summary", resp.Messages[1])
 	}
-	if resp.Messages[2].Role != "assistant" || resp.Messages[2].Content != "final assistant reply" {
-		t.Fatalf("final assistant message = %#v, want final assistant reply", resp.Messages[2])
+	if resp.Messages[2].Role != "assistant" || resp.Messages[2].Content != "visible tool output" {
+		t.Fatalf("interim assistant message = %#v, want visible tool output", resp.Messages[2])
+	}
+	if resp.Messages[3].Role != "assistant" || resp.Messages[3].Content != "final assistant reply" {
+		t.Fatalf("final assistant message = %#v, want final assistant reply", resp.Messages[3])
 	}
 }

@@ -400,8 +406,76 @@ func TestHandleListSessions_MessageCountUsesVisibleTranscript(t *testing.T) {
 	if len(items) != 1 {
 		t.Fatalf("len(items) = %d, want 1", len(items))
 	}
-	if items[0].MessageCount != 2 {
-		t.Fatalf("items[0].MessageCount = %d, want 2", items[0].MessageCount)
+	if items[0].MessageCount != 3 {
+		t.Fatalf("items[0].MessageCount = %d, want 3", items[0].MessageCount)
+	}
+}
+
+func TestHandleGetSession_PreservesToolSummaryAndAssistantContent(t *testing.T) {
+	configPath, cleanup := setupOAuthTestEnv(t)
+	defer cleanup()
+
+	dir := sessionsTestDir(t, configPath)
+	store, err := memory.NewJSONLStore(dir)
+	if err != nil {
+		t.Fatalf("NewJSONLStore() error = %v", err)
+	}
+
+	sessionKey := picoSessionPrefix + "detail-tool-summary-and-content"
+	for _, msg := range []providers.Message{
+		{Role: "user", Content: "check file"},
+		{
+			Role:    "assistant",
+			Content: "model final reply",
+			ToolCalls: []providers.ToolCall{
+				{
+					ID:   "call_1",
+					Type: "function",
+					Function: &providers.FunctionCall{
+						Name:      "read_file",
+						Arguments: `{"path":"README.md","start_line":1,"end_line":10}`,
+					},
+				},
+			},
+		},
+	} {
+		if err := store.AddFullMessage(nil, sessionKey, msg); err != nil {
+			t.Fatalf("AddFullMessage() error = %v", err)
+		}
+	}
+
+	h := NewHandler(configPath)
+	mux := http.NewServeMux()
+	h.RegisterRoutes(mux)
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodGet, "/api/sessions/detail-tool-summary-and-content", nil)
+	mux.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String())
+	}
+
+	var resp struct {
+		Messages []struct {
+			Role    string `json:"role"`
+			Content string `json:"content"`
+		} `json:"messages"`
+	}
+	if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("Unmarshal() error = %v", err)
+	}
+	if len(resp.Messages) != 3 {
+		t.Fatalf("len(resp.Messages) = %d, want 3", len(resp.Messages))
+	}
+	if resp.Messages[0].Role != "user" || resp.Messages[0].Content != "check file" {
+		t.Fatalf("first message = %#v, want user/check file", resp.Messages[0])
+	}
+	if !strings.Contains(resp.Messages[1].Content, "`read_file`") {
+		t.Fatalf("tool summary message = %#v, want read_file summary", resp.Messages[1])
+	}
+	if resp.Messages[2].Role != "assistant" || resp.Messages[2].Content != "model final reply" {
+		t.Fatalf("assistant message = %#v, want model final reply", resp.Messages[2])
 	}
 }