fix(tool-feedback): unify fallback and single-message delivery
This commit is contained in:
parent
04733d9f37
commit
9ea8e6167a
13 changed files with 139 additions and 42 deletions
|
|
@ -54,7 +54,7 @@ Discord can show three different kinds of "working" feedback:
|
|||
|
||||
```text
|
||||
🔧 `web_search`
|
||||
{"query":"picoclaw release notes"}
|
||||
Checking the latest PicoClaw release notes before I answer.
|
||||
```
|
||||
|
||||
If you only see `Bot is typing`, check that `placeholder.enabled` or `tool_feedback.enabled` is actually set in your runtime config.
|
||||
|
|
|
|||
|
|
@ -851,13 +851,10 @@ func outboundMessageForTurnWithKind(ts *turnState, content, kind string) bus.Out
|
|||
return msg
|
||||
}
|
||||
|
||||
func previousAssistantContent(messages []providers.Message) string {
|
||||
func latestUserContent(messages []providers.Message) string {
|
||||
for i := len(messages) - 1; i >= 0; i-- {
|
||||
msg := messages[i]
|
||||
if msg.Role == "user" {
|
||||
break
|
||||
}
|
||||
if msg.Role != "assistant" {
|
||||
if msg.Role != "user" {
|
||||
continue
|
||||
}
|
||||
if content := strings.TrimSpace(msg.Content); content != "" {
|
||||
|
|
@ -883,7 +880,10 @@ func toolFeedbackExplanationFromResponse(
|
|||
explanation = strings.TrimSpace(response.ReasoningContent)
|
||||
}
|
||||
if explanation == "" {
|
||||
explanation = previousAssistantContent(messages)
|
||||
explanation = latestUserContent(messages)
|
||||
if explanation != "" {
|
||||
explanation = utils.ToolFeedbackContinuationHint + ": " + explanation
|
||||
}
|
||||
}
|
||||
return utils.Truncate(explanation, maxLen)
|
||||
}
|
||||
|
|
@ -2738,6 +2738,12 @@ turnLoop:
|
|||
for _, tc := range normalizedToolCalls {
|
||||
argumentsJSON, _ := json.Marshal(tc.Arguments)
|
||||
extraContent := tc.ExtraContent
|
||||
if strings.TrimSpace(toolFeedbackExplanation) != "" {
|
||||
if extraContent == nil {
|
||||
extraContent = &providers.ExtraContent{}
|
||||
}
|
||||
extraContent.ToolFeedbackExplanation = toolFeedbackExplanation
|
||||
}
|
||||
thoughtSignature := ""
|
||||
if tc.Function != nil {
|
||||
thoughtSignature = tc.Function.ThoughtSignature
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ import (
|
|||
"github.com/sipeed/picoclaw/pkg/routing"
|
||||
"github.com/sipeed/picoclaw/pkg/session"
|
||||
"github.com/sipeed/picoclaw/pkg/tools"
|
||||
"github.com/sipeed/picoclaw/pkg/utils"
|
||||
)
|
||||
|
||||
type fakeChannel struct{ id string }
|
||||
|
|
@ -1827,7 +1828,7 @@ func TestToolFeedbackExplanationFromResponse_FallsBackToReasoningContent(t *test
|
|||
}
|
||||
}
|
||||
|
||||
func TestToolFeedbackExplanationFromResponse_UsesPreviousAssistantContentAsLastResort(t *testing.T) {
|
||||
func TestToolFeedbackExplanationFromResponse_UsesLatestUserContentAsLastResort(t *testing.T) {
|
||||
response := &providers.LLMResponse{
|
||||
Content: "",
|
||||
ReasoningContent: "",
|
||||
|
|
@ -1835,12 +1836,14 @@ func TestToolFeedbackExplanationFromResponse_UsesPreviousAssistantContentAsLastR
|
|||
messages := []providers.Message{
|
||||
{Role: "user", Content: "check file"},
|
||||
{Role: "assistant", Content: "Previous turn explanation"},
|
||||
{Role: "user", Content: "Inspect README.md and update the config example."},
|
||||
{Role: "tool", Content: "tool output", ToolCallID: "call_1"},
|
||||
}
|
||||
|
||||
got := toolFeedbackExplanationFromResponse(response, messages, 300)
|
||||
if got != "Previous turn explanation" {
|
||||
t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want previous assistant content", got)
|
||||
want := utils.ToolFeedbackContinuationHint + ": Inspect README.md and update the config example."
|
||||
if got != want {
|
||||
t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want latest user content fallback", got)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -3744,8 +3747,14 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) {
|
|||
if !strings.Contains(outbound.Content, "`read_file`") {
|
||||
t.Fatalf("tool feedback content = %q, want read_file summary", outbound.Content)
|
||||
}
|
||||
if strings.Contains(outbound.Content, "Why this tool should be executed") {
|
||||
t.Fatalf("tool feedback content = %q, want no fallback explanation", outbound.Content)
|
||||
if !strings.Contains(outbound.Content, utils.ToolFeedbackContinuationHint) {
|
||||
t.Fatalf("tool feedback content = %q, want continuation hint fallback", outbound.Content)
|
||||
}
|
||||
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, "Previous turn explanation") {
|
||||
t.Fatalf("tool feedback content = %q, want no previous assistant fallback", outbound.Content)
|
||||
}
|
||||
if outbound.AgentID != "main" {
|
||||
t.Fatalf("tool feedback agent_id = %q, want main", outbound.AgentID)
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ import (
|
|||
"github.com/sipeed/picoclaw/pkg/health"
|
||||
"github.com/sipeed/picoclaw/pkg/logger"
|
||||
"github.com/sipeed/picoclaw/pkg/media"
|
||||
"github.com/sipeed/picoclaw/pkg/utils"
|
||||
)
|
||||
|
||||
const (
|
||||
|
|
@ -820,14 +821,16 @@ func (m *Manager) runWorker(ctx context.Context, name string, w *channelWorker)
|
|||
if m.config != nil && m.config.Agents.Defaults.SplitOnMarker {
|
||||
if markerChunks := SplitByMarker(msg.Content); len(markerChunks) > 1 {
|
||||
for _, chunk := range markerChunks {
|
||||
chunks = append(chunks, splitByLength(chunk, maxLen)...)
|
||||
chunkMsg := msg
|
||||
chunkMsg.Content = chunk
|
||||
chunks = append(chunks, splitOutboundMessageContent(chunkMsg, maxLen)...)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Step 2: Fallback to length-based splitting if no chunks from marker
|
||||
if len(chunks) == 0 {
|
||||
chunks = splitByLength(msg.Content, maxLen)
|
||||
chunks = splitOutboundMessageContent(msg, maxLen)
|
||||
}
|
||||
|
||||
// Step 3: Send all chunks
|
||||
|
|
@ -842,12 +845,16 @@ func (m *Manager) runWorker(ctx context.Context, name string, w *channelWorker)
|
|||
}
|
||||
}
|
||||
|
||||
// splitByLength splits content by maxLen if needed, otherwise returns single chunk.
|
||||
func splitByLength(content string, maxLen int) []string {
|
||||
if maxLen > 0 && len([]rune(content)) > maxLen {
|
||||
return SplitMessage(content, maxLen)
|
||||
// splitOutboundMessageContent splits regular outbound content by maxLen, but
|
||||
// keeps tool feedback in a single message by truncating the explanation body.
|
||||
func splitOutboundMessageContent(msg bus.OutboundMessage, maxLen int) []string {
|
||||
if maxLen > 0 && len([]rune(msg.Content)) > maxLen {
|
||||
if outboundMessageIsToolFeedback(msg) {
|
||||
return []string{utils.FitToolFeedbackMessage(msg.Content, maxLen)}
|
||||
}
|
||||
return SplitMessage(msg.Content, maxLen)
|
||||
}
|
||||
return []string{content}
|
||||
return []string{msg.Content}
|
||||
}
|
||||
|
||||
// sendWithRetry sends a message through the channel with rate limiting and
|
||||
|
|
@ -1311,13 +1318,16 @@ func (m *Manager) SendMessage(ctx context.Context, msg bus.OutboundMessage) erro
|
|||
if mlp, ok := w.ch.(MessageLengthProvider); ok {
|
||||
maxLen = mlp.MaxMessageLength()
|
||||
}
|
||||
if maxLen > 0 && len([]rune(msg.Content)) > maxLen {
|
||||
for _, chunk := range SplitMessage(msg.Content, maxLen) {
|
||||
if chunks := splitOutboundMessageContent(msg, maxLen); len(chunks) > 1 {
|
||||
for _, chunk := range chunks {
|
||||
chunkMsg := msg
|
||||
chunkMsg.Content = chunk
|
||||
m.sendWithRetry(ctx, channelName, w, chunkMsg)
|
||||
}
|
||||
} else {
|
||||
if len(chunks) == 1 {
|
||||
msg.Content = chunks[0]
|
||||
}
|
||||
m.sendWithRetry(ctx, channelName, w, msg)
|
||||
}
|
||||
return nil
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import (
|
|||
"golang.org/x/time/rate"
|
||||
|
||||
"github.com/sipeed/picoclaw/pkg/bus"
|
||||
"github.com/sipeed/picoclaw/pkg/utils"
|
||||
)
|
||||
|
||||
// mockChannel is a test double that delegates Send to a configurable function.
|
||||
|
|
@ -907,6 +908,30 @@ func TestPreSendMedia_DismissesTrackedMessage(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestSplitOutboundMessageContent_ToolFeedbackTruncatesInsteadOfSplitting(t *testing.T) {
|
||||
msg := testOutboundMessage(bus.OutboundMessage{
|
||||
Channel: "test",
|
||||
ChatID: "123",
|
||||
Content: "\U0001f527 `read_file`\nRead README.md first to confirm the current project structure before editing the config example.",
|
||||
Context: bus.InboundContext{
|
||||
Channel: "test",
|
||||
ChatID: "123",
|
||||
Raw: map[string]string{
|
||||
"message_kind": "tool_feedback",
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
chunks := splitOutboundMessageContent(msg, 40)
|
||||
if len(chunks) != 1 {
|
||||
t.Fatalf("len(chunks) = %d, want 1", len(chunks))
|
||||
}
|
||||
want := utils.FitToolFeedbackMessage(msg.Content, 40)
|
||||
if chunks[0] != want {
|
||||
t.Fatalf("chunk = %q, want %q", chunks[0], want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPreSend_PlaceholderEditFails_FallsThrough(t *testing.T) {
|
||||
m := newTestManager()
|
||||
|
||||
|
|
|
|||
|
|
@ -286,7 +286,7 @@ func (d *AgentDefaults) GetMaxMediaSize() int {
|
|||
return DefaultMaxMediaSize
|
||||
}
|
||||
|
||||
// GetToolFeedbackMaxArgsLength returns the max args preview length for tool feedback messages.
|
||||
// GetToolFeedbackMaxArgsLength returns the max visible text length for tool feedback messages.
|
||||
func (d *AgentDefaults) GetToolFeedbackMaxArgsLength() int {
|
||||
if d.ToolFeedback.MaxArgsLength > 0 {
|
||||
return d.ToolFeedback.MaxArgsLength
|
||||
|
|
|
|||
|
|
@ -11,7 +11,8 @@ type ToolCall struct {
|
|||
}
|
||||
|
||||
type ExtraContent struct {
|
||||
Google *GoogleExtra `json:"google,omitempty"`
|
||||
Google *GoogleExtra `json:"google,omitempty"`
|
||||
ToolFeedbackExplanation string `json:"tool_feedback_explanation,omitempty"`
|
||||
}
|
||||
|
||||
type GoogleExtra struct {
|
||||
|
|
|
|||
|
|
@ -5,6 +5,8 @@ import (
|
|||
"strings"
|
||||
)
|
||||
|
||||
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.
|
||||
|
|
@ -21,3 +23,35 @@ func FormatToolFeedbackMessage(toolName, explanation string) string {
|
|||
|
||||
return fmt.Sprintf("\U0001f527 `%s`\n%s", toolName, explanation)
|
||||
}
|
||||
|
||||
// FitToolFeedbackMessage keeps tool feedback within a single outbound message.
|
||||
// It preserves the first line when possible and truncates the explanation body
|
||||
// instead of letting the message be split into multiple chunks.
|
||||
func FitToolFeedbackMessage(content string, maxLen int) string {
|
||||
content = strings.TrimSpace(content)
|
||||
if content == "" || maxLen <= 0 {
|
||||
return ""
|
||||
}
|
||||
if len([]rune(content)) <= maxLen {
|
||||
return content
|
||||
}
|
||||
|
||||
firstLine, rest, hasRest := strings.Cut(content, "\n")
|
||||
firstLine = strings.TrimSpace(firstLine)
|
||||
rest = strings.TrimSpace(rest)
|
||||
|
||||
if !hasRest || rest == "" {
|
||||
return Truncate(firstLine, maxLen)
|
||||
}
|
||||
|
||||
if len([]rune(firstLine)) >= maxLen {
|
||||
return Truncate(firstLine, maxLen)
|
||||
}
|
||||
|
||||
remaining := maxLen - len([]rune(firstLine)) - 1
|
||||
if remaining <= 0 {
|
||||
return Truncate(firstLine, maxLen)
|
||||
}
|
||||
|
||||
return firstLine + "\n" + Truncate(rest, remaining)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -25,3 +25,19 @@ func TestFormatToolFeedbackMessage_EmptyToolNameOmitsToolLine(t *testing.T) {
|
|||
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.", 40)
|
||||
want := "\U0001f527 `read_file`\nRead README.md first to..."
|
||||
if got != want {
|
||||
t.Fatalf("FitToolFeedbackMessage() = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFitToolFeedbackMessage_TruncatesSingleLineMessage(t *testing.T) {
|
||||
got := FitToolFeedbackMessage("\U0001f527 `read_file`", 10)
|
||||
want := "\U0001f527 `read..."
|
||||
if got != want {
|
||||
t.Fatalf("FitToolFeedbackMessage() = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -529,31 +529,23 @@ func visibleAssistantToolSummaryMessages(
|
|||
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 = "{}"
|
||||
explanation := ""
|
||||
if tc.ExtraContent != nil {
|
||||
explanation = strings.TrimSpace(tc.ExtraContent.ToolFeedbackExplanation)
|
||||
}
|
||||
messages = append(messages, sessionChatMessage{
|
||||
Role: "assistant",
|
||||
Content: utils.FormatToolFeedbackMessage(name, utils.Truncate(argsPreview, toolFeedbackMaxArgsLength)),
|
||||
Content: utils.FormatToolFeedbackMessage(name, utils.Truncate(explanation, toolFeedbackMaxArgsLength)),
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -629,6 +629,7 @@ func TestHandleGetSession_UsesConfiguredToolFeedbackMaxArgsLength(t *testing.T)
|
|||
}
|
||||
|
||||
argsJSON := `{"path":"README.md","start_line":1,"end_line":10,"extra":"abcdefghijklmnopqrstuvwxyz"}`
|
||||
explanation := "Read README.md first to confirm the current project structure before editing the config example."
|
||||
sessionKey := picoSessionPrefix + "detail-tool-summary-max-args"
|
||||
err = store.AddFullMessage(nil, sessionKey, providers.Message{Role: "user", Content: "check file"})
|
||||
if err != nil {
|
||||
|
|
@ -643,6 +644,9 @@ func TestHandleGetSession_UsesConfiguredToolFeedbackMaxArgsLength(t *testing.T)
|
|||
Name: "read_file",
|
||||
Arguments: argsJSON,
|
||||
},
|
||||
ExtraContent: &providers.ExtraContent{
|
||||
ToolFeedbackExplanation: explanation,
|
||||
},
|
||||
}},
|
||||
})
|
||||
if err != nil {
|
||||
|
|
@ -675,7 +679,7 @@ func TestHandleGetSession_UsesConfiguredToolFeedbackMaxArgsLength(t *testing.T)
|
|||
t.Fatalf("len(resp.Messages) = %d, want at least 2", len(resp.Messages))
|
||||
}
|
||||
|
||||
wantPreview := utils.Truncate(argsJSON, 20)
|
||||
wantPreview := utils.Truncate(explanation, 20)
|
||||
if !strings.Contains(resp.Messages[1].Content, wantPreview) {
|
||||
t.Fatalf("tool summary = %q, want preview %q", resp.Messages[1].Content, wantPreview)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -592,9 +592,9 @@
|
|||
"split_on_marker": "Chatty Mode",
|
||||
"split_on_marker_hint": "Split long messages into short ones like real human chatting.",
|
||||
"tool_feedback_enabled": "Tool Feedback",
|
||||
"tool_feedback_enabled_hint": "Send a short tool-call preview into the current chat before each tool execution.",
|
||||
"tool_feedback_max_args_length": "Tool Feedback Args Preview Length",
|
||||
"tool_feedback_max_args_length_hint": "Maximum number of argument characters shown in each tool feedback message. Set to 0 to use the default.",
|
||||
"tool_feedback_enabled_hint": "Send a short execution note into the current chat before each tool runs.",
|
||||
"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.",
|
||||
"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",
|
||||
|
|
|
|||
|
|
@ -592,9 +592,9 @@
|
|||
"split_on_marker": "连续短消息",
|
||||
"split_on_marker_hint": "像真人聊天一样,把长难句拆成多条短消息快速发出",
|
||||
"tool_feedback_enabled": "工具反馈",
|
||||
"tool_feedback_enabled_hint": "在每次执行工具前,先向当前会话发送一条简短的工具调用预览",
|
||||
"tool_feedback_max_args_length": "工具反馈参数预览长度",
|
||||
"tool_feedback_max_args_length_hint": "每条工具反馈消息中展示的参数字符上限。设为 0 时使用默认值",
|
||||
"tool_feedback_enabled_hint": "在每次执行工具前,先向当前会话发送一条简短的执行说明",
|
||||
"tool_feedback_max_args_length": "工具反馈长度",
|
||||
"tool_feedback_max_args_length_hint": "每条工具反馈消息中展示的字符上限。设为 0 时使用默认值",
|
||||
"exec_enabled": "允许命令执行",
|
||||
"exec_enabled_hint": "控制应用是否允许执行命令。关闭后,所有命令请求都不会执行",
|
||||
"allow_remote": "允许远程命令执行",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue