New Feature: Support sending files - #167
Conversation
fix rebase mangling
…l is publicly accessible
…ts through). - FileReader path strips the data-URL prefix so bytes carries raw base64, uses the normalized MIME type, includes the filename, and properly rejects on read errors (previously an unreadable file hung the promise forever). - sendFile no longer mutates the caller's RTVIFile (spreads a copy). - messages.ts: "hief" → "heif", removed the duplicate "ogg". Rebuilt client-js — your running example picks up the new dist on refresh, and since the local pipecat branch now advertises 2.2.0, local end-to-end still works.
| export type FileBytes = { | ||
| type: Extract<FileSourceType, "bytes">; | ||
| bytes: string; | ||
| width?: number; | ||
| height?: number; | ||
| }; |
There was a problem hiding this comment.
It does feels a bit odd having width and height inside FileBytes. I believe this is needed in case we are sending images, right ?
Maybe do something like this:
type ImageFileBytes = FileBytes & {
width: number;
height: number;
};
or
type FileBytes = {
type: "bytes";
bytes: string;
metadata?: ImageMetadata;
};
type ImageMetadata = {
width: number;
height: number;
};
What do you think ?
There was a problem hiding this comment.
i like it, though, even for images width/height is optional. TBH, maybe they can just go away? I originally had it because I thought it was required in some scenarios. In pipecat, size is required as part of the UserImageRawFrame that gets generated from the message, but as an already encoded image, the value isn't used.
In the RTVI processor, I just default the size to 0x0 if it's not provided. Maybe we remove this as part of the spec and always use 0x0?
There was a problem hiding this comment.
I think it would be nice to provide a way to send the size to keep consistency with the server side API, even if this isn't currently needed for Pipecat.
| export type SendFileOptions = { | ||
| run_immediately?: boolean; | ||
| audio_response?: boolean; | ||
| // for things like 'detail' in openAI or 'citations' in Bedrock |
There was a problem hiding this comment.
this is a TODO to actually support. Right now, these options fall on the floor.
| export type FileSupport = { | ||
| formats: string[]; | ||
| sources: FileSourceType[]; | ||
| maxSize: number; // bytes | ||
| }; |
There was a problem hiding this comment.
It looks like we are not using this anywhere. Should we keep it?
There was a problem hiding this comment.
In my original spec, I wanted a way for clients to get support requirements from the server. I haven't built it out yet. So I dunno. Should I? Should I comment it and leave as a todo? The general idea was so that UIs could pre-filter out files that won't work and avoid unnecessary hefty uploads.
There was a problem hiding this comment.
Should I comment it and leave as a todo?
I think so. At least it would make it clear that we are not supporting it yet.
| // RTVI definition takes the Mime type here, but in client-js, we support | ||
| // clients providing shorthands defined above and we map them to Mime types | ||
| format: string; | ||
| source: FileBytes | FileUrl | FileId; |
There was a problem hiding this comment.
In summary, we're basically sending either the file itself (as bytes), a URL to a file, or the ID of a file that was previously uploaded somewhere. Is that correct ?
I don't see FileId being used anywhere else. Are we planning to support it directly ?
There was a problem hiding this comment.
Correct. Right now I think FileId only works with a runner that supports uploading larger files and accessing them from on disc. However, I did think that it should be spec'd and written in a way that could support the various llm's File APIs. All of which either take an id (anthropic & openai) or are built in a way i'm not sure makes sense using from a client anyway (gemini).
There was a problem hiding this comment.
But for now, i'm explicitly not supporting it. The id must start with pipecat:. Now, whether this is the right prefix or approach to this... not sure. I'd love feedback.
There was a problem hiding this comment.
Agreed. Let's keep it like this for now.
We should probably just document it or create an example in pipecat-examples showing how to create this runner.
| this._botVersion[0] < 1 || | ||
| (this._botVersion[0] === 1 && this._botVersion[1] < 3) |
There was a problem hiding this comment.
Shouldn't this check be like this to use this feature ?
| this._botVersion[0] < 1 || | |
| (this._botVersion[0] === 1 && this._botVersion[1] < 3) | |
| this._botVersion[0] < 2 || (this._botVersion[0] === 2 && this._botVersion[1] < 2) |
| if (estimatedEncodedSize > this._transport.maxMessageSize) { | ||
| uploadFile = file; | ||
| } else { | ||
| return new Promise<void>((resolve) => { |
There was a problem hiding this comment.
Suggestion from Claude:
_The inline-file-read path returns new Promise<void>((resolve) => { ... }) with an async reader.onload handler, and never rejects. Three error paths leave the promise permanently unsettled, so await client.sendFile(...) hangs forever:
- the bare
throw new RTVIErrors.RTVIError("Could not read file data")whene.target?.resultis missing (client.ts:1211) rejects only the handler's own internal promise, whichFileReadernever awaits; await sendFileMessage()(client.ts:1223) can reject —_sendMessagethrowsMessageTooLargeErrorwhenmessageSizeWithinLimitfails (client.ts:806), which is reachable because thefile.size * 1.37branch decision is only a heuristic;- no
reader.onerroris registered, so anyFileReaderfailure never settles the promise either.
Suggested fix: wrap the onload body in try/catch and call reject(err) on failure; add reader.onerror = (err) => reject(err)._
There was a problem hiding this comment.
i believe this was resolved when I did the refactor.
| | "webp" | ||
| | "gif" | ||
| | "heic" | ||
| | "hief"; |
There was a problem hiding this comment.
Typo: I believe it should be heif.
| webp: "image/webp", | ||
| gif: "image/gif", | ||
| heic: "image/heic", | ||
| hief: "image/heif", |
| } | ||
|
|
||
| @transportReady | ||
| public async sendFile( |
There was a problem hiding this comment.
This function is a bit hard to follow. It is doing three things at once: normalizing the input, deciding inline vs. upload, and sending.
Would it be worth restructuring it ? Maybe something like:
public async sendFile(file: RTVIFile | File, content: string, options: SendFileOptions = {}) {
this.assertBotSupportsSendFile();
const resolved = file instanceof File
? await this.resolveBrowserFile(file)
: await this.resolveRTVIFile(file);
await this._sendMessage(
new RTVIMessage(RTVIMessageType.SEND_FILE, { file: resolved, content, options })
);
}
/** Small files go inline as base64; anything larger is uploaded first. */
private async resolveBrowserFile(file: File): Promise<RTVIFile> {
if (this.exceedsMessageLimit(estimateBase64Size(file.size))) {
return this.uploadFile(file);
}
return {
name: file.name,
format: file.type,
source: { type: "bytes", bytes: await readAsBase64(file) },
};
}
/** Only inline byte payloads need resolving — url and id sources are already remote. */
private async resolveRTVIFile(file: RTVIFile): Promise<RTVIFile> {
const normalized = { ...file, format: toMimeType(file.format) };
if (normalized.source.type !== "bytes") return normalized;
if (!this.exceedsMessageLimit(normalized.source.bytes.length)) return normalized;
return this.uploadFile(toUploadableFile(normalized));
}
- Version gate checks protocol < 2.2 (was < 1.3, which let 2.0/2.1 bo…
- sendFile split into _assertBotSupportsSendFile, _resolveBrowserFile, and _resolveRTVIFile helpers so each concern is separate and the public method reads linearly.
- FileBytes no longer carries width/height; new ImageFileBytes = FileBytes & { width?, height? } for callers that need image dimensions without changing the wire format.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
filipi87
left a comment
There was a problem hiding this comment.
LGTM!
Great to see this API out! 🚀
|
Note: I don't think I should merge this until the Pipecat changes are in and released. |
sendFile()method