Skip to content

Commit 26cb898

Browse files
Fix Codex GPT model selection (#120)
* Fix Codex GPT model selection * Address Codex GPT parsing review * Handle Codex-incompatible OpenAI model IDs
1 parent 86ba9da commit 26cb898

2 files changed

Lines changed: 123 additions & 4 deletions

File tree

src/ucode/agents/codex.py

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,15 @@
5656
["model_providers", CODEX_MODEL_PROVIDER_NAME, "http_headers"],
5757
]
5858

59+
_GPT_RE = re.compile(r"(?:databricks-)?gpt-(\d+)(?:[.-](\d+))?(?:[.-](\d+))?(-.+|[a-z].*)?")
60+
61+
# These models should use the Databricks ID, not the OpenAI ID, as the OpenAI
62+
# ID is incompatible with Codex.
63+
CODEX_OPENAI_ID_INCOMPATIBLE_MODELS = {
64+
"databricks-gpt-5-2-codex",
65+
"databricks-gpt-5-4-nano",
66+
}
67+
5968

6069
def is_update_available() -> tuple[str, str] | None:
6170
return available_npm_package_update(SPEC["package"])
@@ -177,9 +186,44 @@ def _remove_legacy_ucode_profile() -> None:
177186
write_toml_file(path, doc)
178187

179188

189+
def _openai_model_id(model: str | None) -> str | None:
190+
"""Map Databricks GPT endpoint ids to OpenAI model ids for Codex metadata."""
191+
parsed = _parse_gpt(model)
192+
if parsed is None:
193+
return model
194+
major, minor, patch, suffix = parsed
195+
version = str(major)
196+
if minor is not None:
197+
version += f".{minor}"
198+
if patch is not None:
199+
version += f".{patch}"
200+
return f"gpt-{version}{suffix}"
201+
202+
203+
def _codex_model_id(model: str | None) -> str | None:
204+
if model in CODEX_OPENAI_ID_INCOMPATIBLE_MODELS:
205+
return model
206+
return _openai_model_id(model)
207+
208+
209+
def _parse_gpt(model: str | None) -> tuple[int, int | None, int | None, str] | None:
210+
if not model:
211+
return None
212+
match = _GPT_RE.fullmatch(model.split("/")[-1])
213+
if not match:
214+
return None
215+
major, minor, patch, suffix = match.groups()
216+
return (
217+
int(major),
218+
int(minor) if minor is not None else None,
219+
int(patch) if patch is not None else None,
220+
suffix or "",
221+
)
222+
223+
180224
def write_tool_config(state: dict, model: str | None = None) -> dict:
181225
workspace = state["workspace"]
182-
chosen_model = model or default_model(state)
226+
chosen_model = _codex_model_id(model or default_model(state))
183227
databricks_profile = state.get("profile")
184228

185229
if _use_legacy_layout():
@@ -208,8 +252,26 @@ def write_tool_config(state: dict, model: str | None = None) -> dict:
208252

209253

210254
def default_model(state: dict) -> str | None:
255+
"""Pick the newest GPT model when multiple are available.
256+
257+
The discovery list is alphabetically sorted, which can put
258+
"databricks-gpt-5" ahead of "databricks-gpt-5-5". Prefer the
259+
highest semantic version instead. Falls back to the first
260+
discovered entry when parsing fails.
261+
"""
211262
codex_models = state.get("codex_models") or []
212-
return codex_models[0] if codex_models else None
263+
if not codex_models:
264+
return None
265+
266+
def _gpt_version_key(mid: str) -> tuple[int, int, int, int]:
267+
parsed = _parse_gpt(mid)
268+
if parsed is None:
269+
return (0, 0, 0, 0)
270+
major, minor, patch, suffix = parsed
271+
base_bonus = 1 if not suffix else 0
272+
return (major, minor or 0, patch or 0, base_bonus)
273+
274+
return max(codex_models, key=_gpt_version_key)
213275

214276

215277
def launch(state: dict, tool_args: list[str]) -> None:

tests/test_agent_codex.py

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,39 @@ def test_writes_ucode_profile_config_file(self, tmp_path, monkeypatch):
9191
assert doc["model"] == "gpt-5"
9292
assert "profiles" not in doc
9393

