implement dropdown when hf inference provider is selected. (#14)

* implement dropdown when hf inference provider is selected.

* address feedback

Author: sayakpaul

docs/web-app.md

@@ -59,7 +59,10 @@ The most-specific matching config wins when a review is submitted.
 1. Open the New Review page.
 2. Enter a PR URL or `owner/repo#123`.
 3. Enter a trigger comment, for example `@askserge please review`.
-4. Pick the provider and model.
+4. Pick the provider and model. When the provider is Hugging Face, the model
+   field becomes a dropdown of tool-capable models served by the
+   [HF Inference Providers](https://router.huggingface.co) router; other
+   providers keep a free-text model field.
 5. Start the review and watch the stream.
 6. Edit the summary and comments.
 7. Publish or discard the draft.

reviewbot/static/admin.html

@@ -73,6 +73,7 @@ <h2 id="form-title">Add a provider config</h2>
           <label for="default_model">Default model</label>
           <div class="hint">Used when the submit form leaves the model field empty.</div>
           <input type="text" id="default_model" name="default_model" placeholder="model name">
+          <select id="default-model-select" style="display: none;"></select>
         </div>
       </div>
 

reviewbot/static/admin.js

@@ -9,6 +9,7 @@
   const apiKeyEl = document.getElementById("api_key");
   const apiKeyHint = document.getElementById("api-key-hint");
   const defaultModelEl = document.getElementById("default_model");
+  const defaultModelSelectEl = document.getElementById("default-model-select");
   const repoPatternEl = document.getElementById("repo_pattern");
   const allowedUsersEl = document.getElementById("allowed_users");
   const allowedOrgsEl = document.getElementById("allowed_orgs");
@@ -51,6 +52,61 @@
     if (def) defaultModelEl.value = def;
   }
 
+  // HF Router model catalogue, lazily fetched once and cached. null until
+  // loaded; [] when unreachable (we then keep the free-text input).
+  let hfModels = null;
+
+  async function ensureHfModels() {
+    if (hfModels !== null) return hfModels;
+    try {
+      const r = await fetch("/llm-options/hf-models");
+      const data = r.ok ? await r.json() : {};
+      hfModels = Array.isArray(data.models) ? data.models : [];
+    } catch {
+      hfModels = [];
+    }
+    return hfModels;
+  }
+
+  function populateModelSelect() {
+    // The text input stays the source of truth; the dropdown mirrors it.
+    // A leading blank option keeps "no default" expressible (the submit
+    // form's model then wins). Preserve any current value even if the
+    // router doesn't list it.
+    const current = defaultModelEl.value.trim();
+    const models = [...(hfModels || [])];
+    if (current && !models.includes(current)) models.unshift(current);
+    defaultModelSelectEl.replaceChildren();
+    const blank = document.createElement("option");
+    blank.value = "";
+    blank.textContent = "— none (use submit-form model) —";
+    defaultModelSelectEl.appendChild(blank);
+    for (const m of models) {
+      const opt = document.createElement("option");
+      opt.value = m;
+      opt.textContent = m;
+      defaultModelSelectEl.appendChild(opt);
+    }
+    defaultModelSelectEl.value = current;
+  }
+
+  // Show a dropdown of HF Router models when the HF provider is selected;
+  // other providers keep the free-text input. Falls back to the text input
+  // when the model list can't be fetched.
+  async function updateModelControl() {
+    if (providerEl.value === "hf") {
+      const models = await ensureHfModels();
+      if (providerEl.value === "hf" && models.length) {
+        populateModelSelect();
+        defaultModelSelectEl.style.display = "";
+        defaultModelEl.style.display = "none";
+        return;
+      }
+    }
+    defaultModelSelectEl.style.display = "none";
+    defaultModelEl.style.display = "";
+  }
+
   function resetForm() {
     editing = null;
     configIdEl.value = "";
@@ -69,6 +125,7 @@
     submitBtn.textContent = "Save";
     cancelBtn.style.display = "none";
     updateProviderRow();
+    updateModelControl();
   }
 
   function startEdit(cfg) {
@@ -88,6 +145,7 @@
     submitBtn.textContent = "Save changes";
     cancelBtn.style.display = "";
     updateProviderRow();
+    updateModelControl();
     window.scrollTo({ top: form.offsetTop - 16, behavior: "smooth" });
   }
 
@@ -264,6 +322,11 @@
     if (!editing) {
       defaultModelEl.value = defaultModels[providerEl.value] || "";
     }
+    updateModelControl();
+  });
+
+  defaultModelSelectEl.addEventListener("change", () => {
+    defaultModelEl.value = defaultModelSelectEl.value;
   });
 
   resetForm();

