Remove unused SWE-Bench and SWT-Bench build agent-type flags#696
Remove unused SWE-Bench and SWT-Bench build agent-type flags#696all-hands-bot merged 1 commit intomainfrom
Conversation
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable - Clean removal of dead code with appropriate compatibility handling.
This PR correctly removes an unused parameter that was parsed but never referenced in the actual build logic. The workflow compatibility layer is pragmatic. See inline comments for minor notes.
| ) | ||
| build_builder_image.assert_called_once_with(push=False, force_build=False) | ||
| build_all_base_images.assert_called_once() | ||
| assemble_all_agent_images.assert_called_once() |
There was a problem hiding this comment.
🟡 Suggestion: This test uses 5 mocks and only verifies function calls, which doesn't catch regressions in actual behavior. However, this is consistent with the existing test patterns in this file (58 total @patch decorators) and is pragmatic given that running real Docker builds in tests would be impractical.
The test does verify that main() completes successfully without the removed parameter, which is the key regression we want to catch.
| default: '' | ||
| agent-type: | ||
| description: 'Agent type: default (skip ACP), acp-claude, acp-codex (keep ACP)' | ||
| description: 'Deprecated compatibility input. Ignored for SWE-Bench image builds.' |
There was a problem hiding this comment.
🟢 Acceptable: Good backward compatibility strategy. Keeping the workflow input with a clear deprecation message ensures external callers won't break, while making it clear this parameter is no longer used.
This is pragmatic engineering - the CLI change is breaking (intentionally), but the workflow API stays compatible.
simonrosenberg
force-pushed
the
simon/remove-unused-swebench-agent-type
branch
2 times, most recently
from
4569362 to
e05f73c
Compare
simonrosenberg
changed the title
simonrosenberg
force-pushed
the
simon/remove-unused-swebench-agent-type
branch
from
e05f73c to
d62ddb5
Compare
3 participants
Summary
--agent-typeargument frombenchmarks/swebench/build_images.pyagent-typeinto the SWE-Bench and SWT-Bench image build commandsagent-typeworkflow inputs from the SWE-Bench and SWT-Bench build workflowsagent_typehandling untouched where ACP behavior is still realTesting
python3YAML parse of.github/workflows/build-swebench-images.ymlpython3YAML parse of.github/workflows/build-swtbench-images.ymlPYTHONPATH=/Users/simonrosenberg/worktrees/benchmarks-remove-unused-agent-type /Users/simonrosenberg/repositories/benchmarks/.venv/bin/pytest /Users/simonrosenberg/worktrees/benchmarks-remove-unused-agent-type/tests/test_phased_build.pyRelated to #695 and paired with OpenHands/evaluation#526.