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
This commit is contained in:
parent
fb0d3ae8f1
commit
7becffcc89
11 changed files with 2332 additions and 65 deletions
|
|
@ -222,7 +222,7 @@ async def approve_god_mode(
|
||||||
)
|
)
|
||||||
|
|
||||||
if request.action == "reject":
|
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."})
|
run_store.update(run_id, {"status": "failed", "error": "Rejected by user in God Mode."})
|
||||||
return CouncilResultResponse(
|
return CouncilResultResponse(
|
||||||
run_id=run_id,
|
run_id=run_id,
|
||||||
|
|
|
||||||
|
|
@ -366,9 +366,6 @@ def build_graph_from_blueprint(
|
||||||
if not nodes:
|
if not nodes:
|
||||||
raise ValueError("Blueprint has no 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
|
# Find entry point: the node that has no incoming edges
|
||||||
targets = {e["target"] for e in edges}
|
targets = {e["target"] for e in edges}
|
||||||
entry_candidates = [n["id"] for n in nodes if n["id"] not in targets]
|
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:
|
if not nodes:
|
||||||
raise ValueError("Blueprint has no nodes.")
|
raise ValueError("Blueprint has no nodes.")
|
||||||
|
|
||||||
node_lookup = {n["id"]: n for n in nodes}
|
|
||||||
targets = {e["target"] for e in edges}
|
targets = {e["target"] for e in edges}
|
||||||
entry_candidates = [n["id"] for n in nodes if n["id"] not in targets]
|
entry_candidates = [n["id"] for n in nodes if n["id"] not in targets]
|
||||||
if not entry_candidates:
|
if not entry_candidates:
|
||||||
|
|
|
||||||
|
|
@ -13,7 +13,7 @@ import pytest
|
||||||
import pytest_asyncio
|
import pytest_asyncio
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine
|
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 (
|
from services.blueprint_service import (
|
||||||
create_blueprint,
|
create_blueprint,
|
||||||
delete_blueprint,
|
delete_blueprint,
|
||||||
|
|
|
||||||
|
|
@ -23,7 +23,7 @@ from services.dynamic_graph_builder import (
|
||||||
_get_llm,
|
_get_llm,
|
||||||
)
|
)
|
||||||
from services.graph_builder import create_initial_state
|
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):
|
def test_claude_model_creates_instance(self):
|
||||||
with patch("services.dynamic_graph_builder.ChatAnthropic") as MockLLM:
|
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()
|
MockLLM.assert_called_once()
|
||||||
|
|
||||||
def test_gpt4o_model_creates_instance(self):
|
def test_gpt4o_model_creates_instance(self):
|
||||||
with patch("services.dynamic_graph_builder.ChatOpenAI") as MockLLM:
|
with patch("services.dynamic_graph_builder.ChatOpenAI") as MockLLM:
|
||||||
llm = _get_llm("gpt-4o")
|
_get_llm("gpt-4o")
|
||||||
MockLLM.assert_called_once()
|
MockLLM.assert_called_once()
|
||||||
|
|
|
||||||
|
|
@ -12,7 +12,6 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||||
import pytest
|
import pytest
|
||||||
from unittest.mock import patch, MagicMock
|
from unittest.mock import patch, MagicMock
|
||||||
|
|
||||||
from state import CouncilState
|
|
||||||
|
|
||||||
|
|
||||||
class TestBuildGraphGodMode:
|
class TestBuildGraphGodMode:
|
||||||
|
|
|
||||||
|
|
@ -9,7 +9,6 @@ import os
|
||||||
|
|
||||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||||
|
|
||||||
import pytest
|
|
||||||
from unittest.mock import patch, MagicMock
|
from unittest.mock import patch, MagicMock
|
||||||
|
|
||||||
from state import CouncilState, APPROVAL_THRESHOLD, MAX_ITERATIONS
|
from state import CouncilState, APPROVAL_THRESHOLD, MAX_ITERATIONS
|
||||||
|
|
|
||||||
|
|
@ -9,7 +9,6 @@ import os
|
||||||
|
|
||||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||||
|
|
||||||
import pytest
|
|
||||||
from unittest.mock import patch, MagicMock
|
from unittest.mock import patch, MagicMock
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -24,7 +23,7 @@ class TestWebSearchTool:
|
||||||
assert "TAVILY_API_KEY" in result
|
assert "TAVILY_API_KEY" in result
|
||||||
|
|
||||||
@patch.dict(os.environ, {"TAVILY_API_KEY": "test-key"}, clear=False)
|
@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):
|
def test_web_search_returns_formatted_results(self, mock_client_cls):
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
mock_client.search.return_value = {
|
mock_client.search.return_value = {
|
||||||
|
|
@ -46,7 +45,7 @@ class TestWebSearchTool:
|
||||||
assert "Some content here" in result
|
assert "Some content here" in result
|
||||||
|
|
||||||
@patch.dict(os.environ, {"TAVILY_API_KEY": "test-key"}, clear=False)
|
@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):
|
def test_web_search_handles_empty_results(self, mock_client_cls):
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
mock_client.search.return_value = {"results": []}
|
mock_client.search.return_value = {"results": []}
|
||||||
|
|
@ -58,7 +57,7 @@ class TestWebSearchTool:
|
||||||
assert "No results" in result
|
assert "No results" in result
|
||||||
|
|
||||||
@patch.dict(os.environ, {"TAVILY_API_KEY": "test-key"}, clear=False)
|
@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):
|
def test_web_search_handles_api_error(self, mock_client_cls):
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
mock_client.search.side_effect = Exception("API rate limit")
|
mock_client.search.side_effect = Exception("API rate limit")
|
||||||
|
|
@ -137,7 +136,7 @@ class TestPdfIngestion:
|
||||||
"""Tests for PDF ingestion into ChromaDB."""
|
"""Tests for PDF ingestion into ChromaDB."""
|
||||||
|
|
||||||
@patch("tools.pdf_reader._get_chroma_collection")
|
@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):
|
def test_ingest_pdf_processes_pages(self, mock_pdf_reader_cls, mock_get_collection):
|
||||||
# Mock PDF with 2 pages of text
|
# Mock PDF with 2 pages of text
|
||||||
mock_page1 = MagicMock()
|
mock_page1 = MagicMock()
|
||||||
|
|
@ -158,7 +157,7 @@ class TestPdfIngestion:
|
||||||
mock_collection.upsert.assert_called_once()
|
mock_collection.upsert.assert_called_once()
|
||||||
|
|
||||||
@patch("tools.pdf_reader._get_chroma_collection")
|
@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):
|
def test_ingest_pdf_empty_file(self, mock_pdf_reader_cls, mock_get_collection):
|
||||||
mock_reader = MagicMock()
|
mock_reader = MagicMock()
|
||||||
mock_reader.pages = []
|
mock_reader.pages = []
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,6 @@
|
||||||
import { describe, it, expect } from "vitest";
|
import { describe, it, expect } from "vitest";
|
||||||
import type {
|
import type {
|
||||||
AgentNodeData,
|
AgentNodeData,
|
||||||
CouncilBlueprint,
|
|
||||||
ExecutionMode,
|
ExecutionMode,
|
||||||
GodModeAction,
|
GodModeAction,
|
||||||
GodModeState,
|
GodModeState,
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,6 @@
|
||||||
"use client";
|
"use client";
|
||||||
|
|
||||||
import { useEffect, useState } from "react";
|
import { useMemo } from "react";
|
||||||
import { X, ArrowRight } from "lucide-react";
|
import { X, ArrowRight } from "lucide-react";
|
||||||
import { EdgeType } from "@/app/types/council";
|
import { EdgeType } from "@/app/types/council";
|
||||||
import { useCouncilStore } from "@/app/store/council-store";
|
import { useCouncilStore } from "@/app/store/council-store";
|
||||||
|
|
@ -15,15 +15,15 @@ export function EdgeSettingsPanel() {
|
||||||
|
|
||||||
const edge = edges.find((e) => e.id === selectedEdgeId);
|
const edge = edges.find((e) => e.id === selectedEdgeId);
|
||||||
|
|
||||||
const [edgeType, setEdgeType] = useState<EdgeType>("linear");
|
// Derive state directly from the store instead of syncing via useEffect
|
||||||
const [condition, setCondition] = useState("");
|
const edgeType = useMemo(
|
||||||
|
() => (edge?.data?.type as EdgeType) ?? "linear",
|
||||||
useEffect(() => {
|
[edge?.data?.type]
|
||||||
if (edge) {
|
);
|
||||||
setEdgeType((edge.data?.type as EdgeType) ?? "linear");
|
const condition = useMemo(
|
||||||
setCondition((edge.data?.condition as string) ?? "");
|
() => (edge?.data?.condition as string) ?? "",
|
||||||
}
|
[edge?.data?.condition]
|
||||||
}, [selectedEdgeId, edge]);
|
);
|
||||||
|
|
||||||
if (!selectedEdgeId || !edge) return null;
|
if (!selectedEdgeId || !edge) return null;
|
||||||
|
|
||||||
|
|
@ -31,12 +31,10 @@ export function EdgeSettingsPanel() {
|
||||||
const targetNode = nodes.find((n) => n.id === edge.target);
|
const targetNode = nodes.find((n) => n.id === edge.target);
|
||||||
|
|
||||||
const handleTypeChange = (newType: EdgeType) => {
|
const handleTypeChange = (newType: EdgeType) => {
|
||||||
setEdgeType(newType);
|
|
||||||
updateEdgeData(selectedEdgeId, newType, newType === "conditional" ? condition : undefined);
|
updateEdgeData(selectedEdgeId, newType, newType === "conditional" ? condition : undefined);
|
||||||
};
|
};
|
||||||
|
|
||||||
const handleConditionChange = (value: string) => {
|
const handleConditionChange = (value: string) => {
|
||||||
setCondition(value);
|
|
||||||
updateEdgeData(selectedEdgeId, edgeType, value);
|
updateEdgeData(selectedEdgeId, edgeType, value);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,5 @@
|
||||||
"use client";
|
"use client";
|
||||||
|
|
||||||
import { useEffect, useState } from "react";
|
|
||||||
import { X, Bot } from "lucide-react";
|
import { X, Bot } from "lucide-react";
|
||||||
import { AgentNodeData, LLMModel } from "@/app/types/council";
|
import { AgentNodeData, LLMModel } from "@/app/types/council";
|
||||||
import { useCouncilStore } from "@/app/store/council-store";
|
import { useCouncilStore } from "@/app/store/council-store";
|
||||||
|
|
@ -21,18 +20,9 @@ export function NodeSettingsPanel() {
|
||||||
const node = nodes.find((n) => n.id === selectedNodeId);
|
const node = nodes.find((n) => n.id === selectedNodeId);
|
||||||
const data = node?.data as AgentNodeData | undefined;
|
const data = node?.data as AgentNodeData | undefined;
|
||||||
|
|
||||||
// Local draft to avoid re-renders on every keystroke
|
if (!selectedNodeId || !data) return null;
|
||||||
const [draft, setDraft] = useState<AgentNodeData | null>(null);
|
|
||||||
|
|
||||||
useEffect(() => {
|
|
||||||
setDraft(data ?? null);
|
|
||||||
}, [selectedNodeId, data]);
|
|
||||||
|
|
||||||
if (!selectedNodeId || !draft) return null;
|
|
||||||
|
|
||||||
const commit = (partial: Partial<AgentNodeData>) => {
|
const commit = (partial: Partial<AgentNodeData>) => {
|
||||||
const updated = { ...draft, ...partial };
|
|
||||||
setDraft(updated);
|
|
||||||
updateNodeData(selectedNodeId, partial);
|
updateNodeData(selectedNodeId, partial);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
@ -57,7 +47,7 @@ export function NodeSettingsPanel() {
|
||||||
<label className="text-xs font-medium text-slate-500">Name</label>
|
<label className="text-xs font-medium text-slate-500">Name</label>
|
||||||
<input
|
<input
|
||||||
type="text"
|
type="text"
|
||||||
value={draft.label}
|
value={data.label}
|
||||||
onChange={(e) => commit({ label: e.target.value })}
|
onChange={(e) => 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"
|
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
|
System-Prompt
|
||||||
</label>
|
</label>
|
||||||
<textarea
|
<textarea
|
||||||
value={draft.systemPrompt}
|
value={data.systemPrompt}
|
||||||
onChange={(e) => commit({ systemPrompt: e.target.value })}
|
onChange={(e) => commit({ systemPrompt: e.target.value })}
|
||||||
rows={6}
|
rows={6}
|
||||||
placeholder="Beschreibe die Rolle und das Verhalten dieses Agenten..."
|
placeholder="Beschreibe die Rolle und das Verhalten dieses Agenten..."
|
||||||
|
|
@ -81,7 +71,7 @@ export function NodeSettingsPanel() {
|
||||||
<div className="flex flex-col gap-1">
|
<div className="flex flex-col gap-1">
|
||||||
<label className="text-xs font-medium text-slate-500">Modell</label>
|
<label className="text-xs font-medium text-slate-500">Modell</label>
|
||||||
<select
|
<select
|
||||||
value={draft.model}
|
value={data.model}
|
||||||
onChange={(e) => commit({ model: e.target.value as LLMModel })}
|
onChange={(e) => commit({ model: e.target.value as LLMModel })}
|
||||||
className="rounded-lg border border-slate-200 px-3 py-2 text-sm focus:outline-none focus:ring-2 focus:ring-indigo-300"
|
className="rounded-lg border border-slate-200 px-3 py-2 text-sm focus:outline-none focus:ring-2 focus:ring-indigo-300"
|
||||||
>
|
>
|
||||||
|
|
@ -100,9 +90,9 @@ export function NodeSettingsPanel() {
|
||||||
<label className="flex items-center gap-3 cursor-pointer">
|
<label className="flex items-center gap-3 cursor-pointer">
|
||||||
<input
|
<input
|
||||||
type="checkbox"
|
type="checkbox"
|
||||||
checked={draft.tools.webSearch}
|
checked={data.tools.webSearch}
|
||||||
onChange={(e) =>
|
onChange={(e) =>
|
||||||
commit({ tools: { ...draft.tools, webSearch: e.target.checked } })
|
commit({ tools: { ...data.tools, webSearch: e.target.checked } })
|
||||||
}
|
}
|
||||||
className="h-4 w-4 rounded border-slate-300 text-indigo-600 focus:ring-indigo-300"
|
className="h-4 w-4 rounded border-slate-300 text-indigo-600 focus:ring-indigo-300"
|
||||||
/>
|
/>
|
||||||
|
|
@ -112,9 +102,9 @@ export function NodeSettingsPanel() {
|
||||||
<label className="flex items-center gap-3 cursor-pointer">
|
<label className="flex items-center gap-3 cursor-pointer">
|
||||||
<input
|
<input
|
||||||
type="checkbox"
|
type="checkbox"
|
||||||
checked={draft.tools.pdfReader}
|
checked={data.tools.pdfReader}
|
||||||
onChange={(e) =>
|
onChange={(e) =>
|
||||||
commit({ tools: { ...draft.tools, pdfReader: e.target.checked } })
|
commit({ tools: { ...data.tools, pdfReader: e.target.checked } })
|
||||||
}
|
}
|
||||||
className="h-4 w-4 rounded border-slate-300 text-indigo-600 focus:ring-indigo-300"
|
className="h-4 w-4 rounded border-slate-300 text-indigo-600 focus:ring-indigo-300"
|
||||||
/>
|
/>
|
||||||
|
|
|
||||||
2321
frontend/package-lock.json
generated
2321
frontend/package-lock.json
generated
File diff suppressed because it is too large
Load diff
Loading…
Add table
Add a link
Reference in a new issue