review_runner: surface real crash cause instead of "(see pod log)"#55
Merged
Conversation
The read-only review runner's catch-all reported a generic
"{type}: review crashed (see pod log)", discarding str(exc). But on the
self-reported-error path serge's watcher reaps the runner pod as soon as
the callback marks the job errored — it never waits for the TTL and skips
the crash-path log-tail capture — so "(see pod log)" is a dead reference:
the pod (and its traceback) is gone within seconds. A GLM-5.2 review that
hit an LLMResponseError from the HF router was left with no legible cause.
Bring review_runner to parity with task_runner: add dedicated
LLMResponseError (status + body excerpt via format_llm_error) and
requests.HTTPError (format_github_http_error) handlers, and embed
crash_detail(exc) in the catch-all. crash_detail moves to errors.py (its
shared home) so both runner entrypoints use one copy.
Also fix the Makefile to select the newest available Python >= 3.10 and
hard-fail otherwise: bare `python3` is 3.9 on stock macOS, which silently
built a venv that violates requires-python.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tarekziade
added a commit
that referenced
this pull request
Jul 7, 2026
…ce (#56) ## Why A GLM-5.2 review of `huggingface/peft#3395` failed with (once PR #55 made it legible): ``` LLM endpoint returned 403 Forbidden: {"error":"This authentication method does not have sufficient permissions to call Inference Providers on behalf of org huggingface"} ``` The HF token can list models fine but lacks permission to **call Inference Providers on behalf of the org**. Nothing on the Settings page let an admin catch that ahead of time — you only found out when a review died. ## What New endpoint **`POST /admin/providers/{id}/test`** that exercises a saved config end-to-end: - Resolves endpoint / model / billing **exactly as a real review would** (`_api_base_for_provider`, `default_model` → provider default, `X-HF-Bill-To` for HF). - Makes **one tiny `max_tokens=1` chat-completion** with the stored key. A `/models` GET would *not* catch this failure class — the missing permission only shows on a real inference call, and `max_tokens=1` keeps it cheap (the 401/403 fires before any generation). - Returns `{ok: true, model}` on success, or `{ok: false, error}` carrying the provider's own message. Always **HTTP 200** — a bad token is a verdict, not a server error. Settings page gets a per-row **Test** button with inline ✓/✗ feedback; failures also surface the provider's error text in the banner, so the fix (grant the token the *Inference Providers* permission / authorize it for the org) is obvious. ## Tests `tests/test_webapp_admin.py::ProviderTestEndpointTests` — valid token → `ok` + model + billing org echoed; a 403 → `ok:false` with the provider message intact; missing config → 404. Full suite **401 passed**, ruff clean. ## Note Depends on the error-surfacing fix from #55 (already merged) for the runner path, but this endpoint is independent and works on its own. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A UI review of
huggingface/peft#3395onzai-org/GLM-5.2failed twice with only:…and the pod log was unrecoverable. Two compounding issues:
review_runner.pydiscarded the detail. Its catch-all reported the generic"{type}: review crashed (see pod log)", throwing awaystr(exc)— which forLLMResponseErroralready carries the LLM endpoint's status + body excerpt. (task_runner.py, the write-capable path, already handled this correctly; the read-only review path had regressed.)(see pod log)is a dead reference. On the self-reported-error path the Job watcher seesstatus=errorfrom the callback and reaps the runner pod immediately — it never waits forttlSecondsAfterFinished, and skips thecollect_task_resultlog-tail capture the crash path uses. The pod (and its traceback) is gone within seconds.Net effect: an upstream HF-router failure for GLM-5.2 left the operator with no legible cause.
Fix
Bring
review_runner.run()to parity withtask_runner:except LLMResponseError→format_llm_error(exc)(status + body excerpt: 429/400/auth become legible).except requests.HTTPError→format_github_http_error(exc).crash_detail(exc)(cause + crashing frame) instead of(see pod log).crash_detailmoves toerrors.py(its shared home);task_runnerimports it — one copy for both entrypoints.Also
make format/testbuilt a Python 3.9 venv because barepython3is 3.9 on stock macOS, violatingrequires-python >=3.10(and stalling dependency resolution). The Makefile now selects the newest available ≥3.10 interpreter and hard-fails with a clear message otherwise. CI (3.12) unaffected.Tests
tests/test_review_runner.py: strengthened the crash test (asserts the cause travels, never"see pod log") and added one proving a 429's status + body land on the job. Full suite: 398 passed, ruff clean.🤖 Generated with Claude Code