reviewbot/static/index.html

@@ -50,6 +50,7 @@ <h2>Ask Serge to review a PR</h2>
         <div>
           <label for="llm-model">Model</label>
           <input type="text" id="llm-model" name="llm_model" required placeholder="model name">
+          <select id="llm-model-select" style="display: none;"></select>
         </div>
       </div>
       <div id="custom-base-row" style="display: none;">

reviewbot/static/index.js

@@ -6,6 +6,7 @@
   const commentEl = document.getElementById("comment");
   const providerEl = document.getElementById("llm-provider");
   const modelEl = document.getElementById("llm-model");
+  const modelSelectEl = document.getElementById("llm-model-select");
   const baseUrlEl = document.getElementById("llm-base-url");
   const customBaseRow = document.getElementById("custom-base-row");
   const providerHint = document.getElementById("provider-hint");
@@ -125,6 +126,66 @@
     modelEl.value = providerDefaultModels[providerEl.value] || "";
   }
 
+  // HF Router model catalogue, lazily fetched once and cached. null until
+  // loaded; [] when the endpoint is unreachable (we then fall back to the
+  // free-text input rather than an empty dropdown).
+  let hfModels = null;
+
+  async function ensureHfModels() {
+    if (hfModels !== null) return hfModels;
+    try {
+      const r = await fetch("/llm-options/hf-models");
+      const data = r.ok ? await r.json() : {};
+      hfModels = Array.isArray(data.models) ? data.models : [];
+    } catch {
+      hfModels = [];
+    }
+    return hfModels;
+  }
+
+  function populateModelSelect() {
+    // The text input stays the source of truth for the submitted value;
+    // the dropdown mirrors it. Keep the current value selectable even if
+    // the router doesn't list it (e.g. a config default not yet live), so
+    // switching to HF never silently drops a configured model.
+    const current = modelEl.value.trim();
+    const options = [...(hfModels || [])];
+    if (current && !options.includes(current)) options.unshift(current);
+    modelSelectEl.replaceChildren();
+    for (const m of options) {
+      const opt = document.createElement("option");
+      opt.value = m;
+      opt.textContent = m;
+      modelSelectEl.appendChild(opt);
+    }
+    if (current && options.includes(current)) {
+      modelSelectEl.value = current;
+    } else if (options.length) {
+      modelSelectEl.value = options[0];
+      modelEl.value = options[0];
+    }
+  }
+
+  // Show a dropdown of HF Router models when the HF provider is selected;
+  // every other provider keeps the free-text input. Falls back to the text
+  // input when the model list can't be fetched.
+  async function updateModelControl() {
+    if (providerEl.value === "hf") {
+      const models = await ensureHfModels();
+      // Guard against the provider changing while the fetch was in flight.
+      if (providerEl.value === "hf" && models.length) {
+        populateModelSelect();
+        modelSelectEl.style.display = "";
+        modelEl.style.display = "none";
+        modelEl.required = false;
+        return;
+      }
+    }
+    modelSelectEl.style.display = "none";
+    modelEl.style.display = "";
+    modelEl.required = true;
+  }
+
   function ingestProviderDefaults(providers) {
     if (!Array.isArray(providers)) return;
     for (const p of providers) {
@@ -151,10 +212,12 @@
       applySavedLlmPrefs();
       applyProviderModelDefault();
       updateProviderFields();
+      await updateModelControl();
     } catch {
       applySavedLlmPrefs();
       applyProviderModelDefault();
       updateProviderFields();
+      await updateModelControl();
     }
   }
 
@@ -276,6 +339,7 @@
       baseUrlEl.value = match.api_base;
     }
     updateProviderFields();
+    updateModelControl();
     const modelPart = match.default_model ? ` · model ${match.default_model}` : "";
     const scope =
       match.repo_pattern && match.repo_pattern !== `${parsed.owner}/${parsed.repo}`
@@ -323,9 +387,14 @@
   providerEl.addEventListener("change", () => {
     updateProviderFields();
     resetModelToProviderDefault();
+    updateModelControl();
     saveLlmPrefs();
   });
   modelEl.addEventListener("change", saveLlmPrefs);
