Merge PR #836: fix Codex provider routing for GitHub Copilot models
This commit is contained in:
@@ -287,11 +287,25 @@ class Config(BaseSettings):
|
|||||||
from nanobot.providers.registry import PROVIDERS
|
from nanobot.providers.registry import PROVIDERS
|
||||||
|
|
||||||
model_lower = (model or self.agents.defaults.model).lower()
|
model_lower = (model or self.agents.defaults.model).lower()
|
||||||
|
model_normalized = model_lower.replace("-", "_")
|
||||||
|
model_prefix = model_lower.split("/", 1)[0] if "/" in model_lower else ""
|
||||||
|
normalized_prefix = model_prefix.replace("-", "_")
|
||||||
|
|
||||||
|
def _kw_matches(kw: str) -> bool:
|
||||||
|
kw = kw.lower()
|
||||||
|
return kw in model_lower or kw.replace("-", "_") in model_normalized
|
||||||
|
|
||||||
|
# Explicit provider prefix wins — prevents `github-copilot/...codex` matching openai_codex.
|
||||||
|
for spec in PROVIDERS:
|
||||||
|
p = getattr(self.providers, spec.name, None)
|
||||||
|
if p and model_prefix and normalized_prefix == spec.name:
|
||||||
|
if spec.is_oauth or p.api_key:
|
||||||
|
return p, spec.name
|
||||||
|
|
||||||
# Match by keyword (order follows PROVIDERS registry)
|
# Match by keyword (order follows PROVIDERS registry)
|
||||||
for spec in PROVIDERS:
|
for spec in PROVIDERS:
|
||||||
p = getattr(self.providers, spec.name, None)
|
p = getattr(self.providers, spec.name, None)
|
||||||
if p and any(kw in model_lower for kw in spec.keywords):
|
if p and any(_kw_matches(kw) for kw in spec.keywords):
|
||||||
if spec.is_oauth or p.api_key:
|
if spec.is_oauth or p.api_key:
|
||||||
return p, spec.name
|
return p, spec.name
|
||||||
|
|
||||||
|
|||||||
@@ -88,11 +88,22 @@ class LiteLLMProvider(LLMProvider):
|
|||||||
# Standard mode: auto-prefix for known providers
|
# Standard mode: auto-prefix for known providers
|
||||||
spec = find_by_model(model)
|
spec = find_by_model(model)
|
||||||
if spec and spec.litellm_prefix:
|
if spec and spec.litellm_prefix:
|
||||||
|
model = self._canonicalize_explicit_prefix(model, spec.name, spec.litellm_prefix)
|
||||||
if not any(model.startswith(s) for s in spec.skip_prefixes):
|
if not any(model.startswith(s) for s in spec.skip_prefixes):
|
||||||
model = f"{spec.litellm_prefix}/{model}"
|
model = f"{spec.litellm_prefix}/{model}"
|
||||||
|
|
||||||
return model
|
return model
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _canonicalize_explicit_prefix(model: str, spec_name: str, canonical_prefix: str) -> str:
|
||||||
|
"""Normalize explicit provider prefixes like `github-copilot/...`."""
|
||||||
|
if "/" not in model:
|
||||||
|
return model
|
||||||
|
prefix, remainder = model.split("/", 1)
|
||||||
|
if prefix.lower().replace("-", "_") != spec_name:
|
||||||
|
return model
|
||||||
|
return f"{canonical_prefix}/{remainder}"
|
||||||
|
|
||||||
def _apply_model_overrides(self, model: str, kwargs: dict[str, Any]) -> None:
|
def _apply_model_overrides(self, model: str, kwargs: dict[str, Any]) -> None:
|
||||||
"""Apply model-specific parameter overrides from the registry."""
|
"""Apply model-specific parameter overrides from the registry."""
|
||||||
model_lower = model.lower()
|
model_lower = model.lower()
|
||||||
|
|||||||
@@ -80,7 +80,7 @@ class OpenAICodexProvider(LLMProvider):
|
|||||||
|
|
||||||
|
|
||||||
def _strip_model_prefix(model: str) -> str:
|
def _strip_model_prefix(model: str) -> str:
|
||||||
if model.startswith("openai-codex/"):
|
if model.startswith("openai-codex/") or model.startswith("openai_codex/"):
|
||||||
return model.split("/", 1)[1]
|
return model.split("/", 1)[1]
|
||||||
return model
|
return model
|
||||||
|
|
||||||
|
|||||||
@@ -384,10 +384,18 @@ def find_by_model(model: str) -> ProviderSpec | None:
|
|||||||
"""Match a standard provider by model-name keyword (case-insensitive).
|
"""Match a standard provider by model-name keyword (case-insensitive).
|
||||||
Skips gateways/local — those are matched by api_key/api_base instead."""
|
Skips gateways/local — those are matched by api_key/api_base instead."""
|
||||||
model_lower = model.lower()
|
model_lower = model.lower()
|
||||||
for spec in PROVIDERS:
|
model_normalized = model_lower.replace("-", "_")
|
||||||
if spec.is_gateway or spec.is_local:
|
model_prefix = model_lower.split("/", 1)[0] if "/" in model_lower else ""
|
||||||
continue
|
normalized_prefix = model_prefix.replace("-", "_")
|
||||||
if any(kw in model_lower for kw in spec.keywords):
|
std_specs = [s for s in PROVIDERS if not s.is_gateway and not s.is_local]
|
||||||
|
|
||||||
|
# Prefer explicit provider prefix — prevents `github-copilot/...codex` matching openai_codex.
|
||||||
|
for spec in std_specs:
|
||||||
|
if model_prefix and normalized_prefix == spec.name:
|
||||||
|
return spec
|
||||||
|
|
||||||
|
for spec in std_specs:
|
||||||
|
if any(kw in model_lower or kw.replace("-", "_") in model_normalized for kw in spec.keywords):
|
||||||
return spec
|
return spec
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|||||||
@@ -6,6 +6,10 @@ import pytest
|
|||||||
from typer.testing import CliRunner
|
from typer.testing import CliRunner
|
||||||
|
|
||||||
from nanobot.cli.commands import app
|
from nanobot.cli.commands import app
|
||||||
|
from nanobot.config.schema import Config
|
||||||
|
from nanobot.providers.litellm_provider import LiteLLMProvider
|
||||||
|
from nanobot.providers.openai_codex_provider import _strip_model_prefix
|
||||||
|
from nanobot.providers.registry import find_by_model
|
||||||
|
|
||||||
runner = CliRunner()
|
runner = CliRunner()
|
||||||
|
|
||||||
@@ -90,3 +94,37 @@ def test_onboard_existing_workspace_safe_create(mock_paths):
|
|||||||
assert "Created workspace" not in result.stdout
|
assert "Created workspace" not in result.stdout
|
||||||
assert "Created AGENTS.md" in result.stdout
|
assert "Created AGENTS.md" in result.stdout
|
||||||
assert (workspace_dir / "AGENTS.md").exists()
|
assert (workspace_dir / "AGENTS.md").exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_config_matches_github_copilot_codex_with_hyphen_prefix():
|
||||||
|
config = Config()
|
||||||
|
config.agents.defaults.model = "github-copilot/gpt-5.3-codex"
|
||||||
|
|
||||||
|
assert config.get_provider_name() == "github_copilot"
|
||||||
|
|
||||||
|
|
||||||
|
def test_config_matches_openai_codex_with_hyphen_prefix():
|
||||||
|
config = Config()
|
||||||
|
config.agents.defaults.model = "openai-codex/gpt-5.1-codex"
|
||||||
|
|
||||||
|
assert config.get_provider_name() == "openai_codex"
|
||||||
|
|
||||||
|
|
||||||
|
def test_find_by_model_prefers_explicit_prefix_over_generic_codex_keyword():
|
||||||
|
spec = find_by_model("github-copilot/gpt-5.3-codex")
|
||||||
|
|
||||||
|
assert spec is not None
|
||||||
|
assert spec.name == "github_copilot"
|
||||||
|
|
||||||
|
|
||||||
|
def test_litellm_provider_canonicalizes_github_copilot_hyphen_prefix():
|
||||||
|
provider = LiteLLMProvider(default_model="github-copilot/gpt-5.3-codex")
|
||||||
|
|
||||||
|
resolved = provider._resolve_model("github-copilot/gpt-5.3-codex")
|
||||||
|
|
||||||
|
assert resolved == "github_copilot/gpt-5.3-codex"
|
||||||
|
|
||||||
|
|
||||||
|
def test_openai_codex_strip_prefix_supports_hyphen_and_underscore():
|
||||||
|
assert _strip_model_prefix("openai-codex/gpt-5.1-codex") == "gpt-5.1-codex"
|
||||||
|
assert _strip_model_prefix("openai_codex/gpt-5.1-codex") == "gpt-5.1-codex"
|
||||||
|
|||||||
Reference in New Issue
Block a user