Remove the dead execute_parallel and execute_sequential methods - #271
Open
Nitjsefnie wants to merge 2 commits into
Open
Remove the dead execute_parallel and execute_sequential methods#271Nitjsefnie wants to merge 2 commits into
Nitjsefnie wants to merge 2 commits into
Conversation
Neither method has any production caller; the pipeline uses Executor.gather(). Their only in-tree references were two dedicated unit tests, which are removed alongside them. Also drops the now-unused Coroutine import. Closes OpenAgentHQ#52. Co-Authored-By: Kimi K3 <noreply@kimi.com>
Co-Authored-By: Kimi K3 <noreply@kimi.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes
Executor.execute_parallelandExecutor.execute_sequential, which nothing in the package calls, along with the two unit tests that were their only callers.Related Issues and Pull Requests
Fixes #52
Changes
openagent_eval/core/executor.py— deletes both methods.tests/unit/test_core/test_engine.py— deletestest_execute_parallelandtest_execute_sequential.CHANGELOG.md— entry under[Unreleased]→### Removed, following fix: use metadata parameter in ChunkingQualityAnalyzer (#65) #204's shape.Chose deletion over the TODO marker your issue also offered. The precedent is this repo's own: #75 and #77 were both dead-code findings resolved by deletion and merged. There's no TODO-marker precedent to follow, the methods appear in no docs, examples, changelog or release notes, and the package is pre-1.0 with no deprecation convention. Happy to switch to markers if you'd rather keep them as an intended API.
Testing
One correction to the issue worth stating plainly: "never called anywhere" isn't quite right — they had two dedicated unit tests. No production caller exists, which is what makes deletion correct, but
tests/unit/test_core/test_engine.pycalled them at four points, so those tests had to go with the methods or the suite wouldAttributeError. That's why the unit count drops by exactly two.Verified as an enumeration rather than a spot check. Post-change,
grep -rn "execute_parallel\|execute_sequential"over the whole tree — source, tests, docs, examples, scripts, config — returns exactly one hit: the CHANGELOG entry itself. Ahasattrcheck on the class confirms both are gone.pytest tests/unitexecutor.pygoes 55 stmts/85% → 33 stmts/100%mkdocs build --strictpython -m build+twine checkThe soft gates add no new findings — checked at finding level against a base worktree rather than by comparing totals: ruff 224 vs 228 at base, format clean, mypy unchanged, integration 54 passed.
Footer
Generated by Claude Opus 5 (review), Kimi K3 (brief, implementation, verification)