fix(eval): persist plan-execute token usage for cost/tokens reporting - #500
Open
AmirF194 wants to merge 1 commit into
Open
fix(eval): persist plan-execute token usage for cost/tokens reporting#500AmirF194 wants to merge 1 commit into
AmirF194 wants to merge 1 commit into
Conversation
PlanExecuteRunner already meters real token usage via _TokenMeter and writes it onto the OTel span, but never passed it to persist_trajectory(). StepResult has no token field, so _from_plan_execute() in metrics.py always built OpsMetrics with tokens_in=tokens_out=0, and _estimate_cost short-circuited to None. Every plan-execute evaluation report showed zero tokens and no cost, while every SDK-based runner reported correctly because their Trajectory dataclass carries per-turn token fields. Thread the meter's totals through persist_trajectory() as optional keyword arguments (omitted from the record when not given, so other runners' persisted shape is unchanged), and have metrics_from_trajectory() read them back into _from_plan_execute() for tokens_in/tokens_out and the cost estimate. Fixes IBM#499 Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
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.
Description
PlanExecuteRunnermeters real token usage via_TokenMeterand writes accurate totals onto the OTel span (gen_ai.usage.input_tokens/output_tokens), but that data never reaches the persisted trajectory.persist_trajectory()only receivestrajectory=list[StepResult], andStepResulthas no token field, so_from_plan_execute()insrc/evaluation/metrics.pyalways buildsOpsMetricswithtokens_in=tokens_out=0, and_estimate_cost(model, 0, 0)short-circuits toNone. Every offline evaluation report shows zero tokens and no cost for the plan-execute agent specifically; every SDK-based runner reports correctly because theirTrajectorydataclass carries per-turn token fields.Fix Details
Thread the meter's totals through as two new optional keyword arguments on
persist_trajectory()(tokens_in,tokens_out), written onto the record only when given, so the six other runners' calls and their persisted shape are unchanged.PlanExecuteRunner.run()now passesself._meter.input_tokens/output_tokens.metrics_from_trajectory()reads them back off the record (getattr(..., None) or 0, so older persisted files without these keys still parse) and passes them into_from_plan_execute(), which now populatesOpsMetrics.tokens_in/tokens_outand the cost estimate instead of hardcoding zeros.Impact on Benchmarking
tokens_in,tokens_outandest_cost_usdfor plan-execute scenario results move from always0/Noneto the run's real usage.aggregate_ops()'s existing sums (tokens_in_total,tokens_out_total,est_cost_usd_total) are the only readers of these fields elsewhere in the codebase, and they already treat aNonecost as excluded, so no other code path assumed the old zeros.Related Issues
Verification Steps
test_orchestrator_persists_token_usage_alongside_trajectory,test_persist_includes_run_level_tokens_when_given,test_plan_execute_list_trajectory_reads_run_level_tokens) fail against unmodifiedmainand pass on this branch; ran in a cleanpython:3.12-slimcontainer withuv sync,__file__-checked against/repo/src/....src/agent/tests/,src/evaluation/tests/,src/observability/tests/): 187 passed, 8 pre-existing failures unrelated to this change (missinggoogle.protobufintest_file_exporter.py, an unrelated scorer issue intest_static_json_scorer.py), same 8 failing identically on unmodifiedmain.coverage run/reportconfirms every changed line in the three touched files is covered.AGENT_TRAJECTORY_DIR/evaluation-CLI run against a real LLM backend; the repro is the unit/integration test suite above, not a manual end-to-end run.Checklist