test(app): add PR0.4 low-end perf profile#610
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughIntroduces a ChangesLow-End Performance Profiling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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 unit tests (beta)
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 |
github-actions
Bot
added
ci
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/testing/perf-metrics.test.ts, packages/app/src/testing/perf-metrics.ts, packages/app/src/testing/perf-workflow.test.ts)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces performance profiles, including a "low-end" profile that implements CPU throttling via the Chrome DevTools Protocol. It updates the E2E performance suite to support profile-specific scenarios and thresholds, adds a new timeline recomputation test, and provides a script for merging performance results. A review comment suggests parallelizing the session seeding process to improve setup efficiency, especially under throttled conditions.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/app/e2e/perf/probe.ts (1)
2-2: ⚡ Quick winType
scenarioasPerfScenarioNamein this API.This adds compile-time protection against misspelled scenario keys in perf summary outputs.
Proposed diff
-import { aggregatePerfRuns, summarizePerfRun, type PerfProfile, type PerfRunSummary } from "../../src/testing/perf-metrics" +import { + aggregatePerfRuns, + summarizePerfRun, + type PerfProfile, + type PerfRunSummary, + type PerfScenarioName, +} from "../../src/testing/perf-metrics" ... -export function summarizeScenarioRuns(input: { branch: string; profile?: PerfProfile; scenario: string; runs: PerfRunSummary[] }) { +export function summarizeScenarioRuns(input: { + branch: string + profile?: PerfProfile + scenario: PerfScenarioName + runs: PerfRunSummary[] +}) { return aggregatePerfRuns(input) }Also applies to: 161-161
🤖 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 `@packages/app/e2e/perf/probe.ts` at line 2, The exported API that accepts a scenario key (used by aggregatePerfRuns / summarizePerfRun and their outputs PerfProfile / PerfRunSummary) should type that parameter as the PerfScenarioName union to prevent misspelled keys; update the function signatures and any related variables named scenario to use the PerfScenarioName type (and adjust callers to satisfy the new type) so compile-time checks enforce valid scenario keys across aggregatePerfRuns, summarizePerfRun, and any perf summary construction code that assigns to scenario.
🤖 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 `@packages/app/e2e/perf/perf-probe.spec.ts`:
- Line 20: The module-level constant perfProfile should follow test constant
naming conventions; rename the identifier perfProfile to PERF_PROFILE and update
all usages accordingly (including the initialization call readPerfProfile() and
any references in perf-probe.spec.ts such as assertions or function calls that
use perfProfile) to maintain SCREAMING_SNAKE_CASE for test constants.
---
Nitpick comments:
In `@packages/app/e2e/perf/probe.ts`:
- Line 2: The exported API that accepts a scenario key (used by
aggregatePerfRuns / summarizePerfRun and their outputs PerfProfile /
PerfRunSummary) should type that parameter as the PerfScenarioName union to
prevent misspelled keys; update the function signatures and any related
variables named scenario to use the PerfScenarioName type (and adjust callers to
satisfy the new type) so compile-time checks enforce valid scenario keys across
aggregatePerfRuns, summarizePerfRun, and any perf summary construction code that
assigns to scenario.
🪄 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: Pro Plus
Run ID: e50084bf-21e6-4069-99d5-c1093922b52a
📒 Files selected for processing (9)
.github/workflows/perf-probe-baseline.ymlpackages/app/e2e/perf/perf-probe.spec.tspackages/app/e2e/perf/probe.tspackages/app/e2e/perf/profiles.tspackages/app/e2e/perf/timeline-fixture.tspackages/app/script/merge-perf-artifacts.tspackages/app/src/testing/perf-metrics.test.tspackages/app/src/testing/perf-metrics.tspackages/app/src/testing/perf-workflow.test.ts
Perf delta summaryComparator: pass
|
11 tasks
11 tasks
Astro-Han
added a commit
that referenced
this pull request
May 14, 2026
Root cause: - PR #610 added `packages/app/src/testing/perf-workflow.test.ts`, which read `.github/workflows/perf-probe-baseline.yml` and matched a literal LF-only snippet. - Git for Windows checked the workflow file out with CRLF, so Windows advisory `unit-windows-app` failed even though the workflow content was correct. Change boundary: - Normalize line endings inside the perf workflow contract test before doing snippet assertions. - Add a focused CRLF regression case matching the Windows advisory failure. - Test-only change; no product runtime, packaging, updater, or workflow execution behavior changed. Verification: - Added the CRLF regression test first and confirmed the old assertion failed. - `bun test --preload ./happydom.ts ./src/testing/perf-workflow.test.ts` passed: 2 tests, 0 failures. - `bun run test:ci` in `packages/app` passed: 1067 tests, 0 failures. - `git diff --check` passed. - PR #623 checks passed, including CodeQL, CI, desktop smoke, e2e-artifacts, perf-probe-baseline, and PR title checks. - Manual Windows advisory workflow `25872283970` on `codex/fix-windows-perf-workflow-eol` passed, including `unit-windows-app` and all Windows advisory jobs. Review and risk: - No unresolved review threads. - Residual risk is low and limited to the test assertion helper; this does not alter release artifacts or application behavior.
This was referenced May 14, 2026
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
low-endperf profile that runs only the Session Timeline recompute scenario.Why
PR0.4 needs a low-cost CPU pressure profile before Area A visual work lands. This is intentionally not a real-device low-end phone simulation. It is a repeatable CI signal for Session Timeline main-thread cost under a smaller CPU budget.
Related Issue
Refs #600.
Human Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
Review Focus
session-timeline-recomputeis the right minimal Area A-facing scenario.Risk Notes
This adds optional extra perf E2E work only for workflow dispatch or Session Timeline/perf-related paths. It does not change product runtime behavior. I requested maintainer labeling through this PR body instead of applying labels directly.
How To Verify
Screenshots or Recordings
Not applicable. This PR changes perf harness and CI workflow behavior, not visible UI.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
Release Notes
New Features
Tests