feat(llm): transport-aware per-rung LLM fallback walk + GLM (glm-5.2) support - #375
Merged
Merged
Conversation
…ability
Replace litellm-native fallback with a reflexio-owned walk in `_make_request`.
Both the plain and validator/repair paths walk `[primary, *fallbacks]` one rung
at a time via `_resolve_ladder`, rebuilding transport params per rung (structured
-output strategy, api_base, and a PER-SINGLE-ATTEMPT hard timeout — not the old
`(1+len(fallbacks))×per_attempt` ladder-wide budget). No `fallbacks` kwarg is ever
handed to `litellm.completion` on any path. `num_retries=0` per rung preserves the
PYTHON-FASTAPI-62 fix (a hung primary is abandoned after one attempt, then advances).
- Delete the transport guard `_validate_structured_fallback_strategies`: mixed
-strategy ladders are now safe because each rung builds its own transport.
- Per-rung repair: the primary keeps one same-model parse-retry (plain) or one
same-model corrective turn (validator). A fallback rung reached via repair gets
the ORIGINAL prompt, never the prior rung's repair conversation. Delete
`_repair_target_kwargs`'s cross-model escalation (the walk owns advancement).
- Error taxonomy: transport errors, StructuredOutput{Parse,Repair}Error, and
ProviderCapSaturatedError (Task 1) are all advance-worthy; the final rung
preserves the typed repair error and surfaces others as LiteLLMClientError.
- Observability: replace `_emit_fallback_observability`/`_same_observed_model`
(which relied on litellm rewriting response.model and never fired post-change)
with a loop-driven `_emit_fallback_signal` + `_rung_reason`. Preserves the
`event=llm_fallback_used` log + Sentry tag keys; adds `llm.fallback_reason`.
- Advisory ladder wall-clock budget warning.
Guard deletion + owned walk are atomic (Global Constraint). Tests rewritten by
class: delete 3 guard tests + ladder-wide-timeout scaling; rewrite forwarding /
escalation / sentry / hard-timeout tests to the owned walk; add L1 contract class
TestOwnedFallbackWalk (transport/advance/slot/cap/signal); consolidator E2E updated.
…me-transport claims)
…lback verification
…cuit ladder under custom endpoint; doc fail-closed cap Addresses review-loop findings: boot-time validation no longer refuses boot for litellm providers outside _ENV_TO_PROVIDER (bedrock/vertex/etc.) — warns instead; a configured custom endpoint short-circuits the fallback ladder to one rung (no wasted rungs, no false llm_fallback_used signal); README concurrency-cap bullet documents REFLEXIO_LLM_FAIL_CLOSED_PROVIDERS + overrides.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRefactors LiteLLM fallback handling into a Reflexio-owned per-rung ladder with structured-output repair, per-rung timeouts, and explicit fallback telemetry. Adds configurable provider concurrency caps, fail-closed saturation errors, startup fallback-provider validation, GLM 5.2 defaults, and expanded unit, integration, and e2e coverage. ChangesLLM fallback and provider resilience
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant Reflexio
participant TextGenerationMixin
participant ProviderSlot
participant LiteLLM
Reflexio->>TextGenerationMixin: Submit generation request
TextGenerationMixin->>ProviderSlot: Acquire provider slot
ProviderSlot->>LiteLLM: Execute current rung
LiteLLM-->>TextGenerationMixin: Response or classified failure
TextGenerationMixin->>TextGenerationMixin: Retry or repair within rung
TextGenerationMixin->>ProviderSlot: Acquire next provider slot
ProviderSlot->>LiteLLM: Execute fallback rung
LiteLLM-->>TextGenerationMixin: Fallback response
TextGenerationMixin-->>Reflexio: Return generated result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e_tests/test_glm_fallback_real_llm.py`:
- Around line 182-202: Update test_glm_as_primary_pipeline to clear
REFLEXIO_LLM_FALLBACK_MODELS before reflexio_instance is constructed, then
create or obtain the instance within that cleared-configuration scope so client
initialization cannot inherit fallback models. Preserve the existing GLM
forcing, publish, tagging drain, and row assertions.
In `@tests/server/llm/test_litellm_client_unit.py`:
- Line 3428: Update the match pattern in the pytest.raises assertion for
LiteLLMClientError to use a raw string and escape the dot in “zai/glm-5.2”,
ensuring the regex matches the exact model name and satisfies RUF043.
In `@tests/server/llm/test_model_defaults.py`:
- Line 369: Update the pytest.raises match patterns in the affected assertions
to raw string literals, including the assertions near the fallback model checks,
while preserving the existing regex text and matching behavior.
- Around line 363-402: Update the shared test environment cleanup helper
_clean_env to remove REFLEXIO_LLM_FALLBACK_MODELS alongside the other
LLM-related environment variables. Ensure every availability test starts without
a stale configured fallback while preserving the existing cleanup behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4f008b65-d1db-45eb-977e-db52f57e3343
📒 Files selected for processing (10)
reflexio/server/README.mdreflexio/server/llm/_litellm_text_generation.pyreflexio/server/llm/_litellm_types.pyreflexio/server/llm/_provider_concurrency.pyreflexio/server/llm/model_defaults.pytests/e2e_tests/test_glm_fallback_real_llm.pytests/server/llm/test_litellm_client_unit.pytests/server/llm/test_model_defaults.pytests/server/llm/test_provider_concurrency.pytests/server/services/playbook/test_playbook_consolidator_integration.py
…t + _clean_env; raw-string regex matches
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.
Summary
Make the LLM fallback ladder transport-aware so a fallback can mix providers whose structured-output transports differ (e.g. MiniMax
native_json_schemaprimary → Z.ai GLMprompt_json_objectfallback), and defaultzaito the live-verified flagshipglm-5.2.Previously reflexio delegated the fallback ladder to LiteLLM's native
fallbacks=[...], which reuses a single request-params dict across the ladder — so the primary'sresponse_formatwas sent verbatim to every fallback, and a guard hard-rejected mixed-transport ladders. That blocked using GLM as a fallback behind a MiniMax primary.What changed
_litellm_text_generation.py): reflexio walks[primary, *fallbacks]itself, rebuilding request params per rung via the existing_build_completion_params(transport/api_base/timeout are already keyed on the model).fallbacksis never handed tolitellm.completion. The transport-compatibility guard (_validate_structured_fallback_strategies) is deleted — mixed-transport ladders are safe by construction.num_retries=0, preserving the PYTHON-FASTAPI-62 property (a hung primary can't block the fallback) per rung.event=llm_fallback_usedsignal preserving the existing Sentry tag keys (llm.fallback_used/llm.primary_model/llm.fallback_model) + a newreason(transport_error | parse_exhausted | cap_saturated)._provider_concurrency.py):REFLEXIO_LLM_FAIL_CLOSED_PROVIDERSmakes a provider raise on cap saturation (protects the fixed-quota GLM coding plan from a fallback stampede);REFLEXIO_LLM_PROVIDER_MAX_CONCURRENCY_OVERRIDESsets per-provider caps. Default = unchanged fail-open.model_defaults.py): fails loud at startup when a known key-based fallback provider has no key; warns (doesn't refuse boot) for litellm providers reflexio can't key-validate (bedrock/vertex/etc.).glm-5.1 → glm-5.2(only 5.2 is live-verified).Verification
fallbacksto litellm; parse-exhaustion advances; entered-once + original-prompt-to-fallback; per-rung timeout; cap-saturation advances; observability fires).check_model_support.py --model zai/glm-5.2= 17/18 (all 13 structured-output ops + tool-calling + embedding; only optional vision unsupported on the coding endpoint).@skip_low_priority): GLM-as-primary pipeline + prod-shape MiniMax→GLM fallback via the faithful publish path (3/3 passed).Caveat
GLM-5.2 on the Z.ai coding endpoint rejects image inputs — an image-bearing interaction that hits the GLM fallback (MiniMax down + image content) would fail that rung. Narrow; documented.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests