Merge pull request 'fix(agent): make same-call repeat guard progress-aware' (#21) from fix/progress-aware-same-call-guard into main

agent/agent.go

@@ -125,10 +125,12 @@ func WithCompactor(fn func(ctx context.Context, msgs []llm.Message) ([]llm.Messa
 }
 
 // WithToolErrorLimits installs loop guards: maxConsecutiveErrors bounds
-// successive steps whose tool results were ALL errors, and
-// maxSameCallRepeats bounds identical (name + arguments) tool calls within
-// one run. Either guard tripping ends the run with ErrToolLoop and the
-// partial result. Zero disables a guard.
+// successive steps whose tool results were ALL errors, and maxSameCallRepeats
+// bounds identical (name + arguments) tool calls that ALSO return an unchanged
+// result within one run — a call whose result keeps advancing (e.g. polling a
+// long-running background job) is progress and never trips this guard. Either
+// guard tripping ends the run with ErrToolLoop and the partial result. Zero
+// disables a guard.
 func WithToolErrorLimits(maxConsecutiveErrors, maxSameCallRepeats int) Option {
 	return func(a *Agent) {
 		a.maxConsecutiveToolErrors = maxConsecutiveErrors
@@ -251,6 +253,15 @@ func (a *Agent) mergedTools() (map[string]llm.Tool, []llm.Tool, error) {
 // Run executes the loop: send the conversation; while the model requests
 // tools, execute them and feed results back; stop on a final answer,
 // MaxSteps, or an unrecoverable model error.
+// repeatState is the per-signature bookkeeping for the progress-aware same-call
+// guard: count is the run-length of consecutive identical calls that returned
+// lastResult (the previous call's encoded result). A changed result resets count
+// to 1. See the guard block in Run.
+type repeatState struct {
+	count      int
+	lastResult string
+}
+
 func (a *Agent) Run(ctx context.Context, input string, opts ...RunOption) (*Result, error) {
 	var rc runConfig
 	for _, opt := range opts {
@@ -274,9 +285,12 @@ func (a *Agent) Run(ctx context.Context, input string, opts ...RunOption) (*Resu
 	reqOpts := append(append([]llm.Option(nil), a.reqOpts...), rc.reqOpts...)
 	system := a.systemPrompt()
 
-	// Loop-guard state (WithToolErrorLimits).
+	// Loop-guard state (WithToolErrorLimits). repeatStates tracks, per identical
+	// (name+arguments) signature, the run-length of consecutive calls that
+	// returned the same result and that last result — see the same-call guard
+	// below.
 	consecutiveErrorSteps := 0
-	callCounts := make(map[string]int)
+	repeatStates := make(map[string]*repeatState)
 
 	maxSteps := func() int {
 		if a.maxStepsFunc != nil {
@@ -330,13 +344,6 @@ func (a *Agent) Run(ctx context.Context, input string, opts ...RunOption) (*Resu
 				result.Messages = msgs
 				return result, err
 			}
-			if a.maxSameCallRepeats > 0 {
-				sig := call.Name + "\x00" + string(call.Arguments)
-				callCounts[sig]++
-				if callCounts[sig] > a.maxSameCallRepeats {
-					repeatTripped = call.Name
-				}
-			}
 			tool, ok := byName[call.Name]
 			if !ok {
 				results = append(results, llm.ToolResult{
@@ -356,6 +363,42 @@ func (a *Agent) Run(ctx context.Context, input string, opts ...RunOption) (*Resu
 		a.notify(rc, step)
 		msgs = append(msgs, llm.ToolResultsMessage(results...))
 
+		// Same-call repeat guard (progress-aware). An identical (name+arguments)
+		// call only counts toward the loop trip when its result is unchanged from
+		// the previous identical call: a call whose result keeps advancing —
+		// canonically polling a long-running background job — is progress and
+		// resets its count, while a genuinely stuck call returning the same output
+		// trips once it exceeds the ceiling. Result equality is exact-string on
+		// the full encoded content, chosen to err toward NOT tripping: a hung job
+		// whose poll still reports a ticking field is left to MaxRuntime / the
+		// job's own ceiling rather than risking a false kill of real progress.
+		// results[i] pairs with resp.ToolCalls[i] — every call appends exactly one
+		// result (unknown tools append an error result before continue) and the
+		// only early exit above is a full return on ctx cancellation.
+		if a.maxSameCallRepeats > 0 {
+			for i, call := range resp.ToolCalls {
+				sig := call.Name + "\x00" + string(call.Arguments)
+				resKey := results[i].Content
+				if results[i].IsError {
+					resKey = "e\x00" + resKey
+				}
+				st := repeatStates[sig]
+				if st == nil {
+					st = &repeatState{}
+					repeatStates[sig] = st
+				}
+				if st.count > 0 && st.lastResult == resKey {
+					st.count++
+				} else {
+					st.count = 1
+				}
+				st.lastResult = resKey
+				if st.count > a.maxSameCallRepeats {
+					repeatTripped = call.Name
+				}
+			}
+		}
+
 		if repeatTripped != "" {
 			result.Messages = msgs
 			return result, fmt.Errorf("%w: %q called identically more than %d times",

agent/hooks_test.go

@@ -173,3 +173,51 @@ func TestSameCallRepeatGuard(t *testing.T) {
 		t.Errorf("varied calls must not trip the guard: %v", err)
 	}
 }
+
+// TestSameCallRepeatGuardProgressAware: identical (name+args) calls whose
+// RESULT keeps changing — canonically polling a long-running background job
+// whose progress advances — do not trip the repeat guard even well past the
+// limit; but identical calls returning an unchanged result still trip it.
+func TestSameCallRepeatGuardProgressAware(t *testing.T) {
+	// A poll tool that advances every call, so identical args yield a
+	// different result each time.
+	calls := 0
+	polling := llm.NewToolbox("jobs", llm.Tool{
+		Name: "poll",
+		Handler: func(context.Context, json.RawMessage) (any, error) {
+			calls++
+			return map[string]any{"status": "running", "elapsed": calls}, nil
+		},
+	})
+	n := 0
+	fp := fake.New("fp", fake.WithDefault(func(string, llm.Request) fake.Step {
+		n++
+		if n > 6 { // six identical polls, well past the limit of 3
+			return fake.Reply("done")
+		}
+		return toolCallReply("c", "poll", `{"job":"x"}`)
+	}))
+	a := New(newModel(t, fp), "", WithToolbox(polling), WithToolErrorLimits(0, 3), WithMaxSteps(20))
+	res, err := a.Run(context.Background(), "go")
+	if err != nil {
+		t.Fatalf("advancing-result polls must not trip the guard: %v", err)
+	}
+	if res.Output != "done" {
+		t.Errorf("output = %q, want run to complete after polling", res.Output)
+	}
+
+	// A call that returns an UNCHANGED result each time still trips the guard.
+	frozen := llm.NewToolbox("jobs", llm.Tool{
+		Name: "poll",
+		Handler: func(context.Context, json.RawMessage) (any, error) {
+			return map[string]any{"status": "running"}, nil // never advances
+		},
+	})
+	fp2 := fake.New("fp", fake.WithDefault(func(string, llm.Request) fake.Step {
+		return toolCallReply("c", "poll", `{"job":"x"}`)
+	}))
+	a2 := New(newModel(t, fp2), "", WithToolbox(frozen), WithToolErrorLimits(0, 3), WithMaxSteps(20))
+	if _, err := a2.Run(context.Background(), "go"); !errors.Is(err, ErrToolLoop) {
+		t.Fatalf("frozen identical result must still trip the guard: %v", err)
+	}
+}