Add cuVS performance dashboard docs#2126
Open
ManasSingh12345 wants to merge 2 commits into
Open
Conversation
aamijar
added
non-breaking
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a performance dashboard to the docs: benchmark data export and JSON/TS assets, filtering and chart-data logic, a React dashboard with Chart.js charts and table, CSS styling, and docs integration with build automation and a performance docs page. ChangesPerformance Dashboard
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@fern/styles/performance-dashboard.css`:
- Around line 1-9: The stylelint error is caused by missing an empty line
between the custom property declarations and the normal declarations in the
.performance-dashboard rule; update the .performance-dashboard block (the lines
defining --pd-nv-green, --pd-intel-blue, --pd-border, --pd-muted) to add a
single blank line before the first regular declaration (margin) so the custom
property section is separated from the rest to satisfy
declaration-empty-line-before.
In `@fern/theme/nvidia/components/PerformanceDashboard.tsx`:
- Around line 90-112: chartLoaderPromise is permanently set to a rejected
promise on script load failure, preventing retries; modify the error handlers
for both script.onerror and plugin.onerror inside the chartLoaderPromise
initializer to clear/reset chartLoaderPromise (e.g., set chartLoaderPromise =
undefined or null) before calling reject(new Error(...)), so subsequent attempts
can recreate the loader and retry loading CHART_JS_URL and CHART_DATALABELS_URL;
update both error branches associated with script and plugin (the script element
named script and the plugin element named plugin that handle window.Chart and
window.ChartDataLabels registration) to perform this reset.
- Around line 35-38: The component currently injects Chart.js and
chartjs-plugin-datalabels at runtime using CHART_JS_URL and CHART_DATALABELS_URL
and caches the result in chartLoaderPromise, which can be permanently rejected
and also breaks CSP/offline environments; replace runtime CDN loading by
bundling/importing Chart.js and the datalabels plugin via package dependencies
(or serve local static assets) and import/register them in the module (remove
script injection logic such as the chartLoaderPromise loader function), or if
you must keep remote loading, add SRI integrity and crossOrigin attributes and
modify the loader (chartLoaderPromise and its loader function) to reset/recreate
the promise on error so retries are possible and surface errors to the UI;
update registration code that currently expects global Chart to instead use the
imported Chart and plugin symbols.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b3e88a24-9793-4583-bcba-0007fcb315bf
⛔ Files ignored due to path filters (1)
data/benchmarks/results_cuvs_26_04.csvis excluded by!**/*.csv
📒 Files selected for processing (9)
fern/assets/data/benchmark_results.jsonfern/build_docs.shfern/docs.ymlfern/pages/performance.mdfern/scripts/export_performance_dashboard_data.pyfern/styles/performance-dashboard.cssfern/theme/nvidia/components/PerformanceDashboard.tsxfern/theme/nvidia/components/benchmarkData.tsfern/theme/nvidia/components/performanceDashboardLogic.ts
Comment on lines
+1
to
+9
| .performance-dashboard { | ||
| --pd-nv-green: #76b900; | ||
| --pd-intel-blue: #0071c5; | ||
| --pd-border: #e0e0e0; | ||
| --pd-muted: #666; | ||
| margin: 0 0 24px; | ||
| font-size: 13px; | ||
| color: #222; | ||
| } |
There was a problem hiding this comment.
Fix stylelint declaration-empty-line-before at Line 6.
Insert an empty line between the custom property block and regular declarations to satisfy the configured stylelint rule.
Suggested fix
.performance-dashboard {
--pd-nv-green: `#76b900`;
--pd-intel-blue: `#0071c5`;
--pd-border: `#e0e0e0`;
--pd-muted: `#666`;
+
margin: 0 0 24px;
font-size: 13px;
color: `#222`;
}📝 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.
Suggested change
| .performance-dashboard { | |
| --pd-nv-green: #76b900; | |
| --pd-intel-blue: #0071c5; | |
| --pd-border: #e0e0e0; | |
| --pd-muted: #666; | |
| margin: 0 0 24px; | |
| font-size: 13px; | |
| color: #222; | |
| } | |
| .performance-dashboard { | |
| --pd-nv-green: `#76b900`; | |
| --pd-intel-blue: `#0071c5`; | |
| --pd-border: `#e0e0e0`; | |
| --pd-muted: `#666`; | |
| margin: 0 0 24px; | |
| font-size: 13px; | |
| color: `#222`; | |
| } |
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 6-6: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 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 `@fern/styles/performance-dashboard.css` around lines 1 - 9, The stylelint
error is caused by missing an empty line between the custom property
declarations and the normal declarations in the .performance-dashboard rule;
update the .performance-dashboard block (the lines defining --pd-nv-green,
--pd-intel-blue, --pd-border, --pd-muted) to add a single blank line before the
first regular declaration (margin) so the custom property section is separated
from the rest to satisfy declaration-empty-line-before.
Comment on lines
+35
to
+38
| const CHART_JS_URL = | ||
| "https://cdn.jsdelivr.net/npm/chart.js@4.4.1/dist/chart.umd.min.js"; | ||
| const CHART_DATALABELS_URL = | ||
| "https://cdn.jsdelivr.net/npm/chartjs-plugin-datalabels@2.2.0/dist/chartjs-plugin-datalabels.min.js"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="fern/theme/nvidia/components/PerformanceDashboard.tsx"
# Show relevant portions around the referenced line ranges
nl -ba "$FILE" | sed -n '1,220p'
# Search for script injection / dynamic loading / caching / retry logic within the file
rg -n "CHART_JS_URL|CHART_DATALABELS_URL|script|appendChild|createElement|integrity|crossorigin|retry|Promise|loader|loadFailures|catch|then|onload" "$FILE" || true
# If there are other occurrences of these URLs elsewhere, list them
rg -n "cdn\.jsdelivr\.net/npm/chart\.js|chartjs-plugin-datalabels" -S . || trueRepository: rapidsai/cuvs
Length of output: 100
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="fern/theme/nvidia/components/PerformanceDashboard.tsx"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
# Print file with line numbers using awk (no nl dependency)
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE" | sed -n '1,240p'
# Focused searches for CDN/runtime loader/caching/retry/integrity behavior
rg -n "CHART_JS_URL|CHART_DATALABELS_URL|jsdelivr|chart\.umd\.min|datalabels\.min|chartjs-plugin-datalabels|script|appendChild|createElement|integrity|crossorigin|retry|Promise|loader|load failures|catch|throw|onload|onerror" "$FILE" || true
# Check for any other jsdelivr/chart references across repo
rg -n "cdn\.jsdelivr\.net/npm/chart\.js|chartjs-plugin-datalabels" -S . || trueRepository: rapidsai/cuvs
Length of output: 9850
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="fern/theme/nvidia/components/PerformanceDashboard.tsx"
# Print a bit around where loadChartJs() is used (based on earlier rg hit at line ~417)
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE" | sed -n '360,470p'
# Find all call sites of loadChartJs
rg -n "loadChartJs\\(" "$FILE" || true
# Show the exact block around the matching call(s)
# (use awk to print some window around each line number)
python3 - <<'PY'
import re, subprocess, os, sys
file_path="fern/theme/nvidia/components/PerformanceDashboard.tsx"
out=subprocess.check_output(["rg","-n","loadChartJs\\(","-S",file_path], text=True).strip().splitlines()
if not out or out==['']:
print("No matches")
sys.exit(0)
lines=[]
for line in out:
m=re.match(r":(\d+):", line)
if m:
lines.append(int(m.group(1)))
for ln in sorted(set(lines)):
start=max(1, ln-40)
end=ln+80
# print with awk
cmd=f"awk '{{printf \"%6d\\t%s\\n\", NR, $0}}' {shlex_quote(file_path)} | sed -n '{start},{end}p'"
PYRepository: rapidsai/cuvs
Length of output: 4348
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="fern/theme/nvidia/components/PerformanceDashboard.tsx"
# Show the ChartPanel and anything using chartsReady/chart instance creation
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE" | sed -n '220,560p'
# Search for chartsReady usage and for chartLoaderPromise mutations
rg -n "chartsReady" "$FILE" || true
rg -n "chartLoaderPromise" "$FILE" || true
# Look for any retry/reset logic on failure
rg -n "setChartsReady\\(|setError\\(|retry|reset.*chart|chartLoaderPromise\\s*=\\s*null" "$FILE" || trueRepository: rapidsai/cuvs
Length of output: 10162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="fern/theme/nvidia/components/PerformanceDashboard.tsx"
rg -n "integrity|crossorigin|crossOrigin" "$FILE" || true
rg -n "chartLoaderPromise\\s*=\\s*null|chartLoaderPromise\\s*=" "$FILE"Repository: rapidsai/cuvs
Length of output: 118
Avoid runtime jsDelivr CDN loading for Chart.js + datalabels
PerformanceDashboard.tsx loads chart.js and chartjs-plugin-datalabels at runtime from https://cdn.jsdelivr.net/... via injected <script> tags (no integrity/crossOrigin), which can fail under strict CSP/offline/internal-docs environments and provides no integrity pinning (lines ~35-38, ~85-115).
The loader uses a module-level cached chartLoaderPromise; if either script load fails, the rejected promise is never reset, leaving the dashboard stuck in an error state with no retry (lines ~83-115, call site ~413-418).
🤖 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 `@fern/theme/nvidia/components/PerformanceDashboard.tsx` around lines 35 - 38,
The component currently injects Chart.js and chartjs-plugin-datalabels at
runtime using CHART_JS_URL and CHART_DATALABELS_URL and caches the result in
chartLoaderPromise, which can be permanently rejected and also breaks
CSP/offline environments; replace runtime CDN loading by bundling/importing
Chart.js and the datalabels plugin via package dependencies (or serve local
static assets) and import/register them in the module (remove script injection
logic such as the chartLoaderPromise loader function), or if you must keep
remote loading, add SRI integrity and crossOrigin attributes and modify the
loader (chartLoaderPromise and its loader function) to reset/recreate the
promise on error so retries are possible and surface errors to the UI; update
registration code that currently expects global Chart to instead use the
imported Chart and plugin symbols.
Comment on lines
+90
to
+112
| chartLoaderPromise = new Promise((resolve, reject) => { | ||
| const script = document.createElement("script"); | ||
| script.src = CHART_JS_URL; | ||
| script.async = true; | ||
| script.onload = () => { | ||
| const plugin = document.createElement("script"); | ||
| plugin.src = CHART_DATALABELS_URL; | ||
| plugin.async = true; | ||
| plugin.onload = () => { | ||
| if (window.Chart && window.ChartDataLabels) { | ||
| const pluginModule = | ||
| (window.ChartDataLabels as { default?: unknown }).default ?? | ||
| window.ChartDataLabels; | ||
| window.Chart.register(pluginModule); | ||
| } | ||
| resolve(); | ||
| }; | ||
| plugin.onerror = () => reject(new Error("Failed to load Chart.js datalabels plugin")); | ||
| document.head.appendChild(plugin); | ||
| }; | ||
| script.onerror = () => reject(new Error("Failed to load Chart.js")); | ||
| document.head.appendChild(script); | ||
| }); |
There was a problem hiding this comment.
Reset chartLoaderPromise when script loading fails.
A transient load error permanently poisons the cached promise, so subsequent retries in the same session will always fail. Clear the cache in the error path so retry is possible.
Suggested fix
chartLoaderPromise = new Promise((resolve, reject) => {
+ const fail = (message: string) => {
+ chartLoaderPromise = null;
+ reject(new Error(message));
+ };
+
const script = document.createElement("script");
script.src = CHART_JS_URL;
script.async = true;
script.onload = () => {
const plugin = document.createElement("script");
plugin.src = CHART_DATALABELS_URL;
plugin.async = true;
@@
}
resolve();
};
- plugin.onerror = () => reject(new Error("Failed to load Chart.js datalabels plugin"));
+ plugin.onerror = () => fail("Failed to load Chart.js datalabels plugin");
document.head.appendChild(plugin);
};
- script.onerror = () => reject(new Error("Failed to load Chart.js"));
+ script.onerror = () => fail("Failed to load Chart.js");
document.head.appendChild(script);
});📝 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.
Suggested change
| chartLoaderPromise = new Promise((resolve, reject) => { | |
| const script = document.createElement("script"); | |
| script.src = CHART_JS_URL; | |
| script.async = true; | |
| script.onload = () => { | |
| const plugin = document.createElement("script"); | |
| plugin.src = CHART_DATALABELS_URL; | |
| plugin.async = true; | |
| plugin.onload = () => { | |
| if (window.Chart && window.ChartDataLabels) { | |
| const pluginModule = | |
| (window.ChartDataLabels as { default?: unknown }).default ?? | |
| window.ChartDataLabels; | |
| window.Chart.register(pluginModule); | |
| } | |
| resolve(); | |
| }; | |
| plugin.onerror = () => reject(new Error("Failed to load Chart.js datalabels plugin")); | |
| document.head.appendChild(plugin); | |
| }; | |
| script.onerror = () => reject(new Error("Failed to load Chart.js")); | |
| document.head.appendChild(script); | |
| }); | |
| chartLoaderPromise = new Promise((resolve, reject) => { | |
| const fail = (message: string) => { | |
| chartLoaderPromise = null; | |
| reject(new Error(message)); | |
| }; | |
| const script = document.createElement("script"); | |
| script.src = CHART_JS_URL; | |
| script.async = true; | |
| script.onload = () => { | |
| const plugin = document.createElement("script"); | |
| plugin.src = CHART_DATALABELS_URL; | |
| plugin.async = true; | |
| plugin.onload = () => { | |
| if (window.Chart && window.ChartDataLabels) { | |
| const pluginModule = | |
| (window.ChartDataLabels as { default?: unknown }).default ?? | |
| window.ChartDataLabels; | |
| window.Chart.register(pluginModule); | |
| } | |
| resolve(); | |
| }; | |
| plugin.onerror = () => fail("Failed to load Chart.js datalabels plugin"); | |
| document.head.appendChild(plugin); | |
| }; | |
| script.onerror = () => fail("Failed to load Chart.js"); | |
| document.head.appendChild(script); | |
| }); |
🤖 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 `@fern/theme/nvidia/components/PerformanceDashboard.tsx` around lines 90 - 112,
chartLoaderPromise is permanently set to a rejected promise on script load
failure, preventing retries; modify the error handlers for both script.onerror
and plugin.onerror inside the chartLoaderPromise initializer to clear/reset
chartLoaderPromise (e.g., set chartLoaderPromise = undefined or null) before
calling reject(new Error(...)), so subsequent attempts can recreate the loader
and retry loading CHART_JS_URL and CHART_DATALABELS_URL; update both error
branches associated with script and plugin (the script element named script and
the plugin element named plugin that handle window.Chart and
window.ChartDataLabels registration) to perform this reset.
Member
|
/ok to test 1ed98f7 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
How to test
From a fresh checkout of this branch:
git fetch origin docs/performance-dashboard git checkout docs/performance-dashboard node --version # should be 22+ fern/build_docs.sh checkTo preview the page locally:
Then open:
Verify that:
Local validation performed
fern/build_docs.sh check/cuvs/getting-started/performancewithout the Fern error badge.