diff --git a/CHANGELOG.md b/CHANGELOG.md index 705ddf38..69bc3e21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,12 @@ All notable changes to GoClaw are documented here. For full documentation, see [ ### Fixed +- **Intermediate Replies are model-generated** — Tool-call progress no longer + appends fixed "I'll use ..." text or emits synthetic fallback bubbles when the + model returns tool calls without assistant text. The system prompt now nudges + the model to write any short progress sentence naturally in the user's + language without mentioning internal tool names. + - **Multi-attachment messages no longer trigger N agent replies (#63).** Three coalescing surfaces hardened so a single user action produces ONE agent run regardless of how the platform delivers attachments: diff --git a/docs/05-channels-messaging.md b/docs/05-channels-messaging.md index 04dec281..1159a40f 100644 --- a/docs/05-channels-messaging.md +++ b/docs/05-channels-messaging.md @@ -78,8 +78,8 @@ Normal channel messages pass through the shared inbound debouncer before agent e - `quick_ack.mode = "llm_generated"` uses the existing main-turn `block.reply` event as the natural progress response for non-streaming channel runs. It does not make a separate LLM call. - `quick_ack.templates` are fallback messages. In generated mode, the first template is sent only if no generated `block.reply` arrives before `quick_ack.min_delay_ms`. - `quick_ack.mode = "fixed_template"` preserves the older fixed-template acknowledgement behavior. -- `quick_ack.mode = "off"` disables chat-behavior quick acknowledgement. When explicit `gateway.block_reply` is on, the first generic pre-tool `block.reply` is treated as the immediate acknowledgement and suppressed; deterministic tool announcements still deliver before tool execution because they are part of Intermediate Replies. -- If a model requests tools without assistant text, or with text that does not name the tools, the pipeline emits a safe `tool_announcement` fallback using only sanitized tool names. Tool arguments are never included in this message. +- `quick_ack.mode = "off"` disables chat-behavior quick acknowledgement. When explicit `gateway.block_reply` is on, the first generic pre-tool `block.reply` is treated as the immediate acknowledgement and suppressed; model-generated tool progress still delivers before tool execution because it is part of Intermediate Replies. +- If a model requests tools without assistant text, the pipeline does not emit a synthetic Intermediate Reply. This avoids repetitive server templates; visible progress messages come from the LLM and should match the user's language. - `final_split` splits long final text replies into a bounded number of paragraph messages. - Per-channel `chat_behavior` overrides inherit the gateway config unless a field is explicitly set. diff --git a/docs/project-changelog.md b/docs/project-changelog.md index df610919..889ac293 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -105,17 +105,27 @@ Significant changes, features, and fixes in reverse chronological order. **Fixes** -- Intermediate Replies now guarantees a pre-tool announcement when the model - requests tools without assistant text, or with text that does not name the - tools. The fallback uses sanitized tool names only and is tagged as - `tool_announcement`. +- Intermediate Replies now preserve natural assistant progress text as-is + instead of appending a deterministic tool-name sentence. +- Empty-content tool calls no longer emit synthetic server-generated + Intermediate Reply bubbles. This prevents repeated template messages and + keeps visible progress model-generated. +- Tool-call progress keeps the `tool_announcement` source when the model does + provide assistant text, so Quick acknowledgement suppression still treats it + as explicit progress. +- The full-mode system prompt now tells the LLM to write any short progress + sentence naturally in the user's language and to describe user-visible action + instead of internal tool names. - Quick acknowledgement off still suppresses generic first acknowledgements, - but no longer suppresses explicit tool announcements. + but no longer suppresses explicit model-generated tool progress. **Tests** -- Added pipeline coverage for empty-content tool calls and channel coverage for - `tool_announcement` delivery when Quick acknowledgement is off. +- Added pipeline coverage for empty-content tool calls so they do not emit + template `block.reply` messages. +- Added prompt coverage for natural, user-language progress guidance. +- Kept channel coverage for `tool_announcement` delivery when Quick + acknowledgement is off. --- diff --git a/internal/agent/systemprompt_mode_test.go b/internal/agent/systemprompt_mode_test.go index 034c51f2..201f09e8 100644 --- a/internal/agent/systemprompt_mode_test.go +++ b/internal/agent/systemprompt_mode_test.go @@ -38,6 +38,19 @@ func TestFullModeAllSections(t *testing.T) { } } +func TestFullModeToolCallStyleGuidesNaturalProgress(t *testing.T) { + prompt := BuildSystemPrompt(fullTestConfig()) + for _, want := range []string{ + "short progress sentence before tool calls", + "write it naturally in the user's language", + "not the tool", + } { + if !strings.Contains(prompt, want) { + t.Fatalf("tool call style missing progress guidance %q", want) + } + } +} + // --- Minimal mode tests --- func TestMinimalModeExclusions(t *testing.T) { diff --git a/internal/agent/systemprompt_sections.go b/internal/agent/systemprompt_sections.go index 9069196a..50125604 100644 --- a/internal/agent/systemprompt_sections.go +++ b/internal/agent/systemprompt_sections.go @@ -319,6 +319,7 @@ func buildToolCallStyleSection() []string { "", "Default: call tools without narration. Narrate only for multi-step work or when user asks.", "Never mention tool names or internal mechanics to users.", + "If you include a short progress sentence before tool calls, write it naturally in the user's language and describe the user-visible action, not the tool.", "", "WRONG: \"I searched memory_search and...\" RIGHT: \"I recall you mentioned...\"", "", diff --git a/internal/pipeline/stages_test.go b/internal/pipeline/stages_test.go index 39185e10..05d72be1 100644 --- a/internal/pipeline/stages_test.go +++ b/internal/pipeline/stages_test.go @@ -125,7 +125,7 @@ func TestThinkStage_WithToolCalls_ReturnsContinue(t *testing.T) { } } -func TestThinkStage_EmptyToolCallContent_EmitsToolAnnouncement(t *testing.T) { +func TestThinkStage_EmptyToolCallContent_DoesNotEmitTemplateAnnouncement(t *testing.T) { t.Parallel() var gotContent, gotSource string deps := &PipelineDeps{ @@ -152,18 +152,44 @@ func TestThinkStage_EmptyToolCallContent_EmitsToolAnnouncement(t *testing.T) { if err != nil { t.Fatalf("Execute() error: %v", err) } - if gotSource != protocol.BlockReplySourceToolAnnouncement { - t.Fatalf("block reply source = %q, want %q", gotSource, protocol.BlockReplySourceToolAnnouncement) - } - if !strings.Contains(gotContent, "search secret") { - t.Fatalf("announcement content = %q, want sanitized tool name", gotContent) - } - if strings.Contains(gotContent, "must-not-leak") || strings.Contains(gotContent, "api_key") { - t.Fatalf("announcement leaked tool arguments: %q", gotContent) + if gotContent != "" || gotSource != "" { + t.Fatalf("empty tool-call content emitted template block reply: content=%q source=%q", gotContent, gotSource) } } -func TestThinkStage_GeneratedToolCallContent_AppendsMissingToolAnnouncement(t *testing.T) { +func TestThinkStage_EmptyMultiToolCallContent_DoesNotEmitRawToolNames(t *testing.T) { + t.Parallel() + var gotContent, gotSource string + deps := &PipelineDeps{ + Config: PipelineConfig{MaxIterations: 10, MaxTokens: 1000}, + CallLLM: func(_ context.Context, _ *RunState, _ providers.ChatRequest) (*providers.ChatResponse, error) { + return &providers.ChatResponse{ + FinishReason: "tool_calls", + ToolCalls: []providers.ToolCall{ + {ID: "tc1", Name: "skill_search"}, + {ID: "tc2", Name: "read_file", Arguments: map[string]any{"path": "notes.md"}}, + {ID: "tc3", Name: "web_fetch", Arguments: map[string]any{"url": "https://example.test"}}, + }, + }, nil + }, + EmitBlockReplyWithSource: func(content, source string) { + gotContent = content + gotSource = source + }, + } + stage := NewThinkStage(deps) + state := defaultState() + + err := stage.Execute(context.Background(), state) + if err != nil { + t.Fatalf("Execute() error: %v", err) + } + if gotContent != "" || gotSource != "" { + t.Fatalf("empty multi-tool content emitted template block reply: content=%q source=%q", gotContent, gotSource) + } +} + +func TestThinkStage_GeneratedToolCallContent_PreservesNaturalProgress(t *testing.T) { t.Parallel() var gotContent, gotSource string deps := &PipelineDeps{ @@ -188,7 +214,7 @@ func TestThinkStage_GeneratedToolCallContent_AppendsMissingToolAnnouncement(t *t if err != nil { t.Fatalf("Execute() error: %v", err) } - want := "Tôi sẽ tìm trong kho kỹ năng trước.\n\nTôi sẽ dùng `skill_search` để xử lý bước tiếp theo." + want := "Tôi sẽ tìm trong kho kỹ năng trước." if gotContent != want { t.Fatalf("block reply content = %q", gotContent) } diff --git a/internal/pipeline/think_stage.go b/internal/pipeline/think_stage.go index d6714790..50e365a1 100644 --- a/internal/pipeline/think_stage.go +++ b/internal/pipeline/think_stage.go @@ -6,9 +6,7 @@ import ( "log/slog" "strings" - "github.com/nextlevelbuilder/goclaw/internal/i18n" "github.com/nextlevelbuilder/goclaw/internal/providers" - "github.com/nextlevelbuilder/goclaw/internal/store" "github.com/nextlevelbuilder/goclaw/pkg/protocol" ) @@ -181,17 +179,10 @@ func isEmptyLengthResponse(resp *providers.ChatResponse) bool { len(resp.Images) == 0 } -func (s *ThinkStage) emitToolIterationBlockReply(ctx context.Context, resp *providers.ChatResponse) { +func (s *ThinkStage) emitToolIterationBlockReply(_ context.Context, resp *providers.ChatResponse) { content := resp.Content source := protocol.BlockReplySourceLLMProgress - announcement := defaultToolAnnouncement(ctx, resp.ToolCalls) - if strings.TrimSpace(content) == "" { - content = announcement - source = protocol.BlockReplySourceToolAnnouncement - } else if announcement != "" { - if !contentMentionsAnnouncedTools(content, resp.ToolCalls) { - content = strings.TrimRight(content, "\r\n\t ") + "\n\n" + announcement - } + if len(resp.ToolCalls) > 0 { source = protocol.BlockReplySourceToolAnnouncement } if strings.TrimSpace(content) == "" { @@ -206,69 +197,6 @@ func (s *ThinkStage) emitToolIterationBlockReply(ctx context.Context, resp *prov } } -func defaultToolAnnouncement(ctx context.Context, calls []providers.ToolCall) string { - names := announcedToolNames(calls) - if len(names) == 0 { - return "" - } - list := strings.Join(names, ", ") - locale := store.LocaleFromContext(ctx) - if len(names) == 1 { - return i18n.T(locale, i18n.MsgToolAnnouncementSingle, list) - } - return i18n.T(locale, i18n.MsgToolAnnouncementMulti, list) -} - -func announcedToolNames(calls []providers.ToolCall) []string { - names := sanitizedAnnouncementToolNames(calls) - for i, name := range names { - names[i] = "`" + name + "`" - } - return names -} - -func contentMentionsAnnouncedTools(content string, calls []providers.ToolCall) bool { - names := sanitizedAnnouncementToolNames(calls) - if len(names) == 0 { - return false - } - lowerContent := strings.ToLower(content) - for _, name := range names { - if !strings.Contains(lowerContent, strings.ToLower(name)) { - return false - } - } - return true -} - -func sanitizedAnnouncementToolNames(calls []providers.ToolCall) []string { - names := make([]string, 0, len(calls)) - seen := make(map[string]struct{}, len(calls)) - for _, call := range calls { - name := sanitizeToolAnnouncementName(call.Name) - if name == "" { - continue - } - if _, ok := seen[name]; ok { - continue - } - seen[name] = struct{}{} - names = append(names, name) - } - return names -} - -func sanitizeToolAnnouncementName(name string) string { - name = strings.TrimSpace(name) - name = strings.NewReplacer("`", "", "\n", " ", "\r", " ", "\t", " ").Replace(name) - name = strings.Join(strings.Fields(name), " ") - runes := []rune(name) - if len(runes) > 64 { - name = string(runes[:64]) - } - return name -} - // maybeInjectNudge injects iteration budget warnings at 70% and 90%. func (s *ThinkStage) maybeInjectNudge(state *RunState) { maxIter := s.deps.Config.MaxIterations