Repository navigation
fix(responses_id_security): handle a non-streaming response in the streaming iterator hook - #42263
AloysJehwin wants to merge 1 commit into
Conversation
|
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| request_encryption_cache: Final[dict[str, str]] = {} | ||
|
|
||
| async for chunk in response: | ||
| def _encrypt(chunk: Any) -> Any: |
There was a problem hiding this comment.
The helper adds Any parameter and return annotations, violating the repository's strong typing requirement. This requirement must be met before merging
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| class TestStreamingIteratorNonStreamingResponse: | ||
| """A streaming chat request can still hand this hook a single, non-streaming | ||
| ModelResponse — e.g. when an agentic loop (websearch_interception) resolves the | ||
| turn without streaming. That object has no __aiter__, so `async for` used to | ||
| raise `TypeError: 'async for' requires ... got ModelResponse` (#41411). The hook | ||
| must pass such a response through as one item.""" |
There was a problem hiding this comment.
This docstring repeats implementation and failure context, violating the repository rule that source comments must explain complex business logic. Remove it before merging
| class TestStreamingIteratorNonStreamingResponse: | |
| """A streaming chat request can still hand this hook a single, non-streaming | |
| ModelResponse — e.g. when an agentic loop (websearch_interception) resolves the | |
| turn without streaming. That object has no __aiter__, so `async for` used to | |
| raise `TypeError: 'async for' requires ... got ModelResponse` (#41411). The hook | |
| must pass such a response through as one item.""" | |
| class TestStreamingIteratorNonStreamingResponse: |
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…reaming iterator hook async_post_call_streaming_iterator_hook did 'async for chunk in response' unconditionally. A streaming chat request can still hand it a single, non-streaming ModelResponse — e.g. when an agentic loop (websearch_interception) resolves the turn without streaming. That object has no __aiter__, so the hook raised TypeError. Wrap a single non-streaming response as a one-item async stream (_single_chunk) and iterate only when the response is actually async iterable. Fixes BerriAI#41411
3c56d01 to
fbe9219
Compare
This branch has not been deployed
2 participants
The bug (#41411)
ResponsesIDSecurity.async_post_call_streaming_iterator_hookruns:unconditionally. But a streaming chat request can still produce a single, non-streaming
ModelResponse— e.g. whenwebsearch_interceptionruns an agentic loop that resolves the turn without streaming.ModelResponsehas no__aiter__, so the hook raises:Reported on v1.100.0 and v1.101.0, with both an OpenAI-compatible model and
anthropic.claude-haiku-4-5, using a SearXNG search provider.The fix
Guard for the non-async-iterable case: if
responsehas no__aiter__, encrypt it if the responses-id-security rules apply andyieldit as the single item; otherwise iterate as before. The per-chunk encryption logic is unchanged — it's factored into a local_encryptused by both paths, so streaming behaviour and the responses-API id encryption are identical to before.Test
TestStreamingIteratorNonStreamingResponse::test_non_streaming_response_passed_throughfeeds a plainModelResponseto the hook and asserts it comes back as a single item. Verified it fails onmainwith the exactTypeError: 'async for' requires an object with __aiter__ method, got ModelResponse, and passes with the change. The fulltests/test_litellm/test_responses_id_security.pyfile passes (65 passed, 2 pre-existing skips). No new ruff errors introduced (the file's pre-existing B010/SIM103 findings are untouched and out of scope).Fixes #41411