[codex] add performance benchmarks#2076
Merged
Merged
Conversation
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (9)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds benchmark configuration and suites, updates Capacitor CLI typings to async, configures TypeScript/Vitest for bench runs, adds a bench script and CI step change, and removes device-comparison logging. ChangesBenchmark Infrastructure and Suite Addition
Sequence Diagram(s)sequenceDiagram
participant CodSpeed
participant GitHubWorkflow
participant Bun
participant Vitest
CodSpeed->>GitHubWorkflow: run simulation step
GitHubWorkflow->>Bun: bun run bench
Bun->>Vitest: vitest bench --config vitest.config.bench.ts --run
Vitest->>Vitest: execute benches/**/*.bench.ts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Merging this PR will improve performance by ×53
Performance ChangesComparing 
Footnotes
|
riderx
force-pushed
the
codex/perf-test-coverage
branch
from
cb6c369 to
4035a99
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
benches/plugin-hot-paths.bench.ts (1)
19-60: ⚡ Quick winUse non-numeric UUID fixtures to prevent security-scan false positives.
Several
device_idfixtures (for example at Line 21) are numeric-heavy UUIDs and can be flagged as payment-card-like tokens by scanners. Swapping them to valid hex UUIDs with letters keeps schema realism and reduces noisy alerts.Proposed fixture-only update
- device_id: '11111111-1111-4111-8111-111111111111', + device_id: 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa', - device_id: '22222222-2222-4222-8222-222222222222', + device_id: 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb', - device_id: '33333333-3333-4333-8333-333333333333', + device_id: 'cccccccc-cccc-4ccc-8ccc-cccccccccccc', - device_id: '44444444-4444-4444-8444-444444444444', + device_id: 'dddddddd-dddd-4ddd-8ddd-dddddddddddd',🤖 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 `@benches/plugin-hot-paths.bench.ts` around lines 19 - 60, Several device_id fixtures (in updatePayload, statsPayload, channelSelfPayload, and channelSelfQuery) use numeric-heavy UUIDs that can trigger security scanners; replace those UUID strings with valid hex-containing UUIDs (include letters a–f) to keep realism and avoid false positives—update the device_id values in the objects named updatePayload, statsPayload, channelSelfPayload, and the device_id in channelSelfQuery accordingly while leaving all other fields unchanged.
🤖 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.
Nitpick comments:
In `@benches/plugin-hot-paths.bench.ts`:
- Around line 19-60: Several device_id fixtures (in updatePayload, statsPayload,
channelSelfPayload, and channelSelfQuery) use numeric-heavy UUIDs that can
trigger security scanners; replace those UUID strings with valid hex-containing
UUIDs (include letters a–f) to keep realism and avoid false positives—update the
device_id values in the objects named updatePayload, statsPayload,
channelSelfPayload, and the device_id in channelSelfQuery accordingly while
leaving all other fields unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7db86d64-7da2-4e50-9957-dd40b43c0127
📒 Files selected for processing (12)
.github/workflows/codspeed.ymlbenches/cli-hot-paths.bench.tsbenches/cloudflare-utils.bench.tsbenches/device-comparison.bench.tsbenches/password-policy.bench.tsbenches/plugin-hot-paths.bench.tscli/src/types/capacitor__cli.d.tscli/tsconfig.jsonpackage.jsonsupabase/functions/_backend/utils/deviceComparison.tstsconfig.jsonvitest.config.bench.ts
riderx
force-pushed
the
codex/perf-test-coverage
branch
from
4035a99 to
cd73dbb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@benches/cli-hot-paths.bench.ts`:
- Around line 62-66: The fixture assertion in the bench 'validate bundle semver
candidates' only ensures each bundleVersions entry matches regexSemver or is
'invalid-version' but doesn't ensure the 'invalid-version' fixture itself does
not accidentally match; update the loop that iterates bundleVersions to also
assert that when version === 'invalid-version' it fails regexSemver (i.e., throw
an error if regexSemver.test(version) is true for the 'invalid-version'
fixture), referencing the bench name, the bundleVersions array, and the
regexSemver variable so both positive and negative expectations are protected.
🪄 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
Run ID: eabea51f-ee1a-4089-b3fa-27c47ce53755
📒 Files selected for processing (12)
.github/workflows/codspeed.ymlbenches/cli-hot-paths.bench.tsbenches/cloudflare-utils.bench.tsbenches/device-comparison.bench.tsbenches/password-policy.bench.tsbenches/plugin-hot-paths.bench.tscli/src/types/capacitor__cli.d.tscli/tsconfig.jsonpackage.jsonsupabase/functions/_backend/utils/deviceComparison.tstsconfig.jsonvitest.config.bench.ts
✅ Files skipped from review due to trivial changes (8)
- benches/password-policy.bench.ts
- tsconfig.json
- benches/device-comparison.bench.ts
- benches/cloudflare-utils.bench.ts
- vitest.config.bench.ts
- .github/workflows/codspeed.yml
- cli/src/types/capacitor__cli.d.ts
- cli/tsconfig.json
riderx
force-pushed
the
codex/perf-test-coverage
branch
from
cd73dbb to
e33e3fb
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
benches/cli-hot-paths.bench.ts (1)
90-95: ⚡ Quick winSplit compatibility benchmarks to improve regression attribution.
getCompatibilityDetailsandisCompatibleare timed together, which makes it harder to identify which helper regressed when this bench moves.Proposed patch
describe('cli compatibility helpers', () => { - bench('classify native package compatibility', () => { + bench('classify native package compatibility details', () => { for (const entry of compatibilityMatrix) { getCompatibilityDetails(entry) + } + }) + + bench('classify native package compatibility boolean', () => { + for (const entry of compatibilityMatrix) { isCompatible(entry) } }) })🤖 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 `@benches/cli-hot-paths.bench.ts` around lines 90 - 95, The benchmark currently times getCompatibilityDetails and isCompatible together; split them into two separate benches so each helper is measured independently: create one bench named something like "classify native package compatibility - getCompatibilityDetails" that loops over compatibilityMatrix and calls getCompatibilityDetails(entry), and another bench named "classify native package compatibility - isCompatible" that loops and calls isCompatible(entry); keep the same iteration and input (compatibilityMatrix) so results remain comparable.
🤖 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.
Nitpick comments:
In `@benches/cli-hot-paths.bench.ts`:
- Around line 90-95: The benchmark currently times getCompatibilityDetails and
isCompatible together; split them into two separate benches so each helper is
measured independently: create one bench named something like "classify native
package compatibility - getCompatibilityDetails" that loops over
compatibilityMatrix and calls getCompatibilityDetails(entry), and another bench
named "classify native package compatibility - isCompatible" that loops and
calls isCompatible(entry); keep the same iteration and input
(compatibilityMatrix) so results remain comparable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84f34295-2c4a-455f-ba6e-1fd48b55248a
📒 Files selected for processing (12)
.github/workflows/codspeed.ymlbenches/cli-hot-paths.bench.tsbenches/cloudflare-utils.bench.tsbenches/device-comparison.bench.tsbenches/password-policy.bench.tsbenches/plugin-hot-paths.bench.tscli/src/types/capacitor__cli.d.tscli/tsconfig.jsonpackage.jsonsupabase/functions/_backend/utils/deviceComparison.tstsconfig.jsonvitest.config.bench.ts
✅ Files skipped from review due to trivial changes (8)
- tsconfig.json
- benches/device-comparison.bench.ts
- .github/workflows/codspeed.yml
- package.json
- cli/tsconfig.json
- benches/cloudflare-utils.bench.ts
- benches/password-policy.bench.ts
- vitest.config.bench.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/src/types/capacitor__cli.d.ts
riderx
force-pushed
the
codex/perf-test-coverage
branch
from
e33e3fb to
44d9063
Compare
|
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
1 participant
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 (AI generated)
Motivation (AI generated)
Recent production performance regressions need to be caught before merge. CodSpeed was installed but only covered a few utility helpers and the workflow still invoked the benchmark runner through npm-style tooling. This expands PR performance coverage around the paths most likely to affect CLI uploads and plugin traffic.
Business Impact (AI generated)
This reduces the chance of shipping changes that slow down update checks, stats ingestion, channel self-selection, and CLI upload preparation. Better PR performance signal protects production reliability for high-volume plugin traffic and lowers incident risk from accidental hot-path regressions.
Test Plan (AI generated)
Generated with AI
Summary by CodeRabbit
New Features
Chores
Bug Fixes
Tests