feat: Dependency Graph V2 - Impact Analysis & Visual Redesign#227
Conversation
- Add useImpactAnalysis hook for traversing dependents - Add custom GraphNode component with risk badges and state styling - Add ImpactPanel slide-out with direct/transitive dependents - Add GraphToolbar with show all/filter controls - Refactor to modular component structure - Default to top 15 files by importance - Click-to-highlight with cascade animation - LR layout for better import flow visualization
…h lucide icons, add light mode
|
@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. |
📝 WalkthroughWalkthroughThis PR adds a dev helper script, streamlines Makefile targets, switches the frontend Docker build to Bun, replaces the monolithic DependencyGraph component with a modular implementation (graph rendering, nodes, toolbar, impact analysis hook), and enforces a min-height for DashboardHome tabs. Changes
Sequence Diagram(s)mermaid User->>DG: open graph view Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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: 3
🤖 Fix all issues with AI agents
In `@frontend/Dockerfile`:
- Around line 19-20: The RUN line "RUN bun install --frozen-lockfile || bun
install" makes installs non-deterministic; instead, make the intent explicit by
checking for the presence of the lockfile (the COPY step uses "COPY package.json
bun.lock* ./") and run either "bun install --frozen-lockfile" when a bun.lock*
exists or fail the build (or explicitly run unfrozen install only when you
intentionally want that behavior). Update the Dockerfile around the COPY/CMD
block so the build fails on missing/outdated lockfile (or clearly branches based
on existence of bun.lock*) and replace the ambiguous "RUN bun install
--frozen-lockfile || bun install" with the explicit conditional or single
deterministic "bun install --frozen-lockfile" call referenced here.
In `@frontend/src/components/DependencyGraph/GraphNode.tsx`:
- Around line 32-82: getFileType currently collapses .json/.yaml and similar
into 'config' and never returns 'json', 'yaml' or 'markdown', so FILE_ICONS and
LANGUAGE_COLORS for those keys are unused; update the getFileType function to
detect extensions (.json -> 'json', .yaml/.yml -> 'yaml', .md/.markdown ->
'markdown') and return those specific strings, keep existing detection for
'test', 'config' (for generic config filenames), and language-based return for
'python'/'javascript'/'typescript' so the constants FILE_ICONS and
LANGUAGE_COLORS are actually applied.
In `@frontend/src/components/DependencyGraph/ImpactPanel.tsx`:
- Around line 1-116: The CollapsibleSection component's local isOpen state
(initialized from defaultOpen) doesn't update when switching files, so add a
useEffect inside CollapsibleSection that watches defaultOpen and files and calls
setIsOpen(defaultOpen) to sync the collapse state on prop changes; locate the
isOpen and setIsOpen usage in CollapsibleSection and ensure the effect runs when
either defaultOpen or files changes to reset the UI when a new file is selected.
🧹 Nitpick comments (9)
frontend/Dockerfile (1)
1-2: Pin the Bun base image to a concrete version/digest.
A floating tag likeoven/bun:1can change unexpectedly and break reproducibility.Makefile (1)
4-38: Include all alias targets in.PHONY.
Ensuresfrontend,backend,up,logs-backend, andpscan’t be shadowed by files.♻️ Proposed fix
-.PHONY: f b all logs restart down status clean help +.PHONY: f frontend b backend all up down restart logs logs-backend status ps clean helpfrontend/src/components/DependencyGraph/GraphToolbar.tsx (1)
2-2: Unused import:FilterThe
Filtericon is imported but not used in the component. Consider removing it to keep imports clean.-import { RotateCcw, Maximize2, Filter, Eye, EyeOff } from 'lucide-react' +import { RotateCcw, Maximize2, Eye, EyeOff } from 'lucide-react'frontend/src/components/DependencyGraph/index.tsx (4)
96-96: Consider typingrawGraphDatainstead of usingany.Using
anyloses type safety. Consider defining a proper type or reusing theGraphDatainterface fromuseImpactAnalysis.ts.+interface GraphData { + nodes: Array<{ id: string; label?: string; language?: string; import_count?: number; imports?: number }> + edges: Array<{ source: string; target: string }> +} + -const [rawGraphData, setRawGraphData] = useState<any>(null) +const [rawGraphData, setRawGraphData] = useState<GraphData | null>(null)This would also enable proper typing on lines 112, 137-140, 177-178, 238, 253, 264, and 293.
205-210: MissingfitViewin dependency array.The
fitViewfunction is used inside this effect but not listed in the dependency array. WhilefitViewfromuseReactFlowis typically stable, omitting it may cause linting warnings or stale closure issues.useEffect(() => { if (nodes.length > 0) { const minZoom = nodes.length > 20 ? 0.5 : 0.3 setTimeout(() => fitView({ padding: 0.2, duration: 300, minZoom }), 100) } - }, [showAll, showTests]) + }, [showAll, showTests, fitView, nodes.length])Note: Adding
nodes.lengthwould trigger fitView on every node change. If that's undesired, consider using a ref to track if this is a toggle-triggered change.
109-127: Dependency array includes unusedimpact.fileMetrics.The
visibleNodeIdsmemo depends onimpact.fileMetrics(line 127), but the memo body only usesimpact.isReadyandimpact.getTopFiles. SincegetTopFilesis already memoized withfileMetricsas its dependency, this extra dependency may cause unnecessary recalculations.- }, [rawGraphData, impact.isReady, impact.fileMetrics, showAll, showTests]) + }, [rawGraphData, impact.isReady, impact.getTopFiles, showAll, showTests])Alternatively, if
getTopFilesidentity changes frequently, consider inlining the slice logic here.
302-303: Fixed height may limit responsiveness.The graph container has a hardcoded
600pxheight. This works but may not adapt well to different viewport sizes. Consider using a responsive approach likecalc(100vh - offset)or a CSS class if the design requirements allow flexibility.frontend/src/components/DependencyGraph/hooks/useImpactAnalysis.ts (2)
73-97: Performance: Use Sets instead of arrays withincludes()checks.The
directDependents.includes(depId)andtransitiveDependents.includes(depId)calls (lines 89, 91) are O(n) operations inside a loop, leading to O(n²) complexity for large graphs. Using Sets for membership checks would be O(1).♻️ Suggested improvement
const getDependents = useCallback((fileId: string, maxDepth = Infinity): ImpactResult => { - const directDependents: string[] = [] - const transitiveDependents: string[] = [] + const directDependentsSet = new Set<string>() + const transitiveDependentsSet = new Set<string>() const visited = new Set<string>() function traverse(currentId: string, depth: number) { if (visited.has(currentId) || depth > maxDepth) return visited.add(currentId) const dependents = fileToDependents.get(currentId) if (!dependents) return dependents.forEach(depId => { if (depId === fileId) return // skip self if (depth === 0) { - if (!directDependents.includes(depId)) directDependents.push(depId) + directDependentsSet.add(depId) } else { - if (!transitiveDependents.includes(depId) && !directDependents.includes(depId)) { - transitiveDependents.push(depId) + if (!directDependentsSet.has(depId)) { + transitiveDependentsSet.add(depId) } } traverse(depId, depth + 1) }) } traverse(fileId, 0) + const directDependents = Array.from(directDependentsSet) + const transitiveDependents = Array.from(transitiveDependentsSet) const allDependents = [...directDependents, ...transitiveDependents]
158-161: Consider using a Map for O(1) lookups.
getFileMetricsusesArray.find()which is O(n). Since it's called for every node during rendering (inindex.tsxline 141), this becomes O(n²) for large graphs. A pre-computed Map would provide O(1) lookups.♻️ Suggested improvement
+ // Pre-compute map for O(1) lookups + const fileMetricsMap = useMemo(() => { + return new Map(fileMetrics.map(f => [f.id, f])) + }, [fileMetrics]) + // Get metrics for a specific file const getFileMetrics = useCallback((fileId: string): FileMetrics | null => { - return fileMetrics.find(f => f.id === fileId) || null - }, [fileMetrics]) + return fileMetricsMap.get(fileId) || null + }, [fileMetricsMap])
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/DependencyGraph/GraphNode.tsx`:
- Around line 44-53: LANGUAGE_COLORS is missing a 'markdown' entry so when
getFileType returns 'markdown' the lookup falls back to 'unknown'; update
LANGUAGE_COLORS to include a 'markdown' key with the appropriate Tailwind
classes (match the visual intent used in FILE_ICONS/FileText) so markdown files
use the intended styling; verify getFileType and the lookup at where
LANGUAGE_COLORS is referenced still work without further changes.
🧹 Nitpick comments (3)
frontend/src/components/DependencyGraph/GraphNode.tsx (1)
1-18: Unused icon imports.
AlertTriangle,Flame,CheckCircle2, andCircleAlertare imported but not used in this component. The localRISK_CONFIGonly usesvariant,label, andclassNameproperties—not icons. These icons are used inImpactPanel.tsxinstead.♻️ Suggested fix
import { FileCode2, FileJson, FileText, TestTube2, Settings, - File, - AlertTriangle, - Flame, - CheckCircle2, - CircleAlert + File } from 'lucide-react'frontend/src/components/DependencyGraph/ImpactPanel.tsx (2)
245-252: Parameter naming inconsistency.
onAnalyzeInSearchis typed as(fileId: string) => void(line 28), but line 247 passesfullPath. While both are strings and likely equivalent in this context, the naming mismatch betweenfileIdandfullPathcould cause confusion for future maintainers.Consider renaming the parameter type to match the actual usage:
- onAnalyzeInSearch?: (fileId: string) => void + onAnalyzeInSearch?: (fullPath: string) => void
131-144: Consider addingaria-expandedfor accessibility.The collapsible section toggle would benefit from
aria-expandedto convey state to screen readers.♻️ Suggested enhancement
<button onClick={() => setIsOpen(!isOpen)} + aria-expanded={isOpen} className="w-full flex items-center gap-2 text-left hover:bg-zinc-100 dark:hover:bg-zinc-800/50 -mx-2 px-2 py-1 rounded transition-colors" >
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
1 participant
Description
Complete redesign of the Dependency Graph with click-to-highlight impact analysis.
Changes
Core Impact Analysis
Visual Improvements
UX Features
Commits
Summary by CodeRabbit
New Features
Performance
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.