conductor(track): plan for public_api_migration_and_ui_polish_20260615 (7 phases, 28 tasks)
This commit is contained in:
@@ -0,0 +1,380 @@
|
||||
# Plan: Public API Migration + UI Polish Test Cleanup
|
||||
|
||||
**Track:** `public_api_migration_and_ui_polish_20260615`
|
||||
**Spec:** `spec.md`
|
||||
**Status:** Active (plan approved 2026-06-15)
|
||||
|
||||
## TDD Protocol (MANDATORY)
|
||||
|
||||
For each phase, the order is:
|
||||
1. **Red**: verify the test/failure is present (TDD red phase)
|
||||
2. **Green**: implement the fix; run the test; confirm it passes
|
||||
3. **Verify green**: run the targeted test batch to confirm no regression
|
||||
4. **Commit**: one atomic commit per task with a clear message
|
||||
5. **Git note**: attach a 3-5 sentence summary to the commit
|
||||
|
||||
Per the project rule (see `AGENTS.md` "Critical Anti-Patterns"), per-task atomic commits. The 1-space indentation rule is in effect (see `conductor/product-guidelines.md` "AI-Optimized Compact Style").
|
||||
|
||||
**Style enforcement:** Every task delegation to a Tier 3 worker MUST include the reminder "Use exactly 1-space indentation for Python code" to prevent style drift.
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Production call site migration (1 day)
|
||||
|
||||
**Focus:** Migrate 3 production call sites from `ai_client.send()` to `ai_client.send_result()`. This is the highest-risk phase (MMA worker has 5 callbacks; production behavior must be preserved).
|
||||
|
||||
### Task 1.1: Migrate `src/conductor_tech_lead.py:68` (easiest; 2-arg call)
|
||||
|
||||
- [ ] **Task 1.1a**: Verify the call is currently using `ai_client.send()` (no test change needed; this is a refactor, not a bug fix)
|
||||
- **Command:** `uv run rg "ai_client\.send\(" src/conductor_tech_lead.py`
|
||||
- **EXPECTED:** 1 hit at line 68
|
||||
- **COMMIT:** No new commit; this is a verification step.
|
||||
|
||||
- [ ] **Task 1.1b**: Migrate to `send_result()` with Result handling
|
||||
- **WHERE:** `src/conductor_tech_lead.py:60-90` (the `try/except` block containing the call)
|
||||
- **WHAT:** Replace the `ai_client.send(md_content="", user_message=user_message)` call with the Result pattern. On error, log to comms as `WARN/tech_lead_send_failed` and `return None` (the function returns a list of ticket definitions or None on failure).
|
||||
- **HOW:** Use `manual-slop_edit_file` with `old_string` (the `response = ai_client.send(...)` line + the comment block) and `new_string` (the new `result = ai_client.send_result(...)` block with `if not result.ok: ...` handling).
|
||||
- **SAFETY:** The `set_custom_system_prompt` and `set_current_tier` calls before the `send()` MUST be preserved. The `try/except` outer block at line 64 MUST be preserved.
|
||||
- **REFERENCES:** `docs/guide_ai_client.md` "Result API" section; `doeh_test_thinking_cleanup_20260615/spec.md` §3.1 (G1 fix pattern).
|
||||
- **VERIFY:** `uv run rg "ai_client\.send\(" src/conductor_tech_lead.py` returns 0 hits
|
||||
- **COMMIT:** `refactor(conductor_tech_lead): migrate to send_result() (G1, public_api_migration_and_ui_polish_20260615 Phase 1.1)`
|
||||
|
||||
- [ ] **Task 1.1c**: Verify the Tier 2 dispatch tests still pass
|
||||
- **Command:** `uv run pytest tests/test_conductor_tech_lead.py tests/test_orchestrator_pm.py -v 2>&1 | tee tests/artifacts/public_api_phase1_1.log` (if tests exist) OR `uv run pytest tests/ -k "conductor or tech_lead or orchestrator_pm" -v 2>&1 | tee tests/artifacts/public_api_phase1_1.log`
|
||||
- **EXPECTED:** No regression
|
||||
- **COMMIT:** No new commit; this is a verification step.
|
||||
|
||||
### Task 1.2: Migrate `src/orchestrator_pm.py:86` (3-arg call)
|
||||
|
||||
- [ ] **Task 1.2a**: Verify the call is currently using `ai_client.send()`
|
||||
- **Command:** `uv run rg "ai_client\.send\(" src/orchestrator_pm.py`
|
||||
- **EXPECTED:** 1 hit at line 86
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
- [ ] **Task 1.2b**: Migrate to `send_result()` with Result handling
|
||||
- **WHERE:** `src/orchestrator_pm.py:80-100` (the `try/except` block containing the call)
|
||||
- **WHAT:** Replace the `ai_client.send(md_content="", user_message=user_message, enable_tools=False)` call with the Result pattern. On error, log to comms as `WARN/orchestrator_send_failed` and `return None`.
|
||||
- **HOW:** Same pattern as Task 1.1b.
|
||||
- **SAFETY:** The `set_provider` call before the `send()` MUST be preserved.
|
||||
- **VERIFY:** `uv run rg "ai_client\.send\(" src/orchestrator_pm.py` returns 0 hits
|
||||
- **COMMIT:** `refactor(orchestrator_pm): migrate to send_result() (G2, public_api_migration_and_ui_polish_20260615 Phase 1.2)`
|
||||
|
||||
- [ ] **Task 1.2c**: Verify the orchestrator tests pass
|
||||
- **Command:** `uv run pytest tests/ -k "orchestrator_pm or orchestrator or tier1" -v 2>&1 | tee tests/artifacts/public_api_phase1_2.log`
|
||||
- **EXPECTED:** No regression
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
### Task 1.3: Migrate `src/multi_agent_conductor.py:591` (HARDEST; 8-arg call with 5 callbacks)
|
||||
|
||||
- [ ] **Task 1.3a**: Verify the call is currently using `ai_client.send()` and the 5 callbacks are passed
|
||||
- **Command:** `uv run rg "ai_client\.send\(" src/multi_agent_conductor.py`
|
||||
- **EXPECTED:** 1 hit at line 591; the call has `md_content=`, `user_message=`, `base_dir="."`, `pre_tool_callback=`, `qa_callback=`, `patch_callback=`, `stream_callback=`
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
- [ ] **Task 1.3b**: TDD red - verify a known MMA test fails with the current code (or at least the test catches the call path)
|
||||
- **Command:** `uv run pytest tests/test_mma_concurrent_tracks_sim.py tests/test_mma_step_mode_sim.py -v 2>&1 | tee tests/artifacts/public_api_phase1_3_red.log`
|
||||
- **EXPECTED:** Tests pass currently (no regression in baseline); this is a baseline check
|
||||
- **NOTE:** If tests are slow or hit live_gui, use a smaller subset: `uv run pytest tests/test_undo_redo_sim.py -v 2>&1 | tee tests/artifacts/public_api_phase1_3_red.log` (or any single MMA-adjacent test)
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
- [ ] **Task 1.3c**: Migrate to `send_result()` with per-ticket Result handling
|
||||
- **WHERE:** `src/multi_agent_conductor.py:580-605` (the `try/except` block containing the call)
|
||||
- **WHAT:** Replace the `ai_client.send(...)` call with `ai_client.send_result(...)`. On `!result.ok`:
|
||||
1. Log to comms via the existing `worker_comms_callback` (already set at line 587) with `WARN/worker_send_failed` and `err.ui_message()` as the status entry content
|
||||
2. Return early from `run_worker_lifecycle` with a sentinel value (e.g., `None` or `("error", err.ui_message())`); the worker exits with non-zero status
|
||||
- **HOW:** Use `manual-slop_edit_file`. The change is ~10-15 lines.
|
||||
- **SAFETY:** The `set_comms_log_callback`, `set_current_tier`, and `comms_baseline` calls before the `send_result()` MUST be preserved. The `try/except` outer block MUST be preserved.
|
||||
- **REFERENCES:** `docs/guide_mma.md` "Worker Lifecycle" section; the `doeh_test_thinking_cleanup_20260615/spec.md` G1 fix at `src/app_controller.py:265-295` is the canonical Result pattern (adapted for MMA's comms log instead of HTTPException).
|
||||
- **VERIFY:** `uv run rg "ai_client\.send\(" src/multi_agent_conductor.py` returns 0 hits
|
||||
- **COMMIT:** `refactor(multi_agent_conductor): migrate worker dispatch to send_result() (G3, public_api_migration_and_ui_polish_20260615 Phase 1.3)`
|
||||
|
||||
- [ ] **Task 1.3d**: Verify the MMA tests pass
|
||||
- **Command:** `uv run pytest tests/test_mma_concurrent_tracks_sim.py tests/test_mma_step_mode_sim.py tests/test_undo_redo_sim.py -v 2>&1 | tee tests/artifacts/public_api_phase1_3_green.log`
|
||||
- **EXPECTED:** No regression
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
### Task 1.4: Phase 1 verification
|
||||
|
||||
- [ ] **Task 1.4**: Full Phase 1 verification
|
||||
- **Command:** `uv run rg "ai_client\.send\(" src/` (should return 0 hits)
|
||||
- **EXPECTED:** 0 hits
|
||||
- **COMMIT:** `conductor(checkpoint): Phase 1 complete - 3 production call sites migrated to send_result()`
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Test file migration (1 day)
|
||||
|
||||
**Focus:** Migrate 12 test files using `ai_client.send()` to use `send_result()`. Mechanical pattern (per `doeh_test_thinking_cleanup_20260615` Phase 2).
|
||||
|
||||
**The canonical migration pattern:**
|
||||
```python
|
||||
# Before:
|
||||
result = ai_client.send(md_content, user_message, base_dir)
|
||||
assert result == "expected text"
|
||||
|
||||
# After:
|
||||
result = ai_client.send_result(md_content, user_message, base_dir)
|
||||
assert result.ok, f"send_result failed: {result.errors[0].ui_message() if result.errors else 'no error info'}"
|
||||
assert result.data == "expected text"
|
||||
```
|
||||
|
||||
**OR (when the test does NOT need to assert on success):**
|
||||
```python
|
||||
# Before:
|
||||
response = ai_client.send(...)
|
||||
assert response == "x"
|
||||
|
||||
# After:
|
||||
result = ai_client.send_result(...)
|
||||
assert result.ok and result.data == "x"
|
||||
```
|
||||
|
||||
### 2A: Simple files (1 call site each, 6 files)
|
||||
|
||||
- [ ] **Task 2.1**: Migrate `tests/test_ai_client_cli.py:22`
|
||||
- **WHERE:** `tests/test_ai_client_cli.py:22` (`response = ai_client.send(...)`)
|
||||
- **WHAT:** Change to `result = ai_client.send_result(...)` + `assert result.ok` + use `result.data`
|
||||
- **VERIFY:** `uv run pytest tests/test_ai_client_cli.py -v` passes
|
||||
- **COMMIT:** `test(ai_client_cli): migrate to send_result() (Phase 2.1)`
|
||||
|
||||
- [ ] **Task 2.2**: Migrate `tests/test_ai_cache_tracking.py:47`
|
||||
- **WHERE:** `tests/test_ai_cache_tracking.py:47`
|
||||
- **VERIFY:** `uv run pytest tests/test_ai_cache_tracking.py -v` passes
|
||||
- **COMMIT:** `test(ai_cache_tracking): migrate to send_result() (Phase 2.2)`
|
||||
|
||||
- [ ] **Task 2.3**: Migrate `tests/test_gemini_cli_edge_cases.py:38`
|
||||
- **VERIFY:** `uv run pytest tests/test_gemini_cli_edge_cases.py -v` passes
|
||||
- **COMMIT:** `test(gemini_cli_edge): migrate to send_result() (Phase 2.3)`
|
||||
|
||||
- [ ] **Task 2.4**: Migrate `tests/test_gemini_cli_parity_regression.py:12`
|
||||
- **VERIFY:** `uv run pytest tests/test_gemini_cli_parity_regression.py -v` passes
|
||||
- **COMMIT:** `test(gemini_cli_parity): migrate to send_result() (Phase 2.4)`
|
||||
|
||||
- [ ] **Task 2.5**: Migrate `tests/test_gui2_mcp.py:47`
|
||||
- **VERIFY:** `uv run pytest tests/test_gui2_mcp.py -v` passes
|
||||
- **COMMIT:** `test(gui2_mcp): migrate to send_result() (Phase 2.5)`
|
||||
|
||||
- [ ] **Task 2.6**: Migrate `tests/test_token_usage.py:34`
|
||||
- **VERIFY:** `uv run pytest tests/test_token_usage.py -v` passes
|
||||
- **COMMIT:** `test(token_usage): migrate to send_result() (Phase 2.6)`
|
||||
|
||||
### 2B: `test_ai_client_result.py` (3 sites; includes the deprecation test)
|
||||
|
||||
- [ ] **Task 2.7**: Migrate `tests/test_ai_client_result.py` (3 sites) and DELETE the `test_send_deprecated_emits_warning` test (it will be obsolete in Phase 6)
|
||||
- **WHERE:** `tests/test_ai_client_result.py:10-25` (the 3 tests using `send()`)
|
||||
- **WHAT:**
|
||||
- DELETE `test_send_deprecated_emits_warning` (line 16) - obsolete after Phase 6
|
||||
- MIGRATE the other 2 `send()` tests to `send_result()`
|
||||
- KEEP `test_send_result_does_not_emit_deprecation` (line 18) as a regression test
|
||||
- **VERIFY:** `uv run pytest tests/test_ai_client_result.py -v` passes (3 tests, not 4)
|
||||
- **COMMIT:** `test(ai_client_result): migrate to send_result(); drop test_send_deprecated (Phase 2.7)`
|
||||
|
||||
### 2C: `test_api_events.py` (2 sites)
|
||||
|
||||
- [ ] **Task 2.8**: Migrate `tests/test_api_events.py:63,106`
|
||||
- **VERIFY:** `uv run pytest tests/test_api_events.py -v` passes
|
||||
- **COMMIT:** `test(api_events): migrate 2 sites to send_result() (Phase 2.8)`
|
||||
|
||||
### 2D: `test_deepseek_provider.py` (6 sites)
|
||||
|
||||
- [ ] **Task 2.9**: Migrate `tests/test_deepseek_provider.py:31,54,96,122,142,171` (6 sites in 1 file)
|
||||
- **VERIFY:** `uv run pytest tests/test_deepseek_provider.py -v` passes (6+ tests)
|
||||
- **COMMIT:** `test(deepseek): migrate 6 sites to send_result() (Phase 2.9)`
|
||||
|
||||
### 2E: `test_gemini_cli_integration.py` (2 sites)
|
||||
|
||||
- [ ] **Task 2.10**: Migrate `tests/test_gemini_cli_integration.py:15,29`
|
||||
- **VERIFY:** `uv run pytest tests/test_gemini_cli_integration.py -v` passes
|
||||
- **COMMIT:** `test(gemini_cli_integration): migrate 2 sites to send_result() (Phase 2.10)`
|
||||
|
||||
### 2F: `test_tier4_interceptor.py` (1 site; complex setup)
|
||||
|
||||
- [ ] **Task 2.11**: Migrate `tests/test_tier4_interceptor.py:83`
|
||||
- **NOTE:** This test has complex callback setup (`qa_callback=qa_callback`); the Result handling may need `with patch('src.ai_client.send_result', return_value=Result(data="response"))` for the `qa_callback` to work
|
||||
- **VERIFY:** `uv run pytest tests/test_tier4_interceptor.py -v` passes
|
||||
- **COMMIT:** `test(tier4_interceptor): migrate to send_result() (Phase 2.11)`
|
||||
|
||||
### Task 2.12: Phase 2 verification
|
||||
|
||||
- [ ] **Task 2.12**: Full Phase 2 verification
|
||||
- **Command:** `uv run rg "ai_client\.send\(" tests/ | grep -v test_ai_client_result.py` (should be 0 hits after Phase 2)
|
||||
- **EXPECTED:** 0 hits outside `test_ai_client_result.py` (which is handled in Task 2.7)
|
||||
- **COMMIT:** `conductor(checkpoint): Phase 2 complete - 11 test files migrated to send_result()`
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: `test_qwen_provider.py` fix (1 hour)
|
||||
|
||||
**Focus:** Fix the 2 pre-existing test failures in `test_qwen_provider.py` by using the `Result` API assertion pattern (mirrors what `doeh_test_thinking_cleanup_20260615` did for grok/llama).
|
||||
|
||||
- [ ] **Task 3.1**: TDD red - verify the 2 Qwen tests fail
|
||||
- **Command:** `uv run pytest tests/test_qwen_provider.py::test_send_qwen_routes_to_dashscope tests/test_qwen_provider.py::test_qwen_vision_vl_model_accepts_image -v 2>&1 | tee tests/artifacts/public_api_phase3_red.log`
|
||||
- **EXPECTED:** 2 failures with `AssertionError: assert 'hi from qwen' == Result(data='hi from qwen', ...)` (or similar)
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
- [ ] **Task 3.2**: Fix both tests
|
||||
- **WHERE:** `tests/test_qwen_provider.py:13-20` (`test_send_qwen_routes_to_dashscope`) and `:22-31` (`test_qwen_vision_vl_model_accepts_image`)
|
||||
- **WHAT:**
|
||||
- For `test_send_qwen_routes_to_dashscope`: Change `assert result == "hi from qwen"` to `assert result.ok and result.data == "hi from qwen"`
|
||||
- For `test_qwen_vision_vl_model_accepts_image`: Change `assert "cat" in result.lower()` to `assert result.ok and "cat" in result.data.lower()`
|
||||
- **HOW:** Use `manual-slop_edit_file` with the exact old/new strings.
|
||||
- **REFERENCES:** `doeh_test_thinking_cleanup_20260615/plan.md` Task 2.4 (test_llama_ollama_native pattern is the closest reference).
|
||||
- **VERIFY:** `uv run pytest tests/test_qwen_provider.py -v` passes (5/5)
|
||||
- **COMMIT:** `test(qwen): adapt 2 tests to Result API (Phase 3, fixes 2 pre-existing failures)`
|
||||
|
||||
- [ ] **Task 3.3**: Verify no regression
|
||||
- **Command:** `uv run pytest tests/test_qwen_provider.py tests/test_minimax_provider.py tests/test_grok_provider.py tests/test_llama_provider.py tests/test_llama_ollama_native.py -v 2>&1 | tee tests/artifacts/public_api_phase3_green.log`
|
||||
- **EXPECTED:** All vendor tests pass
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4: `test_symbol_parsing.py` fix (30 min)
|
||||
|
||||
**Focus:** Fix the 2 pre-existing test failures by mocking `send_result` not `send`.
|
||||
|
||||
- [ ] **Task 4.1**: TDD red - verify the 2 symbol_parsing tests fail
|
||||
- **Command:** `uv run pytest tests/test_symbol_parsing.py -v 2>&1 | tee tests/artifacts/public_api_phase4_red.log`
|
||||
- **EXPECTED:** 2 failures with `Expected 'send' to have been called once. Called 0 times.`
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
- [ ] **Task 4.2**: Fix both tests
|
||||
- **WHERE:** `tests/test_symbol_parsing.py:45,74`
|
||||
- **WHAT:**
|
||||
- For `test_handle_request_event_appends_definitions` (line 45): Change `patch('src.ai_client.send') as mock_send` to `patch('src.ai_client.send_result') as mock_send_result` AND add `mock_send_result.return_value = Result(data="mocked response")` to the with block
|
||||
- For `test_handle_request_event_no_symbols` (line 74): Same pattern
|
||||
- **HOW:** Use `manual-slop_edit_file`. Add `from src.result_types import Result` to imports if not already present.
|
||||
- **REFERENCES:** `doeh_test_thinking_cleanup_20260615/plan.md` Task 2.7 (the headless_service `test_generate_endpoint` mock migration is the canonical reference).
|
||||
- **VERIFY:** `uv run pytest tests/test_symbol_parsing.py -v` passes (2/2)
|
||||
- **COMMIT:** `test(symbol_parsing): mock send_result not send (Phase 4, fixes 2 pre-existing failures)`
|
||||
|
||||
- [ ] **Task 4.3**: Verify no regression
|
||||
- **Command:** `uv run pytest tests/test_symbol_parsing.py tests/test_api_events.py -v 2>&1 | tee tests/artifacts/public_api_phase4_green.log`
|
||||
- **EXPECTED:** No regression
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
---
|
||||
|
||||
## Phase 5: UI Polish test fixes (30 min)
|
||||
|
||||
**Focus:** Fix the 2 pre-existing test failures in `test_discussion_truncate_layout.py` and `test_log_management_refresh.py`. The production code is already correct (user commits `d0b06575` and `df7bda6e`); the test `find()` logic locates the comment block instead of the actual code.
|
||||
|
||||
- [ ] **Task 5.1**: TDD red - verify the 2 UI Polish tests fail
|
||||
- **Command:** `uv run pytest tests/test_discussion_truncate_layout.py tests/test_log_management_refresh.py -v 2>&1 | tee tests/artifacts/public_api_phase5_red.log`
|
||||
- **EXPECTED:** 2 failures with `AssertionError: ... 'set_next_item_width(140)' in ...` (truncated snippet) and similar for the second test
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
- [ ] **Task 5.2**: Fix `test_discussion_truncate_layout.py`
|
||||
- **WHERE:** `tests/test_discussion_truncate_layout.py:7` (`idx = src.find(marker)`)
|
||||
- **WHAT:** Change `src.find(marker)` to `src.rfind(marker)`. The `find()` locates the comment block at line 5113; `rfind()` locates the actual code at line 5130.
|
||||
- **HOW:** Use `manual-slop_edit_file` with `old_string` = `idx = src.find(marker)` and `new_string` = `idx = src.rfind(marker)`.
|
||||
- **VERIFY:** `uv run pytest tests/test_discussion_truncate_layout.py -v` passes (1/1)
|
||||
- **COMMIT:** `test(discussion_truncate): use rfind() to locate code (Phase 5.1, fixes 1 pre-existing failure)`
|
||||
|
||||
- [ ] **Task 5.3**: Fix `test_log_management_refresh.py`
|
||||
- **WHERE:** `tests/test_log_management_refresh.py:6` (`idx = src.find(marker)`)
|
||||
- **WHAT:** Change `src.find(marker)` to `src.rfind(marker)`. The `find()` locates the comment block at line 2090; `rfind()` locates the actual code at line 2111.
|
||||
- **HOW:** Same as Task 5.2.
|
||||
- **VERIFY:** `uv run pytest tests/test_log_management_refresh.py -v` passes (1/1)
|
||||
- **COMMIT:** `test(log_management_refresh): use rfind() to locate code (Phase 5.2, fixes 1 pre-existing failure)`
|
||||
|
||||
- [ ] **Task 5.4**: Verify no regression
|
||||
- **Command:** `uv run pytest tests/test_discussion_truncate_layout.py tests/test_log_management_refresh.py -v 2>&1 | tee tests/artifacts/public_api_phase5_green.log`
|
||||
- **EXPECTED:** 2/2 pass
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
---
|
||||
|
||||
## Phase 6: Deprecation removal (30 min)
|
||||
|
||||
**Focus:** Remove the legacy `send()` function + the `filterwarnings` entry + the obsolete test file. **MUST be after Phases 1 + 2 + 3 + 4 + 5** (so no caller is left using `send()`).
|
||||
|
||||
- [ ] **Task 6.1**: TDD red - verify no caller of `send()` remains in `src/` or `tests/`
|
||||
- **Command:** `uv run rg "ai_client\.send\(" src/ tests/ | wc -l` (should return 0)
|
||||
- **EXPECTED:** 0 hits
|
||||
- **COMMIT:** No new commit.
|
||||
|
||||
- [ ] **Task 6.2**: Remove the `@deprecated` decorator and the legacy `send()` function in `src/ai_client.py`
|
||||
- **WHERE:** `src/ai_client.py:2939-3040` (the `def send(...)` function with the `@deprecated` decorator at line 2939)
|
||||
- **WHAT:** Delete the decorator and the entire function body. The `send_result()` function (at line 3002) is the permanent replacement.
|
||||
- **HOW:** Use `manual-slop_edit_file` or `set_file_slice` to delete the range. Verify the line range first with `get_file_slice`.
|
||||
- **SAFETY:** The function is the ONLY public `send()`; all production and test callers have been migrated in Phases 1-5. Verify `rg "ai_client\.send\(" src/ tests/` returns 0 BEFORE the deletion.
|
||||
- **REFERENCES:** `conductor/tracks/data_oriented_error_handling_20260606/spec.md` §3.5 (deprecation strategy).
|
||||
- **VERIFY:** `uv run rg "def send\(" src/ai_client.py` returns 0 hits (only `def send_result(` should remain)
|
||||
- **COMMIT:** `refactor(ai_client): remove deprecated send() function (Phase 6.1)`
|
||||
|
||||
- [ ] **Task 6.3**: Delete `tests/test_deprecation_warnings.py`
|
||||
- **WHERE:** `tests/test_deprecation_warnings.py` (entire file, 25 lines)
|
||||
- **WHAT:** Delete the file. Both tests in it are obsolete:
|
||||
- `test_send_deprecated_warning_emitted_once_per_site` — cannot run after `send()` is removed
|
||||
- `test_send_result_does_not_emit_deprecation` — trivially true after `send()` is removed
|
||||
- **HOW:** `rm tests/test_deprecation_warnings.py` (or use the file removal MCP tool if available)
|
||||
- **VERIFY:** `uv run pytest tests/test_deprecation_warnings.py -v 2>&1` should fail with "file not found"
|
||||
- **COMMIT:** `test(ai_client): delete obsolete test_deprecation_warnings.py (Phase 6.2)`
|
||||
|
||||
- [ ] **Task 6.4**: Remove the `filterwarnings` entry in `pyproject.toml`
|
||||
- **WHERE:** `pyproject.toml:46-47` (the `filterwarnings = [...]` block)
|
||||
- **WHAT:** Delete the `"ignore:Use ai_client.send_result.*:DeprecationWarning"` line. If the `filterwarnings` block becomes empty after the deletion, delete the block entirely.
|
||||
- **HOW:** Use `manual-slop_edit_file` with `old_string` and `new_string`.
|
||||
- **VERIFY:** `uv run rg "ignore:Use ai_client.send_result" pyproject.toml` returns 0 hits
|
||||
- **COMMIT:** `chore(pyproject): remove send_result deprecation filterwarnings (Phase 6.3)`
|
||||
|
||||
- [ ] **Task 6.5**: Phase 6 verification
|
||||
- **Command:** `uv run rg "ai_client\.send\(" src/ tests/ pyproject.toml` (should return 0)
|
||||
- **EXPECTED:** 0 hits
|
||||
- **COMMIT:** `conductor(checkpoint): Phase 6 complete - deprecation removed`
|
||||
|
||||
---
|
||||
|
||||
## Phase 7: Docs + housekeep (1 hour)
|
||||
|
||||
**Focus:** Update docs, run the full test suite, update metadata + tracks.md, attach final report.
|
||||
|
||||
- [ ] **Task 7.1**: Update `docs/guide_ai_client.md` to remove deprecation references
|
||||
- **WHERE:** `docs/guide_ai_client.md` (search for "deprecat" case-insensitive)
|
||||
- **WHAT:** Remove or update any mention of "deprecat" + "send()" together. The Result API section should no longer note "send() is deprecated".
|
||||
- **HOW:** Use `manual-slop_edit_file` per occurrence.
|
||||
- **VERIFY:** `uv run rg -i "deprecat" docs/guide_ai_client.md | grep -i send` returns 0 hits
|
||||
- **COMMIT:** `docs(ai_client): remove send() deprecation references (Phase 7.1)`
|
||||
|
||||
- [ ] **Task 7.2**: Update `conductor/product-guidelines.md` to remove deprecation language
|
||||
- **WHERE:** `conductor/product-guidelines.md` (search for "deprecat" case-insensitive)
|
||||
- **WHAT:** Mark the "Public API deprecation" section as RESOLVED. Remove or update "send() is deprecated; use send_result()" mentions.
|
||||
- **HOW:** Use `manual-slop_edit_file` per occurrence.
|
||||
- **VERIFY:** `uv run rg -i "send.*deprecat|deprecat.*send" conductor/product-guidelines.md` returns 0 hits
|
||||
- **COMMIT:** `docs(product): mark public API deprecation as resolved (Phase 7.2)`
|
||||
|
||||
- [ ] **Task 7.3**: Run the full test suite
|
||||
- **Command:** `uv run pytest tests/ 2>&1 | tee tests/artifacts/public_api_phase7_full.log`
|
||||
- **EXPECTED:** 4 fewer failures than pre-track baseline (10 - 6 = 4 RAG failures remain)
|
||||
- **ACTION:** If NEW failures appear (not in the 4 RAG pre-existing list), STOP and report to the user.
|
||||
- **COMMIT:** No new commit; this is a verification step.
|
||||
|
||||
- [ ] **Task 7.4**: Update `metadata.json` to mark the track complete
|
||||
- **WHERE:** `conductor/tracks/public_api_migration_and_ui_polish_20260615/metadata.json`
|
||||
- **WHAT:** Change `"status": "active"` to `"status": "completed"`. Update `verification_criteria` to reflect what was actually verified.
|
||||
- **HOW:** Direct file edit.
|
||||
- **COMMIT:** `conductor(track): mark public_api_migration_and_ui_polish_20260615 as completed`
|
||||
|
||||
- [ ] **Task 7.5**: Conductor - User Manual Verification (Protocol in workflow.md)
|
||||
- **ACTION:** Announce the track is complete. Provide the user with a summary of the 18 fixes (3 production + 12 test + 4 pre-existing-failure + 1 deprecation removal + 2 doc updates) and the test delta (1280 + 6 = 1286 pass; 4 RAG failures deferred).
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
- **Total tasks:** ~28 (across 7 phases)
|
||||
- **Total atomic commits:** ~28 (1 per task) + 6 phase checkpoints = ~28
|
||||
- **Total estimated effort:** 2-3 days Tier 2 work (16-24 hours)
|
||||
- **Dependencies:** None (independent track; no `blocked_by`)
|
||||
- **Out of scope (deferred to separate tracks, documented in spec §7):**
|
||||
- 4 RAG test fixes (separate RAG subsystem track)
|
||||
- The `_send_<vendor>()` → `_send_<vendor>_result()` rename (not needed; tests work with current names)
|
||||
- 23 lower-impact weak-type files (next major track: `data_structure_strengthening_20260606`)
|
||||
- `live_gui_mock_injection_20260615` infrastructure (separate infrastructure track)
|
||||
|
||||
## Test count math
|
||||
|
||||
- **Pre-track baseline:** 1280 pass + 4 skip + 10 fail (verified 2026-06-15)
|
||||
- **After this track:** 1286 pass + 4 skip + 4 fail (6 newly-passing: 2 Qwen + 2 symbol_parsing + 1 truncate + 1 refresh)
|
||||
- **The 4 remaining failures are all RAG subsystem; deferred to the next track**
|
||||
Reference in New Issue
Block a user