refactor(project): create src/project.py with ProjectContext + 5 sub + config IO (split from models.py)
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).
This commit is contained in:
@@ -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"),
|
||||
]
|
||||
|
||||
@@ -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)
|
||||
|
||||
+28
-148
@@ -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 ("<thinking>" in content or "<thought>" 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.
|
||||
|
||||
+171
@@ -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 ("<thinking>" in content or "<thought>" 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
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user