From 91d2bc6df382f38b6576e438d7714055556d6699 Mon Sep 17 00:00:00 2001 From: Michael Chihlas Date: Wed, 11 Mar 2026 01:59:03 -0400 Subject: [PATCH] fix: KB Accelerator tree builder, approve all, canvas delete button - Fix _build_troubleshooting_tree() to handle deep nesting, warning nodes, and DAG deduplication (placed set prevents duplicate IDs) - Fix step_sync VARCHAR(255) overflow on publish by truncating title - Add "Approve All" button to KB review screen - Add delete button (hover-reveal) to flow canvas nodes Co-Authored-By: Claude Opus 4.6 --- backend/app/api/endpoints/kb_accelerator.py | 78 +++++++++++++++---- backend/app/core/step_sync.py | 2 +- .../kb-accelerator/ReviewScreen.tsx | 22 +++++- .../src/components/tree-editor/FlowCanvas.tsx | 7 +- .../components/tree-editor/FlowCanvasNode.tsx | 23 +++++- .../tree-editor/TreeEditorLayout.tsx | 3 + .../components/tree-editor/useTreeLayout.ts | 1 + frontend/src/pages/KBAcceleratorPage.tsx | 41 +++++++--- frontend/src/pages/TreeEditorPage.tsx | 1 + 9 files changed, 143 insertions(+), 35 deletions(-) diff --git a/backend/app/api/endpoints/kb_accelerator.py b/backend/app/api/endpoints/kb_accelerator.py index c1a37a21..52274640 100644 --- a/backend/app/api/endpoints/kb_accelerator.py +++ b/backend/app/api/endpoints/kb_accelerator.py @@ -608,53 +608,81 @@ async def delete_import( def _build_troubleshooting_tree(nodes: list[KBImportNode]) -> dict: - """Build a troubleshooting tree_structure from import nodes.""" + """Build a troubleshooting tree_structure from import nodes. + + The tree editor expects a deeply nested structure where each decision + node's `children` array contains all reachable descendant nodes. + Action/solution nodes use `title`/`description` (not `question`). + + The AI generates a DAG (shared nodes reachable from multiple paths), + but the tree editor requires unique IDs — each node can only appear + once. We embed each node the first time it's encountered; subsequent + references just use next_node_id / options[].next_node_id to point + back to the already-embedded node. + """ if not nodes: return {"id": "root", "type": "decision", "question": "Empty", "children": []} - # Map original IDs to proper tree node structure + # Map original IDs to import nodes original_id_map: dict[str, KBImportNode] = {} for node in nodes: orig_id = node.content.get("original_id", str(node.id)) original_id_map[orig_id] = node - def _build_node(import_node: KBImportNode) -> dict: + # Track which nodes have been placed in the tree to avoid duplicates + placed: set[str] = set() + + def _build_node(import_node: KBImportNode) -> dict | None: content = import_node.content node_type = import_node.node_type + node_id = content.get("original_id", str(import_node.id)) + + # Already placed in the tree — don't create a duplicate. + # The reference (next_node_id / options) is sufficient. + if node_id in placed: + return None + placed.add(node_id) + + question_text = content.get("question", "") if node_type == "resolution": return { - "id": content.get("original_id", str(import_node.id)), + "id": node_id, "type": "solution", - "question": content.get("question", ""), - "children": [], + "title": question_text, + "description": content.get("description", ""), } - if node_type == "action": - result = { - "id": content.get("original_id", str(import_node.id)), + if node_type in ("action", "warning"): + result: dict = { + "id": node_id, "type": "action", - "question": content.get("question", ""), - "children": [], + "title": question_text, + "description": content.get("description", ""), } next_id = content.get("next_node_id") if next_id and next_id in original_id_map: result["next_node_id"] = next_id return result - # question/decision type + # question/decision type — recursively build children options = content.get("options", []) children = [] for opt in options: next_id = opt.get("next_node_id") if next_id and next_id in original_id_map: child_node = _build_node(original_id_map[next_id]) - children.append(child_node) + if child_node is not None: + children.append(child_node) + # If the child is an action with a next_node_id, also + # build that target as a sibling (the tree editor + # expects reachable nodes nested under the decision) + _collect_action_chain(child_node, children) return { - "id": content.get("original_id", str(import_node.id)), + "id": node_id, "type": "decision", - "question": content.get("question", ""), + "question": question_text, "options": [ {"label": opt.get("label", ""), "next_node_id": opt.get("next_node_id", "")} for opt in options @@ -662,8 +690,26 @@ def _build_troubleshooting_tree(nodes: list[KBImportNode]) -> dict: "children": children, } + def _collect_action_chain(node: dict, siblings: list[dict]) -> None: + """Follow action node next_node_id chains and add targets as siblings.""" + if node.get("type") != "action": + return + next_id = node.get("next_node_id") + if not next_id or next_id not in original_id_map: + return + # Don't add if already in this siblings list or already placed + if any(s["id"] == next_id for s in siblings): + return + target = _build_node(original_id_map[next_id]) + if target is None: + return + siblings.append(target) + # Continue chain if the target is also an action + _collect_action_chain(target, siblings) + root_node = nodes[0] - return _build_node(root_node) + result = _build_node(root_node) + return result or {"id": "root", "type": "decision", "question": "Empty", "children": []} def _build_procedural_tree(nodes: list[KBImportNode]) -> dict: diff --git a/backend/app/core/step_sync.py b/backend/app/core/step_sync.py index b31f02fa..fc13526f 100644 --- a/backend/app/core/step_sync.py +++ b/backend/app/core/step_sync.py @@ -162,7 +162,7 @@ async def sync_steps_from_tree( is_active = true """), { - "title": step_data["title"], + "title": step_data["title"][:255], "step_type": step_data["step_type"], "content": json.dumps(step_data["content"]), "created_by": str(resolved_author_id), diff --git a/frontend/src/components/kb-accelerator/ReviewScreen.tsx b/frontend/src/components/kb-accelerator/ReviewScreen.tsx index 2f0877a2..042b5a29 100644 --- a/frontend/src/components/kb-accelerator/ReviewScreen.tsx +++ b/frontend/src/components/kb-accelerator/ReviewScreen.tsx @@ -1,5 +1,5 @@ import { useState } from 'react' -import { CheckCircle2, AlertTriangle, BarChart3 } from 'lucide-react' +import { CheckCircle2, AlertTriangle, BarChart3, CheckCheck } from 'lucide-react' import { cn } from '@/lib/utils' import { NodeCard } from './NodeCard' import { SourcePanel } from './SourcePanel' @@ -8,12 +8,13 @@ import type { KBImport, KBNodeEditRequest, KBCommitRequest } from '@/types/kbAcc interface ReviewScreenProps { kbImport: KBImport onEditNode: (nodeId: string, data: KBNodeEditRequest) => Promise + onApproveAll: () => Promise onCommit: (data?: KBCommitRequest) => Promise onDiscard: () => Promise loading: boolean } -export function ReviewScreen({ kbImport, onEditNode, onCommit, onDiscard, loading }: ReviewScreenProps) { +export function ReviewScreen({ kbImport, onEditNode, onApproveAll, onCommit, onDiscard, loading }: ReviewScreenProps) { const [highlightExcerpt, setHighlightExcerpt] = useState(null) const nodes = kbImport.nodes @@ -57,6 +58,21 @@ export function ReviewScreen({ kbImport, onEditNode, onCommit, onDiscard, loadin )} + {approvedCount < nodes.length && ( + + )} @@ -98,7 +114,7 @@ export function ReviewScreen({ kbImport, onEditNode, onCommit, onDiscard, loadin {/* Actions */} -
+
+ )}
{/* Decision options preview */} diff --git a/frontend/src/components/tree-editor/TreeEditorLayout.tsx b/frontend/src/components/tree-editor/TreeEditorLayout.tsx index be26846f..c2e81d7b 100644 --- a/frontend/src/components/tree-editor/TreeEditorLayout.tsx +++ b/frontend/src/components/tree-editor/TreeEditorLayout.tsx @@ -19,6 +19,7 @@ interface TreeEditorLayoutProps { editingNodeId: string | null onNodeSelect: (nodeId: string | null) => void onSelectAnswerType: (nodeId: string, type: 'decision' | 'action' | 'solution') => void + onNodeDelete?: (nodeId: string) => void onNodeContextMenu?: (e: React.MouseEvent, nodeId: string) => void } @@ -29,6 +30,7 @@ export function TreeEditorLayout({ editingNodeId, onNodeSelect, onSelectAnswerType, + onNodeDelete, onNodeContextMenu, }: TreeEditorLayoutProps) { const editorMode = useTreeEditorStore(s => s.editorMode) @@ -72,6 +74,7 @@ export function TreeEditorLayout({ selectedNodeId={editingNodeId} onNodeSelect={onNodeSelect} onSelectAnswerType={onSelectAnswerType} + onNodeDelete={onNodeDelete} onNodeContextMenu={onNodeContextMenu} />
diff --git a/frontend/src/components/tree-editor/useTreeLayout.ts b/frontend/src/components/tree-editor/useTreeLayout.ts index d6aa5ba9..e1dd487c 100644 --- a/frontend/src/components/tree-editor/useTreeLayout.ts +++ b/frontend/src/components/tree-editor/useTreeLayout.ts @@ -190,6 +190,7 @@ export function useTreeLayout(selectedNodeId?: string | null): UseTreeLayoutResu isCollapsed, hasValidationErrors: hasErrors, isNew: false, + isRoot: node.id === treeStructure?.id, onToggleCollapse: () => {}, // placeholder — set by FlowCanvas } satisfies FlowCanvasNodeData, style: { width: NODE_WIDTH }, diff --git a/frontend/src/pages/KBAcceleratorPage.tsx b/frontend/src/pages/KBAcceleratorPage.tsx index 9ad0dc57..12fd607a 100644 --- a/frontend/src/pages/KBAcceleratorPage.tsx +++ b/frontend/src/pages/KBAcceleratorPage.tsx @@ -89,6 +89,26 @@ export default function KBAcceleratorPage() { } } + const handleApproveAll = async () => { + if (!importId || !kbImport) return + const unapproved = kbImport.nodes.filter(n => !n.user_approved) + if (unapproved.length === 0) return + setLoading(true) + try { + await Promise.all( + unapproved.map(n => kbAcceleratorApi.editNode(importId, n.id, { operation: 'approve' })) + ) + setKbImport(prev => { + if (!prev) return prev + return { ...prev, nodes: prev.nodes.map(n => ({ ...n, user_approved: true })) } + }) + } catch { + toast.error('Failed to approve all nodes') + } finally { + setLoading(false) + } + } + const handleEditNode = async (nodeId: string, data: KBNodeEditRequest) => { if (!importId) return const updatedNode = await kbAcceleratorApi.editNode(importId, nodeId, data) @@ -149,9 +169,9 @@ export default function KBAcceleratorPage() { } return ( -
+
{/* Page title */} -
+

KB Accelerator

@@ -181,13 +201,16 @@ export default function KBAcceleratorPage() { )} {phase === 'review' && kbImport && ( - +
+ +
)} {phase === 'success' && commitResult && ( diff --git a/frontend/src/pages/TreeEditorPage.tsx b/frontend/src/pages/TreeEditorPage.tsx index 9c7f4e12..a6d3f6e0 100644 --- a/frontend/src/pages/TreeEditorPage.tsx +++ b/frontend/src/pages/TreeEditorPage.tsx @@ -815,6 +815,7 @@ export function TreeEditorPage() { editingNodeId={editorAI.isOpen ? null : editingNodeId} onNodeSelect={handleNodeSelect} onSelectAnswerType={handleSelectAnswerType} + onNodeDelete={deleteNode} onNodeContextMenu={editorAI.openContextMenu} />