Rename FeatureBench-branded outer loop classes to generic names - #1449
Open
lukeinglis wants to merge 1 commit into
Open
lukeinglis wants to merge 1 commit into
lukeinglis wants to merge 1 commit into
Conversation
Collaborator
Author
Factory CEO Review — KEEP RecommendationWhy the gate reverted (technical issues, not quality):
QA results (all passed):
What was done:
Resolves #1421. |
lukeinglis
marked this pull request as ready for review
September 4, 2026 03:46
|
✅ Conflicts resolved This PR no longer has merge conflicts with |
lukeinglis
force-pushed
the
factory/run-1618e1c1
branch
from
September 4, 2026 16:29
824e92f to
e565e96
Compare
Sentrux Quality ReportAbsoluteDiff (vs base branch) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1449 +/- ##
=======================================
Coverage 83.59% 83.59%
=======================================
Files 225 225
Lines 25398 25398
Branches 4128 4128
=======================================
Hits 21232 21232
- Misses 3202 3203 +1
+ Partials 964 963 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collaborator
Author
|
@ceo-review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Factory Review: KEEP
Verdict: KEEP
Reason: QA: CLEAN — 601 tests pass, composite score 0.963, all 7 code review categories PASS, 13/13 adversarial criteria verified, 0 issues found
QA Analysis
Deep-QA Verification Report — PR #1449
PR: Rename FeatureBench-branded outer loop classes to generic names
Pipeline Results
| Specialist | Verdict | Key Findings |
|---|---|---|
| Health Checker | ✅ PASS | 601 tests pass, composite score 0.963, all primary dimensions at 1.0, no regression |
| Code Reviewer | ✅ CLEAN (7/7) | All categories pass. 1 minor style nit (docs text carryover). Spec fidelity 10/10 |
| Adversarial Tester | ✅ PASS (13/13) | All import paths work (new + shim), class identity confirmed, 19/19 targeted tests pass, codebase grep clean |
Health Check Detail
| Dimension | Score |
|---|---|
| tests | 1.000 |
| lint | 1.000 |
| type_check | 1.000 |
| coverage | 1.000 |
| observability | 0.556 |
| Composite | 0.963 |
Code Review — 7-Category Checklist
| # | Category | Result |
|---|---|---|
| 1 | Correctness | ✅ PASS — exact logic copies, shims alias correctly, all imports updated |
| 2 | Security | ✅ PASS — pure rename, no security-relevant changes |
| 3 | Edge Cases | ✅ PASS — shims have all exports |
| 4 | Missing Tests | ✅ PASS — test file renamed, imports from canonical module |
| 5 | Style & Consistency | ✅ PASS — 1 minor nit: docs carryover text |
| 6 | Scope Compliance | ✅ PASS — 10/10 criteria met |
| 7 | Guardrails | ✅ PASS — all files under limits |
Adversarial Testing — Evidence Summary
- New imports verified:
BenchmarkEvaluator,BenchmarkInnerLoopimport cleanly - Backward-compat shims verified:
FeatureBenchEvaluator is BenchmarkEvaluator→ True - Functional equivalence:
get_info(),parse(),parse_pytest_stdout()produce identical results - Codebase grep: No orphaned references;
DirectFeatureBenchEvaluatorcorrectly left unchanged (out of scope) - Test suites: 19/19 targeted tests pass (14 evaluator + 5 backward-compat)
- Log strings:
benchmark_step/benchmark_step_doneconfirmed; no oldfeaturebench_stepstrings remain - Dynamic resolution:
EvaluatorRef("pytest")resolves through shim toBenchmarkEvaluator
Final Verdict
QA: CLEAN — All three specialists report PASS. No issues found. The rename refactoring is complete, correct, and backward-compatible.
Posted by Factory CEO
Rebase of #1449 scoped to the evaluator rename only. The inner-loop half of the original PR (FeatureBenchInnerLoop → BenchmarkInnerLoop) is dropped because main removed the class entirely in #1458 (Unified Task.run()); re-adding it would regress that refactor. - git mv featurebench_evaluator.py → benchmark_evaluator.py (class FeatureBenchEvaluator → BenchmarkEvaluator) - Backward-compat shim at old path (assignment alias + __all__) - Update parse_pytest_stdout imports in inner_loop.py and direct_evaluator.py - Rename test file and test class; test_multi_benchmark_e2e.py backward-compat alias test continues to validate the shim - Update CLAUDE.md, docs/outer-loop.md, codecov.yml - Also fix stale references to featurebench_inner_loop.py left on main by #1458 (CLAUDE.md module list, codecov.yml ignore list) - DirectFeatureBenchEvaluator and 'featurebench' benchmark name string unchanged (genuinely FeatureBench-specific) Closes #1421 Signed-off-by: Luke Inglis <lukeinglis21@yahoo.com>
lukeinglis
force-pushed
the
factory/run-1618e1c1
branch
from
September 10, 2026 15:07
e565e96 to
91d3474
Compare
Collaborator
Author
|
Rebased onto current Why the inner-loop half was dropped: #1458 (Unified Task.run()) deleted What's verified locally:
Also fixed two stale references #1458 left on main ( |
Collaborator
Author
|
@ceo-review |
There was a problem hiding this comment.
✅ Factory Review: KEEP
Verdict: KEEP
Reason: QA: CLEAN — 615 tests pass, 0 issues. Health check: composite 0.9629, lint/mypy clean. Code review: 7/7 PASS, zero logic delta, correct backward-compat shim. Adversarial: 7/7 VERIFIED with evidence — imports, tests, grep all confirm clean rename.
QA Analysis
Adversarial QA — PR #1449: Rename FeatureBenchEvaluator → BenchmarkEvaluator
Detected project type: Library (Python package with CLI)
Date: 2026-09-10
Smoke Test
Command:
uv run pytest tests/test_models.py tests/test_guards.py tests/test_runners.py -x -q --tb=short -k 'not (BobAuth or preflight_error_unchanged)'
Result: ✅ 165 passed in 4.93s
Test Plan (derived from PR acceptance criteria)
| # | Criterion | Status |
|---|---|---|
| 1 | Import BenchmarkEvaluator from new path works |
VERIFIED |
| 2 | Import FeatureBenchEvaluator from old path works via shim |
VERIFIED |
| 3 | parse_pytest_stdout importable from both old and new paths |
VERIFIED |
| 4 | Renamed test file passes: test_benchmark_evaluator.py |
VERIFIED |
| 5 | Backward compat test passes: test_backward_compat_import_alias |
VERIFIED |
| 6 | No stale direct imports from featurebench_evaluator (except shim) |
VERIFIED |
| 7 | inner_loop.py and direct_evaluator.py import from new path |
VERIFIED |
Feature Tests — Evidence
Test 1: Import BenchmarkEvaluator from new path
Command:
uv run python -c "from factory.outer_loop.benchmark_evaluator import BenchmarkEvaluator; e = BenchmarkEvaluator(); print('OK:', type(e).__name__, e.get_info())"Output:
OK: BenchmarkEvaluator {'benchmark': 'featurebench', 'scoring': 'partial_credit', 'metrics': ['tests_passed', 'tests_total', 'pass_rate']}
Status: VERIFIED ✅
Test 2: Import FeatureBenchEvaluator from old path via shim
Command:
uv run python -c "from factory.outer_loop.featurebench_evaluator import FeatureBenchEvaluator; e = FeatureBenchEvaluator(); print('OK:', type(e).__name__, e.get_info())"Output:
OK: BenchmarkEvaluator {'benchmark': 'featurebench', 'scoring': 'partial_credit', 'metrics': ['tests_passed', 'tests_total', 'pass_rate']}
Note: type(e).__name__ is BenchmarkEvaluator (the canonical name), confirming FeatureBenchEvaluator is an alias, not a copy. This is correct behavior.
Status: VERIFIED ✅
Test 3: parse_pytest_stdout importable from both paths
Command:
uv run python -c "
from factory.outer_loop.benchmark_evaluator import parse_pytest_stdout as new_fn
from factory.outer_loop.featurebench_evaluator import parse_pytest_stdout as old_fn
r1 = new_fn('3 passed, 1 failed')
r2 = old_fn('3 passed, 1 failed')
print('new path:', r1)
print('old path:', r2)
assert r1 == r2, 'Mismatch!'
print('OK: Both paths return identical results and are the same function:', new_fn is old_fn)
"Output:
new path: {'tests_passed': 3.0, 'tests_total': 4.0, 'pass_rate': 0.75}
old path: {'tests_passed': 3.0, 'tests_total': 4.0, 'pass_rate': 0.75}
OK: Both paths return identical results and are the same function: True
Status: VERIFIED ✅
Test 4: Renamed test file passes
Command:
uv run pytest tests/test_outer_loop/test_benchmark_evaluator.py -vOutput:
14 passed in 0.22s
All 14 tests pass (10 TestBenchmarkEvaluator + 4 TestParsePytestStdout).
Status: VERIFIED ✅
Test 5: Backward compat test passes
Command:
uv run pytest tests/test_outer_loop/test_multi_benchmark_e2e.py -v -k 'backward_compat'Output:
tests/test_outer_loop/test_multi_benchmark_e2e.py::TestSwarmConfigMultiBenchmark::test_default_values_backward_compat PASSED
tests/test_outer_loop/test_multi_benchmark_e2e.py::TestE2EFeatureBenchBackwardCompat::test_backward_compat_import_alias PASSED
2 passed, 65 deselected in 0.27s
Status: VERIFIED ✅
Test 6: No stale direct imports from featurebench_evaluator
Command:
grep -rn "from factory.outer_loop.featurebench_evaluator import\|import factory.outer_loop.featurebench_evaluator" --include="*.py" . | grep -v "featurebench_evaluator.py" | grep -v "test_multi_benchmark_e2e.py"Output: (empty — no stale imports)
Breakdown of all featurebench_evaluator references in codebase:
factory/outer_loop/featurebench_evaluator.py— the shim itself (expected)tests/test_outer_loop/test_multi_benchmark_e2e.py:458— backward compat test that deliberately imports old name (expected)factory/outer_loop/evaluators/pytest_evaluator.py:3— docstring comment "Moved from featurebench_evaluator.py" (harmless)
No production code imports from the old path. All production imports use benchmark_evaluator.
Status: VERIFIED ✅
Test 7: inner_loop.py and direct_evaluator.py import from new path
Command:
grep -n "benchmark_evaluator\|featurebench_evaluator" factory/inner_loop.pyOutput:
535: from factory.outer_loop.benchmark_evaluator import parse_pytest_stdout
Command:
grep -n "benchmark_evaluator\|featurebench_evaluator" factory/outer_loop/direct_evaluator.pyOutput:
445: from factory.outer_loop.benchmark_evaluator import parse_pytest_stdout
Both files import exclusively from benchmark_evaluator (the new canonical path). No references to featurebench_evaluator remain.
Status: VERIFIED ✅
Shim Correctness Verification
The shim file (factory/outer_loop/featurebench_evaluator.py) is exactly 6 lines:
"""Backward-compat shim. Canonical: factory.outer_loop.benchmark_evaluator."""
from factory.outer_loop.benchmark_evaluator import BenchmarkEvaluator, parse_pytest_stdout
FeatureBenchEvaluator = BenchmarkEvaluator
__all__ = ["FeatureBenchEvaluator", "parse_pytest_stdout"]This correctly:
- Re-exports
BenchmarkEvaluatorunder the old nameFeatureBenchEvaluator - Re-exports
parse_pytest_stdoutfor old import paths - Uses alias (not copy) so
isinstancechecks and identity hold
Adversarial Verdict: PASS ✅
All 7 acceptance criteria verified with evidence. The rename is clean:
- New canonical path works
- Old import path preserved via minimal shim
- No stale references in production code
- All tests pass (14 evaluator tests + 2 backward compat tests)
Posted by Factory CEO
colehurwitz
added a commit
that referenced
this pull request
Sep 11, 2026
…r loop (#1478) Phase 1 of the task→workflow→outer-loop creation pipeline. - Add resolve_task() to factory/task.py: 3-step resolution (TOML file → Python file → module:Class string) with project-relative path support - Add --task flag to both calibrate and evaluate subcommands in factory/cli/outer_loop.py, using the shared resolver (one helper, not two — lesson from #1449) - Keep --task-module as a functional escape hatch - Resolved tasks attach via set_task() for highest precedence in SwarmConfig.get_task() - 12 new tests covering all resolution paths and equivalence - Updated docs/outer-loop.md with --task documentation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.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.
Closes #1421
Changes
git mvfeaturebench_evaluator.py→benchmark_evaluator.py(classFeatureBenchEvaluator→BenchmarkEvaluator)FeatureBenchEvaluator = BenchmarkEvaluator) with__all__parse_pytest_stdoutimports infactory/inner_loop.pyandfactory/outer_loop/direct_evaluator.pytest_featurebench_evaluator.py→test_benchmark_evaluator.pyand test classTestFeatureBenchEvaluator→TestBenchmarkEvaluatortest_backward_compat_import_aliasintest_multi_benchmark_e2e.pyleft unchanged (validates the shim)CLAUDE.md,docs/outer-loop.md,codecov.ymlStale-reference cleanup (missed by #1458)
mainstill referenced the deletedfeaturebench_inner_loop.py:CLAUDE.mdmodule list — replaced with abenchmark_evaluator.pyentryCLAUDE.mdevaluator.py description mentionedFeatureBenchInnerLoop— updated toInnerLoopcodecov.ymlignore list — stale entry replaced with the shim pathDropped from original PR (superseded by #1458)
featurebench_inner_loop.py→benchmark_inner_loop.pyrename and shim: feat: Unified Task.run() — wire Task into InnerLoop and Outer Loop (#1448) #1458 (Unified Task.run()) deleted the class from main; evaluation now goes throughInnerLoopdirectly viacompose(). Re-adding it under a new name would regress that refactor.benchmark_step/benchmark_step_donelog event renames (moved with the deleted class)Not changed (by design):
DirectFeatureBenchEvaluator— genuinely FeatureBench-specific"featurebench"benchmark name string in CLI, configs, andget_info()benchmarks/configs/featurebench.tomland related FeatureBench artifacts