tests/unittest/llmapi/test_llm_pytorch.py (1)

820-832:Simplify patch and fix unused variable; ensure no heavy init happens

Raise directly from the patched factory and remove the unused variable assignment flagged by Ruff (F841).

-    # Test that the error is properly caught and re-raised by LLM-    # We patch GenerationExecutor.create directly to return our failing worker-    with patch('tensorrt_llm.executor.executor.GenerationExecutor.create',-               side_effect=lambda *args, **kwargs: FailingExecutorWorker(-                   *args, **kwargs)):-        with pytest.raises(-                RuntimeError,-                match="Mock GenerationExecutorWorker initialization failed"):-            llm = LLM(model=llama_model_path,-                      kv_cache_config=global_kvcache_config)+    # Patch the executor factory to fail immediately (no engine/GPU work).+    with patch('tensorrt_llm.executor.executor.GenerationExecutor.create',+               side_effect=RuntimeError("Mock GenerationExecutorWorker initialization failed")):+        with pytest.raises(RuntimeError,+                           match="Mock GenerationExecutorWorker initialization failed"):+            LLM(model=llama_model_path, kv_cache_config=global_kvcache_config)

tensorrt_llm/executor/proxy.py (2)

327-332:Harden abort reason typing and exception chaining

• Pass a string to shutdown_abort; don’t rely on implicit str(Exception).
• Only use “raise … from …” if the status is an Exception; otherwise include the status representation in the message. Also enrich logging with the status repr.

-        if ready_signal != GenerationExecutorProxy.READY_SIGNAL:-            logger.error(f"Executor worker initialization error: {error_trace}")-            self.mpi_session.shutdown_abort(reason=ready_signal)-            raise RuntimeError(-                "Executor worker returned error") from ready_signal+        if status != GenerationExecutorProxy.READY_SIGNAL:+            logger.error(+                f"Executor worker initialization error: status={status!r}, trace:\n{error_trace}"+            )+            self.mpi_session.shutdown_abort(reason=str(status))+            if isinstance(status, Exception):+                raise RuntimeError("Executor worker returned error") from status+            else:+                raise RuntimeError(f"Executor worker returned error: {status!r}")

---

1-1:Missing NVIDIA copyright header

Per project guidelines, add the NVIDIA copyright header at the top.

tensorrt_llm/executor/worker.py (3)

777-783:Consider a more robust wire format for errors

Pickling arbitrary Exceptions across process boundaries can fail for some exception types. A robust approach is to serialize the exception class name and message, and keep the full traceback string; reconstruct or wrap upstream.

Example shape: ({"exc_type": type(e).name, "message": str(e)}, traceback.format_exc())

---

645-650:Fix type annotation for ready_signal

Proxy uses a bytes READY signal (b"READY"). Update the worker_main signature to reflect bytes.

-    ready_signal: Optional[str] = None,+    ready_signal: Optional[bytes] = None,

---

1-1:Missing NVIDIA copyright header

Per project guidelines, add the NVIDIA copyright header at the top.

tests/unittest/llmapi/test_llm_pytorch.py (2)

6-6:Avoid symbol import; keep namespace per guidelines (or remove entirely)

The direct symbol import breaks the “maintain namespace” guideline and is unnecessary if you raise directly in the patched factory (see below). Remove this import.

-from tensorrt_llm.executor import GenerationExecutorWorker

---

814-832:Optional: add a unit test that exercises the proxy’s 2-tuple init handshake

Current test fails early in the factory and does not cover the new (status, traceback) path via worker_init_status_queue. Consider adding a focused unit test that constructs a GenerationExecutorProxy with mocked mpi_session and worker_init_status_queue to return (Exception(...), "trace..."), then asserts the raised RuntimeError and logging.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: moraxuPR: NVIDIA/TensorRT-LLM#6303File: tests/integration/test_lists/qa/examples_test_list.txt:494-494Timestamp: 2025-07-28T17:06:08.621ZLearning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.