Clarify reply routing and add diagnostics
This commit is contained in:
parent
51933ecd09
commit
a20ec7fe1e
5 changed files with 93 additions and 27 deletions
|
|
@ -463,6 +463,7 @@ func buildReplyRoutingContext(channel string, replyCtx *ReplyContextInfo) string
|
||||||
"- `[[reply:parent]]` replies to the parent/replied-to message when there is one\n"+
|
"- `[[reply:parent]]` replies to the parent/replied-to message when there is one\n"+
|
||||||
"- `[[reply:message_id=123]]` replies to a specific known message ID\n\n"+
|
"- `[[reply:message_id=123]]` replies to a specific known message ID\n\n"+
|
||||||
"After the directive, add a blank line and then the user-visible message.\n"+
|
"After the directive, add a blank line and then the user-visible message.\n"+
|
||||||
|
"Do not use the `message` tool for the normal reply in this chat; use the final answer plus a directive when you need reply routing.\n"+
|
||||||
"If you do not need special routing, answer normally without a directive.\n"+
|
"If you do not need special routing, answer normally without a directive.\n"+
|
||||||
"Never mention the directive in the visible message body.",
|
"Never mention the directive in the visible message body.",
|
||||||
replyCtx.CurrentMessageID,
|
replyCtx.CurrentMessageID,
|
||||||
|
|
|
||||||
|
|
@ -156,6 +156,9 @@ func TestBuildMessages_TelegramReplyRoutingContext(t *testing.T) {
|
||||||
if !strings.Contains(sys, "[[reply:current]]") {
|
if !strings.Contains(sys, "[[reply:current]]") {
|
||||||
t.Fatal("system prompt missing final reply directive guidance")
|
t.Fatal("system prompt missing final reply directive guidance")
|
||||||
}
|
}
|
||||||
|
if !strings.Contains(sys, "Do not use the `message` tool for the normal reply in this chat") {
|
||||||
|
t.Fatal("system prompt missing guidance to avoid message tool for normal replies")
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestMtimeAutoInvalidation verifies that the cache detects source file changes
|
// TestMtimeAutoInvalidation verifies that the cache detects source file changes
|
||||||
|
|
|
||||||
|
|
@ -380,12 +380,18 @@ func (al *AgentLoop) Run(ctx context.Context) error {
|
||||||
"channel": msg.Channel,
|
"channel": msg.Channel,
|
||||||
"chat_id": msg.ChatID,
|
"chat_id": msg.ChatID,
|
||||||
"content_len": len(response.Content),
|
"content_len": len(response.Content),
|
||||||
|
"reply_to_message_id": response.ReplyToMessageID,
|
||||||
})
|
})
|
||||||
} else {
|
} else {
|
||||||
logger.DebugCF(
|
logger.DebugCF(
|
||||||
"agent",
|
"agent",
|
||||||
"Skipped outbound (message tool already sent)",
|
"Skipped outbound (message tool already sent)",
|
||||||
map[string]any{"channel": msg.Channel},
|
map[string]any{
|
||||||
|
"channel": msg.Channel,
|
||||||
|
"chat_id": msg.ChatID,
|
||||||
|
"content_len": len(response.Content),
|
||||||
|
"reply_to_message_id": response.ReplyToMessageID,
|
||||||
|
},
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -882,6 +888,42 @@ func resolveFinalResponse(
|
||||||
rawContent string,
|
rawContent string,
|
||||||
) agentResponse {
|
) agentResponse {
|
||||||
content, replyToMessageID := parseFinalReplyDirective(channel, replyCtx, rawContent)
|
content, replyToMessageID := parseFinalReplyDirective(channel, replyCtx, rawContent)
|
||||||
|
if channel == "telegram" {
|
||||||
|
firstLine, _, _ := strings.Cut(rawContent, "\n")
|
||||||
|
directive := strings.TrimSpace(firstLine)
|
||||||
|
hasDirective := strings.HasPrefix(directive, "[[reply:") && strings.HasSuffix(directive, "]]")
|
||||||
|
directiveMode := ""
|
||||||
|
if hasDirective {
|
||||||
|
directiveMode = strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(directive, "[[reply:"), "]]"))
|
||||||
|
}
|
||||||
|
directiveStatus := "none"
|
||||||
|
switch {
|
||||||
|
case !hasDirective:
|
||||||
|
directiveStatus = "none"
|
||||||
|
case directiveMode == "chat":
|
||||||
|
directiveStatus = "applied_chat"
|
||||||
|
case replyToMessageID != "":
|
||||||
|
directiveStatus = "applied_reply"
|
||||||
|
default:
|
||||||
|
directiveStatus = "dropped"
|
||||||
|
}
|
||||||
|
|
||||||
|
fields := map[string]any{
|
||||||
|
"directive_status": directiveStatus,
|
||||||
|
"reply_to_message_id": replyToMessageID,
|
||||||
|
"raw_content_len": len(rawContent),
|
||||||
|
"final_content_len": len(content),
|
||||||
|
}
|
||||||
|
if hasDirective {
|
||||||
|
fields["directive"] = directive
|
||||||
|
fields["directive_mode"] = directiveMode
|
||||||
|
}
|
||||||
|
if replyCtx != nil {
|
||||||
|
fields["current_message_id"] = strings.TrimSpace(replyCtx.CurrentMessageID)
|
||||||
|
fields["parent_message_id"] = strings.TrimSpace(replyCtx.ParentMessageID)
|
||||||
|
}
|
||||||
|
logger.DebugCF("agent", "Resolved final reply routing", fields)
|
||||||
|
}
|
||||||
return agentResponse{
|
return agentResponse{
|
||||||
Content: content,
|
Content: content,
|
||||||
ReplyToMessageID: replyToMessageID,
|
ReplyToMessageID: replyToMessageID,
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@ import (
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
|
|
||||||
"github.com/sipeed/picoclaw/pkg/bus"
|
"github.com/sipeed/picoclaw/pkg/bus"
|
||||||
|
"github.com/sipeed/picoclaw/pkg/logger"
|
||||||
)
|
)
|
||||||
|
|
||||||
type SendCallback func(msg bus.OutboundMessage) error
|
type SendCallback func(msg bus.OutboundMessage) error
|
||||||
|
|
@ -31,7 +32,7 @@ func (t *MessageTool) Name() string {
|
||||||
}
|
}
|
||||||
|
|
||||||
func (t *MessageTool) Description() string {
|
func (t *MessageTool) Description() string {
|
||||||
return "Send a message to the user on a chat channel. Use this when you want to communicate something or explicitly control reply threading."
|
return "Send an out-of-band message to a chat channel. Do not use this for the normal final reply in the current conversation."
|
||||||
}
|
}
|
||||||
|
|
||||||
func (t *MessageTool) Parameters() map[string]any {
|
func (t *MessageTool) Parameters() map[string]any {
|
||||||
|
|
@ -50,15 +51,6 @@ func (t *MessageTool) Parameters() map[string]any {
|
||||||
"type": "string",
|
"type": "string",
|
||||||
"description": "Optional: target chat/user ID",
|
"description": "Optional: target chat/user ID",
|
||||||
},
|
},
|
||||||
"reply_mode": map[string]any{
|
|
||||||
"type": "string",
|
|
||||||
"enum": []string{replyModeChat, replyModeCurrent, replyModeParent},
|
|
||||||
"description": "Optional: threading mode. chat sends a normal message, current replies to the current inbound message, parent replies to the parent/replied-to inbound message.",
|
|
||||||
},
|
|
||||||
"reply_to_message_id": map[string]any{
|
|
||||||
"type": "string",
|
|
||||||
"description": "Optional: explicit platform message ID to reply to. Overrides reply_mode when provided.",
|
|
||||||
},
|
|
||||||
},
|
},
|
||||||
"required": []string{"content"},
|
"required": []string{"content"},
|
||||||
}
|
}
|
||||||
|
|
@ -99,6 +91,32 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *ToolRes
|
||||||
return &ToolResult{ForLLM: "No target channel/chat specified", IsError: true}
|
return &ToolResult{ForLLM: "No target channel/chat specified", IsError: true}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
currentChannel := ToolChannel(ctx)
|
||||||
|
currentChatID := ToolChatID(ctx)
|
||||||
|
replyMode, _ := args["reply_mode"].(string)
|
||||||
|
replyMode = strings.ToLower(strings.TrimSpace(replyMode))
|
||||||
|
explicitReplyTo, _ := args["reply_to_message_id"].(string)
|
||||||
|
explicitReplyTo = strings.TrimSpace(explicitReplyTo)
|
||||||
|
|
||||||
|
if replyMode != "" || explicitReplyTo != "" {
|
||||||
|
logger.WarnCF("tool", "Message tool received deprecated reply routing args", map[string]any{
|
||||||
|
"channel": channel,
|
||||||
|
"chat_id": chatID,
|
||||||
|
"reply_mode": replyMode,
|
||||||
|
"reply_to_message_id": explicitReplyTo,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
if currentChannel != "" && currentChatID != "" && channel == currentChannel && chatID == currentChatID {
|
||||||
|
logger.InfoCF("tool", "Message tool targeting current conversation", map[string]any{
|
||||||
|
"channel": channel,
|
||||||
|
"chat_id": chatID,
|
||||||
|
"content_len": len(content),
|
||||||
|
"reply_mode": replyMode,
|
||||||
|
"same_target": true,
|
||||||
|
"session_key": ToolSessionKey(ctx),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
replyToMessageID, err := resolveReplyTarget(ctx, args)
|
replyToMessageID, err := resolveReplyTarget(ctx, args)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return &ToolResult{
|
return &ToolResult{
|
||||||
|
|
@ -127,6 +145,14 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *ToolRes
|
||||||
}
|
}
|
||||||
|
|
||||||
t.sentInRound.Store(true)
|
t.sentInRound.Store(true)
|
||||||
|
logger.InfoCF("tool", "Message tool sent outbound message", map[string]any{
|
||||||
|
"channel": channel,
|
||||||
|
"chat_id": chatID,
|
||||||
|
"content_len": len(content),
|
||||||
|
"reply_to_message_id": replyToMessageID,
|
||||||
|
"same_target": currentChannel != "" && currentChatID != "" && channel == currentChannel && chatID == currentChatID,
|
||||||
|
})
|
||||||
|
|
||||||
// Silent: user already received the message directly
|
// Silent: user already received the message directly
|
||||||
status := fmt.Sprintf("Message sent to %s:%s", channel, chatID)
|
status := fmt.Sprintf("Message sent to %s:%s", channel, chatID)
|
||||||
if replyToMessageID != "" {
|
if replyToMessageID != "" {
|
||||||
|
|
|
||||||
|
|
@ -284,6 +284,9 @@ func TestMessageTool_Description(t *testing.T) {
|
||||||
if desc == "" {
|
if desc == "" {
|
||||||
t.Error("Description should not be empty")
|
t.Error("Description should not be empty")
|
||||||
}
|
}
|
||||||
|
if desc == "Send a message to the user on a chat channel. Use this when you want to communicate something or explicitly control reply threading." {
|
||||||
|
t.Fatal("description still advertises reply threading")
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestMessageTool_Parameters(t *testing.T) {
|
func TestMessageTool_Parameters(t *testing.T) {
|
||||||
|
|
@ -334,19 +337,10 @@ func TestMessageTool_Parameters(t *testing.T) {
|
||||||
t.Error("Expected chat_id type to be 'string'")
|
t.Error("Expected chat_id type to be 'string'")
|
||||||
}
|
}
|
||||||
|
|
||||||
replyModeProp, ok := props["reply_mode"].(map[string]any)
|
if _, ok := props["reply_mode"]; ok {
|
||||||
if !ok {
|
t.Error("Did not expect 'reply_mode' property in advertised schema")
|
||||||
t.Error("Expected 'reply_mode' property")
|
|
||||||
}
|
}
|
||||||
if replyModeProp["type"] != "string" {
|
if _, ok := props["reply_to_message_id"]; ok {
|
||||||
t.Error("Expected reply_mode type to be 'string'")
|
t.Error("Did not expect 'reply_to_message_id' property in advertised schema")
|
||||||
}
|
|
||||||
|
|
||||||
replyToProp, ok := props["reply_to_message_id"].(map[string]any)
|
|
||||||
if !ok {
|
|
||||||
t.Error("Expected 'reply_to_message_id' property")
|
|
||||||
}
|
|
||||||
if replyToProp["type"] != "string" {
|
|
||||||
t.Error("Expected reply_to_message_id type to be 'string'")
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue