Staging #47
Staging #47
Conversation
add latency ratios, scaling and update charts components
added sheets & checkbox added contact us sheet and send lead to hubspot
restyling benchmark
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces several new UI components and refines existing ones. A new multi-step contact form and a hardware information card component are added. Chart components are updated with revised interfaces and data handling, while the dashboard and footer components now include enhanced state management and conditional rendering. The sidebar navigation and configuration have been overhauled, and benchmark type interfaces have been expanded. Additional dependencies have been incorporated, and data files have been updated with more generalized identifiers and new metrics. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CS as ContactUsSheet
participant API as HubSpot API
U->>CS: Open Contact Us form via footer button
CS->>U: Display multi-step form (Step 1: options, Step 2: user details)
U->>CS: Select options and enter name, email, company
U->>CS: Click Submit
CS->>CS: Validate inputs (including email regex check)
alt Valid Input
CS->>API: Send data for submission
API-->>CS: Return success response
CS->>U: Display success confirmation
note right of CS: Form resets after a brief delay
else Invalid Input / Error
CS->>U: Log error and display error message
end
sequenceDiagram
participant U as User
participant SN as NavMain
participant HS as HardwareInfo
U->>SN: Select a workload type in the sidebar
SN->>SN: Filter sidebar groups based on workload type
alt Workload type includes "concurrent"
SN-->>U: Render menu excluding "Queries"
else
SN-->>U: Render menu with "Clients", "Realistic Workload", "Hardware"
end
U->>SN: Choose an option from the sidebar
alt Option is “Hardware”
SN->>HS: Render detailed hardware info card
end
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
ui/app/types/benchmark.ts (1)
43-46:⚠️ Potential issueFix duplicate interface definition and naming.
The interface has the following issues:
- The interface is defined twice, which is redundant.
- The property name
unrealstichas a typo.Apply this diff to fix the interface definition:
-export interface BenchmarkData { - runs: Run[]; - platforms: Platforms; -} -export interface BenchmarkData { +export interface BenchmarkData { runs: Run[]; - unrealstic: UnRealsticData[]; + unrealistic: UnrealisticData[]; platforms: Platforms; }Also applies to: 54-58
🧹 Nitpick comments (12)
ui/app/components/HorizontalBarChart.tsx (2)
28-31: Consider improving type safety for the data prop.The current type
{ [key: string]: any }[]for thedataprop reduces type safety. Consider creating a specific interface for your data structure.+interface ChartDataItem { + vendor: string; + [key: string]: string | number; // for dynamic data keys +} interface HorizontalBarChartProps { - data: { [key: string]: any }[]; + data: ChartDataItem[]; dataKey: string; chartLabel: string;
89-94: Improve type safety in the datalabels formatter.The formatter function uses
anytype and could benefit from proper typing.- formatter: (_: any, context: { dataIndex: any; dataset: { data: { [x: string]: any; }; }; }) => { + formatter: (_value: unknown, context: { dataIndex: number; dataset: { data: number[] } }) => { const index = context.dataIndex; const value = context.dataset.data[index]; return value === maxValue ? ` x${ratio} ` : ""; },ui/app/components/VerticalBarChart.tsx (2)
32-42: Refine the type ofchartData
Currently declared asany; consider introducing a more precise type to aid maintainability and prevent potential runtime errors.interface VerticalBarChartProps { chartId: string; - chartData: any; + chartData: { + labels: string[]; + datasets: { label: string; data: number[]; backgroundColor: string }[]; + }; unit: string; latencyStats: { p50: LatencyStats; p95: LatencyStats; p99: LatencyStats; }; }
64-93: Consider naming the threshold for max-value checks
UsingMath.abs(value - maxValue) < 0.5introduces a "magic number." Defining a named constant or clarifying the threshold usage can improve readability.ui/app/components/dashboard.tsx (2)
138-172: ComputeStats with edge cases
computeStatsworks well, but consider an empty array scenario to avoid Infinity or NaN when computing min/max.
182-226: chartDataForUnrealistic
Sorting p99 values and computing the ratio is logical. For single-vendor or empty sets, ensure the code gracefully bypasses ratio calculation.ui/app/components/footer.tsx (1)
46-54: Fix the layout of the contact button container.The div containing the "SPEAK WITH US" button has
w-fullwhich might affect the layout. Consider adjusting the width to maintain proper spacing.Apply this diff to fix the layout:
- <div className="flex items-center h-16 w-full bg-muted/50 p-4"> + <div className="flex items-center h-16 bg-muted/50 p-4">ui/components/ui/SidebarNavigation.tsx (2)
46-54: Consider memoizing filtered items.The
filteredItemsarray is recalculated on every render. Consider usinguseMemoto optimize performance.- const filteredItems = items.filter((group) => { + const filteredItems = useMemo(() => items.filter((group) => { if (group.title === "Queries" && isRealisticWorkloadOn) return false; if ( (group.title === "Clients" || group.title === "Realistic Workload" || group.title === "Hardware") && !isRealisticWorkloadOn ) return false; return true; - }); + }, [items, isRealisticWorkloadOn]);
131-141: Extract button style logic to a separate function.The
getButtonClassesfunction should be moved outside the map callback to prevent recreation on each iteration.+const getButtonClasses = (isSelected: boolean, optionId: string) => { + if (isSelected) { + if (optionId === "falkordb") + return "bg-[#F5F4FF] text-FalkorDB border-FalkorDB"; + if (optionId === "neo4j") + return "bg-[#F5F4FF] text-Neo4j border-Neo4j"; + return "bg-[#F5F4FF] text-[#7466FF] border-[#7466FF]"; + } + return "bg-gray-100 text-gray-800 border-transparent"; +}; {group.options.map((option, index) => { const isSelected = selectedOptions[group.title]?.includes( option.id ); - const getButtonClasses = () => { - if (isSelected) { - if (option.id === "falkordb") - return "bg-[#F5F4FF] text-FalkorDB border-FalkorDB"; - if (option.id === "neo4j") - return "bg-[#F5F4FF] text-Neo4j border-Neo4j"; - return "bg-[#F5F4FF] text-[#7466FF] border-[#7466FF]"; - } - return "bg-gray-100 text-gray-800 border-transparent"; - };ui/app/components/ContactUsSheet.tsx (1)
15-18: Consider using a more robust email validation.The current email regex might not catch all edge cases. Consider using a library like
validatorfor more thorough validation.+import isEmail from 'validator/lib/isEmail'; -const isValidEmail = (email: string) => { - const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/; - return emailRegex.test(email); -}; +const isValidEmail = (email: string) => isEmail(email);ui/public/resultData.json (2)
892-1221: Consider adding schema validation for the data structure.The complex data structure would benefit from JSON schema validation to ensure data integrity.
Create a new file
schemas/resultData.schema.json:{ "$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "required": ["runs", "unrealstic", "platforms"], "properties": { "unrealstic": { "type": "array", "items": { "type": "object", "required": ["vendor", "node_count", "relation_count", "query_count", "histogram_for_type"], "properties": { "vendor": { "type": "string" }, "node_count": { "type": "number" }, "relation_count": { "type": "number" }, "query_count": { "type": "number" }, "histogram_for_type": { "type": "object", "additionalProperties": { "type": "array", "items": { "type": "number" } } } } } } } }
1222-1235: Consider adding metadata about the benchmark environment.The platforms section could include more details about the testing environment for better reproducibility.
"platforms": [ { "id": "Macbook-Air-2024", "cpu": "M3 (ARM, 8 cores)", "ram": "24GB", - "storage": "512gb SSD" + "storage": "512gb SSD", + "os": "macOS 14.2", + "kernel": "23.2.0", + "date_tested": "2024-02-15" }, { "id": "intel-Lenovo-ultra7", "cpu": "Intel Ultra 7 155H 22 CPU 16 cores", "ram": "30GB", - "storage": "MVMe 930GB" + "storage": "MVMe 930GB", + "os": "Ubuntu 22.04 LTS", + "kernel": "5.15.0", + "date_tested": "2024-02-15" } ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
ui/app/components/ContactUsSheet.tsx(1 hunks)ui/app/components/HardwareInfo.tsx(1 hunks)ui/app/components/HorizontalBarChart.tsx(2 hunks)ui/app/components/VerticalBarChart.tsx(1 hunks)ui/app/components/dashboard.tsx(6 hunks)ui/app/components/footer.tsx(2 hunks)ui/app/data/sideBarData.ts(1 hunks)ui/app/types/benchmark.ts(2 hunks)ui/components/ui/SidebarNavigation.tsx(1 hunks)ui/components/ui/app-sidebar.tsx(3 hunks)ui/components/ui/checkbox.tsx(1 hunks)ui/components/ui/nav-main.tsx(0 hunks)ui/components/ui/sidebar.tsx(1 hunks)ui/package.json(2 hunks)ui/public/resultData.json(41 hunks)
💤 Files with no reviewable changes (1)
- ui/components/ui/nav-main.tsx
✅ Files skipped from review due to trivial changes (1)
- ui/components/ui/sidebar.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Rust project - Build and Test (stable)
- GitHub Check: test
🔇 Additional comments (35)
ui/package.json (2)
12-13: Dependency Addition: UI EnhancementsThe addition of
@heroicons/reactand@radix-ui/react-checkboxsupports the new UI components (e.g., the multi‐step ContactUsSheet). Ensure that these packages are correctly integrated in your components and that their versions meet any required peer dependency constraints.
27-27: Dependency Addition: Chart.js PluginThe inclusion of
chartjs-plugin-datalabelswith version^2.2.0is a good move to enhance chart visualizations. Please verify that its version is fully compatible with your existingchart.jsdependency (^4.4.7) to prevent any runtime issues.ui/app/components/HorizontalBarChart.tsx (3)
39-47: LGTM!The props destructuring is clean and matches the interface definition.
83-87: LGTM!The styling is consistent across the chart with standardized fonts and colors.
Also applies to: 119-124
110-110:⚠️ Potential issuePrevent potential division by zero in stepSize calculation.
The stepSize calculation
minValue / 0.5could throw an error if minValue is 0.- stepSize: dataKey === "memory" ? maxValue / 5 : minValue / 0.5, + stepSize: dataKey === "memory" ? maxValue / 5 : Math.max(minValue / 0.5, 0.1),Likely an incorrect or invalid review comment.
ui/app/components/VerticalBarChart.tsx (8)
14-14: Good use of Chart.js plugin
The import ofChartDataLabelsis appropriate for displaying data labels.
22-23: Plugin registration confirmed
RegisteringChartDataLabelsalongsideTitleis consistent with Chart.js usage.
26-30: LatencyStats interface is clear
The interface is straightforward, encapsulating essential metrics for min, max, and ratio.
45-49: Refactored props look good
Switching to a clearly defined prop structure (chartId, chartData, unit, latencyStats) is consistent.
54-54: Clear legend configuration
Displaying the legend on top is a valid and straightforward choice.
60-60: Adaptive tooltip unit
Usingunitdynamically helps generalize the chart for different measurements.
100-110: x-axis label retrieval
Using the callback to map indices tochartData.labelsis clear and consistent.
118-125: Consistent unit application in y-axis
Appendinguniton the y-axis ensures uniform labeling of chart values.ui/app/components/dashboard.tsx (16)
6-6: Use of React hooks
ImportinguseCallback,useEffect, anduseMemodemonstrates good performance optimization practices.
15-16: Introduction of gridKey and p99SingleRatio
Creating separate states to track grid rerenders and P99 ratio for single runs is clean and understandable.
19-23: Latent stats initialization
Storing defaults forp50,p95, andp99is correct. They align well with theLatencyStatsinterface inVerticalBarChart.tsx.
24-26: Separate unrealstic data handling
UsingfilteredUnrealisticto track distinct sets of data for unrealistic workloads is a solid approach.
30-36: Verify default selected options
Check if the keys ("Workload Type",Clients,Throughput, etc.) are indeed valid side bar items and confirm that["1"]for"Realistic Workload"is recognized downstream.
39-52: Async fetch with useCallback
Fetching data is neatly handled, and errors are reported via toast. Confirm partial or malformed data cases are handled gracefully.
54-58: Proper effect usage
CallingfetchDatainsideuseEffectis correct, ensuring data is fetched once.
81-97: Filtering unrealistic data
Logic for extracting histograms keyed byQueriesis sound. Validate the shape ofdata.unrealsticin all scenarios.
121-127: Ensure client & throughput fields are consistently typed
Verifying thatrun.clientsandrun["target-messages-per-second"]are always numeric or string-convertible helps avoid type errors.
174-180: Dynamic vendor color retrieval
Leveraging CSS custom properties is an elegant approach for consistent theming.
228-257: chartDataForRealistic
Combining vendor data into stacked bars is clean and well-structured.
259-272: Throughput ratio in empty scenarios
Math.max(...[])orMath.min(...[])can produce -Infinity or Infinity for empty arrays, causing potential errors. Add a safeguard.
273-296: Memory usage ratio
Similar to throughput, ensure the memory data arrays are non-empty before computing min/max or ratio.
297-299: Trigger re-render with gridKey
IncrementinggridKeyon Workload Type change resets the layout effectively. No issues found.
327-327: Updated selection handler
RenaminghandleSideBarSelectionis consistent with the new sidebar naming convention.
331-443: Conditional layout
Switching chart rows based on concurrency or single is cleanly implemented. Ensure ratio labels remain consistent in each mode.ui/app/components/HardwareInfo.tsx (2)
1-1: Proper import for hover card
The HoverCard components are correctly imported from your UI library.
3-26: Simple and intuitive hardware info component
The structured display of CPU, RAM, and storage in a HoverCard is clear, maintainable, and user-friendly.ui/components/ui/checkbox.tsx (1)
9-28: LGTM! Well-structured and accessible checkbox component.The implementation follows React best practices, properly handles refs, and adheres to WAI-ARIA guidelines. The use of Radix UI ensures accessibility and consistent behavior across browsers.
ui/components/ui/app-sidebar.tsx (1)
4-4: LGTM! Improved naming and consistency.The changes enhance code clarity through:
- More consistent import path naming
- More descriptive function name that better indicates its purpose
Also applies to: 25-25, 30-30, 47-47
ui/app/data/sideBarData.ts (2)
9-17: LGTM! Well-structured type definitions.The type definitions are clear and properly constrained, making the configuration type-safe.
18-87: LGTM! Clear and organized sidebar configuration.The sidebar configuration is well-structured with clear grouping of related items and consistent formatting.
minor refactor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
ui/app/components/ContactUsSheet.tsx (3)
15-18: Consider enhancing email validation.While the current regex is functional, consider:
- Adding more restrictive validation for enterprise emails
- Adding unit tests for the validation function
const isValidEmail = (email: string) => { - const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/; + // More restrictive regex that prevents common invalid patterns + const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z]{2,})+$/; return emailRegex.test(email); };
20-31: Add loading state for better UX.The state management is clean, but adding a loading state would improve user experience during form submission.
export default function ContactUsSheet({ isOpen, setIsOpen }: ContactUsSheetProps) { const [step, setStep] = useState(1); const [selectedOptions, setSelectedOptions] = useState<string[]>([]); const [formData, setFormData] = useState({ name: "", email: "", company: "", message: "" }); const [submitted, setSubmitted] = useState(false); + const [isSubmitting, setIsSubmitting] = useState(false); const resetForm = () => { setStep(1); setSelectedOptions([]); setFormData({ name: "", email: "", company: "", message: "" }); setSubmitted(false); + setIsSubmitting(false); };
137-139: Remove duplicated privacy notice.The privacy notice is duplicated in both steps. Consider extracting it to a shared component.
+const PrivacyNotice = () => ( + <p className="mt-4 text-xs text-gray-500 text-center"> + I agree that my submitted data is being collected and stored. <strong>We don't resell your data.</strong> + </p> +); // Then use <PrivacyNotice /> in both stepsAlso applies to: 171-173
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ui/app/components/ContactUsSheet.tsx(1 hunks)ui/app/types/benchmark.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ui/app/types/benchmark.ts
🔇 Additional comments (2)
ui/app/components/ContactUsSheet.tsx (2)
1-13: LGTM! Clean imports and well-defined interface.The imports are properly organized and the interface clearly defines the component's props.
39-90: Enhance error handling and user feedback.The form submission error handling could be improved with better user feedback and retry logic.
PR Type
Enhancement, Tests, Bug fix
Description
Introduced a new
ContactUsSheetcomponent for user interaction and HubSpot integration.Enhanced chart components with scaling, data labels, and improved styling.
Updated sidebar navigation with new options and dynamic filtering.
Refactored dashboard logic for better performance and added realistic/unrealistic workload filtering.
Fixed linting issues and improved code readability across components.
Changes walkthrough 📝
12 files
Added a new Contact Us Sheet component with HubSpot integration.Added a hover card for displaying hardware information.Enhanced horizontal bar chart with scaling and data labels.Enhanced vertical bar chart with latency stats and data labels.Refactored dashboard logic and added workload filtering.Updated footer with a "Speak With Us" button.Updated sidebar data with new options and icons.Added types for unrealistic data and updated benchmark types.Added dynamic sidebar navigation with filtering.Updated app sidebar to use new navigation component.Added a reusable checkbox component.Adjusted sidebar width for better usability.1 files
Removed deprecated navigation component.2 files
Updated dependencies for new features and plugins.Added new dependencies for charts and UI components.1 files
Updated result data with new platforms and workloads.Summary by CodeRabbit
New Features
Chores