Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes a bug where Responses API
Confidence Score: 5/5Safe to merge; the change is narrowly scoped to the Responses-API-to-chat-completion transformation path and does not touch authentication, routing, or database access. The core merging logic is correct for all typical inputs: string content, list-of-content-blocks, and the instructions field are all handled and tested. No new issues beyond those already raised in prior review threads were found. The fix resolves the original bug (developer-role messages reaching providers that reject the role) for the common case. No files require special attention beyond what has already been discussed in prior review comments on transformation.py.
|
| Filename | Overview |
|---|---|
| litellm/responses/litellm_completion_transformation/transformation.py | Adds _get_text_from_message_content and _merge_responses_system_messages helpers, then wires them into transform_responses_api_input_to_messages so all system/developer role messages are coalesced into a single leading system message before the chat-completion bridge; logic is correct for the happy path but has edge-case fragility around empty-content system messages and non-dict message objects (flagged in prior review threads). |
| tests/test_litellm/responses/litellm_completion_transformation/test_litellm_completion_responses.py | Adds three new unit tests: merging instructions + developer messages, preserving messages without system content, and text-block extraction; all are pure mocked tests with no network calls, satisfying the repo's test-isolation rule. |
Reviews (2): Last reviewed commit: "fix: merge responses developer messages ..." | Re-trigger Greptile
2cc9940 to
7356b87
Compare
|
@samrusani — could you add a screenshot or short video showing that this change works as expected? It really helps reviewers verify the fix quickly. Thanks! |
|
Thanks. This is a backend Responses API transformation fix, so there is no UI flow to capture. I added proof in the PR body with the targeted regression test and coverage command output. The regression test verifies that Responses API Commands run: uv run pytest tests/test_litellm/responses/litellm_completion_transformation/test_litellm_completion_responses.py -q
# 65 passed in 0.24s
uv run pytest tests/test_litellm/responses/litellm_completion_transformation/test_litellm_completion_responses.py --cov=litellm.responses.litellm_completion_transformation.transformation --cov-report=term-missing -q
# 65 passed in 0.30s
# The new merge helper lines are not listed as missing.Codecov also now reports: |
|
Hi maintainers, just checking in on this one. This PR is intentionally scoped to the Responses API bridge path for #26879, with CI passing, Codecov passing, Veria passing, and Greptile at 5/5. Happy to adjust if you prefer a different shape for the fix. |
|
Withdrawing this PR to reduce stale contributor backlog. It may be reopened if maintainers want the work continued. |
Relevant issues
Fixes #26879
Pre-Submission checklist
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Type
🐛 Bug Fix
✅ Test
Changes
instructionsanddeveloperrole input items into one leading chat-completionsystemmessage before the Responses-to-chat bridge forwards the request.input_text/textcontent blocks so Codex-style developer messages are preserved in the merged system prompt.instructionsplus multiple developer messages, requests without system content, and supported/ignored text block extraction so non-OpenAI providers no longer receive multiplesystemmessages.Notes
make format-checkcurrently reports unrelated formatting drift in existing enterprise files outside this PR's diff. The two changed files pass Black directly.