feat(resilience): provider fallback chain for retryable failures - #128
Merged
Conversation
Introduce FallbackChain DTO + FallbackChainExecutor so an LlmConfiguration can list other configurations to retry when the primary fails with a connection error, HTTP 5xx, or rate-limit (429). Non-retryable errors (4xx other than 429, UnsupportedFeature, Configuration) still bubble up unchanged. Storage: new nullable fallback_chain TEXT column on tx_nrllm_configuration holding a JSON list of configuration identifiers. Fallback is shallow (recursion disabled) and does not apply to streaming. Wired through LlmServiceManager::chatWithConfiguration and completeWithConfiguration; streaming is intentionally untouched and documented. Optional constructor dependency keeps existing tests passing without changes. See ADR-021 for rationale and scope limitations. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
github-actions
Bot
added
documentation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #128 +/- ##
============================================
+ Coverage 93.54% 93.65% +0.11%
- Complexity 2146 2192 +46
============================================
Files 82 85 +3
Lines 7905 8074 +169
============================================
+ Hits 7395 7562 +167
- Misses 510 512 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Contributor
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
There was a problem hiding this comment.
Pull request overview
Adds a per-configuration provider fallback chain so retryable provider failures (connection/5xx/429) automatically retry the request against alternate LlmConfigurations in order, plus persistence/UI support and test coverage.
Changes:
- Adds
fallback_chainstorage (DB + Extbase model + TCA + i18n labels) backed by a JSON DTO (FallbackChain). - Introduces
FallbackChainExecutorand wires it intoLlmServiceManager::chatWithConfiguration()/completeWithConfiguration()(streaming explicitly excluded). - Adds unit/integration/functional tests and ADR-021 documenting the design.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ext_tables.sql | Adds tx_nrllm_configuration.fallback_chain column for persisted fallback chain JSON. |
| Configuration/TCA/tx_nrllm_configuration.php | Adds backend form field + new “Fallback Chain” tab. |
| Resources/Private/Language/locallang_tca.xlf | Adds labels/descriptions for fallback chain UI (EN). |
| Resources/Private/Language/de.locallang_tca.xlf | Adds labels/descriptions for fallback chain UI (DE). |
| Classes/Domain/Model/LlmConfiguration.php | Adds raw JSON field + DTO accessors/mutators and hasFallbackChain(). |
| Classes/Domain/DTO/FallbackChain.php | Adds DTO/value object for ordered, deduped fallback identifiers with JSON (de)serialization. |
| Classes/Service/FallbackChainExecutor.php | Adds executor that applies fallback behavior on retryable exceptions. |
| Classes/Service/LlmServiceManager.php | Wraps non-streaming config-based calls with fallback executor when available. |
| Classes/Provider/Exception/FallbackChainExhaustedException.php | Adds exception carrying attempt history when the chain is exhausted. |
| Documentation/Adr/Index.rst | Registers ADR-021 in the ADR index. |
| Documentation/Adr/Adr021ProviderFallbackChain.rst | Documents fallback-chain rationale, scope, and constraints. |
| Tests/Unit/Domain/DTO/FallbackChainTest.php | Unit tests for DTO behavior and JSON round-trips. |
| Tests/Unit/Service/FallbackChainExecutorTest.php | Unit tests for retryability rules, skipping behavior, and exhaustion. |
| Tests/Unit/Provider/Exception/FallbackChainExhaustedExceptionTest.php | Unit tests for exception messaging and attempt capture. |
| Tests/Integration/Service/FallbackChainIntegrationTest.php | Integration tests across manager → executor → providers using stubbed HTTP. |
| Tests/Functional/Repository/LlmConfigurationFallbackChainTest.php | Functional persistence round-trip tests for the new DB column. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a provider fallback mechanism for LLM requests, allowing automatic retries against a chain of alternative configurations when primary providers encounter connection issues or rate limits. It introduces a FallbackChain DTO, a FallbackChainExecutor service, and integrates these into the LlmServiceManager, though streaming requests are intentionally excluded. Feedback focuses on enhancing the FallbackChain DTO by normalizing identifiers—specifically trimming whitespace and converting to lowercase—to ensure consistency and robustness against manual configuration edits.
- PSR-3 logging: pass Throwable in reserved `exception` context key;
add separate `exceptionClass` for the class name.
- Rethrow primary's retryable error verbatim when the fallback chain
contains only the primary's own identifier (otherwise we wrap a
single attempt as "every configuration failed", which is misleading).
- Reword exhausted-chain message ("All N configuration(s) in the
fallback chain failed") so it matches getAttemptedConfigurations()
(which includes the primary).
- TCA fallback_chain field gets eval=trim for consistency with other
text columns on this table.
- FallbackChain::sanitize() now tolerates non-string entries in the
input array (skips them) instead of crashing with "Illegal offset
type" during hydration of malformed JSON.
- FallbackChain normalises identifiers (trim + lowercase) on sanitize,
contains, withLink, and without so they match the TCA-enforced
lowercase-trimmed form of tx_nrllm_configuration.identifier.
Hand-edited JSON with stray whitespace / capitals now still resolves.
- Class docblock updated to describe actual sanitisation behaviour
(constructor does NOT sanitise; fromArray/fromJson + withLink do).
- New tests for every adjusted path: non-string sanitize drop, empty
post-filter chain rethrow, case-insensitive contains / withLink /
without, non-array configurationIdentifiers bail-out.
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Contributor
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
This was referenced Apr 22, 2026
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
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
Adds a per-configuration fallback chain so a retryable provider failure (connection error, HTTP 5xx, 429 rate-limit) transparently re-runs the request against the next configuration in the chain.
FallbackChainDTO (readonly value object, JSON-persisted) stored on a new nullabletx_nrllm_configuration.fallback_chaincolumnFallbackChainExecutorservice wrapsLlmServiceManager::chatWithConfiguration()/completeWithConfiguration(); only retryable exceptions trigger fallback — misconfig / 4xx / unsupported-feature bubble up immediatelyFallbackChainExhaustedExceptioncarries every attempt so callers can surface the full failure sequenceLlmServiceManagerkeeps existing tests passing without changesTest plan
Design notes
See ADR-021 for the Retryable vs. non-retryable decision, why streaming is excluded, and why fallback is shallow (not recursive).