fix: KB tree builder — post-build repair, validation gate, and root ID handling
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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 = []
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user