feat: Add /v1/responses support to the gateway - #992
Conversation
Codecov Report❌ Patch coverage is
... and 60 files with indirect coverage changes 🚀 New features to boost your workflow:
|
PR #992 Review: feat: Add
|
| File | Assessment |
|---|---|
src/any_llm/gateway/api/main.py |
Clean. Import added, router registered. No issues. |
src/any_llm/gateway/streaming.py |
Clean. New RESPONSES_STREAM_FORMAT follows existing patterns correctly. yield_done_on_error=False matches the Responses API SSE spec. |
src/any_llm/gateway/api/routes/responses.py |
Generally solid, follows the messages.py pattern well. Several issues noted below. |
tests/gateway/test_responses_endpoint.py |
Decent coverage. A few structural issues and missing cases. |
Issues Found
1. data: [DONE] sentinel may not match the Responses API spec
streaming.py:32 sets done_marker="data: [DONE]\n\n". The OpenAI Responses API streaming spec does NOT send a data: [DONE] sentinel. The Responses API uses named SSE events (event: response.completed, etc.) and simply closes the stream. Sending data: [DONE] is a Chat Completions convention that could confuse clients using the Responses API. This should be verified against the actual OpenAI spec and corrected if needed.
2. _usage_to_completion_usage uses getattr instead of direct attribute access
responses.py:36-38 uses getattr(usage, "input_tokens", None) etc. Per project guidelines: "Prefer direct attribute access over getattr when the field is typed." The usage parameter is typed Any, so this is borderline acceptable, but the function is only ever called on ResponseResource.usage / Response.usage which have typed fields. Consider narrowing the type or documenting why getattr is needed.
3. Coverage gap: 82.5% patch coverage (14 lines missing)
The Codecov report flags responses.py at 81.81%, below the ~85% target. Missing coverage likely includes the error paths. Tests should be added for:
- Provider call raising an exception (non-streaming)
- Provider call raising an exception (streaming)
- Non-streaming with no usage data returned
- The
_usage_to_completion_usageedge cases (all None fields)
4. No test for API key auth (non-master-key)
The messages endpoint tests include test_messages_endpoint_bearer_auth and test_messages_endpoint_x_api_key_header. The responses tests only test master key auth. Should add a test with a regular API key.
5. ConfigDict(extra="allow") on ResponsesRequest is undocumented
The chat and messages request models don't use extra="allow". This is correct for passthrough of Responses-only fields (reasoning, include, etc.), but a brief comment explaining why would help future readers.
Compliance with Project Guidelines
- Naming conventions: Follows existing patterns (router prefix, tags, etc.) ✅
- Type hints: Present throughout. Uses
Anyin a few places but consistent with codebase style ✅ - Test structure: Standalone functions, no class-based grouping, imports at top ✅
- Commit format:
feat: Add /v1/responses support to the gatewayfollows Conventional Commits ✅ getattrusage: See issue Add an acompletion method to api #2 above⚠️ - No emdashes: Clean ✅
Suggestions (non-blocking)
- Consider narrowing
_usage_to_completion_usageto accept the specific OpenAIResponseUsagetype instead ofAnyfor better type checking. - The
request_fields["input_data"] = request_fields.pop("input")rename at line 101 is clear but could use a brief comment explaining the mapping from OpenAI'sinputto the SDK'sinput_dataparameter name.
Verdict
Request Changes
The PR is well-structured and follows existing gateway patterns closely. Two items to resolve before merging:
- Blocking: The
data: [DONE]done marker inRESPONSES_STREAM_FORMATmay not match the Responses API streaming protocol. Please verify against the OpenAI spec and correct if needed. - Blocking: Coverage is at 82.5% (target ~85%). Add error-path tests for provider failures in both streaming and non-streaming modes, and a basic API key auth test.
- Verified `data: [DONE]` sentinel is correct per OpenAI Responses API streaming spec (no change needed) - Replace getattr with direct attribute access in _usage_to_completion_usage, narrowing the type to ResponseUsage | None - Document why ConfigDict(extra="allow") is needed on ResponsesRequest - Add comment explaining the input -> input_data field rename - Add tests for API key auth, provider errors (streaming and non-streaming), missing usage data, and mid-stream errors to bring coverage above 85%
|
Hi @brightsparc , Could you follow this guide to allow maintainers of any-llm to push to your fork branch: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork? This would help us avoid the back and forth to let me help get the PRs over the finish line slightly faster |
Hi @njbrake I added you as a maintainer on the fork, because I forked into via an organization repo I can't add any mnaintainer, but can add others if required. For next time I can fork via my personal profile. |
|
I’ve also updated this from base banch. Let me know if there are any other changes you would like to see @njbrake |
|
Hey @brightsparc First, thanks so much for the contributions, and sorry for the late answers. We would love to bring your work to this new codebase. If you are ok, we can merge your pull requests, and I will bring over those features on the new gateway. Unfortunately, I will not be able to bring directly your commits with your name in the new repository. If this is an issue for you, I can suggest that you make those PRs against the new repository. If not, then I will merge your PR, and I will migrate your code. The timing made it seem like everything happened at once 😅 |
Hi @njbrake I am not precious about need to own this contribution, but also happy to create a PR and land if they are not yet in the repo. Perhaps you could just include in the comment like co-wrote with @brightsparc similar to what Claude does. |
|
perfect. I will merge those PRs and make a last gateway release so you can get those nice contributions. I will then move over all those features to the new repo (with you as co-authored 😄 ) |
|
@brightsparc if you don't want to bother fixing the tests I will just close your PRs since they were all migrated to the new gateway. Let me know if you migrated or if you want those to be merged. These PRs will be the last ones before I will close all PRs against this gateway. |
|
Yes I realized. Fine to close. |
Fixes #991
Description
This PR adds OpenAI-compatible
/v1/responsessupport to the gateway.The SDK already supports the Responses API through
responses(...)/aresponses(...), but the gateway only exposed/v1/chat/completionsand/v1/messages. That meant clients using Responses-specific functionality had to bypass the gateway entirely, which breaks centralized auth, rate limiting, budgeting, logging, and observability.This change adds a new
POST /v1/responsesroute that:aresponses(...)implementationSUPPORTS_RESPONSES=Truereasoning,previous_response_id, andinclude=["reasoning.encrypted_content"]The rationale for adding this route is that the Responses API is not just a rename of chat completions. It enables additional capabilities, including encrypted reasoning passthrough via
include=["reasoning.encrypted_content"], which are not available via the chat completions gateway route.PR Type
Relevant issues
Closes the manually-filed issue for adding
/v1/responsessupport to the gateway.Checklist
AI Usage Information
When answering questions by the reviewer, please respond yourself, do not copy/paste the reviewer comments into an AI system and paste back its answer. We want to discuss with you, not your AI :)