[https://nvbugs/6265490][fix] Defer LoRA path validation

[yibinl-nvidia]

Dev Engineer Review

• Defers lora_path existence validation from LoRARequest.__post_init__ to adapter loading.
• Raises a sanitized FileNotFoundError when the requested LoRA adapter path is missing.
• Preserves lora_ckpt_source validation.
• Returns generic validation errors for non-visual-gen and disaggregated endpoints.
• Preserves visual-gen validation responses and error metrics.
• No configuration or test-list files changed.

QA Engineer Review

• Added a regression test for nonexistent LoRA paths in tests/unittest/executor/test_base_worker.py.
• Updated validation tests in tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py.
• No corresponding entries were identified in tests/integration/test_lists/.
• Verdict: needs follow-up.

Description

This PR defers LoRA path existence checks until adapter loading so request construction does not leak local filesystem path information through validation errors. Missing adapter paths now fail during _load_lora_adapter with a sanitized FileNotFoundError.

It also keeps OpenAI serving request-validation responses generic for non-visual-gen and disaggregated endpoints, so malformed requests do not echo sensitive request data back to clients.

Test Coverage

• tests/unittest/executor/test_base_worker.py::test_lora_request_missing_path_fails_at_load_time
• tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py::TestNonVisualGenValidationResponse
• Local pre-commit run --show-diff-on-failure --verbose --files ... on the six PR files passed after the watcher fix.
• Local targeted pytest was attempted but collection is blocked in this checkout by missing transformers.

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.

[yibinl-nvidia]

/bot run

[tensorrt-cicd]

PR_Github #63109 [ run ] triggered by Bot. Commit: d46a4f9 Link to invocation

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8ce09e94-186f-4b0a-9bbe-ae896f2b82b3

📥 Commits

Reviewing files that changed from the base of the PR and between e289c89 and 940bdca.

📒 Files selected for processing (2)

• tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py
• tests/unittest/executor/test_base_worker.py

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py
• tests/unittest/executor/test_base_worker.py

---

Walkthrough

The change moves missing LoRA path checks to adapter loading and sanitizes validation error responses in OpenAI servers. Tests cover early LoRA failure and prevention of sensitive request data in error messages.

Changes

Validation and error handling

Layer / File(s)	Summary
LoRA path validation tensorrt_llm/executor/base_worker.py, tensorrt_llm/executor/request.py, tests/unittest/executor/test_base_worker.py	LoRARequest no longer checks path existence during construction. _load_lora_adapter raises FileNotFoundError before loading a missing adapter. Tests verify the error and loading order.
Generic validation errors tensorrt_llm/serve/openai_disagg_server.py, tensorrt_llm/serve/openai_server.py, tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py	Validation handlers return "Request validation failed" without exception details. Tests verify that private-looking request data is not returned.

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

Suggested reviewers: allisonlim-nv

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows the required format and clearly describes the primary change: deferring LoRA path validation.
Description check	✅ Passed	The description explains the problem and solution, lists relevant tests, records test limitations, and includes the required checklist.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py`:
- Line 1505: Update the test’s local _handler to exercise the production handler
registered by OpenAIServer.__init__ or OpenAIDisaggServer.__init__ instead of
returning a hard-coded JSONResponse. Invoke the production app or reuse a shared
production handler so the assertion fails if either handler regresses to
returning str(exc).
- Around line 1533-1538: Update the response validation in the affected test to
assert the complete JSON body equals the expected validation-error payload,
rather than checking only body["error"]. Preserve the status-code assertion and
ensure the expected full-body comparison verifies that secret is absent from
every response field.

In `@tests/unittest/executor/test_base_worker.py`:
- Around line 61-64: Initialize the cleanup fields required by
BaseWorker.__del__ in this object.__new__(BaseWorker) test setup: set
doing_shutdown and engine to the same safe defaults expected after BaseWorker
initialization, while preserving the existing LoRA and model configuration
fields.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: da640f9b-0a4e-454c-8d0e-13409f160484

📥 Commits

Reviewing files that changed from the base of the PR and between 7443b7f and d46a4f9.

📒 Files selected for processing (6)

• tensorrt_llm/executor/base_worker.py
• tensorrt_llm/executor/request.py
• tensorrt_llm/serve/openai_disagg_server.py
• tensorrt_llm/serve/openai_server.py
• tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py
• tests/unittest/executor/test_base_worker.py

💤 Files with no reviewable changes (1)

• tensorrt_llm/executor/request.py

[yibinl-nvidia]

/bot kill

[coderabbitai]

🧹 Nitpick comments (2)

tensorrt_llm/serve/openai_server.py (1)

769-780: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add type annotations to the new exception handler.

validation_exception_handler has untyped parameters and no return annotation. Add _: Request, exc: RequestValidationError, and -> Response.

As per coding guidelines, annotate every function and use precise types.

Proposed change

-        async def validation_exception_handler(_, exc):
+        async def validation_exception_handler(
+                _: Request, exc: RequestValidationError) -> Response:

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/serve/openai_server.py` around lines 769 - 780, Update
validation_exception_handler with the requested precise annotations: type the
ignored request parameter as Request, exc as RequestValidationError, and the
return value as Response, preserving the existing visual and non-visual response
behavior.

Source: Coding guidelines

tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py (1)

1495-1495: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the preserved request-error metric.

The test sets server.metrics_collector to None. It cannot detect a regression that removes log_request_error(http_code=400). Inject a fake collector and assert the 400 call in addition to the exact response-body assertion.

Also applies to: 1509-1518

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py` at line
1495, Update the affected endpoint tests around the metrics_collector setup to
inject a fake collector instead of None, preserve the exact response-body
assertion, and verify that log_request_error is called with http_code=400. Apply
the same coverage to the additional test block at the referenced request-error
cases.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tensorrt_llm/serve/openai_server.py`:
- Around line 769-780: Update validation_exception_handler with the requested
precise annotations: type the ignored request parameter as Request, exc as
RequestValidationError, and the return value as Response, preserving the
existing visual and non-visual response behavior.

In `@tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py`:
- Line 1495: Update the affected endpoint tests around the metrics_collector
setup to inject a fake collector instead of None, preserve the exact
response-body assertion, and verify that log_request_error is called with
http_code=400. Apply the same coverage to the additional test block at the
referenced request-error cases.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 33884d3a-a0d7-4415-9a5d-d7fce4e8e6bd

📥 Commits

Reviewing files that changed from the base of the PR and between d46a4f9 and 00c125e.

📒 Files selected for processing (3)

• tensorrt_llm/serve/openai_server.py
• tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py
• tests/unittest/executor/test_base_worker.py

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/unittest/executor/test_base_worker.py

[yibinl-nvidia]

/bot run

[tensorrt-cicd]

PR_Github #63119 [ kill ] triggered by Bot. Commit: c390441 Link to invocation

[tensorrt-cicd]

PR_Github #63109 [ run ] completed with state ABORTED. Commit: d46a4f9

Link to invocation

[tensorrt-cicd]

PR_Github #63119 [ kill ] completed with state SUCCESS. Commit: c390441
Successfully killed previous jobs for commit c390441

Link to invocation

[tensorrt-cicd]

PR_Github #63121 [ run ] triggered by Bot. Commit: c390441 Link to invocation

[yibinl-nvidia]

/bot kill

[tensorrt-cicd]

PR_Github #63124 [ kill ] triggered by Bot. Commit: e289c89 Link to invocation

[tensorrt-cicd]

PR_Github #63121 [ run ] completed with state ABORTED. Commit: c390441

Link to invocation

[tensorrt-cicd]

PR_Github #63124 [ kill ] completed with state SUCCESS. Commit: e289c89
Successfully killed previous jobs for commit e289c89

Link to invocation

[yibinl-nvidia]

/bot run

[tensorrt-cicd]

PR_Github #63128 [ run ] triggered by Bot. Commit: 940bdca Link to invocation

[tensorrt-cicd]

PR_Github #63128 [ run ] completed with state FAILURE. Commit: 940bdca
/LLM/main/L0_MergeRequest_PR pipeline #51216 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

• Please check the failed tests and fix your PR
• If you cannot view the failures, ask the CI triggerer to share details
• Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

[yibinl-nvidia]

/bot run

[tensorrt-cicd]

PR_Github #63149 [ run ] triggered by Bot. Commit: 940bdca Link to invocation

[tensorrt-cicd]

PR_Github #63149 [ run ] completed with state FAILURE. Commit: 940bdca
/LLM/main/L0_MergeRequest_PR pipeline #51233 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

• Please check the failed tests and fix your PR
• If you cannot view the failures, ask the CI triggerer to share details
• Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

[yibinl-nvidia]

/bot run

[tensorrt-cicd]

PR_Github #63167 [ run ] triggered by Bot. Commit: 940bdca Link to invocation

[tensorrt-cicd]

PR_Github #63167 [ run ] completed with state FAILURE. Commit: 940bdca
/LLM/main/L0_MergeRequest_PR pipeline #51250 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

• Please check the failed tests and fix your PR
• If you cannot view the failures, ask the CI triggerer to share details
• Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

[yibinl-nvidia]

/bot run

[tensorrt-cicd]

PR_Github #63178 [ run ] triggered by Bot. Commit: 940bdca Link to invocation

[tensorrt-cicd]

PR_Github #63178 [ run ] completed with state SUCCESS. Commit: 940bdca
/LLM/main/L0_MergeRequest_PR pipeline #51262 completed with status: 'SUCCESS'

CI Report

Link to invocation

[BowenFu]

The scope of the openai_server.py / openai_disagg_server.py change is much wider than the LoRA path leak it is meant to fix. validation_exception_handler handles every request-validation failure, so replacing {"error": str(exc)} with the constant {"error": "Request validation failed"} strips field-level detail from all 400s on the OpenAI-compatible endpoints — a bad temperature, a missing model, a malformed messages all now return the same opaque string. That is a user-visible regression for existing clients, and the detail is not logged anywhere else on that path either (the logger.warning above it only prints a truncated brief under the sampling gate), so it is lost rather than moved.

Suggestion: keep str(exc) and redact only what actually leaks. RequestValidationError.errors() gives you structured entries — echo loc/type and drop or mask input for the path-bearing fields. That preserves the debuggability clients depend on while closing the leak.

Two smaller points:

• The path that leaks is LoRARequest.__post_init__'s ValueError(f"lora_path ({self.lora_path}) does not exist."). Worth stating in the description how that string reaches an HTTP response body — if it only ever surfaces in a local traceback, dropping construction-time validation is a real usability loss (the error now shows up per-request instead of at request-build time, wrapped as RequestError) for no security gain.
• _load_lora_adapter's new os.path.exists check duplicates validation LoraManager.load_from_ckpt already does. If the point is only the message text, fixing the message at its source is less surface than adding a second gate.