fix(execution): gate retry admission by error category - #2691
Open
1688mengdie wants to merge 3 commits into
Open
fix(execution): gate retry admission by error category#26911688mengdie wants to merge 3 commits into
1688mengdie wants to merge 3 commits into
Conversation
added 3 commits
August 30, 2026 17:29
The round_executor retry ladder only gates retries on a budget (local_attempt_index < max_attempts - 1). Error category is consulted only after the budget is exhausted (to classify the terminal error), so deterministic request errors (400/401/403/404/413/422/context_length_exceeded) are retried up to max_attempts regardless of category. is_retryable_category already encodes the retry-vs-terminal semantic (Network/RateLimit/Timeout/ProviderUnavailable are retryable; everything else is terminal). Mark it pub so the runtime assembly crate can reuse it as the retry admission gate without re-implementing a separate classifier. Add a unit test asserting the retryable-vs-terminal classification so the gate's decision predicate is covered: transient categories stay retryable (preserved behavior) while deterministic categories are terminal (no retry). No production behavior change: the fn is pure and its internal callers (AiProviderError::detail, ai_error_detail_from_message) are unchanged. Test: cargo test -p bitfun-core-types --jobs 4 AI: lightly tested
The round_executor retry ladder replayed every provider request error up to max_attempts based only on a budget check, so deterministic request errors (4xx, context_length_exceeded) were re-issued in full even though retrying them cannot succeed. Both request_error and stream_error branches consulted the category only after the budget was exhausted, when they already had to build the terminal error. Reuse the now-public is_retryable_category to decide retry admission at the top of the C1 (request_error) and C5 (stream_error) gates: transient categories (Network/RateLimit/Timeout/ProviderUnavailable) keep retrying, while deterministic categories turn into an immediate terminal error (attempt 1). ContextOverflow is NOT retryable, so it falls through to the existing RecoverableContextOverflow conversion and the overflow recovery chain is preserved unchanged. C2/C3/C4 (partial_stream_error, invalid_tool_arguments, no_effective_output) keep their original budget + containment semantics because content-invalid results are a model-quality issue where a retry can still produce valid output. Test: cargo test -p bitfun-core --features agent-runtime --jobs 4 AI: lightly tested
Retry admission is now gated on the provider error category, so a deterministic request error (403/4xx) terminates the turn at attempt 1 while a transient error (network/rate/timeout/overload) keeps retrying and ContextOverflow keeps its recovery chain. The exec contract tests still asserted the prior full-retry counts: - two http_403 tests expected 10 provider requests; 403 is Permission (terminal), so each now reaches the provider once before terminating. - the disconnect-then-403 test expected 10; the mid-stream disconnect is transient (Network), so it is retried once, then the deterministic 403 terminates the turn (2 provider requests total). - the malformed-SSE test expected a retry-to-success; a malformed SSE frame is a deterministic provider protocol error, so it now terminates at attempt 1 and the test asserts a terminal DialogTurnFailed instead of a successful retry. Test: cargo test --locked -p bitfun-cli --jobs 4 AI: lightly tested
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
RoundExecutorreplays every provider-request error up tomax_attemptsbasedonly on a budget check, so deterministic request failures — 4xx statuses and
context_length_exceeded— are re-issued in full even though retrying them cannotsucceed. A single bad request is re-sent the whole configured number of times,
amplifying latency/cost and delaying the actual error handling until the budget is
exhausted.
Root cause
The retry ladder in
round_executor(resource-request recovery) consults thebudget (
local_attempt_index < max_attempts - 1) but not the error category. Boththe
request_error(C1) andstream_error(C5) gates classify the category onlyafter the budget is exhausted, when they already have to build the terminal error.
The retry-vs-terminal semantic already lives in
core-types'sis_retryable_category(Network / RateLimit / Timeout / ProviderUnavailable areretryable; everything else is terminal), but it is private and is not consulted for
admission.
Fix
is_retryable_categoryaspubso the assembly runtime crate can reusethe existing classifier instead of re-implementing one.
request_error) and C5(
stream_error) branches on that predicate: transient categories keep retrying,while deterministic categories become an immediate terminal error (attempt 1).
ContextOverflowis NOT retryable, so it falls through to the existingRecoverableContextOverflowconversion and the overflow recovery chain ispreserved unchanged. C2/C3/C4 keep their original budget + containment semantics
because content-invalid results are a model-quality issue where a retry can still
produce valid output.
Testing
is_retryable_category_matches_transient_vs_terminal_semanticcovering thetransient-vs-terminal classification for the full category set.
cargo test -p bitfun-core-types --jobs 4—errors::testsgreen (33 passed).cargo test -p bitfun-core --features agent-runtime --jobs 4— applies to theround_executor path.
git diff --checkclean.Closes #2690
Commit list:
2155d6ea6fix(core-types): expose is_retryable_category— marks theclassifier
pub; adds the classification unit test.f88e9066afix(execution): gate retry admission by error category— C1/C5admission gate on
is_retryable_category.6fa58cb06fix(cli): align exec contracts to terminal retry— syncs theexec_cli_contracts assertions to the new terminal-retry semantics.