Repository navigation
fix(server): refuse non-finite temperature and top_p - #862
Draft
Avicennasis wants to merge 1 commit into
Draft
Avicennasis wants to merge 1 commit into
Avicennasis wants to merge 1 commit into
Conversation
This branch has not been deployed
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.
Add
allow_inf_nan=Falsetotemperatureandtop_pon both OpenAI request models, matching the existing penalty fields. RawNaN,Infinity,-Infinity, and overflowing1e9999now produce a field-named 422 before generation. Omitted/null fields and finite values keep their previous behavior.The four field constraints alone produced 500 responses: FastAPI includes the rejected float in its validation errors, and Starlette refuses to serialize it. The OpenAI validation handler keeps the 422
detailenvelope and renders non-finite diagnostic values as strings. Anthropic route registration delegates non-Anthropic validation errors to that handler; its own 400 envelope is unchanged.Idea adapted from JustVugg/colibri #2342, commit
3a329791460bdb0efd88d7a56f094e2951faa0b9, and #2370, commitef410a3d80a0e98f465528b5842faac995c44fdc. Both projects use Apache-2.0. No donor code was copied.Verification used Python 3.12.3 and torch 2.14.1+cpu on Linux x86_64, without a GPU or checkpoint. Some declared runtime dependencies were missing from the test environment and were installed locally; no dependency declarations were changed. From the repository root:
export PYTHONPATH=python python -m pytest tests/server/test_openai_api.py -k sampling_routes -q --tb=short python -m pytest tests/server -q --tb=short python -m pytest tests -q --tb=shortThe focused command reproduced 16 failures (HTTP 200), then 32 failures (HTTP 500 with the field-only fix, including combined route registration), then passed all 34 cases with the complete fix; 37 existing cases were deselected. The server suite passed all 690 tests. A local compatibility probe (not part of this change) passed 62 checks on both the base and the branch, covering omitted/null values, finite values outside the usual sampling ranges, and unchanged penalty bounds/non-finite rejection.
Full suite: branch 20 failed / 1699 passed / 919 skipped / 1 warning; base 20 failed / 1665 passed / 919 skipped / 1 warning. The exit code is 1 for both. All 2604 existing test outcomes match, the 20 failing IDs are the same known CPU-only environment failures (CUDA/flashinfer/native-extension tests) on both trees, and all 34 added cases pass. With an unwritable default TVM-FFI cache, both trees show 165 additional failures; pointing
TVM_FFI_CACHE_DIRat a writable directory removes them.NOT VERIFIED: CUDA sampling, real-model generation, checkpoint-provided non-finite defaults, direct engine callers, or non-finite sampling validation on Anthropic/Responses requests. The optional guard in
resolve_sampling.pick()is outside this fix. No performance claim is made.Based directly on
628b88ff795b5ef2c7f9baff088f0e7720d8ebb4. The open PRs #334, #459, #565, #592, #593 and draft #656 share no files with this change; no stacking dependency is needed. No maintainer decision is needed to review the fix.