fix: use LLM abstraction instead of hardcoded OpenAI in hierarchical process #889
Conversation
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
…process - Replace direct OpenAI client instantiation with LLM class - Support all LLM providers (Gemini, Anthropic, etc.) in manager tasks - Remove unused OpenAI imports from process.py - Fix issue #873 where hierarchical workflow required OpenAI API key Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Contributor
WalkthroughThe code refactors manager instruction retrieval in the hierarchical process to use a unified Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AutoAgents
participant Process
participant LLM
User->>AutoAgents: start(process="hierarchical", llm="gemini" or mock)
AutoAgents->>Process: run manager task
Process->>Process: _create_llm_instance()
Process->>LLM: get_response(manager_prompt, ...)
LLM-->>Process: response
Process->>Process: _parse_manager_instructions(response, ManagerInstructions)
Process-->>AutoAgents: instructions
AutoAgents-->>User: workflow result
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Summary of Changes
Hello @MervinPraison, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request resolves an issue where the AutoAgents hierarchical workflow incorrectly mandated an OpenAI API key, even when configured to use alternative LLM providers. The core change involves refactoring the manager task's LLM interactions to use a unified LLM abstraction, thereby enabling broader compatibility with various LLM services and eliminating the hardcoded OpenAI dependency.
Highlights
- LLM Abstraction: Replaced direct instantiations of
OpenAIandAsyncOpenAIclients with a genericLLMclass, centralizing LLM interactions. - Multi-LLM Provider Support: Ensured that the hierarchical process's manager tasks can now seamlessly utilize any configured LLM provider (e.g., Gemini, Anthropic) by leveraging the new
LLMabstraction, removing the previous hardcoded dependency on OpenAI. - Code Cleanup: Removed unused
openaiimports fromprocess.pyto streamline the codebase. - New Test Coverage: Added a new test file (
test_autoagents_hierarchical_fix.py) to specifically verify that the hierarchical workflow functions correctly with non-OpenAI LLMs and does not require an OpenAI API key.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Contributor
There was a problem hiding this comment.
Code Review
The code changes replace the hardcoded OpenAI client with the LLM abstraction, making the system more flexible. The addition of a dedicated test file is excellent. The review includes suggestions focused on improving maintainability by reducing code duplication in process.py and enhancing the structure of the new test file.
| # Create OpenAI client | ||
| client = OpenAI() | ||
| # Create LLM instance with the manager_llm | ||
| llm = LLM(model=self.manager_llm, temperature=0.7) |
Contributor
There was a problem hiding this comment.
The LLM instantiation is repeated in _get_structured_response_async (line 231) and _get_json_response_async (line 254) with the same configuration. This duplication can lead to inconsistencies if the configuration needs to be changed in the future. Consider creating a private helper method to centralize the creation of this LLM instance.
def _create_llm_instance(self):
return LLM(model=self.manager_llm, temperature=0.7)
def _get_manager_instructions_with_fallback(self, manager_task, manager_prompt, ManagerInstructions):
llm = self._create_llm_instance()
Comment on lines
183
to
191
| # Parse the response and validate with Pydantic | ||
| try: | ||
| parsed_json = json.loads(response) | ||
| return ManagerInstructions(**parsed_json) | ||
| except (json.JSONDecodeError, ValueError) as e: | ||
| # If parsing fails, try direct instantiation in case response is already parsed | ||
| if isinstance(response, ManagerInstructions): | ||
| return response | ||
| raise Exception(f"Failed to parse response: {response}") from e |
Contributor
There was a problem hiding this comment.
This response parsing and validation logic is duplicated in _get_structured_response_async on lines 241-249. Extracting this logic into a private helper method would reduce code duplication. Additionally, _get_json_response_async has an inconsistent implementation of this parsing logic, as it's missing the isinstance check that is present here and in _get_structured_response_async.
def _parse_manager_instructions(self, response, ManagerInstructions):
try:
parsed_json = json.loads(response)
return ManagerInstructions(**parsed_json)
except (json.JSONDecodeError, ValueError) as e:
if isinstance(response, ManagerInstructions):
return response
raise Exception(f"Failed to parse response: {response}") from e
def _get_manager_instructions_with_fallback(self, manager_task, manager_prompt, ManagerInstructions):
response = llm.get_response(...)
return self._parse_manager_instructions(response, ManagerInstructions)| if "api_key" in str(e) and "OPENAI_API_KEY" in str(e): | ||
| print("\n=== TEST FAILED ===") | ||
| print(f"Error: Still requires OpenAI API key: {e}") | ||
| sys.exit(1) |
Contributor
There was a problem hiding this comment.
Using sys.exit(1) in a test function will terminate the entire test process immediately upon the first failure, preventing other tests from running. A better approach would be to use pytest and its assertion mechanisms or collect failures and exit with a non-zero status code only at the end of the script's execution.
import pytest
@pytest.mark.skip(reason="Use pytest assertions instead of sys.exit")
def test_hierarchical_without_openai():
assert True == False, "Test failed: OpenAI API key still required"![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: LLM Interface Misuse Causes JSON Parsing Error
The _get_manager_instructions_with_fallback (sync) and _get_structured_response_async (async) methods incorrectly use the LLM interface. They pass both output_pydantic and output_json=True, then unconditionally attempt to parse the response as JSON. If the LLM returns a Pydantic object (as implied by output_pydantic), json.loads() raises a TypeError. This TypeError is not caught by the except (json.JSONDecodeError, ValueError) clause, preventing the intended isinstance fallback and causing an unhandled exception.
src/praisonai-agents/praisonaiagents/process/process.py#L175-L191
PraisonAI/src/praisonai-agents/praisonaiagents/process/process.py
Lines 175 to 191 in 262d46e
src/praisonai-agents/praisonaiagents/process/process.py#L233-L249
PraisonAI/src/praisonai-agents/praisonaiagents/process/process.py
Lines 233 to 249 in 262d46e
BugBot free trial expires on July 22, 2025
Learn more in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
Owner
Author
|
@claude review this pull request and do a detailed analysis and fix this if the existing code doesn't have the solution implemented. Making sure it has backward compatibility, no existing features removed. After making those changes, again review the applied changes. Use @web to search if you dont know any information or to find the latest documentation or to find the latest version. Run the code if you think you need to run it to test it. Minimal code change to start with if required any changes. |
Contributor
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Contributor
|
Claude finished @MervinPraison's task —— View job ✅ PR Review and Fixes CompletedI've reviewed this pull request and implemented necessary fixes to address the issues identified by other reviewers. Summary of Changes1. Fixed Critical Bug 🐛The cursor bot correctly identified that the LLM interface was being misused. I fixed this by:
2. Improved Code Quality 📈Addressed code duplication issues by:
3. Enhanced Testing 🧪
Review Verdict ✅The PR successfully fixes issue #873. After my improvements:
The hierarchical workflow now works correctly with all LLM providers without requiring an OpenAI API key. Commit DetailsI've pushed the fixes in commit
The PR is now ready for merge! |
- Remove duplicate output_json=True when using output_pydantic - Add helper methods to reduce code duplication - Improve test file to use proper assertions instead of sys.exit() - Add unit tests for the new helper methods - Fix TypeError handling in JSON parsing This ensures the hierarchical workflow works correctly with all LLM providers. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Owner
Author
|
@claude review if the changes made are as per the requirements |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/praisonai-agents/test_manager_llm_fix.py (1)
9-9: Remove unused import.The
jsonmodule is imported but never used in this file.-import jsonsrc/praisonai-agents/test_autoagents_hierarchical_fix.py (1)
1-143: Consider moving test file to the tests/ directory.Similar to the other test file, this should be placed in the
tests/directory according to codebase conventions.Move this file to
src/praisonai-agents/tests/test_autoagents_hierarchical_fix.py.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/praisonai-agents/praisonaiagents/process/process.py(4 hunks)src/praisonai-agents/test_autoagents_hierarchical_fix.py(1 hunks)src/praisonai-agents/test_manager_llm_fix.py(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Use the unified LLM wrapper in `praisonaiagents/llm/` for integrating with multiple LLM providers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/{llm,agent,agents,task}/**/*.ts : Replace all references to 'LLM' or 'litellm' with 'aisdk' usage in TypeScript code.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/llm/llm.ts : Replace all references to 'LLM' or 'litellm' with 'aisdk' usage for large language model calls in Node.js/TypeScript code.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/llm/llm.ts : The 'LLM' class in 'llm.ts' should wrap 'aisdk.generateText' calls for generating text responses.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Use the `Agent` class from `praisonaiagents/agent/` for core agent implementations, supporting LLM integration, tool calling, and self-reflection.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/{llm,agent,agents,task}/**/*.ts : Use the 'aisdk' library for all large language model (LLM) calls in TypeScript, such as using 'generateText' for text generation.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/agent/agent.ts : The 'Agent' class in 'src/agent/agent.ts' should encapsulate a single agent's role, name, and methods for calling the LLM using 'aisdk'.
src/praisonai-agents/test_manager_llm_fix.py (10)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/agents/agents.ts : The 'PraisonAIAgents' class in 'src/agents/agents.ts' should manage multiple agents, tasks, memory, and process type, mirroring the Python 'agents.py'.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/agent/agent.ts : The 'Agent' class in 'src/agent/agent.ts' should encapsulate a single agent's role, name, and methods for calling the LLM using 'aisdk'.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/tests/**/*.py : Test files should be placed in the `tests/` directory and demonstrate specific usage patterns, serving as both test and documentation.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Use the unified LLM wrapper in `praisonaiagents/llm/` for integrating with multiple LLM providers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/llm/llm.ts : The 'LLM' class in 'llm.ts' should wrap 'aisdk.generateText' calls for generating text responses.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/{llm,agent,agents,task}/**/*.ts : Replace all references to 'LLM' or 'litellm' with 'aisdk' usage in TypeScript code.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Use the `Agent` class from `praisonaiagents/agent/` for core agent implementations, supporting LLM integration, tool calling, and self-reflection.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/{memory,knowledge}/**/*.py : Place memory-related implementations in `praisonaiagents/memory/` and knowledge/document processing in `praisonaiagents/knowledge/`.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Run individual test files as scripts (e.g., `python tests/basic-agents.py`) rather than using a formal test runner.
src/praisonai-agents/praisonaiagents/process/process.py (10)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/llm/llm.ts : Replace all references to 'LLM' or 'litellm' with 'aisdk' usage for large language model calls in Node.js/TypeScript code.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Use the unified LLM wrapper in `praisonaiagents/llm/` for integrating with multiple LLM providers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/{llm,agent,agents,task}/**/*.ts : Replace all references to 'LLM' or 'litellm' with 'aisdk' usage in TypeScript code.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Use the `Agent` class from `praisonaiagents/agent/` for core agent implementations, supporting LLM integration, tool calling, and self-reflection.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/llm/llm.ts : The 'LLM' class in 'llm.ts' should wrap 'aisdk.generateText' calls for generating text responses.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/agent/agent.ts : The 'Agent' class in 'src/agent/agent.ts' should encapsulate a single agent's role, name, and methods for calling the LLM using 'aisdk'.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/agents/agents.ts : The 'PraisonAIAgents' class in 'src/agents/agents.ts' should manage multiple agents, tasks, memory, and process type, mirroring the Python 'agents.py'.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/{llm,agent,agents,task}/**/*.ts : Use the 'aisdk' library for all large language model (LLM) calls in TypeScript, such as using 'generateText' for text generation.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/{memory,knowledge}/**/*.py : Place memory-related implementations in `praisonaiagents/memory/` and knowledge/document processing in `praisonaiagents/knowledge/`.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
src/praisonai-agents/test_autoagents_hierarchical_fix.py (2)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Run individual test files as scripts (e.g., `python tests/basic-agents.py`) rather than using a formal test runner.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/tests/**/*.py : Test files should be placed in the `tests/` directory and demonstrate specific usage patterns, serving as both test and documentation.
🧬 Code Graph Analysis (1)
src/praisonai-agents/test_autoagents_hierarchical_fix.py (1)
src/praisonai-agents/praisonaiagents/agents/autoagents.py (1)
AutoAgents(41-503)
🪛 Ruff (0.11.9)
src/praisonai-agents/test_manager_llm_fix.py
9-9: json imported but unused
Remove unused import: json
(F401)
src/praisonai-agents/test_autoagents_hierarchical_fix.py
97-97: Local variable result is assigned to but never used
Remove assignment to unused variable result
(F841)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: performance-test
- GitHub Check: test-examples
- GitHub Check: quick-test
- GitHub Check: test-core (3.11)
- GitHub Check: Run tests and collect coverage
- GitHub Check: quick-test
🔇 Additional comments (3)
src/praisonai-agents/praisonaiagents/process/process.py (3)
9-9: LGTM!The LLM import correctly uses the unified LLM wrapper as per codebase conventions.
37-59: Excellent refactoring to reduce code duplication!These helper methods successfully address the code duplication issues mentioned in previous reviews. The centralized JSON parsing with comprehensive error handling (including TypeError) is particularly well done.
191-238: Well-executed migration to LLM abstraction!The refactoring successfully replaces direct OpenAI client usage with the unified LLM interface while preserving the fallback mechanism. The use of helper methods improves code maintainability.
Comment on lines
+1
to
+104
| #!/usr/bin/env python3 | ||
| """ | ||
| Test script to verify that the manager LLM methods correctly handle Pydantic models. | ||
| This tests the fix for the LLM interface misuse bug. | ||
| """ | ||
|
|
||
| import os | ||
| import sys | ||
| import json | ||
| from pydantic import BaseModel | ||
|
|
||
| # Add the parent directory to the Python path | ||
| sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) | ||
|
|
||
| from praisonaiagents.process.process import Process | ||
| from praisonaiagents.task.task import Task | ||
| from praisonaiagents.agent.agent import Agent | ||
|
|
||
| class ManagerInstructions(BaseModel): | ||
| task_id: int | ||
| agent_name: str | ||
| action: str | ||
|
|
||
| def test_parse_manager_instructions(): | ||
| """Test the _parse_manager_instructions helper method""" | ||
| print("Testing _parse_manager_instructions method...") | ||
|
|
||
| # Create a dummy process instance | ||
| tasks = {"1": Task(name="test_task", description="Test task")} | ||
| agents = [Agent(name="TestAgent", role="Test role", goal="Test goal")] | ||
| process = Process(tasks=tasks, agents=agents, manager_llm="test/model") | ||
|
|
||
| # Test valid JSON response | ||
| valid_json = '{"task_id": 1, "agent_name": "TestAgent", "action": "execute"}' | ||
| try: | ||
| result = process._parse_manager_instructions(valid_json, ManagerInstructions) | ||
| assert isinstance(result, ManagerInstructions) | ||
| assert result.task_id == 1 | ||
| assert result.agent_name == "TestAgent" | ||
| assert result.action == "execute" | ||
| print("✓ Valid JSON parsing works correctly") | ||
| except Exception as e: | ||
| print(f"✗ Failed to parse valid JSON: {e}") | ||
| return False | ||
|
|
||
| # Test invalid JSON | ||
| invalid_json = "not a json string" | ||
| try: | ||
| result = process._parse_manager_instructions(invalid_json, ManagerInstructions) | ||
| print("✗ Should have failed on invalid JSON") | ||
| return False | ||
| except Exception as e: | ||
| print(f"✓ Correctly raised exception for invalid JSON: {type(e).__name__}") | ||
|
|
||
| # Test JSON with missing fields | ||
| incomplete_json = '{"task_id": 1}' | ||
| try: | ||
| result = process._parse_manager_instructions(incomplete_json, ManagerInstructions) | ||
| print("✗ Should have failed on incomplete JSON") | ||
| return False | ||
| except Exception as e: | ||
| print(f"✓ Correctly raised exception for incomplete JSON: {type(e).__name__}") | ||
|
|
||
| return True | ||
|
|
||
| def test_create_llm_instance(): | ||
| """Test the _create_llm_instance helper method""" | ||
| print("\nTesting _create_llm_instance method...") | ||
|
|
||
| # Create a dummy process instance | ||
| tasks = {"1": Task(name="test_task", description="Test task")} | ||
| agents = [Agent(name="TestAgent", role="Test role", goal="Test goal")] | ||
| manager_llm = "gemini/gemini-2.5-flash-lite-preview-06-17" | ||
| process = Process(tasks=tasks, agents=agents, manager_llm=manager_llm) | ||
|
|
||
| # Test LLM instance creation | ||
| try: | ||
| llm = process._create_llm_instance() | ||
| assert llm.model == manager_llm | ||
| assert llm.temperature == 0.7 | ||
| print("✓ LLM instance created successfully") | ||
| return True | ||
| except Exception as e: | ||
| print(f"✗ Failed to create LLM instance: {e}") | ||
| return False | ||
|
|
||
| if __name__ == "__main__": | ||
| print("=== Testing Manager LLM Fix ===\n") | ||
|
|
||
| # Run tests | ||
| test_results = [] | ||
| test_results.append(("_parse_manager_instructions", test_parse_manager_instructions())) | ||
| test_results.append(("_create_llm_instance", test_create_llm_instance())) | ||
|
|
||
| # Summary | ||
| print("\n=== TEST SUMMARY ===") | ||
| all_passed = True | ||
| for test_name, passed in test_results: | ||
| status = "PASSED" if passed else "FAILED" | ||
| print(f"{test_name}: {status}") | ||
| if not passed: | ||
| all_passed = False | ||
|
|
||
| sys.exit(0 if all_passed else 1) No newline at end of file |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider moving test file to the tests/ directory.
According to the codebase conventions, test files should be placed in the tests/ directory. This file is currently in the root of src/praisonai-agents/.
Move this file to src/praisonai-agents/tests/test_manager_llm_fix.py to follow the established testing conventions.
🏁 Script executed:
#!/bin/bash
# Check if tests directory exists and verify test file conventions
fd -t d "tests" src/praisonai-agents
fd -e py "test_" src/praisonai-agents | head -20Length of output: 1038
Relocate test_manager_llm_fix.py into the tests directory
To adhere to project conventions, move the file from the module root into the existing tests/ folder:
• src/praisonai-agents/test_manager_llm_fix.py
→ src/praisonai-agents/tests/test_manager_llm_fix.py
- src/praisonai-agents/test_manager_llm_fix.py
+ src/praisonai-agents/tests/test_manager_llm_fix.py📝 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.
Suggested change
| #!/usr/bin/env python3 | |
| """ | |
| Test script to verify that the manager LLM methods correctly handle Pydantic models. | |
| This tests the fix for the LLM interface misuse bug. | |
| """ | |
| import os | |
| import sys | |
| import json | |
| from pydantic import BaseModel | |
| # Add the parent directory to the Python path | |
| sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) | |
| from praisonaiagents.process.process import Process | |
| from praisonaiagents.task.task import Task | |
| from praisonaiagents.agent.agent import Agent | |
| class ManagerInstructions(BaseModel): | |
| task_id: int | |
| agent_name: str | |
| action: str | |
| def test_parse_manager_instructions(): | |
| """Test the _parse_manager_instructions helper method""" | |
| print("Testing _parse_manager_instructions method...") | |
| # Create a dummy process instance | |
| tasks = {"1": Task(name="test_task", description="Test task")} | |
| agents = [Agent(name="TestAgent", role="Test role", goal="Test goal")] | |
| process = Process(tasks=tasks, agents=agents, manager_llm="test/model") | |
| # Test valid JSON response | |
| valid_json = '{"task_id": 1, "agent_name": "TestAgent", "action": "execute"}' | |
| try: | |
| result = process._parse_manager_instructions(valid_json, ManagerInstructions) | |
| assert isinstance(result, ManagerInstructions) | |
| assert result.task_id == 1 | |
| assert result.agent_name == "TestAgent" | |
| assert result.action == "execute" | |
| print("✓ Valid JSON parsing works correctly") | |
| except Exception as e: | |
| print(f"✗ Failed to parse valid JSON: {e}") | |
| return False | |
| # Test invalid JSON | |
| invalid_json = "not a json string" | |
| try: | |
| result = process._parse_manager_instructions(invalid_json, ManagerInstructions) | |
| print("✗ Should have failed on invalid JSON") | |
| return False | |
| except Exception as e: | |
| print(f"✓ Correctly raised exception for invalid JSON: {type(e).__name__}") | |
| # Test JSON with missing fields | |
| incomplete_json = '{"task_id": 1}' | |
| try: | |
| result = process._parse_manager_instructions(incomplete_json, ManagerInstructions) | |
| print("✗ Should have failed on incomplete JSON") | |
| return False | |
| except Exception as e: | |
| print(f"✓ Correctly raised exception for incomplete JSON: {type(e).__name__}") | |
| return True | |
| def test_create_llm_instance(): | |
| """Test the _create_llm_instance helper method""" | |
| print("\nTesting _create_llm_instance method...") | |
| # Create a dummy process instance | |
| tasks = {"1": Task(name="test_task", description="Test task")} | |
| agents = [Agent(name="TestAgent", role="Test role", goal="Test goal")] | |
| manager_llm = "gemini/gemini-2.5-flash-lite-preview-06-17" | |
| process = Process(tasks=tasks, agents=agents, manager_llm=manager_llm) | |
| # Test LLM instance creation | |
| try: | |
| llm = process._create_llm_instance() | |
| assert llm.model == manager_llm | |
| assert llm.temperature == 0.7 | |
| print("✓ LLM instance created successfully") | |
| return True | |
| except Exception as e: | |
| print(f"✗ Failed to create LLM instance: {e}") | |
| return False | |
| if __name__ == "__main__": | |
| print("=== Testing Manager LLM Fix ===\n") | |
| # Run tests | |
| test_results = [] | |
| test_results.append(("_parse_manager_instructions", test_parse_manager_instructions())) | |
| test_results.append(("_create_llm_instance", test_create_llm_instance())) | |
| # Summary | |
| print("\n=== TEST SUMMARY ===") | |
| all_passed = True | |
| for test_name, passed in test_results: | |
| status = "PASSED" if passed else "FAILED" | |
| print(f"{test_name}: {status}") | |
| if not passed: | |
| all_passed = False | |
| sys.exit(0 if all_passed else 1) | |
| - src/praisonai-agents/test_manager_llm_fix.py | |
| + src/praisonai-agents/tests/test_manager_llm_fix.py |
🧰 Tools
🪛 Ruff (0.11.9)
9-9: json imported but unused
Remove unused import: json
(F401)
🤖 Prompt for AI Agents
In src/praisonai-agents/test_manager_llm_fix.py from lines 1 to 104, the test
file is currently located at the module root which does not follow project
conventions. Move this entire file into the existing tests directory at
src/praisonai-agents/tests/ so the new path is
src/praisonai-agents/tests/test_manager_llm_fix.py. This relocation aligns with
the project's organizational standards for test files.
|
|
||
| # This should fail with connection error, not OpenAI API key error | ||
| try: | ||
| result = agents.start() |
Contributor
There was a problem hiding this comment.
Remove or rename unused variable.
The result variable is assigned but never used. Either remove the assignment or use underscore to indicate it's intentionally unused.
- result = agents.start()
+ agents.start()Or alternatively:
- result = agents.start()
+ _ = agents.start()📝 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.
Suggested change
| result = agents.start() | |
| - result = agents.start() | |
| + agents.start() |
🧰 Tools
🪛 Ruff (0.11.9)
97-97: Local variable result is assigned to but never used
Remove assignment to unused variable result
(F841)
🤖 Prompt for AI Agents
In src/praisonai-agents/test_autoagents_hierarchical_fix.py at line 97, the
variable 'result' is assigned the value from agents.start() but never used. To
fix this, either remove the assignment entirely or rename the variable to '_' to
indicate it is intentionally unused.
Contributor
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Owner
Author
|
@claude again getting error, fix it |
Contributor
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
1 similar comment
Contributor
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Owner
Author
|
@claude fix above |
Contributor
|
Claude finished @MervinPraison's task —— View job Fixing JSON Parsing and Mock LLM Issues
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #873
This PR fixes the issue where AutoAgents hierarchical workflow fails with OpenAI API key error even when using non-OpenAI LLMs like Gemini.
Changes
Testing
Added
test_autoagents_hierarchical_fix.pythat verifies:Generated with Claude Code
Summary by CodeRabbit
Refactor
Tests