fix(ui): restore provider wave indicator tones#9
Conversation
There was a problem hiding this comment.
Pull request overview
Restores provider-specific wave indicator tones across the UI by introducing a shared helper in the provider model catalog and applying it consistently to wave indicators in chat, task tabs/list, and workspace sidebar.
Changes:
- Add
getProviderWaveToneClassto the provider model catalog and cover it with a unit test. - Update ChatPanel, task tabs, and task list items to apply the provider tone directly on
WaveIndicator. - Update workspace sidebar responding-wave rendering to use provider tone when a workspace has a single responding provider, otherwise fall back to neutral.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/model-catalog.test.ts | Adds coverage for the new provider wave tone helper. |
| src/lib/providers/model-catalog.ts | Introduces getProviderWaveToneClass to centralize tone mapping. |
| src/components/session/ChatPanel.tsx | Uses shared helper for responding wave indicator tone (including neutral for user/stave). |
| src/components/layout/WorkspaceTaskTabs.tsx | Applies provider tone directly to WaveIndicator in task tabs. |
| src/components/layout/TaskItem.tsx | Applies provider tone directly to WaveIndicator in task list items. |
| src/components/layout/ProjectWorkspaceSidebar.tsx | Computes workspace-level responding tone (single-provider vs mixed/neutral) and applies it to waves. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return getWorkspaceRespondingTasks(workspaceId).length; | ||
| } | ||
|
|
||
| function getWorkspaceRespondingToneClass(workspaceId: string) { | ||
| const providers = Array.from(new Set(getWorkspaceRespondingTasks(workspaceId).map((task) => task.provider))); |
There was a problem hiding this comment.
getWorkspaceRespondingTaskCount and getWorkspaceRespondingToneClass both call getWorkspaceRespondingTasks(workspaceId), which filters the workspace task list each time. Consider computing the responding tasks once (e.g., return { respondingTasks, count, toneClass } from a single helper or pass the filtered list into both calculations) to avoid duplicate O(n) scans on every render.
| const workspaceBusy = busyWorkspaceKey === `${entry.projectPath}:${entry.workspaceId}`; | ||
| const respondingTaskCount = getWorkspaceRespondingTaskCount(entry.workspaceId); | ||
| const isResponding = respondingTaskCount > 0; | ||
| const respondingToneClass = getWorkspaceRespondingToneClass(entry.workspaceId); |
There was a problem hiding this comment.
respondingToneClass is computed unconditionally for each collapsed workspace entry, even when isResponding is false. Since getWorkspaceRespondingToneClass scans tasks (filter/map/Set), consider only computing it inside the isResponding branch (or defaulting to "text-primary" when not responding) to avoid unnecessary work during sidebar renders.
| const respondingToneClass = getWorkspaceRespondingToneClass(entry.workspaceId); | |
| const respondingToneClass = isResponding | |
| ? getWorkspaceRespondingToneClass(entry.workspaceId) | |
| : "text-primary"; |
| const workspaceBusy = busyWorkspaceKey === `${project.projectPath}:${workspace.id}`; | ||
| const respondingTaskCount = getWorkspaceRespondingTaskCount(workspace.id); | ||
| const isResponding = respondingTaskCount > 0; | ||
| const respondingToneClass = getWorkspaceRespondingToneClass(workspace.id); |
There was a problem hiding this comment.
respondingToneClass is computed for every workspace row even when isResponding is false. Since getWorkspaceRespondingToneClass performs a task scan, consider computing it only when needed (e.g., when respondingTaskCount > 0) to reduce repeated work during renders.
| const respondingToneClass = getWorkspaceRespondingToneClass(workspace.id); | |
| const respondingToneClass = isResponding ? getWorkspaceRespondingToneClass(workspace.id) : ""; |
Summary
Verification