feat(dependency-graph): Phase 2 - Directory clustering with expand/collapse#228
Conversation
|
@DevanshuNEU is attempting to deploy a commit to the Dev's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughIntroduces directory-level clustering to the Dependency Graph component, adding a new DirectoryNode component, a cluster-toggle UI button, and graph logic to group files by directory with expandable folder nodes and aggregated metrics including file counts, dependency counts, and risk badges. Changes
Sequence DiagramssequenceDiagram
actor User
participant Toolbar as GraphToolbar
participant Graph as DependencyGraph
participant React as React Flow
participant DNode as DirectoryNode
User->>Toolbar: Click Cluster Button
Toolbar->>Graph: onToggleCluster()
Graph->>Graph: Update clusterByDir state
Graph->>Graph: Group files by directory<br/>Create directory node data
Graph->>Graph: Rebuild edges between<br/>directories/files
Graph->>React: Update nodes & edges
React->>DNode: Render DirectoryNode instances
DNode->>DNode: Show folder icon +<br/>file/dependency counts
User->>DNode: Click Directory Node
DNode->>Graph: Handle node click
Graph->>Graph: Toggle expandedDirs state
Graph->>Graph: Recalculate visibility<br/>and edges
Graph->>React: Update node/edge rendering
React->>React: Animate layout change
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/DependencyGraph/index.tsx`:
- Around line 163-203: clusteredData currently omits directory nodes when a dir
is expanded, so expandedDirs only grows and there is no node to click to
collapse, and dirNodes/counts are wrong; modify the loop that builds dirNodes
(in clusteredData) to always push a directory node for each dirPath (using id
`dir:${dirPath}`) and set its data.isExpanded based on expandedDirs.has(dirPath)
(and data.state accordingly), while still adding individual files to
visibleFiles when expanded; keep fileCount, totalDependents (using
impact.getFileMetrics) and maxRisk (getMaxRisk) computed the same way but ensure
the dir node remains present for expanded dirs so collapse UI and directory
metrics work correctly.
🧹 Nitpick comments (3)
frontend/src/components/DependencyGraph/GraphToolbar.tsx (1)
44-74: Expose toggle state to assistive tech + clarify cluster tooltip.
Consider addingaria-pressedand a stateful tooltip for the cluster toggle (and tests toggle) so screen readers and tooltips reflect the current state.♿ Suggested tweak
<Button variant={clusterByDir ? 'default' : 'secondary'} size="sm" onClick={onToggleCluster} className="h-8" - title="Group files by directory" + title={clusterByDir ? 'Ungroup files by directory' : 'Group files by directory'} + aria-pressed={clusterByDir} > <FolderTree className="w-3.5 h-3.5 mr-1.5" /> Cluster </Button> @@ <Button variant={showTests ? 'secondary' : 'outline'} size="sm" onClick={onToggleTests} className={cn('h-8', !showTests && 'opacity-70 line-through')} title={showTests ? 'Click to hide test files' : 'Click to show test files'} + aria-pressed={showTests} >frontend/src/components/DependencyGraph/hooks/useDirectoryClustering.ts (1)
48-76: Avoid O(N²) lookups when building clusters.
nodes.findinside the per-directory loop scales poorly on large graphs. A simple id→node map avoids repeated scans.♻️ Suggested change
const clusters = useMemo(() => { const clusterMap = new Map<string, DirectoryCluster>() + const nodeById = new Map(nodes.map(n => [n.id, n])) @@ - const nodeData = files.map(f => nodes.find(n => n.id === f)!).filter(Boolean) + const nodeData = files + .map(f => nodeById.get(f)) + .filter((n): n is (typeof nodes)[number] => Boolean(n))frontend/src/components/DependencyGraph/index.tsx (1)
93-104: Consider centralizing dir/risk helpers to avoid drift.
getDirPath/getMaxRiskduplicate logic in the clustering hook; a shared utility would reduce maintenance.
- Add aria-pressed to toggle buttons for accessibility - Build nodeMap for O(1) lookups in clustering hook - Always render dir node so expand/collapse works correctly
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/DependencyGraph/hooks/useDirectoryClustering.ts`:
- Around line 42-116: The useDirectoryClustering hook is unused and duplicates
logic present in index.tsx; either remove it or consolidate to avoid
duplication. Option A: Delete useDirectoryClustering (and its helpers
getDirectoryPath/getMaxRisk) and update any imports if they exist; Option B:
Replace the inline clustering code in index.tsx with a call to
useDirectoryClustering (keeping helper functions only in the hook) and remove
the duplicated helper implementations from index.tsx; ensure exported symbol
useDirectoryClustering and its API (clusters, rootDirs, getClusterForFile,
toggleCluster) match how index.tsx consumes it and update imports accordingly.
🧹 Nitpick comments (3)
frontend/src/components/DependencyGraph/hooks/useDirectoryClustering.ts (1)
21-40: Helper functions duplicated in index.tsx.
getDirectoryPath,getParentDir, andgetMaxRiskare also defined inindex.tsxasgetDirPathandgetMaxRisk. If this hook is intended for use, consider exporting these helpers from a shared location to avoid duplication.frontend/src/components/DependencyGraph/index.tsx (2)
93-104: Duplicate helper functions — consider extracting to shared module.
getDirPathandgetMaxRiskare also defined inuseDirectoryClustering.ts. If clustering logic evolves, maintaining two copies could lead to drift.
243-271: File hover highlighting not applied in clustered mode.In non-clustered mode (lines 298),
hoveredFileIdupgrades a dimmed node to'direct'state. This logic is missing for clustered mode, so hovering over files in the ImpactPanel won't highlight them in the graph when clustering is active.💡 Suggested fix to add hover highlighting in clustered mode
else if (selectedImpact?.transitiveDependents.includes(node.id)) state = 'transitive' else if (selectedNodeId && !selectedNodeId.startsWith('dir:')) state = 'dimmed' + + if (hoveredFileId === node.id && state === 'dimmed') state = 'direct' flowNodes.push({
- Remove unused useDirectoryClustering.ts (dead code) - Add hover highlighting for files in clustered mode - Keeps helpers in index.tsx (single source of truth)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
1 participant
Description
Adds directory clustering to the dependency graph - collapse files by folder for cleaner high-level architecture view.
Changes
New:
DirectoryNode.tsx- Collapsible folder node with aggregated metricsUpdated:
GraphToolbar.tsx- Added cluster buttonindex.tsx- Clustering logic, directory expansion stateindex.css- Smoother animationsFeatures
/tests/pathsKnown Limitation
Summary by CodeRabbit
Release Notes
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.