94+
def test_writes_openai_model_id_for_databricks_gpt_endpoint(self, tmp_path, monkeypatch):
95+
config_path = tmp_path / ".codex" / "ucode.config.toml"
96+
backup_path = tmp_path / "codex-ucode-config.backup.toml"
97+
monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", config_path)
98+
monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", backup_path)
99+
monkeypatch.setattr(codex, "agent_version", lambda binary: "0.134.0")
100+
monkeypatch.setattr(codex, "save_state", lambda state: None)
101+
102+
codex.write_tool_config(
103+
{"workspace": WS, "codex_models": ["databricks-gpt-5", "databricks-gpt-5-5"]}
104+
)
105+
106+
doc = read_toml_safe(config_path)
107+
assert doc["model"] == "gpt-5.5"
108+
109+
def test_preserves_databricks_model_id_when_openai_id_is_incompatible(
110+
self, tmp_path, monkeypatch
111+
):
112+
config_path = tmp_path / ".codex" / "ucode.config.toml"
113+
backup_path = tmp_path / "codex-ucode-config.backup.toml"
114+
monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", config_path)
115+
monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", backup_path)
116+
monkeypatch.setattr(codex, "agent_version", lambda binary: "0.134.0")
117+
monkeypatch.setattr(codex, "save_state", lambda state: None)
118+
119+
codex.write_tool_config(
120+
{"workspace": WS, "codex_models": ["databricks-gpt-5-2-codex"]},
121+
"databricks-gpt-5-2-codex",
122+
)
123+
124+
doc = read_toml_safe(config_path)
125+
assert doc["model"] == "databricks-gpt-5-2-codex"
126+
94127
def test_removes_legacy_ucode_profile_from_shared_config(self, tmp_path, monkeypatch):
95128
config_dir = tmp_path / ".codex"
96129
config_dir.mkdir()
@@ -187,12 +220,36 @@ def test_unknown_version_uses_modern_layout(self, monkeypatch):
187220

188221

189222
class TestCodexDefaultModel:
190-
def test_returns_first_codex_model(self):
191-
assert codex.default_model({"codex_models": ["gpt-5", "gpt-4o"]}) == "gpt-5"
223+
def test_picks_highest_semver_over_alpha(self):
224+
state = {"codex_models": ["databricks-gpt-5", "databricks-gpt-5-5"]}
225+
226+
assert codex.default_model(state) == "databricks-gpt-5-5"
192227

193228
def test_none_when_no_models(self):
194229
assert codex.default_model({}) is None
195230

231+
def test_prefers_base_over_suffixed_same_version(self):
232+
models = ["gpt-5-5-mini", "gpt-5-5", "gpt-5"]
233+
234+
assert codex.default_model({"codex_models": models}) == "gpt-5-5"
235+
236+
def test_namespaced_models_use_same_version_parser(self):
237+
models = ["served-models/databricks-gpt-5", "served-models/databricks-gpt-5-5"]
238+
239+
assert codex.default_model({"codex_models": models}) == "served-models/databricks-gpt-5-5"
240+
241+
def test_openai_model_id_maps_databricks_naming(self):
242+
assert codex._openai_model_id("databricks-gpt-5-5") == "gpt-5.5"
243+
assert codex._openai_model_id("databricks-gpt-5-5-mini") == "gpt-5.5-mini"
244+
assert codex._openai_model_id("databricks-gpt-4o") == "gpt-4o"
245+
assert codex._openai_model_id("served-models/databricks-gpt-5-5") == "gpt-5.5"
246+
assert codex._openai_model_id("gpt-5.5") == "gpt-5.5"
247+
248+
def test_codex_model_id_preserves_openai_incompatible_models(self):
249+
assert codex._codex_model_id("databricks-gpt-5-2-codex") == "databricks-gpt-5-2-codex"
250+
assert codex._codex_model_id("databricks-gpt-5-4-nano") == "databricks-gpt-5-4-nano"
251+
assert codex._codex_model_id("databricks-gpt-5-5") == "gpt-5.5"
252+
196253

197254
class TestCodexValidateCmd:
198255
def test_starts_with_binary(self):

0 commit comments

Comments
 (0)