fix: prevent dummy_key from overriding provider-specific API keys

[yanukadeneth99]

Summary

• Issue: When using Anthropic without OpenAI configured, the placeholder litellm.api_key (DUMMY_LITELLM_API_KEY) was being forwarded to litellm calls, preventing litellm from using the provider-specific litellm.anthropic_key internally. This caused AuthenticationError: invalid x-api-key. Recommending this specific fix due to it catching False, empty strings, or 0

• Fix: Added a guard condition in chat_completion() to only forward litellm.api_key if it's a real, valid key, rejecting both None and the placeholder sentinel value.

• Tests: Wrote four comprehensive test cases to verify the guard blocks dummy/None keys, allow real keys, and specifically replicate the Anthropic-without-OpenAI scenario (litellm.AuthenticationError: invalid x-api-key when using Anthropic #2292).

Issue

Fixes a bug where the LiteLLM fallback dummy key was being unconditionally passed to all AI providers, breaking authentication for non-OpenAI models (Anthropic, Groq, etc.).

References: #2042

The Problem

When OPENAI_API_KEY is not configured:

• Line 42 sets litellm.api_key = "dummy_key" as a fallback
• Line 409 unconditionally passed this to ALL providers via kwargs["api_key"]
• Non-OpenAI providers (e.g., Anthropic) received the dummy key instead of their own configured keys
• Result: Authentication failures with "invalid x-api-key" errors

Example Scenario

# User configures only Anthropic (no OpenAI)
litellm.anthropic_key = "sk-ant-v0-..."  # Real key set

# But in kwargs:
kwargs["api_key"] = "dummy_key"  # Overrides provider-specific key

# LiteLLM sees explicit api_key parameter and uses it, ignoring anthropic_key
# → Authentication fails

The Solution

Only pass api_key in kwargs if it's a real key, not the dummy fallback:

# Before
kwargs["api_key"] = litellm.api_key

# After
if litellm.api_key and litellm.api_key != "dummy_key":
   kwargs["api_key"] = litellm.api_key

When api_key is not in kwargs, LiteLLM falls back to provider-specific keys (litellm.anthropic_key, litellm.groq_key, etc.), which is the correct behavior.

Impact Analysis

✅ Anthropic-only config - Now works (was broken)
✅ OpenAI-only config - Still works (unchanged)
✅ Azure OpenAI - Still works (uses azure_key)
✅ Groq/X-AI/Ollama - Still works (real keys passed)
✅ Azure AD tokens - Still works (real tokens passed)
✅ Bedrock - Unaffected (uses AWS env vars)

Testing

Comprehensive test suite executed to verify no regressions:

• ✅ 287 total unit tests — All passed
• ✅ 25 LiteLLM-specific tests — All passed
  • GPT-5 reasoning_effort configuration
  • Model detection and routing
  • Provider-specific key handling
  • Logging behavior validation
• ✅ Provider integration tests — All passed
  • GitHub, GitLab, Bitbucket, Azure DevOps
  • Token calculation for multiple Claude models
  • File handling and encoding
• ✅ Configuration tests — All passed
  • YAML parsing
  • Secret provider factory
  • Edge cases and error handling

Test Environment: Python 3.13.11
Test Duration: 40.30 seconds
Result: 287 passed, 0 failed, 0 skipped

Root Cause

The bug exists in the initialization sequence:

1. Line 38-40: OpenAI key is set (if configured)
2. Line 41-42: If OpenAI is not configured, litellm.api_key = "dummy_key" (fallback)
3. Line 68: Anthropic key is set via litellm.anthropic_key (provider-specific)
4. Line 409 (OLD): Unconditionally passed api_key="dummy_key" to kwargs
5. Result: Even though litellm.anthropic_key is set correctly, the explicit kwargs["api_key"] takes precedence in LiteLLM's parameter handling

The fix respects LiteLLM's design: provider-specific attributes take precedence when the generic api_key is just a fallback placeholder.

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

Review Summary by Qodo

Prevent dummy_key from overriding provider-specific API keys

🐞 Bug fix

Walkthroughs

Description

• Prevent dummy_key from overriding provider-specific API keys
• Only pass api_key to kwargs if it's a real key, not fallback
• Fixes authentication failures for Anthropic and other non-OpenAI providers
• Allows LiteLLM to use correct provider-specific keys when generic key is unavailable

Diagram

flowchart LR
  A["litellm.api_key = dummy_key<br/>fallback set"] --> B{"Is api_key real<br/>and not dummy?"}
  B -->|Yes| C["Pass api_key<br/>to kwargs"]
  B -->|No| D["Skip api_key<br/>in kwargs"]
  C --> E["LiteLLM uses<br/>explicit api_key"]
  D --> F["LiteLLM uses<br/>provider-specific key"]
  F --> G["✓ Anthropic/Groq<br/>authentication works"]

Loading

File Changes

1. pr_agent/algo/ai_handlers/litellm_ai_handler.py 🐞 Bug fix +2/-1

Conditional api_key assignment to prevent dummy override

• Added conditional check before passing api_key to kwargs
• Only includes api_key if it's a real key, not the dummy fallback value
• Prevents dummy_key from overriding provider-specific keys like litellm.anthropic_key
• Allows LiteLLM to correctly route to provider-specific authentication when generic key is
 unavailable

pr_agent/algo/ai_handlers/litellm_ai_handler.py

---

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

Code Review by Qodo

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

1. Broken LiteLLM response mock 🐞 Bug ☼ Reliability ⭐ New

Description

The new unit tests’ _mock_response() assigns a lambda to mock.__getitem__, but
_get_completion() requires response["choices"] to return a real list and checks
len(response["choices"]); this mock setup is likely not honored by response[...] and can cause
openai.APIError during the tests. As written, the tests are unstable and may not actually validate
the intended behavior.

Code

tests/unittest/test_litellm_api_key_guard.py[R37-44]

+def _mock_response():
+    """Minimal acompletion response."""
+    mock = MagicMock()
+    mock.__getitem__ = lambda self, key: {
+        "choices": [{"message": {"content": "ok"}, "finish_reason": "stop"}]
+    }[key]
+    mock.dict.return_value = {"choices": [{"message": {"content": "ok"}, "finish_reason": "stop"}]}
+    return mock

Evidence

_get_completion() executes the non-streaming branch for models like gpt-4o (streaming-required
list does not include it), and indexes response["choices"] then calls len(...); therefore the
mocked response must correctly implement subscription to return a concrete list. The current
_mock_response() sets mock.__getitem__ via direct assignment rather than configuring the mock’s
magic method behavior, which can result in response["choices"] not returning the intended list and
triggering the openai.APIError path.

tests/unittest/test_litellm_api_key_guard.py[37-44]
pr_agent/algo/init.py[282-285]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[440-459]

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 tests’ `_mock_response()` does not reliably satisfy the production access pattern `response["choices"]` + `len(response["choices"])` in `_get_completion()`. This can cause the tests to fail (or validate the wrong behavior) depending on mock internals.

### Issue Context
`LiteLLMAIHandler._get_completion()` expects an object that:
- supports `response["choices"]` returning a list
- supports `response.dict()` returning a dict-like structure for logging

### Fix Focus Areas
- tests/unittest/test_litellm_api_key_guard.py[37-44]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[440-459]

### Implementation direction
Update `_mock_response()` to use a backing dict and configure magic methods in a standard way, e.g.:
- `data = {"choices": [{"message": {"content": "ok"}, "finish_reason": "stop"}]}`
- `mock = MagicMock()`
- `mock.__getitem__.side_effect = data.__getitem__`
- `mock.dict.return_value = data`
This ensures `response["choices"]` returns a real list and the handler’s logging path still works.

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

---

2. Secret-like keys in tests 📘 Rule violation ⛨ Security

Description

The new unit test hardcodes API-key-looking strings (e.g., sk-ant-..., sk-...), which can
trigger secret scanners and violates the no-committed-secrets requirement. Even if intended as fake
values, committed key-like tokens increase leakage/audit risk and should be replaced with clearly
non-secret dummy strings.

Code

tests/unittest/test_litellm_api_key_guard.py[R58-122]

+    anthropic_key = "sk-ant-test-real-key"
+    return type("Settings", (), {
+        "config": type("Config", (), {
+            "reasoning_effort": None,
+            "ai_timeout": 30,
+            "custom_reasoning_model": False,
+            "max_model_tokens": 32000,
+            "verbosity_level": 0,
+            "seed": -1,
+            "get": lambda self, key, default=None: default,
+        })(),
+        "litellm": type("LiteLLM", (), {
+            "get": lambda self, key, default=None: default,
+        })(),
+        "anthropic": type("Anthropic", (), {
+            "key": anthropic_key
+        })(),
+        # Return the Anthropic key when settings.get("ANTHROPIC.KEY") is called
+        "get": lambda self, key, default=None: (
+            anthropic_key if key == "ANTHROPIC.KEY" else default
+        ),
+    })()
+
+
+class TestApiKeyGuard:
+
+    @pytest.mark.asyncio
+    async def test_dummy_key_not_forwarded(self, monkeypatch):
+        """api_key must NOT appear in kwargs when litellm.api_key is the placeholder."""
+        monkeypatch.setattr(litellm, "api_key", DUMMY_LITELLM_API_KEY)
+
+        with patch("pr_agent.algo.ai_handlers.litellm_ai_handler.acompletion",
+                   new_callable=AsyncMock) as mock_call:
+            mock_call.return_value = _mock_response()
+            handler = LiteLLMAIHandler()
+            await handler.chat_completion(model="gpt-4o", system="sys", user="usr")
+
+        assert "api_key" not in mock_call.call_args[1]
+
+    @pytest.mark.asyncio
+    async def test_none_api_key_not_forwarded(self, monkeypatch):
+        """api_key must NOT appear in kwargs when litellm.api_key is None.
+
+        This is the OpenAI-key path: OPENAI.KEY sets litellm.openai_key,
+        leaving litellm.api_key at None.
+        """
+        monkeypatch.setattr(litellm, "api_key", None)
+
+        with patch("pr_agent.algo.ai_handlers.litellm_ai_handler.acompletion",
+                   new_callable=AsyncMock) as mock_call:
+            mock_call.return_value = _mock_response()
+            handler = LiteLLMAIHandler()
+            await handler.chat_completion(model="gpt-4o", system="sys", user="usr")
+
+        assert "api_key" not in mock_call.call_args[1]
+
+    @pytest.mark.asyncio
+    async def test_real_key_forwarded(self, monkeypatch):
+        """api_key IS injected when a real provider key is in litellm.api_key (e.g. Groq, XAI).
+
+        The key is set after __init__ to simulate a provider having stored its key there
+        during initialization, without triggering the placeholder value in __init__.
+        """
+        real_key = "sk-test-real-provider-key"
+

Evidence

PR Compliance ID 13 prohibits committed secrets/tokens in code and tests; the added test file
includes hardcoded key-like token strings (sk-ant-test-real-key, sk-test-real-provider-key) on
newly added lines.

AGENTS.md
tests/unittest/test_litellm_api_key_guard.py[58-122]

Agent prompt

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

## Issue description
A new unit test hardcodes API-key-looking strings (e.g., `sk-ant-test-real-key`, `sk-test-real-provider-key`). Even if fake, these can be flagged as secrets and violate the repo policy.
## Issue Context
The test only needs distinct, non-empty identifiers to validate the `api_key` forwarding guard; it does not require realistic key formats.
## Fix Focus Areas
- tests/unittest/test_litellm_api_key_guard.py[58-59]
- tests/unittest/test_litellm_api_key_guard.py[121-122]

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

---

3. Flaky Anthropic guard test ☑ 🐞 Bug ≡ Correctness

Description

test_anthropic_key_not_shadowed_by_dummy_key asserts LiteLLMAIHandler.__init__ sets litellm.api_key
to the dummy placeholder, but __init__ only does that when OPENAI_API_KEY is absent. If
OPENAI_API_KEY is set in CI or a developer environment, this assertion fails and the test becomes
flaky.

Code

tests/unittest/test_litellm_api_key_guard.py[R144-155]

+        # Override settings to simulate Anthropic configured, OpenAI not configured
+        monkeypatch.setattr(litellm_handler, "get_settings", _make_anthropic_settings)
+
+        with patch("pr_agent.algo.ai_handlers.litellm_ai_handler.acompletion",
+                   new_callable=AsyncMock) as mock_call:
+            mock_call.return_value = _mock_response()
+            handler = LiteLLMAIHandler()
+
+            # After init: litellm.api_key should be the dummy (OpenAI fallback),
+            # but litellm.anthropic_key is the real Anthropic key
+            assert litellm.api_key == DUMMY_LITELLM_API_KEY
+

Evidence

LiteLLMAIHandler.__init__ assigns the dummy key only under an env-guarded branch; the new test
asserts the dummy value without controlling OPENAI_API_KEY, so it will fail whenever that env var
exists.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[37-44]
tests/unittest/test_litellm_api_key_guard.py[134-165]

Agent prompt

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

## Issue description
`test_anthropic_key_not_shadowed_by_dummy_key` assumes `LiteLLMAIHandler.__init__` will set `litellm.api_key` to `DUMMY_LITELLM_API_KEY`, but that only happens when `OPENAI_API_KEY` is not present in `os.environ`. If `OPENAI_API_KEY` is set in the test environment, the test’s assertion becomes false and the test is flaky.
### Issue Context
The handler’s init uses:
- `elif 'OPENAI_API_KEY' not in os.environ: litellm.api_key = DUMMY_LITELLM_API_KEY`
The test currently does not delete or override `OPENAI_API_KEY` before instantiating `LiteLLMAIHandler`.
### Fix Focus Areas
- tests/unittest/test_litellm_api_key_guard.py[134-165]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[37-44]
### Suggested change
In `test_anthropic_key_not_shadowed_by_dummy_key`, ensure deterministic preconditions by deleting the env var before constructing the handler:
- `monkeypatch.delenv("OPENAI_API_KEY", raising=False)`
Optionally also reset `litellm.api_key` to a known value before init to avoid cross-test state (`monkeypatch.setattr(litellm, "api_key", None, raising=False)`).

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

---

4. None-key test not exercising path ☑ 🐞 Bug ⚙ Maintainability

Description

test_none_api_key_not_forwarded sets litellm.api_key=None before constructing LiteLLMAIHandler, but
__init__ can overwrite it with the dummy placeholder when OPENAI_API_KEY is absent, so the test can
pass without actually validating the intended “None is not injected” condition.

Code

tests/unittest/test_litellm_api_key_guard.py[R98-112]

+    async def test_none_api_key_not_forwarded(self, monkeypatch):
+        """api_key must NOT appear in kwargs when litellm.api_key is None.
+
+        This is the OpenAI-key path: OPENAI.KEY sets litellm.openai_key,
+        leaving litellm.api_key at None.
+        """
+        monkeypatch.setattr(litellm, "api_key", None)
+
+        with patch("pr_agent.algo.ai_handlers.litellm_ai_handler.acompletion",
+                   new_callable=AsyncMock) as mock_call:
+            mock_call.return_value = _mock_response()
+            handler = LiteLLMAIHandler()
+            await handler.chat_completion(model="gpt-4o", system="sys", user="usr")
+
+        assert "api_key" not in mock_call.call_args[1]

Evidence

The test claims it covers the “OpenAI-key path” where litellm.api_key remains None, but the
handler init assigns the dummy placeholder whenever OPENAI.KEY is not set and OPENAI_API_KEY is not
in the environment; the test does not enforce either condition needed to keep litellm.api_key as
None at call time.

tests/unittest/test_litellm_api_key_guard.py[98-112]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[37-43]

Agent prompt

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

## Issue description
`test_none_api_key_not_forwarded` intends to validate that when `litellm.api_key` is `None`, `chat_completion()` does not inject `api_key` into the `acompletion(**kwargs)` call. However, `LiteLLMAIHandler.__init__` may overwrite `litellm.api_key` with the dummy placeholder (depending on `OPENAI_API_KEY`), so the test may not actually run with `litellm.api_key is None`.
### Issue Context
`LiteLLMAIHandler.__init__` sets `litellm.api_key = DUMMY_LITELLM_API_KEY` when:
- `OPENAI.KEY` is not configured, and
- `OPENAI_API_KEY` is not in `os.environ`.
### Fix Focus Areas
- In the test, explicitly control env/settings so that after handler initialization, `litellm.api_key` is guaranteed to be `None` (e.g., set `OPENAI_API_KEY` in env so init won’t assign dummy, and/or set `litellm.api_key = None` after constructing the handler).
- Optionally assert `litellm.api_key is None` immediately before calling `chat_completion()` to ensure the test is validating the intended state.
- tests/unittest/test_litellm_api_key_guard.py[98-112]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[37-43]

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

---

5. Cross-provider key override 🐞 Bug ≡ Correctness

Description

chat_completion() still injects kwargs["api_key"] = litellm.api_key for any model whenever
litellm.api_key is non-dummy, but __init__ assigns litellm.api_key from provider-specific
settings like GROQ.KEY/XAI.KEY/OLLAMA.API_KEY. When the process switches models/providers
(e.g., via fallback_models), the wrong provider can receive a different provider’s key, causing
authentication failures.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R409-410]

