[Sonica-B]

Dev Engineer Review

ChatCompletionRequest now declares optional safety_identifier: Optional[str] = None. This allows the request schema to accept the field without changing other fields. The API stability reference records it as optional. The field is accepted for compatibility and is not used for end-user abuse detection.

QA Engineer Review

The new unit test covers accepted string values, JSON input, the None default, invalid value types, unchanged legacy user and other dumped fields, and rejection of an unknown field. The CPU test list includes the test file. Coverage verdict: sufficient for the request-schema change. The PR reports 30 passing local tests; this result was not independently verified. No CI result is provided.

Per-File QA Perspective

• tensorrt_llm/serve/openai_protocol.py — Verify requests with and without safety_identifier validate, and omitted values default to None.
• tests/unittest/llmapi/apps/test_openai_protocol_safety_identifier.py — Covers accepted and rejected inputs and checks that other request fields remain unchanged. Listed in the CPU CI test list.
• tests/integration/test_lists/test-db/l0_cpu.yml — Adds the new unit test to the CPU CI list.
• tests/unittest/api_stability/references/trtllm_serve_api.yaml — Records safety_identifier as an optional ChatCompletionRequest field.

Description

OpenAI's Chat Completions API includes safety_identifier: a stable, hashed end-user ID that OpenAI uses for abuse detection. The OpenAI SDK documents user as "being replaced by safety_identifier and prompt_cache_key", so clients that migrate off user send safety_identifier instead. ChatCompletionRequest inherits extra="forbid" from OpenAIBaseModel and declares user but not safety_identifier, so these requests fail with 400 extra_forbidden (#20962).

This PR declares safety_identifier: Optional[str] = None next to user. Like user, it is accepted for compatibility and otherwise ignored, because the server does no end-user abuse detection. Unknown fields are still rejected, and other fields are unchanged. It is the companion of #20368 (prompt_cache_key, the other replacement for user) and follows the same pattern as #20864 (store).

Why accept and ignore it rather than act on it: the field only carries meaning for OpenAI's own abuse monitoring. Nothing in trtllm-serve keys on an end-user identity (user is ignored the same way). Prefix isolation between tenants is cache_salt's job. vLLM accepts the field (extra fields are allowed and logged as ignored); SGLang's ChatCompletionRequest uses Pydantic's default, which drops unknown fields. OpenAI caps the value at 128 characters. That limit is not enforced here, since the value is unused (it matches how user is declared).

Fixes #20962

Label request: this adds a row to trtllm_serve_api.yaml, so it should carry the api-compatible label, like #20368 and #20864. I can't set labels from a fork; could a maintainer add it?

Test Coverage

• tests/unittest/llmapi/apps/test_openai_protocol_safety_identifier.py (new, cpu_only, registered in l0_cpu.yml): the field is accepted from kwargs and from JSON, defaults to None, rejects non-string values, leaves other fields (including user) unchanged, and unknown fields are still rejected.
• tests/unittest/api_stability/test_serve_api.py: the new ChatCompletionRequest.safety_identifier row in references/trtllm_serve_api.yaml (kind: openai, status: stable, default null, not required).
• Run locally on CPU (Windows, Python 3.11, pydantic 2.14.1). openai_protocol.py was imported directly, with the CUDA-dependent tensorrt_llm package __init__s stubbed, because import tensorrt_llm needs the Linux CUDA build. Results: the new test file, test_openai_protocol_{parallel_tool_calls,tools,mm_processor_kwargs}.py and test_serve_api.py gave 30 passed. Without the one-line model change, 5 of the new tests and test_yaml_fields_exist_on_live_model fail. CI is the authoritative run.

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)

• If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

• 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.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: NVIDIA/TensorRT-LLM
• Branch: main
• File: .coderabbit.yaml
• Commit: 0746a23
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 23537436-1ad2-4a3f-a4d2-030d96bbe176

Commits

Reviewing files that changed from the base of the PR and between c719583 and aec1891.

Files selected for processing: 3

• tensorrt_llm/serve/openai_protocol.py
• tests/integration/test_lists/test-db/l0_cpu.yml
• tests/unittest/api_stability/references/trtllm_serve_api.yaml

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

ChatCompletionRequest now accepts an optional safety_identifier string and ignores it. The change adds CPU tests, updates the API stability reference, and registers the tests in the CPU integration list.

Changes

Chat completion safety identifier

Layer / File(s)	Summary
Request field and validation tensorrt_llm/serve/openai_protocol.py, tests/unittest/api_stability/references/trtllm_serve_api.yaml, tests/unittest/llmapi/apps/test_openai_protocol_safety_identifier.py, tests/integration/test_lists/test-db/l0_cpu.yml	ChatCompletionRequest accepts the optional field and ignores it. The API stability reference marks it as an optional string. CPU tests cover accepted and rejected values, default behavior, and compatibility with user; the CPU integration list registers the tests.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related issues

• [Bug]: trtllm-serve /v1/chat/completions rejects the standard OpenAI field safety_identifier with 400 #20962: Adds safety_identifier to ChatCompletionRequest, addressing the request validation rejection described in the issue.

Possibly related PRs

• NVIDIA/TensorRT-LLM#20864: Adds a similar optional OpenAI compatibility field to ChatCompletionRequest, with API stability updates and CPU tests.
• NVIDIA/TensorRT-LLM#20368: Adds another optional OpenAI compatibility field to ChatCompletionRequest, with API stability coverage and CPU tests.

Suggested labels: api-compatible

Merge Risk: Minimal · up to aec18

The change appears ready to merge after normal CI checks. The suspected CPU test-collection failure is not supported by the inspected setup.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly identifies the fix and the OpenAI safety_identifier field accepted by Chat Completions requests. It is concise, specific, and follows the repository title format.
Description check		The description includes the required Description, Test Coverage, and PR Checklist sections. It explains the issue, solution, compatibility behavior, tests, and requested api-compatible label.
Linked Issues check		[#20962] requires /v1/chat/completions to accept safety_identifier instead of returning extra_forbidden. The supplied PR evidence shows ChatCompletionRequest adds `safety_identifier: Optional[…
Out of Scope Changes check		The supplied PR evidence connects every changed file to [#20962]. The model declaration implements the compatibility field. The new test file verifies the request behavior. The CPU test-list entry ena…
Docstring Coverage		Docstring coverage is inconclusive: files from earlier reviews still need to be rechecked within the check's analysis limits. Not blocking.

Full details: Linked Issues check

Explanation

[#20962] requires /v1/chat/completions to accept safety_identifier instead of returning extra_forbidden. The supplied PR evidence shows ChatCompletionRequest adds safety_identifier: Optional[str] = None. The field is handled like user and is not used for server behavior. The new tests cover keyword and JSON input, the None default, string validation, preservation of user, and rejection of unknown fields. The API reference records the field as optional and stable. The issue does not require a length limit. No unmet coding requirement is identified.

Full details: Out of Scope Changes check

Explanation

The supplied PR evidence connects every changed file to [#20962]. The model declaration implements the compatibility field. The new test file verifies the request behavior. The CPU test-list entry enables the test. The API reference row documents the public field, and the API stability test validates it. No unrelated behavior change is shown. References to companion issues do not add their work to this PR.

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.}