Repository navigation
Harbor: make endpoint validation work with hosted providers (probe retry/eval fallback, router provider suffixes) - #1565
Open
sergiopaniego wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c90083f. Configure here.
This branch has not been deployed
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
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.

Summary
Two fixes to
validate_llmthat came up runningopenenv harbor rolloutagainst the HF Inference Providers router at scale. A probe error that names no parameter (a bare 400, a 5xx, a timeout) aborted the run instead of being retried or falling back to an eval rollout, and the CLI rejected provider-suffixed model ids such asQwen/Qwen3.5-397B-A17B:novita.Type of Change
What was wrong
Probe.
harbor rolloutsometimes aborted before running the task:Once transiently (1 task in ~150, the same endpoint served the others), and deterministically when the router's default provider for a model rejects
return_token_idswith a 400 that names no parameter, although the message itself says such an endpoint works as an eval backend. The probe loop (src/openenv/core/harness/capture/validate_llm.py:633) only recovers from a 400 thatcompat.diagnosemaps to a parameter. Anything else ends the probe on the first attempt withreachable=False, andprepare()raises.Provider suffix.
--llm-url https://router.huggingface.co/v1 --model Qwen/Qwen3.5-397B-A17B:novitafailed withmodel ... is not served here. The router routes<id>:<provider>(and:fastest,:cheapest) but/v1/modelslists bare ids.validate_llm.py:577already strips the suffix, but only forprovider == "hf", which the web UI passes (src/openenv/harbor/ui.py:599) and the CLI can't.Fix
PROBE_RETRIES = 2times, 1 s apart, so a transient error on a vLLM endpoint still gradestokens.return_token_ids,logprobsandtop_logprobs. If that answers, the level is graded from the reply (text), the run is an eval rollout, and the dropped params show inparam_fixes, whichharbor infoandharbor rolloutalready print.<listed id>:<suffix>is accepted there when<listed id>is listed, and the suffixed id is what gets sent. Unlisted models are still reported, and other endpoints still need an exact match.This follows RFC 012: with
purpose="auto"(whatharbor rolloutuses) or"eval", an endpoint without token capture runs as eval. Withpurpose="train",rollout_type_forstill rejects anything belowtokens.RFC Status
Test Plan
tests/envs/test_harbor_capture_level.py, 12 new tests with a scripted fake_postand a faked/modelslist, no network:text/eval without the capture params, and a 400 that survives the fallback is still reported afterPROBE_RETRIES + 2callspurpose="train"still refusestextQwen/Qwen3.5-397B-A17B:novitaand every probe body carries the suffixed id (failed before the fix), still rejects an unlisted suffixed model, and a non-router URL still rejects<listed id>:novitapytest tests/envs/test_harbor_capture_level.py: 53 passed. Acrosstests/envs/test_harbor_*.py, the only failure (test_harbor_capture_server.py::test_start_refuses_a_port_held_by_a_non_capture_listener) also fails on main.ruff format,ruff checkandusortare clean.Not yet run end to end against the router with the fix: the errors above come from ~3,000 rollouts of Qwen3.5-397B-A17B with OpenCode in HF Sandboxes before it.
Claude Code Review
N/A
Note
Medium Risk
Touches core endpoint validation for Harbor rollouts (classification, retries, when capture params are dropped); wrong branching could mislabel backends, though behavior is narrow and heavily covered by new tests.
Overview
Harbor
validate_llm: The completion probe now treats unnamed HTTP failures as transient first: it resends the same body up to two times with a short delay, then—only for 400/422 after retries exhaust—dropsreturn_token_ids,logprobs(and thustop_logprobs) viaParamFix, records that inparam_fixes, and grades the reply so hosted routers that reject capture params still land as eval instead of failing the run. 401/403/404 still stop immediately; 429/503 and similar never trigger that param strip, so busy backends are not downgraded to eval. Timeouts (including nested onHTTPError) get the same retry budget.Model listing: When the base host is
router.huggingface.co,/v1/modelsmatching uses the id without the:providersuffix (same behavior asprovider=="hf"), so CLI URLs likemodel/id:novitavalidate while the suffixed id is still sent in requests.Tests:
test_harbor_capture_level.pyadds coverage for retries, eval fallback, no-fallback on overload codes, fast-fail auth, router suffix acceptance/rejection, androllout_type_fortrain vs auto.Reviewed by Cursor Bugbot for commit 8c77816. Bugbot is set up for automated code reviews on this repo. Configure here.