+            if litellm.api_key and litellm.api_key != "dummy_key":
+                kwargs["api_key"] = litellm.api_key

Evidence

The handler stores multiple providers’ credentials in the single global litellm.api_key (e.g.,
Groq/XAI/Ollama/OpenRouter), and the modified code still forwards that single value as api_key for
*all* model calls whenever it’s non-dummy. Separately, the PR supports trying multiple models via
fallback_models, so a single run can invoke different providers, making this override path
reachable.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[36-86]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[409-413]
pr_agent/algo/pr_processing.py[320-354]
docs/docs/usage-guide/changing_a_model.md[62-66]

Agent prompt

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

## Issue description
`chat_completion()` currently sets `kwargs["api_key"]` from the global `litellm.api_key` whenever it’s non-dummy, regardless of which provider the `model` targets. Since `__init__` also reuses `litellm.api_key` to store multiple different providers’ keys, this can send the wrong key to the wrong provider (especially when `fallback_models` switches providers).
### Issue Context
- `litellm.api_key` is assigned from multiple provider settings (e.g., Groq/XAI/Ollama/OpenRouter).
- The request provider is encoded in the `model` string (e.g., `anthropic/...`, `ollama/...`, `openai/...`, `azure/...`).
- The system supports iterating across `fallback_models`, which can be provider-mixed.
### Fix Focus Areas
- Adjust `kwargs["api_key"]` injection to be provider-aware (e.g., only add a key that corresponds to the current model’s provider; or avoid setting `api_key` globally and instead rely on provider-specific configuration/attributes).
- Ensure multi-provider fallback sequences do not leak one provider’s key into another provider’s call.
### Fix Focus Areas (code references)
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[36-86]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[269-413]
- pr_agent/algo/pr_processing.py[320-354]

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

