From c23966061c4e7c90b8d26585d178212c89b3e204 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Wed, 25 Feb 2026 22:05:28 -0500 Subject: [PATCH] fix(conductor): Apply review suggestions for track 'test_curation_20260225' --- ai_client.py | 2 +- conductor/tracks/test_curation_20260225/plan.md | 3 +++ gui_2.py | 12 ++++++------ tests/test_ai_client_cli.py | 4 +++- tests/test_gemini_cli_adapter.py | 7 +++++-- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/ai_client.py b/ai_client.py index 4155b34..a2f9191 100644 --- a/ai_client.py +++ b/ai_client.py @@ -841,7 +841,7 @@ def _send_gemini_cli(md_content: str, user_message: str, base_dir: str, # Clean up the tool calls format to match comms log expectation log_calls = [] for c in calls: - log_calls.append({"name": c.get("name"), "args": c.get("args")}) + log_calls.append({"name": c.get("name"), "args": c.get("args"), "id": c.get("id")}) _append_comms("IN", "response", { "round": r_idx, diff --git a/conductor/tracks/test_curation_20260225/plan.md b/conductor/tracks/test_curation_20260225/plan.md index 38822a9..65f4afa 100644 --- a/conductor/tracks/test_curation_20260225/plan.md +++ b/conductor/tracks/test_curation_20260225/plan.md @@ -30,3 +30,6 @@ This plan outlines the process for categorizing, organizing, and curating the ex - [x] Task: Verify 100% pass rate for all non-blacklisted tests (Verified) - [x] Task: Generate a final test coverage report (Verified) - [x] Task: Conductor - User Manual Verification 'Phase 4: Final Verification' (Protocol in workflow.md) + +## Phase: Review Fixes +- [~] Task: Apply review suggestions diff --git a/gui_2.py b/gui_2.py index c29a2b9..74a662e 100644 --- a/gui_2.py +++ b/gui_2.py @@ -2294,28 +2294,28 @@ class App: if not tcs: imgui.text_colored(C_VAL, " (none)") for tc_i, tc in enumerate(tcs): imgui.text_colored(C_KEY, f" call[{tc_i}] {tc.get('name', '?')}") - if "id" in tc: + if tc.get("id"): imgui.text_colored(C_LBL, " id:") imgui.same_line() - imgui.text_colored(C_VAL, tc["id"]) + imgui.text_colored(C_VAL, str(tc["id"])) if "args" in tc or "input" in tc: self._render_heavy_text(f"call_{tc_i}_args", str(tc.get("args") or tc.get("input"))) elif k == "tool_call": imgui.text_colored(C_KEY, payload.get("name", "?")) - if "id" in payload: + if payload.get("id"): imgui.text_colored(C_LBL, " id:") imgui.same_line() - imgui.text_colored(C_VAL, payload["id"]) + imgui.text_colored(C_VAL, str(payload["id"])) if "script" in payload: self._render_heavy_text("script", payload["script"]) if "args" in payload: self._render_heavy_text("args", str(payload["args"])) elif k == "tool_result": imgui.text_colored(C_KEY, payload.get("name", "?")) - if "id" in payload: + if payload.get("id"): imgui.text_colored(C_LBL, " id:") imgui.same_line() - imgui.text_colored(C_VAL, payload["id"]) + imgui.text_colored(C_VAL, str(payload["id"])) if "output" in payload: self._render_heavy_text("output", payload["output"]) if "results" in payload: for r_i, r in enumerate(payload["results"]): diff --git a/tests/test_ai_client_cli.py b/tests/test_ai_client_cli.py index 7dfae03..efa8734 100644 --- a/tests/test_ai_client_cli.py +++ b/tests/test_ai_client_cli.py @@ -16,8 +16,10 @@ def test_ai_client_send_gemini_cli(): # 1. Mock 'ai_client.GeminiCliAdapter' (which we will add) with patch('ai_client.GeminiCliAdapter') as MockAdapterClass: mock_adapter_instance = MockAdapterClass.return_value - mock_adapter_instance.send.return_value = test_response + mock_adapter_instance.send.return_value = {"text": test_response, "tool_calls": []} mock_adapter_instance.last_usage = {"total_tokens": 100} + mock_adapter_instance.last_latency = 0.5 + mock_adapter_instance.session_id = "test-session" # Verify that 'events' are emitted correctly with patch.object(ai_client.events, 'emit') as mock_emit: diff --git a/tests/test_gemini_cli_adapter.py b/tests/test_gemini_cli_adapter.py index 43966c8..d050e24 100644 --- a/tests/test_gemini_cli_adapter.py +++ b/tests/test_gemini_cli_adapter.py @@ -75,7 +75,8 @@ class TestGeminiCliAdapter(unittest.TestCase): result = self.adapter.send("test message") - self.assertEqual(result, "The quick brown fox jumps.") + self.assertEqual(result["text"], "The quick brown fox jumps.") + self.assertEqual(result["tool_calls"], []) @patch('subprocess.Popen') def test_send_handles_tool_use_events(self, mock_popen): @@ -100,7 +101,9 @@ class TestGeminiCliAdapter(unittest.TestCase): result = self.adapter.send("read test.txt") # Result should contain the combined text from all 'message' events - self.assertEqual(result, "Calling tool...\nFile read successfully.") + self.assertEqual(result["text"], "Calling tool...\nFile read successfully.") + self.assertEqual(len(result["tool_calls"]), 1) + self.assertEqual(result["tool_calls"][0]["name"], "read_file") @patch('subprocess.Popen') def test_send_captures_usage_metadata(self, mock_popen):