Conversation
Signed-off-by: JiaxinD <djx2048@gmail.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
Signed-off-by: JiaxinD <djx2048@gmail.com>
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.
Background
HTTP generation requests can pass invalid inputs into later processing:
temperature: 1e400becomes infinity during JSON decoding, while a prompt containing an escaped lone Unicode surrogate raisesUnicodeEncodeErrorduring UTF-8 length calculation. Both should receive HTTP 400 before worker acquisition. Found while reviewing the server request path; no originating issue.Exit Criteria
Both completion routes reject nonfinite temperatures and surrogate text before acquiring a worker. Subsequent requests with zero temperature and valid Unicode, including emoji, remain valid.
Implementation
Set
allow_inf_nan=Falseon the shared temperature field. Add an inherited string-field validator toStrictRequestthat verifies UTF-8 encodability and emits a constant validation message, covering completion prompts, chat content and nested text parts. Saturated fake-registry regressions detect unintended worker acquisition. No native protocol, ABI, bundle or dependency changes.Change categories
Validation
Commands and Results
With
PYTHONPATH=apps/server/python;core/builder;.on Windows:python -m pytest apps/server/python/tests/test_app.py -k nonfinite -q -p no:cacheprovider: before the temperature fix, all six new cases failed (429 instead of 400).UnicodeEncodeErrorduring UTF-8 length calculation. The eight added cases cover high/low surrogates in completion prompts, chat user/system content and nested text parts.python -m pytest apps/server/python/tests -q -p no:cacheprovider: after both fixes, 24 passed.ruff check apps/server/python/trtmc_server/schemas.py apps/server/python/tests/test_app.py: passed.PYTHONPATH=core/builder;apps/benchmark;.:python -m tools.model_ci validateandpython tools/test_impact.py --validate: passed.git diff --check: passed. Independent read-only review found no actionable findings.Hardware, Environment, and Revisions
CPU-only Windows, Python 3.13.2, FastAPI 0.139.0, Pydantic 2.12.5. Tested source committed as 128fd03, based on upstream 613bbf0. No model weights or datasets involved.
Not Run / Remaining Gaps
No native worker process, TensorRT engine, GPU inference or throughput test was run. HTTP tests use the existing fake registry/session; they prove boundary rejection and subsequent valid-request routing, not hardware behavior.
Contributor Self-Review
Notes For Future Readers
This HTTP-boundary fix is independent of #1417, which handles numeric overflow in direct native JSONL input. Native validation remains necessary for direct callers. Review-ready within scope; Draft remains solely because the repository's three non-draft slots are occupied.
Risk level
Invalid inputs are rejected earlier. Finite temperatures, defaults and valid Unicode retain their behavior. The shared validator also covers string fields such as model and role. No artifact rebuild is needed beyond deploying the server Python change.