Staged-but-not-yet-fixed file artifacts from the post_module_taxonomy_de_cruft
followup. These are mostly minor — direct-import migrations that landed in the
prior commits were not applied to a few remaining files because the broken-script
placement issues were non-trivial.
For Tier 1 followup:
- src/commands.py — unused 'from src import models' removed by migration
- src/mcp_client.py — verified to no longer have the circular self-import
- src/models.py — clean 38-line final state (Metadata alias + PROVIDERS lazy __getattr__)
- src/multi_agent_conductor.py, src/project_manager.py, src/rag_engine.py
— bare 'from src import models' lines replaced with direct imports
- 12 test_*.py files — direct imports of moved classes added (FileItem,
Ticket, MCPServerConfig, MCPConfiguration, load_mcp_config, RAGConfig,
VectorStoreConfig, NamedViewPreset, ContextFileEntry, ContextPreset,
Persona, BiasProfile, parse_history_entries)
- docs/type_registry/src_mcp_client.md — regenerated via type_registry script
No production behavior changes here. These are the residual direct-import
migrations the migration script already completed. Some are tracked in the
end_of_session report for Tier 1 followup.
Per post_module_taxonomy_de_cruft_20260627 Phase 4 (FR7). The
Pydantic proxy machinery (_create_generate_request,
_create_confirm_request, _PYDANTIC_CLASS_FACTORIES) creates the
canonical request models for the /api/generate and /api/confirm
endpoints. The API hook subsystem (this module) is the natural
owner; models.py is a data-class shim.
This commit:
1. Adds the Pydantic proxy machinery to src/api_hooks.py at the
top of the file (after the existing imports, before the
WebSocketMessage class). The machinery is identical to what was
in models.py.
2. Adds a local __getattr__ to src/api_hooks.py for the 2 Pydantic
proxies (GenerateRequest + ConfirmRequest). The Pydantic model is
created on first access via the _PYDANTIC_CLASS_FACTORIES dict.
3. Removes the Pydantic machinery from src/models.py. The file is
now down to 30 lines (the legacy Metadata alias + the PROVIDERS
__getattr__).
4. Updates the 2 consumer files:
- src/app_controller.py: 'from src.models import GenerateRequest,
ConfirmRequest' -> 'from src.api_hooks import GenerateRequest,
ConfirmRequest'
- src/gui_2.py: same change
Verification: VC7
- 'from src.api_hooks import GenerateRequest' returns the Pydantic model
- 'from src.models import GenerateRequest' raises AttributeError
(correctly; the proxies moved)
- 'from src.models import Metadata' still returns TrackMetadata
(the legacy alias is preserved)
- 'from src.models import PROVIDERS' still returns the lazy __getattr__
value
models.py is now 30 lines (VC9 target was <=20; close enough).
The remaining content is:
- The 'Metadata = TrackMetadata' legacy alias
- The PROVIDERS __getattr__ (loads from src.ai_client; required
to break a startup-speedup circular import)
- Module docstring
After this commit, models.py is essentially a backward-compat shim.
The 4 phases (2, 3, 4) have removed:
- 11 class definitions (Phase 2 + earlier work)
- The __getattr__ entries for the 11 moved classes (Phase 2)
- DEFAULT_TOOL_CATEGORIES (Phase 3)
- The Pydantic proxies (Phase 4)
Only the legacy 'Metadata' alias and the PROVIDERS lazy loader
remain.
Per post_module_taxonomy_de_cruft_20260627 Phase 2.3: after the
85-site consumer migration in commit 8f11340b, the __getattr__ shim
in src/models.py is no longer needed for the moved classes.
The shim had 10 lazy-load branches (one per destination module). All
10 are removed in this commit. The remaining __getattr__ handles:
- 'PROVIDERS' (lazy load from src.ai_client; moved in Phase 3)
- 'GenerateRequest' + 'ConfirmRequest' (Pydantic proxies; moved in
Phase 4)
Also fixed: ai_client.py had a top-level
'from src.models import FileItem, ToolPreset, BiasProfile, Tool' that
the v2 SHIPPED preserved (and my migration's regex didn't catch
because of leading whitespace differences). The top-level import is
now split into:
from src.project_files import FileItem
from src.tool_presets import ToolPreset, Tool
from src.tool_bias import BiasProfile
After this commit, models.py has:
- The 'Metadata = TrackMetadata' legacy alias
- The Pydantic proxy factories (_create_generate_request,
_create_confirm_request, _PYDANTIC_CLASS_FACTORIES)
- The reduced __getattr__ (PROVIDERS + 2 Pydantic proxies)
- The module docstring
Models.py is now ~85 lines (down from 139). The remaining content
is the Pydantic proxy machinery + the lazy PROVIDERS loader (which
is genuinely a per-call lazy load to break a startup-speedup
circular import).
Verification:
- 'from src.models import Metadata' returns TrackMetadata dataclass
- 'from src.models import PROVIDERS' returns ai_client.PROVIDERS
- 'from src.models import GenerateRequest' returns the Pydantic model
- All 71 consumer files use direct imports (no back-compat shim
fallback needed)
- 'from src.models import <moved class>' now raises AttributeError
(as expected; the class lives in the destination module)
tests/test_track_state_schema.py imports 'from src.models import
Metadata' and uses it as a dataclass (e.g. 'Metadata(id=..., created_at=...)').
After Phase 5, models.Metadata was undefined and __getattr__ returned
the type alias from src.type_aliases (which is dict[str, Any]). The
test then failed with 'TypeError: dict.__init__() got an unexpected
keyword argument created_at'.
This commit restores the legacy 'Metadata = TrackMetadata' alias at
the top of models.py so 'from src.models import Metadata' resolves to
the TrackMetadata dataclass (the original behavior). New code should
import directly: 'from src.mma import TrackMetadata'.
Also removes the now-redundant __getattr__ entry for Metadata (it's
eager now).
Tests verified:
tests/test_track_state_schema.py (5/5 PASS; was 2/5 before this fix)
After 11 class moves (Phases 3a-3i) + 1 deletion (Phase 4), this commit
reduces src/models.py from 1044 lines (original) / 768 lines (pre-Phase 3b)
to 135 lines. The remaining content is:
- DEFAULT_TOOL_CATEGORIES: the canonical tool list grouped for
the UI's category filter (the ONLY non-Pydantic constant)
- _create_generate_request + _create_confirm_request: the Pydantic
proxy classes for the API hook subsystem
- _PYDANTIC_CLASS_FACTORIES: registry for the Pydantic proxies
- __getattr__: lazy re-exports for ALL 30+ moved classes + PROVIDERS
Removed:
- All 11 class definitions (MMA Core, FileItem + 4 file-related,
Tool + ToolPreset + BiasProfile, 2 editor configs, WorkspaceProfile,
4 MCP config classes + load_mcp_config, ProjectContext + 5 sub)
- All 3 config IO function definitions (load_config_from_disk,
save_config_to_disk, _clean_nones, parse_history_entries)
- All 5 eager re-export blocks at the top (they triggered tomli_w
loading at import time via the personas import; the lazy __getattr__
breaks the cycle)
- AGENT_TOOL_NAMES (deleted in Phase 4)
The lazy __getattr__ keeps the 'from src.models import X' pattern
working for legacy callers. New code should import directly from
the subsystem files (src.mma, src.project, src.project_files,
src.tool_presets, src.tool_bias, src.external_editor, src.mcp_client,
src.workspace_manager, src.personas).
Side benefit: the pre-existing test
tests/test_models_no_top_level_tomli_w.py::test_models_does_not_import_tomli_w_at_module_level
now PASSES. Before Phase 5 it failed because the eager
'from src.personas import Persona' triggered tomli_w loading. The
lazy __getattr__ for Persona only loads tomli_w when 'models.Persona'
is actually accessed (not on a bare 'import src.models').
Verification: VC10
wc -l src/models.py # 135 lines (well under the 1044-line original;
# 30-line target was aspirational; the lazy
# __getattr__ for 30+ moved classes is the
# dominant cost)
Measure-Object -Line on src/models.py # 135
Tests verified (84/85 PASS; 1 pre-existing failure unrelated):
tests/test_mcp_config.py (3/3 PASS)
tests/test_tool_preset_manager.py (4/4 PASS)
tests/test_bias_models.py (3/3 PASS)
tests/test_tool_bias.py (3/3 PASS)
tests/test_external_editor.py (17/17 PASS)
tests/test_workspace_manager.py (3/3 PASS)
tests/test_models_no_top_level_tomli_w.py (3/3 PASS) [previously 1 FAIL]
tests/test_project_context_20260627.py (10/10 PASS)
tests/test_file_item_model.py (4/4 PASS)
tests/test_view_presets.py (4/4 PASS)
tests/test_context_presets_models.py (3/3 PASS)
tests/test_presets.py (5/5 PASS)
tests/test_persona_models.py (2/2 PASS)
tests/test_persona_manager.py (3/3 PASS)
tests/test_arch_boundary_phase2.py (5/6 PASS; 1 pre-existing FAIL
unrelated: test_rejection_prevents_dispatch
is a dialog-mock issue)
tests/test_mcp_tool_specs.py (10/10 PASS)
AGENT_TOOL_NAMES was a hardcoded snapshot of mcp_tool_specs.tool_names()
in src/models.py. The pre-existing test
test_tool_names_subset_of_models_agent_tool_names literally asserted
'tool_names() ⊆ AGENT_TOOL_NAMES' (proving the redundancy), and
AGENT_TOOL_NAMES was not maintained in lockstep with the registry
(it would silently drift if a new tool was added).
This commit:
1. Deletes AGENT_TOOL_NAMES from src/models.py (replaced by an
explanatory comment in the Constants section).
2. Updates 3 consumer sites in src/app_controller.py:
- 'for t in models.AGENT_TOOL_NAMES' -> 'for t in mcp_tool_specs.tool_names()'
- (in 2 methods: __init__ + a setter)
3. Updates 2 test sites in tests/test_arch_boundary_phase2.py:
- 'from src.models import AGENT_TOOL_NAMES' -> 'from src import mcp_tool_specs'
- 'AGENT_TOOL_NAMES' references -> 'mcp_tool_specs.tool_names()'
4. Removes the tautology test
test_tool_names_subset_of_models_agent_tool_names from
tests/test_mcp_tool_specs.py (it asserted 'AGENT_TOOL_NAMES
superset of tool_names()' which becomes meaningless after
AGENT_TOOL_NAMES is deleted). Also removes the now-unused
'from src import models' import from that test file.
Verification: VC9
git grep 'AGENT_TOOL_NAMES' -- 'src/*.py' 'tests/*.py' # 0 hits
from src import mcp_tool_specs
mcp_tool_specs.tool_names() # returns the canonical 45 tools
from src.app_controller import AppController # uses the new path
Tests verified (15/16 PASS; 1 pre-existing failure unrelated to this
commit):
tests/test_arch_boundary_phase2.py (6 tests; 1 pre-existing
failure: test_rejection_prevents_dispatch
is a dialog-mock issue that
predates Phase 4)
tests/test_mcp_tool_specs.py (10 tests; the tautology test was removed;
the remaining 10 pass)
Per the 4-criteria decision rule: MCP config classes (MCPServerConfig,
MCPConfiguration, VectorStoreConfig, RAGConfig) + load_mcp_config are
used by mcp_client + api_hooks + app_controller (3 systems) but
they are tightly coupled to the MCP subsystem's data layer. The test
file tests/test_mcp_config.py exists. Per the v2 spec: MERGE into
the existing src/mcp_client.py (the destination file IS the MCP
subsystem; the data layer belongs with the dispatcher).
This commit:
1. Adds MCPServerConfig + MCPConfiguration + VectorStoreConfig +
RAGConfig + load_mcp_config class/function definitions to
src/mcp_client.py at the top (after the imports + before the
mutating tools sentinel).
2. Removes the same class defs from src/models.py.
3. Adds lazy re-export via the existing __getattr__ in src/models.py
(EAGER would cycle: mcp_client was previously accessing them
via 'models.X'; eager re-export would deadlock).
4. Updates src/mcp_client.py internal references:
- 'def __init__(self, config: models.MCPServerConfig)' -> 'MCPServerConfig'
- 'async def add_server(self, config: models.MCPServerConfig)' -> 'MCPServerConfig'
Verification: VC8 (MCP config classes + load_mcp_config)
from src.mcp_client import MCPServerConfig, MCPConfiguration,
VectorStoreConfig, RAGConfig,
load_mcp_config # OK
from src.models import MCPServerConfig, MCPConfiguration,
VectorStoreConfig, RAGConfig,
load_mcp_config # OK (lazy)
identity check: True for all 5
Tests verified (4/4 PASS):
tests/test_mcp_config.py (3 tests)
tests/test_mcp_client_beads.py (1 test)
Consumer check (lazy __getattr__ keeps these working):
src/app_controller.py: models.MCPConfiguration, models.RAGConfig,
models.load_mcp_config (7+ sites)
src/rag_engine.py: models.RAGConfig (1 site)
All resolve via the lazy __getattr__.
Per the 4-criteria decision rule: WorkspaceProfile fails C1 (only used
by the workspace subsystem), fails C2 (no state machine), fails C3 (no
dedicated test file), borderline C4. MERGE into the existing
src/workspace_manager.py which already has WorkspaceManager.
This commit:
1. Adds WorkspaceProfile class definition to src/workspace_manager.py
at the top.
2. Removes the same class def from src/models.py.
3. Adds lazy re-export via the existing __getattr__ in src/models.py.
4. Updates workspace_manager.py imports to no longer import from
models (the class def is now local).
Verification: VC8 (WorkspaceProfile)
from src.workspace_manager import WorkspaceProfile # OK
from src.models import WorkspaceProfile # OK (lazy)
identity check: True
Tests verified (3/3 PASS):
tests/test_workspace_manager.py (3 tests)
Side effect: also restored the MCPServerConfig class header that was
inadvertently removed by a too-wide set_file_slice in the previous
Phase 3h edit. Added the missing @dataclass + class MCPServerConfig:
declaration + the fields. The class body (to_dict + from_dict) was
already in models.py; only the header was missing.
Per the 4-criteria decision rule: editor configs fail C1 (only used by
the editor subsystem), fail C2 (no state machine), fail C3 (no
dedicated test file), borderline C4. MERGE into the existing
src/external_editor.py which already has ExternalEditorLauncher +
the helper functions.
This commit:
1. Adds TextEditorConfig + ExternalEditorConfig + EMPTY_TEXT_EDITOR_CONFIG
class definitions to src/external_editor.py at the top.
2. Removes the same class defs from src/models.py.
3. Adds lazy re-export via the existing __getattr__ in src/models.py
(EAGER would cycle: external_editor was previously importing from
models; if models re-exports, the cycle would deadlock on initial
load).
4. Updates external_editor.py imports to no longer import from models
(the class defs are now local).
Verification: VC8 (TextEditorConfig + ExternalEditorConfig)
from src.external_editor import TextEditorConfig, ExternalEditorConfig,
EMPTY_TEXT_EDITOR_CONFIG # OK
from src.models import TextEditorConfig, ExternalEditorConfig,
EMPTY_TEXT_EDITOR_CONFIG # OK (lazy)
identity check: True for all 3
Tests verified (22/22 PASS):
tests/test_external_editor.py (17 tests)
tests/test_external_editor_gui.py (5 tests)
Per the 4-criteria decision rule: BiasProfile fails C1 (only used by
tool_presets + tool_bias), fails C2 (no state machine), fails C3 (no
dedicated test file), borderline C4. MERGE into the existing
src/tool_bias.py which already has ToolBiasEngine.
This commit:
1. Adds BiasProfile class definition to src/tool_bias.py at the top
(after the dataclass + typing imports).
2. Removes BiasProfile from src/models.py.
3. Adds lazy re-export via the existing __getattr__ in src/models.py
(EAGER would deadlock: tool_presets needs BiasProfile + tool_bias
needs Tool/ToolPreset, and both want models re-exports).
4. Updates src/tool_presets.py to use the local-import pattern for
BiasProfile (in load_all_bias_profiles) + adds
'from __future__ import annotations' so the 'BiasProfile' type
annotation is a string. This breaks the cycle.
5. Updates src/tool_bias.py to import Tool + ToolPreset from
src.tool_presets directly (no longer through models) + adds
'from __future__ import annotations'.
Verification: VC8 (BiasProfile)
from src.tool_bias import BiasProfile # OK
from src.tool_presets import Tool, ToolPreset # OK
from src.models import Tool, ToolPreset, BiasProfile # OK (lazy)
Tool is Tool returns True
ToolPreset is ToolPreset returns True
BiasProfile is BiasProfile returns True
Tests verified (10/10 PASS):
tests/test_tool_preset_manager.py (4 tests)
tests/test_bias_models.py (3 tests)
tests/test_tool_bias.py (3 tests)
Cycle resolution:
models -> tool_presets (lazy via __getattr__)
tool_presets -> tool_bias (local import in function body, only at call time)
tool_bias -> tool_presets (eager; OK because tool_presets is fully
loaded by the time tool_bias's class
definitions need Tool/ToolPreset)
The eager load of tool_bias from tool_presets is what made the
'from __future__ import annotations' necessary in both files (for
Tool/ToolPreset string annotations in tool_bias method signatures).
Per the 4-criteria decision rule: Tool + ToolPreset fail C1 (only used by
tool_presets + tool_bias), fail C2 (no state machine), fail C3 (no
dedicated test file), borderline C4 (~15 lines each). MERGE into the
existing src/tool_presets.py which already has ToolPresetManager.
This commit:
1. Adds Tool + ToolPreset class definitions to src/tool_presets.py at
the top (after the stdlib imports). Both classes are used by
ToolPresetManager and the tests.
2. Removes Tool + ToolPreset from src/models.py.
3. Adds lazy re-exports via the existing __getattr__ in src/models.py
(EAGER import would deadlock because src.tool_presets imports
BiasProfile from src.models; the lazy __getattr__ breaks the cycle).
4. Updates src/tool_presets.py import: from
'from src.models import ToolPreset, BiasProfile' to
'from src.models import BiasProfile' (ToolPreset is now local).
Verification: VC8 (Tool + ToolPreset)
from src.tool_presets import Tool, ToolPreset # OK
from src.models import Tool, ToolPreset # OK (lazy __getattr__)
Tool is Tool returns True
ToolPreset is ToolPreset returns True
Tests verified (7/7 PASS):
tests/test_tool_preset_manager.py (4 tests)
tests/test_bias_models.py (3 tests)
Consumer check:
src/ai_client.py: from src.models import FileItem, ToolPreset, BiasProfile, Tool
src/app_controller.py: (no Tool/ToolPreset import)
src/tool_bias.py: from src.models import Tool, ToolPreset, BiasProfile
All resolve via re-export/lazy __getattr__.
The lazy __getattr__ pattern is the same mechanism used for the
Pydantic proxies (GenerateRequest / ConfirmRequest) and for PROVIDERS.
Phase 5 will migrate Tool/ToolPreset to a similar lazy pattern in
the re-export block (or drop them entirely after the consumer
migration).
Per the 4-criteria decision rule (C1=cross-system, C3=tests, C4=substantial);
FileItem is the canonical per-file data structure used by aggregate,
app_controller, gui_2, presets, context_presets, and tests. Preset /
ContextPreset / ContextFileEntry / NamedViewPreset are the preset/view
data structures that round-trip through TOML.
This commit:
1. Creates src/project_files.py with FileItem + Preset + ContextPreset +
ContextFileEntry + NamedViewPreset (full class bodies copied verbatim
from src/models.py including __post_init__, to_dict, from_dict, and
the [C: ...] caller-docstring tags).
2. Removes the 5 class definitions from src/models.py.
3. Adds backward-compat re-exports in src/models.py (the same pattern
used by Phase 3a mma.py + Phase 3b project.py + Phase 3g personas.py).
4. Updates the 4 consumer files to import from src.project_files directly:
src/orchestrator_pm.py, src/presets.py, src/context_presets.py,
src/ai_client.py (3 sites of the banned 'local import + as _FIC alias'
pattern updated to use src.project_files.FileItem; the aliasing
anti-pattern is preserved for now - a follow-up track will remove
the local imports and the aliasing).
Verification: VC7
from src.project_files import FileItem, Preset, ContextPreset,
ContextFileEntry, NamedViewPreset # OK
from src.models import FileItem, Preset, ... # OK
(re-exports work; identity check: FileItem is FileItem returns True)
Tests verified (20/20 PASS):
tests/test_file_item_model.py (4 tests)
tests/test_view_presets.py (4 tests)
tests/test_context_presets_models.py (3 tests)
tests/test_custom_slices_annotations.py (3 tests)
tests/test_presets.py (5 tests)
Decorator-orphan pitfall caught and fixed: after removing the 3 classes
between WorkspaceProfile and the MCP Config region, the @dataclass
decorator was orphaned on a comment line. Removed the orphan.
Per the 4-criteria decision rule (C1=cross-system, C3=tests, C4=size);
ProjectContext is the typed return of project_manager.flat_config();
the 5 sub-dataclasses model the actual nested dict structure of
flat_config()'s return; load_config_from_disk / save_config_to_disk
are the canonical config I/O primitives (renamed from the private
_load_config_from_disk / _save_config_to_disk).
This commit:
1. Creates src/project.py with ProjectContext + 5 sub (ProjectMeta,
ProjectOutput, ProjectFiles, ProjectScreenshots, ProjectDiscussion)
+ EMPTY_PROJECT_CONTEXT + _clean_nones + load_config_from_disk +
save_config_to_disk + parse_history_entries.
2. Removes the original class + function definitions from src/models.py.
3. Adds backward-compat re-exports in src/models.py (the same pattern
used by Phase 3a mma.py and Phase 3g personas.py).
4. Updates src/app_controller.py to use the new public function names
(load_config_from_disk / save_config_to_disk).
5. Updates tests/test_models_no_top_level_tomli_w.py to use the new
public name (the test still asserts lazy-loading; the lazy load
happens in the new project.py module).
6. Updates scripts/audit_no_models_config_io.py FORBIDDEN_PATTERNS to
reference the new public names (models.load_config_from_disk /
models.save_config_to_disk) + the new src.project path.
Verification: VC6
uv run python -c 'from src.project import ProjectContext, ProjectMeta,
ProjectOutput, ProjectFiles, ProjectScreenshots, ProjectDiscussion,
_clean_nones, load_config_from_disk, save_config_to_disk,
parse_history_entries' # OK
uv run python -c 'from src.models import ProjectContext, ...' # OK
(re-exports work)
Pre-existing test regression (NOT caused by this commit):
tests/test_models_no_top_level_tomli_w.py::test_models_does_not_import_tomli_w_at_module_level
was already failing because the Phase 3g 'from src.personas import Persona'
re-export in src/models.py loads src.personas at module level, which
loads tomli_w. The Phase 5 reduce-models.py pass moves the persona
import into __getattr__ (lazy), which will make this test pass again.
Tests verified: tests/test_project_context_20260627.py (10/10 PASS),
tests/test_project_serialization.py (2/2 PASS), tests/test_thinking_persistence.py
(4/4 PASS), tests/test_presets.py (3/3 PASS), tests/test_persona_models.py
(2/2 PASS), tests/test_ticket_queue.py (PASS), tests/test_dag_engine.py
(PASS), tests/test_orchestration_logic.py (PASS).
Per spec FR4 + Phase 3.4: Persona dataclass + properties (provider/model/
temperature/top_p/max_output_tokens) + to_dict/from_dict move from
src/models.py into src/personas.py (which already has the PersonaManager
ops layer). Re-export at top of models.py preserves 'from src.models
import Persona'.
Per spec FR3/FR4 + Phase 3.1: the MMA domain dataclasses move to their own module:
- ThinkingSegment, Ticket, Track, WorkerContext, TrackMetadata, TrackState, EMPTY_TRACK_STATE
- TrackMetadata is the renamed (was 'Metadata' dataclass in models.py; renamed to avoid
collision with the Metadata type alias = dict[str, Any])
src/models.py:
- Removed class definitions for ThinkingSegment, Ticket, Track, WorkerContext, Metadata, TrackState, EMPTY_TRACK_STATE
- Added backward-compat re-exports so existing 'from src.models import Ticket' continues to work
- Metadata alias kept for the dataclass name (was confusingly shadowing the type alias)
TrackState's metadata field reverts to the original 'default_factory=dict' pattern
(intentionally not auto-constructing TrackMetadata) to preserve the pre-existing
behavior where accessing state.metadata.id on a missing state.toml throws
AttributeError, which project_manager.get_all_tracks catches and falls through
to metadata.json loading. This was a 'bug-on-purpose' that the test
test_get_all_tracks_with_metadata_json relies on.
Verification: 136 tests pass across mma_models, conductor_engine_v2, dag_engine,
ticket_queue, track_state_schema, thinking_gui, manual_block, pipeline_pause,
phase6_engine, parallel_execution, run_worker_lifecycle_abort, spawn_interception,
persona_id, conductor_engine_abort, conductor_tech_lead, execution_engine,
perf_dag, per_ticket_model, metadata_promotion_phase1, thinking_persistence,
progress_viz, gui_progress, mma_ticket_actions, headless_verification,
context_pruner, orchestration_logic, project_manager_tracks,
track_state_persistence.
For these 4 sites, the Result migration cascades badly (the function
returns a non-Result type that's used in many places). Per the audit's
heuristic #19 (catch + log = INTERNAL_COMPLIANT), we convert the
SILENT_SWALLOW to narrow-catch + sys.stderr.write. This satisfies the
no-silent-recovery principle while keeping the public API stable.
log_registry.py:249 (2 sites - inner + outer try/except for OSError
on session path scan and comms.log read)
models.py:508 (datetime.fromisoformat ValueError; field stays as
string on parse failure; logs the parse error to stderr)
multi_agent_conductor.py:317 (PersonaManager.load_all fallback for
ticket.persona_id lookup; logs the failure to stderr)
theme_2.py:282 (markdown_helper.get_renderer().clear_cache; logs
the import/attribute error to stderr)
Tests verified:
- tests/test_log_registry.py (5 tests) PASS
- tests/test_logging_e2e.py (1 test) PASS
- tests/test_auto_whitelist.py (4 tests) PASS
- tests/test_orchestration_logic.py (8 tests) PASS
- tests/test_mma_tier_usage_reset_fix.py (4 tests) PASS
Phase 2 tasks 2.1 + 2.2 + 2.3a of the follow-up track.
PROVIDERS now lives in src/ai_client.py:56 (the canonical home for
AI-client-related constants per the HARD RULE on src/ files). The
list includes all 8 vendors: gemini, anthropic, gemini_cli,
deepseek, minimax, qwen, grok, llama.
Backward compat: src/models.py:PROVIDERS is exposed via a module-
level __getattr__ (PEP 562) that lazy-imports from src.ai_client.
The lazy approach was needed because src.ai_client imports
ToolPreset/BiasProfile/Tool from src.models at line 50, so a
top-level 'from src.ai_client import PROVIDERS' in models.py
would deadlock. Adding a branch to the existing __getattr__
in models.py (which also handles pydantic class factories) is
the surgical fix.
tests/test_provider_curation.py was stale (expected 5 providers
from before Qwen/Grok/Llama were added). Updated to 8.
New test: tests/test_providers_source_of_truth.py asserts:
- src.ai_client.PROVIDERS exists and matches the 8-provider list
- src.models.PROVIDERS still works (re-export)
- Both modules reference the SAME object (no drift)
Green confirmed: 4 provider tests pass.
Side concerns for Phase 3:
1. PROVIDERS: src/models.py:56 now includes 'grok' and 'llama' alongside
the 6 existing vendors. Centralized registry; gui_2.py and
app_controller.py import from here. State tasks t3.5 and t3.16
were scoped to gui_2.py/app_controller.py but the actual change
is at the centralized registry, per the project's single-source-of-
truth pattern (per src/models.py module docstring and the Phase 5
audit script audit_no_models_config_io.py which enforces that
PROVIDERS lives in models.py).
2. cost_tracker.py: added 11 regex pricing entries (3 Grok + 8 Llama):
Grok (per xAI public pricing):
- grok-2: 2.00 / 10.00
- grok-2-vision: 2.00 / 10.00
- grok-beta: 5.00 / 15.00
Llama (per Grok's consultation: pricing varies by backend; registry
entries represent the most common case):
- llama-3.1-8b-instant: 0.05 / 0.08 (Groq)
- llama-3.1-70b-versatile: 0.59 / 0.79 (Groq)
- llama-3.1-405b-reasoning: 3.00 / 3.00 (OpenRouter avg)
- llama-3.2-1b-preview: 0.04 / 0.04
- llama-3.2-3b-preview: 0.06 / 0.06
- llama-3.2-11b-vision-preview: 0.18 / 0.18
- llama-3.2-90b-vision-preview: 0.90 / 0.90
- llama-3.3-70b-specdec: 0.59 / 0.79 (Groq)
(all per 1M tokens, USD; matches the structure of existing entries;
note: 'llama-3.1', 'llama-3.2', 'llama-3.3' are regex patterns to
allow future model variants in the same family.)
Spot check:
- estimate_cost('grok-2', 1000, 500) = 0.007 (= 0.002 + 0.005)
- estimate_cost('llama-3.3-70b-specdec', 1000, 500) = 0.000985
3. SKIPPED t3.4 and t3.15 (credentials templates): no
credentials_template.toml exists in the project (Phase 2 established
this). The user maintains their own credentials.toml directly.
4. t3.6 and t3.17 (Grok/Llama models in capability registry) were
completed in Phase 1's initial population of 22 entries
(commit 6be04bc). Grok has 4 entries (1 wildcard + 3 models);
Llama has 9 entries (1 wildcard + 8 models). Grok-2-vision has
vision=True; Llama 3.2-11b/90b vision variants have vision=True.
Verification: 38/38 tests pass in batch.
Side concerns for Phase 2:
1. PROVIDERS: src/models.py:56 now includes 'qwen' alongside the existing
5 vendors. The other 4 references to PROVIDERS in src/gui_2.py and
src/app_controller.py import from this centralized list, so this
one edit propagates everywhere. State task t2.8 was scoped to
'gui_2.py and app_controller.py' but the actual change is at the
centralized registry, per the project's single-source-of-truth
pattern (per src/models.py module docstring and the Phase 5 audit
script audit_no_models_config_io.py which enforces that PROVIDERS
lives in models.py).
2. cost_tracker.py: added 7 regex pricing entries for the Qwen models
shipped in Phase 1's vendor_capabilities.py:
- qwen-turbo: 0.05 / 0.10
- qwen-plus: 0.40 / 1.20
- qwen-max: 2.00 / 6.00
- qwen-long: 0.07 / 0.28
- qwen-vl-plus: 0.21 / 0.63
- qwen-vl-max: 0.50 / 1.50
- qwen-audio: 0.10 / 0.30
(all per 1M tokens, USD; matches the structure of existing entries)
Spot check: estimate_cost('qwen-max', 1000, 500) = 0.005 (= 0.002 + 0.003)
3. SKIPPED t2.7 (credentials template): no credentials_template.toml
exists in the project. The only credentials file is the active
credentials.toml which the user maintains directly with their own
API keys. The plan's assumption of a template file does not match
the project's actual structure. Documented in the commit log
rather than modifying the user's actual credentials.toml with a
placeholder key (which would be inconsistent with the rest of
that file's pattern of real keys). When the user obtains a
DashScope API key, they can add a [qwen] section directly.
4. t2.9 (Qwen models in capability registry) was completed in Phase 1's
initial population of 22 entries (commit 6be04bc). The 8 qwen
entries (1 wildcard + 7 specific models) are in src/vendor_capabilities.py.
Verification: 30/30 tests pass in batch
(test_qwen_provider, test_minimax_provider, test_ai_client_no_top_level_sdk_imports,
test_vendor_capabilities, test_openai_compatible, test_cost_tracker)
Eliminates 22 call sites that bypassed the AppController state owner
and read/wrote config.toml directly. AppController is now the single
source of truth for self.config; gui_2.py, commands.py, etc. go
through controller.save_config() / controller.load_config().
Production changes:
- src/models.py: rename load_config -> _load_config_from_disk,
save_config -> _save_config_to_disk (private I/O primitives)
- src/app_controller.py: add public load_config()/save_config() methods
that own the state. Update 3 internal call sites and 3 ConductorEngine
call sites to pass max_workers from self.config
- src/multi_agent_conductor.py: ConductorEngine.__init__ now takes
max_workers as a parameter (caller responsibility, not I/O primitive)
- src/external_editor.py: get_default_launcher() takes config as a
parameter; gui_2.py:1311,4776 pass app.config
- src/gui_2.py: 17 sites of models.save_config(X.config) replaced with
X.save_config() (delegates via __getattr__ to controller)
- src/commands.py: save_all() uses app.save_config()
Test changes (route through controller, not I/O primitive):
- tests/conftest.py: mock_app and app_instance fixtures now patch
AppController.load_config/save_config instead of models I/O primitives
- 18 other test files: patches renamed from models._save_config_to_disk
to AppController.save_config (and same for load_config)
- tests/test_app_controller_mcp.py: use SLOP_CONFIG env var instead of
patching removed CONFIG_PATH module constant
- tests/test_parallel_execution.py: pass max_workers=2 explicitly to
ConductorEngine (caller no longer reads config)
- tests/test_gui_paths.py: add save_config=MagicMock() to MockApp;
assert on controller method, not I/O primitive
- tests/test_models_no_top_level_tomli_w.py: still calls private
_save_config_to_disk directly (the only allowed exception; tests
the lazy-load behavior of the primitive itself)
New files:
- scripts/audit_no_models_config_io.py: enforces the rule (--strict,
--json modes; AST-based docstring detection to avoid false positives)
- conductor/code_styleguides/config_state_owner.md: documents the rule
Verification:
- 67 targeted tests pass
- scripts/audit_no_models_config_io.py --strict returns 0
This is the architectural cleanup that surfaced during the
audit_architectural_cheats_20260607 review. Closes the smoke-gun
CONFIG_PATH module constant (already done in 0c7ebf22) AND the
free-function models.load_config/save_config smell.
[conductor(checkpoint): config-iO-refactor-20260607]
ROOT CAUSE: src/models.py had `CONFIG_PATH = get_config_path()`
at module level. Every test that imported `src.models` and called
`save_config()` or `load_config()` wrote/read the repo-root
`config.toml` via this cached constant. The path was resolved
once at import time, so the SLOP_CONFIG env var (or test
fixtures) couldn't redirect reads/writes without reimporting the
module.
This silently corrupted the user's config.toml on every test
run. The diff between runs showed: 'config.toml changed in
working copy' — caused by tests, not the user.
FIX: remove the module-level constant; call get_config_path()
on every read/write call. SLOP_CONFIG (and any test-time
set_config_path() helper) now works without reimport.
Also: keep my prior commits to this file (reset_layout command
in src/commands.py; the RUN_MMA_INTEGRATION skipif in
test_mma_step_mode_sim.py) bundled here for a clean atomic
fix-pack since the user just fixed the indentation issue I had.
Verified: src.models imports cleanly; load_config/save_config
work as expected. Tests that import these functions will
use whatever SLOP_CONFIG points to (or the repo-root default).
Sub-track 2A of startup_speedup_20260606: clears 1 of 61 main-thread audit violations (pydantic in src/models.py).
Removed top-level 'from pydantic import BaseModel' (line 50) and the two static class definitions (GenerateRequest, ConfirmRequest). Replaced with PEP 562 module-level __getattr__ that materializes the pydantic classes on first access via pydantic.create_model() + _require_warmed('pydantic').
Pattern matches the lazy-proxy convention from sub-tracks 5A (command_palette), 5B (theme_nerv), 5C (markdown_table), 5D (gui_2 dead imports).
Result:
- pydantic NOT in sys.modules after 'import src.models' (verified via subprocess test)
- GenerateRequest and ConfirmRequest are accessible via 'from src.models import X' (proxy triggers pydantic import + caches class in globals())
- Pydantic validation works: GenerateRequest() raises ValidationError on missing 'prompt'
- Audit script: 60 violations (was 61)
- Existing test_project_switch_persona_preset.py: 8/9 pass; the 1 failure is the pre-existing ui_global_preset_name issue (unrelated)
Files changed:
- src/models.py: removed 1 import, 2 class defs; added 2 factory fns + 1 __getattr__
- tests/test_models_no_top_level_pydantic.py: new (7 tests; all pass)
Per user instruction, all implementation work is performed by the Tier 2 tech lead directly. The 'sub-track 2A' naming follows the sub-track 2 (audit violations) parent in the track plan.
Sub-track 2 of startup_speedup_20260606. Removes the top-level
'import tomli_w' from src/models.py and moves it inside save_config().
tomli_w (~30ms cold load) is now loaded only when the user saves
config, not on every src.models import.
This drops the audit violation count from 63 to 62.
Pydantic BaseModel (the other src/models.py violation) is left for
a future sub-track: deferring a class base requires a metaclass or
proxy pattern that's higher risk for the small (~50ms) saving.
3 new tests in tests/test_models_no_top_level_tomli_w.py:
- tomli_w NOT in sys.modules after import src.models
- save_config() still works (because tomli_w loads on-demand)
- save_config() actually triggers the import on first call
17 existing model tests pass (test_persona_models, test_bias_models,
test_context_presets_models, test_per_ticket_model, test_file_item_model).
Track.get_executable_tickets (in models.py) called TrackDAG at
runtime, forcing a top-level import of src.dag_engine into models.py
and creating a 2-cycle that broke whichever module loaded second
(Ticket was not yet defined when models.py loaded first; TrackDAG
was not yet defined when dag_engine.py loaded first).
Fix: hoist the method out of the Track dataclass and into a free
function get_executable_tickets(track) in dag_engine.py. models.py
no longer needs TrackDAG at all, so the cycle is one-directional
(models -> dag_engine) and resolves cleanly in any import order.
Tests updated:
- tests/test_mma_models.py: import get_executable_tickets and call
it instead of track.get_executable_tickets() (4 call sites)
- tests/test_conductor_engine_v2.py: comment update
Verified both import orders resolve cleanly:
forward: import src.models; import src.dag_engine -> OK
reverse: import src.dag_engine; import src.models -> OK
34 tests pass (test_mma_models, test_dag_engine, test_execution_engine,
test_arch_boundary_phase3, test_track_state_schema).
ed