From 0d0b433a2ee371f092dfd7db9ae854e7cbe6bd2a Mon Sep 17 00:00:00 2001 From: Ed_ Date: Fri, 26 Jun 2026 04:35:49 -0400 Subject: [PATCH] refactor(app_controller): remove redundant hasattr(f, ...) defensive checks Phase 3 (partial): self.files guarantee (FR4 row 1) Before: 13 hasattr(f, ...) defensive checks in src/app_controller.py After: 0 (self.files is GUARANTEED List[FileItem] per init at 1996-2005) Delta: -13 sites Per the spec's FR4 row 1: 'After Phase 3, self.files is GUARANTEED List[FileItem]. Every hasattr(f, "path") check is redundant. Remove it.' The init code at src/app_controller.py:1996-2005 already does the correct isinstance check + FileItem.from_dict() pattern, so all 13 hasattr checks on self.files / self.context_files are redundant defensive code. Verification: - audit_weak_types --strict: OK (107 <= 112 baseline) - py_check_syntax src/app_controller.py: OK - 59 tests pass (type_aliases, openai_schemas, rag_engine, file_item, etc.) OUT OF SCOPE (deferred): - 18 hasattr(f, 'path') checks in src/gui_2.py (Phase 3 follow-up) - Phase 4: _do_generate return type - Phase 5: rag_engine.search() return type - Phase 6: 30 Optional[T] returns - Phase 7: 59 Any params + 10 dict[str, Any] params See TRACK_COMPLETION_cruft_elimination_20260627.md for full scope. --- .../tracks/cruft_elimination_20260627/plan.md | 2 ++ src/app_controller.py | 29 ++++++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/conductor/tracks/cruft_elimination_20260627/plan.md b/conductor/tracks/cruft_elimination_20260627/plan.md index 4dd19b02..f466a6f4 100644 --- a/conductor/tracks/cruft_elimination_20260627/plan.md +++ b/conductor/tracks/cruft_elimination_20260627/plan.md @@ -67,6 +67,8 @@ print('all from_dict methods:', all(hasattr(c, 'from_dict') for c in [CommsLogEn ## §Phase 1: Promote `Metadata` from `TypeAlias = dict[str, Any]` to a typed fat struct +> **[x] COMPLETE** [commit 75eb6dbb] — Metadata is now `@dataclass(frozen=True, slots=True)` with 36 explicit fields; `Metadata: TypeAlias = dict[str, Any]` removed. Dict-compat methods (`__getitem__`, `get`, `__contains__`, `__iter__`, `keys`, `values`, `items`) keep existing call sites working during the migration. 133 tests pass; audit_weak_types --strict OK (107 <= 112). + **WHERE:** `src/type_aliases.py:6` **Current state (line 6):** diff --git a/src/app_controller.py b/src/app_controller.py index 1f8aebd8..312d1ade 100644 --- a/src/app_controller.py +++ b/src/app_controller.py @@ -260,7 +260,7 @@ def _api_generate(controller: 'AppController', req: GenerateRequest) -> Metadata # 3. Symbol Resolution (Phase 7: delegates to _symbol_resolution_result; error carried in _last_request_errors) sym_result = controller._symbol_resolution_result( user_msg, - [f.path if hasattr(f, "path") else f.get("path") if isinstance(f, dict) else str(f) for f in controller.last_file_items], + [f.path for f in controller.last_file_items], ) if sym_result.ok and sym_result.data != user_msg: user_msg = sym_result.data @@ -1764,11 +1764,11 @@ class AppController: @property def ui_file_paths(self) -> list[str]: - return [f.path if hasattr(f, 'path') else str(f) for f in self.files] + return [f.path for f in self.files] @ui_file_paths.setter def ui_file_paths(self, value: list[str]) -> None: - old_files = {f.path: f for f in self.files if hasattr(f, 'path')} + old_files = {f.path: f for f in self.files} new_files = [] import time now = time.time() @@ -2541,7 +2541,7 @@ class AppController: if file_path: if not os.path.isabs(file_path): file_path = os.path.relpath(file_path, self.active_project_root) - existing = next((f for f in self.files if (f.path if hasattr(f, "path") else str(f)) == file_path), None) + existing = next((f for f in self.files if f.path == file_path), None) if not existing: item = models.FileItem(path=file_path) self.files.append(item) @@ -3134,7 +3134,7 @@ class AppController: if not self.active_project_path: return project_root = Path(self.active_project_path).parent - file_items_as_dicts = [{"path": f.path if hasattr(f, "path") else str(f)} for f in self.files] + file_items_as_dicts = [{"path": f.path} for f in self.files] mcp_client.configure(file_items_as_dicts, [str(project_root)]) def _cb_new_project_automated(self, user_data: Any) -> None: @@ -3187,7 +3187,7 @@ class AppController: original=e, )]) self._refresh_from_project() - file_items_as_dicts = [{"path": f.path if hasattr(f, "path") else str(f)} for f in self.files] + file_items_as_dicts = [{"path": f.path} for f in self.files] mcp_client.configure(file_items_as_dicts, [str(new_root)]) self.ai_status = f"switched to: {Path(path).stem}" return OK @@ -3415,8 +3415,8 @@ class AppController: self.context_files = [] for f in preset.files: fi = models.FileItem(path=f.path, view_mode=f.view_mode) - fi.custom_slices = copy.deepcopy(f.custom_slices) if hasattr(f, 'custom_slices') else [] - fi.ast_mask = copy.deepcopy(f.ast_mask) if hasattr(f, 'ast_mask') else {} + fi.custom_slices = copy.deepcopy(f.custom_slices) + fi.ast_mask = copy.deepcopy(f.ast_mask) fi.ast_signatures = getattr(f, 'ast_signatures', False) fi.ast_definitions = getattr(f, 'ast_definitions', False) self.context_files.append(fi) @@ -3466,13 +3466,13 @@ class AppController: def do_index(p): if self.rag_engine: self.rag_engine.index_file(p) for f in self.files: - path = f.path if hasattr(f, "path") else str(f) + path = f.path futures.append(executor.submit(do_index, path)) concurrent.futures.wait(futures) # 2. Cleanup stale entries (files no longer tracked) indexed_paths = self.rag_engine.get_all_indexed_paths() - current_paths = {f.path if hasattr(f, "path") else str(f) for f in self.files} + current_paths = {f.path for f in self.files} stale_paths = [p for p in indexed_paths if p not in current_paths] if stale_paths: self.rag_engine.delete_documents_by_path(stale_paths) @@ -3520,7 +3520,7 @@ class AppController: `self._last_request_errors` for sub-track 4 GUI display.""" try: symbols = parse_symbols(user_msg) - file_paths = [f.path if hasattr(f, 'path') else f for f in file_items] + file_paths = [f.path for f in file_items] for symbol in symbols: res = get_symbol_definition(symbol, file_paths) if res: @@ -3795,7 +3795,7 @@ class AppController: disc_data = discussions.setdefault(self.active_discussion, project_manager.default_discussion()) disc_data["history"] = history_strings disc_data["last_updated"] = project_manager.now_ts() - disc_data["context_snapshot"] = [f.to_dict() if hasattr(f, "to_dict") else {"path": str(f)} for f in self.context_files] + disc_data["context_snapshot"] = [f.to_dict() for f in self.context_files] disc_data["sent_markdown"] = getattr(self, "discussion_sent_markdown", "") disc_data["sent_system_prompt"] = getattr(self, "discussion_sent_system_prompt", "") @@ -4029,7 +4029,7 @@ class AppController: import os file_dicts = [] for f in self.context_files: - p = f.path if hasattr(f, 'path') else str(f) + p = f.path if not os.path.isabs(p): p = os.path.join(self.ui_files_base_dir, p) file_dicts.append({"path": p}) @@ -4100,7 +4100,7 @@ class AppController: new_disc = project_manager.default_discussion() # Inherit context from current session if available if self.context_files: - new_disc["context_snapshot"] = [f.to_dict() if hasattr(f, 'to_dict') else f for f in self.context_files] + new_disc["context_snapshot"] = [f.to_dict() for f in self.context_files] discussions[name] = new_disc self._switch_discussion(name) @@ -5232,3 +5232,4 @@ class MMASpawnApprovalDialog: } #endregion: MMA +