Fall back on guessed MIME type when requested multi-modal content doesn't have Content-Type header

[kylesf]

@Kludex I needed to add during testing of agents I built. I am not sure why tests did not catch the issue and whats the best long term approach for the project. Please share your thoughts.

[kylesf]

I did see difference in how bed rock is handling audio, video etc compared to that of other models.

[kylesf]

@DouweM Thoughts here?

[DouweM]

@kylesf Thanks! We'll want to address the issue of Content-Type not containing a useful value everywhere we currently use it, not just for Gemini. The fallback logic should be centralized in {Image,Video,Audio,Document}Url.media_type so all models can use it. The one on DocumentUrl already uses the same guess_type function you're using here, which likely makes sense for the other types as well. Can you have a look at implementing this in that more generic way, please?

[kylesf]

idk lot of things going on. guessing type is not ideal if we have fixed supported types. i.e. VideoMediaType - at the same time we have a test case like image_url = ImageUrl(url='https://goo.gle/instrument-img') where we need to do web req to get our type. prob needs more thinking.

[kylesf]

i do think this change allows better support as if we do req and that info is not given up, then we can use the extension.

[kylesf]

@DouweM thoughts?

[DouweM]

@kylesf Falling back on media_type if there's no Content-Type header makes sense. Can you please see if we can also fix that wherever else we currently use Content-Type (also written lowercase in some places)?

[kylesf]

@DouweM got them all I believe

[DouweM]

@kylesf Thank you! The code looks good, but CI is complaining about coverage. To prevent having to add a test of this for each model, I suggest moving this logic to a helper function, and then adding a single test for this scenario that passes through that function.

[kylesf]

@DouweM - in my attempts to do this, I have not been happy with the resulting code, more complex and not as simple to implement. I will probably leave it here, for the team to decide what to do. I will just monkey patch on my side for now. Thanks!

[DouweM]

@kylesf I had a chance to implement it, let's see if the test are happy :)

[Kludex]

I think checking the header may be wrong, don't merge this yet please

[DouweM]

@Kludex I've marked this as a draft so I don't accidentally merge it once the tests go green 😄

When would checking the header be wrong? The URL won't always have an extension, and browsers prioritize Content-Type.

[Kludex]

When would checking the header be wrong? The URL won't always have an extension, and browsers prioritize Content-Type.

Sometimes it can be application/octet-stream... 😞

[DouweM]

@Kludex Aargh, you're right! So I think we can just ignore that value and fall back on item.media_type in that case? Or do you foresee other issues?

[DouweM]

This has been integrated into #1136.