I think we can also replace these 2 fields with completion_token_output. Then inside send_partial_response, we can std::move it to res

P/s: we cannot std::move it, because inside process_token, result is still being used after send_partial_response

I'm not really sure that the "return_tokens" logic is necessary. The tokens array should be similar in JSON length to the content string, though I am not sure performance wise how much slower it is to serialize an array of integers compared to a string. Anyway, I've added the flag and added tests.

Note that with "stream": true we always return the tokens field in the partial responses (i.e. this is not affected by the "return_tokens" flag).

What I'm thinking is that this should not degrade the performance of JSON serializing/parsing. But I'm just thinking about the bandwidth, because it seems like in most cases, we're now using double the bandwidth.

For stream, I don't think it's a problem because time to serialize/send/receive/parse is minor compared to the time a token is generated.

But I think for now we can keep it this way. The non-OAI /completion is a playground anw so it's fine to expose everything. The OAI compat /v1/completions that I'm planning to do next will be more prod-ready, thus it won't have these data in the response.

Edit: I didn't notice that you implemented return_tokens, that's good then, let's keep it 👍

But I think for now we can keep it this way. The non-OAI /completion is a playground anw so it's fine to expose everything. The OAI compat /v1/completions that I'm planning to do next will be more prod-ready, thus it won't have these data in the response.

Yes, I agree we can keep /v1/completions strongly OAI-compat (i.e. not even have extra fields like tokens) and only have these in the non-OAI endpoints like /completions.