feat: add AI LLM gateway integration support#2112
feat: add AI LLM gateway integration support#2112praisonai-triage-agent[bot] wants to merge 2 commits into
Conversation
- Add gateway providers for OpenRouter, LiteLLM Proxy, and custom gateways - Support for 100+ LLM providers through unified OpenAI-compatible endpoints - Built-in caching, fallback, and analytics through gateway services - Minimal implementation using existing LiteLLM infrastructure - Add comprehensive tests and examples Co-authored-by: praisonai-triage-agent[bot] <praisonai-triage-agent[bot]@users.noreply.github.com>
|
@coderabbitai review |
|
/review |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
✅ Action performedReview finished.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds an LLM gateway abstraction to PraisonAI via a new ChangesLLM Gateway Provider Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
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 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 |
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above from Qodo, Coderabbit, and Gemini first — incorporate their findings. Review areas:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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
🧹 Nitpick comments (2)
src/praisonai/examples/llm_gateway_example.py (1)
137-140: ⚡ Quick winAvoid hardcoded API keys in examples; read from env/config instead.
Even placeholder secrets in examples encourage unsafe copy/paste patterns.
Suggested fix
self.endpoints = [ - {"base_url": "https://primary.api.com/v1", "api_key": "key1"}, - {"base_url": "https://fallback.api.com/v1", "api_key": "key2"}, + { + "base_url": os.getenv("PRIMARY_GATEWAY_URL", "https://primary.api.com/v1"), + "api_key": os.getenv("PRIMARY_GATEWAY_API_KEY", "") + }, + { + "base_url": os.getenv("FALLBACK_GATEWAY_URL", "https://fallback.api.com/v1"), + "api_key": os.getenv("FALLBACK_GATEWAY_API_KEY", "") + }, ]🤖 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/praisonai/examples/llm_gateway_example.py` around lines 137 - 140, The endpoints list initialization contains hardcoded API key values ("key1" and "key2") which encourages unsafe practices. Replace these hardcoded strings with environment variable lookups using os.getenv() or similar method, providing meaningful environment variable names (such as PRIMARY_API_KEY and FALLBACK_API_KEY) that users can configure safely without modifying the example code. Ensure appropriate error handling or defaults if the environment variables are not set.src/praisonai/tests/unit/llm/test_gateway_providers.py (1)
84-86: ⚡ Quick winUnskip the generate-path test by stubbing
litellminstead of skipping.This currently leaves the most important request-shaping path unverified in CI.
Suggested direction
- `@pytest.mark.skip`(reason="litellm required for this test") - `@patch`("litellm.completion") - def test_gateway_generate_method(self, mock_completion): + `@patch`("litellm.completion") + def test_gateway_generate_method(self, mock_completion):If
litellmmight be missing in test environments, inject a stub module withmonkeypatch/sys.modulesin the test setup so this test always runs.🤖 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/praisonai/tests/unit/llm/test_gateway_providers.py` around lines 84 - 86, Remove the `@pytest.mark.skip` decorator from the test_gateway_generate_method test and instead use monkeypatch to inject a stub litellm module into sys.modules in a conftest fixture or directly within the test setup, so the test can run even when the actual litellm library is not installed in the test environment. This ensures the important request-shaping path is verified in CI without requiring external dependencies.
🤖 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/praisonai/praisonai/llm/gateways.py`:
- Around line 45-49: The current implementation of assigning self.config =
config or {} creates a direct reference to the caller-provided dictionary, so
when _setup_config() mutates self.config, it modifies the original dictionary
passed by the caller. To fix this, create a shallow copy of the config
dictionary using the copy method or dict() constructor before assigning it to
self.config. This ensures that any mutations performed by _setup_config() or
other internal methods only affect the instance's copy and do not leak state
back to the caller's original configuration dictionary.
---
Nitpick comments:
In `@src/praisonai/examples/llm_gateway_example.py`:
- Around line 137-140: The endpoints list initialization contains hardcoded API
key values ("key1" and "key2") which encourages unsafe practices. Replace these
hardcoded strings with environment variable lookups using os.getenv() or similar
method, providing meaningful environment variable names (such as PRIMARY_API_KEY
and FALLBACK_API_KEY) that users can configure safely without modifying the
example code. Ensure appropriate error handling or defaults if the environment
variables are not set.
In `@src/praisonai/tests/unit/llm/test_gateway_providers.py`:
- Around line 84-86: Remove the `@pytest.mark.skip` decorator from the
test_gateway_generate_method test and instead use monkeypatch to inject a stub
litellm module into sys.modules in a conftest fixture or directly within the
test setup, so the test can run even when the actual litellm library is not
installed in the test environment. This ensures the important request-shaping
path is verified in CI without requiring external dependencies.
🪄 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: 50af9c5a-0d28-41d4-9aec-ad3873bc62d4
📒 Files selected for processing (4)
src/praisonai/examples/llm_gateway_example.pysrc/praisonai/praisonai/llm/__init__.pysrc/praisonai/praisonai/llm/gateways.pysrc/praisonai/tests/unit/llm/test_gateway_providers.py
| self.config = config or {} | ||
| self.provider_id = self.__class__.__name__.lower().replace("provider", "") | ||
|
|
||
| # Merge environment variables with config | ||
| self._setup_config() |
There was a problem hiding this comment.
Avoid mutating caller-owned config dicts in-place.
self.config = config or {} keeps the original reference, so _setup_config() mutates the caller’s dict (and can leak state across reused configs).
Suggested fix
- self.config = config or {}
+ self.config = dict(config) if config else {}📝 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.
| self.config = config or {} | |
| self.provider_id = self.__class__.__name__.lower().replace("provider", "") | |
| # Merge environment variables with config | |
| self._setup_config() | |
| self.config = dict(config) if config else {} | |
| self.provider_id = self.__class__.__name__.lower().replace("provider", "") | |
| # Merge environment variables with config | |
| self._setup_config() |
🤖 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/praisonai/praisonai/llm/gateways.py` around lines 45 - 49, The current
implementation of assigning self.config = config or {} creates a direct
reference to the caller-provided dictionary, so when _setup_config() mutates
self.config, it modifies the original dictionary passed by the caller. To fix
this, create a shallow copy of the config dictionary using the copy method or
dict() constructor before assigning it to self.config. This ensures that any
mutations performed by _setup_config() or other internal methods only affect the
instance's copy and do not leak state back to the caller's original
configuration dictionary.
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
|
Claude finished @MervinPraison's task —— View job 🔧 Final Architecture Review & FixesI've reviewed this PR against AGENTS.md architecture principles and fixed the critical issues identified by reviewers. Todo List:
Architecture Review✅ Protocol-driven design: The gateway providers follow the protocol pattern correctly - base class ✅ Backward compatibility: No existing features broken, this is purely additive. ✅ Performance: No hot-path regressions, uses lazy loading appropriately. Fixed Issues1. ✅ Critical Lazy Loading Registration Gap (Greptile)
2. ✅ Provider ID Inconsistency
3. ✅ Config Dictionary Mutation (CodeRabbit)
4. ✅ Hardcoded API Keys in Examples
5. ✅ Enabled Skipped Test
Testing Results✅ All gateway providers registered correctly Files Modified
✅ Final VerdictPR APPROVED - All critical issues have been resolved. The gateway integration is properly implemented following the architecture guidelines, with all reviewer concerns addressed. The implementation is clean, backward compatible, and adds valuable functionality without introducing regressions. |
- Fix lazy loading registration gap by eagerly registering gateways on module import - Fix provider_id inconsistency for LiteLLMProxyProvider (now correctly returns 'litellm-proxy') - Fix config dictionary mutation by creating shallow copy in __init__ - Remove hardcoded API keys from example, use environment variables instead - Enable previously skipped test using monkeypatch to mock litellm module Addresses review feedback from Greptile, CodeRabbit, and other reviewers
1 participant
Fixes #2111
Summary
This PR adds support for AI LLM gateways like LiteLLM.ai, OpenRouter.ai, and custom OpenAI-compatible endpoints, providing unified access to 100+ LLM providers through a single interface.
Changes
Benefits
Testing
Generated with Claude Code
Summary by CodeRabbit
Release Notes