DeepBoner / docs /specs /archive /SPEC_05_ORCHESTRATOR_CLEANUP.md
VibecoderMcSwaggins's picture
feat(search): SPEC_13 Evidence Deduplication (#98)
2c5db87 unverified
|
raw
history blame
4.38 kB

SPEC 05: Orchestrator Module Cleanup

Priority: P3 (Code Hygiene)

Problem Statement

The codebase has an inconsistent orchestrator organization:

src/
β”œβ”€β”€ orchestrator/              # EMPTY folder (just . and ..)
β”œβ”€β”€ orchestrator.py            # Simple mode (15KB, 67% coverage)
β”œβ”€β”€ orchestrator_factory.py    # Factory pattern (2.5KB, 87% coverage)
β”œβ”€β”€ orchestrator_hierarchical.py  # Unused (3KB, 0% coverage)
└── orchestrator_magentic.py   # Advanced mode (11KB, 68% coverage)

Related Issues

  • GitHub Issue #67: Clean up empty src/orchestrator/ folder

Analysis

Empty Folder

The src/orchestrator/ folder was created but never populated. All orchestrator implementations remain flat in src/.

Dead Code

orchestrator_hierarchical.py has 0% test coverage and appears to be an early prototype that was never integrated:

  • Not imported anywhere in production code
  • Not referenced in any tests
  • Pattern doesn't match current architecture

Import Pattern

All 30+ imports use the flat structure:

from src.orchestrator import Orchestrator
from src.orchestrator_factory import create_orchestrator
from src.orchestrator_magentic import MagenticOrchestrator

Options

Option A: Minimal Cleanup (Recommended)

Delete the empty folder and dead code:

rm -rf src/orchestrator/
rm src/orchestrator_hierarchical.py

Pros: Zero import changes, minimal risk, quick Cons: Flat structure remains

Option B: Full Consolidation (Future)

Move everything into a proper module:

src/orchestrator/
β”œβ”€β”€ __init__.py        # Re-export for backwards compat
β”œβ”€β”€ base.py            # Shared protocols/types
β”œβ”€β”€ simple.py          # From orchestrator.py
β”œβ”€β”€ magentic.py        # From orchestrator_magentic.py
└── factory.py         # From orchestrator_factory.py

Pros: Cleaner organization, better separation Cons: 30+ import changes, risk of breakage, time investment

Option C: Hybrid (Pragmatic)

Delete empty folder + dead code now. Create src/orchestrator/__init__.py that re-exports from flat files:

# src/orchestrator/__init__.py
from src.orchestrator import Orchestrator
from src.orchestrator_factory import create_orchestrator
from src.orchestrator_magentic import MagenticOrchestrator

__all__ = ["Orchestrator", "create_orchestrator", "MagenticOrchestrator"]

Problem: This creates confusing import semantics (src.orchestrator would be both a module and a file).

Recommendation

Option A for now. The flat structure works fine and changing it provides no functional benefit. The empty folder and dead code should be removed.

Option B can be revisited post-hackathon when there's time for a proper refactor.

Implementation

Step 1: Remove Empty Folder

rm -rf src/orchestrator/

Step 2: Remove Dead Code (Optional)

rm src/orchestrator_hierarchical.py

If keeping for reference, add a deprecation notice:

# src/orchestrator_hierarchical.py
"""
DEPRECATED: Unused hierarchical orchestrator prototype.
Kept for reference only. See orchestrator.py (simple) or
orchestrator_magentic.py (advanced) for active implementations.
"""

Step 3: Verify

make check  # All 142 tests should pass

Acceptance Criteria

  • Empty src/orchestrator/ folder deleted
  • No broken imports (grep for from src.orchestrator/)
  • Tests pass (154 unit tests)
  • orchestrator_hierarchical.py removed

Status: IMPLEMENTED (commit cb46aac)

Files to Modify

  1. src/orchestrator/ - DELETE (empty folder)
  2. src/orchestrator_hierarchical.py - DELETE or add deprecation notice

Test Plan

# Verify nothing imports from the folder path
grep -r "from src.orchestrator/" src tests
# Should return nothing

# Verify nothing imports hierarchical
grep -r "orchestrator_hierarchical" src tests
# Should return nothing (except possibly this spec)

# Run full test suite
make check

Risk Assessment

Action Risk Mitigation
Delete empty folder None It's empty, nothing uses it
Delete hierarchical.py Low 0% coverage, no imports
Full consolidation Medium Many import changes

Time Estimate

  • Option A: 5 minutes
  • Option B: 1-2 hours (plus testing)