Optimize e2e parallel#562
Conversation
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a reusable GitHub Actions E2E workflow with smoke and nightly full variants, wires it into PR and nightly workflows, increases Playwright concurrency, and updates three Playwright specs to create and clean up unique per-test Kubernetes resources. ChangesE2E CI workflow
E2E test execution and fixtures
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Trigger
participant E2ECommon as e2e-common.yaml
participant DevServer
participant Playwright
participant ArtifactStore
Trigger->>E2ECommon: workflow_call (suite input)
E2ECommon->>DevServer: start plugin dev server
E2ECommon->>DevServer: poll http://localhost:9001
DevServer-->>E2ECommon: ready
E2ECommon->>Playwright: run smoke or full suite
Playwright-->>E2ECommon: test results
E2ECommon->>ArtifactStore: upload playwright-report-${suite}
E2ECommon->>E2ECommon: run ./e2e/teardown.sh
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
hey nice work on getting the CI time down! couple of thoughts: reusable workflow instead of duplicating yaml instead of having two separate workflow files with mostly the same setup steps, could we use a reusable workflow? something like making parallel collision risks the
basically anything creating resources with fixed names could collide with |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
.github/workflows/e2e-common.yaml (4)
42-45: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBackgrounded server start swallows failures until the readiness timeout.
yarn clean && yarn start &backgrounds immediately, so this step always exits 0 even ifyarn clean/yarn startfail outright. Failures are only surfaced up to 60s later via the readiness loop, and stdout/stderr from the background process isn't captured to a log for debugging. Redirecting output to a file would ease triage on failures.♻️ Suggested improvement
- name: Start plugin dev server - run: yarn clean && yarn start & + run: (yarn clean && yarn start) > /tmp/plugin-dev-server.log 2>&1 &🤖 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 @.github/workflows/e2e-common.yaml around lines 42 - 45, The Start plugin dev server step backgrounds too early, so failures from yarn clean or yarn start are hidden until the readiness loop times out. Update the workflow step in the Start plugin dev server job to run the command in a way that preserves the exit status and captures stdout/stderr to a log file for later inspection, so issues are surfaced immediately and are easier to debug.
17-17: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable credential persistence on checkout.
actions/checkout@v4persists theGITHUB_TOKENcredential in the local git config by default. Since this job uploads artifacts (playwright-report/) and doesn't need git write access afterwards, disable persistence to avoid inadvertent credential exposure.🔒 Proposed fix
- uses: actions/checkout@v4 + with: + persist-credentials: false🤖 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 @.github/workflows/e2e-common.yaml at line 17, The checkout step currently uses actions/checkout@v4 with default credential persistence, which is unnecessary for this artifact-only job. Update the checkout configuration in the e2e-common workflow to disable persisted git credentials by setting the checkout action’s credential persistence option off, so the job does not leave the GITHUB_TOKEN in local git config.Source: Linters/SAST tools
26-31: 🔒 Security & Privacy | 🔵 Trivial | ⚖️ Poor tradeoffNo integrity verification for the downloaded
oincbinary.The binary is fetched over HTTPS and executed directly without checksum/signature verification, which is a minor supply-chain hardening gap for CI tooling.
Please check whether the
oincGitHub releases publish checksums or signatures that could be verified beforechmod +x/execution.🤖 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 @.github/workflows/e2e-common.yaml around lines 26 - 31, The Install oinc step downloads and installs a binary without verifying its integrity. Update the workflow job around the oinc install commands to check whether the GitHub release provides a checksum or signature, and verify the downloaded file before chmod +x and sudo mv. If a verification artifact exists, add the corresponding validation step in the same install block so the oinc binary is only installed after a successful integrity check.
62-74: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSilent no-op if
suitedoesn't matchsmokeorfull.If
inputs.suiteis ever anything other than'smoke'or'full'(e.g. a typo in a future caller), neither test step's condition is satisfied, no tests run, and the job still reports success. Consider adding a fallback validation step to fail fast on an unrecognized suite value.🛡️ Proposed fix
+ - name: Validate suite input + run: | + if [[ "${{ inputs.suite }}" != "smoke" && "${{ inputs.suite }}" != "full" ]]; then + echo "Invalid suite input: ${{ inputs.suite }}" + exit 1 + fi + - name: Run E2E tests (smoke)🤖 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 @.github/workflows/e2e-common.yaml around lines 62 - 74, The E2E workflow currently does nothing when inputs.suite is not exactly smoke or full, so add an explicit validation/fallback in the same job to fail fast on unsupported values. Update the logic around the Run E2E tests (smoke) and Run E2E tests (full) steps in e2e-common.yaml so an unexpected inputs.suite value triggers a clear error before either Playwright command is skipped..github/workflows/e2e.yaml (1)
15-18: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd an explicit least-privilege
permissionsblock.No
permissions:block is set, so the job inherits the repository's defaultGITHUB_TOKENscope, which may be broader than required. This job only needs to check out code and upload artifacts (in the shared workflow).🔒 Proposed fix
+permissions: + contents: read + jobs: e2e-smoke: uses: ./.github/workflows/e2e-common.yaml🤖 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 @.github/workflows/e2e.yaml around lines 15 - 18, The e2e-smoke workflow job currently relies on the default inherited GITHUB_TOKEN scope, so add an explicit least-privilege permissions block for the reusable job defined in e2e-smoke. Update the workflow near the existing uses: ./.github/workflows/e2e-common.yaml stanza so the job only has the token access needed for checkout and artifact upload, and keep the scope as narrow as possible.Source: Linters/SAST tools
e2e/tests/apikey-approvals.spec.ts (1)
92-147: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNamespace cleanup isn't guaranteed on test failure.
Unlike the other suites in this file (bob, bob2, carol/dave, ellen/frank, george all use
test.beforeEach/test.afterEach), this test createsaliceNsand only deletes it at line 146, after all assertions. If anyexpect(...)in between throws (e.g. the approval modal or toast timeout), the namespace is never cleaned up, leaking test resources on the cluster.♻️ Move fixture lifecycle into beforeEach/afterEach, matching sibling suites
- test('approve request shows success toast', async ({ page }) => { - const uid = String(Date.now()); - const aliceNs = `consumer-alice-approve-${uid}`; + let uid: string; + let aliceNs: string; + let aliceKey: string; + let aliceEmail: string; + + test.beforeEach(async () => { + uid = String(Date.now()); + aliceNs = `consumer-alice-approve-${uid}`; ... + }); + + test.afterEach(async () => { + execSync(`kubectl delete namespace ${aliceNs} --ignore-not-found=true`, { stdio: 'inherit' }); + }); + + test('approve request shows success toast', async ({ page }) => { ... - execSync(`kubectl delete namespace ${aliceNs} --ignore-not-found=true`, { stdio: 'inherit' }); });🤖 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 `@e2e/tests/apikey-approvals.spec.ts` around lines 92 - 147, The alice approval test creates aliceNs inline and only deletes it at the end, so failures skip cleanup and leak cluster resources. Move the fixture setup/teardown for this test into test.beforeEach and test.afterEach, following the same pattern used by the sibling approval suites, and make sure aliceNs, aliceKey, and aliceEmail are owned by that lifecycle so the namespace is always deleted even when an assertion fails.e2e/tests/apiproduct-crud.spec.ts (1)
37-84: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winScope this fixture to the edit/delete tests.
test.beforeEach/test.afterEachcurrently wrap the wholeAPIProduct CRUD Operationssuite, so the create/list/view tests also pay for an extrakubectl apply/kubectl deleteon every run. Move this into a nesteddescribefor the tests that useeditProductName.🤖 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 `@e2e/tests/apiproduct-crud.spec.ts` around lines 37 - 84, Move the edit fixture setup/teardown out of the top-level APIProduct CRUD suite and into a nested describe that only contains the edit/delete tests. The current test.beforeEach and test.afterEach around editProductName make every test run kubectl apply/delete unnecessarily; keep the existing beforeEach/afterEach logic but scope it to the tests that actually use editProductName, preserving the page.goto and dismissConsoleTour flow for those tests only.
🤖 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 `@e2e/tests/apikey-approvals.spec.ts`:
- Line 372: The concurrent polling in execSync uses backgrounded timeout loops
followed by a bare wait, which hides failures when either APIKeyRequest never
appears. Update the APIKey approvals test to capture and validate each
background job’s exit status instead of relying on plain wait, and apply the
same fix to both the carol/dave and ellen/frank polling blocks in
apikey-approvals.spec.ts so the test fails immediately if either resource does
not show up.
- Around line 92-95: The current `uid = String(Date.now())` pattern in this test
file can still collide across parallel workers, so update the shared test
identifier generation used for `aliceNs`, `aliceKey`, and `aliceEmail` to
include extra entropy such as the Playwright worker index or a random suffix.
Apply the same fix to the repeated `uid` creation in this file and the related
specs `apiproduct-apikeys-tab.spec.ts` and `apiproduct-crud.spec.ts` so all
generated namespaces, keys, and emails remain unique under `fullyParallel` with
multiple workers.
---
Nitpick comments:
In @.github/workflows/e2e-common.yaml:
- Around line 42-45: The Start plugin dev server step backgrounds too early, so
failures from yarn clean or yarn start are hidden until the readiness loop times
out. Update the workflow step in the Start plugin dev server job to run the
command in a way that preserves the exit status and captures stdout/stderr to a
log file for later inspection, so issues are surfaced immediately and are easier
to debug.
- Line 17: The checkout step currently uses actions/checkout@v4 with default
credential persistence, which is unnecessary for this artifact-only job. Update
the checkout configuration in the e2e-common workflow to disable persisted git
credentials by setting the checkout action’s credential persistence option off,
so the job does not leave the GITHUB_TOKEN in local git config.
- Around line 26-31: The Install oinc step downloads and installs a binary
without verifying its integrity. Update the workflow job around the oinc install
commands to check whether the GitHub release provides a checksum or signature,
and verify the downloaded file before chmod +x and sudo mv. If a verification
artifact exists, add the corresponding validation step in the same install block
so the oinc binary is only installed after a successful integrity check.
- Around line 62-74: The E2E workflow currently does nothing when inputs.suite
is not exactly smoke or full, so add an explicit validation/fallback in the same
job to fail fast on unsupported values. Update the logic around the Run E2E
tests (smoke) and Run E2E tests (full) steps in e2e-common.yaml so an unexpected
inputs.suite value triggers a clear error before either Playwright command is
skipped.
In @.github/workflows/e2e.yaml:
- Around line 15-18: The e2e-smoke workflow job currently relies on the default
inherited GITHUB_TOKEN scope, so add an explicit least-privilege permissions
block for the reusable job defined in e2e-smoke. Update the workflow near the
existing uses: ./.github/workflows/e2e-common.yaml stanza so the job only has
the token access needed for checkout and artifact upload, and keep the scope as
narrow as possible.
In `@e2e/tests/apikey-approvals.spec.ts`:
- Around line 92-147: The alice approval test creates aliceNs inline and only
deletes it at the end, so failures skip cleanup and leak cluster resources. Move
the fixture setup/teardown for this test into test.beforeEach and
test.afterEach, following the same pattern used by the sibling approval suites,
and make sure aliceNs, aliceKey, and aliceEmail are owned by that lifecycle so
the namespace is always deleted even when an assertion fails.
In `@e2e/tests/apiproduct-crud.spec.ts`:
- Around line 37-84: Move the edit fixture setup/teardown out of the top-level
APIProduct CRUD suite and into a nested describe that only contains the
edit/delete tests. The current test.beforeEach and test.afterEach around
editProductName make every test run kubectl apply/delete unnecessarily; keep the
existing beforeEach/afterEach logic but scope it to the tests that actually use
editProductName, preserving the page.goto and dismissConsoleTour flow for those
tests only.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fbbe4f25-317a-4c87-9c76-6c39830c3452
📒 Files selected for processing (7)
.github/workflows/e2e-common.yaml.github/workflows/e2e-nightly.yaml.github/workflows/e2e.yamle2e/playwright.config.tse2e/tests/apikey-approvals.spec.tse2e/tests/apiproduct-apikeys-tab.spec.tse2e/tests/apiproduct-crud.spec.ts
- Fix parallel conflicts by using unique resource names per test - Refactor edit test to use unique APIProduct instead of shared fixture Signed-off-by: Anton-Fil <a.filkach@gmail.com>
…able workflow, retry flakiness fixes Signed-off-by: Anton-Fil <a.filkach@gmail.com>
688a772 to
c2116d6
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/tests/apikey-approvals.spec.ts (1)
92-148: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNamespace cleanup isn't guaranteed on test failure.
kubectl delete namespace ${aliceNs}(line 146) only runs if every preceding assertion in the test body succeeds. If anyexpect(...)between lines 129-144 times out or fails, the test throws and this cleanup step never executes, leaking theconsumer-alice-approve-*namespace. Every other suite in this file (bob, bob2, carol/dave, ellen/frank, george) avoids this by placing cleanup intest.afterEach, which Playwright always runs regardless of outcome. WithfullyParallel: trueand repeated nightly full-suite runs, failed/flaky runs of this specific test will accumulate orphaned namespaces in the cluster.🛠️ Suggested fix: move setup/cleanup into beforeEach/afterEach like the other suites
-test.describe('APIKey Approvals - Approve single request', () => { - test('approve single request shows success toast', async ({ page }) => { - const uid = `${Date.now()}-${Math.random().toString(36).slice(2, 6)}`; - const aliceNs = `consumer-alice-approve-${uid}`; - ... - execSync(`kubectl delete namespace ${aliceNs} --ignore-not-found=true`, { stdio: 'inherit' }); - }); -}); +test.describe('APIKey Approvals - Approve single request', () => { + let uid: string; + let aliceNs: string; + ... + test.beforeEach(async ({ page }) => { + uid = `${Date.now()}-${Math.random().toString(36).slice(2, 6)}`; + aliceNs = `consumer-alice-approve-${uid}`; + ... + }); + + test.afterEach(async () => { + execSync(`kubectl delete namespace ${aliceNs} --ignore-not-found=true`, { stdio: 'inherit' }); + }); + + test('approve single request shows success toast', async ({ page }) => { + ... + }); +});🤖 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 `@e2e/tests/apikey-approvals.spec.ts` around lines 92 - 148, The cleanup for the alice approval test is only executed at the end of the test body, so it is skipped on assertion failures and can leave orphaned namespaces. Move the namespace creation and APIKey setup into a shared setup path and add the `kubectl delete namespace ${aliceNs}` cleanup to a `test.afterEach` block, matching the pattern used by the other approval suites in `apikey-approvals.spec.ts`. Use the existing `aliceNs`, `aliceKey`, and `aliceEmail` test variables to keep the teardown tied to the correct test instance.
🧹 Nitpick comments (1)
.github/workflows/e2e-common.yaml (1)
36-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo TypeScript compile-time check before running tests.
The coding guidelines require running TypeScript type checks throughout the codebase, but this reusable e2e workflow goes straight from
yarn installto Playwright browser install/tests with notsc --noEmit(or equivalent lint/type-check) step, so type errors in test specs or app code would only surface as opaque runtime failures during the e2e run.As per coding guidelines, "
**/*.ts?(x): Run TypeScript for type safety and compile-time checks throughout the codebase".🤖 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 @.github/workflows/e2e-common.yaml around lines 36 - 41, The reusable e2e workflow skips the required TypeScript safety check before browser setup and tests. Update the workflow around the Install dependencies / Install Playwright browsers steps to run a TypeScript compile-time check first (such as tsc --noEmit or the repo’s equivalent type-check command) so failures in specs or app code are caught early. Keep the change in the e2e-common workflow and preserve the existing Playwright install/test flow after the new type-check step.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@e2e/tests/apikey-approvals.spec.ts`:
- Around line 92-148: The cleanup for the alice approval test is only executed
at the end of the test body, so it is skipped on assertion failures and can
leave orphaned namespaces. Move the namespace creation and APIKey setup into a
shared setup path and add the `kubectl delete namespace ${aliceNs}` cleanup to a
`test.afterEach` block, matching the pattern used by the other approval suites
in `apikey-approvals.spec.ts`. Use the existing `aliceNs`, `aliceKey`, and
`aliceEmail` test variables to keep the teardown tied to the correct test
instance.
---
Nitpick comments:
In @.github/workflows/e2e-common.yaml:
- Around line 36-41: The reusable e2e workflow skips the required TypeScript
safety check before browser setup and tests. Update the workflow around the
Install dependencies / Install Playwright browsers steps to run a TypeScript
compile-time check first (such as tsc --noEmit or the repo’s equivalent
type-check command) so failures in specs or app code are caught early. Keep the
change in the e2e-common workflow and preserve the existing Playwright
install/test flow after the new type-check step.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a438a6c-cbbb-4045-a27c-61c19cd440d8
📒 Files selected for processing (7)
.github/workflows/e2e-common.yaml.github/workflows/e2e-nightly.yaml.github/workflows/e2e.yamle2e/playwright.config.tse2e/tests/apikey-approvals.spec.tse2e/tests/apiproduct-apikeys-tab.spec.tse2e/tests/apiproduct-crud.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/e2e-nightly.yaml
- e2e/playwright.config.ts
- e2e/tests/apiproduct-apikeys-tab.spec.ts
Signed-off-by: Anton-Fil <a.filkach@gmail.com>
|
👀 |
R-Lawton
left a comment
There was a problem hiding this comment.
nice work on the ci speedup, couple things before merging
| let editProductName = ''; | ||
|
|
||
| test.beforeEach(async ({ page }) => { | ||
| // Create a unique APIProduct for the edit test to avoid conflicts with other parallel tests |
There was a problem hiding this comment.
the beforeEach creates a unique APIProduct for every test but only edit and delete use editProductName. the other ~10 tests create and tear down a k8s resource for nothing. wrapping edit and delete in a nested test.describe with their own setup/teardown would fix this
| await expect(page.locator('td:has-text("alice@example.com")')).toBeVisible(); | ||
| }); | ||
| test.beforeEach(async ({ page }) => { | ||
| const uid = `${Date.now()}-${Math.random().toString(36).slice(2, 6)}`; |
There was a problem hiding this comment.
same thing here. the beforeEach creates a dynamic namespace + APIKey + Secret but the "cancel approval modal" test never uses them, it just operates on the setup.sh alice@example.com. these should be moved into their own describe or the cancel test should use the dynamic resources
| execSync('timeout 30 bash -c \'until kubectl get apikeyrequests -n kuadrant-test | grep -q carol-bulk; do sleep 1; done\'', { stdio: 'inherit' }); | ||
| execSync('timeout 30 bash -c \'until kubectl get apikeyrequests -n kuadrant-test | grep -q dave-bulk; do sleep 1; done\'', { stdio: 'inherit' }); | ||
| execSync(`bash -c 'timeout 30 bash -c "until kubectl get apikeyrequests -n kuadrant-test | grep -q ${carolKey}; do sleep 1; done" & PID1=$!; timeout 30 bash -c "until kubectl get apikeyrequests -n kuadrant-test | grep -q ${daveKey}; do sleep 1; done" & PID2=$!; wait $PID1 || exit 1; wait $PID2 || exit 1'`, { stdio: 'inherit' }); | ||
|
|
There was a problem hiding this comment.
the parallel kubectl wait works but it's pretty dense. a short comment explaining what it's doing would help
|
|
||
| - name: Start plugin dev server | ||
| run: yarn clean && yarn start & | ||
| env: |
There was a problem hiding this comment.
if yarn start dies in the background the wait loop sits there for 60s before failing with no useful output. since you're already improving ci reliability here, capturing the pid and checking it's still alive during the loop would make it fail fast
Description
Closes #559
Reduces e2e CI time from ~30 minutes to ~11 minutes by enabling parallel test execution, fixing parallel conflicts, and splitting tests into smoke (PR) and nightly runs.
Type of change
Parallel test execution
fullyParallel: trueand increaseworkersfrom 1 to 3Fix parallel conflicts
apiproduct-apikeys-tab: use unique namespace names per test run(
george-${uid}) instead of fixed names to avoid conflictsapiproduct-crud: addmode: serialto prevent race conditionwith
apiproduct-overview-tabreading sharedtest-edit-productbeforeEachand clean up inafterEachSmoke / Nightly split
CI optimization
yarn startin parallel with cluster setup to save ~40sHow to verify
Smoke tests (runs automatically on PR):
E2E Testsworkflow automaticallyNightly tests (full suite):
E2E Tests (Nightly)in your forkRun workflow→Run workflowto trigger manuallySummary by CodeRabbit