feat: AI flow builder, visibility model, dashboard tabs, fork UI (#88)
- AI flow builder: scaffold → branch detail → assemble → review flow - Generate All one-click branch generation with stop/cancel - Regenerate scaffold suggestions button - 3-action review screen: Start Flow, Open in Editor, Build Another - Fix Publish button gated behind !isDirty - Fix visibility column enforcement in tree access filter - Add ?visibility filter and author_name to GET /trees - Dashboard tabbed flows: My Flows / My Team / Public / All - Create button in My Flows tab, window focus reload (stale data fix) - Fork UI with optional reason modal - Fix account_id nullability in User type and schema - Keep is_public and visibility in sync on updates Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit was merged in pull request #88.
This commit is contained in:
@@ -32,7 +32,7 @@ from app.core.tree_validation import can_publish_tree
|
||||
router = APIRouter(prefix="/trees", tags=["trees"])
|
||||
|
||||
|
||||
def build_tree_response(tree: Tree) -> TreeListResponse:
|
||||
def build_tree_response(tree: Tree, author_map: dict | None = None) -> TreeListResponse:
|
||||
"""Build TreeListResponse with category_info and tags."""
|
||||
category_info = None
|
||||
if tree.category_rel:
|
||||
@@ -42,6 +42,8 @@ def build_tree_response(tree: Tree) -> TreeListResponse:
|
||||
slug=tree.category_rel.slug
|
||||
)
|
||||
|
||||
author_name = (author_map or {}).get(tree.author_id)
|
||||
|
||||
return TreeListResponse(
|
||||
id=tree.id,
|
||||
name=tree.name,
|
||||
@@ -52,10 +54,12 @@ def build_tree_response(tree: Tree) -> TreeListResponse:
|
||||
category_info=category_info,
|
||||
tags=tree.tag_names,
|
||||
author_id=tree.author_id,
|
||||
author_name=author_name,
|
||||
account_id=tree.account_id,
|
||||
is_active=tree.is_active,
|
||||
is_public=tree.is_public,
|
||||
is_default=tree.is_default,
|
||||
visibility=tree.visibility,
|
||||
status=tree.status,
|
||||
version=tree.version,
|
||||
usage_count=tree.usage_count,
|
||||
@@ -125,6 +129,7 @@ async def list_trees(
|
||||
is_active: Optional[bool] = Query(None, description="Filter by active status"),
|
||||
author_id: Optional[UUID] = Query(None, description="Filter by author ID"),
|
||||
is_public: Optional[bool] = Query(None, description="Filter by public status"),
|
||||
visibility: Optional[str] = Query(None, description="Filter by visibility: private, team, link, public"),
|
||||
sort_by: Optional[str] = Query("usage_count", description="Sort order: usage_count, updated_at, created_at, name, name_desc, version"),
|
||||
skip: int = Query(0, ge=0),
|
||||
limit: int = Query(100, ge=1, le=100)
|
||||
@@ -158,6 +163,8 @@ async def list_trees(
|
||||
query = query.where(Tree.author_id == author_id)
|
||||
if is_public is not None:
|
||||
query = query.where(Tree.is_public == is_public)
|
||||
if visibility:
|
||||
query = query.where(Tree.visibility == visibility)
|
||||
|
||||
# Filter by tags (all specified tags must be present)
|
||||
if tags:
|
||||
@@ -206,7 +213,17 @@ async def list_trees(
|
||||
result = await db.execute(query)
|
||||
trees = result.scalars().unique().all()
|
||||
|
||||
return [build_tree_response(tree) for tree in trees]
|
||||
# Fetch author names in one query (avoids N+1)
|
||||
author_ids = {t.author_id for t in trees if t.author_id}
|
||||
author_map: dict = {}
|
||||
if author_ids:
|
||||
authors_result = await db.execute(
|
||||
select(User.id, User.name, User.email).where(User.id.in_(author_ids))
|
||||
)
|
||||
for row in authors_result:
|
||||
author_map[row.id] = row.name or row.email
|
||||
|
||||
return [build_tree_response(tree, author_map) for tree in trees]
|
||||
|
||||
|
||||
@router.get("/categories", response_model=list[str])
|
||||
@@ -612,6 +629,13 @@ async def update_tree(
|
||||
for field, value in update_data.items():
|
||||
setattr(tree, field, value)
|
||||
|
||||
# Keep visibility and is_public in sync
|
||||
if tree_data.is_public is not None:
|
||||
if tree_data.is_public and tree.visibility not in ('public',):
|
||||
tree.visibility = 'public'
|
||||
elif not tree_data.is_public and tree.visibility == 'public':
|
||||
tree.visibility = 'team' # downgrade from public to team
|
||||
|
||||
# Increment version if tree structure changed
|
||||
if "tree_structure" in update_data:
|
||||
tree.version += 1
|
||||
@@ -1057,6 +1081,7 @@ async def update_tree_visibility(
|
||||
# Update visibility
|
||||
old_visibility = tree.visibility
|
||||
tree.visibility = visibility_data.visibility
|
||||
tree.is_public = (visibility_data.visibility == 'public')
|
||||
|
||||
await log_audit(db, current_user.id, "tree.visibility.update", "tree", tree.id,
|
||||
{"tree_name": tree.name, "old_visibility": old_visibility,
|
||||
|
||||
@@ -97,7 +97,16 @@ CORRECTIVE_PROMPT_TEMPLATE = """Your previous JSON was invalid for ResolutionFlo
|
||||
Validation errors:
|
||||
{error_list}
|
||||
|
||||
IMPORTANT: If any error mentions a next_node_id referencing a non-existent child, you must ensure every option's next_node_id exactly matches the "id" field of one of the node's direct children. The child node must exist in the "children" array of the same parent node.
|
||||
CRITICAL RULES TO FIX THESE ERRORS:
|
||||
|
||||
1. Decision node options → next_node_id MUST match the "id" of a direct child in that SAME decision node's "children" array.
|
||||
Example: if decision node has children [A, B, C], then option next_node_id must be "A", "B", or "C".
|
||||
|
||||
2. Action node → next_node_id MUST match the "id" of a SIBLING node — another child of the SAME parent decision node.
|
||||
Example: if parent decision has children [action-1, solution-1, solution-2], then action-1's next_node_id must be "solution-1" or "solution-2".
|
||||
The next node must ALREADY EXIST in the parent's children array — do NOT nest the next node inside the action node.
|
||||
|
||||
3. Every referenced ID must physically exist somewhere in the tree as a node with that exact "id" value.
|
||||
|
||||
Return a corrected full JSON object only. No markdown, no prose, no code fences.
|
||||
Fix ALL listed errors while maintaining the same troubleshooting/procedural logic."""
|
||||
@@ -224,6 +233,12 @@ async def generate_branch_detail(
|
||||
len(response.content),
|
||||
response.usage.output_tokens,
|
||||
)
|
||||
if response.stop_reason == "max_tokens":
|
||||
logger.warning(
|
||||
"branch_detail attempt=%d hit max_tokens limit (%d output tokens) — response may be truncated",
|
||||
attempt,
|
||||
response.usage.output_tokens,
|
||||
)
|
||||
raw_text = _strip_markdown_fences(response.content[0].text) if response.content else ""
|
||||
if not raw_text:
|
||||
logger.warning("branch_detail attempt=%d returned empty text, stop_reason=%s", attempt, response.stop_reason)
|
||||
|
||||
@@ -40,7 +40,8 @@ def validate_generated_tree(tree: dict[str, Any]) -> list[str]:
|
||||
|
||||
# Collect all node IDs and validate structure
|
||||
all_ids: set[str] = set()
|
||||
all_referenced_ids: set[str] = set()
|
||||
all_referenced_ids: set[str] = set() # option next_node_ids (already checked locally)
|
||||
action_next_ids: set[str] = set() # action next_node_ids (checked globally below)
|
||||
node_count = 0
|
||||
solution_count = 0
|
||||
|
||||
@@ -121,6 +122,7 @@ def validate_generated_tree(tree: dict[str, Any]) -> list[str]:
|
||||
)
|
||||
else:
|
||||
all_referenced_ids.add(next_id)
|
||||
action_next_ids.add(next_id)
|
||||
|
||||
elif node_type == "solution":
|
||||
solution_count += 1
|
||||
@@ -131,6 +133,13 @@ def validate_generated_tree(tree: dict[str, Any]) -> list[str]:
|
||||
|
||||
_validate_node(tree, "root")
|
||||
|
||||
# Check that all action next_node_ids actually exist in the tree
|
||||
for ref_id in action_next_ids:
|
||||
if ref_id not in all_ids:
|
||||
errors.append(
|
||||
f"Action next_node_id '{ref_id}' references a node that does not exist in the tree"
|
||||
)
|
||||
|
||||
# Global checks
|
||||
if node_count < 5:
|
||||
errors.append(
|
||||
|
||||
@@ -16,24 +16,32 @@ if TYPE_CHECKING:
|
||||
def build_tree_access_filter(current_user: User):
|
||||
"""Build the access filter for trees based on user permissions.
|
||||
|
||||
Returns trees that are:
|
||||
- All trees (for super admins)
|
||||
- Default/system trees (visible to all)
|
||||
- Public trees
|
||||
- User's own trees
|
||||
- Trees from user's account
|
||||
Visibility rules:
|
||||
- super_admin: sees everything
|
||||
- is_default: visible to all authenticated users
|
||||
- visibility='public': visible to all authenticated users
|
||||
- author_id == me: always visible (regardless of visibility setting)
|
||||
- visibility='team' AND account_id == mine: visible to account members
|
||||
- visibility='private': only visible to author (covered by author_id check above)
|
||||
- visibility='link': only visible to author (share token access is handled separately)
|
||||
"""
|
||||
from app.models.tree import Tree
|
||||
|
||||
if current_user.is_super_admin:
|
||||
return sa_true()
|
||||
|
||||
conditions = [
|
||||
Tree.is_default == True,
|
||||
Tree.is_public == True,
|
||||
Tree.visibility == 'public',
|
||||
Tree.author_id == current_user.id,
|
||||
]
|
||||
if current_user.account_id:
|
||||
conditions.append(Tree.account_id == current_user.account_id)
|
||||
conditions.append(
|
||||
and_(
|
||||
Tree.visibility == 'team',
|
||||
Tree.account_id == current_user.account_id
|
||||
)
|
||||
)
|
||||
return or_(*conditions)
|
||||
|
||||
|
||||
|
||||
@@ -176,10 +176,12 @@ class TreeListResponse(BaseModel):
|
||||
category_info: Optional[CategoryInfo] = None
|
||||
tags: list[str] = [] # List of tag names
|
||||
author_id: Optional[UUID] = None
|
||||
author_name: Optional[str] = None # Display name or email of author
|
||||
account_id: Optional[UUID] = None
|
||||
is_active: bool
|
||||
is_public: bool
|
||||
is_default: bool
|
||||
visibility: str = 'team'
|
||||
status: str # draft or published
|
||||
version: int
|
||||
usage_count: int
|
||||
|
||||
@@ -40,8 +40,8 @@ class UserLogin(BaseModel):
|
||||
class UserResponse(UserBase):
|
||||
id: UUID
|
||||
role: str = "engineer"
|
||||
account_id: UUID
|
||||
account_role: str
|
||||
account_id: Optional[UUID] = None
|
||||
account_role: Optional[str] = None
|
||||
is_super_admin: bool = False
|
||||
is_active: bool = True
|
||||
must_change_password: bool = False
|
||||
|
||||
@@ -124,6 +124,13 @@ class TestReferenceIntegrity:
|
||||
errors = validate_generated_tree(tree)
|
||||
assert any("non-existent child" in e for e in errors)
|
||||
|
||||
def test_action_next_node_id_references_nonexistent_node(self):
|
||||
"""Action next_node_id pointing to a node that doesn't exist anywhere in the tree."""
|
||||
tree = _make_valid_tree()
|
||||
tree["children"][1]["next_node_id"] = "ghost-node"
|
||||
errors = validate_generated_tree(tree)
|
||||
assert any("ghost-node" in e for e in errors)
|
||||
|
||||
def test_duplicate_option_ids(self):
|
||||
tree = _make_valid_tree()
|
||||
tree["options"][0]["id"] = "same"
|
||||
|
||||
@@ -391,3 +391,59 @@ class TestTrees:
|
||||
assert response.status_code == 200
|
||||
trees = response.json()
|
||||
assert isinstance(trees, list)
|
||||
|
||||
|
||||
class TestVisibilityFilter:
|
||||
"""Test that visibility filtering and author_name work correctly."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_private_tree_only_visible_to_author(
|
||||
self, client: AsyncClient, auth_headers: dict, test_user: dict
|
||||
):
|
||||
"""A private tree created by test_user should appear in their own list."""
|
||||
tree_data = {
|
||||
"name": "Private Flow Test",
|
||||
"tree_structure": {"id": "root", "type": "decision", "question": "Q?", "options": [], "children": []},
|
||||
}
|
||||
create_resp = await client.post("/api/v1/trees", json=tree_data, headers=auth_headers)
|
||||
assert create_resp.status_code == 201
|
||||
tree_id = create_resp.json()["id"]
|
||||
|
||||
# Set visibility to private
|
||||
vis_resp = await client.patch(
|
||||
f"/api/v1/trees/{tree_id}/visibility",
|
||||
json={"visibility": "private"},
|
||||
headers=auth_headers
|
||||
)
|
||||
assert vis_resp.status_code == 200
|
||||
|
||||
# Verify it still appears for the author
|
||||
list_resp = await client.get("/api/v1/trees", headers=auth_headers)
|
||||
assert list_resp.status_code == 200
|
||||
ids = [t["id"] for t in list_resp.json()]
|
||||
assert tree_id in ids
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_visibility_query_param_filters_correctly(
|
||||
self, client: AsyncClient, auth_headers: dict
|
||||
):
|
||||
"""?visibility=public should only return trees with visibility='public'."""
|
||||
resp = await client.get("/api/v1/trees?visibility=public", headers=auth_headers)
|
||||
assert resp.status_code == 200
|
||||
trees = resp.json()
|
||||
for tree in trees:
|
||||
assert tree["visibility"] == "public", f"Tree {tree['id']} has visibility={tree['visibility']}"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_author_name_present_in_list_response(
|
||||
self, client: AsyncClient, auth_headers: dict, test_tree: dict
|
||||
):
|
||||
"""TreeListResponse should include author_name field."""
|
||||
resp = await client.get("/api/v1/trees", headers=auth_headers)
|
||||
assert resp.status_code == 200
|
||||
trees = resp.json()
|
||||
assert len(trees) >= 1
|
||||
# author_name key should be present (value may be None for system/default trees)
|
||||
assert "author_name" in trees[0]
|
||||
# visibility key should be present
|
||||
assert "visibility" in trees[0]
|
||||
|
||||
Reference in New Issue
Block a user