13 KiB
Live-GUI State Sync — Design
Date: 2026-06-05 Status: Draft Track: live_gui_state_sync_20260605 (sub-project of v2)
Problem Statement
App (src/gui_2.py) and AppController (src/app_controller.py) maintain parallel state for the same logical fields. set_value writes to the Controller, but several code paths read from the App, returning stale or wrong values.
Concrete failures (from 2026-06-05 batched test run, batches 7, 46, 65, 68)
-
test_auto_switch_sim::test_auto_switch_sim— setsui_separate_tier1=Trueandshow_windows['Diagnostics']=True, savesTier3Profile, sets to False, triggers tier-3 auto-switch. Expectsshow_windows['Diagnostics']=Truerestored. Fails: profile captures from App but is set on Controller. -
test_workspace_profiles_restoration::test_workspace_profiles_restoration— setsui_separate_tier1=True, savestest_restore, sets to False, loads. Expects True. Fails: same root cause. -
test_undo_redo_lifecycle::test_undo_redo_lifecycle(NEW regression) — setsai_input="Initial Input", modifies to"Modified Input", clicksbtn_undo. Expectsai_input="Initial Input". Fails: snapshot readsapp.ui_ai_inputbutset_valuewrites tocontroller.ui_ai_input.
Discovery (2026-06-05 execution): State sync is NOT the root cause
Initial hypothesis: App and Controller maintain parallel state for settable fields. Verified during execution: the App class already has __getattr__ (line 478) and __setattr__ (line 483) that auto-delegate to the controller. Writes go through __setattr__ → controller. Reads go through __getattr__ → controller. The state is correctly synced at the descriptor level. The original spec assumption was wrong.
REAL root cause: _capture_workspace_profile is not a class method
During execution, AST analysis of src/gui_2.py reveals the actual bug:
$ uv run python -c "import ast; ..."
App methods (count): 59
WORKSPACE METHOD: _apply_workspace_profile # ← exists
# ← _capture_workspace_profile MISSING
_capture_workspace_profile is defined at line 607 of src/gui_2.py with 2-space indent (intended as a class method), but the AST walks it as nested inside _apply_snapshot (line 572). The body of _apply_snapshot (lines 573-635) absorbs the next def as a nested function.
This means when the live_gui calls self._app._capture_workspace_profile(name), Python's normal class lookup fails to find _capture_workspace_profile on the App class. __getattr__('_capture_workspace_profile') is triggered, which delegates to self.controller._capture_workspace_profile. The controller does NOT have this method. AttributeError is raised. The save callback fails silently. The test's load_workspace_profile finds no profile to load (because save failed). The test fails.
Why AST sees it as nested
The likely cause is the user's recent cleanup commit 873edf42 ("began to go through the files and organize imports and gui_2.py's new context defs") which touched src/gui_2.py:261 lines. The cleanup reorganized method placement. Either:
- Indentation was accidentally off by 1 space on some lines.
- A blank line or comment that closed a function body was removed.
- Method definitions were moved but their indentation wasn't updated.
Specific to the bug: _apply_snapshot has a try: (line 574) without an except (only a finally: at line 604). This is valid Python syntax, but the indentation of subsequent lines may have been off, causing the AST to consume the next def into the try block.
Audit of duplicated fields (retained from original spec, for context)
Static analysis of the 71 settable fields in AppController._settable_fields vs the 12 panel_states keys captured in App._capture_workspace_profile, plus the show_windows dict and snapshot fields:
| Field | In _settable_fields (Controller)? |
Read by App code? | Sync bug? |
|---|---|---|---|
show_windows |
yes | _capture_workspace_profile (line 627), _apply_workspace_profile (line 633) |
YES |
ui_separate_task_dag |
yes | _capture_workspace_profile (line 615) |
YES |
ui_separate_usage_analytics |
yes | _capture_workspace_profile (line 616) |
YES |
ui_separate_tier1 |
yes | _capture_workspace_profile (line 617) |
YES |
ui_separate_tier2 |
yes | _capture_workspace_profile (line 618) |
YES |
ui_separate_tier3 |
yes | _capture_workspace_profile (line 619) |
YES |
ui_separate_tier4 |
yes | _capture_workspace_profile (line 620) |
YES |
ui_ai_input |
yes (ai_input -> ui_ai_input) |
_take_snapshot (line 551), _apply_snapshot (line 569) |
YES |
ui_separate_context_preview |
no (NOT in settable_fields) | _capture_workspace_profile (line 611) |
no — App-only |
ui_separate_message_panel |
no | _capture_workspace_profile (line 612) |
no — App-only |
ui_separate_response_panel |
no | _capture_workspace_profile (line 613) |
no — App-only |
ui_separate_tool_calls_panel |
no | _capture_workspace_profile (line 614) |
no — App-only |
ui_separate_external_tools |
no | _capture_workspace_profile (line 621) |
no — App-only |
ui_discussion_split_h |
no | _capture_workspace_profile (line 622) |
no — App-only |
8 confirmed sync bugs. Plus ui_ai_input (snapshot) is a 9th.
Root Cause
App.__init__ creates a separate AppController instance and later sets self.controller._app = self (bidirectional link). The two objects each declare their own self.ui_separate_tier1 = False (App) and self.ui_separate_tier1 = False (Controller) in their respective __init__s. They are independent Python attributes.
set_value (src/api_hooks.py, line 614) calls setattr(controller, attr_name, value) — writes to Controller. But _capture_workspace_profile reads self.ui_separate_tier1 where self is the App — never updated.
Design
Goal
Eliminate the dual state. Single source of truth: the Controller. The App becomes a thin "view" layer that exposes Controller fields as Python properties. set_value continues to write to the Controller. All reads (from save, snapshot, render) transparently read from the Controller.
Approach: Properties on App that delegate to Controller
Add @property definitions on the App class for each field that has a Controller counterpart. The getter returns self.controller.X. The setter (where App code writes, e.g. snapshot restore) also delegates to self.controller.X.
Hypothetical example for ui_separate_tier1:
# In App class (src/gui_2.py)
@property
def ui_separate_tier1(self) -> bool:
return self.controller.ui_separate_tier1
@ui_separate_tier1.setter
def ui_separate_tier1(self, value: bool) -> None:
self.controller.ui_separate_tier1 = value
This makes app.ui_separate_tier1 and controller.ui_separate_tier1 the same value, regardless of which path writes. The only writes are via the property setter (or set_value via the Controller directly), and all reads go through the getter.
Why this approach
- Minimal blast radius: The App class only adds properties; no method bodies change. Methods that read
self.Xcontinue to work — they just get the Controller's value via the property. - Bidirectional: Setter support is critical for
_apply_snapshotand_apply_workspace_profilewhich set App fields directly (self.ui_ai_input = snapshot.ai_input). They go through the property setter, which writes to the Controller. - No double-write footgun: A "sync on set_value" alternative requires remembering to write to BOTH objects. A property approach is a single point of truth.
- Easy to migrate incrementally: Each field is one property pair. Can be added one at a time with a regression test for each.
Alternatives considered
- A2: Merge App and Controller into one class. Rejected: would be a 5532-line → 4000-line merge with high risk. The Controller already lives in a separate file; the App delegates to it via
self.controller.X. Merging would lose the existing boundary. - A3: Sync on every set_value (write to both). Rejected: requires touching every writer; easy to miss a site. Property approach is one place per field.
- A4: Pass Controller as a method argument everywhere. Rejected: invasive; requires changing method signatures throughout
gui_2.pyandapp_controller.py.
File Changes
Modify: src/gui_2.py (App class)
Add @property + @X.setter for each of the 8 sync-bug fields, plus ui_ai_input:
@property
def ui_separate_tier1(self) -> bool:
return self.controller.ui_separate_tier1
@ui_separate_tier1.setter
def ui_separate_tier1(self, value: bool) -> None:
self.controller.ui_separate_tier1 = value
Fields to add properties for:
ui_ai_input(snapshot bug)ui_separate_task_dagui_separate_usage_analyticsui_separate_tier1throughui_separate_tier4show_windows(special: dict, not bool)
For show_windows, the property needs care — set_value may pass a new dict; the property should do self.controller.show_windows = value to allow full replacement, but for in-place updates (self.show_windows["X"] = True), the property getter returns the Controller's dict reference (so in-place mutations work) and the property setter can either replace or do nothing (since the dict is shared).
@property
def show_windows(self) -> Dict[str, bool]:
return self.controller.show_windows
@show_windows.setter
def show_windows(self, value: Dict[str, bool]) -> None:
self.controller.show_windows = value
Do NOT add properties for fields that are App-only (no Controller counterpart): ui_separate_context_preview, ui_separate_message_panel, ui_separate_response_panel, ui_separate_tool_calls_panel, ui_separate_external_tools, ui_discussion_split_h, etc. — they remain as plain App attributes.
Add: tests/test_app_controller_state_sync.py (new)
A new unit test that encodes the contract: for every field in _settable_fields that is also referenced as self.X in the App class's _capture_workspace_profile and _take_snapshot/_apply_snapshot, writes to app.X and controller.X must be observed by both.
def test_ui_separate_tier1_setter_delegates_to_controller():
"""The App's ui_separate_tier1 property is a delegate to the Controller.
Writes through app.ui_separate_tier1 = X are visible at controller.ui_separate_tier1,
and writes through set_value (which goes to controller) are visible at app.ui_separate_tier1."""
from src import app_controller, gui_2
from src.app_controller import AppController
# Don't fully init App (too heavy); use lightweight setup
app = gui_2.App.__new__(gui_2.App)
app.controller = AppController()
app._app = app # back-ref
# set_value goes to controller
app.controller.ui_separate_tier1 = True
assert app.ui_separate_tier1 is True # reads through property
# direct set through app's property
app.ui_separate_tier1 = False
assert app.controller.ui_separate_tier1 is False # write visible at controller
This is a regression test for the contract.
Test impact
After the fix, these tests should pass:
test_auto_switch_sim::test_auto_switch_sim(writes toapp.show_windowsandapp.ui_separate_tier1are observed by save)test_workspace_profiles_sim::test_workspace_profiles_restoration(same)test_undo_redo_lifecycle::test_undo_redo_lifecycle(snapshot reads fromapp.ui_ai_inputget the Controller's value)
If test_undo_redo_lifecycle is also a flake or a regression from the user's recent cleanup commit 873edf42, the property fix may not be sufficient. In that case, the test will continue to fail and need its own investigation track.
Risk Assessment
| Risk | Likelihood | Impact | Mitigation |
|---|---|---|---|
Existing App code does del app.ui_X to reset state |
Low | Low | property setter can be a no-op for del (raises AttributeError); review call sites |
| App class is 5532 lines — risk of regression | High | Medium | Per-field property addition; one regression test per field; ship in a single atomic commit |
User's recent cleanup commit 873edf42 may have added or removed attribute references |
Medium | Low | Run targeted regression test after each property addition |
| New properties shadow existing class attributes | Low | High | Use dir(app) to verify no shadow before commit |
Out of Scope
- prior_session test mock setup — separate track (
prior_session_test_harden_20260605). - wait-for-ready test pattern — separate track (
wait_for_ready_test_pattern_20260605). - Other App/Controller sync bugs not in the 8 listed — audit will continue; if more found, queue as v3 sub-track.
- Refactoring App and Controller into one class — deferred; property approach is sufficient for now.