docs(spec): MMA quarantine + RAG test decoupling design

docs/superpowers/specs/2026-07-01-mma-quarantine-rag-test-decoupling-design.md

@@ -0,0 +1,192 @@
+# Design: MMA Quarantine + RAG Test Decoupling
+
+**Date:** 2026-07-01
+**Status:** Draft (pending user review)
+**Scope:** Two surgical interventions. (1) Quarantine the MMA automation engine behind a config flag so it stops consuming test-suite time and stops breaking when adjacent code changes. (2) Decouple the RAG tests from the live_gui subprocess + chromadb file locks so the default test batch stops bleeding on RAG.
+
+**Out of scope:** The discussion/session system redesign (owned by the nagent research track). Full removal of MMA code (deferred to a follow-up track if quarantine maintenance becomes painful). Any change to the RAG algorithm itself (`index_file`, `search`, chunking — unchanged). The `conductor/` track system (the `conductor_tech_lead.py` / `project_manager.py` / `dag_engine.py` shared-types layer is preserved as load-bearing).
+
+## Context
+
+### Why this work (grounded in git history, not track docs)
+
+The last 10 days of git history (2026-06-20 through 2026-06-30) show two subsystems consuming disproportionate debugging time:
+
+**RAG test debugging churn (2026-06-27):** ~10 commits chasing RAG test failures — `_get_chromadb()` NameError in dim check, file-lock dim-check failures, silent `index_file` no-ops on missing files, session-scoped subprocess pollution, hotpatched state instead of project-switch. Two "ADDENDUM" reports because the first root-cause diagnosis was wrong. The commits are fighting the *environment* (chromadb file locks under the live_gui subprocess, CWD drift across the spawn boundary, lazy global teardown/rebuild), not the RAG algorithm.
+
+**MMA concurrent tracks sim fix (2026-06-27):** `fix_mma_concurrent_tracks_sim_20260627` — 5 fixes to `mock_concurrent_mma.py` (session_id fallback removal, epic branch catch-all, sprint routing by prompt content) plus a `refresh_from_project` task that was overwriting `self.tracks`. The mock itself is brittle; the production engine is brittle in the same shape.
+
+The user's direction: sunset MMA constructively (quarantine, not delete — full removal is a follow-up if quarantine maintenance hurts), keep RAG but make the tests sane.
+
+### The shared-types finding (why this is quarantine, not removal)
+
+`src/mma.py` is a shared types module, not just the MMA engine. Non-MMA code depends on it:
+- `thinking_parser.py` → `ThinkingSegment` (thinking-trace parsing, used in every AI turn — NOT an MMA feature)
+- `project_manager.py` → `TrackState`, `EMPTY_TRACK_STATE` (the conductor track system — persists `conductor/tracks/<id>/state.toml`)
+- `models.py` → `TrackMetadata` (re-exported as legacy `Metadata` alias)
+- `dag_engine.py` → `Ticket`
+- `conductor_tech_lead.py` → `Ticket`, `TrackDAG`
+
+`dag_engine.py` is shared between the MMA loop AND `conductor_tech_lead.py` (the Tier 2 tech lead system). Full removal would require migrating `ThinkingSegment` and `TrackState`/`TrackMetadata` out of `mma.py` first — a mutation of the shared-types module that risks the conductor track system. That work is deferred to a follow-up track.
+
+### The RAG fragility root cause
+
+The RAG algorithm is ~50 lines (`index_file` + `search` + chunking). The other ~250 lines of `rag_engine.py` are defensive scaffolding for the test/subprocess environment:
+- `_validate_collection_dim_result` uses `shutil.rmtree(ignore_errors=True)` to survive Windows file locks held by the live_gui subprocess
+- `index_file` has a CWD-fallback band-aid for path resolution drift across the spawn boundary
+- `_sync_rag_engine` is called from 6 sites in `app_controller.py` (files change, project switch, session reset, RAG toggle) — 6 race surfaces
+- Lazy module globals (`_CHROMADB`, `_GOOGLE_GENAI`, `_SENTENCE_TRANSFORMERS`) tear down and rebuild per-test with no guarantee of state
+
+The RAG *tests* are testing "does RAG survive the live_gui subprocess lifecycle + chromadb file locks + CWD drift + lazy global teardown/rebuild" — not "does RAG retrieve relevant chunks." That's the bleed.
+
+## Section 1: MMA Quarantine
+
+### Mechanism
+
+Config flag `mma.enabled`, default `false`, in `[ai_settings.toml]`. Per `conductor/code_styleguides/feature_flags.md` §2: this is a persistent preference (off by default, not recoverable by a single regenerate command), so config flag + GUI checkbox is the correct pattern — not file presence, not env-var-only.
+
+### What the flag gates
+
+#### 1.1 `app_controller.py` (~20 sites)
+
+State fields initialized to empty/zero-init when `mma.enabled == false`:
+- `self.engines: Dict[str, ConductorEngine] = {}` (stays empty)
+- `self.mma_streams: Dict[str, str] = {}` (stays empty)
+- `self.mma_step_mode: bool = False` (stays False)
+- `self.mma_tier_usage: Dict[str, Metadata] = {...}` (stays zero-init)
+- `self.tracks: list[Metadata] = []` (stays empty — note: this is the MMA track list, NOT the conductor track system; `project_manager.get_all_tracks` is still callable for the conductor UI if needed)
+
+Engine methods return early when the flag is off:
+- `start_mma` / engine-start paths — return early, no `ConductorEngine` instantiation
+- `approve_step` / `approve_spawn` — return early, no-op
+- abort / reset paths — clear the (already empty) state
+
+The `multi_agent_conductor` import becomes lazy: imported inside the gated methods only, gated on the flag. When the flag is off, the module is never imported at runtime (reduces import-graph weight).
+
+The `rag_engine` sync paths are untouched — RAG is a separate subsystem (Section 2).
+
+#### 1.2 `gui_2.py` (12 render functions + dashboard window + modals)
+
+Every `render_mma_*` function and `render_task_dag_panel` checks `if not app.controller.mma_enabled: return` at the top:
+- `render_mma_dashboard` (gui_2.py:6610)
+- `render_mma_modals` (gui_2.py:6658)
+- `render_mma_track_summary` (gui_2.py:6757)
+- `render_mma_epic_planner` (gui_2.py:6800)
+- `render_mma_conductor_setup` (gui_2.py:6818)
+- `render_mma_track_browser` (gui_2.py:6837)
+- `render_mma_global_controls` (gui_2.py:6884)
+- `render_mma_usage_section` (gui_2.py:6924)
+- `render_mma_ticket_editor` (gui_2.py:7003)
+- `render_mma_agent_streams` (gui_2.py:7043)
+- `render_task_dag_panel` (gui_2.py:7351)
+- `render_mma_focus_selector` (gui_2.py:7570)
+
+The MMA Dashboard window is not registered in the panel registry when the flag is off (gui_2.py:1995 `_render_window_if_open("MMA Dashboard", ...)` — gated on flag). The approval modals (MMA Step Approval, MMA Spawn Approval) no-op. The ~600 lines of render code stay in-tree but are dead at runtime.
+
+#### 1.3 `multi_agent_conductor.py`
+
+Kept in-tree. Imported lazily only inside the gated `app_controller` methods. `ConductorEngine` / `WorkerPool` never instantiate when the flag is off.
+
+#### 1.4 What stays active (shared types, NOT gated)
+
+- `mma.py` — `Ticket`, `Track`, `TrackState`, `TrackMetadata`, `WorkerContext`, `ThinkingSegment` — load-bearing for non-MMA code
+- `dag_engine.py` — `TrackDAG`, `ExecutionEngine` — used by `conductor_tech_lead.py`
+- `mma_prompts.py` — used by `ai_client.py`, `conductor_tech_lead.py`, `orchestrator_pm.py` (verify whether these usages are MMA-specific or general during implementation; if MMA-specific, gate; if general, leave)
+- `mma.py` / `dag_engine.py` / `mma_prompts.py` imports in non-MMA consumers remain unchanged
+
+#### 1.5 GUI surface
+
+A single `[ ] Enable MMA (deprecated, quarantined)` checkbox in AI Settings. The full MMA dashboard is hidden when the flag is off. The checkbox is the only MMA UI surface. Per `feature_flags.md` §2: the GUI checkbox is a projection of the config file; the config file is the source of truth.
+
+#### 1.6 Tests (env-gated, opt-in)
+
+MMA tests gated behind `SLOP_MMA_TESTS=1` env var via `@pytest.mark.skipif`. Skip reason documents: "MMA is quarantined; run with `SLOP_MMA_TESTS=1` to enable." This is the *test* gate — separate from the *runtime* config flag, per `feature_flags.md` §6 (layered flags: file presence / config for runtime, env var for test opt-in).
+
+Affected test files (the `test_mma_*`, `test_concurrent_*`, `test_conductor_*`, `test_dag_*`, `test_parallel_*`, `test_worker_*`, `test_visual_mma*`, `test_*sim*mma*` set):
+- `test_mma_agent_focus_phase1.py`, `test_mma_agent_focus_phase3.py`
+- `test_mma_approval_indicators.py`, `test_mma_concurrent_tracks_sim.py`, `test_mma_concurrent_tracks_stress_sim.py`
+- `test_mma_dashboard_refresh.py`, `test_mma_dashboard_streams.py`, `test_mma_models.py`, `test_mma_node_editor.py`
+- `test_mma_orchestration_gui.py`, `test_mma_prompts.py`, `test_mma_skeleton.py`, `test_mma_step_mode_sim.py`
+- `test_mma_ticket_actions.py`, `test_mma_tier_usage_reset_fix.py`, `test_mma_usage_stats.py`
+- `test_mma_concurrent_tracks_sim.py`, `test_mma_concurrent_tracks_stress_sim.py`
+- `test_reset_session_clears_mma_and_rag.py` (split: MMA portion gated, RAG portion stays — see Section 2)
+- `test_visual_mma.py`, `test_visual_sim_mma_v2.py`
+- `mock_concurrent_mma.py` (test helper, not a test file — kept as-is for opt-in runs)
+- `test_conductor_abort_event.py`, `test_conductor_api_hook_integration.py`, `test_conductor_engine_abort.py`, `test_conductor_engine_v2.py`, `test_conductor_tech_lead.py`
+- `test_dag_engine.py`, `test_gui_dag_beads.py`, `test_perf_dag.py`, `test_task_dag_popout_sim.py`
+- `test_parallel_execution.py`, `test_run_worker_lifecycle_abort.py`
+
+Note: `test_conductor_tech_lead.py` and `test_dag_engine.py` may test the shared-types layer (not the MMA engine). During implementation, classify each: if it tests the shared `dag_engine`/`conductor_tech_lead` layer (not the MMA engine), it stays in the default batch. If it tests the MMA engine specifically, it's gated. The classifier: does the test import or instantiate `multi_agent_conductor.ConductorEngine` / `WorkerPool`? If yes → gated. If no → stays.
+
+## Section 2: RAG Test Decoupling
+
+### Mechanism
+
+Three-tier test classification. The RAG algorithm is unchanged. The mock provider already exists (`rag_engine.py`: `provider == 'mock'` short-circuits chromadb). The tests are reclassified, not rewritten from scratch.
+
+### Tier 1 — Unit tests (default batch, no chromadb, no subprocess)
+
+Test the 50-line algorithm in isolation against the mock provider:
+- `test_rag_chunk.py` — `RAGChunk` dataclass (already isolated; stays)
+- `test_rag_engine.py` — rewrite to use mock provider exclusively; test `index_file` no-op behavior on mock, `search` returns `[]` on mock, chunking logic (`_chunk_text`, `_chunk_code_result`), `is_empty` semantics, `RAGChunk.from_dict`/`to_dict` round-trip
+- `test_rag_engine_result.py` — Result wrapping (already isolated; stays)
+- `test_rag_sync_none_error.py` — sync error handling (mock the engine, not chromadb)
+
+### Tier 2 — Controller lifecycle tests (default batch, mock RAGEngine, no chromadb)
+
+Test `app_controller.py`'s RAG lifecycle wiring using a mock/stub `RAGEngine`:
+- `test_rag_engine_ready_status_bug.py` — mock the `rag_engine` on `AppController`, test the ready-status state machine
+- `test_rag_gui_presence.py` — test the GUI panel renders when RAG is enabled/disabled (no real engine)
+- `test_sync_rag_engine_coalescing.py` — test `_sync_rag_engine` coalescing logic with a mock engine (controller logic, not RAG logic)
+- `test_reset_session_clears_mma_and_rag.py` — RAG portion stays (test reset clears the RAG state fields with a mock engine); MMA portion gated per Section 1.6
+
+### Tier 3 — Integration tests (opt-in, env-gated, real chromadb / live_gui)
+
+Gated behind `SLOP_RAG_INTEGRATION=1` env var via `@pytest.mark.skipif`. Skip reason: "Integration test requires real chromadb + live_gui subprocess; run with `SLOP_RAG_INTEGRATION=1` to enable." These are the tests that kept breaking — now opt-in, not default batch:
+- `test_rag_phase4_final_verify.py` — the 3-ADDENDUM fragile one
+- `test_rag_phase4_stress.py` — stress test
+- `test_rag_visual_sim.py` — live_gui visual sim
+- `test_rag_integration.py` — full integration
+
+### What this kills
+
+The recurring bleed. The default test batch no longer touches chromadb file locks, the live_gui subprocess RAG state, or CWD drift. When adjacent code changes, Tier 1+2 verify RAG logic + controller wiring in isolation. Tier 3 only runs on explicit opt-in (e.g., before a release, or when actively working on RAG).
+
+### What this preserves
+
+The RAG feature itself. `RAGEngine` is unchanged. The mock provider already exists. The integration tests still exist — just not in the default batch.
+
+## Verification
+
+### MMA quarantine
+- `mma.enabled = false` (default): MMA dashboard does not render; engine methods no-op; `multi_agent_conductor` not imported at runtime; MMA tests skipped (not failed)
+- `mma.enabled = true`: MMA dashboard renders; engine starts; MMA tests run when `SLOP_MMA_TESTS=1`
+- No regression in non-MMA code (thinking_parser, project_manager, models, conductor_tech_lead — shared types intact)
+
+### RAG test decoupling
+- Default batch: Tier 1+2 run in milliseconds, no chromadb, no subprocess, no file locks
+- `SLOP_RAG_INTEGRATION=1`: Tier 3 runs (the previously-fragile tests)
+- RAG feature functional end-to-end when `rag.enabled = true` in config (unchanged)
+
+## Risks
+
+- **Lazy import correctness:** the `multi_agent_conductor` lazy import inside gated methods must not introduce a circular import or a startup-time import when the flag is off. Verify via `scripts/audit_main_thread_imports.py` after implementation.
+- **Shared-types boundary:** `mma.py` / `dag_engine.py` / `mma_prompts.py` must remain importable by non-MMA consumers. The flag gates the *engine*, not the *types*. If `mma_prompts.py` usages in `ai_client.py` / `conductor_tech_lead.py` / `orchestrator_pm.py` are MMA-specific (e.g., prompt templates only used by the engine), gate them; if general, leave. Classify during implementation.
+- **Test classifier drift:** the MMA test classifier ("does it import `ConductorEngine` / `WorkerPool`?") must be applied consistently. A test that tests shared `dag_engine` types but not the engine stays in the default batch.
+- **Quarantine maintenance cost:** if the quarantined code rots (imports drift, shared types change underneath it), follow-up full removal (Option A) becomes necessary. The quarantine is the lower-risk path *now*; it's not a permanent commitment.
+
+## Out of Scope
+
+- The discussion/session system redesign (nagent research track)
+- Full removal of MMA code (follow-up track if quarantine maintenance hurts)
+- RAG algorithm changes (`index_file`, `search`, chunking — unchanged)
+- The `conductor/` track system (`conductor_tech_lead.py` / `project_manager.py` / `dag_engine.py` shared-types layer preserved)
+- Migrating `ThinkingSegment` / `TrackState` / `TrackMetadata` out of `mma.py` (follow-up to full removal)
+
+## See Also
+
+- `conductor/code_styleguides/feature_flags.md` — the config-flag-vs-file-presence decision tree (§2: persistent preference → config flag + GUI checkbox)
+- `docs/guide_mma.md` — the MMA engine architecture (the thing being quarantined)
+- `docs/guide_rag.md` — the RAG subsystem architecture (unchanged)
+- `conductor/tracks/fix_mma_concurrent_tracks_sim_20260627/` — the recent MMA brittleness
+- Git history 2026-06-27 — the RAG test debugging churn (10 commits, 2 ADDENDUM reports)