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(review): align tool feedback reconstruction with runtime behavior

Browse Source
This commit is contained in:
lc6464
2026-04-09 23:52:02 +08:00
parent 9982ee29a8
commit bd13092831
5 changed files with 142 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/agent/loop.go
+13 -3
View File
@@ -2255,11 +2255,21 @@ turnLoop:
if al.bus != nil && ts.channel == "pico" && len(response.ToolCalls) > 0 {
if strings.TrimSpace(response.Content) != "" {
al.bus.PublishOutbound(turnCtx, bus.OutboundMessage{
outCtx, outCancel := context.WithTimeout(turnCtx, 3*time.Second)
err := al.bus.PublishOutbound(outCtx, bus.OutboundMessage{
Channel: ts.channel,
ChatID: ts.chatID,
Content: response.Content,
})
outCancel()
if err != nil {
logger.WarnCF("agent", "Failed to publish pico interim tool-call content", map[string]any{
"error": err.Error(),
"channel": ts.channel,
"chat_id": ts.chatID,
"iteration": iteration,
})
}
}
}
@@ -2400,7 +2410,7 @@ turnLoop:
string(argsJSON),
al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
)
feedbackMsg := fmt.Sprintf("\U0001f527 `%s`\n```\n%s\n```", toolName, feedbackPreview)
feedbackMsg := utils.FormatToolFeedbackMessage(toolName, feedbackPreview)
fbCtx, fbCancel := context.WithTimeout(turnCtx, 3*time.Second)
_ = al.bus.PublishOutbound(fbCtx, bus.OutboundMessage{
Channel: ts.channel,
@@ -2682,7 +2692,7 @@ turnLoop:
string(argsJSON),
al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
)
feedbackMsg := fmt.Sprintf("\U0001f527 `%s`\n```\n%s\n```", tc.Name, feedbackPreview)
feedbackMsg := utils.FormatToolFeedbackMessage(tc.Name, feedbackPreview)
fbCtx, fbCancel := context.WithTimeout(turnCtx, 3*time.Second)
_ = al.bus.PublishOutbound(fbCtx, bus.OutboundMessage{
Channel: ts.channel,
pkg/utils/tool_feedback.go
+9
View File
@@ -0,0 +1,9 @@
package utils
import "fmt"
// FormatToolFeedbackMessage renders the tool name and arguments preview in the
// same markdown shape used by live tool feedback and session reconstruction.
func FormatToolFeedbackMessage(toolName, argsPreview string) string {
return fmt.Sprintf("\U0001f527 `%s`\n```\n%s\n```", toolName, argsPreview)
}
pkg/utils/tool_feedback_test.go
+11
View File
@@ -0,0 +1,11 @@
package utils
import "testing"
func TestFormatToolFeedbackMessage(t *testing.T) {
got := FormatToolFeedbackMessage("read_file", "{\"path\":\"README.md\"}")
want := "\U0001f527 `read_file`\n```\n{\"path\":\"README.md\"}\n```"
if got != want {
t.Fatalf("FormatToolFeedbackMessage() = %q, want %q", got, want)
}
}
web/backend/api/session.go
+32 -19
View File
@@ -4,7 +4,6 @@ import (
"bufio"
"encoding/json"
"errors"
"fmt"
"net/http"
"os"
"path/filepath"
@@ -74,12 +73,15 @@ 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."
)
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) {
@@ -206,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" {
@@ -223,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,
@@ -264,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 {
@@ -279,7 +281,7 @@ func visibleSessionMessages(messages []providers.Message) []sessionChatMessage {
}
case "assistant":
toolSummaryMessages := visibleAssistantToolSummaryMessages(msg.ToolCalls)
toolSummaryMessages := visibleAssistantToolSummaryMessages(msg.ToolCalls, toolFeedbackMaxArgsLength)
if len(toolSummaryMessages) > 0 {
transcript = append(transcript, toolSummaryMessages...)
}
@@ -311,10 +313,13 @@ func assistantMessageInternalOnly(msg providers.Message) bool {
return strings.TrimSpace(msg.Content) == handledToolResponseSummaryText
}
func visibleAssistantToolSummaryMessages(toolCalls []providers.ToolCall) []sessionChatMessage {
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 {
@@ -344,17 +349,13 @@ func visibleAssistantToolSummaryMessages(toolCalls []providers.ToolCall) []sessi
messages = append(messages, sessionChatMessage{
Role: "assistant",
Content: formatToolCallSummary(name, utils.Truncate(argsPreview, sessionToolFeedbackMaxArgsLength)),
Content: utils.FormatToolFeedbackMessage(name, utils.Truncate(argsPreview, toolFeedbackMaxArgsLength)),
})
}
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
@@ -400,7 +401,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")
@@ -416,14 +429,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
@@ -507,7 +520,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)
@@ -555,7 +568,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
@@ -582,7 +595,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
+77
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 {
@@ -479,6 +480,82 @@ func TestHandleGetSession_PreservesToolSummaryAndAssistantContent(t *testing.T)
}
}
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)
}
}
func TestHandleGetSession_IncludesMediaOnlyMessages(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()