ed/manual_slop
Private
Public Access
0
0
Fork 0
Code Activity

refactor(app_controller): remove redundant hasattr(f, ...) defensive checks

Browse Source 
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.
This commit is contained in:
Edward R. Gonzalez
2026-06-26 04:35:49 -04:00
parent 75eb6dbbbb
commit 0d0b433a2e
2 changed files with 17 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
conductor/tracks/cruft_elimination_20260627/plan.md
+2
View File
@@ -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):**
src/app_controller.py
+15 -14
View File
@@ -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