lzh/picoclaw
1
0
Fork 0
mirror of https://github.com/sipeed/picoclaw.git synced 2026-06-12 18:08:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(chat): keep tool-call summary and assistant output in sync (#2449)

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

* fix(pico): stream assistant text between tool calls

* fix(pico): avoid duplicate final websocket message

* fix(review): align tool feedback reconstruction with runtime behavior

* style(lint): satisfy gci and golines for review fixes

* fix(agent): gate pico interim publish for internal turns
This commit is contained in:
Guoguo
2026-04-10 15:08:30 +08:00
committed by GitHub
parent 187189ad4a bd88385923
commit 748ac58dd1
6 changed files with 459 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/backend/api/session.go
+79 -10
View File
@@ -14,6 +14,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.
@@ -76,6 +77,11 @@ const (
handledToolResponseSummaryText = "Requested output delivered via tool attachment."
)
func defaultToolFeedbackMaxArgsLength() int {
defaults := config.AgentDefaults{}
return defaults.GetToolFeedbackMaxArgsLength()
}
// extractPicoSessionID extracts the session UUID from a full session key.
// Returns the UUID and true if the key matches the Pico session pattern.
func extractPicoSessionID(key string) (string, bool) {
@@ -202,7 +208,7 @@ func (h *Handler) readJSONLSession(dir, sessionID string) (sessionFile, error) {
}, nil
}
func buildSessionListItem(sessionID string, sess sessionFile) sessionListItem {
func buildSessionListItem(sessionID string, sess sessionFile, toolFeedbackMaxArgsLength int) sessionListItem {
preview := ""
for _, msg := range sess.Messages {
if msg.Role == "user" {
@@ -219,7 +225,7 @@ func buildSessionListItem(sessionID string, sess sessionFile) sessionListItem {
}
title := preview
validMessageCount := len(visibleSessionMessages(sess.Messages))
validMessageCount := len(visibleSessionMessages(sess.Messages, toolFeedbackMaxArgsLength))
return sessionListItem{
ID: sessionID,
@@ -260,7 +266,7 @@ func sessionMessagePreview(msg providers.Message) string {
return ""
}
func visibleSessionMessages(messages []providers.Message) []sessionChatMessage {
func visibleSessionMessages(messages []providers.Message, toolFeedbackMaxArgsLength int) []sessionChatMessage {
transcript := make([]sessionChatMessage, 0, len(messages))
for _, msg := range messages {
@@ -275,6 +281,11 @@ func visibleSessionMessages(messages []providers.Message) []sessionChatMessage {
}
case "assistant":
toolSummaryMessages := visibleAssistantToolSummaryMessages(msg.ToolCalls, toolFeedbackMaxArgsLength)
if len(toolSummaryMessages) > 0 {
transcript = append(transcript, toolSummaryMessages...)
}
visibleToolMessages := visibleAssistantToolMessages(msg.ToolCalls)
if len(visibleToolMessages) > 0 {
transcript = append(transcript, visibleToolMessages...)
@@ -283,7 +294,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 +313,52 @@ func assistantMessageInternalOnly(msg providers.Message) bool {
return strings.TrimSpace(msg.Content) == handledToolResponseSummaryText
}
func visibleAssistantToolSummaryMessages(
toolCalls []providers.ToolCall,
toolFeedbackMaxArgsLength int,
) []sessionChatMessage {
if len(toolCalls) == 0 {
return nil
}
if toolFeedbackMaxArgsLength <= 0 {
toolFeedbackMaxArgsLength = defaultToolFeedbackMaxArgsLength()
}
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: utils.FormatToolFeedbackMessage(name, utils.Truncate(argsPreview, toolFeedbackMaxArgsLength)),
})
}
return messages
}
func visibleAssistantToolMessages(toolCalls []providers.ToolCall) []sessionChatMessage {
if len(toolCalls) == 0 {
return nil
@@ -347,7 +404,19 @@ func (h *Handler) sessionsDir() (string, error) {
return "", err
}
workspace := cfg.Agents.Defaults.Workspace
return resolveSessionsDir(cfg.Agents.Defaults.Workspace), nil
}
func (h *Handler) sessionRuntimeSettings() (string, int, error) {
cfg, err := config.LoadConfig(h.configPath)
if err != nil {
return "", 0, err
}
return resolveSessionsDir(cfg.Agents.Defaults.Workspace), cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(), nil
}
func resolveSessionsDir(workspace string) string {
if workspace == "" {
home, _ := os.UserHomeDir()
workspace = filepath.Join(home, ".picoclaw", "workspace")
@@ -363,14 +432,14 @@ func (h *Handler) sessionsDir() (string, error) {
}
}
return filepath.Join(workspace, "sessions"), nil
return filepath.Join(workspace, "sessions")
}
// handleListSessions returns a list of Pico session summaries.
//
// GET /api/sessions
func (h *Handler) handleListSessions(w http.ResponseWriter, r *http.Request) {
dir, err := h.sessionsDir()
dir, toolFeedbackMaxArgsLength, err := h.sessionRuntimeSettings()
if err != nil {
http.Error(w, "failed to resolve sessions directory", http.StatusInternalServerError)
return
@@ -454,7 +523,7 @@ func (h *Handler) handleListSessions(w http.ResponseWriter, r *http.Request) {
}
seen[sessionID] = struct{}{}
items = append(items, buildSessionListItem(sessionID, sess))
items = append(items, buildSessionListItem(sessionID, sess, toolFeedbackMaxArgsLength))
}
// Sort by updated descending (most recent first)
@@ -502,7 +571,7 @@ func (h *Handler) handleGetSession(w http.ResponseWriter, r *http.Request) {
return
}
dir, err := h.sessionsDir()
dir, toolFeedbackMaxArgsLength, err := h.sessionRuntimeSettings()
if err != nil {
http.Error(w, "failed to resolve sessions directory", http.StatusInternalServerError)
return
@@ -529,7 +598,7 @@ func (h *Handler) handleGetSession(w http.ResponseWriter, r *http.Request) {
}
}
messages := visibleSessionMessages(sess.Messages)
messages := visibleSessionMessages(sess.Messages, toolFeedbackMaxArgsLength)
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]any{
web/backend/api/session_test.go
+163 -12
View File
@@ -13,6 +13,7 @@ import (
"github.com/sipeed/picoclaw/pkg/memory"
"github.com/sipeed/picoclaw/pkg/providers"
"github.com/sipeed/picoclaw/pkg/session"
"github.com/sipeed/picoclaw/pkg/utils"
)
func sessionsTestDir(t *testing.T, configPath string) string {
@@ -273,11 +274,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 +340,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 +407,152 @@ 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])
}
}
func TestHandleGetSession_UsesConfiguredToolFeedbackMaxArgsLength(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()
cfg, err := config.LoadConfig(configPath)
if err != nil {
t.Fatalf("LoadConfig() error = %v", err)
}
cfg.Agents.Defaults.ToolFeedback.MaxArgsLength = 20
err = config.SaveConfig(configPath, cfg)
if err != nil {
t.Fatalf("SaveConfig() error = %v", err)
}
dir := sessionsTestDir(t, configPath)
store, err := memory.NewJSONLStore(dir)
if err != nil {
t.Fatalf("NewJSONLStore() error = %v", err)
}
argsJSON := `{"path":"README.md","start_line":1,"end_line":10,"extra":"abcdefghijklmnopqrstuvwxyz"}`
sessionKey := picoSessionPrefix + "detail-tool-summary-max-args"
err = store.AddFullMessage(nil, sessionKey, providers.Message{Role: "user", Content: "check file"})
if err != nil {
t.Fatalf("AddFullMessage(user) error = %v", err)
}
err = store.AddFullMessage(nil, sessionKey, providers.Message{
Role: "assistant",
ToolCalls: []providers.ToolCall{{
ID: "call_1",
Type: "function",
Function: &providers.FunctionCall{
Name: "read_file",
Arguments: argsJSON,
},
}},
})
if err != nil {
t.Fatalf("AddFullMessage(assistant) 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-max-args", 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"`
}
err = json.Unmarshal(rec.Body.Bytes(), &resp)
if err != nil {
t.Fatalf("Unmarshal() error = %v", err)
}
if len(resp.Messages) < 2 {
t.Fatalf("len(resp.Messages) = %d, want at least 2", len(resp.Messages))
}
wantPreview := utils.Truncate(argsJSON, 20)
if !strings.Contains(resp.Messages[1].Content, wantPreview) {
t.Fatalf("tool summary = %q, want preview %q", resp.Messages[1].Content, wantPreview)
}
if strings.Contains(resp.Messages[1].Content, argsJSON) {
t.Fatalf("tool summary = %q, expected configured truncation", resp.Messages[1].Content)
}
}