Revert "fix: resolve BudgetExceededError CLI traceback issues (fixes #1627)"#1642
Conversation
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR consolidates budget handling by removing the ChangesBudget Handling Consolidation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 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:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the 'max_budget' convenience parameter from the Agent constructor and deletes the '_execute_agent_with_budget_handling' helper method in the CLI, replacing it with direct calls to agent methods. The review feedback highlights that removing 'max_budget' is a breaking change and suggests using deprecation instead. Furthermore, the removal of the budget handling logic in the CLI is identified as a regression that will lead to unhandled exceptions and a poor user experience when budget limits are reached.
| verification_hooks: Optional[List[Any]] = None, # Deprecated: use autonomy=AutonomyConfig(verification_hooks=[...]) | ||
| output: Optional[Union[bool, str, Dict[str, Any], 'OutputConfig']] = None, | ||
| execution: Optional[Union[bool, str, Dict[str, Any], 'ExecutionConfig']] = None, | ||
| max_budget: Optional[float] = None, # Budget limit in USD - convenience alias for ExecutionConfig(max_budget=...) |
There was a problem hiding this comment.
Removing the "max_budget" parameter from the Agent constructor is a breaking change for users who rely on this convenience alias. If this feature was already released, it is recommended to keep the parameter as deprecated instead of removing it entirely to avoid breaking existing implementations.
|
|
||
| with Live(Panel(Spinner("dots", text="Generating..."), border_style="cyan"), refresh_per_second=10, transient=True): | ||
| result = self._execute_agent_with_budget_handling(auto_rag, 'chat', prompt) | ||
| result = auto_rag.chat(prompt) |
There was a problem hiding this comment.
By reverting the graceful error handling, this direct call to "auto_rag.chat(prompt)" will now cause the CLI to crash with a full traceback if a BudgetExceededError is raised. This regresses the fix for issue #1627 and negatively impacts the user experience for budget-constrained runs.
Greptile SummaryThis PR reverts #1635, which had introduced graceful
Confidence Score: 3/5Merging re-introduces a known CLI regression where budget overruns produce a raw Python traceback instead of a clean error message, and removes a constructor parameter that users may already be relying on. Both changes are straight reverts with no compensating fixes:
Important Files Changed
Sequence DiagramsequenceDiagram
participant User as CLI User
participant CLI as cli/main.py
participant Agent as agent.start() / agent.chat()
participant Mixin as chat_mixin.py
Note over CLI,Mixin: After this revert (no budget error handling)
User->>CLI: praisonai "prompt"
CLI->>Agent: agent.start(prompt)
Agent->>Mixin: _call_llm(...)
Mixin-->>Agent: raises BudgetExceededError
Agent-->>CLI: BudgetExceededError propagates (unhandled)
CLI-->>User: ❌ Full Python traceback
Note over CLI,Mixin: Before this revert (PR #1635 behaviour)
User->>CLI: praisonai "prompt"
CLI->>CLI: _execute_agent_with_budget_handling(agent, 'start', prompt)
CLI->>Agent: agent.start(prompt)
Agent->>Mixin: _call_llm(...)
Mixin-->>Agent: raises BudgetExceededError
Agent-->>CLI: BudgetExceededError caught
CLI-->>User: ✅ Clean error + sys.exit(1)
Reviews (1): Last reviewed commit: "Revert "fix: resolve BudgetExceededError..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -4643,26 +4611,26 @@ def level_based_approve(function_name, arguments, risk_level): | |||
| from rich.panel import Panel | |||
There was a problem hiding this comment.
By removing _execute_agent_with_budget_handling, this revert restores the original bug from #1627. BudgetExceededError is raised directly in chat_mixin.py (line 635) when an agent's cost hits its configured limit, and there is now no catch in main.py — it will propagate as a raw, multi-line Python traceback to the CLI user instead of a clean, actionable message. The grep confirms zero remaining BudgetExceededError references in main.py after this revert.
| @@ -544,7 +544,6 @@ def __init__( | |||
| verification_hooks: Optional[List[Any]] = None, # Deprecated: use autonomy=AutonomyConfig(verification_hooks=[...]) | |||
There was a problem hiding this comment.
max_budget convenience parameter removed from public API
The max_budget keyword argument was part of the public Agent(...) constructor after #1635. Any user or integration that adopted Agent(max_budget=1.00) from documentation or examples will now receive a TypeError: __init__() got an unexpected keyword argument 'max_budget' at runtime. Users still need to pass the full execution=ExecutionConfig(max_budget=...) form, which is less discoverable.
|
@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 • Architecture Review and Fix Implementation ✅Phase 1: Review per AGENTS.md
Phase 2: Fix Valid Issues
Phase 3: Final Verdict
✅ ISSUES RESOLVEDAll critical issues identified by automated reviewers have been fixed: 🔧 Issue 1: Breaking API Change - FIXED
🔧 Issue 2: BudgetExceededError Regression - FIXED
📊 Architecture Compliance Assessment
🚀 RECOMMENDATION: APPROVERationale:
The original revert was too aggressive - these fixes restore essential user-facing functionality while maintaining the architectural benefits. 📝 Files Modified
🔗 Next StepsJob Run: View details |
1 participant
Reverts #1635
Summary by CodeRabbit
max_budgetconvenience parameter from Agent constructor initialization. Budget limits must now be configured exclusively through theExecutionConfigparameter when creating agents.