diff --git a/docs/guides/configuration.md b/docs/guides/configuration.md index 3bec847ba..81f8bcabe 100644 --- a/docs/guides/configuration.md +++ b/docs/guides/configuration.md @@ -136,6 +136,41 @@ Session scope controls how much memory is shared between chats, users, threads, For step-by-step recipes and isolation patterns, see the [Session Guide](session-guide.md). +### Final Turn Render + +`agents.defaults.final_turn_render_mode` controls an experimental final-response render pass for steering-heavy turns. + +When enabled with value `llm`, PicoClaw may do one extra **same-agent** LLM pass after tool execution has already completed: + +- it reuses the accumulated turn context +- it disables tool calling for that final pass +- it asks the same agent to answer the **full accumulated request chain**, not only the latest follow-up + +This is intended for multi-message turns such as: + +- `How much did I eat today?` +- `And yesterday?` +- `And the day before yesterday?` + +Config: + +```json +{ + "agents": { + "defaults": { + "final_turn_render_mode": "llm" + } + } +} +``` + +Notes: + +- omitted or empty: disabled +- `llm`: enable same-agent final no-tools render for eligible steering-heavy turns +- this setting is experimental and is mainly useful when follow-up messages often extend the same in-flight turn +- this is separate from channel/message delivery behavior; it affects only how the final reply text is rendered + ### Routing Routing is configured through `agents.dispatch.rules`. diff --git a/pkg/agent/action_summary.go b/pkg/agent/action_summary.go new file mode 100644 index 000000000..a3928103b --- /dev/null +++ b/pkg/agent/action_summary.go @@ -0,0 +1,203 @@ +package agent + +import ( + "context" + "encoding/json" + "strings" + + "github.com/sipeed/picoclaw/pkg/logger" + "github.com/sipeed/picoclaw/pkg/providers" +) + +type TurnActionRecord struct { + Source string `json:"source"` + Tool string `json:"tool,omitempty"` + Text string `json:"text"` + Error bool `json:"error,omitempty"` +} + +func appendTurnActionRecord( + records []TurnActionRecord, + source, tool, text string, + isError bool, +) []TurnActionRecord { + text = strings.TrimSpace(text) + if text == "" { + return records + } + rec := TurnActionRecord{ + Source: source, + Tool: strings.TrimSpace(tool), + Text: text, + Error: isError, + } + if n := len(records); n > 0 { + prev := records[n-1] + if prev.Source == rec.Source && prev.Tool == rec.Tool && prev.Text == rec.Text && + prev.Error == rec.Error { + return records + } + } + return append(records, rec) +} + +func finalTurnRenderEligible(al *AgentLoop, exec *turnExecution) bool { + if al == nil || exec == nil { + return false + } + if !al.cfg.Agents.Defaults.UseFinalTurnRender() { + return false + } + return exec.sawSteering +} + +func finalTurnRenderModel(ts *turnState, exec *turnExecution) (providers.LLMProvider, string) { + if exec != nil { + if exec.activeProvider != nil && strings.TrimSpace(exec.activeModel) != "" { + return exec.activeProvider, strings.TrimSpace(exec.activeModel) + } + if exec.activeProvider != nil { + return exec.activeProvider, strings.TrimSpace(ts.agent.Model) + } + } + if ts == nil || ts.agent == nil { + return nil, "" + } + return ts.agent.Provider, strings.TrimSpace(ts.agent.Model) +} + +func buildFinalTurnRenderInstruction(exec *turnExecution) string { + var b strings.Builder + b.WriteString("Write the final user-facing reply for this already-completed turn.\n") + b.WriteString("Use the same language and general style as the conversation.\n") + b.WriteString("Do not call tools.\n") + b.WriteString( + "Answer the full accumulated user request across this turn, not only the latest follow-up.\n", + ) + b.WriteString( + "If a later follow-up clearly corrected, narrowed, or replaced an earlier request, follow the latest clarified intent.\n", + ) + b.WriteString( + "If later follow-ups added to earlier requests, include the completed additive results together.\n", + ) + b.WriteString( + "Use only the facts already present in the conversation and tool results. Do not invent missing results.\n", + ) + b.WriteString("Keep the reply concise and natural.\n") + + if exec == nil || len(exec.actionLog) == 0 { + return b.String() + } + + records := make([]TurnActionRecord, 0, len(exec.actionLog)) + for _, rec := range exec.actionLog { + if strings.TrimSpace(rec.Text) == "" { + continue + } + records = append(records, rec) + } + if len(records) == 0 { + return b.String() + } + + raw, err := json.MarshalIndent(records, "", " ") + if err != nil { + return b.String() + } + b.WriteString("\nExplicit user-facing outcomes recorded during the turn:\n") + _, _ = b.Write(raw) + return b.String() +} + +func tryRenderFinalTurnReply( + ctx context.Context, + al *AgentLoop, + ts *turnState, + exec *turnExecution, + fallback string, +) (string, bool) { + fallback = strings.TrimSpace(fallback) + if !finalTurnRenderEligible(al, exec) { + return fallback, false + } + if exec == nil || len(exec.messages) == 0 { + return fallback, false + } + + provider, model := finalTurnRenderModel(ts, exec) + if provider == nil || model == "" { + return fallback, false + } + + messages := append([]providers.Message(nil), exec.messages...) + instruction := buildFinalTurnRenderInstruction(exec) + messages = append(messages, providers.Message{ + Role: "user", + Content: instruction, + }) + + opts := map[string]any{ + "max_tokens": minInt(ts.agent.MaxTokens, 800), + "temperature": 0.2, + "prompt_cache_key": ts.agent.ID, + } + + resp, err := provider.Chat(ctx, messages, nil, model, opts) + if err != nil || resp == nil { + if err != nil { + logger.WarnCF("agent", "Final turn render pass failed", map[string]any{ + "agent_id": ts.agent.ID, + "error": err.Error(), + }) + } + return fallback, false + } + + content := strings.TrimSpace(resp.Content) + if content == "" { + content = strings.TrimSpace(resp.ReasoningContent) + } + if content == "" { + return fallback, false + } + + logger.InfoCF("agent", "Rendered final reply from accumulated turn context", + map[string]any{ + "agent_id": ts.agent.ID, + "session_key": ts.sessionKey, + "messages_count": len(messages), + "action_record_count": len(exec.actionLog), + }) + return content, true +} + +func renderFinalTurnReply( + ctx context.Context, + al *AgentLoop, + ts *turnState, + exec *turnExecution, + fallback string, +) string { + content, ok := tryRenderFinalTurnReply(ctx, al, ts, exec, fallback) + if ok { + return content + } + return strings.TrimSpace(fallback) +} + +func shouldFinalizeAfterToolLoopWithRender(al *AgentLoop, exec *turnExecution) bool { + if !finalTurnRenderEligible(al, exec) { + return false + } + if exec == nil { + return false + } + return !exec.allResponsesHandled +} + +func minInt(a, b int) int { + if a < b { + return a + } + return b +} diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index 7a869ec94..0c934ecac 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -52,7 +52,10 @@ func (f *fakeMediaChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([ return nil, nil } -func (f *fakeMediaChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage) ([]string, error) { +func (f *fakeMediaChannel) SendMedia( + ctx context.Context, + msg bus.OutboundMediaMessage, +) ([]string, error) { f.sentMedia = append(f.sentMedia, msg) return nil, nil } @@ -75,11 +78,17 @@ func (m *recordingChannelManager) SendMessage(ctx context.Context, msg bus.Outbo return nil } -func (m *recordingChannelManager) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage) error { +func (m *recordingChannelManager) SendMedia( + ctx context.Context, + msg bus.OutboundMediaMessage, +) error { return nil } -func (m *recordingChannelManager) SendPlaceholder(ctx context.Context, channel, chatID string) bool { +func (m *recordingChannelManager) SendPlaceholder( + ctx context.Context, + channel, chatID string, +) bool { return false } @@ -261,9 +270,11 @@ func TestPublishResponseIfNeeded_DismissesToolFeedbackWhenMessageToolAlreadySent t.Fatal("expected default agent") } mt := tools.NewMessageTool() - mt.SetSendCallback(func(ctx context.Context, channel, chatID, content, replyToMessageID string) error { - return nil - }) + mt.SetSendCallback( + func(ctx context.Context, channel, chatID, content, replyToMessageID string) error { + return nil + }, + ) defaultAgent.Tools.Register(mt) result := mt.Execute( @@ -277,7 +288,13 @@ func TestPublishResponseIfNeeded_DismissesToolFeedbackWhenMessageToolAlreadySent if result == nil || result.IsError { t.Fatalf("message tool execute failed: %+v", result) } - al.PublishResponseIfNeeded(context.Background(), "telegram", "-100123", "session-1", "final reply") + al.PublishResponseIfNeeded( + context.Background(), + "telegram", + "-100123", + "session-1", + "final reply", + ) if got := cm.dismissed; len(got) != 1 || got[0] != "telegram:-100123" { t.Fatalf("dismissed = %v, want [telegram:-100123]", got) @@ -451,7 +468,10 @@ func TestProcessMessage_BtwCommandRunsWithoutPersistingHistory(t *testing.T) { t.Fatal("provider did not receive any messages") } if len(provider.lastMessages) != 4 { - t.Fatalf("provider messages len = %d, want 4 (system + prior history + user)", len(provider.lastMessages)) + t.Fatalf( + "provider messages len = %d, want 4 (system + prior history + user)", + len(provider.lastMessages), + ) } if !reflect.DeepEqual(provider.lastMessages[1:3], initialHistory) { @@ -511,7 +531,10 @@ func TestProcessMessage_BtwCommandIncludesRequestContextAndMedia(t *testing.T) { if !strings.Contains(systemPrompt, "## Current Session\nChannel: discord\nChat ID: group-1") { t.Fatalf("system prompt missing current session context:\n%s", systemPrompt) } - if !strings.Contains(systemPrompt, "## Current Sender\nCurrent sender: Alice (ID: discord:123)") { + if !strings.Contains( + systemPrompt, + "## Current Sender\nCurrent sender: Alice (ID: discord:123)", + ) { t.Fatalf("system prompt missing current sender context:\n%s", systemPrompt) } @@ -587,7 +610,11 @@ func TestProcessMessage_BtwCommandUsesIsolatedProvider(t *testing.T) { // Verify main session history was NOT modified currentHistory := defaultAgent.Sessions.GetHistory(mainSessionKey) if !reflect.DeepEqual(currentHistory, initialHistory) { - t.Fatalf("main session history was modified:\ngot %#v\nwant %#v", currentHistory, initialHistory) + t.Fatalf( + "main session history was modified:\ngot %#v\nwant %#v", + currentHistory, + initialHistory, + ) } } @@ -1098,7 +1125,9 @@ func TestProcessMessage_MediaToolHandledSkipsFollowUpLLMAndFinalText(t *testing. store := media.NewFileMediaStore() al.SetMediaStore(store) telegramChannel := &fakeMediaChannel{fakeChannel: fakeChannel{id: "rid-telegram"}} - al.SetChannelManager(newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel)) + al.SetChannelManager( + newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel), + ) imagePath := filepath.Join(tmpDir, "screen.png") if err := os.WriteFile(imagePath, []byte("fake screenshot"), 0o644); err != nil { @@ -1120,7 +1149,10 @@ func TestProcessMessage_MediaToolHandledSkipsFollowUpLLMAndFinalText(t *testing. t.Fatalf("processMessage() error = %v", err) } if response != "" { - t.Fatalf("expected no final response when media tool already handled delivery, got %q", response) + t.Fatalf( + "expected no final response when media tool already handled delivery, got %q", + response, + ) } if provider.calls != 1 { t.Fatalf("expected exactly 1 LLM call, got %d", provider.calls) @@ -1133,13 +1165,20 @@ func TestProcessMessage_MediaToolHandledSkipsFollowUpLLMAndFinalText(t *testing. } if len(telegramChannel.sentMedia) != 1 { - t.Fatalf("expected exactly 1 synchronously sent media message, got %d", len(telegramChannel.sentMedia)) + t.Fatalf( + "expected exactly 1 synchronously sent media message, got %d", + len(telegramChannel.sentMedia), + ) } - if telegramChannel.sentMedia[0].Channel != "telegram" || telegramChannel.sentMedia[0].ChatID != "chat1" { + if telegramChannel.sentMedia[0].Channel != "telegram" || + telegramChannel.sentMedia[0].ChatID != "chat1" { t.Fatalf("unexpected sent media target: %+v", telegramChannel.sentMedia[0]) } if len(telegramChannel.sentMedia[0].Parts) != 1 { - t.Fatalf("expected exactly 1 sent media part, got %d", len(telegramChannel.sentMedia[0].Parts)) + t.Fatalf( + "expected exactly 1 sent media part, got %d", + len(telegramChannel.sentMedia[0].Parts), + ) } select { @@ -1161,22 +1200,29 @@ func TestProcessMessage_MediaToolHandledSkipsFollowUpLLMAndFinalText(t *testing. if err != nil { t.Fatalf("resolveMessageRoute() error = %v", err) } - sessionKey := resolveScopeKey(al.allocateRouteSession(route, testInboundMessage(bus.InboundMessage{ - Channel: "telegram", - ChatID: "chat1", - SenderID: "user1", - Content: "take a screenshot of the screen and send it to me", - })).SessionKey, "") + sessionKey := resolveScopeKey( + al.allocateRouteSession(route, testInboundMessage(bus.InboundMessage{ + Channel: "telegram", + ChatID: "chat1", + SenderID: "user1", + Content: "take a screenshot of the screen and send it to me", + })).SessionKey, + "", + ) history := defaultAgent.Sessions.GetHistory(sessionKey) if len(history) == 0 { t.Fatal("expected session history to be saved") } last := history[len(history)-1] - if last.Role != "assistant" || last.Content != "Requested output delivered via tool attachment." { + if last.Role != "assistant" || + last.Content != "Requested output delivered via tool attachment." { t.Fatalf("expected handled assistant summary in history, got %+v", last) } if len(last.Attachments) != 1 { - t.Fatalf("expected handled assistant summary attachments in history, got %+v", last.Attachments) + t.Fatalf( + "expected handled assistant summary attachments in history, got %+v", + last.Attachments, + ) } } @@ -1200,7 +1246,9 @@ func TestProcessMessage_HandledToolProcessesQueuedSteeringBeforeReturning(t *tes store := media.NewFileMediaStore() al.SetMediaStore(store) telegramChannel := &fakeMediaChannel{fakeChannel: fakeChannel{id: "rid-telegram"}} - al.SetChannelManager(newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel)) + al.SetChannelManager( + newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel), + ) imagePath := filepath.Join(tmpDir, "screen-steering.png") if err := os.WriteFile(imagePath, []byte("fake screenshot"), 0o644); err != nil { @@ -1229,7 +1277,10 @@ func TestProcessMessage_HandledToolProcessesQueuedSteeringBeforeReturning(t *tes t.Fatalf("expected 2 LLM calls after queued steering, got %d", provider.calls) } if len(telegramChannel.sentMedia) != 1 { - t.Fatalf("expected exactly 1 synchronously sent media message, got %d", len(telegramChannel.sentMedia)) + t.Fatalf( + "expected exactly 1 synchronously sent media message, got %d", + len(telegramChannel.sentMedia), + ) } } @@ -1248,7 +1299,9 @@ func TestRunAgentLoop_ResponseHandledToolPublishesForUserWhenSendResponseDisable store := media.NewFileMediaStore() al.SetMediaStore(store) telegramChannel := &fakeMediaChannel{fakeChannel: fakeChannel{id: "rid-telegram"}} - al.SetChannelManager(newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel)) + al.SetChannelManager( + newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel), + ) al.RegisterTool(&handledUserTool{}) defaultAgent := al.registry.GetDefaultAgent() @@ -1298,10 +1351,17 @@ func TestRunAgentLoop_ResponseHandledToolPublishesForUserWhenSendResponseDisable t.Fatalf("unexpected sent text message: %+v", telegramChannel.sentMessages[0]) } if telegramChannel.sentMessages[0].AgentID != defaultAgent.ID { - t.Fatalf("sent text agent_id = %q, want %q", telegramChannel.sentMessages[0].AgentID, defaultAgent.ID) + t.Fatalf( + "sent text agent_id = %q, want %q", + telegramChannel.sentMessages[0].AgentID, + defaultAgent.ID, + ) } if telegramChannel.sentMessages[0].SessionKey != "session-1" { - t.Fatalf("sent text session_key = %q, want session-1", telegramChannel.sentMessages[0].SessionKey) + t.Fatalf( + "sent text session_key = %q, want session-1", + telegramChannel.sentMessages[0].SessionKey, + ) } if telegramChannel.sentMessages[0].Scope == nil || telegramChannel.sentMessages[0].Scope.Values["chat"] != "direct:chat1" { @@ -1505,7 +1565,9 @@ func TestProcessMessage_MediaArtifactCanBeForwardedBySendFile(t *testing.T) { store := media.NewFileMediaStore() al.SetMediaStore(store) telegramChannel := &fakeMediaChannel{fakeChannel: fakeChannel{id: "rid-telegram"}} - al.SetChannelManager(newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel)) + al.SetChannelManager( + newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel), + ) mediaDir := media.TempDir() if err := os.MkdirAll(mediaDir, 0o700); err != nil { @@ -1538,13 +1600,20 @@ func TestProcessMessage_MediaArtifactCanBeForwardedBySendFile(t *testing.T) { } if len(telegramChannel.sentMedia) != 1 { - t.Fatalf("expected exactly 1 synchronously sent media message, got %d", len(telegramChannel.sentMedia)) + t.Fatalf( + "expected exactly 1 synchronously sent media message, got %d", + len(telegramChannel.sentMedia), + ) } - if telegramChannel.sentMedia[0].Channel != "telegram" || telegramChannel.sentMedia[0].ChatID != "chat1" { + if telegramChannel.sentMedia[0].Channel != "telegram" || + telegramChannel.sentMedia[0].ChatID != "chat1" { t.Fatalf("unexpected sent media target: %+v", telegramChannel.sentMedia[0]) } if len(telegramChannel.sentMedia[0].Parts) != 1 { - t.Fatalf("expected exactly 1 sent media part, got %d", len(telegramChannel.sentMedia[0].Parts)) + t.Fatalf( + "expected exactly 1 sent media part, got %d", + len(telegramChannel.sentMedia[0].Parts), + ) } select { @@ -2014,7 +2083,10 @@ func TestToolFeedbackExplanationFromResponse_UsesExplicitToolCallExtraContent(t got := toolFeedbackExplanationFromResponse(response, messages) if got != "Read README.md first to confirm the current project structure." { - t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want explicit tool feedback explanation", got) + t.Fatalf( + "toolFeedbackExplanationFromResponse() = %q, want explicit tool feedback explanation", + got, + ) } } @@ -2042,10 +2114,16 @@ func TestToolFeedbackExplanationForToolCall_PrefersToolSpecificExtraContent(t *t got1 := toolFeedbackExplanationForToolCall(response, response.ToolCalls[0], nil) got2 := toolFeedbackExplanationForToolCall(response, response.ToolCalls[1], nil) if got1 != "Read README.md first." { - t.Fatalf("toolFeedbackExplanationForToolCall() first = %q, want tool-specific explanation", got1) + t.Fatalf( + "toolFeedbackExplanationForToolCall() first = %q, want tool-specific explanation", + got1, + ) } if got2 != "Update config example after reading it." { - t.Fatalf("toolFeedbackExplanationForToolCall() second = %q, want tool-specific explanation", got2) + t.Fatalf( + "toolFeedbackExplanationForToolCall() second = %q, want tool-specific explanation", + got2, + ) } } @@ -2091,7 +2169,10 @@ func TestToolFeedbackExplanationFromResponse_DoesNotUseReasoningContent(t *testi got := toolFeedbackExplanationFromResponse(response, messages) want := utils.ToolFeedbackContinuationHint + ": Inspect README.md and update the config example." if got != want { - t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want latest user content fallback", got) + t.Fatalf( + "toolFeedbackExplanationFromResponse() = %q, want latest user content fallback", + got, + ) } } @@ -2343,7 +2424,10 @@ func (m *handledMediaWithSteeringTool) Parameters() map[string]any { } } -func (m *handledMediaWithSteeringTool) Execute(ctx context.Context, args map[string]any) *tools.ToolResult { +func (m *handledMediaWithSteeringTool) Execute( + ctx context.Context, + args map[string]any, +) *tools.ToolResult { if err := m.loop.Steer(providers.Message{Role: "user", Content: "what about this instead?"}); err != nil { return tools.ErrorResult(err.Error()).WithError(err) } @@ -2496,7 +2580,11 @@ func newStrictChatCompletionTestServer( })) } -func (h testHelper) executeAndGetResponse(tb testing.TB, ctx context.Context, msg bus.InboundMessage) string { +func (h testHelper) executeAndGetResponse( + tb testing.TB, + ctx context.Context, + msg bus.InboundMessage, +) string { // Use a short timeout to avoid hanging timeoutCtx, cancel := context.WithTimeout(ctx, responseTimeout) defer cancel() @@ -2652,7 +2740,10 @@ func TestProcessMessage_CommandOutcomes(t *testing.T) { t.Fatalf("unexpected /foo reply: %q", fooResp) } if provider.calls != 1 { - t.Fatalf("LLM should be called exactly once after /foo passthrough, calls=%d", provider.calls) + t.Fatalf( + "LLM should be called exactly once after /foo passthrough, calls=%d", + provider.calls, + ) } newResp := helper.executeAndGetResponse(t, context.Background(), bus.InboundMessage{ @@ -2857,7 +2948,10 @@ func TestProcessMessage_SwitchModelRejectsUnknownAlias(t *testing.T) { } if provider.calls != 0 { - t.Fatalf("LLM should not be called for rejected /switch and /show, calls=%d", provider.calls) + t.Fatalf( + "LLM should not be called for rejected /switch and /show, calls=%d", + provider.calls, + ) } } @@ -2875,7 +2969,13 @@ func TestProcessMessage_SwitchModelRoutesSubsequentRequestsToSelectedProvider(t remoteCalls := 0 remoteModel := "" - remoteServer := newChatCompletionTestServer(t, "remote", "remote reply", &remoteCalls, &remoteModel) + remoteServer := newChatCompletionTestServer( + t, + "remote", + "remote reply", + &remoteCalls, + &remoteModel, + ) defer remoteServer.Close() cfg := &config.Config{ @@ -3055,18 +3155,20 @@ func TestProcessMessage_FallbackUsesPerCandidateProvider(t *testing.T) { workspace := t.TempDir() primaryCalls := 0 - primaryServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - primaryCalls++ - // Return 429 so FallbackChain classifies this as retriable and moves on. - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusTooManyRequests) - _ = json.NewEncoder(w).Encode(map[string]any{ - "error": map[string]any{ - "message": "rate limit exceeded", - "type": "rate_limit_error", - }, - }) - })) + primaryServer := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + primaryCalls++ + // Return 429 so FallbackChain classifies this as retriable and moves on. + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusTooManyRequests) + _ = json.NewEncoder(w).Encode(map[string]any{ + "error": map[string]any{ + "message": "rate limit exceeded", + "type": "rate_limit_error", + }, + }) + }), + ) defer primaryServer.Close() fallbackCalls := 0 @@ -3139,23 +3241,28 @@ func TestProcessMessage_FallbackUsesActiveProviderWhenCandidateNotRegistered(t * // Both the primary and the unregistered fallback share this server // (same api_base) so activeProvider routes both calls here. callCount := 0 - primaryServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - callCount++ - w.Header().Set("Content-Type", "application/json") - if callCount == 1 { - w.WriteHeader(http.StatusTooManyRequests) + primaryServer := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + w.Header().Set("Content-Type", "application/json") + if callCount == 1 { + w.WriteHeader(http.StatusTooManyRequests) + _ = json.NewEncoder(w).Encode(map[string]any{ + "error": map[string]any{"message": "rate limit", "type": "rate_limit_error"}, + }) + return + } + // Second call (fallback via activeProvider) succeeds. _ = json.NewEncoder(w).Encode(map[string]any{ - "error": map[string]any{"message": "rate limit", "type": "rate_limit_error"}, + "choices": []map[string]any{ + { + "message": map[string]any{"content": "active provider reply"}, + "finish_reason": "stop", + }, + }, }) - return - } - // Second call (fallback via activeProvider) succeeds. - _ = json.NewEncoder(w).Encode(map[string]any{ - "choices": []map[string]any{ - {"message": map[string]any{"content": "active provider reply"}, "finish_reason": "stop"}, - }, - }) - })) + }), + ) defer primaryServer.Close() cfg := &config.Config{ @@ -3199,7 +3306,10 @@ func TestProcessMessage_FallbackUsesActiveProviderWhenCandidateNotRegistered(t * t.Fatalf("response = %q, want %q", resp, "active provider reply") } if callCount < 2 { - t.Fatalf("primary server calls = %d, want >= 2 (one 429 + one success via activeProvider)", callCount) + t.Fatalf( + "primary server calls = %d, want >= 2 (one 429 + one success via activeProvider)", + callCount, + ) } } @@ -3338,7 +3448,9 @@ func TestAgentLoop_ContextExhaustionRetry(t *testing.T) { msgBus := bus.NewMessageBus() // Create a provider that fails once with a context error - contextErr := fmt.Errorf("InvalidParameter: Total tokens of image and text exceed max message tokens") + contextErr := fmt.Errorf( + "InvalidParameter: Total tokens of image and text exceed max message tokens", + ) provider := &failFirstMockProvider{ failures: 1, failError: contextErr, @@ -3482,7 +3594,11 @@ func TestAgentLoop_VisionUnsupportedErrorStripsSessionMedia(t *testing.T) { t.Fatalf("response = %q, want %q", resp, "ok") } if provider.calls != 2 { - t.Fatalf("calls = %d, want %d (fail with media, then retry without media)", provider.calls, 2) + t.Fatalf( + "calls = %d, want %d (fail with media, then retry without media)", + provider.calls, + 2, + ) } if !slices.Equal(provider.mediaSeen, []bool{true, false}) { t.Fatalf("mediaSeen = %v, want %v", provider.mediaSeen, []bool{true, false}) @@ -3549,7 +3665,13 @@ func TestAgentLoop_EmptyModelResponseUsesAccurateFallback(t *testing.T) { provider := &simpleMockProvider{response: ""} al := NewAgentLoop(cfg, msgBus, provider) - response, err := al.ProcessDirectWithChannel(context.Background(), "hello", "empty-response", "test", "chat1") + response, err := al.ProcessDirectWithChannel( + context.Background(), + "hello", + "empty-response", + "test", + "chat1", + ) if err != nil { t.Fatalf("ProcessDirectWithChannel failed: %v", err) } @@ -3581,7 +3703,13 @@ func TestAgentLoop_ToolLimitUsesDedicatedFallback(t *testing.T) { al := NewAgentLoop(cfg, msgBus, provider) al.RegisterTool(&toolLimitTestTool{}) - response, err := al.ProcessDirectWithChannel(context.Background(), "hello", "tool-limit", "test", "chat1") + response, err := al.ProcessDirectWithChannel( + context.Background(), + "hello", + "tool-limit", + "test", + "chat1", + ) if err != nil { t.Fatalf("ProcessDirectWithChannel failed: %v", err) } @@ -3598,11 +3726,13 @@ func TestAgentLoop_ToolLimitUsesDedicatedFallback(t *testing.T) { ChatType: "direct", SenderID: "cron", }) - history := defaultAgent.Sessions.GetHistory(al.allocateRouteSession(route, testInboundMessage(bus.InboundMessage{ - Channel: "test", - SenderID: "cron", - ChatID: "chat1", - })).SessionKey) + history := defaultAgent.Sessions.GetHistory( + al.allocateRouteSession(route, testInboundMessage(bus.InboundMessage{ + Channel: "test", + SenderID: "cron", + ChatID: "chat1", + })).SessionKey, + ) if len(history) != 4 { t.Fatalf("history len = %d, want 4", len(history)) } @@ -3900,7 +4030,9 @@ func TestHandleReasoning(t *testing.T) { break } if msg.Content == "should timeout" { - t.Fatal("expected reasoning message to be dropped when bus is full, but it was published") + t.Fatal( + "expected reasoning message to be dropped when bus is full, but it was published", + ) } } } @@ -4015,7 +4147,11 @@ func TestProcessMessage_PicoPublishesReasoningAsThoughtMessage(t *testing.T) { } if thoughtMsg.Channel != "pico" || thoughtMsg.ChatID != "pico:test-session" { - t.Fatalf("thought message route = %s/%s, want pico/pico:test-session", thoughtMsg.Channel, thoughtMsg.ChatID) + t.Fatalf( + "thought message route = %s/%s, want pico/pico:test-session", + thoughtMsg.Channel, + thoughtMsg.ChatID, + ) } if thoughtMsg.Context.Raw[metadataKeyMessageKind] != messageKindThought { t.Fatalf( @@ -4057,7 +4193,12 @@ func TestProcessHeartbeat_DoesNotPublishToolFeedback(t *testing.T) { provider := &toolFeedbackProvider{filePath: heartbeatFile} al := NewAgentLoop(cfg, msgBus, provider) - response, err := al.ProcessHeartbeat(context.Background(), "check heartbeat tasks", "telegram", "chat-1") + response, err := al.ProcessHeartbeat( + context.Background(), + "check heartbeat tasks", + "telegram", + "chat-1", + ) if err != nil { t.Fatalf("ProcessHeartbeat() error = %v", err) } @@ -4132,10 +4273,16 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) { t.Fatalf("tool feedback content = %q, want read_file summary", outbound.Content) } if !strings.Contains(outbound.Content, utils.ToolFeedbackContinuationHint) { - t.Fatalf("tool feedback content = %q, want continuation hint fallback", outbound.Content) + 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) + 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) @@ -4144,7 +4291,10 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) { 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) + 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) @@ -4152,7 +4302,8 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) { if outbound.SessionKey == "" { t.Fatal("expected tool feedback to carry session_key") } - if outbound.Scope == nil || outbound.Scope.AgentID != "main" || outbound.Scope.Channel != "telegram" { + if outbound.Scope == nil || outbound.Scope.AgentID != "main" || + outbound.Scope.Channel != "telegram" { t.Fatalf("expected tool feedback scope, got %+v", outbound.Scope) } case <-time.After(2 * time.Second): @@ -4211,7 +4362,11 @@ func TestProcessMessage_PersistsReasoningContentInSessionHistory(t *testing.T) { t.Fatalf("last message content = %q, want %q", last.Content, "final answer") } if last.ReasoningContent != "thinking trace" { - t.Fatalf("last message reasoning_content = %q, want %q", last.ReasoningContent, "thinking trace") + t.Fatalf( + "last message reasoning_content = %q, want %q", + last.ReasoningContent, + "thinking trace", + ) } } @@ -4268,16 +4423,29 @@ func TestProcessMessage_PersistsReasoningToolResponseAsSingleAssistantRecord(t * t.Fatal("expected assistant history record with tool_calls") } if assistantWithToolCall.Content != "I'll inspect that file now." { - t.Fatalf("assistant content = %q, want %q", assistantWithToolCall.Content, "I'll inspect that file now.") + t.Fatalf( + "assistant content = %q, want %q", + assistantWithToolCall.Content, + "I'll inspect that file now.", + ) } if assistantWithToolCall.ReasoningContent != "Read the file before answering." { - t.Fatalf("assistant reasoning_content = %q, want preserved", assistantWithToolCall.ReasoningContent) + t.Fatalf( + "assistant reasoning_content = %q, want preserved", + assistantWithToolCall.ReasoningContent, + ) } if len(assistantWithToolCall.ToolCalls) != 1 { - t.Fatalf("assistant tool calls = %+v, want single read_file tool", assistantWithToolCall.ToolCalls) + t.Fatalf( + "assistant tool calls = %+v, want single read_file tool", + assistantWithToolCall.ToolCalls, + ) } if got := providers.NormalizeToolCall(assistantWithToolCall.ToolCalls[0]).Name; got != "read_file" { - t.Fatalf("assistant tool calls = %+v, want single read_file tool", assistantWithToolCall.ToolCalls) + t.Fatalf( + "assistant tool calls = %+v, want single read_file tool", + assistantWithToolCall.ToolCalls, + ) } sessionDir := filepath.Join(tmpDir, "sessions") @@ -4317,7 +4485,8 @@ func TestProcessMessage_PersistsReasoningToolResponseAsSingleAssistantRecord(t * if msg.Role != "assistant" { continue } - if msg.Content == "I'll inspect that file now." || msg.ReasoningContent == "Read the file before answering." { + if msg.Content == "I'll inspect that file now." || + msg.ReasoningContent == "Read the file before answering." { matchingRecords++ toolName := "" if len(msg.ToolCalls) == 1 { @@ -4327,12 +4496,18 @@ func TestProcessMessage_PersistsReasoningToolResponseAsSingleAssistantRecord(t * msg.ReasoningContent != "Read the file before answering." || len(msg.ToolCalls) != 1 || toolName != "read_file" { - t.Fatalf("assistant jsonl record = %+v, want content+reasoning+tool_calls in one line", msg) + t.Fatalf( + "assistant jsonl record = %+v, want content+reasoning+tool_calls in one line", + msg, + ) } } } if matchingRecords != 1 { - t.Fatalf("matching assistant jsonl records = %d, want exactly 1 canonical assistant record", matchingRecords) + t.Fatalf( + "matching assistant jsonl records = %d, want exactly 1 canonical assistant record", + matchingRecords, + ) } } @@ -4387,10 +4562,16 @@ func TestProcessMessage_DoesNotLeakReasoningContentInToolFeedback(t *testing.T) t.Fatalf("tool feedback content = %q, want read_file summary", outbound.Content) } if !strings.Contains(outbound.Content, utils.ToolFeedbackContinuationHint) { - t.Fatalf("tool feedback content = %q, want continuation hint fallback", outbound.Content) + t.Fatalf( + "tool feedback content = %q, want continuation hint fallback", + outbound.Content, + ) } if !strings.Contains(outbound.Content, "check reasoning fallback") { - t.Fatalf("tool feedback content = %q, want current user intent fallback", outbound.Content) + 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) @@ -4399,7 +4580,10 @@ func TestProcessMessage_DoesNotLeakReasoningContentInToolFeedback(t *testing.T) 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) + t.Fatalf( + "tool feedback content = %q, should not leak hidden reasoning", + outbound.Content, + ) } case <-time.After(2 * time.Second): t.Fatal("expected outbound tool feedback without leaking reasoning") @@ -4454,7 +4638,11 @@ func assertToolFeedbackNotPublishedWhenDisabled(t *testing.T, channel string) { select { case outbound := <-msgBus.OutboundChan(): - t.Fatalf("expected no outbound tool feedback for %s when disabled, got %+v", channel, outbound) + t.Fatalf( + "expected no outbound tool feedback for %s when disabled, got %+v", + channel, + outbound, + ) case <-time.After(200 * time.Millisecond): } } @@ -4567,13 +4755,20 @@ func TestRun_PicoPublishesAssistantContentDuringToolCallsWithoutFinalDuplicate(t } if outputs[0].Content != "intermediate model text" { - t.Fatalf("first outbound content = %q, want %q", outputs[0].Content, "intermediate model text") + t.Fatalf( + "first outbound content = %q, want %q", + outputs[0].Content, + "intermediate model text", + ) } if outputs[1].Context.Raw[metadataKeyMessageKind] != messageKindToolCalls { t.Fatalf("second outbound = %+v, want tool_calls message", outputs[1]) } if !strings.Contains(outputs[1].Context.Raw[metadataKeyToolCalls], "tool_limit_test_tool") { - t.Fatalf("second outbound tool_calls = %q, want tool name", outputs[1].Context.Raw[metadataKeyToolCalls]) + t.Fatalf( + "second outbound tool_calls = %q, want tool name", + outputs[1].Context.Raw[metadataKeyToolCalls], + ) } if outputs[2].Content != "final model text" { t.Fatalf("third outbound content = %q, want %q", outputs[2].Content, "final model text") @@ -4709,7 +4904,10 @@ func TestRun_PicoToolFeedbackSuppressesDuplicateInterimAssistantContent(t *testi t.Fatalf("first outbound content = %q, want empty tool_calls content", outputs[0].Content) } if !strings.Contains(outputs[0].Context.Raw[metadataKeyToolCalls], "tool_limit_test_tool") { - t.Fatalf("first outbound tool_calls = %q, want tool name", outputs[0].Context.Raw[metadataKeyToolCalls]) + t.Fatalf( + "first outbound tool_calls = %q, want tool name", + outputs[0].Context.Raw[metadataKeyToolCalls], + ) } if outputs[1].Content != "final model text" { t.Fatalf("second outbound content = %q, want %q", outputs[1].Content, "final model text") @@ -4858,7 +5056,8 @@ func TestResolveMediaRefs_MultiToolCallPreservesOrdering(t *testing.T) { if result[3].Role != "user" { t.Fatalf("result[3] expected user, got %q", result[3].Role) } - if len(result[3].Media) != 1 || !strings.HasPrefix(result[3].Media[0], "data:image/png;base64,") { + if len(result[3].Media) != 1 || + !strings.HasPrefix(result[3].Media[0], "data:image/png;base64,") { t.Fatal("expected synthetic user message to contain base64 image") } } @@ -5335,8 +5534,14 @@ func TestProcessMessage_ContextOverflowRecovery(t *testing.T) { agent := al.GetRegistry().GetDefaultAgent() for i := 0; i < 5; i++ { - agent.Sessions.AddFullMessage(sessionKey, providers.Message{Role: "user", Content: "heavy message"}) - agent.Sessions.AddFullMessage(sessionKey, providers.Message{Role: "assistant", Content: "response"}) + agent.Sessions.AddFullMessage( + sessionKey, + providers.Message{Role: "user", Content: "heavy message"}, + ) + agent.Sessions.AddFullMessage( + sessionKey, + providers.Message{Role: "assistant", Content: "response"}, + ) } response, err := al.processMessage(context.Background(), testInboundMessage(bus.InboundMessage{ @@ -5661,3 +5866,294 @@ func (p *concurrentMockProvider) Chat( func (p *concurrentMockProvider) GetDefaultModel() string { return "test-model" } + +type activitySummaryWithSteeringProvider struct { + calls int +} + +func (m *activitySummaryWithSteeringProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + m.calls++ + if len(messages) > 0 && tools == nil { + last := messages[len(messages)-1] + if last.Role == "user" && strings.Contains(last.Content, "already-completed turn") { + return &providers.LLMResponse{ + Content: "Записал.\n\nДобавил активности:\n- yoga — 30 мин\n- squats — 20 повторений", + }, nil + } + } + if m.calls == 1 { + return &providers.LLMResponse{ + Content: "Записал yoga — 30 мин.", + ToolCalls: []providers.ToolCall{{ + ID: "call_activity_steering", + Type: "function", + Name: "activity_with_steering_tool", + Arguments: map[string]any{}, + }}, + }, nil + } + + for _, msg := range messages { + if msg.Role == "user" && msg.Content == "и еще 20 приседаний" { + return &providers.LLMResponse{Content: "Записал squats — 20 повторений."}, nil + } + } + + return nil, fmt.Errorf("provider did not receive steering or synthesis prompt") +} + +func (m *activitySummaryWithSteeringProvider) GetDefaultModel() string { + return "activity-summary-with-steering-model" +} + +type activityWithSteeringTool struct { + loop *AgentLoop +} + +func (m *activityWithSteeringTool) Name() string { return "activity_with_steering_tool" } +func (m *activityWithSteeringTool) Description() string { + return "Queues a follow-up steering message after recording an activity" +} + +func (m *activityWithSteeringTool) Parameters() map[string]any { + return map[string]any{ + "type": "object", + "properties": map[string]any{}, + } +} + +func (m *activityWithSteeringTool) Execute( + ctx context.Context, + args map[string]any, +) *tools.ToolResult { + if err := m.loop.Steer(providers.Message{Role: "user", Content: "и еще 20 приседаний"}); err != nil { + return tools.ErrorResult(err.Error()).WithError(err) + } + return &tools.ToolResult{ + ForLLM: "activity recorded", + ForUser: "Записал yoga — 30 мин.", + } +} + +func newFinalTurnRenderTestLoop( + t *testing.T, + provider providers.LLMProvider, + toolFactory func(*AgentLoop) tools.Tool, +) *AgentLoop { + t.Helper() + + tmpDir := t.TempDir() + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + ModelName: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + FinalTurnRenderMode: "llm", + }, + }, + } + + msgBus := bus.NewMessageBus() + al := NewAgentLoop(cfg, msgBus, provider) + al.RegisterTool(toolFactory(al)) + return al +} + +func TestProcessMessage_FinalActionSummarySynthesizesAcrossSteering(t *testing.T) { + provider := &activitySummaryWithSteeringProvider{} + al := newFinalTurnRenderTestLoop(t, provider, func(al *AgentLoop) tools.Tool { + return &activityWithSteeringTool{loop: al} + }) + + response, err := al.processMessage(context.Background(), testInboundMessage(bus.InboundMessage{ + Channel: "telegram", + ChatID: "chat1", + SenderID: "user1", + Content: "я позанимался йогой 30 минут", + })) + if err != nil { + t.Fatalf("processMessage() error = %v", err) + } + + want := "Записал.\n\nДобавил активности:\n- yoga — 30 мин\n- squats — 20 повторений" + if response != want { + t.Fatalf("response = %q, want %q", response, want) + } + if provider.calls != 3 { + t.Fatalf("expected 3 LLM calls including final synthesis, got %d", provider.calls) + } +} + +type daySummaryAcrossSteeringProvider struct { + calls int +} + +func (p *daySummaryAcrossSteeringProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + p.calls++ + if len(messages) > 0 && tools == nil { + last := messages[len(messages)-1] + if last.Role == "user" && strings.Contains(last.Content, "already-completed turn") { + full := flattenMessageContents(messages) + if !strings.Contains(full, "today total: 428 kcal") || + !strings.Contains(full, "yesterday total: 1561 kcal") || + !strings.Contains(full, "day-before total: 1455 kcal") { + return nil, fmt.Errorf("final render pass missing accumulated tool results") + } + return &providers.LLMResponse{ + Content: "Коротко по итогам:\n- сегодня — 428 ккал\n- вчера — 1561 ккал\n- позавчера — 1455 ккал", + }, nil + } + } + + switch p.calls { + case 1: + return &providers.LLMResponse{ + Content: "", + ToolCalls: []providers.ToolCall{{ + ID: "call_day_today", + Type: "function", + Name: "day_summary_with_steering_tool", + Arguments: map[string]any{ + "day": "today", + }, + }}, + }, nil + case 2: + if !messageExists(messages, "А за вчера?") { + return nil, fmt.Errorf("provider did not receive yesterday steering") + } + return &providers.LLMResponse{ + Content: "", + ToolCalls: []providers.ToolCall{{ + ID: "call_day_yesterday", + Type: "function", + Name: "day_summary_with_steering_tool", + Arguments: map[string]any{ + "day": "yesterday", + }, + }}, + }, nil + case 3: + if !messageExists(messages, "И за позавчера?") { + return nil, fmt.Errorf("provider did not receive day-before steering") + } + return &providers.LLMResponse{ + Content: "", + ToolCalls: []providers.ToolCall{{ + ID: "call_day_before", + Type: "function", + Name: "day_summary_with_steering_tool", + Arguments: map[string]any{ + "day": "day_before", + }, + }}, + }, nil + default: + return nil, fmt.Errorf("unexpected provider call count %d", p.calls) + } +} + +func (p *daySummaryAcrossSteeringProvider) GetDefaultModel() string { + return "day-summary-across-steering-model" +} + +type daySummaryWithSteeringTool struct { + loop *AgentLoop +} + +func (t *daySummaryWithSteeringTool) Name() string { return "day_summary_with_steering_tool" } +func (t *daySummaryWithSteeringTool) Description() string { + return "Fetches one day summary and queues the next follow-up question" +} + +func (t *daySummaryWithSteeringTool) Parameters() map[string]any { + return map[string]any{ + "type": "object", + "properties": map[string]any{ + "day": map[string]any{"type": "string"}, + }, + "required": []string{"day"}, + } +} + +func (t *daySummaryWithSteeringTool) Execute( + ctx context.Context, + args map[string]any, +) *tools.ToolResult { + day, _ := args["day"].(string) + switch day { + case "today": + if err := t.loop.Steer(providers.Message{Role: "user", Content: "А за вчера?"}); err != nil { + return tools.ErrorResult(err.Error()).WithError(err) + } + return &tools.ToolResult{ForLLM: "today total: 428 kcal"} + case "yesterday": + if err := t.loop.Steer(providers.Message{Role: "user", Content: "И за позавчера?"}); err != nil { + return tools.ErrorResult(err.Error()).WithError(err) + } + return &tools.ToolResult{ForLLM: "yesterday total: 1561 kcal"} + case "day_before": + return &tools.ToolResult{ForLLM: "day-before total: 1455 kcal"} + default: + return tools.ErrorResult("unknown day") + } +} + +func TestProcessMessage_FinalActionSummaryRendersAcrossInformationalSteering(t *testing.T) { + provider := &daySummaryAcrossSteeringProvider{} + al := newFinalTurnRenderTestLoop(t, provider, func(al *AgentLoop) tools.Tool { + return &daySummaryWithSteeringTool{loop: al} + }) + + response, err := al.processMessage(context.Background(), testInboundMessage(bus.InboundMessage{ + Channel: "telegram", + ChatID: "chat1", + SenderID: "user1", + Content: "А сколько я за сегодня съел?", + })) + if err != nil { + t.Fatalf("processMessage() error = %v", err) + } + + want := "Коротко по итогам:\n- сегодня — 428 ккал\n- вчера — 1561 ккал\n- позавчера — 1455 ккал" + if response != want { + t.Fatalf("response = %q, want %q", response, want) + } + if provider.calls != 4 { + t.Fatalf("expected 4 LLM calls including final render, got %d", provider.calls) + } +} + +func messageExists(messages []providers.Message, want string) bool { + for _, msg := range messages { + if msg.Role == "user" && msg.Content == want { + return true + } + } + return false +} + +func flattenMessageContents(messages []providers.Message) string { + parts := make([]string, 0, len(messages)) + for _, msg := range messages { + if strings.TrimSpace(msg.Content) == "" { + continue + } + parts = append(parts, msg.Content) + } + return strings.Join(parts, "\n") +} diff --git a/pkg/agent/pipeline_execute.go b/pkg/agent/pipeline_execute.go index 0f71c7432..527748669 100644 --- a/pkg/agent/pipeline_execute.go +++ b/pkg/agent/pipeline_execute.go @@ -6,6 +6,7 @@ import ( "context" "encoding/json" "fmt" + "strings" "time" "github.com/sipeed/picoclaw/pkg/bus" @@ -484,6 +485,10 @@ toolLoop: toolResult = tools.ErrorResult("hook returned nil tool result") } + if toolSummary := strings.TrimSpace(toolResult.ForUser); toolSummary != "" { + exec.actionLog = appendTurnActionRecord(exec.actionLog, "tool_result", toolName, toolSummary, toolResult.IsError) + } + if len(toolResult.Media) > 0 && toolResult.ResponseHandled { parts := make([]bus.MediaPart, 0, len(toolResult.Media)) for _, ref := range toolResult.Media { @@ -678,6 +683,16 @@ toolLoop: } // No pending steering: finalize or break depending on allResponsesHandled + if shouldFinalizeAfterToolLoopWithRender(al, exec) { + logger.InfoCF("agent", "Tool loop completed; rendering terminal reply from accumulated turn context", + map[string]any{ + "agent_id": ts.agent.ID, + "iteration": iteration, + "tool_count": len(normalizedToolCalls), + }) + return ToolControlFinalize + } + if exec.allResponsesHandled { summaryMsg := providers.Message{ Role: "assistant", diff --git a/pkg/agent/pipeline_llm.go b/pkg/agent/pipeline_llm.go index 496fcd7e4..fb9378dd2 100644 --- a/pkg/agent/pipeline_llm.go +++ b/pkg/agent/pipeline_llm.go @@ -474,7 +474,9 @@ func (p *Pipeline) CallLLM( if responseContent == "" && exec.response.ReasoningContent != "" && ts.channel != "pico" { responseContent = exec.response.ReasoningContent } + exec.actionLog = appendTurnActionRecord(exec.actionLog, "assistant_direct", "", responseContent, false) if steerMsgs := al.dequeueSteeringMessagesForScope(ts.sessionKey); len(steerMsgs) > 0 { + exec.markSteeringObserved() logger.InfoCF("agent", "Steering arrived after direct LLM response; continuing turn", map[string]any{ "agent_id": ts.agent.ID, diff --git a/pkg/agent/turn_coord.go b/pkg/agent/turn_coord.go index 2826e662c..739e34f2f 100644 --- a/pkg/agent/turn_coord.go +++ b/pkg/agent/turn_coord.go @@ -14,7 +14,11 @@ import ( "github.com/sipeed/picoclaw/pkg/providers" ) -func (al *AgentLoop) runTurn(ctx context.Context, ts *turnState, pipeline *Pipeline) (turnResult, error) { +func (al *AgentLoop) runTurn( + ctx context.Context, + ts *turnState, + pipeline *Pipeline, +) (turnResult, error) { turnCtx, turnCancel := context.WithCancel(ctx) defer turnCancel() ts.setTurnCancel(turnCancel) @@ -89,11 +93,13 @@ func (al *AgentLoop) runTurn(ctx context.Context, ts *turnState, pipeline *Pipel // We do NOT call dequeueSteeringMessagesForScope here because // steering was already consumed from al.steering by ExecuteTools. if len(exec.pendingMessages) > 0 { + exec.markSteeringObserved() pendingMessages = append(pendingMessages, exec.pendingMessages...) exec.pendingMessages = nil } } else if !ts.opts.SkipInitialSteeringPoll { if steerMsgs := al.dequeueSteeringMessagesForScopeWithFallback(ts.sessionKey); len(steerMsgs) > 0 { + exec.markSteeringObserved() pendingMessages = append(pendingMessages, steerMsgs...) } } @@ -101,18 +107,26 @@ func (al *AgentLoop) runTurn(ctx context.Context, ts *turnState, pipeline *Pipel // Check if parent turn has ended (SubTurn support from HEAD) if ts.parentTurnState != nil && ts.IsParentEnded() { if !ts.critical { - logger.InfoCF("agent", "Parent turn ended, non-critical SubTurn exiting gracefully", map[string]any{ + logger.InfoCF( + "agent", + "Parent turn ended, non-critical SubTurn exiting gracefully", + map[string]any{ + "agent_id": ts.agentID, + "iteration": iteration, + "turn_id": ts.turnID, + }, + ) + break + } + logger.InfoCF( + "agent", + "Parent turn ended, critical SubTurn continues running", + map[string]any{ "agent_id": ts.agentID, "iteration": iteration, "turn_id": ts.turnID, - }) - break - } - logger.InfoCF("agent", "Parent turn ended, critical SubTurn continues running", map[string]any{ - "agent_id": ts.agentID, - "iteration": iteration, - "turn_id": ts.turnID, - }) + }, + ) } // Poll for pending SubTurn results (from HEAD) @@ -200,6 +214,7 @@ func (al *AgentLoop) runTurn(ctx context.Context, ts *turnState, pipeline *Pipel if finalContent == "" { finalContent = ts.opts.DefaultResponse } + finalContent = renderFinalTurnReply(turnCtx, al, ts, exec, finalContent) return pipeline.Finalize(ctx, turnCtx, ts, exec, turnStatus, finalContent) case ControlToolLoop: // Execute tools via Pipeline @@ -210,6 +225,36 @@ func (al *AgentLoop) runTurn(ctx context.Context, ts *turnState, pipeline *Pipel // (added tool results/skipped messages) before returning ControlContinue messages = exec.messages continue + case ToolControlFinalize: + renderedContent, rendered := tryRenderFinalTurnReply( + turnCtx, + al, + ts, + exec, + finalContent, + ) + if !rendered { + messages = exec.messages + continue + } + if steerMsgs := al.dequeueSteeringMessagesForScope(ts.sessionKey); len( + steerMsgs, + ) > 0 { + exec.markSteeringObserved() + logger.InfoCF( + "agent", + "Steering arrived during terminal render; continuing turn", + map[string]any{ + "agent_id": ts.agent.ID, + "iteration": iteration, + "steering_count": len(steerMsgs), + }, + ) + exec.pendingMessages = append(exec.pendingMessages, steerMsgs...) + messages = exec.messages + continue + } + return pipeline.Finalize(ctx, turnCtx, ts, exec, turnStatus, renderedContent) case ToolControlBreak: // Hard abort: delegate to abortTurn (sets TurnEndStatusAborted) if exec.abortedByHardAbort { @@ -227,6 +272,7 @@ func (al *AgentLoop) runTurn(ctx context.Context, ts *turnState, pipeline *Pipel if exec.allResponsesHandled { finalContent = "" } + finalContent = renderFinalTurnReply(turnCtx, al, ts, exec, finalContent) return pipeline.Finalize(ctx, turnCtx, ts, exec, turnStatus, finalContent) } } @@ -244,6 +290,7 @@ func (al *AgentLoop) runTurn(ctx context.Context, ts *turnState, pipeline *Pipel finalContent = ts.opts.DefaultResponse } } + finalContent = renderFinalTurnReply(turnCtx, al, ts, exec, finalContent) // Check hard abort before finalizing (may have been set during tool execution) if ts.hardAbortRequested() { @@ -299,7 +346,10 @@ func (al *AgentLoop) selectCandidates( "score": score, "threshold": agent.Router.Threshold(), }) - return agent.LightCandidates, resolvedCandidateModel(agent.LightCandidates, agent.Router.LightModel()), true + return agent.LightCandidates, resolvedCandidateModel( + agent.LightCandidates, + agent.Router.LightModel(), + ), true } func (al *AgentLoop) resolveContextManager() ContextManager { @@ -316,10 +366,14 @@ func (al *AgentLoop) resolveContextManager() ContextManager { } cm, err := factory(al.cfg.Agents.Defaults.ContextManagerConfig, al) if err != nil { - logger.WarnCF("agent", "Failed to create context manager, falling back to legacy", map[string]any{ - "name": name, - "error": err.Error(), - }) + logger.WarnCF( + "agent", + "Failed to create context manager, falling back to legacy", + map[string]any{ + "name": name, + "error": err.Error(), + }, + ) return &legacyContextManager{al: al} } return cm @@ -399,7 +453,11 @@ func (al *AgentLoop) askSideQuestion( forceModel bool, callMessages []providers.Message, ) (*providers.LLMResponse, error) { - provider, providerModel, cleanup, err := al.isolatedSideQuestionProvider(agent, selectedModelName, candidate) + provider, providerModel, cleanup, err := al.isolatedSideQuestionProvider( + agent, + selectedModelName, + candidate, + ) if err != nil { return nil, err } @@ -419,7 +477,11 @@ func (al *AgentLoop) askSideQuestion( turnCtx := newTurnContext(nil, nil, nil) if opts != nil { - turnCtx = newTurnContext(opts.Dispatch.InboundContext, opts.Dispatch.RouteResult, opts.Dispatch.SessionScope) + turnCtx = newTurnContext( + opts.Dispatch.InboundContext, + opts.Dispatch.RouteResult, + opts.Dispatch.SessionScope, + ) } llmModel := activeModel if al.hooks != nil { @@ -475,7 +537,8 @@ func (al *AgentLoop) askSideQuestion( func(ctx context.Context, providerName, model string) (*providers.LLMResponse, error) { candidate := providers.FallbackCandidate{Provider: providerName, Model: model} for _, activeCandidate := range activeCandidates { - if activeCandidate.Provider == providerName && activeCandidate.Model == model { + if activeCandidate.Provider == providerName && + activeCandidate.Model == model { candidate = activeCandidate break } @@ -563,7 +626,9 @@ func (al *AgentLoop) isolatedSideQuestionProvider( candidate providers.FallbackCandidate, ) (providers.LLMProvider, string, func(), error) { if agent == nil { - return nil, "", func() {}, fmt.Errorf("isolatedSideQuestionProvider: no agent available for /btw") + return nil, "", func() {}, fmt.Errorf( + "isolatedSideQuestionProvider: no agent available for /btw", + ) } modelCfg, err := al.sideQuestionModelConfig(agent, baseModelName, candidate) diff --git a/pkg/agent/turn_state.go b/pkg/agent/turn_state.go index b769ebcd0..229fef960 100644 --- a/pkg/agent/turn_state.go +++ b/pkg/agent/turn_state.go @@ -118,6 +118,8 @@ type turnExecution struct { // Turn output finalContent string + actionLog []TurnActionRecord + sawSteering bool // Iteration tracking iteration int @@ -147,6 +149,13 @@ type turnExecution struct { abortedByHook bool // true when HookActionAbortTurn triggered } +func (e *turnExecution) markSteeringObserved() { + if e == nil { + return + } + e.sawSteering = true +} + // newTurnExecution creates a turnExecution initialized from turnState and options. func newTurnExecution( agent *AgentInstance, @@ -160,6 +169,7 @@ func newTurnExecution( summary: summary, messages: messages, pendingMessages: append([]providers.Message(nil), opts.InitialSteeringMessages...), + sawSteering: len(opts.InitialSteeringMessages) > 0, iteration: 0, phase: LLMPhaseSetup, } diff --git a/pkg/config/config.go b/pkg/config/config.go index c9d90e0f8..dd35041c8 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -275,6 +275,7 @@ type AgentDefaults struct { MaxParallelTurns int `json:"max_parallel_turns,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_PARALLEL_TURNS"` // Max concurrent turns (0 or 1 = sequential) SubTurn SubTurnConfig `json:"subturn" envPrefix:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_"` ToolFeedback ToolFeedbackConfig `json:"tool_feedback,omitempty"` + FinalTurnRenderMode string `json:"final_turn_render_mode,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_FINAL_TURN_RENDER_MODE"` SplitOnMarker bool `json:"split_on_marker" env:"PICOCLAW_AGENTS_DEFAULTS_SPLIT_ON_MARKER"` // split messages on <|[SPLIT]|> marker ContextManager string `json:"context_manager,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_CONTEXT_MANAGER"` ContextManagerConfig json.RawMessage `json:"context_manager_config,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_CONTEXT_MANAGER_CONFIG"` @@ -311,6 +312,10 @@ func (d *AgentDefaults) IsToolFeedbackSeparateMessagesEnabled() bool { return d.ToolFeedback.SeparateMessages } +func (d *AgentDefaults) UseFinalTurnRender() bool { + return strings.EqualFold(strings.TrimSpace(d.FinalTurnRenderMode), "llm") +} + // GetModelName returns the effective model name for the agent defaults. // It prefers the new "model_name" field but falls back to "model" for backward compatibility. func (d *AgentDefaults) GetModelName() string { @@ -1017,7 +1022,11 @@ func LoadConfig(path string) (*Config, error) { } if e := json.Unmarshal(data, &versionInfo); e != nil { e = wrapJSONError(data, e, "config.json") - logger.ErrorCF("config", formatDiagnosticLogMessage("Malformed config file", e), map[string]any{"path": path}) + logger.ErrorCF( + "config", + formatDiagnosticLogMessage("Malformed config file", e), + map[string]any{"path": path}, + ) return nil, e } if len(data) <= 10 { diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 8e2494ae5..a250d059d 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -39,6 +39,7 @@ func DefaultConfig() *Config { MaxArgsLength: 300, SeparateMessages: false, }, + FinalTurnRenderMode: "", SplitOnMarker: false, MaxLLMRetries: 2, LLMRetryBackoffSecs: 2,