feat(adapters): OpenAI-compatible LLM adapter - #550
Conversation
Add OpenAIProvider implementing the LLMProvider ABC with full support
for OpenAI and any OpenAI-compatible endpoint (Ollama, vLLM, Groq, etc.)
via a configurable base_url. Includes tool use, streaming, stop-reason
mapping, and credential manager integration.
- Create codeframe/adapters/llm/openai.py — OpenAIProvider(LLMProvider)
- Update codeframe/adapters/llm/__init__.py — export + get_provider("openai")
- Create tests/adapters/test_llm_openai.py — 22 tests, all passing
Closes #543
|
Caution Review failedPull request was closed or merged during review WalkthroughAdds an OpenAI-compatible LLM adapter: new Changes
Sequence DiagramsequenceDiagram
participant Client
participant OpenAIProvider
participant CredentialMgr
participant OpenAI_SDK
participant OpenAI_API
Client->>OpenAIProvider: initialize(api_key?, base_url?, model?, credential_manager?)
OpenAIProvider->>CredentialMgr: credential_manager.get_credential() (if provided)
OpenAIProvider-->>Client: resolved api_key / error
Client->>OpenAIProvider: complete(messages, tools, system?, temperature, max_tokens)
OpenAIProvider->>OpenAIProvider: _convert_messages() / _convert_tools()
OpenAIProvider->>OpenAI_SDK: chat.completions.create(model, messages, tools..., stream=False)
OpenAI_SDK->>OpenAI_API: POST /chat/completions
OpenAI_API-->>OpenAI_SDK: response (choices, usage, finish_reason)
OpenAI_SDK-->>OpenAIProvider: response
OpenAIProvider->>OpenAIProvider: parse response (content, tool_calls, tokens, stop_reason)
OpenAIProvider-->>Client: LLMResponse (content/tool_calls/tokens/stop_reason)
Client->>OpenAIProvider: stream(messages...)
OpenAIProvider->>OpenAI_SDK: chat.completions.create(..., stream=True)
OpenAI_SDK->>OpenAI_API: streaming events
OpenAI_API-->>OpenAI_SDK: stream chunks
OpenAI_SDK-->>OpenAIProvider: stream chunks
OpenAIProvider-->>Client: yield text chunks
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@codeframe/adapters/llm/openai.py`:
- Around line 44-54: The constructor currently accepts model_selector and
complete()/stream() accept purpose, but get_model() always returns self.model,
silently ignoring model_selector and purpose; change get_model (or the
model-resolution flow used by complete/stream) to check if a model_selector is
provided and if so call model_selector.for_purpose(purpose) (falling back to
self.model if selector returns None), or if an explicit single-model override
behavior is intended, make that explicit via a flag; update references in the
OpenAI adapter methods (get_model, complete, stream) to pass the Purpose enum
value to model_selector.for_purpose and use its result for API calls so
PLANNING/EXECUTION/GENERATION routes can select different models.
- Around line 130-131: The code currently drops explicit temperature=0.0 because
it only sets kwargs["temperature"] when temperature > 0; update both complete()
and stream() to forward temperature whenever the caller provided it (i.e., check
for temperature is not None rather than > 0) and assign kwargs["temperature"] =
temperature so that a 0.0 value is sent to the OpenAI API, preserving caller
intent (modify the branches that populate the kwargs dict in complete() and
stream()).
In `@tests/adapters/test_llm_openai.py`:
- Around line 303-337: Update the two tests test_authentication_error_surfaced
and test_rate_limit_error_surfaced to assert the provider contract (that
OpenAIProvider.complete raises a ValueError with the expected message fragment)
instead of allowing either the wrapped ValueError or the raw openai exception;
call provider.complete inside a with pytest.raises(ValueError) block and assert
the exception message contains the expected fragment (e.g., "authentication" or
"rate limit") to ensure the adapter is wrapping underlying
openai.AuthenticationError and openai.RateLimitError into a ValueError.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71757c9b-4b77-4bdf-97b5-70975facca8d
📒 Files selected for processing (3)
codeframe/adapters/llm/__init__.pycodeframe/adapters/llm/openai.pytests/adapters/test_llm_openai.py
| model_selector: Optional[ModelSelector] = None, | ||
| credential_manager: Optional["CredentialManager"] = None, | ||
| ): | ||
| """Initialize the OpenAI provider. | ||
|
|
||
| Args: | ||
| api_key: OpenAI API key (defaults to OPENAI_API_KEY env var) | ||
| model: Default model to use for all purposes | ||
| base_url: Custom endpoint URL for OpenAI-compatible APIs | ||
| model_selector: Custom model selector (ignored — provider uses self.model) | ||
| credential_manager: Optional credential manager for secure key retrieval |
There was a problem hiding this comment.
Don't make purpose and model_selector inert.
complete() and stream() both accept purpose, and the constructor still accepts model_selector, but get_model() always returns self.model. That makes every purpose route identically and silently ignores any caller-supplied selector. If single-model mode is intentional, make it an explicit override; otherwise honor model_selector.for_purpose(purpose) at least when a selector is provided.
As per coding guidelines: codeframe/adapters/llm/**/*.py: Support Purpose enum for model selection (PLANNING, EXECUTION, GENERATION).
Also applies to: 81-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@codeframe/adapters/llm/openai.py` around lines 44 - 54, The constructor
currently accepts model_selector and complete()/stream() accept purpose, but
get_model() always returns self.model, silently ignoring model_selector and
purpose; change get_model (or the model-resolution flow used by complete/stream)
to check if a model_selector is provided and if so call
model_selector.for_purpose(purpose) (falling back to self.model if selector
returns None), or if an explicit single-model override behavior is intended,
make that explicit via a flag; update references in the OpenAI adapter methods
(get_model, complete, stream) to pass the Purpose enum value to
model_selector.for_purpose and use its result for API calls so
PLANNING/EXECUTION/GENERATION routes can select different models.
- Honor explicit model_selector when provided (Purpose routing works) - Always forward temperature=0.0 to API (OpenAI defaults to 1 when omitted) - Tighten error tests to assert ValueError contract, not raw openai types
Follow-up ReviewChecking the current state against the issues raised in my previous review: Fixed:
Still outstanding (blocking or near-blocking): 1.
try:
for chunk in self.client.chat.completions.create(**kwargs):
content = chunk.choices[0].delta.content
if content is not None:
yield content
except openai.AuthenticationError as exc:
raise ValueError(f"OpenAI authentication failed: {exc}") from exc
except openai.RateLimitError as exc:
raise ValueError(f"OpenAI rate limit exceeded: {exc}") from exc
except openai.NotFoundError as exc:
raise ValueError(f"OpenAI model not found: {exc}") from exc2. elif provider_type == "openai":
return OpenAIProvider()The entire Ollama/vLLM use case depends on def get_provider(provider_type: str = "anthropic", **kwargs) -> LLMProvider:
...
elif provider_type == "openai":
return OpenAIProvider(**kwargs)3. The PR adds Minor (unchanged from before):
|
1 participant
Summary
OpenAIProvider(LLMProvider)incodeframe/adapters/llm/openai.py— mirrorsAnthropicProviderstructure exactlybase_urlconstructor param covers the entire OpenAI-compatible ecosystem: OpenAI, Ollama, vLLM, LM Studio, Groq, Together, etc.codeframe/adapters/llm/__init__.pyto exportOpenAIProviderand wireget_provider("openai")tests/adapters/test_llm_openai.pywith 22 tests covering init, complete, stream, base_url routing, error handling, tool round-trip, and message conversionCloses #543 | Part of #542
Acceptance criteria
OpenAIProviderpasses same interface contract asAnthropicProviderbase_urloverride routes to custom endpoint (tested: Ollama pattern)base_urloverride (requires live Ollama instance)Test plan
uv run pytest tests/adapters/test_llm_openai.py— 22 passeduv run pytest tests/adapters/— 81 passed (no regressions)uv run ruff check codeframe/adapters/llm/openai.py tests/adapters/test_llm_openai.py— cleanSummary by CodeRabbit
New Features
Tests