From 611801d5c031a39290adbc8c5dd29a49c41ffb16 Mon Sep 17 00:00:00 2001
From: Parthiban Sivakumar
 <228711334+parthiban-sivakumar@users.noreply.github.com>
Date: Tue, 8 Sep 2026 17:31:39 +0530
Subject: [PATCH] fix(doctor): skip LLM checks when models block is empty
 (#5296)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`config.example.yaml` ships a `models:` key with every entry commented
out, so it parses as None rather than an empty list and the `[]` default
in `.get("models", [])` never applies. Iterating that None raised
TypeError, which the surrounding broad handler rendered as a check
result:

    ✗ LLM API key check  ('NoneType' object is not iterable)
    ✗ LLM auth check  ('NoneType' object is not iterable)
    ✗ LLM package check  ('NoneType' object is not iterable)

Every fresh install hit this before configuring a model, turning one
actionable error into four and hiding the real "models configured" hint
behind internal exception text.

Fall back on a falsy value at the three iteration sites so the checks
return no results when nothing is configured. `check_models_configured`
gets the same treatment for consistency; it was already correct because
it tests truthiness rather than iterating.

The existing tests missed this because they use `models: []`, an
explicit empty list, which iterates fine. The added regression tests use
the commented-out shape that `make config` actually produces.

`make doctor` now reports 1 error instead of 4 on a fresh clone.

Fixes #5295

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
---
 backend/tests/test_doctor.py | 22 ++++++++++++++++++++++
 scripts/doctor.py            |  8 ++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/backend/tests/test_doctor.py b/backend/tests/test_doctor.py
index 556177e78..97c0ba97d 100644
--- a/backend/tests/test_doctor.py
+++ b/backend/tests/test_doctor.py
@@ -187,6 +187,14 @@ class TestCheckModelsConfigured:
         result = doctor.check_models_configured(tmp_path / "config.yaml")
         assert result.status == "skip"
 
+    def test_commented_out_models_block(self, tmp_path):
+        # config.example.yaml ships a `models:` key whose entries are all
+        # commented out, so it parses as None rather than an empty list.
+        cfg = tmp_path / "config.yaml"
+        cfg.write_text("config_version: 5\nmodels:\n  # - name: default\n")
+        result = doctor.check_models_configured(cfg)
+        assert result.status == "fail"
+
 
 # ---------------------------------------------------------------------------
 # check_llm_api_key
@@ -216,6 +224,14 @@ class TestCheckLLMApiKey:
         results = doctor.check_llm_api_key(tmp_path / "config.yaml")
         assert results == []
 
+    def test_commented_out_models_block_returns_empty(self, tmp_path):
+        # Regression: iterating a null `models:` raised TypeError, which the
+        # broad handler rendered as "('NoneType' object is not iterable)".
+        cfg = tmp_path / "config.yaml"
+        cfg.write_text("config_version: 5\nmodels:\n  # - name: default\n")
+        results = doctor.check_llm_api_key(cfg)
+        assert results == []
+
 
 # ---------------------------------------------------------------------------
 # check_llm_auth
@@ -237,6 +253,12 @@ class TestCheckLLMAuth:
         results = doctor.check_llm_auth(cfg)
         assert any(result.status == "ok" and "Claude auth available" in result.label for result in results)
 
+    def test_commented_out_models_block_returns_empty(self, tmp_path):
+        cfg = tmp_path / "config.yaml"
+        cfg.write_text("config_version: 5\nmodels:\n  # - name: default\n")
+        assert doctor.check_llm_auth(cfg) == []
+        assert doctor.check_llm_package(cfg) == []
+
 
 # ---------------------------------------------------------------------------
 # check_web_search
diff --git a/scripts/doctor.py b/scripts/doctor.py
index 79c2728b3..b2830fd3b 100644
--- a/scripts/doctor.py
+++ b/scripts/doctor.py
@@ -280,7 +280,7 @@ def check_models_configured(config_path: Path) -> CheckResult:
         return CheckResult("models configured", "skip")
     try:
         data = _load_yaml_file(config_path)
-        models = data.get("models", [])
+        models = data.get("models") or []
         if models:
             return CheckResult("models configured", "ok", f"{len(models)} model(s)")
         return CheckResult(
@@ -326,7 +326,7 @@ def check_llm_api_key(config_path: Path) -> list[CheckResult]:
         with open(config_path, encoding="utf-8") as f:
             data = yaml.safe_load(f) or {}
 
-        for model in data.get("models", []):
+        for model in data.get("models") or []:
             # Collect all values that look like $ENV_VAR references
             def _collect_env_refs(obj: object) -> list[str]:
                 refs: list[str] = []
@@ -373,7 +373,7 @@ def check_llm_package(config_path: Path) -> list[CheckResult]:
             data = yaml.safe_load(f) or {}
 
         seen_packages: set[str] = set()
-        for model in data.get("models", []):
+        for model in data.get("models") or []:
             use = model.get("use", "")
             if ":" in use:
                 package_path = use.split(":")[0]
@@ -408,7 +408,7 @@ def check_llm_auth(config_path: Path) -> list[CheckResult]:
     results: list[CheckResult] = []
     try:
         data = _load_yaml_file(config_path)
-        for model in data.get("models", []):
+        for model in data.get("models") or []:
             use = model.get("use", "")
             model_name = model.get("name", "default")