feat(dashboard): Add SOC data visualization charts with interactive filtering (#126)#193
Conversation
- Updated App.css to improve chart layout and responsiveness, including new styles for chart headers, controls, and heatmap components. - Modified App.tsx to integrate advanced filtering options, including day of the week and hour range, enhancing the event filtering capabilities. - Refactored filter handling logic to accommodate new filter states and improve overall performance. These changes aim to provide a more user-friendly experience and better data visualization on the dashboard.
|
@Ingole712521 is attempting to deploy a commit to the s3dfx-cyber's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds five interactive Recharts-based chart components (threat type breakdown, detections over time, severity distribution, top attack vectors, detection heatmap) with shared aggregation utilities, type definitions, chart constants, and mock data. ChangesSOC Dashboard Analytics Charts & Interactive Filtering
Sequence Diagram(s)sequenceDiagram
rect rgba(30, 100, 200, 0.5)
note over User,AlertFeedUI: Chart click-to-filter flow
end
participant User
participant ChartComponent
participant applyChartFilter
participant FilterState
participant URLSearchParams
participant filteredEvents
participant AlertFeedUI
User->>ChartComponent: clicks bar / segment / heatmap cell
ChartComponent->>applyChartFilter: onFilter(ChartFilterAction)
applyChartFilter->>FilterState: merge ChartFilterAction into current filters
applyChartFilter->>URLSearchParams: serialize updated FilterState to URL
applyChartFilter->>AlertFeedUI: switch active tab to events
applyChartFilter->>AlertFeedUI: show advanced filters panel
FilterState->>filteredEvents: recompute with threatType + dayOfWeek + hour constraints
filteredEvents->>AlertFeedUI: render filtered events table with Threat Type column
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
4 issues found across 14 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
dashboard/src/App.tsx (1)
483-499: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffResolve threat types once instead of per-row/per-event.
withThreatType([event])[0].resolvedThreatTypeis invoked per table row (Line 486), and the same pattern runs per event infilteredEvents(Line 231) and again forthreatTypes(Line 278). For larger event sets this repeats the regex inference several times per render. Consider computingwithThreatType(events)once (memoized) and threadingresolvedThreatTypethrough filtering, the option list, and the table.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/src/App.tsx` around lines 483 - 499, The withThreatType([event]) function is being called per-event inside the filteredEvents.map() function, causing redundant threat type resolution on every render. Refactor by computing withThreatType() once for the entire filteredEvents array before the map function (or at the component level using useMemo for memoization), store the result, and then access the pre-computed resolvedThreatType values in the map function instead of calling withThreatType for each individual event. This avoids repeating the same regex inference across multiple renders and filtering operations.dashboard/src/App.css (3)
201-211: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace deprecated
clipproperty with modern alternative.The
clipproperty on line 208 is deprecated. The modern screen-reader-only pattern usesclip-path: inset(50%)instead.♻️ Proposed fix
.chart-sr-summary { position: absolute; width: 1px; height: 1px; padding: 0; margin: -1px; overflow: hidden; - clip: rect(0, 0, 0, 0); + clip-path: inset(50%); white-space: nowrap; border: 0; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/src/App.css` around lines 201 - 211, The .chart-sr-summary class uses the deprecated clip property with clip: rect(0, 0, 0, 0). Replace this deprecated property with the modern alternative clip-path: inset(50%) to maintain the screen-reader-only pattern while using the current CSS standard. Remove the old clip property line and add clip-path: inset(50%) in its place.Source: Linters/SAST tools
392-417: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting tooltip colors to CSS variables.
The Recharts tooltip overrides use hardcoded colors (
#18181b,#27272a,#e4e4e7,#a1a1aa) while the rest of the stylesheet uses CSS variables. Extracting these to variables would improve consistency and make future theme adjustments easier.Example approach
/* Recharts tooltip — ensure readable text on dark backgrounds */ .recharts-default-tooltip { background-color: var(--tooltip-bg) !important; border: 1px solid var(--tooltip-border) !important; border-radius: 8px !important; box-shadow: 0 4px 12px rgba(0, 0, 0, 0.4) !important; } .recharts-tooltip-label { color: var(--tooltip-label) !important; } .recharts-tooltip-item { color: var(--tooltip-text) !important; } .recharts-tooltip-item-name, .recharts-tooltip-item-separator { color: var(--tooltip-secondary) !important; } .recharts-tooltip-item-value, .recharts-tooltip-item-unit { color: var(--tooltip-text) !important; }Then define the variables alongside your other color declarations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/src/App.css` around lines 392 - 417, The Recharts tooltip styles in the stylesheet use hardcoded color values (`#18181b` for background, `#27272a` for border, `#e4e4e7` for label and item text, `#a1a1aa` for secondary text) instead of CSS variables, which is inconsistent with the rest of the design system. Replace these hardcoded colors in the .recharts-default-tooltip, .recharts-tooltip-label, .recharts-tooltip-item, .recharts-tooltip-item-name, .recharts-tooltip-item-separator, .recharts-tooltip-item-value, and .recharts-tooltip-item-unit selectors with corresponding CSS variable references (such as var(--tooltip-bg), var(--tooltip-border), var(--tooltip-label), var(--tooltip-text), and var(--tooltip-secondary)), then define these new variables in the CSS variables section alongside other color declarations to maintain consistency with the rest of the stylesheet.
241-249: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider using CSS variables for toggle button colors.
Lines 243 and 248 use hardcoded colors (
#1f1f23,#1e293b) while the rest of the stylesheet consistently uses CSS variables like--border,--text-primary, and--accent-primary. Using variables would improve maintainability and theme consistency.♻️ Suggested approach
.chart-toggle-group button:hover { color: var(--text-primary); - background: `#1f1f23`; + background: var(--hover-bg); } .chart-toggle-group button.active { color: var(--accent-primary); - background: `#1e293b`; + background: var(--active-bg); }Note: Ensure
--hover-bgand--active-bgare defined in your CSS variable declarations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/src/App.css` around lines 241 - 249, Replace the hardcoded background colors in the `.chart-toggle-group button:hover` and `.chart-toggle-group button.active` selectors with CSS variables to match the stylesheet's consistent use of variables. Change the background color `#1f1f23` in the hover state and `#1e293b` in the active state to use CSS variables such as `var(--hover-bg)` and `var(--active-bg)` respectively, ensuring these variables are defined in the CSS variable declarations at the top of the stylesheet.dashboard/src/components/charts/ChartCard.tsx (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePotential ID collision in aria-describedby sanitization.
The ID generation
title.replace(/\s+/g, '-').toLowerCase()could produce collisions if two chart titles differ only in punctuation or special characters (e.g., "Threat: Analysis" and "Threat Analysis" both becomethreat-analysis).Consider using a more robust ID generation strategy if multiple charts with similar titles are possible:
const chartId = useId(); // React 19 built-in hook // ... aria-describedby={chartId} // ... <p id={chartId} className="chart-sr-summary">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/src/components/charts/ChartCard.tsx` at line 17, Replace the title-based ID generation in the ChartCard component's aria-describedby attribute with React's useId hook to prevent ID collisions. Import useId from React, call it within the component to generate a unique chartId, and use that ID in both the aria-describedby attribute and the corresponding description element's id property. This ensures each chart gets a unique identifier regardless of title similarities.dashboard/src/components/charts/ThreatTypeBreakdownChart.tsx (1)
59-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider moving inline styles to CSS.
The inline
style={{ color: '#a1a1aa', fontSize: 12 }}on line 61 duplicates the color value used elsewhere and reduces maintainability.Consider defining a CSS class for legend text styling to match the existing
.chart-sr-summaryand other chart typography patterns.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/src/components/charts/ThreatTypeBreakdownChart.tsx` around lines 59 - 62, The inline style object in the Legend formatter function contains hardcoded color and fontSize values that should be extracted to a CSS class for better maintainability. Create a new CSS class following the existing pattern used in `.chart-sr-summary` that defines the legend text styling with the color '`#a1a1aa`' and fontSize 12, then replace the inline style object in the formatter function with a className prop that references this new CSS class instead.dashboard/src/components/charts/DetectionHeatmap.tsx (1)
32-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider optional chaining for the grid construction.
Line 36 uses a non-null assertion
byDay.get(c.day)!.push(c)after checking!byDay.has(c.day)on line 35. While this is safe, optional chaining would make the code more defensive:if (!byDay.has(c.day)) byDay.set(c.day, []); - byDay.get(c.day)!.push(c); + const dayArray = byDay.get(c.day); + if (dayArray) dayArray.push(c);Alternatively, use a single operation:
cells.forEach(c => { - if (!byDay.has(c.day)) byDay.set(c.day, []); - byDay.get(c.day)!.push(c); + const existing = byDay.get(c.day) ?? []; + existing.push(c); + byDay.set(c.day, existing); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/src/components/charts/DetectionHeatmap.tsx` around lines 32 - 39, The grid useMemo hook uses a non-null assertion (!) when pushing cells to the byDay Map after checking if the key exists. Replace the two-line logic in the cells.forEach callback (the if statement checking !byDay.has and the following push with non-null assertion) with a single more defensive operation using optional chaining and logical operators that either pushes to the existing array or creates a new one with the cell, eliminating the need for the non-null assertion operator.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dashboard/src/App.tsx`:
- Around line 195-197: The events handler in App.tsx is conflating
successful-but-empty responses with request failures by checking the array
length instead of just verifying the response status. Replace the guard
condition that checks `eventsRes.value.data.events?.length` with
`Array.isArray(eventsRes.value.data.events)` to ensure that empty event arrays
from successful API responses properly call `setEvents()` and replace the stale
`MOCK_EVENTS` data. This aligns the events handling pattern with the statsRes
handler which only checks the fulfilled status without additionally validating
array contents.
In `@dashboard/src/components/charts/ChartCard.tsx`:
- Around line 13-18: In the ChartCard component's section element, remove the
role="img" attribute that is incorrectly hiding interactive chart children from
screen readers. The section element starting with className="chart-container"
should retain the aria-label and aria-describedby attributes which already
provide proper semantic labeling without preventing access to interactive chart
elements like clickable segments and controls. Simply delete the role="img" line
while keeping all other accessibility attributes intact.
In `@dashboard/src/components/charts/DetectionHeatmap.tsx`:
- Around line 49-90: The DetectionHeatmap component has an ARIA structure
violation where the heatmap-wrapper uses role="grid" and contains buttons with
className "heatmap-cell" directly, but buttons should be nested inside elements
with role="gridcell" according to WCAG guidelines. Fix this by either removing
the role="grid" from the wrapper and role="row" attributes to rely on native
button semantics, or wrap each button element in a div with role="gridcell"
inside the heatmap-cells container. Choose whichever approach aligns with your
accessibility requirements, but the gridcell wrapper pattern is preferred if you
need semantic grid structure.
In `@dashboard/src/utils/chartAggregations.ts`:
- Around line 25-35: In the `inferThreatType` function, the verdict check for
'benign' is currently executed after all the prompt pattern heuristics, allowing
pattern matching to override the authoritative verdict classification. Move the
line checking `if (event.verdict === 'benign') return 'benign';` to execute
immediately after the `event.threat_type` check and before the prompt pattern
tests. This ensures the authoritative verdict classification takes precedence
over heuristic prompt inference, preventing events with a 'benign' verdict from
being misclassified as attack types.
- Around line 91-165: The aggregateDetectionsOverTime function uses the current
time (const now = new Date()) to create fixed rolling time windows, which
silently excludes events outside these windows and makes the function
non-deterministic for testing. To fix this, implement one of the suggested
approaches: either derive the time range from the min/max timestamps of the
input events array to ensure all filtered events are included, or accept
optional startDate and endDate parameters to allow explicit control over the
aggregation window, or add an optional now parameter (defaulting to new Date())
that allows tests to inject a fixed reference time. Any of these approaches will
ensure that all provided events are considered in the aggregation and make the
function behavior reproducible and testable.
- Line 116: The ensureBucket calls on lines 116, 134, and 152 use
locale-dependent formatting methods (toLocaleTimeString and toLocaleDateString)
without specifying an explicit locale, causing inconsistent time and date label
formatting across users in different regions. Fix this by specifying an explicit
locale string (such as 'en-US') as the first argument to each toLocaleTimeString
and toLocaleDateString call to ensure consistent formatting regardless of the
user's browser locale settings.
---
Nitpick comments:
In `@dashboard/src/App.css`:
- Around line 201-211: The .chart-sr-summary class uses the deprecated clip
property with clip: rect(0, 0, 0, 0). Replace this deprecated property with the
modern alternative clip-path: inset(50%) to maintain the screen-reader-only
pattern while using the current CSS standard. Remove the old clip property line
and add clip-path: inset(50%) in its place.
- Around line 392-417: The Recharts tooltip styles in the stylesheet use
hardcoded color values (`#18181b` for background, `#27272a` for border, `#e4e4e7` for
label and item text, `#a1a1aa` for secondary text) instead of CSS variables, which
is inconsistent with the rest of the design system. Replace these hardcoded
colors in the .recharts-default-tooltip, .recharts-tooltip-label,
.recharts-tooltip-item, .recharts-tooltip-item-name,
.recharts-tooltip-item-separator, .recharts-tooltip-item-value, and
.recharts-tooltip-item-unit selectors with corresponding CSS variable references
(such as var(--tooltip-bg), var(--tooltip-border), var(--tooltip-label),
var(--tooltip-text), and var(--tooltip-secondary)), then define these new
variables in the CSS variables section alongside other color declarations to
maintain consistency with the rest of the stylesheet.
- Around line 241-249: Replace the hardcoded background colors in the
`.chart-toggle-group button:hover` and `.chart-toggle-group button.active`
selectors with CSS variables to match the stylesheet's consistent use of
variables. Change the background color `#1f1f23` in the hover state and
`#1e293b` in the active state to use CSS variables such as `var(--hover-bg)` and
`var(--active-bg)` respectively, ensuring these variables are defined in the CSS
variable declarations at the top of the stylesheet.
In `@dashboard/src/App.tsx`:
- Around line 483-499: The withThreatType([event]) function is being called
per-event inside the filteredEvents.map() function, causing redundant threat
type resolution on every render. Refactor by computing withThreatType() once for
the entire filteredEvents array before the map function (or at the component
level using useMemo for memoization), store the result, and then access the
pre-computed resolvedThreatType values in the map function instead of calling
withThreatType for each individual event. This avoids repeating the same regex
inference across multiple renders and filtering operations.
In `@dashboard/src/components/charts/ChartCard.tsx`:
- Line 17: Replace the title-based ID generation in the ChartCard component's
aria-describedby attribute with React's useId hook to prevent ID collisions.
Import useId from React, call it within the component to generate a unique
chartId, and use that ID in both the aria-describedby attribute and the
corresponding description element's id property. This ensures each chart gets a
unique identifier regardless of title similarities.
In `@dashboard/src/components/charts/DetectionHeatmap.tsx`:
- Around line 32-39: The grid useMemo hook uses a non-null assertion (!) when
pushing cells to the byDay Map after checking if the key exists. Replace the
two-line logic in the cells.forEach callback (the if statement checking
!byDay.has and the following push with non-null assertion) with a single more
defensive operation using optional chaining and logical operators that either
pushes to the existing array or creates a new one with the cell, eliminating the
need for the non-null assertion operator.
In `@dashboard/src/components/charts/ThreatTypeBreakdownChart.tsx`:
- Around line 59-62: The inline style object in the Legend formatter function
contains hardcoded color and fontSize values that should be extracted to a CSS
class for better maintainability. Create a new CSS class following the existing
pattern used in `.chart-sr-summary` that defines the legend text styling with
the color '`#a1a1aa`' and fontSize 12, then replace the inline style object in the
formatter function with a className prop that references this new CSS class
instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ae972e5-00f1-4110-89db-188d76f24ebc
📒 Files selected for processing (14)
dashboard/src/App.cssdashboard/src/App.tsxdashboard/src/components/charts/ChartCard.tsxdashboard/src/components/charts/DetectionHeatmap.tsxdashboard/src/components/charts/DetectionsOverTimeChart.tsxdashboard/src/components/charts/SeverityDistributionChart.tsxdashboard/src/components/charts/ThreatTypeBreakdownChart.tsxdashboard/src/components/charts/TopAttackVectorsChart.tsxdashboard/src/components/charts/chartTooltipProps.tsdashboard/src/components/charts/index.tsdashboard/src/constants/charts.tsdashboard/src/data/mockEvents.tsdashboard/src/types/security.tsdashboard/src/utils/chartAggregations.ts
|
@Ingole712521 pls provide working proof when working on ui |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|

fix #126
Summary
Adds interactive data visualization components to the SOC Dashboard so analysts can spot trends, patterns, and anomalies quickly. Builds on the existing Recharts setup and live
/v1/eventsdata.threat_type(with prompt-based inference when the field is missing)Summary by cubic
Adds interactive SOC dashboard charts with click-to-filter and improved dark theme accessibility so analysts can spot trends and jump straight to filtered alerts. Addresses #126 with threat, severity, time-series, and heatmap views tied to the Alert Feed.
New Features
Bug Fixes
rechartstooltip styling for dark mode using shared tooltip props and CSS to ensure readable hover text.Written for commit 80f6a7a. Summary will update on new commits.
Summary by CodeRabbit
New Features
Style