fix(agent): report per-call usage metrics on kickoff results#6506
fix(agent): report per-call usage metrics on kickoff results#6506joaomdmoura wants to merge 1 commit into
Conversation
Agent.kickoff() populated result.usage_metrics from the LLM instance's lifetime token accumulator, so counts grew across calls and pooled across agents sharing one LLM object — a second agent's first turn appeared to cost the whole preceding session. Snapshot the accumulator when a kickoff starts and report the delta on the result (guardrail retries included), via the new UsageMetrics.delta_since(). The LLM instance's cumulative counters are untouched: get_token_usage_summary() keeps lifetime totals for crew-level aggregation, and its docstring now states that scope explicitly. Applies to both Agent and the deprecated LiteAgent, sync and async paths. Fixes EPD-177. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
lib/crewai/src/crewai/lite_agent.py (1)
293-293: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBaseline stored on
selfinstead of threaded as a parameter.Unlike
Agent.kickoff()(core.py), which keepsusage_baselineas a local variable passed through the call chain,LiteAgentstores it onself._kickoff_usage_baseline. Sincekickoff()already mutates other shared instance state without synchronization, this isn't a regression in isolation, but it's inconsistent with the safer pattern used elsewhere in this PR and would corrupt usage attribution if the sameLiteAgentinstance'skickoff()/kickoff_async()is invoked concurrently.Consider threading the baseline through
_execute_core(..., usage_baseline=...)as a parameter instead of an instance attribute, matchingAgent's approach.Also applies to: 521-524
🤖 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/src/crewai/lite_agent.py` at line 293, Replace the instance-level _kickoff_usage_baseline state with a local baseline in LiteAgent.kickoff() and kickoff_async(), and thread it explicitly through _execute_core(..., usage_baseline=...) and any intermediate call sites, matching Agent’s implementation. Remove the PrivateAttr declaration and update usage attribution to read only from the passed parameter.lib/crewai/src/crewai/agent/core.py (2)
1686-1696: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate logic with
LiteAgent._current_usage_summary.This method is byte-for-byte identical (code and docstring) to
LiteAgent._current_usage_summaryinlib/crewai/src/crewai/lite_agent.py(lines 551-561). SinceAgentandLiteAgentdon't share a common base for this concern, consider extracting a small shared helper (e.g. a module-level function takingllmandtoken_process) to avoid maintaining two copies.🤖 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/src/crewai/agent/core.py` around lines 1686 - 1696, Extract the duplicated usage-summary logic from Agent._current_usage_summary and LiteAgent._current_usage_summary into a shared module-level helper accepting the LLM and token process, then update both methods to delegate to it while preserving their UsageMetrics behavior and documentation as appropriate.
1611-1634: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a guardrail-retry usage_metrics regression test.
The retry path correctly threads
usage_baselinethrough_process_kickoff_guardrailso retries report whole-call usage, but the new tests only cover sync/async/shared-LLM/LiteAgent flows — not a guardrail-retry scenario, which is the most usage-sensitive path here (each retry adds more LLM usage that must roll up into one delta).🧪 Suggested test outline
def test_guardrail_retry_usage_includes_all_attempts(self): # LLM that fails guardrail once, succeeds second time ... result = agent.kickoff("question") # usage_metrics should reflect 2 LLM calls' worth of tokens, not just the last assert result.usage_metrics["successful_requests"] == 2As per path instructions,
**/tests/**/*.pyshould "Write unit tests for new functionality that focus on behavior rather than implementation details."Also applies to: 1812-1888
🤖 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/src/crewai/agent/core.py` around lines 1611 - 1634, The usage_metrics coverage is missing the guardrail-retry path. Add a behavior-focused regression test in the relevant tests module that uses an LLM returning an output rejected by the guardrail once and accepted on the second attempt, then calls the agent kickoff flow and asserts result.usage_metrics reflects both LLM calls, including successful_requests == 2; ensure the test exercises the public behavior rather than internal implementation details.Source: Path instructions
🤖 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/src/crewai/agent/core.py`:
- Around line 1686-1696: Extract the duplicated usage-summary logic from
Agent._current_usage_summary and LiteAgent._current_usage_summary into a shared
module-level helper accepting the LLM and token process, then update both
methods to delegate to it while preserving their UsageMetrics behavior and
documentation as appropriate.
- Around line 1611-1634: The usage_metrics coverage is missing the
guardrail-retry path. Add a behavior-focused regression test in the relevant
tests module that uses an LLM returning an output rejected by the guardrail once
and accepted on the second attempt, then calls the agent kickoff flow and
asserts result.usage_metrics reflects both LLM calls, including
successful_requests == 2; ensure the test exercises the public behavior rather
than internal implementation details.
In `@lib/crewai/src/crewai/lite_agent.py`:
- Line 293: Replace the instance-level _kickoff_usage_baseline state with a
local baseline in LiteAgent.kickoff() and kickoff_async(), and thread it
explicitly through _execute_core(..., usage_baseline=...) and any intermediate
call sites, matching Agent’s implementation. Remove the PrivateAttr declaration
and update usage attribution to read only from the passed parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 66707a74-f8ed-47f6-8bc0-c456d530540e
📒 Files selected for processing (6)
lib/crewai/src/crewai/agent/core.pylib/crewai/src/crewai/lite_agent.pylib/crewai/src/crewai/lite_agent_output.pylib/crewai/src/crewai/llms/base_llm.pylib/crewai/src/crewai/types/usage_metrics.pylib/crewai/tests/agents/test_lite_agent.py
Summary
Fixes EPD-177:
Agent.kickoff()populatedresult.usage_metricsfromllm.get_token_usage_summary(), which returns the LLM instance's lifetime accumulator. Counts therefore grew across calls and pooled across agents sharing oneLLMobject — with a shared module-level LLM, a second agent's first turn appeared to "cost" the whole preceding session. Broken cost attribution and dashboards, hit by an enterprise customer in production migration.Fix
UsageMetrics.delta_since(baseline): field-wise difference between two snapshots of the same accumulator, clamped at zero.Agent(sync + async kickoff): snapshot the accumulator right after kickoff preparation and thread the baseline through output building and the guardrail-retry path, so the result reports usage for that call only — including all guardrail retry attempts.LiteAgent(deprecated, same bug): snapshot at kickoff start (_kickoff_usage_baseline) and report deltas at both output-building sites.get_token_usage_summary()still returns instance-lifetime totals (crew-level aggregation increw.pyand the event-based flow aggregation depend on the existing semantics). Its docstring now states the instance-lifetime scope explicitly, andLiteAgentOutput.usage_metrics's field description documents the per-call semantics.Known limitation (inherent to counter diffing): two kickoffs running concurrently against the same LLM instance can still attribute each other's overlapping usage; sequential calls — the reported scenario — are now exact.
Testing
TestKickoffUsageMetricsArePerCall,TestUsageMetricsDeltaSince): shared-LLM agents, repeated kickoffs on one agent, async kickoff, LiteAgent, plus delta_since field-wise math and negative-clamp.tests/agents/test_lite_agent.py(35),tests/agents/test_agent.py(97), usage-related tests intest_crew.py/test_llm.py,test_flow_usage_metrics.py+events/test_llm_usage_event.py(44), ruff, and mypy.🤖 Generated with Claude Code
Note
Medium Risk
Changes the meaning of
LiteAgentOutput.usage_metricsfor dashboards and billing; behavior is intentional but any consumer that relied on cumulative kickoff totals will see different numbers.Overview
Kickoff
usage_metricsnow reflect a single call, not the shared LLM’s lifetime totals. PreviouslyAgent.kickoff()/LiteAgentcopiedget_token_usage_summary(), so costs stacked across repeated kickoffs and across agents sharing oneLLMinstance.Adds
UsageMetrics.delta_since(baseline)(field-wise difference, clamped at zero) and snapshots usage at kickoff start inAgent(sync/async, including guardrail retries) andLiteAgent(_kickoff_usage_baseline).get_token_usage_summary()andLiteAgentOutput.usage_metricsdocs clarify lifetime vs per-call semantics; cumulative LLM counters are unchanged for crew/flow aggregation.Regression tests cover shared LLM, repeated kickoffs, async kickoff, LiteAgent, and
delta_since.Reviewed by Cursor Bugbot for commit 439c32b. Bugbot is set up for automated code reviews on this repo. Configure here.