Staging #71
Staging #71
Conversation
Update benchmark
Updated dependencies to fix Next.js and React CVE vulnerabilities. The fix-react2shell-next tool automatically updated the following packages to their secure versions: - next - react-server-dom-webpack - react-server-dom-parcel - react-server-dom-turbopack All package.json files have been scanned and vulnerable versions have been patched to the correct fixed versions based on the official React advisory. Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: gkorland <753206+gkorland@users.noreply.github.com>
Fix Rust clippy lints for manual is_multiple_of and io::Error::other
…ve-vu-gjkikg Fix React Server Components CVE vulnerabilities
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Static Code Review 📊 ✅ All quality checks passed! |
📝 WalkthroughWalkthroughReplaced modulo-based periodic checks with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![typo-app[bot]](https://avatars.githubusercontent.com/in/360763?s=60&v=4){kind=small}
| </h2> | ||
| <p className="pb-1 text-gray-600 text-center font-fira"> | ||
| (LOWER IS BETTER) | ||
| (HIGHER IS BETTER) |
There was a problem hiding this comment.
logic: Misleading chart label. A reversed Y-axis indicates lower latency is better, but the label states 'HIGHER IS BETTER'.
| (HIGHER IS BETTER) | |
| (LOWER IS BETTER) |
| </h2> | ||
| <p className="text-gray-600 text-center font-fira"> | ||
| (LOWER IS BETTER) | ||
| (HIGHER IS BETTER) |
There was a problem hiding this comment.
logic: Misleading chart label. A reversed Y-axis indicates lower memory usage is better, but the label states 'HIGHER IS BETTER'.
| (HIGHER IS BETTER) | |
| (LOWER IS BETTER) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
ui/app/components/VerticalBarChart.tsx (1)
82-86: Consider more robust percentile key inference.The percentile key is inferred by checking if the label string includes "P50", "P95", or "P99". This approach is fragile and will break if the label format changes (e.g., if vendor prefix changes or label structure is modified).
🔎 Consider passing percentile information explicitly
Instead of inferring from the label string, consider:
- Adding a
percentileproperty to the dataset configuration- Passing the percentile information through the data structure
- Using a more structured label format that can be parsed reliably
This would make the code more maintainable and less prone to breaking when label formats change.
ui/app/components/dashboard.tsx (1)
346-346: Consider clarifying the "(HIGHER IS BETTER)" label to reduce ambiguity.The label "HIGHER IS BETTER" for LATENCY and MEMORY USAGE ratios is technically correct but ambiguous—it refers to a larger ratio value (bigger performance gap), not higher latency or memory being better. While the contextual text ("Superior Latency: Xx faster at P99", "lower overall costs") clarifies the actual meaning, users glancing at only the header could misinterpret it as suggesting higher latency or memory usage is desirable.
The y-axis reversal (
reverse: true) in both VerticalBarChart.tsx and MemoryBarChart.tsx visually reinforces lower values at the top, which is a reasonable design choice but doesn't eliminate the label ambiguity.Consider using a clearer label such as "(BETTER PERFORMANCE DIFFERENTIATION)" or adding explicit clarification like "(Larger gaps indicate greater relative advantages)" to reduce potential user confusion.
📜 Review details
Configuration used: defaults
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 (7)
src/main.rssrc/neo4j_client.rssrc/utils.rsui/app/components/MemoryBarChart.tsxui/app/components/VerticalBarChart.tsxui/app/components/dashboard.tsxui/package.json
⏰ 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). (3)
- GitHub Check: Rust project - Build and Test (stable)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (8)
src/utils.rs (1)
33-33: LGTM! Cleaner error construction.The change from
io::Error::new(std::io::ErrorKind::Other, ...)toio::Error::other(...)is a good refactor that uses the more concise and idiomatic API introduced in recent Rust versions. The functionality remains identical.ui/app/components/VerticalBarChart.tsx (1)
66-67: Verify the coordinated semantic inversion across chart configuration.The chart configuration has been updated with several coordinated changes:
- Data label positioning changed from end/top to start/start
- X-axis moved to "top"
- Y-axis inverted with
reverse: true- Logic now uses
minValueinstead ofmaxValueThese changes align with the dashboard label shift from "LOWER IS BETTER" to "HIGHER IS BETTER". Ensure this semantic inversion is intentional and correctly represents the underlying data interpretation.
Based on the coordinated changes across dashboard.tsx and chart components, verify that the visual representation correctly reflects the intended metric semantics.
Also applies to: 104-104, 122-122
ui/app/components/MemoryBarChart.tsx (2)
26-33: Note: Breaking change in component props.The
MemoryBarChartPropsinterface has changed frommaxValuetominValue(line 31 showsmaxValuestill in props definition, but line 39 destructures onlyminValue). This is a breaking change for any callers of this component.However, the calling code in
dashboard.tsxhas been updated to passminValue, maintaining consistency across the refactor.Note: Ensure all usages of
MemoryBarCharthave been updated to passminValueinstead ofmaxValue.Also applies to: 39-39
80-81: LGTM! Chart configuration updated consistently with semantic shift.The chart configuration changes are consistent with the broader semantic inversion:
- Data labels repositioned (anchor/align from end/top to start/start)
- Formatter uses
minValuefor ratio labeling- X-axis moved to top
- Y-axis inverted
These changes coordinate with the dashboard label updates and maintain internal consistency.
Also applies to: 89-89, 95-95, 110-110
src/main.rs (2)
201-201: LGTM! More readable periodic check.The change from
counter % 1000 == 0tocounter.is_multiple_of(1000)improves readability by using the more expressive method introduced in Rust 1.82.0. The functionality remains identical.
330-330: LGTM! Consistent refactor for periodic logging.The same improvement applied here - using
is_multiple_of(1000)for clearer intent. This maintains consistency with the change inspawn_neo4j_worker.src/neo4j_client.rs (2)
132-132: LGTM! Improved readability for progress logging.The change from
count % 10000 == 0tocount.is_multiple_of(10000)makes the periodic progress check more readable and expressive.
177-177: LGTM! Consistent use of is_multiple_of().This change aligns with the refactoring pattern applied throughout the codebase, using
is_multiple_of(1000)for clearer intent in periodic logging.
| "clsx": "^2.1.1", | ||
| "lucide-react": "^0.469.0", | ||
| "next": "15.1.3", | ||
| "next": "15.1.11", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Next.js 15.1.11 exists on npm registry
echo "=== Checking Next.js 15.1.11 existence ==="
curl -s https://registry.npmjs.org/next/15.1.11 | jq '.version, .name' 2>/dev/null || echo "Version not found or error querying registry"
# Get Next.js 15.1.11 metadata including peer dependencies
echo -e "\n=== Checking peer dependencies for compatibility ==="
curl -s https://registry.npmjs.org/next/15.1.11 | jq '.peerDependencies // .peerDependenciesMeta' 2>/dev/null || echo "Could not retrieve peer dependencies"
# Check npm audit/security database for known vulnerabilities in Next.js 15.1.x
echo -e "\n=== Checking for security advisories ==="
curl -s "https://registry.npmjs.org/-/npm/v1/security/advisories/bulk" \
-X POST \
-H "Content-Type: application/json" \
-d '{"next": ["15.1.11"]}' 2>/dev/null | jq '.' || echo "Could not retrieve security advisories"Repository: FalkorDB/benchmark
Length of output: 2575
Upgrade Next.js to patch critical security vulnerabilities.
Next.js 15.1.11 contains a critical authorization bypass vulnerability in middleware (GHSA-f82v-jwr5-mffw, CVSS 9.1) affecting versions <15.2.3, plus four additional security issues (SSRF, content injection, cache key confusion, and information exposure). Upgrade to at least 15.2.3 or later. React 19.0.0 is compatible with all patched versions.
🤖 Prompt for AI Agents
In ui/package.json around line 31, the Next.js dependency is pinned to "15.1.11"
which is vulnerable; update the version to at least "15.2.3" (or a later stable
release), then run your package manager to update node_modules and regenerate
the lockfile (npm install or yarn install), run the app's test suite and a
security audit (npm audit or yarn audit) to confirm no remaining
vulnerabilities, and commit the updated package.json and lockfile changes.
Co-authored-by: gkorland <753206+gkorland@users.noreply.github.com>
Fix clippy errors: manual_is_multiple_of and io_other_error
PR Summary by Typo
Overview
This PR refines the display and interpretation of performance metrics on the dashboard, updates UI chart components for improved visualization, and includes minor Rust code enhancements and Next.js dependency updates. The changes alter how latency and memory usage are presented to users.
Key Changes
@next/swcpackages to version15.1.11.MemoryBarChart,VerticalBarChart) to adjust datalabel positioning, axis rendering, and logic for displaying percentile labels.Work Breakdown
To turn off PR summary, please visit Notification settings.
Summary by CodeRabbit
Refactor
Style
Chores
✏️ Tip: You can customize this high-level summary in your review settings.