Private
Public Access
0
0

feat(directives): checkpoint pre-batch-4 staging

This commit is contained in:
2026-07-04 01:26:10 -04:00
parent b696b30f22
commit 3fcf7dccb5
5 changed files with 78 additions and 0 deletions
@@ -0,0 +1,7 @@
# adapt_test_mocks_to_production_api_change
## v1
**Why this iteration:** Lifted verbatim from `docs/superpowers/plans/2026-06-05-regression-fixes.md` Task 1 (the `C_LBL` theme-caused regression fix; the explicit "Per AGENTS.md: DO NOT CREATE MOCK PATCHES..." citation).
**Source:** `docs/superpowers/plans/2026-06-05-regression-fixes.md` §A.Theme (Task 1, Step 1.3; "Fix: Adapt the test...")
**Lifted:** 2026-07-03 scavenge sweep batch 3/5: docs/superpowers/plans/
@@ -0,0 +1,11 @@
# Adapt tests when production code changes its public API; never patch around new APIs with mock stubs
When a refactor changes a production function signature or shape (e.g. a module-level `C_LBL: ImVec4` becomes a `def C_LBL() -> ImVec4` callable), the test must be adapted to the new contract, not bent to keep working with old mocks.
**The wrong move:** Add a wrapping mock that converts the new return type to the old one, just so the existing `assert_any_call` / `to_dict()` keeps using the old shape. This silently masks the real public surface and propagates the wrong contract to anyone who reads the test as documentation.
**The right move:** Update the test's mock setup so the test exercises the new public API end-to-end. Example from the theme refactor regression: the test originally patched `src.gui_2.imgui` and `src.imgui_scopes.imgui`, but `theme.get_color('text_disabled')` now resolves through `src.theme_2.imgui`. The fix was to add `patch("src.theme_2.imgui", new=mock_imgui)` to the context manager chain, not to wrap `C_LBL()` in another layer of mocks.
**General rule:** when production code changes, the test must mock all transitively-imported modules that the production code's new code path now reaches. Re-walk the call graph; do not patch the same surface as before and add an `unittest.mock.patch("module.old_name", new=old_value)` shim.
References: AGENTS.md "DO NOT CREATE MOCK PATCHES TO PSEUDO API CALLS OR HOOKS BECAUSE THE APP SOURCE WAS CHANGED. ADAPT TESTS PROPERLY."; `docs/superpowers/plans/2026-06-05-regression-fixes.md` §A.Theme (Task 1, Step 1.3).
@@ -0,0 +1,7 @@
# controller_property_delegation_no_dual_state
## v1
**Why this iteration:** Lifted verbatim from `docs/superpowers/plans/2026-06-05-live-gui-state-sync.md` (Tasks 1-5: the property-pair pattern that fixed the App/Controller dual-state bug; tests in `tests/test_app_controller_state_sync.py`).
**Source:** `docs/superpowers/plans/2026-06-05-live-gui-state-sync.md` (Tasks 1-5; Architecture section)
**Lifted:** 2026-07-03 scavenge sweep batch 3/5: docs/superpowers/plans/
@@ -0,0 +1,29 @@
# App must not duplicate Controller state; expose Controller fields as @property + @X.setter pairs
## Pattern: Controller is the single source of truth; App is a thin facade
When the App class holds its own copies of fields that the Controller also manages, two writers (the App's getter and the Controller's setter) silently diverge. Tests that read `app.X` after calling `set_value('X', v)` see stale values; save/load flows capture the wrong half.
**The fix is property delegation:** add `@property` + `@X.setter` pairs on the App class so the getter reads `self.controller.X` and the setter writes `self.controller.X`. Both the legacy App-only fields (no Controller counterpart, kept as plain attributes) and the dual-state fields are kept distinct.
```python
@property
def ui_ai_input(self) -> str:
return self.controller.ui_ai_input
@ui_ai_input.setter
def ui_ai_input(self, value: str) -> None:
self.controller.ui_ai_input = value
```
**In-place mutation of mutable fields works** because the property returns the same reference the Controller owns. `app.show_windows['X'] = True` mutates the Controller's dict; `app.show_windows = new_dict` goes through the setter.
**App-only fields stay as plain attributes.** Don't promote fields the Controller doesn't manage to properties. Validate by listing: `getattr(type(app), 'X', None)` should return `None` for App-only fields and a `property` instance for Controller-owned fields.
**Regression test pattern:**
- Instantiate App via `App.__new__(App)` to skip `__init__`; attach a Controller (`app.controller = AppController()`).
- Set via Controller, read via App; set via App, read via Controller. Assert equality in both directions.
- For dict fields, test both replacement (`=` assignment) and in-place (`[key]=`) semantics.
- For App-only fields, assert `not hasattr(type(app), 'X')` so a future property leak is caught.
References: docs/superpowers/plans/2026-06-05-live-gui-state-sync.md (Tasks 1-5).
@@ -0,0 +1,24 @@
# Tests must not read or write real TOML files; route through `enforce_no_real_toml` autouse fixture
Tests must never read or write the user's real `manual_slop.toml`, `config.toml`, `credentials.toml`, `presets.toml`, `personas.toml`, `tool_presets.toml`, `workspace_profiles.toml` from `cwd`. Doing so silently mutates the user's live config and gives the test unreproducible state across machines.
**Enforcement is two-layered:**
1. **Static audit** (CI gate): `scripts/check_test_toml_paths.py` greps every `tests/test_*.py` file for direct `./<name>.toml`, `Path("<name>.toml")`, `open("<name>.toml")` references. Excludes `tests/artifacts/`, `tests/logs/`, `__pycache__/`, `snapshots/`. Exits 0 on clean; 1 on any violation.
2. **Runtime guard** (autouse fixture): `enforce_no_real_toml` in `tests/conftest.py` snapshots every real TOML in `cwd` before the test, removes them, restores them after. Tests that need TOML data must use `tmp_path` + `monkeypatch` to access it.
**Pattern for sandboxed tests:**
```python
def test_load_presets(tmp_path, monkeypatch):
from src import paths
presets_path = tmp_path / "presets.toml"
presets_path.write_text("[presets]\ndefault = {}\n")
monkeypatch.setattr(paths, "get_global_presets_path", lambda: presets_path)
data = tomllib.loads(presets_path.read_text())
assert "default" in data
```
**Migration rule:** when a test is found using a real TOML, migrate it to `tmp_path` + `monkeypatch` (or reuse the existing `isolate_workspace` fixture). Do not add an exception list to the audit; fix the test, not the gate.
References: `docs/superpowers/plans/2026-06-02-test-consolidation.md` (Tasks 1-2; the `TOML_BASENAMES` set + the `enforce_no_real_toml` fixture).