---

6. Sentinel key collision ☑ 🐞 Bug ⚙ Maintainability

Description

The guard treats the literal string "dummy_key" as a sentinel; if a real API key ever equals this
value, it will be suppressed and authentication will fail.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[18]

+DUMMY_LITELLM_API_KEY = "dummy_key"  # placeholder set when no OpenAI key is configured

Evidence

The sentinel is defined as a common short string and the forwarding logic uses direct string
equality to decide whether to drop the key, so any real key matching that string is
indistinguishable from the placeholder.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[17-19]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[410-413]

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 placeholder sentinel is the literal string `"dummy_key"`, which is compared via equality when deciding whether to forward `api_key`.
### Issue Context
If a real key ever equals `"dummy_key"`, it will be dropped and requests will fail.
### Fix Focus Areas
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[17-19]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[410-413]
### Suggested approach
Use a more unique sentinel value (e.g., a clearly namespaced constant unlikely to collide) and keep the comparison against that constant.

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

---

7. Magic dummy_key sentinel ☑ 🐞 Bug ⚙ Maintainability

Description

The sentinel string "dummy_key" is duplicated at assignment and at the filtering check. If the
sentinel ever changes, it can be updated in one place but missed in the other, reintroducing
dummy-key injection bugs.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R409-410]

+            if litellm.api_key and litellm.api_key != "dummy_key":
+                kwargs["api_key"] = litellm.api_key

