server : add bad input handling in embeddings#10866
server : add bad input handling in embeddings#10866krystiancha wants to merge 1 commit intoggml-org:masterfrom
Conversation
| // with "content", we only support single prompt | ||
| if (!oaicompat && prompt.type() != json::value_t::string) { | ||
| res_error(res, format_error_response("\"content\" must be a string", ERROR_TYPE_INVALID_REQUEST)); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
I wonder what is the reason to apply this restriction on the "content" field? Why don't we treat it as an alias to "input" and allow the same inputs for it? cc @ngxson
There was a problem hiding this comment.
Hmm I have no idea why, seems like remnant from the past. It makes more sense to consider content as an alias of input as you said.
| std::vector<server_task> tasks; | ||
| std::vector<llama_tokens> tokenized_prompts = tokenize_input_prompts(ctx_server.ctx, prompt, /* add_special */ false, true); | ||
| for (size_t i = 0; i < tokenized_prompts.size(); i++) { | ||
| if (tokenized_prompts[i].size() == 0) { |
There was a problem hiding this comment.
This test should not be here. Also, empty string is still a valid input.
Please remove this check
There was a problem hiding this comment.
Hmm ok sorry seems like OAI embedding does not accept empty string either. So this check is relevant, it's just not in the correct place.
Ref: https://platform.openai.com/docs/api-reference/embeddings/create
There was a problem hiding this comment.
Should be fixed in my PR and it won't crash if the input is empty. We're now adding BOS token to the sequence.
This patch improves the behavior of the embedding endpoint with bad input.