mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 00:16:38 +00:00
fix(channels): rely on generated intermediate progress (#143)
* fix(channels): add reasoning delivery modes * fix(telegram): show tool status without placeholder * docs(plans): mark issue 67 behavior plan complete * feat(config): add behavior setting purpose tooltips * fix(channels): rely on generated intermediate progress
This commit is contained in:
1 parent
ed1327db16
commit
23b18fa6cd
7 files changed
+78
-94
No files matched your search
@@ -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:
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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...\"",
|
||||
"",
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user