Private
Public Access
0
0
Files
manual_slop/scripts/audit_no_models_config_io.py
T
ed 7bcb5a8c07 refactor(config): Route all config I/O through AppController
Eliminates 22 call sites that bypassed the AppController state owner
and read/wrote config.toml directly. AppController is now the single
source of truth for self.config; gui_2.py, commands.py, etc. go
through controller.save_config() / controller.load_config().

Production changes:
- src/models.py: rename load_config -> _load_config_from_disk,
  save_config -> _save_config_to_disk (private I/O primitives)
- src/app_controller.py: add public load_config()/save_config() methods
  that own the state. Update 3 internal call sites and 3 ConductorEngine
  call sites to pass max_workers from self.config
- src/multi_agent_conductor.py: ConductorEngine.__init__ now takes
  max_workers as a parameter (caller responsibility, not I/O primitive)
- src/external_editor.py: get_default_launcher() takes config as a
  parameter; gui_2.py:1311,4776 pass app.config
- src/gui_2.py: 17 sites of models.save_config(X.config) replaced with
  X.save_config() (delegates via __getattr__ to controller)
- src/commands.py: save_all() uses app.save_config()

Test changes (route through controller, not I/O primitive):
- tests/conftest.py: mock_app and app_instance fixtures now patch
  AppController.load_config/save_config instead of models I/O primitives
- 18 other test files: patches renamed from models._save_config_to_disk
  to AppController.save_config (and same for load_config)
- tests/test_app_controller_mcp.py: use SLOP_CONFIG env var instead of
  patching removed CONFIG_PATH module constant
- tests/test_parallel_execution.py: pass max_workers=2 explicitly to
  ConductorEngine (caller no longer reads config)
- tests/test_gui_paths.py: add save_config=MagicMock() to MockApp;
  assert on controller method, not I/O primitive
- tests/test_models_no_top_level_tomli_w.py: still calls private
  _save_config_to_disk directly (the only allowed exception; tests
  the lazy-load behavior of the primitive itself)

New files:
- scripts/audit_no_models_config_io.py: enforces the rule (--strict,
  --json modes; AST-based docstring detection to avoid false positives)
- conductor/code_styleguides/config_state_owner.md: documents the rule

Verification:
- 67 targeted tests pass
- scripts/audit_no_models_config_io.py --strict returns 0

This is the architectural cleanup that surfaced during the
audit_architectural_cheats_20260607 review. Closes the smoke-gun
CONFIG_PATH module constant (already done in 0c7ebf22) AND the
free-function models.load_config/save_config smell.

[conductor(checkpoint): config-iO-refactor-20260607]
2026-06-07 19:54:17 -04:00

167 lines
6.2 KiB
Python

"""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())