Skip to content

Develop #598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MervinPraison merged 5 commits into from
Jun 4, 2025
Merged

Develop #598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
67807ea
Enhance test workflows by replacing test runner with pytest for impro…
MervinPraison Jun 4, 2025
0ebbbf3
Update coverage workflow to include installation of duckduckgo_search…
MervinPraison Jun 4, 2025
8c59943
Update test workflow to use custom test runner for 'all' pattern
MervinPraison Jun 4, 2025
9f1c50e
Merge branch 'main' of https://github.com/MervinPraison/PraisonAI int…
MervinPraison Jun 4, 2025
13503f0
agent sequential thinking
MervinPraison Jun 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions .github/workflows/coverage.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
name: Run tests and upload coverage

on:
push:
branches: [ main, develop ]
pull_request:
branches: [ main, develop ]

jobs:
test:
name: Run tests and collect coverage
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 2

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: 3.11

- name: Install UV
run: |
curl -LsSf https://astral.sh/uv/install.sh | sh
echo "$HOME/.local/bin" >> $GITHUB_PATH

- name: Install dependencies
run: |
cd src/praisonai
uv pip install --system ."[ui,gradio,api,agentops,google,openai,anthropic,cohere,chat,code,realtime,call,crewai,autogen]"
uv pip install --system duckduckgo_search
uv pip install --system pytest pytest-cov pytest-asyncio
# Install knowledge dependencies from praisonai-agents
uv pip install --system "praisonaiagents[knowledge]"

- name: Set environment variables
run: |
echo "OPENAI_API_KEY=${{ secrets.OPENAI_API_KEY || 'sk-test-key-for-github-actions-testing-only-not-real' }}" >> $GITHUB_ENV
echo "OPENAI_API_BASE=${{ secrets.OPENAI_API_BASE || 'https://api.openai.com/v1' }}" >> $GITHUB_ENV
echo "OPENAI_MODEL_NAME=${{ secrets.OPENAI_MODEL_NAME || 'gpt-4o-mini' }}" >> $GITHUB_ENV
echo "LOGLEVEL=DEBUG" >> $GITHUB_ENV
echo "PYTHONPATH=${{ github.workspace }}/src/praisonai-agents:$PYTHONPATH" >> $GITHUB_ENV

- name: Run tests
run: |
cd src/praisonai
pytest --cov=praisonai --cov-branch --cov-report=xml tests/unit/

- name: Upload results to Codecov
uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
file: src/praisonai/coverage.xml
flags: unit-tests
name: unit-tests-coverage
fail_ci_if_error: false
verbose: true
20 changes: 15 additions & 5 deletions .github/workflows/test-comprehensive.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ jobs:

case $TEST_TYPE in
"unit")
cd src/praisonai && python tests/test_runner.py --pattern unit
cd src/praisonai && python -m pytest tests/unit/ -v --tb=short --disable-warnings --cov=praisonai --cov-report=xml --cov-branch
;;
"integration")
cd src/praisonai && python tests/test_runner.py --pattern integration
cd src/praisonai && python -m pytest tests/integration/ -v --tb=short --disable-warnings --cov=praisonai --cov-report=xml --cov-branch
;;
"fast")
cd src/praisonai && python tests/test_runner.py --pattern fast
Expand Down Expand Up @@ -139,16 +139,26 @@ jobs:
echo "- 🔌 MCP server connections" >> comprehensive_report.md
echo "- 💬 LLM integrations (OpenAI, Anthropic, etc.)" >> comprehensive_report.md

- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
file: src/praisonai/coverage.xml
flags: comprehensive-tests
name: comprehensive-tests-coverage
fail_ci_if_error: false
verbose: true

- name: Upload Comprehensive Test Results
uses: actions/upload-artifact@v4
if: always()
with:
name: comprehensive-test-results-python-${{ matrix.python-version }}
path: |
comprehensive_report.md
htmlcov/
coverage.xml
.coverage
src/praisonai/htmlcov/
src/praisonai/coverage.xml
src/praisonai/.coverage
retention-days: 30

test-matrix-summary:
Expand Down
20 changes: 16 additions & 4 deletions .github/workflows/test-core.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ jobs:

- name: Run Unit Tests
run: |
cd src/praisonai && python -m pytest tests/unit/ -v --tb=short --disable-warnings --cov=praisonai --cov-report=term-missing
cd src/praisonai && python -m pytest tests/unit/ -v --tb=short --disable-warnings --cov=praisonai --cov-report=term-missing --cov-report=xml --cov-branch

