This is the key part - according to the api docs, we need to access the message field in the non-streaming case.

Good to know. But if that's subtle and very important, consider adding a comment in the code to say just that. (So that the next person stumbling across the code has a hint as to why this needs to be Message instead of Delta.

By adding this here, we now support streaming and non-streaming with the same struct. I suppose it could be a bit cleaner if we were to separate this into two.

I would strongly suggest we separate these two. Since otherwise it's confusing as to what data should/should not be present. (e.g. if you have type fireworksResponse, it isn't clear what is required for it to be well formed.

It also makes maintaining the API surface area easier, since things are more explicit. (Even at the cost of having two types that are nearly identical, just one being called fireworksStreamingResponse or something.

I assume this is what you want, but all of our completion endpoints receive the "max tokens to sample" as a parameter. So if we are "standardizing" completions having a specific LLM model, token count, etc. then perhaps we should move this out into a dedicated function?

e.g. func buildCompletionRequestParams(promptText string) types.CompletionRequestParams?

It just seems like we are hard coding some things here that might better be suited for being package level, e.g. const maxTokensToSample = 2000 or having this isolated to a dedicated function. (Rather than having these all be fixed in generateMetadataForChunk.)

Does that make sense? This isn't a major issue, just a minor code smell. So if none of this resonates with you feel free to ignore.

I understand your concern. However, this is purely experimental for now so I'll leave it as is. If this ends up working well we will likely revisit and refactor this module anyway, otherwise it's going to be deleted soon.

Good to know. But if that's subtle and very important, consider adding a comment in the code to say just that. (So that the next person stumbling across the code has a hint as to why this needs to be Message instead of Delta.

I would strongly suggest we separate these two. Since otherwise it's confusing as to what data should/should not be present. (e.g. if you have type fireworksResponse, it isn't clear what is required for it to be well formed.

It also makes maintaining the API surface area easier, since things are more explicit. (Even at the cost of having two types that are nearly identical, just one being called fireworksStreamingResponse or something.