Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBrowser-resume and focus events now trigger time-label refreshes and resume health checks that may request websocket reconnects; WebSocket provider ignores stale socket events and logs debug info; stream sync/refetch was centralized; several tests were added/updated and one integration-style stream-context test file was removed. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant Component as BrainLeftSidebarWaveDropTime
participant Health as useWebSocketHealth
participant Provider as WebSocketProvider
participant Context as MyStreamContext
Browser->>Component: visibilitychange / focus
Component->>Component: refreshNow() (useEffectEvent)
Component->>Component: re-render -> getTimeAgoShort(time)
Browser->>Health: visibilitychange / focus
Health->>Health: dedupe & cooldown checks
alt No reconnect needed
Health->>Health: no action
else Reconnect needed
Health->>Provider: request connect(currentToken)
Provider->>Provider: create new WebSocket (ws2)
Provider->>Provider: ignore events from stale ws1
Provider->>Health: ws2.onopen -> report CONNECTED
end
Health->>Context: trigger sync + refetch
Context->>Context: registerWave + refetchAllWaves
Context->>Browser: UI updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f201afa382
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
components/brain/left-sidebar/waves/BrainLeftSidebarWaveDropTime.tsx (1)
29-47: Consider lifting these global listeners out of each row component.Every
BrainLeftSidebarWaveDropTimeinstance now adds its ownvisibilitychangeandfocuslisteners. In a longer sidebar list, one tab resume fans out into N listeners and N state updates, and both events can fire for the same resume. A shared resume tick at the list/context level would keep the refresh behavior without multiplying listeners per row.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/left-sidebar/waves/BrainLeftSidebarWaveDropTime.tsx` around lines 29 - 47, The component BrainLeftSidebarWaveDropTime currently registers document.visibilitychange and window.focus listeners inside its useEffect, causing N listeners and repeated refreshNow calls; lift these global listeners to the parent/list/context level (e.g., the waves list component or a new useWindowResume hook) so a single handler calls a shared refresh tick and then propagate that tick into each row via a prop or context subscription; remove the visibility/focus useEffect from BrainLeftSidebarWaveDropTime and instead call the existing refreshNow from the single centralized handler to avoid per-row listener multiplication.contexts/wave/MyStreamContext.tsx (1)
182-197: Avoid double full-wave refetches on browser resume.
runBrowserResumeSync()always callssyncActiveWaveAndRefetch(). On the samevisibilitychange/focusevent,services/websocket/useWebSocketHealth.tscan reconnect the socket, and Line 199 then runs the same sync again when status flips back toCONNECTED. That gives the common stale-tab-resume path tworefetchAllWaves()calls back-to-back.♻️ Possible adjustment
lastBrowserResumeSyncAtRef.current = now; - syncActiveWaveAndRefetch(); + if (websocketStatus === WebSocketStatus.CONNECTED) { + syncActiveWaveAndRefetch(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contexts/wave/MyStreamContext.tsx` around lines 182 - 197, runBrowserResumeSync currently always calls syncActiveWaveAndRefetch which can duplicate a full refetch when useWebSocketHealth also triggers a reconnect-triggered refetch; modify runBrowserResumeSync so it first checks the websocket health/status (from useWebSocketHealth) and only calls syncActiveWaveAndRefetch if the socket is not already CONNECTED OR if the last full refetch timestamp is older than BROWSER_RESUME_SYNC_COOLDOWN_MS (e.g., track lastRefetchAllWavesAt similar to lastBrowserResumeSyncAtRef); this prevents back-to-back refetchAllWaves calls while keeping the existing cooldown logic.__tests__/services/websocket/WebSocketProvider.test.tsx (1)
683-770: Add a stale-message regression case too.The provider now guards
onmessage/onerrorfor replaced sockets, but these tests only pinopenandclose. A latemessagefromws1afterconnect("token2")is the path that would duplicate downstream updates, so it is worth asserting that subscribers are not notified from the replaced socket.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/services/websocket/WebSocketProvider.test.tsx` around lines 683 - 770, Add a regression test that after calling result.current.connect("token2") and obtaining ws1 and ws2, firing a late message from the replaced socket (call ws1.triggerMessage(...) or the MockWebSocket equivalent) does not notify subscribers or mutate context state; e.g., assert the context’s state (result.current.status) and any message handler mocks remain unchanged after ws1.triggerMessage and only change when ws2.triggerMessage is fired. Locate the test block near the existing "ignores open events from a replaced socket" and reuse MockWebSocket, result.current.connect, and WebSocketStatus to implement this assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@__tests__/components/brain/left-sidebar/waves/BrainLeftSidebarWaveDropTime.test.tsx`:
- Around line 34-67: The test still asserts two-argument calls to
getTimeAgoShort even though BrainLeftSidebarWaveDropTime now calls
getTimeAgoShort(time) — update the test to stop expecting the second timestamp:
either change the expectations to check call counts/that getTimeAgoShort was
called with a single arg (e.g. toHaveBeenLastCalledWith(10) or
toHaveBeenCalledTimes(...)) and/or assert the rendered output/rerender count
after triggering setDocumentVisibilityState("visible")/visibilitychange and
window.dispatchEvent(new Event("focus")); alternatively adjust the mocked
getTimeAgoShort implementation in this test to itself call Date.now() so the
single-argument call still yields different return values across the simulated
times. Ensure references to getTimeAgoShort and BrainLeftSidebarWaveDropTime are
updated accordingly.
In `@services/websocket/useWebSocketHealth.ts`:
- Around line 145-149: The reconnect logic only resumes sockets when
webSocketStateRef.current.status === WebSocketStatus.CONNECTED, skipping sockets
stuck in CONNECTING; update the recovery in the branch that reads const {
status: currentStatus, connect: currentConnect } = webSocketStateRef.current to
also handle WebSocketStatus.CONNECTING (i.e., treat CONNECTING like CONNECTED or
explicitly replace/resume that socket), so call currentConnect(currentToken)
when currentStatus is either CONNECTED or CONNECTING (or otherwise ensure
CONNECTING sockets are recreated), and keep performHealthCheck() behavior intact
to avoid leaving stale in-flight connections.
---
Nitpick comments:
In `@__tests__/services/websocket/WebSocketProvider.test.tsx`:
- Around line 683-770: Add a regression test that after calling
result.current.connect("token2") and obtaining ws1 and ws2, firing a late
message from the replaced socket (call ws1.triggerMessage(...) or the
MockWebSocket equivalent) does not notify subscribers or mutate context state;
e.g., assert the context’s state (result.current.status) and any message handler
mocks remain unchanged after ws1.triggerMessage and only change when
ws2.triggerMessage is fired. Locate the test block near the existing "ignores
open events from a replaced socket" and reuse MockWebSocket,
result.current.connect, and WebSocketStatus to implement this assertion.
In `@components/brain/left-sidebar/waves/BrainLeftSidebarWaveDropTime.tsx`:
- Around line 29-47: The component BrainLeftSidebarWaveDropTime currently
registers document.visibilitychange and window.focus listeners inside its
useEffect, causing N listeners and repeated refreshNow calls; lift these global
listeners to the parent/list/context level (e.g., the waves list component or a
new useWindowResume hook) so a single handler calls a shared refresh tick and
then propagate that tick into each row via a prop or context subscription;
remove the visibility/focus useEffect from BrainLeftSidebarWaveDropTime and
instead call the existing refreshNow from the single centralized handler to
avoid per-row listener multiplication.
In `@contexts/wave/MyStreamContext.tsx`:
- Around line 182-197: runBrowserResumeSync currently always calls
syncActiveWaveAndRefetch which can duplicate a full refetch when
useWebSocketHealth also triggers a reconnect-triggered refetch; modify
runBrowserResumeSync so it first checks the websocket health/status (from
useWebSocketHealth) and only calls syncActiveWaveAndRefetch if the socket is not
already CONNECTED OR if the last full refetch timestamp is older than
BROWSER_RESUME_SYNC_COOLDOWN_MS (e.g., track lastRefetchAllWavesAt similar to
lastBrowserResumeSyncAtRef); this prevents back-to-back refetchAllWaves calls
while keeping the existing cooldown logic.
🪄 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: dbd84d2e-d75b-491e-845e-b84d5977f781
📒 Files selected for processing (9)
__tests__/components/brain/left-sidebar/waves/BrainLeftSidebarWaveDropTime.test.tsx__tests__/contexts/wave/MyStreamContext.test.tsx__tests__/services/websocket/WebSocketProvider.test.tsx__tests__/services/websocket/useWebSocketHealth.test.tscomponents/brain/left-sidebar/waves/BrainLeftSidebarWaveDropTime.tsxcontexts/wave/MyStreamContext.tsxservices/websocket/AppWebSocketProvider.tsxservices/websocket/WebSocketProvider.tsxservices/websocket/useWebSocketHealth.ts
💤 Files with no reviewable changes (1)
- tests/contexts/wave/MyStreamContext.test.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/websocket/useWebSocketHealth.ts`:
- Around line 125-128: The resume-dedupe logic currently prevents a valid resume
check if a focus happened just before a hide/show cycle because
lastResumeCheckAtRef.current isn't cleared on hide; update the hide handler
(where hiddenAtRef is set) to also reset lastResumeCheckAtRef.current (or set it
to 0/null) so each new hidden->visible cycle triggers one resume check; apply
the same change in both places noted around the checks that reference
RESUME_EVENT_DEDUPE_WINDOW_MS, lastResumeCheckAtRef, and hiddenAtRef so the
dedupe window is cleared whenever the document becomes hidden.
- Around line 88-98: The two branches that call currentConnect(currentToken) and
set action = "connect" are duplicated; replace them with a single conditional
that checks currentToken && (currentStatus === WebSocketStatus.DISCONNECTED ||
currentToken !== previousToken) and then calls currentConnect(currentToken) and
sets action = "connect" (references: currentToken, currentStatus,
WebSocketStatus.DISCONNECTED, previousToken, currentConnect, action).
🪄 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: 87fbd852-4c8d-4b8a-8827-dde345f820ef
📒 Files selected for processing (2)
__tests__/services/websocket/useWebSocketHealth.test.tsservices/websocket/useWebSocketHealth.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
services/websocket/useWebSocketHealth.ts (2)
145-149:⚠️ Potential issue | 🟠 MajorHandle
CONNECTINGsockets in the forced-resume path.This still skips recovery when the provider is stuck in
CONNECTINGafter a long hidden period, so the tab can resume on a stale in-flight socket.🔧 Minimal fix
- if (currentStatus === WebSocketStatus.CONNECTED) { + if (currentStatus !== WebSocketStatus.DISCONNECTED) { currentConnect(currentToken); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/websocket/useWebSocketHealth.ts` around lines 145 - 149, The forced-resume branch only calls currentConnect when webSocketStateRef.current.status equals WebSocketStatus.CONNECTED, which skips recovery if the socket is stuck in WebSocketStatus.CONNECTING; update the condition in the forced-resume path to also treat WebSocketStatus.CONNECTING as a case that should call currentConnect(currentToken) (i.e., when webSocketStateRef.current.status === WebSocketStatus.CONNECTED || webSocketStateRef.current.status === WebSocketStatus.CONNECTING) so both CONNECTED and CONNECTING sockets are re-attached; references: webSocketStateRef, WebSocketStatus.CONNECTED, WebSocketStatus.CONNECTING, currentConnect, currentToken.
161-164:⚠️ Potential issue | 🟠 MajorReset the resume dedupe marker when a new hidden cycle starts.
Without clearing
lastResumeCheckAtRef.currenthere, a focus shortly before a real hide/show cycle can suppress the next valid resume check and delay reconnect until the interval tick.🔧 Minimal fix
const handleVisibilityChange = () => { if (document.visibilityState === "hidden") { + lastResumeCheckAtRef.current = 0; hiddenAtRef.current = Date.now(); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/websocket/useWebSocketHealth.ts` around lines 161 - 164, In handleVisibilityChange, when document.visibilityState === "hidden" reset the resume-dedupe marker by clearing lastResumeCheckAtRef.current (in addition to setting hiddenAtRef.current = Date.now()) so a new hidden/shown cycle won't be suppressed by a prior lastResumeCheckAtRef value; update the function that manages visibility events (handleVisibilityChange) to set lastResumeCheckAtRef.current = 0 or null immediately when starting a hidden cycle.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@services/websocket/useWebSocketHealth.ts`:
- Around line 145-149: The forced-resume branch only calls currentConnect when
webSocketStateRef.current.status equals WebSocketStatus.CONNECTED, which skips
recovery if the socket is stuck in WebSocketStatus.CONNECTING; update the
condition in the forced-resume path to also treat WebSocketStatus.CONNECTING as
a case that should call currentConnect(currentToken) (i.e., when
webSocketStateRef.current.status === WebSocketStatus.CONNECTED ||
webSocketStateRef.current.status === WebSocketStatus.CONNECTING) so both
CONNECTED and CONNECTING sockets are re-attached; references: webSocketStateRef,
WebSocketStatus.CONNECTED, WebSocketStatus.CONNECTING, currentConnect,
currentToken.
- Around line 161-164: In handleVisibilityChange, when document.visibilityState
=== "hidden" reset the resume-dedupe marker by clearing
lastResumeCheckAtRef.current (in addition to setting hiddenAtRef.current =
Date.now()) so a new hidden/shown cycle won't be suppressed by a prior
lastResumeCheckAtRef value; update the function that manages visibility events
(handleVisibilityChange) to set lastResumeCheckAtRef.current = 0 or null
immediately when starting a hidden cycle.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a5f7525-cc5a-4961-a065-91a5d49592ab
📒 Files selected for processing (2)
__tests__/services/websocket/useWebSocketHealth.test.tsservices/websocket/useWebSocketHealth.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/services/websocket/useWebSocketHealth.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/websocket/WebSocketProvider.tsx (1)
152-196:⚠️ Potential issue | 🟠 MajorReconnect timer calls a stale
connectclosure whenconfig.urlchanges.
attemptReconnectat Line 152 schedules a timeout that callsconnect(reconnectTokenRef.current)at Line 194. However,connectis not listed in the dependency array at Line 196 ([config.maxReconnectAttempts, config.reconnectDelay]). Sinceconnectdepends onconfig.url(used at Line 234), when the URL changes,connectis recreated butattemptReconnectretains the old closure, causing scheduled retries to reconnect with outdated config.Store
connectin a ref and update it after each render to ensure scheduled callbacks use the latest version without creating circular hook dependencies.💡 Proposed fix
const reconnectAttemptsRef = useRef(0); const reconnectTimerRef = useRef<NodeJS.Timeout | null>(null); const isManualDisconnectRef = useRef(false); const reconnectTokenRef = useRef<string | undefined>(undefined); + const connectRef = useRef<(token?: string) => void>(() => {}); const connect = useCallback( (token?: string) => { // ... existing implementation ... }, [config.url, handleMessage, clearReconnectTimer, attemptReconnect] ); + + connectRef.current = connect; reconnectTimerRef.current = setTimeout(() => { reconnectAttemptsRef.current += 1; logWebSocketDebug("Running scheduled reconnect attempt", { attempt: reconnectAttemptsRef.current, hasToken: Boolean(reconnectTokenRef.current), }); - connect(reconnectTokenRef.current); + connectRef.current(reconnectTokenRef.current); }, delay);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/websocket/WebSocketProvider.tsx` around lines 152 - 196, The scheduled reconnect uses a stale connect closure because attemptReconnect doesn't capture updates to connect when config.url changes; fix this by creating a ref (e.g., connectRef) that is assigned connect on every render and updating attemptReconnect to call connectRef.current(reconnectTokenRef.current) inside the setTimeout callback, ensuring reconnectTimerRef and reconnectAttemptsRef behavior is unchanged; update any hook dependencies accordingly so attemptReconnect only depends on config.maxReconnectAttempts and config.reconnectDelay (not connect) and ensure the ref is kept in sync after each render.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/services/websocket/useWebSocketHealth.test.ts`:
- Around line 72-77: The test helper setDocumentVisibilityState currently
overrides document.visibilityState via Object.defineProperty but never restores
the original descriptor; update the test teardown (the afterAll block) to remove
or restore that override by calling Object.defineProperty(document,
"visibilityState", originalDescriptor) or delete the property so the jsdom
global isn't mutated, and capture the original descriptor before
setDocumentVisibilityState mutates it (store it in a variable near
setDocumentVisibilityState) so afterAll can reassign it back or delete the
custom descriptor to avoid leaking into other suites.
---
Outside diff comments:
In `@services/websocket/WebSocketProvider.tsx`:
- Around line 152-196: The scheduled reconnect uses a stale connect closure
because attemptReconnect doesn't capture updates to connect when config.url
changes; fix this by creating a ref (e.g., connectRef) that is assigned connect
on every render and updating attemptReconnect to call
connectRef.current(reconnectTokenRef.current) inside the setTimeout callback,
ensuring reconnectTimerRef and reconnectAttemptsRef behavior is unchanged;
update any hook dependencies accordingly so attemptReconnect only depends on
config.maxReconnectAttempts and config.reconnectDelay (not connect) and ensure
the ref is kept in sync after each render.
🪄 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: 34ee60cf-aa89-4550-b8f5-86e658149dfe
📒 Files selected for processing (5)
__tests__/services/websocket/useWebSocketHealth.test.tscontexts/wave/MyStreamContext.tsxservices/websocket/WebSocketProvider.tsxservices/websocket/useWebSocketHealth.tsservices/websocket/webSocketDebug.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- contexts/wave/MyStreamContext.tsx
- services/websocket/useWebSocketHealth.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
services/websocket/useWebSocketHealth.ts (1)
269-275:⚠️ Potential issue | 🟠 MajorReset the resume dedupe marker when the tab becomes hidden.
This earlier issue still looks unresolved. If a focus event lands shortly before a real hide/show cycle,
lastResumeCheckAtRef.currentcan still be inside the 1s dedupe window, so the next visible event is discarded,hiddenAtRefgets cleared inperformResumeHealthCheck, and the long-hidden reconnect path is skipped until the interval check runs. Reset the dedupe timestamp in the hidden branch.🔧 Minimal fix
const handleVisibilityChange = () => { if (document.visibilityState === "hidden") { + lastResumeCheckAtRef.current = 0; hiddenAtRef.current = Date.now(); logWebSocketDebug("Document became hidden", { hiddenAt: hiddenAtRef.current, }); return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/websocket/useWebSocketHealth.ts` around lines 269 - 275, In handleVisibilityChange's "hidden" branch, clear the resume-dedupe marker so a subsequent visible event won't be dropped; specifically reset lastResumeCheckAtRef.current (e.g., set to 0 or null) when you set hiddenAtRef.current and log the hide, so performResumeHealthCheck / the next visibility "visible" will not be suppressed by the 1s dedupe window.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/websocket/useWebSocketHealth.ts`:
- Line 52: Replace the nullish coalescing with logical OR in the health check so
deleted-cookie hits aren't ignored when matchChanged is explicitly false: update
the return expression in useWebSocketHealth (the line returning
Boolean(matchChanged ?? matchDeleted)) to use Boolean(matchChanged ||
matchDeleted) so that either matchChanged or matchDeleted being true will
trigger the disconnection logic.
---
Duplicate comments:
In `@services/websocket/useWebSocketHealth.ts`:
- Around line 269-275: In handleVisibilityChange's "hidden" branch, clear the
resume-dedupe marker so a subsequent visible event won't be dropped;
specifically reset lastResumeCheckAtRef.current (e.g., set to 0 or null) when
you set hiddenAtRef.current and log the hide, so performResumeHealthCheck / the
next visibility "visible" will not be suppressed by the 1s dedupe window.
🪄 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: c07583c9-17b0-4c38-a0b0-c329d890f0d1
📒 Files selected for processing (1)
services/websocket/useWebSocketHealth.ts
|
ragnep
approved these changes
Apr 9, 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
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 by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores