feat(provider): multi-provider support (Anthropic / OpenAI / Ollama / Gemini / DeepSeek / Groq)#28
Merged
Merged
Conversation
Switching provider is now a config change, not a code change. complete() dispatches on a new keyword-only `provider`: Anthropic keeps its own transport (byte-for-byte unchanged), while openai/ollama/gemini/deepseek/ groq share one OpenAI-compatible transport via the `openai` SDK. One path unlocks five providers; both transports collapse their SDK's exception hierarchy into the existing ProviderError, so the error taxonomy stays unified and callers never import a provider SDK. Ollama works with zero setup — local, no API key — removing the paid-key try-it blocker that gated the tool. - config.py: expand Provider enum; add LlmConfig.provider/base_url; from_dict coerces an unknown provider back to the default with a warning rather than letting it crash later inside complete(). - key_store.py: replace the hardcoded ANTHROPIC_ENV_VAR with a provider->env-var table; get() is one loop. Ollama is absent on purpose (keyless), so it never reads an env var. - provider.py: _complete_anthropic + _complete_openai_compat behind complete(); per-provider default base URLs with config override. - suggest/test: pass provider+base_url (two lines each, no SDK import). - README: document multi-provider, Ollama (no key), and base_url override. Closes #27 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR extends noidea from Anthropic-only to support five additional LLM providers (OpenAI, Ollama, Gemini, DeepSeek, Groq) via configuration changes. The transport layer refactors into two paths—Anthropic-native and OpenAI-compatible—unified under a single ChangesMulti-provider transport and configuration
Sequence DiagramsequenceDiagram
participant Caller
participant complete
participant _complete_anthropic
participant _complete_openai_compat
Caller->>complete: complete(messages, provider="openai", base_url="")
alt provider == "anthropic"
complete->>_complete_anthropic: route to Anthropic SDK
_complete_anthropic->>_complete_anthropic: client.messages.create()
else provider in OpenAI-compatible set
complete->>_complete_openai_compat: route to OpenAI SDK
_complete_openai_compat->>_complete_openai_compat: resolve base_url from defaults
_complete_openai_compat->>_complete_openai_compat: select API key (keyless for ollama)
_complete_openai_compat->>_complete_openai_compat: client.chat.completions.create()
else unknown provider
complete->>complete: raise ValueError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
noidea/provider.py (2)
128-132:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard empty completion payloads before indexing into response arrays.
Both transports index the first element without checking for an empty list, which can raise uncaught
IndexErrorand bypass normalized provider error handling.Proposed fix
- block = message.content[0] + if not message.content: + raise ProviderError(ErrorKind.STATUS, "Anthropic response contained no content blocks.") + block = message.content[0] @@ - content = response.choices[0].message.content + if not response.choices: + raise ProviderError(ErrorKind.STATUS, "Provider response contained no choices.") + content = response.choices[0].message.contentAlso applies to: 179-183
🤖 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 `@noidea/provider.py` around lines 128 - 132, The code indexes message.content[0] without checking for an empty list which can raise an uncaught IndexError and bypass normalized provider error handling; update both locations (the block = message.content[0] sites) to first check that message.content is non-empty (e.g., if not message.content or len(message.content) == 0) and, if empty, raise the provider-normalized error used elsewhere (replace with the project's ProviderError or the same error class the provider layer expects) with a clear message; after that safe-guard, proceed to check isinstance(block, TextBlock) and return block.text as before.
85-88:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject
boolfor numeric parameters (max_tokens,temperature).
boolis a subclass ofint, soTrue/Falsecurrently pass the numeric guards and can be treated as valid inputs.Proposed fix
- if not isinstance(max_tokens, int) or max_tokens <= 0: + if isinstance(max_tokens, bool) or not isinstance(max_tokens, int) or max_tokens <= 0: raise TypeError(f"max_tokens must be a positive integer, got {type(max_tokens).__name__}") - if not isinstance(temperature, (int, float)) or temperature < 0: + if ( + isinstance(temperature, bool) + or not isinstance(temperature, (int, float)) + or temperature < 0 + ): raise TypeError(f"temperature must be a non-negative number, got {temperature!r}")🤖 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 `@noidea/provider.py` around lines 85 - 88, The current validators for max_tokens and temperature accept bool because bool is a subclass of int; update the checks in provider.py (the max_tokens and temperature validation block) to explicitly reject booleans before numeric validation — e.g., first if isinstance(max_tokens, bool): raise TypeError(...), then ensure isinstance(max_tokens, int) and >0; similarly for temperature check isinstance(temperature, bool) then ensure isinstance(temperature, (int, float)) and temperature >= 0 — use the function/variable names max_tokens and temperature in the error messages to make the failures clear.
🧹 Nitpick comments (1)
tests/test_config.py (1)
45-47: ⚡ Quick winDerive expected provider names from the enum instead of hardcoding them.
This test can go stale when
Providerchanges. Building cases fromProviderkeeps coverage aligned automatically.Proposed refactor
-from noidea.config import ( - LlmConfig, - deep_merge, - initialize, - load_config, -) +from noidea.config import LlmConfig, Provider, deep_merge, initialize, load_config @@ def test_from_dict_accepts_every_known_provider(self): - for name in ("anthropic", "openai", "ollama", "gemini", "deepseek", "groq"): + for name in (member.value for member in Provider): assert LlmConfig.from_dict({"provider": name}).provider == name🤖 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 `@tests/test_config.py` around lines 45 - 47, The test test_from_dict_accepts_every_known_provider hardcodes provider names and should derive them from the Provider enum; update the test to import Provider and iterate over its members (e.g., for provider in Provider: use provider.value or provider.name as appropriate) and assert LlmConfig.from_dict({"provider": provider.value}).provider == provider.value so the cases stay in sync with the Provider enum and avoid staleness.
🤖 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 `@noidea/provider.py`:
- Around line 58-61: Replace all assert-based argument validations in this file
with explicit exceptions: in get_api_key() replace the provider assert with
raising TypeError or ValueError (e.g., non-string or empty provider) and replace
the key existence assert with a ValueError if key is missing; in complete()
replace the provider and base_url asserts with explicit TypeError/ValueError as
appropriate; and update the other assert validations (model, max_tokens,
provider checks) to raise specific exceptions (TypeError for wrong types,
ValueError for invalid values) so checks are not stripped with python -O. Use
the function names get_api_key and complete and the existing validation
locations (e.g., model/max_tokens checks) to find and update each assertion.
---
Outside diff comments:
In `@noidea/provider.py`:
- Around line 128-132: The code indexes message.content[0] without checking for
an empty list which can raise an uncaught IndexError and bypass normalized
provider error handling; update both locations (the block = message.content[0]
sites) to first check that message.content is non-empty (e.g., if not
message.content or len(message.content) == 0) and, if empty, raise the
provider-normalized error used elsewhere (replace with the project's
ProviderError or the same error class the provider layer expects) with a clear
message; after that safe-guard, proceed to check isinstance(block, TextBlock)
and return block.text as before.
- Around line 85-88: The current validators for max_tokens and temperature
accept bool because bool is a subclass of int; update the checks in provider.py
(the max_tokens and temperature validation block) to explicitly reject booleans
before numeric validation — e.g., first if isinstance(max_tokens, bool): raise
TypeError(...), then ensure isinstance(max_tokens, int) and >0; similarly for
temperature check isinstance(temperature, bool) then ensure
isinstance(temperature, (int, float)) and temperature >= 0 — use the
function/variable names max_tokens and temperature in the error messages to make
the failures clear.
---
Nitpick comments:
In `@tests/test_config.py`:
- Around line 45-47: The test test_from_dict_accepts_every_known_provider
hardcodes provider names and should derive them from the Provider enum; update
the test to import Provider and iterate over its members (e.g., for provider in
Provider: use provider.value or provider.name as appropriate) and assert
LlmConfig.from_dict({"provider": provider.value}).provider == provider.value so
the cases stay in sync with the Provider enum and avoid staleness.
🪄 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: 391d1357-0b16-483d-bef6-8921d86bf7ab
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
README.mdnoidea/commands/keys.pynoidea/commands/suggest.pynoidea/commands/test.pynoidea/config.pynoidea/key_store.pynoidea/provider.pypyproject.tomltests/test_config.pytests/test_key_store.pytests/test_provider.py
Comment on lines
+58
to
+61
| def get_api_key(provider: str = "anthropic") -> str: | ||
| # key_store consults the keyring first, then the provider's *_API_KEY env var for CI/headless. | ||
| assert isinstance(provider, str) and provider, "provider must be a non-empty string" | ||
| key = key_store.get(provider) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file (in case of casing/structure differences)
ls -la
find . -maxdepth 4 -type f -path '*noidea/provider.py' -print
# Show relevant portions around the referenced lines
python3 - <<'PY'
from pathlib import Path
paths = []
for p in Path('.').rglob('noidea/provider.py'):
paths.append(p)
if not paths:
raise SystemExit("noidea/provider.py not found")
path = paths[0]
print(f"Using: {path}")
def show(start, end):
print(f"\n--- {path}:{start}-{end} ---")
with path.open('r', encoding='utf-8') as f:
for i, line in enumerate(f, start=1):
if start <= i <= end:
print(f"{i:4d}: {line.rstrip()}")
show(40, 80)
show(80, 110)
PY
# Search for asserts in this file (and any get_api_key / validation logic)
rg -n --hidden --no-ignore-vcs "assert\s" noidea/provider.py || true
rg -n --hidden --no-ignore-vcs "def get_api_key|get_api_key\(" noidea/provider.py || trueRepository: AccursedGalaxy/noidea
Length of output: 6259
Replace assert-based argument validation with explicit exceptions in noidea/provider.py.
get_api_key()usesassertforproviderandkey(lines 60 and 64); replace with explicitTypeError/ValueErrorso checks can’t be stripped by Python-O.complete()usesassertforproviderandbase_url(lines 89-90); replace with explicit exceptions.- There are additional
assertvalidations elsewhere in this file (e.g., model/max_tokens and provider checks); handle them the same way for consistency with the coding guideline.
🤖 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 `@noidea/provider.py` around lines 58 - 61, Replace all assert-based argument
validations in this file with explicit exceptions: in get_api_key() replace the
provider assert with raising TypeError or ValueError (e.g., non-string or empty
provider) and replace the key existence assert with a ValueError if key is
missing; in complete() replace the provider and base_url asserts with explicit
TypeError/ValueError as appropriate; and update the other assert validations
(model, max_tokens, provider checks) to raise specific exceptions (TypeError for
wrong types, ValueError for invalid values) so checks are not stripped with
python -O. Use the function names get_api_key and complete and the existing
validation locations (e.g., model/max_tokens checks) to find and update each
assertion.
- Remove try-catch for openai import; it's a core dependency - Add TestProviderTablesStayInSync to prevent provider enum drift - Document reasoning model incompatibility in README
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
1 participant
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.
Closes #27.
What
Switching LLM provider is now a config change, not a code change. The transport in
provider.pywas hard-wired to Anthropic; this puts two private transports behind the existing publiccomplete():_complete_anthropic(...)— today's logic, extracted, byte-for-byte behaviorally unchanged._complete_openai_compat(...)— one transport via theopenaiSDK serving five providers (openai/ollama/gemini/deepseek/groq), since they all speak OpenAI's chat-completions format.complete(..., *, provider="anthropic", base_url="")dispatches onprovider. The additions are keyword-only and fully backward-compatible. Both transports collapse their SDK's exception hierarchy into the existingProviderError(ErrorKind.*), so the error taxonomy stays unified in one place and callers never import a provider SDK.Ollama works with zero setup — local, no API key — removing the single biggest try-it blocker (you can now try noidea without a paid key).
Changes
config.py— expand theProviderenum (openai/ollama/gemini/deepseek/groq); addLlmConfig.provider+base_url.from_dictcoerces an unknown provider back to the default with a warning (so a hand-edited typo warns at load time rather than crashing later insidecomplete()).key_store.py— replace the hardcodedANTHROPIC_ENV_VARwith a_PROVIDER_ENV_VARStable;get()is one loop. Ollama is absent on purpose (keyless), so it never reads an env var.provider.py— two transports + dispatch; per-provider default base URLs (ollama → http://localhost:11434/v1, etc.) overridable via configbase_url.commands/suggest.py+commands/test.py— passprovider/base_url(two lines each, no SDK import).commands/keys.py— help note that Ollama needs no key.pyproject.toml— addopenai(transport for 5 of 6 providers).base_urloverride.How it was tested
poetry run pytest -q). All pre-existingtest_provider.pyAnthropic tests pass untouched.openai.OpenAI), Ollama default base-url + no-key path, base_url override, unknown-provider rejection, multi-provider env fallback inkey_store, and unknown-provider config coercion.noidea suggestwithprovider=ollamaand no API key configured against a local Ollama (gemma4), which generatedfeat(calc): add add function to calculatorvia the OpenAI-compat path — proving the gating acceptance criterion.black+isortclean; functions ≤70 lines, ≥2 assertions each, ≤100 cols (TigerStyle).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests