fix: remove tool notification rate limit#97
Merged
Conversation
- Remove rate limiting from notify_telegram_before_tool - Each parallel tool call now gets its own notification - Keep rate limiting for intermediate LLM text notifications - Update tests to expect all notifications through
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes the rate-limiting logic for Telegram tool notifications, ensuring that a notification is sent for every tool call. The changes include the removal of rate-limiting state variables and the associated logic within the notification function, along with updates to the test suite to reflect the new behavior. I have no feedback to provide as there were no review comments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Remove rate limiting from Telegram tool notifications so each parallel tool call gets its own notification.
Why
The previous implementation used a 100ms rate limit window for tool notifications. When the agent made 3 parallel
log_mealcalls within 7ms, only the first notification was sent. This was confusing because the user only saw one notification but the agent reported calling the tool 3 times.How
notify_telegram_before_tool()insrc/blacki/callbacks.py_TOOL_NOTIFY_*state variablesnotify_telegram_after_model()(intermediate LLM text) since streaming could still spamtest_notify_rate_limits_per_chat→test_notify_sends_for_each_tool_callTests