From b2d6a512d5778b6fdf2445c3d674c63060d7b109 Mon Sep 17 00:00:00 2001
From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com>
Date: Wed, 8 Jul 2026 01:29:22 +0530
Subject: [PATCH] fix: normalize string stop + surface dropped
 stream/tool_choice in Converse shim
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 agent/auxiliary_client.py               | 24 +++++++++++++++-
 tests/agent/test_bedrock_integration.py | 37 +++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py
index b53a7520ee2..d55f7df4fcd 100644
--- a/agent/auxiliary_client.py
+++ b/agent/auxiliary_client.py
@@ -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,
         )
 
 
diff --git a/tests/agent/test_bedrock_integration.py b/tests/agent/test_bedrock_integration.py
index a7593bd9240..d8840bc979e 100644
--- a/tests/agent/test_bedrock_integration.py
+++ b/tests/agent/test_bedrock_integration.py
@@ -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