Mass-download

[KazeKaze93]

• Integrated global download state management using useDownloadStore to track download progress across components.
• Updated DownloadAllButton to disable during active downloads and provide contextual tooltips.
• Enhanced PendingDownloadBanner to manage download state and display appropriate messages based on download status.
• Refined useDownloadAll hook to synchronize global download state, ensuring consistent UI behavior during downloads.
• Improved post count display in Favorites and Updates pages to reflect total downloads accurately.
• Added query invalidation for playlist entries to ensure UI reflects the latest state after modifications.

[github-actions]

🛡️ Архитектор (AI Review)

Слушай сюда. Твой PR на "Mass-download" — это классический пример того, как полезная фича превращается в мину замедленного действия для производительности Main-процесса. Ты добавил кучу полезного функционала, но забил на базовые принципы Electron.

Вот твой список грехов:

🚨 Критично

1. Блокировка Main-процесса (I/O Sync):
   В FileController.ts ты используешь fs.writeFileSync, fs.readFileSync, fs.mkdirSync и fs.unlinkSync внутри цикла загрузки и при каждом обновлении прогресса (writeQueueFile).
   Почему это плохо: Main-процесс в Electron один. Пока ты пишешь JSON на диск синхронно (особенно на медленных HDD), весь UI будет "фризить", а IPC-сообщения — стоять в очереди.
   Как исправить: Используй fs.promises (асинхронные методы) или fs-extra.

   // src/main/ipc/controllers/FileController.ts
   private async writeQueueFile(data: any): Promise<void> {
     try {
       await fs.promises.writeFile(this.getQueueFilePath(), JSON.stringify(data), "utf-8");
     } catch (e) {
       log.warn("[FileController] Failed to write queue file:", e);
     }
   }

2. Ручные миграции (Brittle Migrations):
   В src/main/db/client.ts ты развел зоопарк с ручной проверкой entry.tag === "0012_add_download_folder".
   Почему это плохо: Это путь в ад поддержки. Drizzle имеет встроенный механизм миграций. Если ты начал писать if (tag === "...") для каждой колонки, твоя архитектура БД мертва.
   Как исправить: Используй migrate(db, { migrationsFolder: '...' }). Если SQLite не поддерживает ADD COLUMN IF NOT EXISTS, используй транзакции и нормальный логгер миграций, а не этот if-else лабиринт.

3. Отсутствие санитизации путей:
   В getFilePath ты доверяешь item.filename. Хотя там есть regex, root берется из настроек. Если пользователь (или злоумышленник через конфиг) прокинет туда что-то вроде ../../, ты можешь начать писать файлы вне downloadRoot.
   Как исправить: Всегда делай path.resolve и проверяй, что итоговый путь начинается с downloadRoot.

⚠️ Надо исправить

1. Polling в Renderer:
   В PendingDownloadBanner.tsx ты используешь setInterval каждые 5 секунд для проверки getPendingDownload.
   Почему это плохо: Это неэффективно. У тебя уже есть IPC.
   Как исправить: Вызывай проверку один раз при монтировании, а дальше пусть Main-процесс сам присылает событие, если состояние очереди изменилось.

2. Zustand Anti-pattern:
   В DownloadAllButton.tsx: const isAnyDownloadActive = useDownloadStore((s) => s.isDownloading);.
   Это нормально, но в PendingDownloadBanner ты вытаскиваешь весь стор: const { isDownloading, setDownloading } = useDownloadStore();.
   Как исправить: Всегда используй селекторы, чтобы избежать лишних ре-рендеров, когда стор разрастется.

3. Безопасность IPC (Validation):
   В FileController.ts метод downloadAll принимает массив объектов. Ты используешь DownloadAllSchema.safeParse(items), это хорошо. Но лимит в 500 файлов зашит жестко.
   Совет: Вынеси магическое число 500 в константу или конфиг.

💡 Совет

1. Drizzle Type Safety:
   В getDownloadSettings ты используешь приведение типов as "skip" | "overwrite".
   Вместо этого используй zod для валидации данных, приходящих из БД, или настрой drizzle так, чтобы он возвращал enum (хотя в SQLite это просто text).

