Skip to content

Commit aec3684

Browse files
committed
fix(openai-agents): stop double-counting a failed run's token spend
Two sources describe the same spend and they overlap. The run hooks add each turn as it finishes, so by the time the run raises they already hold every completed turn, and the exception carries the SDK's own aggregate over those same turns. The error path added the aggregate to the accumulator, so any run that failed after paid turns reported roughly twice what it cost. MaxTurnsExceeded does that by definition, which makes this the common case rather than an edge one. A three-turn run reporting 70 input tokens reported 140. The aggregate is the authoritative figure, so it now replaces the accumulator rather than adding to it, matching what the TypeScript handler does. When the error carries no aggregate, which is what a tool handler's own error looks like, the accumulator is all there is and is used instead. Neither having anything still writes nothing, because all-zero attributes would assert the run cost nothing. Three tests, one per branch. The double-count one fails with 140 against 70 when the fix is reverted, which is how I checked it pins the bug rather than the behaviour. Found by Bugbot on #33, severity High.
1 parent 63ded86 commit aec3684

2 files changed

Lines changed: 125 additions & 10 deletions

File tree

packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
AiConfigRep,
2727
LDContext,
2828
ProviderHandler,
29+
RunUsage,
2930
SpanUsage,
3031
config,
3132
create_handler,
@@ -289,6 +290,34 @@ def abandon_open_spans(self, ended: set[int]) -> None:
289290
self.open_model_span = None
290291

291292

