fix(cli): graceful BudgetExceededError — wrapper-only, no Agent param bloat (fixes #1627)#1645
Conversation
…nt param bloat) Re-implements the CLI UX half of reverted PR #1635 without the core SDK changes that violated AGENTS.md. ## What changed - Add _execute_agent_with_budget_handling(agent, method_name, *args, **kwargs) helper on PraisonAIApp: calls agent.<method_name>(...) and catches BudgetExceededError to print a single-line actionable CLI message and exit(1) instead of leaking a raw traceback. - Route all 17 agent.start(prompt) / agent.chat(prompt) / auto_rag.chat(prompt) call sites in the direct-prompt dispatch through the helper. Covers every display mode: silent, quiet, verbose, debug, jsonl, json, flow, editor, default. ## What is NOT changed (intentionally) - Zero edits to praisonaiagents (core SDK). - NO new 'max_budget=' parameter on Agent.__init__. - Users continue to configure budgets via the already-consolidated execution=ExecutionConfig(max_budget=...) pattern per AGENTS.md §5.3 'Parameter Consolidation'. ## Why wrapper-only AGENTS.md §4.1 reserves core SDK for protocols/hooks/adapters — CLI UX belongs in the wrapper. AGENTS.md §5.3 explicitly consolidates execution- related knobs (max_iter, max_rpm, max_budget, rate_limiter, ...) into ExecutionConfig precisely to avoid adding more top-level Agent params. ## Error message Points users to the correct API: Budget limit exceeded: <reason>. Hint: set budget via execution=ExecutionConfig(max_budget=1.00) on your Agent. Closes #1627
ⓘ 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 (1)
📝 WalkthroughWalkthroughThis PR adds centralized budget error handling to the PraisonAI CLI by introducing a wrapper method that catches ChangesBudget Error Handling Centralization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
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:
|
Greptile SummaryThis PR adds a single helper method
Confidence Score: 3/5The error handler contains a defect that causes it to crash at the exact moment it is supposed to recover gracefully. The central helper method will raise a src/praisonai/praisonai/cli/main.py — specifically the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[handle_direct_prompt] --> B{display_mode?}
B --> |silent/quiet/verbose/debug/jsonl/json/flow/editor/default| C[_execute_agent_with_budget_handling]
B --> |auto_rag| D[_execute_agent_with_budget_handling\nauto_rag.chat]
C --> E{agent has 'start'?}
E --> |yes| F[agent.start prompt]
E --> |no| G[agent.chat prompt]
F --> H{BudgetExceededError?}
G --> H
H --> |no| I[return result]
H --> |yes| J[rich_print error message\n⚠️ str e contains markup]
J --> K[sys.exit 1\n⚠️ teardown skipped in verbose/debug/flow]
|
![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 introduces a centralized _execute_agent_with_budget_handling method to manage BudgetExceededError across CLI agent executions. The review identifies a missing application of this helper in _run_inline_workflow for consistency and highlights critical issues regarding sys.exit(1) bypassing cleanup logic and potential output corruption of machine-readable formats. A code suggestion was provided to redirect errors to stderr and escape markup tags for safer rendering.
|
|
||
| return results[-1].get("output", "") if results else "" | ||
|
|
||
| def _execute_agent_with_budget_handling(self, agent, method_name, *args, **kwargs): |
There was a problem hiding this comment.
This helper method should also be applied to the agent.chat(full_prompt) call site within _run_inline_workflow (around line 4030) to ensure consistent budget error handling across all direct-prompt execution paths.
| except BudgetExceededError as e: | ||
| from rich import print as rich_print | ||
| rich_print( | ||
| f"[red]Budget limit exceeded: {e!s}. " | ||
| "Hint: set budget via " | ||
| "execution=ExecutionConfig(max_budget=1.00) on your Agent.[/red]" | ||
| ) | ||
| sys.exit(1) |
There was a problem hiding this comment.
This error handling block has two issues:
- Cleanup Bypass: Calling
sys.exit(1)here prevents the execution of cleanup logic at the call sites (e.g.,disable_status_output(),flow.stop(),disable_editor_output()). To fix this, call sites should wrap the execution in atry...finallyblock to ensure terminal state is restored even on exit. - Output Corruption: Printing the error to
stdoutcan break machine-readable outputs like JSON/JSONL. It should be directed tostderr.
Additionally, it is recommended to escape the exception message to prevent rich from misinterpreting characters like [ or ] as markup tags.
| except BudgetExceededError as e: | |
| from rich import print as rich_print | |
| rich_print( | |
| f"[red]Budget limit exceeded: {e!s}. " | |
| "Hint: set budget via " | |
| "execution=ExecutionConfig(max_budget=1.00) on your Agent.[/red]" | |
| ) | |
| sys.exit(1) | |
| except BudgetExceededError as e: | |
| from rich.console import Console | |
| from rich.markup import escape | |
| Console(stderr=True).print( | |
| f"[red]Budget limit exceeded: {escape(str(e))}. " | |
| "Hint: set budget via " | |
| "execution=ExecutionConfig(max_budget=1.00) on your Agent.[/red]" | |
| ) | |
| sys.exit(1) |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| except BudgetExceededError as e: | ||
| from rich import print as rich_print | ||
| rich_print( | ||
| f"[red]Budget limit exceeded: {e!s}. " | ||
| "Hint: set budget via " | ||
| "execution=ExecutionConfig(max_budget=1.00) on your Agent.[/red]" | ||
| ) |
There was a problem hiding this comment.
PraisonAIError.__str__() returns "[budget] <message> (agent: ..., run: ...)". When {e!s} is interpolated directly into a Rich markup string, the [budget] token is treated as a markup tag by Rich. Since budget is not a recognised style, Rich raises a MarkupError, causing the error handler itself to crash — the opposite of graceful recovery. The fix is to escape the exception message before embedding it in the markup string.
| except BudgetExceededError as e: | |
| from rich import print as rich_print | |
| rich_print( | |
| f"[red]Budget limit exceeded: {e!s}. " | |
| "Hint: set budget via " | |
| "execution=ExecutionConfig(max_budget=1.00) on your Agent.[/red]" | |
| ) | |
| except BudgetExceededError as e: | |
| from rich import print as rich_print | |
| from rich.markup import escape as _escape | |
| rich_print( | |
| f"[red]Budget limit exceeded: {_escape(str(e))}. " | |
| "Hint: set budget via " | |
| "execution=ExecutionConfig(max_budget=1.00) on your Agent.[/red]" | |
| ) |
| rich_print( | ||
| f"[red]Budget limit exceeded: {e!s}. " | ||
| "Hint: set budget via " | ||
| "execution=ExecutionConfig(max_budget=1.00) on your Agent.[/red]" | ||
| ) | ||
| sys.exit(1) |
There was a problem hiding this comment.
When display_mode is jsonl or json, the budget error message is printed to stdout via rich_print, injecting a plain-text line into what is supposed to be a machine-readable JSON or JSONL stream. Any downstream consumer that parses stdout will fail to decode the envelope. Printing the message to stderr instead (e.g. rich_print("...", file=sys.stderr)) would preserve the structured stdout contract while still surfacing the error to the user.
|
@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 ✅Acting as final architecture reviewer for PR #1645. Analyzing per AGENTS.md and addressing issues found by other reviewers. Todo List:
Phase 1: Architecture Review ✅ AGENTS.md Compliance Check:
Phase 2: Critical Issues Found & FIXED ✅ Fixed Issues:
Phase 3: Final Verdict ✅ APPROVED - All blocking issues resolved:
Files Modified:
Recommendation: |
1 participant
Context
Re-implements the CLI-UX half of reverted PR #1635 without the core SDK changes that violated AGENTS.md.
Original PR #1635 did two things:
max_budget=param toAgent.__init__— violates AGENTS.md §5.3 Parameter Consolidation (execution knobs belong inExecutionConfig, not as new top-level Agent params) and also introduced anUnboundLocalErrorregression.BudgetExceededErrorso users get a clean message instead of a raw traceback.This PR keeps only the legitimate (2) and drops (1) entirely.
What changed
src/praisonai/praisonai/cli/main.py— wrapper-only, single file._execute_agent_with_budget_handling(agent, method_name, *args, **kwargs)onPraisonAIApp: invokesagent.<method_name>(…)and catchesBudgetExceededErrorto print a single-line rich-formatted message andexit(1).agent.start(prompt)/agent.chat(prompt)/auto_rag.chat(prompt)call-sites in the direct-prompt dispatch now route through the helper. Every display mode is covered:silent,quiet,verbose,debug,jsonl,json,flow,editor, default.What is NOT changed (deliberately)
praisonaiagents(core SDK).max_budget=param onAgent.__init__.Error message
Points users to the canonical API per AGENTS.md §5.3 — not to a deprecated top-level param.
AGENTS.md compliance
praisonai/wrapper; core SDK untouchedExecutionConfig.max_budget; no new Agent paramCloses #1627.
Summary by CodeRabbit