Open
Conversation
Signed-off-by: Max Holland <max@livepeer.org>
📝 WalkthroughWalkthroughA new React hook Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StreamPage
participant Hook as useCloudWorkflowBackup
participant Storage as localStorage
participant Toast as Toast UI
Note over User,Toast: Connected State - Backup Flow
User->>StreamPage: Updates workflow state
StreamPage->>Hook: Workflow state changes
Hook->>Hook: Debounce (500ms window)
Hook->>Hook: Build ScopeWorkflow
Hook->>Storage: Write backup with timestamp & pipelineId
Note over User,Toast: Disconnect Flow
User->>StreamPage: Cloud connection lost
StreamPage->>Hook: isCloudConnected = false
Hook->>Storage: Mark disconnect flag
Note over User,Toast: Reconnect Flow - Restore Prompt
User->>StreamPage: Cloud reconnected
StreamPage->>Hook: isCloudConnected = true
Hook->>Storage: Check disconnect flag
Hook->>Storage: Read backup data
Hook->>Toast: Show restore prompt (30s duration)
Note over User,Toast: User Actions
alt User Clicks Restore
User->>Toast: Click restore
Toast->>Hook: Trigger onRestoreRequest
Hook->>StreamPage: Pass backed-up ScopeWorkflow
StreamPage->>StreamPage: Load workflow import dialog
Hook->>Storage: Clear backup & disconnect flag
else User Dismisses or Timeout
Toast-->>Hook: Toast closes
Hook->>Storage: Clear backup & disconnect flag
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Contributor
🚀 fal.ai Preview Deployment
TestingConnect to this preview deployment by running this on your branch: 🧪 E2E tests will run automatically against this deployment. |
Contributor
✅ E2E Tests passed
Test ArtifactsCheck the workflow run for screenshots. |
Signed-off-by: Max Holland <max@livepeer.org>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/pages/StreamPage.tsx (1)
1999-2014: InlinepromptStateobject causes effect to re-run on every render.The
promptStateobject is created inline, generating a new reference on each render. Since it's a dependency of the auto-save effect inuseCloudWorkflowBackup, this resets the debounce timer unnecessarily on every render, even when the actual values haven't changed.While the backup will still eventually fire (2 seconds after the last render), this is inefficient and could delay backups during active UI interaction.
♻️ Memoize promptState to avoid unnecessary effect re-runs
+ const promptState = useMemo( + () => ({ + promptItems, + interpolationMethod, + transitionSteps, + temporalInterpolationMethod, + }), + [promptItems, interpolationMethod, transitionSteps, temporalInterpolationMethod] + ); + useCloudWorkflowBackup({ settings, timelinePrompts, - promptState: { - promptItems, - interpolationMethod, - transitionSteps, - temporalInterpolationMethod, - }, + promptState, pipelineInfoMap: pipelines, loraFiles, plugins, scopeVersion: scopeVersion ?? "unknown", isCloudConnected, onRestoreRequest: handleCloudRestore, });You'll also need to add
useMemoto the imports at the top of the file:-import { useState, useEffect, useRef, useCallback } from "react"; +import { useState, useEffect, useRef, useCallback, useMemo } from "react";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/StreamPage.tsx` around lines 1999 - 2014, The inline promptState object passed into useCloudWorkflowBackup causes a new reference each render and retriggers the effect; fix this by importing useMemo and memoizing promptState (containing promptItems, interpolationMethod, transitionSteps, temporalInterpolationMethod) with useMemo so the object identity only changes when its actual values change, then pass the memoized promptState into useCloudWorkflowBackup.frontend/src/hooks/useCloudWorkflowBackup.ts (1)
168-189: Consider documenting the callback stability requirement.The
onRestoreRequestcallback is in the effect's dependency array. While the current integration inStreamPagecorrectly memoizes this callback withuseCallback, future consumers should be aware that passing an unstable callback could cause the effect to re-run unexpectedly during reconnection scenarios.Consider adding a JSDoc note to the
onRestoreRequestprop in the interface:interface UseCloudWorkflowBackupOptions { // ... - /** Opens the workflow import dialog with the backup workflow preloaded. */ + /** Opens the workflow import dialog with the backup workflow preloaded. + * Should be memoized (e.g., via useCallback) to avoid effect re-runs. */ onRestoreRequest: (workflow: ScopeWorkflow) => void; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/useCloudWorkflowBackup.ts` around lines 168 - 189, The effect in useCloudWorkflowBackup depends on onRestoreRequest and can re-run if callers pass an unstable function; update the API docs by adding a JSDoc on the onRestoreRequest prop (in the interface used by useCloudWorkflowBackup/its consumer, e.g., StreamPage) that states the callback must be stable (memoized with React.useCallback) to avoid unintended effect re-runs during reconnection and restore toast logic, and include a brief example/mention of useCallback for future integrators.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/hooks/useCloudWorkflowBackup.ts`:
- Around line 168-189: The effect in useCloudWorkflowBackup depends on
onRestoreRequest and can re-run if callers pass an unstable function; update the
API docs by adding a JSDoc on the onRestoreRequest prop (in the interface used
by useCloudWorkflowBackup/its consumer, e.g., StreamPage) that states the
callback must be stable (memoized with React.useCallback) to avoid unintended
effect re-runs during reconnection and restore toast logic, and include a brief
example/mention of useCallback for future integrators.
In `@frontend/src/pages/StreamPage.tsx`:
- Around line 1999-2014: The inline promptState object passed into
useCloudWorkflowBackup causes a new reference each render and retriggers the
effect; fix this by importing useMemo and memoizing promptState (containing
promptItems, interpolationMethod, transitionSteps, temporalInterpolationMethod)
with useMemo so the object identity only changes when its actual values change,
then pass the memoized promptState into useCloudWorkflowBackup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3a8b84dc-72fb-4382-97d8-fd168b8ce45b
📒 Files selected for processing (2)
frontend/src/hooks/useCloudWorkflowBackup.tsfrontend/src/pages/StreamPage.tsx
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 by CodeRabbit
You can test by manually disconnecting from cloud and reconnecting
