[https://nvbugs/6007352][fix] Accept strict field in tools and store field in chat requests#12482
Conversation
…hat requests - Add `strict: Optional[bool]` to `FunctionDefinition` in openai_protocol.py so OpenAI-compatible clients can pass `tools[].function.strict` without getting an HTTP 400 "extra_forbidden" error. - Add `store: Optional[bool]` to `ChatCompletionRequest` so the `store` field is accepted (but not acted on) matching the OpenAI API spec. - When `strict=True`, build structural tag guided decoding params to constrain tool call arguments to the function's JSON Schema via the existing `structural_tag` machinery. - Add 14 unit tests covering the new fields and guided decoding logic. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
|
/bot run |
|
PR_Github #40064 [ run ] triggered by Bot. Commit: |
📝 WalkthroughWalkthroughThis PR adds support for strict tool definitions in the OpenAI protocol by introducing an optional Changes
Sequence DiagramsequenceDiagram
participant Client
participant Server
participant ToolParser
participant GuidedDecoding
participant Generation
Client->>Server: OpenAI chat request with strict tools
activate Server
Server->>Server: Parse request, create sampling_params
alt Strict tools present and no guided_decoding set
Server->>ToolParser: Query tool structure info
activate ToolParser
ToolParser-->>Server: Return structural constraints
deactivate ToolParser
Server->>GuidedDecoding: Build GuidedDecodingParams with structural_tag
activate GuidedDecoding
GuidedDecoding-->>Server: Return configured response format
deactivate GuidedDecoding
Server->>Server: Set sampling_params.guided_decoding
end
Server->>Generation: Execute generation with guided_decoding
activate Generation
Generation-->>Server: Generated response
deactivate Generation
deactivate Server
Server-->>Client: OpenAI response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tensorrt_llm/serve/openai_protocol.py (1)
617-621:⚠️ Potential issue | 🟠 Major
storeis still rejected on/v1/chat/completions.
ChatCompletionRequeststill inheritsextra="forbid"fromOpenAIBaseModel, but it does not declare astorefield. Clients sendingstorewill therefore keep gettingextra_forbidden, so the chat-side compatibility half of this fix is still missing.Suggested fix
class ChatCompletionRequest(OpenAIBaseModel): @@ tools: Optional[List[ChatCompletionToolsParam]] = None tool_choice: Optional[Union[Literal["none", "auto"], ChatCompletionNamedToolChoiceParam]] = "none" + store: Optional[bool] = None user: Optional[str] = NoneAlso applies to: 638-665
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tensorrt_llm/serve/openai_protocol.py` around lines 617 - 621, ChatCompletionRequest (which inherits extra="forbid" from OpenAIBaseModel) is missing a store field so clients sending store get extra_forbidden; add a store: Optional[bool] = None attribute to the ChatCompletionRequest datamodel (and likewise to any other chat request models in the same region referenced in the comment, e.g., the chat-request classes between lines ~638-665) so incoming payloads with "store" are accepted while retaining strict model validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tensorrt_llm/serve/openai_server.py`:
- Around line 146-154: The code currently uses "if tool.function.strict and
tool.function.parameters" which treats falsy parameters (None or {}) as
non-strict; change the logic to honor tool.function.strict regardless of
parameters by checking strict first (if tool.function.strict:) and then set
content = {"type": "json_schema", "json_schema": tool.function.parameters or {}}
so an absent or empty parameters object still produces a strict json_schema;
otherwise (if not strict) fall back to {"type": "any_text"}.
---
Outside diff comments:
In `@tensorrt_llm/serve/openai_protocol.py`:
- Around line 617-621: ChatCompletionRequest (which inherits extra="forbid" from
OpenAIBaseModel) is missing a store field so clients sending store get
extra_forbidden; add a store: Optional[bool] = None attribute to the
ChatCompletionRequest datamodel (and likewise to any other chat request models
in the same region referenced in the comment, e.g., the chat-request classes
between lines ~638-665) so incoming payloads with "store" are accepted while
retaining strict model validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f0cf88db-1b68-47fd-94ee-c14d9681d380
📒 Files selected for processing (3)
tensorrt_llm/serve/openai_protocol.pytensorrt_llm/serve/openai_server.pytests/unittest/llmapi/apps/test_tool_parsers.py
|
PR_Github #40064 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #40218 [ run ] triggered by Bot. Commit: |
|
PR_Github #40218 [ run ] completed with state
|
|
/bot run |
|
PR_Github #40406 [ run ] triggered by Bot. Commit: |
|
PR_Github #40406 [ run ] completed with state
|
|
/bot run |
|
PR_Github #40569 [ run ] triggered by Bot. Commit: |
|
PR_Github #40569 [ run ] completed with state |
|
/bot run |
|
PR_Github #40910 [ run ] triggered by Bot. Commit: |
|
/bot run |
|
PR_Github #41095 [ run ] triggered by Bot. Commit: |
|
PR_Github #41095 [ run ] completed with state
|
|
/bot run |
|
PR_Github #41123 [ run ] triggered by Bot. Commit: |
|
PR_Github #41123 [ run ] completed with state
|
|
/bot run |
|
PR_Github #41139 [ run ] triggered by Bot. Commit: |
|
PR_Github #41139 [ run ] completed with state |
…field in chat requests (NVIDIA#12482) Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
3 participants
strict: Optional[bool]toFunctionDefinitionin openai_protocol.py so OpenAI-compatible clients can passtools[].function.strictwithout getting an HTTP 400 "extra_forbidden" error.strict=True, build structural tag guided decoding params to constrain tool call arguments to the function's JSON Schema via the existingstructural_tagmachinery.Summary by CodeRabbit
Release Notes
New Features
Tests
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.