From 7623f679cf26371121fae7e55d449596d765e022 Mon Sep 17 00:00:00 2001 From: Darren Davison Date: Sat, 4 Apr 2026 12:48:28 +0100 Subject: [PATCH] fix: count all descendants in collapsed badge and prune stale localStorage IDs Address two Greptile review comments: 1. Collapsed parent badge now shows total descendant count at all depths rather than direct-child count only. Add `countDescendants` utility to issue-tree.ts (recursive, uses existing childMap) and replace `children.length` with it in the titleSuffix badge. 2. Add a useEffect that prunes stale IDs from `collapsedParents` whenever the issues prop changes. Deleted or reassigned issues previously left orphan IDs in localStorage indefinitely; the effect filters to only IDs that appear as a parentId in the current issue list and persists the cleaned array via updateView. Add four unit tests for countDescendants: leaf node, single-level, multi-level, and unknown ID. Co-Authored-By: Paperclip --- ui/src/components/IssuesList.tsx | 28 ++++++++++++++++++++++++-- ui/src/lib/issue-tree.test.ts | 34 +++++++++++++++++++++++++++++++- ui/src/lib/issue-tree.ts | 9 +++++++++ 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/ui/src/components/IssuesList.tsx b/ui/src/components/IssuesList.tsx index 5ac50ee9..eca0c692 100644 --- a/ui/src/components/IssuesList.tsx +++ b/ui/src/components/IssuesList.tsx @@ -23,7 +23,7 @@ import { Checkbox } from "@/components/ui/checkbox"; import { Collapsible, CollapsibleTrigger, CollapsibleContent } from "@/components/ui/collapsible"; import { CircleDot, Plus, Filter, ArrowUpDown, Layers, Check, X, ChevronRight, List, Columns3, User, Search } from "lucide-react"; import { KanbanBoard } from "./KanbanBoard"; -import { buildIssueTree } from "../lib/issue-tree"; +import { buildIssueTree, countDescendants } from "../lib/issue-tree"; import type { Issue } from "@paperclipai/shared"; /* ── Helpers ── */ @@ -249,6 +249,29 @@ export function IssuesList({ return next; }); }, [scopedKey]); + + // Prune stale IDs from collapsedParents whenever the issue list changes. + // Deleted or reassigned issues leave orphan IDs in localStorage; this keeps + // the stored array bounded to only current parent IDs. + useEffect(() => { + const parentIds = new Set(issues.map((i) => i.parentId).filter(Boolean) as string[]); + const pruned = viewState.collapsedParents.filter((id) => parentIds.has(id)); + if (pruned.length !== viewState.collapsedParents.length) { + updateView({ collapsedParents: pruned }); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [issues]); + + const { data: searchedIssues = [] } = useQuery({ + queryKey: [ + ...queryKeys.issues.search(selectedCompanyId!, normalizedIssueSearch, projectId), + searchFilters ?? {}, + ], + queryFn: () => issuesApi.list(selectedCompanyId!, { q: normalizedIssueSearch, projectId, ...searchFilters }), + enabled: !!selectedCompanyId && normalizedIssueSearch.length > 0, + placeholderData: (previousData) => previousData, + }); + const agentName = useCallback((id: string | null) => { if (!id || !agents) return null; return agents.find((a) => a.id === id)?.name ?? null; @@ -673,6 +696,7 @@ export function IssuesList({ const renderIssueRow = (issue: Issue, depth: number) => { const children = childMap.get(issue.id) ?? []; const hasChildren = children.length > 0; + const totalDescendants = hasChildren ? countDescendants(issue.id, childMap) : 0; const isExpanded = !viewState.collapsedParents.includes(issue.id); const toggleCollapse = (e: { preventDefault: () => void; stopPropagation: () => void }) => { e.preventDefault(); @@ -691,7 +715,7 @@ export function IssuesList({ issueLinkState={issueLinkState} titleSuffix={hasChildren && !isExpanded ? ( - ({children.length} sub-task{children.length !== 1 ? "s" : ""}) + ({totalDescendants} sub-task{totalDescendants !== 1 ? "s" : ""}) ) : undefined} mobileLeading={ diff --git a/ui/src/lib/issue-tree.test.ts b/ui/src/lib/issue-tree.test.ts index ddb0e6ae..75d38f87 100644 --- a/ui/src/lib/issue-tree.test.ts +++ b/ui/src/lib/issue-tree.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { Issue } from "@paperclipai/shared"; -import { buildIssueTree } from "./issue-tree"; +import { buildIssueTree, countDescendants } from "./issue-tree"; function makeIssue(id: string, parentId: string | null = null): Issue { return { @@ -96,3 +96,35 @@ describe("buildIssueTree", () => { expect(childMap.get("p1")?.map((c) => c.id)).toEqual(["c1", "c2"]); }); }); + +describe("countDescendants", () => { + it("returns 0 for a leaf node", () => { + const { childMap } = buildIssueTree([makeIssue("a")]); + expect(countDescendants("a", childMap)).toBe(0); + }); + + it("returns direct child count for a single-level parent", () => { + const { childMap } = buildIssueTree([ + makeIssue("p"), + makeIssue("c1", "p"), + makeIssue("c2", "p"), + ]); + expect(countDescendants("p", childMap)).toBe(2); + }); + + it("counts all descendants across multiple levels", () => { + // P → C → G1, G2 (P has 3 total descendants: C, G1, G2) + const { childMap } = buildIssueTree([ + makeIssue("p"), + makeIssue("c", "p"), + makeIssue("g1", "c"), + makeIssue("g2", "c"), + ]); + expect(countDescendants("p", childMap)).toBe(3); + }); + + it("returns 0 for an id not in the childMap", () => { + const { childMap } = buildIssueTree([makeIssue("a"), makeIssue("b")]); + expect(countDescendants("nonexistent", childMap)).toBe(0); + }); +}); diff --git a/ui/src/lib/issue-tree.ts b/ui/src/lib/issue-tree.ts index c5fa8fc4..e88709ed 100644 --- a/ui/src/lib/issue-tree.ts +++ b/ui/src/lib/issue-tree.ts @@ -25,3 +25,12 @@ export function buildIssueTree(items: Issue[]): IssueTree { } return { roots, childMap }; } + +/** + * Returns the total number of descendants (all depths) of `id` in `childMap`. + * Used to accurately label collapsed parent badges like "(3 sub-tasks)". + */ +export function countDescendants(id: string, childMap: Map): number { + const children = childMap.get(id) ?? []; + return children.reduce((sum, c) => sum + 1 + countDescendants(c.id, childMap), 0); +}