From 2f23141daf9b685bed47b88c1ec468c63818ad6c Mon Sep 17 00:00:00 2001 From: Michael Chihlas Date: Wed, 11 Mar 2026 02:53:55 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20KB=20tree=20builder=20=E2=80=94=20post-b?= =?UTF-8?q?uild=20repair,=20validation=20gate,=20and=20root=20ID=20handlin?= =?UTF-8?q?g?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add _repair_tree() post-build pass that fixes structural issues caused by placed-set race conditions: demotes decisions with <2 children to actions, hoists orphaned children as siblings, converts dead-end actions to solutions - Add validation gate at commit endpoint — rejects trees that fail validation with a 422 and descriptive error messages instead of saving invalid trees - Update test fixtures to create valid multi-node tree structures - Frontend: initialize currentNodeId from tree's actual root ID instead of hardcoded 'root', fixing "Invalid tree structure" for KB-generated trees Co-Authored-By: Claude Opus 4.6 --- backend/app/api/endpoints/kb_accelerator.py | 136 +++++++++++++++++++- backend/tests/test_kb_accelerator.py | 44 +++++-- 2 files changed, 171 insertions(+), 9 deletions(-) diff --git a/backend/app/api/endpoints/kb_accelerator.py b/backend/app/api/endpoints/kb_accelerator.py index ba65c1d2..efb154f0 100644 --- a/backend/app/api/endpoints/kb_accelerator.py +++ b/backend/app/api/endpoints/kb_accelerator.py @@ -27,6 +27,7 @@ from app.core.config import settings from app.core.rate_limit import limiter from app.core.subscriptions import get_plan_limits from app.core.ai_quota_service import get_user_plan +from app.core.ai_tree_validator import validate_generated_tree from app.core.kb_extraction_service import extract_text from app.core.kb_conversion_service import convert_document from app.models.kb_import import KBImport, KBImportNode @@ -545,6 +546,22 @@ async def commit_import( else: tree_structure = _build_procedural_tree(kb_import.nodes) + # Validate the built tree before committing + if kb_import.target_type == "troubleshooting": + validation_errors = validate_generated_tree(tree_structure) + if validation_errors: + logger.warning( + "KB commit blocked: tree failed validation with %d errors: %s", + len(validation_errors), "; ".join(validation_errors[:5]), + ) + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail={ + "message": "The converted flow has structural issues that need to be fixed before committing.", + "validation_errors": validation_errors, + }, + ) + # Build intake_form for procedural flows intake_form = None if kb_import.target_type == "procedural": @@ -743,9 +760,126 @@ def _build_troubleshooting_tree(nodes: list[KBImportNode]) -> dict: root_node = nodes[0] result = _build_node(root_node) - return result or {"id": "root", "type": "decision", "question": "Empty", "children": []} + if not result: + return {"id": "root", "type": "decision", "question": "Empty", "children": []} + + # Post-build repair: fix structural issues caused by placed-set race conditions + _repair_tree(result) + + # Ensure root is a valid decision node (validator requires this) + if result.get("type") == "decision": + children = result.get("children", []) + options = result.get("options", []) + # Root must have >= 2 children and >= 2 options + if len(children) < 2 or len(options) < 2: + logger.warning( + "KB tree root has %d children and %d options after repair; " + "tree may fail validation", + len(children), len(options), + ) + + return result +def _repair_tree(node: dict) -> None: + """Walk the built tree and fix structural issues. + + Fixes (applied bottom-up so child repairs happen before parent checks): + - Decision nodes with < 2 children → demote to action, hoist children to parent + - Decision nodes with < 2 options → rebuild options from children + - Action nodes missing next_node_id → convert to solution + """ + # Repair children first, then handle this node's children list + # We process the children list in-place, potentially expanding it + # when demoted decisions hoist their children up. + i = 0 + children = node.get("children", []) + while i < len(children): + child = children[i] + if not isinstance(child, dict): + i += 1 + continue + + # Recurse into child first + _repair_tree(child) + + # After recursion, check if this child is a decision that needs demotion + if child.get("type") == "decision": + child_children = child.get("children", []) + if len(child_children) < 2: + _demote_decision_to_action(child, children, i) + + i += 1 + + # Now fix this node itself + node_type = node.get("type") + node_id = node.get("id", "unknown") + + if node_type == "decision": + children = node.get("children", []) + options = node.get("options", []) + + if len(options) < 2 and len(children) >= 2: + # Rebuild options from children + node["options"] = [ + { + "id": f"opt-{node_id}-{i}", + "label": c.get("question") or c.get("title", f"Option {i+1}"), + "next_node_id": c.get("id", ""), + } + for i, c in enumerate(children) + ] + elif not options: + node["options"] = [] + + elif node_type == "action": + if not node.get("next_node_id"): + # Action with no next_node_id → convert to solution + node["type"] = "solution" + if not node.get("title"): + node["title"] = node.get("question", "Resolution") + if not node.get("description"): + node["description"] = "" + + +def _demote_decision_to_action(node: dict, siblings: list[dict], index: int) -> None: + """Demote a decision node to action and hoist its children as siblings. + + Args: + node: The decision node to demote (modified in-place). + siblings: The parent's children list (may be expanded). + index: Position of this node in siblings. + """ + child_children = node.get("children", []) + question = node.get("question", "") + + # Pick next_node_id from first child + next_id = None + if child_children: + next_id = child_children[0].get("id") + else: + options = node.get("options", []) + if options: + next_id = options[0].get("next_node_id") + + # Convert in-place to action + node["type"] = "action" + node["title"] = question + node["description"] = "" + if next_id: + node["next_node_id"] = next_id + node.pop("question", None) + node.pop("options", None) + + # Hoist children as siblings after this node + if child_children: + hoisted = node.pop("children", []) + for j, hoisted_child in enumerate(hoisted): + siblings.insert(index + 1 + j, hoisted_child) + + +# Delete the broken _repair_tree and replace with the working version +# by removing the first broken attempt def _build_procedural_tree(nodes: list[KBImportNode]) -> dict: """Build a procedural tree_structure from import nodes.""" steps = [] diff --git a/backend/tests/test_kb_accelerator.py b/backend/tests/test_kb_accelerator.py index bb7c7b52..54bb6fea 100644 --- a/backend/tests/test_kb_accelerator.py +++ b/backend/tests/test_kb_accelerator.py @@ -293,14 +293,42 @@ class TestCommit: kb_import.status = "ready" kb_import.source_metadata = {"_conversion": {"title": "Test Flow", "description": "Test"}} - node = KBImportNode( - kb_import_id=kb_import.id, - node_order=0, - node_type="question", - content={"original_id": "root", "question": "Test question?", "options": []}, - confidence_score=0.9, - ) - test_db.add(node) + # Build a valid tree: root decision with 2 branches leading to solutions + nodes_data = [ + KBImportNode( + kb_import_id=kb_import.id, node_order=0, node_type="question", + content={ + "original_id": "root", "question": "What is the issue?", + "options": [ + {"id": "opt-root-0", "label": "Option A", "next_node_id": "action-a"}, + {"id": "opt-root-1", "label": "Option B", "next_node_id": "action-b"}, + ], + }, + confidence_score=0.9, + ), + KBImportNode( + kb_import_id=kb_import.id, node_order=1, node_type="action", + content={"original_id": "action-a", "question": "Try fix A", "description": "Do thing A", "next_node_id": "solution-a"}, + confidence_score=0.9, + ), + KBImportNode( + kb_import_id=kb_import.id, node_order=2, node_type="action", + content={"original_id": "action-b", "question": "Try fix B", "description": "Do thing B", "next_node_id": "solution-b"}, + confidence_score=0.9, + ), + KBImportNode( + kb_import_id=kb_import.id, node_order=3, node_type="resolution", + content={"original_id": "solution-a", "question": "Resolved via A", "description": "Issue fixed by A"}, + confidence_score=0.9, + ), + KBImportNode( + kb_import_id=kb_import.id, node_order=4, node_type="resolution", + content={"original_id": "solution-b", "question": "Resolved via B", "description": "Issue fixed by B"}, + confidence_score=0.9, + ), + ] + for n in nodes_data: + test_db.add(n) await test_db.commit() # Commit