Skip to content

feat(agent): control model retry attempts with a deadline-aware policy (issue #313) - #356

Open
konlue wants to merge 2 commits into
agentic-ai-java:mainfrom
konlue:feat/issue313-controlled-model-retry
Open

konlue wants to merge 2 commits into
agentic-ai-java:mainfrom
konlue:feat/issue313-controlled-model-retry

Conversation

@konlue

@konlue konlue commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Describe what this PR does / why we need it

ModelRetryInterceptor repeats a failed model call, but three properties of that sequence were not controlled. The retryability of a failure was guessed from the text of the exception message, so a permanent rejection that happened to mention a timeout was repeated, while a transient failure with an unfamiliar message was not. The sequence ignored the deadline of the run and started the next provider call even after the run had expired. And a cancelled run kept the sequence alive, because cancellation is delivered through the execution guard of the run while the loop only observed thread interruption. The backoff itself blocked the calling thread with Thread.sleep, while the streaming path waited reactively, so a single policy had two blocking semantics and neither of them observed the run control.

This PR turns the retry into a controlled policy. The decision is made by a neutral contract that classifies the failure, bounds the number of attempts and observes the deadline and the cancellation of the run, and it reports a stable stop reason instead of an exhausted retry.

Does this pull request fix one issue?

Fixes #313

Describe how you did it

The contract lives in io.github.agentic.ai.agent.retry in argi-agent-api, which depends neither on Reactor nor on the graph runtime, so the independent ReAct engine and the graph based framework share it. ModelCallFailureCategory and ModelCallFailure describe the classification of a failure, ModelCallFailureClassifier and TypeBasedModelCallFailureClassifier classify by failure type and cause chain instead of by message text, RetryPolicy carries the bounded configuration, RetryStopReason and RetryDecision describe the terminal state of a sequence, and ModelRetryController owns the attempt counter, the deadline and the decision.

The run guard reaches the interceptor through ModelRequest, which gained an optional ExecutionGuard field. The field is typed and is never part of the request attributes that the adapters forward to a provider. The independent ReAct engine passes the control of its execution context and the framework node passes the guard of the run configuration, so both engines propagate the guard of the run they execute, and a copied request keeps the guard.

ModelRetryInterceptor now delegates its decisions to the controller. Before every attempt the controller checks the guard and the deadline: when the deadline has passed, when less than the configured reserve remains, or when the next backoff would outlive the deadline, the sequence stops before another call is dispatched. With a guard present the backoff waits in slices and checks the guard before every slice, so a cancelled or expired run ends the wait instead of dispatching another call. A stopped run is reported with the reason of the guard and is never reported as an exhausted retry, and the stop signal is propagated unchanged.

The existing configuration keeps its behaviour: the retryable exception predicate stays the default decision and every existing builder option keeps its default value. The type based classifier, the retryable categories, the reserved deadline time and the clock are opt in. The backoff calculation moved into the neutral policy, so the framework helper that duplicated it was removed together with its test; the growth, the cap and the zero delay stay covered by the controller test.

How was it tested / verified?

The new tests are offline and model independent. ModelRetryControllerTest covers the attempt bound, the classification of a permanent failure, the growth and the bound of the backoff, an expired deadline, a deadline without room for another attempt, a backoff that would outlive the deadline, a cancelled run and a classification that ignores the text of the message. ModelRetryInterceptorControlledTest covers the interceptor end to end: three consecutive failures perform exactly three calls, an explicit classifier keeps a permanent failure at one call even when its message mentions a timeout, the legacy predicate still decides when no classifier is configured, a cancellation during the backoff performs no further call and reports the stop signal of the guard, a deadline that cannot fit another attempt stops the sequence, the usage reported to the engine comes from the successful attempt only, and a copied request keeps the guard. ReActAgentExecutionGuardTest and AgentLlmNodeExecutionGuardTest assert that both engines propagate the guard of the run into the model request. The existing retry, cancellation and node tests are unchanged and stay green.

The change was verified with the full reactor build including the tests, the format validation, the checkstyle check, the spelling check and the newline check.

Additional notes

The scope of this PR is the repetition of one model call. It does not switch models, it does not replay tool calls, it does not own a persistent retry budget and it does not change the boundary of a streaming response that already produced output. The assembly of the retry behaviour as a lifecycle strategy, together with the routing and the fallback policies, belongs to the lifecycle chain and to the model policy work.

One message changes in an edge case: a failure that is not retryable and that happens on the last allowed attempt is now reported as a non retryable failure instead of an exhausted retry, because the reason of the stop is reported instead of the attempt count.

…y (issue agentic-ai-java#313)

The retry sequence of a model call was not bounded by its run. The
retryability of a failure was guessed from the text of the exception
message, the deadline of the run was ignored, and a cancelled run kept
receiving provider calls because the loop observed thread interruption
only.

The decision now belongs to a neutral contract in argi-agent-api
(io.github.agentic.ai.agent.retry) that classifies the failure by type
and cause chain, bounds the number of attempts and observes the deadline
and the cancellation of the run. The run guard reaches the interceptor
through an optional ExecutionGuard on ModelRequest and is propagated by
the independent ReAct engine and by the framework node, so a cancelled
or expired run ends the backoff instead of dispatching another call.

The existing configuration keeps its behaviour and every existing builder
option keeps its default value; the type based classifier, the retryable
categories, the reserved deadline time and the clock are opt in.
@yuluo-yx

yuluo-yx commented Nov 4, 2026

Copy link
Copy Markdown
Member

Two remaining correctness issues:

  • [P2] Retry state leaks across stream subscriptions. ModelRetryInterceptor.java:173 captures the same mutable controller for every subscription. Once the first subscription exhausts its attempts, the controller retains ATTEMPTS_EXHAUSTED, so a second subscription receives no retry budget. Reproduced with maxAttempts=2, zero delay and an always-failing stream: the first subscription invokes the retry handler, but the second does not. Please create independent retry state per subscription, as the method's documented contract promises.

  • [P2] Streaming backoff does not observe run cancellation while waiting. At ModelRetryInterceptor.java:213, the reactive delay waits in full before checking the guard; it neither registers onLoss nor performs the sliced checks used by the synchronous path. In a local probe, cancelling the guard immediately after entering a 1,500 ms backoff delivered cancellation only after approximately 1,531 ms, with zero onLoss registrations. A long configured backoff therefore keeps a cancelled subscription pending. Please make the wait terminate when the run stops.

Validation: targeted offline Java probes against this PR head; the full reactor test suite was not run.

@konlue

konlue commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Both fixed in f81fa2b (+94/−10):

  1. Per-subscription retry state: withStreamingRetry no longer receives the interceptor's controller. The controller is now created inside the Flux.defer, so every subscription builds its own ModelRetryController from the request (guard, deadline and policy identical, attempt counter fresh). Reproducing your probe — maxAttempts=2, zero delay, an always-failing stream — the first subscription consumes its own budget and exhausts, and the second subscription dispatches its retry again instead of inheriting ATTEMPTS_EXHAUSTED. The attempt chain inside one subscription is still carried explicitly through onFailure(failure, attemptsMade), so the bound of the sequence is unchanged.

  2. Cancellable streaming backoff: delay now mirrors the synchronous pause — when the request carries a guard, the wait is sliced into 100 ms ticks (Flux.interval + take), and each slice calls guard.assertActive(), so a cancelled or expired run terminates the wait at the next slice. Cancelling immediately after entering a 30 s backoff now surfaces the guard's ExecutionStoppedException within one slice; a request without a guard keeps the plain delayed completion.

Two new tests: testStreamingRetryStateDoesNotLeakAcrossSubscriptions (the second subscription must dispatch its own retry) in ModelRetryInterceptorTest, and aStreamingBackoffTerminatesWhenTheRunIsCancelled in ModelRetryInterceptorControlledTest. Both classes green (15/15 + 8/8), plus the 9 cancellation regression cases; checkstyle clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Task][P1] 将模型退避重试迁移为受控生命周期策略

2 participants