Evidence

The code assigns litellm.api_key = "dummy_key" during initialization and later compares
litellm.api_key != "dummy_key" before injecting it into kwargs, without a shared constant tying
those usages together.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[38-43]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[409-410]

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 dummy key sentinel value is hard-coded in multiple places. This increases the chance of future edits updating only one occurrence and silently breaking the dummy-key filter.
### Issue Context
The sentinel is used both when setting `litellm.api_key` and when deciding whether to forward it into request kwargs.
### Fix Focus Areas
- Introduce a module-level constant (e.g., `DUMMY_LITELLM_API_KEY = "dummy_key"`) and use it for both the assignment and the comparison.
### Fix Focus Areas (code references)
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[38-43]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[409-410]

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

---

ⓘ The new review experience is currently in Beta. Learn more

Previous review results

Review updated until commit 295f5c4

Results up to commit N/A

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

1. Secret-like keys in tests 📘 Rule violation ⛨ Security

Description

The new unit test hardcodes API-key-looking strings (e.g., sk-ant-..., sk-...), which can
trigger secret scanners and violates the no-committed-secrets requirement. Even if intended as fake
values, committed key-like tokens increase leakage/audit risk and should be replaced with clearly
non-secret dummy strings.

Code

tests/unittest/test_litellm_api_key_guard.py[R58-122]

+    anthropic_key = "sk-ant-test-real-key"
+    return type("Settings", (), {
+        "config": type("Config", (), {
+            "reasoning_effort": None,
+            "ai_timeout": 30,
+            "custom_reasoning_model": False,
+            "max_model_tokens": 32000,
+            "verbosity_level": 0,
+            "seed": -1,
+            "get": lambda self, key, default=None: default,
+        })(),
+        "litellm": type("LiteLLM", (), {
+            "get": lambda self, key, default=None: default,
+        })(),
+        "anthropic": type("Anthropic", (), {
+            "key": anthropic_key
+        })(),
+        # Return the Anthropic key when settings.get("ANTHROPIC.KEY") is called
+        "get": lambda self, key, default=None: (
+            anthropic_key if key == "ANTHROPIC.KEY" else default
+        ),
+    })()
+
+
+class TestApiKeyGuard:
+
+    @pytest.mark.asyncio
+    async def test_dummy_key_not_forwarded(self, monkeypatch):
+        """api_key must NOT appear in kwargs when litellm.api_key is the placeholder."""
+        monkeypatch.setattr(litellm, "api_key", DUMMY_LITELLM_API_KEY)
+
+        with patch("pr_agent.algo.ai_handlers.litellm_ai_handler.acompletion",
+                   new_callable=AsyncMock) as mock_call:
+            mock_call.return_value = _mock_response()
+            handler = LiteLLMAIHandler()
+            await handler.chat_completion(model="gpt-4o", system="sys", user="usr")
+
+        assert "api_key" not in mock_call.call_args[1]
+
+    @pytest.mark.asyncio
+    async def test_none_api_key_not_forwarded(self, monkeypatch):
+        """api_key must NOT appear in kwargs when litellm.api_key is None.
+
+        This is the OpenAI-key path: OPENAI.KEY sets litellm.openai_key,
+        leaving litellm.api_key at None.
+        """
+        monkeypatch.setattr(litellm, "api_key", None)
+
+        with patch("pr_agent.algo.ai_handlers.litellm_ai_handler.acompletion",
+                   new_callable=AsyncMock) as mock_call:
+            mock_call.return_value = _mock_response()
+            handler = LiteLLMAIHandler()
+            await handler.chat_completion(model="gpt-4o", system="sys", user="usr")
+
+        assert "api_key" not in mock_call.call_args[1]
+
+    @pytest.mark.asyncio
+    async def test_real_key_forwarded(self, monkeypatch):
+        """api_key IS injected when a real provider key is in litellm.api_key (e.g. Groq, XAI).
+
+        The key is set after __init__ to simulate a provider having stored its key there
+        during initialization, without triggering the placeholder value in __init__.
+        """
+        real_key = "sk-test-real-provider-key"
+

