opencode/picoclaw
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(tool-feedback): dedupe duplicate content and keep full explanations

Browse source
This commit is contained in:
lc6464 2026-04-26 00:40:55 +08:00
parent 5cd10b594a
commit 6d04d15ce0
No known key found for this signature in database
GPG key ID: 53C61B42FEC71D6D
14 changed files with 288 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
pkg/agent/agent_outbound.go
View file

@ -160,7 +160,14 @@ func (al *AgentLoop) publishPicoToolCallInterim(
return
}
if strings.TrimSpace(content) != "" {
visibleToolCalls := utils.BuildVisibleToolCalls(
toolCalls,
al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
)
duplicateToolCallContent := len(visibleToolCalls) > 0 &&
utils.ToolCallExplanationDuplicatesContent(content, toolCalls)
if strings.TrimSpace(content) != "" && !duplicateToolCallContent {
pubCtx, pubCancel := context.WithTimeout(ctx, 3*time.Second)
err := al.bus.PublishOutbound(pubCtx, outboundMessageForTurn(ts, content))
pubCancel()
@ -175,10 +182,6 @@ func (al *AgentLoop) publishPicoToolCallInterim(
}
}
visibleToolCalls := utils.BuildVisibleToolCalls(
toolCalls,
al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
)
if len(visibleToolCalls) == 0 {
return
}

90
pkg/agent/agent_test.go
View file

@ -1892,7 +1892,7 @@ func TestToolFeedbackExplanationFromResponse_UsesCurrentContentFirst(t *testing.
{Role: "tool", Content: "tool output", ToolCallID: "call_1"},
}
got := toolFeedbackExplanationFromResponse(response, messages, 300)
got := toolFeedbackExplanationFromResponse(response, messages)
if got != "Read README.md first" {
t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want current content", got)
}
@ -1936,7 +1936,7 @@ func TestToolFeedbackExplanationFromResponse_UsesExplicitToolCallExtraContent(t
{Role: "tool", Content: "tool output", ToolCallID: "call_1"},
}
got := toolFeedbackExplanationFromResponse(response, messages, 300)
got := toolFeedbackExplanationFromResponse(response, messages)
if got != "Read README.md first to confirm the current project structure." {
t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want explicit tool feedback explanation", got)
}
@ -1963,8 +1963,8 @@ func TestToolFeedbackExplanationForToolCall_PrefersToolSpecificExtraContent(t *t
},
}
got1 := toolFeedbackExplanationForToolCall(response, response.ToolCalls[0], nil, 300)
got2 := toolFeedbackExplanationForToolCall(response, response.ToolCalls[1], nil, 300)
got1 := toolFeedbackExplanationForToolCall(response, response.ToolCalls[0], nil)
got2 := toolFeedbackExplanationForToolCall(response, response.ToolCalls[1], nil)
if got1 != "Read README.md first." {
t.Fatalf("toolFeedbackExplanationForToolCall() first = %q, want tool-specific explanation", got1)
}
@ -1993,7 +1993,7 @@ func TestToolFeedbackExplanationForToolCall_DoesNotReuseAnotherToolCallExplanati
{Role: "user", Content: "inspect the config and update the example"},
}
got := toolFeedbackExplanationForToolCall(response, response.ToolCalls[0], messages, 300)
got := toolFeedbackExplanationForToolCall(response, response.ToolCalls[0], messages)
want := utils.ToolFeedbackContinuationHint + ": inspect the config and update the example"
if got != want {
t.Fatalf("toolFeedbackExplanationForToolCall() = %q, want %q", got, want)
@ -2012,13 +2012,31 @@ func TestToolFeedbackExplanationFromResponse_DoesNotUseReasoningContent(t *testi
{Role: "tool", Content: "tool output", ToolCallID: "call_1"},
}
got := toolFeedbackExplanationFromResponse(response, messages, 300)
got := toolFeedbackExplanationFromResponse(response, messages)
want := utils.ToolFeedbackContinuationHint + ": Inspect README.md and update the config example."
if got != want {
t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want latest user content fallback", got)
}
}
func TestToolFeedbackExplanationForToolCall_DoesNotTruncateLongExplanation(t *testing.T) {
explanation := "Read README.md first to confirm the current project structure before editing the config example."
response := &providers.LLMResponse{
ToolCalls: []providers.ToolCall{{
ID: "call_1",
Name: "read_file",
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: explanation,
},
}},
}
got := toolFeedbackExplanationForToolCall(response, response.ToolCalls[0], nil)
if got != explanation {
t.Fatalf("toolFeedbackExplanationForToolCall() = %q, want full explanation", got)
}
}
func TestToolFeedbackArgsPreview_UsesJSONAndTruncates(t *testing.T) {
got := toolFeedbackArgsPreview(map[string]any{
"path": "README.md",
@ -2064,6 +2082,43 @@ func (m *picoInterleavedContentProvider) GetDefaultModel() string {
return "pico-interleaved-content-model"
}
type picoDistinctToolCallContentProvider struct {
calls int
}
func (m *picoDistinctToolCallContentProvider) 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{
Content: "intermediate model text",
ToolCalls: []providers.ToolCall{{
ID: "call_tool_limit_test",
Type: "function",
Name: "tool_limit_test_tool",
Arguments: map[string]any{"value": "x"},
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: "Read the file before replying.",
},
}},
}, nil
}
return &providers.LLMResponse{
Content: "final model text",
ToolCalls: []providers.ToolCall{},
}, nil
}
func (m *picoDistinctToolCallContentProvider) GetDefaultModel() string {
return "pico-distinct-tool-call-content-model"
}
type toolLimitOnlyProvider struct{}
func (m *toolLimitOnlyProvider) Chat(
@ -4398,7 +4453,7 @@ func TestRun_PicoPublishesAssistantContentDuringToolCallsWithoutFinalDuplicate(t
}
msgBus := bus.NewMessageBus()
provider := &picoInterleavedContentProvider{}
provider := &picoDistinctToolCallContentProvider{}
al := NewAgentLoop(cfg, msgBus, provider)
agent := al.GetRegistry().GetDefaultAgent()
@ -4562,7 +4617,7 @@ func TestRun_PicoToolFeedbackSuppressesDuplicateInterimAssistantContent(t *testi
outputs := make([]bus.OutboundMessage, 0, 3)
deadline := time.After(2 * time.Second)
for len(outputs) < 3 {
for len(outputs) < 2 {
select {
case outbound := <-msgBus.OutboundChan():
outputs = append(outputs, outbound)
@ -4571,20 +4626,17 @@ func TestRun_PicoToolFeedbackSuppressesDuplicateInterimAssistantContent(t *testi
}
}
if outputs[0].Content != "intermediate model text" {
t.Fatalf("first outbound content = %q, want %q", outputs[0].Content, "intermediate model text")
if outputs[0].Context.Raw[metadataKeyMessageKind] != messageKindToolCalls {
t.Fatalf("first outbound = %+v, want tool_calls message", outputs[0])
}
if outputs[1].Context.Raw[metadataKeyMessageKind] != messageKindToolCalls {
t.Fatalf("second outbound = %+v, want tool_calls message", outputs[1])
if outputs[0].Content != "" {
t.Fatalf("first outbound content = %q, want empty tool_calls content", outputs[0].Content)
}
if outputs[1].Content != "" {
t.Fatalf("second outbound content = %q, want empty tool_calls content", outputs[1].Content)
if !strings.Contains(outputs[0].Context.Raw[metadataKeyToolCalls], "tool_limit_test_tool") {
t.Fatalf("first outbound tool_calls = %q, want tool name", outputs[0].Context.Raw[metadataKeyToolCalls])
}
if !strings.Contains(outputs[1].Context.Raw[metadataKeyToolCalls], "tool_limit_test_tool") {
t.Fatalf("second outbound tool_calls = %q, want tool name", outputs[1].Context.Raw[metadataKeyToolCalls])
}
if outputs[2].Content != "final model text" {
t.Fatalf("third outbound content = %q, want %q", outputs[2].Content, "final model text")
if outputs[1].Content != "final model text" {
t.Fatalf("second outbound content = %q, want %q", outputs[1].Content, "final model text")
}
runCancel()

10
pkg/agent/agent_utils.go
View file

@ -115,7 +115,6 @@ func latestUserContent(messages []providers.Message) string {
func toolFeedbackExplanationFromResponse(
response *providers.LLMResponse,
messages []providers.Message,
maxLen int,
) string {
if response == nil {
return ""
@ -127,7 +126,7 @@ func toolFeedbackExplanationFromResponse(
if explanation == "" {
explanation = toolFeedbackExplanationFromMessages(messages)
}
return utils.Truncate(explanation, maxLen)
return explanation
}
func toolFeedbackExplanationFromToolCalls(toolCalls []providers.ToolCall) string {
@ -146,22 +145,21 @@ 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)
return explanation
}
}
if response == nil {
return utils.Truncate(toolFeedbackExplanationFromMessages(messages), maxLen)
return toolFeedbackExplanationFromMessages(messages)
}
explanation := strings.TrimSpace(response.Content)
if explanation == "" {
explanation = toolFeedbackExplanationFromMessages(messages)
}
return utils.Truncate(explanation, maxLen)
return explanation
}
func toolFeedbackExplanationFromMessages(messages []providers.Message) string {

2
pkg/agent/pipeline_execute.go
View file

@ -86,7 +86,6 @@ toolLoop:
exec.response,
tc,
messages,
toolFeedbackMaxLen,
)
feedbackMsg := utils.FormatToolFeedbackMessage(
toolName,
@ -368,7 +367,6 @@ toolLoop:
exec.response,
tc,
messages,
toolFeedbackMaxLen,
)
feedbackMsg := utils.FormatToolFeedbackMessage(
toolName,

1
pkg/agent/pipeline_llm.go
View file

@ -478,7 +478,6 @@ func (p *Pipeline) CallLLM(
exec.response,
tc,
exec.messages,
al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
)
extraContent := tc.ExtraContent
if strings.TrimSpace(toolFeedbackExplanation) != "" {

2
pkg/config/config.go
View file

@ -287,7 +287,7 @@ func (d *AgentDefaults) GetMaxMediaSize() int {
return DefaultMaxMediaSize
}
// GetToolFeedbackMaxArgsLength returns the max visible text length for tool feedback messages.
// GetToolFeedbackMaxArgsLength returns the max visible text length for tool argument previews.
func (d *AgentDefaults) GetToolFeedbackMaxArgsLength() int {
if d.ToolFeedback.MaxArgsLength > 0 {
return d.ToolFeedback.MaxArgsLength

39
pkg/utils/tool_feedback_dedupe.go Normal file
View file

@ -0,0 +1,39 @@
package utils
import (
"strings"
"github.com/sipeed/picoclaw/pkg/providers"
)
func normalizeToolFeedbackComparisonText(text string) string {
text = strings.ReplaceAll(text, "\r\n", "\n")
text = strings.ReplaceAll(text, "\r", "\n")
text = strings.TrimSpace(text)
if text == "" {
return ""
}
return strings.Join(strings.Fields(text), " ")
}
func ToolCallExplanationDuplicatesContent(content string, toolCalls []providers.ToolCall) bool {
normalizedContent := normalizeToolFeedbackComparisonText(content)
if normalizedContent == "" || len(toolCalls) == 0 {
return false
}
for _, tc := range toolCalls {
if tc.ExtraContent == nil {
continue
}
explanation := normalizeToolFeedbackComparisonText(tc.ExtraContent.ToolFeedbackExplanation)
if explanation == "" {
continue
}
if explanation == normalizedContent {
return true
}
}
return false
}

55
pkg/utils/tool_feedback_dedupe_test.go Normal file
View file

@ -0,0 +1,55 @@
package utils
import (
"testing"
"github.com/sipeed/picoclaw/pkg/providers"
)
func TestToolCallExplanationDuplicatesContent(t *testing.T) {
t.Run("exact duplicate", func(t *testing.T) {
toolCalls := []providers.ToolCall{{
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: "Read the file before replying.",
},
}}
if !ToolCallExplanationDuplicatesContent("Read the file before replying.", toolCalls) {
t.Fatal("expected duplicated content to be detected")
}
})
t.Run("whitespace normalized duplicate", func(t *testing.T) {
toolCalls := []providers.ToolCall{{
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: "Read the file\nbefore replying.",
},
}}
if !ToolCallExplanationDuplicatesContent(" Read the file before replying. ", toolCalls) {
t.Fatal("expected whitespace-only differences to be ignored")
}
})
t.Run("distinct content", func(t *testing.T) {
toolCalls := []providers.ToolCall{{
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: "Read the file before replying.",
},
}}
if ToolCallExplanationDuplicatesContent(
"I will summarize the findings after reading the file.",
toolCalls,
) {
t.Fatal("expected distinct content to remain visible")
}
})
t.Run("missing explanation", func(t *testing.T) {
toolCalls := []providers.ToolCall{{}}
if ToolCallExplanationDuplicatesContent("Read the file before replying.", toolCalls) {
t.Fatal("expected empty tool explanations to skip dedupe")
}
})
}

3
pkg/utils/visible_tool_calls.go
View file

@ -39,9 +39,6 @@ func BuildVisibleToolCalls(
explanation := ""
if tc.ExtraContent != nil {
explanation = strings.TrimSpace(tc.ExtraContent.ToolFeedbackExplanation)
if maxArgsLen > 0 {
explanation = Truncate(explanation, maxArgsLen)
}
}
if name == "" && explanation == "" && argsPreview == "" {
continue

33
pkg/utils/visible_tool_calls_test.go Normal file
View file

@ -0,0 +1,33 @@
package utils
import (
"testing"
"github.com/sipeed/picoclaw/pkg/providers"
)
func TestBuildVisibleToolCalls_DoesNotTruncateExplanation(t *testing.T) {
explanation := "Read README.md first to confirm the current project structure before editing the config example."
toolCalls := []providers.ToolCall{{
ID: "call_1",
Type: "function",
Function: &providers.FunctionCall{
Name: "read_file",
Arguments: `{"path":"README.md","start_line":1,"end_line":10,"extra":"abcdefghijklmnopqrstuvwxyz"}`,
},
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: explanation,
},
}}
visible := BuildVisibleToolCalls(toolCalls, 20)
if len(visible) != 1 {
t.Fatalf("len(visible) = %d, want 1", len(visible))
}
if visible[0].ExtraContent == nil || visible[0].ExtraContent.ToolFeedbackExplanation != explanation {
t.Fatalf("visible explanation = %#v, want %q", visible[0].ExtraContent, explanation)
}
if visible[0].Function == nil || visible[0].Function.Arguments == "" {
t.Fatalf("visible function = %#v, want truncated args preview", visible[0].Function)
}
}

3
web/backend/api/session.go
View file

@ -549,6 +549,9 @@ func sessionTranscriptMessages(
}
content = ""
}
if hasToolCallsMsg && utils.ToolCallExplanationDuplicatesContent(content, msg.ToolCalls) {
content = ""
}
chatMsg := sessionChatMessage{
Role: "assistant",

83
web/backend/api/session_test.go
View file

@ -901,6 +901,67 @@ func TestHandleListSessions_MessageCountUsesVisibleTranscript(t *testing.T) {
}
}
func TestHandleListSessions_DeduplicatesAssistantToolCallContentFromVisibleTranscript(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 + "list-deduped-tool-content"
for _, msg := range []providers.Message{
{Role: "user", Content: "check file"},
{
Role: "assistant",
Content: "Read the file before replying.",
ToolCalls: []providers.ToolCall{
{
ID: "call_1",
Type: "function",
Function: &providers.FunctionCall{
Name: "read_file",
Arguments: `{"path":"README.md"}`,
},
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: "Read the file before replying.",
},
},
},
},
{Role: "tool", Content: "raw read_file result", ToolCallID: "call_1"},
} {
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", 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 items []sessionListItem
if err := json.Unmarshal(rec.Body.Bytes(), &items); err != nil {
t.Fatalf("Unmarshal() error = %v", err)
}
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)
}
}
func TestHandleGetSession_DoesNotDuplicateAssistantToolCallContent(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()
@ -956,16 +1017,13 @@ func TestHandleGetSession_DoesNotDuplicateAssistantToolCallContent(t *testing.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) != 2 {
t.Fatalf("len(resp.Messages) = %d, want 2", 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 resp.Messages[1].Content != "Read the file before replying." {
t.Fatalf("assistant content = %#v, want preserved assistant content", resp.Messages[1])
}
toolCall := assertVisibleToolCallMessage(t, resp.Messages[2], "read_file")
toolCall := assertVisibleToolCallMessage(t, resp.Messages[1], "read_file")
if toolCall.ExtraContent == nil ||
toolCall.ExtraContent.ToolFeedbackExplanation != "Read the file before replying." {
t.Fatalf("tool call = %#v, want explanation", toolCall)
@ -1097,8 +1155,8 @@ func TestHandleGetSession_PreservesMediaWhenAssistantToolCallContentDuplicatesSu
if resp.Messages[1].Role != "assistant" {
t.Fatalf("assistant message role = %q, want assistant", resp.Messages[1].Role)
}
if resp.Messages[1].Content != "Reviewing the generated screenshot." {
t.Fatalf("assistant content = %q, want preserved duplicated content with media", resp.Messages[1].Content)
if resp.Messages[1].Content != "" {
t.Fatalf("assistant content = %q, want duplicate content suppressed", resp.Messages[1].Content)
}
if len(resp.Messages[1].Media) != 1 || resp.Messages[1].Media[0] != "data:image/png;base64,abc123" {
t.Fatalf("assistant media = %#v, want preserved media", resp.Messages[1].Media)
@ -1176,8 +1234,8 @@ func TestHandleGetSession_PreservesAttachmentsWhenAssistantToolCallContentDuplic
if resp.Messages[1].Role != "assistant" {
t.Fatalf("assistant message role = %q, want assistant", resp.Messages[1].Role)
}
if resp.Messages[1].Content != "Reviewing the generated report." {
t.Fatalf("assistant content = %q, want preserved duplicated content", resp.Messages[1].Content)
if resp.Messages[1].Content != "" {
t.Fatalf("assistant content = %q, want duplicate content suppressed", resp.Messages[1].Content)
}
if len(resp.Messages[1].Attachments) != 1 {
t.Fatalf("len(assistant.Attachments) = %d, want 1", len(resp.Messages[1].Attachments))
@ -1256,13 +1314,12 @@ func TestHandleGetSession_UsesConfiguredToolFeedbackMaxArgsLength(t *testing.T)
t.Fatalf("len(resp.Messages) = %d, want at least 2", len(resp.Messages))
}
wantPreview := utils.Truncate(explanation, 20)
wantArgsPreview := visibleAssistantToolArgsPreview(providers.ToolCall{
Function: &providers.FunctionCall{Arguments: argsJSON},
}, 20)
toolCall := assertVisibleToolCallMessage(t, resp.Messages[1], "read_file")
if toolCall.ExtraContent == nil || toolCall.ExtraContent.ToolFeedbackExplanation != wantPreview {
t.Fatalf("tool call = %#v, want preview %q", toolCall, wantPreview)
if toolCall.ExtraContent == nil || toolCall.ExtraContent.ToolFeedbackExplanation != explanation {
t.Fatalf("tool call = %#v, want full explanation %q", toolCall, explanation)
}
if toolCall.Function == nil || toolCall.Function.Arguments != wantArgsPreview {
t.Fatalf("tool call = %#v, want args preview %q", toolCall, wantArgsPreview)

4
web/frontend/src/i18n/locales/en.json
View file

@ -612,8 +612,8 @@
"tool_feedback_enabled_hint": "Send a short execution note into the current chat before each tool runs.",
"tool_feedback_separate_messages": "Separate Feedback Messages",
"tool_feedback_separate_messages_hint": "Keep each tool feedback update as its own chat message instead of reusing a single placeholder/progress message.",
"tool_feedback_max_args_length": "Tool Feedback Length",
"tool_feedback_max_args_length_hint": "Maximum number of characters shown in each tool feedback message. Set to 0 to use the default.",
"tool_feedback_max_args_length": "Tool Args Preview Length",
"tool_feedback_max_args_length_hint": "Maximum number of characters shown in each tool argument preview. Set to 0 to use the default.",
"exec_enabled": "Allow Commands",
"exec_enabled_hint": "Enable or disable command execution for the app. When disabled, no command requests will run.",
"allow_remote": "Allow Remote Commands",

4
web/frontend/src/i18n/locales/zh.json
View file

@ -612,8 +612,8 @@
"tool_feedback_enabled_hint": "在每次执行工具前，先向当前会话发送一条简短的执行说明",
"tool_feedback_separate_messages": "分开发送反馈消息",
"tool_feedback_separate_messages_hint": "让每次工具反馈都保留为独立消息，而不是反复复用同一条占位/进度消息",
"tool_feedback_max_args_length": "工具反馈长度",
"tool_feedback_max_args_length_hint": "每条工具反馈消息中展示的字符上限。设为 0 时使用默认值",
"tool_feedback_max_args_length": "工具参数预览长度",
"tool_feedback_max_args_length_hint": "每条工具参数预览中展示的字符上限。设为 0 时使用默认值",
"exec_enabled": "允许命令执行",
"exec_enabled_hint": "控制应用是否允许执行命令。关闭后，所有命令请求都不会执行",
"allow_remote": "允许远程命令执行",