Add workflow reconnection logic for page reload scenarios #3672
Conversation
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReworks session workflow orchestration: adds sessionStorage helpers to track workflow-in-progress, introduces determineWorkflowAction to choose none/start/replay on load, extends useStream to expose replay and manage workflow flags across stream exit paths, and updates SessionDetailPageClient to invoke actions and unify error handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SessionPage as SessionDetailPageClient
participant Store as workflowStorage (sessionStorage)
participant Decide as determineWorkflowAction
participant Stream as useStream
User->>SessionPage: Load session (messages, designSessionId)
SessionPage->>Store: getWorkflowInProgress(designSessionId)
SessionPage->>Decide: determineWorkflowAction(messages, isInProgress, hasTriggered)
Decide-->>SessionPage: Action (none | replay | start(userInput))
alt Action == start
SessionPage->>Store: setWorkflowInProgress(designSessionId)
SessionPage->>Stream: start(designSessionId, userInput, isDeepModelingEnabled)
else Action == replay
SessionPage->>Store: setWorkflowInProgress(designSessionId)
SessionPage->>Stream: replay(designSessionId, isDeepModelingEnabled)
else Action == none
Note over SessionPage: No-op
end
rect rgb(242,248,255)
Note over Stream: Streaming lifecycle (isStreaming, messages, error)
Stream->>Store: clearWorkflowInProgress(designSessionId) on complete/abort
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Updates to Preview Branch (session-reconnect) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts (1)
59-63: Strengthen the non-string content testOnce the extraction logic is fixed, please cover the real multi-part case and assert the reconstructed string so this regression can’t slip back in.
- it('handles when content is not a string', () => { - const message = new HumanMessage({ content: 'test' }) - const result = determineWorkflowAction([message], false, false) - expect(result.type).toBe('start') - }) + it('extracts text when content is provided as message parts', () => { + const message = new HumanMessage({ + content: [{ type: 'text', text: 'hello' }], + }) + const result = determineWorkflowAction([message], false, false) + expect(result).toEqual({ + type: 'start', + userInput: 'hello', + }) + })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx(3 hunks)frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts(9 hunks)frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts(1 hunks)frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts(1 hunks)frontend/apps/app/components/SessionDetailPage/utils/workflowStorage.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.tsfrontend/apps/app/components/SessionDetailPage/utils/workflowStorage.tsfrontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}: Prefer early returns for readability
Use named exports only (no default exports)
Prefer const arrow functions over function declarations for simple utilities (e.g., const toggle = () => {})
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.tsfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/SessionDetailPage/utils/workflowStorage.tsfrontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts
frontend/apps/**
📄 CodeRabbit inference engine (AGENTS.md)
Next.js apps live under frontend/apps; target app-specific scripts and configs there
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.tsfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/SessionDetailPage/utils/workflowStorage.tsfrontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Follow existing import patterns and tsconfig path aliases
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.tsfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/SessionDetailPage/utils/workflowStorage.tsfrontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts
frontend/apps/**/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
frontend/apps/**/app/**/*.{ts,tsx}: Use Server Components for server-side data fetching
Do client-side data fetching only when necessary
Align data fetching responsibilities with component roles
Use Server Actions for all data mutations (create/update/delete)
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.tsfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/SessionDetailPage/utils/workflowStorage.tsfrontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase and use TSX (e.g., App.tsx)
Prefix event handler functions with “handle” (e.g., handleClick)
Files:
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write unit tests with filenames ending in .test.ts or .test.tsx colocated near source
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts
🧬 Code graph analysis (3)
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx (3)
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
useStream(66-300)frontend/apps/app/components/SessionDetailPage/utils/workflowStorage.ts (1)
getWorkflowInProgress(6-10)frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts (1)
determineWorkflowAction(17-45)
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
frontend/apps/app/components/SessionDetailPage/utils/workflowStorage.ts (2)
clearWorkflowInProgress(24-28)setWorkflowInProgress(15-19)
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts (1)
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts (1)
determineWorkflowAction(17-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: CodeQL
- GitHub Check: Supabase Preview
- GitHub Check: frontend-ci
- GitHub Check: frontend-lint
- GitHub Check: security-review
- GitHub Check: Supabase Preview
| const firstMessage = messages[0] | ||
| if (firstMessage && isHumanMessage(firstMessage)) { | ||
| return { | ||
| type: 'start', | ||
| userInput: String(firstMessage.content), | ||
| } |
There was a problem hiding this comment.
Don't coerce structured HumanMessage content
BaseMessage.content is often an array of message parts in LangChain; String(firstMessage.content) collapses that to "[object Object]", so the resumed workflow would receive gibberish instead of the actual user text. Please extract the text safely (e.g., join text parts and bail if nothing usable) before starting the workflow.
- if (firstMessage && isHumanMessage(firstMessage)) {
- return {
- type: 'start',
- userInput: String(firstMessage.content),
- }
- }
+ if (firstMessage && isHumanMessage(firstMessage)) {
+ const { content } = firstMessage
+ const userInput =
+ typeof content === 'string'
+ ? content
+ : Array.isArray(content)
+ ? content
+ .map((part) => {
+ if (typeof part === 'string') return part
+ if (part && typeof part === 'object' && 'text' in part) {
+ const maybeText = (part as { text?: unknown }).text
+ return typeof maybeText === 'string' ? maybeText : ''
+ }
+ return ''
+ })
+ .filter(Boolean)
+ .join('')
+ : ''
+
+ if (!userInput) {
+ return { type: 'none' }
+ }
+
+ return {
+ type: 'start',
+ userInput,
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const firstMessage = messages[0] | |
| if (firstMessage && isHumanMessage(firstMessage)) { | |
| return { | |
| type: 'start', | |
| userInput: String(firstMessage.content), | |
| } | |
| const firstMessage = messages[0] | |
| if (firstMessage && isHumanMessage(firstMessage)) { | |
| const { content } = firstMessage | |
| const userInput = | |
| typeof content === 'string' | |
| ? content | |
| : Array.isArray(content) | |
| ? content | |
| .map((part) => { | |
| if (typeof part === 'string') return part | |
| if (part && typeof part === 'object' && 'text' in part) { | |
| const maybeText = (part as { text?: unknown }).text | |
| return typeof maybeText === 'string' ? maybeText : '' | |
| } | |
| return '' | |
| }) | |
| .filter(Boolean) | |
| .join('') | |
| : '' | |
| if (!userInput) { | |
| return { type: 'none' } | |
| } | |
| return { | |
| type: 'start', | |
| userInput, | |
| } | |
| } |
🤖 Prompt for AI Agents
In
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts
around lines 34 to 39, the code coerces a structured HumanMessage content with
String(firstMessage.content) which collapses arrays/objects into "[object
Object]"; instead, detect and extract usable text from the message content
(handle string, array of parts, or objects with text fields), join text parts
into a single string, trim and validate non-empty result, and if nothing usable
bail (return undefined or fallback) rather than passing the malformed "[object
Object]" into the workflow start action.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}: Prefer early returns for readability
Use named exports only (no default exports)
Prefer const arrow functions over function declarations for simple utilities (e.g., const toggle = () => {})
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts
frontend/apps/**
📄 CodeRabbit inference engine (AGENTS.md)
Next.js apps live under frontend/apps; target app-specific scripts and configs there
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Follow existing import patterns and tsconfig path aliases
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts
frontend/apps/**/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
frontend/apps/**/app/**/*.{ts,tsx}: Use Server Components for server-side data fetching
Do client-side data fetching only when necessary
Align data fetching responsibilities with component roles
Use Server Actions for all data mutations (create/update/delete)
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Supabase Preview
- GitHub Check: Supabase Preview
- GitHub Check: frontend-ci
- GitHub Check: Supabase Preview
🔇 Additional comments (3)
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts (3)
1-2: LGTM!The imports are correct and appropriate for the workflow action determination logic.
4-7: LGTM!The WorkflowAction type is well-structured as a discriminated union, enabling safe type narrowing in consuming code.
22-30: LGTM!The priority checks for
hasTriggeredandisWorkflowInProgressare correctly ordered and use early returns for clarity.
| if (firstMessage && isHumanMessage(firstMessage)) { | ||
| return { | ||
| type: 'start', | ||
| userInput: firstMessage.text, | ||
| } | ||
| } |
There was a problem hiding this comment.
Validate non-empty userInput before starting workflow.
The code correctly uses firstMessage.text (the LangChain getter that safely extracts text), which addresses the past review concern about content coercion. However, if .text returns an empty string, the function returns { type: 'start', userInput: '' }, which may cause issues when attempting to start a workflow with no actual user input.
Apply this diff to validate non-empty userInput:
if (firstMessage && isHumanMessage(firstMessage)) {
+ const userInput = firstMessage.text.trim()
+ if (!userInput) {
+ return { type: 'none' }
+ }
return {
type: 'start',
- userInput: firstMessage.text,
+ userInput,
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (firstMessage && isHumanMessage(firstMessage)) { | |
| return { | |
| type: 'start', | |
| userInput: firstMessage.text, | |
| } | |
| } | |
| if (firstMessage && isHumanMessage(firstMessage)) { | |
| const userInput = firstMessage.text.trim() | |
| if (!userInput) { | |
| return { type: 'none' } | |
| } | |
| return { | |
| type: 'start', | |
| userInput, | |
| } | |
| } |
🤖 Prompt for AI Agents
In
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts
around lines 35 to 40, the code returns a start action using firstMessage.text
even when that text is an empty string; change the guard to only return the
start action when firstMessage exists, is a human message, and firstMessage.text
is non-empty (e.g., trim and length check), otherwise fall through (do not start
the workflow). Ensure you use the LangChain .text getter as before and only
create { type: 'start', userInput: firstMessage.text } when the validated
non-empty string check passes.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx (1)
151-157: Guard against empty/whitespace user input before startAvoid starting a workflow with blank input (e.g., accidental whitespace-only first message). Trim and short‑circuit.
- } else if (action.type === 'start') { - // Trigger the workflow for the initial user message - await start({ - designSessionId, - userInput: action.userInput, - isDeepModelingEnabled, - }) - } + } else if (action.type === 'start') { + // Trigger the workflow for the initial user message + const trimmed = action.userInput.trim() + if (!trimmed) return + await start({ + designSessionId, + userInput: trimmed, + isDeepModelingEnabled, + }) + }frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts (1)
46-58: Add a blank/whitespace-only HumanMessage caseEnsure we don’t start for empty/whitespace input. This both documents intended behavior and protects UX.
describe('Priority 3: Single unanswered user message', () => { it('returns start when there is a single HumanMessage', () => { @@ }) + it('returns none when HumanMessage is only whitespace', () => { + const result = determineWorkflowAction( + [new HumanMessage(' ')], + false, + false, + ) + expect(result).toEqual({ type: 'none' }) + })frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (2)
236-251: Replay: retry policy skips retries on network/unknown errorsRight now you return on first err; consider retrying non‑abort errors with a small backoff.
const replay = useCallback( async (params: ReplayParams): Promise<Result<void, StreamError>> => { for (let attempt = 1; attempt <= MAX_RETRIES; attempt += 1) { retryCountRef.current = attempt const result = await runStreamAttempt('/api/chat/replay', params) - if (result.isErr()) { - return err(result.error) - } + if (result.isErr()) { + // Abort is user-driven; bubble it immediately. + if (result.error.type === 'abort') { + return err(result.error) + } + // For transient errors (network/unknown), retry. + // Simple linear backoff. + await new Promise((r) => setTimeout(r, attempt * 500)) + continue + } if (result.value === 'complete') { return ok(undefined) } }
253-261: On terminal timeout, consider clearing the flag to avoid sticky replays on reloadAfter exhausting retries, the in‑progress flag remains set, causing auto‑replay on every reload. Clear it so the user can start fresh.
- const timeoutMessage = ERROR_MESSAGES.CONNECTION_TIMEOUT - abortWorkflow() + const timeoutMessage = ERROR_MESSAGES.CONNECTION_TIMEOUT + abortWorkflow() + // Prevent endless auto-replays on next load after terminal failure + clearWorkflowInProgress(params.designSessionId) setError(timeoutMessage) return err({ type: 'timeout', message: timeoutMessage, })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx(3 hunks)frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts(10 hunks)frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts(1 hunks)frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts(1 hunks)frontend/apps/app/components/SessionDetailPage/utils/workflowStorage.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/apps/app/components/SessionDetailPage/utils/workflowStorage.ts
- frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}: Use runtime type validation with valibot for external data validation
Prefer early returns for readability
Write simple, direct code without backward compatibility shims; update all call sites together
Use const-assigned arrow functions instead of function declarations for small utilities (e.g., const toggle = () => {})
Follow existing import patterns and tsconfig path aliases
Files:
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts
frontend/apps/**
📄 CodeRabbit inference engine (AGENTS.md)
Next.js apps live under frontend/apps; target app-specific scripts and configs there
Files:
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase and use TSX (e.g., App.tsx)
**/*.tsx: Prefix React event handler functions with "handle" (e.g., handleClick)
Import UI components from @liam-hq/ui when available
Import icons from @liam-hq/ui
Files:
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx
**/!(page).tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use named exports only (no default exports) for React/TSX modules
Files:
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write unit tests with filenames ending in .test.ts or .test.tsx colocated near source
Files:
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts
🧬 Code graph analysis (3)
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (3)
frontend/apps/app/components/SessionDetailPage/utils/workflowStorage.ts (2)
clearWorkflowInProgress(25-29)setWorkflowInProgress(16-20)frontend/apps/app/app/api/chat/replay/route.ts (1)
start(142-161)frontend/apps/app/app/api/chat/stream/route.ts (1)
start(130-149)
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx (3)
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
useStream(66-298)frontend/apps/app/components/SessionDetailPage/utils/workflowStorage.ts (1)
getWorkflowInProgress(6-11)frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts (1)
determineWorkflowAction(17-45)
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts (2)
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.ts (1)
determineWorkflowAction(17-45)frontend/apps/app/components/SessionDetailPage/components/Chat/components/Messages/HumanMessage/HumanMessage.tsx (1)
HumanMessage(18-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Supabase Preview
- GitHub Check: Supabase Preview
- GitHub Check: codeql / languages (javascript) / Perform CodeQL for javascript
- GitHub Check: Supabase Preview
🔇 Additional comments (2)
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx (1)
118-121: useStream replay integration looks goodDestructuring replay and wiring it through the effect aligns with the new flow.
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
84-96: Workflow flag semantics on complete vs. abort are correctClearing the flag only on complete and preserving it on abort matches the reconnection objective. Good fix.
| it('handles when content is not a string', () => { | ||
| const message = new HumanMessage({ content: 'test' }) | ||
| const result = determineWorkflowAction([message], false, false) | ||
| expect(result.type).toBe('start') | ||
| }) |
There was a problem hiding this comment.
Test name mismatch: actually still string content
This case claims “not a string” but still passes string content. Use a non‑string content shape to truly exercise the path.
- it('handles when content is not a string', () => {
- const message = new HumanMessage({ content: 'test' })
- const result = determineWorkflowAction([message], false, false)
- expect(result.type).toBe('start')
- })
+ it('handles when content is not a string', () => {
+ const message = new HumanMessage({
+ // non-string content (array of parts)
+ content: [{ type: 'text', text: 'hello from parts' }],
+ })
+ const result = determineWorkflowAction([message], false, false)
+ expect(result.type).toBe('start')
+ })🤖 Prompt for AI Agents
frontend/apps/app/components/SessionDetailPage/utils/determineWorkflowAction.test.ts
around lines 59 to 63: the test claims to check the "not a string" path but
constructs a HumanMessage with string content; replace the message content with
a non-string shape (for example an object like { foo: 'bar' } or an array) so
the function truly receives non-string content, then keep the same expectation
that result.type is 'start'.
- Add determineWorkflowAction: Pure function to determine start/replay/none - Add workflowStorage helpers: Manage workflow state in sessionStorage - Add comprehensive tests for workflow action determination Implements workflow reconnection detection logic for resolving page reload interruption issues during workflow execution. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Dynamic imports were unnecessary as these utilities are always needed. Static imports improve type safety and code clarity. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove unnecessary dynamic imports for workflowStorage helpers. Static imports are simpler and provide better type safety. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Replace 'unknown' payload type with explicit 'StartParams | ReplayParams'. Remove redundant sessionId parameter - use params.designSessionId instead. This makes the code more type-safe and eliminates parameter duplication. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
HumanMessage.text is already a string getter, no need for String() conversion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
With workflow reconnection feature, users can safely reload the page during streaming - the workflow will automatically resume from the last checkpoint. The confirmation dialog is no longer necessary. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Separate workflow completion from abortion to preserve reconnection ability. When a workflow is aborted or errors, keep the in-progress flag so users can reconnect on page reload. Only clear the flag when workflow completes successfully. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
MH4GF
force-pushed
the
session-reconnect
branch
from
8a35e14 to
991254b
Compare
This ensures the abort controller is properly cleaned up when navigating away during streaming. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
258-258: Consider clearing the workflow flag after MAX_RETRIES timeout.After
MAX_RETRIESfailures,abortWorkflow()leaves the workflow flag set. If the underlying issue persists (e.g., the stream never sends an END event), every page reload will attempt replay and timeout again, potentially creating an indefinite retry loop. If this becomes a concern in practice, consider callingclearWorkflowInProgress(params.designSessionId)after the timeout to prevent automatic replay on reload.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts(11 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}: Use runtime type validation with valibot for external data validation
Prefer early returns for readability
Write simple, direct code without backward compatibility shims; update all call sites together
Use const-assigned arrow functions instead of function declarations for small utilities (e.g., const toggle = () => {})
Follow existing import patterns and tsconfig path aliases
Files:
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts
frontend/apps/**
📄 CodeRabbit inference engine (AGENTS.md)
Next.js apps live under frontend/apps; target app-specific scripts and configs there
Files:
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts
🧬 Code graph analysis (1)
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
frontend/apps/app/components/SessionDetailPage/utils/workflowStorage.ts (2)
clearWorkflowInProgress(25-29)setWorkflowInProgress(16-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Supabase Preview
- GitHub Check: frontend-lint
- GitHub Check: frontend-ci
- GitHub Check: Supabase Preview
🔇 Additional comments (5)
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (5)
93-106: Clean separation of workflow completion and abort paths.The distinction between
completeWorkflow(clears flag on success) andabortWorkflow(preserves flag for reconnection) is clear and aligns with the PR's objective of enabling workflow resumption after page reloads.
112-117: Navigation guard correctly triggers abort without clearing workflow flag.Calling
abortWorkflow()instead of a confirm dialog ensures the workflow flag persists across page reloads, enabling the reconnection logic to detect and resume the interrupted workflow on the next page load.
172-239: Stream lifecycle management is correct and consistent.The orchestration of
abortWorkflow(for errors/aborts, preserving the flag) andcompleteWorkflow(for successful completion, clearing the flag) is sound. TheshouldRetrypath correctly avoids clearing state, allowing the retry loop to continue.
288-290: LGTM: Workflow flag set before streaming.Setting the flag before initiating the stream ensures the workflow is marked in progress even if the initial request fails immediately, allowing reconnection attempts on page reload.
312-319: API change:replayadded,stopremoved.The hook now exposes
replayto support the reconnection flow and no longer exposesstop, removing the ability for users to manually abort streaming via the hook interface. This aligns with the PR's focus on page-reload-driven reconnection, where the navigation guard handles abort scenarios.
MH4GF
requested review from
Copilot,
junkisai,
NoritakaIkeda and
sasamuku
and removed request for
a team
There was a problem hiding this comment.
Pull Request Overview
This PR adds workflow reconnection logic to handle page reload scenarios during workflow execution. When users reload the page, the streaming is interrupted and this change implements a mechanism to detect and resume interrupted workflows using sessionStorage.
- Workflow reconnection mechanism - Implements pure function logic to determine whether to start, replay, or skip workflow execution
- SessionStorage persistence - Adds helpers to track workflow state across page reloads with tab isolation
- Automatic workflow resumption - Integrates reconnection logic into existing components to handle interrupted workflows
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| workflowStorage.ts | New utility functions for managing workflow state in sessionStorage |
| determineWorkflowAction.ts | Pure function to determine workflow action based on state and message history |
| determineWorkflowAction.test.ts | Comprehensive unit tests for workflow action determination logic |
| useStream.ts | Enhanced streaming hook with workflow state management and replay functionality |
| SessionDetailPageClient.tsx | Updated client to use new workflow determination logic for automatic reconnection |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts
Show resolved
Hide resolved
Issue
Why is this change needed?
When users reload the page during workflow execution, the streaming is interrupted and doesn't automatically resume. This PR implements a reconnection mechanism to detect and resume interrupted workflows using sessionStorage.
replay.mov
What does this PR do?
New workflow reconnection logic
Pure function for action determination (
determineWorkflowAction)start,replay, or dononeSessionStorage helpers (
workflowStorage)liam:workflow:${designSessionId}'in_progress'string flagIntegration with existing components
SessionDetailPageClient: Uses determination logicuseStream: Exportsreplayfunction, manages flagsTechnical details
/api/chat/replayendpointTest results
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests