Fix/litellm custom provider override

[Taooo-habitus]

This came out of trying to run PR-Agent against Snowflake Cortex through its OpenAI-compatible API.

The main change is adding support for LITELLM__CUSTOM_LLM_PROVIDER, so PR-Agent can keep raw hosted model IDs like claude-sonnet-4-5 while still telling LiteLLM which provider path to use.

I also added a small Snowflake-specific streaming workaround. In my testing, non-streaming responses on the Cortex OpenAI-compatible endpoint could trip LiteLLM response normalization, so forcing streaming for that path avoids the failure.

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Add LiteLLM custom provider override and configurable streaming support

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Add support for LITELLM__CUSTOM_LLM_PROVIDER to preserve raw model IDs
• Implement configurable forced streaming for OpenAI-compatible endpoints
• Add validation and normalization for streaming configuration settings
• Comprehensive test coverage for custom provider and streaming behavior

Diagram

flowchart LR
  A["chat_completion"] -->|reads custom_llm_provider| B["kwargs setup"]
  B -->|passes to| C["_get_completion"]
  C -->|checks force_streaming config| D["Streaming Decision"]
  D -->|matches provider & api_base| E["Enable Streaming"]
  D -->|no match| F["Standard Response"]
  E -->|handles| G["_handle_streaming_response"]
  F -->|handles| H["acompletion"]

Loading

File Changes

1. pr_agent/algo/ai_handlers/litellm_ai_handler.py ✨ Enhancement +46/-2

Add custom provider override and streaming configuration

• Added support for custom_llm_provider setting to pass through to LiteLLM without rewriting model
 IDs
• Implemented configurable forced streaming logic in _get_completion method based on provider and
 API base URL matching
• Added validation for force_streaming_api_base_substrings configuration with warning for invalid
 types
• Enhanced logging to distinguish between required model streaming and compatibility-driven
 streaming

pr_agent/algo/ai_handlers/litellm_ai_handler.py

---

2. tests/unittest/test_litellm_custom_provider.py 🧪 Tests +306/-0

Add comprehensive LiteLLM custom provider tests

• Created comprehensive test suite with 10 test cases covering custom provider functionality
• Tests verify custom provider forwarding, model ID preservation, and streaming behavior
• Tests validate configuration normalization, edge cases, and invalid input handling
• Tests ensure warning messages are logged for misconfigured settings

tests/unittest/test_litellm_custom_provider.py

---

3. pr_agent/settings/configuration.toml ⚙️ Configuration changes +3/-0

Add LiteLLM streaming and provider configuration options

• Added three new configuration options for LiteLLM custom provider support
• custom_llm_provider: allows specifying custom provider override
• force_streaming_custom_llm_provider: enables conditional streaming for specific providers
• force_streaming_api_base_substrings: list of API base URL substrings triggering forced streaming

pr_agent/settings/configuration.toml

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Per-request warning spam ☑ 🐞 Bug ◔ Observability

Description

When force_streaming_api_base_substrings is misconfigured as a non-empty non-collection value
(e.g., a string), _get_completion() emits a warning on every invocation, which can flood logs and
obscure other warnings. Since chat_completion() calls _get_completion() for each request, this
repeats once per request until the config is fixed.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R467-472]

+            if raw_force_streaming_api_base_substrings:
+                get_logger().warning(
+                    "LITELLM.FORCE_STREAMING_API_BASE_SUBSTRINGS must be a list, tuple, or set. "
+                    "Ignoring invalid value."
+                )
+            force_streaming_api_base_substrings = []

Evidence

The warning is inside the hot path (_get_completion), and chat_completion awaits
_get_completion(**kwargs) for each LLM call. The new unit test asserts that the warning is emitted
for an invalid (string) configuration, which implies this will occur each time _get_completion
runs under that misconfiguration.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[446-478]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[416-423]
tests/unittest/test_litellm_custom_provider.py[249-306]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`LiteLLMAIHandler._get_completion()` warns every time it runs if `litellm.force_streaming_api_base_substrings` is set to a non-empty, non-collection value (e.g., a string). Because `_get_completion()` is invoked per request, this can spam logs.
## Issue Context
The code already ignores invalid values safely; the problem is repeated warning emission on a hot path.
## Fix Focus Areas
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[446-472]
## Suggested approach
- Parse/validate `force_streaming_api_base_substrings` once (e.g., in `__init__`) and store a normalized list on `self`.
- If runtime validation must remain, rate-limit the warning (e.g., emit once per process using a module-level flag, or once per handler instance with `self._warned_invalid_force_streaming_substrings = True`).
- Continue to fall back to `[]` for invalid values.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. litellm settings access inconsistent ☑ 📘 Rule violation ⚙ Maintainability

Description

