Repository navigation
[#20367][fix] Accept prompt_cache_key in Chat Completions requests - #20368
lucifer1004 wants to merge 6 commits into
Conversation
ChatCompletionRequest forbids unknown fields, so requests carrying OpenAI's standard prompt_cache_key were rejected with 400. Accept the field and ignore it, like user and parallel_tool_calls. Signed-off-by: Zihua Wu <13583761+lucifer1004@users.noreply.github.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Add and validate the request field tensorrt_llm/serve/openai_protocol.py, tests/unittest/api_stability/references/trtllm_serve_api.yaml, tests/unittest/llmapi/apps/test_openai_protocol_prompt_cache_key.py, tests/integration/test_lists/test-db/l0_cpu.yml |
ChatCompletionRequest accepts the optional prompt_cache_key field and ignores it. The API stability reference records the field as optional and stable. CPU tests cover valid strings, the default value, JSON parsing, unchanged other fields, and rejection of unknown fields and non-string values. The CPU test list includes the new test module. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
- [Bug]: trtllm-serve /v1/chat/completions rejects the standard OpenAI field safety_identifier with 400 #20962: Reports the same
ChatCompletionRequestextra-field rejection for another standard OpenAI field and identifiesprompt_cache_keysupport as a counterpart change.
Possibly related PRs
- NVIDIA/TensorRT-LLM#20864: Adds an optional ignored
storefield toChatCompletionRequestusing the same schema, stability-reference, and test pattern. - NVIDIA/TensorRT-LLM#20963: Adds an optional ignored
safety_identifierfield toChatCompletionRequestusing the same schema, stability-reference, and validation-test pattern.
Merge Risk: 
Minimal · up to fb68c
-
No actionable merge-blocking risk remains for this compatibility change.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Linked Issues check
-
Explanation
Issue
#20367is active and directly linked.ChatCompletionRequestaddsprompt_cache_key: Optional[str] = Noneand retainsextra="forbid". The change accepts and ignores the OpenAI field while preserving rejection of other unknown fields. The added tests cover keyword and JSON input, the default, non-string rejection, unchanged fields, and unknown-field rejection. The API reference entry and CPU test registration are included.
Full details: Out of Scope Changes check
-
Explanation
The source change directly addresses issue
#20367. The tests verify the required compatibility and validation behavior. The CPU test registration and stable API reference entry support the same public API change. The supplied whole-PR summary shows no unrelated changes.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
/bot run |
|
PR_Github #78107 [ run ] triggered by Bot. Commit: |
|
PR_Github #78107 [ run ] completed with state
|
mikeiovine
left a comment
There was a problem hiding this comment.
Stamp on behalf of runtime devs, delegating proper review to @NVIDIA/trt-llm-noncommitted-api-review-committee
|
/bot run |
|
PR_Github #78261 [ run ] triggered by Bot. Commit: |
|
PR_Github #78261 [ run ] completed with state
|
Signed-off-by: Zihua Wu <13583761+lucifer1004@users.noreply.github.com>
|
The last CI run failed only on tests that this PR does not touch. |
|
/bot run |
|
PR_Github #78355 [ run ] triggered by Bot. Commit: |
|
PR_Github #78355 [ run ] completed with state
|
|
CI #78355 failed outside this PR. The CPU failure is |
Signed-off-by: Zihua Wu <13583761+lucifer1004@users.noreply.github.com>
|
CI #78860 failed outside this PR:
Rerunning. |
|
/bot run |
|
PR_Github #78926 [ run ] triggered by Bot. Commit: |
|
Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements. |
|
PR_Github #78926 [ run ] completed with state |
Signed-off-by: Zihua Wu <13583761+lucifer1004@users.noreply.github.com>
|
/bot run |
|
PR_Github #80461 [ run ] triggered by Bot. Commit: |
|
PR_Github #80461 [ run ] completed with state
|
|
/bot run |
|
PR_Github #80474 [ run ] triggered by Bot. Commit: |
|
PR_Github #80474 [ run ] completed with state |
Signed-off-by: Zihua Wu <13583761+lucifer1004@users.noreply.github.com>
|
/bot run |
|
PR_Github #80549 [ run ] triggered by Bot. Commit: |
|
PR_Github #80549 [ run ] completed with state
|
|
CI #80549 failed for reasons unrelated to this PR (it only adds
This PR's |
|
/bot run |
|
PR_Github #80607 [ run ] triggered by Bot. Commit: |
|
PR_Github #80607 [ run ] completed with state |
Description
Fixes #20367.
ChatCompletionRequestisextra="forbid", so a request carrying OpenAI's standardprompt_cache_keyfield was rejected with HTTP 400 (extra_forbidden). This PR adds the field asOptional[str] = None, accepted and otherwise ignored, likeuserandparallel_tool_calls: KV cache reuse needs no client hint, andcache_saltremains the field that isolates cached prefixes.ResponsesRequestalready accepts the field throughextra="allow".Other unknown fields are still rejected, and non-string values of
prompt_cache_keyfail validation.tensorrt_llm/serve/openai_protocol.py: new field onChatCompletionRequest.tests/unittest/api_stability/references/trtllm_serve_api.yaml: the matching reference row (kind: openai,status: stable).Test Coverage
tests/unittest/llmapi/apps/test_openai_protocol_prompt_cache_key.py(CPU only), registered inl0_cpu.yml: accepted as keyword and from JSON, defaultNone, non-string values rejected, other fields unchanged by it, unknown fields still rejected.tests/unittest/api_stability/test_serve_api.pycovers the reference row.PR Checklist
Dev Engineer Review
ChatCompletionRequestadds optionalprompt_cache_key: str, defaulting toNone, so chat-completion requests can include this OpenAI field without changing other request data. The field is accepted for compatibility and ignored; other unknown fields remain rejected. The stable API reference records the field as optional.QA Engineer Review
The new CPU-only test file covers string values, including an empty string; the
Nonedefault; JSON input; invalid types; unchanged fields; and rejection of unknown fields. The test is listed intests/integration/test_lists/test-db/l0_cpu.yml. CI comments report that the new test passed, and a later pipeline completed successfully. The tests cover model validation but not endpoint-level serving behavior. Coverage: needs follow-up for endpoint-level behavior.Per-File QA Perspective
tensorrt_llm/serve/openai_protocol.py: Verify chat-completion requests accept and ignoreprompt_cache_key, default it toNone, and continue to reject invalid types and other unknown fields.tests/unittest/llmapi/apps/test_openai_protocol_prompt_cache_key.py: Covers accepted values, defaults, JSON input, invalid types, unchanged fields, and unknown-field rejection. Listed in the CPU CI test list.tests/integration/test_lists/test-db/l0_cpu.yml: Adds the new CPU-only test to the CI list.tests/unittest/api_stability/references/trtllm_serve_api.yaml: Addsprompt_cache_keyas a stable optional API field. Verify that the reference stays consistent with the request model.