Repository navigation
fix(server): refuse n != 1 on /v1/completions instead of ignoring it - #842
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #682
/v1/completions now answers n != 1 with a 400, the same refusal /v1/chat/completions already gives, instead of a 200 with one choice. No change for n = 1.
Summary
CompletionRequest declares n, but handle_completion never read it: n=2 got HTTP 200 and a single choice, so a client asking for several samples (best-of selection, self-consistency, eval harnesses) silently got one and had no way to tell. Chat already refuses the field with 400 "Only n=1 is supported".
{"error": {"message": "Only n=1 is supported", "type": "invalid_request_error", "param": "n", "code": null}}Why now: a silent wrong answer is worse than a refusal, and this is the smallest change that makes the two routes agree. Serving n > 1 on completions is a separate, larger change in a follow-up PR.
Tests
tests/server/test_openai_api.py::test_completion_refuses_n_other_than_one, parametrized over n in {0, 2} and stream on and off: the response is a 400 with param "n", and nothing is sent to the engine (state.sent is None). With main's handle_completion, all four cases fail.
Test environment
Unit tests
This branch: 15 passed, 17 failed, 1 error. Main: 11 passed, 17 failed, 1 error. The 4 extra passes are the new test; with only the test file from this branch, main fails all 4. The 17 failures and the error are all "ModuleNotFoundError: No module named 'flashlib'", which needs CUDA, and are the same on main.
End-to-end A/B against main
Not run yet: it needs a GPU, which I don't have access to yet. Once I do, I will run it and add the results here; if a maintainer or reviewer has one sooner, the repro attached to #682 should show HTTP 400 and PASS on the four completions rows on this branch, and HTTP 200 with one choice (FAIL) on main: