Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion docs/web-app.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions reviewbot/static/admin.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
63 changes: 63 additions & 0 deletions reviewbot/static/admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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 = "";
Expand All @@ -69,6 +125,7 @@
submitBtn.textContent = "Save";
cancelBtn.style.display = "none";
updateProviderRow();
updateModelControl();
}

function startEdit(cfg) {
Expand All @@ -88,6 +145,7 @@
submitBtn.textContent = "Save changes";
cancelBtn.style.display = "";
updateProviderRow();
updateModelControl();
window.scrollTo({ top: form.offsetTop - 16, behavior: "smooth" });
}

Expand Down Expand Up @@ -264,6 +322,11 @@
if (!editing) {
defaultModelEl.value = defaultModels[providerEl.value] || "";
}
updateModelControl();
});

defaultModelSelectEl.addEventListener("change", () => {
defaultModelEl.value = defaultModelSelectEl.value;
});

resetForm();
Expand Down
1 change: 1 addition & 0 deletions reviewbot/static/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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;">
Expand Down
69 changes: 69 additions & 0 deletions reviewbot/static/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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) {
Expand All @@ -151,10 +212,12 @@
applySavedLlmPrefs();
applyProviderModelDefault();
updateProviderFields();
await updateModelControl();
} catch {
applySavedLlmPrefs();
applyProviderModelDefault();
updateProviderFields();
await updateModelControl();
}
}

Expand Down Expand Up @@ -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}`
Expand Down Expand Up @@ -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
Expand Down
76 changes: 76 additions & 0 deletions reviewbot/webapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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)
Expand Down
Loading