diff --git a/conductor/code_styleguides/config_state_owner.md b/conductor/code_styleguides/config_state_owner.md new file mode 100644 index 00000000..72fa7b9a --- /dev/null +++ b/conductor/code_styleguides/config_state_owner.md @@ -0,0 +1,106 @@ +# Config I/O State Ownership + +**Rule:** The `AppController` is the single source of truth for the +in-memory config (`self.config`) and the only authorized caller of +the file I/O primitives in `src/models.py`. + +## Why + +1. **The controller owns the in-memory state.** If other modules + write to `config.toml` directly, the controller's `self.config` + silently drifts from disk. Tests can corrupt the user's TOML + files; users lose data without warning. +2. **Test isolation breaks.** When `models.save_config(...)` is + called from anywhere in `src/`, tests cannot intercept the + write without patching the I/O primitive. The test then + couples to the file format, not the controller's behavior. +3. **Path resolution can't be enforced.** The controller respects + `SLOP_CONFIG` env var at call time. Direct calls to + `models.save_config` would only respect it if the path is + re-resolved (which it is in `_save_config_to_disk`, but only + because someone remembered). + +## What is Forbidden in `src/` + +- `models.load_config(...)` (legacy public function) +- `models.save_config(...)` (legacy public function) +- `models._load_config_from_disk(...)` (private I/O primitive) +- `models._save_config_to_disk(...)` (private I/O primitive) + +The only allowed call sites are inside `AppController` itself +(`load_config()` and `save_config()` methods). + +## The Public API + +```python +# In AppController: +def load_config(self) -> Dict[str, Any]: + """Re-read the global config.toml from disk and update self.config.""" + self.config = models._load_config_from_disk() + return self.config + +def save_config(self) -> None: + """Flush self.config to disk.""" + models._save_config_to_disk(self.config) +``` + +Callers (including `gui_2.py`, `commands.py`, etc.) go through +the controller: + +```python +# In App class methods (gui_2.py): __getattr__ delegates to controller +self.save_config() # -> controller.save_config() +app.save_config() # -> controller.save_config() (via __getattr__) +app.load_config() # -> controller.load_config() (via __getattr__) + +# In AppController: +self.save_config() # direct +self.load_config() # direct +``` + +## Test Patterns + +Tests should mock the **controller methods**, not the I/O primitives: + +```python +# CORRECT: route through the controller +with patch('src.app_controller.AppController.load_config', + return_value={'ai': {...}, 'projects': {...}}): + app = App() # controller's load_config returns the mock + +with patch('src.app_controller.AppController.save_config'): + app._save_paths() # controller's save_config is a no-op + app.save_config.assert_called_once() # verify the call + +# WRONG: patch the I/O primitive +with patch('src.models._save_config_to_disk'): # bypasses the controller + app._save_paths() # still hits the I/O primitive if production bypasses +``` + +The `mock_app` and `app_instance` fixtures in `tests/conftest.py` +follow the correct pattern: they patch +`AppController.load_config` and `AppController.save_config` to +prevent real I/O and to provide a default config. + +## Exceptions + +The only allowed non-controller call site is the +`test_models_no_top_level_tomli_w.py` test, which specifically +verifies the lazy-load behavior of the I/O primitive itself +(tomli_w import timing). This test is exempt from the audit. + +## Enforcement + +The `scripts/audit_no_models_config_io.py` script enforces this rule. + +- `python scripts/audit_no_models_config_io.py` — human report +- `python scripts/audit_no_models_config_io.py --strict` — exit 1 on violation +- `python scripts/audit_no_models_config_io.py --json` — machine output + +CI should run the `--strict` mode on every PR. + +## See Also + +- `docs/guide_app_controller.md` — the AppController's role +- `docs/guide_models.md` — the models module +- `conductor/product.md` — "Modular Controller Pattern" principle diff --git a/conductor/product-guidelines.md b/conductor/product-guidelines.md index 4fb8d70b..02d689c0 100644 --- a/conductor/product-guidelines.md +++ b/conductor/product-guidelines.md @@ -56,3 +56,4 @@ The product guidelines are best understood alongside the per-source-file guides - **[docs/guide_multi_agent_conductor.md](../docs/guide_multi_agent_conductor.md):** §"Thread Safety" — `threading.local()` source tier tagging, lock-protected event queue. - **[docs/guide_models.md](../docs/guide_models.md):** §"Design Principles" + §"SDM Tags" — centralized registry, pydantic validation, `[C: ...]` / `[M: ...]` tags in docstrings. - **[docs/guide_testing.md](../docs/guide_testing.md):** §"Structural Testing Contract" — Ban on Arbitrary Core Mocking, `live_gui` Standard, Artifact Isolation. +- **[code_styleguides/config_state_owner.md](code_styleguides/config_state_owner.md):** Config I/O state ownership — `AppController` is the single source of truth; direct calls to `models.save_config`/`models.load_config` in `src/` are forbidden (enforced by `scripts/audit_no_models_config_io.py`). diff --git a/scripts/audit_no_models_config_io.py b/scripts/audit_no_models_config_io.py new file mode 100644 index 00000000..a931997a --- /dev/null +++ b/scripts/audit_no_models_config_io.py @@ -0,0 +1,166 @@ +"""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). + +The only allowed call sites are inside AppController itself. + +Usage: + python scripts/audit_no_models_config_io.py # human-readable report + python scripts/audit_no_models_config_io.py --json # JSON output for CI + python scripts/audit_no_models_config_io.py --strict # exit 1 on violations +""" +from __future__ import annotations +import argparse +import json +import os +import re +import sys +from pathlib import Path + +# Patterns that are architectural smells in production code. +# These are the I/O primitives; only AppController should call them. +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"), +] + +# The OLD public names. After the rename these should not exist anywhere. +LEGACY_NAMES = [ + (re.compile(r"\bmodels\.load_config\s*\("), "models.load_config"), + (re.compile(r"\bmodels\.save_config\s*\("), "models.save_config"), +] + +# Files where these calls are LEGITIMATE. +ALLOWED_FILES = { + "src/app_controller.py", # the only public owner of the I/O + "src/models.py", # the module that defines them + "tests/test_models_no_top_level_tomli_w.py", # tests lazy-load behavior +} + +# Source roots to scan +SOURCE_ROOTS = ["src"] + + +def find_violations() -> list[dict[str, object]]: + """Scan src/ for direct calls to the forbidden config I/O primitives.""" + violations: list[dict[str, object]] = [] + for root in SOURCE_ROOTS: + if not os.path.isdir(root): + continue + for dirpath, _dirs, files in os.walk(root): + for fname in files: + if not fname.endswith(".py"): + continue + path = os.path.join(dirpath, fname) + # Normalize to forward slashes for matching + norm = path.replace(os.sep, "/") + if norm in ALLOWED_FILES: + continue + with open(path, encoding="utf-8", errors="replace") as f: + src = f.read() + docstring_lines = _docstring_lines(src) + for lineno, line in enumerate(src.splitlines(), start=1): + if lineno in docstring_lines: + continue + stripped = line.lstrip() + if stripped.startswith("#"): + continue + for pattern, name in FORBIDDEN_PATTERNS: + if pattern.search(line): + violations.append({ + "file": path, + "line": lineno, + "pattern": name, + "text": line.rstrip(), + "severity": "error", + }) + for pattern, name in LEGACY_NAMES: + if pattern.search(line): + violations.append({ + "file": path, + "line": lineno, + "pattern": name, + "text": line.rstrip(), + "severity": "error", + }) + return violations + + +def _docstring_lines(src: str) -> set[int]: + """Return a set of 1-based line numbers that are inside a docstring. + + Uses the AST to find module/class/function docstrings, then expands + the string node's line range. Multi-line strings are included in + full so any code-looking text inside them is ignored. + """ + import ast + lines: set[int] = set() + try: + tree = ast.parse(src) + except SyntaxError: + return lines + for node in ast.walk(tree): + if not isinstance(node, (ast.Module, ast.FunctionDef, ast.AsyncFunctionDef, + ast.ClassDef)): + continue + doc = ast.get_docstring(node, clean=False) + if not doc or not node.body: + continue + first = node.body[0] + if not isinstance(first, ast.Expr) or not isinstance(first.value, ast.Constant): + continue + if not isinstance(first.value.value, str): + continue + start = first.lineno + end = getattr(first, "end_lineno", None) or start + for ln in range(start, end + 1): + lines.add(ln) + return lines + + +def main() -> int: + parser = argparse.ArgumentParser( + description="Audit for direct calls to models config I/O primitives in src/" + ) + parser.add_argument("--json", action="store_true", help="JSON output for CI") + parser.add_argument("--strict", action="store_true", help="Exit 1 on any violation") + args = parser.parse_args() + + violations = find_violations() + + if args.json: + print(json.dumps({"violations": violations, "count": len(violations)}, indent=2)) + else: + print("=" * 70) + print("Architectural audit: models config I/O usage in src/") + print("=" * 70) + print() + print("Rule: AppController owns config I/O. Direct calls to") + print(" - models._load_config_from_disk(...)") + print(" - models._save_config_to_disk(...)") + print(" - models.load_config(...) (legacy)") + print(" - models.save_config(...) (legacy)") + print("from outside AppController are architectural smells.") + print() + print(f"Allowed call sites: {sorted(ALLOWED_FILES)}") + print() + if not violations: + print("OK - no violations found.") + else: + print(f"Found {len(violations)} violation(s):") + print() + for v in violations: + print(f" {v['file']}:{v['line']}: {v['pattern']}") + print(f" {v['text']}") + print() + + if args.strict and violations: + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/app_controller.py b/src/app_controller.py index 0df96230..7992df0d 100644 --- a/src/app_controller.py +++ b/src/app_controller.py @@ -1707,7 +1707,7 @@ class AppController: self.ui_separate_response_panel = False self.ui_separate_tool_calls_panel = False self.ui_separate_external_tools = False - self.config = models.load_config() + self.config = self.load_config() theme.load_from_config(self.config) ai_cfg = self.config.get("ai", {}) self._current_provider = ai_cfg.get("provider", "gemini") @@ -2685,7 +2685,7 @@ class AppController: def _cb_project_save(self) -> None: self._flush_to_project() self._flush_to_config() - models.save_config(self.config) + self.save_config() self.ai_status = "config saved" def _do_project_switch(self, path: str) -> None: @@ -3360,7 +3360,7 @@ class AppController: """ self._flush_to_project() self._flush_to_config() - models.save_config(self.config) + self.save_config() track_id = self.active_track.id if self.active_track else None flat = project_manager.flat_config(self.project, self.active_discussion, track_id=track_id) @@ -3997,7 +3997,9 @@ class AppController: if self.active_track and self.active_track.id == track_id: # Use the active track object directly to start execution self.mma_status = "running" - engine = multi_agent_conductor.ConductorEngine(self.active_track, self.event_queue, auto_queue=not self.mma_step_mode) + _mma_cfg = self.config.get("mma", {}) + _max_workers = int(_mma_cfg.get("max_workers", 4)) + engine = multi_agent_conductor.ConductorEngine(self.active_track, self.event_queue, auto_queue=not self.mma_step_mode, max_workers=_max_workers) self.engines[self.active_track.id] = engine flat = project_manager.flat_config(self.project, self.active_discussion, track_id=self.active_track.id) full_md, _, _ = aggregate.run(flat) @@ -4008,7 +4010,9 @@ class AppController: self._cb_load_track(track_id) if self.active_track and self.active_track.id == track_id: self.mma_status = "running" - engine = multi_agent_conductor.ConductorEngine(self.active_track, self.event_queue, auto_queue=not self.mma_step_mode) + _mma_cfg = self.config.get("mma", {}) + _max_workers = int(_mma_cfg.get("max_workers", 4)) + engine = multi_agent_conductor.ConductorEngine(self.active_track, self.event_queue, auto_queue=not self.mma_step_mode, max_workers=_max_workers) self.engines[self.active_track.id] = engine flat = project_manager.flat_config(self.project, self.active_discussion, track_id=self.active_track.id) full_md, _, _ = aggregate.run(flat) @@ -4084,7 +4088,9 @@ class AppController: # 4. Initialize ConductorEngine and run loop sys.stderr.write(f"[DEBUG] _start_track_logic: Initializing engine for {track_id}...\n") sys.stderr.flush() - engine = multi_agent_conductor.ConductorEngine(track, self.event_queue, auto_queue=not self.mma_step_mode) + _mma_cfg = self.config.get("mma", {}) + _max_workers = int(_mma_cfg.get("max_workers", 4)) + engine = multi_agent_conductor.ConductorEngine(track, self.event_queue, auto_queue=not self.mma_step_mode, max_workers=_max_workers) self.engines[track.id] = engine # Use current full markdown context for the track execution track_id_param = track.id @@ -4369,6 +4375,32 @@ class AppController: except Exception as e: print(f"Error loading beads: {e}") + #region: --- Config I/O (single source of truth) --- + def load_config(self) -> Dict[str, Any]: + """ + Re-read the global config.toml from disk and update self.config. + Returns the dict (also stored in self.config). Single source of + truth for the in-memory config is self.config. Direct callers + from outside the controller (e.g. models.load_config) are an + architectural smell and will be flagged by + scripts/audit_no_models_config_io.py. + [C: src/app_controller.py:AppController.__init__] + """ + self.config = models._load_config_from_disk() + return self.config + + def save_config(self) -> None: + """ + Flush self.config to disk. Single source of truth = self.config. + This method owns the write path. Direct callers from outside the + controller (e.g. models.save_config) are an architectural smell + and will be flagged by + 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) + #endregion: --- Config I/O (single source of truth) --- + #endregion: MMA (Controller) #region: MMA diff --git a/src/commands.py b/src/commands.py index 6c4c4b55..f2ba893d 100644 --- a/src/commands.py +++ b/src/commands.py @@ -143,7 +143,7 @@ def save_all(app: "App") -> None: if hasattr(app, "_flush_to_config"): app._flush_to_config() if hasattr(app, "config"): try: - models.save_config(app.config) + app.save_config() except Exception as e: if hasattr(app, "ai_status"): app.ai_status = f"save error: {e}" diff --git a/src/external_editor.py b/src/external_editor.py index 2ad28f77..d1a3ee17 100644 --- a/src/external_editor.py +++ b/src/external_editor.py @@ -7,7 +7,7 @@ import tempfile # TODO(Ed): Eliminate these? from pathlib import Path -from typing import Optional, List +from typing import Optional, List, Dict, Any from src.models import ExternalEditorConfig, TextEditorConfig @@ -113,14 +113,16 @@ def auto_detect_vscode() -> Optional[TextEditorConfig]: return _cached_vscode_config -def get_default_launcher() -> ExternalEditorLauncher: +def get_default_launcher(config: Optional[Dict[str, Any]] = None) -> ExternalEditorLauncher: """ + Caller passes the live config dict (typically app.config from the + AppController). Direct file I/O (the legacy public functions on src.models) is an + architectural smell (bypasses the controller state owner) and is + forbidden by scripts/audit_no_models_config_io.py. [C: src/gui_2.py:App._open_patch_in_external_editor, src/gui_2.py:App._render_external_editor_panel] """ - from src import models - config = models.load_config() - editors_config = config.get("tools", {}).get("text_editors", {}) - default_editor = config.get("tools", {}).get("default_editor", {}).get("default_editor") + editors_config = config.get("tools", {}).get("text_editors", {}) if config else {} + default_editor = config.get("tools", {}).get("default_editor", {}).get("default_editor") if config else None ext_config = ExternalEditorConfig.from_dict({ "editors": editors_config, "default_editor": default_editor, diff --git a/src/gui_2.py b/src/gui_2.py index a77a9852..360c2b1e 100644 --- a/src/gui_2.py +++ b/src/gui_2.py @@ -1009,7 +1009,7 @@ class App: if imgui.menu_item("Save All", "", False)[0]: self._flush_to_project() self._flush_to_config() - models.save_config(self.config) + self.save_config() self.ai_status = "config saved" if imgui.menu_item("Reset Session", "", False)[0]: ai_client.reset_session() @@ -1167,7 +1167,7 @@ class App: 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 - models.save_config(self.config) + self.save_config() self.ai_status = f"Default editor set to: {editor_name}" _render_path_field("Logs Directory", "ui_logs_dir", "logs_dir", "Directory where session JSON-L logs and artifacts are stored.") @@ -1192,7 +1192,7 @@ class App: } cfg_path = paths.get_config_path() if cfg_path.exists(): shutil.copy(cfg_path, str(cfg_path) + ".bak") - models.save_config(self.config) + self.save_config() paths.reset_resolved() self.init_state() self.ai_status = 'paths applied and session reset' @@ -1308,7 +1308,7 @@ class App: if not self._pending_patch_files: self._patch_error_message = "No files to edit" return - launcher = get_default_launcher() + launcher = get_default_launcher(self.config) editor = launcher.config.get_default() if not editor: self._patch_error_message = "No external editor configured" @@ -1479,7 +1479,7 @@ def render_main_interface(app: App) -> None: try: app._flush_to_project() app._flush_to_config() - models.save_config(app.config) + app.save_config() except Exception: pass # silent — don't disrupt the GUI loop @@ -2022,7 +2022,7 @@ def render_projects_panel(app: App) -> None: if imgui.button("Save All"): app._flush_to_project() app._flush_to_config() - models.save_config(app.config) + app.save_config() app.ai_status = "config saved" ch, app.ui_word_wrap = imgui.checkbox("Word-Wrap (Read-only panels)", app.ui_word_wrap) ch, app.ui_auto_scroll_comms = imgui.checkbox("Auto-scroll Comms History", app.ui_auto_scroll_comms) @@ -2415,7 +2415,7 @@ def render_save_preset_modal(app: App) -> None: "multi_viewport": app.ui_multi_viewport } app.config["layout_presets"] = app.layout_presets - models.save_config(app.config) + app.save_config() app._show_save_preset_modal = False app._new_preset_name = "" imgui.close_current_popup() @@ -4211,7 +4211,7 @@ def render_discussion_entry_controls(app: App) -> None: imgui.same_line() if imgui.button("Clear All"): app.disc_entries.clear() imgui.same_line() - if imgui.button("Save"): app._flush_to_project(); app._flush_to_config(); models.save_config(app.config); app.ai_status = "discussion saved" + if imgui.button("Save"): app._flush_to_project(); app._flush_to_config(); app.save_config(); app.ai_status = "discussion saved" imgui.same_line() if imgui.button("Compress"): app.controller._handle_compress_discussion() _, app.ui_auto_add_history = imgui.checkbox("Auto-add message & response to history", app.ui_auto_add_history) @@ -4773,7 +4773,7 @@ def render_external_editor_panel(app: App) -> None: imgui.text("External Editor for Diff Viewing") imgui.separator() try: - launcher = get_default_launcher() + launcher = get_default_launcher(app.config) editors = launcher.config.editors default_name = launcher.config.default_editor if not editors: @@ -4876,7 +4876,7 @@ def render_theme_panel(app: App) -> None: if imgui.selectable(p, p == cp)[0]: theme.apply(p) app._flush_to_config() - models.save_config(app.config) + app.save_config() imgui.end_combo() imgui.separator() @@ -4907,7 +4907,7 @@ def render_theme_panel(app: App) -> None: imgui.same_line() if imgui.button("Apply Font (Requires Restart)"): app._flush_to_config() - models.save_config(app.config) + app.save_config() app.ai_status = "Font settings saved. Restart required." imgui.separator() imgui.text("UI Scale (DPI)") @@ -4915,14 +4915,14 @@ def render_theme_panel(app: App) -> None: if ch: theme.set_scale(scale) app._flush_to_config() - models.save_config(app.config) + app.save_config() imgui.text("Panel Transparency") ch_t, trans = imgui.slider_float("##trans", theme.get_transparency(), 0.1, 1.0, "%.2f") if ch_t: theme.set_transparency(trans) app._flush_to_config() - models.save_config(app.config) + app.save_config() imgui.text("Panel Item Transparency") ch_ct, ctrans = imgui.slider_float("##ctrans", theme.get_child_transparency(), 0.1, 1.0, "%.2f") @@ -4934,14 +4934,14 @@ def render_theme_panel(app: App) -> None: gui_cfg = app.config.setdefault("gui", {}) gui_cfg["bg_shader_enabled"] = bg.enabled app._flush_to_config() - models.save_config(app.config) + app.save_config() ch_crt, app.ui_crt_filter = imgui.checkbox("CRT Filter", app.ui_crt_filter) if ch_crt: gui_cfg = app.config.setdefault("gui", {}) gui_cfg["crt_filter_enabled"] = app.ui_crt_filter app._flush_to_config() - models.save_config(app.config) + app.save_config() imgui.separator() imgui.text("Tone Mapping (Per-Palette)") @@ -4949,20 +4949,20 @@ def render_theme_panel(app: App) -> None: imgui.text("Brightness") ch_b, b = imgui.slider_float("##tm_b", theme.get_brightness(curr_p), 0.1, 2.0, "%.2f") - if ch_b: theme.set_brightness(curr_p, b); app._flush_to_config(); models.save_config(app.config) + if ch_b: theme.set_brightness(curr_p, b); app._flush_to_config(); app.save_config() imgui.text("Contrast") ch_c, c = imgui.slider_float("##tm_c", theme.get_contrast(curr_p), 0.1, 2.0, "%.2f") - if ch_c: theme.set_contrast(curr_p, c); app._flush_to_config(); models.save_config(app.config) + if ch_c: theme.set_contrast(curr_p, c); app._flush_to_config(); app.save_config() imgui.text("Gamma") ch_g, g = imgui.slider_float("##tm_g", theme.get_gamma(curr_p), 0.1, 3.0, "%.2f") - if ch_g: theme.set_gamma(curr_p, g); app._flush_to_config(); models.save_config(app.config) + if ch_g: theme.set_gamma(curr_p, g); app._flush_to_config(); app.save_config() if imgui.button("Reset Tone Mapping"): theme.reset_tone_mapping(curr_p) app._flush_to_config() - models.save_config(app.config) + app.save_config() imgui.end() if app.perf_profiling_enabled: app.perf_monitor.end_component("_render_theme_panel") diff --git a/src/models.py b/src/models.py index 05e2f181..ad96ddd2 100644 --- a/src/models.py +++ b/src/models.py @@ -163,14 +163,20 @@ def _clean_nones(data: Any) -> Any: return [_clean_nones(v) for v in data if v is not None] return data -def load_config() -> dict[str, Any]: +def _load_config_from_disk() -> dict[str, Any]: """ - [C: src/multi_agent_conductor.py:ConductorEngine.__init__] + 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(config: dict[str, Any]) -> None: +def _save_config_to_disk(config: dict[str, Any]) -> 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 diff --git a/src/multi_agent_conductor.py b/src/multi_agent_conductor.py index cb4f8d72..37268b59 100644 --- a/src/multi_agent_conductor.py +++ b/src/multi_agent_conductor.py @@ -113,7 +113,7 @@ class ConductorEngine: Orchestrates the execution of tickets within a track. """ - def __init__(self, track: Track, event_queue: Optional[events.AsyncEventQueue] = None, auto_queue: bool = False) -> None: + def __init__(self, track: Track, event_queue: Optional[events.AsyncEventQueue] = None, auto_queue: bool = False, max_workers: int = 4) -> None: self.track = track self.event_queue = event_queue self.tier_usage = { @@ -124,15 +124,7 @@ class ConductorEngine: } self.dag = TrackDAG(self.track.tickets) self.engine = ExecutionEngine(self.dag, auto_queue=auto_queue) - - # Load MMA config - try: - config = models.load_config() - mma_cfg = config.get("mma", {}) - max_workers = mma_cfg.get("max_workers", 4) - except Exception: - max_workers = 4 - + self.pool = WorkerPool(max_workers=max_workers) self._workers_lock = threading.Lock() self._active_workers: dict[str, threading.Thread] = {} diff --git a/tests/conftest.py b/tests/conftest.py index 3ceb878e..b9d52bb8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -285,12 +285,12 @@ def mock_app() -> Generator[App, None, None]: Mock version of the App for simple unit tests that don't need a loop. """ with ( - patch('src.models.load_config', return_value={ + patch('src.app_controller.AppController.load_config', return_value={ 'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {'paths': [], 'active': ''}, 'gui': {'show_windows': {}} }), - patch('src.models.save_config'), + patch('src.app_controller.AppController.save_config'), patch('src.gui_2.project_manager'), patch('src.gui_2.session_logger'), patch('src.gui_2.immapp.run'), @@ -320,12 +320,12 @@ def app_instance() -> Generator[App, None, None]: [C: tests/test_gui2_events.py:test_app_subscribes_to_events] """ with ( - patch('src.models.load_config', return_value={ + patch('src.app_controller.AppController.load_config', return_value={ 'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {'paths': [], 'active': ''}, 'gui': {'show_windows': {}} }), - patch('src.models.save_config'), + patch('src.app_controller.AppController.save_config'), patch('src.gui_2.project_manager'), patch('src.gui_2.session_logger'), patch('src.gui_2.immapp.run'), diff --git a/tests/test_api_hook_extensions.py b/tests/test_api_hook_extensions.py index 1e4b012c..b9d3fed0 100644 --- a/tests/test_api_hook_extensions.py +++ b/tests/test_api_hook_extensions.py @@ -33,7 +33,7 @@ def test_get_indicator_state_integration(live_gui: Any) -> None: def test_app_processes_new_actions() -> None: from src import gui_2 - with patch('src.models.load_config', return_value={}), \ + with patch('src.app_controller.AppController.load_config', return_value={}), \ patch('src.performance_monitor.PerformanceMonitor'), \ patch('src.session_logger.open_session'), \ patch('src.session_logger.reset_session'), \ diff --git a/tests/test_app_controller_mcp.py b/tests/test_app_controller_mcp.py index 1651bd9f..06bda40f 100644 --- a/tests/test_app_controller_mcp.py +++ b/tests/test_app_controller_mcp.py @@ -49,7 +49,7 @@ def controller(tmp_path): def test_app_controller_mcp_loading(tmp_path, monkeypatch): # Mock CONFIG_PATH to point to our temp config config_file = tmp_path / "config.toml" - monkeypatch.setattr(models, "CONFIG_PATH", str(config_file)) + monkeypatch.setenv("SLOP_CONFIG", str(config_file)) mcp_global_file = tmp_path / "mcp_global.json" mcp_global_file.write_text(json.dumps({"mcpServers": {"global": {"command": "echo"}}})) @@ -75,7 +75,7 @@ active = "" def test_app_controller_mcp_project_override(tmp_path, monkeypatch): config_file = tmp_path / "config.toml" - monkeypatch.setattr(models, "CONFIG_PATH", str(config_file)) + monkeypatch.setenv("SLOP_CONFIG", str(config_file)) project_file = tmp_path / "project.toml" mcp_project_file = tmp_path / "mcp_project.json" diff --git a/tests/test_arch_boundary_phase2.py b/tests/test_arch_boundary_phase2.py index e822cb52..235e9cdf 100644 --- a/tests/test_arch_boundary_phase2.py +++ b/tests/test_arch_boundary_phase2.py @@ -44,7 +44,7 @@ class TestArchBoundaryPhase2(unittest.TestCase): from src.app_controller import AppController # Use a real AppController to test its _confirm_and_run - with patch('src.models.load_config', return_value={}), \ + with patch('src.app_controller.AppController.load_config', return_value={}), \ patch('src.performance_monitor.PerformanceMonitor'), \ patch('src.session_logger.open_session'), \ patch('src.session_logger.reset_session'), \ @@ -67,7 +67,7 @@ class TestArchBoundaryPhase2(unittest.TestCase): """When pre_tool_callback returns None (rejected), dispatch must NOT be called.""" from src.app_controller import AppController - with patch('src.models.load_config', return_value={}), \ + with patch('src.app_controller.AppController.load_config', return_value={}), \ patch('src.performance_monitor.PerformanceMonitor'), \ patch('src.session_logger.open_session'), \ patch('src.session_logger.reset_session'), \ diff --git a/tests/test_auto_slices.py b/tests/test_auto_slices.py index d75ff319..e0d2632a 100644 --- a/tests/test_auto_slices.py +++ b/tests/test_auto_slices.py @@ -6,7 +6,7 @@ from src import models @pytest.fixture def mock_app(): with ( - patch('src.models.load_config', return_value={ + patch('src.app_controller.AppController.load_config', return_value={ "ai": {"provider": "gemini", "model": "model-1"}, "projects": {"paths": [], "active": ""}, "gui": {"show_windows": {}} diff --git a/tests/test_discussion_takes_gui.py b/tests/test_discussion_takes_gui.py index 85c8b4ed..6ab5b52b 100644 --- a/tests/test_discussion_takes_gui.py +++ b/tests/test_discussion_takes_gui.py @@ -6,8 +6,8 @@ from src.gui_2 import App @pytest.fixture def app_instance(): with ( - patch('src.models.load_config', return_value={'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {}}), - patch('src.models.save_config'), + patch('src.app_controller.AppController.load_config', return_value={'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {}}), + patch('src.app_controller.AppController.save_config'), patch('src.gui_2.project_manager'), patch('src.gui_2.session_logger'), patch('src.gui_2.immapp.run'), diff --git a/tests/test_gui_events_v2.py b/tests/test_gui_events_v2.py index 5503bdf0..8921dfca 100644 --- a/tests/test_gui_events_v2.py +++ b/tests/test_gui_events_v2.py @@ -6,7 +6,7 @@ from src.events import UserRequestEvent @pytest.fixture def mock_gui() -> App: with ( - patch('src.models.load_config', return_value={ + patch('src.app_controller.AppController.load_config', return_value={ "ai": {"provider": "gemini", "model": "model-1"}, "projects": {"paths": [], "active": ""}, "gui": {"show_windows": {}} diff --git a/tests/test_gui_paths.py b/tests/test_gui_paths.py index d0b5dd33..70817a7c 100644 --- a/tests/test_gui_paths.py +++ b/tests/test_gui_paths.py @@ -10,6 +10,7 @@ class MockApp: self.ui_scripts_dir = '/mock/scripts' self.config = {"paths": {}} self.ai_status = "" + self.save_config = MagicMock() def init_state(self): """ @@ -23,8 +24,7 @@ class MockApp: def test_save_paths(): mock_app = MockApp() - with patch('src.models.save_config') as mock_save, \ - patch('shutil.copy') as mock_copy, \ + with patch('shutil.copy') as mock_copy, \ patch('src.paths.get_config_path') as mock_get_cfg, \ patch('src.paths.reset_resolved') as mock_reset, \ patch.object(MockApp, 'init_state') as mock_init: @@ -37,7 +37,7 @@ def test_save_paths(): # Verify config update assert 'conductor_dir' not in mock_app.config['paths'] - mock_save.assert_called_once() + mock_app.save_config.assert_called_once() mock_copy.assert_called_once() assert 'applied' in mock_app.ai_status mock_reset.assert_called_once() diff --git a/tests/test_gui_synthesis.py b/tests/test_gui_synthesis.py index f2b74b57..9e7929f3 100644 --- a/tests/test_gui_synthesis.py +++ b/tests/test_gui_synthesis.py @@ -6,8 +6,8 @@ from src.gui_2 import App @pytest.fixture def app_instance(): with ( - patch('src.models.load_config', return_value={'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {}}), - patch('src.models.save_config'), + patch('src.app_controller.AppController.load_config', return_value={'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {}}), + patch('src.app_controller.AppController.save_config'), patch('src.gui_2.project_manager'), patch('src.gui_2.session_logger'), patch('src.gui_2.immapp.run'), diff --git a/tests/test_headless_service.py b/tests/test_headless_service.py index 2ea99ede..066f2086 100644 --- a/tests/test_headless_service.py +++ b/tests/test_headless_service.py @@ -11,7 +11,7 @@ from src.app_controller import AppController class TestHeadlessAPI(unittest.TestCase): def setUp(self) -> None: - with patch('src.models.load_config', return_value={'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {}, 'gui': {'show_windows': {}}}), \ + with patch('src.app_controller.AppController.load_config', return_value={'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {}, 'gui': {'show_windows': {}}}), \ patch('src.session_logger.open_session'), \ patch('src.session_logger.reset_session'), \ patch('src.ai_client.set_provider'), \ diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 3d7e75c7..e61b4f5f 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -11,7 +11,7 @@ def test_hooks_enabled_via_cli(monkeypatch: pytest.MonkeyPatch) -> None: from src.gui_2 import App from unittest.mock import patch monkeypatch.setattr("sys.argv", ["sloppy.py", "--enable-test-hooks"]) - with patch('src.models.load_config', return_value={}), \ + with patch('src.app_controller.AppController.load_config', return_value={}), \ patch('src.performance_monitor.PerformanceMonitor'), \ patch('src.session_logger.open_session'), \ patch('src.session_logger.reset_session'), \ @@ -23,7 +23,7 @@ def test_hooks_enabled_via_cli(monkeypatch: pytest.MonkeyPatch) -> None: def test_hooks_disabled_by_default() -> None: from src.gui_2 import App from unittest.mock import patch - with patch('src.models.load_config', return_value={}), \ + with patch('src.app_controller.AppController.load_config', return_value={}), \ patch('src.performance_monitor.PerformanceMonitor'), \ patch('src.session_logger.open_session'), \ patch('src.session_logger.reset_session'), \ diff --git a/tests/test_layout_reorganization.py b/tests/test_layout_reorganization.py index bd55f984..4d5d8fce 100644 --- a/tests/test_layout_reorganization.py +++ b/tests/test_layout_reorganization.py @@ -45,7 +45,7 @@ def test_old_windows_removed_from_gui2(app_instance_simple: Any) -> None: def app_instance_simple() -> Any: from unittest.mock import patch from src.gui_2 import App - with patch('src.models.load_config', return_value={'ai': {}, 'projects': {}, 'gui': {'show_windows': {}}}), \ + with patch('src.app_controller.AppController.load_config', return_value={'ai': {}, 'projects': {}, 'gui': {'show_windows': {}}}), \ patch('src.app_controller.AppController._init_ai_and_hooks'), \ patch('src.app_controller.AppController._fetch_models'), \ patch('src.app_controller.AppController._prune_old_logs'), \ diff --git a/tests/test_mma_dashboard_refresh.py b/tests/test_mma_dashboard_refresh.py index dd04ed37..5dc64e9e 100644 --- a/tests/test_mma_dashboard_refresh.py +++ b/tests/test_mma_dashboard_refresh.py @@ -7,8 +7,8 @@ from src.gui_2 import App @pytest.fixture def app_instance() -> Any: with ( - patch("src.models.load_config", return_value={"ai": {}, "projects": {}}), - patch("src.models.save_config"), + patch("src.app_controller.AppController.load_config", return_value={"ai": {}, "projects": {}}), + patch("src.app_controller.AppController.save_config"), patch("src.gui_2.project_manager"), patch("src.app_controller.project_manager") as mock_pm, patch("src.gui_2.session_logger"), diff --git a/tests/test_models_no_top_level_tomli_w.py b/tests/test_models_no_top_level_tomli_w.py index 69b1ddd8..99fb2758 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(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({"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. diff --git a/tests/test_parallel_execution.py b/tests/test_parallel_execution.py index e811ab30..9a9683b8 100644 --- a/tests/test_parallel_execution.py +++ b/tests/test_parallel_execution.py @@ -71,20 +71,17 @@ from src.models import Track, Ticket from src.multi_agent_conductor import ConductorEngine @patch('src.multi_agent_conductor.run_worker_lifecycle') -@patch('src.models.load_config') -def test_conductor_engine_pool_integration(mock_load_config, mock_lifecycle): - # Mock config to set max_workers=2 - mock_load_config.return_value = {"mma": {"max_workers": 2}} - +def test_conductor_engine_pool_integration(mock_lifecycle): # Create 4 independent tickets tickets = [ Ticket(id=f"t{i}", description=f"task {i}", status="todo") for i in range(4) ] track = Track(id="test_track", description="test", tickets=tickets) - - # Set up engine with auto_queue - engine = ConductorEngine(track, auto_queue=True) + + # Set up engine with auto_queue and explicit max_workers=2. + # ConductorEngine no longer reads config itself; the caller passes max_workers. + engine = ConductorEngine(track, auto_queue=True, max_workers=2) sys.stderr.write(f"[TEST] engine.pool.max_workers = {engine.pool.max_workers}\n") assert engine.pool.max_workers == 2 diff --git a/tests/test_phase6_engine.py b/tests/test_phase6_engine.py index 85d39060..81b48d47 100644 --- a/tests/test_phase6_engine.py +++ b/tests/test_phase6_engine.py @@ -47,7 +47,7 @@ def test_per_tier_model_persistence(): patch("src.gui_2.project_manager.load_project", return_value={}), patch("src.gui_2.project_manager.migrate_from_legacy_config", return_value={}), patch("src.gui_2.project_manager.save_project"), - patch("src.models.save_config"), + patch("src.app_controller.AppController.save_config"), patch("src.gui_2.theme.load_from_config"), patch("src.gui_2.ai_client.set_provider"), patch("src.gui_2.ai_client.list_models", return_value=["gpt-4", "claude-3"]), diff --git a/tests/test_process_pending_gui_tasks.py b/tests/test_process_pending_gui_tasks.py index 9e8ab61a..9463c717 100644 --- a/tests/test_process_pending_gui_tasks.py +++ b/tests/test_process_pending_gui_tasks.py @@ -7,8 +7,8 @@ from src.gui_2 import App @pytest.fixture def app_instance() -> Generator[App, None, None]: with ( - patch('src.models.load_config', return_value={'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {}}), - patch('src.models.save_config'), + patch('src.app_controller.AppController.load_config', return_value={'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {}}), + patch('src.app_controller.AppController.save_config'), patch('src.gui_2.project_manager'), patch('src.gui_2.session_logger'), patch('src.gui_2.immapp.run'), diff --git a/tests/test_rag_integration.py b/tests/test_rag_integration.py index 3f3126c9..dbf63f1b 100644 --- a/tests/test_rag_integration.py +++ b/tests/test_rag_integration.py @@ -31,7 +31,7 @@ def test_rag_integration(mock_project): # 1. Initializes a mock project and AppController. # We patch several components to avoid side effects during initialization. with patch('src.app_controller.AppController._fetch_models'), \ - patch('src.models.load_config', return_value={}), \ + patch('src.app_controller.AppController.load_config', return_value={}), \ patch('src.paths.get_full_path_info', return_value={'logs_dir': {'path': mock_project}, 'scripts_dir': {'path': mock_project}}), \ patch('src.theme_2.load_from_config'): app = AppController() diff --git a/tests/test_symbol_parsing.py b/tests/test_symbol_parsing.py index eca567b8..8ca26290 100644 --- a/tests/test_symbol_parsing.py +++ b/tests/test_symbol_parsing.py @@ -8,7 +8,7 @@ from src import events @pytest.fixture def controller(): with ( - patch('src.models.load_config', return_value={ + patch('src.app_controller.AppController.load_config', return_value={ "ai": {"provider": "gemini", "model": "model-1"}, "projects": {"paths": [], "active": ""}, "gui": {"show_windows": {}} diff --git a/tests/test_system_prompt_exposure.py b/tests/test_system_prompt_exposure.py index b96432bf..61a696bf 100644 --- a/tests/test_system_prompt_exposure.py +++ b/tests/test_system_prompt_exposure.py @@ -49,7 +49,7 @@ class TestSystemPromptExposure(unittest.TestCase): self.assertIn("You are a helpful coding assistant", combined) self.assertNotIn("Overridden Prompt", combined) - @patch('src.models.load_config') + @patch('src.app_controller.AppController.load_config') @patch('src.paths.get_full_path_info') @patch('src.project_manager.load_project') @patch('src.ai_client.set_tool_preset') diff --git a/tests/test_tiered_aggregation.py b/tests/test_tiered_aggregation.py index e987490b..cf8b709f 100644 --- a/tests/test_tiered_aggregation.py +++ b/tests/test_tiered_aggregation.py @@ -31,7 +31,7 @@ def test_app_controller_do_generate_uses_persona_strategy(mock_build): with patch("pathlib.Path.write_text"): with patch.object(app, "_flush_to_project"): with patch.object(app, "_flush_to_config"): - with patch("src.models.save_config"): + with patch("src.app_controller.AppController.save_config"): full_md, path, file_items, stable_md, disc = app._do_generate() # Verify aggregate.run and build_markdown_no_history received aggregation_strategy="full"