fix(adk): handle empty choices and zero-content streaming chunks from guardrail-blocked responses#1993
Conversation
combrink
requested review from
EItanya,
iplay88keys,
jmhbh,
peterj,
supreme-gg-gg and
yuval-k
as code owners
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds guardrail-safe handling for OpenAI responses/streams that contain no content (e.g., fully filtered), preventing IndexError and returning a SAFETY finish reason with a placeholder message.
Changes:
- Return a SAFETY
LlmResponsewhenChatCompletion.choicesis empty. - Ensure streaming always emits a final response even when no content/tool parts were produced.
- Add unit tests covering both empty-choices and “no content chunks” streaming cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| python/packages/kagent-adk/src/kagent/adk/models/_openai.py | Adds fallback logic for empty choices and empty streaming parts to avoid IndexError and mark responses as SAFETY. |
| python/packages/kagent-adk/tests/unittests/models/test_openai.py | Adds tests to ensure empty/filtered responses don’t crash and produce expected SAFETY output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not response.choices: | ||
| return LlmResponse( | ||
| content=types.Content( | ||
| role="model", | ||
| parts=[types.Part.from_text(text="Response blocked by content policy.")], | ||
| ), | ||
| finish_reason=types.FinishReason.SAFETY, | ||
| ) |
There was a problem hiding this comment.
Fixed in f49c71d — usage extraction is now hoisted above the empty-choices guard and included in the early return.
| # Guardrail or content filter can produce zero content/tool chunks. | ||
| # An empty parts list causes downstream IndexError; emit a placeholder. | ||
| if not final_parts: | ||
| final_parts.append(types.Part.from_text(text="Response blocked by content policy.")) | ||
| final_reason = types.FinishReason.SAFETY |
There was a problem hiding this comment.
Fixed in f49c71d — an empty stream that finished with length now keeps MAX_TOKENS (with an empty text part to avoid the IndexError) instead of being relabeled SAFETY. Note we intentionally don't narrow the SAFETY fallback to only finish_reason == "content_filter": the motivating case is Bedrock guardrails via agentgateway, which can block without setting that finish reason.
| return LlmResponse( | ||
| content=types.Content( | ||
| role="model", | ||
| parts=[types.Part.from_text(text="Response blocked by content policy.")], |
There was a problem hiding this comment.
Fixed in f49c71d — extracted to a module-level _CONTENT_BLOCKED_PLACEHOLDER constant used by both paths.
| # Guardrail or content filter can produce zero content/tool chunks. | ||
| # An empty parts list causes downstream IndexError; emit a placeholder. | ||
| if not final_parts: | ||
| final_parts.append(types.Part.from_text(text="Response blocked by content policy.")) |
| class MockDelta: | ||
| role = "assistant" | ||
| tool_calls = None | ||
| content = None | ||
|
|
||
| class MockChunkChoice: | ||
| def __init__(self, finish_reason=None): | ||
| self.delta = MockDelta() | ||
| self.finish_reason = finish_reason | ||
| self.index = 0 | ||
|
|
||
| class MockChunk: | ||
| id = "chatcmpl-test" | ||
| created = 1234567890 | ||
| model = "gpt-3.5-turbo" | ||
| object = "chat.completion.chunk" | ||
| usage = None | ||
|
|
||
| def __init__(self, finish_reason=None): | ||
| self.choices = [MockChunkChoice(finish_reason)] |
There was a problem hiding this comment.
Leaving as-is: the existing streaming tests in this module (e.g. test_streaming_usage_metadata_propagation) already define per-test mock chunks, so this follows the established local style. Happy to extract a shared fixture in a follow-up if maintainers prefer.
| # Guardrail or content filter can produce zero content/tool chunks. | ||
| # An empty parts list causes downstream IndexError; emit a placeholder. | ||
| if not final_parts: | ||
| if final_reason == types.FinishReason.MAX_TOKENS: | ||
| # Truncated by length before any content; not a safety block. | ||
| final_parts.append(types.Part.from_text(text="")) | ||
| else: | ||
| final_parts.append(types.Part.from_text(text=_CONTENT_BLOCKED_PLACEHOLDER)) | ||
| final_reason = types.FinishReason.SAFETY |
There was a problem hiding this comment.
Keeping the broad fallback intentionally: the motivating case is AWS Bedrock guardrails (via agentgateway), which block responses without setting finish_reason == "content_filter" — narrowing to an explicit filter signal would reintroduce the original crash for exactly the case this PR fixes. A stop finish with literally zero content and zero tool calls does not occur in normal operation, so treating it as a filtered response is the safer default. MAX_TOKENS is preserved since that one has a legitimate empty-content interpretation.
| class MockDelta: | ||
| role = "assistant" | ||
| tool_calls = None | ||
| content = None |
There was a problem hiding this comment.
Done in 77a0fa0 — renamed to EmptyContentDelta/EmptyContentChunkChoice/EmptyContentChunk.
… guardrail-blocked responses When an OpenAI-compatible content filter (e.g. an AWS Bedrock guardrail) blocks a response, it can return a ChatCompletion with an empty choices list, or a stream where every chunk is filtered out. Both previously caused an IndexError or an empty-parts response. Return a SAFETY finish reason with a placeholder part instead. Signed-off-by: Combrink van der Vyver <combrink@gmail.com>
- Preserve usage_metadata when choices is empty - Keep MAX_TOKENS finish reason when an empty stream was truncated by length rather than blocked by a content filter - Centralize the blocked-content placeholder in a module constant Signed-off-by: Combrink van der Vyver <combrink@gmail.com>
combrink
force-pushed
the
fix/openai-empty-choices-guardrail
branch
from
f49c71d to
834673e
Compare
Signed-off-by: Combrink van der Vyver <combrink@gmail.com>
Summary
IndexErrorwhen an OpenAI-compatible response has an emptychoiceslist (e.g. an AWS Bedrock guardrail blocks the response entirely).partslist which causes downstreamIndexErrors.LlmResponsewithfinish_reason=FinishReason.SAFETYand a placeholder text part, so callers get a recoverable signal instead of a crash.Motivation / Context
This was triggered by Bedrock guardrail activations on
eu.anthropic.claude-sonnet-4-6via the agentgateway integration. The guardrail correctly blocked content, but the unhandled empty response caused the agent runtime to crash rather than surface aSAFETYfinish reason.Changes
_convert_openai_response_to_llm_response(non-streaming): guard againstresponse.choicesbeing empty before indexing into it.BaseOpenAIstreaming loop: after consuming all chunks, iffinal_partsis empty, append a placeholder part and setfinal_reason = FinishReason.SAFETY.Test plan
test_empty_choices_returns_safety_finish_reasoncovering the non-streaming empty-choicescase.test_streaming_with_no_content_chunks_returns_safety_finish_reasoncovering the streaming all-filtered-chunks case.uv run pytest tests/unittests/models/test_openai.py— all 28 tests pass.ruff checkpasses on changed files.