Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions velvetflow/planner/structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,31 @@ def _build_validation_error(message: str, **extra: Any) -> Dict[str, Any]:
payload.update(extra)
return payload

def _build_loop_depends_on_subgraph_conflict(
*, loop_id: str, depends_on: List[str], sub_graph_nodes: List[str]
) -> Dict[str, Any] | None:
depends_on_set = {dep for dep in depends_on if isinstance(dep, str)}
sub_graph_set = {nid for nid in sub_graph_nodes if isinstance(nid, str)}
overlap = sorted(depends_on_set.intersection(sub_graph_set))
if not overlap:
return None
suggestion = (
"Loop node depends_on includes nodes inside its body_subgraph. "
f"loop_id={loop_id}, depends_on={sorted(depends_on_set)}, "
f"sub_graph_nodes={sorted(sub_graph_set)}. "
"Please rename the node name either in depends_on or in the loop subgraph "
"to avoid this overlap."
)
return {
"status": "needs_more_work",
"message": "loop 节点的 depends_on 不能包含 body_subgraph 内的节点,请调整后重试。",
"node_id": loop_id,
"depends_on": sorted(depends_on_set),
"sub_graph_nodes": sorted(sub_graph_set),
"overlap_nodes": overlap,
"requirements_suggestions": [suggestion],
}

def _collect_result_of_node_ids(params: Mapping[str, Any]) -> set[str]:
node_ids: set[str] = set()

Expand Down Expand Up @@ -1387,6 +1412,13 @@ def add_loop_node(
if sub_graph_error:
result = {"status": "error", **sub_graph_error}
return _return_tool_result("add_loop_node", result)
conflict_error = _build_loop_depends_on_subgraph_conflict(
loop_id=id,
depends_on=depends_on or [],
sub_graph_nodes=normalized_nodes,
)
if conflict_error:
return _return_tool_result("add_loop_node", conflict_error)

merged_params = dict(params or {})
merged_params.update({"loop_kind": loop_kind, "source": source, "item_alias": item_alias})
Expand Down Expand Up @@ -2007,6 +2039,24 @@ def update_loop_node(
result = {"status": "error", **sub_graph_error}
return _return_tool_result("update_loop_node", result)

existing_depends_on = builder.nodes.get(id, {}).get("depends_on") if isinstance(builder.nodes.get(id), dict) else []
if not isinstance(existing_depends_on, list):
existing_depends_on = []
effective_depends_on = depends_on if depends_on is not None else existing_depends_on
current_body_nodes = [
node_id
for node_id, node in builder.nodes.items()
if isinstance(node, dict) and node.get("parent_node_id") == id
]
effective_sub_graph_nodes = normalized_nodes if sub_graph_nodes is not None else current_body_nodes
conflict_error = _build_loop_depends_on_subgraph_conflict(
loop_id=id,
depends_on=effective_depends_on or [],
sub_graph_nodes=effective_sub_graph_nodes,
Comment on lines +2088 to +2092
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate against existing loop body nodes

When update_loop_node receives sub_graph_nodes, the conflict check only uses normalized_nodes, but the update path later only attaches nodes (it never detaches existing body nodes). That means the effective body is still the existing current_body_nodes plus the newly attached ones. If a caller updates depends_on while adding new sub_graph_nodes, depends_on can still reference an already-attached body node and the check will pass, leaving a loop that depends on its own body. Consider validating against the union of current_body_nodes and normalized_nodes (or recompute after attaching) so existing body nodes are included.

Useful? React with 👍 / 👎.

)
if conflict_error:
return _return_tool_result("update_loop_node", conflict_error)

updates: Dict[str, Any] = {}
if display_name is not None:
updates["display_name"] = display_name
Expand Down
Loading