pipu-music/docs/reviews/2026-10-24-review-1.md

Project Review — 2026-10-24 (1)

Вывод и область проверки

Модульный монолит, contract-first API и общий core соответствуют масштабу проекта. После предыдущего рефакторинга у library/uploads/player появились полезные границы. Перестраивать проект целиком ради SOLID оснований нет. Приоритет — изоляция асинхронных операций при смене аккаунта, авторизация записей и согласованность состояния рекомендаций.

Проверены карта документации и архитектура, локальные AGENTS, выбранные потоки auth, playback/events, wave, social/listening, uploads/placement, library/schedule, web persistence/dialogs, Android offline/events/downloads/onboarding, desktop proxy, Python wave/API и текущий diff. Это статическое ревью перечисленных потоков с целевыми тестами, а не построчная проверка каждого файла или полный security-аудит. Физический Android, реальные аудиопотоки, NFS и производительность под нагрузкой не проверялись. Замечания ниже подтверждены чтением кода; отдельные новые воспроизводящие тесты для них в этой работе не добавлялись.

Prior Reviews Summary

В каталоге были два отчёта: 2026-10-22-review-1.md и 2026-10-21-review-1.md. Все 17 задач отмечены [x]; закрытые задачи повторно не открывались.

Still Open (carried forward)

Нет. Перенесено: 0.

Resolved Since Last Review

Нет ранее открытых задач для перепроверки. Подтверждено новых закрытий: 0.

Phase 1: Code Quality

SOLID

F7, P3 — WaveService собирает инфраструктуру и выполняет сценарий. Конструктор сам создаёт IndexHolder, SessionStore и RankerHolder, batch открывает PostgreSQL-соединения, загружает профиль, меняет Redis-сессию, вызывает алгоритм и пишет обучающие признаки. Изолированная проверка сценария требует подмены конкретных инфраструктурных деталей. Передавать узкие порты каталога/профиля, сессии и impression sink из _serve, оставив SQL/Redis в адаптерах. Не нужно оборачивать каждую математическую функцию интерфейсом. Источник: services/recommender/pipu_rec/wave/service.py:58, :79, :112; composition root: pipu_rec/cli.py:_serve.

Доказанных нарушений LSP в просмотренных реализациях не найдено. Остаточная длина Player и наличие конкретных адаптеров в composition root сами по себе не считаются дефектами. Закрытые замечания предыдущего SOLID-ревью не дублируются.

Performance

F5, P2 — размер исключений растёт без ограничения и нарушает внутренний контракт. Go servedIDs загружает все показы сессии и передаёт весь массив как exclude_ids, а Python BatchRequest ограничивает его 1000 элементами. После превышения 1000 показов запросы этой сессии получают 422, и Go каждый раз переходит на fallback. Это подтверждённый конфликт границ, а не результат нагрузочного замера. Согласовать bounded history/хранение исключений на стороне рекомендателя, сохранив политику неповторов. Источники: services/api/internal/wave/wave.go:291, :349; services/recommender/pipu_rec/wave/service.py:40.

Иных измеренных проблем производительности не установлено.

Correctness & Bugs

F1, P1 — незавершённая загрузка Android продолжает работу в новом аккаунте. attach меняет this.owner/server и очищает active; run захватывает старую session, но после сетевого await строит путь через изменяемый this.file и URL через this.abs, затем без проверки поколения снова вставляет DownloadResumable в active. Сценарий: медленный /tracks/batch → logout/смена сервера → старый ответ. Старая задача может писать в каталог нового владельца и занимать его очередь; её finally может удалить active новой задачи с тем же track ID. Проверка owner внутри status не защищает создание загрузки, progress callback и файловые операции. Захватывать контекст задачи (owner/server/пути), проверять поколение после await и удалять active только по идентичности задачи. Источник: apps/mobile/src/offline/downloads.ts:84, :228, :235, :261.

