diff --git a/conductor/tracks/fix_remaining_tests_20260513/spec.md b/conductor/tracks/fix_remaining_tests_20260513/spec.md new file mode 100644 index 00000000..bf24955a --- /dev/null +++ b/conductor/tracks/fix_remaining_tests_20260513/spec.md @@ -0,0 +1,32 @@ +# Track: Fix Pre-Existing Test Failures + +## Context + +Two test failures that are not related to the ai_client_stub integration fix but need to be resolved for full test suite passing. + +## Failed Tests + +### 1. test_ai_client_proxy_run.py::test_initial_state_variables + +**File:** `tests/test_ai_client_proxy_run.py` +**Error:** `AssertionError: Missing _pending_lock` + +**Root Cause:** The test expects `AIProxyClient` to have a `_pending_lock` attribute (a threading lock), but the class doesn't have it. + +**Fix:** Add `_pending_lock: threading.Lock` to `AIProxyClient.__init__` in `src/ai_client_proxy.py` + +### 2. test_discussion_takes_gui.py (both tests) + +**File:** `tests/test_discussion_takes_gui.py` +**Error:** `ValueError: not enough values to unpack (expected 2, got 0)` at `src/gui_2.py:3668` + +**Root Cause:** The test mocks `imgui.input_text` and `imgui.input_int` but NOT `imgui.input_text_multiline`. When `_render_synthesis_panel` calls `imgui.input_text_multiline(...)`, the mock returns `None` (not unpacked), causing the unpacking failure. + +**Fix:** Add mock for `imgui.input_text_multiline` in the test setup. + +## Tasks + +1. [ ] Fix AIProxyClient - add `_pending_lock` threading.Lock to __init__ +2. [ ] Fix test_discussion_takes_gui.py - add mock for input_text_multiline +3. [ ] Run tests to verify both fixes +4. [ ] Run full test suite to confirm all pass \ No newline at end of file diff --git a/conductor/tracks/test_patch_fixes_20260513/plan.md b/conductor/tracks/test_patch_fixes_20260513/plan.md new file mode 100644 index 00000000..d0c24954 --- /dev/null +++ b/conductor/tracks/test_patch_fixes_20260513/plan.md @@ -0,0 +1,61 @@ +# Track: Fix Test Patches for ai_client_stub Integration + +## Context + +After the refactor to use `ai_client_stub` as the module alias for `app_controller`, several tests fail because they use `patch('src.ai_client.X')` which doesn't properly reach the stub's module-level functions. This is a pre-existing architectural issue that needs fixing. + +## Root Cause Analysis + +When tests use `patch('src.ai_client.get_current_tier', return_value='Tier 3')`: + +1. `patch` creates/overrides `src.ai_client` in the `src` package namespace +2. But `app_controller` does `from src import ai_client_stub as ai_client` at module load time +3. The `ai_client` local reference in `app_controller` points directly to `ai_client_stub` module +4. Patch modifies `src.ai_client` (a different object), not `src.ai_client_stub` +5. Result: Functions in `ai_client_stub` aren't patched + +## Solution Applied + +Changed all patches from `patch('src.ai_client.X')` to `patch('src.ai_client_stub.X')` where the stub was the actual target. + +Also updated module imports in tests to use `ai_client_stub` instead of `ai_client` where appropriate. + +## Tasks Completed + +1. [x] Fix `test_on_tool_log_offloading` - change patch path to `src.ai_client_stub.get_current_tier` +2. [x] Fix `test_redundant_calls_in_process_pending_gui_tasks` - change patches to `src.ai_client_stub` +3. [x] Fix `test_gcli_path_updates_adapter` - use ai_client_stub module reference +4. [x] Fix `test_telemetry_data_updates_correctly` - change patch to `src.ai_client_stub.get_token_stats` +5. [x] Fix `test_gui_updates_on_event` - same as above +6. [x] Run batch test to verify affected tests pass + +## Test Results + +**51 tests passing in the core batch:** +- test_symbol_lookup.py (7) +- test_ai_client_concurrency.py (1) +- test_app_controller_offloading.py (2) +- test_mma_agent_focus_phase1.py (8) +- test_conductor_engine_v2.py (10) +- test_conductor_tech_lead.py (7) +- test_mma_dashboard_refresh.py (2) +- test_mma_ticket_actions.py (2) +- test_gui_phase3.py (3) +- test_process_pending_gui_tasks.py (4) +- test_gui_updates.py (3) + +## Known Pre-Existing Failures (NOT related to this fix) + +1. **test_ai_client_proxy_run.py::test_initial_state_variables** - AIProxyClient missing `_pending_lock` attribute +2. **test_discussion_takes_gui.py** - incomplete mock setup for imgui.input_text_multiline + +## Files Modified + +- tests/test_app_controller_offloading.py +- tests/test_process_pending_gui_tasks.py +- tests/test_gui_updates.py + +## Checkpoint + +Commit: 169fe520 - fix(ai_client_stub): add module-level import for GeminiCliAdapter +Additional commits for test fixes to follow. \ No newline at end of file diff --git a/conductor/tracks/test_patch_fixes_20260513/spec.md b/conductor/tracks/test_patch_fixes_20260513/spec.md new file mode 100644 index 00000000..9ff9ec57 --- /dev/null +++ b/conductor/tracks/test_patch_fixes_20260513/spec.md @@ -0,0 +1,84 @@ +# Track: Fix Test Patches for ai_client_stub Integration + +## Context + +After the refactor to use `ai_client_stub` as the module alias for `app_controller`, several tests fail because they use `patch('src.ai_client.X')` which doesn't properly reach the stub's module-level functions. This is a pre-existing architectural issue that needs fixing. + +## Root Cause Analysis + +When tests use `patch('src.ai_client.get_current_tier', return_value='Tier 3')`: + +1. `patch` creates/overrides `src.ai_client` in the `src` package namespace +2. But `app_controller` does `from src import ai_client_stub as ai_client` at module load time +3. The `ai_client` local reference in `app_controller` points directly to `ai_client_stub` module +4. Patch modifies `src.ai_client` (a different object), not `src.ai_client_stub` +5. Result: Functions in `ai_client_stub` aren't patched + +## Failed Tests + +1. `tests/test_app_controller_offloading.py::test_on_tool_log_offloading` + - Patches `src.ai_client.get_current_tier` expecting it to affect `app_controller._on_tool_log()` + - But `app_controller` imported the stub directly, so patch doesn't reach it + +2. `tests/test_process_pending_gui_tasks.py::test_redundant_calls_in_process_pending_gui_tasks` + - Patches `src.ai_client.set_provider` and `src.ai_client.reset_session` + - Same root cause + +3. `tests/test_process_pending_gui_tasks.py::test_gcli_path_updates_adapter` + - Sets `ai_client._gemini_cli_adapter = None` expecting `app_controller` to see it + - Same import aliasing issue + +4. `tests/test_gui_updates.py::test_telemetry_data_updates_correctly` + - Patches `src.ai_client.get_token_stats` + - Same issue + +5. `tests/test_gui_updates.py::test_gui_updates_on_event` + - Same issue with `get_token_stats` patch + +## Solution Options + +### Option A: Fix Tests to Use Correct Patch Path +Change all patches from `patch('src.ai_client.X')` to `patch('src.ai_client_stub.X')`. + +**Pros:** +- Minimal code change +- Resolves the actual issue +- Follows proper patch semantics + +**Cons:** +- Must update all affected tests + +### Option B: Add Module-Level Alias in src package +Create `src/ai_client.py` that re-exports from `ai_client_stub`: +```python +from src.ai_client_stub import * +``` + +**Pros:** +- Makes `src.ai_client` available as actual module +- Tests using `patch('src.ai_client.X')` would work + +**Cons:** +- Creates dual namespace confusion +- May conflict with existing `ai_client_proxy.py` + +### Option C: Fix app_controller to Not Use Alias +Change all `from src import ai_client_stub as ai_client` to `from src import ai_client` and ensure `src.ai_client` is the stub. + +**Cons:** +- Significant refactor +- May break other imports + +## Recommended Solution: Option A + +Fix the tests to use the correct patch path (`src.ai_client_stub`). This is surgical and addresses the root cause without creating architectural complexity. + +## Tasks + +1. [ ] Fix `test_on_tool_log_offloading` - change patch path to `src.ai_client_stub.get_current_tier` +2. [ ] Fix `test_redundant_calls_in_process_pending_gui_tasks` - change patches to `src.ai_client_stub` +3. [ ] Fix `test_gcli_path_updates_adapter` - use correct patch/module path +4. [ ] Fix `test_telemetry_data_updates_correctly` - change patch to `src.ai_client_stub.get_token_stats` +5. [ ] Fix `test_gui_updates_on_event` - same as above +6. [ ] Run full batch test to verify all tests pass +7. [ ] Create checkpoint commit \ No newline at end of file diff --git a/src/ai_client_proxy.py b/src/ai_client_proxy.py index 6f64a694..124a1df7 100644 --- a/src/ai_client_proxy.py +++ b/src/ai_client_proxy.py @@ -13,6 +13,7 @@ class AIProxyClient: self._status: str = "disconnected" self._pending: dict[str, Any] = {} self._reader_thread: Optional[threading.Thread] = None + self._pending_lock: threading.Lock = threading.Lock() @property def status(self) -> str: diff --git a/tests/test_app_controller_offloading.py b/tests/test_app_controller_offloading.py index 3c679d0e..3a8152cf 100644 --- a/tests/test_app_controller_offloading.py +++ b/tests/test_app_controller_offloading.py @@ -88,7 +88,7 @@ def test_on_tool_log_offloading(app_controller, tmp_session_dir): script = "Get-Process" result = "Process list..." - with patch("src.ai_client.get_current_tier", return_value="Tier 3"): + with patch("src.ai_client_stub.get_current_tier", return_value="Tier 3"): app_controller._on_tool_log(script, result) # Verify files were created in session directory diff --git a/tests/test_discussion_takes_gui.py b/tests/test_discussion_takes_gui.py index d5046ef7..0b56af05 100644 --- a/tests/test_discussion_takes_gui.py +++ b/tests/test_discussion_takes_gui.py @@ -17,8 +17,8 @@ def app_instance(): patch('src.app_controller.AppController._prune_old_logs'), patch('src.app_controller.AppController.start_services'), patch('src.api_hooks.HookServer'), - patch('src.ai_client.set_provider'), - patch('src.ai_client.reset_session') + patch('src.ai_client_stub.set_provider'), + patch('src.ai_client_stub.reset_session') ): app = App() # Setup project discussions @@ -47,12 +47,13 @@ def test_render_discussion_tabs(app_instance): mock_imgui.begin_combo.return_value = False mock_imgui.input_text.return_value = (False, "") mock_imgui.input_int.return_value = (False, 0) + mock_imgui.input_text_multiline.return_value = (False, "") mock_imgui.button.return_value = False mock_imgui.checkbox.return_value = (False, False) mock_imgui.begin_child.return_value = True mock_imgui.selectable.return_value = (False, False) mock_imgui.ListClipper.return_value.step.return_value = False - + # Mock tab bar calls mock_imgui.begin_tab_bar.return_value = True mock_imgui.begin_tab_item.return_value = (False, False) @@ -86,6 +87,7 @@ def test_switching_discussion_via_tabs(app_instance): mock_imgui.begin_combo.return_value = False mock_imgui.input_text.return_value = (False, "") mock_imgui.input_int.return_value = (False, 0) + mock_imgui.input_text_multiline.return_value = (False, "") mock_imgui.button.return_value = False mock_imgui.checkbox.return_value = (False, False) mock_imgui.begin_child.return_value = True @@ -93,7 +95,7 @@ def test_switching_discussion_via_tabs(app_instance): mock_imgui.ListClipper.return_value.step.return_value = False mock_imgui.begin_tab_bar.return_value = True - + # Setup imscope mocks mock_imscope.window.return_value.__enter__.return_value = (True, True) mock_imscope.child.return_value.__enter__.return_value = True diff --git a/tests/test_gui_updates.py b/tests/test_gui_updates.py index efc82e57..5a005302 100644 --- a/tests/test_gui_updates.py +++ b/tests/test_gui_updates.py @@ -31,7 +31,7 @@ def test_telemetry_data_updates_correctly(app_instance: Any) -> None: "percentage": 75.0, } # 3. Patch the dependencies - with patch('src.ai_client.get_token_stats', return_value=mock_stats) as mock_get_stats: + with patch('src.ai_client_stub.get_token_stats', return_value=mock_stats) as mock_get_stats: # 4. Call the method under test app_instance._refresh_api_metrics({}, md_content="test content") # 5. Assert the results @@ -60,7 +60,7 @@ def test_gui_updates_on_event(app_instance: App) -> None: """ mock_stats = {"percentage": 50.0, "current": 500, "limit": 1000} app_instance.last_md = "mock_md" - with patch('src.ai_client.get_token_stats', return_value=mock_stats): + with patch('src.ai_client_stub.get_token_stats', return_value=mock_stats): # Drain the queue while not app_instance.event_queue.empty(): app_instance.event_queue.get() diff --git a/tests/test_process_pending_gui_tasks.py b/tests/test_process_pending_gui_tasks.py index 7ede74bf..7cf9ba64 100644 --- a/tests/test_process_pending_gui_tasks.py +++ b/tests/test_process_pending_gui_tasks.py @@ -1,7 +1,7 @@ from typing import Generator import pytest from unittest.mock import patch, Mock -from src import ai_client +from src import ai_client_stub from src.gui_2 import App @pytest.fixture @@ -20,8 +20,8 @@ def app_instance() -> Generator[App, None, None]: patch('src.app_controller.AppController.start_services'), # Do not patch _init_ai_and_hooks to ensure _settable_fields is initialized patch('src.api_hooks.HookServer'), - patch('src.ai_client.set_provider'), - patch('src.ai_client.reset_session') + patch('src.ai_client_stub.set_provider'), + patch('src.ai_client_stub.reset_session') ): app = App() yield app @@ -30,8 +30,8 @@ def test_redundant_calls_in_process_pending_gui_tasks(app_instance: App) -> None app_instance.controller._pending_gui_tasks = [ {'action': 'set_value', 'item': 'current_provider', 'value': 'anthropic'} ] - with patch('src.ai_client.set_provider') as mock_set_provider, \ - patch('src.ai_client.reset_session') as mock_reset_session: + with patch('src.ai_client_stub.set_provider') as mock_set_provider, \ + patch('src.ai_client_stub.reset_session') as mock_reset_session: app_instance.controller._process_pending_gui_tasks() assert mock_set_provider.call_count == 1 assert mock_reset_session.call_count == 1 @@ -42,10 +42,10 @@ def test_gcli_path_updates_adapter(app_instance: App) -> None: {'action': 'set_value', 'item': 'gcli_path', 'value': '/new/path/to/gemini'} ] # Initialize adapter if it doesn't exist (it shouldn't in mock env) - ai_client._gemini_cli_adapter = None + ai_client_stub._gemini_cli_adapter = None app_instance.controller._process_pending_gui_tasks() - assert ai_client._gemini_cli_adapter is not None - assert ai_client._gemini_cli_adapter.binary_path == '/new/path/to/gemini' + assert ai_client_stub._gemini_cli_adapter is not None + assert ai_client_stub._gemini_cli_adapter.binary_path == '/new/path/to/gemini' def test_process_pending_gui_tasks_drag(app_instance: App) -> None: """Test that the drag action is correctly processed and dispatches to the registered callback."""