Private
Public Access
archive public api migration
This commit is contained in:
@@ -1,445 +0,0 @@
|
||||
{
|
||||
"track_id": "public_api_migration_and_ui_polish_20260615",
|
||||
"name": "Public API Migration + UI Polish Test Cleanup",
|
||||
"initialized": "2026-06-15",
|
||||
"owner": "tier2-tech-lead",
|
||||
"priority": "A",
|
||||
"status": "completed",
|
||||
"type": "refactor + bugfix + test_cleanup + documentation",
|
||||
"scope": {
|
||||
"new_files": [],
|
||||
"modified_files": [
|
||||
"src/ai_client.py",
|
||||
"src/conductor_tech_lead.py",
|
||||
"src/orchestrator_pm.py",
|
||||
"src/multi_agent_conductor.py",
|
||||
"tests/test_ai_client_cli.py",
|
||||
"tests/test_ai_cache_tracking.py",
|
||||
"tests/test_ai_client_result.py",
|
||||
"tests/test_api_events.py",
|
||||
"tests/test_deepseek_provider.py",
|
||||
"tests/test_gemini_cli_edge_cases.py",
|
||||
"tests/test_gemini_cli_integration.py",
|
||||
"tests/test_gemini_cli_parity_regression.py",
|
||||
"tests/test_gui2_mcp.py",
|
||||
"tests/test_tier4_interceptor.py",
|
||||
"tests/test_token_usage.py",
|
||||
"tests/test_symbol_parsing.py",
|
||||
"tests/test_qwen_provider.py",
|
||||
"tests/test_discussion_truncate_layout.py",
|
||||
"tests/test_log_management_refresh.py",
|
||||
"pyproject.toml",
|
||||
"docs/guide_ai_client.md",
|
||||
"conductor/product-guidelines.md"
|
||||
],
|
||||
"deleted_files": [
|
||||
"tests/test_deprecation_warnings.py"
|
||||
]
|
||||
},
|
||||
"blocked_by": [],
|
||||
"blocks": [
|
||||
"data_structure_strengthening_20260606",
|
||||
"mcp_architecture_refactor_20260606 (transitively)"
|
||||
],
|
||||
"estimated_phases": 7,
|
||||
"spec": "spec.md",
|
||||
"plan": "plan.md",
|
||||
|
||||
"regressions_and_pre_existing_failures": [
|
||||
{
|
||||
"id": "G1_conductor_tech_lead_send",
|
||||
"severity": "high",
|
||||
"category": "production_deprecation",
|
||||
"introduced_by": "data_oriented_error_handling_20260606 commit 73cf321c (marked send() @deprecated)",
|
||||
"file_line": "src/conductor_tech_lead.py:68",
|
||||
"symptom": "Production code uses deprecated ai_client.send() (emits DeprecationWarning at runtime)",
|
||||
"fix_phase": 1,
|
||||
"fix": "Migrate to ai_client.send_result() with Result handling (log to comms on error, return None)"
|
||||
},
|
||||
{
|
||||
"id": "G2_orchestrator_pm_send",
|
||||
"severity": "high",
|
||||
"category": "production_deprecation",
|
||||
"introduced_by": "data_oriented_error_handling_20260606 commit 73cf321c",
|
||||
"file_line": "src/orchestrator_pm.py:86",
|
||||
"symptom": "Production code uses deprecated ai_client.send()",
|
||||
"fix_phase": 1,
|
||||
"fix": "Migrate to ai_client.send_result() with Result handling (log to comms on error, return None)"
|
||||
},
|
||||
{
|
||||
"id": "G3_multi_agent_conductor_send",
|
||||
"severity": "high",
|
||||
"category": "production_deprecation",
|
||||
"introduced_by": "data_oriented_error_handling_20260606 commit 73cf321c",
|
||||
"file_line": "src/multi_agent_conductor.py:591",
|
||||
"symptom": "Production code uses deprecated ai_client.send() (8-arg call with 5 callbacks)",
|
||||
"fix_phase": 1,
|
||||
"fix": "Migrate to ai_client.send_result() with per-ticket Result handling (log to worker_comms_callback on error, return sentinel value so worker exits with non-zero status)"
|
||||
},
|
||||
{
|
||||
"id": "G4_test_ai_client_cli",
|
||||
"severity": "medium",
|
||||
"category": "test_deprecation",
|
||||
"file_line": "tests/test_ai_client_cli.py:22",
|
||||
"fix_phase": 2,
|
||||
"fix": "Migrate to send_result() + assert result.ok"
|
||||
},
|
||||
{
|
||||
"id": "G5_test_ai_cache_tracking",
|
||||
"severity": "medium",
|
||||
"category": "test_deprecation",
|
||||
"file_line": "tests/test_ai_cache_tracking.py:47",
|
||||
"fix_phase": 2,
|
||||
"fix": "Migrate to send_result() + assert result.ok"
|
||||
},
|
||||
{
|
||||
"id": "G6_test_ai_client_result",
|
||||
"severity": "medium",
|
||||
"category": "test_deprecation",
|
||||
"file_line": "tests/test_ai_client_result.py:10-25 (3 sites; 1 to delete, 2 to migrate)",
|
||||
"fix_phase": 2,
|
||||
"fix": "Delete test_send_deprecated_emits_warning (obsolete after Phase 6); migrate the other 2 send() tests to send_result(); keep test_send_result_does_not_emit_deprecation as regression test"
|
||||
},
|
||||
{
|
||||
"id": "G7_test_api_events",
|
||||
"severity": "medium",
|
||||
"category": "test_deprecation",
|
||||
"file_line": "tests/test_api_events.py:63,106",
|
||||
"fix_phase": 2,
|
||||
"fix": "Migrate 2 sites to send_result()"
|
||||
},
|
||||
{
|
||||
"id": "G8_test_deepseek_provider",
|
||||
"severity": "medium",
|
||||
"category": "test_deprecation",
|
||||
"file_line": "tests/test_deepseek_provider.py:31,54,96,122,142,171 (6 sites)",
|
||||
"fix_phase": 2,
|
||||
"fix": "Migrate 6 sites to send_result() (1 atomic commit for the file)"
|
||||
},
|
||||
{
|
||||
"id": "G9_test_gemini_cli_edge_cases",
|
||||
"severity": "medium",
|
||||
"category": "test_deprecation",
|
||||
"file_line": "tests/test_gemini_cli_edge_cases.py:38",
|
||||
"fix_phase": 2,
|
||||
"fix": "Migrate to send_result()"
|
||||
},
|
||||
{
|
||||
"id": "G10_test_gemini_cli_integration",
|
||||
"severity": "medium",
|
||||
"category": "test_deprecation",
|
||||
"file_line": "tests/test_gemini_cli_integration.py:15,29",
|
||||
"fix_phase": 2,
|
||||
"fix": "Migrate 2 sites to send_result()"
|
||||
},
|
||||
{
|
||||
"id": "G11_test_gemini_cli_parity_regression",
|
||||
"severity": "medium",
|
||||
"category": "test_deprecation",
|
||||
"file_line": "tests/test_gemini_cli_parity_regression.py:12",
|
||||
"fix_phase": 2,
|
||||
"fix": "Migrate to send_result()"
|
||||
},
|
||||
{
|
||||
"id": "G12_test_gui2_mcp",
|
||||
"severity": "medium",
|
||||
"category": "test_deprecation",
|
||||
"file_line": "tests/test_gui2_mcp.py:47",
|
||||
"fix_phase": 2,
|
||||
"fix": "Migrate to send_result()"
|
||||
},
|
||||
{
|
||||
"id": "G13_test_tier4_interceptor",
|
||||
"severity": "medium",
|
||||
"category": "test_deprecation",
|
||||
"file_line": "tests/test_tier4_interceptor.py:83",
|
||||
"fix_phase": 2,
|
||||
"fix": "Migrate to send_result() (with Result(data=...) wrapper for the qa_callback mock)"
|
||||
},
|
||||
{
|
||||
"id": "G14_test_token_usage",
|
||||
"severity": "medium",
|
||||
"category": "test_deprecation",
|
||||
"file_line": "tests/test_token_usage.py:34",
|
||||
"fix_phase": 2,
|
||||
"fix": "Migrate to send_result()"
|
||||
},
|
||||
{
|
||||
"id": "G15_test_symbol_parsing",
|
||||
"severity": "high",
|
||||
"category": "test_mock_bug",
|
||||
"file_line": "tests/test_symbol_parsing.py:45,74",
|
||||
"symptom": "Mocks src.ai_client.send but production now uses send_result; mock receives 0 calls; test fails with 'send was called 0 times'",
|
||||
"fix_phase": 4,
|
||||
"fix": "Change patch('src.ai_client.send') to patch('src.ai_client.send_result') with return_value=Result(data='mocked response')"
|
||||
},
|
||||
{
|
||||
"id": "G16_test_qwen_provider",
|
||||
"severity": "high",
|
||||
"category": "test_mock_bug",
|
||||
"file_line": "tests/test_qwen_provider.py:17,27",
|
||||
"symptom": "_send_qwen() now returns Result[str] (per data_oriented_error_handling refactor); tests assert against raw str",
|
||||
"fix_phase": 3,
|
||||
"fix": "Change 'assert result == x' to 'assert result.ok and result.data == x' (same pattern as doeh_test_thinking_cleanup used for grok/llama/llama_native)"
|
||||
},
|
||||
{
|
||||
"id": "G17_test_discussion_truncate_layout",
|
||||
"severity": "high",
|
||||
"category": "ui_polish_test_bug",
|
||||
"file_line": "tests/test_discussion_truncate_layout.py:7",
|
||||
"symptom": "Test uses find() which locates the comment block at src/gui_2.py:5113; the 200-char snippet doesn't reach the actual code at line 5130. Production code (set_next_item_width(140) + drag_int) is already correct (user commit d0b06575)",
|
||||
"fix_phase": 5,
|
||||
"fix": "Change src.find(marker) to src.rfind(marker) to locate the actual code, not the comment"
|
||||
},
|
||||
{
|
||||
"id": "G18_test_log_management_refresh",
|
||||
"severity": "high",
|
||||
"category": "ui_polish_test_bug",
|
||||
"file_line": "tests/test_log_management_refresh.py:6",
|
||||
"symptom": "Test uses find() which locates the comment block at src/gui_2.py:2090; the 400-char snippet doesn't reach the actual code at line 2111. Production code (in-place load_registry()) is already correct (user commit df7bda6e)",
|
||||
"fix_phase": 5,
|
||||
"fix": "Change src.find(marker) to src.rfind(marker) to locate the actual code, not the comment"
|
||||
},
|
||||
{
|
||||
"id": "G19_deprecated_send_function",
|
||||
"severity": "high",
|
||||
"category": "deprecation_removal",
|
||||
"file_line": "src/ai_client.py:2939-3040",
|
||||
"symptom": "Legacy send() function still exists; emits DeprecationWarning at runtime; filterwarnings in pyproject.toml silences it",
|
||||
"fix_phase": 6,
|
||||
"fix": "Remove the @deprecated decorator + the entire send() function body; remove the filterwarnings entry in pyproject.toml:46-47; delete tests/test_deprecation_warnings.py (both tests are obsolete)"
|
||||
}
|
||||
],
|
||||
|
||||
"pre_existing_failures_fixed": [
|
||||
{
|
||||
"id": "PE_1",
|
||||
"test": "tests/test_qwen_provider.py::test_send_qwen_routes_to_dashscope",
|
||||
"fix_phase": 3,
|
||||
"root_cause": "_send_qwen() returns Result[str]; test asserts against raw str"
|
||||
},
|
||||
{
|
||||
"id": "PE_2",
|
||||
"test": "tests/test_qwen_provider.py::test_qwen_vision_vl_model_accepts_image",
|
||||
"fix_phase": 3,
|
||||
"root_cause": "Same as PE_1"
|
||||
},
|
||||
{
|
||||
"id": "PE_3",
|
||||
"test": "tests/test_symbol_parsing.py::test_handle_request_event_appends_definitions",
|
||||
"fix_phase": 4,
|
||||
"root_cause": "Mocks src.ai_client.send but production uses send_result"
|
||||
},
|
||||
{
|
||||
"id": "PE_4",
|
||||
"test": "tests/test_symbol_parsing.py::test_handle_request_event_no_symbols",
|
||||
"fix_phase": 4,
|
||||
"root_cause": "Same as PE_3"
|
||||
},
|
||||
{
|
||||
"id": "PE_5",
|
||||
"test": "tests/test_discussion_truncate_layout.py::test_keep_pairs_input_uses_adequate_width",
|
||||
"fix_phase": 5,
|
||||
"root_cause": "Test uses find() which locates the comment block, not the actual code at line 5130"
|
||||
},
|
||||
{
|
||||
"id": "PE_6",
|
||||
"test": "tests/test_log_management_refresh.py::test_refresh_registry_button_calls_load_registry",
|
||||
"fix_phase": 5,
|
||||
"root_cause": "Test uses find() which locates the comment block, not the actual code at line 2111"
|
||||
}
|
||||
],
|
||||
|
||||
"pre_existing_failures_remaining": [
|
||||
{
|
||||
"id": "PR_1",
|
||||
"test": "tests/test_rag_integration.py::test_rag_integration",
|
||||
"root_cause": "Pre-existing RAG subsystem issue (NoneType.get error in RAG config lookup code)",
|
||||
"defer_to": "RAG subsystem track (planned; not yet specced)"
|
||||
},
|
||||
{
|
||||
"id": "PR_2",
|
||||
"test": "tests/test_rag_phase4_final_verify.py::test_phase4_final_verify",
|
||||
"root_cause": "Same as PR_1",
|
||||
"defer_to": "RAG subsystem track"
|
||||
},
|
||||
{
|
||||
"id": "PR_3",
|
||||
"test": "tests/test_rag_phase4_stress.py::test_rag_large_codebase_verification_sim",
|
||||
"root_cause": "Same as PR_1",
|
||||
"defer_to": "RAG subsystem track"
|
||||
},
|
||||
{
|
||||
"id": "PR_4",
|
||||
"test": "tests/test_rag_visual_sim.py::test_rag_full_lifecycle_sim",
|
||||
"root_cause": "Same as PR_1",
|
||||
"defer_to": "RAG subsystem track"
|
||||
}
|
||||
],
|
||||
|
||||
"deferred_to_followup_tracks": [
|
||||
{
|
||||
"id": "rag_test_failures",
|
||||
"title": "RAG Subsystem Test Fixes",
|
||||
"description": "Fix the 4 pre-existing RAG test failures (test_rag_integration, test_rag_phase4_final_verify, test_rag_phase4_stress, test_rag_visual_sim). The error is in RAG config lookup code, not AI client code. A partial fix was attempted in commit 16412ad5 (RAG Phase 4 dim-mismatch recovery).",
|
||||
"track_status": "recommended; not yet specced"
|
||||
},
|
||||
{
|
||||
"id": "private_api_rename",
|
||||
"title": "Private API Rename (_send_<vendor> -> _send_<vendor>_result)",
|
||||
"description": "Per data_oriented_error_handling_20260606 spec §3.5 line 611, the original plan was to rename the private _send_<vendor>() functions to _send_<vendor>_result() to match their Result return type. The rename was NOT done in the data_oriented_error_handling track; the tests work with the current names. A future track could do the rename if needed.",
|
||||
"track_status": "not needed for now; tests work with current names"
|
||||
},
|
||||
{
|
||||
"id": "data_structure_strengthening_20260606",
|
||||
"title": "Data Structure Strengthening (Type Aliases + NamedTuples)",
|
||||
"description": "Introduce 6 TypeAlias definitions in src/type_aliases.py; replace 370+ anonymous dict[str, Any] sites in 6 high-traffic files (src/ai_client.py, src/app_controller.py, src/models.py, src/api_hook_client.py, src/project_manager.py, src/aggregate.py). The 23 lower-impact files remain. Spec already exists; plan pending.",
|
||||
"track_status": "ready to start; blocked by this track (cleaner Result API usage makes type-alias replacement easier)"
|
||||
},
|
||||
{
|
||||
"id": "live_gui_mock_injection_20260615",
|
||||
"title": "Live GUI Mock Injection Infrastructure",
|
||||
"description": "Infrastructure for mock injection into the live_gui subprocess. Unblocks proper end-to-end live_gui + AI client tests.",
|
||||
"track_status": "recommended; not yet specced"
|
||||
}
|
||||
],
|
||||
|
||||
"verification_criteria": {
|
||||
"g1_three_production_call_sites_migrated": "uv run rg 'ai_client\\.send\\(' src/ returns 0 hits",
|
||||
"g2_twelve_test_files_migrated": "uv run rg 'ai_client\\.send\\(' tests/ returns 0 hits",
|
||||
"g3_qwen_test_passes": "uv run pytest tests/test_qwen_provider.py -v passes 5/5 (was 3/5; fixes 2 pre-existing failures)",
|
||||
"g4_symbol_parsing_test_passes": "uv run pytest tests/test_symbol_parsing.py -v passes 2/2 (fixes 2 pre-existing failures)",
|
||||
"g5_truncate_layout_test_passes": "uv run pytest tests/test_discussion_truncate_layout.py -v passes 1/1 (fixes 1 pre-existing failure)",
|
||||
"g6_log_management_refresh_test_passes": "uv run pytest tests/test_log_management_refresh.py -v passes 1/1 (fixes 1 pre-existing failure)",
|
||||
"g7_deprecated_send_removed": "uv run rg 'def send\\(' src/ai_client.py returns 0 hits (only def send_result() should remain)",
|
||||
"g8_test_deprecation_warnings_deleted": "tests/test_deprecation_warnings.py does not exist",
|
||||
"g9_filterwarnings_removed": "uv run rg 'ignore:Use ai_client.send_result' pyproject.toml returns 0 hits",
|
||||
"g10_guide_ai_client_deprecation_removed": "uv run rg -i 'deprecat' docs/guide_ai_client.md | grep -i send returns 0 hits",
|
||||
"g11_product_guidelines_deprecation_removed": "uv run rg -i 'send.*deprecat|deprecat.*send' conductor/product-guidelines.md returns 0 hits",
|
||||
"g12_no_new_regressions": "uv run pytest tests/ shows 4 fewer failures than the pre-track baseline (10 - 6 = 4 RAG failures remain); no new failures",
|
||||
"g13_per_task_atomic_commits": "~28 git commits; each commit is buildable + testable",
|
||||
"g14_per_commit_git_notes": "All ~28 commits have git notes summarizing the task",
|
||||
"g15_style_preserved": "1-space indentation, no comments, type hints in all changed code; uv run python -c 'import ast; ast.parse(open(\"src/ai_client.py\").read())' succeeds"
|
||||
},
|
||||
|
||||
"fr_to_phase_mapping": {
|
||||
"G1_conductor_tech_lead_send": {
|
||||
"phase": 1,
|
||||
"fix_files": ["src/conductor_tech_lead.py:60-90"],
|
||||
"test_files": ["tests/test_conductor_tech_lead.py (if exists)"]
|
||||
},
|
||||
"G2_orchestrator_pm_send": {
|
||||
"phase": 1,
|
||||
"fix_files": ["src/orchestrator_pm.py:80-100"],
|
||||
"test_files": ["tests/test_orchestrator_pm.py (if exists)"]
|
||||
},
|
||||
"G3_multi_agent_conductor_send": {
|
||||
"phase": 1,
|
||||
"fix_files": ["src/multi_agent_conductor.py:580-605"],
|
||||
"test_files": ["tests/test_mma_concurrent_tracks_sim.py", "tests/test_mma_step_mode_sim.py", "tests/test_undo_redo_sim.py", "30+ MMA live_gui tests"]
|
||||
},
|
||||
"G4-G14_test_migration": {
|
||||
"phase": 2,
|
||||
"fix_files": ["tests/test_ai_client_cli.py", "tests/test_ai_cache_tracking.py", "tests/test_ai_client_result.py", "tests/test_api_events.py", "tests/test_deepseek_provider.py", "tests/test_gemini_cli_edge_cases.py", "tests/test_gemini_cli_integration.py", "tests/test_gemini_cli_parity_regression.py", "tests/test_gui2_mcp.py", "tests/test_tier4_interceptor.py", "tests/test_token_usage.py"],
|
||||
"min_test_count": 12
|
||||
},
|
||||
"G15_symbol_parsing_fix": {
|
||||
"phase": 4,
|
||||
"fix_files": ["tests/test_symbol_parsing.py:45,74"],
|
||||
"min_test_count": 2
|
||||
},
|
||||
"G16_qwen_test_fix": {
|
||||
"phase": 3,
|
||||
"fix_files": ["tests/test_qwen_provider.py:13-20, 22-31"],
|
||||
"min_test_count": 2
|
||||
},
|
||||
"G17_G18_ui_polish_test_fixes": {
|
||||
"phase": 5,
|
||||
"fix_files": ["tests/test_discussion_truncate_layout.py:7", "tests/test_log_management_refresh.py:6"],
|
||||
"min_test_count": 2
|
||||
},
|
||||
"G19_deprecation_removal": {
|
||||
"phase": 6,
|
||||
"fix_files": ["src/ai_client.py:2939-3040", "pyproject.toml:46-47"],
|
||||
"deleted_files": ["tests/test_deprecation_warnings.py"],
|
||||
"min_test_count": 0
|
||||
},
|
||||
"G20_doc_updates": {
|
||||
"phase": 7,
|
||||
"fix_files": ["docs/guide_ai_client.md", "conductor/product-guidelines.md"],
|
||||
"min_test_count": 0
|
||||
}
|
||||
},
|
||||
|
||||
"estimated_effort": {
|
||||
"phase_1": "1 day - 3 production call sites migrated (1 hardest with 5 callbacks)",
|
||||
"phase_2": "1 day - 12 test files migrated to send_result() (mechanical)",
|
||||
"phase_3": "1 hour - 2 Qwen tests fixed",
|
||||
"phase_4": "30 min - 2 symbol_parsing tests fixed",
|
||||
"phase_5": "30 min - 2 UI Polish test bugs fixed (find -> rfind)",
|
||||
"phase_6": "30 min - deprecation removed (send() function + filterwarnings + test_deprecation_warnings.py deleted)",
|
||||
"phase_7": "1 hour - docs updated + full suite sweep + metadata + tracks.md",
|
||||
"total": "2-3 days Tier 2 work (16-24 hours)"
|
||||
},
|
||||
|
||||
"risk_register": {
|
||||
"R1_multi_agent_conductor_migration_breaks_MMA": {
|
||||
"likelihood": "medium",
|
||||
"impact": "high",
|
||||
"mitigation": "TDD red first; verify a known MMA test fails before the fix; verify it passes after. Use the doeh_test_thinking_cleanup_20260615 G1 fix pattern (adapted for MMA's comms log instead of HTTPException)."
|
||||
},
|
||||
"R2_send_removal_breaks_indirect_imports": {
|
||||
"likelihood": "low",
|
||||
"impact": "medium",
|
||||
"mitigation": "Run 'rg ai_client\\.send\\( src/ tests/' before AND after Phase 6 to confirm 0 hits."
|
||||
},
|
||||
"R3_filterwarnings_removal_causes_test_failures": {
|
||||
"likelihood": "low",
|
||||
"impact": "low",
|
||||
"mitigation": "The filter was added in data_oriented_error_handling_20260606 specifically to silence send() deprecation; no other deprecation in the codebase is silenced by it. Verified by checking the rg history."
|
||||
},
|
||||
"R4_ui_polish_test_fixes_mask_real_production_bug": {
|
||||
"likelihood": "low",
|
||||
"impact": "medium",
|
||||
"mitigation": "The production code at src/gui_2.py:5130-5131 and :2111-2112 was already verified to have the correct values. The test bug is just the search logic."
|
||||
},
|
||||
"R5_qwen_test_fix_uses_different_pattern": {
|
||||
"likelihood": "low",
|
||||
"impact": "low",
|
||||
"mitigation": "Plan uses the same 'assert result.ok and result.data == x' pattern as doeh_test_thinking_cleanup_20260615 (commits d7e42a4a, 439a0ac0, dbdf9ba9)."
|
||||
},
|
||||
"R6_test_deprecation_warnings_deletion_misinterpreted": {
|
||||
"likelihood": "low",
|
||||
"impact": "low",
|
||||
"mitigation": "Both tests in the file are obsolete after send() removal. The first test cannot run without send(). The second test is trivially true. Document in the commit message."
|
||||
},
|
||||
"R7_rag_failures_regress_during_track": {
|
||||
"likelihood": "low",
|
||||
"impact": "medium",
|
||||
"mitigation": "Run full test suite in Phase 7 and compare to the pre-track baseline. The 4 RAG failures are documented as pre-existing with their defer-to track recorded."
|
||||
}
|
||||
},
|
||||
|
||||
"critical_audit_findings": {
|
||||
"ui_polish_status": {
|
||||
"phase_1_markdown_tables": "SHIPPED (commit 79ac9210); src/markdown_table.py exists",
|
||||
"phase_2_keep_pairs": "Code SHIPPED (user commit d0b06575, src/gui_2.py:5130-5131); test FAILING (find() locates comment block, not code)",
|
||||
"phase_3_refresh_registry": "Code SHIPPED (user commit df7bda6e, src/gui_2.py:2111-2112); test FAILING (find() locates comment block, not code)",
|
||||
"phase_4_vendor_state": "SHIPPED (commit 3a864076); src/vendor_state.py exists",
|
||||
"phase_5_files_directory_tree": "SHIPPED (commit 74e02485); src/gui_2.py:render_files_and_media uses directory grouping"
|
||||
},
|
||||
"send_state": {
|
||||
"production_call_sites_remaining": 3,
|
||||
"production_call_sites_migrated_by_doeh_track": 2,
|
||||
"test_files_using_send_directly": 12,
|
||||
"test_files_using_send_directly_in_parent_spec": 63,
|
||||
"discrepancy_reason": "Parent spec (data_oriented_error_handling_20260606) verified 63 test files on 2026-06-11; since then, doeh_test_thinking_cleanup_20260615 migrated 11 of them (Phase 2 of that track), leaving 12 today. The current count is verified via rg 2026-06-15."
|
||||
},
|
||||
"deprecated_send_function_state": {
|
||||
"decorator": "src/ai_client.py:2939 (@deprecated from typing_extensions)",
|
||||
"function_body_lines": "src/ai_client.py:2940-3040",
|
||||
"filterwarnings_entry": "pyproject.toml:46-47 (filterwarnings = [\"ignore:Use ai_client.send_result.*:DeprecationWarning\"])",
|
||||
"obsolete_test_file": "tests/test_deprecation_warnings.py (2 tests; both will be deleted in Phase 6)"
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,437 +0,0 @@
|
||||
# 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)`
|
||||
|
||||
### 2G: Test mock migrations for production-affected tests (added 2026-06-15 during Phase 1)
|
||||
|
||||
**CRITICAL DISCOVERY during Phase 1.1:** The original Phase 2 list of 12 test files covered files that *call* `ai_client.send(...)`. However, several test files use `patch('src.ai_client.send')` to *mock* the deprecated function for tests of the production code paths. When the production code is migrated to `send_result()` (Phases 1.1-1.3), the mocks receive 0 calls and the tests fail with `'send' was called 0 times`.
|
||||
|
||||
**Affected test files (8 discovered; the plan/spec missed them):**
|
||||
- `tests/test_conductor_tech_lead.py` (3 mocks; breaks after Phase 1.1) - was the regression I hit
|
||||
- `tests/test_orchestration_logic.py` (1 mock; breaks after Phase 1.1) - was the regression I hit
|
||||
- `tests/test_orchestrator_pm.py` (3 mocks; breaks after Phase 1.2)
|
||||
- `tests/test_orchestrator_pm_history.py` (1 mock; breaks after Phase 1.2)
|
||||
- `tests/test_phase6_engine.py` (1 mock; breaks after Phase 1.3 if migration touches worker_comms_callback path)
|
||||
- `tests/test_run_worker_lifecycle_abort.py` (1 mock; breaks after Phase 1.3)
|
||||
- `tests/test_spawn_interception_v2.py` (1 mock; breaks after Phase 1.3)
|
||||
- `tests/test_rag_integration.py` (1 mock; already pre-existing failure; deferred to RAG track per spec §7.1 OOS1)
|
||||
|
||||
**Migration pattern for mocks:**
|
||||
```python
|
||||
# Before:
|
||||
with patch('src.ai_client.send') as mock_send:
|
||||
mock_send.return_value = '[{"id": "T1"}]'
|
||||
...
|
||||
|
||||
# After:
|
||||
with patch('src.ai_client.send_result') as mock_send_result:
|
||||
mock_send_result.return_value = Result(data='[{"id": "T1"}]')
|
||||
...
|
||||
```
|
||||
|
||||
Must also add `from src.result_types import Result` to imports if not already present.
|
||||
|
||||
- [ ] **Task 2.12**: Migrate test_conductor_tech_lead.py (3 mocks)
|
||||
- **VERIFY:** `uv run pytest tests/test_conductor_tech_lead.py -v` passes
|
||||
- **COMMIT:** `test(conductor_tech_lead): mock send_result not send (Phase 2.12, fixes Phase 1.1 regression)`
|
||||
|
||||
- [ ] **Task 2.13**: Migrate test_orchestration_logic.py (1 mock)
|
||||
- **VERIFY:** `uv run pytest tests/test_orchestration_logic.py -v` passes
|
||||
- **COMMIT:** `test(orchestration_logic): mock send_result not send (Phase 2.13, fixes Phase 1.1 regression)`
|
||||
|
||||
- [ ] **Task 2.14**: Migrate test_orchestrator_pm.py (3 mocks; pre-empt Phase 1.2 regression)
|
||||
- **VERIFY:** `uv run pytest tests/test_orchestrator_pm.py -v` passes
|
||||
- **COMMIT:** `test(orchestrator_pm): mock send_result not send (Phase 2.14, pre-empts Phase 1.2 regression)`
|
||||
|
||||
- [ ] **Task 2.15**: Migrate test_orchestrator_pm_history.py (1 mock; pre-empt Phase 1.2 regression)
|
||||
- **VERIFY:** `uv run pytest tests/test_orchestrator_pm_history.py -v` passes
|
||||
- **COMMIT:** `test(orchestrator_pm_history): mock send_result not send (Phase 2.15, pre-empts Phase 1.2 regression)`
|
||||
|
||||
- [ ] **Task 2.16**: Migrate test_phase6_engine.py (1 mock; pre-empt Phase 1.3 regression)
|
||||
- **VERIFY:** `uv run pytest tests/test_phase6_engine.py -v` passes
|
||||
- **COMMIT:** `test(phase6_engine): mock send_result not send (Phase 2.16, pre-empts Phase 1.3 regression)`
|
||||
|
||||
- [ ] **Task 2.17**: Migrate test_run_worker_lifecycle_abort.py (1 mock; pre-empt Phase 1.3 regression)
|
||||
- **VERIFY:** `uv run pytest tests/test_run_worker_lifecycle_abort.py -v` passes
|
||||
- **COMMIT:** `test(run_worker_lifecycle_abort): mock send_result not send (Phase 2.17, pre-empts Phase 1.3 regression)`
|
||||
|
||||
- [ ] **Task 2.18**: Migrate test_spawn_interception_v2.py (1 mock; pre-empt Phase 1.3 regression)
|
||||
- **VERIFY:** `uv run pytest tests/test_spawn_interception_v2.py -v` passes
|
||||
- **COMMIT:** `test(spawn_interception_v2): mock send_result not send (Phase 2.18, pre-empts Phase 1.3 regression)`
|
||||
|
||||
### Task 2.19: Phase 2 verification
|
||||
|
||||
- [ ] **Task 2.19**: 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 - 18 test files migrated to send_result()` (11 call-site + 7 mock)
|
||||
|
||||
---
|
||||
|
||||
## 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**
|
||||
@@ -1,585 +0,0 @@
|
||||
# Track Specification: Public API Migration + UI Polish Test Cleanup
|
||||
|
||||
**Track ID:** `public_api_migration_and_ui_polish_20260615`
|
||||
**Status:** Active (spec approved 2026-06-15)
|
||||
**Priority:** A (foundational; precedes `data_structure_strengthening_20260606`)
|
||||
**Owner:** Tier 2 Tech Lead
|
||||
**Type:** refactor + bugfix + test_cleanup + documentation
|
||||
**Estimated effort:** 2-3 days Tier 2 work (16-24 hours)
|
||||
**Parent tracks:** `data_oriented_error_handling_20260606` (shipped 2026-06-12), `ai_loop_regressions_20260614` (shipped 2026-06-15), `doeh_test_thinking_cleanup_20260615` (shipped 2026-06-15)
|
||||
**Blocks:** `data_structure_strengthening_20260606` (cleaner `Result` API usage makes type-alias replacement easier), `mcp_architecture_refactor_20260606` (transitively)
|
||||
|
||||
---
|
||||
|
||||
## 0. TL;DR
|
||||
|
||||
This is a **stability track** that finishes the cleanup work started by `data_oriented_error_handling_20260606` and `doeh_test_thinking_cleanup_20260615`. Two concerns, one track:
|
||||
|
||||
1. **Public API Migration**: remove the deprecated `ai_client.send()` legacy wrapper; migrate 3 remaining production call sites + 12 test files to `send_result()`; fix 4 of the 10 pre-existing test failures (2 Qwen + 2 symbol_parsing) as a side effect of the migration.
|
||||
2. **UI Polish Test Cleanup**: fix 2 broken test assertions in `test_discussion_truncate_layout.py` and `test_log_management_refresh.py` (the production code was already fixed by user commits `d0b06575` and `df7bda6e`; the tests use `find()` which locates the comment block instead of the actual code).
|
||||
|
||||
**Result:** 6 of 10 pre-existing test failures fixed. Remaining 4 RAG failures are deferred to the next track (a separate RAG subsystem track — out of scope for this one). Project reaches a stable state suitable for the `data_structure_strengthening_20260606` track.
|
||||
|
||||
---
|
||||
|
||||
## 1. Overview
|
||||
|
||||
### 1.1 Current State (as of 2026-06-15)
|
||||
|
||||
The `data_oriented_error_handling_20260606` track (shipped 2026-06-12) introduced the `Result[T, ErrorInfo]` pattern and `send_result()` as the new public API. The legacy `ai_client.send()` was marked `@deprecated` and routed through `send_result()` internally. Two follow-up tracks shipped fixes for the immediate user-blocking issues (`ai_loop_regressions_20260614`) and the easy test mock bugs (`doeh_test_thinking_cleanup_20260615`).
|
||||
|
||||
**As of 2026-06-15:**
|
||||
- 3 production call sites of the deprecated `send()` remain in `src/`
|
||||
- 12 test files use `ai_client.send()` directly
|
||||
- 1 test file uses `_send_<vendor>()` with the new `Result` return type but the old assertion pattern (causing 2 of 10 pre-existing failures)
|
||||
- 2 test files mock `ai_client.send` directly (causing 2 of 10 pre-existing failures)
|
||||
- 2 UI Polish test files use `find()` to locate a comment block instead of the actual code (causing 2 of 10 pre-existing failures)
|
||||
- 4 RAG test files fail (separate subsystem; deferred to a follow-up RAG track)
|
||||
|
||||
### 1.2 Gaps to Fill (this Track's Scope)
|
||||
|
||||
| Gap | Count | Type | Spec Section |
|
||||
|---|---|---|---|
|
||||
| Production `ai_client.send()` callers | 3 | refactor (deprecation removal) | §3.1 |
|
||||
| Test files using `ai_client.send()` | 12 | refactor (deprecation removal) | §3.2 |
|
||||
| Test files using `_send_<vendor>()` with old assertions | 1 | test fix (G3 in pre-existing failures) | §3.3 |
|
||||
| Test files mocking `ai_client.send` | 1 | test fix (G4 in pre-existing failures) | §3.4 |
|
||||
| UI Polish test bugs (`find()` not `rfind()`) | 2 | test fix (G6, G7 in pre-existing failures) | §3.5 |
|
||||
| Deprecation marker + legacy `send()` function | 1 | refactor (deprecation removal) | §3.6 |
|
||||
| `filterwarnings` conftest entry | 1 | housekeeping (deprecation removal) | §3.6 |
|
||||
| `test_deprecation_warnings.py` | 1 file (2 tests) | delete (tests obsolete) | §3.6 |
|
||||
| `docs/guide_ai_client.md` deprecation references | multiple | documentation | §3.7 |
|
||||
| `conductor/product-guidelines.md` deprecation language | multiple | documentation | §3.7 |
|
||||
|
||||
### 1.3 Already Implemented (DO NOT re-implement)
|
||||
|
||||
Verified by code audit (2026-06-15) — the following already work and are NOT in this track's scope:
|
||||
|
||||
- **`send_result()` public API** — added in commit `9f86b2be` by `data_oriented_error_handling_20260606`
|
||||
- **`_send_<vendor>()` returning `Result[str]`** — all 6 vendors (`_send_gemini`, `_send_gemini_cli`, `_send_grok`, `_send_minimax`, `_send_qwen`, `_send_llama`, `_send_llama_native`) already return `Result[str]` (refactored in commits `0282f9ff`, `943a21bf`, `e384afce`, `64d6ba2d`)
|
||||
- **The 2 in-flight `_api_generate` and `_handle_request_event` migrations in `app_controller.py`** — already done by `doeh_test_thinking_cleanup_20260615` (commits `24ba2499` and `7b323e3e`)
|
||||
- **`test_ai_client_result.py::test_send_result_does_not_emit_deprecation`** — passes; the deprecation warning filter works
|
||||
- **11 test mock fixes from `doeh_test_thinking_cleanup_20260615`** — 29/29 tests in 5 files (`test_grok_provider`, `test_llama_provider`, `test_llama_ollama_native`, `test_ai_client_tool_loop_builder`, `test_headless_service`) now use the `Result` API
|
||||
- **UI Polish Phase 1 (markdown tables) — `src/markdown_table.py`** — shipped by commit `79ac9210`
|
||||
- **UI Polish Phase 2 (Keep Pairs input)** — code fix shipped by user commit `d0b06575` (`src/gui_2.py:5130-5131`); test bug remains (this track fixes)
|
||||
- **UI Polish Phase 3 (Refresh Registry)** — code fix shipped by user commit `df7bda6e` (`src/gui_2.py:2111-2112`); test bug remains (this track fixes)
|
||||
- **UI Polish Phase 4 (Vendor State tab)** — shipped by commit `3a864076` (`src/vendor_state.py`)
|
||||
- **UI Polish Phase 5 (Files & Media directory tree)** — shipped by commit `74e02485` (`src/gui_2.py:render_files_and_media`)
|
||||
|
||||
---
|
||||
|
||||
## 2. Goals
|
||||
|
||||
### 2.1 Functional Goals
|
||||
|
||||
| ID | Goal | Acceptance Criterion |
|
||||
|---|---|---|
|
||||
| **G1** | Migrate `src/conductor_tech_lead.py:68` to `send_result()` + Result handling | `uv run pytest tests/test_conductor_tech_lead.py` (or nearest tests) passes; no regression in tier-2 dispatch |
|
||||
| **G2** | Migrate `src/orchestrator_pm.py:86` to `send_result()` + Result handling | No regression in tier-1 dispatch tests |
|
||||
| **G3** | Migrate `src/multi_agent_conductor.py:591` to `send_result()` + Result handling | `test_mma_concurrent_tracks_sim`, `test_mma_step_mode_sim`, `test_undo_redo_sim`, and 30+ MMA live_gui tests pass |
|
||||
| **G4** | Migrate 12 test files using `ai_client.send()` to use `send_result()` | All migrated tests pass; no test calls `ai_client.send()` after this track |
|
||||
| **G5** | Fix `test_qwen_provider.py` (2 tests) to use `Result` API assertion pattern | 2/2 tests pass; same approach as `doeh_test_thinking_cleanup_20260615` used for grok/llama |
|
||||
| **G6** | Fix `test_symbol_parsing.py` (2 tests) to mock `send_result` not `send` | 2/2 tests pass |
|
||||
| **G7** | Fix `test_discussion_truncate_layout.py` (1 test) to use `rfind()` not `find()` | 1/1 test passes |
|
||||
| **G8** | Fix `test_log_management_refresh.py` (1 test) to use `rfind()` not `find()` | 1/1 test passes |
|
||||
| **G9** | Remove `@deprecated` decorator + legacy `send()` function in `src/ai_client.py` | `ai_client.send` AttributeError if called; `rg "ai_client\.send\(" src/ tests/` returns 0 hits |
|
||||
| **G10** | Delete `tests/test_deprecation_warnings.py` (2 obsolete tests) | File removed; no test imports or calls `ai_client.send` |
|
||||
| **G11** | Remove `filterwarnings` entry in `pyproject.toml:46-47` | `rg "ignore:Use ai_client.send_result" pyproject.toml` returns 0 hits |
|
||||
| **G12** | Update `docs/guide_ai_client.md` to remove deprecation references | No `@deprecated` mention; Result API section no longer notes "send() is deprecated" |
|
||||
| **G13** | Update `conductor/product-guidelines.md` to remove deprecation language | No "send() is deprecated; use send_result()" in product guidelines |
|
||||
|
||||
### 2.2 Non-Functional Goals
|
||||
|
||||
| ID | Goal | Acceptance Criterion |
|
||||
|---|---|---|
|
||||
| **NF1** | Zero new test regressions | `uv run pytest tests/` shows 4 fewer failures than the pre-track baseline (10 - 6 = 4 remaining; all RAG) |
|
||||
| **NF2** | All 28 production changes atomic per-task | 28 git commits; each commit is buildable + testable |
|
||||
| **NF3** | All changes follow the project's 1-space indentation, no-comments, type-hinting rules | `uv run python -c "import ast; ast.parse(open('src/ai_client.py').read())"` succeeds; production code has zero `#` comments in changed lines |
|
||||
| **NF4** | Per-commit git notes attached | `git log --format='%H %s' --grep="^public_api_migration_and_ui_polish_20260615" \| wc -l` matches task count |
|
||||
| **NF5** | `doeh_test_thinking_cleanup_20260615` state.toml remains parseable | `python -c "import tomllib; tomllib.load(open('conductor/tracks/doeh_test_thinking_cleanup_20260615/state.toml','rb'))"` succeeds |
|
||||
|
||||
---
|
||||
|
||||
## 3. Per-File Design
|
||||
|
||||
### 3.1 Production call sites to migrate
|
||||
|
||||
**Why these 3 only:** `data_oriented_error_handling_20260606` spec §12.1 lists 5 production call sites. Two of the five (`src/app_controller.py:282` and `src/app_controller.py:3674`) were already migrated by `doeh_test_thinking_cleanup_20260615` (commits `7b323e3e` and `24ba2499`). One was a misidentification — `src/mcp_client.py:2274` is an MCP tool schema for `py_check_syntax`, not a `send()` call. The remaining 3 are real.
|
||||
|
||||
| File:Line | Current code | After this track | Difficulty |
|
||||
|---|---|---|---|
|
||||
| `src/conductor_tech_lead.py:68` | `response = ai_client.send(md_content="", user_message=user_message)` (2-arg) | `result = ai_client.send_result(md_content="", user_message=user_message); if not result.ok: <log warn + return None>; response = result.data` | Easy (2-arg call) |
|
||||
| `src/orchestrator_pm.py:86` | `response = ai_client.send(md_content="", user_message=user_message, enable_tools=False)` (3-arg) | `result = ai_client.send_result(md_content="", user_message=user_message, enable_tools=False); if not result.ok: <log warn + return None>; response = result.data` | Easy (3-arg call) |
|
||||
| `src/multi_agent_conductor.py:591` | `response = ai_client.send(md_content=..., user_message=..., base_dir=".", pre_tool_callback=..., qa_callback=..., patch_callback=..., stream_callback=...)` (8-arg, with 5 callbacks) | `result = ai_client.send_result(md_content=..., ...); if not result.ok: <log warn via comms + return per-ticket error>; response = result.data` | Hard (5 callbacks; per-ticket error handling needed in MMA) |
|
||||
|
||||
**MMA per-ticket error handling:** the existing `_handle_request_event` pattern in `app_controller.py:3674` (already migrated by `doeh_test_thinking_cleanup_20260615`) uses `raise HTTPException(status_code=502, detail=err.ui_message())`. The MMA worker does not have an HTTP layer; the per-ticket error should be:
|
||||
- Logged to the comms log as `WARN/deprecated_send_with_errors` (or `WARN/worker_send_failed`)
|
||||
- Returned via `worker_comms_callback` as a status entry (per `multi_agent_conductor.py:584` callback)
|
||||
- The worker exits with a non-zero status so the DAG engine marks the ticket as failed
|
||||
|
||||
**Reference:** `conductor/tracks/data_oriented_error_handling_20260606/spec.md` §12.1 lines 677-688; `doeh_test_thinking_cleanup_20260615/spec.md` §3.1 (the G1 fix pattern at `src/app_controller.py:265-295` is the canonical reference for Result handling).
|
||||
|
||||
### 3.2 Test files using `ai_client.send()` to migrate
|
||||
|
||||
**Why 12 not 63:** the parent spec claimed "63 test files (verified 2026-06-11)". The current count (rg verified 2026-06-15) is **12 files with 20 call sites**. The discrepancy is because the spec was written when there were more legacy call sites; `data_oriented_error_handling_20260606` Phase 3 + `doeh_test_thinking_cleanup_20260615` Phase 2 already migrated the rest.
|
||||
|
||||
| File | Call sites | Migration pattern |
|
||||
|---|---|---|
|
||||
| `tests/test_ai_client_cli.py` | 1 | `response = ai_client.send(...)` → `result = ai_client.send_result(...); assert result.ok; response = result.data` |
|
||||
| `tests/test_ai_cache_tracking.py` | 1 | Same pattern |
|
||||
| `tests/test_ai_client_result.py` | 3 | One is the existing `test_send_deprecated_emits_warning` (will be DELETED in Phase 6); the other two are pre-existing tests that test `send()` directly — they need to be rewritten to test `send_result()` semantics |
|
||||
| `tests/test_api_events.py` | 2 | Same pattern |
|
||||
| `tests/test_deepseek_provider.py` | 6 | Same pattern (per-call-site migration; 6 commits is too many; consolidate to 1-2 commits) |
|
||||
| `tests/test_gemini_cli_edge_cases.py` | 1 | Same pattern |
|
||||
| `tests/test_gemini_cli_integration.py` | 2 | Same pattern |
|
||||
| `tests/test_gemini_cli_parity_regression.py` | 1 | Same pattern |
|
||||
| `tests/test_gui2_mcp.py` | 1 | Same pattern |
|
||||
| `tests/test_tier4_interceptor.py` | 1 | Same pattern |
|
||||
| `tests/test_token_usage.py` | 1 | Same pattern |
|
||||
| **Total** | **20 call sites in 11 files** | The 12th file is `test_symbol_parsing.py` which mocks `send` not calls it; handled separately in §3.4 |
|
||||
|
||||
**Migration pattern (canonical):**
|
||||
```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"
|
||||
```
|
||||
|
||||
**Special case: `test_ai_client_result.py`:** The current file has 3 tests for the deprecated `send()`. The track DELETES the `test_send_deprecated_emits_warning` test (send() is removed in Phase 6) and KEEPS the `test_send_result_does_not_emit_deprecation` test (it remains a regression test for the new API). The 3rd test (`test_send_result_does_not_emit_deprecation` is the 2nd) needs review — see the file directly.
|
||||
|
||||
**Test isolation:** Group migration by file. Per-file atomic commits preserve the file as a rollback unit. 11 files = 11 atomic commits (consolidate `test_ai_client_result.py` and `test_deepseek_provider.py` since they have multiple sites per file).
|
||||
|
||||
**Reference:** `doeh_test_thinking_cleanup_20260615/plan.md` Phase 2 (Tasks 2.1-2.5) for the exact migration pattern; `conductor/code_styleguides/error_handling.md` §3.1 (AND over OR pattern).
|
||||
|
||||
### 3.3 `test_qwen_provider.py` fix (2 tests)
|
||||
|
||||
**Current state (verified 2026-06-15):**
|
||||
- `_send_qwen()` returns `Result[str]` (refactored by `data_oriented_error_handling_20260606` commit `64d6ba2d`)
|
||||
- Tests at `tests/test_qwen_provider.py:17-19` and `:27-28` assert against raw `str`:
|
||||
```python
|
||||
result = ai_client._send_qwen("system", "user", ".", None, "", False, None, None, None)
|
||||
assert result == "hi from qwen" # FAILS: result is Result(data="hi from qwen")
|
||||
```
|
||||
|
||||
**Fix:** Mirror the pattern used by `doeh_test_thinking_cleanup_20260615` for `test_grok_provider`, `test_llama_provider`, `test_llama_ollama_native`:
|
||||
```python
|
||||
result = ai_client._send_qwen("system", "user", ".", None, "", False, None, None, None)
|
||||
assert result.ok and result.data == "hi from qwen"
|
||||
```
|
||||
|
||||
And for the image test:
|
||||
```python
|
||||
result = ai_client._send_qwen("system", "describe this image", ".", file_items, "", False, None, None, None)
|
||||
assert result.ok and "cat" in result.data.lower()
|
||||
```
|
||||
|
||||
**Why this approach and not renaming `_send_qwen` → `_send_qwen_result`:** the parent spec at line 611 planned the rename, but commit `64d6ba2d` only changed the return type (not the name). The function name `_send_qwen` is stable; only the return type changed. Migrating the tests to handle `Result` is the right scope for this track. A future "rename to `_send_qwen_result`" track could be planned separately if needed.
|
||||
|
||||
**Test isolation:** 1 atomic commit for both test fixes (per-file atomicity).
|
||||
|
||||
**Reference:** `doeh_test_thinking_cleanup_20260615/spec.md` §1.1 (G2-G11 test mock bugs); `doeh_test_thinking_cleanup_20260615/plan.md` Phase 2.1-2.3 (the grok/llama/llama_native patterns).
|
||||
|
||||
### 3.4 `test_symbol_parsing.py` fix (2 tests)
|
||||
|
||||
**Current state (verified 2026-06-15):**
|
||||
- `tests/test_symbol_parsing.py:45,74` mock `src.ai_client.send`
|
||||
- Production now calls `src.ai_client.send_result` (per the migration done by `doeh_test_thinking_cleanup_20260615` commit `24ba2499`)
|
||||
- Mock receives 0 calls; test fails with `Expected 'send' to have been called once. Called 0 times.`
|
||||
|
||||
**Fix:**
|
||||
```python
|
||||
# Before:
|
||||
with patch('src.ai_client.send') as mock_send:
|
||||
...
|
||||
mock_send.assert_called_once()
|
||||
|
||||
# After:
|
||||
with patch('src.ai_client.send_result') as mock_send_result:
|
||||
mock_send_result.return_value = Result(data="mocked response")
|
||||
...
|
||||
mock_send_result.assert_called_once()
|
||||
```
|
||||
|
||||
**Test isolation:** 1 atomic commit for both test fixes (per-file atomicity).
|
||||
|
||||
**Reference:** `doeh_test_thinking_cleanup_20260615/plan.md` Task 2.7 (the headless_service `test_generate_endpoint` mock migration is the canonical reference).
|
||||
|
||||
### 3.5 UI Polish test fixes (2 tests)
|
||||
|
||||
**Current state (verified 2026-06-15):**
|
||||
|
||||
The UI Polish Five Issues track (`docs/superpowers/specs/2026-06-03-ui-polish-design.md`) has 5 phases. Per the code audit (2026-06-15):
|
||||
|
||||
| Phase | Status | Code location | Test status |
|
||||
|---|---|---|---|
|
||||
| 1. Markdown tables | SHIPPED (commit `79ac9210`) | `src/markdown_table.py` | passing |
|
||||
| 2. Keep Pairs input | SHIPPED (user commit `d0b06575`) | `src/gui_2.py:5130-5131` (now `set_next_item_width(140)` + `drag_int`) | FAILING (test bug — see below) |
|
||||
| 3. Refresh Registry | SHIPPED (user commit `df7bda6e`) | `src/gui_2.py:2111-2112` (in-place `load_registry()`) | FAILING (test bug — see below) |
|
||||
| 4. Vendor State tab | SHIPPED (commit `3a864076`) | `src/vendor_state.py` | passing |
|
||||
| 5. Files & Media directory tree | SHIPPED (commit `74e02485`) | `src/gui_2.py:render_files_and_media` | passing |
|
||||
|
||||
**Test bug in Phase 2 (`test_discussion_truncate_layout.py`):**
|
||||
```python
|
||||
def test_keep_pairs_input_uses_adequate_width():
|
||||
src = inspect.getsource(gui_2)
|
||||
marker = "Keep Pairs:"
|
||||
idx = src.find(marker) # ← BUG: finds comment block at line 5113
|
||||
assert idx != -1, "Could not find Keep Pairs label in gui_2.py"
|
||||
snippet = src[idx:idx + 200] # ← snippet window doesn't reach line 5130
|
||||
assert "set_next_item_width(80)" not in snippet, ... # passes (vacuously)
|
||||
assert "set_next_item_width(140)" in snippet, ... # FAILS: snippet ends at the comment
|
||||
assert "drag_int" in snippet, ... # FAILS: snippet ends at the comment
|
||||
```
|
||||
|
||||
The first occurrence of "Keep Pairs:" is in a comment at line 5113 (in the docstring of `render_discussion_entry_controls`). The actual code is at line 5130. The 200-char snippet window only reaches into the docstring.
|
||||
|
||||
**Fix:** Use `rfind()` instead of `find()` to find the LAST occurrence (the actual code):
|
||||
```python
|
||||
def test_keep_pairs_input_uses_adequate_width():
|
||||
src = inspect.getsource(gui_2)
|
||||
marker = "Keep Pairs:"
|
||||
idx = src.rfind(marker) # ← finds the code at line 5130
|
||||
assert idx != -1, "Could not find Keep Pairs label in gui_2.py"
|
||||
snippet = src[idx:idx + 200] # ← snippet now includes line 5130-5131
|
||||
assert "set_next_item_width(80)" not in snippet, ...
|
||||
assert "set_next_item_width(140)" in snippet, ... # passes
|
||||
assert "drag_int" in snippet, ... # passes
|
||||
```
|
||||
|
||||
**Test bug in Phase 3 (`test_log_management_refresh.py`):**
|
||||
```python
|
||||
def test_refresh_registry_button_calls_load_registry():
|
||||
src = inspect.getsource(gui_2)
|
||||
marker = "Refresh Registry"
|
||||
idx = src.find(marker) # ← BUG: finds comment block at line 2090
|
||||
assert idx != -1, "Could not find Refresh Registry button in gui_2.py"
|
||||
snippet = src[idx:idx + 400] # ← snippet window doesn't reach line 2111
|
||||
assert "load_registry" in snippet, ... # FAILS
|
||||
```
|
||||
|
||||
The first occurrence of "Refresh Registry" is in a comment at line 2090. The actual code is at line 2111. The 400-char snippet window doesn't reach the code.
|
||||
|
||||
**Fix:** Same pattern — use `rfind()` to find the actual code:
|
||||
```python
|
||||
def test_refresh_registry_button_calls_load_registry():
|
||||
src = inspect.getsource(gui_2)
|
||||
marker = "Refresh Registry"
|
||||
idx = src.rfind(marker) # ← finds the code at line 2111
|
||||
assert idx != -1, "Could not find Refresh Registry button in gui_2.py"
|
||||
snippet = src[idx:idx + 400]
|
||||
assert "load_registry" in snippet, ... # passes
|
||||
assert snippet.count("log_registry.LogRegistry(") <= 1, ... # passes
|
||||
```
|
||||
|
||||
**Test isolation:** 1 atomic commit for both test fixes (per-file atomicity; they're both 1-character changes in the same test fixture style).
|
||||
|
||||
**Reference:** `docs/superpowers/specs/2026-06-03-ui-polish-design.md` §3.2 (Phase 2 design) and §3.3 (Phase 3 design).
|
||||
|
||||
### 3.6 Deprecation removal
|
||||
|
||||
**Files to modify:**
|
||||
|
||||
1. **`src/ai_client.py:2939-3040`** — Remove the `@deprecated` decorator on `def send(...)` and the entire function body. The function is replaced by `send_result()` (which already exists at `src/ai_client.py:3002`).
|
||||
- Verify: `rg "def send\(" src/ai_client.py` returns 0 hits (only `def send_result(` should remain).
|
||||
|
||||
2. **`tests/test_deprecation_warnings.py`** — Delete the file. Both tests are obsolete:
|
||||
- `test_send_deprecated_warning_emitted_once_per_site` — tests `send()`; can't run after `send()` is removed
|
||||
- `test_send_result_does_not_emit_deprecation` — tests `send_result()` doesn't emit a deprecation; trivially true after `send()` is removed (no deprecation source)
|
||||
|
||||
3. **`pyproject.toml:46-47`** — Remove the `filterwarnings` entry:
|
||||
```toml
|
||||
filterwarnings = [
|
||||
"ignore:Use ai_client.send_result.*:DeprecationWarning", # DELETE THIS LINE
|
||||
]
|
||||
```
|
||||
- Verify: `rg "ignore:Use ai_client.send_result" pyproject.toml` returns 0 hits.
|
||||
|
||||
**Test isolation:** 1 atomic commit for the 3 changes (consecutive cleanup; the changes are meaningless without each other).
|
||||
|
||||
**Reference:** `conductor/tracks/data_oriented_error_handling_20260606/spec.md` §3.5 (deprecation strategy); `pyproject.toml:46-47` (current entry).
|
||||
|
||||
### 3.7 Documentation updates
|
||||
|
||||
**1. `docs/guide_ai_client.md` — remove deprecation references:**
|
||||
|
||||
Search for "deprecat" (case-insensitive) and remove:
|
||||
- "Use ai_client.send_result() instead" mentions
|
||||
- "The deprecated send() will be removed in..." warnings
|
||||
- The entire deprecation warning table at the bottom of the `send_result` section
|
||||
|
||||
**2. `conductor/product-guidelines.md` — remove deprecation language:**
|
||||
|
||||
Search for "deprecat" (case-insensitive) and remove or update:
|
||||
- "send() is deprecated" mentions
|
||||
- "Use send_result()" instructions (the deprecation is being removed)
|
||||
- Update the "Public API deprecation" section to mark as resolved
|
||||
|
||||
**Test isolation:** 1 atomic commit for the 2 doc updates (consecutive cleanup).
|
||||
|
||||
**Reference:** `conductor/product-guidelines.md` "Data-Oriented Error Handling > Public API deprecation" section (search for the heading; mark as RESOLVED).
|
||||
|
||||
---
|
||||
|
||||
## 4. Architecture Reference
|
||||
|
||||
### 4.1 The Result API (Fleury Pattern)
|
||||
|
||||
The `Result[T, ErrorInfo]` pattern from `conductor/code_styleguides/error_handling.md` is the foundation. This track is the **removal of the deprecation** that the data_oriented_error_handling track introduced; the new API is the permanent one.
|
||||
|
||||
**Key files:**
|
||||
- `src/result_types.py` — `Result`, `ErrorInfo`, `ErrorKind`, `NilPath`, `NilRAGState`
|
||||
- `src/ai_client.py:3002` — `def send_result(...)` (the permanent public API after this track)
|
||||
- `src/ai_client.py:_send_<vendor>()` (6 vendors) — return `Result[str]`
|
||||
|
||||
**Per-call-site error handling pattern (canonical):**
|
||||
```python
|
||||
result = ai_client.send_result(md_content, user_message, base_dir, ...)
|
||||
if not result.ok:
|
||||
err = result.errors[0]
|
||||
# call-site-specific error handling:
|
||||
# - HTTP layer (app_controller:_api_generate): raise HTTPException(502, detail=err.ui_message())
|
||||
# - GUI layer (app_controller:_handle_request_event): log to comms + add error entry
|
||||
# - MMA worker (multi_agent_conductor): log to comms + return per-ticket error
|
||||
# - Tier 1/2 sub-agents (orchestrator_pm, conductor_tech_lead): log warn + return None or empty
|
||||
response = result.data
|
||||
```
|
||||
|
||||
### 4.2 The deprecated send() function
|
||||
|
||||
Per `conductor/tracks/data_oriented_error_handling_20260606/spec.md` §3.5 lines 183-206, the `send()` function:
|
||||
- Was added in `data_oriented_error_handling_20260606` Phase 3 (commit `73cf321c`)
|
||||
- Wraps `send_result()` and unwraps the `Result` to return `str`
|
||||
- Is marked `@deprecated` via `typing_extensions.deprecated` (Python 3.11+ backport)
|
||||
- Emits a `DeprecationWarning` at runtime (cached per call site)
|
||||
|
||||
The `filterwarnings` entry in `pyproject.toml:46-47` silences the warning during the transition period. This track removes both the function and the filter entry.
|
||||
|
||||
### 4.3 Threading & Locking
|
||||
|
||||
The production call site migrations MUST preserve the existing locking:
|
||||
- `multi_agent_conductor.py:591` — runs in a worker thread; the `set_comms_log_callback` and `set_current_tier` calls before the `send()` call MUST be preserved
|
||||
- `orchestrator_pm.py:86` — runs in the orchestrator thread; lock acquisition patterns must be preserved
|
||||
- `conductor_tech_lead.py:68` — runs in a sub-agent thread; the `set_custom_system_prompt` and `set_current_tier` calls before the `send()` call MUST be preserved
|
||||
|
||||
**Reference:** `docs/guide_ai_client.md` "Threading Model" section; `docs/guide_app_controller.md` "AI Loop Lifecycle" section.
|
||||
|
||||
### 4.4 The MMA per-ticket error handling
|
||||
|
||||
The MMA worker (`multi_agent_conductor.py:run_worker_lifecycle`) currently does NOT have per-ticket error handling — it expects `send()` to return a `str` (and raises an exception on internal errors which the worker catches). After this track, `send_result()` returns a `Result[str]` with the errors in `result.errors`. The migration must:
|
||||
|
||||
1. Check `result.ok` immediately after the call
|
||||
2. If `!result.ok`:
|
||||
- Log the error to the comms log via `worker_comms_callback` (status entry with `err.ui_message()`)
|
||||
- Return a sentinel value that the DAG engine marks as failed (e.g., return `None` and the worker exits with non-zero status)
|
||||
3. If `result.ok`: continue with `result.data` as before
|
||||
|
||||
**Reference:** `docs/guide_mma.md` "Worker Lifecycle" section; the `multi_agent_conductor.py:584` `worker_comms_callback` (already wired up).
|
||||
|
||||
---
|
||||
|
||||
## 5. Test Plan
|
||||
|
||||
### 5.1 Per-phase test verification
|
||||
|
||||
Each phase must pass targeted tests before moving to the next:
|
||||
|
||||
| Phase | Test command | Expected |
|
||||
|---|---|---|
|
||||
| 1 | `uv run pytest tests/test_conductor_tech_lead.py tests/test_orchestrator_pm.py 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.log` | All pass |
|
||||
| 2 | `uv run pytest tests/test_ai_client_cli.py tests/test_ai_cache_tracking.py tests/test_ai_client_result.py tests/test_api_events.py tests/test_deepseek_provider.py tests/test_gemini_cli_*.py tests/test_gui2_mcp.py tests/test_tier4_interceptor.py tests/test_token_usage.py -v 2>&1 \| tee tests/artifacts/public_api_phase2.log` | All pass |
|
||||
| 3 | `uv run pytest tests/test_qwen_provider.py -v` | 5/5 pass (2 of which were the pre-existing failures) |
|
||||
| 4 | `uv run pytest tests/test_symbol_parsing.py -v` | 2/2 pass (which were the pre-existing failures) |
|
||||
| 5 | `uv run pytest tests/test_discussion_truncate_layout.py tests/test_log_management_refresh.py -v` | 2/2 pass (which were the pre-existing failures) |
|
||||
| 6 | `uv run pytest tests/test_deprecation_warnings.py -v 2>&1` (should fail — file is deleted) + `uv run rg "ai_client\.send\(" src/ tests/` (should return 0) | File deleted; 0 rg hits |
|
||||
| 7 | `uv run pytest tests/ 2>&1 \| tee tests/artifacts/public_api_phase7_full.log` | 4 fewer failures than pre-track (10 - 6 = 4 RAG failures remain) |
|
||||
|
||||
### 5.2 Per-task TDD red verification
|
||||
|
||||
For each task that introduces a new test, the implementer MUST:
|
||||
1. Verify the test FAILS as expected (red phase)
|
||||
2. Implement the fix
|
||||
3. Verify the test PASSES (green phase)
|
||||
4. Commit
|
||||
|
||||
**Anti-pattern guard:** per `AGENTS.md` "Critical Anti-Patterns", no skipping tests just because they fail. If a test fails for an unexpected reason, the implementer MUST investigate before committing.
|
||||
|
||||
### 5.3 Test isolation
|
||||
|
||||
Per `docs/guide_testing.md` "Structural Testing Contract":
|
||||
- No `unittest.mock.patch` on core infrastructure (event queues, `ai_client` internals, threading primitives) unless explicitly authorized
|
||||
- All integration tests use `live_gui` fixture
|
||||
- Test artifacts in `tests/artifacts/` or `tests/logs/` (gitignored)
|
||||
|
||||
This track's tests are mostly UNIT tests (no `live_gui` needed). The MMA migration test (Phase 1) MAY need `live_gui` for the worker dispatch path; verify by running targeted tests first.
|
||||
|
||||
---
|
||||
|
||||
## 6. Migration Strategy
|
||||
|
||||
### 6.1 The order matters
|
||||
|
||||
**Phase 1 must complete before Phase 6:**
|
||||
- Phase 1 migrates the 3 production call sites to `send_result()`
|
||||
- Phase 6 removes the legacy `send()` function
|
||||
- If Phase 6 runs first, the production code (still using `send()`) crashes
|
||||
|
||||
**Phase 2 must complete before Phase 6:**
|
||||
- Phase 2 migrates the 12 test files to `send_result()`
|
||||
- Phase 6 removes the legacy `send()` function
|
||||
- If Phase 6 runs first, the tests (still using `send()`) crash
|
||||
|
||||
**Phase 3, 4, 5 can run in any order after Phase 1** (they're independent test fixes).
|
||||
|
||||
**Phase 7 is the final sweep** (docs + tracks.md + full suite).
|
||||
|
||||
### 6.2 Per-commit safety
|
||||
|
||||
Each atomic commit must:
|
||||
- Be buildable (`python -c "import src.ai_client"` succeeds)
|
||||
- Pass its targeted tests
|
||||
- Not introduce a regression in the previously-passing tests
|
||||
- Have a clear commit message with the task number
|
||||
|
||||
The per-task commit pattern (per `conductor/workflow.md`):
|
||||
```
|
||||
fix(ai_client): migrate conductor_tech_lead.py:68 to send_result() (G1, public_api_migration_and_ui_polish_20260615 Phase 1.1)
|
||||
```
|
||||
|
||||
The per-phase checkpoint pattern:
|
||||
```
|
||||
conductor(checkpoint): Phase 1 complete - 3 production call sites migrated
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 7. Out of Scope
|
||||
|
||||
### 7.1 Deferred to separate tracks
|
||||
|
||||
| ID | Item | Defer to | Why |
|
||||
|---|---|---|---|
|
||||
| OOS1 | 4 RAG test failures (test_rag_integration, test_rag_phase4_final_verify, test_rag_phase4_stress, test_rag_visual_sim) | RAG subsystem track (planned; not yet specced) | Pre-existing RAG subsystem issues; error is in RAG config lookup code, not AI client code. A partial fix was attempted in commit `16412ad5`; the remaining issue is a different code path. |
|
||||
| OOS2 | The `_send_<vendor>()` → `_send_<vendor>_result()` rename per the data_oriented_error_handling spec §3.4 line 611 | Separate "private API rename" track (if needed) | Not blocking; tests work with current names. The function names are stable; only the return type changed. |
|
||||
| OOS3 | The 23 lower-impact files with weak types (per `data_structure_strengthening_20260606/spec.md` §1 line 20) | `data_structure_strengthening_20260606` (the next major track after this) | That's exactly what data_structure_strengthening is for. |
|
||||
| OOS4 | The 4 remaining UI Polish track phases that ARE NOT in this scope (none — all 5 are either shipped or addressed by this track's test fixes) | N/A | All 5 UI Polish phases are accounted for. |
|
||||
| OOS5 | `live_gui_mock_injection_20260615` infrastructure | Separate infrastructure track | Not blocking. Recommended but not required. |
|
||||
|
||||
### 7.2 Explicitly NOT in this track
|
||||
|
||||
- **Renaming `_send_<vendor>()` to `_send_<vendor>_result()`** — not needed; tests work with current names after assertion pattern fix
|
||||
- **Adding TypedDict / @dataclass schemas** — that's data_structure_strengthening's scope
|
||||
- **MMA per-ticket Result returns (per `data_oriented_error_handling_20260606/spec.md` §12.1 line 677 "Adds any new public API surface needed (e.g., per-ticket Result returns in the MMA conductor)")** — the MMA worker already gets `Result[str]` from `send_result()`; the existing `worker_comms_callback` already handles per-ticket status updates. The spec's mention of "per-ticket Result returns" was speculative; the current Result-based flow is sufficient.
|
||||
- **Removing the `filterwarnings` for the `Optional[T]` ban** — the `audit_optional_in_3_files.py` audit (per `data_oriented_error_handling_20260606/spec.md`) is unrelated to this track's deprecation removal.
|
||||
|
||||
---
|
||||
|
||||
## 8. Risks & Mitigations
|
||||
|
||||
| ID | Risk | Likelihood | Impact | Mitigation |
|
||||
|---|---|---|---|---|
|
||||
| **R1** | `multi_agent_conductor.py:591` migration breaks MMA worker dispatch (5 callbacks) | Medium | High | TDD red first: verify a known MMA test fails before the fix; verify it passes after. The existing `doeh_test_thinking_cleanup_20260615` G1 fix pattern is the canonical reference for Result handling. |
|
||||
| **R2** | Removing `send()` breaks a test that imports it indirectly | Low | Medium | Run `rg "ai_client\.send\(" src/ tests/` before AND after Phase 6 to confirm 0 hits. |
|
||||
| **R3** | `pyproject.toml` filterwarnings removal causes test suite to fail with `DeprecationWarning` (e.g., from another library) | Low | Low | The filter was added in `data_oriented_error_handling_20260606` specifically to silence `send()` deprecation; no other deprecation in the codebase is silenced by it. Verified by checking the rg history. |
|
||||
| **R4** | UI Polish test fixes (`find()` → `rfind()`) mask a real production bug | Low | Medium | The production code at `src/gui_2.py:5130-5131` and `:2111-2112` was already verified to have the correct values (`set_next_item_width(140)` + `drag_int` and in-place `load_registry()`). The test bug is just the search logic. |
|
||||
| **R5** | Qwen test fix uses a different pattern than grok/llama/llama_native | Low | Low | The plan uses the same `assert result.ok and result.data == "x"` pattern as `doeh_test_thinking_cleanup_20260615` (commits `d7e42a4a`, `439a0ac0`, `dbdf9ba9`). |
|
||||
| **R6** | `test_deprecation_warnings.py` deletion is misinterpreted as "deleting tests instead of fixing them" | Low | Low | Both tests in the file are obsolete after `send()` removal. The first test (test_send_deprecated) literally cannot run without `send()`. The second test (test_send_result_does_not_emit_deprecation) is trivially true. Document in the commit message. |
|
||||
| **R7** | The 4 RAG test failures get introduced or regressed during this track | Low | Medium | Run full test suite in Phase 7 and compare to the pre-track baseline. The 4 RAG failures are documented as "pre-existing" with their defer-to track recorded. |
|
||||
|
||||
---
|
||||
|
||||
## 9. Verification Criteria (definition of "done")
|
||||
|
||||
The track is DONE when **ALL** of the following are true:
|
||||
|
||||
1. **G1-G3 production migrations complete**: 3 call sites use `send_result()`; no `ai_client.send(` in `src/`
|
||||
2. **G4 test migration complete**: 12 test files use `send_result()`; no `ai_client.send(` in `tests/`
|
||||
3. **G5 Qwen test fix complete**: `test_qwen_provider.py` 5/5 pass
|
||||
4. **G6 symbol_parsing test fix complete**: `test_symbol_parsing.py` 2/2 pass
|
||||
5. **G7-G8 UI Polish test fixes complete**: `test_discussion_truncate_layout.py` 1/1 + `test_log_management_refresh.py` 1/1 pass
|
||||
6. **G9 deprecation removed**: `@deprecated` decorator and `send()` function gone from `src/ai_client.py`
|
||||
7. **G10 test_deprecation_warnings.py deleted**: file does not exist
|
||||
8. **G11 filterwarnings removed**: no `ignore:Use ai_client.send_result` in `pyproject.toml`
|
||||
9. **G12-G13 docs updated**: no `@deprecated` or "send is deprecated" mentions in `docs/guide_ai_client.md` or `conductor/product-guidelines.md`
|
||||
10. **NF1 no regressions**: full test suite has 4 RAG failures remaining (down from 10); no new failures
|
||||
11. **NF2 per-task commits**: ~28 atomic commits with clear messages
|
||||
12. **NF3 style preserved**: 1-space indentation, no comments, type hints in all changed code
|
||||
13. **NF4 per-commit git notes**: all 28 commits have git notes summarizing the task
|
||||
14. **NF5 doeh state.toml parseable**: `tomllib.load()` succeeds (unchanged from previous track; sanity check)
|
||||
15. **Final state**: 1280 + 6 newly-passing = 1286 tests pass; 4 RAG failures documented as deferred
|
||||
|
||||
**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
|
||||
|
||||
---
|
||||
|
||||
## 10. Execution Order & Dependencies
|
||||
|
||||
**No external blockers.** This track can start immediately after the Tier 1 review approves the spec.
|
||||
|
||||
**Execution order (the plan):**
|
||||
1. Phase 1 (production migration) — 1 day
|
||||
2. Phase 2 (test migration, 12 files) — 1 day
|
||||
3. Phase 3 (Qwen test fix) — 1 hour (can be combined with Phase 2)
|
||||
4. Phase 4 (symbol_parsing test fix) — 30 min (can be combined with Phase 2)
|
||||
5. Phase 5 (UI Polish test fixes) — 30 min (independent)
|
||||
6. Phase 6 (deprecation removal) — 30 min (MUST be after Phases 1 + 2)
|
||||
7. Phase 7 (docs + housekeep) — 1 hour (after Phase 6)
|
||||
|
||||
**Total:** 2-3 days Tier 2 work (the estimate accounts for the per-commit overhead + per-task git notes + 7 phase checkpoints).
|
||||
|
||||
**Followed by:** the user can start `data_structure_strengthening_20260606` track (already has spec, plan pending).
|
||||
|
||||
---
|
||||
|
||||
## 11. References
|
||||
|
||||
### Architecture docs
|
||||
- `docs/guide_ai_client.md` — multi-provider LLM client; `send_result()` is the canonical public API
|
||||
- `docs/guide_app_controller.md` — headless controller; `app_controller.py:_handle_request_event` was migrated by `doeh_test_thinking_cleanup_20260615`
|
||||
- `docs/guide_mma.md` — 4-tier MMA orchestration; `multi_agent_conductor.py:run_worker_lifecycle` is the worker entry point
|
||||
- `docs/guide_mcp_client.md` — MCP tool registry (note: `mcp_client.py:2274` was a misidentification in the parent spec)
|
||||
- `docs/guide_testing.md` — `live_gui` fixture + structural testing contract
|
||||
|
||||
### Styleguides
|
||||
- `conductor/code_styleguides/error_handling.md` — `Result[T]` pattern + the AND-over-OR convention
|
||||
- `conductor/code_styleguides/data_oriented_design.md` — canonical DOD reference
|
||||
- `conductor/product-guidelines.md` — 1-space indentation, no comments, type hints, SDM tags
|
||||
|
||||
### Parent tracks
|
||||
- `conductor/tracks/data_oriented_error_handling_20260606/spec.md` §3.5 (deprecation strategy), §12.1 (follow-up scope)
|
||||
- `conductor/tracks/data_oriented_error_handling_20260606/state.toml` — the parent track's state
|
||||
- `conductor/tracks/doeh_test_thinking_cleanup_20260615/spec.md` — the previous track; the migration pattern reference
|
||||
- `conductor/tracks/doeh_test_thinking_cleanup_20260615/plan.md` Phase 2 — exact test mock fix pattern (Tasks 2.1-2.5)
|
||||
- `docs/reports/TRACK_COMPLETION_doeh_test_thinking_cleanup_20260615.md` — the 11 mock fixes that established the pattern
|
||||
|
||||
### UI Polish track
|
||||
- `docs/superpowers/specs/2026-06-03-ui-polish-design.md` — the 5-phase UI Polish spec
|
||||
- `docs/superpowers/plans/2026-06-03-ui-polish.md` — the 5-phase UI Polish plan
|
||||
- User commits: `d0b06575` (Phase 2 code fix), `df7bda6e` (Phase 3 code fix)
|
||||
- Track commits: `79ac9210` (Phase 1), `3a864076` (Phase 4), `74e02485` (Phase 5)
|
||||
|
||||
### Test files (the 12 + 1 to migrate, the 4 UI Polish fixes)
|
||||
- 12 send() test files: `test_ai_client_cli`, `test_ai_cache_tracking`, `test_ai_client_result`, `test_api_events`, `test_deepseek_provider`, `test_gemini_cli_edge_cases`, `test_gemini_cli_integration`, `test_gemini_cli_parity_regression`, `test_gui2_mcp`, `test_tier4_interceptor`, `test_token_usage`, `test_symbol_parsing`
|
||||
- 1 _send_ test file: `test_qwen_provider`
|
||||
- 2 UI Polish test files: `test_discussion_truncate_layout`, `test_log_management_refresh`
|
||||
- 1 file to delete: `test_deprecation_warnings`
|
||||
|
||||
### Production call sites (3 to migrate)
|
||||
- `src/conductor_tech_lead.py:68`
|
||||
- `src/orchestrator_pm.py:86`
|
||||
- `src/multi_agent_conductor.py:591`
|
||||
|
||||
### Codebase locations
|
||||
- `src/ai_client.py:2939-3040` — the deprecated `send()` function (to be deleted)
|
||||
- `src/ai_client.py:3002` — the new `send_result()` public API (kept)
|
||||
- `pyproject.toml:46-47` — the `filterwarnings` entry (to be deleted)
|
||||
- `tests/test_deprecation_warnings.py` — the 2 obsolete tests (to be deleted)
|
||||
- `docs/guide_ai_client.md` — deprecation references (to be removed)
|
||||
- `conductor/product-guidelines.md` — deprecation language (to be removed)
|
||||
@@ -1,91 +0,0 @@
|
||||
# Track state for public_api_migration_and_ui_polish_20260615
|
||||
# Updated by Tier 2 Tech Lead as tasks complete
|
||||
|
||||
[meta]
|
||||
track_id = "public_api_migration_and_ui_polish_20260615"
|
||||
name = "Public API Migration + UI Polish Test Cleanup"
|
||||
status = "completed"
|
||||
current_phase = 7
|
||||
last_updated = "2026-06-15"
|
||||
|
||||
[blocked_by]
|
||||
# No external blockers
|
||||
|
||||
[blocks]
|
||||
data_structure_strengthening_20260606 = "planned in this track"
|
||||
mcp_architecture_refactor_20260606 = "transitively"
|
||||
|
||||
[phases]
|
||||
phase_1 = { status = "completed", checkpointsha = "b7fd4e4f", name = "Production call site migration" }
|
||||
phase_2 = { status = "completed", checkpointsha = "da6e0848", name = "Test file migration" }
|
||||
phase_3 = { status = "completed", checkpointsha = "3be28cc5", name = "Qwen test fix" }
|
||||
phase_4 = { status = "completed", checkpointsha = "effa24a7", name = "Symbol parsing test fix" }
|
||||
phase_5 = { status = "completed", checkpointsha = "c50367c6", name = "UI Polish test fixes" }
|
||||
phase_6 = { status = "completed", checkpointsha = "0e55ebaf", name = "Deprecation removal" }
|
||||
phase_7 = { status = "completed", checkpointsha = "", name = "Docs + housekeep" }
|
||||
|
||||
[tasks]
|
||||
# Phase 1
|
||||
t1_1 = { status = "completed", commit_sha = "bbb3d597", description = "Migrate src/conductor_tech_lead.py:68 to send_result()" }
|
||||
t1_2 = { status = "completed", commit_sha = "7ea802ab", description = "Migrate src/orchestrator_pm.py:86 to send_result()" }
|
||||
t1_3 = { status = "completed", commit_sha = "bdd46299", description = "Migrate src/multi_agent_conductor.py:591 to send_result()" }
|
||||
t1_4 = { status = "completed", commit_sha = "b7fd4e4f", description = "Phase 1 checkpoint" }
|
||||
|
||||
# Phase 2 (11 call-site migrations + 7 production-affected mock migrations)
|
||||
t2_1 = { status = "completed", commit_sha = "ba0df1fa", description = "Migrate test_ai_client_cli.py" }
|
||||
t2_2 = { status = "completed", commit_sha = "fab9196b", description = "Migrate test_ai_cache_tracking.py" }
|
||||
t2_3 = { status = "completed", commit_sha = "b4c9ebd9", description = "Migrate test_gemini_cli_edge_cases.py" }
|
||||
t2_4 = { status = "completed", commit_sha = "fe520243", description = "Migrate test_gemini_cli_parity_regression.py" }
|
||||
t2_5 = { status = "completed", commit_sha = "c59bac59", description = "Migrate test_gui2_mcp.py" }
|
||||
t2_6 = { status = "completed", commit_sha = "1e2c3431", description = "Migrate test_token_usage.py" }
|
||||
t2_7 = { status = "completed", commit_sha = "01929786", description = "Migrate test_ai_client_result.py" }
|
||||
t2_8 = { status = "completed", commit_sha = "d9a79efa", description = "Migrate test_api_events.py" }
|
||||
t2_9 = { status = "completed", commit_sha = "363fe91d", description = "Migrate test_deepseek_provider.py" }
|
||||
t2_10 = { status = "completed", commit_sha = "cfeb3cb3", description = "Migrate test_gemini_cli_integration.py" }
|
||||
t2_11 = { status = "completed", commit_sha = "36962ef6", description = "Migrate test_tier4_interceptor.py" }
|
||||
t2_12 = { status = "completed", commit_sha = "48825452", description = "Migrate test_conductor_tech_lead.py (mock)" }
|
||||
t2_13 = { status = "completed", commit_sha = "953689c8", description = "Migrate test_orchestration_logic.py (mock)" }
|
||||
t2_14 = { status = "completed", commit_sha = "e4a2a204", description = "Migrate test_orchestrator_pm.py (mock)" }
|
||||
t2_15 = { status = "completed", commit_sha = "499762d8", description = "Migrate test_orchestrator_pm_history.py (mock)" }
|
||||
t2_16 = { status = "completed", commit_sha = "bb2add12", description = "Migrate test_phase6_engine.py (mock)" }
|
||||
t2_17 = { status = "completed", commit_sha = "7a6ffd89", description = "Migrate test_run_worker_lifecycle_abort.py (mock)" }
|
||||
t2_18 = { status = "completed", commit_sha = "16c6705b", description = "Migrate test_spawn_interception_v2.py (mock)" }
|
||||
t2_followup_1 = { status = "completed", commit_sha = "64278d53", description = "Wrap test_conductor_engine_v2.py mock returns in Result" }
|
||||
t2_followup_2 = { status = "completed", commit_sha = "58576fc", description = "Wrap test_context_pruner.py lambda mock in Result" }
|
||||
t2_followup_3 = { status = "completed", commit_sha = "26e1b652", description = "Wrap test_rag_integration.py _send_gemini mock in Result" }
|
||||
t2_followup_4 = { status = "completed", commit_sha = "13f32f52", description = "Wrap test_tiered_aggregation.py mock return in Result" }
|
||||
t2_19 = { status = "completed", commit_sha = "da6e0848", description = "Phase 2 checkpoint" }
|
||||
|
||||
# Phase 3
|
||||
t3_1 = { status = "completed", commit_sha = "3be28cc5", description = "Fix test_qwen_provider.py (2 tests)" }
|
||||
t3_2 = { status = "completed", commit_sha = "3be28cc5", description = "Verify no regression" }
|
||||
|
||||
# Phase 4
|
||||
t4_1 = { status = "completed", commit_sha = "effa24a7", description = "Fix test_symbol_parsing.py (2 tests)" }
|
||||
t4_2 = { status = "completed", commit_sha = "effa24a7", description = "Verify no regression" }
|
||||
|
||||
# Phase 5
|
||||
t5_1 = { status = "completed", commit_sha = "f663a34f", description = "Fix test_discussion_truncate_layout.py" }
|
||||
t5_2 = { status = "completed", commit_sha = "c50367c6", description = "Fix test_log_management_refresh.py" }
|
||||
t5_3 = { status = "completed", commit_sha = "c50367c6", description = "Verify no regression" }
|
||||
|
||||
# Phase 6
|
||||
t6_1 = { status = "completed", commit_sha = "8c81b727", description = "Remove send() function from src/ai_client.py" }
|
||||
t6_2 = { status = "completed", commit_sha = "e40b122b", description = "Delete test_deprecation_warnings.py" }
|
||||
t6_3 = { status = "completed", commit_sha = "90122df3", description = "Remove filterwarnings from pyproject.toml" }
|
||||
t6_4 = { status = "completed", commit_sha = "0e55ebaf", description = "Phase 6 checkpoint" }
|
||||
|
||||
# Phase 7
|
||||
t7_1 = { status = "completed", commit_sha = "b37a095b", description = "Update docs/guide_ai_client.md" }
|
||||
t7_2 = { status = "completed", commit_sha = "33fcedef", description = "Update conductor/product-guidelines.md" }
|
||||
t7_3 = { status = "completed", commit_sha = "PENDING", description = "Run full test suite" }
|
||||
t7_4 = { status = "completed", commit_sha = "PENDING", description = "Update metadata.json" }
|
||||
|
||||
[verification]
|
||||
phase_1_production_migration_complete = true
|
||||
phase_2_test_migration_complete = true
|
||||
phase_3_qwen_tests_pass = true
|
||||
phase_4_symbol_parsing_tests_pass = true
|
||||
phase_5_ui_polish_tests_pass = true
|
||||
phase_6_deprecation_removed = true
|
||||
phase_7_docs_complete = true
|
||||
Reference in New Issue
Block a user