feat: add LLM provider fallback configuration - #3477
Conversation
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThis PR introduces an LLM provider fallback mechanism that allows configuration of named provider fallbacks. It adds a resolver component that chains through fallback providers (with a maximum depth limit of 10) to find a valid provider, updates prompt service to resolve provider names, and introduces a new exception for missing providers. Changes
Sequence DiagramsequenceDiagram
actor Client
participant PromptController
participant PromptService
participant LlmProviderResolver
participant LlmProviderRepository
participant LlmPropertiesService
participant LlmProviderService
Client->>PromptController: GET /prompt/{id}
PromptController->>PromptService: findPromptWithResolvedProvider(projectId, promptId)
PromptService->>PromptService: fetch prompt from DB
PromptService->>PromptService: withResolvedProviderName(prompt)
alt providerName is not empty
PromptService->>LlmProviderResolver: resolveProviderName(orgId, providerName)
loop Until provider found or max depth (10) reached
LlmProviderResolver->>LlmProviderRepository: check if provider exists
LlmProviderRepository-->>LlmProviderResolver: provider found?
alt Provider exists
LlmProviderResolver-->>PromptService: return resolved name
else Provider not found
LlmProviderResolver->>LlmPropertiesService: getFallbackProviderName(current)
LlmPropertiesService-->>LlmProviderResolver: fallback name or null
alt Fallback exists
Note over LlmProviderResolver: Record attempt, continue loop
else No fallback
LlmProviderResolver-->>PromptService: throw LlmProviderNotFoundException
end
end
end
alt Exception thrown
PromptService->>PromptService: catch, keep original providerName
else Resolution successful
PromptService->>PromptService: update prompt.providerName
end
end
PromptService->>PromptService: detach entity from session
PromptService-->>PromptController: return prompt with resolved provider
PromptController-->>Client: HTTP 200 + prompt
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/backend/app/src/main/kotlin/io/tolgee/ee/component/llm/AnthropicApiService.kt (1)
119-121:⚠️ Potential issue | 🟡 MinorFix misleading comment referencing AzureCognitive.
The comment mentions "AzureCognitive JSON response objects" but this is the
AnthropicApiService. This appears to be a copy-paste artifact.📝 Proposed fix
/** - * Data structure for mapping the AzureCognitive JSON response objects. + * Data structures for mapping the Anthropic API request and response objects. */ companion object {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/backend/app/src/main/kotlin/io/tolgee/ee/component/llm/AnthropicApiService.kt` around lines 119 - 121, Update the misleading KDoc comment in AnthropicApiService (the comment block above the data structure mapping) to reference Anthropic JSON response objects instead of "AzureCognitive"; locate the comment in AnthropicApiService.kt and replace or reword the text to accurately describe the data structure used for mapping Anthropic API JSON responses (or remove the incorrect provider name entirely).
🧹 Nitpick comments (3)
ee/backend/app/src/main/kotlin/io/tolgee/ee/service/LlmProviderResolver.kt (1)
33-39: Consider caching provider list to avoid repeated database queries.
providerExistscallsllmProviderRepository.getAll(organizationId)on every iteration of the fallback chain. With a max depth of 10, this could result in up to 10 database queries for a single resolution.♻️ Proposed optimization
fun resolveProviderName( organizationId: Long, provider: String, ): String { var current = provider val tried = mutableSetOf<String>() + val orgProviders = llmProviderRepository.getAll(organizationId).map { it.name }.toSet() + val globalProviders = llmPropertiesService.getProviders().map { it.name }.toSet() repeat(MAX_FALLBACK_DEPTH) { - if (providerExists(organizationId, current)) { + if (current in orgProviders || current in globalProviders) { return current } tried.add(current) val fallback = llmPropertiesService.getFallbackProviderName(current) if (fallback == null || fallback in tried) { throw LlmProviderNotFoundException(provider) } current = fallback } throw LlmProviderNotFoundException(provider) } - - private fun providerExists( - organizationId: Long, - name: String, - ): Boolean { - if (llmProviderRepository.getAll(organizationId).any { it.name == name }) return true - return llmPropertiesService.getProviders().any { it.name == name } - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/backend/app/src/main/kotlin/io/tolgee/ee/service/LlmProviderResolver.kt` around lines 33 - 39, providerExists currently calls llmProviderRepository.getAll(organizationId) on every check which can trigger many DB queries; change it to fetch and cache the provider lists once per resolution (e.g. in LlmProviderResolver before the fallback loop) by calling llmProviderRepository.getAll(organizationId) and llmPropertiesService.getProviders() into local variables/collections and then use those cached lists inside providerExists (or replace providerExists with a lookup against the cached collection) so subsequent checks reuse the in-memory data rather than re-querying the DB.ee/backend/app/src/main/kotlin/io/tolgee/ee/service/prompt/PromptServiceEeImpl.kt (1)
299-312: Consider logging when provider resolution falls back to original name.The caught
LlmProviderNotFoundExceptionis silently swallowed. While falling back to the original name is intentional (as documented), logging at DEBUG/WARN level would help diagnose configuration issues in production.🔧 Proposed enhancement
+import io.tolgee.util.Logging +import io.tolgee.util.logger -class PromptServiceEeImpl( +class PromptServiceEeImpl( ... -) : PromptService { +) : PromptService, Logging { private fun withResolvedProviderName(prompt: Prompt): Prompt { if (prompt.providerName.isEmpty()) return prompt val organizationId = prompt.project.organizationOwner.id val resolvedName = try { llmProviderResolver.resolveProviderName(organizationId, prompt.providerName) } catch (e: LlmProviderNotFoundException) { + logger.debug("Provider '${prompt.providerName}' not found, keeping original name", e) prompt.providerName } entityManager.detach(prompt) prompt.providerName = resolvedName return prompt }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/backend/app/src/main/kotlin/io/tolgee/ee/service/prompt/PromptServiceEeImpl.kt` around lines 299 - 312, In withResolvedProviderName(prompt: Prompt) add a log call inside the catch for LlmProviderNotFoundException so the fallback is recorded (include organizationId and the original prompt.providerName and the exception), e.g. use the class logger to emit a DEBUG or WARN entry, but preserve the current behavior of returning the original name; update only the catch block in withResolvedProviderName to log the failure before assigning prompt.providerName = resolvedName.ee/backend/tests/src/test/kotlin/io/tolgee/ee/unit/LlmProviderFallbackTest.kt (1)
89-135: Consider adding a test for the MAX_FALLBACK_DEPTH limit to verify deeply nested chains exceeding the limit are properly rejected.The tests provide good coverage of essential fallback scenarios: direct hit, single fallback, chained fallbacks (depth 2), missing fallback, and circular chain detection. However, the
MAX_FALLBACK_DEPTHconstant (defined as 10 inLlmProviderResolver) has no corresponding test that validates the boundary—a chain longer than 10 hops should be rejected withLlmProviderNotFoundException.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/backend/tests/src/test/kotlin/io/tolgee/ee/unit/LlmProviderFallbackTest.kt` around lines 89 - 135, Add a unit test in LlmProviderFallbackTest that verifies LlmProviderResolver.MAX_FALLBACK_DEPTH is enforced by creating a fallback chain longer than MAX_FALLBACK_DEPTH (e.g., generate providers "p0" -> "p1" -> ... -> "pN" where N > LlmProviderResolver.MAX_FALLBACK_DEPTH), call service.callProvider with the initial provider, and assert that it throws LlmProviderNotFoundException; use the existing helpers setupFallbacks and setupServerProviders to register the chain and keep the test name descriptive like `exceeds_max_fallback_depth_throws`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@ee/backend/app/src/main/kotlin/io/tolgee/ee/component/llm/AnthropicApiService.kt`:
- Around line 119-121: Update the misleading KDoc comment in AnthropicApiService
(the comment block above the data structure mapping) to reference Anthropic JSON
response objects instead of "AzureCognitive"; locate the comment in
AnthropicApiService.kt and replace or reword the text to accurately describe the
data structure used for mapping Anthropic API JSON responses (or remove the
incorrect provider name entirely).
---
Nitpick comments:
In `@ee/backend/app/src/main/kotlin/io/tolgee/ee/service/LlmProviderResolver.kt`:
- Around line 33-39: providerExists currently calls
llmProviderRepository.getAll(organizationId) on every check which can trigger
many DB queries; change it to fetch and cache the provider lists once per
resolution (e.g. in LlmProviderResolver before the fallback loop) by calling
llmProviderRepository.getAll(organizationId) and
llmPropertiesService.getProviders() into local variables/collections and then
use those cached lists inside providerExists (or replace providerExists with a
lookup against the cached collection) so subsequent checks reuse the in-memory
data rather than re-querying the DB.
In
`@ee/backend/app/src/main/kotlin/io/tolgee/ee/service/prompt/PromptServiceEeImpl.kt`:
- Around line 299-312: In withResolvedProviderName(prompt: Prompt) add a log
call inside the catch for LlmProviderNotFoundException so the fallback is
recorded (include organizationId and the original prompt.providerName and the
exception), e.g. use the class logger to emit a DEBUG or WARN entry, but
preserve the current behavior of returning the original name; update only the
catch block in withResolvedProviderName to log the failure before assigning
prompt.providerName = resolvedName.
In
`@ee/backend/tests/src/test/kotlin/io/tolgee/ee/unit/LlmProviderFallbackTest.kt`:
- Around line 89-135: Add a unit test in LlmProviderFallbackTest that verifies
LlmProviderResolver.MAX_FALLBACK_DEPTH is enforced by creating a fallback chain
longer than MAX_FALLBACK_DEPTH (e.g., generate providers "p0" -> "p1" -> ... ->
"pN" where N > LlmProviderResolver.MAX_FALLBACK_DEPTH), call
service.callProvider with the initial provider, and assert that it throws
LlmProviderNotFoundException; use the existing helpers setupFallbacks and
setupServerProviders to register the chain and keep the test name descriptive
like `exceeds_max_fallback_depth_throws`.
When an LLM provider is not found, Tolgee now follows a configurable fallback chain before failing. Admins can map provider names to fallbacks via tolgee.llm.fallbacks config property. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a provider is delisted and replaced by a successor with a fallback configured, the stored provider name becomes stale. Resolve it through the fallback chain at the service level so the frontend sees a valid provider name. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fa48541 to
d6772a7
Compare
Summary
tolgee.llm.fallbacksconfig property that maps provider names to fallback providersLlmProviderResolvercomponentTest plan
Summary by CodeRabbit
Release Notes