From 7becffcc899d5bad8acfdd7ae79aee7a452ba452 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 22 Feb 2026 10:29:54 +0000 Subject: [PATCH] Fix React hook anti-patterns, incorrect mock paths, and unused code - Replace useState+useEffect sync pattern with useMemo in EdgeSettingsPanel.tsx to eliminate cascading re-renders - Remove redundant draft state in NodeSettingsPanel.tsx and use store data directly, eliminating useEffect sync loop - Fix mock paths in test_tools.py: patch tavily.TavilyClient and pypdf.PdfReader at their source modules (lazy imports) - Remove unused variable assignment in routes.py (god mode reject) - Remove unused node_lookup dicts in dynamic_graph_builder.py - Remove unused imports across test files (Blueprint, CouncilState, pytest, llm assignments) - Remove unused CouncilBlueprint type import in types.test.ts - Run npm audit fix to resolve moderate vulnerability All 107 backend tests and 26 frontend tests pass. Ruff, ESLint, and TypeScript checks are clean. https://claude.ai/code/session_01XqzyT6fhS8sUe9P5fCmuVU --- backend/api/routes.py | 2 +- backend/services/dynamic_graph_builder.py | 4 - backend/tests/test_blueprint_service.py | 2 +- backend/tests/test_dynamic_graph_builder.py | 6 +- backend/tests/test_god_mode.py | 1 - backend/tests/test_routing.py | 1 - backend/tests/test_tools.py | 11 +- frontend/app/__tests__/types.test.ts | 1 - .../components/panels/EdgeSettingsPanel.tsx | 22 +- .../components/panels/NodeSettingsPanel.tsx | 26 +- frontend/package-lock.json | 2321 ++++++++++++++++- 11 files changed, 2332 insertions(+), 65 deletions(-) diff --git a/backend/api/routes.py b/backend/api/routes.py index b909b33..a6ef3c3 100644 --- a/backend/api/routes.py +++ b/backend/api/routes.py @@ -222,7 +222,7 @@ async def approve_god_mode( ) if request.action == "reject": - state = await resume_god_mode(run_id, action="reject") + await resume_god_mode(run_id, action="reject") run_store.update(run_id, {"status": "failed", "error": "Rejected by user in God Mode."}) return CouncilResultResponse( run_id=run_id, diff --git a/backend/services/dynamic_graph_builder.py b/backend/services/dynamic_graph_builder.py index b49bd5e..78d2bdb 100644 --- a/backend/services/dynamic_graph_builder.py +++ b/backend/services/dynamic_graph_builder.py @@ -366,9 +366,6 @@ def build_graph_from_blueprint( if not nodes: raise ValueError("Blueprint has no nodes.") - # Build node lookup - node_lookup = {n["id"]: n for n in nodes} - # Find entry point: the node that has no incoming edges targets = {e["target"] for e in edges} entry_candidates = [n["id"] for n in nodes if n["id"] not in targets] @@ -559,7 +556,6 @@ def _build_graph_with_checkpointer( if not nodes: raise ValueError("Blueprint has no nodes.") - node_lookup = {n["id"]: n for n in nodes} targets = {e["target"] for e in edges} entry_candidates = [n["id"] for n in nodes if n["id"] not in targets] if not entry_candidates: diff --git a/backend/tests/test_blueprint_service.py b/backend/tests/test_blueprint_service.py index 41a232d..6d996c4 100644 --- a/backend/tests/test_blueprint_service.py +++ b/backend/tests/test_blueprint_service.py @@ -13,7 +13,7 @@ import pytest import pytest_asyncio from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine -from models.blueprint import Base, Blueprint +from models.blueprint import Base from services.blueprint_service import ( create_blueprint, delete_blueprint, diff --git a/backend/tests/test_dynamic_graph_builder.py b/backend/tests/test_dynamic_graph_builder.py index 91d6831..6482cc5 100644 --- a/backend/tests/test_dynamic_graph_builder.py +++ b/backend/tests/test_dynamic_graph_builder.py @@ -23,7 +23,7 @@ from services.dynamic_graph_builder import ( _get_llm, ) from services.graph_builder import create_initial_state -from state import CouncilState, APPROVAL_THRESHOLD, MAX_ITERATIONS +from state import APPROVAL_THRESHOLD, MAX_ITERATIONS # --------------------------------------------------------------------------- @@ -321,10 +321,10 @@ class TestModelLookup: def test_claude_model_creates_instance(self): with patch("services.dynamic_graph_builder.ChatAnthropic") as MockLLM: - llm = _get_llm("claude-3-5-sonnet") + _get_llm("claude-3-5-sonnet") MockLLM.assert_called_once() def test_gpt4o_model_creates_instance(self): with patch("services.dynamic_graph_builder.ChatOpenAI") as MockLLM: - llm = _get_llm("gpt-4o") + _get_llm("gpt-4o") MockLLM.assert_called_once() diff --git a/backend/tests/test_god_mode.py b/backend/tests/test_god_mode.py index 7f4c86f..72bf813 100644 --- a/backend/tests/test_god_mode.py +++ b/backend/tests/test_god_mode.py @@ -12,7 +12,6 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) import pytest from unittest.mock import patch, MagicMock -from state import CouncilState class TestBuildGraphGodMode: diff --git a/backend/tests/test_routing.py b/backend/tests/test_routing.py index dfa8378..31bc4cb 100644 --- a/backend/tests/test_routing.py +++ b/backend/tests/test_routing.py @@ -9,7 +9,6 @@ import os sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) -import pytest from unittest.mock import patch, MagicMock from state import CouncilState, APPROVAL_THRESHOLD, MAX_ITERATIONS diff --git a/backend/tests/test_tools.py b/backend/tests/test_tools.py index 2e8bac3..626e786 100644 --- a/backend/tests/test_tools.py +++ b/backend/tests/test_tools.py @@ -9,7 +9,6 @@ import os sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) -import pytest from unittest.mock import patch, MagicMock @@ -24,7 +23,7 @@ class TestWebSearchTool: assert "TAVILY_API_KEY" in result @patch.dict(os.environ, {"TAVILY_API_KEY": "test-key"}, clear=False) - @patch("tools.web_search.TavilyClient") + @patch("tavily.TavilyClient") def test_web_search_returns_formatted_results(self, mock_client_cls): mock_client = MagicMock() mock_client.search.return_value = { @@ -46,7 +45,7 @@ class TestWebSearchTool: assert "Some content here" in result @patch.dict(os.environ, {"TAVILY_API_KEY": "test-key"}, clear=False) - @patch("tools.web_search.TavilyClient") + @patch("tavily.TavilyClient") def test_web_search_handles_empty_results(self, mock_client_cls): mock_client = MagicMock() mock_client.search.return_value = {"results": []} @@ -58,7 +57,7 @@ class TestWebSearchTool: assert "No results" in result @patch.dict(os.environ, {"TAVILY_API_KEY": "test-key"}, clear=False) - @patch("tools.web_search.TavilyClient") + @patch("tavily.TavilyClient") def test_web_search_handles_api_error(self, mock_client_cls): mock_client = MagicMock() mock_client.search.side_effect = Exception("API rate limit") @@ -137,7 +136,7 @@ class TestPdfIngestion: """Tests for PDF ingestion into ChromaDB.""" @patch("tools.pdf_reader._get_chroma_collection") - @patch("tools.pdf_reader.PdfReader") + @patch("pypdf.PdfReader") def test_ingest_pdf_processes_pages(self, mock_pdf_reader_cls, mock_get_collection): # Mock PDF with 2 pages of text mock_page1 = MagicMock() @@ -158,7 +157,7 @@ class TestPdfIngestion: mock_collection.upsert.assert_called_once() @patch("tools.pdf_reader._get_chroma_collection") - @patch("tools.pdf_reader.PdfReader") + @patch("pypdf.PdfReader") def test_ingest_pdf_empty_file(self, mock_pdf_reader_cls, mock_get_collection): mock_reader = MagicMock() mock_reader.pages = [] diff --git a/frontend/app/__tests__/types.test.ts b/frontend/app/__tests__/types.test.ts index eef7e54..f4edb98 100644 --- a/frontend/app/__tests__/types.test.ts +++ b/frontend/app/__tests__/types.test.ts @@ -1,7 +1,6 @@ import { describe, it, expect } from "vitest"; import type { AgentNodeData, - CouncilBlueprint, ExecutionMode, GodModeAction, GodModeState, diff --git a/frontend/app/components/panels/EdgeSettingsPanel.tsx b/frontend/app/components/panels/EdgeSettingsPanel.tsx index c233089..6ac96fd 100644 --- a/frontend/app/components/panels/EdgeSettingsPanel.tsx +++ b/frontend/app/components/panels/EdgeSettingsPanel.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useState } from "react"; +import { useMemo } from "react"; import { X, ArrowRight } from "lucide-react"; import { EdgeType } from "@/app/types/council"; import { useCouncilStore } from "@/app/store/council-store"; @@ -15,15 +15,15 @@ export function EdgeSettingsPanel() { const edge = edges.find((e) => e.id === selectedEdgeId); - const [edgeType, setEdgeType] = useState("linear"); - const [condition, setCondition] = useState(""); - - useEffect(() => { - if (edge) { - setEdgeType((edge.data?.type as EdgeType) ?? "linear"); - setCondition((edge.data?.condition as string) ?? ""); - } - }, [selectedEdgeId, edge]); + // Derive state directly from the store instead of syncing via useEffect + const edgeType = useMemo( + () => (edge?.data?.type as EdgeType) ?? "linear", + [edge?.data?.type] + ); + const condition = useMemo( + () => (edge?.data?.condition as string) ?? "", + [edge?.data?.condition] + ); if (!selectedEdgeId || !edge) return null; @@ -31,12 +31,10 @@ export function EdgeSettingsPanel() { const targetNode = nodes.find((n) => n.id === edge.target); const handleTypeChange = (newType: EdgeType) => { - setEdgeType(newType); updateEdgeData(selectedEdgeId, newType, newType === "conditional" ? condition : undefined); }; const handleConditionChange = (value: string) => { - setCondition(value); updateEdgeData(selectedEdgeId, edgeType, value); }; diff --git a/frontend/app/components/panels/NodeSettingsPanel.tsx b/frontend/app/components/panels/NodeSettingsPanel.tsx index ee71fb5..69af145 100644 --- a/frontend/app/components/panels/NodeSettingsPanel.tsx +++ b/frontend/app/components/panels/NodeSettingsPanel.tsx @@ -1,6 +1,5 @@ "use client"; -import { useEffect, useState } from "react"; import { X, Bot } from "lucide-react"; import { AgentNodeData, LLMModel } from "@/app/types/council"; import { useCouncilStore } from "@/app/store/council-store"; @@ -21,18 +20,9 @@ export function NodeSettingsPanel() { const node = nodes.find((n) => n.id === selectedNodeId); const data = node?.data as AgentNodeData | undefined; - // Local draft to avoid re-renders on every keystroke - const [draft, setDraft] = useState(null); - - useEffect(() => { - setDraft(data ?? null); - }, [selectedNodeId, data]); - - if (!selectedNodeId || !draft) return null; + if (!selectedNodeId || !data) return null; const commit = (partial: Partial) => { - const updated = { ...draft, ...partial }; - setDraft(updated); updateNodeData(selectedNodeId, partial); }; @@ -57,7 +47,7 @@ export function NodeSettingsPanel() { commit({ label: e.target.value })} className="rounded-lg border border-slate-200 px-3 py-2 text-sm focus:outline-none focus:ring-2 focus:ring-indigo-300" /> @@ -69,7 +59,7 @@ export function NodeSettingsPanel() { System-Prompt