From 01ddf9f163065dee484b474926b23f24397d6573 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Sun, 7 Jun 2026 10:01:40 -0400 Subject: [PATCH] refactor(models): remove top-level pydantic import; lazy pydantic via PEP 562 __getattr__ 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. --- src/models.py | 41 +++++-- tests/test_models_no_top_level_pydantic.py | 130 +++++++++++++++++++++ 2 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 tests/test_models_no_top_level_pydantic.py diff --git a/src/models.py b/src/models.py index ee1d528b..b64570c3 100644 --- a/src/models.py +++ b/src/models.py @@ -47,7 +47,6 @@ import tomllib from dataclasses import dataclass, field from pathlib import Path from typing import Any, Dict, List, Optional, Union -from pydantic import BaseModel from src.paths import get_config_path @@ -209,16 +208,38 @@ def parse_history_entries(history_strings: list[str], roles: list[str]) -> list[ #region: Pydantic Models -class GenerateRequest(BaseModel): - prompt: str - auto_add_history: bool = True - temperature: float | None = None - top_p: float | None = None - max_tokens: int | None = None +def _create_generate_request() -> type: + from src.module_loader import _require_warmed + pydantic = _require_warmed("pydantic") + return pydantic.create_model( + "GenerateRequest", + prompt=(str, ...), + auto_add_history=(bool, True), + temperature=(float | None, None), + top_p=(float | None, None), + max_tokens=(int | None, None), + ) -class ConfirmRequest(BaseModel): - approved: bool - script: Optional[str] = None +def _create_confirm_request() -> type: + from src.module_loader import _require_warmed + pydantic = _require_warmed("pydantic") + return pydantic.create_model( + "ConfirmRequest", + approved=(bool, ...), + script=(Optional[str], None), + ) + +_PYDANTIC_CLASS_FACTORIES: dict[str, callable] = { + "GenerateRequest": _create_generate_request, + "ConfirmRequest": _create_confirm_request, +} + +def __getattr__(name: str) -> Any: + if name in _PYDANTIC_CLASS_FACTORIES: + cls = _PYDANTIC_CLASS_FACTORIES[name]() + globals()[name] = cls + return cls + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") #region: MMA Core diff --git a/tests/test_models_no_top_level_pydantic.py b/tests/test_models_no_top_level_pydantic.py new file mode 100644 index 00000000..e85bf393 --- /dev/null +++ b/tests/test_models_no_top_level_pydantic.py @@ -0,0 +1,130 @@ +"""Tests that src/models.py has NO top-level pydantic import. + +Per the Main Thread Purity Invariant (spec.md:2.1), pydantic is heavy and +must not appear in the main-thread import chain. The two pydantic classes +(GenerateRequest, ConfirmRequest) are exposed via a module-level +__getattr__ proxy (PEP 562) that materializes them on first access. + +These tests run in a fresh subprocess to ensure no warmup state leaks +from the test runner. We assert: + - pydantic is NOT imported as a side effect of `import src.models` + - GenerateRequest and ConfirmRequest can be imported and instantiated + - Accessing the proxy triggers the pydantic import + - The static audit no longer flags src/models.py for pydantic +""" + +import subprocess +import sys +import textwrap +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent + + +def _run_in_subprocess(snippet: str) -> subprocess.CompletedProcess: + script = textwrap.dedent(snippet) + return subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + cwd=str(ROOT), + timeout=30, + ) + + +def test_models_does_not_import_pydantic_at_module_level() -> None: + res = _run_in_subprocess(""" + import sys + import src.models + print('pydantic' in sys.modules) + """) + assert res.returncode == 0, f"stderr: {res.stderr}" + assert res.stdout.strip() == "False", ( + f"src.models triggered pydantic import: {res.stdout}" + ) + + +def test_generate_request_works_when_explicitly_imported() -> None: + res = _run_in_subprocess(""" + from src.models import GenerateRequest + req = GenerateRequest(prompt="hello") + print(req.prompt) + print(req.auto_add_history) + """) + assert res.returncode == 0, f"stderr: {res.stderr}" + assert res.stdout.strip() == "hello\nTrue" + + +def test_confirm_request_works_when_explicitly_imported() -> None: + res = _run_in_subprocess(""" + from src.models import ConfirmRequest + req = ConfirmRequest(approved=True, script="echo hi") + print(req.approved) + print(req.script) + """) + assert res.returncode == 0, f"stderr: {res.stderr}" + assert res.stdout.strip() == "True\necho hi" + + +def test_pydantic_only_loaded_after_explicit_class_access() -> None: + res = _run_in_subprocess(""" + import sys + import src.models + assert 'pydantic' not in sys.modules, 'pydantic leaked into sys.modules at import time' + from src.models import GenerateRequest + print('pydantic' in sys.modules) + print(GenerateRequest.__bases__[0].__name__) + print(GenerateRequest.__bases__[0].__module__) + """) + assert res.returncode == 0, f"stderr: {res.stderr}" + lines = res.stdout.strip().splitlines() + assert lines[0] == "True", f"GenerateRequest access did not trigger pydantic import: {res.stdout}" + assert lines[1] == "BaseModel", f"GenerateRequest base is not BaseModel: {res.stdout}" + assert lines[2].startswith("pydantic"), f"GenerateRequest base is not from pydantic package: {res.stdout}" + + +def test_proxy_caches_real_class_for_repeated_access() -> None: + res = _run_in_subprocess(""" + from src.models import GenerateRequest + cls1 = GenerateRequest + cls2 = GenerateRequest + print(cls1 is cls2) + """) + assert res.returncode == 0, f"stderr: {res.stderr}" + assert res.stdout.strip() == "True", f"proxy did not cache the real class: {res.stdout}" + + +def test_generate_request_validation_rejects_missing_prompt() -> None: + res = _run_in_subprocess(""" + from src.models import GenerateRequest + try: + GenerateRequest() + print("NO_RAISE") + except Exception as e: + print(type(e).__name__) + """) + assert res.returncode == 0, f"stderr: {res.stderr}" + assert res.stdout.strip() != "NO_RAISE", "pydantic validation did not fire on missing required field" + assert "ValidationError" in res.stdout, f"expected ValidationError, got: {res.stdout}" + + +def test_audit_sees_no_pydantic_violation_in_models() -> None: + res = _run_in_subprocess(""" + import ast + from pathlib import Path + root = Path('.').resolve() + models_path = root / 'src' / 'models.py' + tree = ast.parse(models_path.read_text(encoding='utf-8')) + for node in tree.body: + if isinstance(node, ast.Import): + for alias in node.names: + if alias.name == 'pydantic' or alias.name.startswith('pydantic.'): + print('VIOLATION:', alias.name) + elif isinstance(node, ast.ImportFrom): + if node.module and (node.module == 'pydantic' or node.module.startswith('pydantic.')): + print('VIOLATION:', node.module) + print('OK') + """) + assert res.returncode == 0, f"stderr: {res.stderr}" + assert "OK" in res.stdout, f"audit script errored: {res.stderr}" + assert "VIOLATION" not in res.stdout, f"src/models.py still has top-level pydantic: {res.stdout}"