fix: Robustness improvements for RAG tests and GUI stability

- Added import sys to src/api_hook_client.py. - Fixed App.__getattr__ to use direct attribute access on controller to avoid recursion. - Simplified _get_app_attr and _has_app_attr in src/api_hooks.py. - Centralized RAG and symbol enrichment in AppController._handle_request_event. - Updated ests/test_symbol_parsing.py to match the new enrichment flow. - Removed redundant task appending from i_status and mma_status setters. - Improved _sync_rag_engine to only set 'ready' status after indexing is confirmed. - Updated est_status_encapsulation.py to reflect setter changes.
2026-05-15 17:17:05 -04:00
parent a2a6d4cb65
commit 7f2f9c1989
8 changed files with 130 additions and 91 deletions
@@ -63,11 +63,16 @@ class ApiHookClient:
    response = requests.post(url, json=data, headers=headers, timeout=timeout)
   elif method == 'DELETE':
    response = requests.delete(url, headers=headers, timeout=timeout)
+   
   if response.status_code == 200:
    return response.json()
+   else:
+    sys.stderr.write(f"[DEBUG Client] Request failed: {method} {path} - status={response.status_code}\n")
+    sys.stderr.flush()
   return None
-  except Exception:
-  # Silently ignore connection errors unless we are in a wait loop
+  except Exception as e:
+   sys.stderr.write(f"[DEBUG Client] Request error: {method} {path} - {e}\n")
+   sys.stderr.flush()
   return None

 def wait_for_server(self, timeout: int = 15) -> bool:
@@ -43,28 +43,19 @@ See Also:

 def _get_app_attr(app: Any, name: str, default: Any = None) -> Any:
 """Retrieves an attribute from the App or its Controller."""
- if hasattr(app, name):
+ try:
  val = getattr(app, name)
  return val
- if hasattr(app, 'controller') and hasattr(app.controller, name):
-  val = getattr(app.controller, name)
-  return val
- return default
+ except AttributeError:
+  return default

 def _has_app_attr(app: Any, name: str) -> bool:
 """Checks if an attribute exists on the App or its Controller."""
- if hasattr(app, name): return True
- if hasattr(app, 'controller') and hasattr(app.controller, name): return True
- return False
+ return hasattr(app, name)

 def _set_app_attr(app: Any, name: str, value: Any) -> None:
 """Sets an attribute on the App or its Controller."""
- if hasattr(app, name):
-  setattr(app, name, value)
- elif hasattr(app, 'controller'):
-  setattr(app.controller, name, value)
- else:
-  setattr(app, name, value)
+ setattr(app, name, value)

 class HookServerInstance(ThreadingHTTPServer):
 """Custom HTTPServer that carries a reference to the main App instance."""
@@ -93,6 +84,7 @@ class HookHandler(BaseHTTPRequestHandler):
  """Handles GET requests by routing to the appropriate state provider."""
  try:
   app = self.server.app
+   print(f'[HOOKS] GET {self.path}')
   session_logger.log_api_hook("GET", self.path, "")
   if self.path == "/status":
    self.send_response(200)
@@ -152,9 +144,10 @@ class HookHandler(BaseHTTPRequestHandler):
      combined = {**settable, **gettable}
      if field_tag in combined:
       attr = combined[field_tag]
-       result["value"] = _get_app_attr(app, attr, None)
+       val = _get_app_attr(app, attr, None)
+       result["value"] = _serialize_for_api(val)
      else:
-       sys.stderr.write(f"[DEBUG] Hook API: field {field_tag} not found in settable or gettable\n")
+       sys.stderr.write(f"Hook API: field {field_tag} not found in settable or gettable\n")
       sys.stderr.flush()
     finally: event.set()
    lock = _get_app_attr(app, "_pending_gui_tasks_lock")
@@ -982,6 +982,7 @@ class AppController:
   'rag_mcp_tool': 'rag_mcp_tool',
   'rag_chunk_size': 'rag_chunk_size',
   'rag_chunk_overlap': 'rag_chunk_overlap',
+   'rag_collection_name': 'rag_collection_name',
   'mcp_config_json': 'mcp_config_json',
   'mma_active_tier': 'active_tier',
   'ui_new_track_name': 'ui_new_track_name',
