fix: resolve task_name undefined error in async execution #959
Conversation
Addresses issue #947 where task_name undefined errors occurred during async parallel task execution. Key Changes: - Enhanced Agent.achat() method to accept task context parameters (task_name, task_description, task_id) - Updated aexecute_task() to pass task metadata when calling agent.achat() - Added callback execution to both async execution paths in achat() method - Updated callback system to properly propagate task context Root Cause: Async execution path was missing task metadata propagation to LLM callbacks. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
|
Warning Rate limit exceeded@github-actions[bot] has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 58 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (1)
WalkthroughThe changes update agent interaction methods to accept and propagate additional task metadata parameters ( Changes
Sequence Diagram(s)sequenceDiagram
participant PraisonAIAgents
participant Agent
participant Callback
participant Display
PraisonAIAgents->>Agent: achat(prompt, ..., task_name, task_description, task_id)
Agent->>Agent: _execute_callback_and_display(..., task_name, task_description, task_id)
Agent->>Callback: execute_sync_callback(..., task_name, task_description, task_id)
Agent->>Display: display_interaction(..., task_name, task_description, task_id)
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. 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 (
|
|
@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. |
|
Claude finished @MervinPraison's task —— View job Detailed PR Analysis and Review ✅I've completed a comprehensive review of this PR and made necessary improvements. Here's my final assessment: Todo List:
🎯 Final Assessment: APPROVED WITH ENHANCEMENTS✅ Original Implementation Strengths:
🔧 Issues Found & Fixed:Missing Callback Execution Paths - I identified and fixed 2 execution paths in the
This ensures consistent task metadata propagation across all async execution flows. ✅ Technical Validation:
🔍 Code Review Summary:
agent_output = await executor_agent.achat(
task_prompt,
# ... other params
task_name=task.name, # ✅ Correct propagation
task_description=task.description, # ✅ Correct propagation
task_id=task.id # ✅ Correct propagation
)
🚀 Final Recommendation: MERGEThis PR successfully resolves the Key Benefits:
Changes pushed to branch: |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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 a critical bug where task metadata was not being correctly propagated during asynchronous agent executions, leading to undefined errors in callbacks. My changes involve extending method signatures to carry task context and updating the call chain to ensure this metadata reaches the callback system, thereby stabilizing async operations and improving callback utility.
Highlights
- Bug Fix: Task Metadata Propagation: I've addressed the
task_name is not definederror occurring during async parallel task execution. The root cause was identified as missing task metadata propagation (task_name,task_description,task_id) through the async execution path to LLM callbacks. - API Enhancement:
Agent.achat()Signature: TheAgent.achat()method signature has been enhanced to accept optionaltask_name,task_description, andtask_idparameters, allowing task context to be passed during asynchronous agent interactions. - Callback System Update: The internal
_execute_callback_and_displayhelper method and its callers (both synchronouschatand asynchronousachatpaths) have been updated to properly propagate and utilize the task context parameters (task_name,task_description,task_id) to the callback system, replacing previously hardcodedNonevalues. - Async Task Execution Integration: The
aexecute_tasklogic (withinpraisonaiagents/agents/agents.py) now explicitly passes thetask.name,task.description, andtask.idfrom thetaskobject when invokingexecutor_agent.achat(), ensuring the metadata flows correctly from the task level down to the agent's LLM calls. - Backward Compatibility: All new parameters introduced are optional with default
Nonevalues, ensuring existing code callingagent.achat()continues to function without modification. Legacy callback functions are also supported through parameter filtering.
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. ↩
There was a problem hiding this comment.
Code Review
This pull request fixes an undefined task_name error in async execution paths by propagating task metadata through achat and to the callback system. The changes correctly update the async path and the call sites in aexecute_task.
However, a critical issue exists in the synchronous chat path that will cause a NameError due to the same missing parameter propagation. Additionally, there are opportunities to improve code maintainability by reducing duplication and managing the growing number of method parameters. Address the critical issue before merging.
| return None | ||
|
|
||
| def _execute_callback_and_display(self, prompt: str, response: str, generation_time: float): | ||
| def _execute_callback_and_display(self, prompt: str, response: str, generation_time: float, task_name=None, task_description=None, task_id=None): |
There was a problem hiding this comment.
The _execute_callback_and_display helper method is correctly updated to accept task context parameters. However, the synchronous chat method calls _chat_completion (e.g., at line 1360) without passing the task_name, task_description, and task_id parameters. The _chat_completion method uses these variables when calling self.llm_instance.get_response (lines 1104-1106 and 1124-1126), but they are not defined in its scope, which will cause a NameError.
To fix this, update the signature of _chat_completion to accept task_name, task_description, and task_id and pass these parameters from chat when calling _chat_completion.
| validated_response = self._apply_guardrail_with_retry(response_text, original_prompt, temperature, tools) | ||
| # Execute callback after validation | ||
| self._execute_callback_and_display(original_prompt, validated_response, time.time() - start_time) | ||
| self._execute_callback_and_display(original_prompt, validated_response, time.time() - start_time, task_name, task_description, task_id) |
There was a problem hiding this comment.
The try...except block containing _apply_guardrail_with_retry and _execute_callback_and_display is repeated multiple times in the chat method (e.g., lines 1396, 1407, 1475, 1493). Refactor this logic into a private helper method to improve maintainability and reduce code duplication. This would centralize the guardrail and callback logic, making the chat method cleaner and easier to modify in the future.
| return cleaned | ||
|
|
||
| async def achat(self, prompt: str, temperature=0.2, tools=None, output_json=None, output_pydantic=None, reasoning_steps=False): | ||
| async def achat(self, prompt: str, temperature=0.2, tools=None, output_json=None, output_pydantic=None, reasoning_steps=False, task_name=None, task_description=None, task_id=None): |
There was a problem hiding this comment.
The number of parameters for the achat method is growing (currently 9), which can impact readability and maintainability. The chat method has a similar issue with 11 parameters. Consider encapsulating related parameters into a context object, such as a TaskContext dataclass to group task_name, task_description, and task_id. This would make the method signatures cleaner and more extensible.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/praisonai-agents/praisonaiagents/agents/agents.py (1)
361-369: DRY up duplicated kwargs & exposestreamfor async pathThe two
executor_agent.achatinvocations are now identical except for the prompt (_get_multimodal_messagevs raw string).
Re-listing the same 8 keyword arguments twice makes the code brittle and easy to forget (e.g.streamis still missing in the async branch, while it exists in the sync version). A small helper dict keeps the call sites concise and guarantees both paths stay in lock-step when new parameters are added.- agent_output = await executor_agent.achat( - _get_multimodal_message(task_prompt, task.images), - tools=tools, - output_json=task.output_json, - output_pydantic=task.output_pydantic, - task_name=task.name, - task_description=task.description, - task_id=task.id - ) + achat_kwargs = dict( + tools=tools, + output_json=task.output_json, + output_pydantic=task.output_pydantic, + task_name=task.name, + task_description=task.description, + task_id=task.id, + stream=self.stream, # keep parity with sync `chat` + ) + agent_output = await executor_agent.achat( + _get_multimodal_message(task_prompt, task.images), + **achat_kwargs, + ) ... - agent_output = await executor_agent.achat( - task_prompt, - tools=tools, - output_json=task.output_json, - output_pydantic=task.output_pydantic, - task_name=task.name, - task_description=task.description, - task_id=task.id - ) + agent_output = await executor_agent.achat( + task_prompt, + **achat_kwargs, + )Benefits:
- Single source of truth for the ever-growing kwargs list.
- Restores streaming parity between sync & async paths (
stream=self.stream).- Easier to notice / propagate future signature changes.
Please confirm that
Agent.achatindeed supports astreamflag similar tochat; if not, feel free to drop that key from the suggested dict.Also applies to: 371-379
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/praisonai-agents/praisonaiagents/agent/agent.py(11 hunks)src/praisonai-agents/praisonaiagents/agents/agents.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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/task/task.ts : The 'Task' class in 'src/task/task.ts' should encapsulate a single unit of work, referencing an agent, with optional callback, memory usage, and task type.
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 `Task` class from `praisonaiagents/task/` for defining tasks, supporting context, callbacks, output specifications, and guardrails.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Implement guardrails for tasks and agents using either function-based or LLM-based (string description) validation.
src/praisonai-agents/praisonaiagents/agent/agent.py (6)
<retrieved_learning>
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/.
</retrieved_learning>
<retrieved_learning>
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 Task class from praisonaiagents/task/ for defining tasks, supporting context, callbacks, output specifications, and guardrails.
</retrieved_learning>
<retrieved_learning>
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/task/task.ts : The 'Task' class in 'src/task/task.ts' should encapsulate a single unit of work, referencing an agent, with optional callback, memory usage, and task type.
</retrieved_learning>
<retrieved_learning>
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'.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Implement guardrails for tasks and agents using either function-based or LLM-based (string description) validation.
</retrieved_learning>
src/praisonai-agents/praisonaiagents/agents/agents.py (5)
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 `Task` class from `praisonaiagents/task/` for defining tasks, supporting context, callbacks, output specifications, and guardrails.
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/task/task.ts : The 'Task' class in 'src/task/task.ts' should encapsulate a single unit of work, referencing an agent, with optional callback, memory usage, and task type.
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-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/agents/autoagents.ts : The 'AutoAgents' class in 'src/agents/autoagents.ts' should provide high-level convenience for automatically generating agent/task configuration from user instructions, using 'aisdk' to parse config.
⏰ 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). (4)
- GitHub Check: Cursor BugBot
- GitHub Check: quick-test
- GitHub Check: test-core (3.11)
- GitHub Check: Run tests and collect coverage
🔇 Additional comments (5)
src/praisonai-agents/praisonaiagents/agent/agent.py (5)
1161-1191: LGTM! Task metadata parameters correctly added and propagated.The method signature correctly adds the three task metadata parameters with None defaults for backward compatibility. The parameters are properly passed through to both
execute_sync_callbackanddisplay_interactioncalls, ensuring task context is available throughout the callback execution chain.
1192-1192: LGTM! Method signature correctly updated for task metadata propagation.The
chatmethod signature properly adds the three task metadata parameters with None defaults, maintaining backward compatibility while enabling task context to be passed through synchronous execution paths.
1377-1377: LGTM! All callback execution points correctly pass task metadata.All calls to
_execute_callback_and_displaythroughout thechatmethod correctly pass the task metadata parameters (task_name,task_description,task_id). This ensures consistent task context propagation across all execution paths including:
- JSON output handling (line 1377)
- Reasoning content handling (line 1396)
- Regular response handling (line 1407)
- Self-reflection validation (lines 1475, 1493)
Also applies to: 1396-1396, 1407-1407, 1475-1475, 1493-1493
1539-1539: LGTM! Async method signature correctly updated for task metadata propagation.The
achatmethod signature properly adds the three task metadata parameters with None defaults, maintaining backward compatibility while enabling task context to be passed through asynchronous execution paths. This directly addresses the undefinedtask_nameerror mentioned in the PR objectives.
1625-1625: LGTM! Async callback execution points correctly pass task metadata.Both calls to
_execute_callback_and_displayin theachatmethod correctly pass the task metadata parameters:
- Line 1625: Custom LLM execution path
- Line 1811: OpenAI client execution path
This ensures consistent task context propagation across all asynchronous execution paths, resolving the core issue where
task_namewas undefined in async parallel task execution.Also applies to: 1811-1811
Adds missing callback execution to tool-based responses and JSON/Pydantic output paths in the achat() method to ensure consistent task metadata propagation across all execution flows. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
|
@claude review if the changes made in the above branch are as per the requirements and make changes to the above branch if issues still exist. |
|
Claude finished @MervinPraison's task —— View job Detailed PR Analysis and Review ✅I've completed a comprehensive review of this PR and made necessary improvements. Here's my final assessment: Todo List:
🎯 Final Assessment: APPROVED WITH ENHANCEMENTS✅ Original Implementation Strengths:
🔧 Issues Found & Fixed:Missing Callback Execution Paths - I identified and fixed 2 execution paths in the
This ensures consistent task metadata propagation across all async execution flows. ✅ Technical Validation:
🔍 Code Review Summary:
agent_output = await executor_agent.achat(
task_prompt,
# ... other params
task_name=task.name, # ✅ Correct propagation
task_description=task.description, # ✅ Correct propagation
task_id=task.id # ✅ Correct propagation
)
🚀 Final Recommendation: MERGEThis PR successfully resolves the Key Benefits:
Changes pushed to branch: |
- Enhanced _chat_completion method signature to accept task metadata parameters - Updated all calls to _chat_completion to pass task context - Fixed NameError where task_name, task_description, task_id were undefined - Updated _apply_guardrail_with_retry to propagate task metadata - Ensures complete task metadata propagation in sync chat execution path Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Fixes #947
Addresses the
task_name is not definederror that occurred during async parallel task execution. This fix was developed through comprehensive multi-agent analysis to identify the real root cause.Root Cause
The async execution path (
aexecute_task→agent.achat→ LLM callbacks) was missing task metadata propagation, causing callbacks to receive undefinedtask_nameparameters.Changes
Agent.achat()method signature to accept task context parameters (task_name,task_description,task_id)aexecute_task()to pass task metadata when callingagent.achat()achat()methodBackward Compatibility
Nonevaluesagent.achat()will continue to work without modificationTesting
Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor