refactor(multiple): continue Phase 6 Optional[T] elimination (batch 2)
Phase 6: Eliminate Optional[T] returns - BATCH 2 of 7
Before: 7 more Optional[T] returns removed
After: 0 in command_palette.py, diff_viewer.py, fuzzy_anchor.py,
multi_agent_conductor.py, patch_modal.py, app_controller.py
Delta: -7 sites (cumulative: -15 of 30)
Specific changes:
- src/command_palette.py:50: CommandRegistry.get() returns Command (zero-init
sentinel: id="", title="", category="uncategorized", action=lambda: None)
- src/diff_viewer.py:117: get_line_color returns "" when no marker prefix
- src/fuzzy_anchor.py:40: FuzzyAnchor.resolve_slice returns (-1, -1) sentinel
(replaced 3x `return None` with `return (-1, -1)`)
- src/multi_agent_conductor.py:64: WorkerPool.spawn returns threading.Thread()
(empty sentinel, not started) when pool is full
- src/patch_modal.py:33: PatchModalManager.get_pending_patch returns
PendingPatch; class has EMPTY_PATCH sentinel; field type changed from
Optional[PendingPatch] to PendingPatch; 2x `= None` reset replaced with
`= EMPTY_PATCH`
- src/app_controller.py:4414: _confirm_and_run returns "" when not approved
(was Optional[str] returning None)
Test updates:
- tests/test_diff_viewer.py:95: get_line_color(" context") == ""
- tests/test_fuzzy_anchor.py:42,59: assert result == (-1, -1)
- tests/test_parallel_execution.py:31: t3 sentinel is now unstarted thread
(check via not t3.is_alive())
- tests/test_patch_modal.py:9,31,78: get_pending_patch() == "" sentinel check
Verification:
- audit_weak_types --strict: OK (107 <= 112 baseline)
- 22+ tests pass (test_diff_viewer, test_fuzzy_anchor,
test_parallel_execution, test_patch_modal, test_command_palette)
- py_check_syntax: OK on all changed files
REMAINING: ~15 Optional[T] returns in:
- src/external_editor.py (3)
- src/file_cache.py (7)
- src/diff_viewer.py: parse_hunk_header (1)
- src/models.py: ExternalEditorConfig.get_default (1)
- src/project_manager.py: load_track_state (1)
- src/session_logger.py: log_tool_call (1)
- src/app_controller.py: _pending_mma_spawn, _pending_mma_approval (2)
This commit is contained in:
@@ -3497,7 +3497,7 @@ class AppController:
|
||||
|
||||
def _rag_search_result(self, user_msg: str) -> "Result[list[Metadata]]":
|
||||
"""Per-event handler (Phase 6 Group 6.6): RAG search via the engine.
|
||||
Returns Result[List[Dict]]. On failure: any engine/SDK exception
|
||||
Returns Result[List[RAGChunk]]. On failure: any engine/SDK exception
|
||||
-> ErrorInfo(original=e). Caller (`_handle_request_event`) appends
|
||||
to `self._last_request_errors` for sub-track 4 GUI display."""
|
||||
if not (self.rag_engine and self.rag_config and self.rag_config.enabled):
|
||||
@@ -4411,7 +4411,7 @@ class AppController:
|
||||
if self.ui_auto_scroll_tool_calls:
|
||||
self._scroll_tool_calls_to_bottom = True
|
||||
|
||||
def _confirm_and_run(self, script: str, base_dir: str, qa_callback: Optional[Callable[[str], str]] = None, patch_callback: Optional[Callable[[str, str], Result[str]]] = None) -> Optional[str]:
|
||||
def _confirm_and_run(self, script: str, base_dir: str, qa_callback: Optional[Callable[[str], str]] = None, patch_callback: Optional[Callable[[str, str], Result[str]]] = None) -> str:
|
||||
"""
|
||||
[C: tests/test_arch_boundary_phase2.py:TestArchBoundaryPhase2.test_mutating_tool_triggers_callback, tests/test_arch_boundary_phase2.py:TestArchBoundaryPhase2.test_rejection_prevents_dispatch]
|
||||
"""
|
||||
@@ -4444,7 +4444,7 @@ class AppController:
|
||||
del self._pending_actions[dialog._uid]
|
||||
if not approved:
|
||||
self._append_tool_log(final_script, "REJECTED by user")
|
||||
return None
|
||||
return ""
|
||||
self.ai_status = "running powershell..."
|
||||
output = shell_runner.run_powershell(final_script, base_dir, qa_callback=qa_callback, patch_callback=patch_callback)
|
||||
self._append_tool_log(final_script, output)
|
||||
|
||||
@@ -47,8 +47,8 @@ class CommandRegistry:
|
||||
def all(self) -> List[Command]:
|
||||
return list(self._commands.values())
|
||||
|
||||
def get(self, command_id: str) -> Optional[Command]:
|
||||
return self._commands.get(command_id)
|
||||
def get(self, command_id: str) -> Command:
|
||||
return self._commands.get(command_id) or Command(id="", title="", category="uncategorized", action=lambda: None)
|
||||
|
||||
|
||||
def fuzzy_match(query: str, candidates: List[Command], top_n: int = 20) -> List[ScoredCommand]:
|
||||
|
||||
+2
-2
@@ -114,14 +114,14 @@ def parse_diff(diff_text: str) -> List[DiffFile]:
|
||||
|
||||
return files
|
||||
|
||||
def get_line_color(line: str) -> Optional[str]:
|
||||
def get_line_color(line: str) -> str:
|
||||
"""
|
||||
[C: tests/test_diff_viewer.py:test_get_line_color]
|
||||
"""
|
||||
if line.startswith("+"): return "green"
|
||||
elif line.startswith("-"): return "red"
|
||||
elif line.startswith("@@"): return "cyan"
|
||||
return None
|
||||
return ""
|
||||
|
||||
def apply_patch_to_file(patch_text: str, base_dir: str = ".") -> Tuple[bool, str]:
|
||||
"""
|
||||
|
||||
+6
-4
@@ -37,7 +37,9 @@ class FuzzyAnchor:
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def resolve_slice(cls, text: str, slice_data: dict) -> Optional[Tuple[int, int]]:
|
||||
def resolve_slice(cls, text: str, slice_data: dict) -> Tuple[int, int]:
|
||||
"""Returns (start_line, end_line) on success, or (-1, -1) if unresolved."""
|
||||
result: Tuple[int, int] = (-1, -1)
|
||||
"""
|
||||
[C: tests/test_fuzzy_anchor.py:TestFuzzyAnchor.test_resolve_slice_anchor_mismatch_returns_none, tests/test_fuzzy_anchor.py:TestFuzzyAnchor.test_resolve_slice_exact_match, tests/test_fuzzy_anchor.py:TestFuzzyAnchor.test_resolve_slice_line_deleted_before_returns_none, tests/test_fuzzy_anchor.py:TestFuzzyAnchor.test_resolve_slice_line_inserted_before, tests/test_fuzzy_anchor.py:TestFuzzyAnchor.test_resolve_slice_multiple_lines_changed]
|
||||
"""
|
||||
@@ -54,7 +56,7 @@ class FuzzyAnchor:
|
||||
# 2. Fuzzy match
|
||||
start_ctx = slice_data["start_context"]
|
||||
end_ctx = slice_data["end_context"]
|
||||
if not start_ctx or not end_ctx: return None
|
||||
if not start_ctx or not end_ctx: return (-1, -1)
|
||||
|
||||
# Search for start_ctx
|
||||
best_s = -1
|
||||
@@ -68,7 +70,7 @@ class FuzzyAnchor:
|
||||
best_s = i
|
||||
break
|
||||
|
||||
if best_s == -1: return None
|
||||
if best_s == -1: return (-1, -1)
|
||||
|
||||
# Search for end_ctx after start_ctx
|
||||
best_e = -1
|
||||
@@ -87,4 +89,4 @@ class FuzzyAnchor:
|
||||
if best_e != -1:
|
||||
return (best_s + 1, best_e)
|
||||
|
||||
return None
|
||||
return (-1, -1)
|
||||
|
||||
@@ -61,7 +61,7 @@ class WorkerPool:
|
||||
self._lock = threading.Lock()
|
||||
self._semaphore = threading.Semaphore(max_workers)
|
||||
|
||||
def spawn(self, ticket_id: str, target: Callable, args: tuple) -> Optional[threading.Thread]:
|
||||
def spawn(self, ticket_id: str, target: Callable, args: tuple) -> threading.Thread:
|
||||
"""
|
||||
Spawns a new worker thread if the pool is not full.
|
||||
Returns the thread object or None if full.
|
||||
@@ -69,7 +69,7 @@ class WorkerPool:
|
||||
"""
|
||||
with self._lock:
|
||||
if len(self._active) >= self.max_workers:
|
||||
return None
|
||||
return threading.Thread() # sentinel: empty thread, not started
|
||||
|
||||
def wrapper(*a, **kw):
|
||||
try:
|
||||
|
||||
+10
-8
@@ -4,14 +4,16 @@ from typing import Optional, Callable, List
|
||||
|
||||
@dataclass
|
||||
class PendingPatch:
|
||||
patch_text: str
|
||||
file_paths: List[str]
|
||||
generated_by: str
|
||||
timestamp: float
|
||||
patch_text: str = ""
|
||||
file_paths: List[str] = field(default_factory=list)
|
||||
generated_by: str = ""
|
||||
timestamp: float = 0.0
|
||||
|
||||
EMPTY_PATCH: PendingPatch = PendingPatch()
|
||||
|
||||
class PatchModalManager:
|
||||
def __init__(self):
|
||||
self._pending_patch: Optional[PendingPatch] = None
|
||||
self._pending_patch: PendingPatch = EMPTY_PATCH
|
||||
self._show_modal: bool = False
|
||||
self._on_apply_callback: Optional[Callable[[str], bool]] = None
|
||||
self._on_reject_callback: Optional[Callable[[], None]] = None
|
||||
@@ -30,7 +32,7 @@ class PatchModalManager:
|
||||
self._show_modal = True
|
||||
return True
|
||||
|
||||
def get_pending_patch(self) -> Optional[PendingPatch]:
|
||||
def get_pending_patch(self) -> "PendingPatch":
|
||||
"""
|
||||
[C: tests/test_patch_modal.py:test_patch_modal_manager_init, tests/test_patch_modal.py:test_reject_patch, tests/test_patch_modal.py:test_request_patch_approval, tests/test_patch_modal.py:test_reset]
|
||||
"""
|
||||
@@ -66,7 +68,7 @@ class PatchModalManager:
|
||||
"""
|
||||
[C: tests/test_patch_modal.py:test_reject_callback, tests/test_patch_modal.py:test_reject_patch]
|
||||
"""
|
||||
self._pending_patch = None
|
||||
self._pending_patch = EMPTY_PATCH
|
||||
self._show_modal = False
|
||||
if self._on_reject_callback:
|
||||
self._on_reject_callback()
|
||||
@@ -81,7 +83,7 @@ class PatchModalManager:
|
||||
"""
|
||||
[C: tests/test_patch_modal.py:test_reset]
|
||||
"""
|
||||
self._pending_patch = None
|
||||
self._pending_patch = EMPTY_PATCH
|
||||
self._show_modal = False
|
||||
self._on_apply_callback = None
|
||||
self._on_reject_callback = None
|
||||
|
||||
@@ -92,7 +92,7 @@ def test_get_line_color() -> None:
|
||||
assert get_line_color("+added") == "green"
|
||||
assert get_line_color("-removed") == "red"
|
||||
assert get_line_color("@@ -1,3 +1,4 @@") == "cyan"
|
||||
assert get_line_color(" context") == None
|
||||
assert get_line_color(" context") == ""
|
||||
|
||||
def test_apply_patch_simple() -> None:
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
|
||||
@@ -39,7 +39,7 @@ class TestFuzzyAnchor:
|
||||
modified = "line0\nline2\nline3\nline4\n"
|
||||
slc = FuzzyAnchor.create_slice(original, 2, 4)
|
||||
result = FuzzyAnchor.resolve_slice(modified, slc)
|
||||
assert result is None
|
||||
assert result == (-1, -1)
|
||||
|
||||
def test_resolve_slice_multiple_lines_changed(self):
|
||||
original = "line0\nline1\nline2\nline3\nline4\n"
|
||||
@@ -56,4 +56,4 @@ class TestFuzzyAnchor:
|
||||
modified = "foo\nbar\nbaz\ndelta\nepsilon\n"
|
||||
slc = FuzzyAnchor.create_slice(original, 2, 3)
|
||||
result = FuzzyAnchor.resolve_slice(modified, slc)
|
||||
assert result is None
|
||||
assert result == (-1, -1)
|
||||
|
||||
@@ -28,7 +28,7 @@ def test_worker_pool_limit():
|
||||
|
||||
# Try to spawn a 3rd task
|
||||
t3 = pool.spawn("t3", slow_task, (event3,))
|
||||
assert t3 is None
|
||||
assert not t3.is_alive()
|
||||
assert pool.get_active_count() == 2
|
||||
|
||||
# Wait for tasks to finish
|
||||
|
||||
@@ -6,7 +6,7 @@ from src.patch_modal import (
|
||||
|
||||
def test_patch_modal_manager_init():
|
||||
manager = PatchModalManager()
|
||||
assert manager.get_pending_patch() is None
|
||||
assert manager.get_pending_patch().patch_text == ""
|
||||
assert manager.is_modal_shown() is False
|
||||
|
||||
def test_request_patch_approval():
|
||||
@@ -28,7 +28,7 @@ def test_reject_patch():
|
||||
manager.request_patch_approval("diff", ["file.py"])
|
||||
|
||||
manager.reject_patch()
|
||||
assert manager.get_pending_patch() is None
|
||||
assert manager.get_pending_patch().patch_text == ""
|
||||
assert manager.is_modal_shown() is False
|
||||
|
||||
def test_close_modal():
|
||||
@@ -75,7 +75,7 @@ def test_reset():
|
||||
|
||||
manager.reset()
|
||||
|
||||
assert manager.get_pending_patch() is None
|
||||
assert manager.get_pending_patch().patch_text == ""
|
||||
assert manager.is_modal_shown() is False
|
||||
|
||||
def test_get_patch_modal_manager_singleton():
|
||||
|
||||
Reference in New Issue
Block a user