293+
def _write_failed_run_usage(
294+
span: Any,
295+
config: AiConfigRep,
296+
error: BaseException,
297+
run_usage: RunUsage,
298+
) -> None:
299+
"""Writes what a failed run spent onto the root, without counting it twice.
300+
301+
Two sources describe the same spend, and they overlap. The run hooks add each turn as it
302+
finishes, so by the time the run raises they already hold every completed turn. The exception
303+
also carries the SDK's own aggregate over those same turns.
304+
305+
Adding the aggregate to the accumulator therefore roughly doubles the reported cost of any run
306+
that failed after paid turns, which MaxTurnsExceeded does by definition. The aggregate is the
307+
authoritative figure, so it replaces the accumulator rather than adding to it.
308+
309+
When the error carries no aggregate, which is what a tool handler's own error looks like, the
310+
accumulator is all there is and is used instead. Nothing is written when neither has anything:
311+
all-zero attributes would assert the run cost nothing, which a run that died on its first call
312+
cannot claim.
313+
"""
314+
spent = _usage_from_error(error)
315+
if spent is not None:
316+
finish_root_span(span, config, spent)
317+
elif run_usage.reported:
318+
finish_root_span(span, config, run_usage.total)
319+
320+
292321
def _usage_from_error(error: BaseException) -> SpanUsage | None:
293322
"""The run's spend at the point it raised, when the SDK attached one.
294323
@@ -373,11 +402,7 @@ async def _call_impl(
373402
}
374403
except Exception as exc:
375404
hooks.close_open_spans(exc)
376-
spent = _usage_from_error(exc)
377-
if spent is not None:
378-
run_usage.add(spent)
379-
if run_usage.reported:
380-
finish_root_span(span, config, run_usage.total)
405+
_write_failed_run_usage(span, config, exc, run_usage)
381406
fail_span(span, exc)
382407
raise
383408

@@ -493,11 +518,7 @@ async def _stream_gen(
493518

494519
except Exception as exc:
495520
hooks.close_open_spans(exc)
496-
spent = _usage_from_error(exc)
497-
if spent is not None:
498-
run_usage.add(spent)
499-
if run_usage.reported:
500-
finish_root_span(span, config, run_usage.total)
521+
_write_failed_run_usage(span, config, exc, run_usage)
501522
fail_span(span, exc, ended)
502523
raise
503524
finally:

packages/openai-agents/tests/test_handler.py

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,3 +1381,97 @@ def test_history_without_prior_instructions(self) -> None:
13811381
assert instructions is not None
13821382
assert "Conversation History:" in instructions
13831383
assert "user: What is feature flagging?" in instructions
1384+
1385+
1386+
# ---------------------------------------------------------------------------
1387+
# TELEMETRY-CONTRACT.md section 6: what a failed run reports it spent
1388+
# ---------------------------------------------------------------------------
1389+
1390+
1391+
class _AgentsError(Exception):
1392+
"""An AgentsException-shaped error: it carries the SDK's own run aggregate.
1393+
1394+
MaxTurnsExceeded is the common case, and by definition it happens after paid turns.
1395+
"""
1396+
1397+
def __init__(self, input_tokens_: int, output_tokens_: int) -> None:
1398+
super().__init__("max turns exceeded")
1399+
1400+
class _Usage:
1401+
# Matches agents.Usage, which is snake_case.
1402+
input_tokens = input_tokens_
1403+
output_tokens = output_tokens_
1404+
input_tokens_details = None
1405+
1406+
class _Ctx:
1407+
usage = _Usage()
1408+
1409+
class _RunData:
1410+
context_wrapper = _Ctx()
1411+
1412+
self.run_data = _RunData()
1413+
1414+
1415+
class TestFailedRunUsage:
1416+
async def test_reports_the_sdk_aggregate_once_not_twice(self) -> None:
1417+
# The hooks already added every completed turn by the time the run raises, and the exception
1418+
# carries the SDK's aggregate over those same turns. Adding one to the other roughly doubles
1419+
# the reported cost of any run that failed after paid turns.
1420+
turns = [
1421+
{
1422+
"output": _text_output("one"),
1423+
"usage": {"input_tokens": 30, "output_tokens": 5},
1424+
},
1425+
{
1426+
"output": _text_output("two"),
1427+
"usage": {"input_tokens": 40, "output_tokens": 7},
1428+
},
1429+
]
1430+
1431+
async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any:
1432+
await _drive_turns(hooks, agent, prompt, turns)
1433+
raise _AgentsError(70, 12)
1434+
1435+
ctx, rec = _recording()
1436+
agents_mod = _fake_agents_module(run=run)
1437+
with ctx, _patched_agents(agents_mod), pytest.raises(_AgentsError):
1438+
await create_openai_agent_handler()(CONFIG, "q", {}, {})
1439+
1440+
# 70 and 12 are the aggregate. 140 and 24 would be the aggregate counted twice.
1441+
assert rec.root.attributes["gen_ai.usage.input_tokens"] == 70
1442+
assert rec.root.attributes["gen_ai.usage.output_tokens"] == 12
1443+
1444+
async def test_falls_back_to_the_turns_it_saw_when_the_error_carries_nothing(
1445+
self,
1446+
) -> None:
1447+
# A tool handler's own error propagates unwrapped and has no run_data, so the accumulated
1448+
# turns are the only record of what the run spent.
1449+
turns = [
1450+
{
1451+
"output": _text_output("one"),
1452+
"usage": {"input_tokens": 30, "output_tokens": 5},
1453+
}
1454+
]
1455+
1456+
async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any:
1457+
await _drive_turns(hooks, agent, prompt, turns)
1458+
raise RuntimeError("tool exploded")
1459+
1460+
ctx, rec = _recording()
1461+
agents_mod = _fake_agents_module(run=run)
1462+
with ctx, _patched_agents(agents_mod), pytest.raises(RuntimeError):
1463+
await create_openai_agent_handler()(CONFIG, "q", {}, {})
1464+
1465+
assert rec.root.attributes["gen_ai.usage.input_tokens"] == 30
1466+
1467+
async def test_writes_nothing_when_the_run_died_before_any_turn(self) -> None:
1468+
# All-zero attributes would assert the run cost nothing, which is a different claim.
1469+
async def run(agent: Any, prompt: str, hooks: Any = None, **kw: Any) -> Any:
1470+
raise RuntimeError("died immediately")
1471+
1472+
ctx, rec = _recording()
1473+
agents_mod = _fake_agents_module(run=run)
1474+
with ctx, _patched_agents(agents_mod), pytest.raises(RuntimeError):
1475+
await create_openai_agent_handler()(CONFIG, "q", {}, {})
1476+
1477+
assert "gen_ai.usage.input_tokens" not in rec.root.attributes

0 commit comments

Comments
 (0)