Repository navigation
fix(wework): restore streaming content after refresh - #1309
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:
📝 WalkthroughWalkthroughThis PR restores cached streaming assistant content when opening a task after refresh, removes voice input UI from the compact composer, and adjusts sidebar spacing. The cached-content restoration dispatches a new ChangesCached content restoration
Voice input removal from composer UI
Desktop sidebar spacing adjustments
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
wework/src/features/workbench/messageReducer.ts (1)
929-937: ⚡ Quick winConsider checking for cached_content presence explicitly.
The current condition checks
joinResponse?.streaming, but doesn't verify thatcached_contentexists. Adding an explicit check would make the code more defensive and self-documenting:🛡️ Suggested defensive check
- if (joinResponse?.streaming) { + if (joinResponse?.streaming?.cached_content) { dispatchMessages({ type: 'assistant_cached', taskId,🤖 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 `@wework/src/features/workbench/messageReducer.ts` around lines 929 - 937, The condition currently checks joinResponse?.streaming but may access joinResponse.cached_content without verifying it exists; update the logic in the messageReducer (the code that handles joinResponse in the reducer function handling streaming updates) to require joinResponse?.streaming && !!joinResponse.cached_content (or a similar explicit null/undefined check) before using cached_content, ensuring you guard any subsequent access to joinResponse.cached_content and adjust any early-return or branch handling accordingly.wework/src/features/workbench/WorkbenchProvider.tsx (1)
930-937: ⚡ Quick winConsider validating nested streaming properties before dispatch.
The code checks
joinResponse?.streamingbut doesn't validate thatstreaming.subtask_idandstreaming.cached_contentare defined. If the streaming object exists but these nested properties are missing, the dispatch would passundefinedvalues to the reducer.Consider adding explicit checks:
if (joinResponse?.streaming?.subtask_id && joinResponse.streaming.cached_content !== undefined) { dispatchMessages({ type: 'assistant_cached', taskId, subtaskId: joinResponse.streaming.subtask_id, content: joinResponse.streaming.cached_content, }) }Note: Using
!== undefinedforcached_contentallows empty strings to pass through if that's intentional behavior.🤖 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 `@wework/src/features/workbench/WorkbenchProvider.tsx` around lines 930 - 937, The current guard only checks joinResponse?.streaming but can still pass undefined nested fields to dispatchMessages; update the condition around the dispatch that references joinResponse.streaming.subtask_id and joinResponse.streaming.cached_content to explicitly verify subtask_id is present and cached_content !== undefined (so empty strings are allowed), then only call dispatchMessages({ type: 'assistant_cached', taskId, subtaskId: joinResponse.streaming.subtask_id, content: joinResponse.streaming.cached_content }) when those checks pass to avoid sending undefined values to the reducer.
🤖 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.
Nitpick comments:
In `@wework/src/features/workbench/messageReducer.ts`:
- Around line 929-937: The condition currently checks joinResponse?.streaming
but may access joinResponse.cached_content without verifying it exists; update
the logic in the messageReducer (the code that handles joinResponse in the
reducer function handling streaming updates) to require joinResponse?.streaming
&& !!joinResponse.cached_content (or a similar explicit null/undefined check)
before using cached_content, ensuring you guard any subsequent access to
joinResponse.cached_content and adjust any early-return or branch handling
accordingly.
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 930-937: The current guard only checks joinResponse?.streaming but
can still pass undefined nested fields to dispatchMessages; update the condition
around the dispatch that references joinResponse.streaming.subtask_id and
joinResponse.streaming.cached_content to explicitly verify subtask_id is present
and cached_content !== undefined (so empty strings are allowed), then only call
dispatchMessages({ type: 'assistant_cached', taskId, subtaskId:
joinResponse.streaming.subtask_id, content:
joinResponse.streaming.cached_content }) when those checks pass to avoid sending
undefined values to the reducer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3432398-050a-4183-8af1-ab65c7edb17e
📒 Files selected for processing (3)
wework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/messageReducer.ts
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)
frontend/src/features/tasks/components/chat/ChatArea.tsx (1)
488-490:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAllow the URL-selected team path to run before the empty-list bailout.
Line 489 returns before the
teamIdbranch executes. When/chat?teamId=...resolves a team throughteamApis.getTeambutfilteredTeamsis still empty,fetchedTeamFromUrlis never applied and the chat falls back to no/default selection instead of the requested team.Suggested fix
useEffect(() => { - if (filteredTeams.length === 0) return - // Extract team ID from task detail const detailTeamId = selectedTaskDetail?.team ? typeof selectedTaskDetail.team === 'number' ? selectedTaskDetail.team : (selectedTaskDetail.team as Team).id @@ if (!taskIdFromUrl && parsedTeamIdFromUrl) { const teamFromUrl = filteredTeams.find(t => t.id === parsedTeamIdFromUrl) || teams.find(t => t.id === parsedTeamIdFromUrl) || (fetchedTeamFromUrl?.id === parsedTeamIdFromUrl ? fetchedTeamFromUrl : null) if (teamFromUrl) { handleTeamChange(teamFromUrl) hasInitializedTeamRef.current = true lastSyncedTaskIdRef.current = null return } } + + if (filteredTeams.length === 0) returnAlso applies to: 536-540
🤖 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 `@frontend/src/features/tasks/components/chat/ChatArea.tsx` around lines 488 - 490, The early return in the useEffect that checks filteredTeams (in the ChatArea useEffect) prevents the teamId URL branch from running; update the effect so the URL-selected team logic (the teamId handling that calls teamApis.getTeam and sets fetchedTeamFromUrl) executes before or regardless of the filteredTeams.length bail-out, then only return early after that logic if appropriate; specifically, move or reorder the teamId / fetchedTeamFromUrl handling to run prior to the filteredTeams.length === 0 check (or change the condition to allow the teamId branch to proceed) so fetchedTeamFromUrl is applied when a team is resolved from the URL.
🤖 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 `@frontend/src/features/tasks/components/chat/ChatArea.tsx`:
- Around line 488-490: The early return in the useEffect that checks
filteredTeams (in the ChatArea useEffect) prevents the teamId URL branch from
running; update the effect so the URL-selected team logic (the teamId handling
that calls teamApis.getTeam and sets fetchedTeamFromUrl) executes before or
regardless of the filteredTeams.length bail-out, then only return early after
that logic if appropriate; specifically, move or reorder the teamId /
fetchedTeamFromUrl handling to run prior to the filteredTeams.length === 0 check
(or change the condition to allow the teamId branch to proceed) so
fetchedTeamFromUrl is applied when a team is resolved from the URL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9683f8c1-4600-4c49-90fe-9c0f38b124ff
📒 Files selected for processing (3)
frontend/e2e/tests/tasks/chat-image-browser-e2e.spec.tsfrontend/src/apis/team.tsfrontend/src/features/tasks/components/chat/ChatArea.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
wework/src/components/chat/composer/CompactChatComposer.tsx (1)
68-76: 💤 Low valueRefactor: Redundant dependency in useEffect array.
The dependency array includes both
explicitLineCountandvalue, butexplicitLineCountis directly derived fromvalue(line 49). This means the effect will run whenever either changes, potentially executing twice per value update. Consider removingvaluefrom the dependencies sinceexplicitLineCountalready captures the relevant changes.♻️ Suggested optimization
useEffect(() => { const textarea = textareaRef.current if (!textarea) { setCanExpandInput(explicitLineCount > 4) return } setCanExpandInput(explicitLineCount > 4 || textarea.scrollHeight > 124) - }, [explicitLineCount, textareaRef, value]) + }, [explicitLineCount, textareaRef])🤖 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 `@wework/src/components/chat/composer/CompactChatComposer.tsx` around lines 68 - 76, The effect in useEffect (which reads textareaRef.current and calls setCanExpandInput) redundantly lists value in its dependency array even though explicitLineCount is derived from value; remove value from the dependency array so the effect depends only on explicitLineCount (and textareaRef if you need the ref itself) to avoid double runs per value update, keeping the logic in the effect unchanged and ensuring textareaRef remains a stable ref (use textareaRef.current inside the effect).
🤖 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.
Nitpick comments:
In `@wework/src/components/chat/composer/CompactChatComposer.tsx`:
- Around line 68-76: The effect in useEffect (which reads textareaRef.current
and calls setCanExpandInput) redundantly lists value in its dependency array
even though explicitLineCount is derived from value; remove value from the
dependency array so the effect depends only on explicitLineCount (and
textareaRef if you need the ref itself) to avoid double runs per value update,
keeping the logic in the effect unchanged and ensuring textareaRef remains a
stable ref (use textareaRef.current inside the effect).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52a9006c-82fc-4e17-a93d-c1a4c4704cb4
📒 Files selected for processing (5)
wework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/composer/CompactChatComposer.tsxwework/src/components/chat/composer/ComposerToolbar.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
💤 Files with no reviewable changes (2)
- wework/src/i18n/locales/zh-CN/common.json
- wework/src/i18n/locales/en/common.json
b78f327 to
2b715d2
Compare
2 participants
Summary by CodeRabbit
New Features
Improvements
Tests