Skip to content

Commit e1cdf8f

Browse files
raphaelraphael
committed
fix(runtime): count the failure streak per decision point, not per parallel call
The consecutive-failure cap summed every failed call in a tool batch and only reset on fully clean batches, so one exploratory parallel batch with several domain failures (unconfigured weather, energy rates) terminated a run that was making progress on its other calls. The streak now measures what it always meant to: planner decision points with no budgeted progress. A batch whose budgeted (non-bookkeeping) calls all fail consumes one unit regardless of parallel width, any budgeted success resets the counter, and bookkeeping results never move it. Sequential single-call behavior is unchanged.
1 parent f2a3722 commit e1cdf8f

6 files changed

Lines changed: 189 additions & 93 deletions

File tree

‎runtime/agent/api/types.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,9 @@ type (
136136
// calls a run may execute.
137137
MaxToolCalls int
138138

139-
// MaxConsecutiveFailedToolCalls caps the number of consecutive failing tool calls before finalizing.
139+
// MaxConsecutiveFailedToolCalls caps the number of consecutive failing
140+
// tool batches before finalizing: a batch whose budgeted calls all fail
141+
// consumes one unit, and any budgeted success resets the streak.
140142
MaxConsecutiveFailedToolCalls int
141143

142144
// TimeBudget caps the total wall-clock runtime budget for the run.

‎runtime/agent/policy/policy.go‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -146,15 +146,17 @@ type (
146146
// are permitted.
147147
RemainingToolCalls int
148148

149-
// MaxConsecutiveFailedToolCalls caps consecutive failures per run. Zero means
150-
// the cap is not configured. Used for circuit breaking: if N tools fail in
151-
// a row, terminate.
149+
// MaxConsecutiveFailedToolCalls caps consecutive failing planner decision
150+
// points per run. Zero means the cap is not configured. Used for circuit
151+
// breaking: if N successive tool batches fail outright, terminate.
152152
MaxConsecutiveFailedToolCalls int
153153

154-
// RemainingConsecutiveFailedToolCalls tracks how many consecutive failures are allowed
155-
// before circuit breaking. The runtime decrements this on each failure and resets
156-
// it to MaxConsecutiveFailedToolCalls on success. When this reaches zero, the
157-
// run is terminated.
154+
// RemainingConsecutiveFailedToolCalls tracks how many failing decision
155+
// points are allowed before circuit breaking. A tool batch whose budgeted
156+
// (non-bookkeeping) calls all fail consumes one unit regardless of its
157+
// parallel width; any budgeted success resets the counter to
158+
// MaxConsecutiveFailedToolCalls; bookkeeping results never move it. When
159+
// this reaches zero, the run is terminated.
158160
RemainingConsecutiveFailedToolCalls int
159161

160162
// ExpiresAt conveys when the run-level budgets expire (wall-clock deadline).

‎runtime/agent/runtime/failure_streak_test.go‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
package runtime
2+
3+
// failure_streak_test.go verifies the consecutive-failure cap counts planner
4+
// decision points, not individual parallel calls: any budgeted success resets
5+
// the streak, an all-failure batch consumes exactly one unit, and bookkeeping
6+
// results never move the counter.
7+
8+
import (
9+
"testing"
10+
11+
"github.com/stretchr/testify/require"
12+
"goa.design/goa-ai/runtime/agent/planner"
13+
"goa.design/goa-ai/runtime/agent/policy"
14+
"goa.design/goa-ai/runtime/agent/tools"
15+
)
16+
17+
func TestApplyFailureStreak(t *testing.T) {
18+
t.Parallel()
19+
20+
cases := []struct {
21+
name string
22+
caps policy.CapsState
23+
progress bool
24+
failed bool
25+
wantRemaining int
26+
wantTripped bool
27+
}{
28+
{
29+
name: "budgeted success resets streak",
30+
caps: policy.CapsState{MaxConsecutiveFailedToolCalls: 3, RemainingConsecutiveFailedToolCalls: 1},
31+
progress: true,
32+
failed: true,
33+
wantRemaining: 3,
34+
},
35+
{
36+
name: "all-failure batch consumes one unit",
37+
caps: policy.CapsState{MaxConsecutiveFailedToolCalls: 3, RemainingConsecutiveFailedToolCalls: 3},
38+
failed: true,
39+
wantRemaining: 2,
40+
},
41+
{
42+
name: "final all-failure batch trips the cap",
43+
caps: policy.CapsState{MaxConsecutiveFailedToolCalls: 3, RemainingConsecutiveFailedToolCalls: 1},
44+
failed: true,
45+
wantRemaining: 0,
46+
wantTripped: true,
47+
},
48+
{
49+
name: "bookkeeping-only batch leaves the counter unchanged",
50+
caps: policy.CapsState{MaxConsecutiveFailedToolCalls: 3, RemainingConsecutiveFailedToolCalls: 2},
51+
wantRemaining: 2,
52+
},
53+
{
54+
name: "unset cap never trips",
55+
caps: policy.CapsState{},
56+
failed: true,
57+
wantRemaining: 0,
58+
},
59+
}
60+
61+
for _, tc := range cases {
62+
t.Run(tc.name, func(t *testing.T) {
63+
caps := tc.caps
64+
tripped := applyFailureStreak(&caps, tc.progress, tc.failed)
65+
require.Equal(t, tc.wantTripped, tripped)
66+
require.Equal(t, tc.wantRemaining, caps.RemainingConsecutiveFailedToolCalls)
67+
})
68+
}
69+
}
70+
71+
func TestBudgetedBatchOutcome(t *testing.T) {
72+
t.Parallel()
73+
74+
budgeted := newAnyJSONSpec("ada.get_energy_rates", "ada")
75+
budgetedOK := newAnyJSONSpec("ada.get_weather_forecast", "ada")
76+
progressSpec := newBookkeepingSpec("tasks.progress.update")
77+
78+
rt := New()
79+
seedTestToolSpecs(rt, budgeted, budgetedOK, progressSpec)
80+
81+
record := func(name tools.Ident, failed bool) stepToolRecord {
82+
result := &planner.ToolResult{Name: name, ToolCallID: "call-" + string(name)}
83+
if failed {
84+
result.Error = planner.NewToolError("boom")
85+
}
86+
return stepToolRecord{
87+
call: planner.ToolRequest{Name: name, ToolCallID: "call-" + string(name)},
88+
result: result,
89+
}
90+
}
91+
92+
cases := []struct {
93+
name string
94+
records []stepToolRecord
95+
wantProgress bool
96+
wantFailed bool
97+
}{
98+
{
99+
name: "mixed parallel batch reports progress and failure",
100+
records: []stepToolRecord{
101+
record(budgeted.Name, true),
102+
record(budgetedOK.Name, false),
103+
},
104+
wantProgress: true,
105+
wantFailed: true,
106+
},
107+
{
108+
name: "all budgeted failures report failure only",
109+
records: []stepToolRecord{
110+
record(budgeted.Name, true),
111+
record(budgetedOK.Name, true),
112+
},
113+
wantFailed: true,
114+
},
115+
{
116+
name: "bookkeeping success is not progress",
117+
records: []stepToolRecord{
118+
record(progressSpec.Name, false),
119+
record(budgeted.Name, true),
120+
},
121+
wantFailed: true,
122+
},
123+
{
124+
name: "bookkeeping-only batch reports neither",
125+
records: []stepToolRecord{
126+
record(progressSpec.Name, false),
127+
},
128+
},
129+
}
130+
131+
for _, tc := range cases {
132+
t.Run(tc.name, func(t *testing.T) {
133+
progress, failed := rt.budgetedBatchOutcome(tc.records)
134+
require.Equal(t, tc.wantProgress, progress)
135+
require.Equal(t, tc.wantFailed, failed)
136+
})
137+
}
138+
}

‎runtime/agent/runtime/helpers.go‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -593,16 +593,40 @@ func decrementCap(current int, delta int) int {
593593
// Contract:
594594
// - Every tool result error counts as one failed attempt.
595595
// - Retry hints may shape the next planner attempt, but they do not make the
596-
// failed attempt free for cap accounting.
597-
func capFailures(results []*planner.ToolResult) int {
598-
count := 0
599-
for _, res := range results {
600-
if res == nil || res.Error == nil {
596+
// budgetedBatchOutcome classifies a step batch's budgeted (non-bookkeeping)
597+
// results for failure-streak accounting: progress reports at least one
598+
// budgeted success and failed reports at least one budgeted failure.
599+
func (r *Runtime) budgetedBatchOutcome(records []stepToolRecord) (progress, failed bool) {
600+
for _, record := range records {
601+
if record.result == nil || r.isBookkeeping(record.call.Name) {
601602
continue
602603
}
603-
count++
604+
if record.result.Error != nil {
605+
failed = true
606+
} else {
607+
progress = true
608+
}
609+
}
610+
return progress, failed
611+
}
612+
613+
// applyFailureStreak advances the consecutive-failure cap for one step batch
614+
// and reports whether the cap tripped. Progress resets the streak, an
615+
// all-failure batch consumes one unit, and batches without budgeted results
616+
// leave the counter unchanged.
617+
func applyFailureStreak(caps *policy.CapsState, progress, failed bool) bool {
618+
switch {
619+
case progress:
620+
if caps.MaxConsecutiveFailedToolCalls > 0 {
621+
caps.RemainingConsecutiveFailedToolCalls = caps.MaxConsecutiveFailedToolCalls
622+
}
623+
case failed:
624+
caps.RemainingConsecutiveFailedToolCalls = decrementCap(caps.RemainingConsecutiveFailedToolCalls, 1)
625+
if caps.MaxConsecutiveFailedToolCalls > 0 && caps.RemainingConsecutiveFailedToolCalls <= 0 {
626+
return true
627+
}
604628
}
605-
return count
629+
return false
606630
}
607631

608632
// mergeCaps merges policy decision caps into the current caps state. Policy

‎runtime/agent/runtime/helpers_test.go‎

Lines changed: 0 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import (
44
"testing"
55

66
"github.com/stretchr/testify/require"
7-
"goa.design/goa-ai/runtime/agent/planner"
8-
"goa.design/goa-ai/runtime/agent/tools"
97
)
108

119
func TestGenerateDeterministicToolCallID_UniqueAcrossAttempts(t *testing.T) {
@@ -20,69 +18,3 @@ func TestGenerateDeterministicToolCallID_DeterministicForSameInputs(t *testing.T
2018
require.Equal(t, id1, id2)
2119
}
2220

23-
func TestCapFailures_CountsEveryToolError(t *testing.T) {
24-
t.Parallel()
25-
26-
tests := []struct {
27-
name string
28-
results []*planner.ToolResult
29-
want int
30-
}{
31-
{
32-
name: "tool_unavailable_with_hint_counts",
33-
results: []*planner.ToolResult{{
34-
Name: tools.Ident("svc.data.discover"),
35-
ToolCallID: "tc1",
36-
Error: planner.NewToolError("no healthy providers"),
37-
RetryHint: &planner.RetryHint{
38-
Reason: planner.RetryReasonToolUnavailable,
39-
Tool: tools.Ident("svc.data.discover"),
40-
},
41-
}},
42-
want: 1,
43-
},
44-
{
45-
name: "invalid_arguments_with_hint_counts",
46-
results: []*planner.ToolResult{{
47-
Name: tools.Ident("svc.data.discover"),
48-
ToolCallID: "tc-invalid",
49-
Error: planner.NewToolError("invalid arguments"),
50-
RetryHint: &planner.RetryHint{
51-
Reason: planner.RetryReasonInvalidArguments,
52-
Tool: tools.Ident("svc.data.discover"),
53-
},
54-
}},
55-
want: 1,
56-
},
57-
{
58-
name: "rate_limited_counts",
59-
results: []*planner.ToolResult{{
60-
Name: tools.Ident("svc.data.discover"),
61-
ToolCallID: "tc2",
62-
Error: planner.NewToolError("rate limited"),
63-
RetryHint: &planner.RetryHint{
64-
Reason: planner.RetryReasonRateLimited,
65-
Tool: tools.Ident("svc.data.discover"),
66-
},
67-
}},
68-
want: 1,
69-
},
70-
{
71-
name: "no_hint_counts",
72-
results: []*planner.ToolResult{{
73-
Name: tools.Ident("svc.data.discover"),
74-
ToolCallID: "tc3",
75-
Error: planner.NewToolError("boom"),
76-
}},
77-
want: 1,
78-
},
79-
}
80-
81-
for _, tt := range tests {
82-
t.Run(tt.name, func(t *testing.T) {
83-
t.Parallel()
84-
85-
require.Equal(t, tt.want, capFailures(tt.results))
86-
})
87-
}
88-
}

‎runtime/agent/runtime/workflow_step.go‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -295,16 +295,14 @@ func (l *workflowLoop) advanceStep(batch stepBatch) (*RunOutput, error) {
295295
}
296296

297297
results := batch.results()
298-
if capFailures(results) > 0 {
299-
l.st.Caps.RemainingConsecutiveFailedToolCalls = decrementCap(
300-
l.st.Caps.RemainingConsecutiveFailedToolCalls,
301-
capFailures(results),
302-
)
303-
if l.st.Caps.MaxConsecutiveFailedToolCalls > 0 && l.st.Caps.RemainingConsecutiveFailedToolCalls <= 0 {
304-
return l.finalizeStep(planner.TerminationReasonFailureCap, "failure-cap finalization skipped without hard deadline")
305-
}
306-
} else if l.st.Caps.MaxConsecutiveFailedToolCalls > 0 {
307-
l.st.Caps.RemainingConsecutiveFailedToolCalls = l.st.Caps.MaxConsecutiveFailedToolCalls
298+
// The failure streak counts planner decision points whose budgeted work
299+
// failed outright: any budgeted success resets the streak, an all-failure
300+
// batch consumes one unit regardless of its parallel width, and
301+
// bookkeeping results never move the counter. One exploratory batch that
302+
// partially fails is progress, not thrash.
303+
progress, failed := l.r.budgetedBatchOutcome(batch.records)
304+
if applyFailureStreak(&l.st.Caps, progress, failed) {
305+
return l.finalizeStep(planner.TerminationReasonFailureCap, "failure-cap finalization skipped without hard deadline")
308306
}
309307

310308
if out, err := l.r.handleMissingFieldsPolicy(

0 commit comments

Comments
 (0)