F3, P2 — поздний refresh восстанавливает устаревшую авторизацию. doRefresh без проверки версии вызывает apply; logout/signOut и новый login не инвалидируют уже отправленный refresh. Если успешный refresh был обработан сервером до logout, но его ответ задержался, он может после signOut вернуть локальный signed_in и записать старые токены в RefreshStore. Аналогично ответ старого refresh может перезаписать новый login. Нужны поколение auth-операций и отбрасывание устаревшего результата до изменения access/store/state. Источник: packages/core/src/auth.ts:143, :159, :167, :197.

Code Quality

Отдельных задач по косметике, размеру файлов и magic numbers не создано: они уступают проверенным нарушениям поведения.

Phase 2: Logical Consistency

Domain & Application Layer

F2, P1 — событие пользователя может изменить исход чужой wave-сессии. Клиент передаёт произвольный UUID в context_id; Accept передаёт его в MarkImpression, где UPDATE ограничен только sessionID/trackID. Проверки wave_sessions.user_id = p.UserID нет. Зная UUID чужой сессии и трека, авторизованный клиент может записать like/dislike/skip в чужой показ, влияя на метрики и обучающие данные. UUID усложняет угадывание, но не заменяет авторизацию. Передавать userID и ограничивать запись владельцем сессии в той же SQL-операции; добавить тест изоляции двух пользователей. Источники: services/api/internal/playback/events.go:75, services/api/internal/playback/store.go:354; контракт api/openapi.yaml:PlayEvent разрешает context_id как строку.

Data Flow

Сохранены положительные границы: core не импортирует платформенные реализации, uploads use cases отделены от pgx, read-проекции могут делать межмодульные JOIN. Целевая проверка pnpm check:boundaries проходит; это не доказательство отсутствия всех циклов импортов.

State Management

F4, P2 — drain реакций может перезаписать более свежий выбор. drain читает snapshot pending_reactions и отправляет его без координации с react. Возможен порядок: drain прочитал старый like → react успешно записал новый dislike и удалил pending → drain позже отправил старый like. Условие reaction IS ? защищает удаление SQLite-строки, но не порядок запросов к серверу. Сериализовать операции по track ID либо добавить версию и согласовать запись на сервере. Источник: apps/mobile/src/offline/events.ts:50, :65, :77, :82.

F6, P2 — параллельные запросы теряют обновления Redis-сессии волны. API запускает batch через asyncio.to_thread; каждый batch отдельно делает get → меняет session → put. Два запроса одной сессии могут прочитать одинаковый snapshot, выбрать пересекающиеся треки и последней записью затереть feedback/served/recent_artists первого. PostgreSQL-обновление tracks_served не защищает Redis и выбор кандидатов. Нужна сериализация/версионирование всей операции по session_id с учётом нескольких процессов, а не только локальный thread lock. Источники: services/recommender/pipu_rec/api/app.py:wave_batch, pipu_rec/wave/service.py:84, :106, pipu_rec/wave/store.py:SessionStore.

Consistency

Конфликт внутреннего контракта Go/Python описан как F5; отдельная дублирующая задача не создаётся.

Phase 3: UI/UX

Usability

F8, P2 — Android молча пропускает onboarding при ошибке его загрузки. После завершения запроса с ошибкой query.isPending становится false, data отсутствует, и !query.data?.required возвращает основной интерфейс без ошибки и retry. Для нового пользователя временный сбой выглядит как отсутствие настройки рекомендаций. Разделить unknown/error и подтверждённое required=false; предусмотреть retry и осознанный вход в offline-режим без блокирования скачанной музыки. Источник: apps/mobile/src/ui/Onboarding.tsx:29.

Visual & Consistency

Новых подтверждённых визуальных замечаний нет. Старые закрытые mobile-web задачи не переоткрывались. В этой работе браузер использовался для функциональных E2E; полного визуального аудита всех экранов не было.

