From 94aeecd2d3544c851b4350aa7b0c7dda4bd6324d Mon Sep 17 00:00:00 2001 From: Ed_ Date: Thu, 11 Jun 2026 17:50:11 -0400 Subject: [PATCH] docs(reports): add namespace_cleanup_sidetrack_report_20260611.md Documents the side-track surfaced during Phase 2 of qwen_llama_grok_followup_20260611: src/models.py is bloated with ~10 non-MMA types (Tool, ToolPreset, BiasProfile, MCPConfiguration, ContextPreset, RAGConfig, Persona, ExternalEditorConfig, FileItem, ThinkingSegment) that should live in their parent modules per the HARD RULE. The report captures: - Evidence: which types, lines, target modules - Why it matters: PROVIDERS move had to use __getattr__ to break a circular import that wouldn't have existed if ToolPreset lived in src/ai_client.py - Proposed move map (10 types) - Prerequisites (1-6) - Estimated scope: 3-5 days - Open questions for the user - Linkage to the follow-up track and the broader deferred_work list NOT EXECUTED. User decision: proceed to Phase 3 of the follow-up. This report is the next agent's reference when the namespace cleanup track is eventually picked up. --- ...space_cleanup_sidetrack_report_20260611.md | 220 ++++++++++++++++++ 1 file changed, 220 insertions(+) create mode 100644 docs/reports/namespace_cleanup_sidetrack_report_20260611.md diff --git a/docs/reports/namespace_cleanup_sidetrack_report_20260611.md b/docs/reports/namespace_cleanup_sidetrack_report_20260611.md new file mode 100644 index 00000000..678a9b74 --- /dev/null +++ b/docs/reports/namespace_cleanup_sidetrack_report_20260611.md @@ -0,0 +1,220 @@ +# Namespace Cleanup Side-Track — Report (2026-06-11) + +> Decision: NOT executed. Deferred to its own track. This report +> documents the analysis, the proposed move map, and the prerequisites +> so the next agent (or the user) can pick this up cleanly when +> desired. + +## Context + +`src/models.py` (1074+ lines) is overloaded. It declares the MMA +core types (`Ticket`, `Track`, `Metadata`, `TrackState`, +`WorkerContext`, `ThinkingSegment`) but also hosts ~10 type +definitions that belong in their respective sub-system modules per +the AGENTS.md HARD RULE on `src/` files. + +This side-track was surfaced on 2026-06-11 during the +`qwen_llama_grok_followup_20260611` Phase 2 (PROVIDERS move). +The user said: *"models.py is filled to the brim with data types +not directly related to mma... a ton of things related to the +'persona' is dumped in here."* + +The user decided: do not side-track now. Document the proposed +cleanup and proceed to Phase 3 of the follow-up track. + +## Symptom (Evidence) + +`grep` of `src/models.py` for non-MMA type declarations shows: + +| Type | Lines | Declared owner (target module) | Why it belongs there | +|---|---|---|---| +| `Tool` | ~50 lines | `src/ai_client.py` | AI-client tool schema model | +| `ToolPreset` | ~30 lines | `src/ai_client.py` | Preset for tool weighting (used by ai_client) | +| `BiasProfile` | ~30 lines | `src/ai_client.py` | Bias profile for tool selection (used by ai_client) | +| `MCPConfiguration` | ~80 lines | `src/mcp_client.py` | MCP server config; consumed by mcp_client | +| `ExternalEditorConfig` | ~50 lines | `src/external_editor.py` | External editor config (file already exists) | +| `ContextPreset` | ~50 lines | `src/context_presets.py` | Context composition presets (file already exists) | +| `FileViewPreset` | ~40 lines | `src/context_presets.py` | File view config (related to context) | +| `RAGConfig` | ~30 lines | `src/rag_engine.py` | RAG config (file already exists) | +| `Persona` | ~40 lines | `src/personas.py` | Agent persona (file already exists) | +| `FileItem` | ~50 lines | `src/app_controller.py` (or new `src/file_item.py`) | File display item config | + +That's ~450 lines (40%+ of `src/models.py`) that should be in +parent modules. The MMA core is the other ~600 lines +(`Ticket`, `Track`, `Metadata`, `TrackState`, `WorkerContext`, +`ThinkingSegment`, dataclass helpers). + +## Why this matters (the user's concern) + +The user's framing: when you're working in a sub-system +(MCP, RAG, context, personas) and you need to import the +type definition, you go to `src/models.py`. But that file +is supposed to be the MMA core. The sprawl makes it hard +to: + +1. **Find types.** A contributor looking for `ToolPreset` + shouldn't have to scroll past 600 lines of MMA types. +2. **Reason about ownership.** The HARD RULE says + sub-system code goes in the parent module. `src/models.py` + is a violation of that rule for ~10 types. +3. **Avoid regressions.** A type definition in the wrong + namespace is a magnet for circular imports (we hit + this exact problem during the PROVIDERS move: + `src/ai_client.py` imports `ToolPreset` from + `src/models.py`, so we couldn't add a top-level + `from src.ai_client import PROVIDERS` re-export). +4. **Reduce merge conflicts.** `src/models.py` is on the + import chain of ~20 files. Any change to it has + project-wide blast radius. + +The PROVIDERS move (Phase 2 of the follow-up) had to use +`__getattr__` to break the circular import — that hack +would not have been needed if `ToolPreset`/`BiasProfile` +lived in `src/ai_client.py` (the canonical parent). + +## Proposed Move Map (per the HARD RULE) + +For each type, the target module is its current consumer's +parent. The move is mechanical: + +| From | Type | To | Reason | +|---|---|---|---| +| `src/models.py` | `Tool` | `src/ai_client.py` | consumed by ai_client + tool_bias | +| `src/models.py` | `ToolPreset` | `src/ai_client.py` | consumed by ai_client + tool_presets | +| `src/models.py` | `BiasProfile` | `src/ai_client.py` | consumed by ai_client + tool_presets | +| `src/models.py` | `MCPConfiguration` | `src/mcp_client.py` | consumed by mcp_client | +| `src/models.py` | `ExternalEditorConfig` | `src/external_editor.py` | consumed by external_editor | +| `src/models.py` | `ContextPreset` | `src/context_presets.py` | consumed by context_presets | +| `src/models.py` | `FileViewPreset` | `src/context_presets.py` | consumed by context_presets | +| `src/models.py` | `RAGConfig` | `src/rag_engine.py` | consumed by rag_engine | +| `src/models.py` | `Persona` | `src/personas.py` | consumed by personas | +| `src/models.py` | `FileItem` | `src/app_controller.py` (or new `src/file_item.py`) | consumed by app_controller + gui_2 | + +`ThinkingSegment` is borderline — it's used by the AI +client's reasoning capture (could go in `src/ai_client.py`) +but also by the GUI (could stay in models). Recommend: +move to `src/ai_client.py` and have `src/gui_2.py` import +from there. + +## Prerequisites Before Executing + +1. **Confirm types are stable** — no in-flight track is + modifying `Tool`, `ToolPreset`, `BiasProfile`, etc. (Check + `conductor/tracks.md` and the `__doc__` headers for "WIP" + markers.) + +2. **Map all import sites** — `grep "from src.models import"` + across `src/` and `tests/`. For each match, decide: + - If the type moves to module X, change to + `from src.X import TypeName` (or + `from src.X import TypeName as TypeName` for backward + compat shim). + - If the type stays in models.py (MMA core), no change. + +3. **Update `_REGISTRY` and similar module-level state** + — some types register themselves in a module-level + dict (e.g., `src/vendor_capabilities.py:REGISTRY`). Make + sure the move preserves the registration order. + +4. **Update tests** — most type tests are in + `tests/test_*_models.py`. Rename or move as needed. + +5. **Decide on backward-compat shims** — for any type + that has external consumers (the tool presets + `tool_presets.py:8` does `from src.models import + ToolPreset, BiasProfile`), do we: + - **(a) Hard move** — update all import sites + atomically. Cleanest, but breaks any third-party + code (none in this project). + - **(b) Re-export shim** — keep the symbol in + `src/models.py` via a re-export (`from src.ai_client + import ToolPreset as ToolPreset`). The PROVIDERS + pattern in Phase 2 used `__getattr__` to break a + circular import; this case has no circular import + (since `ai_client.py` would import `ToolPreset` from + `ai_client.py` itself, not from `models.py`), so + a direct re-export works. + + **Recommendation: (b) re-export shim** for non-circular + cases. Lower-risk, less churn. (a) is acceptable for + the MMA-core types that stay in models. + +6. **Audit script** — add `scripts/audit_models_types.py` + that flags types in `src/models.py` that have + consumers in sub-system modules. Companion to + `audit_providers_source_of_truth.py`. + +## Estimated Scope + +Based on the search results, ~10 types to move, ~30-40 +import sites to update (rough count from grep), ~10-15 +test files to update. + +| Phase | Effort | Risk | +|---|---|---| +| Red test: assert all "moved" types are imported from their parent module | 30 min | low | +| Green: move 1 type + update import sites | 1-2 hours/type | medium (circular imports possible) | +| Audit script | 30 min | low | +| Backward-compat shim verification | 1 hour | low | +| Phase checkpoint + git note | 15 min | low | +| **Total** | **~3-5 days** for 10 types | **medium** | + +The PROVIDERS move (Phase 2 of the follow-up) is a +useful template: same pattern (target file + +backward-compat re-export + update import sites + audit +script). + +## Open Questions for the User + +1. **Should the move be one big commit or 10 small commits + (one per type)?** Small commits are easier to review and + revert. The follow-up track's per-file atomic-commit + rule suggests small. + +2. **Should the `src/models.py` file be deleted after the + moves or kept as a re-export shim?** If kept, it + documents the MMA core (Ticket, Track, etc.) which is + its original purpose. If deleted, the MMA types + move to a new `src/mma_types.py` or `src/mma_models.py`. + +3. **Order of moves**: do the highest-leverage ones first + (Tool/ToolPreset/BiasProfile — these are in the + `src/ai_client.py` import chain, the most-frequent + circular-import culprits). Or do the leaf nodes first + (MCPConfiguration, RAGConfig, ExternalEditorConfig — + fewer downstream consumers). + +## Linkage + +- Parent follow-up track: `qwen_llama_grok_followup_20260611` +- Surfaced during: Phase 2 (PROVIDERS move) — the circular + import that required `__getattr__` was caused by + `src/ai_client.py` importing `ToolPreset` from + `src/models.py`. +- HARD RULE reference: `AGENTS.md` "File Size and Naming + Convention" + "Hard rule on creating new `src/.py` + files" (codified 2026-06-11). +- Related deferred tracks (from + `conductor/tracks/qwen_llama_grok_followup_20260611/state.toml` + `deferred_work`): + - `ai_client_codepath_consolidation_20260611` — + refactor `src/ai_client.py` to reduce duplication + (VendorHistory class, shared reasoning extraction, + per-HTTP-code error classifier). NOT file size; the + file is already at 2800+ lines and that's OK. + - `mcp_architecture_refactor_20260606` — already + specced but moves in the OPPOSITE direction of the + user's preference (creates new `src/mcp_*` files). + May want to abort. + +## Recommendation + +Schedule this for a dedicated session, not mid-track. The +follow-up's Phase 3 (UX adaptations) and Phase 4 (local-first ++ matrix v2) are smaller, more focused work that doesn't +depend on the namespace cleanup. Run namespace cleanup as +its own follow-up track (`namespace_cleanup_20260611` per +the deferred_work section), with its own per-type atomic +commits and audit script. + +**Status: NOT EXECUTED. Documented and deferred.**