Add an optional mimeType property to TextContent #662
Conversation
| */ | ||
| arguments?: { [key: string]: string }; | ||
| }; | ||
| /** |
There was a problem hiding this comment.
Sorry, my IDE is fixing formatting in unrelated places. But I think we should keep them.
hesreallyhim
left a comment
hesreallyhim
left a comment
There was a problem hiding this comment.
suggestion: it sounds like we're going to end up with three ContentTypes:
TextContent | ImageContent | AudioContent
and they're each going to have type, mimeType, and annotations? properties, which sounds to me like a good opportunity for a shared interface from which to extend
|
@hesreallyhim Ah, actually this is a small patch before we potentially rock the boat and unify types: #180 (reply in thread) |
ok, thanks for the context 👍 |
|
I'd prefer this was fixed to |
|
@evalstate can you elaborate? What does "fixing" mean in this context? Assume all |
|
Yes, my view is that TextContent should have the guarantee that it is
The potential breakage here is that Server developers think they want to send There are rare exceptions (e.g. models like StarVector) but the general case is that having a modifiable mime type on TextContent this will cause unnecessary complexity and breakages. At the moment |
|
LGTM. @atesgoral - mind resolving the schema conflicts in this PR? |
5 participants
Motivation and Context
Addresses #180
Where @jspahrsummers said:
How Has This Been Tested?
No. Just adding an optional property to the schema.
Breaking Changes
No.
Types of changes
Checklist
Additional context