Private
Public Access
0
0

docs(gui_2): add __getattr__/__setattr__ delegation pattern + indentation gotcha

This commit is contained in:
2026-06-06 01:59:20 -04:00
parent 4ee22dedb9
commit e276bac093
+44
View File
@@ -421,6 +421,50 @@ This pattern unblocks 4-5 live_gui tests that were crashing the GUI subprocess d
**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 (`TypeError: Object of type 'bytes' is not TOML serializable`), 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. The fix was a 1-character change (`b""` → `""`). The regression test in `tests/test_workspace_profile_serialization.py` encodes this contract.
### The `__getattr__` / `__setattr__` State Delegation Pattern
The `App` class (around line 478-487) defines two descriptor hooks that delegate state to the `AppController`:
```python
def __getattr__(self, name: str) -> Any:
if name == 'controller':
raise AttributeError(name)
return getattr(self.controller, name)
def __setattr__(self, name: str, value: Any) -> None:
if name != 'controller' and hasattr(self, 'controller') and hasattr(self.controller, name):
setattr(self.controller, name, value)
else:
object.__setattr__(self, name, value)
```
**Why this matters:**
- The `Controller` is the single source of truth for settable state (e.g. `ui_ai_input`, `ui_separate_tier1`, `show_windows`, `temperature`).
- The `App` is a thin view layer that delegates reads (`__getattr__`) and writes (`__setattr__`) to the Controller.
- This means: **do NOT add `self.ui_ai_input = ""` in `App.__init__` for fields that the Controller owns.** The Controller initializes them via its own `__init__`. If the App initializes them too, the App's value shadows the Controller's (and `__getattr__` returns the App's value, not the Controller's).
**Safe App-only state (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` — these are NOT in the Controller's `_settable_fields`, so `__setattr__` falls through to `object.__setattr__` and stores them on the App.
- Private App state (`_ini_capture_ready`, `_pending_gui_tasks`, etc.) is also App-only.
**Subtle gotcha:** the `hasattr(self.controller, name)` check in `__setattr__` returns `False` for App-only fields on the **first** write (because the Controller doesn't have the attribute yet). The write goes to the App. The Controller never gets the attribute. This is the **correct** behavior for App-only fields, but **wrong** for Controller-owned fields that haven't been initialized in the Controller's `__init__`. Always make sure Controller-owned fields are initialized in `AppController.__init__` (or in `init_state` called from there) so `__setattr__`'s `hasattr` check returns `True`.
### Indentation Gotcha (CRITICAL)
**The bug:** A class method defined with the right intent (2-space indent) may be parsed as **nested inside the previous function** if indentation is off by even one space. The file "passes" syntactically (imports OK) but the method is **not** on the class. `hasattr(App, 'method_name')` returns `False`. Any production code that calls `app.method_name` falls through to `__getattr__`, delegates to the Controller (which also doesn't have the method), and a cryptic `AttributeError` is raised at runtime.
**How to detect:** Use AST to list all App methods. The skeleton via `manual-slop_py_get_skeleton` should show the method as a class member. If the AST walk doesn't find the method, it's nested.
```bash
uv run python -c "import ast; tree = ast.parse(open('src/gui_2.py').read()); [print(item.name) for n in ast.walk(tree) if isinstance(n, ast.ClassDef) and n.name == 'App' for item in n.body if isinstance(item, ast.FunctionDef)]"
```
**How to fix:** Re-indent the affected method to 2-space class level. This bit the project in 2026-06-05 during a cleanup commit: `_capture_workspace_profile` was being parsed as nested inside `_apply_snapshot` due to a 1-space indentation drift, breaking 3 live_gui tests (test_auto_switch_sim, test_workspace_profiles_restoration, test_undo_redo_lifecycle).
---
---
## See Also