-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: properly reset cost limit tracking when user clicks "Reset and Continue" #6890
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
Conversation
…ontinue" - Track the message index at the time of cost reset - Only calculate costs from messages after the reset point - Prevents the issue where cost limit immediately triggers again after reset Fixes #6889
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.
Reviewing my own code is like debugging in production - technically possible but morally questionable.
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.
LGTM
This logic feels a lot messier than it needs to be. It seems like we should use the same approach and methods for tracking number of requests and total cost. |
…dler - Use consistent message-based tracking for both request count and cost limit - Track reset point using lastResetMessageIndex for both limits - Calculate request count from API messages similar to cost calculation - Simplify logic by removing separate tracking mechanisms - Update tests to match new message-based counting approach This addresses review feedback about inconsistent tracking approaches between request counting and cost tracking.
@mrubens Can you please take a look? |
Summary
This PR fixes issue #6889 where the cost limit was not being properly reset when users clicked "Reset and Continue" after hitting the auto-approved cost limit.
Problem
When a user hit the cost limit and clicked "Reset and Continue", the cost would immediately trigger the limit again on the next API request because the total cost was still being calculated from all messages in the conversation history.
Solution
The fix tracks the message index at the time of cost reset and only calculates costs from messages after that reset point. This ensures that:
Changes
costResetMessageIndex
tracking inAutoApprovalHandler
Testing
Fixes #6889
Important
Fixes cost limit reset issue in
AutoApprovalHandler
by tracking message index at reset and updating cost calculations accordingly.AutoApprovalHandler
by trackinglastResetMessageIndex
.lastResetMessageIndex
to track message index at reset inAutoApprovalHandler
.checkRequestLimit()
andcheckCostLimit()
to use messages after reset.resetRequestCount()
to resetlastResetMessageIndex
and cost tracking.AutoApprovalHandler.spec.ts
for cost reset functionality.This description was created by
for 07283bf. You can customize this summary. It will automatically update as commits are pushed.