Evidence

PR Compliance ID 13 prohibits committed secrets/tokens in code and tests; the added test file
includes hardcoded key-like token strings (sk-ant-test-real-key, sk-test-real-provider-key) on
newly added lines.

AGENTS.md
tests/unittest/test_litellm_api_key_guard.py[58-122]

Agent prompt

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

## Issue description
A new unit test hardcodes API-key-looking strings (e.g., `sk-ant-test-real-key`, `sk-test-real-provider-key`). Even if fake, these can be flagged as secrets and violate the repo policy.
## Issue Context
The test only needs distinct, non-empty identifiers to validate the `api_key` forwarding guard; it does not require realistic key formats.
## Fix Focus Areas
- tests/unittest/test_litellm_api_key_guard.py[58-59]
- tests/unittest/test_litellm_api_key_guard.py[121-122]

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

---

2. Flaky Anthropic guard test ☑ 🐞 Bug ≡ Correctness

Description

test_anthropic_key_not_shadowed_by_dummy_key asserts LiteLLMAIHandler.__init__ sets litellm.api_key
to the dummy placeholder, but __init__ only does that when OPENAI_API_KEY is absent. If
OPENAI_API_KEY is set in CI or a developer environment, this assertion fails and the test becomes
flaky.

Code

tests/unittest/test_litellm_api_key_guard.py[R144-155]

+        # Override settings to simulate Anthropic configured, OpenAI not configured
+        monkeypatch.setattr(litellm_handler, "get_settings", _make_anthropic_settings)
+
+        with patch("pr_agent.algo.ai_handlers.litellm_ai_handler.acompletion",
+                   new_callable=AsyncMock) as mock_call:
+            mock_call.return_value = _mock_response()
+            handler = LiteLLMAIHandler()
+
+            # After init: litellm.api_key should be the dummy (OpenAI fallback),
+            # but litellm.anthropic_key is the real Anthropic key
+            assert litellm.api_key == DUMMY_LITELLM_API_KEY
+

Evidence

LiteLLMAIHandler.__init__ assigns the dummy key only under an env-guarded branch; the new test
asserts the dummy value without controlling OPENAI_API_KEY, so it will fail whenever that env var
exists.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[37-44]
tests/unittest/test_litellm_api_key_guard.py[134-165]

Agent prompt

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

## Issue description
`test_anthropic_key_not_shadowed_by_dummy_key` assumes `LiteLLMAIHandler.__init__` will set `litellm.api_key` to `DUMMY_LITELLM_API_KEY`, but that only happens when `OPENAI_API_KEY` is not present in `os.environ`. If `OPENAI_API_KEY` is set in the test environment, the test’s assertion becomes false and the test is flaky.
### Issue Context
The handler’s init uses:
- `elif 'OPENAI_API_KEY' not in os.environ: litellm.api_key = DUMMY_LITELLM_API_KEY`
The test currently does not delete or override `OPENAI_API_KEY` before instantiating `LiteLLMAIHandler`.
### Fix Focus Areas
- tests/unittest/test_litellm_api_key_guard.py[134-165]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[37-44]
### Suggested change
In `test_anthropic_key_not_shadowed_by_dummy_key`, ensure deterministic preconditions by deleting the env var before constructing the handler:
- `monkeypatch.delenv("OPENAI_API_KEY", raising=False)`
Optionally also reset `litellm.api_key` to a known value before init to avoid cross-test state (`monkeypatch.setattr(litellm, "api_key", None, raising=False)`).

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

---

3. None-key test not exercising path ☑ 🐞 Bug ⚙ Maintainability

Description

test_none_api_key_not_forwarded sets litellm.api_key=None before constructing LiteLLMAIHandler, but
__init__ can overwrite it with the dummy placeholder when OPENAI_API_KEY is absent, so the test can
pass without actually validating the intended “None is not injected” condition.

Code

tests/unittest/test_litellm_api_key_guard.py[R98-112]

+    async def test_none_api_key_not_forwarded(self, monkeypatch):
+        """api_key must NOT appear in kwargs when litellm.api_key is None.
+
+        This is the OpenAI-key path: OPENAI.KEY sets litellm.openai_key,
+        leaving litellm.api_key at None.
+        """
+        monkeypatch.setattr(litellm, "api_key", None)
+
+        with patch("pr_agent.algo.ai_handlers.litellm_ai_handler.acompletion",
+                   new_callable=AsyncMock) as mock_call:
+            mock_call.return_value = _mock_response()
+            handler = LiteLLMAIHandler()
+            await handler.chat_completion(model="gpt-4o", system="sys", user="usr")
+
+        assert "api_key" not in mock_call.call_args[1]

Evidence

The test claims it covers the “OpenAI-key path” where litellm.api_key remains None, but the
handler init assigns the dummy placeholder whenever OPENAI.KEY is not set and OPENAI_API_KEY is not
in the environment; the test does not enforce either condition needed to keep litellm.api_key as
None at call time.

tests/unittest/test_litellm_api_key_guard.py[98-112]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[37-43]

Agent prompt

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

