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.
This commit is contained in:
@@ -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/<thing>.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.**
|
||||
Reference in New Issue
Block a user