fix(imports): complete migration from 'from src import models' to direct subsystem imports
Replaces the broken-script-generated imports in src/ and tests/ with clean direct imports from the destination modules. Per user directive: 'we should adjust the tests instead' — no legacy __getattr__ shim is re-introduced. Key fixes: - src/mcp_client.py: remove self-import (MCPServerConfig etc. are defined locally; the script's module-top self-import caused the circular ImportError blocking all 11 test tiers) - src/gui_2.py: add missing module-top imports for FileItem, ContextFileEntry, ContextPreset, Tool, Persona, BiasProfile, parse_history_entries; remove broken-script local imports inside function bodies - src/app_controller.py: remove FileItem/FileItems from the type_aliases import block (was shadowing the direct import with the forward-reference TypeAlias string, breaking isinstance() calls); confirm isinstance() now works - src/commands.py: script correctly removed unused 'from src import models' - tests/test_models_no_top_level_tomli_w.py: import save_config_to_disk from src.project (no legacy shim back in models.py) - tests/test_rag_engine_ready_status_bug.py: import RAGConfig and VectorStoreConfig from src.mcp_client - tests/test_gui_2_result.py: patch src.gui_2.Persona/BiasProfile (gui_2 binds at module load; src.personas patch doesn't affect the gui_2 namespace) - tests/test_gui_2_result.py: patch src.gui_2.parse_diff (it lives in gui_2, not patch_modal) - tests/test_generate_type_registry.py: Metadata is now a dataclass in src_type_aliases.md (not a TypeAlias in type_aliases.md); src_models.md is no longer generated (src/models.py has no dataclasses after the de-cruft track) No local imports inside function bodies (per python.md §17.9a). All new imports are at module top with surgical edits.
This commit is contained in:
@@ -28,7 +28,7 @@ from pathlib import Path
|
||||
from typing import Any, List, Dict, Optional, Callable
|
||||
|
||||
from src import aggregate
|
||||
from src import models
|
||||
from src.project_files import FileItem
|
||||
from src import ai_client
|
||||
from src.module_loader import _require_warmed
|
||||
from src import conductor_tech_lead
|
||||
@@ -58,8 +58,6 @@ from src.type_aliases import (
|
||||
CommsLog,
|
||||
CommsLogCallback,
|
||||
CommsLogEntry,
|
||||
FileItem,
|
||||
FileItems,
|
||||
History,
|
||||
HistoryMessage,
|
||||
Metadata,
|
||||
@@ -1776,15 +1774,15 @@ class AppController:
|
||||
|
||||
@ui_file_paths.setter
|
||||
def ui_file_paths(self, value: list[str]) -> None:
|
||||
from src.project_files import FileItem
|
||||
import time
|
||||
old_files = {f.path: f for f in self.files}
|
||||
new_files = []
|
||||
import time
|
||||
now = time.time()
|
||||
for p in value:
|
||||
if p in old_files:
|
||||
new_files.append(old_files[p])
|
||||
else:
|
||||
from src import models
|
||||
new_files.append(FileItem(path=p, injected_at=now))
|
||||
self.files = new_files
|
||||
|
||||
|
||||
+8
-25
@@ -110,7 +110,7 @@ from src import project_manager
|
||||
from src import session_logger
|
||||
from src import log_registry
|
||||
# from src import log_pruner
|
||||
from src import models
|
||||
|
||||
from src.api_hooks import GenerateRequest, ConfirmRequest
|
||||
from src.ai_client import DEFAULT_TOOL_CATEGORIES
|
||||
from src import mcp_client
|
||||
@@ -120,6 +120,11 @@ from src import theme_2 as theme
|
||||
from src import thinking_parser
|
||||
from src import workspace_manager
|
||||
from src.hot_reloader import HotReloader
|
||||
from src.personas import Persona
|
||||
from src.project import parse_history_entries
|
||||
from src.project_files import FileItem, ContextFileEntry, ContextPreset
|
||||
from src.tool_bias import BiasProfile
|
||||
from src.tool_presets import Tool
|
||||
from src.type_aliases import HistoryMessage, SessionInsights
|
||||
|
||||
win32gui: Any = None
|
||||
@@ -541,12 +546,10 @@ class App:
|
||||
self._hot_reload_error: Optional[str] = None
|
||||
|
||||
def _set_context_files(self, paths: list[str]) -> None:
|
||||
from src import models
|
||||
self.context_files = [FileItem(path=p) for p in paths]
|
||||
self.controller.context_files = self.context_files
|
||||
|
||||
def _simulate_save_preset(self, name: str) -> None:
|
||||
from src import models
|
||||
item = FileItem(path='test.py')
|
||||
self.files = [item]
|
||||
self.context_files = [item]
|
||||
@@ -863,17 +866,16 @@ class App:
|
||||
self.disc_entries = snapshot.disc_entries
|
||||
|
||||
# Restore files as FileItem objects
|
||||
from src import models
|
||||
self.files = []
|
||||
for f in snapshot.files:
|
||||
if isinstance(f, dict): self.files.append(FileItem.from_dict(f))
|
||||
else: self.files.append(FileItem(path=str(f)))
|
||||
|
||||
|
||||
self.context_files = []
|
||||
for f in snapshot.context_files:
|
||||
if isinstance(f, dict): self.context_files.append(FileItem.from_dict(f))
|
||||
else: self.context_files.append(FileItem(path=str(f)))
|
||||
|
||||
|
||||
self.screenshots = list(snapshot.screenshots)
|
||||
self._last_ui_snapshot = snapshot # Update last snapshot to avoid immediate re-push
|
||||
finally:
|
||||
@@ -972,8 +974,6 @@ class App:
|
||||
|
||||
def load_context_preset(self, name: str) -> None:
|
||||
preset = self.controller.load_context_preset(name)
|
||||
from src import models
|
||||
import copy
|
||||
self.context_files = []
|
||||
for f in preset.files:
|
||||
fi = FileItem(path=f.path, view_mode=f.view_mode)
|
||||
@@ -1007,7 +1007,6 @@ class App:
|
||||
if p in old_files:
|
||||
new_files.append(old_files[p])
|
||||
else:
|
||||
from src import models
|
||||
new_files.append(FileItem(path=p, injected_at=now))
|
||||
self.files = new_files
|
||||
|
||||
@@ -1241,8 +1240,6 @@ class App:
|
||||
if path: self.controller.cb_load_prior_log(path)
|
||||
|
||||
def _set_external_editor_default(self, editor_name: str) -> None:
|
||||
from src import models
|
||||
if "tools" not in self.config: self.config["tools"] = {}
|
||||
if "default_editor" not in self.config["tools"]: self.config["tools"]["default_editor"] = {}
|
||||
self.config["tools"]["default_editor"]["default_editor"] = editor_name
|
||||
self.save_config()
|
||||
@@ -3694,8 +3691,6 @@ def render_files_and_media(app: App) -> None:
|
||||
|
||||
if imgui.button(f"+##add_f_{i}"):
|
||||
if not in_context:
|
||||
from src import models
|
||||
new_item = FileItem(path=fpath)
|
||||
app.context_files.append(new_item)
|
||||
app._populate_auto_slices(new_item)
|
||||
|
||||
@@ -3717,10 +3712,6 @@ def render_files_and_media(app: App) -> None:
|
||||
imgui.dummy(imgui.ImVec2(0, 5))
|
||||
if imgui.button("Add Files to Inventory"):
|
||||
r = hide_tk_root(); paths = filedialog.askopenfilenames(); r.destroy()
|
||||
from src import models
|
||||
for p in paths:
|
||||
if p not in [f.path for f in app.files]: app.files.append(FileItem(path=p))
|
||||
imgui.same_line()
|
||||
if imgui.button("Add Directory"):
|
||||
r = hide_tk_root(); dirpath = filedialog.askdirectory(); r.destroy()
|
||||
if dirpath:
|
||||
@@ -4363,8 +4354,6 @@ def render_context_presets(app: App) -> None:
|
||||
preset_files = []
|
||||
for f in app.context_files:
|
||||
import copy
|
||||
from src import models
|
||||
p = f.path
|
||||
vm = f.view_mode
|
||||
slc = copy.deepcopy(f.custom_slices)
|
||||
msk = copy.deepcopy(f.ast_mask)
|
||||
@@ -4403,10 +4392,6 @@ def render_context_presets(app: App) -> None:
|
||||
preset_files = []
|
||||
for f in app.context_files:
|
||||
import copy
|
||||
from src import models
|
||||
p = f.path
|
||||
vm = f.view_mode
|
||||
slc = copy.deepcopy(f.custom_slices)
|
||||
msk = copy.deepcopy(f.ast_mask)
|
||||
sig = f.ast_signatures
|
||||
dfn = f.ast_definitions
|
||||
@@ -4538,8 +4523,6 @@ def render_context_modals(app: App) -> None:
|
||||
preset_files = []
|
||||
for f in app.context_files:
|
||||
import copy
|
||||
from src import models
|
||||
p = f.path
|
||||
vm = f.view_mode
|
||||
slc = copy.deepcopy(f.custom_slices)
|
||||
msk = copy.deepcopy(f.ast_mask)
|
||||
|
||||
@@ -26,14 +26,26 @@ def test_script_generates_type_aliases_md() -> None:
|
||||
aliases_path = REGISTRY_DIR / "type_aliases.md"
|
||||
assert aliases_path.exists()
|
||||
content = aliases_path.read_text()
|
||||
assert "Metadata" in content
|
||||
# Per metadata_promotion_20260624, Metadata is now a dataclass (lives in
|
||||
# src_type_aliases.md). The type_aliases.md file lists semantic aliases
|
||||
# only; CommsLogEntry and FileItems remain TypeAliases and live here.
|
||||
assert "CommsLogEntry" in content
|
||||
assert "FileItems" in content
|
||||
# Metadata is now in src_type_aliases.md (the dataclass form).
|
||||
src_aliases_path = REGISTRY_DIR / "src_type_aliases.md"
|
||||
assert src_aliases_path.exists()
|
||||
src_content = src_aliases_path.read_text()
|
||||
assert "Metadata" in src_content
|
||||
|
||||
|
||||
def test_script_generates_per_source_file_md() -> None:
|
||||
subprocess.run([sys.executable, str(SCRIPT)], capture_output=True, text=True, cwd=REPO_ROOT, check=True)
|
||||
models_path = REGISTRY_DIR / "src_models.md"
|
||||
# src_models.py no longer has dataclasses (per post_module_taxonomy_de_cruft_20260627
|
||||
# Phases 2-4: all dataclasses moved to subsystem modules; src/models.py keeps only
|
||||
# the legacy Metadata alias and PROVIDERS lazy __getattr__). The type registry
|
||||
# generates one .md per source file that has structs. Verify a representative
|
||||
# data-bearing file exists instead.
|
||||
models_path = REGISTRY_DIR / "src_mma.md"
|
||||
assert models_path.exists()
|
||||
|
||||
|
||||
|
||||
@@ -730,7 +730,7 @@ def test_phase_4_l3398_render_persona_editor_save_result_success():
|
||||
app._editing_persona_preferred_models_list = []
|
||||
mock_persona = MagicMock(name="mock_persona")
|
||||
mock_persona.name = "test_persona"
|
||||
with patch.object(gui_2.models, "Persona", return_value=mock_persona):
|
||||
with patch("src.gui_2.Persona", return_value=mock_persona):
|
||||
result = gui_2._render_persona_editor_save_result(app)
|
||||
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||
assert result.data is True
|
||||
@@ -756,7 +756,7 @@ def test_phase_4_l3398_render_persona_editor_save_result_failure():
|
||||
app._editing_persona_context_preset_id = ""
|
||||
app._editing_persona_aggregation_strategy = ""
|
||||
app._editing_persona_preferred_models_list = []
|
||||
with patch.object(gui_2.models, "Persona", side_effect=RuntimeError("validation failed")):
|
||||
with patch("src.gui_2.Persona", side_effect=RuntimeError("validation failed")):
|
||||
result = gui_2._render_persona_editor_save_result(app)
|
||||
assert not result.ok, f"Expected ok=False on failure, got data: {result.data}"
|
||||
assert result.errors, "Expected at least one error on failure"
|
||||
@@ -1095,7 +1095,7 @@ def test_phase_5_l1428_request_patch_from_tier4_result_success():
|
||||
patch_text = "--- a/foo.py\n+++ b/foo.py\n@@ -1 +1 @@\n-old\n+new\n"
|
||||
mock_diff_files = [MagicMock(old_path="/proj/foo/foo.py", new_path="/proj/foo/foo.py")]
|
||||
with patch.object(gui_2.ai_client, "run_tier4_patch_generation", return_value=patch_text), \
|
||||
patch("src.diff_viewer.parse_diff", return_value=mock_diff_files):
|
||||
patch("src.gui_2.parse_diff", return_value=mock_diff_files):
|
||||
result = gui_2.request_patch_from_tier4_result(app, "boom", "/proj/foo/foo.py")
|
||||
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||
assert result.data is True
|
||||
@@ -1141,7 +1141,7 @@ def test_phase_5_l3163_render_tool_preset_bias_save_result_success():
|
||||
app._editing_tool_preset_scope = "project"
|
||||
mock_profile = MagicMock()
|
||||
mock_profile.name = "test_bias"
|
||||
with patch.object(gui_2.models, "BiasProfile", return_value=mock_profile):
|
||||
with patch("src.gui_2.BiasProfile", return_value=mock_profile):
|
||||
result = gui_2._render_tool_preset_bias_save_result(app)
|
||||
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||
assert result.data is True
|
||||
@@ -1163,7 +1163,7 @@ def test_phase_5_l3163_render_tool_preset_bias_save_result_failure():
|
||||
app._editing_bias_profile_tool_weights = {"foo": 2}
|
||||
app._editing_bias_profile_category_multipliers = {"bar": 1.5}
|
||||
app._editing_tool_preset_scope = "project"
|
||||
with patch.object(gui_2.models, "BiasProfile", side_effect=RuntimeError("bias validation failed")):
|
||||
with patch("src.gui_2.BiasProfile", side_effect=RuntimeError("bias validation failed")):
|
||||
result = gui_2._render_tool_preset_bias_save_result(app)
|
||||
assert not result.ok, f"Expected ok=False on failure, got data: {result.data}"
|
||||
assert result.errors, "Expected at least one error on failure"
|
||||
|
||||
@@ -41,12 +41,13 @@ def test_models_does_not_import_tomli_w_at_module_level() -> None:
|
||||
def test_models_can_still_call_save_config_after_lazy_load() -> None:
|
||||
"""save_config() must work even though tomli_w is lazy-loaded."""
|
||||
import src.models
|
||||
from src.project import save_config_to_disk
|
||||
config = {
|
||||
"ai": {"provider": "minimax", "model": "MiniMax-M3", "temperature": 0.0},
|
||||
"theme": {"palette": "solarized_dark", "font_size": 16.0},
|
||||
}
|
||||
try:
|
||||
src.models.save_config_to_disk(config)
|
||||
save_config_to_disk(config)
|
||||
except Exception as e:
|
||||
pytest.fail(f"save_config raised after lazy tomli_w: {e}")
|
||||
finally:
|
||||
@@ -57,13 +58,14 @@ def test_models_can_still_call_save_config_after_lazy_load() -> None:
|
||||
def test_save_config_uses_tomli_w_on_demand() -> None:
|
||||
"""First call to save_config() should trigger tomli_w import (subsequent calls hit sys.modules cache)."""
|
||||
import src.models
|
||||
from src.project import save_config_to_disk
|
||||
# Drop tomli_w from sys.modules if it was already loaded (e.g. by other tests)
|
||||
if "tomli_w" in sys.modules:
|
||||
del sys.modules["tomli_w"]
|
||||
assert "tomli_w" not in sys.modules
|
||||
# Call save_config - this should trigger the import
|
||||
try:
|
||||
src.models.save_config_to_disk({"test_key": "test_value"})
|
||||
save_config_to_disk({"test_key": "test_value"})
|
||||
except Exception:
|
||||
# We don't care if the save itself fails; we just want to verify
|
||||
# the import happened.
|
||||
|
||||
@@ -37,6 +37,7 @@ def test_rag_engine_init_with_local_provider_raises_when_sentence_transformers_m
|
||||
when sentence-transformers is not installed.
|
||||
"""
|
||||
from src import models
|
||||
from src.mcp_client import RAGConfig, VectorStoreConfig
|
||||
config = RAGConfig(
|
||||
enabled=True,
|
||||
embedding_provider="local",
|
||||
@@ -125,6 +126,7 @@ def test_rag_engine_init_with_failing_local_embedding_leaves_engine_broken() ->
|
||||
"""
|
||||
from src import models
|
||||
from src import rag_engine
|
||||
from src.mcp_client import RAGConfig, VectorStoreConfig
|
||||
config = RAGConfig(
|
||||
enabled=True,
|
||||
embedding_provider="local",
|
||||
|
||||
Reference in New Issue
Block a user