## Issue description
`test_none_api_key_not_forwarded` intends to validate that when `litellm.api_key` is `None`, `chat_completion()` does not inject `api_key` into the `acompletion(**kwargs)` call. However, `LiteLLMAIHandler.__init__` may overwrite `litellm.api_key` with the dummy placeholder (depending on `OPENAI_API_KEY`), so the test may not actually run with `litellm.api_key is None`.
### Issue Context
`LiteLLMAIHandler.__init__` sets `litellm.api_key = DUMMY_LITELLM_API_KEY` when:
- `OPENAI.KEY` is not configured, and
- `OPENAI_API_KEY` is not in `os.environ`.
### Fix Focus Areas
- In the test, explicitly control env/settings so that after handler initialization, `litellm.api_key` is guaranteed to be `None` (e.g., set `OPENAI_API_KEY` in env so init won’t assign dummy, and/or set `litellm.api_key = None` after constructing the handler).
- Optionally assert `litellm.api_key is None` immediately before calling `chat_completion()` to ensure the test is validating the intended state.
- tests/unittest/test_litellm_api_key_guard.py[98-112]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[37-43]

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

---

4. Cross-provider key override 🐞 Bug ≡ Correctness

Description

chat_completion() still injects kwargs["api_key"] = litellm.api_key for any model whenever
litellm.api_key is non-dummy, but __init__ assigns litellm.api_key from provider-specific
settings like GROQ.KEY/XAI.KEY/OLLAMA.API_KEY. When the process switches models/providers
(e.g., via fallback_models), the wrong provider can receive a different provider’s key, causing
authentication failures.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R409-410]

+            if litellm.api_key and litellm.api_key != "dummy_key":
+                kwargs["api_key"] = litellm.api_key

Evidence

The handler stores multiple providers’ credentials in the single global litellm.api_key (e.g.,
Groq/XAI/Ollama/OpenRouter), and the modified code still forwards that single value as api_key for
*all* model calls whenever it’s non-dummy. Separately, the PR supports trying multiple models via
fallback_models, so a single run can invoke different providers, making this override path
reachable.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[36-86]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[409-413]
pr_agent/algo/pr_processing.py[320-354]
docs/docs/usage-guide/changing_a_model.md[62-66]

Agent prompt

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

## Issue description
`chat_completion()` currently sets `kwargs["api_key"]` from the global `litellm.api_key` whenever it’s non-dummy, regardless of which provider the `model` targets. Since `__init__` also reuses `litellm.api_key` to store multiple different providers’ keys, this can send the wrong key to the wrong provider (especially when `fallback_models` switches providers).
### Issue Context
- `litellm.api_key` is assigned from multiple provider settings (e.g., Groq/XAI/Ollama/OpenRouter).
- The request provider is encoded in the `model` string (e.g., `anthropic/...`, `ollama/...`, `openai/...`, `azure/...`).
- The system supports iterating across `fallback_models`, which can be provider-mixed.
### Fix Focus Areas
- Adjust `kwargs["api_key"]` injection to be provider-aware (e.g., only add a key that corresponds to the current model’s provider; or avoid setting `api_key` globally and instead rely on provider-specific configuration/attributes).
- Ensure multi-provider fallback sequences do not leak one provider’s key into another provider’s call.
### Fix Focus Areas (code references)
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[36-86]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[269-413]
- pr_agent/algo/pr_processing.py[320-354]

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

---

5. Sentinel key collision ☑ 🐞 Bug ⚙ Maintainability

Description

The guard treats the literal string "dummy_key" as a sentinel; if a real API key ever equals this
value, it will be suppressed and authentication will fail.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[18]

+DUMMY_LITELLM_API_KEY = "dummy_key"  # placeholder set when no OpenAI key is configured

Evidence

The sentinel is defined as a common short string and the forwarding logic uses direct string
equality to decide whether to drop the key, so any real key matching that string is
indistinguishable from the placeholder.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[17-19]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[410-413]

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 placeholder sentinel is the literal string `"dummy_key"`, which is compared via equality when deciding whether to forward `api_key`.
### Issue Context
If a real key ever equals `"dummy_key"`, it will be dropped and requests will fail.
### Fix Focus Areas
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[17-19]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[410-413]
### Suggested approach
Use a more unique sentinel value (e.g., a clearly namespaced constant unlikely to collide) and keep the comparison against that constant.

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

---

6. Magic dummy_key sentinel ☑ 🐞 Bug ⚙ Maintainability

Description

The sentinel string "dummy_key" is duplicated at assignment and at the filtering check. If the
sentinel ever changes, it can be updated in one place but missed in the other, reintroducing
dummy-key injection bugs.

Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R409-410]

+            if litellm.api_key and litellm.api_key != "dummy_key":
+                kwargs["api_key"] = litellm.api_key

Evidence

The code assigns litellm.api_key = "dummy_key" during initialization and later compares
litellm.api_key != "dummy_key" before injecting it into kwargs, without a shared constant tying
those usages together.

pr_agent/algo/ai_handlers/litellm_ai_handler.py[38-43]
pr_agent/algo/ai_handlers/litellm_ai_handler.py[409-410]

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 dummy key sentinel value is hard-coded in multiple places. This increases the chance of future edits updating only one occurrence and silently breaking the dummy-key filter.
### Issue Context
The sentinel is used both when setting `litellm.api_key` and when deciding whether to forward it into request kwargs.
### Fix Focus Areas
- Introduce a module-level constant (e.g., `DUMMY_LITELLM_API_KEY = "dummy_key"`) and use it for both the assignment and the comparison.
### Fix Focus Areas (code references)
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[38-43]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[409-410]

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

---

[yanukadeneth99]

Reviewing - 1. Cross-provider key override

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

Persistent review updated to latest commit 2452b6d

[yanukadeneth99]

Reviewed both with the help of Claude.

Review 2 should be complete.

Regarding Review 1:

Investigated thoroughly and intentionally deferred. Here's why:

