examples/llm-api/llm_multilora.py (4)

4-4: Avoid introducing a new runtime dependency just for CLI parsing.

Click is not guaranteed in the test venv (other llm-api examples don’t install extra deps). Prefer stdlib argparse; or, if you keep Click, add it to a requirements file that CI installs.

Option A (preferred): switch to argparse. I can provide a patch on request.
Option B: keep Click but add examples/llm-api/requirements.txt with click>=8.1 and ensure tests install it.

Also applies to: 12-24

---

8-9: Follow import style guideline: keep module namespace.

Import modules, not classes, to avoid polluting the namespace.

Apply this diff:

-from tensorrt_llm.executor import LoRARequest
-from tensorrt_llm.lora_helper import LoraConfig
+from tensorrt_llm import executor, lora_helper

And update usages:

-lora_config = LoraConfig(
+lora_config = lora_helper.LoraConfig(
@@
-    LoRARequest("chatbot", 1, chatbot_lora_dir),
+    executor.LoRARequest("chatbot", 1, chatbot_lora_dir),
@@
-    LoRARequest("mental-health", 2, mental_health_lora_dir),
+    executor.LoRARequest("mental-health", 2, mental_health_lora_dir),
@@
-    LoRARequest("tarot", 3, tarot_lora_dir)
+    executor.LoRARequest("tarot", 3, tarot_lora_dir)

---

28-39: Make network behavior explicit and fail early when paths are wrong.

To improve UX in offline/CI runs, either validate provided dirs up-front and give a clear message, or add an --offline flag to prevent accidental downloads.

Example tweak:

+    # Validate provided dirs to fail early with a clear error
+    for name, d in {"chatbot": chatbot_lora_dir,
+                    "mental_health": mental_health_lora_dir,
+                    "tarot": tarot_lora_dir}.items():
+        if d is not None and not os.path.isdir(d):
+            raise FileNotFoundError(f"--{name}_lora_dir not found: {d}")

If staying with Click, you can also use click.Path(exists=True, file_okay=False) for option types.

---

42-45: Consider registering all provided LoRA dirs in build config.

One dir is sufficient to enable LoRA, but passing all available dirs can reduce surprises at runtime and improves parity between build and use.

Apply this diff:

-    lora_config = LoraConfig(lora_dir=[chatbot_lora_dir],
+    lora_dirs = [d for d in (chatbot_lora_dir, mental_health_lora_dir, tarot_lora_dir) if d]
+    lora_config = LoraConfig(lora_dir=lora_dirs or [chatbot_lora_dir],
                              max_lora_rank=64,
                              max_loras=3,
                              max_cpu_loras=3)

tests/integration/defs/llmapi/test_llm_examples.py (1)

112-122: Guard the test with a skip if adapters aren’t present.

Prevents hard failures on environments missing these assets while keeping signal where available.

Apply this diff:

 def test_llmapi_example_multilora(llm_root, engine_dir, llm_venv):
-    cmd_line_args = [
-        "--chatbot_lora_dir",
-        f"{llm_models_root()}/llama-models-v2/sft-tiny-chatbot",
-        "--mental_health_lora_dir",
-        f"{llm_models_root()}/llama-models-v2/TinyLlama-1.1B-Chat-v1.0-mental-health-conversational",
-        "--tarot_lora_dir",
-        f"{llm_models_root()}/llama-models-v2/tinyllama-tarot-v1"
-    ]
+    chatbot_dir = f"{llm_models_root()}/llama-models-v2/sft-tiny-chatbot"
+    mental_dir = f"{llm_models_root()}/llama-models-v2/TinyLlama-1.1B-Chat-v1.0-mental-health-conversational"
+    tarot_dir = f"{llm_models_root()}/llama-models-v2/tinyllama-tarot-v1"
+    missing = [p for p in (chatbot_dir, mental_dir, tarot_dir) if not os.path.isdir(p)]
+    if missing:
+        pytest.skip(f"Missing LoRA adapter dirs: {missing}")
+    cmd_line_args = [
+        "--chatbot_lora_dir", chatbot_dir,
+        "--mental_health_lora_dir", mental_dir,
+        "--tarot_lora_dir", tarot_dir,
+    ]
     _run_llmapi_example(llm_root, engine_dir, llm_venv, "llm_multilora.py",
                         *cmd_line_args)

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: moraxu
PR: NVIDIA/TensorRT-LLM#6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: 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.