feat: add OpenAI-compatible LLM provider#307
Conversation
|
@fatinghenji is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds OpenAI as a supported LLM provider. Extends ProviderType and config detection for OPENAI_API_KEY, implements OpenAIProvider (chat-completions via fetch with optional reasoning_effort and Azure-aware headers/URL), wires it into the provider factory, and documents new OPENAI_* environment variables. ChangesOpenAI Provider
Sequence Diagram(s)sequenceDiagram
participant Client
participant OpenAIProvider
participant OpenAI_API
Client->>OpenAIProvider: compress(systemPrompt, userPrompt)
OpenAIProvider->>OpenAIProvider: call(systemPrompt,userPrompt,reasoning_effort?)
OpenAIProvider->>OpenAI_API: POST /v1/chat/completions (model, messages, reasoning_effort)
OpenAI_API-->>OpenAIProvider: JSON {choices:[{message:{content|reasoning}}]}
OpenAIProvider->>Client: returns extracted text
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
@rohitg00 This is the cleaned-up version of #240. All review feedback has been addressed:
Ready for review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/config.ts (1)
53-53: ⚡ Quick winPrefer naming over WHAT comments in provider detection.
Please remove/reword this branch comment and rely on clear naming/structure instead.
As per coding guidelines, "Avoid code comments explaining WHAT — use clear naming instead".
🤖 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/config.ts` at line 53, Remove the inline "// OpenAI-compatible: supports OpenAI, DeepSeek, SiliconFlow, Azure, vLLM, LM Studio" comment and instead express that intent in code by renaming the related symbol(s) (e.g., a boolean/branch, array, or function) to a descriptive name such as openAICompatibleProviders or isOpenAICompatible(provider); update the provider detection branch to use that renamed identifier and adjust any usages accordingly so the code reads self-documentingly without the WHAT-style comment.src/providers/openai.ts (1)
7-30: ⚡ Quick winRemove WHAT-style comments and keep only intent/constraints.
This block/inline comment content is mostly descriptive of implementation details already clear from code.
As per coding guidelines, "Avoid code comments explaining WHAT — use clear naming instead".
Also applies to: 90-90
🤖 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/providers/openai.ts` around lines 7 - 30, The header block comment for the OpenAI-compatible provider is written in WHAT-style/descriptive details; remove or shrink it to a concise intent and constraints note (e.g., "OpenAI-compatible LLM provider; requires OPENAI_API_KEY; supports configurable OPENAI_BASE_URL, OPENAI_MODEL, MAX_TOKENS, OPENAI_REASONING_EFFORT") and drop implementation-specific examples and long prose so the top-of-file comment only states purpose and configuration constraints referenced by the module (symbols: the module header comment, OPENAI_API_KEY, OPENAI_BASE_URL, OPENAI_MODEL, MAX_TOKENS, OPENAI_REASONING_EFFORT).
🤖 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/config.ts`:
- Around line 169-170: The condition in detectLlmProviderKind currently treats
any presence of OPENAI_API_KEY as opting into the "llm" provider; change the
check that uses hasRealValue(env["OPENAI_API_KEY"]) so it also respects
OPENAI_API_KEY_FOR_LLM being explicitly disabled. Specifically, update the logic
around hasRealValue(env["OPENAI_API_KEY"]) (in detectLlmProviderKind) to require
that OPENAI_API_KEY_FOR_LLM is not set to a falsey/disabled value (e.g., treat
"false" case-insensitively as disabling) — for example, only consider
OPENAI_API_KEY when hasRealValue(env["OPENAI_API_KEY_FOR_LLM"]) is false or
env["OPENAI_API_KEY_FOR_LLM"].toLowerCase() !== "false" (or use a helper
parseBoolean) so that an explicit OPENAI_API_KEY_FOR_LLM=false prevents
reporting "llm".
In `@src/providers/openai.ts`:
- Around line 68-75: The outbound fetch to the OpenAI-compatible endpoint (the
call creating `response` with `fetch(url, { method: "POST", headers: { ...
Authorization: Bearer ${this.apiKey} }, body: JSON.stringify(body) })`) needs a
timeout using an AbortController: create an AbortController, pass its signal
into fetch, set a timer to call controller.abort() after a configurable timeout
(e.g., DEFAULT_TIMEOUT_MS), clear the timer once fetch completes, and handle the
abort error to surface a clear timeout error instead of letting the call hang;
update the method that performs this request to use the controller and ensure
the timer is cleaned up on success or error.
---
Nitpick comments:
In `@src/config.ts`:
- Line 53: Remove the inline "// OpenAI-compatible: supports OpenAI, DeepSeek,
SiliconFlow, Azure, vLLM, LM Studio" comment and instead express that intent in
code by renaming the related symbol(s) (e.g., a boolean/branch, array, or
function) to a descriptive name such as openAICompatibleProviders or
isOpenAICompatible(provider); update the provider detection branch to use that
renamed identifier and adjust any usages accordingly so the code reads
self-documentingly without the WHAT-style comment.
In `@src/providers/openai.ts`:
- Around line 7-30: The header block comment for the OpenAI-compatible provider
is written in WHAT-style/descriptive details; remove or shrink it to a concise
intent and constraints note (e.g., "OpenAI-compatible LLM provider; requires
OPENAI_API_KEY; supports configurable OPENAI_BASE_URL, OPENAI_MODEL, MAX_TOKENS,
OPENAI_REASONING_EFFORT") and drop implementation-specific examples and long
prose so the top-of-file comment only states purpose and configuration
constraints referenced by the module (symbols: the module header comment,
OPENAI_API_KEY, OPENAI_BASE_URL, OPENAI_MODEL, MAX_TOKENS,
OPENAI_REASONING_EFFORT).
🪄 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
Run ID: 3e6eb155-a046-45d9-9423-c61876ecf37b
📒 Files selected for processing (5)
README.mdsrc/config.tssrc/providers/index.tssrc/providers/openai.tssrc/types.ts
|
any update? |
|
@fatinghenji — pushed two small fixes to your branch via maintainer-edit access. Please review the diff and shout if anything looks wrong.
Skipped findings (won't block merge):
Stale-branch note: this branch is currently 10 commits behind main. Will land this PR + close superseded issues (#232 Ollama LLM is fully covered by Thanks for cleaning #240 up into this shape. |
Disable AUTO_COMPRESS, GRAPH_EXTRACTION_ENABLED, and CONSOLIDATION_ENABLED — all three call Gemini Flash for summarization/compression. Keys retained for future use when agentmemory ships an OpenAI-compatible provider (rohitg00/agentmemory#307) to target local vLLM/Gemma4. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add OpenAIProvider using raw fetch (no SDK dependency) - Supports any /v1/chat/completions endpoint: OpenAI, DeepSeek, SiliconFlow, Azure OpenAI, vLLM, LM Studio, Ollama - Auto-detects OPENAI_API_KEY with OPENAI_API_KEY_FOR_LLM opt-out - Add OPENAI_REASONING_EFFORT passthrough for thinking models (e.g. Ollama Cloud kimi-k2.6) to ensure content is populated - Update README with OpenAI provider table, env vars, and reasoning config
`detectProvider()` correctly gates OpenAI auto-detection on OPENAI_API_KEY_FOR_LLM !== "false", but `detectLlmProviderKind()` did not — so users who set OPENAI_API_KEY only for embeddings (via the existing OPENAI_BASE_URL + OPENAI_EMBEDDING_MODEL flow from rohitg00#186) would see /agentmemory/config/flags report `provider: llm` even though detectProvider() routed them to the noop provider. Also clarify in the README that OPENAI_REASONING_EFFORT is honored only by reasoning models (o1, o3, gpt-*-reasoning) and providers that mirror that schema (Ollama Cloud thinking models). Standard chat models reject the field with 400. Verified: - OPENAI_API_KEY=sk-... + OPENAI_API_KEY_FOR_LLM=false now returns "noop" from detectLlmProviderKind (was "llm" before the fix). - OPENAI_API_KEY=sk-... alone still returns "llm" (intended default). - npm run build clean. Note: 10 pre-existing test failures on test/mcp-standalone.test.ts are a stale-branch artefact — this branch is 10 commits behind main and is missing the MCP shim fixes that landed via rohitg00#311 / rohitg00#327. Recommend rebasing on main (or "Update branch" via the GitHub UI) before merge.
fatinghenji
force-pushed
the
feat/openai-llm-provider-v2
branch
from
691d47c to
d0e99bc
Compare
|
@rohitg00 Done. Here is what I have completed:
The branch has been force-pushed and is ready for merge. |
|
@rohitg00 Additional verification: I tested the Results:
The provider implementation is fully compatible with OpenAI-compatible endpoints including DeepSeek, SiliconFlow, etc. |
…I_KEY scope hint
Three follow-ups against the v2 PR so reviewers can see what's left
before merge:
1. Outbound fetch timeout. The other raw-fetch providers
(anthropic / gemini / openrouter / minimax) all lack one too —
that's a same-pattern repo-wide concern tracked as a follow-up
issue. This PR fixes the bound on the new surface only:
AbortController + setTimeout, default 60s, overridable via
OPENAI_TIMEOUT_MS. Abort messages explain how to raise the bound.
2. Azure OpenAI auto-detection. Azure is in the README's supported
list but the code only emitted the standard OpenAI shape. Now
detects `.openai.azure.com` hostnames at construction time and:
- swaps `Authorization: Bearer` for `api-key: <KEY>`
- drops the `/v1` path prefix (deployment is baked into base URL)
- appends `api-version=<version>` query param (env-overridable)
Default api-version is `2024-08-01-preview` per Azure docs.
Existing users of the standard OpenAI shape are unaffected — the
detection is purely hostname-driven.
3. README clarity on `OPENAI_API_KEY` shared use. Embeddings (rohitg00#186)
and this PR's LLM both auto-activate from the same key, which
surprises users who only meant one or the other. Added an
explicit callout above the LLM section pointing at the
`OPENAI_API_KEY_FOR_LLM=false` opt-out. Also documents the new
timeout + Azure base-URL shape inline.
Build clean. Tests 954/954 (10 pre-existing mcp-standalone failures
on main are not in this branch's snapshot).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/providers/openai.ts (1)
9-35: ⚡ Quick winRemove explanatory WHAT comments to match repo TypeScript rules.
Several comments describe behavior that should be conveyed by naming/structure instead.
As per coding guidelines, "Do not use code comments explaining WHAT — use clear naming instead".
Also applies to: 72-74, 83-83, 110-114, 152-152, 170-172
🤖 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/providers/openai.ts` around lines 9 - 35, Remove the explanatory "WHAT" comments in the OpenAI provider header and related inline comments and replace them with concise identifiers or clearer naming: delete the verbose behavior descriptions in the file-level comment block and near the env var listings (references: OPENAI_API_KEY, OPENAI_BASE_URL, OPENAI_MODEL, OPENAI_API_VERSION, OPENAI_TIMEOUT_MS, MAX_TOKENS, OPENAI_REASONING_EFFORT) and the other noted comment locations (around the blocks referenced at 72-74, 83, 110-114, 152, 170-172); keep only short, factual JSDoc or single-line notes that state purpose or required env vars, and ensure any behavior previously described is expressed via clear function/variable names and types instead of explanatory comments.
🤖 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/providers/openai.ts`:
- Around line 149-156: The current truthy checks on content and reasoning treat
empty strings as absent; update the logic to explicitly check for undefined/null
rather than truthiness: replace the `if (content)` and `if (reasoning)` checks
with type-safe checks (e.g., content !== undefined && content !== null) so
valid-but-empty "" responses are returned. Locate the variables `content` and
`message?.reasoning` in the openai provider function and adjust those
conditionals accordingly to return empty-string outputs while still falling back
only when values are actually undefined/null.
---
Nitpick comments:
In `@src/providers/openai.ts`:
- Around line 9-35: Remove the explanatory "WHAT" comments in the OpenAI
provider header and related inline comments and replace them with concise
identifiers or clearer naming: delete the verbose behavior descriptions in the
file-level comment block and near the env var listings (references:
OPENAI_API_KEY, OPENAI_BASE_URL, OPENAI_MODEL, OPENAI_API_VERSION,
OPENAI_TIMEOUT_MS, MAX_TOKENS, OPENAI_REASONING_EFFORT) and the other noted
comment locations (around the blocks referenced at 72-74, 83, 110-114, 152,
170-172); keep only short, factual JSDoc or single-line notes that state purpose
or required env vars, and ensure any behavior previously described is expressed
via clear function/variable names and types instead of explanatory comments.
🪄 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
Run ID: e4dc6ee1-a1ab-4dbb-a851-7e539080489c
📒 Files selected for processing (2)
README.mdsrc/providers/openai.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
| if (content) { | ||
| return content; | ||
| } | ||
| // Fallback: some thinking models return reasoning but no content | ||
| const reasoning = message?.reasoning; | ||
| if (reasoning) { | ||
| return reasoning; | ||
| } |
There was a problem hiding this comment.
Handle empty-string responses explicitly.
if (content)/if (reasoning) treats "" as absent and can throw on valid-but-empty model output. Check type instead of truthiness.
Suggested fix
- const content = message?.content;
- if (content) {
+ const content = message?.content;
+ if (typeof content === "string") {
return content;
}
// Fallback: some thinking models return reasoning but no content
const reasoning = message?.reasoning;
- if (reasoning) {
+ if (typeof reasoning === "string") {
return reasoning;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (content) { | |
| return content; | |
| } | |
| // Fallback: some thinking models return reasoning but no content | |
| const reasoning = message?.reasoning; | |
| if (reasoning) { | |
| return reasoning; | |
| } | |
| if (typeof content === "string") { | |
| return content; | |
| } | |
| // Fallback: some thinking models return reasoning but no content | |
| const reasoning = message?.reasoning; | |
| if (typeof reasoning === "string") { | |
| return reasoning; | |
| } |
🤖 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/providers/openai.ts` around lines 149 - 156, The current truthy checks on
content and reasoning treat empty strings as absent; update the logic to
explicitly check for undefined/null rather than truthiness: replace the `if
(content)` and `if (reasoning)` checks with type-safe checks (e.g., content !==
undefined && content !== null) so valid-but-empty "" responses are returned.
Locate the variables `content` and `message?.reasoning` in the openai provider
function and adjust those conditionals accordingly to return empty-string
outputs while still falling back only when values are actually undefined/null.
|
Thanks @fatinghenji |
…lish (#432) Patch bump per the established rule: additive surface only. OpenAI provider is a new optional surface that activates only when OPENAI_API_KEY is set, gated by OPENAI_API_KEY_FOR_LLM. Telemetry project_name pin is pure observability metadata. Compare polish is docs/website only. PRs included since v0.9.16: #307 — OpenAI-compatible LLM provider (universal adapter for OpenAI, Azure OpenAI auto-detected by hostname, DeepSeek, SiliconFlow, vLLM, LM Studio, Ollama). Plus the maintainer- pushed Azure detection + fetch timeout + README scope hint follow-ups. Closes #185, #232, #312, supersedes #240. #426 — pin worker telemetry project_name #427 — Compare section polish (title + native plugins cell + grid) Files bumped (9): package.json, packages/mcp/package.json, plugin/.claude-plugin/plugin.json, plugin/.codex-plugin/plugin.json, src/version.ts, src/types.ts, src/functions/export-import.ts, test/export-import.test.ts, CHANGELOG.md
3 participants
Summary
Adds a new
openaiLLM provider that uses raw fetch to call any OpenAI-compatible/v1/chat/completionsendpoint.Changes
src/types.ts: AddopenaitoProviderTypeunionsrc/providers/openai.ts: NewOpenAIProviderclass using raw fetch (no SDK dependency)/v1/chat/completionsendpointOPENAI_API_KEY,OPENAI_BASE_URL,OPENAI_MODELenv varsOPENAI_REASONING_EFFORTpassthrough for thinking models (Ollama Cloud, etc.)reasoningifcontentis emptysrc/providers/index.ts: Wireopenaicase intocreateBaseProvider()src/config.ts:OPENAI_API_KEYdetection todetectProvider()(withOPENAI_API_KEY_FOR_LLMopt-out)OPENAI_API_KEYtodetectLlmProviderKind()openaitoVALID_PROVIDERSsetREADME.md: Add OpenAI to LLM provider table and document all env varsSupported Endpoints
Configuration Example
Backwards Compatibility
OPENAI_API_KEYis now checked first indetectProvider(), but only activates when the key is presentOPENAI_API_KEYfor embedding and prefer another LLM provider can setOPENAI_API_KEY_FOR_LLM=falseto skip auto-detectionTesting
npm run buildpassesnpm testpassesChecklist
Closes #185
Supersedes #240
Summary by CodeRabbit
New Features
Documentation