stop responding with "..." (#1491)

This commit is contained in:
bilogic authored and GitHub committed 2026-08-02 07:29:30 +07:00
1 parent 89e4560f20
commit 6e61bd1531
9 files changed
+423 -17

No files matched your search

+3 -6
View File
@@ -67,16 +67,13 @@ func (l *Loop) finalizeRun(
}
// 7. Fallback only when there is no other deliverable output. Media-only
// runs must remain media-only instead of gaining a visible "..." caption.
// runs must remain media-only instead of gaining a visible caption. Use a
// meaningful localized message instead of the old meaningless "...".
hasDeliverableOutput := len(rs.mediaResults) > 0 ||
len(req.ForwardMedia) > 0 ||
req.ContentSuffix != ""
if rs.finalContent == "" && !hasDeliverableOutput {
if len(rs.asyncToolCalls) > 0 {
rs.finalContent = "..."
} else {
rs.finalContent = "..."
}
rs.finalContent = i18n.T(store.LocaleFromContext(ctx), i18n.MsgEmptyReplyFallback)
}
// Append content suffix (e.g. image markdown for WS) before saving to session.
+1
View File
@@ -233,6 +233,7 @@ func init() {
MsgSkillNudgePostscript: "This task involved several steps. Want me to save the process as a reusable skill? Reply **\"save as skill\"** or **\"skip\"**.",
MsgSkillNudge70Pct: "[System] You are at 70% of your iteration budget. Consider whether any patterns from this session would make a good skill.",
MsgSkillNudge90Pct: "[System] You are at 90% of your iteration budget. If this session involved reusable patterns, consider saving them as a skill before completing.",
MsgEmptyReplyFallback: "⚠️ Agent couldn't generate a response. Note: some tool actions may have already been executed — please verify before retrying",
MsgInvalidRole: "invalid role: allowed values are owner, admin, operator, member, viewer",
+1
View File
@@ -233,6 +233,7 @@ func init() {
MsgSkillNudgePostscript: "Tác vụ này cần nhiều bước. Bạn muốn tôi lưu quy trình này thành kỹ năng tái sử dụng không? Trả lời **\"lưu kỹ năng\"** hoặc **\"bỏ qua\"**.",
MsgSkillNudge70Pct: "[System] Bạn đã dùng 70% ngân sách vòng lặp. Cân nhắc xem các mẫu trong phiên này có nên lưu thành kỹ năng không.",
MsgSkillNudge90Pct: "[System] Bạn đã dùng 90% ngân sách vòng lặp. Nếu phiên này có quy trình tái sử dụng, hãy cân nhắc lưu thành kỹ năng trước khi hoàn thành.",
MsgEmptyReplyFallback: "⚠️ Agent không thể tạo phản hồi. Lưu ý: một số thao tác công cụ có thể đã được thực hiện — vui lòng kiểm tra trước khi thử lại",
MsgInvalidRole: "vai trò không hợp lệ: giá trị cho phép là owner, admin, operator, member, viewer",
+1
View File
@@ -233,6 +233,7 @@ func init() {
MsgSkillNudgePostscript: "此任务涉及多个步骤。要我将此过程保存为可重用技能吗?回复 **\"保存技能\"** 或 **\"跳过\"**。",
MsgSkillNudge70Pct: "[System] 您已使用 70% 的迭代预算。请考虑本次会话中的模式是否值得保存为技能。",
MsgSkillNudge90Pct: "[System] 您已使用 90% 的迭代预算。如果本次会话涉及可重用的模式,请考虑在完成前将其保存为技能。",
MsgEmptyReplyFallback: "⚠️ 代理无法生成响应。注意:部分工具操作可能已经执行 — 请先确认后再重试",
MsgInvalidRole: "无效角色:允许的值为 owner、admin、operator、member、viewer",
+8 -4
View File
@@ -209,10 +209,10 @@ const (
MsgFailedToDeleteFile = "error.failed_to_delete_file" // "failed to delete"
// --- OAuth ---
MsgNoPendingOAuth = "error.no_pending_oauth" // "no pending OAuth flow"
MsgFailedToSaveToken = "error.failed_to_save_token" // "failed to save token"
MsgOAuthCallbackSuccess = "oauth.callback_success" // "Authorization successful. You may close this window."
MsgOAuthCallbackFailed = "oauth.callback_failed" // "Authorization failed. You may close this window."
MsgNoPendingOAuth = "error.no_pending_oauth" // "no pending OAuth flow"
MsgFailedToSaveToken = "error.failed_to_save_token" // "failed to save token"
MsgOAuthCallbackSuccess = "oauth.callback_success" // "Authorization successful. You may close this window."
MsgOAuthCallbackFailed = "oauth.callback_failed" // "Authorization failed. You may close this window."
// --- Intent Classify (channel-facing status replies) ---
MsgStatusWorking = "status.working" // "🔄 I'm working on your request... Please wait."
@@ -271,6 +271,10 @@ const (
MsgSkillNudge70Pct = "skill.nudge_70_pct"
MsgSkillNudge90Pct = "skill.nudge_90_pct"
// Empty reply fallback (user-facing) — shown when a run finishes with no text
// output and no deliverable media, replacing the old meaningless "...".
MsgEmptyReplyFallback = "chat.empty_reply_fallback"
// Tool progress announcements (user-facing)
MsgToolAnnouncementSingle = "progress.tool_announcement.single" // "I'll use %s to handle the next step."
MsgToolAnnouncementMulti = "progress.tool_announcement.multi" // "I'll use %s to handle the next step."
+11 -3
View File
@@ -8,6 +8,7 @@ import (
"github.com/google/uuid"
"github.com/nextlevelbuilder/goclaw/internal/hooks"
"github.com/nextlevelbuilder/goclaw/internal/i18n"
"github.com/nextlevelbuilder/goclaw/internal/providers"
"github.com/nextlevelbuilder/goclaw/internal/store"
)
@@ -42,9 +43,16 @@ func (s *FinalizeStage) Execute(ctx context.Context, state *RunState) error {
// Must run BEFORE session flush so the agent message is persisted even if suppressed.
isSilent := s.deps.IsSilentReply != nil && s.deps.IsSilentReply(state.Observe.FinalContent)
// 2b. Fallback for empty content (matching v2: channels need non-empty content to deliver).
if state.Observe.FinalContent == "" && !isSilent {
state.Observe.FinalContent = "..."
// 2b. Fallback for empty content (matching v2: channels need non-empty content
// to deliver). Media-only runs stay media-only — no text caption (matching v2
// hasDeliverableOutput). The placeholder is a meaningful localized message, not
// a bare "..." — ThinkStage already nudges the model for empty text responses,
// so this only fires when the model truly produced nothing.
hasDeliverableOutput := len(state.Tool.MediaResults) > 0 ||
len(state.Input.ForwardMedia) > 0 ||
state.Input.ContentSuffix != ""
if state.Observe.FinalContent == "" && !isSilent && !hasDeliverableOutput {
state.Observe.FinalContent = i18n.T(store.LocaleFromContext(ctx), i18n.MsgEmptyReplyFallback)
}
// 2c. Append content suffix (e.g. image markdown for WS) with dedup.
+5 -4
View File
@@ -36,10 +36,11 @@ type ThinkState struct {
// prompt sent to the model — the session's current context. Consumed by
// FinalizeStage → UpdateMetadata → SetLastPromptTokens for the sessions
// context-usage display and compaction calibration.
LastUsage providers.Usage
TruncRetries int // consecutive truncation retries (max 3)
OverflowRetries int // context overflow compact+retry attempts (max 1)
StreamingActive bool // true during active stream
LastUsage providers.Usage
TruncRetries int // consecutive truncation retries (max 3)
OverflowRetries int // context overflow compact+retry attempts (max 1)
EmptyReplyRetries int // consecutive empty final-reply nudges (max maxEmptyReplyRetries)
StreamingActive bool // true during active stream
// Tools is populated by ContextStage (iteration=0) for overhead calculation.
// It holds the best-effort tool list at run start and is used exclusively by
+37
View File
@@ -13,6 +13,16 @@ import (
const maxTruncRetries = 3
// maxEmptyReplyRetries bounds how many times ThinkStage nudges the model after an
// empty final response (no text, no tool calls) before falling back to a
// placeholder. Small bound: a model that keeps returning empty is unlikely to
// answer on repeated nudges, and each nudge consumes an iteration.
const maxEmptyReplyRetries = 2
// emptyReplyHint nudges the model to produce a visible answer when its final
// response came back empty. English-only (LLM consumption, matches truncation hints).
const emptyReplyHint = "[System] Your response was empty. Give the user your final answer now."
// ThinkStage runs per iteration. Calls LLM, handles truncation retries,
// accumulates usage, returns BreakLoop when response has no tool calls.
type ThinkStage struct {
@@ -173,6 +183,21 @@ func (s *ThinkStage) Execute(ctx context.Context, state *RunState) error {
// message with sanitization + MediaRefs, so skip AppendPending here to avoid
// a duplicate. Matches v2 behavior where loop breaks before appending.
if len(resp.ToolCalls) == 0 {
// Empty final answer: the model finished with no visible text. When there
// are no deliverables to carry the reply, nudge the model (bounded) so the
// user gets a real answer instead of a "..." placeholder. Media-only runs
// stay as-is — finalize delivers the media without a text caption. Only
// nudge when another iteration remains; on the last iteration a nudge
// would never be answered and would pollute persisted history.
maxIter := s.deps.Config.MaxIterations
if strings.TrimSpace(resp.Content) == "" &&
!s.hasDeliverableOutput(state) &&
state.Think.EmptyReplyRetries < maxEmptyReplyRetries &&
state.Iteration+1 < maxIter {
state.Think.EmptyReplyRetries++
state.Messages.AppendPending(providers.Message{Role: "user", Content: emptyReplyHint, Transient: true})
return nil // Continue to next iteration for a real answer
}
s.result = BreakLoop
return nil
}
@@ -382,6 +407,18 @@ func isRequestBudgetExceededErr(err error) bool {
return false
}
// hasDeliverableOutput reports whether the run already produced non-text output
// (media results, forwarded media, or a content suffix) that can carry the reply
// without a text message. Mirrors v2 finalizeRun's hasDeliverableOutput.
func (s *ThinkStage) hasDeliverableOutput(state *RunState) bool {
if state.Input == nil {
return false
}
return len(state.Tool.MediaResults) > 0 ||
len(state.Input.ForwardMedia) > 0 ||
state.Input.ContentSuffix != ""
}
// reduceForBudgetExceeded runs one pass of the reduction chain
// (prune_history -> compact_history -> shrink_memory) against the current
// state, stopping at the first step that changes anything. It increments
@@ -0,0 +1,356 @@
package pipeline
import (
"context"
"strings"
"testing"
"github.com/nextlevelbuilder/goclaw/internal/bus"
"github.com/nextlevelbuilder/goclaw/internal/i18n"
"github.com/nextlevelbuilder/goclaw/internal/providers"
"github.com/nextlevelbuilder/goclaw/internal/store"
)
// --- ThinkStage empty-reply nudge ---
func emptyReplyState(iteration int) *RunState {
state := defaultState()
state.Iteration = iteration
return state
}
// TestThinkStage_EmptyFinalReply_NudgesModel verifies that a final response with
// no text and no tool calls triggers a bounded nudge (Continue) instead of
// BreakLoop, so the user gets a real answer rather than a "..." placeholder.
func TestThinkStage_EmptyFinalReply_NudgesModel(t *testing.T) {
t.Parallel()
deps := &PipelineDeps{
Config: PipelineConfig{MaxIterations: 10, MaxTokens: 1000},
CallLLM: func(_ context.Context, _ *RunState, _ providers.ChatRequest) (*providers.ChatResponse, error) {
return &providers.ChatResponse{
Content: "",
FinishReason: "stop",
}, nil
},
}
stage := NewThinkStage(deps)
state := emptyReplyState(0)
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if stage.Result() != Continue {
t.Errorf("Result() = %v, want Continue (nudge for a real answer)", stage.Result())
}
if state.Think.EmptyReplyRetries != 1 {
t.Errorf("EmptyReplyRetries = %d, want 1", state.Think.EmptyReplyRetries)
}
pending := state.Messages.Pending()
if len(pending) != 1 {
t.Fatalf("pending len = %d, want 1 nudge message", len(pending))
}
if pending[0].Role != "user" || !strings.Contains(pending[0].Content, emptyReplyHint) {
t.Errorf("nudge = %q/%q, want user/%q", pending[0].Role, pending[0].Content, emptyReplyHint)
}
if !pending[0].Transient {
t.Errorf("nudge should be Transient so it never pollutes persisted history")
}
}
// TestThinkStage_EmptyFinalReply_LastIterationBreaks verifies the nudge is
// skipped on the final iteration: there is no iteration left to answer it, so
// the run breaks and FinalizeStage provides the fallback.
func TestThinkStage_EmptyFinalReply_LastIterationBreaks(t *testing.T) {
t.Parallel()
deps := &PipelineDeps{
Config: PipelineConfig{MaxIterations: 3, MaxTokens: 1000},
CallLLM: func(_ context.Context, _ *RunState, _ providers.ChatRequest) (*providers.ChatResponse, error) {
return &providers.ChatResponse{
Content: "",
FinishReason: "stop",
}, nil
},
}
stage := NewThinkStage(deps)
state := emptyReplyState(2) // last of 3 iterations
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if stage.Result() != BreakLoop {
t.Errorf("Result() = %v, want BreakLoop on final iteration", stage.Result())
}
if state.Think.EmptyReplyRetries != 0 {
t.Errorf("EmptyReplyRetries = %d, want 0 (no nudge on final iteration)", state.Think.EmptyReplyRetries)
}
if len(state.Messages.Pending()) != 0 {
t.Errorf("pending = %v, want empty (no nudge on final iteration)", state.Messages.Pending())
}
}
// TestThinkStage_EmptyFinalReply_RetriesExhaustedBreaks verifies the nudge is
// bounded: after maxEmptyReplyRetries unanswered nudges, the run breaks.
func TestThinkStage_EmptyFinalReply_RetriesExhaustedBreaks(t *testing.T) {
t.Parallel()
deps := &PipelineDeps{
Config: PipelineConfig{MaxIterations: 10, MaxTokens: 1000},
CallLLM: func(_ context.Context, _ *RunState, _ providers.ChatRequest) (*providers.ChatResponse, error) {
return &providers.ChatResponse{
Content: "",
FinishReason: "stop",
}, nil
},
}
stage := NewThinkStage(deps)
state := emptyReplyState(5)
state.Think.EmptyReplyRetries = maxEmptyReplyRetries
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if stage.Result() != BreakLoop {
t.Errorf("Result() = %v, want BreakLoop after retries exhausted", stage.Result())
}
if len(state.Messages.Pending()) != 0 {
t.Errorf("pending = %v, want empty after retries exhausted", state.Messages.Pending())
}
}
// TestThinkStage_EmptyFinalReply_MediaOnlyBreaks verifies media-only runs break
// immediately: the media IS the deliverable, no text caption needed (matching
// v2 hasDeliverableOutput semantics).
func TestThinkStage_EmptyFinalReply_MediaOnlyBreaks(t *testing.T) {
t.Parallel()
deps := &PipelineDeps{
Config: PipelineConfig{MaxIterations: 10, MaxTokens: 1000},
CallLLM: func(_ context.Context, _ *RunState, _ providers.ChatRequest) (*providers.ChatResponse, error) {
return &providers.ChatResponse{
Content: "",
FinishReason: "stop",
}, nil
},
}
stage := NewThinkStage(deps)
state := emptyReplyState(0)
state.Tool.MediaResults = []MediaResult{{Path: "/tmp/img.png", ContentType: "image/png"}}
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if stage.Result() != BreakLoop {
t.Errorf("Result() = %v, want BreakLoop for media-only run", stage.Result())
}
if state.Think.EmptyReplyRetries != 0 {
t.Errorf("EmptyReplyRetries = %d, want 0 (media-only)", state.Think.EmptyReplyRetries)
}
if len(state.Messages.Pending()) != 0 {
t.Errorf("pending = %v, want empty for media-only run", state.Messages.Pending())
}
}
// TestThinkStage_EmptyFinalReply_ForwardMediaBreaks covers the forwarded-media
// deliverable path (inbound attachments carried through the run).
func TestThinkStage_EmptyFinalReply_ForwardMediaBreaks(t *testing.T) {
t.Parallel()
deps := &PipelineDeps{
Config: PipelineConfig{MaxIterations: 10, MaxTokens: 1000},
CallLLM: func(_ context.Context, _ *RunState, _ providers.ChatRequest) (*providers.ChatResponse, error) {
return &providers.ChatResponse{
Content: "",
FinishReason: "stop",
}, nil
},
}
stage := NewThinkStage(deps)
state := emptyReplyState(0)
state.Input.ForwardMedia = []bus.MediaFile{{Path: "/tmp/doc.pdf", MimeType: "application/pdf"}}
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if stage.Result() != BreakLoop {
t.Errorf("Result() = %v, want BreakLoop for forwarded-media run", stage.Result())
}
if len(state.Messages.Pending()) != 0 {
t.Errorf("pending = %v, want empty for forwarded-media run", state.Messages.Pending())
}
}
// TestThinkStage_EmptyFinalReply_ContentSuffixBreaks covers the content-suffix
// deliverable path (e.g. WS image markdown appended at finalize).
func TestThinkStage_EmptyFinalReply_ContentSuffixBreaks(t *testing.T) {
t.Parallel()
deps := &PipelineDeps{
Config: PipelineConfig{MaxIterations: 10, MaxTokens: 1000},
CallLLM: func(_ context.Context, _ *RunState, _ providers.ChatRequest) (*providers.ChatResponse, error) {
return &providers.ChatResponse{
Content: "",
FinishReason: "stop",
}, nil
},
}
stage := NewThinkStage(deps)
state := emptyReplyState(0)
state.Input.ContentSuffix = "\n![img](/media/img.png)"
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if stage.Result() != BreakLoop {
t.Errorf("Result() = %v, want BreakLoop for content-suffix run", stage.Result())
}
if len(state.Messages.Pending()) != 0 {
t.Errorf("pending = %v, want empty for content-suffix run", state.Messages.Pending())
}
}
// TestThinkStage_EmptyFinalReply_WhitespaceOnlyNudges verifies whitespace-only
// content is treated as empty (TrimSpace), nudging rather than delivering " ".
func TestThinkStage_EmptyFinalReply_WhitespaceOnlyNudges(t *testing.T) {
t.Parallel()
deps := &PipelineDeps{
Config: PipelineConfig{MaxIterations: 10, MaxTokens: 1000},
CallLLM: func(_ context.Context, _ *RunState, _ providers.ChatRequest) (*providers.ChatResponse, error) {
return &providers.ChatResponse{
Content: " \n\t ",
FinishReason: "stop",
}, nil
},
}
stage := NewThinkStage(deps)
state := emptyReplyState(0)
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if stage.Result() != Continue {
t.Errorf("Result() = %v, want Continue (whitespace-only nudges)", stage.Result())
}
if state.Think.EmptyReplyRetries != 1 {
t.Errorf("EmptyReplyRetries = %d, want 1", state.Think.EmptyReplyRetries)
}
}
// --- FinalizeStage localized fallback ---
// TestFinalizeStage_EmptyContent_LocalizedFallback verifies the final
// placeholder is the localized message, never a bare "...".
func TestFinalizeStage_EmptyContent_LocalizedFallback(t *testing.T) {
t.Parallel()
ctx := store.WithLocale(context.Background(), "en")
deps := &PipelineDeps{}
stage := NewFinalizeStage(deps)
state := defaultState()
state.Observe.FinalContent = ""
if err := stage.Execute(ctx, state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
want := i18n.T(store.LocaleFromContext(ctx), i18n.MsgEmptyReplyFallback)
if state.Observe.FinalContent != want {
t.Errorf("FinalContent = %q, want localized fallback %q (not \"...\")", state.Observe.FinalContent, want)
}
if state.Observe.FinalContent == "..." {
t.Errorf("FinalContent must never be the old bare \"...\" placeholder")
}
}
// TestFinalizeStage_EmptyContent_LocaleSpecific verifies the fallback respects
// the active locale (vi here).
func TestFinalizeStage_EmptyContent_LocaleSpecific(t *testing.T) {
t.Parallel()
ctx := store.WithLocale(context.Background(), "vi")
deps := &PipelineDeps{}
stage := NewFinalizeStage(deps)
state := defaultState()
state.Observe.FinalContent = ""
if err := stage.Execute(ctx, state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
want := i18n.T("vi", i18n.MsgEmptyReplyFallback)
if state.Observe.FinalContent != want {
t.Errorf("FinalContent = %q, want vi fallback %q", state.Observe.FinalContent, want)
}
}
// TestFinalizeStage_EmptyContent_MediaOnlySkipsFallback verifies media-only
// runs stay media-only — no text caption injected (v2 parity).
func TestFinalizeStage_EmptyContent_MediaOnlySkipsFallback(t *testing.T) {
t.Parallel()
deps := &PipelineDeps{}
stage := NewFinalizeStage(deps)
state := defaultState()
state.Observe.FinalContent = ""
state.Tool.MediaResults = []MediaResult{{Path: "/tmp/img.png", ContentType: "image/png"}}
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if state.Observe.FinalContent != "" {
t.Errorf("FinalContent = %q, want empty (media-only run)", state.Observe.FinalContent)
}
}
// TestFinalizeStage_EmptyContent_ForwardMediaSkipsFallback covers the
// forwarded-media deliverable path at finalize.
func TestFinalizeStage_EmptyContent_ForwardMediaSkipsFallback(t *testing.T) {
t.Parallel()
deps := &PipelineDeps{}
stage := NewFinalizeStage(deps)
state := defaultState()
state.Observe.FinalContent = ""
state.Input.ForwardMedia = []bus.MediaFile{{Path: "/tmp/doc.pdf", MimeType: "application/pdf"}}
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if state.Observe.FinalContent != "" {
t.Errorf("FinalContent = %q, want empty (forwarded-media run)", state.Observe.FinalContent)
}
}
// TestFinalizeStage_EmptyContent_ContentSuffixSkipsFallback covers the
// content-suffix deliverable path (suffix still appended, no placeholder).
func TestFinalizeStage_EmptyContent_ContentSuffixSkipsFallback(t *testing.T) {
t.Parallel()
suffix := "\n![img](/media/img.png)"
deps := &PipelineDeps{
DeduplicateMediaSuffix: func(content, toAppend string) string {
if strings.HasSuffix(content, toAppend) {
return ""
}
return toAppend
},
}
stage := NewFinalizeStage(deps)
state := defaultState()
state.Observe.FinalContent = ""
state.Input.ContentSuffix = suffix
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if state.Observe.FinalContent != suffix {
t.Errorf("FinalContent = %q, want suffix-only %q (no placeholder)", state.Observe.FinalContent, suffix)
}
}
// TestFinalizeStage_EmptyContent_IsSilentSkipsFallback verifies silent
// (NO_REPLY) runs keep empty delivery — the fallback must not fire.
func TestFinalizeStage_EmptyContent_IsSilentSkipsFallback(t *testing.T) {
t.Parallel()
deps := &PipelineDeps{
IsSilentReply: func(_ string) bool { return true },
}
stage := NewFinalizeStage(deps)
state := defaultState()
state.Observe.FinalContent = ""
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if state.Observe.FinalContent != "" {
t.Errorf("FinalContent = %q, want empty (silent run suppressed)", state.Observe.FinalContent)
}
}