New code reads LiteLLM settings via get_settings().get("litellm.*") (lowercase dotted path) while
other LiteLLM settings in the same module are accessed via get_settings().litellm... and/or
uppercase keys, which risks misreading configuration depending on Dynaconf normalization and makes
behavior harder to reason about.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R398-403]

+            custom_llm_provider = str(
+                get_settings().get("litellm.custom_llm_provider", "") or ""
+            ).strip().lower()
+            if custom_llm_provider:
+                kwargs["custom_llm_provider"] = custom_llm_provider
+

Evidence

PR Compliance ID 17 requires a single consistent settings access pattern and validated/normalized
configuration values. The added code introduces additional get_settings().get("litellm.*")
accesses (and log text referencing uppercase key names), increasing inconsistency and potential
key-path/casing mismatch.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[398-403]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[454-472]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new LiteLLM configuration reads are using `get_settings().get("litellm.*")` (lowercase dotted keys), which is inconsistent with the module’s other settings access patterns (e.g., `get_settings().litellm...` and uppercase key checks). This inconsistency can lead to subtle configuration bugs if key normalization differs across sources.
## Issue Context
The PR introduces `custom_llm_provider` and streaming-force settings. These should follow the project’s single, consistent settings access pattern and use consistent key naming/casing.
## Fix Focus Areas
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[398-403]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[451-472]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. None becomes substring ☑ 🐞 Bug ☼ Reliability

Description

In LiteLLMAIHandler.__init__, force_streaming_api_base_substrings elements are coerced via
str(value), so a None/null entry becomes the literal substring "none" and is kept, which can
(rarely) cause unintended force-streaming if the api_base contains that substring. This is an
avoidable misconfiguration foot-gun because the filter checks the stringified value, not the
original value/type.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R161-166]

+        if isinstance(raw_force_streaming_api_base_substrings, (list, tuple, set)):
+            self.force_streaming_api_base_substrings = [
+                str(value).strip().lower()
+                for value in raw_force_streaming_api_base_substrings
+                if str(value).strip()
+            ]

Evidence

The new list normalization stringifies each element and filters based on the stringified value being
non-empty; this will keep values like None as "none" after lowercasing, instead of ignoring them.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[155-173]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`force_streaming_api_base_substrings` normalization currently does `str(value).strip().lower()` and filters on the resulting string being non-empty. If the configured collection contains `None`/`null`, it becomes the substring `"none"` and is retained, which can lead to unexpected matching behavior.
### Issue Context
This occurs during `LiteLLMAIHandler.__init__` when building `self.force_streaming_api_base_substrings`.
### Fix Focus Areas
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[155-173]
### Expected change
Filter out `None` (and optionally non-strings) before string conversion, e.g.:
- `if value is None: continue`
- or `if not isinstance(value, str): continue` (if you want to be strict)
Then apply `.strip().lower()` to the remaining values.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Redundant api_base get ☑ 🐞 Bug ⚙ Maintainability

Description

In _get_completion, api_base_value is retrieved but api_base is computed from kwargs.get("api_base")
again instead of reusing the local, making the logic harder to read and easier to accidentally
diverge in future edits. This is purely a maintainability issue.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R471-472]

+        api_base_value = kwargs.get("api_base")
+        api_base = kwargs.get("api_base").strip().lower() if isinstance(api_base_value, str) else ""

Evidence

The new code stores api_base_value = kwargs.get('api_base') and then re-fetches the same key to
call .strip().lower(), which is unnecessary duplication.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[465-478]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`api_base_value` is fetched from `kwargs` but the next line re-fetches `kwargs.get('api_base')` instead of using `api_base_value`.
### Issue Context
This is in `_get_completion` while computing `api_base` for force-streaming matching.
### Fix Focus Areas
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[465-478]
### Expected change
Compute `api_base` from the cached value, e.g.:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Previous review results

Review updated until commit 9b9cb64

Results up to commit N/A

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Per-request warning spam ☑ 🐞 Bug ◔ Observability

Description

When force_streaming_api_base_substrings is misconfigured as a non-empty non-collection value
(e.g., a string), _get_completion() emits a warning on every invocation, which can flood logs and
obscure other warnings. Since chat_completion() calls _get_completion() for each request, this
repeats once per request until the config is fixed.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R467-472]

+            if raw_force_streaming_api_base_substrings:
+                get_logger().warning(
+                    "LITELLM.FORCE_STREAMING_API_BASE_SUBSTRINGS must be a list, tuple, or set. "
+                    "Ignoring invalid value."
+                )
+            force_streaming_api_base_substrings = []

Evidence