Interaction & Feedback

В текущих висящих изменениях завершён lifecycle listening: закрытие диалога после отправки, отсутствие стрима до admission, отдельная ошибка listening, явный запуск на втором устройстве ведущего, защита от позднего GET после выхода и сохранение refresh во время запроса. Дополнительно исправлено освобождение пустой скрытой комнаты после отмены/истечения последнего запроса; музыка источника продолжает играть. Эти исправленные в рамках сдачи дефекты не входят в открытые Tasks.

Accessibility

У просмотренного web Dialog есть именование, focus trap, Escape и возврат фокуса; у Android Field/Button есть доступные имена/роли. Новых доказанных accessibility-дефектов не заявлено. Проверка screen reader и всех контрастов не проводилась.

Tasks

Новых открытых задач: 8. Перенесено: 0. Ранее открытых, исправленных после прошлых ревью: 0. Исправление всех найденных замечаний не входило в просьбу завершить уже висящие изменения listening.

• 1. [Bug] [P1, F1] Изолировать Download jobs по поколению owner/server, включая URL, пути, progress и active cleanup — apps/mobile/src/offline/downloads.ts line 261.
• 2. [Logic] [P1, F2] Проверять владельца wave-сессии при MarkImpression и покрыть изоляцию пользователей — services/api/internal/playback/store.go line 354.
• 3. [Bug] [P2, F3] Инвалидировать поздние refresh/apply после logout и нового login — packages/core/src/auth.ts line 197.
• 4. [Bug] [P2, F4] Согласовать порядок drain и новых реакций по track ID — apps/mobile/src/offline/events.ts line 82.
• 5. [Performance] [P2, F5] Согласовать bounded exclude history Go/Python, исключить постоянный fallback после 1000 показов — services/api/internal/wave/wave.go line 349.
• 6. [Logic] [P2, F6] Защитить read-modify-write wave-сессии от параллельных batch — services/recommender/pipu_rec/wave/service.py line 84.
• 7. [UX] [P2, F8] Отделить ошибку onboarding от required=false и дать retry/offline-путь — apps/mobile/src/ui/Onboarding.tsx line 29.
• 8. [SOLID] [P3, F7] Передавать узкие IO-порты в WaveService из composition root — services/recommender/pipu_rec/wave/service.py line 58.

Проверки и Git

• До изменений этого ревью: HEAD cb22f15; после успешного git fetch origin — HEAD и origin/main совпадали, ahead=0, behind=0.
• В рабочем дереве изначально: 13 изменённых tracked-файлов listening/тестов/переводов/docs, новый apps/web/e2e/listening.spec.ts, а также отдельные scripts/share-stats.py, artifacts/wave-palette/, services/api/api.exe. Последние три не менялись и не включаются в коммит listening.
• Core: 69 тестов прошли, в том числе новый тест освобождения пустой комнаты.
• Web: App + player/listening — 51 тест прошёл в итоговом прогоне. В первом запуске при параллельных проверках упал lossless-hint test; изолированно и в повторном прогоне проходит. Стабильность этого теста не считается доказанной одним повтором.
• Mobile: 6 тестов прошли. Web/mobile/core typecheck и i18n check прошли (1111 ключей × 2 языка).
• pnpm check:boundaries прошёл. git diff --check прошёл.
• Go: unit-тесты social/playback/wave и vet social прошли. Integration TestActivityRoomsAndChatPrivacy собирается, но SKIP: TEST_DATABASE_URL не задан. Docker daemon недоступен. Транзакционные сценарии на реальной БД этой проверкой не подтверждены.
• Playwright listening: 4 сценария прошли. Используется fake API; ошибки аудиопрокси ECONNREFUSED в сценариях не являются проверкой воспроизведения настоящего аудио.
• Публичный OpenAPI не менялся, генерация не требуется. Реальный Android/background и PostgreSQL остаются ограничениями проверки.