Repository navigation
Add file frames and universal LLM context support for files - #5826
Conversation
| @dataclass | ||
| class UserImageRawFrame(InputImageRawFrame): |
There was a problem hiding this comment.
Would it make sense for UserImageRawFrame to extend UserFileRawFrame? Or are we still handling them differently enough?
There was a problem hiding this comment.
maybe? I think originally I was attempting to not touch the image stuff. i'll look at it.
There was a problem hiding this comment.
ok i/claude took a look and while i agree they feel like they can/should be combined, here's the gotcha:
ImageRawFrame and FileRawFrame are genuinely different. ImageRawFrame is raw bytes + size, while FileRawFrame is an opaque string that encompasses either a URL or base64 data URL.
Claude gave a lengthier answer with more reasoning that i'm not sure i agree with, but i think the big one is the core difference of what is stored. And since ImageRawFrame is pre-existing, re-molding it to work as a File is even trickier.
There was a problem hiding this comment.
I think this is okay for now. Let's save this refactor until all the file support lands, and only do it if we feel we need it.
| if image_appended and frame.run_llm is not False: | ||
| await self.push_context_frame(FrameDirection.UPSTREAM) | ||
|
|
||
| async def _handle_user_file_frame(self, frame: UserFileRawFrame): |
There was a problem hiding this comment.
Claude mentioned this possible issue here:
-
Medium —
src/pipecat/processors/aggregators/llm_response_universal.py:2090_handle_user_file_frameadds the file message to the context before it callspush_aggregation(). If the assistant aggregator is holding text (the bot is partway through a reply when the file arrives), that partial reply is written as anassistantmessage after the user's file message, so the context order is wrong.This also defeats the cleanup.
LLMContext.remove_invalid_file_message()only looks at messages after the last assistant message, so a file that sits before that misplaced assistant message is never removed. If the provider rejects the file, every later completion fails on it — the stuck state the cleanup is meant to prevent.The direct
push_aggregation()call also skips_trigger_assistant_turn_stopped, soon_assistant_turn_stoppedandLLMContextAssistantTurnFrameare never emitted for the text it commits. The image path (_handle_user_image_frame) does not flush the aggregation at all.Suggested fix: flush before appending the file, preferably with
await self._trigger_assistant_turn_stopped()rather than a barepush_aggregation(). Otherwise, drop the flush to match the image handler.
There was a problem hiding this comment.
fixed push_aggregation to now get called before the file load. note: this ordering was also in the image handling, so claude went ahead and fixed it there too. Claude wanted me to tell you this, though, about the _trigger_assistant_turn_stopped() comment and why it refused to make that change 😂
Note on the _trigger_assistant_turn_stopped part of the comment: I deliberately did not close the turn there — closing it mid-stream would make the turn-stopped guard swallow the rest of the reply at LLMFullResponseEndFrame. The plain flush keeps the turn open (remaining text commits at the natural turn end as a second assistant message) and turn events fire once, at the real boundary. Worth saying that in your reply so the "skips turn-stopped" half of the comment is addressed explicitly. New test pins the [assistant, user-file] ordering with in-progress aggregation.
There was a problem hiding this comment.
plain flush keeps the turn open (remaining text commits at the natural turn end as a second assistant message) and turn events fire once, at the real boundary
I think this makes sense. I’m not even sure if it’s possible for us to hit this race condition, where a file is sent while the LLM is replying. But in any case, what we have now feels like enough.
There was a problem hiding this comment.
I added some comments below. I believe even invoking push_aggregation directly here maybe isn’t the right approach, since we only want to add the messages to the context.
| except Exception as e: | ||
| await self.push_error(error_msg=f"Error during completion: {e}", exception=e) | ||
| # OpenAI explicitly flags the invalid file_id parameter. | ||
| if ( | ||
| getattr(e, "type", None) == "invalid_request_error" | ||
| and getattr(e, "param", None) == "file_id" | ||
| ): | ||
| frame.context.remove_invalid_file_message() |
There was a problem hiding this comment.
Claude also found another possible issue here:
-
Medium —
src/pipecat/services/openai/base_llm.py:626The OpenAI service only removes a file after an API error when
e.type == "invalid_request_error"ande.param == "file_id". The OpenAI adapter never sends afile_id. It only sends inline{"type": "file", "file": {"filename", "file_data"}}blocks, so when OpenAI rejects a corrupt, encrypted, or oversized PDF,paramis thefile_datapath or null. The cleanup never runs, and the bad file is resent and fails on every later turn. Unsupported MIME types and file URLs are still cleaned up, because they fail earlier withLLMContextConversionError.Suggested fix: use the same rule as the Anthropic service. Remove the file on
openai.BadRequestError,UnprocessableEntityError, or HTTP 413, or at least also match aparamthat refers to the file content (e.g. one containingfile_data).
There was a problem hiding this comment.
good catch. fixed.
78838e6 to
f281181
Compare
| # Commit any in-progress assistant aggregation before appending, | ||
| # so the image message lands after it in the context. | ||
| await self.push_aggregation() |
There was a problem hiding this comment.
I believe we may need some variation of this where we only add the message to the context.
Because, if I’m not mistaken, push_aggregation will always invoke push_context_frame, which I don’t think is what we want in this case.
The same applies to _handle_user_file_frame.
| # message written before a later-committed assistant message could | ||
| # never be cleaned up if the provider rejects it. The assistant turn | ||
| # stays open; any remaining reply text commits at the turn's end. | ||
| await self.push_aggregation() |
There was a problem hiding this comment.
Same here. We only want to add the in progress assistant aggregation to the context, but not perform the other things that self.push_aggregation does, like invoking self.push_context_frame
filipi87
left a comment
There was a problem hiding this comment.
The only thing missing is the current self.push_aggregation that we’re doing inside _handle_user_image_frame and _handle_user_file_frame. Maybe leave that commented out for now, and we can handle it in the Part 3 PR.
As I mentioned, I believe we may need some variation of this where we only add the message to the context.
Because, if I’m not mistaken, push_aggregation will always invoke push_context_frame, which I don’t think is what we want in this case.
But I’m already leaving my approval here so we can unblock this, merge Parts 1 and 2, and start reviewing Part 3. 🔥
64e0af3 to
e1fc821
Compare
Introduce FileRawFrame, InputFileRawFrame, and UserFileRawFrame, carrying a file payload as a base64 data URL or a URL. LLMContext gains create_file_message(), create_file_url_message(), and add_file_frame_message(), producing universal file_base64 and file_url content items that each adapter (Anthropic, AWS Bedrock, Gemini, OpenAI Chat, OpenAI Responses) translates to its provider format, mirroring the existing image support. The assistant aggregator appends UserFileRawFrame to the context, with run_llm controlling whether it triggers a completion. Unsupported MIME types or source combinations raise during conversion rather than being silently dropped, and LLMContext.remove_invalid_file_message() lets the OpenAI, Anthropic, Bedrock, and Gemini services drop the offending file message when a request is rejected, so the context can't get permanently stuck. The minimum aiohttp version is now 3.14.0 (required by google-genai >= 2.8.0).
1. openai: clean up rejected file messages on payload-shaped API errors The cleanup previously matched an invalid_request_error with param == "file_id", which never occurs: the OpenAI adapter sends files inline as file_data, so a rejected file surfaces as a plain bad-request error and the invalid file message was resent, and failed, on every later turn. Match error classes instead, the same rule as the Anthropic service: bad request (400), unprocessable content (422), and payload too large (413, matched by status code as OpenAI has no dedicated exception class for it) remove the pending file message; the rest of the 4xx range says nothing about whether the file was bad, so it's kept for retry. The tests exercised the old condition with RuntimeErrors carrying bolted-on type/param attributes — which is how they passed while the production check was unreachable — and now build real OpenAI exceptions. 2. aggregator: commit in-progress assistant text before appending a user file A file (or image) arriving while the assistant aggregator holds partial reply text now flushes that aggregation before the user message is appended, so the context keeps chronological order. The order matters beyond chronology: LLMContext.remove_invalid_file_message() only considers messages after the last assistant message, so a file message written before a later-committed assistant message could never be cleaned up when the provider rejects it. The flush is push_aggregation() rather than closing the assistant turn: the turn stays open, any remaining reply text commits at the turn's natural end as a second assistant message, and the turn start/stop events still fire once at the real boundaries.
Committing in-progress assistant text ahead of an arriving user file or image now goes through _add_aggregation_to_context, which writes the context message and nothing else. push_aggregation (unchanged for its existing callers) additionally pushes the downstream LLMContextFrame and assistant-timestamp frame — turn-boundary signals that belong to the turn's real end, not to a mid-turn flush. Routing the flush through it also cleared the push-context-on-bot-stopped-speaking flag as a side effect, which could swallow a pending context push.
e1fc821 to
476a61e
Compare
Part 1 of File Support Work
Part 1
Part 2
Part 3
Part 4
Corresponding Client-JS changes
New File Input Example
Introduce FileRawFrame, InputFileRawFrame, and UserFileRawFrame, carrying a file payload as a base64 data URL or a URL. LLMContext gains create_file_message(), create_file_url_message(), and add_file_frame_message(), producing universal file_base64 and file_url content items that each adapter (Anthropic, AWS Bedrock, Gemini, OpenAI Chat, OpenAI Responses) translates to its provider format, mirroring the existing image support. The assistant aggregator appends UserFileRawFrame to the context, with run_llm controlling whether it triggers a completion.
Unsupported MIME types or source combinations raise during conversion rather than being silently dropped, and
LLMContext.remove_invalid_file_message() lets the OpenAI, Anthropic, Bedrock, and Gemini services drop the offending file message when a request is rejected, so the context can't get permanently stuck.
The minimum aiohttp version is now 3.14.0 (required by google-genai >= 2.8.0).