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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 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), 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
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
| const response = await fetch(url, { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| Authorization: `Bearer ${this.apiKey}`, | ||
| }, | ||
| body: JSON.stringify(body), | ||
| }); |
There was a problem hiding this comment.
Add a timeout to outbound OpenAI-compatible requests.
These fetch calls can hang indefinitely on network/upstream stalls, which can block provider operations.
Suggested fix
- const response = await fetch(url, {
- method: "POST",
- headers: {
- "Content-Type": "application/json",
- Authorization: `Bearer ${this.apiKey}`,
- },
- body: JSON.stringify(body),
- });
+ const controller = new AbortController();
+ const timeout = setTimeout(() => controller.abort(), 30_000);
+ let response: Response;
+ try {
+ response = await fetch(url, {
+ method: "POST",
+ headers: {
+ "Content-Type": "application/json",
+ Authorization: `Bearer ${this.apiKey}`,
+ },
+ body: JSON.stringify(body),
+ signal: controller.signal,
+ });
+ } finally {
+ clearTimeout(timeout);
+ }📝 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.
| const response = await fetch(url, { | |
| method: "POST", | |
| headers: { | |
| "Content-Type": "application/json", | |
| Authorization: `Bearer ${this.apiKey}`, | |
| }, | |
| body: JSON.stringify(body), | |
| }); | |
| const controller = new AbortController(); | |
| const timeout = setTimeout(() => controller.abort(), 30_000); | |
| let response: Response; | |
| try { | |
| response = await fetch(url, { | |
| method: "POST", | |
| headers: { | |
| "Content-Type": "application/json", | |
| Authorization: `Bearer ${this.apiKey}`, | |
| }, | |
| body: JSON.stringify(body), | |
| signal: controller.signal, | |
| }); | |
| } finally { | |
| clearTimeout(timeout); | |
| } |
🤖 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 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.
|
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. |
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