Skip to content

Commit 1bcfe0b

Browse files
committed
fix(aws-strands-py): label OpenAI's Responses model and validate usage fixtures
The provider table missed `OpenAIResponsesModel`, which Strands ships as a second OpenAI model class, so a run served by OpenAI's Responses API reported its counts with no provider label. The exhaustiveness test caught it on the newest release; the label is `openai`, the same vendor its sibling class reports. The TypeScript table is unchanged on purpose: that SDK reaches the Responses API through a config on its single `OpenAIModel`, so it has no such class to key on. The token-usage fixtures then get the audit that failure implies. Strands declares `inputTokens`, `outputTokens` and `totalTokens` as `Required` on its own `Usage`, and its metrics accumulation and telemetry subscript them bare, so a payload missing one aborts the run inside the SDK on a version that does not default them. Ten fixtures were minimised to the single count their test talked about. Two failed at the declared floor; the other eight passed only because their assertions held on the RUN_ERROR that the incomplete payload caused, which made them tests of which Strands is installed rather than of this bridge. Every payload on a real model path now carries all three counts, and every one of those tests asserts the terminal event is RUN_FINISHED so the shortcut cannot come back silently. The three shapes that cannot carry all three and still make their point (no `usage` key at all, a counts-free usage object, string counts) move to the scripted core, which is the pattern the file already uses for a non-mapping payload, and each says why in its docstring.
1 parent 21b1026 commit 1bcfe0b

2 files changed

Lines changed: 152 additions & 25 deletions

File tree

