fix: stop double-counting tool usage, halving max_usage_count - #6769
fix: stop double-counting tool usage, halving max_usage_count#6769NishchayMahor wants to merge 1 commit into
Conversation
CrewStructuredTool.invoke()/ainvoke() already increment the tool's usage count, but ToolUsage._use/_ause incremented the same tool object a second time after running it. A tool with max_usage_count=N was therefore blocked after N/2 successful calls. Remove the redundant increment in ToolUsage and keep the authoritative one co-located with the limit check in invoke(). Tools without a self-incrementing invoke (the elif branch) are unaffected.
📝 WalkthroughWalkthroughChangesTool usage counting
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/crewai/tests/tools/test_tool_usage_limit.py (1)
155-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for asynchronous execution.
The new test covers
ToolUsage.use()only. This PR also changesToolUsage._ause()at Line 422-424 inlib/crewai/src/crewai/tools/tool_usage.py. Add an async test that callsawait tool_usage.ause(...)and verifies one increment per call and blocking aftermax_usage_count.As per coding guidelines, write behavior-focused unit tests for new functionality.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/tools/test_tool_usage_limit.py` around lines 155 - 193, Add an asynchronous regression test alongside test_tool_usage_use_does_not_double_count that invokes ToolUsage.ause(...) with the CountingTool setup, verifies current_usage_count increases once per allowed call, confirms both allowed calls execute, and confirms a subsequent call over max_usage_count neither executes nor increments the count.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/crewai/tests/tools/test_tool_usage_limit.py`:
- Around line 155-193: Add an asynchronous regression test alongside
test_tool_usage_use_does_not_double_count that invokes ToolUsage.ause(...) with
the CountingTool setup, verifies current_usage_count increases once per allowed
call, confirms both allowed calls execute, and confirms a subsequent call over
max_usage_count neither executes nor increments the count.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dd5b81e5-bfab-41db-af67-6de5a9763d97
📒 Files selected for processing (2)
lib/crewai/src/crewai/tools/tool_usage.pylib/crewai/tests/tools/test_tool_usage_limit.py
Summary
A tool configured with
max_usage_count=Nis blocked after only N/2 successful calls because its usage count is incremented twice per call.Root cause
CrewStructuredTool.invoke()andainvoke()already callself._increment_usage_count()(co-located with thehas_reached_max_usage_count()check).ToolUsage._use/_auserun the tool via that sameinvoke()/ainvoke(), then increment the same tool object a second time viaavailable_tool._increment_usage_count(). Socurrent_usage_countrises by 2 on every successful (non-cached) call.The two increments were introduced by separate PRs (#3362 added the one in
structured_tool.py; #4258 added the one intool_usage.py), so this is a merge-time regression.Fix
Remove the redundant increment in
ToolUsage._use/_ause, keeping the authoritative one insideinvoke()/ainvoke()where it is paired with the limit check. Theelifbranch — for tools that exposecurrent_usage_countbut have no self-incrementinginvoke— still increments manually and is unchanged.Verification
Added
test_tool_usage_use_does_not_double_countwhich drives the realToolUsage.use()path and asserts the tool runs exactlymax_usage_counttimes. Fails onmain, passes with the fix.ruff clean.
This fix was developed with AI assistance; I verified the double-count and the fix end-to-end with the reproduction above and reviewed every line.