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
32 changes: 24 additions & 8 deletions src/openenv/core/harness/capture/upstream.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import json
import logging
from typing import Any
from urllib.parse import urlsplit

import httpx

Expand Down Expand Up @@ -114,15 +115,26 @@ class UpstreamTransportError(UpstreamError):
def normalise_engine_base(url: str) -> str:
"""The engine root, with any trailing `/v1` removed.

Every route this module builds is already `/v1/...`, so a caller who passes the OpenAI-style
base (`http://host:8000/v1`, which is what most SDKs and most people hand you) would otherwise
get `/v1/v1/chat/completions` and see a healthy engine reported as unreachable. Accept both
forms and normalise here rather than making every call site remember which one it holds.
Anthropic's convention: the base excludes `/v1` and routes are `/v1/...`, so a caller who
passes `https://api.anthropic.com/v1` would otherwise get `/v1/v1/messages`. OpenAI-compatible
routes use `openai_api_base` instead.
"""
base = url.rstrip("/")
return base[: -len("/v1")] if base.endswith("/v1") else base


def openai_api_base(url: str) -> str:
"""The OpenAI-compatible API root that `/chat/completions` and `/models` are built on.

The OpenAI SDK rule, as `OpenAIClient` applies it (#1237): a base with no path gets `/v1`, and
a base with a path is already the API root. Stripping `/v1` and re-adding it broke every
provider whose root is not `/v1`: Gemini's `.../v1beta/openai` became
`.../v1beta/openai/v1/chat/completions`.
"""
base = url.rstrip("/")
return base if urlsplit(base).path else f"{base}/v1"