2. UX / A11y:
   В PostCard.tsx ты убрал aria-hidden="true", но не добавил aria-label для QuickAddToPlaylistMenu. Скринридеры скажут тебе "спасибо" (нет).

3. Оптимизация getDownloadItems:
   Ты делаешь this.getPosts(...), а потом мапишь их. Если в БД 500 постов, ты создаешь 500 объектов в памяти, потом еще 500 для IPC. Для Electron это не смертельно, но лучше сразу делать select только нужных полей (fileUrl, postId, artistId), чтобы не тащить тяжелые метаданные через мост.

Вердикт:
Код рабочий, но "грязный" с точки зрения производительности Electron. Исправляй синхронный I/O в Main-процессе, иначе на больших пачках загрузок приложение превратится в тыкву.

Отправлено на доработку. 🔴

Analyzed by: gemini-3-flash-preview

[github-actions]

🛡️ Архитектор (AI Review)

Так, посмотрим, что ты тут наворотил. "Mass-download", говоришь? Звучит как отличный способ заблокировать Main-процесс и угробить производительность. Поехали.

🚨 Критично

1. Блокировка Main-процесса при массовой загрузке
src/main/ipc/controllers/FileController.ts

Ты серьезно? Массовая загрузка файлов, с сетевыми запросами и записью на диск, прямо в Main-процессе? Даже с BATCH_DOWNLOAD_CONCURRENCY в 3, это все равно I/O-интенсивная операция, которая будет блокировать UI. Main-процесс — это не место для таких тяжелых задач. Я же ясно сказал: "Тяжелые операции (хэширование, парсинг больших JSON, работа с ФС) должны быть асинхронными или вынесены в Worker Threads/Utility Process". Это прямое нарушение принципов высокопроизводительных Electron-приложений.

--- a/src/main/ipc/controllers/FileController.ts
+++ b/src/main/ipc/controllers/FileController.ts
@@ -301,6 +301,10 @@
    * Uses concurrency limit and delay between requests to avoid bans (see docs/download-batch-risks.md)
    */
   private async downloadAll(
-    _event: IpcMainInvokeEvent,
-    items: Array<{ url: string; filename: string }>
-  ): Promise<{
-    success: boolean;
-    downloaded: number;
-    failed: number;
-    canceled: boolean;
-    error?: string;
-  }> {
+    // _event: IpcMainInvokeEvent, // Event object is not needed in a Worker
+    items: Array<{ url: string; filename: string }>,
+    // Add a way to communicate progress back to Main process, e.g., a callback or event emitter
+    // This method should be executed in a Worker Thread.
+  ): Promise<{ /* ... */ }> {
+    // ... весь код downloadAll должен быть перенесен в Worker Thread
+    // Main процесс должен только инициировать Worker и слушать его события.

Как исправить:
Вынеси всю логику downloadAll (включая runOne, getFilePath, getDownloadRoot, getDownloadSettings, writeQueueFile, readQueueFile, deleteQueueFile) в отдельный Worker Thread или Utility Process. Main-процесс должен только:

1. Получить запрос на downloadAll через IPC.
2. Запустить Worker Thread, передав ему items и необходимые настройки (например, downloadFolder, duplicateFileBehavior, downloadFolderStructure).
3. Слушать события от Worker Thread (прогресс, завершение, ошибки) и передавать их в Renderer через IPC.
4. Обрабатывать запросы на cancelDownloadAll, pauseDownloadAll, resumeDownloadAll, отправляя соответствующие сообщения в Worker Thread.

⚠️ Надо исправить

1. Доступность (A11y) в PostCard
src/renderer/features/artists/components/PostCard.tsx

Удаление aria-hidden="true" и role="presentation" с интерактивного элемента (div, который содержит QuickAddToPlaylistMenu) — это хорошо. Но ты не добавил aria-label или aria-labelledby для самого QuickAddToPlaylistMenu или его триггера, если он не имеет видимого текста. Убедись, что пользователи скринридеров понимают, что это за кнопка.

--- a/src/renderer/features/artists/components/PostCard.tsx
+++ b/src/renderer/features/artists/components/PostCard.tsx
@@ -219,17 +219,14 @@
         onClick={(e) => e.stopPropagation()}
         onMouseDown={(e) => e.stopPropagation()}
         onKeyDown={(e) => {
-          // A11y: Handle keyboard navigation for wrapper div
-          // Prevent card click when Space/Enter is pressed on wrapper
           if (e.key === "Enter" || e.key === " ") {
             e.stopPropagation();
           }
         }}
-        role="presentation"
-        aria-hidden="true"
       >
         <QuickAddToPlaylistMenu
           postId={post.id}
+          // Добавь aria-label или убедись, что trigger имеет понятный текст
           trigger={
             <span
               className="inline-flex items-center justify-center rounded-full bg-black/50 p-1.5 backdrop-blur-sm hover:bg-black/70 transition-colors cursor-pointer"

Как исправить:
Убедись, что trigger для QuickAddToPlaylistMenu имеет понятный текстовый контент или добавь aria-label к элементу, который служит триггером.

💡 Совет

1. Зависимости useEffect с JSON.stringify
src/renderer/hooks/useDownloadAll.ts

Использование JSON.stringify в зависимостях useEffect для объектов, таких как fetchParams?.filters, может привести к непредсказуемому поведению или лишним ре-рендерам, если порядок ключей в объекте изменится, но логически объект останется тем же. Для небольших, простых объектов это может быть приемлемо, но для более сложных структур лучше использовать useDeepCompareEffect или явно перечислять все значимые поля.

--- a/src/renderer/hooks/useDownloadAll.ts
+++ b/src/renderer/hooks/useDownloadAll.ts
@@ -70,7 +70,11 @@
           setTotalCount(0);
         }
       });
