electron-rare/Aperant
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: address all PR review findings for bulk delete/archive functionality

Browse Source 
Comprehensive code quality improvements addressing all review comments:

**Critical Fixes:**
- Fixed double-toggle bug where checkbox called onToggleSelect twice
- Added accessibility attributes (role, tabIndex, onKeyDown) to interactive div

**Error Handling:**
- Removed error re-throws in async event handlers to prevent unhandled rejections
- Added error logging to empty catch blocks in session-storage.ts
- Added error handling with .catch() for dropdown menu promise rejections
- Added logging for partial failures in bulk operations

**Code Quality:**
- Replaced non-null assertion (!) with explicit null check in session-manager.ts
- Removed redundant async/await wrappers in archive/unarchive handlers
- Removed duplicate loadInsightsSession calls from store functions
- Added skip-first-run guard to prevent double load on component mount
- Added clarifying comment for showArchived useEffect dependency

**Performance:**
- Eliminated redundant IPC calls by removing duplicate session reloads
- Fixed flicker issue caused by double reload with different filters

All changes maintain backward compatibility and improve user experience.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
This commit is contained in:
AndyMik90
2026-02-18 15:05:54 +01:00
co-authored by Claude Opus 4.6
parent 9f3ac54844
commit 02b40fa342
5 changed files with 40 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/frontend/src/main/insights/session-manager.ts
+3 -2
View File
@@ -20,8 +20,9 @@ export class SessionManager {
*/
loadSession(projectId: string, projectPath: string): InsightsSession | null {
// Check in-memory cache first
if (this.sessions.has(projectId)) {
return this.sessions.get(projectId)!;
const cachedSession = this.sessions.get(projectId);
if (cachedSession) {
return cachedSession;
}
// Migrate old format if needed
apps/frontend/src/main/insights/session-storage.ts
+4 -2
View File
@@ -75,7 +75,8 @@ export class SessionStorage {
session.archivedAt = new Date();
this.saveSession(projectPath, session);
return true;
} catch {
} catch (error) {
console.error(`[SessionStorage] Failed to archive session ${sessionId}:`, error);
return false;
}
}
@@ -91,7 +92,8 @@ export class SessionStorage {
delete session.archivedAt;
this.saveSession(projectPath, session);
return true;
} catch {
} catch (error) {
console.error(`[SessionStorage] Failed to unarchive session ${sessionId}:`, error);
return false;
}
}
apps/frontend/src/renderer/components/ChatHistorySidebar.tsx
+13 -14
View File
@@ -86,7 +86,8 @@ export function ChatHistorySidebar({
});
}, []);
// Clear selection when showArchived toggles
// Clear selection when showArchived toggles - prevents selecting archived sessions when filter changes
// biome-ignore lint/correctness/useExhaustiveDependencies: showArchived is intentionally a dependency to reset selection on filter change
useEffect(() => {
setSelectedIds(new Set());
}, [showArchived]);
@@ -144,8 +145,6 @@ export function ChatHistorySidebar({
setBulkDeleteOpen(false);
} catch (error) {
console.error('Failed to delete sessions:', error);
// Re-throw to allow parent components to handle
throw error;
}
}
};
@@ -157,8 +156,6 @@ export function ChatHistorySidebar({
setSelectedIds(new Set());
} catch (error) {
console.error('Failed to archive sessions:', error);
// Re-throw to allow parent components to handle
throw error;
}
}
};
@@ -304,8 +301,8 @@ export function ChatHistorySidebar({
onCancelEdit={handleCancelEdit}
onEditTitleChange={setEditTitle}
onDelete={() => setDeleteSessionId(session.id)}
onArchive={onArchiveSession ? async () => await onArchiveSession(session.id) : undefined}
onUnarchive={onUnarchiveSession ? async () => await onUnarchiveSession(session.id) : undefined}
onArchive={onArchiveSession ? () => onArchiveSession(session.id).catch((e) => console.error('Archive failed:', e)) : undefined}
onUnarchive={onUnarchiveSession ? () => onUnarchiveSession(session.id).catch((e) => console.error('Unarchive failed:', e)) : undefined}
isArchived={!!session.archivedAt}
isSelectionMode={isSelectionMode}
isSelected={selectedIds.has(session.id)}
@@ -476,23 +473,25 @@ function SessionItem({
return (
<div
role="button"
tabIndex={0}
className={cn(
'group relative cursor-pointer px-2 py-2 transition-colors hover:bg-muted',
isActive && 'bg-primary/10 hover:bg-primary/15',
isArchived && 'opacity-50'
)}
onClick={isSelectionMode ? undefined : onSelect}
onKeyDown={(e) => {
if (!isSelectionMode && (e.key === 'Enter' || e.key === ' ')) {
e.preventDefault();
onSelect();
}
}}
>
{/* Content with reserved space for the menu button */}
<div className="flex items-center gap-1.5 pr-7">
{isSelectionMode ? (
<div
onClick={(e) => {
e.stopPropagation();
onToggleSelect();
}}
className="shrink-0"
>
<div className="shrink-0" onClick={(e) => e.stopPropagation()}>
<Checkbox
checked={isSelected}
onCheckedChange={() => onToggleSelect()}
apps/frontend/src/renderer/components/Insights.tsx
+18 -3
View File
@@ -149,8 +149,13 @@ export function Insights({ projectId }: InsightsProps) {
return cleanup;
}, [projectId]);
// Reload sessions when showArchived changes
// Reload sessions when showArchived changes (skip first run to avoid duplicate load)
const isFirstRun = useRef(true);
useEffect(() => {
if (isFirstRun.current) {
isFirstRun.current = false;
return;
}
loadInsightsSessions(projectId, showArchived);
}, [projectId, showArchived]);
@@ -221,13 +226,23 @@ export function Insights({ projectId }: InsightsProps) {
};
const handleDeleteSessions = async (sessionIds: string[]) => {
await deleteSessions(projectId, sessionIds);
const result = await deleteSessions(projectId, sessionIds);
await loadInsightsSessions(projectId, showArchived);
// Log partial failures for debugging
if (result.failedIds && result.failedIds.length > 0) {
console.warn(`Failed to delete ${result.failedIds.length} session(s):`, result.failedIds);
}
};
const handleArchiveSessions = async (sessionIds: string[]) => {
await archiveSessions(projectId, sessionIds);
const result = await archiveSessions(projectId, sessionIds);
await loadInsightsSessions(projectId, showArchived);
// Log partial failures for debugging
if (result.failedIds && result.failedIds.length > 0) {
console.warn(`Failed to archive ${result.failedIds.length} session(s):`, result.failedIds);
}
};
const handleToggleShowArchived = () => {
apps/frontend/src/renderer/stores/insights-store.ts
+2 -12
View File
@@ -316,7 +316,6 @@ export async function renameSession(projectId: string, sessionId: string, newTit
export async function deleteSessions(projectId: string, sessionIds: string[]): Promise<{ success: boolean; failedIds?: string[] }> {
const result = await window.electronAPI.deleteInsightsSessions(projectId, sessionIds);
if (result.success) {
await loadInsightsSession(projectId);
return { success: true };
}
return { success: false, failedIds: result.data?.failedIds };
@@ -324,17 +323,12 @@ export async function deleteSessions(projectId: string, sessionIds: string[]): P
export async function archiveSession(projectId: string, sessionId: string): Promise<boolean> {
const result = await window.electronAPI.archiveInsightsSession(projectId, sessionId);
if (result.success) {
await loadInsightsSession(projectId);
return true;
}
return false;
return result.success;
}
export async function archiveSessions(projectId: string, sessionIds: string[]): Promise<{ success: boolean; failedIds?: string[] }> {
const result = await window.electronAPI.archiveInsightsSessions(projectId, sessionIds);
if (result.success) {
await loadInsightsSession(projectId);
return { success: true };
}
return { success: false, failedIds: result.data?.failedIds };
@@ -342,11 +336,7 @@ export async function archiveSessions(projectId: string, sessionIds: string[]):
export async function unarchiveSession(projectId: string, sessionId: string): Promise<boolean> {
const result = await window.electronAPI.unarchiveInsightsSession(projectId, sessionId);
if (result.success) {
await loadInsightsSession(projectId);
return true;
}
return false;
return result.success;
}
export async function updateModelConfig(projectId: string, sessionId: string, modelConfig: InsightsModelConfig): Promise<boolean> {