Botched: Need to do a higher reaosning model to fix this mess.

This commit is contained in:
2026-03-04 12:32:14 -05:00
parent 8d3fdb53d0
commit 01b3c26653
18 changed files with 180 additions and 132 deletions

View File

@@ -0,0 +1,50 @@
# Comprehensive Debrief: GUI Decoupling Track (Botched Implementation)
## 1. Track Overview
* **Track Name:** GUI Decoupling & Controller Architecture
* **Track ID:** `gui_decoupling_controller_20260302`
* **Primary Objective:** Decouple business logic from `gui_2.py` (3,500+ lines) into a headless `AppController`.
## 2. Phase-by-Phase Failure Analysis
### Phase 1: Controller Skeleton & State Migration
* **Status:** [x] Completed (with major issues)
* **What happened:** State variables (locks, paths, flags) were moved to `AppController`. `App` was given a `__getattr__` and `__setattr__` bridge to delegate to the controller.
* **Failure:** The delegation created a "Phantom State" problem. Sub-agents began treating the two objects as interchangeable, but they are not. Shadowing (where `App` has a variable that blocks `Controller`) became a silent bug source.
### Phase 2: Logic & Background Thread Migration
* **Status:** [x] Completed (with critical regressions)
* **What happened:** Async loops, AI client calls, and project I/O were moved to `AppController`.
* **Failure 1 (Over-deletion):** Tier 3 workers deleted essential UI-thread handlers from `App` (like `_handle_approve_script`). This broke button callbacks and crashed the app on startup.
* **Failure 2 (Thread Violation):** A "fallback queue processor" was added to the Controller thread. This caused two threads to race for the same event queue. If the Controller won, the UI never blinked/updated, causing simulation timeouts.
* **Failure 3 (Property Erasure):** During surgical cleanups in this high-reasoning session, the `current_provider` getter/setter in `AppController` was accidentally deleted while trying to remove a redundant method. `App` now attempts to delegate to a non-existent attribute, causing `AttributeError`.
### Phase 3: Test Suite Refactoring
* **Status:** [x] Completed (fragile)
* **What happened:** `conftest.py` was updated to patch `AppController` methods.
* **Failure:** The `live_gui` sandbox environment (isolated workspace) was broken because the Controller now eagerly checks for `credentials.toml` on startup. The previous agent tried to "fix" this by copying secrets into the sandbox, which is a security regression and fragile.
### Phase 4: Final Validation
* **Status:** [ ] FAILED
* **What happened:** Integration tests and extended simulations fail or timeout consistently.
* **Root Cause:** Broken synchronization between the Controller's background processing and the GUI's rendering loop. The "Brain" (Controller) and "Limb" (GUI) are disconnected.
## 3. Current "Fucked" State of the Codebase
* **`src/gui_2.py`:** Contains rendering but is missing critical property logic. It still shadows core methods that should be purely in the controller.
* **`src/app_controller.py`:** Missing core properties (`current_provider`) and has broken `start_services` logic.
* **`tests/conftest.py`:** Has a messy `live_gui` fixture that uses environment variables (`SLOP_CREDENTIALS`, `SLOP_MCP_ENV`) but points to a sandbox that is missing the actual files.
* **`sloppy.py`:** The entry point works but the underlying classes are in a state of partial migration.
## 4. Immediate Recovery Plan (New Phase 5)
### Phase 5: Stabilization & Cleanup
1. **Task 5.1: AST Synchronization Audit.** Manually (via AST) compare `App` and `AppController`. Ensure every property needed for the UI exists in the Controller and is correctly delegated by `App`.
2. **Task 5.2: Restore Controller Properties.** Re-implement `current_provider` and `current_model` in `AppController` with proper logic (initializing adapters, clearing stats).
3. **Task 5.3: Explicit Delegation.** Remove the "magic" `__getattr__` and `__setattr__`. Replace them with explicit property pass-throughs. This will make `AttributeError` visible during static analysis rather than runtime.
4. **Task 5.4: Fix Sandbox Isolation.** Ensure `live_gui` fixture in `conftest.py` correctly handles `credentials.toml` via `SLOP_CREDENTIALS` env var pointing to the root, and ensure `sloppy.py` respects it.
5. **Task 5.5: Event Loop Consolidation.** Ensure there is EXACTLY ONE `asyncio` loop running, owned by the Controller, and that the GUI thread only reads from `_pending_gui_tasks`.
## 5. Technical Context for Next Session
* **Encoding issues:** `temp_conftest.py` and other git-shipped files often have UTF-16 or different line endings. Use Python-based readers to bypass `read_file` failures.
* **Crucial Lines:** `src/gui_2.py` line 180-210 (Delegation) and `src/app_controller.py` line 460-500 (Event Processing) are the primary areas of failure.
* **Mocking:** All `patch` targets in `tests/` must now be audited to ensure they hit the Controller, not the App.

