[Feature] adk: expose "is last attempt" (or MaxRetries) in TypedRetryContext passed to ShouldRetry
- Dominant language
- Go
- Stars
- 13k
- Forks
- 1.1k
- Avg merge
- 4h 6m
- Merged PRs (30d)
- 41
Description
## Problem
`TypedModelRetryConfig.ShouldRetry` receives a `TypedRetryContext` (`adk/retry_chatmodel.go:114-141`, v0.9.19) that carries `RetryAttempt`, but nothing that tells the callback whether this decision is the **last** one in the retry cycle.
Inside the loop the config's `MaxRetries` decides that, and it is applied *after* the callback returns:
- Generate path: `ShouldRetry` at `:433`; `if attempt >= r.config.MaxRetries { break }` at `:457-458`; `applyDecisionForRetry` only at `:460`.
- Stream path: `ShouldRetry` at `:611`; `if attempt < r.config.MaxRetries { applyDecisionForRetry(...) }` at `:644-645`.
So on the last attempt a decision with `Retry: true` is accepted, but `ModifiedInputMessages`, `PersistModifiedInputMessages`, `AdditionalOptions` and `Backoff` are silently discarded, and the loop falls through to `RetryExhaustedError` (`:473` / `:656`). The callback has no way to know that its decision is about to be dropped.
## Why it matters
A host SDK that layers policy on top of `ShouldRetry` (structured "will retry in N ms" telemetry, its own backoff, context compression via `ModifiedInputMessages`, side-effect-free termination) needs to branch on "is this the final slot" — e.g. not emit a retry-scheduled event, not sleep, not run an expensive compression whose output will never be used. Today it can only guess.
Note that returning `Retry: false` on the last attempt is not equivalent: it returns the raw error / message instead of `RetryExhaustedError`, so callers that classify exhaustion via `errors.Is(err, ErrExceedMaxRetries)` must still answer `Retry: true` on the final slot and then work around the discarded fields.
## Proposed API (additive)
```go
type TypedRetryContext[M MessageType] struct {
RetryAttempt int
// MaxRetries mirrors TypedModelRetryConfig.MaxRetries for this cycle.
MaxRetries int
// IsLastAttempt is true when a Retry:true decision will not produce another
// model call; ModifiedInputMessages / AdditionalOptions / Backoff are ignored.
IsLastAttempt bool
// ... existing fields unchanged
}
```
Either field alone would be enough; `IsLastAttempt` is the least ambiguous and stays correct if the attempt numbering ever changes. `failover_chatmodel.go:45-56` already threads an equivalent `hasMoreAttempts` flag through context for the failover path, so this would make the retry path symmetric with it.
Population is a one-liner at each `retryCtx := &TypedRetryContext[M]{...}` site (`:426`, `:555`, `:604`): `IsLastAttempt: attempt >= r.config.MaxRetries`.
## Workaround today & its cost
The callback closes over the same `MaxRetries` value that was put into the config and tests `retryCtx.RetryAttempt > maxRetries`. This duplicates a config invariant in user code, depends on `RetryAttempt` being exactly `attempt+1`, and breaks silently if the retry config is wrapped/rewritten by another layer (e.g. composed with `FailoverChatModel`, or populated via `ModelRequest.ModelRetryConfig` in `handler.go:66-69`).
## Minimal reproduction (v0.9.19)
```go
cfg := &adk.ModelRetryConfig{
MaxRetries: 1,
ShouldRetry: func(ctx context.Context, rc *adk.RetryContext) *adk.RetryDecision {
// attempt 2 is the last one; this Backoff and ModifiedInputMessages are dropped
return &adk.RetryDecision{Retry: true, Backoff: 5 * time.Second,
ModifiedInputMessages: compress(rc.InputMessages)}
},
}
// Model always fails -> ShouldRetry is called with RetryAttempt=1 and RetryAttempt=2.
// Nothing in rc distinguishes the two; only the second decision is discarded.
```
Contributor guide
Research direction
Read `adk/retry_chatmodel.go`, focusing on the `TypedRetryContext` definition and the three `retryCtx` construction sites cited in the issue. Compare the generate and stream retry paths to confirm when the final decision is applied or discarded, then check existing retry tests for coverage. Done means the callback can identify its last retry attempt in both paths, with tests covering the exposed value.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100