-  }, [fetchParams?.artistId, JSON.stringify(fetchParams?.filters)]);
+  }, [
+    fetchParams?.artistId,
+    fetchParams?.filters?.aiFilter, // Перечисли конкретные поля фильтров
+    fetchParams?.filters?.mediaType,
+    fetchParams?.filters?.tags, // Или используй useDeepCompareEffect
+  ]);
 
   const backendResult = useDownloadAllFromBackend(
     fetchParams ? { ...fetchParams, page: 1, limit: 50 } : null,

Как исправить:
Либо явно перечисли все поля объекта filters, которые влияют на запрос, либо используй кастомный хук для глубокого сравнения зависимостей, если объект filters может быть очень сложным и часто меняться.

2. DropdownMenu в QuickAddToPlaylistMenu
src/renderer/components/playlists/QuickAddToPlaylistMenu.tsx

Использование modal={false} для DropdownMenu может быть полезно для UX, но иногда создает проблемы с доступностью, так как фокус может уходить за пределы меню. Убедись, что это не нарушает навигацию с клавиатуры и работу скринридеров.

--- a/src/renderer/components/playlists/QuickAddToPlaylistMenu.tsx
+++ b/src/renderer/components/playlists/QuickAddToPlaylistMenu.tsx
@@ -136,7 +136,7 @@
 
   return (
     <>
-      <DropdownMenu open={isMenuOpen} onOpenChange={setIsMenuOpen}>
+      <DropdownMenu open={isMenuOpen} onOpenChange={setIsMenuOpen} modal={false}> {/* Проверь A11y */}
         <DropdownMenuTrigger asChild>
           {trigger || defaultTrigger}
         </DropdownMenuTrigger>

Как исправить:
Проверь поведение меню с modal={false} с помощью скринридера и навигации с клавиатуры, чтобы убедиться, что оно не создает проблем с доступностью. Если проблемы есть, рассмотри возможность удаления modal={false} или реализации более сложного управления фокусом.

---

В целом, много работы проделано, особенно в части IPC, Drizzle и UI. Но критическая ошибка с блокировкой Main-процесса перечеркивает все эти усилия для "High-Performance" приложения. Исправь это, и тогда поговорим.

Analyzed by: gemini-2.5-flash

[github-actions]

🛡️ Архитектор (AI Review)

Ладно, посмотрим, что ты тут наваял. "Массовая загрузка", говоришь? Обычно это заканчивается блокировкой UI и дырами в безопасности. Но ты, кажется, попытался. Посмотрим, насколько успешно.

🚨 Критично

1. src/main/ipc/controllers/FileController.ts - Path Traversal Prevention (Symlinks)

   --- a/src/main/ipc/controllers/FileController.ts
   +++ b/src/main/ipc/controllers/FileController.ts
   @@ -291,17 +679,13 @@ export class FileController extends BaseController {
      let resolvedPath: string;
      try {
        resolvedPath = await realpath(normalizedPath);
   -  } catch (error) {
   -    // Path doesn't exist or is inaccessible, fallback to download root
   -    log.warn(`[FileController] Failed to resolve real path: ${normalizedPath}`, error);
   -    try {
   -      await access(downloadRoot);
   -      await shell.openPath(downloadRoot);
   -      return true;
   -    } catch {
   -      return false;
   -    }
   +  } catch (error: unknown) { // <-- Здесь
   +    // ...
    }

   Проблема: В блоке catch для realpath ты используешь error без явной типизации. Это потенциальная any и потеря информации о типе ошибки.
   Исправление: Типизируй error как unknown и используй type guards, если нужно обращаться к его свойствам.

2. src/main/workers/downloadWorker.ts - Path Traversal Prevention (Symlinks)

   --- a/src/main/workers/downloadWorker.ts
   +++ b/src/main/workers/downloadWorker.ts
   @@ -255,7 +255,7 @@ runWorker().catch((err) => {

parentPort?.postMessage({
type: "error",
error: err instanceof Error ? err.message : String(err),
});
```
Проблема: Аналогично, в runWorker().catch((err) => { ... }) `err` не типизирован.
Исправление: Типизируй `err` как `unknown`.

⚠️ Надо исправить

1. src/renderer/components/downloads/PendingDownloadBanner.tsx - Логика завершения загрузки

   --- a/src/renderer/components/downloads/PendingDownloadBanner.tsx
   +++ b/src/renderer/components/downloads/PendingDownloadBanner.tsx
   @@ -40,11 +40,10 @@ export const PendingDownloadBanner: React.FC = () => {
      const handleResume = async () => {
        setDownloading(true);
        setPending(null);
   -    try {
   -      const result = await window.api.resumePendingDownload();
   -      if (result.success) {
   -        let unsub: () => void;
   -        const timeout = setTimeout(() => {
   -          setDownloading(false);
   -          unsub?.();
   -        }, 600_000);
   -        unsub = window.api.onDownloadAllProgress((data) => {
   -          if (data.total > 0 && data.done >= data.total) {
   -            clearTimeout(timeout);
   -            setDownloading(false);
   -            unsub();
   -          }
   -        });
   -      } else {
   -        setDownloading(false);
   -      }
   -    } catch {
   -      setDownloading(false);
   -    }
   +    // ...
   +    // Здесь отсутствует обработка результата resumePendingDownload
   +    // и логика подписки на onDownloadAllProgress.
   +    // Это может привести к тому, что isDownloading останется true,
   +    // если загрузка завершится или произойдет ошибка.
   +    // ...
      };

   Проблема: Логика в handleResume для обработки результата resumePendingDownload и подписки на onDownloadAllProgress была удалена. Это означает, что после вызова resumePendingDownload глобальное состояние isDownloading может остаться true даже после завершения или отмены загрузки, что приведет к некорректному поведению UI (баннер не появится, другие загрузки будут заблокированы).
   Исправление: Верни логику подписки на onDownloadAllProgress и обработку завершения/ошибки, чтобы корректно сбрасывать isDownloading.

2. src/renderer/components/playlists/QuickAddToPlaylistMenu.tsx - Избыточная инвалидация кэша

   --- a/src/renderer/components/playlists/QuickAddToPlaylistMenu.tsx
   +++ b/src/renderer/components/playlists/QuickAddToPlaylistMenu.tsx
   @@ -203,7 +203,7 @@ export const QuickAddToPlaylistMenu: React.FC<QuickAddToPlaylistMenuProps> = ({
      }
      invalidatePostPlaylists(effectivePostId, [playlistId]);
      onSuccess?.();
   -    } catch (error) {
   +    } catch (error: unknown) { // <-- Здесь
      log.error("[QuickAddToPlaylistMenu] Failed to toggle playlist:", error);
      setSelectedPlaylistIds(prevSet);
    }

   **

Analyzed by: gemini-2.5-flash

[github-actions]

🛡️ Архитектор (AI Review)

LGTM 🟢. Архитектура чиста, но я слежу за тобой.

Analyzed by: gemini-2.5-flash-lite