View File

@@ -23,3 +23,10 @@
- [ ] HOW: Ensure 100% pass rate. - [ ] HOW: Ensure 100% pass rate.
- [ ] SAFETY: Watch out for lingering thread closure issues. - [ ] SAFETY: Watch out for lingering thread closure issues.
- [ ] Task: Conductor - User Manual Verification 'Phase 4: Final Validation' (Protocol in workflow.md) - [ ] Task: Conductor - User Manual Verification 'Phase 4: Final Validation' (Protocol in workflow.md)
## Phase 5: Stabilization & Cleanup (RECOVERY)
- [ ] Task: Task 5.1: AST Synchronization Audit
- [ ] Task: Task 5.2: Restore Controller Properties (Restore `current_provider`)
- [ ] Task: Task 5.3: Replace magic `__getattr__` with Explicit Delegation
- [ ] Task: Task 5.4: Fix Sandbox Isolation logic in `conftest.py`
- [ ] Task: Task 5.5: Event Loop Consolidation & Single-Writer Sync

67
run_repro.py Normal file
View File

@@ -0,0 +1,67 @@
import time
import requests
import sys
import os
# Ensure src/ is in path
project_root = os.path.dirname(os.path.abspath(__file__))
src_path = os.path.join(project_root, "src")
sys.path.insert(0, src_path)
from api_hook_client import ApiHookClient
def run_repro():
client = ApiHookClient("http://127.0.0.1:8999")
if not client.wait_for_server(timeout=15):
print("Failed to connect to GUI Hook Server.")
return
print("[REPRO] Connected to GUI.")
# 1. Reset and Setup
client.click("btn_reset")
time.sleep(1)
client.set_value("auto_add_history", True)
client.set_value("manual_approve", False) # Auto-approve for simulation
client.set_value("current_provider", "gemini_cli")
mock_script = os.path.abspath("tests/mock_gemini_cli.py")
client.set_value("gcli_path", f'"{sys.executable}" "{mock_script}"')
# 2. Trigger Chat
msg = "What is the current date and time? Answer in one sentence."
print(f"[REPRO] Sending message: {msg}")
client.set_value("ai_input", msg)
client.click("btn_gen_send")
# 3. Wait and Monitor
start_time = time.time()
while time.time() - start_time < 30:
status = client.get_value("ai_status")
print(f"[REPRO] Status: {status}")
if status == "error":
print("[REPRO] DETECTED ERROR STATUS!")
# Try to get more info if possible
break
if status == "done":
print("[REPRO] Success! Status is done.")
break
# Check events
events = client.get_events()
for ev in events:
print(f"[REPRO] Received Event: {ev.get('type')}")
time.sleep(1)
# 4. Check Session
session = client.get_session()
entries = session.get('session', {}).get('entries', [])
print(f"[REPRO] History Entries: {len(entries)}")
for i, entry in enumerate(entries):
print(f" {i}: [{entry.get('role')}] {entry.get('content')[:100]}...")
if __name__ == "__main__":
run_repro()

View File