- name: Run Integration Tests
run: |
Expand Down Expand Up @@ -263,12 +263,24 @@ jobs:
export OPENAI_MODEL_NAME="$OPENAI_MODEL_NAME"
cd src/praisonai && python -m pytest tests/test.py -v --tb=short --disable-warnings

- name: Upload Coverage Reports
- name: Upload coverage reports to Codecov
if: matrix.python-version == '3.11'
uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
file: src/praisonai/coverage.xml
flags: core-tests
name: core-tests-coverage
fail_ci_if_error: false
verbose: true

- name: Upload Coverage Reports (Artifacts)
if: matrix.python-version == '3.11'
uses: actions/upload-artifact@v4
with:
name: coverage-reports
path: |
.coverage
htmlcov/
src/praisonai/.coverage
src/praisonai/htmlcov/
src/praisonai/coverage.xml
retention-days: 7
14 changes: 12 additions & 2 deletions .github/workflows/unittest.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,8 +471,8 @@ jobs:

- name: Run Fast Tests
run: |
# Run the fastest, most essential tests
cd src/praisonai && python tests/test_runner.py --pattern fast
# Run the fastest, most essential tests with coverage
cd src/praisonai && python -m pytest tests/unit/test_core_agents.py -v --tb=short --disable-warnings --cov=praisonai --cov-report=xml --cov-branch

- name: Run Real API Tests
run: |
Expand All @@ -491,6 +491,16 @@ jobs:
cd src/praisonai && python -m pytest tests/test.py -v --tb=short --disable-warnings
continue-on-error: true

- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
file: src/praisonai/coverage.xml
flags: quick-validation
name: quick-validation-coverage
fail_ci_if_error: false
verbose: true

- name: Restore Root Config Files
run: |
echo "🔄 Restoring root configuration files..."
Expand Down
137 changes: 80 additions & 57 deletions src/praisonai-agents/praisonaiagents/agent/agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,69 +788,92 @@ def _chat_completion(self, messages, temperature=0.2, tools=None, stream=True, r
)
else:
# Use the standard OpenAI client approach
if stream:
# Process as streaming response with formatted tools
final_response = self._process_stream_response(
messages,
temperature,
start_time,
formatted_tools=formatted_tools if formatted_tools else None,
reasoning_steps=reasoning_steps
)
else:
# Process as regular non-streaming response
final_response = client.chat.completions.create(
model=self.llm,
messages=messages,
temperature=temperature,
tools=formatted_tools if formatted_tools else None,
stream=False
)

tool_calls = getattr(final_response.choices[0].message, 'tool_calls', None)

if tool_calls:
messages.append({
"role": "assistant",
"content": final_response.choices[0].message.content,
"tool_calls": tool_calls
})
# Continue tool execution loop until no more tool calls are needed
max_iterations = 10 # Prevent infinite loops
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The max_iterations value is currently hardcoded as 10. For better maintainability and configurability, would it be beneficial to define this as a class attribute (e.g., self.MAX_TOOL_ITERATIONS) or make it configurable, perhaps through the agent's initialization parameters? This would make it easier to adjust this limit without modifying the method's core logic.

Suggested change
max_iterations = 10 # Prevent infinite loops
max_iterations = self.config.get('max_tool_iterations', 10) # Or use a class constant

iteration_count = 0

while iteration_count < max_iterations:
if stream:
# Process as streaming response with formatted tools
final_response = self._process_stream_response(
messages,
temperature,
start_time,
formatted_tools=formatted_tools if formatted_tools else None,
reasoning_steps=reasoning_steps
)
else:
# Process as regular non-streaming response
final_response = client.chat.completions.create(
model=self.llm,
messages=messages,
temperature=temperature,
tools=formatted_tools if formatted_tools else None,
stream=False
)

for tool_call in tool_calls:
function_name = tool_call.function.name
arguments = json.loads(tool_call.function.arguments)
tool_calls = getattr(final_response.choices[0].message, 'tool_calls', None)

if self.verbose:
display_tool_call(f"Agent {self.name} is calling function '{function_name}' with arguments: {arguments}")
if tool_calls:
messages.append({
"role": "assistant",
"content": final_response.choices[0].message.content,
"tool_calls": tool_calls
})

tool_result = self.execute_tool(function_name, arguments)
results_str = json.dumps(tool_result) if tool_result else "Function returned an empty output"
for tool_call in tool_calls:
function_name = tool_call.function.name
arguments = json.loads(tool_call.function.arguments)

