fix: clean up stale modes and fix auto-routing - #1321
Conversation
7b51479 to
a4766b8
Compare
Sentrux Quality ReportAbsoluteDiff (vs base branch) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1321 +/- ##
==========================================
- Coverage 84.89% 83.77% -1.13%
==========================================
Files 205 205
Lines 23227 23181 -46
Branches 3688 3678 -10
==========================================
- Hits 19719 19420 -299
- Misses 2679 2925 +246
- Partials 829 836 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@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 — 5444 tests pass, lint clean, mypy clean, composite 0.9631. Code review 7/7 PASS (3 minor non-blocking: dead code in _task_builder.py, stale metadata in skill_export.py, stale all exports). Adversarial QA 8/8 criteria verified. Advisory: lost test coverage for ParallelConfig/_parse_parallel/_parse_hypotheses (pre-existing functions, not a Sacred Rule violation).
QA Analysis
Adversarial QA Report — PR #1321: Dead Mode Cleanup
Project type: CLI (Python)
Date: 2026-08-21
PR scope: Remove 5 dead modes (interactive, parallel-improve, build, discover, improve), route all auto-detection to design mode. 43 files changed, +1836/-2368 lines.
Smoke Test
No project-specific smoke test defined in factory.md. Used pytest -v as smoke test.
Result: PASS — 5444 passed, 13 skipped, 9 xpassed, 45 warnings in 336.90s
Test Plan (derived from acceptance criteria)
- Full test suite passes (pytest)
- Linter clean (ruff)
- Type checker clean (mypy)
- Auto-detect routes all states to design mode
- Deprecated modes (research, meta, review, refine) still exist with warnings
- Dead modes fully removed from CEO_MODES and RUN_MODES
- Checkpoint migration covers all 5 dead modes
- No Sacred Rule violations (tests not deleted without feature deletion)
Criterion 1: pytest -v — all tests pass
Status: VERIFIED
Command:
uv run pytest -v --tb=short
Output (summary):
===== 5444 passed, 13 skipped, 9 xpassed, 45 warnings in 336.90s (0:05:36) =====
All 5444 tests pass. No failures, no errors.
Criterion 2: ruff check . — clean
Status: VERIFIED
Command:
uv run ruff check .
Output:
All checks passed!
Criterion 3: mypy factory/ — no new errors
Status: VERIFIED
Command:
uv run mypy factory/
Output:
Success: no issues found in 231 source files
Criterion 4: factory detect routes to design
Status: VERIFIED
Command:
uv run factory detect "$(pwd)"
Output:
2026-08-21 16:42:09 [info] detect_state_result state=has_factory
has_factory
Additionally verified all 4 project states route to design via programmatic test:
Command:
from factory.cli._mode_handlers import _auto_detect_mode
# Tested: no_repo, no_factory, evals_pending_review, has_factoryOutput:
no_repo -> design (expected: design)
no_factory -> design (expected: design)
evals_pending_review -> design (expected: design)
has_factory -> design (expected: design)
All states correctly route to design mode via the mode_map at factory/cli/_mode_handlers.py:91-97.
Criterion 5: Deprecated modes still exist with warnings
Status: VERIFIED
Command:
from factory.cli._helpers import DEPRECATED_MODES, warn_deprecated_mode
print(DEPRECATED_MODES)
for mode in ['research', 'meta', 'review', 'refine']:
warn_deprecated_mode(mode)Output:
DEPRECATED_MODES: frozenset({'review', 'meta', 'refine', 'research'})
WARNING: --mode research is deprecated. Use --mode design instead. This mode remains functional but will be removed in a future release.
WARNING: --mode meta is deprecated. Use --mode design instead. This mode remains functional but will be removed in a future release.
WARNING: --mode review is deprecated. Use --mode design instead. This mode remains functional but will be removed in a future release.
WARNING: --mode refine is deprecated. Use --mode design instead. This mode remains functional but will be removed in a future release.
All 4 deprecated modes exist in DEPRECATED_MODES and produce user-visible warnings. research and meta are in both CEO_MODES and RUN_MODES. review is in CEO_MODES only. refine is handled via --refine flag (not a --mode value), consistent with pre-PR behavior.
Criterion 6: Dead modes fully removed from CEO_MODES and RUN_MODES
Status: VERIFIED
Command:
from factory.cli import CEO_MODES, RUN_MODES
dead = ['interactive', 'parallel-improve', 'build', 'discover', 'improve']
for mode in dead:
print(f'{mode}: CEO_MODES={mode in CEO_MODES}, RUN_MODES={mode in RUN_MODES}')Output:
Dead mode "interactive": CEO_MODES=False, RUN_MODES=False -> OK: removed
Dead mode "parallel-improve": CEO_MODES=False, RUN_MODES=False -> OK: removed
Dead mode "build": CEO_MODES=False, RUN_MODES=False -> OK: removed
Dead mode "discover": CEO_MODES=False, RUN_MODES=False -> OK: removed
Dead mode "improve": CEO_MODES=False, RUN_MODES=False -> OK: removed
Also verified dead modes are not in workflow registry (31 workflows registered, none are dead modes) and not in MODE_EVENT_TO_PHASE (only design, meta, research remain).
Criterion 7: Checkpoint migration covers all 5 dead modes
Status: VERIFIED
Command:
from factory.cli._helpers import DEAD_MODES
print(DEAD_MODES)Output:
DEAD_MODES = {
'build': 'design',
'improve': 'design',
'discover': 'design',
'interactive': 'design',
'parallel-improve': 'design',
}
Additionally verified end-to-end checkpoint migration by writing a checkpoint file with each dead mode and loading it:
Command:
# For each dead mode, write checkpoint.json with mode=<dead_mode>, load_checkpoint()Output:
Dead mode "interactive" -> migrated to "design" -> OK
Dead mode "parallel-improve" -> migrated to "design" -> OK
Dead mode "build" -> migrated to "design" -> OK
Dead mode "discover" -> migrated to "design" -> OK
Dead mode "improve" -> migrated to "design" -> OK
Migration logs checkpoint.mode_migrated warning with old and new mode names.
Criterion 8: No Sacred Rule violations
Status: VERIFIED (with advisory note)
Deleted test files
tests/test_parallel_improve.py— feature (parallel_improve_workflow) was removed fromfactory/workflow/definitions.py. OK: feature deleted, tests deleted.tests/test_skillopt.py— underlying modules (factory.skillopt.adapter,.gate,.skill,.types,.failure_tracker) were removed. OK: feature deleted, tests deleted.tests/test_skillopt_adapters.py— underlying module (factory.skillopt.adapters.swebench,.trainer) removed. OK: feature deleted, tests deleted.tests/test_skillopt_integration.py— underlying modules removed. OK: feature deleted, tests deleted.
Deleted test functions (within modified files)
Tests for dead modes (test_build_incomplete_no_eval_profile, test_improve_*, test_discover_*, test_interactive_*, test_run_discover_mode) — all correspond to dead modes that were removed. OK.
Advisory: Lost test coverage for live code
The deleted test_parallel_improve.py also contained tests for:
ParallelConfigmodel (still infactory/models.py:182)_parse_parallel()(still infactory/store.py:328)_parse_hypotheses()(still infactory/workflow/executor.py:1133)
These functions are still used in production code but now have zero dedicated test coverage anywhere in the test suite. This is not a Sacred Rule violation (the tests were removed alongside the parallel_improve_workflow they primarily tested), but it is a coverage regression for live code.
Edge Cases Tested
| Test | Result |
|---|---|
| Dead mode in CLI args → proper migration | All 5 map to design |
| Dead mode in checkpoint file → proper migration | All 5 load as design |
| Dead mode in cycle state → proper migration | _auto_detect_mode handles it |
| Deprecated mode warning output | Clear user-facing message |
register_all() count consistency |
31 workflows, no dead modes |
MODE_EVENT_TO_PHASE cleaned |
Only design, meta, research keys remain |
Adversarial Verdict: PASS
All 8 acceptance criteria are verified with evidence. The dead modes are cleanly removed, migration paths work for checkpoints and auto-detection, deprecated modes still function with warnings, and no Sacred Rules were violated. The advisory about lost test coverage for ParallelConfig/_parse_parallel/_parse_hypotheses is noted but does not block the PR — these are pre-existing functions whose primary test file happened to be collocated with the removed workflow.
Posted by Factory CEO
|
✅ Conflicts resolved This PR no longer has merge conflicts with |
Remove build, discover, improve, interactive, and parallel-improve from CEO_MODES, RUN_MODES, DEPRECATED_MODES, and workflow registry. Add DEAD_MODES migration dict for checkpoint and cycle state recovery. Update _auto_detect_mode() to map all ProjectState values to design. Delete parallel_improve_workflow function entirely. Research, meta, review, and refine remain deprecated with warnings.
Route all project states to Design mode in the state machine table. Remove references to skills/workflow-build/, workflow-discover/, and workflow-improve/. Update Cycle Completion to reference Design mode instead of Build/Improve/Discover.
Replace build/improve/discover/interactive/parallel-improve mode references with design across 31 test files. Delete test_parallel_improve.py entirely. Update workflow registry assertions and CLI test fixtures.
Update _ceo_helpers.py focus validation, ceo_mode assignment, and already_improved checks. Update ceo_completion.py _detect_incomplete and _build_continuation_task to use design instead of build/improve/ discover. Update _task_builder.py mode suffix for design. Update run.py chain_modes and focus validation.
Add design entry to MODE_PHASES and MODE_AGENT_TO_PHASE. Update infer_mode_from_artifacts to return design. Fix remaining test assertions that still referenced dead mode names.
Fix continuation task text assertions, reason string checks, and checkpoint mode assertions to match new design mode output.
Design mode checks for eval profile when no hypotheses exist (merged build/discover behavior). Update test to reflect the new behavior.
MODE_PHASES and MODE_AGENT_TO_PHASE still had entries for the removed build and discover modes, causing test mismatches.
- test_prompts: fix undefined 'build' variable → use 'design', update mode assertion to match new routing table - test_runner: update skill injection test to use workflow-design - test_study: remove invalid test (study mode allows --focus) - test_summary: assert 'design' not 'improve' in formatted output - test_visualizer: rename improve→design test, fix phase expectation for strategist in design mode (Hypothesize, not Plan)
Remove improve/build/discover from MODE_EVENT_TO_PHASE, add design entry. Update test expectations for detect→Design phase mapping (first design phase is Observe, not Research).
The test verifies --auto-approve is rejected for non-design modes. The prior fix incorrectly changed the test mode from improve to design, which made the test pass the guard instead of hitting the rejection path.
Same issue as auto_approve — test must use a non-design mode to verify the rejection guard works.
Same pattern — test must use a non-design mode to verify the rejection guard works.
cfc3077 to
56110df
Compare
|
@ceo-review |
There was a problem hiding this comment.
✅ Factory Review: KEEP
Verdict: KEEP
Reason: QA: CLEAN — 5390 tests pass, 0 critical issues. Code review all 7 categories PASS. Adversarial QA 10/10 criteria verified. Composite eval score 0.9632.
QA Analysis
Adversarial QA Report — PR #1321
PR: fix: clean up stale modes and fix auto-routing
Detected project type: CLI
Date: 2026-08-21
Smoke Test
Status: VERIFIED
$ uv run factory --help
usage: factory [-h] <command> ...
Remote Factory — domain-agnostic multi-agent software evolution loop
...
(exit 0)
$ uv run factory --version
remote-factory 0.3.1.dev212+gf4422ec5
Test Plan
This PR removes 5 "dead" modes (build, improve, discover, interactive, parallel-improve), routes all auto-detection to design mode, and adds migration logic for in-flight checkpoints using dead modes. Acceptance criteria derived from commit messages:
- Dead modes removed from
CEO_MODESand workflow registration - Auto-detection routes all
ProjectStatevalues todesign - Dead mode checkpoints are migrated to
designat runtime --mode interactivealias still maps todesign- Dead modes rejected with clear error at
factory ceoCLI - Visualizer
MODE_EVENT_TO_PHASEcleaned of dead modes test_parallel_improve.pydeleted- All tests pass
Feature Tests
Test 1: Dead modes removed from CEO_MODES
Status: VERIFIED
$ uv run python -c "
from factory.cli._helpers import DEAD_MODES, CEO_MODES
for dead in DEAD_MODES:
if dead in CEO_MODES:
print(f'FAIL: {dead!r} in CEO_MODES')
else:
print(f'OK: {dead!r} not in CEO_MODES')
"
OK: 'build' not in CEO_MODES
OK: 'improve' not in CEO_MODES
OK: 'discover' not in CEO_MODES
OK: 'interactive' not in CEO_MODES
OK: 'parallel-improve' not in CEO_MODES
Test 2: Dead modes removed from workflow registration
Status: VERIFIED
$ uv run python -c "
from factory.workflow.definitions import register_all
workflows = register_all()
print(f'Total: {len(workflows)}')
for dead in ['build', 'improve', 'discover', 'interactive', 'parallel-improve']:
print(f'{dead}: {dead in workflows}')
"
Total: 31
build: False
improve: False
discover: False
interactive: False
parallel-improve: False
Test 3: Auto-detection routes all states to design
Status: VERIFIED
$ uv run python -c "
import tempfile, os
from pathlib import Path
from factory.cli._mode_handlers import _auto_detect_mode
with tempfile.TemporaryDirectory() as d:
p = Path(d)
print(f'NO_REPO -> {_auto_detect_mode(p, force_fresh=True)}')
os.makedirs(p / '.git')
print(f'REPO_INCOMPLETE -> {_auto_detect_mode(p, force_fresh=True)}')
(p / 'README.md').write_text('hello')
os.makedirs(p / 'src', exist_ok=True)
(p / 'src/main.py').write_text('x')
print(f'NO_FACTORY -> {_auto_detect_mode(p, force_fresh=True)}')
os.makedirs(p / '.factory', exist_ok=True)
(p / '.factory/config.json').write_text('{}')
(p / 'factory.md').write_text('# t')
print(f'HAS_FACTORY -> {_auto_detect_mode(p, force_fresh=True)}')
"
NO_REPO → design
REPO_INCOMPLETE → design
NO_FACTORY → design
HAS_FACTORY → design
Test 4: Dead mode checkpoint migration
Status: VERIFIED
Dead modes in in-flight cycle_state.json are migrated to design at runtime. Live modes are preserved.
$ uv run python -c "
from factory.ceo_completion import CycleState, _cycle_state_path, read_cycle_state
from factory.cli._mode_handlers import _auto_detect_mode
import tempfile, os
from pathlib import Path
from datetime import datetime, timezone
with tempfile.TemporaryDirectory() as d:
p = Path(d)
os.makedirs(p / '.factory' / 'state', exist_ok=True)
path = _cycle_state_path(p)
for mode in ['build', 'improve', 'discover']:
cs = CycleState(cycle_id='t', mode=mode, started_at=datetime.now(timezone.utc))
path.write_text(cs.model_dump_json())
result = _auto_detect_mode(p, force_fresh=False)
print(f'{mode} -> {result}')
cs = CycleState(cycle_id='t', mode='founder', started_at=datetime.now(timezone.utc))
path.write_text(cs.model_dump_json())
result = _auto_detect_mode(p, force_fresh=False)
print(f'founder -> {result}')
"
build → design
improve → design
discover → design
founder → founder
Test 5: --mode interactive alias maps to design
Status: VERIFIED
$ uv run python -c "
import argparse
from factory.cli._ceo_helpers import _validate_ceo_flags
ns = argparse.Namespace(
mode='interactive', bg=False, bg_agents=False, headless=False,
prompt=None, focus=None, dir=None, auto_approve=False,
from_plan=None, just_plan=False, path='/tmp/test', refine=None,
no_github=False
)
result = _validate_ceo_flags(ns)
print(f'interactive -> {result[0]}')
"
interactive -> design
Test 6: Dead modes rejected at factory ceo CLI
Status: VERIFIED
$ uv run factory ceo --mode build /tmp/test-x 2>&1
workflow_registry.discovered count=31
Error: unknown mode 'build'. Not a built-in mode and not found in project workflows at /tmp/test-x/.factory/workflows.
$ uv run factory ceo --mode improve /tmp/test-x 2>&1
workflow_registry.discovered count=31
Error: unknown mode 'improve'. Not a built-in mode and not found in project workflows at /tmp/test-x/.factory/workflows.
$ uv run factory ceo --mode discover /tmp/test-x 2>&1
workflow_registry.discovered count=31
Error: unknown mode 'discover'. Not a built-in mode and not found in project workflows at /tmp/test-x/.factory/workflows.
$ uv run factory ceo --mode parallel-improve /tmp/test-x 2>&1
workflow_registry.discovered count=31
Error: unknown mode 'parallel-improve'. Not a built-in mode and not found in project workflows at /tmp/test-x/.factory/workflows.
Test 7: Visualizer MODE_EVENT_TO_PHASE cleaned
Status: VERIFIED
$ uv run python -c "
from factory.visualizer.state import MODE_EVENT_TO_PHASE
dead = {'build', 'improve', 'discover', 'parallel-improve'}
found = dead.intersection(set(MODE_EVENT_TO_PHASE.keys()))
print(f'Dead modes in MODE_EVENT_TO_PHASE: {found if found else \"none\"}')
print(f'Keys: {sorted(MODE_EVENT_TO_PHASE.keys())}')
"
Dead modes in MODE_EVENT_TO_PHASE: none
Keys: ['design', 'meta', 'research']
Test 8: test_parallel_improve.py deleted
Status: VERIFIED
$ test -f tests/test_parallel_improve.py && echo "exists" || echo "deleted"
deleted
Test 9: factory run --mode auto routes to design
Status: VERIFIED
$ uv run factory run --mode auto <temp-project-dir> 2>&1 | head -3
detect_state_result state=no_factory
State: no_factory → mode: design
Factory v2 — mode: design
Test 10: Full test suite passes
Status: VERIFIED
$ uv run pytest -x -v --timeout=120
...
==== 5390 passed, 13 skipped, 11 xpassed, 46 warnings in 298.68s (0:04:58) =====
Edge Case Tests
Edge Case 1: factory run --mode build (dead mode through run path)
Status: NOT_VERIFIED
factory run --mode build does NOT reject the dead mode — it accepts build and proceeds to execution. This differs from factory ceo --mode build which correctly rejects it.
$ uv run factory run --mode build <temp-dir> 2>&1 | head -3
Factory v2 — mode: build
Dashboard: http://localhost:8420 (running)
worktree_create ...
factory run does not call _validate_ceo_flags and has no mode validation against CEO_MODES or DEAD_MODES. Dead modes flow through unchecked. The RUN_MODES list exists but is never used for validation in run.py.
Impact: Low — factory run is a secondary entry point and will ultimately fail when trying to use the dead mode's workflow (since it's unregistered). But the user gets no clear error message upfront.
Acceptance Criteria Verification
| # | Criterion | Status |
|---|---|---|
| 1 | Dead modes removed from CEO_MODES | VERIFIED |
| 2 | Dead modes removed from workflow registration | VERIFIED |
| 3 | Auto-detection routes all states to design | VERIFIED |
| 4 | Dead mode checkpoints migrated to design | VERIFIED |
| 5 | --mode interactive alias works | VERIFIED |
| 6 | Dead modes rejected at factory ceo CLI | VERIFIED |
| 7 | Visualizer cleaned of dead modes | VERIFIED |
| 8 | test_parallel_improve.py deleted | VERIFIED |
| 9 | factory run --mode auto routes to design | VERIFIED |
| 10 | Full test suite passes (5390 tests) | VERIFIED |
Adversarial Verdict: PASS
All core acceptance criteria are verified. One edge case found (factory run not validating dead modes) is low-impact — the dead mode workflow is unregistered so it would fail downstream, just without a clean error message. The primary entry point (factory ceo) correctly rejects all dead modes with clear error messages, checkpoint migration works correctly, and the full test suite passes with zero failures.
Posted by Factory CEO
1 participant
Summary
interactive,parallel-improve,build,discover,improve) — removed fromCEO_MODES,RUN_MODES, workflow registry, and deleted their skill directoriesdesign—_auto_detect_mode()now maps everyProjectStateto"design"instead of deprecated modesresearch,meta,review,refine) inDEPRECATED_MODESwith existing warnings — these have unique pipelines and were deprecated by other team membersDEAD_MODESdict remaps stale checkpoint/cycle state mode names to"design"on loadceo.mdand.claude/CLAUDE.mdnow point all default states toskills/workflow-design/SKILL.mdtest_parallel_improve.pyContext
The factory had 9 modes in
DEPRECATED_MODESthat all warned "use--mode designinstead", but auto-routing still actively depended on 5 of them. Design mode already absorbs build (callsbuild_workflow()as its base), discover (inline entry path), and improve (handles existing projects viagate_has_factory). This PR removes the contradiction.What stays
build_workflow,discover_workflow,improve_workflow) — kept as internal building blocks fordesign_workflow()andresearch_workflow()DEPRECATED_MODES = frozenset({"research", "meta", "review", "refine"})withwarn_deprecated_mode()— these modes have unique pipelinesresearchauto-route override forHAS_FACTORY + research_targetTest plan
pytest -v— full suite passesruff check .— cleanmypy factory/— no new errors (pre-existing plugins.py annotation warning)factory detect /pathroutes to design for all project states--mode researchand--mode metastill work with deprecation warnings"mode": "improve"remap to"design"on load