@@ -103,9 +103,10 @@ class WorkflowSimulator:
print(f"\n[DEBUG] {elapsed:.1f}s - status: '{status}', roles: {roles}") print(f"\n[DEBUG] {elapsed:.1f}s - status: '{status}', roles: {roles}")
last_debug_time = elapsed last_debug_time = elapsed
if "error" in status: if "error" in status:
print(f"\n[ABORT] GUI reported error status: {status}") resp = self.client.get_value("ai_response")
return last_ai_entry if last_ai_entry else {"role": "AI", "content": f"ERROR: {status}"} print(f"\n[ABORT] GUI reported error status: {status} | AI Response: {resp}")
return last_ai_entry if last_ai_entry else {"role": "AI", "content": f"ERROR: {status}"}
# Turn completion logic: # Turn completion logic:
# 1. Transition: we were busy and now we are not, and the last role is AI. # 1. Transition: we were busy and now we are not, and the last role is AI.

View File

@@ -469,18 +469,7 @@ class AppController:
asyncio.set_event_loop(self._loop) asyncio.set_event_loop(self._loop)
self._loop.create_task(self._process_event_queue()) self._loop.create_task(self._process_event_queue())
# Fallback: process queues even if GUI thread is idling/stuck pass # Loop runs the process_event_queue task
async def queue_fallback() -> None:
while True:
try:
# Headless/fallback queue processing
# Note: In GUI mode, App._gui_func still calls these for low latency.
self._process_pending_gui_tasks()
# _process_pending_history_adds might need more care regarding project state
except: pass
await asyncio.sleep(0.1)
self._loop.create_task(queue_fallback())
self._loop.run_forever() self._loop.run_forever()
async def _process_event_queue(self) -> None: async def _process_event_queue(self) -> None:
@@ -671,100 +660,6 @@ class AppController:
return True return True
return False return False
def _process_pending_gui_tasks(self) -> None:
if not self._pending_gui_tasks:
return
with self._pending_gui_tasks_lock:
tasks = self._pending_gui_tasks[:]
self._pending_gui_tasks.clear()
for task in tasks:
try:
action = task.get("action")
if action == "refresh_api_metrics":
self._refresh_api_metrics(task.get("payload", {}))
elif action == "handle_ai_response":
payload = task.get("payload", {})
text = payload.get("text", "")
stream_id = payload.get("stream_id")
is_streaming = payload.get("status") == "streaming..."
if stream_id:
if is_streaming:
if stream_id not in self.mma_streams: self.mma_streams[stream_id] = ""
self.mma_streams[stream_id] += text
else:
self.mma_streams[stream_id] = text
if stream_id == "Tier 1":
if "status" in payload:
self.ai_status = payload["status"]
else:
if is_streaming:
self.ai_response += text
else:
self.ai_response = text
self.ai_status = payload.get("status", "done")
self._trigger_blink = True
if not stream_id:
self._token_stats_dirty = True
elif action == "mma_stream_append":
payload = task.get("payload", {})
stream_id = payload.get("stream_id")
text = payload.get("text", "")
if stream_id:
if stream_id not in self.mma_streams:
self.mma_streams[stream_id] = ""
self.mma_streams[stream_id] += text
elif action == "mma_state_update":
payload = task.get("payload", {})
self.mma_status = payload.get("status", "idle")
self.active_tier = payload.get("active_tier")
self.mma_tier_usage = payload.get("tier_usage", self.mma_tier_usage)
self.active_tickets = payload.get("tickets", [])
elif action == "mma_step_approval":
dlg = MMAApprovalDialog(str(task.get("ticket_id") or ""), str(task.get("payload") or ""))
self._pending_mma_approval = task
if "dialog_container" in task:
task["dialog_container"][0] = dlg
elif action == "mma_spawn_approval":
spawn_dlg = MMASpawnApprovalDialog(
str(task.get("ticket_id") or ""),
str(task.get("role") or ""),
str(task.get("prompt") or ""),
str(task.get("context_md") or "")
)
self._pending_mma_spawn = task
if "dialog_container" in task:
task["dialog_container"][0] = spawn_dlg
except Exception as e:
print(f"Error executing GUI task: {e}")
def stop_services(self):
"""Stops background threads and async event loop."""
if self._loop:
self._loop.call_soon_threadsafe(self._loop.stop)
if self._loop_thread:
self._loop_thread.join(timeout=2.0)
@property
def current_provider(self) -> str:
return self._current_provider
@current_provider.setter
def current_provider(self, value: str) -> None:
if value != self._current_provider:
self._current_provider = value
ai_client.reset_session()
ai_client.set_provider(value, self.current_model)
if value == "gemini_cli":
if not ai_client._gemini_cli_adapter:
ai_client._gemini_cli_adapter = ai_client.GeminiCliAdapter(binary_path=self.ui_gemini_cli_path)
else:
ai_client._gemini_cli_adapter.binary_path = self.ui_gemini_cli_path
if hasattr(self, 'hook_server'):
self.hook_server.start()
self.available_models = []
self._fetch_models(value)
self._token_stats = {}
self._token_stats_dirty = True
@property @property
def current_model(self) -> str: def current_model(self) -> str:

