ci(benchmark/react): ignore some benchmarks#1625
Conversation
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a new Codspeed API method zeroStats in React benchmark typings; updates React profiling patch to ignore certain profile names and change benchmark name prefix derivation; updates benchx_cli build script to cherry-pick a different commit hash. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Pull Request Overview
This PR updates the benchmark infrastructure to selectively ignore certain performance benchmarks from being recorded. The changes focus on filtering out specific benchmark profiles that may be noisy or less relevant for performance tracking.
- Updates the commit hash reference in the build script for benchmark CLI
- Implements benchmark filtering logic to ignore
commitChangesandReactLynx::diff::*benchmarks - Replaces webpack path reference with a custom prefix for cleaner benchmark naming
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/lynx/benchx_cli/scripts/build.mjs | Updates PICK_COMMIT hash reference |
| benchmark/react/src/patchProfile.ts | Adds benchmark filtering logic and path prefix cleanup |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
benchmark/react/rspeedy-env.d.ts (1)
7-12: Consider augmenting existing types instead of re-declaring CodspeedIf @lynx-js/rspeedy/client already declares Codspeed, prefer declaration merging to avoid duplicate global symbol collisions.
Example:
declare global { const Codspeed: { startBenchmark(): void; stopBenchmark(): void; setExecutedBenchmark(name: string): void; zeroStats(): void; }; } export {};packages/lynx/benchx_cli/scripts/build.mjs (1)
95-103: Build robustness: handle cherry-pick conflicts explicitlyCherry-picking an arbitrary PICK_COMMIT without commit or conflict handling can leave a dirty tree and still proceed to build. Consider failing fast on conflicts or committing the pick.
Option: add
--allow-emptycommit to materialize the pick and verify tree cleanliness.git fetch hzy ${PICK_COMMIT} -git cherry-pick -n ${PICK_COMMIT} +git cherry-pick -n ${PICK_COMMIT} +git diff --quiet || (echo "Uncommitted changes after cherry-pick" && exit 1)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
benchmark/react/rspeedy-env.d.ts(1 hunks)benchmark/react/src/patchProfile.ts(1 hunks)packages/lynx/benchx_cli/scripts/build.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
benchmark/react/src/patchProfile.ts (1)
benchmark/react/src/hook.ts (1)
hook(5-16)
🔇 Additional comments (1)
benchmark/react/rspeedy-env.d.ts (1)
11-12: Confirm runtime implements Codspeed.zeroStats()
patchProfile.ts invokes this new API; ensure the runtime bundle exports zeroStats in all target environments to avoid runtime errors.
React Example#4713 Bundle Size — 237.06KiB (0%).5bc2409(current) vs 63beddd main#4710(baseline) Bundle metrics
|
| Current #4713 |
Baseline #4710 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
158 |
158 |
|
64 |
64 |
|
45.83% |
45.83% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #4713 |
Baseline #4710 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.3KiB |
91.3KiB |
Bundle analysis report Branch hzy:p/hzy/bench_3 Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#4706 Bundle Size — 367.43KiB (0%).5bc2409(current) vs 63beddd main#4703(baseline) Bundle metrics
Bundle size by type
|
| Current #4706 |
Baseline #4703 |
|
|---|---|---|
235.43KiB |
235.43KiB |
|
100.16KiB |
100.16KiB |
|
31.84KiB |
31.84KiB |
Bundle analysis report Branch hzy:p/hzy/bench_3 Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1625 will not alter performanceComparing Summary
Benchmarks breakdown
|
Summary by CodeRabbit
Checklist