Fix Gemini backend OpenRouter header handling #629
Conversation
WalkthroughAdds OpenRouter-aware header construction and configuration resolution to the Gemini connector, including a new context builder and expanded _resolve_gemini_api_config signature. Integrates header provider flow into chat_completions. Introduces unit tests validating header provider invocation, header propagation, and request targeting via a mocked HTTP client. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant GB as GeminiBackend
participant RC as _resolve_gemini_api_config
participant HP as openrouter_headers_provider
participant HTTP as HTTPX Client
participant OR as OpenRouter
Client->>GB: chat_completions(model, api_base, api_key, headers_provider, key_name)
GB->>RC: Resolve base URL and headers
alt OpenRouter base URL provided
RC->>GB: _build_openrouter_header_context()
RC->>HP: provide(key_name) or provide(context)
HP-->>RC: headers or error
alt Headers valid
RC-->>GB: (openrouter_base_url, merged headers)
else Error or missing headers
RC-->>GB: (openrouter_base_url, fallback headers) Note over RC,GB: May raise AuthenticationError
end
else Google Gemini flow
RC-->>GB: (gemini_base_url, standard headers)
end
GB->>HTTP: POST /chat.completions with headers
HTTP->>OR: Forward request
OR-->>HTTP: Response
HTTP-->>GB: Response
GB-->>Client: ChatCompletion result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/connectors/gemini.py (1)
345-347: Align openrouter_headers_provider type in chat_completions
Update its annotation in src/connectors/gemini.py:345-347 from
Callable[[str, str], dict[str, str]]to
Callable[[Any, str], dict[str, str]]
to match stream_chat_completions (line 484) and the resolver/tests.- openrouter_headers_provider: Callable[[str, str], dict[str, str]] | None = None, + openrouter_headers_provider: Callable[[Any, str], dict[str, str]] | None = None,
🧹 Nitpick comments (2)
tests/unit/gemini_connector_tests/test_openrouter_headers.py (1)
55-66: Silence Ruff TRY003 in test provider (nit).Avoid long/custom exception messages in tests. Raising a bare TypeError suffices for this check.
- if isinstance(arg, str): - raise TypeError("legacy signature not supported") + if isinstance(arg, str): + raise TypeErrorsrc/connectors/gemini.py (1)
461-477: Duplicate header-context builder; consider sharing with OpenRouter connector.This mirrors src/connectors/openrouter.py. Factor into a shared util/mixin to avoid drift.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/connectors/gemini.py(4 hunks)tests/unit/gemini_connector_tests/test_openrouter_headers.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Avoid using emojis in Python code comments or docstrings
Follow PEP 8 and use type hints for all functions
Import order must be: standard library, third-party, then local imports, separated by blank lines
Naming conventions: snake_case for variables and functions; PascalCase for classes
Error handling: use specific exceptions and include meaningful error messages
Prefer f-strings for string formatting
Files:
tests/unit/gemini_connector_tests/test_openrouter_headers.pysrc/connectors/gemini.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Catch specific exceptions; avoid broad except Exception blocks
If a broad exception must be caught, log with exc_info=True and re-raise a specific custom exception
Use the most specific exception class from src.core.common.exceptions that accurately describes the error
Provide clear, helpful error messages and include relevant details in the details dictionary of custom exceptions
Files:
src/connectors/gemini.py
🧬 Code graph analysis (2)
tests/unit/gemini_connector_tests/test_openrouter_headers.py (1)
src/core/services/translation_service.py (1)
TranslationService(20-636)
src/connectors/gemini.py (5)
src/core/common/exceptions.py (1)
AuthenticationError(56-65)src/connectors/openrouter.py (1)
_build_openrouter_header_context(43-59)src/connectors/gemini_cloud_project.py (1)
_resolve_gemini_api_config(840-869)src/connectors/gemini_oauth_personal.py (1)
_resolve_gemini_api_config(981-1023)src/core/security/loop_prevention.py (1)
ensure_loop_guard_header(11-15)
🪛 Ruff (0.13.3)
tests/unit/gemini_connector_tests/test_openrouter_headers.py
58-58: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (python)
- GitHub Check: test (3.10)
🔇 Additional comments (2)
tests/unit/gemini_connector_tests/test_openrouter_headers.py (1)
11-86: Solid httpx.MockTransport-based header propagation test.Covers provider call order, URL, and required headers. Looks good.
src/connectors/gemini.py (1)
355-361: Passing new OpenRouter params through to resolver — LGTM.Call site correctly forwards openrouter_headers_provider and key_name.
| normalized_base = base.rstrip("/") | ||
|
|
||
| using_openrouter = openrouter_api_base_url is not None | ||
|
|
||
| headers: dict[str, str] | ||
| if using_openrouter: |
There was a problem hiding this comment.
Fix OpenRouter mode detection when both bases are provided.
Current logic enables OpenRouter headers if openrouter_api_base_url is non-null even when base resolves to gemini_api_base_url. Compute mode by comparing the chosen base to the OpenRouter base.
- normalized_base = base.rstrip("/")
-
- using_openrouter = openrouter_api_base_url is not None
+ normalized_base = base.rstrip("/")
+ openrouter_normalized = (
+ openrouter_api_base_url.rstrip("/") if openrouter_api_base_url else None
+ )
+ using_openrouter = (
+ openrouter_normalized is not None
+ and normalized_base == openrouter_normalized
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| normalized_base = base.rstrip("/") | |
| using_openrouter = openrouter_api_base_url is not None | |
| headers: dict[str, str] | |
| if using_openrouter: | |
| normalized_base = base.rstrip("/") | |
| openrouter_normalized = ( | |
| openrouter_api_base_url.rstrip("/") if openrouter_api_base_url else None | |
| ) | |
| using_openrouter = ( | |
| openrouter_normalized is not None | |
| and normalized_base == openrouter_normalized | |
| ) | |
| headers: dict[str, str] | |
| if using_openrouter: |
🤖 Prompt for AI Agents
In src/connectors/gemini.py around lines 501 to 506, the code sets
using_openrouter simply based on openrouter_api_base_url being non-null which
incorrectly enables OpenRouter mode when the chosen base is actually the Gemini
base; instead determine the actual base being used (normalize both base and
openrouter_api_base_url with rstrip("/") or similar) and set using_openrouter =
(chosen_base == normalized_openrouter_api_base_url) so headers and mode are only
enabled when the selected base exactly matches the OpenRouter base.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68ec25fa5a0c8333923ed0f3169275bf
Summary by CodeRabbit
New Features
Tests