View File

@@ -15,6 +15,10 @@ _ENV_CONFIG: dict = {}
def _load_env_config() -> dict: def _load_env_config() -> dict:
"""Load mcp_env.toml from project root (sibling of this file or parent dir).""" """Load mcp_env.toml from project root (sibling of this file or parent dir)."""
env_path = os.environ.get("SLOP_MCP_ENV")
if env_path and Path(env_path).exists():
with open(env_path, "rb") as f:
return tomllib.load(f)
candidates = [ candidates = [
Path(__file__).parent / "mcp_env.toml", Path(__file__).parent / "mcp_env.toml",
Path(__file__).parent.parent / "mcp_env.toml", Path(__file__).parent.parent / "mcp_env.toml",

BIN
temp_conftest.py Normal file

Binary file not shown.

BIN
temp_conftest2.py Normal file

Binary file not shown.

BIN
temp_gui_old.py Normal file

Binary file not shown.

BIN
temp_gui_old2.py Normal file

Binary file not shown.

View File

@@ -189,6 +189,11 @@ def live_gui() -> Generator[tuple[subprocess.Popen, str], None, None]:
(temp_workspace / "manual_slop.toml").write_text("[project]\nname = 'TestProject'\n", encoding="utf-8") (temp_workspace / "manual_slop.toml").write_text("[project]\nname = 'TestProject'\n", encoding="utf-8")
(temp_workspace / "conductor" / "tracks").mkdir(parents=True, exist_ok=True) (temp_workspace / "conductor" / "tracks").mkdir(parents=True, exist_ok=True)
# Resolve absolute paths for shared resources
project_root = Path(os.getcwd())
cred_file = project_root / "credentials.toml"
mcp_file = project_root / "mcp_env.toml"
# Preserve GUI layout for tests # Preserve GUI layout for tests
layout_file = Path("manualslop_layout.ini") layout_file = Path("manualslop_layout.ini")
if layout_file.exists(): if layout_file.exists():
@@ -209,7 +214,11 @@ def live_gui() -> Generator[tuple[subprocess.Popen, str], None, None]:
# Use environment variable to point to temp config if App supports it, # Use environment variable to point to temp config if App supports it,
# or just run from that CWD. # or just run from that CWD.
env = os.environ.copy() env = os.environ.copy()
env["PYTHONPATH"] = os.getcwd() env["PYTHONPATH"] = str(project_root.absolute())
if cred_file.exists():
env["SLOP_CREDENTIALS"] = str(cred_file.absolute())
if mcp_file.exists():
env["SLOP_MCP_ENV"] = str(mcp_file.absolute())
process = subprocess.Popen( process = subprocess.Popen(
["uv", "run", "python", "-u", gui_script, "--enable-test-hooks"], ["uv", "run", "python", "-u", gui_script, "--enable-test-hooks"],

View File

@@ -34,11 +34,11 @@ def test_get_indicator_state_integration(live_gui: Any) -> None:
def test_app_processes_new_actions() -> None: def test_app_processes_new_actions() -> None:
import gui_2 import gui_2
with patch('gui_2.load_config', return_value={}), \ with patch('src.models.load_config', return_value={}), \
patch('gui_2.PerformanceMonitor'), \ patch('gui_2.PerformanceMonitor'), \
patch('gui_2.session_logger'), \ patch('gui_2.session_logger'), \
patch.object(gui_2.App, '_prune_old_logs'), \ patch('src.app_controller.AppController._prune_old_logs'), \
patch.object(gui_2.App, '_load_active_project'): patch('src.app_controller.AppController._load_active_project'):
app = gui_2.App() app = gui_2.App()
# Test set_value via _pending_gui_tasks # Test set_value via _pending_gui_tasks
# First we need to register a settable field for testing if not present # First we need to register a settable field for testing if not present

View File

@@ -8,19 +8,19 @@ from events import EventEmitter
@pytest.fixture @pytest.fixture
def app_instance() -> Generator[type[App], None, None]: def app_instance() -> Generator[type[App], None, None]:
""" """
Fixture to create an instance of the gui_2.App class for testing. Fixture to create an instance of the gui_2.App class for testing.
It mocks functions that would render a window or block execution. It mocks functions that would render a window or block execution.
""" """
if not hasattr(ai_client, 'events') or ai_client.events is None: if not hasattr(ai_client, 'events') or ai_client.events is None:
ai_client.events = EventEmitter() ai_client.events = EventEmitter()
with ( with (
patch('gui_2.load_config', return_value={'ai': {}, 'projects': {}}), patch('src.models.load_config', return_value={'ai': {}, 'projects': {}}),
patch('gui_2.save_config'), patch('gui_2.save_config'),
patch('gui_2.project_manager'), patch('gui_2.project_manager'),
patch('gui_2.session_logger'), patch('gui_2.session_logger'),
patch('gui_2.immapp.run'), patch('gui_2.immapp.run'),
patch.object(App, '_load_active_project'), patch('src.app_controller.AppController._load_active_project'),
patch.object(App, '_fetch_models'), patch('src.app_controller.AppController._fetch_models'),
patch.object(App, '_load_fonts'), patch.object(App, '_load_fonts'),
patch.object(App, '_post_init') patch.object(App, '_post_init')
): ):
@@ -43,4 +43,4 @@ def test_app_subscribes_to_events(app_instance: type[App]) -> None:
for call in calls: for call in calls:
handler = call.args[1] handler = call.args[1]
assert hasattr(handler, '__self__') assert hasattr(handler, '__self__')
assert handler.__self__ is app assert handler.__self__ is app.controller

View File

@@ -9,7 +9,7 @@ sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "s
def test_gui_updates_on_event(app_instance: App) -> None: def test_gui_updates_on_event(app_instance: App) -> None:
app_instance.last_md = "mock_md" app_instance.last_md = "mock_md"
with patch.object(app_instance, '_refresh_api_metrics') as mock_refresh: with patch.object(app_instance.controller, '_refresh_api_metrics') as mock_refresh:
# Simulate event (bypassing events.emit since _init_ai_and_hooks is mocked) # Simulate event (bypassing events.emit since _init_ai_and_hooks is mocked)
app_instance._on_api_event(payload={"text": "test"}) app_instance._on_api_event(payload={"text": "test"})
# Process tasks manually # Process tasks manually

View File

@@ -9,10 +9,15 @@ from fastapi.testclient import TestClient
class TestHeadlessAPI(unittest.TestCase): class TestHeadlessAPI(unittest.TestCase):
def setUp(self) -> None: def setUp(self) -> None:
with patch('gui_2.session_logger.open_session'), \ with patch('src.models.load_config', return_value={'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {}, 'gui': {'show_windows': {}}}), \
patch('gui_2.session_logger.open_session'), \
patch('gui_2.ai_client.set_provider'), \ patch('gui_2.ai_client.set_provider'), \
patch('gui_2.PerformanceMonitor'), \ patch('gui_2.PerformanceMonitor'), \
patch('gui_2.session_logger.close_session'): patch('gui_2.session_logger.close_session'), \
patch('src.app_controller.AppController._init_ai_and_hooks'), \
patch('src.app_controller.AppController._fetch_models'), \
patch('src.app_controller.AppController._prune_old_logs'), \
patch('src.app_controller.AppController.start_services'):
self.app_instance = gui_2.App() self.app_instance = gui_2.App()
# Set a default API key for tests # Set a default API key for tests
self.test_api_key = "test-secret-key" self.test_api_key = "test-secret-key"

View File

@@ -41,7 +41,11 @@ def test_old_windows_removed_from_gui2(app_instance_simple: Any) -> None:
def app_instance_simple() -> Any: def app_instance_simple() -> Any:
from unittest.mock import patch from unittest.mock import patch
from gui_2 import App from gui_2 import App
with patch('gui_2.load_config', return_value={}): with patch('src.models.load_config', return_value={'ai': {}, 'projects': {}, 'gui': {'show_windows': {}}}), \
patch('src.app_controller.AppController._init_ai_and_hooks'), \
patch('src.app_controller.AppController._fetch_models'), \
patch('src.app_controller.AppController._prune_old_logs'), \
patch('src.app_controller.AppController.start_services'):
app = App() app = App()
return app return app

View File

@@ -7,15 +7,18 @@ from gui_2 import App
def app_instance() -> Any: def app_instance() -> Any:
# We patch the dependencies of App.__init__ to avoid side effects # We patch the dependencies of App.__init__ to avoid side effects
with ( with (
patch('gui_2.load_config', return_value={'ai': {}, 'projects': {}}), patch('src.models.load_config', return_value={'ai': {}, 'projects': {}}),
patch('gui_2.save_config'), patch('gui_2.save_config'),
patch('gui_2.project_manager') as mock_pm, patch('gui_2.project_manager') as mock_pm,
patch('gui_2.session_logger'), patch('gui_2.session_logger'),
patch('gui_2.immapp.run'), patch('gui_2.immapp.run'),
patch.object(App, '_load_active_project'), patch('src.app_controller.AppController._load_active_project'),
patch.object(App, '_fetch_models'), patch('src.app_controller.AppController._fetch_models'),
patch.object(App, '_load_fonts'), patch.object(App, '_load_fonts'),
patch.object(App, '_post_init') patch.object(App, '_post_init'),
patch('src.app_controller.AppController._prune_old_logs'),
patch('src.app_controller.AppController.start_services'),
patch('src.app_controller.AppController._init_ai_and_hooks')
): ):
app = App() app = App()
# Ensure project and ui_files_base_dir are set for _refresh_from_project # Ensure project and ui_files_base_dir are set for _refresh_from_project

View File

@@ -7,15 +7,18 @@ from gui_2 import App
@pytest.fixture @pytest.fixture
def app_instance() -> Generator[App, None, None]: def app_instance() -> Generator[App, None, None]:
with ( with (
patch('gui_2.load_config', return_value={'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {}}), patch('src.models.load_config', return_value={'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, 'projects': {}}),
patch('gui_2.save_config'), patch('gui_2.save_config'),
patch('gui_2.project_manager'), patch('gui_2.project_manager'),
patch('gui_2.session_logger'), patch('gui_2.session_logger'),
patch('gui_2.immapp.run'), patch('gui_2.immapp.run'),
patch.object(App, '_load_active_project'), patch('src.app_controller.AppController._load_active_project'),
patch.object(App, '_fetch_models'), patch('src.app_controller.AppController._fetch_models'),
patch.object(App, '_load_fonts'), patch.object(App, '_load_fonts'),
patch.object(App, '_post_init'), patch.object(App, '_post_init'),
patch('src.app_controller.AppController._prune_old_logs'),
patch('src.app_controller.AppController.start_services'),
patch('src.app_controller.AppController._init_ai_and_hooks'),
patch('ai_client.set_provider'), patch('ai_client.set_provider'),
patch('ai_client.reset_session') patch('ai_client.reset_session')
): ):