-
Notifications
You must be signed in to change notification settings - Fork 1
Fix OpenAI connector payload normalization for list content #630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughType checking in src/connectors/openai.py was adjusted to use isinstance(x, (list, tuple)) in _normalize_content. A new unit test validates normalization of sequence-based message content for OpenAIConnector by capturing the outgoing payload and asserting list-of-text parts are preserved. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
@coderabbitai
coderabbitai
bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/connectors/openai.py (1)
339-355: isinstance tuple-of-types fix is correctAvoids runtime TypeError; change is good.
Optional: generalize to Sequence (excluding str/bytes)
Broaden support beyond list/tuple while keeping strings intact.
- if isinstance(value, (list, tuple)): + from collections.abc import Sequence # at top-level import + if isinstance(value, Sequence) and not isinstance(value, (str, bytes, bytearray)):Add import near other collections.abc imports:
-from collections.abc import AsyncGenerator, AsyncIterator, Mapping +from collections.abc import AsyncGenerator, AsyncIterator, Mapping, Sequencetests/unit/openai_connector_tests/test_processed_messages_normalization.py (1)
41-49: Silence Ruff ARG001 by prefixing unused argsRename unused parameters to underscore-prefixed to satisfy linters.
- async def fake_handle( - self: OpenAIConnector, - url: str, - payload: dict[str, Any], - headers: dict[str, str] | None, - session_id: str, - ) -> ResponseEnvelope: + async def fake_handle( + _self: OpenAIConnector, + _url: str, + payload: dict[str, Any], + _headers: dict[str, str] | None, + _session_id: str, + ) -> ResponseEnvelope:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/connectors/openai.py(1 hunks)tests/unit/openai_connector_tests/test_processed_messages_normalization.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:
src/connectors/openai.pytests/unit/openai_connector_tests/test_processed_messages_normalization.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/openai.py
🧬 Code graph analysis (1)
tests/unit/openai_connector_tests/test_processed_messages_normalization.py (3)
src/connectors/openai.py (1)
responses(540-658)src/core/domain/chat.py (3)
CanonicalChatRequest(321-324)ChatMessage(90-138)MessageContentPartText(14-18)src/core/domain/responses.py (1)
ResponseEnvelope(10-23)
🪛 Ruff (0.13.3)
tests/unit/openai_connector_tests/test_processed_messages_normalization.py
42-42: Unused function argument: self
(ARG001)
43-43: Unused function argument: url
(ARG001)
45-45: Unused function argument: headers
(ARG001)
46-46: Unused function argument: session_id
(ARG001)
⏰ 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). (1)
- GitHub Check: test (3.10)
🔇 Additional comments (1)
tests/unit/openai_connector_tests/test_processed_messages_normalization.py (1)
81-93: Good regression testEffectively verifies list-of-text content normalization and captures payload safely via monkeypatch.
Uh oh!
There was an error while loading. Please reload this page.
Summary
OpenAIConnectornormalizes message content sequences without raising type errorsTesting
python -m pytest -o addopts="" tests/unit/openai_connector_tests/test_processed_messages_normalization.pypython -m pytest -o addopts=""(fails: missing optional test dependencies such as pytest-asyncio and pytest-httpx)https://chatgpt.com/codex/tasks/task_e_68ec25fa5a0c8333923ed0f3169275bf
Summary by CodeRabbit
Bug Fixes
Tests