chore(router): suppress the retry-skip kwargs writes and correct the filter docstring

The two writes that hand the skip list to the next attempt now carry a
`# rebind-ok` reason, which is the sanctioned escape hatch for an unavoidable
parameter mutation and matches how `log_retry` already writes into the same
kwargs dict a few lines above

`get_excluded_filtered_deployments`'s docstring said returning the unfiltered
list would re-include the deployment that just failed. The retry skip does
exactly that on purpose, so the docstring now says each caller decides what an
empty result means

The reliability registry cell the new e2e test claims is marked
`fail_before_fix: proven`: the same config returns 400 at the merge base and
200 off a sibling deployment at the tip
This commit is contained in:
mateo-berri 2026-09-06 00:53:24 -07:00
parent 0fcf0fe06c
commit d664ca139e
3 changed files with 13 additions and 11 deletions

View file

@ -7572,12 +7572,12 @@ class Router:
## LOGGING
if num_retries > 0:
kwargs = self.log_retry(kwargs=kwargs, e=original_exception)
skipped_deployment_ids: Final = self._deployment_ids_to_skip_on_retry(
first_skipped_ids: Final = self._deployment_ids_to_skip_on_retry(
exception=original_exception,
already_skipped=kwargs.get("_retry_skipped_deployment_ids"),
)
if skipped_deployment_ids:
kwargs["_retry_skipped_deployment_ids"] = skipped_deployment_ids
if first_skipped_ids:
kwargs["_retry_skipped_deployment_ids"] = first_skipped_ids # rebind-ok: the next attempt reads it
else:
raise
@ -7647,12 +7647,12 @@ class Router:
except Exception:
raise e
retry_skipped_deployment_ids = self._deployment_ids_to_skip_on_retry(
skipped_ids = self._deployment_ids_to_skip_on_retry(
exception=e,
already_skipped=kwargs.get("_retry_skipped_deployment_ids"),
)
if retry_skipped_deployment_ids:
kwargs["_retry_skipped_deployment_ids"] = retry_skipped_deployment_ids
if skipped_ids:
kwargs["_retry_skipped_deployment_ids"] = skipped_ids # rebind-ok: the next attempt reads it
_timeout = self._time_to_sleep_before_retry(
e=e,
remaining_retries=remaining_retries,

View file

@ -4919,10 +4919,12 @@ def get_excluded_filtered_deployments(
across the remaining deployments in the same model group after one of them
has failed.
If the filter would leave no deployments, an empty list is returned so the
caller raises its usual no-deployments error and the weighted-failover
helper falls through to the cross-group fallback path. Returning the
original unfiltered list here would re-include the just-failed deployment.
If the filter would leave no deployments, an empty list is returned and the
caller decides what that means. Weighted failover lets it raise the usual
no-deployments error and fall through to the cross-group fallback path; the
retry skip in `async_get_healthy_deployments` deliberately falls back to the
unfiltered list, so a request every deployment refused still comes back with
the provider's own error rather than a no-deployments one.
"""
if not excluded_deployment_ids:
return healthy_deployments

View file

@ -7,7 +7,7 @@
- {id: reliability.retry.timeout.succeeds_within_retries, module: reliability, tier: P0, behavior: retry, variant: timeout, assertions: [succeeds_within_retries], exercised_on: [chat_completions, messages], source: "get_retry_from_policy.py:44", rationale: "Timeout retried per policy"}
- {id: reliability.retry.429.succeeds_within_retries, module: reliability, tier: P0, behavior: retry, variant: "429", assertions: [succeeds_within_retries], exercised_on: [chat_completions, messages], source: "get_retry_from_policy.py:46", rationale: "429 retried per RateLimitErrorRetries policy"}
- {id: reliability.retry.auth.succeeds_within_retries, module: reliability, tier: P1, behavior: retry, variant: auth, assertions: [succeeds_within_retries], exercised_on: [chat_completions, messages], source: "get_retry_from_policy.py:42", rationale: "Transient auth glitch retry"}
- {id: reliability.retry.context_window.succeeds_within_retries, module: reliability, tier: P1, behavior: retry, variant: context_window, assertions: [succeeds_within_retries], exercised_on: [chat_completions, messages], source: "get_retry_from_policy.py:51", rationale: "Multi-attempt on context error"}
- {id: reliability.retry.context_window.succeeds_within_retries, module: reliability, tier: P1, behavior: retry, variant: context_window, assertions: [succeeds_within_retries], exercised_on: [chat_completions, messages], source: "get_retry_from_policy.py:51", fail_before_fix: proven, rationale: "A context-window 400 under BadRequestErrorRetries retries onto a sibling deployment in the same model group, instead of coming straight back as the 400 the deployment that just refused it returned"}
- {id: reliability.cooldown.5xx.trips_then_recovers, module: reliability, tier: P0, behavior: cooldown, variant: "5xx", assertions: [trips_then_recovers], exercised_on: [chat_completions, messages], source: "cooldown_handlers.py:40", rationale: "Deployment cools after repeated 5xx, recovers after cooldown_time"}
- {id: reliability.cooldown.429.trips_then_recovers, module: reliability, tier: P0, behavior: cooldown, variant: "429", assertions: [trips_then_recovers], exercised_on: [chat_completions, messages], source: "cooldown_handlers.py:69", rationale: "Cools on 429, avoids hammering exhausted provider"}
- {id: reliability.cooldown.auth.trips_then_recovers, module: reliability, tier: P1, behavior: cooldown, variant: auth, assertions: [trips_then_recovers], exercised_on: [chat_completions, messages], source: "cooldown_handlers.py:74", rationale: "Cools on 401 auth error"}