+  modelSelectEl.addEventListener("change", () => {
+    modelEl.value = modelSelectEl.value;
+    saveLlmPrefs();
+  });
   baseUrlEl.addEventListener("change", saveLlmPrefs);
 
   // Auto-fill provider/model from the matching DB config whenever the

reviewbot/webapp.py

@@ -97,6 +97,16 @@
     _LLM_PROVIDER_ANTHROPIC: "claude-opus-4-6",
 }
 
+# HF Inference Providers catalogue. The public router /v1/models endpoint
+# lists every model the HF Router can serve along with per-provider tool
+# support; we surface the tool-capable subset as a dropdown on the submit /
+# admin forms so users don't have to hand-type model ids. Cached in-process
+# (the list is large and changes slowly) and refreshed lazily on expiry.
+_HF_MODELS_URL = f"{_LLM_PROVIDER_BASES[_LLM_PROVIDER_HF]}/models"
+_HF_MODELS_TTL_SECONDS = 15 * 60
+_hf_models_lock = threading.Lock()
+_hf_models_cache: dict[str, Any] = {"fetched_at": 0.0, "models": []}
+
 
 # ---------------------------------------------------------------------------
 # Session handling: signed cookies via itsdangerous (no DB).
@@ -1343,6 +1353,72 @@ def _provider_entry(pid: str, label: str, base_url: str) -> dict:
     )
 
 
+def _tool_capable_hf_models(payload: Any) -> list[str]:
+    """Extract the ids of HF Router models that can drive an agentic
+    review — at least one ``live`` provider advertising ``supports_tools``.
+    Serge's review loop relies on browse tools, so models that can't call
+    tools would degrade silently; we keep them out of the dropdown. Sorted
+    case-insensitively and de-duplicated."""
+    data = payload.get("data") if isinstance(payload, dict) else None
+    if not isinstance(data, list):
+        return []
+    ids: set[str] = set()
+    for entry in data:
+        if not isinstance(entry, dict):
+            continue
+        model_id = entry.get("id")
+        if not isinstance(model_id, str) or not model_id:
+            continue
+        providers = entry.get("providers")
+        if not isinstance(providers, list):
+            continue
+        if any(
+            isinstance(p, dict)
+            and p.get("status") == "live"
+            and p.get("supports_tools") is True
+            for p in providers
+        ):
+            ids.add(model_id)
+    return sorted(ids, key=str.lower)
+
+
+async def _fetch_hf_router_models() -> list[str]:
+    """Tool-capable HF Router model ids, cached for ``_HF_MODELS_TTL_SECONDS``.
+    On a fetch error the last good list (possibly empty) is returned so a
+    transient router blip doesn't blank out the dropdown — the form falls
+    back to a free-text box when the list is empty."""
+    now = time.monotonic()
+    with _hf_models_lock:
+        # Use fetched_at (0.0 until the first successful fetch) rather than
+        # the list's truthiness to gauge freshness — a valid fetch that
+        # yields no tool-capable models is still a fetch worth caching, and
+        # keying on truthiness would re-hit the router on every call.
+        fetched_at = _hf_models_cache["fetched_at"]
+        if fetched_at > 0 and now - fetched_at < _HF_MODELS_TTL_SECONDS:
+            return _hf_models_cache["models"]
+    try:
+        async with httpx.AsyncClient(timeout=10) as client:
+            resp = await client.get(_HF_MODELS_URL)
+        resp.raise_for_status()
+        models = _tool_capable_hf_models(resp.json())
+    except (httpx.HTTPError, ValueError) as exc:
+        log.warning("Failed to fetch HF router models from %s: %s", _HF_MODELS_URL, exc)
+        with _hf_models_lock:
+            return _hf_models_cache["models"]
+    with _hf_models_lock:
+        _hf_models_cache["models"] = models
+        _hf_models_cache["fetched_at"] = now
+    return models
+
+
+@app.get("/llm-options/hf-models")
+async def hf_models(request: Request) -> JSONResponse:
+    """Tool-capable models served by the HF Router, for the provider
+    dropdown that appears when ``hf`` is the selected provider."""
+    _require_user(request)
+    return JSONResponse({"models": await _fetch_hf_router_models()})
+
+
 @app.get("/reviews/lookup-provider")
 def lookup_provider(request: Request, owner: str, repo: str) -> JSONResponse:
     """Pre-flight match for the submit form: given a (owner, repo)