Private
Public Access
0
0

docs(reports): FOLLOWUP_module_taxonomy_20260627 - models.py audit + refactor plan

User directive: models.py is a dumping ground. Needs clean mma_/project_
taxonomy per AGENTS.md 'File Size and Naming Convention' HARD RULE.

Audit findings:
- models.py is 1044 lines, 13 regions, 5+ unrelated domains
- 36 classes/functions in 1 file
- Top docstring claims MMA + project config but actually contains:
  editor configs, MCP config, file contexts, persona configs, Pydantic proxies
- Phase 2 of cruft_elimination_20260627 just added 6 more (ProjectContext)
  making the mess worse

Proposed taxonomy:
- src/mma.py = main MMA file (Ticket, Track, WorkerContext, ThinkingSegment,
  TrackState)
- src/project.py = main project-config file (ProjectContext + 5 sub + config IO
  + parse_history_entries)
- src/project_files.py = file-related (FileItem, ContextPreset, ContextFileEntry,
  NamedViewPreset, Preset)
- Tool/Persona/Editor/MCP/Workspace dataclasses merge into their existing
  sub-system files (tool_presets.py, tool_bias.py, personas.py, external_editor.py,
  mcp_client.py, workspace_manager.py)
- src/models.py reduced to ~60 lines (Pydantic proxies + AGENT_TOOL_NAMES only)

5-phase refactor plan:
- Phase 1: src/mma.py + 5 file imports updated
- Phase 2: src/project.py + project_manager.py imports updated
- Phase 3: src/project_files.py + 4 file imports updated
- Phase 4: Merge 8+ dataclasses into 6 existing sub-system files
- Phase 5: Reduce src/models.py to ~60 lines

11 VCs. 1 atomic commit per file move. Regression-guard tests after each.

Critical: the cruft_elimination_20260627 Phase 2 spec must be updated to
say 'add ProjectContext to src/project.py' (NOT src/models.py). Tier 2
should re-execute Phase 2 with the corrected file location before this
broader taxonomy refactor starts.

