mirror of
https://github.com/sipeed/picoclaw.git
synced 2026-06-12 18:08:54 +00:00
Merge pull request #2660 from afjcjsbx/fix/tool-feedback-json-format
fix(tool-feedback): format tool args as JSON code blocks
This commit is contained in:
+24
-1
@@ -1965,6 +1965,17 @@ func TestToolFeedbackExplanationFromResponse_DoesNotUseReasoningContent(t *testi
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolFeedbackArgsPreview_UsesJSONAndTruncates(t *testing.T) {
|
||||
got := toolFeedbackArgsPreview(map[string]any{
|
||||
"path": "README.md",
|
||||
"limit": 42,
|
||||
}, 128)
|
||||
want := "{\n \"limit\": 42,\n \"path\": \"README.md\"\n}"
|
||||
if got != want {
|
||||
t.Fatalf("toolFeedbackArgsPreview() = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
type picoInterleavedContentProvider struct {
|
||||
calls int
|
||||
}
|
||||
@@ -3940,6 +3951,12 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) {
|
||||
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, "\"path\":") {
|
||||
t.Fatalf("tool feedback content = %q, want serialized tool arguments", outbound.Content)
|
||||
}
|
||||
if !strings.Contains(outbound.Content, heartbeatFile) {
|
||||
t.Fatalf("tool feedback content = %q, want tool argument value", outbound.Content)
|
||||
}
|
||||
if strings.Contains(outbound.Content, "Previous turn explanation") {
|
||||
t.Fatalf("tool feedback content = %q, want no previous assistant fallback", outbound.Content)
|
||||
}
|
||||
@@ -4012,6 +4029,12 @@ func TestProcessMessage_DoesNotLeakReasoningContentInToolFeedback(t *testing.T)
|
||||
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, "\"path\":") {
|
||||
t.Fatalf("tool feedback content = %q, want serialized tool arguments", outbound.Content)
|
||||
}
|
||||
if !strings.Contains(outbound.Content, heartbeatFile) {
|
||||
t.Fatalf("tool feedback content = %q, want tool argument value", outbound.Content)
|
||||
}
|
||||
if strings.Contains(outbound.Content, "Read README.md first") {
|
||||
t.Fatalf("tool feedback content = %q, should not leak hidden reasoning", outbound.Content)
|
||||
}
|
||||
@@ -4310,7 +4333,7 @@ func TestRun_PicoToolFeedbackSuppressesDuplicateInterimAssistantContent(t *testi
|
||||
}
|
||||
}
|
||||
|
||||
if outputs[0] != "🔧 `tool_limit_test_tool`\nintermediate model text" {
|
||||
if outputs[0] != "🔧 `tool_limit_test_tool`\nintermediate model text\n```json\n{\n \"value\": \"x\"\n}\n```" {
|
||||
t.Fatalf("first outbound content = %q, want tool feedback summary", outputs[0])
|
||||
}
|
||||
if outputs[1] != "final model text" {
|
||||
|
||||
@@ -4,6 +4,7 @@ package agent
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
@@ -170,6 +171,18 @@ func toolFeedbackExplanationFromMessages(messages []providers.Message) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
func toolFeedbackArgsPreview(args map[string]any, maxLen int) string {
|
||||
if args == nil {
|
||||
args = map[string]any{}
|
||||
}
|
||||
|
||||
argsJSON, err := json.MarshalIndent(args, "", " ")
|
||||
if err != nil {
|
||||
return utils.Truncate(fmt.Sprintf("%v", args), maxLen)
|
||||
}
|
||||
return utils.Truncate(string(argsJSON), maxLen)
|
||||
}
|
||||
|
||||
func shouldPublishToolFeedback(cfg *config.Config, ts *turnState) bool {
|
||||
if ts == nil || ts.channel == "" || ts.opts.SuppressToolFeedback {
|
||||
return false
|
||||
|
||||
@@ -81,13 +81,18 @@ toolLoop:
|
||||
)
|
||||
|
||||
if shouldPublishToolFeedback(al.cfg, ts) {
|
||||
toolFeedbackMaxLen := al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength()
|
||||
toolFeedbackExplanation := toolFeedbackExplanationForToolCall(
|
||||
exec.response,
|
||||
tc,
|
||||
messages,
|
||||
al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
|
||||
toolFeedbackMaxLen,
|
||||
)
|
||||
feedbackMsg := utils.FormatToolFeedbackMessage(
|
||||
toolName,
|
||||
toolFeedbackExplanation,
|
||||
toolFeedbackArgsPreview(toolArgs, toolFeedbackMaxLen),
|
||||
)
|
||||
feedbackMsg := utils.FormatToolFeedbackMessage(toolName, toolFeedbackExplanation)
|
||||
fbCtx, fbCancel := context.WithTimeout(turnCtx, 3*time.Second)
|
||||
_ = al.bus.PublishOutbound(fbCtx, outboundMessageForTurnWithKind(ts, feedbackMsg, messageKindToolFeedback))
|
||||
fbCancel()
|
||||
@@ -358,13 +363,18 @@ toolLoop:
|
||||
)
|
||||
|
||||
if shouldPublishToolFeedback(al.cfg, ts) {
|
||||
toolFeedbackMaxLen := al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength()
|
||||
toolFeedbackExplanation := toolFeedbackExplanationForToolCall(
|
||||
exec.response,
|
||||
tc,
|
||||
messages,
|
||||
al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
|
||||
toolFeedbackMaxLen,
|
||||
)
|
||||
feedbackMsg := utils.FormatToolFeedbackMessage(
|
||||
toolName,
|
||||
toolFeedbackExplanation,
|
||||
toolFeedbackArgsPreview(toolArgs, toolFeedbackMaxLen),
|
||||
)
|
||||
feedbackMsg := utils.FormatToolFeedbackMessage(toolName, toolFeedbackExplanation)
|
||||
fbCtx, fbCancel := context.WithTimeout(turnCtx, 3*time.Second)
|
||||
_ = al.bus.PublishOutbound(fbCtx, outboundMessageForTurnWithKind(ts, feedbackMsg, messageKindToolFeedback))
|
||||
fbCancel()
|
||||
|
||||
@@ -65,6 +65,11 @@ func Test_markdownToTelegramHTML(t *testing.T) {
|
||||
input: "a & b < c > d",
|
||||
expected: "a & b < c > d",
|
||||
},
|
||||
{
|
||||
name: "code block with language",
|
||||
input: "```json\n{\n \"path\": \"README.md\"\n}\n```",
|
||||
expected: "<pre><code>{\n \"path\": \"README.md\"\n}\n</code></pre>",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
|
||||
@@ -7,21 +7,31 @@ import (
|
||||
|
||||
const ToolFeedbackContinuationHint = "Continuing the current task."
|
||||
|
||||
// FormatToolFeedbackMessage renders the model-provided explanation for why a
|
||||
// tool is being executed. When the model does not provide one, it keeps only
|
||||
// the tool line and does not expose raw arguments or fallback text.
|
||||
func FormatToolFeedbackMessage(toolName, explanation string) string {
|
||||
// FormatToolFeedbackMessage renders a tool feedback message for chat channels.
|
||||
// It keeps the tool name on the first line for animation and can include both
|
||||
// a human explanation and the serialized tool arguments in the body.
|
||||
func FormatToolFeedbackMessage(toolName, explanation, argsPreview string) string {
|
||||
toolName = strings.TrimSpace(toolName)
|
||||
explanation = strings.TrimSpace(explanation)
|
||||
argsPreview = strings.TrimSpace(argsPreview)
|
||||
|
||||
bodyLines := make([]string, 0, 2)
|
||||
if explanation != "" {
|
||||
bodyLines = append(bodyLines, explanation)
|
||||
}
|
||||
if argsPreview != "" {
|
||||
bodyLines = append(bodyLines, "```json\n"+argsPreview+"\n```")
|
||||
}
|
||||
body := strings.Join(bodyLines, "\n")
|
||||
|
||||
if toolName == "" {
|
||||
return explanation
|
||||
return body
|
||||
}
|
||||
if explanation == "" {
|
||||
if body == "" {
|
||||
return fmt.Sprintf("\U0001f527 `%s`", toolName)
|
||||
}
|
||||
|
||||
return fmt.Sprintf("\U0001f527 `%s`\n%s", toolName, explanation)
|
||||
return fmt.Sprintf("\U0001f527 `%s`\n%s", toolName, body)
|
||||
}
|
||||
|
||||
// FitToolFeedbackMessage keeps tool feedback within a single outbound message.
|
||||
|
||||
@@ -6,29 +6,38 @@ func TestFormatToolFeedbackMessage(t *testing.T) {
|
||||
got := FormatToolFeedbackMessage(
|
||||
"read_file",
|
||||
"I will read README.md first to confirm the current project structure.",
|
||||
"{\n \"path\": \"README.md\"\n}",
|
||||
)
|
||||
want := "\U0001f527 `read_file`\nI will read README.md first to confirm the current project structure."
|
||||
want := "\U0001f527 `read_file`\nI will read README.md first to confirm the current project structure.\n```json\n{\n \"path\": \"README.md\"\n}\n```"
|
||||
if got != want {
|
||||
t.Fatalf("FormatToolFeedbackMessage() = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFormatToolFeedbackMessage_EmptyExplanationKeepsOnlyToolLine(t *testing.T) {
|
||||
got := FormatToolFeedbackMessage("read_file", "")
|
||||
want := "\U0001f527 `read_file`"
|
||||
func TestFormatToolFeedbackMessage_EmptyExplanationShowsArgs(t *testing.T) {
|
||||
got := FormatToolFeedbackMessage("read_file", "", "{\n \"path\": \"README.md\"\n}")
|
||||
want := "\U0001f527 `read_file`\n```json\n{\n \"path\": \"README.md\"\n}\n```"
|
||||
if got != want {
|
||||
t.Fatalf("FormatToolFeedbackMessage() = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFormatToolFeedbackMessage_EmptyToolNameOmitsToolLine(t *testing.T) {
|
||||
got := FormatToolFeedbackMessage("", "Continue drafting the final response.")
|
||||
got := FormatToolFeedbackMessage("", "Continue drafting the final response.", "")
|
||||
want := "Continue drafting the final response."
|
||||
if got != want {
|
||||
t.Fatalf("FormatToolFeedbackMessage() = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFormatToolFeedbackMessage_EmptyExplanationAndArgsKeepsOnlyToolLine(t *testing.T) {
|
||||
got := FormatToolFeedbackMessage("read_file", "", "")
|
||||
want := "\U0001f527 `read_file`"
|
||||
if got != want {
|
||||
t.Fatalf("FormatToolFeedbackMessage() = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFitToolFeedbackMessage_TruncatesBodyWithinSingleMessage(t *testing.T) {
|
||||
got := FitToolFeedbackMessage(
|
||||
"\U0001f527 `read_file`\nRead README.md first to confirm the current project structure.",
|
||||
|
||||
@@ -2,6 +2,7 @@ package api
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
@@ -593,7 +594,15 @@ func toolSummaryContainsContent(summary, content string) bool {
|
||||
}
|
||||
|
||||
_, body, hasBody := strings.Cut(summary, "\n")
|
||||
return hasBody && strings.TrimSpace(body) == content
|
||||
if !hasBody {
|
||||
return false
|
||||
}
|
||||
body = strings.TrimSpace(body)
|
||||
if body == content {
|
||||
return true
|
||||
}
|
||||
firstSection, _, _ := strings.Cut(body, "\n```")
|
||||
return strings.TrimSpace(firstSection) == content
|
||||
}
|
||||
|
||||
func sessionAttachments(msg providers.Message) []sessionChatAttachment {
|
||||
@@ -714,7 +723,8 @@ func visibleAssistantToolSummaryMessages(
|
||||
Role: "assistant",
|
||||
Content: utils.FormatToolFeedbackMessage(
|
||||
name,
|
||||
visibleAssistantToolSummaryText(tc, toolFeedbackMaxArgsLength),
|
||||
visibleAssistantToolFeedbackExplanation(tc, toolFeedbackMaxArgsLength),
|
||||
visibleAssistantToolArgsPreview(tc, toolFeedbackMaxArgsLength),
|
||||
),
|
||||
})
|
||||
}
|
||||
@@ -722,7 +732,7 @@ func visibleAssistantToolSummaryMessages(
|
||||
return messages
|
||||
}
|
||||
|
||||
func visibleAssistantToolSummaryText(
|
||||
func visibleAssistantToolFeedbackExplanation(
|
||||
tc providers.ToolCall,
|
||||
toolFeedbackMaxArgsLength int,
|
||||
) string {
|
||||
@@ -731,18 +741,32 @@ func visibleAssistantToolSummaryText(
|
||||
return utils.Truncate(explanation, toolFeedbackMaxArgsLength)
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func visibleAssistantToolArgsPreview(
|
||||
tc providers.ToolCall,
|
||||
toolFeedbackMaxArgsLength int,
|
||||
) string {
|
||||
argsJSON := ""
|
||||
if tc.Function != nil {
|
||||
argsJSON = tc.Function.Arguments
|
||||
}
|
||||
if strings.TrimSpace(argsJSON) == "" && len(tc.Arguments) > 0 {
|
||||
if encodedArgs, err := json.Marshal(tc.Arguments); err == nil {
|
||||
if encodedArgs, err := json.MarshalIndent(tc.Arguments, "", " "); err == nil {
|
||||
argsJSON = string(encodedArgs)
|
||||
}
|
||||
}
|
||||
argsJSON = strings.TrimSpace(argsJSON)
|
||||
if argsJSON == "" {
|
||||
return ""
|
||||
}
|
||||
var pretty bytes.Buffer
|
||||
if err := json.Indent(&pretty, []byte(argsJSON), "", " "); err == nil {
|
||||
argsJSON = pretty.String()
|
||||
}
|
||||
|
||||
return utils.Truncate(strings.TrimSpace(argsJSON), toolFeedbackMaxArgsLength)
|
||||
return utils.Truncate(argsJSON, toolFeedbackMaxArgsLength)
|
||||
}
|
||||
|
||||
func visibleAssistantToolMessages(toolCalls []providers.ToolCall) []sessionChatMessage {
|
||||
|
||||
@@ -1056,8 +1056,11 @@ func TestHandleGetSession_UsesConfiguredToolFeedbackMaxArgsLength(t *testing.T)
|
||||
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)
|
||||
wantArgsPreview := visibleAssistantToolArgsPreview(providers.ToolCall{
|
||||
Function: &providers.FunctionCall{Arguments: argsJSON},
|
||||
}, 20)
|
||||
if !strings.Contains(resp.Messages[1].Content, wantArgsPreview) {
|
||||
t.Fatalf("tool summary = %q, want args preview %q", resp.Messages[1].Content, wantArgsPreview)
|
||||
}
|
||||
if !strings.Contains(resp.Messages[1].Content, "`read_file`") {
|
||||
t.Fatalf("tool summary = %q, want read_file summary", resp.Messages[1].Content)
|
||||
@@ -1132,7 +1135,9 @@ func TestHandleGetSession_FallsBackToLegacyToolArgumentsWhenExplanationMissing(t
|
||||
t.Fatalf("len(resp.Messages) = %d, want at least 2", len(resp.Messages))
|
||||
}
|
||||
|
||||
wantPreview := utils.Truncate(argsJSON, 20)
|
||||
wantPreview := visibleAssistantToolArgsPreview(providers.ToolCall{
|
||||
Function: &providers.FunctionCall{Arguments: argsJSON},
|
||||
}, 20)
|
||||
if !strings.Contains(resp.Messages[1].Content, "`read_file`") {
|
||||
t.Fatalf("tool summary = %q, want read_file summary", resp.Messages[1].Content)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user