refactor(openai-shim): extract response adapters - #2072
Conversation
📝 WalkthroughWalkthroughThe change extracts response conversion and tool-call parsing from ChangesResponse adapter extraction
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
9025a84 to
59ffd82
Compare
59ffd82 to
eac082d
Compare
![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
🤖 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/openaiShim/responseAdapters.ts`:
- Around line 90-155: Add focused colocated regression tests in
responseAdapters.test.ts that invoke the exported geminiSseToAnthropic and
openaiStreamToAnthropic wrappers with representative SSE responses. Assert
emitted content, usage metadata, and terminal stop behavior for both Gemini and
OpenAI streams, covering the dependency wiring passed into convertGeminiStream
and convertOpenAIStream.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3ae023d5-c99d-442f-8756-23c259fe507f
📒 Files selected for processing (4)
src/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim/responseAdapters.ts
💤 Files with no reviewable changes (1)
- src/services/api/openaiShim.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode and ESM imports throughout the source code.
Run
bun run typecheckandbun run typecheck:type-testsfor TypeScript changes when applicable.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim/responseAdapters.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use React and Ink patterns for terminal UI components.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim/responseAdapters.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Prefer existing service, provider, settings, permission, and UI patterns over introducing new abstractions.
Usechalkfor terminal color andexecafor child-process execution when those capabilities are needed.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim/responseAdapters.ts
src/services/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use existing service and provider integration patterns when implementing API, MCP, OAuth, wiki, voice, or related integrations.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim/responseAdapters.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Add or update tests when behavior changes, and run the narrowest useful focused test checks.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not add new Python code, Python provider paths, or Python dependencies without explicit maintainer approval.
**/*.{ts,tsx,js,jsx}: Follow the existing code style in touched source files, prefer small readable changes, avoid unrelated reformatting, and keep comments useful and concise.
Preserve existing repository patterns unless intentionally refactoring them, and avoid broad rewrites or unnecessary generated changes.
Review AI-assisted code for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting it.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim/responseAdapters.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Add or update tests when a code change affects behavior.
Use focused tests such asbun test ./path/to/test-file.test.tswhen validating a narrowly scoped change.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update documentation when setup, commands, or user-facing behavior changes.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim/responseAdapters.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim/responseAdapters.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim/responseAdapters.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
Add focused regression tests for geminiSseToAnthropic and openaiStreamToAnthropic through the responseAdapters facade wiring. Validated with: bun test src/services/api/openaiShim/responseAdapters.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/openaiShim/responseAdapters.test.ts`:
- Around line 91-134: Extend the test “geminiSseToAnthropic wrapper emits
content, usage, and terminal stop” to locate and assert the emitted
function-call content block has the tool-use type, name “Read”, and input {
file_path: “a.ts” }. Also assert the corresponding content-block stop event when
emitted by the stream contract, while preserving the existing stop reason,
usage, and message-stop assertions.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 921701e8-f430-4019-b23d-e9e715a82ef4
📒 Files selected for processing (1)
src/services/api/openaiShim/responseAdapters.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode and ESM imports throughout the source code.
Run
bun run typecheckandbun run typecheck:type-testsfor TypeScript changes when applicable.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use React and Ink patterns for terminal UI components.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Prefer existing service, provider, settings, permission, and UI patterns over introducing new abstractions.
Usechalkfor terminal color andexecafor child-process execution when those capabilities are needed.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
src/services/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use existing service and provider integration patterns when implementing API, MCP, OAuth, wiki, voice, or related integrations.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Add or update tests when behavior changes, and run the narrowest useful focused test checks.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not add new Python code, Python provider paths, or Python dependencies without explicit maintainer approval.
**/*.{ts,tsx,js,jsx}: Follow the existing code style in touched source files, prefer small readable changes, avoid unrelated reformatting, and keep comments useful and concise.
Preserve existing repository patterns unless intentionally refactoring them, and avoid broad rewrites or unnecessary generated changes.
Review AI-assisted code for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting it.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Add or update tests when a code change affects behavior.
Use focused tests such asbun test ./path/to/test-file.test.tswhen validating a narrowly scoped change.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update documentation when setup, commands, or user-facing behavior changes.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
Extend the responseAdapters geminiSseToAnthropic wrapper test to cover tool_use content_block_start, input_json_delta, and content_block_stop. Remove stale post-extraction imports from the openaiShim facade.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/services/api/openaiShim/responseAdapters.test.ts (1)
1-33: 📐 Maintainability & Code Quality | 🔵 TrivialRun and report the required focused checks.
Run
bun test src/services/api/openaiShim/responseAdapters.test.ts,bun run typecheck, andbun run typecheck:type-tests. Report the exact commands and results in the PR.As per coding guidelines, “Run
bun run typecheckandbun run typecheck:type-testsfor TypeScript changes when applicable.” As per path instructions, “run targeted tests plusbun run typecheck; report exact validation commands in the PR.”🤖 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/openaiShim/responseAdapters.test.ts` around lines 1 - 33, Run the focused test and both required TypeScript checks: bun test src/services/api/openaiShim/responseAdapters.test.ts, bun run typecheck, and bun run typecheck:type-tests. Report each exact command and its result in the PR.Sources: Coding guidelines, Path instructions
src/services/api/openaiShim.ts (1)
71-93: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd facade-level parser export coverage.
The parser tests import both functions directly from
responseAdapters.ts. Add a focused test that imports them fromopenaiShim.tsand asserts the re-exports.🤖 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/openaiShim.ts` around lines 71 - 93, Add a focused test for the facade exports from openaiShim.ts, importing parseTextToolCalls and parseXmlToolCalls through that module and asserting they are re-exported correctly, while retaining the existing direct responseAdapters.ts parser coverage.Source: Path instructions
🤖 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.
Outside diff comments:
In `@src/services/api/openaiShim.ts`:
- Around line 71-93: Add a focused test for the facade exports from
openaiShim.ts, importing parseTextToolCalls and parseXmlToolCalls through that
module and asserting they are re-exported correctly, while retaining the
existing direct responseAdapters.ts parser coverage.
In `@src/services/api/openaiShim/responseAdapters.test.ts`:
- Around line 1-33: Run the focused test and both required TypeScript checks:
bun test src/services/api/openaiShim/responseAdapters.test.ts, bun run
typecheck, and bun run typecheck:type-tests. Report each exact command and its
result in the PR.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3eb5e502-755e-4309-a485-bb0976ee6d12
📒 Files selected for processing (3)
src/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim/responseAdapters.test.ts
💤 Files with no reviewable changes (1)
- src/services/api/openaiShim.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode and ESM imports throughout the source code.
Run
bun run typecheckandbun run typecheck:type-testsfor TypeScript changes when applicable.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use React and Ink patterns for terminal UI components.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Prefer existing service, provider, settings, permission, and UI patterns over introducing new abstractions.
Usechalkfor terminal color andexecafor child-process execution when those capabilities are needed.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.ts
src/services/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use existing service and provider integration patterns when implementing API, MCP, OAuth, wiki, voice, or related integrations.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Add or update tests when behavior changes, and run the narrowest useful focused test checks.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not add new Python code, Python provider paths, or Python dependencies without explicit maintainer approval.
**/*.{ts,tsx,js,jsx}: Follow the existing code style in touched source files, prefer small readable changes, avoid unrelated reformatting, and keep comments useful and concise.
Preserve existing repository patterns unless intentionally refactoring them, and avoid broad rewrites or unnecessary generated changes.
Review AI-assisted code for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting it.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Add or update tests when a code change affects behavior.
Use focused tests such asbun test ./path/to/test-file.test.tswhen validating a narrowly scoped change.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update documentation when setup, commands, or user-facing behavior changes.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiShim/responseAdapters.test.tssrc/services/api/openaiShim.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiShim/responseAdapters.test.ts
🔇 Additional comments (4)
src/services/api/openaiShim.ts (2)
45-45: LGTM!Also applies to: 128-169, 939-939
393-415: 🩺 Stability & AvailabilityPreserve the Anthropic SSE passthrough. It remains routed through
anthropicSsePassthrough, with abort cancellation and idle-timeout handling intact.src/services/api/openaiShim/responseAdapters.test.ts (2)
35-46: 🎯 Functional CorrectnessKeep the shared parser-ID test independent of test order.
responseAdapters.tsuses one shared sequential tool-call ID generator. If this test asserts fixed numeric IDs, an earlier parser call can change the expected values. Assert the prefixes and relative sequence, or reset the generator in test setup.As per path instructions, “Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions.”
Source: Path instructions
48-89: LGTM!Also applies to: 91-153, 155-195
Add a focused openaiShim.test.ts case that imports parseTextToolCalls and parseXmlToolCalls through the public facade and asserts shared sequencing.
2 participants
Summary
openaiShim/responseAdapters.tsparseTextToolCallsandparseXmlToolCallsexports through the facaderesponseAdapters.test.tsopenaiShim.tsfrom 1,208 to 1,019 lines (19 additions, 208 deletions)Why
The facade still owned response-adapter assembly after the earlier transport and Gemini-stream extractions landed on
main. This gives that responsibility an explicit source/test owner without depending on any other follow-up extraction.Rebased onto upstream
mainatc327805e. The architecture guard andgeminiStreamConversion.test.tspairing already exist onmain; this PR only adds theresponseAdaptersmodule and wires the facade to it.Impact
No intended user-facing or provider behavior change. The facade delegates through the same converters and retains its public parser API.
Validation
bun test src/services/api/openaiShim/responseAdapters.test.ts— 5 passbun test src/services/api/openaiShim.architecture.test.ts— 2 passbun test src/services/api/openaiShim.test.ts --test-name-pattern "facade parseTextToolCalls"— 1 passbun run typecheck— passbun run typecheck:type-tests— pass (10 files checked)Contributor checklist
CONTRIBUTING.mdandAGENTS.md.Summary by CodeRabbit
Bug Fixes
Tests