integrations/aws-strands/python/src/ag_ui_strands/agent.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1433,6 +1433,14 @@ def _error_events(
14331433
# ``GeminiModel`` while the TypeScript one lives under ``models/google``, and a
14341434
# derived label would silently split that vendor in two. A class not listed
14351435
# here omits the provider label rather than guessing.
1436+
#
1437+
# Two classes may legitimately share one label. Python ships both
1438+
# ``OpenAIModel`` and ``OpenAIResponsesModel`` for OpenAI's two APIs, where the
1439+
# TypeScript SDK reaches the Responses API through a config on its single
1440+
# ``OpenAIModel``: one vendor either way, so both report ``openai``. Entries
1441+
# with no TypeScript counterpart at all (``litellm``, ``writer``) are the two
1442+
# SDKs shipping different provider classes, not the two bridges disagreeing on
1443+
# a label.
14361444
_STRANDS_PROVIDER_LABELS: Dict[str, str] = {
14371445
"AnthropicModel": "anthropic",
14381446
"BedrockModel": "bedrock",
@@ -1443,6 +1451,7 @@ def _error_events(
14431451
"MistralModel": "mistral",
14441452
"OllamaModel": "ollama",
14451453
"OpenAIModel": "openai",
1454+
"OpenAIResponsesModel": "openai",
14461455
"SageMakerAIModel": "sagemaker",
14471456
"WriterModel": "writer",
14481457
}

integrations/aws-strands/python/tests/test_terminal_event_token_usage.py

Lines changed: 143 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,12 @@
2020
2121
Most cases drive a REAL ``strands.Agent`` over a scripted ``Model``, so the
2222
metadata channel itself is proven rather than assumed, and the real multi-agent
23-
``Graph`` covers the orchestrator path. The two terminal shapes a scripted
24-
model cannot reach (an interrupt outcome, and the post-stream session gates)
25-
use a scripted core, as ``test_interrupt.py`` does.
23+
``Graph`` covers the orchestrator path. Every usage payload on that path is a
24+
VALID Strands payload, for the reason spelled out on ``_metadata``. What a real
25+
model cannot deliver goes through a scripted core, as ``test_interrupt.py``
26+
does: the terminal shapes it cannot reach (an interrupt outcome, the
27+
post-stream session gates) and the usage payloads that break Strands' own
28+
required-key contract.
2629
"""
2730

2831
from __future__ import annotations
@@ -104,7 +107,18 @@ async def stream(
104107

105108

106109
def _metadata(usage: Any = None, **extra: Any) -> dict:
107-
"""The stream event Strands reports one of per model invocation."""
110+
"""The stream event Strands reports one of per model invocation.
111+
112+
Every ``usage`` handed to a REAL model here carries ``inputTokens``,
113+
``outputTokens`` and ``totalTokens``, however little the case under test
114+
needs them: Strands' own ``Usage`` declares all three ``Required``, and its
115+
accumulation and telemetry subscript them bare, so a payload missing one
116+
aborts the run inside the SDK on any version that does not happen to
117+
default them (1.15 does not, 1.18 does). Trimming a fixture back to the one
118+
count a test talks about therefore stops testing this bridge and starts
119+
testing which Strands is installed. A payload that cannot carry all three
120+
and still make its point goes through ``_ScriptedCore`` instead.
121+
"""
108122
payload: dict = {"metrics": {"latencyMs": 12}, **extra}
109123
if usage is not None:
110124
payload["usage"] = usage
@@ -228,16 +242,28 @@ async def _run_scripted(turns: list[list[dict]], **model_kwargs) -> list:
228242
class _ScriptedCore:
229243
"""The ``StrandsAgentCore`` surface the adapter reads, stream scripted.
230244
231-
Used only for the terminal shapes a scripted model cannot reach: a paused
232-
checkpoint, and the post-stream mixed-checkpoint gates.
245+
Used for what a scripted model cannot deliver: the terminal shapes a real
246+
model never reaches (a paused checkpoint, the post-stream
247+
mixed-checkpoint gates), and usage payloads that break Strands' OWN
248+
``Usage`` contract, which its accumulation and telemetry reject before the
249+
adapter is reached. Those payloads still arrive off the wire in the wild,
250+
so the adapter's read of them is worth pinning, just not through a path
251+
that cannot carry them.
233252
"""
234253

235-
def __init__(self, events: list, *, interrupts=None, session_manager=None):
254+
def __init__(
255+
self,
256+
events: list,
257+
*,
258+
interrupts=None,
259+
session_manager=None,
260+
model_id: str = "scripted-1",
261+
):
236262
self.agent_id = "default"
237263
self.tool_registry = MagicMock()
238264
self.tool_registry.registry = {}
239265
self.state = AgentState()
240-
self.model = ScriptedModel([], model_id="scripted-1")
266+
self.model = ScriptedModel([], model_id=model_id)
241267
self.messages: list = []
242268
self.hooks = HookRegistry()
243269
self.session_manager = session_manager
@@ -376,21 +402,59 @@ async def test_a_provider_that_reports_no_usage_omits_the_field(self):
376402

377403
@pytest.mark.asyncio
378404
async def test_a_metadata_event_with_no_usage_key_omits_the_field(self):
379-
events = await _run_scripted([[*_turn(), _metadata(None)]])
405+
"""A metadata event may carry only latency, and that is not usage.
406+
407+
Driven through a scripted core: Strands' own accumulation subscripts
408+
``metadata["usage"]`` on the versions this package supports, so no
409+
real model path can hand the adapter a metadata event without it.
410+
"""
411+
core = _ScriptedCore([_stream_event(None)])
380412

413+
events = await _collect(_wrap(core))
414+
415+
assert _terminal(events).type == EventType.RUN_FINISHED
381416
assert _usage(events) is None
382417

383418
@pytest.mark.asyncio
384419
async def test_usage_with_no_usable_count_omits_the_field(self):
385-
events = await _run_scripted([_turn(usage={"inputTokens": "lots"})])
420+
"""All three counts reported, none of them a number, so none survive.
421+
422+
Driven through a scripted core because Strands accumulates its own
423+
run metrics by ``+=``-ing these values, which a string count breaks
424+
inside the SDK whichever keys are present. Numeric-but-unusable counts
425+
reach the guard over a real model and are covered there.
426+
"""
427+
core = _ScriptedCore(
428+
[
429+
_stream_event(
430+
{
431+
"inputTokens": "lots",
432+
"outputTokens": "a few",
433+
"totalTokens": "some",
434+
}
435+
)
436+
]
437+
)
386438

439+
events = await _collect(_wrap(core))
440+
441+
assert _terminal(events).type == EventType.RUN_FINISHED
387442
assert _usage(events) is None
388443

389444
@pytest.mark.asyncio
390445
async def test_a_labels_only_entry_is_never_emitted(self):
391-
"""The model label alone is not usage, so nothing is reported."""
392-
events = await _run_scripted([_turn(usage={})], model_id="labelled-model")
446+
"""The model label alone is not usage, so nothing is reported.
447+
448+
A usage object with no counts at all breaks Strands' required-key
449+
contract, so this one goes through a scripted core rather than a real
450+
model, which would abort the run inside the SDK before the adapter saw
451+
it.
452+
"""
453+
core = _ScriptedCore([_stream_event({})], model_id="labelled-model")
454+
455+
events = await _collect(_wrap(core))
393456

457+
assert _terminal(events).type == EventType.RUN_FINISHED
394458
assert _usage(events) is None
395459

396460
@pytest.mark.asyncio
@@ -439,21 +503,46 @@ async def test_a_count_beyond_the_safe_wire_range_is_dropped(self):
439503
@pytest.mark.asyncio
440504
async def test_the_largest_carriable_count_still_survives(self):
441505
"""The bound is inclusive, so the ceiling itself is not dropped."""
442-
events = await _run_scripted([_turn(usage={"inputTokens": _MAX_TOKEN_COUNT})])
506+
events = await _run_scripted(
507+
[
508+
_turn(
509+
usage={
510+
"inputTokens": _MAX_TOKEN_COUNT,
511+
"outputTokens": 7,
512+
"totalTokens": 9,
513+
}
514+
)
515+
]
516+
)
443517

518+
assert _terminal(events).type == EventType.RUN_FINISHED
444519
assert _usage(events)[0].input_tokens == _MAX_TOKEN_COUNT
445520

446521
@pytest.mark.asyncio
447522
async def test_an_integer_too_large_to_be_a_float_does_not_abort_the_run(self):
448-
"""``math.isfinite`` raises OverflowError on this, so order matters."""
523+
"""``math.isfinite`` raises OverflowError on this, so order matters.
524+
525+
The other two counts are ordinary and complete, so what the run loses
526+
is one count and not the whole entry, and the payload stays valid by
527+
Strands' own required-key contract.
528+
"""
449529
events = await _run_scripted(
450-
[_turn(usage={"inputTokens": 10**400, "outputTokens": 3})]
530+
[
531+
_turn(
532+
usage={
533+
"inputTokens": 10**400,
534+
"outputTokens": 3,
535+
"totalTokens": 5,
536+
}
537+
)
538+
]
451539
)
452540

453541
assert _terminal(events).type == EventType.RUN_FINISHED
454542
assert _reported(_usage(events)[0]) == {
455543
"model": "scripted-1",
456544
"output_tokens": 3,
545+
"total_tokens": 5,
457546
}
458547

459548
@pytest.mark.asyncio
@@ -478,12 +567,14 @@ async def test_every_other_malformed_shape_is_dropped_too(self):
478567
async def test_a_boolean_is_not_a_token_count(self):
479568
"""``bool`` subclasses ``int``, and ``True`` is not one token."""
480569
events = await _run_scripted(
481-
[_turn(usage={"inputTokens": True, "outputTokens": 2})]
570+
[_turn(usage={"inputTokens": True, "outputTokens": 2, "totalTokens": 3})]
482571
)
483572

573+
assert _terminal(events).type == EventType.RUN_FINISHED
484574
assert _reported(_usage(events)[0]) == {
485575
"model": "scripted-1",
486576
"output_tokens": 2,
577+
"total_tokens": 3,
487578
}
488579

489580
@pytest.mark.asyncio
@@ -503,8 +594,11 @@ async def test_a_wholly_malformed_payload_does_not_fail_the_run(self):
503594

504595
@pytest.mark.asyncio
505596
async def test_a_float_whole_number_is_accepted(self):
506-
events = await _run_scripted([_turn(usage={"inputTokens": 4.0})])
597+
events = await _run_scripted(
598+
[_turn(usage={"inputTokens": 4.0, "outputTokens": 1, "totalTokens": 5})]
599+
)
507600

601+
assert _terminal(events).type == EventType.RUN_FINISHED
508602
assert _usage(events)[0].input_tokens == 4
509603

510604

@@ -590,15 +684,24 @@ def test_every_model_class_the_sdk_ships_has_a_canonical_label(self):
590684
assert shipped
591685
assert shipped <= set(_STRANDS_PROVIDER_LABELS)
592686

593-
def test_the_labels_are_lowercase_and_unique(self):
687+
def test_the_labels_are_lowercase_and_one_vendor_gets_one_label(self):
688+
"""Two classes may share a label; one vendor may not have two.
689+
690+
A repeated label is only ever right when the classes really are one
691+
vendor's two APIs, which today is OpenAI's Chat Completions and
692+
Responses classes. Any other repetition is a vendor spelled twice, so
693+
the exception is enumerated here rather than waved through.
694+
"""
594695
labels = list(_STRANDS_PROVIDER_LABELS.values())
595696
assert labels == [label.lower() for label in labels]
596-
assert len(labels) == len(set(labels))
697+
assert {label for label in labels if labels.count(label) > 1} == {"openai"}
597698

598699
@pytest.mark.asyncio
599700
async def test_an_unrecognised_model_class_omits_the_provider_label(self):
600701
"""``ScriptedModel`` is nobody's provider, so no provider is claimed."""
601-
events = await _run_scripted([_turn(usage={"inputTokens": 1})])
702+
events = await _run_scripted(
703+
[_turn(usage={"inputTokens": 1, "outputTokens": 1, "totalTokens": 2})]
704+
)
602705

603706
entry = _usage(events)[0]
604707
assert entry.provider is None
@@ -611,13 +714,19 @@ def get_config(self):
611714
raise RuntimeError("no config for you")
612715

613716
core = StrandsAgentCore(
614-
model=_HostileModel([_turn(usage={"inputTokens": 8})]),
717+
model=_HostileModel(
718+
[_turn(usage={"inputTokens": 8, "outputTokens": 2, "totalTokens": 10})]
719+
),
615720
callback_handler=None,
616721
)
617722
events = await _collect(_wrap(core))
618723

619724
assert _terminal(events).type == EventType.RUN_FINISHED
620-
assert _reported(_usage(events)[0]) == {"input_tokens": 8}
725+
assert _reported(_usage(events)[0]) == {
726+
"input_tokens": 8,
727+
"output_tokens": 2,
728+
"total_tokens": 10,
729+
}
621730

622731
@pytest.mark.asyncio
623732
async def test_a_config_without_a_model_id_omits_the_model_label(self):
@@ -626,12 +735,19 @@ def get_config(self):
626735
return {"params": {"temperature": 0}}
627736

628737
core = StrandsAgentCore(
629-
model=_UnlabelledModel([_turn(usage={"inputTokens": 8})]),
738+
model=_UnlabelledModel(
739+
[_turn(usage={"inputTokens": 8, "outputTokens": 2, "totalTokens": 10})]
740+
),
630741
callback_handler=None,
631742
)
632743
events = await _collect(_wrap(core))
633744

634-
assert _reported(_usage(events)[0]) == {"input_tokens": 8}
745+
assert _terminal(events).type == EventType.RUN_FINISHED
746+
assert _reported(_usage(events)[0]) == {
747+
"input_tokens": 8,
748+
"output_tokens": 2,
749+
"total_tokens": 10,
750+
}
635751

636752

637753
# ---------------------------------------------------------------------------
@@ -1028,7 +1144,9 @@ async def research(query: str):
10281144
@pytest.mark.asyncio
10291145
async def test_the_metadata_event_is_still_forwarded_as_raw():
10301146
"""Reading usage off this channel must not consume the event."""
1031-
events = await _run_scripted([_turn(usage={"inputTokens": 1})])
1147+
events = await _run_scripted(
1148+
[_turn(usage={"inputTokens": 1, "outputTokens": 1, "totalTokens": 2})]
1149+
)
10321150

10331151
raws = [event for event in events if event.type == EventType.RAW]
10341152
assert any("metadata" in (event.event or {}).get("event", {}) for event in raws)

0 commit comments

Comments
 (0)