-
Notifications
You must be signed in to change notification settings - Fork 267
Enh: Allow extending cost limit in textual UI #567
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.
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
base: main
Are you sure you want to change the base?
Conversation
When cost or step limits are exceeded in mini -v, instead of stopping, the agent now prompts the user to enter new limits and continues execution. Changes: - Added LimitsExceeded exception handling in TextualAgent.query() - Shows warning notification with current limits and spend - Prompts user for new step limit and cost limit with helpful placeholders - Added placeholder parameter to SmartInputContainer.request_input() - Updated tests to verify new behavior
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds the ability to extend cost and step limits on-the-fly when using the textual UI (mini -v), allowing users to continue tasks without restarting when limits are exceeded.
Key changes:
- Added exception handling for
LimitsExceededin the textual agent to prompt for new limits instead of stopping - Enhanced the input container with an optional placeholder parameter for better user guidance
- Updated tests to verify the new limit extension behavior and invalid input handling
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/minisweagent/agents/interactive_textual.py |
Added LimitsExceeded exception handling with user prompts for new limits and placeholder support in input requests |
tests/agents/test_interactive_textual.py |
Updated existing tests and added new test case for invalid input handling when extending limits |
klieret
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, I just tested it and it seems to work very well! I left two things that need to be resolved though, but shouldn't be too hard
| except LimitsExceeded: | ||
| # Show current limits and prompt for new ones (matching interactive.py behavior) | ||
| # Show notification with the limits info | ||
| self.app.notify( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move that into the prompt, i.e., the first prompt is
"Some limit was exceeded. Current number of steps xx, current limit yy. Enter new limit (0 for unconstrained)"
and I expect that the step limit is probably always 0, so it might be good to test if "step limit == 0" (we skip asking for a new step limit, because that's not the one we blew).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, great, let's move that to the prompt, leave the placeholder as it was, and remove the notification for simplicity. Yes, it makes sense that the step limit probably is always 0, so let's add an if statement for that.
| try: | ||
| self.config.step_limit = int(new_step_limit) if new_step_limit.strip() else 0 | ||
| except ValueError: | ||
| self.config.step_limit = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is dangerous, we can't do that, users would think they have set a limit but they haven't, honestly I'd rather have the application crash.
Or we put it in a small while loop that only breaks once it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, let's put a while loop.
- Move limit information from notification to prompt text for better visibility - Skip step_limit prompt when step_limit == 0 (only ask for cost_limit) - Add while loops to retry on invalid input instead of silently setting to 0 - Remove redundant notification when limits are exceeded - Remove unused placeholder parameter from request_input method - Update tests to match new behavior and use type_text() for consistency Addresses feedback from PR review.
for more information, see https://pre-commit.ci
Thanks Kilian 🙌. Great, I applied the suggestions there, they make sense, sorry for some oversights, I tried it and it seems to be working great 👍. |
|
Thanks for the updates, will test this soon! |
Ok, Great 🙌. |
Summary
Currently, in the textual UI (
mini -v), the agent stops immediately when the cost/step limit is exceeded (default $3.00). This PR adds the ability for users to extend these limits on-the-fly when usingmini -v(textual UI), allowing tasks to continue without restarting.Details
LimitsExceededexception handling inTextualAgent.query()placeholderparameter toSmartInputContainer.request_input()Testing
test_agent_with_cost_limitandtest_agent_with_step_limittest_interactive_textual.pypassCloses #335