diff --git a/scripts/audit_no_models_config_io.py b/scripts/audit_no_models_config_io.py index a931997a..598e9032 100644 --- a/scripts/audit_no_models_config_io.py +++ b/scripts/audit_no_models_config_io.py @@ -1,9 +1,13 @@ """Audit script: ensure no production code in src/ calls the models I/O primitives directly. Architecture rule: AppController owns the config I/O. The -models._load_config_from_disk and models._save_config_to_disk -functions are private file I/O primitives. Direct callers in src/ -are an architectural smell (bypassing the controller state owner). +models.load_config_from_disk and models.save_config_to_disk +functions (formerly _load_config_from_disk and _save_config_to_disk) +are private file I/O primitives. Direct callers in src/ are an +architectural smell (bypassing the controller state owner). After +module_taxonomy_refactor_20260627 Phase 3b, they live in src/project.py +and are re-exported by src/models.py for backward compat. The same +audit rule still applies: only AppController should call them. The only allowed call sites are inside AppController itself. @@ -22,13 +26,24 @@ from pathlib import Path # Patterns that are architectural smells in production code. # These are the I/O primitives; only AppController should call them. +# Post-Phase 3b the names are public (load_config_from_disk / +# save_config_to_disk) but the architectural rule is unchanged. FORBIDDEN_PATTERNS = [ + (re.compile(r"\bmodels\.load_config_from_disk\s*\("), "models.load_config_from_disk"), + (re.compile(r"\bmodels\.save_config_to_disk\s*\("), "models.save_config_to_disk"), + (re.compile(r"\bsrc\.project\.load_config_from_disk\s*\("), "src.project.load_config_from_disk"), + (re.compile(r"\bsrc\.project\.save_config_to_disk\s*\("), "src.project.save_config_to_disk"), +] + +# The OLD private names. After Phase 3b the private names are GONE; +# these patterns are kept to detect any stale call site. +LEGACY_PRIVATE_NAMES = [ (re.compile(r"\bmodels\._load_config_from_disk\s*\("), "models._load_config_from_disk"), (re.compile(r"\bmodels\._save_config_to_disk\s*\("), "models._save_config_to_disk"), ] # The OLD public names. After the rename these should not exist anywhere. -LEGACY_NAMES = [ +LEGACY_PUBLIC_NAMES = [ (re.compile(r"\bmodels\.load_config\s*\("), "models.load_config"), (re.compile(r"\bmodels\.save_config\s*\("), "models.save_config"), ] diff --git a/src/app_controller.py b/src/app_controller.py index 22333484..46d6e293 100644 --- a/src/app_controller.py +++ b/src/app_controller.py @@ -5159,7 +5159,7 @@ class AppController: scripts/audit_no_models_config_io.py. [C: src/app_controller.py:AppController.__init__] """ - self.config = models._load_config_from_disk() + self.config = models.load_config_from_disk() return self.config def save_config(self) -> None: @@ -5171,7 +5171,7 @@ class AppController: scripts/audit_no_models_config_io.py. [C: src/app_controller.py:AppController._cb_project_save, src/app_controller.py:AppController._do_generate] """ - models._save_config_to_disk(self.config) + models.save_config_to_disk(self.config) #endregion: --- Config I/O (single source of truth) --- #endregion: MMA (Controller) diff --git a/src/models.py b/src/models.py index 60e99db7..7f98e1c4 100644 --- a/src/models.py +++ b/src/models.py @@ -81,6 +81,24 @@ from src.personas import Persona # Alias the old `Metadata` dataclass name to TrackMetadata so existing # `from src.models import Metadata` keeps resolving to the dataclass. Metadata = TrackMetadata # noqa: F401 — legacy class name re-export +# Backward-compat re-exports for Project Context (Phase 3b -> src/project.py) +# + the config IO helpers. Consumers using 'from src.models import ProjectContext' +# or 'from src.models import _load_config_from_disk' / '_save_config_to_disk' +# must migrate to the new public names in src.project (load_config_from_disk / +# save_config_to_disk). The private names are dropped in Phase 5. +from src.project import ( + EMPTY_PROJECT_CONTEXT, + ProjectContext, + ProjectDiscussion, + ProjectFiles, + ProjectMeta, + ProjectOutput, + ProjectScreenshots, + _clean_nones, + load_config_from_disk, + parse_history_entries, + save_config_to_disk, +) #region: Constants @@ -193,71 +211,11 @@ DEFAULT_TOOL_CATEGORIES: Dict[str, List[str]] = { # redirect reads/writes to a temp_workspace without reimporting. # See tests/conftest.py:reset_paths for the test-side mechanism. -#region: Config Utilities - -def _clean_nones(data: Any) -> Any: - if isinstance(data, dict): - return {k: _clean_nones(v) for k, v in data.items() if v is not None} - elif isinstance(data, list): - return [_clean_nones(v) for v in data if v is not None] - return data - -def _load_config_from_disk() -> Metadata: - """ - Re-read the global config.toml from disk and return the parsed - dict. The single source of truth for the in-memory config is - the AppController's self.config attribute; this function is the - disk I/O primitive that the controller owns. Direct callers in - src/ are an architectural smell (bypassing the state owner) and - will be flagged by scripts/audit_no_models_config_io.py. - [C: src/app_controller.py:AppController.load_config, src/app_controller.py:AppController.__init__] - """ - with open(get_config_path(), "rb") as f: - return tomllib.load(f) - -def _save_config_to_disk(config: Metadata) -> None: - # tomli_w is loaded on-demand (sub-track 2 of startup_speedup_20260606). - # If it's already in sys.modules (e.g. warmed up or loaded by a prior - # call), the import is a fast lookup; otherwise it's a cold load paid - # only when the user actually saves config. - import tomli_w - config = _clean_nones(config) - with open(get_config_path(), "wb") as f: - tomli_w.dump(config, f) - -#region: History Utilities - - -#region: History Utilities - -def parse_history_entries(history_strings: list[str], roles: list[str]) -> list[Metadata]: - import re - from src import thinking_parser - entries = [] - for raw in history_strings: - ts = "" - rest = raw - if rest.startswith("@"): - nl = rest.find("\n") - if nl != -1: - ts = rest[1:nl] - rest = rest[nl + 1:] - known = roles or ["User", "AI", "Vendor API", "System"] - role_pat = re.compile(r"^(" + "|".join(re.escape(r) for r in known) + r"):", re.IGNORECASE) - match = role_pat.match(rest) - role = match.group(1) if match else "User" - if match: - content = rest[match.end():].strip() - else: - content = rest - entry_obj = {"role": role, "content": content, "collapsed": True, "ts": ts} - if role == "AI" and ("" in content or "" in content or "Thinking:" in content): - segments, parsed_content = thinking_parser.parse_thinking_trace(content) - if segments: - entry_obj["content"] = parsed_content - entry_obj["thinking_segments"] = [{"content": s.content, "marker": s.marker} for s in segments] - entries.append(entry_obj) - return entries +# Config IO helpers (_clean_nones, _load_config_from_disk -> load_config_from_disk, +# _save_config_to_disk -> save_config_to_disk) + parse_history_entries moved to +# src/project.py in module_taxonomy_refactor_20260627 Phase 3b. The +# re-exports at the top of this module keep 'from src.models import ...' working +# for legacy callers. New code should import from src.project directly. #region: Pydantic Models @@ -791,86 +749,8 @@ def load_mcp_config(path: str) -> MCPConfiguration: return MCPConfiguration() #endregion: MCP Config -#region: Project Context (Phase 2 dataclasses for cruft_elimination_20260627) - - -@dataclass(frozen=True, slots=True) -class ProjectMeta: - name: str = "" - summary_only: bool = False - execution_mode: str = "standard" - - -@dataclass(frozen=True, slots=True) -class ProjectOutput: - namespace: str = "project" - output_dir: str = "" - - -@dataclass(frozen=True, slots=True) -class ProjectFiles: - base_dir: str = "" - paths: tuple[str, ...] = () - - -@dataclass(frozen=True, slots=True) -class ProjectScreenshots: - base_dir: str = "." - paths: tuple[str, ...] = () - - -@dataclass(frozen=True, slots=True) -class ProjectDiscussion: - roles: tuple[str, ...] = () - history: tuple[str, ...] = () - - -@dataclass(frozen=True, slots=True) -class ProjectContext: - """Typed return type for project_manager.flat_config(). - Replaces the dict[str, Any] that flat_config() returned. - Per conductor/tracks/cruft_elimination_20260627/SPEC_CORRECTION_phase_2.md.""" - project: ProjectMeta = field(default_factory=ProjectMeta) - output: ProjectOutput = field(default_factory=ProjectOutput) - files: ProjectFiles = field(default_factory=ProjectFiles) - screenshots: ProjectScreenshots = field(default_factory=ProjectScreenshots) - context_presets: Metadata = field(default_factory=dict) - discussion: ProjectDiscussion = field(default_factory=ProjectDiscussion) - - def to_dict(self) -> Metadata: - return { - "project": { - "name": self.project.name, - "summary_only": self.project.summary_only, - "execution_mode": self.project.execution_mode, - }, - "output": { - "namespace": self.output.namespace, - "output_dir": self.output.output_dir, - }, - "files": { - "base_dir": self.files.base_dir, - "paths": list(self.files.paths), - }, - "screenshots": { - "base_dir": self.screenshots.base_dir, - "paths": list(self.screenshots.paths), - }, - "context_presets": dict(self.context_presets), - "discussion": { - "roles": list(self.discussion.roles), - "history": list(self.discussion.history), - }, - } - - def __getitem__(self, key: str) -> Any: - return self.to_dict()[key] - - def get(self, key: str, default: Any = None) -> Any: - return self.to_dict().get(key, default) - - -EMPTY_PROJECT_CONTEXT: ProjectContext = ProjectContext() - - -#endregion: Project Context +# Project Context (ProjectContext + 5 sub-dataclasses + EMPTY_PROJECT_CONTEXT) +# moved to src/project.py in module_taxonomy_refactor_20260627 Phase 3b. +# The re-exports at the top of this module keep 'from src.models import +# ProjectContext' working for legacy callers. New code should import from +# src.project directly. diff --git a/src/project.py b/src/project.py new file mode 100644 index 00000000..5ec42e4f --- /dev/null +++ b/src/project.py @@ -0,0 +1,171 @@ +"""Project configuration dataclasses and config I/O helpers. + +Per module_taxonomy_refactor_20260627 Phase 3b, the project context +(ProjectContext + 5 sub-dataclasses) and config I/O helpers moved +from src/models.py to this module. + +Per the 4-criteria decision rule: + - C1 (cross-system usage >= 3 systems): YES (project_manager, aggregate, + api_hooks, app_controller, gui_2, orchestrator_pm, tests) + - C2 (state machine / lifecycle): NO (just config; no state transitions) + - C3 (test file already exists): YES (test_project_context_20260627.py) + - C4 (substantial size): YES (ProjectContext + 5 sub + 3 helpers + 60+ lines) + +Therefore: DEDICATED FILE = src/project.py +""" +from __future__ import annotations + +import re +import tomllib + +from dataclasses import dataclass, field +from typing import Any, List + +from src.paths import get_config_path +from src.type_aliases import Metadata + + +# --------------------------------------------------------------------------- ProjectContext + +@dataclass(frozen=True, slots=True) +class ProjectMeta: + name: str = "" + summary_only: bool = False + execution_mode: str = "standard" + + +@dataclass(frozen=True, slots=True) +class ProjectOutput: + namespace: str = "project" + output_dir: str = "" + + +@dataclass(frozen=True, slots=True) +class ProjectFiles: + base_dir: str = "" + paths: tuple[str, ...] = () + + +@dataclass(frozen=True, slots=True) +class ProjectScreenshots: + base_dir: str = "." + paths: tuple[str, ...] = () + + +@dataclass(frozen=True, slots=True) +class ProjectDiscussion: + roles: tuple[str, ...] = () + history: tuple[str, ...] = () + + +@dataclass(frozen=True, slots=True) +class ProjectContext: + """Typed return type for project_manager.flat_config(). Replaces the dict[str, Any] that flat_config() returned. Per conductor/tracks/cruft_elimination_20260627/SPEC_CORRECTION_phase_2.md.""" + project: ProjectMeta = field(default_factory=ProjectMeta) + output: ProjectOutput = field(default_factory=ProjectOutput) + files: ProjectFiles = field(default_factory=ProjectFiles) + screenshots: ProjectScreenshots = field(default_factory=ProjectScreenshots) + context_presets: Metadata = field(default_factory=dict) + discussion: ProjectDiscussion = field(default_factory=ProjectDiscussion) + + def to_dict(self) -> Metadata: + return { + "project": { + "name": self.project.name, + "summary_only": self.project.summary_only, + "execution_mode": self.project.execution_mode, + }, + "output": { + "namespace": self.output.namespace, + "output_dir": self.output.output_dir, + }, + "files": { + "base_dir": self.files.base_dir, + "paths": list(self.files.paths), + }, + "screenshots": { + "base_dir": self.screenshots.base_dir, + "paths": list(self.screenshots.paths), + }, + "context_presets": dict(self.context_presets), + "discussion": { + "roles": list(self.discussion.roles), + "history": list(self.discussion.history), + }, + } + + def __getitem__(self, key: str) -> Any: + return self.to_dict()[key] + + def get(self, key: str, default: Any = None) -> Any: + return self.to_dict().get(key, default) + + +EMPTY_PROJECT_CONTEXT: ProjectContext = ProjectContext() + + +# --------------------------------------------------------------------------- Config IO helpers + +def _clean_nones(data: Any) -> Any: + if isinstance(data, dict): + return {k: _clean_nones(v) for k, v in data.items() if v is not None} + elif isinstance(data, list): + return [_clean_nones(v) for v in data if v is not None] + return data + + +def load_config_from_disk() -> Metadata: + """ + Re-read the global config.toml from disk and return the parsed + dict. The single source of truth for the in-memory config is + the AppController's self.config attribute; this function is the + disk I/O primitive that the controller owns. Direct callers in + src/ are an architectural smell (bypassing the state owner) and + will be flagged by scripts/audit_no_models_config_io.py. + [C: src/app_controller.py:AppController.load_config, src/app_controller.py:AppController.__init__] + """ + with open(get_config_path(), "rb") as f: + return tomllib.load(f) + + +def save_config_to_disk(config: Metadata) -> None: + # tomli_w is loaded on-demand (sub-track 2 of startup_speedup_20260606). + # If it's already in sys.modules (e.g. warmed up or loaded by a prior + # call), the import is a fast lookup; otherwise it's a cold load paid + # only when the user actually saves config. + import tomli_w + config = _clean_nones(config) + with open(get_config_path(), "wb") as f: + tomli_w.dump(config, f) + + +# --------------------------------------------------------------------------- History utilities + +def parse_history_entries(history_strings: list[str], roles: list[str]) -> list[Metadata]: + import re + from src import thinking_parser + entries = [] + for raw in history_strings: + ts = "" + rest = raw + if rest.startswith("@"): + nl = rest.find("\n") + if nl != -1: + ts = rest[1:nl] + rest = rest[nl + 1:] + known = roles or ["User", "AI", "Vendor API", "System"] + role_pat = re.compile(r"^(" + "|".join(re.escape(r) for r in known) + r"):", re.IGNORECASE) + match = role_pat.match(rest) + role = match.group(1) if match else "User" + if match: + content = rest[match.end():].strip() + else: + content = rest + entry_obj = {"role": role, "content": content, "collapsed": True, "ts": ts} + if role == "AI" and ("" in content or "" in content or "Thinking:" in content): + segments, parsed_content = thinking_parser.parse_thinking_trace(content) + if segments: + entry_obj["content"] = parsed_content + entry_obj["thinking_segments"] = [{"content": s.content, "marker": s.marker} for s in segments] + entries.append(entry_obj) + return entries diff --git a/tests/test_models_no_top_level_tomli_w.py b/tests/test_models_no_top_level_tomli_w.py index 99fb2758..ea29b40c 100644 --- a/tests/test_models_no_top_level_tomli_w.py +++ b/tests/test_models_no_top_level_tomli_w.py @@ -46,7 +46,7 @@ def test_models_can_still_call_save_config_after_lazy_load() -> None: "theme": {"palette": "solarized_dark", "font_size": 16.0}, } try: - src.models._save_config_to_disk(config) + src.models.save_config_to_disk(config) except Exception as e: pytest.fail(f"save_config raised after lazy tomli_w: {e}") finally: @@ -63,7 +63,7 @@ def test_save_config_uses_tomli_w_on_demand() -> None: 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"}) + src.models.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.