docs(run-eval): require maintainer approval before editing verified_models.py - #3479
Conversation
… explicitly requested Update ADDINGMODEL.md to make clear that models should not be added to verified_models.py unless a maintainer explicitly requests it. A passing integration run is necessary but not sufficient for marking a model as verified — maintainer approval is also required. The existing provider-list guidance is preserved as a follow-up note for the case where editing verified_models.py is requested. Co-authored-by: openhands <openhands@all-hands.dev>
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
Review: docs(run-eval): require maintainer approval before editing verified_models.py
This is a clean, well-motivated documentation change. The addition of the ⛔ note effectively addresses the confusion identified in #3315 — contributors previously had no guidance preventing them from adding a model to verified_models.py as part of an eval PR, which inadvertently promoted it to "verified for production." The two-tier structure (⛔ for the gating rule,
The prose is accurate and actionable. No code is touched, so there is no functional risk.
One minor wording suggestion on the new note (see inline), but it is non-blocking.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
all-hands-bot
left a comment
There was a problem hiding this comment.
✅ QA Report: PASS
Rendered the changed ADDINGMODEL.md through GitHub's Markdown API and verified the new maintainer-gated verified_models.py warning appears before the provider-list guidance.
Does this PR achieve its stated goal?
Yes. The goal was to clarify that contributors should not edit verified_models.py unless a maintainer explicitly asks, while preserving provider-specific list guidance for that requested case. The rendered base docs did not contain the new warning; the rendered PR docs contain the new “Do NOT add…” warning, maintainer-approval language, and the renamed provider-list note in the expected order.
| Phase | Result |
|---|---|
| Environment Setup | ✅ Docs-only change; fetched main and verified checkout at faf2a826e9c2e4e03879edafa1e1299fbe173615. |
| CI Status | REST API (OpenAPI)), 9 pending, 1 skipped. I did not rerun tests. |
| Functional Verification | ✅ Rendered the document before/after with GitHub-flavored Markdown and verified the visible text and order. |
Functional Verification
Test 1: GitHub-rendered documentation shows the new warning before provider guidance
Step 1 — Establish baseline without the PR change:
Ran git fetch origin main openhands/update-addingmodel-verified-list-guidance --quiet, rendered origin/main:.github/run-eval/ADDINGMODEL.md with gh api -X POST markdown --input /tmp/base_addingmodel_markdown.json, then inspected the rendered text:
base: old_provider_note=True; new_provider_note=False; do_not_note=False
This shows the baseline docs rendered the old provider-specific warning, but did not render the new “Do NOT add…” maintainer-gated guidance.
Step 2 — Apply the PR's changes:
Used the checked-out PR branch at commit faf2a826e9c2e4e03879edafa1e1299fbe173615 and rendered HEAD:.github/run-eval/ADDINGMODEL.md through the same GitHub Markdown API path.
Step 3 — Re-run with the PR change in place:
Ran the same rendered-text inspection against the PR output:
head: old_provider_note=False; new_provider_note=True; do_not_note=True
new_note_index=2297; provider_note_index=2769; new_before_provider=True
snippet: Do NOT add a model to verified_models.py unless explicitly asked to. "Verified" means the model has been validated against the OpenHands integration test suite and an OpenHands maintainer has approved it for the production UI. A passing integration run is necessary but not sufficient . New models should be added to MODELS in resolve_model_config.py (and model_features.py if applicable) only — leave verified_models.py alone until a maintainer requests it in the PR. ⚠️ When you are explicitly asked to edit verified_models.py : If you add a model to VERIFIED_OPENH
This confirms the rendered PR docs include the new maintainer-approval warning and place it before the provider-specific list note, matching the PR description.
Issues Found
None.
This QA review was created by an AI agent (OpenHands) on behalf of the user.
…odels.py (OpenHands#3479) Co-authored-by: openhands <openhands@all-hands.dev>
Why
When adding a new model via
resolve_model_config.py, contributors sometimes also add the model toverified_models.py. That elevates the model to "verified" in the production UI, which should require maintainer review — a passing integration run alone is not sufficient.Reviewers on PR #3315 asked for this clarification to be added to
ADDINGMODEL.mdso future PRs don't conflate "added for eval" with "verified for production".Summary
.github/run-eval/ADDINGMODEL.mdto make clear thatverified_models.pyshould not be edited unless a maintainer explicitly asks for it.VERIFIED_ANTHROPIC_MODELS,VERIFIED_GEMINI_MODELS, etc.) for the case where editingverified_models.pyis requested.Issue Number
N/A — extracted from #3315.
How to Test
Docs-only change. Render the file and confirm the new ⛔ note appears before the existing⚠️ provider-list note.
Type
Notes
This PR is one of three that replace #3315 (which is being closed):
ADDINGMODEL.mdguidance updategemini-3.5-flashmodel addition (separate PR, depends on this guidance)This PR was created by an AI agent (OpenHands) on behalf of @juanmichelini.
@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.13-nodejs22-slimgolang: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:faf2a82-pythonRun
All tags pushed for this build
About Multi-Architecture Support
faf2a82-python) is a multi-arch manifest supporting both amd64 and arm64faf2a82-python-amd64) are also available if needed