docs(plan): implementation plan for live-gui fragility fixes

2026-06-05 19:20:21 -04:00
parent f6d9c70de8
commit 7a0ed74b5c
1 changed files with 387 additions and 0 deletions
@@ -0,0 +1,387 @@
+# Live-GUI Fragility Fixes Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Fix 3 failing live_gui tests discovered in the 2026-06-05 batched test run (269/272 → 272/272) by repairing a regression in the defer-not-catch fix for `_capture_workspace_profile`, fixing a test mock for `imscope.window`, and adding a regression unit test.
+
+**Architecture:** Surgical 1-line fix on the production code path (the str/bytes sentinel that violated the `WorkspaceProfile.ini_content: str` contract), a 2-line fix on the prior session test mock (add missing tuple-return for `imscope.window`), and a new unit test that encodes the str/bytes contract so future regressions are caught at unit-test speed.
+
+**Tech Stack:** Python 3.11+, pytest 9.0, imgui-bundle (`imgui.save_ini_settings_to_memory()`), tomli_w, tomllib.
+
+---
+
+## File Structure
+
+| File | Change | Purpose |
+|---|---|---|
+| `src/gui_2.py` | Modify lines 601-609 | Fix `ini = b""` → `ini = ""` in defer branch + `except` handler. Add `str()` defensive wrap. |
+| `tests/test_prior_session_no_pop_imbalance.py` | Modify (add 2 lines) | Add `(True, True)` tuple-return mock for `imscope.window`. |
+| `tests/test_workspace_profile_serialization.py` | Create | New unit test for the `ini_content: str` round-trip contract. |
+| `conductor/tracks.md` | Modify (1 line, plan updates) | Register new track. |
+| `docs/superpowers/specs/2026-06-05-live-gui-fragility-fixes-design.md` | (already written) | Spec for this work. |
+
+No new files needed in `src/`. No production-code refactoring. No changes to the workspace profile save/load architecture.
+
+---
+
+## Task 1: Fix `_capture_workspace_profile` str/bytes sentinel
+
+**Files:**
+- Modify: `src/gui_2.py:601-609`
+- Test: deferred to Task 3 (regression unit test)
+
+- [ ] **Step 1.1: Pre-edit checkpoint**
+
+```powershell
+cd C:\projects\manual_slop; git add .
+```
+
+- [ ] **Step 1.2: Read the current state of `_capture_workspace_profile`**
+
+Read `src/gui_2.py:601-609` to confirm the current code.
+
+- [ ] **Step 1.3: Apply the fix**
+
+Replace the current `_capture_workspace_profile` defer-and-except block (lines 601-609) with:
+
+```python
+ def _capture_workspace_profile(self, name: str) -> models.WorkspaceProfile:
+  if not getattr(self, "_ini_capture_ready", False):
+   self._ini_capture_ready = True
+   ini = ""
+  else:
+   try:
+    ini = str(imgui.save_ini_settings_to_memory() or "")
+   except Exception:
+    ini = ""
+  panel_states = {
+```
+
+Use `manual-slop_py_update_definition` with the existing function name `_capture_workspace_profile` to do the surgical replacement. The body change is:
+- Line 604: `ini = b""` → `ini = ""`
+- Line 609: `ini = b""` → `ini = ""`
+- Line 607: `ini = imgui.save_ini_settings_to_memory()` → `ini = str(imgui.save_ini_settings_to_memory() or "")`
+
+Use exactly 1-space indentation.
+
+- [ ] **Step 1.4: Verify the file still parses**
+
+```powershell
+cd C:\projects\manual_slop; uv run python -c "import ast; ast.parse(open('src/gui_2.py').read())"
+```
+
+Expected: no error.
+
+- [ ] **Step 1.5: Run the workspace-profile-related tests to verify the fix**
+
+```powershell
+cd C:\projects\manual_slop; uv run pytest tests/test_workspace_manager.py tests/test_workspace_profiles_sim.py tests/test_auto_switch_sim.py -v --timeout=60
+```
+
+Expected:
+- `test_workspace_manager.py` passes (it tests the manager's save/load semantics with mocked profiles).
+- `test_workspace_profiles_sim.py` passes (it uses `live_gui`).
+- `test_auto_switch_sim.py` passes (it uses `live_gui`).
+
+If `test_workspace_profiles_sim.py` or `test_auto_switch_sim.py` fails, it should be ONLY because of session-state pollution from a prior run. The fix targets the underlying bug; the test infrastructure (live_gui fixture) is what makes these flake. Re-run individually if needed:
+
+```powershell
+cd C:\projects\manual_slop; uv run pytest tests/test_workspace_profiles_sim.py::test_workspace_profiles_restoration -v --timeout=60
+```
+
+- [ ] **Step 1.6: Commit**
+
+```powershell
+cd C:\projects\manual_slop; git add src/gui_2.py
+git -C C:\projects\manual_slop commit -m "fix(gui_2): use str sentinel not bytes in _capture_workspace_profile"
+$h = git -C C:\projects\manual_slop log -1 --format='%H'
+git -C C:\projects\manual_slop notes add -m "WorkspaceProfile.ini_content is str (src/models.py:799) and tomli_w rejects bytes. The d7487af4 defer fix used ini=b'' which crashed TOML serialization, so save_workspace_profile raised TypeError, profile was never saved, and load_workspace_profile became a no-op. Changed both ini=b'' to ini='' and added str() defensive wrap on the non-defer path. Fixes test_auto_switch_sim and test_workspace_profiles_restoration." $h
+```
+
+---
+
+## Task 2: Fix prior session test mock for `imscope.window`
+
+**Files:**
+- Modify: `tests/test_prior_session_no_pop_imbalance.py` (the mock setup loop ~line 75-78, where the test sets up imscope context managers)
+
+- [ ] **Step 2.1: Pre-edit checkpoint**
+
+```powershell
+cd C:\projects\manual_slop; git add .
+```
+
+- [ ] **Step 2.2: Read the current mock setup**
+
+Read `tests/test_prior_session_no_pop_imbalance.py:60-95` to see the `mock_imscope` setup.
+
+- [ ] **Step 2.3: Apply the fix**
+
+Find the loop that sets `__enter__` and `__exit__` for all imscope context managers (it looks like this around line 70-80):
+
+```python
+      for sc in [mock_imscope.style_color, mock_imscope.style_var, mock_imscope.child, mock_imscope.tab_bar, mock_imscope.tab_item, mock_imscope.tree_node_ex, mock_imscope.group, mock_imscope.indent, mock_imscope.id, mock_imscope.text_wrap, mock_imscope.tooltip, mock_imscope.menu, mock_imscope.menu_bar, mock_imscope.popup, mock_imscope.popup_modal, mock_imscope.window, mock_imscope.table]:
+       sc.return_value.__enter__ = MagicMock(side_effect=_scope_enter)
+       sc.return_value.__exit__ = MagicMock(side_effect=_scope_exit)
+```
+
+Note: `mock_imscope.window` is in this list. The default `MagicMock(side_effect=_scope_enter)` returns a bare `MagicMock` (non-iterable), but production code at `src/gui_2.py:2333` does `with imscope.window(...) as (opened, visible):` which expects a 2-tuple.
+
+After the loop (around line 91, after the `mock_imscope.popup_modal.return_value.__enter__ = MagicMock(return_value=(True, None))` line), add:
+
+```python
+      mock_imscope.window.return_value.__enter__ = MagicMock(return_value=(True, True))
+```
+
+This matches the pattern already used for `popup_modal` (which returns `(True, None)`). The `__exit__` from the loop above is preserved (returns `False`, indicating no exception).
+
+Use `manual-slop_edit_file` with the exact `old_string` from the popup_modal mock line and the `new_string` with the additional window mock line right after it.
+
+Use exactly 1-space indentation.
+
+- [ ] **Step 2.4: Run the prior session test**
+
+```powershell
+cd C:\projects\manual_slop; uv run pytest tests/test_prior_session_no_pop_imbalance.py -v --timeout=30
+```
+
+Expected: PASS.
+
+- [ ] **Step 2.5: Commit**
+
+```powershell
+cd C:\projects\manual_slop; git add tests/test_prior_session_no_pop_imbalance.py
+git -C C:\projects\manual_slop commit -m "test(prior_session): mock imscope.window with tuple-return matching popup_modal"
+$h = git -C C:\projects\manual_slop log -1 --format='%H'
+git -C C:\projects\manual_slop notes add -m "The test's mock setup loop for imscope context managers set __enter__ to a bare MagicMock (non-iterable), but render_preset_manager_window at src/gui_2.py:2333 does 'with imscope.window(...) as (opened, visible):' which expects a 2-tuple. popup_modal already had the right setup; window was missing it. Added the tuple-return for window, matching popup_modal's pattern." $h
+```
+
+---
+
+## Task 3: Add regression unit test for `WorkspaceProfile` str/bytes contract
+
+**Files:**
+- Create: `tests/test_workspace_profile_serialization.py`
+
+- [ ] **Step 3.1: Pre-edit checkpoint**
+
+```powershell
+cd C:\projects\manual_slop; git add .
+```
+
+- [ ] **Step 3.2: Write the test file**
+
+Create `tests/test_workspace_profile_serialization.py`:
+
+```python
+import io
+import tomllib
+import pytest
+import tomli_w
+from src.models import WorkspaceProfile
+
+
+def test_workspace_profile_empty_ini_content_roundtrips():
+  """WorkspaceProfile with ini_content='' (empty str) must round-trip through TOML.
+  This is the str/bytes type contract that the defer-not-catch fix in d7487af4 violated
+  (it used ini=b'' which tomli_w rejects with TypeError).
+  """
+  profile = WorkspaceProfile(
+   name="t",
+   ini_content="",
+   show_windows={"A": True, "B": False},
+   panel_states={"x": 1, "y": 2.0, "z": True},
+  )
+  d = profile.to_dict()
+  buf = io.BytesIO()
+  tomli_w.dump({"t": d}, buf)
+  buf.seek(0)
+  back = tomllib.load(buf)
+  loaded = WorkspaceProfile.from_dict("t", back["t"])
+  assert loaded.ini_content == ""
+  assert loaded.show_windows == {"A": True, "B": False}
+  assert loaded.panel_states == {"x": 1, "y": 2.0, "z": True}
+
+
+def test_workspace_profile_with_actual_ini_content_roundtrips():
+  """WorkspaceProfile with real ini content (str) must round-trip through TOML.
+  This mirrors how save_ini_settings_to_memory() returns a str at runtime.
+  """
+  profile = WorkspaceProfile(
+   name="real",
+   ini_content="[Window][Debug]\nPos=10,20\n",
+   show_windows={},
+   panel_states={},
+  )
+  d = profile.to_dict()
+  buf = io.BytesIO()
+  tomli_w.dump({"real": d}, buf)
+  buf.seek(0)
+  back = tomllib.load(buf)
+  loaded = WorkspaceProfile.from_dict("real", back["real"])
+  assert loaded.ini_content == "[Window][Debug]\nPos=10,20\n"
+  assert loaded.name == "real"
+  assert loaded.show_windows == {}
+  assert loaded.panel_states == {}
+
+
+def test_workspace_profile_bytes_ini_content_rejected_by_toml():
+  """Regression guard: a bytes ini_content must raise TypeError from tomli_w.
+  This documents the type contract; if tomli_w ever gains bytes support, the
+  contract should be revisited (e.g. by switching WorkspaceProfile.ini_content
+  to bytes and updating the imgui.load_ini_settings_from_memory call site).
+  """
+  profile = WorkspaceProfile(
+   name="bad",
+   ini_content=b"",  # type: ignore[arg-type]
+   show_windows={},
+   panel_states={},
+  )
+  d = profile.to_dict()
+  buf = io.BytesIO()
+  with pytest.raises(TypeError, match="bytes"):
+   tomli_w.dump({"bad": d}, buf)
+```
+
+Use exactly 1-space indentation. No comments per project style.
+
+- [ ] **Step 3.3: Run the test to verify it passes (Change 1 should already be applied)**
+
+```powershell
+cd C:\projects\manual_slop; uv run pytest tests/test_workspace_profile_serialization.py -v --timeout=15
+```
+
+Expected: 3 passed (one per test).
+
+- [ ] **Step 3.4: Verify the test would catch the regression**
+
+Temporarily revert the fix in `src/gui_2.py:604` from `ini = ""` back to `ini = b""` and re-run:
+
+```powershell
+cd C:\projects\manual_slop; uv run pytest tests/test_workspace_profile_serialization.py -v --timeout=15
+```
+
+Expected: the first two tests should still pass (they test the dataclass round-trip directly, not the defer fix), and the third test confirms the type contract. The test that catches the regression is the integration test in Task 1 (which goes through the live_gui save flow).
+
+Restore the fix:
+
+```powershell
+cd C:\projects\manual_slop; git diff src/gui_2.py  # confirm only the in-scope fix is there
+```
+
+If you reverted the fix, re-apply it via `manual-slop_edit_file` and verify the tests still pass.
+
+- [ ] **Step 3.5: Commit**
+
+```powershell
+cd C:\projects\manual_slop; git add tests/test_workspace_profile_serialization.py
+git -C C:\projects\manual_slop commit -m "test(workspace_profile): add str/bytes TOML serialization contract test"
+$h = git -C C:\projects\manual_slop log -1 --format='%H'
+git -C C:\projects\manual_slop notes add -m "Encodes the WorkspaceProfile.ini_content: str contract. The d7487af4 defer fix used ini=b'' which tomli_w rejects with TypeError. This test would have caught the regression at unit-test speed (no live_gui needed). 3 tests: empty str round-trips, real ini content round-trips, bytes ini_content is rejected (documents the contract)." $h
+```
+
+---
+
+## Task 4: Verify all 3 originally-failing tests now pass
+
+**Files:** (no file changes; verification only)
+
+- [ ] **Step 4.1: Run the 3 originally-failing tests**
+
+```powershell
+cd C:\projects\manual_slop; uv run pytest tests/test_auto_switch_sim.py tests/test_workspace_profiles_sim.py tests/test_prior_session_no_pop_imbalance.py -v --timeout=60
+```
+
+Expected: 3 passed (one file each).
+
+- [ ] **Step 4.2: Run the regression unit test**
+
+```powershell
+cd C:\projects\manual_slop; uv run pytest tests/test_workspace_profile_serialization.py -v --timeout=15
+```
+
+Expected: 3 passed.
+
+- [ ] **Step 4.3: Run the full batched test suite**
+
+```powershell
+cd C:\projects\manual_slop; uv run python scripts/run_tests_batched.py
+```
+
+Expected: 273 files (272 + 1 new), all batches pass (273/273 = 100%).
+
+- [ ] **Step 4.4: Commit plan update**
+
+```powershell
+cd C:\projects\manual_slop; git add conductor/tracks.md docs/superpowers/plans/2026-06-05-live-gui-fragility-fixes.md
+```
+
+Then append the following entry to `conductor/tracks.md` (under the existing `regression_fixes_20260605` entry or as a new entry):
+
+```markdown
+- [x] **Track: Live-GUI Fragility Fixes (post regression_fixes_20260605)** `[checkpoint: <sha>]`
+*Link: [./tracks/live_gui_fragility_fixes_20260605/](./tracks/live_gui_fragility_fixes_20260605/), Spec: [./../../docs/superpowers/specs/2026-06-05-live-gui-fragility-fixes-design.md](./../../docs/superpowers/specs/2026-06-05-live-gui-fragility-fixes-design.md), Plan: [./../../docs/superpowers/plans/2026-06-05-live-gui-fragility-fixes.md](./../../docs/superpowers/plans/2026-06-05-live-gui-fragility-fixes.md)*
+*Goal: Fix 3 remaining live_gui test failures (269/272 → 272/272). 1-line src fix in `_capture_workspace_profile` (str/bytes sentinel that broke TOML serialization), 2-line test mock fix for `imscope.window` tuple-return, 1 new regression unit test for the str/bytes contract. All atomic per-file commits. The d7487af4 defer fix had introduced a TypeError via `ini=b""`; the regression was traced to `WorkspaceProfile.ini_content: str` and tomli_w's bytes rejection.*
+```
+
+(Replace `<sha>` with the actual checkpoint SHA from the last commit.)
+
+```powershell
+cd C:\projects\manual_slop; git -c core.autocrlf=false commit -m "conductor(plan): mark live_gui_fragility_fixes track complete"
+$h = git -C C:\projects\manual_slop log -1 --format='%H'
+git -C C:\projects\manual_slop notes add -m "Track complete. 273/273 tests pass (was 269/272 pre-track, 272/273 mid-track). 3 atomic per-file commits: src/gui_2.py, test_prior_session_no_pop_imbalance.py, new test_workspace_profile_serialization.py." $h
+```
+
+---
+
+## Task 5 (OPTIONAL): Doc hardening of defer-not-catch sections
+
+> **Skip this task if time is short.** Per user review 2026-06-05, this is deferred to the end. If you've reached the end of the track with time to spare, do it; otherwise, leave for a follow-up.
+
+**Files:**
+- Modify: `docs/guide_gui_2.md` "Workspace Profile Defer-Not-Catch" section
+- Modify: `docs/guide_testing.md` "Early-Render C-Level Crashes" section
+- Modify: `conductor/workflow.md` "Defer-Not-Catch Pattern for Native Crashes" section
+
+- [ ] **Step 5.1: Add a one-paragraph note to each of the three docs**
+
+Add this note (paraphrased to fit each doc's voice) to each of the three defer-not-catch sections:
+
+> "**Sentinel type contract.** When implementing a defer-not-catch guard, the early-return sentinel value must match the type contract of the downstream consumer. For `WorkspaceProfile.ini_content: str` (in this codebase), the sentinel must be `""` (str), not `b""` (bytes) — `tomli_w` rejects bytes, and `imgui.load_ini_settings_from_memory(ini_data: str, ...)` also expects str. A previous version of this fix used `b""` and silently broke the save flow via a `TypeError` raised by `tomli_w.dump`; tests passed unit-test-wise but failed in the live_gui save+load round-trip."
+
+- [ ] **Step 5.2: Commit**
+
+```powershell
+cd C:\projects\manual_slop; git add docs/guide_gui_2.md docs/guide_testing.md conductor/workflow.md
+git -C C:\projects\manual_slop commit -m "docs: add sentinel-type-contract note to defer-not-catch sections"
+$h = git -C C:\projects\manual_slop log -1 --format='%H'
+git -C C:\projects\manual_slop notes add -m "Three doc updates: guide_gui_2.md, guide_testing.md, workflow.md. Added a 'Sentinel type contract' note to each defer-not-catch section warning that the early-return sentinel must match the downstream consumer's type contract (str not bytes for WorkspaceProfile.ini_content). Prevents future regressions of the kind introduced by d7487af4." $h
+```
+
+---
+
+## Self-Review
+
+After writing the complete plan, check against the spec:
+
+**1. Spec coverage:**
+- Change 1 (`b""` → `""` fix): Task 1, Step 1.3. ✓
+- Change 2 (test mock fix): Task 2, Step 2.3. ✓
+- Change 3 (regression unit test): Task 3, Step 3.2. ✓
+- Change 4 (doc hardening, deferred): Task 5, marked OPTIONAL. ✓
+- Goals: 100% pass rate (Task 4, Step 4.3). ✓
+- Non-goals respected: no workspace profile refactor, no wait-for-ready framework, no sloppy.py startup changes. ✓
+
+**2. Placeholder scan:** No "TBD"/"TODO"/"implement later" patterns. All code blocks are complete. ✓
+
+**3. Type consistency:** `WorkspaceProfile.ini_content: str` referenced consistently. `b""` → `""` change is the single source of the fix. `_ini_capture_ready` flag is preserved. `str(...) or ""` wrap is documented. ✓
+
+---
+
+## Execution Handoff
+
+This plan is sized for **inline execution** (single agent, no subagents, per the user's stated preference). Execute Tasks 1-4 in order; skip Task 5 if time is short.
+
+After each task's commit, attach the git note (the `$h` line in each task). After all tasks, run Task 4's full suite to confirm 100% pass.
+
+If any task fails, stop and run `/conductor:implement --debug` or escalate to a Tier 4 QA analysis (per `conductor/workflow.md`).