From 7a0ed74b5cde82704c704bbaca0ba8db444ac928 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Fri, 5 Jun 2026 19:20:21 -0400 Subject: [PATCH] docs(plan): implementation plan for live-gui fragility fixes --- .../2026-06-05-live-gui-fragility-fixes.md | 387 ++++++++++++++++++ 1 file changed, 387 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-05-live-gui-fragility-fixes.md diff --git a/docs/superpowers/plans/2026-06-05-live-gui-fragility-fixes.md b/docs/superpowers/plans/2026-06-05-live-gui-fragility-fixes.md new file mode 100644 index 00000000..679fcc59 --- /dev/null +++ b/docs/superpowers/plans/2026-06-05-live-gui-fragility-fixes.md @@ -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: ]` +*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 `` 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`).