Conversation
Removes gpt-5-mini-2025-08-07 from: - .github/run-eval/resolve_model_config.py MODELS dictionary - openhands-sdk/openhands/sdk/llm/utils/verified_models.py VERIFIED_OPENAI_MODELS list Updates example files and tests to use gpt-5.2 instead. Fixes #1747 Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
Review Summary
The PR successfully removes gpt-5-mini-2025-08-07 from the main model lists and updates examples and tests to use gpt-5.2 as a replacement. The replacement model is appropriate since gpt-5.2 supports the same features (Responses API, reasoning effort, etc.).
🟠 Important Issue Found
Incomplete Removal: One reference to gpt-5-mini-2025-08-07 was missed in the codebase:
- File:
tests/sdk/llm/test_model_features.py - Line: 266
- Content:
("openai/gpt-5-mini-2025-08-07", False),
This test case is verifying that gpt-5-mini-2025-08-07 does NOT support prompt_cache_retention. Since this model is being removed from the codebase entirely, this test case should also be removed to maintain consistency.
Suggested fix: Remove the line at tests/sdk/llm/test_model_features.py:266
✅ What Looks Good
- Model properly removed from
.github/run-eval/resolve_model_config.py - Model properly removed from
openhands-sdk/openhands/sdk/llm/utils/verified_models.py - Examples updated to use valid replacement model
gpt-5.2 - Tests updated appropriately
- Replacement model
gpt-5.2supports all the necessary features (Responses API, reasoning effort)
Once the missed reference is removed, this PR will be complete.
| assert api_key, "Set LLM_API_KEY or OPENAI_API_KEY in your environment." | ||
|
|
||
| model = "openhands/gpt-5-mini-2025-08-07" # Use a model that supports Responses API | ||
| model = "openhands/gpt-5.2" # Use a model that supports Responses API |
There was a problem hiding this comment.
I think maybe it can stay here, this is not eval, it's just testing with a real LLM.
| title_gen_llm = LLM( | ||
| usage_id="title-gen-llm", | ||
| model=os.getenv("LLM_MODEL", "openhands/gpt-5-mini-2025-08-07"), | ||
| model=os.getenv("LLM_MODEL", "openhands/gpt-5.2"), |
| "gpt-5.1-codex-mini", | ||
| "gpt-5-codex", | ||
| "gpt-5-2025-08-07", | ||
| "gpt-5-mini-2025-08-07", |
There was a problem hiding this comment.
I'm not sure, but maybe this one makes some sense too? I think it appears in the app-server's web UI in a dropdown for OpenAI 🤔
WDYT, should we remove it?
enyst
left a comment
There was a problem hiding this comment.
If we remove this from examples, then we need to use a more expensive LLM when running test-examples. Is that intended?
|
[Automatic Post]: This PR seems to be currently waiting for review. @all-hands-bot, @enyst, could you please take a look when you have a chance? |
enyst
left a comment
There was a problem hiding this comment.
I think we already removed it from the evaluation list?
Summary
Removes gpt-5-mini-2025-08-07 from the model lists as requested in #1747.
This follows the same pattern as #1734 which removed other unsupported models.
Changes:
gpt-5-mini-2025-08-07from.github/run-eval/resolve_model_config.pyMODELS dictionarygpt-5-mini-2025-08-07fromopenhands-sdk/openhands/sdk/llm/utils/verified_models.pyVERIFIED_OPENAI_MODELS listgpt-5.2instead ofgpt-5-mini-2025-08-07gpt-5.2instead ofgpt-5-mini-2025-08-07Fixes #1747
Checklist
@juanmichelini can click here to continue refining the PR
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:cdc1ba3-pythonRun
All tags pushed for this build
About Multi-Architecture Support
cdc1ba3-python) is a multi-arch manifest supporting both amd64 and arm64cdc1ba3-python-amd64) are also available if needed