Private
Public Access
0
0
Commit Graph

3 Commits

Author SHA1 Message Date
ed dcc82ed781 fix(audit): use LEGACY_PRIVATE_NAMES + LEGACY_PUBLIC_NAMES in audit_no_models_config_io
Per post_module_taxonomy_de_cruft_20260627 Phase 0a (FR1). The audit
script's find_violations() function iterated over 'LEGACY_NAMES' but
only LEGACY_PRIVATE_NAMES + LEGACY_PUBLIC_NAMES were defined (the
single LEGACY_NAMES was split into two in module_taxonomy_refactor
Phase 3b but the function reference wasn't updated). This caused a
NameError that crashed the audit with --strict mode.

The spec claimed the bug was in scripts/generate_type_registry.py but
that was a misdiagnosis. generate_type_registry.py works correctly
(verified: 'Registry in sync (29 files checked)'). The actual bug was
in audit_no_models_config_io.py.

This commit:
 - Updates line 95: 'for pattern, name in LEGACY_NAMES:' ->
   'for pattern, name in LEGACY_PRIVATE_NAMES + LEGACY_PUBLIC_NAMES:'
 - The function now iterates over both legacy name lists (private +
   public), matching the actual variables defined in the file.

Verification: VC3 (audit_no_models_config_io passes --strict)
 uv run python scripts/audit_no_models_config_io.py --strict
 # Output: 'OK - no violations found.'
2026-06-26 14:18:34 -04:00
ed e430df86f1 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).
2026-06-26 09:46:12 -04:00
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