fix: normalize string stop + surface dropped stream/tool_choice in Converse shim

Review findings on the salvaged shim: (a) OpenAI callers may pass stop as
a bare string but Converse's stopSequences requires a list — normalize;
(b) call_llm(stream=True) (MoA aggregator) can reach this client and the
shim silently returned a complete response — keep that behavior (the
streaming consumer's got-final-object path downgrades gracefully) but log
it, and log dropped tool_choice, instead of silently ignoring both.
+2 regression tests.

Follow-up to the salvage of #60217 by @xxxigm.
This commit is contained in:
kshitijk4poor 2026-07-08 01:29:22 +05:30 committed by kshitij
parent e9da629800
commit b2d6a512d5
2 changed files with 60 additions and 1 deletions

View file

@ -1369,6 +1369,28 @@ class _BedrockCompletionsAdapter:
messages = kwargs.get("messages", [])
model = kwargs.get("model", self._model)
max_tokens = kwargs.get("max_tokens") or kwargs.get("max_completion_tokens")
# OpenAI accepts ``stop`` as str or list; Converse requires a list.
stop = kwargs.get("stop")
if isinstance(stop, str):
stop = [stop]
if kwargs.get("tool_choice") is not None:
# Converse's toolChoice isn't wired through call_converse();
# no in-tree auxiliary caller passes tool_choice today. Surface
# the drop instead of silently ignoring it.
logger.debug(
"BedrockAuxiliaryClient: tool_choice=%r not supported by the "
"Converse shim — ignored.", kwargs.get("tool_choice"),
)
if kwargs.get("stream"):
# Converse streaming isn't wired through this shim. Return a
# complete response instead — call_llm's streaming consumer
# detects a final object and downgrades to non-live output.
logger.debug(
"BedrockAuxiliaryClient: stream=True requested for %s"
"returning a complete response (Converse shim does not "
"stream); caller downgrades to non-streaming.",
model,
)
return call_converse(
region=self._region,
model=model,
@ -1377,7 +1399,7 @@ class _BedrockCompletionsAdapter:
max_tokens=int(max_tokens) if max_tokens else 4096,
temperature=kwargs.get("temperature"),
top_p=kwargs.get("top_p"),
stop_sequences=kwargs.get("stop"),
stop_sequences=stop,
)

View file

@ -643,3 +643,40 @@ class TestAuxiliaryClientBedrockResolution:
)
assert isinstance(client, AsyncBedrockAuxiliaryClient)
def test_bedrock_converse_shim_normalizes_string_stop(self, monkeypatch):
"""OpenAI callers may pass stop='STR'; Converse requires a list."""
monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIAIO...MPLE")
monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")
from agent.auxiliary_client import BedrockAuxiliaryClient
client = BedrockAuxiliaryClient("us-east-1", "openai.gpt-oss-20b-1:0")
with patch("agent.bedrock_adapter.call_converse") as mock_converse:
client.chat.completions.create(
model="openai.gpt-oss-20b-1:0",
messages=[{"role": "user", "content": "hi"}],
stop="STOP",
)
assert mock_converse.call_args.kwargs["stop_sequences"] == ["STOP"]
def test_bedrock_converse_shim_stream_returns_complete_response(self, monkeypatch):
"""stream=True is not supported by the shim — a complete response comes
back and call_llm's streaming consumer downgrades gracefully."""
monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIAIO...MPLE")
monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")
from agent.auxiliary_client import BedrockAuxiliaryClient
client = BedrockAuxiliaryClient("us-east-1", "openai.gpt-oss-20b-1:0")
sentinel = object()
with patch("agent.bedrock_adapter.call_converse", return_value=sentinel) as mock_converse:
resp = client.chat.completions.create(
model="openai.gpt-oss-20b-1:0",
messages=[{"role": "user", "content": "hi"}],
stream=True,
)
# Non-streaming call_converse is still used; the caller's
# got-final-object downgrade path handles the rest.
assert resp is sentinel
assert mock_converse.call_count == 1