if self.verbose:
display_tool_call(f"Function '{function_name}' returned: {results_str}")
if self.verbose:
display_tool_call(f"Agent {self.name} is calling function '{function_name}' with arguments: {arguments}")

messages.append({
"role": "tool",
"tool_call_id": tool_call.id,
"content": results_str
})
tool_result = self.execute_tool(function_name, arguments)
results_str = json.dumps(tool_result) if tool_result else "Function returned an empty output"

# Get final response after tool calls
if stream:
final_response = self._process_stream_response(
messages,
temperature,
start_time,
formatted_tools=formatted_tools if formatted_tools else None,
reasoning_steps=reasoning_steps
)
else:
final_response = client.chat.completions.create(
model=self.llm,
messages=messages,
temperature=temperature,
stream=False
)
if self.verbose:
display_tool_call(f"Function '{function_name}' returned: {results_str}")

messages.append({
"role": "tool",
"tool_call_id": tool_call.id,
"content": results_str
})

# Check if we should continue (for tools like sequential thinking)
should_continue = False
for tool_call in tool_calls:
function_name = tool_call.function.name
arguments = json.loads(tool_call.function.arguments)

# For sequential thinking tool, check if nextThoughtNeeded is True
if function_name == "sequentialthinking" and arguments.get("nextThoughtNeeded", False):
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The string "sequentialthinking" is used directly here to identify the tool. To improve code clarity, reduce the risk of typos, and make future refactoring easier, could this be defined as a named constant at the class or module level (e.g., SEQUENTIAL_THINKING_TOOL_NAME = "sequentialthinking")?

Suggested change
if function_name == "sequentialthinking" and arguments.get("nextThoughtNeeded", False):
if function_name == SEQUENTIAL_THINKING_TOOL_NAME and arguments.get("nextThoughtNeeded", False):

should_continue = True
break

if not should_continue:
# Get final response after tool calls
if stream:
final_response = self._process_stream_response(
messages,
temperature,
start_time,
formatted_tools=formatted_tools if formatted_tools else None,
reasoning_steps=reasoning_steps
)
Comment on lines +857 to +863
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When should_continue is False, it implies the tool execution sequence (e.g., sequential thinking) has decided to conclude. However, in the streaming case, formatted_tools are still passed to _process_stream_response. Could this potentially lead to the LLM initiating new, unexpected tool calls even though the sequence was intended to stop? The non-streaming counterpart (lines 865-870) correctly omits tools for what appears to be the final response generation. For consistency and to ensure the sequence truly concludes, perhaps formatted_tools should be None here as well?

Suggested change
final_response = self._process_stream_response(
messages,
temperature,
start_time,
formatted_tools=formatted_tools if formatted_tools else None,
reasoning_steps=reasoning_steps
)
final_response = self._process_stream_response(
messages,
temperature,
start_time,
formatted_tools=None, # Explicitly no tools for final response generation
reasoning_steps=reasoning_steps
)

else:
final_response = client.chat.completions.create(
model=self.llm,
messages=messages,
temperature=temperature,
stream=False
)
break

iteration_count += 1
else:
# No tool calls, we're done
break

return final_response
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If the while loop terminates because iteration_count reaches max_iterations (and should_continue was true in the last iteration), the final_response returned will be from the LLM call before the results of the last set of tool calls were processed by the LLM. The messages list will contain these latest tool results, but the LLM hasn't generated a response based on them.

This could lead to the agent returning a stale response that doesn't reflect the outcome of the final tool operations.

Consider adding logic before this return statement to make a final LLM call if the loop exited due to max_iterations and tools were processed in that last iteration. This final call should likely not allow further tool usage (i.e., tools=None).

            # If the loop terminated due to max_iterations and the last LLM call requested tools
            # (implying those tools were run and their results are in 'messages' but not yet processed by LLM for a final response).
            if iteration_count == max_iterations and getattr(final_response.choices[0].message, 'tool_calls', None):
                logging.warning(f"Agent {self.name} reached max_iterations ({max_iterations}). Generating final response based on accumulated tool results.")
                if stream:
                    final_response = self._process_stream_response(
                        messages, 
                        temperature, 
                        start_time,
                        formatted_tools=None, # No further tools when max_iterations is hit
                        reasoning_steps=reasoning_steps
                    )
                else:
                    final_response = client.chat.completions.create(
                        model=self.llm,
                        messages=messages,
                        temperature=temperature,
                        tools=None, # No further tools when max_iterations is hit
                        stream=False
                    )
            return final_response


Expand Down
Loading