Skip to content

fix(server): refuse non-finite temperature and top_p - #862

Draft
Avicennasis wants to merge 1 commit into
FlashML-org:mainfrom
Avicennasis:rm52585-temperature-inf-nan
Draft

Avicennasis wants to merge 1 commit into
FlashML-org:mainfrom
Avicennasis:rm52585-temperature-inf-nan

Conversation

@Avicennasis

Copy link
Copy Markdown

Add allow_inf_nan=False to temperature and top_p on both OpenAI request models, matching the existing penalty fields. Raw NaN, Infinity, -Infinity, and overflowing 1e9999 now 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 detail envelope 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, commit ef410a3d80a0e98f465528b5842faac995c44fdc. 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=short

The 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_DIR at 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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant