添加openAI compatible的原生response api格式支持 - #1328
Open
gtlab2023 wants to merge 5 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds configurable OpenAI upstream modes, API-key Responses API streaming, routing precedence, incomplete-stream validation, focused tests, design documentation, and dependency updates. ChangesOpenAI Responses API
Dependency updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant queryModelOpenAI
participant ChatGPTAuthentication
participant OpenAIResponsesAPI
participant ChatCompletionsAPI
ChatGPTAuthentication->>queryModelOpenAI: Return authentication state
alt ChatGPT authentication is active
queryModelOpenAI->>ChatGPTAuthentication: Request ChatGPT Responses stream
else Responses mode is configured
queryModelOpenAI->>OpenAIResponsesAPI: POST API-key Responses request
else Default mode
queryModelOpenAI->>ChatCompletionsAPI: Request Chat Completions stream
end
queryModelOpenAI->>queryModelOpenAI: Validate terminal message or assistant content
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/services/api/openai/__tests__/upstreamApiMode.test.ts (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the required source import path.
Line 2 imports a
srcmodule through a relative.jspath. Use thesrc/*alias and the required TypeScript extension for this test import.Proposed change
-import { getUpstreamApiMode } from '../upstreamApiMode.js' +import { getUpstreamApiMode } from 'src/services/api/openai/upstreamApiMode.ts'As per coding guidelines, use the
src/*path alias for imports fromsrc, and include the.tsextension where the project testing path rules require it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/api/openai/__tests__/upstreamApiMode.test.ts` at line 2, Update the test import for getUpstreamApiMode to use the project’s src/* path alias and the required .ts extension instead of the relative .js path, leaving the imported symbol unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/api/openai/__tests__/queryModelOpenAI.isolated.ts`:
- Around line 424-472: Rename
src/services/api/openai/__tests__/queryModelOpenAI.isolated.ts to
queryModelOpenAI.test.ts so normal test discovery includes the routing and
incomplete-stream tests. Update
docs/superpowers/specs/2026-08-02-openai-responses-api-design.md lines 65-69 to
use the renamed file in the verification command and remove the manual-discovery
exception. Update docs/superpowers/plans/2026-08-02-openai-responses-api.md
lines 166-169 and every reference there to the new test filename; no other test
behavior changes are needed.
In `@src/services/api/openai/responsesAdapter.ts`:
- Around line 512-530: The manual request in createOpenAIResponsesStream must
reuse the shared OpenAI request configuration applied by getOpenAIClient,
including organization and project headers plus API_TIMEOUT_MS handling. Update
the Responses fetch setup to apply the same headers, options, and timeout
behavior as the Chat Completions transport, while preserving existing proxy and
abort-signal behavior, and add regression tests covering organization, project,
and timeout configuration.
---
Nitpick comments:
In `@src/services/api/openai/__tests__/upstreamApiMode.test.ts`:
- Line 2: Update the test import for getUpstreamApiMode to use the project’s
src/* path alias and the required .ts extension instead of the relative .js
path, leaving the imported symbol unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b07321a4-bf20-4cf6-aeca-dc557a9cd386
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
docs/superpowers/plans/2026-08-02-openai-responses-api.mddocs/superpowers/specs/2026-08-02-openai-responses-api-design.mdpackage.jsonsrc/services/api/openai/__tests__/queryModelOpenAI.isolated.tssrc/services/api/openai/__tests__/responsesAdapter.test.tssrc/services/api/openai/__tests__/upstreamApiMode.test.tssrc/services/api/openai/index.tssrc/services/api/openai/responsesAdapter.tssrc/services/api/openai/upstreamApiMode.ts
Comment on lines
+424
to
+472
| beforeEach(() => { | ||
| _lastCreateArgs = null | ||
| _isChatGPTAuthEnabled = false | ||
| _chatCompletionsCreateCalls = 0 | ||
| _chatGPTResponsesCalls = 0 | ||
| _openAIResponsesCalls = 0 | ||
| }) | ||
|
|
||
| describe('queryModelOpenAI — upstream API routing', () => { | ||
| test('uses API-key Responses transport when UPSTREAM_API_MODEL is responses', async () => { | ||
| await runQueryModel([], { UPSTREAM_API_MODEL: 'responses' }) | ||
|
|
||
| expect(_openAIResponsesCalls).toBe(1) | ||
| expect(_chatCompletionsCreateCalls).toBe(0) | ||
| expect(_chatGPTResponsesCalls).toBe(0) | ||
| }) | ||
|
|
||
| test('prefers ChatGPT Responses transport over UPSTREAM_API_MODEL', async () => { | ||
| _isChatGPTAuthEnabled = true | ||
|
|
||
| await runQueryModel([], { UPSTREAM_API_MODEL: 'responses' }) | ||
|
|
||
| expect(_chatGPTResponsesCalls).toBe(1) | ||
| expect(_openAIResponsesCalls).toBe(0) | ||
| expect(_chatCompletionsCreateCalls).toBe(0) | ||
| }) | ||
|
|
||
| test('keeps Chat Completions as the default transport', async () => { | ||
| await runQueryModel([], { UPSTREAM_API_MODEL: undefined }) | ||
|
|
||
| expect(_chatCompletionsCreateCalls).toBe(1) | ||
| expect(_openAIResponsesCalls).toBe(0) | ||
| expect(_chatGPTResponsesCalls).toBe(0) | ||
| }) | ||
|
|
||
| test('surfaces an invalid upstream API mode as an API error', async () => { | ||
| const { assistantMessages } = await runQueryModel([], { | ||
| UPSTREAM_API_MODEL: 'completion', | ||
| }) | ||
|
|
||
| expect(assistantMessages).toHaveLength(1) | ||
| expect(assistantMessages[0]?.message.content).toEqual([ | ||
| { | ||
| type: 'text', | ||
| text: expect.stringContaining('Invalid UPSTREAM_API_MODEL: completion'), | ||
| }, | ||
| ]) | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Rename the isolated routing test for normal test discovery.
The added routing and incomplete-stream tests are in a file without the required .test.ts suffix. The verification record confirms that this requires an explicit command.
src/services/api/openai/__tests__/queryModelOpenAI.isolated.ts#L424-L472: rename the file to the requiredqueryModelOpenAI.test.tsconvention.docs/superpowers/specs/2026-08-02-openai-responses-api-design.md#L65-L69: update the verification command and remove the manual-discovery exception.docs/superpowers/plans/2026-08-02-openai-responses-api.md#L166-L169: update the planned test filename and all references to it.
As per coding guidelines, use bun:test, place unit tests under src/**/__tests__/, and name them <module>.test.ts.
📍 Affects 3 files
src/services/api/openai/__tests__/queryModelOpenAI.isolated.ts#L424-L472(this comment)docs/superpowers/specs/2026-08-02-openai-responses-api-design.md#L65-L69docs/superpowers/plans/2026-08-02-openai-responses-api.md#L166-L169
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/api/openai/__tests__/queryModelOpenAI.isolated.ts` around lines
424 - 472, Rename src/services/api/openai/__tests__/queryModelOpenAI.isolated.ts
to queryModelOpenAI.test.ts so normal test discovery includes the routing and
incomplete-stream tests. Update
docs/superpowers/specs/2026-08-02-openai-responses-api-design.md lines 65-69 to
use the renamed file in the verification command and remove the manual-discovery
exception. Update docs/superpowers/plans/2026-08-02-openai-responses-api.md
lines 166-169 and every reference there to the new test filename; no other test
behavior changes are needed.
Source: Coding guidelines
Comment on lines
+512
to
+530
| export async function createOpenAIResponsesStream(params: { | ||
| request: ResponsesRequest | ||
| signal: AbortSignal | ||
| fetchOverride?: typeof fetch | ||
| }): Promise<AsyncIterable<Record<string, unknown>>> { | ||
| const fetchFn = params.fetchOverride ?? (globalThis.fetch as typeof fetch) | ||
| const baseUrl = ( | ||
| process.env.OPENAI_BASE_URL || 'https://api.openai.com/v1' | ||
| ).replace(/\/+$/, '') | ||
| const response = await fetchFn(`${baseUrl}/responses`, { | ||
| method: 'POST', | ||
| headers: { | ||
| Authorization: `Bearer ${process.env.OPENAI_API_KEY || ''}`, | ||
| 'Content-Type': 'application/json', | ||
| Accept: 'text/event-stream', | ||
| }, | ||
| body: JSON.stringify(params.request), | ||
| signal: params.signal, | ||
| ...getProxyFetchOptions({ forAnthropicAPI: false }), |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)responsesAdapter\.ts$|(^|/)openai.*\.ts$|src/services/api/openai' || true
echo "== target outline =="
ast-grep outline src/services/api/openai/responsesAdapter.ts --view expanded || true
echo "== target relevant lines =="
sed -n '1,80p;460,560p' src/services/api/openai/responsesAdapter.ts
echo "== getOpenAIClient occurrences =="
rg -n "getOpenAIClient|OPENAI_ORG_ID|OPENAI_PROJECT_ID|API_TIMEOUT_MS|responsesAdapter|createOpenAIResponsesStream|responses" src -S
echo "== proxy fetch options =="
rg -n "getProxyFetchOptions|forAnthropicAPI" src/services -S || trueRepository: claude-code-best/claude-code
Length of output: 27466
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== openai client =="
sed -n '1,90p' src/services/api/openai/client.ts
echo "== openai index relevant call locations =="
sed -n '320,420p' src/services/api/openai/index.ts
echo "== openai shared timeout/headers helper usage =="
rg -n "API_TIMEOUT_MS|timeout:|fetchOptions|Authorization|OpenAI" src/services/api/client.ts src/services/api/openai/client.ts src/services/api/client.ts || true
echo "== responsesAdapter tests around fetch args =="
sed -n '175,245p' src/services/api/openai/__tests__/responsesAdapter.test.ts
echo "== isolated queryModel env test around responses =="
sed -n '420,450p' src/services/api/openai/__tests__/queryModelOpenAI.isolated.tsRepository: claude-code-best/claude-code
Length of output: 13100
Apply the shared OpenAI request config to Responses requests.
createOpenAIResponsesStream() builds a manual fetch request, so it omits OPENAI_ORG_ID, OPENAI_PROJECT_ID, and API_TIMEOUT_MS that the Chat Completions transport applies through getOpenAIClient(). Ensure API-key Responses requests pass the same headers/options/timeout as the SDK path, and add regression tests for organization, project, and timeout behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/api/openai/responsesAdapter.ts` around lines 512 - 530, The
manual request in createOpenAIResponsesStream must reuse the shared OpenAI
request configuration applied by getOpenAIClient, including organization and
project headers plus API_TIMEOUT_MS handling. Update the Responses fetch setup
to apply the same headers, options, and timeout behavior as the Chat Completions
transport, while preserving existing proxy and abort-signal behavior, and add
regression tests covering organization, project, and timeout configuration.
Owner
|
请问有经过测试吗 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation