-
Notifications
You must be signed in to change notification settings - Fork 62
Entity Graph frontend visualization #2821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
## Changes - Add Entity Graph page with interactive node visualization - Add EntityNode component for custom node rendering - Add DetailSidebar for displaying entity details and relationships - Add EntityGraphContent wrapper component - Add EntityGraphModal for contextual relationship viewing - Add ViewRelationshipsButton component for quick access from tables - Add EntityGraphFocusContext for cross-component state management - Add repository functions for fetching entity graph data - Add EntityGraphSteps for guided tour - Update routes to include Entity Graph page - Add View Relationships button to Risk, Vendor, Model, and Project tables ## Features - Interactive node-based visualization with drag & zoom - Entity visibility toggles (use cases, models, vendors, risks, controls, evidence) - Gap detection with templates (compliance, security, operational) - Detail sidebar with entity information - Search and filter capabilities - Edit mode for customizing graph layout 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughAdds a complete Entity Graph feature: new data repositories (graph data + user preferences), React components/pages for graph visualization (React Flow-based content, node rendering, sidebar, guided steps, demo), a focus context and modal for centered views, a View Relationships button used across lists/cards, routing updates, and a standalone static HTML demo. Changes
Sequence DiagramsequenceDiagram
participant User
participant ViewRelationshipsButton
participant EntityGraphModal
participant FocusContext as EntityGraphFocusContext
participant EntityGraphContent
participant Repo as fetchEntityGraphData
participant DetailSidebar
User->>ViewRelationshipsButton: click
ViewRelationshipsButton->>EntityGraphModal: open(with entity)
EntityGraphModal->>FocusContext: setFocusEntity({id,type,label})
EntityGraphModal->>EntityGraphContent: render (focus context available)
EntityGraphContent->>Repo: fetchEntityGraphData()
activate Repo
Repo-->>EntityGraphContent: graph data (nodes/edges/... )
deactivate Repo
EntityGraphContent->>EntityGraphContent: build nodes/edges & center on focus
User->>EntityGraphContent: click node
EntityGraphContent->>DetailSidebar: show details for node
DetailSidebar-->>User: display metadata
User->>EntityGraphModal: close
EntityGraphModal->>FocusContext: clearFocus()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
## Changes - routes.tsx: Remove FairnessDashboard imports (files don't exist in develop) - ProjectTableView.tsx: Remove unused Stack import - DetailSidebar.tsx: Use MuiChip for styled chip (custom Chip doesn't support sx prop) - EntityGraphContent.tsx: Remove unused imports (Position, Chip, Stack, IconButton, AlertTriangle, X, Toggle), fix entityType property name and add onNavigateToEntity prop - EntityGraphSteps.tsx: Add type assertion for placement "center" - index.tsx: Remove unused imports (MuiDivider, Checkbox, ChevronUp), prefix unused gap config with underscore, change submitButtonDisabled to isSubmitting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
## Changes - routes.tsx: Remove FairnessDashboard routes (components don't exist) - EntityGraphContent.tsx: Simplify component, remove unused variables (riskPriority, isModal, focusEntityType, showProblemsOnly, setVisibleRelationships, getNode, yOffset, relationships handling) - EntityGraphSteps.tsx: Change placement from "center" to "auto" - index.tsx: Remove unused gap configs, getNodes, handleGapRuleToggle 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
## Changes - EntityGraphContent.tsx: Fix SearchBox onChange to accept value directly - EntityGraphSteps.tsx: Change placement to "bottom" (valid Joyride value) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Clients/src/application/config/routes.tsx (1)
159-162: Duplicate route key "sharedView".Lines 159 and 162 both define a route with
key="sharedView". This will cause React key collision warnings and the second route definition is redundant.<Route key="sharedView" path="/shared/:resourceType/:token" element={<SharedView />} />, // Style Guide - Development only ...(isDev ? [<Route key="style-guide" path="/style-guide/:section?" element={<StyleGuide />} />] : []), - <Route key="sharedView" path="/shared/:resourceType/:token" element={<SharedView />} />, <Route key="not-found" path="*" element={<PageNotFound />} />,
🧹 Nitpick comments (14)
Clients/public/reactflow-demo.html (1)
1-98: Clarify the purpose and maintenance plan for this demo file.This standalone HTML file uses CDN-loaded libraries (ReactFlow 11.11.4) which may drift from the versions used in the main application. Consider:
- Is this file intended for documentation, testing, or integration elsewhere?
- Should it be kept in sync with the project's ReactFlow version?
- Would it be better served as a regular React component within the app rather than a standalone HTML file?
If this is purely for demonstration or documentation purposes, consider:
- Adding a comment at the top explaining its purpose
- Adding it to a
/docsor/examplesdirectory instead of/public- Creating a maintenance note about keeping library versions aligned
Clients/src/presentation/pages/EntityGraph/EntityNode.tsx (2)
66-125: Consider consolidating color constants.Multiple color constant objects are defined inline (gap severity, deadline status, evidence freshness, vendor tier). This makes it harder to maintain consistent theming.
Consider extracting these to a shared theme constants file:
// entityGraphTheme.ts export const ENTITY_GRAPH_COLORS = { gapSeverity: { critical: '#dc2626', warning: '#f59e0b', info: '#3b82f6', }, deadlineStatus: { overdue: '#dc2626', upcoming: '#f59e0b', normal: '#6b7280', }, // ... etc } as const;This would make it easier to update colors globally and ensure consistency across the Entity Graph UI.
144-148: Document the sizing scale rationale.The node sizing logic uses specific thresholds (2, 5, 10 connections) with corresponding scales (0.95, 1.0, 1.08, 1.15). Without documentation, these magic numbers are unclear.
Consider adding a comment explaining the design decision:
// Phase 7: Node size based on connections // Scale nodes subtly based on connection count to indicate importance: // - 0-2 connections: 95% (less connected) // - 3-5 connections: 100% (normal) // - 6-10 connections: 108% (well connected) // - 11+ connections: 115% (highly connected) const connections = nodeData.connectionCount || 0;Clients/src/presentation/components/Table/VWProjectRisksTable/VWProjectRisksTableBody.tsx (1)
281-285: Move entity ID offset to a centralized utility function with clear documentation.The offset transformation (+100000 for project risks, +200000 for vendor risks) is intentionally applied to avoid ID collisions between different risk types in the entity graph. However, this logic is currently duplicated—it's defined in
entityGraph.repository.tsduring data transformation and again hardcoded in components like this one.Extract this into a utility function (e.g.,
getEntityGraphId()) with constants documenting the offset values per entity type. This ensures:
- Single source of truth for ID transformation logic
- Clearer intent at the component level
- Easier maintenance if offset strategy changes
- Consistency across all ViewRelationshipsButton usages
Clients/src/presentation/pages/ReactFlowDemo.tsx (1)
17-56: Consider extracting hardcoded node styles to constants or theme.The node styles are defined inline with hardcoded color values (e.g.,
#13715B,#4CAF93,#d0d5dd). For consistency with the rest of the codebase and easier maintenance, consider extracting these to a shared theme or constants file.+const nodeStyles = { + input: { background: '#13715B', color: 'white', border: 'none' }, + default: { background: '#fff', border: '1px solid #d0d5dd' }, + output: { background: '#4CAF93', color: 'white', border: 'none' }, +}; + const initialNodes: Node[] = [ { id: '1', type: 'input', data: { label: 'AI Model' }, position: { x: 250, y: 0 }, - style: { background: '#13715B', color: 'white', border: 'none' }, + style: nodeStyles.input, }, // ... apply similar changes to other nodesClients/src/presentation/components/ProjectsList/ProjectTableView.tsx (1)
14-14: Unused import:Stack.The
Stackcomponent is imported but not used in the changed code. TheViewRelationshipsButtonis rendered directly without aStackwrapper.- Stack,Clients/src/presentation/components/EntityGraphModal/index.tsx (2)
44-55: Consider adding cleanup on unmount.The effect correctly sets focus when the modal opens and clears on close. However, the cleanup function should also clear focus to handle cases where the component unmounts while open (e.g., parent component unmounts).
useEffect(() => { if (open) { setFocusEntity({ id: `${focusEntityType}-${focusEntityId}`, type: focusEntityType, label: focusEntityLabel, }); - } else { - clearFocus(); } + return () => { + clearFocus(); + }; // eslint-disable-next-line react-hooks/exhaustive-deps }, [open, focusEntityId, focusEntityType, focusEntityLabel]);
164-168: Fragile CSS selector for hiding breadcrumbs.The selector
'& > div > div:first-of-type'relies on the DOM structure of theEntityGraphcomponent. This could break silently if the structure changes.Consider a more robust approach:
- Pass a prop to
EntityGraphto hide breadcrumbs (e.g.,hideBreadcrumbs={true})- Use a data attribute or class name for targeting
- Use React Context to signal modal mode
- // Hide the page header/breadcrumbs in modal mode - '& > div > div:first-of-type': { - display: 'none', // Hide PageBreadcrumbs - }, + // Pass isModal prop to EntityGraph instead of CSS hidingThen modify the EntityGraph component to accept an
isModalorhideBreadcrumbsprop:<EntityGraph hideBreadcrumbs />Clients/src/presentation/pages/EntityGraph/EntityGraphContent.tsx (2)
60-71: Remove unusedriskPriorityconstant.The
riskPriorityconstant is defined but never used anywhere in the component.-// Risk level priority for coloring -const riskPriority: Record<string, number> = { - 'Critical': 4, - 'Very high risk': 4, - 'High': 3, - 'High risk': 3, - 'Medium': 2, - 'Medium risk': 2, - 'Low': 1, - 'Low risk': 1, - 'Very low risk': 0, -}; -
185-185: Avoidany[]type for better type safety.Using
any[]bypasses TypeScript's type checking. Consider using a union type or a more specific interface.- const addNodes = (items: any[], type: EntityType, typeKey: string) => { + const addNodes = (items: Array<{ id: number; name?: string; title?: string; model?: string; status?: string; severity?: string; risk_level?: string; [key: string]: unknown }>, type: EntityType, typeKey: string) => {Clients/src/application/repository/entityGraph.repository.ts (2)
169-174: Silent error suppression may hide important failures.The
.catch(() => ({ data: { data: [] } }))silently swallows all errors, including network failures and server errors. Consider logging errors for debugging or providing partial failure feedback.const fetchWithProgress = async (url: string) => { - const result = await apiServices.get(url).catch(() => ({ data: { data: [] } })); + const result = await apiServices.get(url).catch((err) => { + console.warn(`Failed to fetch ${url}:`, err); + return { data: { data: [] } }; + }); loaded++; onProgress?.(loaded, total); return result; };
237-256: Extract ID offset magic numbers as named constants.The ID offsets
100000and200000are used to prevent collisions between risk types but their purpose isn't immediately clear. Consider extracting them as named constants.+// ID offsets to prevent collisions between different risk sources +const PROJECT_RISK_ID_OFFSET = 100000; +const VENDOR_RISK_ID_OFFSET = 200000; + const combinedRisks: EntityGraphData['risks'] = [ ...modelRisks.map(r => ({ id: r.id, // ... })), ...projectRisks.map(r => ({ - id: r.id + 100000, // Offset to avoid ID collision + id: r.id + PROJECT_RISK_ID_OFFSET, // ... })), ...vendorRisks.map(r => ({ - id: r.risk_id + 200000, // Offset to avoid ID collision + id: r.risk_id + VENDOR_RISK_ID_OFFSET, // ... })), ];Clients/src/presentation/pages/EntityGraph/DetailSidebar.tsx (2)
82-95: Remove unusedstatusColorsconstant.The
statusColorsmapping is defined but never used in the component.-const statusColors: Record<string, string> = { - 'Approved': '#4caf50', - 'Pending': '#ff9800', - 'Restricted': '#f44336', - 'Blocked': '#9c27b0', - 'Open': '#2196f3', - 'In Progress': '#ff9800', - 'Resolved': '#4caf50', - 'Accepted': '#8bc34a', - 'Not started': '#9e9e9e', - 'In review': '#ff9800', - 'Reviewed': '#4caf50', - 'Requires follow-up': '#f44336', -}; -
97-107: Consider using existingformatDateutility.A
formatDatefunction already exists inClients/src/presentation/tools/isoDateToString.ts. Consider reusing it for consistency, though note the existing utility throws on invalid dates while this implementation gracefully falls back.If you prefer the graceful fallback behavior, this local implementation is fine. Otherwise, consider wrapping the shared utility with try-catch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
Clients/public/reactflow-demo.html(1 hunks)Clients/src/application/config/routes.tsx(5 hunks)Clients/src/application/repository/entityGraph.repository.ts(1 hunks)Clients/src/application/repository/entityGraphUserData.repository.ts(1 hunks)Clients/src/presentation/components/Cards/ProjectCard/index.tsx(2 hunks)Clients/src/presentation/components/EntityGraphModal/index.tsx(1 hunks)Clients/src/presentation/components/ProjectsList/ProjectTableView.tsx(3 hunks)Clients/src/presentation/components/Table/RisksTable/index.tsx(2 hunks)Clients/src/presentation/components/Table/VWProjectRisksTable/VWProjectRisksTableBody.tsx(2 hunks)Clients/src/presentation/components/Table/WithPlaceholder/index.tsx(2 hunks)Clients/src/presentation/components/ViewRelationshipsButton/index.tsx(1 hunks)Clients/src/presentation/contexts/EntityGraphFocusContext.tsx(1 hunks)Clients/src/presentation/pages/EntityGraph/DetailSidebar.tsx(1 hunks)Clients/src/presentation/pages/EntityGraph/EntityGraphContent.tsx(1 hunks)Clients/src/presentation/pages/EntityGraph/EntityGraphSteps.tsx(1 hunks)Clients/src/presentation/pages/EntityGraph/EntityNode.tsx(1 hunks)Clients/src/presentation/pages/ModelInventory/modelInventoryTable.tsx(2 hunks)Clients/src/presentation/pages/ReactFlowDemo.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-18T05:28:01.115Z
Learnt from: MuhammadKhalilzadeh
Repo: bluewave-labs/verifywise PR: 254
File: Clients/src/presentation/components/Modals/Controlpane/index.tsx:22-24
Timestamp: 2024-11-18T05:28:01.115Z
Learning: In `Clients/src/presentation/components/Modals/Controlpane/index.tsx` (TypeScript React component), avoid changing prop types from specific types like `string` to `any` when the exact type is known. Maintaining explicit types ensures better type safety and code reliability.
Applied to files:
Clients/src/presentation/components/ViewRelationshipsButton/index.tsxClients/src/presentation/components/EntityGraphModal/index.tsx
🧬 Code graph analysis (5)
Clients/src/presentation/pages/EntityGraph/EntityGraphSteps.tsx (1)
Clients/src/domain/interfaces/i.tour.ts (1)
IPageTourStep(1-17)
Clients/src/presentation/components/ViewRelationshipsButton/index.tsx (1)
Clients/src/presentation/components/EntityGraphModal/index.tsx (1)
FocusEntityType(20-20)
Clients/src/application/repository/entityGraph.repository.ts (1)
Clients/src/infrastructure/api/networkServices.ts (1)
apiServices(90-240)
Clients/src/presentation/pages/EntityGraph/DetailSidebar.tsx (1)
Clients/src/presentation/tools/isoDateToString.ts (1)
formatDate(10-23)
Clients/src/application/repository/entityGraphUserData.repository.ts (1)
Clients/src/infrastructure/api/networkServices.ts (1)
apiServices(90-240)
🪛 GitHub Actions: Frontend Checks
Clients/src/application/config/routes.tsx
[error] 25-25: TS2307 Cannot find module '../../presentation/pages/FairnessDashboard/FairnessDashboard' or its corresponding type declarations.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (18)
Clients/src/presentation/contexts/EntityGraphFocusContext.tsx (1)
1-52: LGTM!The focus context implementation is well-structured with:
- Proper memoization to prevent unnecessary re-renders
- Graceful fallback when used outside a provider
- Clean separation of concerns
Clients/src/application/repository/entityGraphUserData.repository.ts (1)
106-123: Good error handling for empty responses.The 204 status handling is appropriate for empty annotation lists, providing a clean fallback without throwing errors.
Clients/src/presentation/pages/ModelInventory/modelInventoryTable.tsx (1)
324-362: Clean integration of ViewRelationshipsButton.The action cell properly integrates the new button alongside existing actions with:
- Horizontal Stack layout for proper spacing
- ViewRelationshipsButton always visible for easy access
- Existing delete/edit actions conditionally rendered based on permissions
- Proper event handling with stopPropagation
Clients/src/presentation/components/Table/VWProjectRisksTable/VWProjectRisksTableBody.tsx (1)
280-297: Action layout integration looks good.The Stack-based layout properly integrates the ViewRelationshipsButton with existing actions, maintaining conditional rendering logic.
Clients/src/presentation/components/Table/WithPlaceholder/index.tsx (1)
457-476: Consistent action cell implementation.The integration follows the same pattern as other tables in this PR:
- Stack layout for horizontal button grouping
- stopPropagation to prevent unintended row navigation
- ViewRelationshipsButton for quick relationship access
- Existing actions preserved
Clients/src/presentation/pages/ReactFlowDemo.tsx (1)
67-94: LGTM!The component correctly implements ReactFlow with proper state management using
useNodesStateanduseEdgesState, and theonConnectcallback is properly memoized with the correct dependency. The viewport styling and ReactFlow configuration are appropriate for a demo page.Clients/src/presentation/components/ProjectsList/ProjectTableView.tsx (1)
409-420: LGTM!The ViewRelationshipsButton is properly integrated with correct props (
entityId,entityType,entityLabel) and theonClickhandler correctly stops propagation to prevent row navigation when clicking the action button.Clients/src/presentation/components/ViewRelationshipsButton/index.tsx (1)
1-75: LGTM!Well-structured reusable component with:
- Clear documentation header with usage example
- Proper TypeScript interface with sensible defaults
- Correct event propagation handling
- Clean separation between button and modal concerns
The component follows React best practices and integrates properly with the
EntityGraphModalandVWTooltipcomponents.Clients/src/presentation/components/Table/RisksTable/index.tsx (2)
580-608: LGTM on the structural changes.The action cell refactoring with
Stackwrapper,stopPropagation, and increasedminWidthare appropriate changes for accommodating multiple action buttons.
590-595: The offset pattern is documented and intentional; the proposed fix would break entity graph lookups.The
(row.risk_id || 0) + 200000offset is not undocumented—it is explicitly documented inentityGraph.repository.tswith the comment// Offset to avoid ID collision. The offsets distinguish different risk types: vendor risks use+200000, project risks use+100000. This offset is applied at the repository layer and expected by the entity graph lookup mechanism.Changing this to
row.risk_id!would break the entity relationship modal, as it relies on the offset IDs to resolve the correct risk from the aggregated data set. The variation in how entities are handled (raw ID for projects vs. offset ID for risks) is consistent with the entity type being passed (useCasevs.risk), not an inconsistency.No changes are required.
Clients/src/presentation/components/Cards/ProjectCard/index.tsx (1)
478-503: LGTM!The
ViewRelationshipsButtonis cleanly integrated alongside the existing "View project details" button. TheStacklayout withjustifyContent="flex-end"andgap={1}provides proper spacing and alignment. Props are consistent with the usage inProjectTableView.Clients/src/presentation/components/EntityGraphModal/index.tsx (1)
76-191: LGTM on modal structure and accessibility.The modal implementation is solid with:
- Proper
aria-labelledbyfor accessibility- Lazy loading with Suspense fallback
- Clean header with entity type/label display
- Proper event handling to prevent unintended backdrop dismissal
Clients/src/application/config/routes.tsx (1)
114-114: LGTM!The EntityGraph route is correctly added within the authenticated dashboard route tree.
Clients/src/presentation/pages/EntityGraph/EntityGraphContent.tsx (2)
326-427: LGTM!The ReactFlow integration with controls panel, MiniMap, and Background is well-structured. The node click handler properly builds entity details from the lookup map.
430-437: LGTM!Wrapping with
ReactFlowProviderat the export level is the correct pattern for ReactFlow components that use hooks likeuseReactFlow.Clients/src/application/repository/entityGraph.repository.ts (1)
176-187: LGTM!Using
Promise.allfor concurrent API fetching is efficient. The progress callback pattern provides good UX feedback during loading.Clients/src/presentation/pages/EntityGraph/DetailSidebar.tsx (2)
260-475: LGTM!The entity-specific detail sections are well-organized with consistent patterns using
DetailRowandDetailTexthelpers. The conditional rendering based onentityTypeprovides appropriate context for each entity type.
536-576: LGTM!The footer actions with "Close" and "Go to" buttons provide clear navigation options. The styling follows the VerifyWise design patterns.
| /** | ||
| * Extract data from API response | ||
| */ | ||
| function extractData<T>(response: { data: { data?: T; message?: string } }): T { | ||
| if (response?.data?.data !== undefined) { | ||
| return response.data.data; | ||
| } | ||
| if (response?.data && !response.data.message) { | ||
| return response.data as T; | ||
| } | ||
| return [] as T; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix unsafe type assertion in extractData.
Line 96 returns [] as T, which assumes T is always an array. This is unsafe when T is a non-array type (e.g., EntityGraphAnnotation | null).
Consider a safer implementation:
-function extractData<T>(response: { data: { data?: T; message?: string } }): T {
+function extractData<T>(response: { data: { data?: T; message?: string } }, defaultValue: T): T {
if (response?.data?.data !== undefined) {
return response.data.data;
}
if (response?.data && !response.data.message) {
return response.data as T;
}
- return [] as T;
+ return defaultValue;
}Then call it with appropriate defaults:
const annotations = extractData<EntityGraphAnnotation[]>(response, []);This makes the fallback value explicit and type-safe.
🤖 Prompt for AI Agents
In Clients/src/application/repository/entityGraphUserData.repository.ts around
lines 86 to 97, the extractData<T> function unsafely asserts [] as T on fallback
which assumes T is an array; change the function signature to accept an explicit
fallback/default value parameter (e.g., extractData<T>(response, defaultValue:
T): T) and return that defaultValue in the final return branch instead of [] as
T, then update all callers to pass an appropriate typed default (for example
extractData<EntityGraphAnnotation[]>(response, []) or
extractData<EntityGraphAnnotation | null>(response, null)).
| const handleGoToEntity = () => { | ||
| const route = entityRoutes[entity.entityType]; | ||
| if (route) { | ||
| // Extract numeric ID from entity id (e.g., "project-1" -> "1") | ||
| const numericId = entity.id.split('-').slice(1).join('-'); | ||
| navigate(`${route}?id=${numericId}`); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find entityRoutes definition to see all possible target routes
rg -n "entityRoutes\s*=" --type=tsx --type=ts -A 15 | head -60Repository: bluewave-labs/verifywise
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Check target route components for id query parameter handling
rg -n "useSearchParams|searchParams|new URLSearchParams|getUrlParameter|query.*id|const.*id.*from.*query|const.*id.*from.*search" --type=tsx --type=ts -A 2 | head -100Repository: bluewave-labs/verifywise
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Also search for common patterns of accessing query params in Next.js/React Router
rg -n "useRouter|useLocation|window\.location|URLSearchParams" --type=tsx --type=ts Clients/src/presentation/pages/ | grep -E "(ProjectView|ModelInventory|RiskManagement|Vendors)" | head -50Repository: bluewave-labs/verifywise
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Find entityRoutes definition
rg -n "entityRoutes\s*=" -A 15 Clients/src/presentation/pages/Repository: bluewave-labs/verifywise
Length of output: 50
🏁 Script executed:
#!/bin/bash
# List structure of pages directory to understand target routes
ls -la Clients/src/presentation/pages/Repository: bluewave-labs/verifywise
Length of output: 2064
🏁 Script executed:
#!/bin/bash
# Search for entityRoutes in the entire codebase
rg -n "entityRoutes" Clients/src/Repository: bluewave-labs/verifywise
Length of output: 298
🏁 Script executed:
#!/bin/bash
# View entityRoutes definition
sed -n '71,100p' Clients/src/presentation/pages/EntityGraph/DetailSidebar.tsxRepository: bluewave-labs/verifywise
Length of output: 819
🏁 Script executed:
#!/bin/bash
# Also view the handleGoToEntity function context
sed -n '150,165p' Clients/src/presentation/pages/EntityGraph/DetailSidebar.tsxRepository: bluewave-labs/verifywise
Length of output: 452
🏁 Script executed:
#!/bin/bash
# Check ProjectView for id query parameter handling
rg -n "useSearchParams|searchParams|URLSearchParams|id" Clients/src/presentation/pages/ProjectView/ | head -40Repository: bluewave-labs/verifywise
Length of output: 4642
🏁 Script executed:
#!/bin/bash
# Check ModelInventory for id query parameter handling
rg -n "useSearchParams|searchParams|URLSearchParams|id" Clients/src/presentation/pages/ModelInventory/ | head -40Repository: bluewave-labs/verifywise
Length of output: 4674
🏁 Script executed:
#!/bin/bash
# Check RiskManagement for id query parameter handling
rg -n "useSearchParams|searchParams|URLSearchParams|id" Clients/src/presentation/pages/RiskManagement/ | head -40Repository: bluewave-labs/verifywise
Length of output: 5097
🏁 Script executed:
#!/bin/bash
# Check Vendors for id query parameter handling
rg -n "useSearchParams|searchParams|URLSearchParams|id" Clients/src/presentation/pages/Vendors/ | head -40Repository: bluewave-labs/verifywise
Length of output: 4231
🏁 Script executed:
#!/bin/bash
# Double-check ModelInventory main component for any query parameter handling
cat Clients/src/presentation/pages/ModelInventory/index.tsx | head -60Repository: bluewave-labs/verifywise
Length of output: 2594
🏁 Script executed:
#!/bin/bash
# Check Vendors main component for query parameter handling
sed -n '1,100p' Clients/src/presentation/pages/Vendors/index.tsx | grep -E "useSearchParams|searchParams|useLocation|location\.search"Repository: bluewave-labs/verifywise
Length of output: 240
🏁 Script executed:
#!/bin/bash
# Verify what searchParams variable is used for in ModelInventory
rg -n "searchParams" Clients/src/presentation/pages/ModelInventory/index.tsxRepository: bluewave-labs/verifywise
Length of output: 297
Fix query parameter names in handleGoToEntity to match target route expectations.
The function sends a generic id parameter to all routes, but each target route expects different parameter names:
/project-viewexpectsprojectId/risk-managementexpectsriskId/model-inventoryexpectsmodelIdorevidenceId(depending on entity type)/vendorsdoes not appear to handle query parameters for entity selection
Update the navigation logic to send route-specific parameter names based on entity.entityType, or standardize all target routes to accept the same parameter name.
🤖 Prompt for AI Agents
In Clients/src/presentation/pages/EntityGraph/DetailSidebar.tsx around lines 153
to 160, the handler always navigates using a generic ?id= query but each target
route expects different parameter names; change handleGoToEntity to map
entity.entityType to the correct query key (e.g., project -> projectId, risk ->
riskId, model -> modelId, evidence -> evidenceId) extract the numeric part of
entity.id as now, build the route-specific query (or omit query for vendors and
simply navigate to the route), and navigate using that constructed query string
so each target route receives the parameter name it expects.
| setTimeout(() => { | ||
| fitView({ padding: 0.2, duration: 300 }); | ||
| }, 100); | ||
| }, [entityData, visibleEntities, debouncedSearchQuery, visibleRelationships, focusEntityId, setNodes, setEdges, fitView]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential memory leak with setTimeout on unmount.
The setTimeout in the layout effect is not cleaned up when the component unmounts, which could cause state updates on unmounted components.
Consider moving the fitView call to a separate effect with proper cleanup:
+ // Fit view after nodes are set
+ const [shouldFitView, setShouldFitView] = useState(false);
+
+ useEffect(() => {
+ if (shouldFitView) {
+ const timer = setTimeout(() => {
+ fitView({ padding: 0.2, duration: 300 });
+ setShouldFitView(false);
+ }, 100);
+ return () => clearTimeout(timer);
+ }
+ }, [shouldFitView, fitView]);Then set setShouldFitView(true) instead of calling setTimeout directly.
Committable suggestion skipped: line range outside the PR's diff.
| placement: "bottom", | ||
| }, | ||
| { | ||
| target: '.react-flow__nodes', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a more specific selector.
The selector .react-flow__nodes targets a library-internal class from ReactFlow. If ReactFlow updates and changes this class name, the tour step will break.
Consider using a data-joyride-id attribute instead for better stability:
- target: '.react-flow__nodes',
+ target: '[data-joyride-id="entity-graph-nodes"]',Then ensure the corresponding element in your ReactFlow component has this attribute.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Clients/src/presentation/pages/EntityGraph/EntityGraphSteps.tsx around line
24, the tour step currently targets the internal ReactFlow class
`.react-flow__nodes` which is brittle; replace this with a stable data attribute
selector (e.g. target: '[data-joyride-id="entity-graph-nodes"]') and add the
matching data-joyride-id attribute to the corresponding element in the ReactFlow
component so the tour targets that stable attribute instead of a
library-internal class.
| const EntityNode: React.FC<NodeProps> = ({ data, sourcePosition, targetPosition }) => { | ||
| const nodeData = data as unknown as EntityNodeData; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid double type assertion.
The double cast data as unknown as EntityNodeData bypasses TypeScript's type checking completely.
Instead, define the proper type for NodeProps or verify the data structure:
-const EntityNode: React.FC<NodeProps> = ({ data, sourcePosition, targetPosition }) => {
- const nodeData = data as unknown as EntityNodeData;
+const EntityNode: React.FC<NodeProps<EntityNodeData>> = ({ data, sourcePosition, targetPosition }) => {
+ const nodeData = data;If @xyflow/react properly types NodeProps with a generic parameter, use it directly. This ensures type safety without dangerous casts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const EntityNode: React.FC<NodeProps> = ({ data, sourcePosition, targetPosition }) => { | |
| const nodeData = data as unknown as EntityNodeData; | |
| const EntityNode: React.FC<NodeProps<EntityNodeData>> = ({ data, sourcePosition, targetPosition }) => { | |
| const nodeData = data; |
🤖 Prompt for AI Agents
In Clients/src/presentation/pages/EntityGraph/EntityNode.tsx around lines
127-128, remove the unsafe double-cast "data as unknown as EntityNodeData" and
instead type the component properly or validate at runtime: if the library's
NodeProps supports a generic, declare the component as
React.FC<NodeProps<EntityNodeData>> (or change the function signature to accept
NodeProps<EntityNodeData>) and use data directly as EntityNodeData; if the
generic is not available, add a small type-guard function that asserts/validates
the expected fields on data and use that to narrow the type before using it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
Clients/src/presentation/pages/EntityGraph/EntityGraphSteps.tsx (1)
24-30: Avoid relying on ReactFlow’s internal class selector for the tour.Step 3 targets
.react-flow__nodes, which is a library-internal class and may change between ReactFlow versions, breaking the tour. Prefer a stable data attribute instead, e.g.:- target: '.react-flow__nodes', + target: '[data-joyride-id="entity-graph-nodes"]',Then add
data-joyride-id="entity-graph-nodes"to the appropriate element in your ReactFlow graph (e.g., node container or wrapper).Clients/src/presentation/pages/EntityGraph/EntityGraphContent.tsx (1)
192-196: Clean upsetTimeoutused forfitViewto avoid updates after unmount.The layout effect schedules a
setTimeoutto callfitViewbut doesn’t clear it on dependency changes or unmount. This can trigger ReactFlow updates on an unmounted or reinitialized canvas.Consider moving this into a dedicated effect with a tracked flag and clearing the timer in the cleanup, e.g.:
const [shouldFitView, setShouldFitView] = useState(false); // When nodes change, request fitView useEffect(() => { if (entityData) { setShouldFitView(true); } }, [entityData, visibleEntities, debouncedSearchQuery, focusEntityId]); useEffect(() => { if (!shouldFitView) return; const timer = setTimeout(() => { fitView({ padding: 0.2, duration: 300 }); setShouldFitView(false); }, 100); return () => clearTimeout(timer); }, [shouldFitView, fitView]);
🧹 Nitpick comments (4)
Clients/src/presentation/pages/EntityGraph/EntityGraphSteps.tsx (1)
6-49: Ensure all tour targets correspond to realdata-joyride-idattributes.The steps target several selectors (
entity-graph-canvas,entity-search,gap-detection,impact-analysis). Please confirm the corresponding elements inEntityGraphContentand related controls have matchingdata-joyride-idattributes; otherwise, the tour will silently fail for those steps.A natural place for:
entity-graph-canvasis the main ReactFlow wrapper/container.entity-searchis the search box wrapper.Clients/src/presentation/pages/EntityGraph/EntityGraphContent.tsx (3)
198-212: Consider also cleaning up thesetTimeoutused for centering on a focused entity.Similar to the
fitVieweffect, this effect schedules asetTimeoutforsetCenterbut doesn’t clear it when unmounting/navigation happens quickly. While less likely to cause issues, you can guard it the same way:useEffect(() => { if (!focusEntityId || loading || nodes.length === 0 || hasFocused.current) return; const targetNode = nodes.find(n => n.id === focusEntityId); if (!targetNode) return; const timer = setTimeout(() => { setCenter(targetNode.position.x + 100, targetNode.position.y + 50, { zoom: 1.5, duration: 500, }); hasFocused.current = true; }, 300); return () => clearTimeout(timer); }, [focusEntityId, loading, nodes, setCenter]);
266-318: Wire updata-joyride-idattributes to support the new tour steps.Given
EntityGraphStepstargets:
entity-graph-canvasentity-searchthis component is the natural place to expose those attributes. For example:
- <Box - ref={reactFlowWrapper} - sx={{ + <Box + ref={reactFlowWrapper} + data-joyride-id="entity-graph-canvas" + sx={{ width: '100%', height: '100%', position: 'relative', }} >And for the search step, you can mark the wrapper:
- <Box sx={{ mb: 2 }}> + <Box sx={{ mb: 2 }} data-joyride-id="entity-search"> <SearchBox value={searchQuery} onChange={(value: string) => setSearchQuery(value)}This keeps selectors stable and aligned with the tour config.
68-76: Unused props (isModal,focusEntityType) in inner component.
EntityGraphContentPropsdefinesisModalandfocusEntityType, butEntityGraphContentInneronly consumesfocusEntityId. If these props are not yet used:
- Either remove them from the props for now, or
- Add a short TODO explaining the planned behavior (e.g., modal-specific layout, type-based styling).
This keeps the API surface aligned with actual behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Clients/src/application/config/routes.tsx(3 hunks)Clients/src/presentation/components/ProjectsList/ProjectTableView.tsx(3 hunks)Clients/src/presentation/pages/EntityGraph/DetailSidebar.tsx(1 hunks)Clients/src/presentation/pages/EntityGraph/EntityGraphContent.tsx(1 hunks)Clients/src/presentation/pages/EntityGraph/EntityGraphSteps.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Clients/src/presentation/components/ProjectsList/ProjectTableView.tsx
- Clients/src/application/config/routes.tsx
- Clients/src/presentation/pages/EntityGraph/DetailSidebar.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-18T05:28:01.115Z
Learnt from: MuhammadKhalilzadeh
Repo: bluewave-labs/verifywise PR: 254
File: Clients/src/presentation/components/Modals/Controlpane/index.tsx:22-24
Timestamp: 2024-11-18T05:28:01.115Z
Learning: In `Clients/src/presentation/components/Modals/Controlpane/index.tsx` (TypeScript React component), avoid changing prop types from specific types like `string` to `any` when the exact type is known. Maintaining explicit types ensures better type safety and code reliability.
Applied to files:
Clients/src/presentation/pages/EntityGraph/EntityGraphSteps.tsx
🧬 Code graph analysis (2)
Clients/src/presentation/pages/EntityGraph/EntityGraphContent.tsx (3)
Clients/src/presentation/pages/EntityGraph/EntityNode.tsx (1)
EntityType(21-21)Clients/src/application/repository/entityGraph.repository.ts (2)
EntityGraphData(3-136)fetchEntityGraphData(141-282)Clients/src/presentation/pages/EntityGraph/DetailSidebar.tsx (1)
EntityDetails(37-53)
Clients/src/presentation/pages/EntityGraph/EntityGraphSteps.tsx (1)
Clients/src/domain/interfaces/i.tour.ts (1)
IPageTourStep(1-17)
| const [nodes, setNodes, onNodesChange] = useNodesState<Node>([]); | ||
| const [edges, , onEdgesChange] = useEdgesState<Edge>([]); | ||
| const [loading, setLoading] = useState(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find and read the EntityGraphContent.tsx file
fd EntityGraphContent.tsx --type fRepository: bluewave-labs/verifywise
Length of output: 133
🏁 Script executed:
wc -l Clients/src/presentation/pages/EntityGraph/EntityGraphContent.tsxRepository: bluewave-labs/verifywise
Length of output: 137
🏁 Script executed:
cat -n Clients/src/presentation/pages/EntityGraph/EntityGraphContent.tsxRepository: bluewave-labs/verifywise
Length of output: 15228
🏁 Script executed:
# Find the EntityGraphData type definition
fd entityGraph.repository -type fRepository: bluewave-labs/verifywise
Length of output: 238
🏁 Script executed:
# Find the entityGraph repository file
fd entityGraph.repositoryRepository: bluewave-labs/verifywise
Length of output: 128
🏁 Script executed:
cat -n Clients/src/application/repository/entityGraph.repository.tsRepository: bluewave-labs/verifywise
Length of output: 10139
Edges are initialized but never populated—no relationships are visualized in the graph.
useEdgesState is set up at line 78 but the setEdges function is intentionally discarded (using the comma pattern). Throughout the component, edges are never constructed, despite EntityGraphData containing explicit relationship fields: projects[] and frameworks[] arrays in models and useCases, model_id/project_id/vendor_id in risks, control_id and project_id in evidence, and projects[] in vendors.
As a result, ReactFlow renders isolated nodes without connections, defeating the purpose of a graph visualization.
Build edges from these relationship fields (e.g., connect risks to their parent model/project/vendor, evidence to controls, models to projects) or remove the edges state until edge construction is implemented.
| const [visibleEntities, setVisibleEntities] = useState<string[]>([ | ||
| 'useCases', 'models', 'vendors', 'risks', 'controls' | ||
| ]); | ||
| const [selectedEntity, setSelectedEntity] = useState<EntityDetails | null>(null); | ||
| const [entityLookup, setEntityLookup] = useState<Map<string, Record<string, unknown>>>(new Map()); | ||
| const [searchQuery, setSearchQuery] = useState(''); | ||
| const [debouncedSearchQuery, setDebouncedSearchQuery] = useState(''); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Visibility toggles and node creation are inconsistent for evidence/frameworks.
Current behavior:
visibleEntitiesdefault excludesevidenceand any framework key.addNodesis called forevidence, but there is no way to toggle it on in the UI.- Frameworks (and users) exist in
EntityGraphDataandentityColorsbut aren’t passed intoaddNodesat all. entityCountsincludesevidencebut the counts are never surfaced in the UI.
This leads to confusing behavior (evidence/framework data fetched but effectively invisible) and diverges from the PR objective of toggles for “use cases, models, vendors, risks, controls, evidence, frameworks”.
Either:
- Add toggles for
evidence(and frameworks, if you intend to render them) and include them invisibleEntitiesdefaults, or - Remove evidence/framework handling from this component and the counts to keep behavior consistent.
Also applies to: 182-189, 239-246, 324-352
🤖 Prompt for AI Agents
In Clients/src/presentation/pages/EntityGraph/EntityGraphContent.tsx (around
lines 82-89, 182-189, 239-246, 324-352) the component fetches evidence and
framework data but never exposes toggles or passes frameworks/users into
node-creation, causing invisible data and inconsistent counts; fix by adding
'evidence' and the framework key(s) to the visibleEntities default array, add
corresponding UI toggle controls that update visibleEntities state, ensure
addNodes is invoked for evidence and frameworks/users the same way as other
entity types (and pass their color entries from entityColors), and surface
entityCounts.evidence (and frameworks) in the UI where other counts are shown;
alternatively, if you intend not to render these types, remove the
evidence/framework fetches and counts and any addNodes handling so the component
behavior is consistent.
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const addNodes = (items: any[], type: EntityType, typeKey: string) => { | ||
| if (!visibleEntities.includes(typeKey)) return; | ||
|
|
||
| items?.forEach((item) => { | ||
| const nodeId = `${type}-${item.id}`; | ||
| const isFocusedNode = focusEntityId === nodeId; | ||
|
|
||
| // Filter by search | ||
| const label = item.name || item.title || item.model || `${type} #${item.id}`; | ||
| if (debouncedSearchQuery && !label.toLowerCase().includes(debouncedSearchQuery.toLowerCase())) { | ||
| return; | ||
| } | ||
|
|
||
| const x = (xOffset % NODES_PER_ROW) * SPACING_X + 100; | ||
| const y = Math.floor(xOffset / NODES_PER_ROW) * SPACING_Y + 100; | ||
| nodePositions.set(nodeId, { x, y }); | ||
|
|
||
| newNodes.push({ | ||
| id: nodeId, | ||
| type: 'entity', | ||
| position: { x, y }, | ||
| data: { | ||
| label, | ||
| sublabel: item.status || item.severity || '', | ||
| entityType: type, | ||
| color: entityColors[type], | ||
| status: item.status, | ||
| riskLevel: item.severity || item.risk_level, | ||
| rawData: item, | ||
| isFocused: isFocusedNode, | ||
| } as ExtendedNodeData, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Node label derivation misses key fields for several entity types.
The generic label logic:
const label = item.name || item.title || item.model || `${type} #${item.id}`;doesn’t cover common fields from EntityGraphData:
- Use cases:
project_title - Vendors:
vendor_name - Risks:
risk_name
Those nodes will appear as “useCase #id”, “vendor #id”, “risk #id”, and search will also key off those placeholders.
Consider making label selection type-aware, e.g.:
const label =
type === 'useCase' ? item.project_title :
type === 'vendor' ? item.vendor_name :
type === 'risk' ? item.risk_name :
item.name || item.title || item.model || `${type} #${item.id}`;This will greatly improve readability and search usefulness.
🤖 Prompt for AI Agents
In Clients/src/presentation/pages/EntityGraph/EntityGraphContent.tsx around
lines 145 to 177, the current generic label derivation (item.name || item.title
|| item.model || `${type} #${item.id}`) omits type-specific fields causing
useCase, vendor and risk nodes to get poor default labels; update the label
logic to be type-aware by mapping type 'useCase' to item.project_title, 'vendor'
to item.vendor_name, and 'risk' to item.risk_name, then fall back to the
existing generic sequence (item.name, item.title, item.model, `${type}
#${item.id}`); ensure the computed label is used for both display and search
filtering so those entity types are searchable and rendered correctly.
Summary
Features
Files changed
Clients/src/presentation/pages/EntityGraph/- Main graph page componentsClients/src/presentation/components/EntityGraphModal/- Modal wrapperClients/src/presentation/components/ViewRelationshipsButton/- Table action buttonClients/src/presentation/contexts/EntityGraphFocusContext.tsx- State managementClients/src/application/repository/entityGraph*.ts- Data fetchingClients/src/application/config/routes.tsx- Route registration