[FEAT] Workspaces#37
Conversation
…ition - Add WorkspaceManagerDialog for creating, renaming, recoloring, and setting default workspaces - Add DeleteWorkspaceDialog with three disposition options: move chats to another workspace, archive chats, or delete chats permanently - Add "Move to workspace" submenu in chat context menu - Add workspace color indicators in sidebar and dialogs - Add accent color theming support with readable foreground calculation - Make workspace color nullable in UpdateWorkspaceRequest to allow explicit clearing - Add i18n translations for workspace management (English and German) - Add comprehensive tests for move, archive, delete dispositions and color handling - Fix workspace_id handling in chat update to support Option<Option<String>>
There was a problem hiding this comment.
Code Review
This pull request introduces workspace management features, allowing users to create, rename, recolor, and delete workspaces, along with options to move, archive, or delete associated chats. The backend has been updated to support these operations, including handling nullable workspace colors and chat workspace IDs, backed by new integration tests. Feedback on these changes includes wrapping workspace deletion and chat disposition operations in a database transaction to ensure atomicity, only clearing the workspace editing state upon a successful rename, adding missing workspace.delete_failed translation keys, and lowering the relative luminance threshold to 0.179 to improve text contrast on colored backgrounds.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| let disposition = match params.action { | ||
| WorkspaceDeleteAction::Move => { | ||
| let Some(target) = params.target_workspace_id else { | ||
| return ErrorBuilder::new(ErrorCode::ValidationFailed).build(); | ||
| }; | ||
| if target == id { | ||
| return ErrorBuilder::new(ErrorCode::ValidationFailed).build(); | ||
| } | ||
| match Chat::verify_workspace_belongs_to_user(&state.db, &target, &user.id).await { | ||
| Ok(true) => Chat::move_all_to_workspace(&state.db, &user.id, &id, &target).await.map(|_| ()), | ||
| Ok(false) => return ErrorBuilder::new(ErrorCode::ValidationFailed).build(), | ||
| Err(e) => Err(e), | ||
| } | ||
| } | ||
| WorkspaceDeleteAction::Archive => Chat::archive_all_in_workspace(&state.db, &user.id, &id).await.map(|_| ()), | ||
| WorkspaceDeleteAction::Delete => Chat::delete_all_in_workspace(&state.db, &user.id, &id).await.map(|_| ()), | ||
| }; | ||
|
|
||
| if let Err(e) = disposition { | ||
| eprintln!("[WORKSPACES] Failed to apply chat disposition: {e}"); | ||
| return ErrorBuilder::new(ErrorCode::InternalError).build(); | ||
| } |
There was a problem hiding this comment.
The workspace deletion and chat disposition operations are performed as separate, non-atomic database queries without a transaction. If any query fails midway (e.g., Workspace::delete fails due to a database issue or a concurrent request inserts a new chat into the workspace after the disposition is applied), the database will be left in an inconsistent state.\n\nTo ensure atomicity and prevent race conditions, these operations should be wrapped in a database transaction. You can change the signature of the helper methods (move_all_to_workspace, archive_all_in_workspace, delete_all_in_workspace, and Workspace::delete) to accept an executor/transaction, or execute the queries within a transaction block.
| async function saveRename(ws: Workspace) { | ||
| const name = editingName.value.trim(); | ||
| if (!name) return; | ||
| await chatStore.updateWorkspace(ws.id, {name, color: editingColor.value}); | ||
| editingId.value = null; | ||
| } |
There was a problem hiding this comment.
If chatStore.updateWorkspace fails (e.g., due to a network error or a duplicate name constraint), editingId is still set to null. This closes the editing mode and discards the user's input, forcing them to start over.\n\nConsider only clearing editingId if the update operation succeeds.
async function saveRename(ws: Workspace) {\n\tconst name = editingName.value.trim();\n\tif (!name) return;\n\tconst success = await chatStore.updateWorkspace(ws.id, {name, color: editingColor.value});\n\tif (success) {\n\t\teditingId.value = null;\n\t}\n}
| ('en', 'workspace.create_failed', 'Could not create the workspace.'), | ||
| ('en', 'workspace.update_failed', 'Could not update the workspace.'), |
There was a problem hiding this comment.
Please add the workspace.delete_failed translation key to the English translations to support error notifications when workspace deletion fails.
('en', 'workspace.create_failed', 'Could not create the workspace.'),\n ('en', 'workspace.update_failed', 'Could not update the workspace.'),\n ('en', 'workspace.delete_failed', 'Could not delete the workspace.'),| ('de', 'workspace.create_failed', 'Arbeitsbereich konnte nicht erstellt werden.'), | ||
| ('de', 'workspace.update_failed', 'Arbeitsbereich konnte nicht aktualisiert werden.'), |
There was a problem hiding this comment.
Please add the workspace.delete_failed translation key to the German translations to support error notifications when workspace deletion fails.
('de', 'workspace.create_failed', 'Arbeitsbereich konnte nicht erstellt werden.'),\n ('de', 'workspace.update_failed', 'Arbeitsbereich konnte nicht aktualisiert werden.'),\n ('de', 'workspace.delete_failed', 'Arbeitsbereich konnte nicht gelöscht werden.'),| function readableForeground(hex: string): string { | ||
| const normalized = hex.replace('#', ''); | ||
| const full = normalized.length === 3 ? normalized.split('').map(c => c + c).join('') : normalized; | ||
| if (full.length !== 6) return 'rgb(255, 255, 255)'; | ||
| const r = parseInt(full.slice(0, 2), 16) / 255; | ||
| const g = parseInt(full.slice(2, 4), 16) / 255; | ||
| const b = parseInt(full.slice(4, 6), 16) / 255; | ||
| const channel = (c: number) => (c <= 0.03928 ? c / 12.92 : Math.pow((c + 0.055) / 1.055, 2.4)); | ||
| const luminance = 0.2126 * channel(r) + 0.7152 * channel(g) + 0.0722 * channel(b); | ||
| return luminance > 0.45 ? 'rgb(23, 23, 23)' : 'rgb(255, 255, 255)'; | ||
| } |
There was a problem hiding this comment.
The relative luminance threshold of 0.45 is too high and results in poor contrast for several colors. For example, green (#22c55e, luminance ~0.41) will receive white text, resulting in a very low contrast ratio of ~2.3:1, whereas dark text would provide a highly readable ~7.6:1 contrast ratio.\n\nThe standard relative luminance threshold to switch between light and dark text for optimal WCAG contrast is 0.179 (derived from the geometric mean of the extreme luminances). Using 0.179 as the threshold will significantly improve readability across the color palette.
function readableForeground(hex: string): string {\n\tconst normalized = hex.replace('#', '');\n\tconst full = normalized.length === 3 ? normalized.split('').map(c => c + c).join('') : normalized;\n\tif (full.length !== 6) return 'rgb(255, 255, 255)';\n\tconst r = parseInt(full.slice(0, 2), 16) / 255;\n\tconst g = parseInt(full.slice(2, 4), 16) / 255;\n\tconst b = parseInt(full.slice(4, 6), 16) / 255;\n\tconst channel = (c: number) => (c <= 0.03928 ? c / 12.92 : Math.pow((c + 0.055) / 1.055, 2.4));\n\tconst luminance = 0.2126 * channel(r) + 0.7152 * channel(g) + 0.0722 * channel(b);\n\treturn luminance > 0.179 ? 'rgb(23, 23, 23)' : 'rgb(255, 255, 255)';\n}…kspace protection - Wrap workspace create/update/delete in database transactions for atomicity - Add `DefaultWorkspaceDelete` error code with i18n translations (en/de) - Prevent deletion of default workspace with dedicated error - Move chat disposition logic into transaction in `delete_with_chat_disposition` - Add transaction variants for chat bulk operations (move/archive/delete) - Remove unused `Message::deactivate_subtree` and `activate_subtree` methods - Fix frontend initialization order: fetch workspaces/preferences before chats - Add error notification for workspace deletion failures
Add workspace management with create, rename, delete, and chat disposition