# Sampling knobs that make a *processed* logprob incomparable to a full-vocab recompute, mapped to the
# value that disables each one.
#
Expand Down Expand Up @@ -427,7 +439,11 @@ def __init__(
if provider == "anthropic":
capture_level = "text"
auth_header = "x-api-key"
self.base_url = normalise_engine_base(base_url)
self.base_url = (
f"{normalise_engine_base(base_url)}/v1"
if provider == "anthropic"
else openai_api_base(base_url)
)
self.served_model = served_model
self.api_key = api_key or None
self.auth_header = auth_header or "Authorization"
Expand Down Expand Up @@ -476,7 +492,7 @@ async def completion(self, request: dict[str, Any]) -> dict[str, Any]:
if key in {"anthropic-beta", "anthropic-version"}
}
payload = await self._post_with_retries(
"/v1/messages", body, headers=native_headers
"/messages", body, headers=native_headers
)
response = anthropic_response(
payload,
Expand All @@ -491,7 +507,7 @@ async def completion(self, request: dict[str, Any]) -> dict[str, Any]:
served_model=self.served_model,
capture_level=self.capture_level,
)
payload = await self._post("/v1/chat/completions", body)
payload = await self._post("/chat/completions", body)
# The body now includes any compatibility edits made by `_post`. Keep the actual
# submitted policy separate from the harness's request; the server removes this marker.
payload["_openenv_sampling"] = {
Expand All @@ -504,7 +520,7 @@ async def completion(self, request: dict[str, Any]) -> dict[str, Any]:
async def list_models(self) -> dict[str, Any]:
client = await self._get_client()
try:
response = await client.get("/v1/models")
response = await client.get("/models")
except httpx.RequestError as exc:
raise self._transport_error(exc) from exc
await self._raise_for_status(response)
Expand Down
26 changes: 16 additions & 10 deletions src/openenv/core/harness/capture/validate_llm.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,12 @@
from dataclasses import dataclass, field

from .compat import behaviour_warnings, diagnose, MAX_FIXES
from .upstream import auth_headers, normalise_engine_base, normalize_response
from .upstream import (
auth_headers,
normalise_engine_base,
normalize_response,
openai_api_base,
)
from .validate import check_upstream_response

# The one place the engine requirements are spelled out. Three call sites used to carry their own
Expand Down Expand Up @@ -166,7 +171,7 @@ def list_models(
"""Served model ids, or [] if the endpoint is unreachable or does not publish a list."""
try:
request = urllib.request.Request(
f"{normalise_engine_base(llm_url)}/v1/models",
f"{openai_api_base(llm_url)}/models",
headers=auth_headers(api_key, auth_header),
)
with urllib.request.urlopen(request, timeout=timeout) as r:
Expand Down Expand Up @@ -291,7 +296,7 @@ def probe_tool_support(
while True:
try:
payload = _post(
f"{base}/v1/chat/completions",
f"{openai_api_base(base)}/chat/completions",
body,
timeout,
api_key=api_key,
Expand Down Expand Up @@ -362,7 +367,7 @@ def probe_logprobs_mode(

Args:
base (`str`):
Engine root, already normalised.
OpenAI-compatible base URL, with or without `/v1`.
model (`str`):
Model id to probe.
timeout (`float`, *optional*, defaults to `60.0`):
Expand Down Expand Up @@ -398,7 +403,7 @@ def probe_logprobs_mode(
fix.apply(body)
try:
payload = _post(
f"{base}/v1/chat/completions",
f"{openai_api_base(base)}/chat/completions",
body,
timeout,
api_key=api_key,
Expand Down Expand Up @@ -538,7 +543,8 @@ def validate_llm(

Args:
llm_url (`str`):
OpenAI-spec endpoint. Accepts both `http://host:8000` and `http://host:8000/v1`.
OpenAI-spec endpoint. Accepts `http://host:8000`, `http://host:8000/v1`, or a
provider base with its own path such as `.../v1beta/openai`.
model (`str`):
Model id to probe.
timeout (`float`, *optional*, defaults to `120.0`):
Expand All @@ -551,15 +557,15 @@ def validate_llm(
Returns:
[`LLMReport`]: `capture_level`, whether it is trainable, and every workaround applied.
"""
# Accepts both `http://host:8000` and `http://host:8000/v1`, so a URL that works with any
# OpenAI SDK also works here instead of probing `/v1/v1/models` and reporting it dead.
# Same base rule as the OpenAI SDK, so a URL that works there also works here instead of
# probing `/v1/v1/models` or `/v1beta/openai/v1/models` and reporting it dead.
if provider not in {"openai", "anthropic", "hf", "vllm"}:
raise ValueError(f"Unknown upstream provider: {provider}")
if provider == "anthropic":
return _validate_anthropic(
llm_url, model, timeout=timeout, api_key=api_key, check_tools=check_tools
)
base = normalise_engine_base(llm_url)
base = openai_api_base(llm_url)
served = list_models(
base, timeout=min(timeout, 30.0), api_key=api_key, auth_header=auth_header
)
Expand Down Expand Up @@ -603,7 +609,7 @@ def validate_llm(
while payload is None:
try:
payload = _post(
f"{base}/v1/chat/completions",
f"{base}/chat/completions",
body,
timeout,
api_key=api_key,
Expand Down
2 changes: 1 addition & 1 deletion tests/envs/test_harbor_native_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def respond(request):
http_client = await client._get_client()
await http_client.aclose()
client._client = httpx.AsyncClient(
base_url="https://test",
base_url=client.base_url,
transport=httpx.MockTransport(respond),
headers={"x-api-key": "test-secret", "anthropic-version": "2023-06-01"},
)
Expand Down
146 changes: 133 additions & 13 deletions tests/envs/test_harbor_upstream_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@

from __future__ import annotations

import httpx
import pytest

upstream = pytest.importorskip("openenv.core.harness.capture.upstream")
validate_llm = pytest.importorskip("openenv.core.harness.capture.validate_llm")
dialects = pytest.importorskip("openenv.core.harness.capture.dialects")
detection = pytest.importorskip("openenv.core.harness.capture.detection")

Expand Down Expand Up @@ -126,23 +128,141 @@ def test_messages_without_reasoning_are_passed_through_untouched():


# --- engine base URL --------------------------------------------------------
#
# The OpenAI SDK rule: a base with no path gets `/v1`, a base with a path is already the API root.
# `http://host:8000/v1` used to become `/v1/v1/models`; a gateway such as Gemini's
# `/v1beta/openai` used to become `/v1beta/openai/v1/chat/completions` and 404.
API_BASES = [
("http://localhost:8000", "http://localhost:8000/v1"),
("http://localhost:8000/", "http://localhost:8000/v1"),
("http://host:8000/v1", "http://host:8000/v1"),
("http://host:8000/v1/", "http://host:8000/v1"),
("https://router.huggingface.co/v1", "https://router.huggingface.co/v1"),
("https://gw.example/openai/v1", "https://gw.example/openai/v1"),
(
"https://generativelanguage.googleapis.com/v1beta/openai",
"https://generativelanguage.googleapis.com/v1beta/openai",
),
(
"https://generativelanguage.googleapis.com/v1beta/openai/",
"https://generativelanguage.googleapis.com/v1beta/openai",
),
("https://api.z.ai/api/paas/v4", "https://api.z.ai/api/paas/v4"),
]


def _mock_client(client, seen):
def respond(request):
seen.append(str(request.url))
if request.method == "GET":
return httpx.Response(200, json={"data": []})
return httpx.Response(
200,
json={"choices": [{"message": {"content": "ok"}, "finish_reason": "stop"}]},
)

client._client = httpx.AsyncClient(
base_url=client.base_url, transport=httpx.MockTransport(respond)
)


@pytest.mark.parametrize("given,api_base", API_BASES)
async def test_upstream_routes_keep_the_provider_base_path(given, api_base):
client = upstream.InferenceClient(given, capture_level="text")
seen: list[str] = []
_mock_client(client, seen)
try:
await client.list_models()
await client.completion({"messages": [{"role": "user", "content": "go"}]})
finally:
await client.aclose()
assert seen == [f"{api_base}/models", f"{api_base}/chat/completions"]


@pytest.mark.parametrize(
"given",
"given,expected",
[
"http://host:8000",
"http://host:8000/",
"http://host:8000/v1",
"http://host:8000/v1/",
("https://api.anthropic.com", "https://api.anthropic.com/v1/messages"),
("https://api.anthropic.com/v1", "https://api.anthropic.com/v1/messages"),
# Anthropic's SDK convention: the base excludes `/v1`, so a gateway path is kept and
# `/v1/messages` goes after it.
(
"https://gw.example/api/anthropic",
"https://gw.example/api/anthropic/v1/messages",
),
],
)
def test_engine_base_is_the_root_whichever_form_is_given(given):
"""Every route this layer builds is `/v1/...`, so a `/v1` base must not double up.

`http://host:8000/v1` is what an OpenAI SDK wants and what people paste, and it used to make
the startup probe request `/v1/v1/models` and report a healthy engine as unreachable.
"""
assert upstream.normalise_engine_base(given) == "http://host:8000"
assert upstream.InferenceClient(given).base_url == "http://host:8000"
async def test_anthropic_route_keeps_the_anthropic_base_convention(given, expected):
client = upstream.InferenceClient(given, provider="anthropic")
seen: list[str] = []

def respond(request):
seen.append(str(request.url))
return httpx.Response(
200,
json={
"id": "msg_1",
"type": "message",
"role": "assistant",
"model": "m",
"content": [{"type": "text", "text": "ok"}],
"stop_reason": "end_turn",
"usage": {"input_tokens": 1, "output_tokens": 1},
},
)

client._client = httpx.AsyncClient(
base_url=client.base_url, transport=httpx.MockTransport(respond)
)
try:
await client.completion(
{"model": "m", "messages": [{"role": "user", "content": "go"}]}
)
finally:
await client.aclose()
assert seen == [expected]


@pytest.mark.parametrize("given,api_base", API_BASES)
def test_validate_llm_probes_keep_the_provider_base_path(monkeypatch, given, api_base):
listed: list[str] = []
posted: list[str] = []

class Listing:
def __enter__(self):
return self

def __exit__(self, *exc):
return False

def read(self):
return b'{"data": [{"id": "m"}]}'

def urlopen(request, timeout):
listed.append(request.full_url)
return Listing()

def fake_post(url, body, timeout, api_key=None, auth_header="Authorization"):
posted.append(url)
return {
"prompt_token_ids": [1, 2],
"choices": [
{
"message": {"content": "ok"},
"finish_reason": "stop",
"token_ids": [7],
"logprobs": {"content": [{"logprob": -0.1, "top_logprobs": []}]},
}
],
}

monkeypatch.setattr(validate_llm.urllib.request, "urlopen", urlopen)
monkeypatch.setattr(validate_llm, "_post", fake_post)
validate_llm.validate_llm(given, "m")
assert listed == [f"{api_base}/models"]
# The completion, tool and logprobs-mode probes all go to the same route.
assert len(posted) > 1
assert set(posted) == {f"{api_base}/chat/completions"}


# --- capture level: what a provider will actually accept --------------------
Expand Down
Loading