User instruction: 'I need top-level prefix for modules that cannot have
their definitions in the single file (mma_ with mma.py being the main one,
project_, with project.py, etc)'.
This commit is contained in:
2026-06-26 05:59:29 -04:00
parent 8f6ae6d983
commit 01b6c68e20
@@ -0,0 +1,193 @@
# Followup: Module Taxonomy Audit + Refactor Plan
**Date:** 2026-06-27
**Reviewer:** Tier 1
**Trigger:** User reported `models.py` is a dumping ground; needs clean `mma_` / `project_` taxonomy per AGENTS.md "File Size and Naming Convention" HARD RULE.
---
## TL;DR
`models.py` is 1044 lines, 13 regions, mixing 5+ unrelated domains. The Phase 2 work in `cruft_elimination_20260627` added 6 more dataclasses to this mess. **The spec said "add to models.py"** — Tier 2 followed the spec, but the spec was wrong. The user's directive: refactor into a clean taxonomy.
**Proposed split:** 1 main MMA file + 1 main project file + N `project_` sub-files (existing `mma_`/`tool_`/`persona_`/etc. files stay as-is; just merge in the dataclasses that currently live in `models.py`).
---
## Audit: What's in `models.py` (1044 lines, 13 regions)
```
REGION: Constants (~20 lines) — AGENT_TOOL_NAMES lazy proxy + PROVIDERS lazy import
REGION: Config Utilities (~30 lines) — _clean_nones, load_config_from_disk, save_config_to_disk
REGION: History Utilities (~20 lines) — parse_history_entries
REGION: Pydantic Models (~30 lines) — _create_generate_request, _create_confirm_request, __getattr__
REGION: MMA Core (~200 lines) — ThinkingSegment, Ticket, Track, WorkerContext
REGION: State & Config (~60 lines) — TrackState, FileItem, Preset
REGION: Tool Models (~80 lines) — Tool, ToolPreset, BiasProfile
REGION: UI/Editor (~40 lines) — TextEditorConfig, ExternalEditorConfig
REGION: Persona (~30 lines) — Persona
REGION: Workspace (~80 lines) — WorkspaceProfile, ContextFileEntry, NamedViewPreset, ContextPreset
REGION: MCP Config (~100 lines) — MCPServerConfig, MCPConfiguration, VectorStoreConfig, RAGConfig, load_mcp_config
REGION: Project Context (Phase 2 from cruft_elimination_20260627) (~150 lines) — ProjectContext + 5 sub-dataclasses
```
**36 classes/functions in 1 file.** **5+ unrelated domains.** **No central theme.** The file's top-level docstring says "Core data structures for MMA orchestration and project configuration" — but it's grown to also include editor configs, MCP config, file contexts, persona configs, and Pydantic proxies.
---
## Proposed Taxonomy
Per the user's directive + AGENTS.md HARD RULE: top-level prefix (`mma_`, `project_`) for modules that cannot fit in a single file.
### MMA-related (`mma_` prefix)
| File | Contents | Status |
|---|---|---|
| **`src/mma.py`** | ThinkingSegment, Ticket, Track, WorkerContext, TrackState (MMA Core) | **NEW** |
| `src/mma_dag.py` | (would be TrackDAG; already in `src/dag_engine.py` — leave alone for now) | NO ACTION |
### Project-config (`project_` prefix)
| File | Contents | Status |
|---|---|---|
| **`src/project.py`** | ProjectContext + 5 sub-dataclasses + config I/O (load_config_from_disk, save_config_to_disk, _clean_nones, parse_history_entries) | **NEW** |
| **`src/project_files.py`** | FileItem, ContextPreset, ContextFileEntry, NamedViewPreset, Preset | **NEW** |
### Merge into existing `mma_`/`tool_`/`persona_`/etc. files
These dataclasses should live with their related system, not in `models.py`:
| Class | Destination | Reason |
|---|---|---|
| `Tool`, `ToolPreset`, `BiasProfile` | split between `src/tool_presets.py` (already exists, 123 lines) and `src/tool_bias.py` (already exists, 63 lines) | Tool classes belong with the tool system |
| `Persona` | `src/personas.py` (already exists, 93 lines) | Persona classes belong with the persona system |
| `TextEditorConfig`, `ExternalEditorConfig` | `src/external_editor.py` (already exists, 129 lines) | Editor configs belong with the editor system |
| `MCPServerConfig`, `MCPConfiguration`, `VectorStoreConfig`, `RAGConfig`, `load_mcp_config` | `src/mcp_client.py` (already exists, 1803 lines) | MCP config belongs with the MCP system |
| `WorkspaceProfile` | `src/workspace_manager.py` (already exists, 73 lines) | Workspace profile belongs with the workspace system |
### `models.py` (what's left after refactor)
After all moves, `models.py` shrinks to JUST:
- `AGENT_TOOL_NAMES` constant (the canonical 45-tool list per `docs/guide_models.md`)
- `_create_generate_request`, `_create_confirm_request`, `__getattr__` (Pydantic lazy proxies for the API)
- Top-level docstring (updated to reflect the new scope)
Estimated: ~60 lines (down from 1044).
---
## Why this taxonomy
The user's directive: "top-level prefix for modules that cannot have their definitions in the single file (mma_ with mma.py being the main one, project_, with project.py, etc)".
- **`mma.py`** is the main MMA file. If MMA grows beyond what fits in a single file, sub-modules use the `mma_` prefix (e.g., `mma_dag.py`, `mma_state.py`).
- **`project.py`** is the main project-config file. If project config grows, sub-modules use the `project_` prefix (e.g., `project_files.py`).
- **Other domains** (tools, personas, editor, MCP) already have their own files; just merge the dataclasses in.
The pattern is the same as the existing `mma_prompts.py`, `mma_tier_usage_reset_fix.py`, etc. — `mma_X.py` for MMA sub-systems. The new convention extends this to `project_X.py` for project sub-systems.
---
## Migration Plan (5 phases, atomic commits per file)
### Phase 1: Create `src/mma.py`
- **Move from `src/models.py`:** ThinkingSegment, Ticket, Track, WorkerContext, TrackState
- **New file `src/mma.py`** with proper docstring (focused on MMA orchestration)
- **Update imports** in 5 files: `src/multi_agent_conductor.py`, `src/dag_engine.py`, `src/orchestrator_pm.py`, `src/conductor_tech_lead.py`, `tests/test_mma_*.py`
- **Verify:** `tests/test_mma_concurrent_tracks_sim.py` + `tests/test_dag_engine.py` + `tests/test_orchestration_logic.py` all pass
### Phase 2: Create `src/project.py`
- **Move from `src/models.py`:** ProjectContext + 5 sub-dataclasses + config I/O helpers (`_clean_nones`, `load_config_from_disk`, `save_config_to_disk`, `parse_history_entries`)
- **New file `src/project.py`** with proper docstring (focused on project configuration)
- **Update imports** in `src/project_manager.py` (where most are used)
- **Verify:** `tests/test_project_manager_*.py` + `tests/test_track_state_persistence.py` + `tests/test_project_paths.py` all pass
### Phase 3: Create `src/project_files.py`
- **Move from `src/models.py`:** FileItem, ContextPreset, ContextFileEntry, NamedViewPreset, Preset
- **New file `src/project_files.py`**
- **Update imports** in `src/aggregate.py`, `src/context_presets.py`, `src/gui_2.py`, `src/app_controller.py`
- **Verify:** `tests/test_context_composition_*.py` + `tests/test_view_presets.py` + `tests/test_custom_slices*.py` all pass
### Phase 4: Merge dataclasses into existing tool/persona/editor/mcp files
- **`src/tool_presets.py`**: add `Tool` + `ToolPreset` from `models.py`
- **`src/tool_bias.py`**: add `BiasProfile` from `models.py`
- **`src/personas.py`**: add `Persona` from `models.py`
- **`src/external_editor.py`**: add `TextEditorConfig` + `ExternalEditorConfig` from `models.py`
- **`src/mcp_client.py`**: add `MCPServerConfig` + `MCPConfiguration` + `VectorStoreConfig` + `RAGConfig` + `load_mcp_config` from `models.py`
- **`src/workspace_manager.py`**: add `WorkspaceProfile` from `models.py`
- **Verify:** all existing tests for these files pass
### Phase 5: Reduce `src/models.py` to ~60 lines
- **Keep in `src/models.py`:** `AGENT_TOOL_NAMES`, `_create_generate_request`, `_create_confirm_request`, `__getattr__`
- **Update top-level docstring** to reflect new scope
- **Verify:** all 7 audit gates pass `--strict`; all batched tiers pass
---
## Risks
| # | Risk | Likelihood | Mitigation |
|---|---|---|---|
| R1 | Circular imports (e.g., `mma.py` needs `Metadata` from `type_aliases.py`; `project.py` needs `Metadata` from `type_aliases.py`) | low | `Metadata` stays in `type_aliases.py` (the canonical wire-format location); no cycle introduced |
| R2 | Sub-system files grow too large after merge (e.g., `mcp_client.py` is already 1803 lines, adding 100 lines of config dataclasses pushes to 1903) | medium | If a sub-system file grows past ~2000 lines, create a sub-module (`mcp_config.py`, `personas_models.py`, etc.) |
| R3 | The "from src.models import *" pattern used by lazy __getattr__ breaks when classes are moved | low | Replace `__getattr__` with explicit imports in each consumer; the lazy proxy was a hack to break a circular import that's no longer needed |
| R4 | The cross-import between `mma.py` and `project.py` creates a cycle (e.g., `Track` references `ProjectContext`) | low | Audit cross-references; if needed, put shared types in `type_aliases.py` |
| R5 | The Phase 2 ProjectContext dataclasses (just shipped in `cruft_elimination_20260627`) would need to be moved again from `models.py` to `project.py` | high | Yes — this is the immediate trigger for the user's frustration. Phase 2 must move ProjectContext to project.py, not models.py. |
---
## Acceptance Criteria
| # | Criterion | Verification |
|---|---|---|
| VC1 | `src/mma.py` exists and contains MMA Core + TrackState | `python -c "from src.mma import ThinkingSegment, Ticket, Track, WorkerContext, TrackState"` |
| VC2 | `src/project.py` exists and contains ProjectContext + 5 sub + config I/O | `python -c "from src.project import ProjectContext, ProjectMeta, ProjectOutput, ProjectFiles, ProjectScreenshots, ProjectDiscussion"` |
| VC3 | `src/project_files.py` exists and contains file-related dataclasses | `python -c "from src.project_files import FileItem, ContextPreset, ContextFileEntry, NamedViewPreset, Preset"` |
| VC4 | Tool dataclasses live in tool_presets.py + tool_bias.py | `python -c "from src.tool_presets import Tool, ToolPreset; from src.tool_bias import BiasProfile"` |
| VC5 | Persona dataclass lives in personas.py | `python -c "from src.personas import Persona"` |
| VC6 | Editor dataclasses live in external_editor.py | `python -c "from src.external_editor import TextEditorConfig, ExternalEditorConfig"` |
| VC7 | MCP config dataclasses live in mcp_client.py | `python -c "from src.mcp_client import MCPServerConfig, MCPConfiguration, VectorStoreConfig, RAGConfig, load_mcp_config"` |
| VC8 | WorkspaceProfile lives in workspace_manager.py | `python -c "from src.workspace_manager import WorkspaceProfile"` |
| VC9 | `src/models.py` is reduced to ~60 lines (only Pydantic proxies + AGENT_TOOL_NAMES) | `wc -l src/models.py` returns < 100 |
| VC10 | All 7 audit gates pass `--strict` | same as current |
| VC11 | All batched test tiers pass (10/11 baseline + RAG flake) | same as current |
---
## Scope vs Original `cruft_elimination_20260627`
The original `cruft_elimination_20260627` track put `ProjectContext` in `models.py`. This follow-up says: **move it out** (to `src/project.py`). Tier 2 should NOT merge the Phase 2 commit as-is. Instead:
1. **Update Phase 2 spec** to say "add to `src/project.py`, NOT `models.py`"
2. **Tier 2 re-executes Phase 2** with the corrected location
3. **After Phase 2 merges**, execute the broader taxonomy refactor (this follow-up)
---
## Recommendation
**Do this as a follow-up track** (`module_taxonomy_refactor_20260627` or similar). Scope: 5 phases, 1 atomic commit per file move, regression-guard tests after each phase. The user is frustrated; clean taxonomy is foundational for future tracks (no more "where does this class go?" confusion).
**Tier 2 should:**
1. Update the `cruft_elimination_20260627` spec correction for Phase 2 (file location)
2. Then plan the broader taxonomy refactor as a new track
**Tier 1 should:**
1. Archive the current `models.py` taxonomy as "broken, needs refactor" (in this follow-up doc)
2. Approve the new taxonomy
3. Plan the 5-phase refactor with Tier 3
---
## See also
- `src/models.py` — current state (1044 lines, 13 regions, 5+ domains)
- `conductor/code_styleguides/data_oriented_design.md` — "Prefer Fewer Types" principle
- `AGENTS.md` — "File Size and Naming Convention" HARD RULE
- `conductor/tracks/cruft_elimination_20260627/SPEC_CORRECTION_phase_2.md` — Phase 2 spec correction (also references `models.py`; this follow-up supersedes it for the file location)
- `docs/guide_models.md` — the canonical data model reference