The warning is inside the hot path (_get_completion), and chat_completion awaits
_get_completion(**kwargs) for each LLM call. The new unit test asserts that the warning is emitted
for an invalid (string) configuration, which implies this will occur each time _get_completion
runs under that misconfiguration.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[446-478]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[416-423]
tests/unittest/test_litellm_custom_provider.py[249-306]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`LiteLLMAIHandler._get_completion()` warns every time it runs if `litellm.force_streaming_api_base_substrings` is set to a non-empty, non-collection value (e.g., a string). Because `_get_completion()` is invoked per request, this can spam logs.
## Issue Context
The code already ignores invalid values safely; the problem is repeated warning emission on a hot path.
## Fix Focus Areas
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[446-472]
## Suggested approach
- Parse/validate `force_streaming_api_base_substrings` once (e.g., in `__init__`) and store a normalized list on `self`.
- If runtime validation must remain, rate-limit the warning (e.g., emit once per process using a module-level flag, or once per handler instance with `self._warned_invalid_force_streaming_substrings = True`).
- Continue to fall back to `[]` for invalid values.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. litellm settings access inconsistent ☑ 📘 Rule violation ⚙ Maintainability

Description

New code reads LiteLLM settings via get_settings().get("litellm.*") (lowercase dotted path) while
other LiteLLM settings in the same module are accessed via get_settings().litellm... and/or
uppercase keys, which risks misreading configuration depending on Dynaconf normalization and makes
behavior harder to reason about.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R398-403]

+            custom_llm_provider = str(
+                get_settings().get("litellm.custom_llm_provider", "") or ""
+            ).strip().lower()
+            if custom_llm_provider:
+                kwargs["custom_llm_provider"] = custom_llm_provider
+

Evidence

PR Compliance ID 17 requires a single consistent settings access pattern and validated/normalized
configuration values. The added code introduces additional get_settings().get("litellm.*")
accesses (and log text referencing uppercase key names), increasing inconsistency and potential
key-path/casing mismatch.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[398-403]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[454-472]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new LiteLLM configuration reads are using `get_settings().get("litellm.*")` (lowercase dotted keys), which is inconsistent with the module’s other settings access patterns (e.g., `get_settings().litellm...` and uppercase key checks). This inconsistency can lead to subtle configuration bugs if key normalization differs across sources.
## Issue Context
The PR introduces `custom_llm_provider` and streaming-force settings. These should follow the project’s single, consistent settings access pattern and use consistent key naming/casing.
## Fix Focus Areas
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[398-403]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[451-472]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. None becomes substring ☑ 🐞 Bug ☼ Reliability

Description

In LiteLLMAIHandler.__init__, force_streaming_api_base_substrings elements are coerced via
str(value), so a None/null entry becomes the literal substring "none" and is kept, which can
(rarely) cause unintended force-streaming if the api_base contains that substring. This is an
avoidable misconfiguration foot-gun because the filter checks the stringified value, not the
original value/type.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R161-166]

+        if isinstance(raw_force_streaming_api_base_substrings, (list, tuple, set)):
+            self.force_streaming_api_base_substrings = [
+                str(value).strip().lower()
+                for value in raw_force_streaming_api_base_substrings
+                if str(value).strip()
+            ]

Evidence

The new list normalization stringifies each element and filters based on the stringified value being
non-empty; this will keep values like None as "none" after lowercasing, instead of ignoring them.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[155-173]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`force_streaming_api_base_substrings` normalization currently does `str(value).strip().lower()` and filters on the resulting string being non-empty. If the configured collection contains `None`/`null`, it becomes the substring `"none"` and is retained, which can lead to unexpected matching behavior.
### Issue Context
This occurs during `LiteLLMAIHandler.__init__` when building `self.force_streaming_api_base_substrings`.
### Fix Focus Areas
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[155-173]
### Expected change
Filter out `None` (and optionally non-strings) before string conversion, e.g.:
- `if value is None: continue`
- or `if not isinstance(value, str): continue` (if you want to be strict)
Then apply `.strip().lower()` to the remaining values.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Redundant api_base get ☑ 🐞 Bug ⚙ Maintainability

Description

In _get_completion, api_base_value is retrieved but api_base is computed from kwargs.get("api_base")
again instead of reusing the local, making the logic harder to read and easier to accidentally
diverge in future edits. This is purely a maintainability issue.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R471-472]

+        api_base_value = kwargs.get("api_base")
+        api_base = kwargs.get("api_base").strip().lower() if isinstance(api_base_value, str) else ""

Evidence

The new code stores api_base_value = kwargs.get('api_base') and then re-fetches the same key to
call .strip().lower(), which is unnecessary duplication.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[465-478]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`api_base_value` is fetched from `kwargs` but the next line re-fetches `kwargs.get('api_base')` instead of using `api_base_value`.
### Issue Context
This is in `_get_completion` while computing `api_base` for force-streaming matching.
### Fix Focus Areas
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[465-478]
### Expected change
Compute `api_base` from the cached value, e.g.:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-free-for-open-source-projects]

Persistent review updated to latest commit 5a9ebe2

[qodo-free-for-open-source-projects]

Persistent review updated to latest commit 60d52db

[qodo-free-for-open-source-projects]

Persistent review updated to latest commit 9b9cb64