The _provider_api_keys dict approach tried is a partial fix that creates a false sense of safety. When a prefixed model like "anthropic/claude-3" falls through the dict (because "anthropic" isn't registered), the elif fallback still passes whatever litellm.api_key contains, which could be a Groq/XAI/Ollama key from init. A complete fix would require:

• Registering ALL providers in the dict (including ones that use litellm.anthropic_key, litellm.cohere_key, etc.)
• Or fundamentally changing how __init__ stores keys (stop reusing litellm.api_key for multiple providers)

That feels like an architectural refactor beyond the scope of a bug fix PR.

The current fix (skip dummy_key) solves the most common real-world scenario: single-provider setups where the dummy key was breaking everything. The multi-provider key leak is a pre-existing issue that existed before this change.

[JiwaniZakir]

The condition in chat_completion — if litellm.api_key and litellm.api_key != DUMMY_LITELLM_API_KEY — implicitly changes behavior beyond the stated fix: previously, kwargs["api_key"] = litellm.api_key was always executed, meaning api_key=None was passed when, for example, OpenAI credentials are configured via openai.key (which sets litellm.openai_key but leaves litellm.api_key at its default). The new guard silently omits api_key in that case too. That's likely correct, but it's a subtle behavior change that deserves an explicit comment and a test covering the OpenAI-key-set path.

The DUMMY_LITELLM_API_KEY constant is a good improvement over the magic string, but its purpose as a sentinel/placeholder value isn't obvious from the name alone — a brief inline comment like # placeholder to satisfy litellm when no real key is needed would clarify intent for future readers who encounter the != DUMMY_LITELLM_API_KEY comparison and wonder why equality against a specific string is meaningful here.

There's also no test covering the scenario this PR fixes: a provider-specific key (e.g., AWS or Azure) being silently overridden by the dummy value. Adding a test that verifies kwargs does not contain api_key when litellm.api_key equals DUMMY_LITELLM_API_KEY would lock in the correct behavior and prevent regression.

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

Persistent review updated to latest commit b5db7ad

[yanukadeneth99]

Hey @JiwaniZakir, first of all, thank you for your time and comments!

I have made the changes as recommended:

• Line 18: Updated constant comment to be more specific: "placeholder set when no OpenAI key is configured" (clearer than generic "no real API key")
• Lines 410-411: Clarified the api_key guard comment to explain what it does: "Inject api_key to the call. This key is populated during init by providers like Groq, XAI, Azure AD, and OpenRouter. Skip if None or placeholder."

Created 4 tests covering all branches:

• test_dummy_key_not_forwarded: Verifies DUMMY_LITELLM_API_KEY is blocked from kwargs
• test_none_api_key_not_forwarded: Verifies None is blocked (OpenAI path where litellm.openai_key is set instead)
• test_real_key_forwarded: Verifies real provider keys (Groq, XAI, Ollama, Azure AD) ARE injected
• test_anthropic_key_not_shadowed_by_dummy_key: Replicates the exact original bug scenario — Anthropic key configured, no OpenAI key → dummy fallback is set during __init__, but is correctly blocked from being passed to acompletion()

The guard respects LiteLLM's design pattern:

• When litellm.api_key is just a placeholder, LiteLLM falls back to provider-specific attributes (litellm.anthropic_key, litellm.groq_key, etc.)
• When litellm.api_key contains a real provider key (set by Groq/XAI/Azure AD during init), it's injected so the provider uses it
• When litellm.api_key is None, nothing is injected—the provider mechanism remains unblocked

Summary:

• ✅ Anthropic-only config now works (was broken before)
• ✅ OpenAI-only config still works (unchanged)
• ✅ All multi-provider configs still work (Groq, XAI, Ollama, Azure AD)
• ✅ All 287 existing tests still pass
• ✅ 4 new guard tests added, all passing

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

Persistent review updated to latest commit a454e4f

[yanukadeneth99]

Followed recommendations as well: The new unit test hardcodes API-key-looking strings (e.g., sk-ant-..., sk-...), which can trigger secret scanners and violates the no-committed-secrets requirement. Even if intended as fake
values, committed key-like tokens increase leakage/audit risk and should be replaced with clearly
non-secret dummy strings.

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

Persistent review updated to latest commit 1dc4223

[JiwaniZakir]

The fix looks correct in principle — conditionally passing api_key only when it's a real key prevents the dummy fallback from clobbering provider-specific credentials. One thing worth verifying: the condition should also handle cases where litellm.api_key was explicitly set by the user to a real OpenAI key (not just the dummy), so make sure the check is if api_key and api_key != "dummy_key" rather than relying solely on the presence of provider-specific keys. Given @PeterDaveHello's report that v0.33 still exhibits the issue, it's worth double-checking whether the fix is actually being reached in the code path they're using — the override might be happening elsewhere or the version cut was made before this commit landed.

[yanukadeneth99]

Followed recommendations: test_anthropic_key_not_shadowed_by_dummy_key asserts LiteLLMAIHandler.init sets litellm.api_key to the dummy placeholder, but init only does that when OPENAI_API_KEY is absent. If OPENAI_API_KEY is set in CI or a developer environment, this assertion fails and the test becomes flaky.

[yanukadeneth99]

@JiwaniZakir, not sure what you mean about his report. AFAIK, this fix is not merged yet, so the issue should be present in the latest released version.

[JiwaniZakir]

The report from @PeterDaveHello that v0.33 still exhibits the issue suggests the fix may not be handling all code paths — worth checking whether there are other places litellm.api_key gets propagated into kwargs beyond the one addressed here. On the test keys concern, using clearly fake placeholders like "fake-anthropic-key-for-testing" instead of sk-ant-... formatted strings would satisfy secret scanners without losing test clarity.

[yanukadeneth99]

Hey @JiwaniZakir, apologies, I'm still confused.

This fix is not in v0.33 or in the main branch yet, so I'm confused how you can clearly say that it's not fixing the issue that was raised in #2042 (or the bug I ran into). Regarding the report, could you please be kind enough to reference it here so that I can read it? Is it an issue, pr comment?

Regarding the test tokens, it should already be done:

• anthropic_key = "test-anthropic-key-12345"
• real_key = "test-provider-key-67890"

[JiwaniZakir]

The secret scanner concern is valid — test strings matching sk-[a-zA-Z0-9]{20,} patterns will trigger tools like detect-secrets or trufflehog regardless of context. Replace them with clearly non-key strings like "test-anthropic-key-123" or "fake-key-for-testing" that don't follow provider key formats. On the condition logic: the check should be litellm.api_key not in (None, DUMMY_KEY) rather than a string comparison against the hardcoded literal, so that if the constant value ever changes, the guard doesn't silently break.

[yanukadeneth99]

@JiwaniZakir, again

• The keys are changed already - let me know if you mean somewhere else
  -- https://github.com/qodo-ai/pr-agent/pull/2293/changes#diff-2f0647d603d71f9125b2e250c6934009c26f19d9cef691027504935515f7e813R58
  -- https://github.com/qodo-ai/pr-agent/pull/2293/changes#diff-2f0647d603d71f9125b2e250c6934009c26f19d9cef691027504935515f7e813R121
• Regarding your conditional logic, if I do that, it might fail to catch stuff like False, empty strings, or 0, I believe, so isn't this better for now? We could always change it later if required.

[JiwaniZakir]

The condition checking for dummy_key is fragile — a more robust approach would be to check whether the configured api_key matches the placeholder constant rather than a hardcoded string comparison, so if the placeholder value ever changes, the logic stays consistent. On the test side, replace strings like sk-ant-... with something clearly fake (e.g., test-anthropic-key-123) to avoid secret scanner false positives, and mock the OPENAI_API_KEY absence explicitly rather than relying on environment state so the test is deterministic in CI. The core fix is correct: letting provider-specific keys take precedence over the LiteLLM fallback is the right behavior.

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

Persistent review updated to latest commit 6a1c384

[JiwaniZakir]

The guard condition approach makes sense here — using if litellm.api_key and litellm.api_key != DUMMY_LITELLM_API_KEY is cleaner than a bare truthiness check since it explicitly documents the intent to filter out the sentinel value. One thing worth confirming: does the fix also handle cases where the dummy key might be set via environment variable (LITELLM_API_KEY) rather than the Python attribute, since litellm can pick that up independently? The four test cases covering the Anthropic-without-OpenAI scenario are a good addition for regression prevention.

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

Persistent review updated to latest commit 295f5c4

[yanukadeneth99]

Hey @naorpeled , I saw that you approved #2288. Both PRs (this and that) are targeting the same bug, but with different scopes.

In #2288's approach, it solves the immediate Ollama Cloud + Gemini conflict, thank you! But it has narrower coverage, and I'm concerned whether the fix might prevent keys like Groq, xAI, Azure AD, etc since they set the key during __init__ and rely on it being forwarded via kwargs["api_key"].

I ran a test on the latest main branch (1-April-2026), configuring Groq, calling chat_completion() with a non-Ollama model (gpt-4o), and checked if api_key appeared in the acompletion() kwargs. Unfortunately, the api_key was NOT forwarded for Groq (non-Ollama model), but was forwarded for Ollama models as expected.

The fix in this PR should work for all providers using litellm.api_key (Ollama, Groq, xAI, Azure AD, OpenRouter), is future-proof for any new provider with a real key, and has regression tests covering the necessary scenarios. So this fix basically validates "Is this a real key?".

I've updated my test cases as well.

I would appreciate it if I could steal a bit of your time to go through this PR, thanks!

[naorpeled]

Hey @yanukadeneth99,
Thanks for this!

[JiwaniZakir]

The guard condition approach is solid — using a truthiness check that rejects False, empty strings, and 0 alongside None is more robust than a strict None check. One thing worth verifying: does the sentinel DUMMY_LITELLM_API_KEY value get set somewhere that could also affect other litellm global state (e.g., litellm.headers) that might still leak into provider-specific calls? The Gemini failure @luisApurata reported suggests there may be a broader pattern of global litellm config bleeding into provider requests beyond just api_key.

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

1. Broken litellm response mock 🐞 Bug ☼ Reliability

The new unit tests’ _mock_response() assigns a lambda to mock.__getitem__, but
_get_completion() requires response["choices"] to return a real list and checks
len(response["choices"]); this mock setup is likely not honored by response[...] and can cause
openai.APIError during the tests. As written, the tests are unstable and may not actually validate
the intended behavior.

Agent Prompt

### Issue description
The tests’ `_mock_response()` does not reliably satisfy the production access pattern `response["choices"]` + `len(response["choices"])` in `_get_completion()`. This can cause the tests to fail (or validate the wrong behavior) depending on mock internals.

### Issue Context
`LiteLLMAIHandler._get_completion()` expects an object that:
- supports `response["choices"]` returning a list
- supports `response.dict()` returning a dict-like structure for logging

### Fix Focus Areas
- tests/unittest/test_litellm_api_key_guard.py[37-44]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[440-459]

### Implementation direction
Update `_mock_response()` to use a backing dict and configure magic methods in a standard way, e.g.:
- `data = {"choices": [{"message": {"content": "ok"}, "finish_reason": "stop"}]}`
- `mock = MagicMock()`
- `mock.__getitem__.side_effect = data.__getitem__`
- `mock.dict.return_value = data`
This ensures `response["choices"]` returns a real list and the handler’s logging path still works.

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

[JiwaniZakir]

The hardcoded key strings in the tests are intentional test fixtures, not real credentials — they follow the sk-ant- prefix format purely to simulate what a valid Anthropic key looks like structurally, so the guard condition (if litellm.api_key and litellm.api_key != DUMMY_LITELLM_API_KEY) can be exercised meaningfully. For the flaky test concern around test_anthropic_key_not_shadowed_by_dummy_key, the assertion depends on the order of initialization in __init__; I can refactor it to patch litellm.api_key directly at the call site rather than relying on constructor state. The _mock_response() lambda issue is a valid bug — __getitem__ needs to be set on the mock instance's class or via MagicMock spec to work correctly, and that to use MagicMock(return_value=...) with proper attribute chaining instead.