Skip to content

Commit 4ea1fe5

Browse files
committed
refactor: centralize retry ownership defaults
1 parent 48a86e1 commit 4ea1fe5

3 files changed

Lines changed: 29 additions & 9 deletions

File tree

‎src/core/task/Task.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,12 @@ import { validateAndFixToolResultIds } from "./validateToolResultIds"
138138
import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages"
139139
import { prepareApiConversationMessage } from "./apiConversationHistory"
140140
import { shouldAddUserMessageToHistory } from "./messageCounting"
141-
import { decideMidStreamFailure, MAX_MID_STREAM_RETRIES, shouldRemoveMidStreamRetryMessage } from "./midStreamRetry"
141+
import {
142+
decideMidStreamFailure,
143+
MAX_MID_STREAM_RETRIES,
144+
shouldRemoveMidStreamRetryMessage,
145+
wasMidStreamRetryMessageAdded,
146+
} from "./midStreamRetry"
142147
import { type TaskExecutionContext } from "./providerHandoff"
143148

144149
const MAX_EXPONENTIAL_BACKOFF_SECONDS = 600 // 10 minutes
@@ -2926,10 +2931,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
29262931
includeFileDetails: boolean
29272932
retryAttempt?: number
29282933
userMessageWasRemoved?: boolean // Track if user message was removed due to empty response
2929-
userMessageWasAdded: boolean
2934+
userMessageWasAdded?: boolean
29302935
}
29312936

2932-
const stack: StackItem[] = [{ userContent, includeFileDetails, retryAttempt: 0, userMessageWasAdded: false }]
2937+
const stack: StackItem[] = [{ userContent, includeFileDetails, retryAttempt: 0 }]
29332938

29342939
while (stack.length > 0) {
29352940
const currentItem = stack.pop()!
@@ -3068,7 +3073,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
30683073
isEmptyUserContent,
30693074
userMessageWasRemoved: currentItem.userMessageWasRemoved,
30703075
})
3071-
let { userMessageWasAdded } = currentItem
3076+
let userMessageWasAdded = wasMidStreamRetryMessageAdded(currentItem.userMessageWasAdded)
30723077
if (shouldAddUserMessage) {
30733078
await this.addToApiConversationHistory({ role: "user", content: finalUserContent })
30743079
this.messageCounts.user++
@@ -3708,7 +3713,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
37083713
// Stryker disable next-line BooleanLiteral: file details are already materialized in the persisted request being retried.
37093714
includeFileDetails: false,
37103715
retryAttempt: 0,
3711-
userMessageWasAdded: false,
37123716
})
37133717

37143718
// Continue to retry the request
@@ -4108,7 +4112,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
41084112
stack.push({
41094113
userContent: [...this.userMessageContent], // Create a copy to avoid mutation issues
41104114
includeFileDetails: false, // Subsequent iterations don't need file details
4111-
userMessageWasAdded: false,
41124115
})
41134116

41144117
// Add periodic yielding to prevent blocking
@@ -4176,7 +4179,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
41764179
includeFileDetails: false,
41774180
retryAttempt: (currentItem.retryAttempt ?? 0) + 1,
41784181
userMessageWasRemoved: removedCurrentUserMessage,
4179-
userMessageWasAdded: false,
41804182
})
41814183

41824184
// Continue to retry the request
@@ -4198,7 +4200,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
41984200
includeFileDetails: false,
41994201
retryAttempt: (currentItem.retryAttempt ?? 0) + 1,
42004202
userMessageWasRemoved: removedCurrentUserMessage,
4201-
userMessageWasAdded: false,
42024203
})
42034204

42044205
// Continue to retry the request

‎src/core/task/__tests__/midStreamRetry.spec.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { describe, expect, it } from "vitest"
22

3-
import { decideMidStreamFailure, MAX_MID_STREAM_RETRIES, shouldRemoveMidStreamRetryMessage } from "../midStreamRetry"
3+
import {
4+
decideMidStreamFailure,
5+
MAX_MID_STREAM_RETRIES,
6+
shouldRemoveMidStreamRetryMessage,
7+
wasMidStreamRetryMessageAdded,
8+
} from "../midStreamRetry"
49

510
describe("decideMidStreamFailure", () => {
611
it.each([0, 1, 2])("retries attempt %i", (attempt) => {
@@ -22,3 +27,13 @@ describe("shouldRemoveMidStreamRetryMessage", () => {
2227
expect(shouldRemoveMidStreamRetryMessage(added, role)).toBe(expected)
2328
})
2429
})
30+
31+
describe("wasMidStreamRetryMessageAdded", () => {
32+
it.each([
33+
{ value: true, expected: true },
34+
{ value: false, expected: false },
35+
{ value: undefined, expected: false },
36+
])("returns $expected for $value", ({ value, expected }) => {
37+
expect(wasMidStreamRetryMessageAdded(value)).toBe(expected)
38+
})
39+
})

‎src/core/task/midStreamRetry.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,7 @@ export function decideMidStreamFailure(retryAttempt: number): MidStreamFailureDe
99
export function shouldRemoveMidStreamRetryMessage(userMessageWasAdded: boolean, lastRole?: string): boolean {
1010
return userMessageWasAdded && lastRole === "user"
1111
}
12+
13+
export function wasMidStreamRetryMessageAdded(value?: boolean): boolean {
14+
return value === true
15+
}

0 commit comments

Comments
 (0)