@@ -1028,7 +1029,7 @@ class AppController:
  }
  self._gettable_fields = dict(self._settable_fields)
  self._gettable_fields.update({
-   'show_windows': 'show_windows',
+   'show_windows': 'show_windows', # Key 'show_windows' maps to field 'show_windows' on controller
   'ui_focus_agent': 'ui_focus_agent',
   'active_discussion': 'active_discussion',
   '_track_discussion_active': '_track_discussion_active',
@@ -1164,8 +1165,6 @@ class AppController:
 @ai_status.setter
 def ai_status(self, value: str) -> None:
  self._ai_status = value
-  with self._pending_gui_tasks_lock:
-   self._pending_gui_tasks.append({"action": "set_ai_status", "value": value})

 @property
 def mma_status(self) -> str:
@@ -1174,8 +1173,6 @@ class AppController:
 @mma_status.setter
 def mma_status(self, value: str) -> None:
  self._mma_status = value
-  with self._pending_gui_tasks_lock:
-   self._pending_gui_tasks.append({"action": "set_mma_status", "value": value})

 @property
 def thinking_indicator(self) -> bool:
@@ -1200,10 +1197,11 @@ class AppController:
    engine = rag_engine.RAGEngine(self.rag_config, self.active_project_root)
    with self._rag_engine_lock:
     self.rag_engine = engine
-    self._set_rag_status("ready")
    # If the engine is empty and we have files, trigger indexing
    if self.rag_engine and self.rag_engine.is_empty() and self.files:
     self._rebuild_rag_index()
+    else:
+     self._set_rag_status("ready")
   except Exception as e:
    self._set_rag_status(f"error: {e}")
    sys.stderr.write(f"[DEBUG RAG] Failed to sync engine: {e}\n")
@@ -1266,6 +1264,15 @@ class AppController:
 def rag_mcp_tool(self, value: str) -> None:
  if self.rag_config: self.rag_config.vector_store.mcp_tool = value

+ @property
+ def rag_collection_name(self) -> str:
+  return self.rag_config.vector_store.collection_name if self.rag_config else "manual_slop"
+ @rag_collection_name.setter
+ def rag_collection_name(self, value: str) -> None:
+  if self.rag_config:
+   self.rag_config.vector_store.collection_name = value
+   self._sync_rag_engine()
+
 @property
 def mcp_config_json(self) -> str:
  return json.dumps(self.mcp_config.to_dict()) if self.mcp_config else "{}"
@@ -1576,7 +1583,7 @@ class AppController:
  self.active_discussion = disc_sec.get("active", "main")
  disc_data = disc_sec.get("discussions", {}).get(self.active_discussion, {})
  with self._disc_entries_lock:
-   self.disc_entries = models.parse_history_entries(disc_data.get("history", []), self.disc_roles)
+   self.disc_entries[:] = models.parse_history_entries(disc_data.get("history", []), self.disc_roles)
  # UI state
  self.ui_output_dir = self.project.get("output", {}).get("output_dir", "./md_gen")
  self.ui_files_base_dir = self.project.get("files", {}).get("base_dir", ".")
@@ -1621,7 +1628,7 @@ class AppController:
  self.rag_engine = None
  if self.rag_config.enabled:
   self._sync_rag_engine()
-
+  
  from src.personas import PersonaManager
  self.persona_manager = PersonaManager(Path(self.active_project_path).parent if self.active_project_path else None)
  self.personas = self.persona_manager.load_all()
@@ -2115,6 +2122,45 @@ class AppController:
    [C: tests/test_live_gui_integration_v2.py:test_user_request_error_handling, tests/test_live_gui_integration_v2.py:test_user_request_integration_flow, tests/test_rag_integration.py:test_rag_integration]
  """
  self.ai_status = 'sending...'
+  
+  user_msg = event.prompt
+  
+  # 1. RAG Retrieval (Enrich prompt before logging to history)
+  if self.rag_engine and self.rag_config and self.rag_config.enabled:
+   try:
+    chunks = self.rag_engine.search(user_msg)
+    if chunks:
+     context_block = "## Retrieved Context\n\n"
+     for i, chunk in enumerate(chunks):
+      path = chunk.get("metadata", {}).get("path", "unknown")
+      context_block += f"### Chunk {i+1} (Source: {path})\n{chunk.get('document', '')}\n\n"
+     user_msg = context_block + user_msg
+   except Exception as e:
+    sys.stderr.write(f"RAG search error: {e}\n")
+    sys.stderr.flush()
+
+  # 2. Symbol Resolution (Enrich prompt before logging to history)
+  try:
+   symbols = parse_symbols(user_msg)
+   file_paths = [f['path'] for f in event.file_items]
+   for symbol in symbols:
+    res = get_symbol_definition(symbol, file_paths)
+    if res:
+     file_path, definition, line = res
+     user_msg += f'\n\n[Definition: {symbol} from {file_path} (line {line})]\n```python\n{definition}\n```'
+  except Exception as e:
+   sys.stderr.write(f"Symbol resolution error: {e}\n")
+   sys.stderr.flush()
+
+  # 3. Log the final enriched prompt to history
+  self.event_queue.put("comms", {
+   "kind": "request",
+   "payload": {
+    "message": user_msg,
+    "collapsed": False
+   }
+  })
+
  ai_client.set_current_tier(None) # Ensure main discussion is untagged
  # Clear response area for new turn
  self.ai_response = ""
@@ -2131,7 +2177,7 @@ class AppController:
  try:
   resp = ai_client.send(
    event.stable_md, 
-    event.prompt, 
+    user_msg, 
    event.base_dir, 
    event.file_items, 
    event.disc_text,
@@ -2140,7 +2186,7 @@ class AppController:
    pre_tool_callback=self._confirm_and_run,
    qa_callback=ai_client.run_tier4_analysis,
    patch_callback=ai_client.run_tier4_patch_callback,
-    rag_engine=self.rag_engine
+    rag_engine=None # Already handled above
   )
   self.event_queue.put("response", {"text": resp, "status": "done", "role": "AI"})
  except ai_client.ProviderError as e:
@@ -2621,7 +2667,7 @@ class AppController:
  self.active_discussion = disc_sec.get("active", "main")
  disc_data = disc_sec.get("discussions", {}).get(self.active_discussion, {})
  with self._disc_entries_lock:
-   self.disc_entries = models.parse_history_entries(disc_data.get("history", []), self.disc_roles)
+   self.disc_entries[:] = models.parse_history_entries(disc_data.get("history", []), self.disc_roles)
  proj = self.project
  self.ui_output_dir = proj.get("output", {}).get("output_dir", "./md_gen")
  self.ui_files_base_dir = proj.get("files", {}).get("base_dir", ".")
@@ -2671,7 +2717,7 @@ class AppController:
   track_history = project_manager.load_track_history(self.active_track.id, self.active_project_root)
   if track_history:
    with self._disc_entries_lock:
-     self.disc_entries = models.parse_history_entries(track_history, self.disc_roles)
+     self.disc_entries[:] = models.parse_history_entries(track_history, self.disc_roles)
  self.preset_manager.project_root = Path(self.active_project_root)
  self.presets = self.preset_manager.load_all()
  self.tool_preset_manager.project_root = Path(self.active_project_root)
@@ -2855,9 +2901,9 @@ class AppController:
    history = project_manager.load_track_history(track_id, self.active_project_root)
    with self._disc_entries_lock:
     if history:
-      self.disc_entries = models.parse_history_entries(history, self.disc_roles)
+      self.disc_entries[:] = models.parse_history_entries(history, self.disc_roles)
     else:
-      self.disc_entries = []
+      self.disc_entries.clear()
    self._recalculate_session_usage()
    self.ai_status = f"Loaded track: {state.metadata.name}"
   except Exception as e:
@@ -2898,7 +2944,7 @@ class AppController:
  disc_sec["active"] = name
  disc_data = discussions[name]
  with self._disc_entries_lock:
-   self.disc_entries = models.parse_history_entries(disc_data.get("history", []), self.disc_roles)
+   self.disc_entries[:] = models.parse_history_entries(disc_data.get("history", []), self.disc_roles)
  if "context_snapshot" in disc_data:
   snapshot_data = disc_data["context_snapshot"]
   self.context_files = [models.FileItem.from_dict(f) if isinstance(f, dict) else models.FileItem(path=str(f)) for f in snapshot_data]
@@ -3154,24 +3200,6 @@ class AppController:
    self.ai_status = "sending..."
    user_msg = self.ui_ai_input

-    # RAG Retrieval
-    if self.rag_engine and self.rag_config and self.rag_config.enabled:
-     chunks = self.rag_engine.search(user_msg)
-     if chunks:
-      context_block = "## Retrieved Context\n\n"
-      for i, chunk in enumerate(chunks):
-       path = chunk.get("metadata", {}).get("path", "unknown")
-       context_block += f"### Chunk {i+1} (Source: {path})\n{chunk.get('document', '')}\n\n"
-      user_msg = context_block + user_msg
-
-    symbols = parse_symbols(user_msg)
-    file_paths = [f['path'] for f in file_items]
-    for symbol in symbols:
-     res = get_symbol_definition(symbol, file_paths)
-     if res:
-      file_path, definition, line = res
-      user_msg += f'\n\n[Definition: {symbol} from {file_path} (line {line})]\n```python\n{definition}\n```'
-
    base_dir = self.active_project_root
    # Prepare event payload
    event_payload = events.UserRequestEvent(
@@ -118,6 +118,8 @@ class App:
  # --- Initialization ---
  self.controller.init_state()
  self.workspace_manager = workspace_manager.WorkspaceManager(project_root=self.controller.active_project_root)
+  self.disc_entries = self.controller.disc_entries
+  self.disc_roles = self.controller.disc_roles
  self.workspace_profiles = self.workspace_manager.load_all_profiles()
  self.controller.start_services(self)
  # --- Controller Callbacks & Actions ---
@@ -379,13 +381,19 @@ class App:
  if name == 'controller':
   raise AttributeError(name)
  try:
-   # Use object.__getattribute__ to avoid recursion if 'controller' isn't initialized yet
   ctrl = object.__getattribute__(self, 'controller')
  except AttributeError:
-   raise AttributeError(f"'{type(self).__name__}' object has no attribute '{name}'")
+   raise AttributeError(name)
+
+  if ctrl is not None:
+   try:
+    val = getattr(ctrl, name)
+    sys.stderr.write(f"[DEBUG __getattr__] name={name}, val_type={type(val).__name__}\n")
+    sys.stderr.flush()
+    return val
+   except AttributeError:
+    pass

-  if ctrl is not None and hasattr(ctrl, name):
-   return getattr(ctrl, name)
  raise AttributeError(f"'{type(self).__name__}' object has no attribute '{name}'")

 def __setattr__(self, name: str, value: Any) -> None:
@@ -26,6 +26,7 @@ def test_phase4_final_verify(live_gui):
 
 try:
  # 2. Configure project through Hook API
+  client.set_value('rag_collection_name', 'test_final_verify')
  client.set_value('files', ['final_test_1.txt', 'final_test_2.py'])
  client.set_value('rag_enabled', True)
  client.set_value('rag_source', 'chroma')
@@ -27,6 +27,7 @@ def test_rag_large_codebase_verification_sim(live_gui):

 try:
  # 2. Configure project through Hook API
+  client.set_value('rag_collection_name', 'test_stress')
  client.set_value('files', file_names)
  client.set_value('rag_enabled', True)
  client.set_value('rag_source', 'chroma')
@@ -97,14 +98,18 @@ def test_rag_large_codebase_verification_sim(live_gui):
  
  # Wait for completion
  success = False
+  status = "unknown"
  for _ in range(50):
   state = client.get_gui_state()
-   if state.get('ai_status') == 'done':
+   status = state.get('ai_status', 'unknown')
+   if status == 'done':
    success = True
    break
+   if "error" in status.lower():
+    pytest.fail(f"AI request failed with error: {status}")
   time.sleep(0.5)
  
-  assert success, "AI request timed out"
+  assert success, f"AI request timed out. Final status: {status}"
  
  # Verify retrieved context in discussion
  session = client.get_session()
@@ -11,7 +11,8 @@ def test_status_properties():
 controller = app_controller.AppController()
 controller.ai_status = 'busy'
 assert controller._ai_status == 'busy'
- assert any(t.get('action') == 'set_ai_status' and t.get('value') == 'busy' for t in controller._pending_gui_tasks)
+ # No longer using tasks for simple status updates as ImGui reads them directly
+ assert not any(t.get('action') == 'set_ai_status' for t in controller._pending_gui_tasks)
 controller.mma_status = 'active'
 assert controller._mma_status == 'active'
- assert any(t.get('action') == 'set_mma_status' and t.get('value') == 'active' for t in controller._pending_gui_tasks)
+ assert not any(t.get('action') == 'set_mma_status' for t in controller._pending_gui_tasks)
@@ -3,6 +3,7 @@ from unittest.mock import MagicMock, patch
 from pathlib import Path
 from src.app_controller import AppController
 from src.events import UserRequestEvent
+from src import events

@pytest.fixture
 def controller():
@@ -26,60 +27,57 @@ def controller():
  c.event_queue = MagicMock()
  return c

-def test_handle_generate_send_appends_definitions(controller):
+def test_handle_request_event_appends_definitions(controller):
 # Setup
 file_items = [{"path": "src/models.py", "entry": "src/models.py"}]
- controller._do_generate = MagicMock(return_value=(
-  "full_md", Path("output.md"), file_items, "stable_md", "disc_text"
- ))
- controller.ui_ai_input = "Explain @Track object"
+ event = UserRequestEvent(
+  prompt="Explain @Track object",
+  stable_md="stable_md",
+  file_items=file_items,
+  disc_text="disc_text",
+  base_dir="."
+ )
 
 # Mock symbol helpers
 with (
  patch('src.app_controller.parse_symbols', return_value=["Track"]) as mock_parse,
   patch('src.app_controller.get_symbol_definition', return_value=("src/models.py", "class Track: pass", 42)) as mock_get_def,
-  patch('threading.Thread') as mock_thread
+  patch('src.ai_client.send') as mock_send
 ):
  # Execute
-  controller._handle_generate_send()
-  
-  # Run worker manually
-  worker = mock_thread.call_args[1]['target']
-  worker()
+  controller._handle_request_event(event)
  
  # Verify
  mock_parse.assert_called_once_with("Explain @Track object")
  mock_get_def.assert_called_once()
  
-  controller.event_queue.put.assert_called_once()
-  event_name, event_payload = controller.event_queue.put.call_args[0]
-  assert event_name == "user_request"
-  assert isinstance(event_payload, UserRequestEvent)
-  
-  # Check if definition was appended
+  # Check if enriched prompt was sent to AI
  expected_suffix = "\n\n[Definition: Track from src/models.py (line 42)]\n```python\nclass Track: pass\n```"
-  assert event_payload.prompt == "Explain @Track object" + expected_suffix
+  mock_send.assert_called_once()
+  args, kwargs = mock_send.call_args
+  sent_prompt = args[1]
+  assert sent_prompt == "Explain @Track object" + expected_suffix

-def test_handle_generate_send_no_symbols(controller):
+def test_handle_request_event_no_symbols(controller):
 # Setup
 file_items = [{"path": "src/models.py", "entry": "src/models.py"}]
- controller._do_generate = MagicMock(return_value=(
-  "full_md", Path("output.md"), file_items, "stable_md", "disc_text"
- ))
- controller.ui_ai_input = "Just a normal prompt"
+ event = UserRequestEvent(
+  prompt="Just a normal prompt",
+  stable_md="stable_md",
+  file_items=file_items,
+  disc_text="disc_text",
+  base_dir="."
+ )
 
 with (
  patch('src.app_controller.parse_symbols', return_value=[]) as mock_parse,
-  patch('threading.Thread') as mock_thread
+  patch('src.ai_client.send') as mock_send
 ):
  # Execute
-  controller._handle_generate_send()
-  
-  # Run worker manually
-  worker = mock_thread.call_args[1]['target']
-  worker()
+  controller._handle_request_event(event)
  
  # Verify
-  controller.event_queue.put.assert_called_once()
-  _, event_payload = controller.event_queue.put.call_args[0]
-  assert event_payload.prompt == "Just a normal prompt"
+  mock_send.assert_called_once()
+  args, kwargs = mock_send.call_args
+  sent_prompt = args[1]
+  assert sent_prompt == "Just a normal prompt"