Repository navigation
feat(wework): implement project chat composer - #1177
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughImplements a project-scoped chat composer for the workbench by adding REST APIs for models/skills/devices, three custom hooks for state management, a new component hierarchy supporting model/skill/attachment selection, tool block display with lifecycle tracking, and wiring everything through the provider and layouts with project-context awareness. ChangesProject chat with models, skills, attachments, and tool blocks
Sequence Diagram(s)sequenceDiagram
participant UI as Composer UI
participant Provider as WorkbenchProvider
participant Hooks as Hooks (Models/Skills/Attach)
participant Stream as ChatStream
participant Reducer as messageReducer
UI->>Provider: onSubmit(text, models, skills, attachments)
Provider->>Hooks: selected model/skills/attachments from state
Provider->>Stream: sendMessage(payload with device_id, model_id, attachment_ids, additional_skills)
Stream-->>Provider: ack(success/failure, task_id?)
Provider->>Reducer: dispatch task_opened if new task
Provider-->>UI: reset attachments, sync state
Note over Stream: Tool blocks arrive via socket
Stream->>Reducer: dispatch block_created / block_updated
Reducer-->>UI: blocks appended/updated in message state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wework/src/features/workbench/WorkbenchProvider.tsx (1)
221-269:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle thrown send errors so composer state always recovers.
If
sendMessagerejects, this path skipssending_finished, so sending can remain stuck and users cannot retry cleanly.Suggested fix
- const ack = await resolvedServices.chatStream.sendMessage(payload) - dispatch({ type: 'sending_finished' }) - - if (!ack.success) { - dispatch({ type: 'error_set', error: ack.error ?? '发送失败' }) - return - } - - attachmentSelection.resetAttachments() - - if (!state.currentTask && ack.task_id) { - dispatch({ - type: 'task_opened', - task: { - id: ack.task_id, - title: message.substring(0, 100), - status: 'RUNNING', - task_type: 'code', - team_id: state.defaultTeam.id, - created_at: new Date().toISOString(), - }, - }) - } + try { + const ack = await resolvedServices.chatStream.sendMessage(payload) + + if (!ack.success) { + dispatch({ type: 'error_set', error: ack.error ?? '发送失败' }) + return + } + + attachmentSelection.resetAttachments() + + if (!state.currentTask && ack.task_id) { + dispatch({ + type: 'task_opened', + task: { + id: ack.task_id, + title: message.substring(0, 100), + status: 'RUNNING', + task_type: 'code', + team_id: state.defaultTeam.id, + created_at: new Date().toISOString(), + }, + }) + } + } catch (error) { + dispatch({ + type: 'error_set', + error: error instanceof Error ? error.message : '发送失败', + }) + } finally { + dispatch({ type: 'sending_finished' }) + }🤖 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 221 - 269, sendCurrentInput can throw when resolvedServices.chatStream.sendMessage rejects, leaving the UI stuck because dispatch({ type: 'sending_finished' }) is skipped; wrap the sendMessage call in a try/catch/finally inside sendCurrentInput (or convert to try { const ack = await ...; handle ack as now } catch (err) { dispatch({ type: 'error_set', error: err?.message ?? '发送失败' }); } finally { dispatch({ type: 'sending_finished' }); }) so that sending_finished always runs and errors are surfaced; keep the existing ack handling when sendMessage resolves successfully and ensure you reference sendCurrentInput, resolvedServices.chatStream.sendMessage, dispatch({ type: 'sending_finished' }), and dispatch({ type: 'error_set', ... }) in your change.
🧹 Nitpick comments (2)
wework/src/features/workbench/WorkbenchProvider.test.tsx (1)
43-60: ⚡ Quick winAdd
data-testidto probe buttons for selector consistency.These new interactive elements are currently text-selected only; adding test IDs keeps selectors stable and aligns with project UI test conventions.
As per coding guidelines, "Add
data-testidattributes to all interactive elements (buttons, inputs, links, selects, etc.) with naming format{action}-{element-type}".🤖 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.test.tsx` around lines 43 - 60, Update the test buttons to include data-testid attributes following the `{action}-{element-type}` convention so selectors are stable; specifically add data-testid values for the buttons that invoke workbench.selectProject (e.g., "select-project-button"), projectChat.setSelectedModel ("select-model-button"), projectChat.setSelectedSkills ("select-skill-button"), projectChat.addExistingAttachment ("add-attachment-button"), workbench.setInput ("set-input-button"), and workbench.sendCurrentInput ("send-button") so tests can reliably query these interactive elements.wework/src/components/chat/composer/ProjectWorkBar.tsx (1)
30-33: ⚡ Quick winAdd explicit menu semantics for better accessibility.
The dropdown trigger/menu currently misses
aria-haspopupandaria-controlslinkage, and the popup lacks arole. Adding these improves screen-reader interpretation.Proposed accessibility polish
- <div + <div + id="project-work-menu" + role="menu" data-testid="project-work-menu" className="absolute bottom-[52px] left-0 z-40 max-h-72 w-80 overflow-y-auto rounded-2xl border border-border bg-base p-2 shadow-[0_16px_44px_rgba(0,0,0,0.16)]" > @@ <button type="button" data-testid="project-work-button" onClick={() => setOpen(current => !current)} + aria-haspopup="menu" + aria-controls="project-work-menu" className="flex h-11 min-w-[44px] items-center gap-2 rounded-full px-1 text-sm font-medium text-text-secondary hover:bg-muted" aria-expanded={open}Also applies to: 61-67
🤖 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/ProjectWorkBar.tsx` around lines 30 - 33, The dropdown in ProjectWorkBar.tsx (the element with data-testid="project-work-menu") is missing explicit accessibility semantics; add aria-haspopup="menu" and aria-controls referencing the popup's id on the dropdown trigger element, and give the popup container a proper role="menu" (and an id that matches aria-controls). Apply the same changes to the other popup instance in the file (the block around lines 61-67) so both trigger elements have aria-haspopup/aria-controls and both popup containers use role="menu" with matching ids.
🤖 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 `@docs/en/user-guide/chat/project-work-chat.md`:
- Line 20: Replace the Chinese default message "请参考附件" in the project work chat
English guide with an English phrase such as "Please refer to the attachment(s)"
(you can optionally append the Chinese phrase in parentheses like "Please refer
to the attachment(s) (请参考附件)" if needed for bilingual clarity); update the
sentence that describes the system's default message when sending
attachments-only to use this English text and ensure the documentation's
example/default-message field reflects the new string.
In `@wework/src/api/adapters.test.ts`:
- Line 62: The test currently stubs vi.mocked(client.get) with an object {
items: [] } that doesn't match the adapter's return type; update the mock so
client.get resolves to a value shaped like the adapter expects (an array of
UnifiedSkill items) so listSkills() receives UnifiedSkill[]; specifically,
replace the { items: [] } stub with an array of objects matching the
UnifiedSkill shape (use realistic fields or a factory/fixture) when calling
vi.mocked(client.get).mockResolvedValue(...) to avoid hiding contract
regressions.
In `@wework/src/api/attachments.ts`:
- Around line 42-44: The uploadAttachment flow currently only checks
isValidFileSize; change uploadAttachment to validate the file extension first
using isSupportedExtension(file.name) (or the appropriate file name/path field)
and immediately reject with a clear Error if the extension is unsupported, then
proceed to size validation (isValidFileSize(file.size)) and reject with the
existing MAX_FILE_SIZE message if too large; reference the uploadAttachment
function and helper functions isSupportedExtension, isValidFileSize and the
constant MAX_FILE_SIZE when making this change.
In `@wework/src/components/chat/composer/AttachmentBadges.tsx`:
- Around line 60-67: The remove button in AttachmentBadges.tsx
(data-testid="remove-attachment-button", onRemoveAttachment handler) is
currently only 20×20 and must meet mobile minimums; update the button's
className to use the recommended sizing (e.g., replace "h-5 w-5" with "h-11
min-w-[44px]" and keep existing layout classes like "shrink-0 items-center
justify-center rounded-full hover:bg-muted") so the interactive hit area is at
least 44×44 while preserving the inner <X /> icon styling.
In `@wework/src/components/chat/composer/ComposerToolbar.tsx`:
- Around line 51-58: The voice button rendered in ComposerToolbar
(data-testid="voice-input-button", Mic icon) is interactive but has no behavior;
either wire an onClick handler that invokes the real voice input flow (or a
temporary stub like openVoiceModal/handleVoiceClick on the ComposerToolbar
component) or make the control non-interactive by adding disabled/aria-disabled
and visually hiding it (or removing it) until the feature is implemented; update
the button element in ComposerToolbar.tsx to reference the chosen handler name
or add the disabled attribute and appropriate aria label to prevent a dead
interaction.
- Line 2: Replace the external react-i18next import with the project hook import
by changing useTranslation import to import { useTranslation } from
'`@/hooks/useTranslation`' and update any useTranslation() calls in the
ComposerToolbar component to the project hook's usage; then fix the inert
control with data-testid="voice-input-button" by wiring it to an actual
interaction: add an onClick that invokes the component's existing voice toggle
handler or create a small toggleVoiceInput function in ComposerToolbar (and lift
to props if necessary), and ensure the button reflects disabled state (add
disabled={...} or aria-disabled and corresponding visual/logic guard) so it is
not a focusable inert element.
In `@wework/src/components/chat/composer/ModelSelector.tsx`:
- Line 3: The file imports useTranslation from 'react-i18next' but should use
the repository's internal translation hook; update the import in
ModelSelector.tsx to import useTranslation from the app's shared/internal i18n
hook module used across the codebase (replace the 'react-i18next' import with
the repo's equivalent), and keep the existing hook usage in the component
(search for useTranslation usage in ModelSelector to confirm no other changes
are needed).
In `@wework/src/components/chat/composer/ProjectWorkBar.tsx`:
- Line 3: Update the i18n import in ProjectWorkBar.tsx: replace the current
import of useTranslation from 'react-i18next' with the repo convention import
from '`@/hooks/useTranslation`' (locate the import at the top of the
ProjectWorkBar component). While there, ensure any menu toggle elements in
ProjectWorkBar that use aria-expanded also have proper accessible linkage (e.g.,
add role="button" or role="menu" as appropriate and connect aria-controls to the
menu container id) so the menu semantics are improved.
In `@wework/src/components/layout/DesktopWorkbenchLayout.test.tsx`:
- Around line 47-53: The test rendering of DesktopWorkbenchLayout is missing the
newly required control props projectChat and projectWork, causing downstream
components to receive undefined controls; update the test render of
DesktopWorkbenchLayout (the instance created with {...baseProps} and state
including currentProject) to pass valid projectChat and projectWork props (e.g.,
from baseProps.controls or mock control objects) into the component so the
layout receives the expected control interfaces instead of undefined.
In `@wework/src/features/workbench/useWorkbenchAttachments.ts`:
- Around line 106-113: The current removeAttachment function performs an
optimistic local remove via setState before awaiting deleteAttachment, causing
UI/server divergence on failure; change it to either await deleteAttachment
first and then call setState to remove the item, or keep the optimistic setState
but capture the previous attachments array and, if deleteAttachment throws,
restore state (rollback) and surface the error; update the removeAttachment
implementation (referencing removeAttachment, deleteAttachment, and setState) to
use one of these patterns and ensure errors are properly handled so local state
matches server state on failure.
- Around line 45-46: The code in useWorkbenchAttachments.ts uses fileId =
file.name which can collide; replace this with a collision-resistant per-file
key (e.g., combine file.name with file.size and file.lastModified or use
crypto.randomUUID()/a hash) so each uploaded file has a unique id; update every
place that uses fileId (progress/error maps, progress/state setters, and any
upload/remove handlers inside the useWorkbenchAttachments hook) to use the new
unique id so progress and error state do not overwrite between same-named files.
- Around line 83-91: In the success branch where setState updates attachments
and uploadingFiles, also clear any prior error for this fileId from the upload
error state so stale error badges are removed; specifically, inside the setState
updater (the function that receives current) remove the fileId from whatever
error map or field you use for upload errors (e.g., uploadErrors, errors, or
uploadErrorsMap) before returning the new state along with uploadingFiles and
attachments so the error entry is deleted on successful upload.
In `@wework/src/features/workbench/WorkbenchProvider.test.tsx`:
- Around line 259-271: Replace the loose negative objectContaining assertion
with direct property-absence checks on the actual sendMessage payload: after
ensuring sendMessage was called (within the existing waitFor), grab the first
call argument from sendMessage.mock.calls and assert payload does not have the
model_id property and does not have the additional_skills property (use
expect(payload).not.toHaveProperty for both) so each forbidden field is
explicitly checked rather than relying on objectContaining negation.
In `@wework/src/types/api.ts`:
- Line 105: Task.model_id is nullable but ChatSendPayload.model_id is not, which
breaks flows that need to explicitly send null; update the ChatSendPayload type
(and any related send payload interfaces such as ChatSendPayload or similar
symbol names in this file) to allow model_id to be string | null (or mark as
optional with type string | null) so it matches Task.model_id's nullability and
preserves explicit "clear override" behavior; ensure any serialization or
validation that consumes ChatSendPayload handles null values accordingly.
---
Outside diff comments:
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 221-269: sendCurrentInput can throw when
resolvedServices.chatStream.sendMessage rejects, leaving the UI stuck because
dispatch({ type: 'sending_finished' }) is skipped; wrap the sendMessage call in
a try/catch/finally inside sendCurrentInput (or convert to try { const ack =
await ...; handle ack as now } catch (err) { dispatch({ type: 'error_set',
error: err?.message ?? '发送失败' }); } finally { dispatch({ type:
'sending_finished' }); }) so that sending_finished always runs and errors are
surfaced; keep the existing ack handling when sendMessage resolves successfully
and ensure you reference sendCurrentInput,
resolvedServices.chatStream.sendMessage, dispatch({ type: 'sending_finished' }),
and dispatch({ type: 'error_set', ... }) in your change.
---
Nitpick comments:
In `@wework/src/components/chat/composer/ProjectWorkBar.tsx`:
- Around line 30-33: The dropdown in ProjectWorkBar.tsx (the element with
data-testid="project-work-menu") is missing explicit accessibility semantics;
add aria-haspopup="menu" and aria-controls referencing the popup's id on the
dropdown trigger element, and give the popup container a proper role="menu" (and
an id that matches aria-controls). Apply the same changes to the other popup
instance in the file (the block around lines 61-67) so both trigger elements
have aria-haspopup/aria-controls and both popup containers use role="menu" with
matching ids.
In `@wework/src/features/workbench/WorkbenchProvider.test.tsx`:
- Around line 43-60: Update the test buttons to include data-testid attributes
following the `{action}-{element-type}` convention so selectors are stable;
specifically add data-testid values for the buttons that invoke
workbench.selectProject (e.g., "select-project-button"),
projectChat.setSelectedModel ("select-model-button"),
projectChat.setSelectedSkills ("select-skill-button"),
projectChat.addExistingAttachment ("add-attachment-button"), workbench.setInput
("set-input-button"), and workbench.sendCurrentInput ("send-button") so tests
can reliably query these interactive elements.
🪄 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: 19d27dee-f4d8-48fd-96a2-c9781bb86688
📒 Files selected for processing (37)
docs/en/user-guide/chat/README.mddocs/en/user-guide/chat/project-work-chat.mddocs/zh/user-guide/chat/README.mddocs/zh/user-guide/chat/project-work-chat.mdfrontend/src/features/tasks/components/chat/useMessageSendQueue.tswework/src/api/adapters.test.tswework/src/api/attachments.tswework/src/api/models.tswework/src/api/skills.tswework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/ChatInput.tsxwework/src/components/chat/composer/AddContextMenu.tsxwework/src/components/chat/composer/AttachmentBadges.tsxwework/src/components/chat/composer/ComposerToolbar.tsxwework/src/components/chat/composer/DesktopChatComposer.tsxwework/src/components/chat/composer/DesktopComposerToolbar.tsxwework/src/components/chat/composer/DesktopContextMenu.tsxwework/src/components/chat/composer/DesktopModeSelector.tsxwework/src/components/chat/composer/DesktopModelSelector.tsxwework/src/components/chat/composer/ModelSelector.tsxwework/src/components/chat/composer/ProjectChatComposer.tsxwework/src/components/chat/composer/ProjectWorkBar.tsxwework/src/components/chat/composer/SkillSelector.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/useWorkbenchAttachments.tswework/src/features/workbench/useWorkbenchModels.tswework/src/features/workbench/useWorkbenchSkills.tswework/src/features/workbench/workbenchProjectChatHooks.test.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/pages/WorkbenchPage.tsxwework/src/types/api.ts
💤 Files with no reviewable changes (5)
- wework/src/components/chat/composer/DesktopModeSelector.tsx
- wework/src/components/chat/composer/DesktopModelSelector.tsx
- wework/src/components/chat/composer/DesktopContextMenu.tsx
- wework/src/components/chat/composer/DesktopComposerToolbar.tsx
- wework/src/components/chat/composer/DesktopChatComposer.tsx
|
|
||
| ## Send Project Messages | ||
|
|
||
| In project work chat, you can type a requirement or upload attachments and send without text. When sending attachments only, the system uses the default message **请参考附件** and submits the attachment IDs to the backend. |
Contributor
There was a problem hiding this comment.
Replace Chinese default message text in the English guide.
The English doc currently uses 请参考附件, which is likely confusing for English readers. Use an English default message (or add the Chinese phrase in parentheses only if needed).
Proposed edit
-In project work chat, you can type a requirement or upload attachments and send without text. When sending attachments only, the system uses the default message **请参考附件** and submits the attachment IDs to the backend.
+In project work chat, you can type a requirement or upload attachments and send without text. When sending attachments only, the system uses the default message **Please refer to the attachment** and submits the attachment IDs to the backend.📝 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.
Suggested change
| In project work chat, you can type a requirement or upload attachments and send without text. When sending attachments only, the system uses the default message **请参考附件** and submits the attachment IDs to the backend. | |
| In project work chat, you can type a requirement or upload attachments and send without text. When sending attachments only, the system uses the default message **Please refer to the attachment** and submits the attachment IDs to the backend. |
🤖 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 `@docs/en/user-guide/chat/project-work-chat.md` at line 20, Replace the Chinese
default message "请参考附件" in the project work chat English guide with an English
phrase such as "Please refer to the attachment(s)" (you can optionally append
the Chinese phrase in parentheses like "Please refer to the attachment(s)
(请参考附件)" if needed for bilingual clarity); update the sentence that describes
the system's default message when sending attachments-only to use this English
text and ensure the documentation's example/default-message field reflects the
new string.
|
|
||
| test('loads unified skills and team skills from existing backend endpoints', async () => { | ||
| const client = mockClient() | ||
| vi.mocked(client.get).mockResolvedValue({ items: [] }) |
Contributor
There was a problem hiding this comment.
Use response mocks that match adapter return types.
This test mocks client.get with { items: [] }, but listSkills() is typed to return UnifiedSkill[]. The current mock can hide contract regressions.
Proposed fix
- vi.mocked(client.get).mockResolvedValue({ items: [] })
+ vi.mocked(client.get)
+ .mockResolvedValueOnce([])
+ .mockResolvedValueOnce({ skills: [], preload_skills: [] })📝 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.
Suggested change
| vi.mocked(client.get).mockResolvedValue({ items: [] }) | |
| vi.mocked(client.get) | |
| .mockResolvedValueOnce([]) | |
| .mockResolvedValueOnce({ skills: [], preload_skills: [] }) |
🤖 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/api/adapters.test.ts` at line 62, The test currently stubs
vi.mocked(client.get) with an object { items: [] } that doesn't match the
adapter's return type; update the mock so client.get resolves to a value shaped
like the adapter expects (an array of UnifiedSkill items) so listSkills()
receives UnifiedSkill[]; specifically, replace the { items: [] } stub with an
array of objects matching the UnifiedSkill shape (use realistic fields or a
factory/fixture) when calling vi.mocked(client.get).mockResolvedValue(...) to
avoid hiding contract regressions.
Comment on lines
+42
to
+44
| if (!isValidFileSize(file.size)) { | ||
| return Promise.reject(new Error(`File size exceeds ${MAX_FILE_SIZE / (1024 * 1024)} MB`)) | ||
| } |
Contributor
There was a problem hiding this comment.
Apply extension validation before uploading.
uploadAttachment only enforces size; unsupported file types still upload despite isSupportedExtension existing. This defeats client-side validation and causes avoidable failed uploads.
Proposed fix
export function uploadAttachment(
file: File,
onProgress?: (progress: number) => void
): Promise<Attachment> {
+ if (!isSupportedExtension(file.name)) {
+ return Promise.reject(new Error('Unsupported file type'))
+ }
+
if (!isValidFileSize(file.size)) {
return Promise.reject(new Error(`File size exceeds ${MAX_FILE_SIZE / (1024 * 1024)} MB`))
}📝 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.
Suggested change
| if (!isValidFileSize(file.size)) { | |
| return Promise.reject(new Error(`File size exceeds ${MAX_FILE_SIZE / (1024 * 1024)} MB`)) | |
| } | |
| export function uploadAttachment( | |
| file: File, | |
| onProgress?: (progress: number) => void | |
| ): Promise<Attachment> { | |
| if (!isSupportedExtension(file.name)) { | |
| return Promise.reject(new Error('Unsupported file type')) | |
| } | |
| if (!isValidFileSize(file.size)) { | |
| return Promise.reject(new Error(`File size exceeds ${MAX_FILE_SIZE / (1024 * 1024)} MB`)) | |
| } |
🤖 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/api/attachments.ts` around lines 42 - 44, The uploadAttachment
flow currently only checks isValidFileSize; change uploadAttachment to validate
the file extension first using isSupportedExtension(file.name) (or the
appropriate file name/path field) and immediately reject with a clear Error if
the extension is unsupported, then proceed to size validation
(isValidFileSize(file.size)) and reject with the existing MAX_FILE_SIZE message
if too large; reference the uploadAttachment function and helper functions
isSupportedExtension, isValidFileSize and the constant MAX_FILE_SIZE when making
this change.
Comment on lines
+60
to
+67
| <button | ||
| type="button" | ||
| data-testid="remove-attachment-button" | ||
| onClick={() => onRemoveAttachment(attachment.id)} | ||
| className="flex h-5 w-5 shrink-0 items-center justify-center rounded-full hover:bg-muted" | ||
| aria-label="Remove attachment" | ||
| > | ||
| <X className="h-3.5 w-3.5" /> |
Contributor
There was a problem hiding this comment.
Increase remove button hit area to mobile minimum size.
Lines 64-67 render a 20×20 interactive target, below the required 44×44 minimum.
Suggested patch
- className="flex h-5 w-5 shrink-0 items-center justify-center rounded-full hover:bg-muted"
+ className="flex h-11 min-w-[44px] shrink-0 items-center justify-center rounded-full hover:bg-muted"📝 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.
Suggested change
| <button | |
| type="button" | |
| data-testid="remove-attachment-button" | |
| onClick={() => onRemoveAttachment(attachment.id)} | |
| className="flex h-5 w-5 shrink-0 items-center justify-center rounded-full hover:bg-muted" | |
| aria-label="Remove attachment" | |
| > | |
| <X className="h-3.5 w-3.5" /> | |
| <button | |
| type="button" | |
| data-testid="remove-attachment-button" | |
| onClick={() => onRemoveAttachment(attachment.id)} | |
| className="flex h-11 min-w-[44px] shrink-0 items-center justify-center rounded-full hover:bg-muted" | |
| aria-label="Remove attachment" | |
| > | |
| <X className="h-3.5 w-3.5" /> |
🤖 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/AttachmentBadges.tsx` around lines 60 -
67, The remove button in AttachmentBadges.tsx
(data-testid="remove-attachment-button", onRemoveAttachment handler) is
currently only 20×20 and must meet mobile minimums; update the button's
className to use the recommended sizing (e.g., replace "h-5 w-5" with "h-11
min-w-[44px]" and keep existing layout classes like "shrink-0 items-center
justify-center rounded-full hover:bg-muted") so the interactive hit area is at
least 44×44 while preserving the inner <X /> icon styling.
| @@ -0,0 +1,71 @@ | |||
| import { ArrowUp, Mic } from 'lucide-react' | |||
| import { useTranslation } from 'react-i18next' | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="wework/src/components/chat/composer/ComposerToolbar.tsx"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "=== Import section (top 40 lines) ==="
sed -n '1,80p' "$FILE" | nl -ba
echo
echo "=== Find voice button ==="
rg -n "voice" "$FILE" || true
echo
rg -n "data-testid" "$FILE" || true
echo
echo "=== Extract voice button JSX (best-effort) ==="
# Print a few lines around occurrences of voice-related testids/labels
python3 - <<'PY'
import re, itertools
path="wework/src/components/chat/composer/ComposerToolbar.tsx"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
hits=[]
for i,l in enumerate(lines, start=1):
if "voice" in l.lower() or "voice-input-button" in l:
hits.append(i)
# Expand window around first few hits
seen=set()
for i in hits[:20]:
for j in range(max(1,i-25), min(len(lines), i+25)+1):
if j not in seen:
seen.add(j)
# Sort and print with line numbers
for j in sorted(seen):
print(f"{j:4d} {lines[j-1].rstrip()}")
PY
echo
echo "=== Check translation hook usage ==="
rg -n "useTranslation\\(" "$FILE" || true
rg -n "react-i18next" "$FILE" || true
echo
echo "=== Check button interactivity: onClick/disabled for voice button ==="
python3 - <<'PY'
import re
path="wework/src/components/chat/composer/ComposerToolbar.tsx"
with open(path,'r',encoding='utf-8') as f:
s=f.read()
# Heuristic: find the element containing data-testid="voice-input-button"
m=re.search(r'data-testid\\s*=\\s*["\\\']voice-input-button["\\\']', s)
if not m:
print("No data-testid=\"voice-input-button\" found.")
raise SystemExit(0)
start=max(0, s.rfind("\n",0,m.start()-1))
end=s.find("\n", m.end()+1)
# take broader window
win_start=max(0, m.start()-800)
win_end=min(len(s), m.start()+1200)
chunk=s[win_start:win_end]
print(chunk)
PYRepository: wecode-ai/Wegent
Length of output: 160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="wework/src/components/chat/composer/ComposerToolbar.tsx"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "=== Top of file (first 80 lines) ==="
# cat -n provides line numbers without needing `nl`
cat -n "$FILE" | sed -n '1,80p'
echo
echo "=== Find voice-related occurrences ==="
rg -n "voice" "$FILE" || true
echo
echo "=== Find data-testid occurrences ==="
rg -n "data-testid" "$FILE" || true
echo
echo "=== Extract code around data-testid=\"voice-input-button\" ==="
python3 - <<'PY'
import re
path="wework/src/components/chat/composer/ComposerToolbar.tsx"
s=open(path,'r',encoding='utf-8').read()
m=re.search(r"data-testid\s*=\s*['\"]voice-input-button['\"]", s)
if not m:
print('No data-testid="voice-input-button" found.')
raise SystemExit(0)
start=max(0, s.rfind("\n",0,m.start())-1)
end=min(len(s), s.find("\n",m.end()+1)+1)
print("... around data-testid line ...")
# Print a larger window to include the element props
win_start=max(0, m.start()-600)
win_end=min(len(s), m.start()+900)
chunk=s[win_start:win_end]
lines=chunk.splitlines()
for i,l in enumerate(lines, start=1):
print(f"{win_start + sum(len(x)+1 for x in lines[:i-1]) :8d} {l.rstrip()}")
PY
echo
echo "=== Translation hook usage checks ==="
rg -n "useTranslation\\(" "$FILE" || true
rg -n "react-i18next" "$FILE" || trueRepository: wecode-ai/Wegent
Length of output: 4999
Switch ComposerToolbar to the project translation hook and fix the inert voice button
wework/src/components/chat/composer/ComposerToolbar.tsximportsuseTranslationfromreact-i18next; required is@/hooks/useTranslation.data-testid="voice-input-button"renders a clickable control but has noonClickand nodisabled, making it an inert UI element.
Proposed fix
-import { useTranslation } from 'react-i18next'
+import { useTranslation } from '`@/hooks/useTranslation`'📝 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.
Suggested change
| import { useTranslation } from 'react-i18next' | |
| import { useTranslation } from '`@/hooks/useTranslation`' |
🤖 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/ComposerToolbar.tsx` at line 2, Replace
the external react-i18next import with the project hook import by changing
useTranslation import to import { useTranslation } from '`@/hooks/useTranslation`'
and update any useTranslation() calls in the ComposerToolbar component to the
project hook's usage; then fix the inert control with
data-testid="voice-input-button" by wiring it to an actual interaction: add an
onClick that invokes the component's existing voice toggle handler or create a
small toggleVoiceInput function in ComposerToolbar (and lift to props if
necessary), and ensure the button reflects disabled state (add disabled={...} or
aria-disabled and corresponding visual/logic guard) so it is not a focusable
inert element.
Comment on lines
+45
to
+46
| const fileId = file.name | ||
|
|
Contributor
There was a problem hiding this comment.
Use a collision-resistant per-file key, not just file.name.
Line 45 can collide for same-named files, which causes progress/error state to overwrite across uploads.
Suggested patch
- for (const file of fileList) {
- const fileId = file.name
+ for (const [index, file] of fileList.entries()) {
+ const fileId = `${file.name}-${file.size}-${file.lastModified}-${index}`📝 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.
Suggested change
| const fileId = file.name | |
| for (const [index, file] of fileList.entries()) { | |
| const fileId = `${file.name}-${file.size}-${file.lastModified}-${index}` |
🤖 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/useWorkbenchAttachments.ts` around lines 45 -
46, The code in useWorkbenchAttachments.ts uses fileId = file.name which can
collide; replace this with a collision-resistant per-file key (e.g., combine
file.name with file.size and file.lastModified or use crypto.randomUUID()/a
hash) so each uploaded file has a unique id; update every place that uses fileId
(progress/error maps, progress/state setters, and any upload/remove handlers
inside the useWorkbenchAttachments hook) to use the new unique id so progress
and error state do not overwrite between same-named files.
Comment on lines
+83
to
+91
| setState(current => { | ||
| const uploadingFiles = new Map(current.uploadingFiles) | ||
| uploadingFiles.delete(fileId) | ||
| return { | ||
| ...current, | ||
| attachments: [...current.attachments, attachment], | ||
| uploadingFiles, | ||
| } | ||
| }) |
Contributor
There was a problem hiding this comment.
Clear stale error state on successful upload.
A prior error for the same file key is never removed in the success path, so error badges can linger after retry succeeds.
Suggested patch
setState(current => {
const uploadingFiles = new Map(current.uploadingFiles)
+ const errors = new Map(current.errors)
uploadingFiles.delete(fileId)
+ errors.delete(fileId)
return {
...current,
attachments: [...current.attachments, attachment],
uploadingFiles,
+ errors,
}
})📝 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.
Suggested change
| setState(current => { | |
| const uploadingFiles = new Map(current.uploadingFiles) | |
| uploadingFiles.delete(fileId) | |
| return { | |
| ...current, | |
| attachments: [...current.attachments, attachment], | |
| uploadingFiles, | |
| } | |
| }) | |
| setState(current => { | |
| const uploadingFiles = new Map(current.uploadingFiles) | |
| const errors = new Map(current.errors) | |
| uploadingFiles.delete(fileId) | |
| errors.delete(fileId) | |
| return { | |
| ...current, | |
| attachments: [...current.attachments, attachment], | |
| uploadingFiles, | |
| errors, | |
| } | |
| }) |
🤖 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/useWorkbenchAttachments.ts` around lines 83 -
91, In the success branch where setState updates attachments and uploadingFiles,
also clear any prior error for this fileId from the upload error state so stale
error badges are removed; specifically, inside the setState updater (the
function that receives current) remove the fileId from whatever error map or
field you use for upload errors (e.g., uploadErrors, errors, or uploadErrorsMap)
before returning the new state along with uploadingFiles and attachments so the
error entry is deleted on successful upload.
Comment on lines
+106
to
+113
| const removeAttachment = useCallback( | ||
| async (attachmentId: number) => { | ||
| setState(current => ({ | ||
| ...current, | ||
| attachments: current.attachments.filter(attachment => attachment.id !== attachmentId), | ||
| })) | ||
| await deleteAttachment(attachmentId) | ||
| }, |
Contributor
There was a problem hiding this comment.
Avoid local/server divergence when delete fails.
Line 108 removes locally before Line 112 confirms server deletion. If delete fails, UI state is wrong.
Suggested patch
const removeAttachment = useCallback(
async (attachmentId: number) => {
- setState(current => ({
- ...current,
- attachments: current.attachments.filter(attachment => attachment.id !== attachmentId),
- }))
await deleteAttachment(attachmentId)
+ setState(current => ({
+ ...current,
+ attachments: current.attachments.filter(attachment => attachment.id !== attachmentId),
+ }))
},
[deleteAttachment]
)🤖 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/useWorkbenchAttachments.ts` around lines 106 -
113, The current removeAttachment function performs an optimistic local remove
via setState before awaiting deleteAttachment, causing UI/server divergence on
failure; change it to either await deleteAttachment first and then call setState
to remove the item, or keep the optimistic setState but capture the previous
attachments array and, if deleteAttachment throws, restore state (rollback) and
surface the error; update the removeAttachment implementation (referencing
removeAttachment, deleteAttachment, and setState) to use one of these patterns
and ensure errors are properly handled so local state matches server state on
failure.
Comment on lines
+259
to
+271
| await waitFor(() => | ||
| expect(sendMessage).toHaveBeenCalledWith( | ||
| expect.not.objectContaining({ | ||
| model_id: 'gpt-5.5-medium', | ||
| additional_skills: [ | ||
| { | ||
| name: 'project-summary', | ||
| namespace: 'default', | ||
| is_public: false, | ||
| }, | ||
| ], | ||
| }) | ||
| ) |
Contributor
There was a problem hiding this comment.
Strengthen the “no overrides” assertion to avoid false passes.
expect.not.objectContaining({ model_id, additional_skills }) can still pass if only one forbidden field is present. Assert property absence directly on the sent payload.
Suggested fix
- await waitFor(() =>
- expect(sendMessage).toHaveBeenCalledWith(
- expect.not.objectContaining({
- model_id: 'gpt-5.5-medium',
- additional_skills: [
- {
- name: 'project-summary',
- namespace: 'default',
- is_public: false,
- },
- ],
- })
- )
- )
+ await waitFor(() => expect(sendMessage).toHaveBeenCalled())
+ const lastPayload = sendMessage.mock.calls.at(-1)?.[0]
+ expect(lastPayload).not.toHaveProperty('model_id')
+ expect(lastPayload).not.toHaveProperty('additional_skills')📝 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.
Suggested change
| await waitFor(() => | |
| expect(sendMessage).toHaveBeenCalledWith( | |
| expect.not.objectContaining({ | |
| model_id: 'gpt-5.5-medium', | |
| additional_skills: [ | |
| { | |
| name: 'project-summary', | |
| namespace: 'default', | |
| is_public: false, | |
| }, | |
| ], | |
| }) | |
| ) | |
| await waitFor(() => expect(sendMessage).toHaveBeenCalled()) | |
| const lastPayload = sendMessage.mock.calls.at(-1)?.[0] | |
| expect(lastPayload).not.toHaveProperty('model_id') | |
| expect(lastPayload).not.toHaveProperty('additional_skills') |
🤖 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.test.tsx` around lines 259 -
271, Replace the loose negative objectContaining assertion with direct
property-absence checks on the actual sendMessage payload: after ensuring
sendMessage was called (within the existing waitFor), grab the first call
argument from sendMessage.mock.calls and assert payload does not have the
model_id property and does not have the additional_skills property (use
expect(payload).not.toHaveProperty for both) so each forbidden field is
explicitly checked rather than relying on objectContaining negation.
| title?: string | ||
| task_type?: 'chat' | 'code' | 'task' | 'knowledge' | 'video' | 'image' | ||
| project_id?: number | ||
| model_id?: string |
Contributor
There was a problem hiding this comment.
Align model_id nullability across task and chat payload contracts.
Task.model_id allows null (Line 54), but ChatSendPayload.model_id does not. This can break “clear override” flows when callers need to send null explicitly.
Proposed fix
- model_id?: string
+ model_id?: string | null🤖 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/types/api.ts` at line 105, Task.model_id is nullable but
ChatSendPayload.model_id is not, which breaks flows that need to explicitly send
null; update the ChatSendPayload type (and any related send payload interfaces
such as ChatSendPayload or similar symbol names in this file) to allow model_id
to be string | null (or mark as optional with type string | null) so it matches
Task.model_id's nullability and preserves explicit "clear override" behavior;
ensure any serialization or validation that consumes ChatSendPayload handles
null values accordingly.
FicoHu
force-pushed
the
codex/feature/wework-project-chat
branch
from
May 27, 2026 16:56
133d6b0 to
5e6dffa
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
wework/src/components/chat/composer/ProjectWorkBar.tsx (1)
27-41: 💤 Low value
getDeviceForProjectrecreated each render, used insideuseMemo.
getDeviceForProjectis defined as a regular function inside the component (lines 27-31) but is called insidesortedProjectsuseMemo and also at line 66. While the current code works becausedevicesis in the useMemo deps, the helper is recreated every render. Consider moving it inside the useMemo or wrapping withuseCallback:Proposed fix - move inside useMemo
const sortedProjects = useMemo(() => { + const getDevice = (project: ProjectWithTasks): DeviceInfo | undefined => { + const deviceId = project.config?.execution?.deviceId + if (!deviceId) return undefined + return devices.find(d => d.device_id === deviceId) + } return [...projects].sort((a, b) => { - const deviceA = getDeviceForProject(a) - const deviceB = getDeviceForProject(b) + const deviceA = getDevice(a) + const deviceB = getDevice(b) const onlineA = deviceA?.status === 'online' ? 1 : 0 const onlineB = deviceB?.status === 'online' ? 1 : 0 return onlineB - onlineA }) }, [projects, devices]) + + const getDeviceForProject = useCallback((project: ProjectWithTasks): DeviceInfo | undefined => { + const deviceId = project.config?.execution?.deviceId + if (!deviceId) return undefined + return devices.find(d => d.device_id === deviceId) + }, [devices])🤖 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/ProjectWorkBar.tsx` around lines 27 - 41, The helper getDeviceForProject is recreated each render but is used inside the sortedProjects useMemo (and elsewhere), causing unnecessary re-creation; either move the getDeviceForProject implementation into the useMemo closure that computes sortedProjects so it is created only when projects/devices change, or wrap getDeviceForProject with useCallback (with devices in its dependency array) and keep using it both inside sortedProjects and at line 66; update references to the same symbol getDeviceForProject (or the new inline helper inside sortedProjects) so the logic and dependencies remain correct.wework/src/components/chat/MessageList.tsx (1)
108-113: 💤 Low valueRedundant streaming indicator when content exists.
Lines 108-110 show "正在思考" when thinking (streaming without content). Lines 111-113 also show "正在思考" when streaming with content. This means users see the same thinking indicator even after content starts appearing, which may be confusing UX. Consider removing the second indicator or changing its text to indicate content is still streaming.
{isThinking && ( <span className="text-[`#999`]">正在思考</span> )} - {isStreaming && hasContent && ( - <span className="text-[`#999`]">正在思考</span> - )}🤖 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/MessageList.tsx` around lines 108 - 113, In MessageList.tsx the UI shows the same "正在思考" message for both isThinking and the isStreaming && hasContent case, causing a confusing redundant indicator; update the render logic around the isThinking, isStreaming, and hasContent checks (the two span blocks) so that once hasContent is true you either remove the second indicator or change its text to something like "正在接收内容" (or similar) to reflect streaming-in-progress; modify the conditional that renders the second span (isStreaming && hasContent) accordingly and ensure only the appropriate message displays based on isThinking, isStreaming, and hasContent.wework/src/stream/chatStream.ts (1)
38-52: 💤 Low valueHardcoded Chinese error strings should use i18n.
The error messages at lines 40 and 45 are hardcoded in Chinese. For consistency and maintainability, consider using translation keys.
- if (!socket.connected) { - resolve({ success: false, error: '连接未建立，请刷新页面重试' }) + if (!socket.connected) { + resolve({ success: false, error: 'Connection not established, please refresh' })🤖 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/stream/chatStream.ts` around lines 38 - 52, The Promise in chatStream.ts returns hardcoded Chinese messages ("连接未建立，请刷新页面重试" and "发送超时，请重试"); replace these with i18n lookups so messages use translation keys instead of literals. Import or access the project's i18n/translate function and return e.g. i18n.t('errors.connection_not_established') when socket.connected is false and i18n.t('errors.send_timeout') for the SEND_TIMEOUT_MS timeout; keep the rest of the flow (socket.emit 'chat:send', clearTimeout, resolve(response)) unchanged and ensure the returned objects retain the same shape ({ success: false, error: <translated string> }).wework/src/features/workbench/messageReducer.ts (1)
84-87: 💤 Low valueUnsafe type cast in
normalizeBlockStatus.Line 86 casts arbitrary strings to
ToolBlockStatuswithout validation. If the server sends an unexpected status value, it will pass through unchecked. Consider validating against known values:Proposed fix
export function normalizeBlockStatus(status?: string): ToolBlockStatus { if (status === 'running') return 'pending' - return (status as ToolBlockStatus) ?? 'pending' + const validStatuses: ToolBlockStatus[] = ['pending', 'streaming', 'done', 'error'] + return validStatuses.includes(status as ToolBlockStatus) ? (status as ToolBlockStatus) : 'pending' }🤖 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 84 - 87, The function normalizeBlockStatus currently unsafely casts arbitrary strings to ToolBlockStatus; replace the cast with an explicit validation against the allowed ToolBlockStatus values (e.g., create a Set or array of known statuses and check status against it), map 'running' to 'pending' as before, and return 'pending' as the fallback for any unknown or undefined input; update normalizeBlockStatus to use that whitelist/type-guard approach instead of (status as ToolBlockStatus).
🤖 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 `@wework/src/api/teams.ts`:
- Around line 15-16: listTeams() currently only fetches page=1&limit=100 and can
miss teams; change it to page through all pages from the teams API by repeatedly
calling client.get<TeamListResponse> for increasing page numbers (starting at 1)
and concatenating response.items until a page returns fewer than the requested
limit or an empty items array. In other words update the logic around the
client.get<TeamListResponse> call in listTeams() to loop (or use a while)
collecting items into a single array (e.g., allItems) and return allItems
instead of returning response.items from a single request; use the existing
TeamListResponse shape and the response.items field to detect completion.
In `@wework/src/components/chat/blocks/ToolBlockItem.tsx`:
- Around line 16-20: Add data-testid attributes and mobile touch-target sizing
to both interactive buttons in the ToolBlockItem component: for the expand
toggle button (the element using onClick={() => setExpanded(!expanded)}) add
data-testid="toggle-button" and include the classes h-11 min-w-[44px] alongside
the existing className; do the same for the other interactive button referenced
near lines 141-142 (add a data-testid like "action-button" or a more specific
action-based name following the {action}-{element-type} pattern) and ensure it
also includes h-11 min-w-[44px] while preserving its other classes and behavior.
- Around line 49-65: Replace hard-coded Chinese status labels in ToolBlockItem
with i18n keys: import useTranslation from '`@/hooks/useTranslation`' with the
single feature namespace, call the hook (e.g., const { t } =
useTranslation('feature')) and replace labels like `已运行 ${shortCmd}`, `已新增
${fileName}`, `已编辑 ${fileName}`, `已读取 ${fileName}` with t('tool.run', { cmd:
shortCmd }), t('tool.create_file', { name: fileName }), t('tool.edit_file', {
name: fileName }), t('tool.read_file', { name: fileName }) respectively; update
the same pattern for other occurrences mentioned (around the other blocks where
ToolBlockItem builds labels), keeping existing icons/variables (TerminalIcon,
FileIcon, EditIcon, getInputField) unchanged and add the corresponding keys to
the feature namespace resource files.
In `@wework/src/components/chat/blocks/ToolBlocksDisplay.tsx`:
- Line 26: Replace the hardcoded Chinese UI strings in ToolBlocksDisplay.tsx
(the ternary expression using isRunning and the `已处理` label that may include
duration) with translated keys from the component's feature namespace by
importing useTranslation from '`@/hooks/useTranslation`'; call the hook (e.g.,
const t = useTranslation('featureNamespace')) and replace '处理中' and '已处理' with
t('running') and t('processed', { duration }) or equivalent keys, ensuring the
duration interpolation is handled via the translation function rather than
string concatenation.
- Around line 20-24: The toggle button in ToolBlocksDisplay is missing a
data-testid and mobile touch-target sizing; update the button (the element using
onClick={() => setExpanded(!expanded)} and reading expanded) to include a
data-testid following the {action}-{element-type} convention (e.g.,
data-testid="toggle-button") and add the mobile sizing classes h-11 and
min-w-[44px] to its className (keeping existing classes like mb-2 flex
items-center gap-1 text-xs ...).
In `@wework/src/components/chat/MessageList.tsx`:
- Around line 136-150: The copy button in MessageList.tsx (the button using
onClick={handleCopy} and the copied state) lacks a data-testid and has an
undersized touch target (uses p-0.5); add a data-testid following the
{action}-{element-type} pattern (e.g., "copy-button") to the button and increase
its interactive area to meet the 44×44px mobile requirement by replacing the
tiny padding class with size classes or explicit min-height/min-width (for
example use padding/size utility classes that result in ≥44px hit area or add
min-h-[44px] min-w-[44px]) while preserving the existing icon visuals and copied
state handling.
In `@wework/src/features/workbench/useWorkbenchModels.ts`:
- Around line 36-38: When filtered is empty we must clear any stale selection
instead of leaving selectedModel set; update the logic around filtered and
setSelectedModelState in useWorkbenchModels.ts so that if filtered.length > 0
you call setSelectedModelState(filtered[0]) and otherwise call
setSelectedModelState(null) (or undefined if your selectedModel type uses
undefined) to explicitly clear the selection; ensure the selected model state
type (selectedModel / setSelectedModelState) accepts null/undefined and adjust
its type if necessary.
---
Nitpick comments:
In `@wework/src/components/chat/composer/ProjectWorkBar.tsx`:
- Around line 27-41: The helper getDeviceForProject is recreated each render but
is used inside the sortedProjects useMemo (and elsewhere), causing unnecessary
re-creation; either move the getDeviceForProject implementation into the useMemo
closure that computes sortedProjects so it is created only when projects/devices
change, or wrap getDeviceForProject with useCallback (with devices in its
dependency array) and keep using it both inside sortedProjects and at line 66;
update references to the same symbol getDeviceForProject (or the new inline
helper inside sortedProjects) so the logic and dependencies remain correct.
In `@wework/src/components/chat/MessageList.tsx`:
- Around line 108-113: In MessageList.tsx the UI shows the same "正在思考" message
for both isThinking and the isStreaming && hasContent case, causing a confusing
redundant indicator; update the render logic around the isThinking, isStreaming,
and hasContent checks (the two span blocks) so that once hasContent is true you
either remove the second indicator or change its text to something like "正在接收内容"
(or similar) to reflect streaming-in-progress; modify the conditional that
renders the second span (isStreaming && hasContent) accordingly and ensure only
the appropriate message displays based on isThinking, isStreaming, and
hasContent.
In `@wework/src/features/workbench/messageReducer.ts`:
- Around line 84-87: The function normalizeBlockStatus currently unsafely casts
arbitrary strings to ToolBlockStatus; replace the cast with an explicit
validation against the allowed ToolBlockStatus values (e.g., create a Set or
array of known statuses and check status against it), map 'running' to 'pending'
as before, and return 'pending' as the fallback for any unknown or undefined
input; update normalizeBlockStatus to use that whitelist/type-guard approach
instead of (status as ToolBlockStatus).
In `@wework/src/stream/chatStream.ts`:
- Around line 38-52: The Promise in chatStream.ts returns hardcoded Chinese
messages ("连接未建立，请刷新页面重试" and "发送超时，请重试"); replace these with i18n lookups so
messages use translation keys instead of literals. Import or access the
project's i18n/translate function and return e.g.
i18n.t('errors.connection_not_established') when socket.connected is false and
i18n.t('errors.send_timeout') for the SEND_TIMEOUT_MS timeout; keep the rest of
the flow (socket.emit 'chat:send', clearTimeout, resolve(response)) unchanged
and ensure the returned objects retain the same shape ({ success: false, error:
<translated string> }).
🪄 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: 289935e3-a21a-415e-b226-534a1e66d2f5
⛔ Files ignored due to path filters (1)
wework/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
README.mdREADME_zh.mdwework/package.jsonwework/src/api/devices.tswework/src/api/teams.tswework/src/components/chat/ChatInput.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/blocks/ToolBlockItem.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.tsxwework/src/components/chat/composer/AttachmentBadges.tsxwework/src/components/chat/composer/ComposerToolbar.tsxwework/src/components/chat/composer/ModelSelector.tsxwework/src/components/chat/composer/ProjectChatComposer.tsxwework/src/components/chat/composer/ProjectWorkBar.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/messageReducer.tswework/src/features/workbench/useWorkbenchModels.tswework/src/features/workbench/workbenchReducer.tswework/src/i18n/locales/zh-CN/common.jsonwework/src/pages/WorkbenchPage.tsxwework/src/stream/chatStream.tswework/src/stream/socketClient.tswework/src/types/api.tswework/src/types/workbench.ts
✅ Files skipped from review due to trivial changes (5)
- wework/package.json
- wework/src/stream/socketClient.ts
- wework/src/i18n/locales/zh-CN/common.json
- README.md
- README_zh.md
🚧 Files skipped from review as they are similar to previous changes (7)
- wework/src/components/chat/composer/ComposerToolbar.tsx
- wework/src/components/chat/ChatInput.tsx
- wework/src/components/chat/composer/ModelSelector.tsx
- wework/src/features/workbench/WorkbenchProvider.tsx
- wework/src/features/workbench/WorkbenchProvider.test.tsx
- wework/src/components/chat/composer/ProjectChatComposer.tsx
- wework/src/components/chat/composer/AttachmentBadges.tsx
Comment on lines
+15
to
+16
| const response = await client.get<TeamListResponse>('/teams?page=1&limit=100') | ||
| return response.items |
Contributor
There was a problem hiding this comment.
listTeams() currently truncates results to the first page.
Fetching only page=1&limit=100 can miss active/default teams beyond the first page, which can break default-team selection.
Suggested fix
async function listTeams(): Promise<Team[]> {
- const response = await client.get<TeamListResponse>('/teams?page=1&limit=100')
- return response.items
+ const limit = 100
+ let page = 1
+ let total = 0
+ const items: Team[] = []
+
+ do {
+ const response = await client.get<TeamListResponse>(`/teams?page=${page}&limit=${limit}`)
+ items.push(...response.items)
+ total = response.total
+ page += 1
+ } while (items.length < total)
+
+ return items
}📝 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.
Suggested change
| const response = await client.get<TeamListResponse>('/teams?page=1&limit=100') | |
| return response.items | |
| async function listTeams(): Promise<Team[]> { | |
| const limit = 100 | |
| let page = 1 | |
| let total = 0 | |
| const items: Team[] = [] | |
| do { | |
| const response = await client.get<TeamListResponse>(`/teams?page=${page}&limit=${limit}`) | |
| items.push(...response.items) | |
| total = response.total | |
| page += 1 | |
| } while (items.length < total) | |
| return items | |
| } |
🤖 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/api/teams.ts` around lines 15 - 16, listTeams() currently only
fetches page=1&limit=100 and can miss teams; change it to page through all pages
from the teams API by repeatedly calling client.get<TeamListResponse> for
increasing page numbers (starting at 1) and concatenating response.items until a
page returns fewer than the requested limit or an empty items array. In other
words update the logic around the client.get<TeamListResponse> call in
listTeams() to loop (or use a while) collecting items into a single array (e.g.,
allItems) and return allItems instead of returning response.items from a single
request; use the existing TeamListResponse shape and the response.items field to
detect completion.
Comment on lines
+16
to
+20
| <button | ||
| type="button" | ||
| onClick={() => setExpanded(!expanded)} | ||
| className="flex items-center gap-1.5 text-[#666] hover:text-[#1a1a1a]" | ||
| > |
Contributor
There was a problem hiding this comment.
Add required data-testid and mobile touch-target sizing for both buttons.
Both interactive buttons need compliant test ids and 44x44 touch targets.
As per coding guidelines, "Add data-testid attributes to all interactive elements (buttons, inputs, links, selects, etc.) with naming format {action}-{element-type}" and "All interactive elements on mobile must be at least 44px × 44px - use h-11 min-w-[44px] for buttons".
Also applies to: 141-142
🤖 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/blocks/ToolBlockItem.tsx` around lines 16 - 20,
Add data-testid attributes and mobile touch-target sizing to both interactive
buttons in the ToolBlockItem component: for the expand toggle button (the
element using onClick={() => setExpanded(!expanded)}) add
data-testid="toggle-button" and include the classes h-11 min-w-[44px] alongside
the existing className; do the same for the other interactive button referenced
near lines 141-142 (add a data-testid like "action-button" or a more specific
action-based name following the {action}-{element-type} pattern) and ensure it
also includes h-11 min-w-[44px] while preserving its other classes and behavior.
Comment on lines
+49
to
+65
| return { icon: <TerminalIcon />, label: `已运行 ${shortCmd}` } | ||
| } | ||
| if (name === 'write' || name === 'create_file' || name === 'write_file') { | ||
| const filePath = getInputField(block, 'file_path', 'path') | ||
| const fileName = filePath ? filePath.split('/').pop() : '文件' | ||
| return { icon: <FileIcon />, label: `已新增 ${fileName}` } | ||
| } | ||
| if (name === 'edit' || name === 'str_replace_editor' || name === 'edit_file') { | ||
| const filePath = getInputField(block, 'file_path', 'path') | ||
| const fileName = filePath ? filePath.split('/').pop() : '文件' | ||
| return { icon: <EditIcon />, label: `已编辑 ${fileName}` } | ||
| } | ||
| if (name === 'read' || name === 'read_file') { | ||
| const filePath = getInputField(block, 'file_path', 'path') | ||
| const fileName = filePath ? filePath.split('/').pop() : '文件' | ||
| return { icon: <FileIcon />, label: `已读取 ${fileName}` } | ||
| } |
Contributor
There was a problem hiding this comment.
Move tool/status display strings to i18n keys.
These user-facing literals should be read via @/hooks/useTranslation in the feature namespace.
As per coding guidelines, "Always import translations from @/hooks/useTranslation, not from react-i18next, and use single namespace matching your feature".
Also applies to: 140-140, 170-174
🤖 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/blocks/ToolBlockItem.tsx` around lines 49 - 65,
Replace hard-coded Chinese status labels in ToolBlockItem with i18n keys: import
useTranslation from '`@/hooks/useTranslation`' with the single feature namespace,
call the hook (e.g., const { t } = useTranslation('feature')) and replace labels
like `已运行 ${shortCmd}`, `已新增 ${fileName}`, `已编辑 ${fileName}`, `已读取 ${fileName}`
with t('tool.run', { cmd: shortCmd }), t('tool.create_file', { name: fileName
}), t('tool.edit_file', { name: fileName }), t('tool.read_file', { name:
fileName }) respectively; update the same pattern for other occurrences
mentioned (around the other blocks where ToolBlockItem builds labels), keeping
existing icons/variables (TerminalIcon, FileIcon, EditIcon, getInputField)
unchanged and add the corresponding keys to the feature namespace resource
files.
Comment on lines
+20
to
+24
| <button | ||
| type="button" | ||
| className="mb-2 flex items-center gap-1 text-xs text-[#999] hover:text-[#666]" | ||
| onClick={() => setExpanded(!expanded)} | ||
| > |
Contributor
There was a problem hiding this comment.
Add required test id and mobile touch-target sizing to the toggle button.
This button is missing data-testid and the required mobile minimum size classes.
Suggested fix
<button
type="button"
- className="mb-2 flex items-center gap-1 text-xs text-[`#999`] hover:text-[`#666`]"
+ data-testid="toggle-tool-blocks-button"
+ className="mb-2 flex h-11 min-w-[44px] items-center gap-1 text-xs text-[`#999`] hover:text-[`#666`]"
onClick={() => setExpanded(!expanded)}
>As per coding guidelines, "Add data-testid attributes to all interactive elements (buttons, inputs, links, selects, etc.) with naming format {action}-{element-type}" and "All interactive elements on mobile must be at least 44px × 44px - use h-11 min-w-[44px] for buttons".
📝 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.
Suggested change
| <button | |
| type="button" | |
| className="mb-2 flex items-center gap-1 text-xs text-[#999] hover:text-[#666]" | |
| onClick={() => setExpanded(!expanded)} | |
| > | |
| <button | |
| type="button" | |
| data-testid="toggle-tool-blocks-button" | |
| className="mb-2 flex h-11 min-w-[44px] items-center gap-1 text-xs text-[`#999`] hover:text-[`#666`]" | |
| onClick={() => setExpanded(!expanded)} | |
| > |
🤖 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/blocks/ToolBlocksDisplay.tsx` around lines 20 -
24, The toggle button in ToolBlocksDisplay is missing a data-testid and mobile
touch-target sizing; update the button (the element using onClick={() =>
setExpanded(!expanded)} and reading expanded) to include a data-testid following
the {action}-{element-type} convention (e.g., data-testid="toggle-button") and
add the mobile sizing classes h-11 and min-w-[44px] to its className (keeping
existing classes like mb-2 flex items-center gap-1 text-xs ...).
| onClick={() => setExpanded(!expanded)} | ||
| > | ||
| <span> | ||
| {isRunning ? '处理中' : `已处理${duration ? ` ${duration}` : ''}`} |
Contributor
There was a problem hiding this comment.
Replace hardcoded UI text with translated keys.
These labels should come from the feature namespace via useTranslation, not hardcoded literals.
As per coding guidelines, "Always import translations from @/hooks/useTranslation, not from react-i18next, and use single namespace matching your feature".
🤖 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/blocks/ToolBlocksDisplay.tsx` at line 26, Replace
the hardcoded Chinese UI strings in ToolBlocksDisplay.tsx (the ternary
expression using isRunning and the `已处理` label that may include duration) with
translated keys from the component's feature namespace by importing
useTranslation from '`@/hooks/useTranslation`'; call the hook (e.g., const t =
useTranslation('featureNamespace')) and replace '处理中' and '已处理' with
t('running') and t('processed', { duration }) or equivalent keys, ensuring the
duration interpolation is handled via the translation function rather than
string concatenation.
Comment on lines
+136
to
+150
| <button | ||
| type="button" | ||
| onClick={handleCopy} | ||
| className="p-0.5 text-[#999] hover:text-[#666]" | ||
| > | ||
| {copied ? ( | ||
| <svg className="h-3.5 w-3.5" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={2}> | ||
| <path strokeLinecap="round" strokeLinejoin="round" d="M5 13l4 4L19 7" /> | ||
| </svg> | ||
| ) : ( | ||
| <svg className="h-3.5 w-3.5" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={2}> | ||
| <path strokeLinecap="round" strokeLinejoin="round" d="M8 16H6a2 2 0 01-2-2V6a2 2 0 012-2h8a2 2 0 012 2v2m-6 12h8a2 2 0 002-2v-8a2 2 0 00-2-2h-8a2 2 0 00-2 2v8a2 2 0 002 2z" /> | ||
| </svg> | ||
| )} | ||
| </button> |
Contributor
There was a problem hiding this comment.
Missing data-testid and undersized touch target on copy button.
Per coding guidelines:
- Interactive elements need
data-testidattributes with format{action}-{element-type} - Mobile touch targets must be at least 44px × 44px
The copy button at line 139 uses p-0.5 which is far too small (~2px padding).
Proposed fix
<button
type="button"
onClick={handleCopy}
- className="p-0.5 text-[`#999`] hover:text-[`#666`]"
+ className="flex h-7 w-7 items-center justify-center text-[`#999`] hover:text-[`#666`] sm:h-auto sm:w-auto sm:p-0.5"
+ data-testid="copy-code-button"
>As per coding guidelines: "Add data-testid attributes to all interactive elements" and "All interactive elements on mobile must be at least 44px × 44px".
🤖 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/MessageList.tsx` around lines 136 - 150, The copy
button in MessageList.tsx (the button using onClick={handleCopy} and the copied
state) lacks a data-testid and has an undersized touch target (uses p-0.5); add
a data-testid following the {action}-{element-type} pattern (e.g.,
"copy-button") to the button and increase its interactive area to meet the
44×44px mobile requirement by replacing the tiny padding class with size classes
or explicit min-height/min-width (for example use padding/size utility classes
that result in ≥44px hit area or add min-h-[44px] min-w-[44px]) while preserving
the existing icon visuals and copied state handling.
Comment on lines
+36
to
+38
| if (filtered.length > 0) { | ||
| setSelectedModelState(filtered[0]) | ||
| } |
Contributor
There was a problem hiding this comment.
Clear stale selection when no eligible model exists.
If the filtered list is empty, selectedModel currently keeps the previous value, which can leave an invalid model selected.
Suggested fix
- if (filtered.length > 0) {
- setSelectedModelState(filtered[0])
- }
+ if (filtered.length > 0) {
+ setSelectedModelState(filtered[0])
+ } else {
+ setSelectedModelState(null)
+ }📝 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.
Suggested change
| if (filtered.length > 0) { | |
| setSelectedModelState(filtered[0]) | |
| } | |
| if (filtered.length > 0) { | |
| setSelectedModelState(filtered[0]) | |
| } else { | |
| setSelectedModelState(null) | |
| } |
🤖 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/useWorkbenchModels.ts` around lines 36 - 38,
When filtered is empty we must clear any stale selection instead of leaving
selectedModel set; update the logic around filtered and setSelectedModelState in
useWorkbenchModels.ts so that if filtered.length > 0 you call
setSelectedModelState(filtered[0]) and otherwise call
setSelectedModelState(null) (or undefined if your selectedModel type uses
undefined) to explicitly clear the selection; ensure the selected model state
type (selectedModel / setSelectedModelState) accepts null/undefined and adjust
its type if necessary.
- Project chat composer with model/skill selection, file attachments - Socket.io streaming with chat:send, chat:start/chunk/done/error events - Tool block display (chat:block_created/block_updated) with Shell/file views - Project work selector with device names and online status sorting - Pass device_id from project config for correct execution context - Connection settings page - Markdown rendering with code blocks, copy buttons - Light theme UI matching main frontend style Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FicoHu
force-pushed
the
codex/feature/wework-project-chat
branch
from
May 27, 2026 17:01
5e6dffa to
94f7af2
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
wework/src/api/teams.ts (1)
15-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPaginated fetch is still truncated to the first page.
Line 15 hardcodes
page=1, solistTeams()can miss active/default teams beyond the first 100 records, which can break default team resolution in Line 22+.Suggested fix
async function listTeams(): Promise<Team[]> { - const response = await client.get<TeamListResponse>('/teams?page=1&limit=100') - return response.items + const limit = 100 + let page = 1 + const allItems: Team[] = [] + + while (true) { + const response = await client.get<TeamListResponse>(`/teams?page=${page}&limit=${limit}`) + allItems.push(...response.items) + if (response.items.length < limit) break + page += 1 + } + + return allItems }🤖 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/api/teams.ts` around lines 15 - 16, listTeams() currently requests only the first page by calling client.get<TeamListResponse>('/teams?page=1&limit=100') so it can miss teams beyond the first 100; change listTeams() to iterate through paginated results (using the TeamListResponse pagination metadata) and accumulate response.items from each page until no more pages remain, then return the combined list so the default/active team resolution later in the function (lines referencing default team resolution) sees all teams.
🧹 Nitpick comments (1)
wework/src/components/chat/composer/AddContextMenu.tsx (1)
61-71: 💤 Low valueRedundant
!disabledcheck inonClickhandler.The button already has
disabled={disabled}, so browser semantics prevent click events when disabled. The manual!disabled &&check is unnecessary.Suggested fix
<button type="button" data-testid="add-context-button" - onClick={() => !disabled && setOpen(current => !current)} + onClick={() => setOpen(current => !current)} disabled={disabled}🤖 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/AddContextMenu.tsx` around lines 61 - 71, The onClick handler for the add-context button redundantly guards with "!disabled &&", because the element already uses disabled={disabled}; remove the redundant check so the handler simply toggles open via setOpen (refer to the button with data-testid "add-context-button" and the setOpen state updater) and keep disabled={disabled} and aria-expanded={open} as-is to preserve accessibility and browser semantics.
🤖 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 `@wework/src/components/chat/composer/AddContextMenu.tsx`:
- Line 4: The file AddContextMenu.tsx currently imports useTranslation from
'react-i18next'; update the import to use the project's translation hook by
importing useTranslation from '`@/hooks/useTranslation`' instead (replace the
import statement that references 'react-i18next' with one that references
'`@/hooks/useTranslation`') so components like AddContextMenu use the standardized
translation hook.
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 311-321: The dependency list for sendCurrentInput includes the
non-memoized attachmentSelection object returned by useWorkbenchAttachments,
causing sendCurrentInput to be recreated each render; remove attachmentSelection
from the useCallback deps and instead depend on the stable attachments value (or
make attachments stable) and update any in-callback reads that reference
attachmentSelection.attachments to use attachments (e.g., when populating
payload.attachment_ids); ensure sendCurrentInput's dependency array contains
attachments (or another memoized selector like attachmentSelection.attachments)
along with isOptionsLocked, modelSelection.selectedModel, resolvedServices,
skillSelection.selectedSkills, state.currentProject?.id, state.currentTask,
state.defaultTeam, and state.input so the callback is stable and uses the
correct attachment list.
---
Duplicate comments:
In `@wework/src/api/teams.ts`:
- Around line 15-16: listTeams() currently requests only the first page by
calling client.get<TeamListResponse>('/teams?page=1&limit=100') so it can miss
teams beyond the first 100; change listTeams() to iterate through paginated
results (using the TeamListResponse pagination metadata) and accumulate
response.items from each page until no more pages remain, then return the
combined list so the default/active team resolution later in the function (lines
referencing default team resolution) sees all teams.
---
Nitpick comments:
In `@wework/src/components/chat/composer/AddContextMenu.tsx`:
- Around line 61-71: The onClick handler for the add-context button redundantly
guards with "!disabled &&", because the element already uses
disabled={disabled}; remove the redundant check so the handler simply toggles
open via setOpen (refer to the button with data-testid "add-context-button" and
the setOpen state updater) and keep disabled={disabled} and aria-expanded={open}
as-is to preserve accessibility and browser semantics.
🪄 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: edfe9cc1-ccad-4dc5-84c1-b77fb55f1f04
⛔ Files ignored due to path filters (1)
wework/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (43)
wework/package.jsonwework/src/api/adapters.test.tswework/src/api/attachments.tswework/src/api/devices.tswework/src/api/models.tswework/src/api/skills.tswework/src/api/teams.tswework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/ChatInput.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/blocks/ToolBlockItem.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.tsxwework/src/components/chat/composer/AddContextMenu.tsxwework/src/components/chat/composer/AttachmentBadges.tsxwework/src/components/chat/composer/ComposerToolbar.tsxwework/src/components/chat/composer/DesktopChatComposer.tsxwework/src/components/chat/composer/DesktopComposerToolbar.tsxwework/src/components/chat/composer/DesktopContextMenu.tsxwework/src/components/chat/composer/DesktopModeSelector.tsxwework/src/components/chat/composer/DesktopModelSelector.tsxwework/src/components/chat/composer/ModelSelector.tsxwework/src/components/chat/composer/ProjectChatComposer.tsxwework/src/components/chat/composer/ProjectWorkBar.tsxwework/src/components/chat/composer/SkillSelector.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/messageReducer.tswework/src/features/workbench/useWorkbenchAttachments.tswework/src/features/workbench/useWorkbenchModels.tswework/src/features/workbench/useWorkbenchSkills.tswework/src/features/workbench/workbenchProjectChatHooks.test.tsxwework/src/features/workbench/workbenchReducer.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/pages/WorkbenchPage.tsxwework/src/stream/chatStream.tswework/src/stream/socketClient.tswework/src/types/api.tswework/src/types/workbench.ts
💤 Files with no reviewable changes (5)
- wework/src/components/chat/composer/DesktopComposerToolbar.tsx
- wework/src/components/chat/composer/DesktopContextMenu.tsx
- wework/src/components/chat/composer/DesktopChatComposer.tsx
- wework/src/components/chat/composer/DesktopModeSelector.tsx
- wework/src/components/chat/composer/DesktopModelSelector.tsx
✅ Files skipped from review due to trivial changes (3)
- wework/src/api/models.ts
- wework/src/i18n/locales/en/common.json
- wework/src/i18n/locales/zh-CN/common.json
🚧 Files skipped from review as they are similar to previous changes (31)
- wework/src/stream/socketClient.ts
- wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
- wework/src/components/chat/composer/ComposerToolbar.tsx
- wework/src/components/chat/composer/ModelSelector.tsx
- wework/src/features/workbench/useWorkbenchModels.ts
- wework/src/api/adapters.test.ts
- wework/src/components/layout/MobileWorkbenchLayout.tsx
- wework/src/features/workbench/workbenchProjectChatHooks.test.tsx
- wework/src/api/attachments.ts
- wework/src/components/chat/composer/SkillSelector.tsx
- wework/src/api/devices.ts
- wework/src/components/layout/DesktopWorkbenchLayout.tsx
- wework/src/components/chat/ChatInput.tsx
- wework/src/components/chat/blocks/ToolBlocksDisplay.tsx
- wework/src/components/layout/DesktopWorkbenchMain.tsx
- wework/src/features/workbench/WorkbenchProvider.test.tsx
- wework/src/api/skills.ts
- wework/src/features/workbench/workbenchReducer.ts
- wework/src/components/chat/composer/ProjectChatComposer.tsx
- wework/src/components/chat/blocks/ToolBlockItem.tsx
- wework/src/pages/WorkbenchPage.tsx
- wework/src/components/chat/composer/AttachmentBadges.tsx
- wework/src/stream/chatStream.ts
- wework/src/features/workbench/useWorkbenchAttachments.ts
- wework/src/types/workbench.ts
- wework/src/types/api.ts
- wework/src/components/chat/composer/ProjectWorkBar.tsx
- wework/src/components/chat/MessageList.tsx
- wework/src/features/workbench/useWorkbenchSkills.ts
- wework/src/features/workbench/messageReducer.ts
- wework/src/components/chat/ChatInput.test.tsx
| import { Paperclip, Plus } from 'lucide-react' | ||
| import type { ChangeEvent } from 'react' | ||
| import { useCallback, useRef, useState } from 'react' | ||
| import { useTranslation } from 'react-i18next' |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Import useTranslation from @/hooks/useTranslation, not react-i18next.
As per coding guidelines: "Always import translations from @/hooks/useTranslation, not from react-i18next".
Suggested fix
-import { useTranslation } from 'react-i18next'
+import { useTranslation } from '`@/hooks/useTranslation`'📝 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.
Suggested change
| import { useTranslation } from 'react-i18next' | |
| import { useTranslation } from '`@/hooks/useTranslation`' |
🤖 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/AddContextMenu.tsx` at line 4, The file
AddContextMenu.tsx currently imports useTranslation from 'react-i18next'; update
the import to use the project's translation hook by importing useTranslation
from '`@/hooks/useTranslation`' instead (replace the import statement that
references 'react-i18next' with one that references '`@/hooks/useTranslation`') so
components like AddContextMenu use the standardized translation hook.
Comment on lines
311
to
321
| }, [ | ||
| attachmentSelection, | ||
| isOptionsLocked, | ||
| modelSelection.selectedModel, | ||
| resolvedServices, | ||
| skillSelection.selectedSkills, | ||
| state.currentProject?.id, | ||
| state.currentTask?.id, | ||
| state.currentTask, | ||
| state.defaultTeam, | ||
| state.input, | ||
| ]) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if useWorkbenchAttachments memoizes its return value
ast-grep --pattern $'function useWorkbenchAttachments($$$) {
$$$
return useMemo($$$)
$$$
}'
# Also check for return statement pattern
rg -n -A5 'return \{' wework/src/features/workbench/useWorkbenchAttachments.tsRepository: wecode-ai/Wegent
Length of output: 1387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "useWorkbenchAttachments" wework/src/features/workbench/useWorkbenchAttachments.ts
echo "----"
sed -n '1,220p' wework/src/features/workbench/useWorkbenchAttachments.ts
echo "----"
sed -n '220,420p' wework/src/features/workbench/useWorkbenchAttachments.tsRepository: wecode-ai/Wegent
Length of output: 4520
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '250,380p' wework/src/features/workbench/WorkbenchProvider.tsx
rg -n "attachmentSelection" wework/src/features/workbench/WorkbenchProvider.tsx
rg -n "const sendCurrentInput|sendCurrentInput" wework/src/features/workbench/WorkbenchProvider.tsxRepository: wecode-ai/Wegent
Length of output: 4348
Remove non-memoized attachmentSelection from sendCurrentInput dependencies
useWorkbenchAttachments returns a freshly created wrapper object (return { state, attachments: ..., resetAttachments, ... }) on every render, so including the whole attachmentSelection in the sendCurrentInput useCallback deps recreates the callback unnecessarily.
}, [
attachmentSelection,
isOptionsLocked,
modelSelection.selectedModel,
resolvedServices,
skillSelection.selectedSkills,
state.currentProject?.id,
state.currentTask,
state.defaultTeam,
state.input,
])Suggested fix
+ const { attachments, resetAttachments } = attachmentSelection
+
const sendCurrentInput = useCallback(async () => {
const trimmedMessage = state.input.trim()
- const hasAttachments = attachmentSelection.attachments.length > 0
+ const hasAttachments = attachments.length > 0
// ... rest of function
- attachmentSelection.resetAttachments()
+ resetAttachments()
// ...
}, [
- attachmentSelection,
+ attachments,
+ resetAttachments,
isOptionsLocked,
// ... rest of deps
])Also update all in-callback reads from attachmentSelection.attachments to use attachments (e.g., when building payload.attachment_ids) so the refactor is complete.
🤖 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 311 - 321,
The dependency list for sendCurrentInput includes the non-memoized
attachmentSelection object returned by useWorkbenchAttachments, causing
sendCurrentInput to be recreated each render; remove attachmentSelection from
the useCallback deps and instead depend on the stable attachments value (or make
attachments stable) and update any in-callback reads that reference
attachmentSelection.attachments to use attachments (e.g., when populating
payload.attachment_ids); ensure sendCurrentInput's dependency array contains
attachments (or another memoized selector like attachmentSelection.attachments)
along with isOptionsLocked, modelSelection.selectedModel, resolvedServices,
skillSelection.selectedSkills, state.currentProject?.id, state.currentTask,
state.defaultTeam, and state.input so the callback is stable and uses the
correct attachment list.
This was referenced May 29, 2026
This was referenced Jun 5, 2026
This was referenced Jun 12, 2026
Merged
This was referenced Jun 19, 2026
This was referenced Jun 29, 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
Release Notes