Files
pipu-music/docs/reviews/2026-10-22-review-1.md
T
mrleo1nid e158558da3
ci / module boundaries / boundaries (push) Successful in 6s
ci / api / api (push) Successful in 47s
ci / contract / contract (сгенерированный код совпадает с openapi.yaml) (push) Successful in 1m2s
ci / db / db (push) Successful in 1m4s
ci / docker / docker (образы собираются — их сервер собирает сам при обновлении) (push) Successful in 1m30s
ci / ts / ts (packages + web) (push) Successful in 2m54s
ci / mobile / mobile (Metro-бандл Android) (push) Successful in 1m14s
refactor: isolate playback and upload use cases
2026-10-22 13:52:16 +03:00

185 lines
30 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Стоимость работы агентов и план SOLID-рефакторинга — 2026-10-22
## Вывод и границы исходного исследования
Проект уже имеет полезные границы: приложения разделены, есть общий `packages/core`, контракт OpenAPI, интерфейсы auth, отдельный Python-рекомендатель. Основная проблема — не количество технологий, а широкий контекст локальных изменений: несколько крупных оркестраторов смешивают правила, IO и состояние, а часть правил повторяется на клиентах.
Проверены структура tracked-файлов, ключевые Go/TS/Python-компоненты, тесты, CI, документация и два примера изменений из git. Это архитектурное исследование с фокусом на трудоёмкости доработок, а не полный аудит безопасности, производительности или визуального UI. Непрочитанные исходники не считаются проверенными. На этапе исходного исследования код приложения не изменялся и тесты не запускались. Реализация и проверки следующего этапа описаны в конце отчёта.
Фактические токены, стоимость и повторные попытки прошлых сессий не исследованы: в материалах проверки нет соответствующей телеметрии. Ниже — подтверждённые свойства репозитория и объяснение того, как они могут увеличивать работу агента. Процент экономии без замера не установлен.
## Prior Reviews Summary
В `docs/reviews/` найден один предыдущий отчёт: `2026-10-21-review-1.md`. Все 9 задач уже отмечены `[x]`, их повторно не открывали. Открытых задач для переноса: 0. Вновь подтверждённых исправлений ранее открытых задач: 0.
## Измерения
Снимок: HEAD `c84f90e`; локальные посторонние untracked-файлы не включены и не изменены.
- 419 tracked-файлов.
- 260 исходников с расширениями `.go`, `.ts`, `.tsx`, `.py`, `.rs`, исключая каталоги `gen`, `generated` и файлы `.gen.go`: 39 605 строк, включая тесты.
- Из этих исходников 24 файла длиннее 400 строк, 9 — длиннее 600. Длина — сигнал для изучения обязанностей, не самостоятельное нарушение SOLID.
- `services/api/internal/api/gen/api.gen.go`: 11 234 строки.
- `packages/api-client/src/generated/schema.ts`: 4 599 строк.
- `pnpm-lock.yaml`: 8 523 строки.
- Итого эти три файла: 24 356 строк. Их полный вывод в контекст обычно избыточен; это не оценка числа токенов.
- `api/openapi.yaml`: 3 717 строк. Контракт нужен, но обычно достаточно конкретной операции и связанных схем.
Воспроизведение: список из `git ls-files`, подсчёт через `[System.IO.File]::ReadAllLines`, отбор расширений и указанных generated-путей. Это физические строки, без семантического анализа. Наличие файла в git не означает, что агент автоматически загружает его целиком.
## Почему работа агента может быть дорогой
### 1. Недостаточно локальной навигации
В проверенном дереве не найдены `AGENTS.md`; корневой README даёт общие команды, `docs/README.md` — карту продуктовых документов. Нет краткой карты «задача → владельцы кода → инварианты → целевые проверки». Новая сессия вынуждена восстанавливать её поиском.
Нужна короткая корневая инструкция и локальные инструкции по областям, с минимальным пересечением. Большая энциклопедия в корневом AGENTS сама увеличит постоянный контекст. Исключения для generated-кода должны означать «читать точечно при необходимости», а не запрещать проверку контракта.
### 2. Документация описывает частично проектируемую архитектуру как существующую
`docs/architecture.md:34`, `:63`, `:101`, `:123`, `:129` обещают River, sqlc, общение модулей через сервисные интерфейсы и отдельные внутренние YAML-спецификации. В проверенном `services/api/go.mod` нет River/sqlc, сканирование запускает `Scanner.Start` (`library/service.go:614`), SQL находится прямо в store/service, в `api/` найден публичный `openapi.yaml`.
Агенту приходится перепроверять описание. Документ следует разделить на «реализовано» и «планируется», описать действующие очереди/воркеры и не внедрять River/sqlc только ради соответствия старому тексту.
### 3. Слишком широкий контекст плеера
`apps/web/src/player/player.ts:84` — 896 строк: очередь, волна, события, HTMLAudioElement, crossfade, preloading, Web Audio, Media Session, Tauri media, localStorage и серверное восстановление. Изменение одной обязанности требует проверки остальных.
`apps/mobile/src/player/controller.ts:78` — 457 строк с похожей оркестрацией, но другим движком, офлайн-хранилищем и фоновым жизненным циклом. Одинаковые `WAVE_REFILL_AT`, `EARLY_SKIP_MS`, накопление feedback и `refillWave` находятся в обоих клиентах (`web:255`, `mobile:375`). Правку правила нужно сопоставлять в двух реализациях.
### 4. Границы backend непоследовательны
- `uploads/service.go:31–52` зависит от конкретных Store, library.Service и Scanner; `:328` обращается к `s.store.pool.Exec`, `:387` принимает `pgx.Tx` в бизнес-операции.
- `library/service.go:49` объединяет источники, подключение, шаблоны, preview, сканирование, расписание и health-check; всего 829 строк.
- `playback/handler.go:37` объединяет стриминг, персонализацию, события, состояние, историю и реакции; всего 609 строк, использует конкретный `library.Store`.
- `cmd/api/main.go:157–160`: DTO ссылается на playback Handler, а Handler получает DTO через замыкание. Это усложняет сборку и изолированное тестирование. Это не цикл импортов Go.
- `genius/service.go:27` сам создаёт внешний клиент, а также работает с БД и запускает worker. Прямая зависимость от внешнего клиента затрудняет его замену в тесте.
Чтение чужих таблиц в поисковой проекции само по себе допустимо. Главный приоритет — изолировать бизнес-операции и записи; не вводить универсальные репозитории для всех JOIN.
### 5. Проверки не всегда соответствуют границам изменений
Уже есть полезные unit-, integration- и E2E-тесты; CI API и PostgreSQL integration разделены. Но `apps/mobile/test/screens.test.ts` проверяет offlineWave/releaseNeed, а не Controller. В проверенном наборе `apps/mobile/test` нет сценариев жизненного цикла Controller.
`apps/web/src/player/audio.test.ts:15` подменяет внутреннее состояние Player через `Object.assign`. Это конкретный признак неудобной тестовой границы: тесту нужен маленький анализатор, но приходится собирать большой плеер и менять его внутренности.
Без быстрых поведенческих проверок изменяемого модуля агенту требуется больше рассуждений и более широкий прогон. Наличие тестов не доказывает покрытие всех гонок.
### 6. Часть ширины изменений оправдана продуктом
Добавление Genius (`73ad61e`) затронуло 34 файла, включая миграцию, оба клиента, переводы и generated-код. Это сквозная возможность, и само число файлов не доказывает плохую архитектуру. Добавление текущего трека в плейлист (`fb20258`) затронуло 8 файлов, не потребовало переделки backend.
Уменьшать нужно непредвиденную связанность и повторное открытие одних и тех же файлов, сохраняя необходимые контрактные, платформенные и локализационные изменения.
## Phase 1: Code Quality / SOLID
### S — одна причина для изменения
Выделить у веб-плеера `AudioEngine`, `AudioAnalysis`, `PlaybackPersistence`, `MediaControls`. Player сохранить как фасад с текущим публичным API. Сначала вынести наиболее независимые обязанности с сохранением поведения; не переносить весь плеер за один PR.
Разделить library по сценариям: управление источниками, шаблоны/preview, запуск сканов, расписание/проверки доступности. Простое разнесение методов того же Service по файлам улучшит поиск, но не уменьшит связанность без разделения зависимостей.
### O — расширение через ограниченные точки замены
Вынести одинаковую политику волны в `packages/core` как `WaveSession`/`WaveCoordinator`: session generation, feedback, пороги refill и отбрасывание устаревших ответов. Сетевой API передавать через небольшой `WaveGateway`.
Web и Android применяют результат к собственным очередям. Native queue, offline playback и особенности ExoPlayer остаются в Android-адаптере; единый абстрактный движок для всех платформ заранее не нужен.
### L — заменяемость
Доказанных нарушений LSP в исследованной области не обнаружено. Для новых адаптеров проверять одинаковые обещания: порядок результатов, ошибки, отмену и отсутствие воздействия устаревшей сессии. Синхронный HTMLAudioElement и асинхронный native player нельзя объявлять взаимозаменяемыми без явного контракта.
### I — узкие интерфейсы
`auth/store.go:43` объединяет пользователей, refresh-токены, администрирование, audit и настройки; `WithTx` возвращает тот же широкий Store. Это кандидат на последующее разделение по потребителям, но auth уже имеет рабочий интерфейс и тестовый memstore — приоритет ниже плеера/uploads.
Новые границы создавать по реально используемым операциям, например `SourceLookup`, `ScanStarter`, `PlacementCatalog`. Не копировать все методы конкретного Store в интерфейс.
### D — зависимость от операций, а не инфраструктуры
В uploads вынести SQL и pgx-транзакции в persistence-адаптер; сервису дать операции `UpdateMetadata`/`Approve`/`Reject` с согласованными результатами. Сохранить атомарность смены статуса, квот и метаданных решения. Интеграционный тест реальной БД обязателен для изменений транзакций.
Выделить персонализацию каталога из HTTP Handler в самостоятельный объект; DTO и playback получают его при сборке. Конкретные зависимости в composition root (`main.go`, `server/router.go`) нормальны, массовое оборачивание всех конструкторов интерфейсами пользы не даст.
### Performance / Correctness
Новые подтверждённые runtime-дефекты и измеренные проблемы производительности этим исследованием не установлены. Размер SQL-запроса или исходника не считается доказательством медленной работы. Метрики качества рекомендаций и сетевого воспроизведения не измерялись.
## Phase 2: Logical Consistency
Целевой путь backend: HTTP handler → операция приложения → узкие порты → PostgreSQL/файлы/HTTP. Границы транзакций остаются явными. Межмодульные операции используют контракты потребителя; read-модели могут сохранять обоснованные JOIN.
Целевой путь клиента: UI → фасад плеера/feature → общие правила → платформенный адаптер. Для каждого состояния нужен один владелец. На Android владельцем воспроизводимой очереди остаётся native player; общий core не становится второй конкурирующей очередью.
Generated-типы оставить на HTTP-границах. Не создавать копии всех DTO ради формального разделения слоёв. Независимую модель вводить там, где правило использует небольшой стабильный набор полей.
## Phase 3: UI/UX
`apps/web/src/pages/admin/UploadQueuePage.tsx` содержит экран, строку очереди, пакетное редактирование, редактирование файла, отклонение и настройки загрузок (`:30`, `:205`, `:353`, `:422`, `:503`, `:542`), всего 640 строк. Настройки экспортируются из файла страницы очереди, сетевые вызовы находятся в компонентах.
Вынести в `features/uploads`: API/queries, экран очереди, отдельные диалоги, настройки. Странице оставить сборку. Сохранять ключи React Query и правила invalidation; единый большой `useUploadsEverything` лишь переместит проблему.
Новых визуальных и accessibility-дефектов не заявлено: браузерная проверка не проводилась. Закрытые задачи мобильного UI из прошлого ревью не дублировались.
## Порядок внедрения и условия готовности
1. **Навигация и достоверность.** Короткие инструкции по областям, актуальная архитектура, карта локальных проверок. Для типовой задачи понятны 2–5 начальных файлов и команды без чтения всех docs. Это ориентир, а не запрет на более широкий поиск.
2. **Упростить Player, сохранив фасад.** Сначала persistence/media/analysis; отдельно engine/crossfade. Проверки: restore без autoplay, ошибка preload и повтор Play, пауза/seek, очистка обработчиков. Существующие тесты сохранить; новые нужны для поведения на новой границе.
3. **Общая политика волны.** Сначала зафиксировать сценарии, затем подключать web и Android по отдельности. Проверки: поздний ответ после смены сессии, retry feedback, refresh очереди, ранний skip. На устройстве — background playback и offline→online.
4. **Uploads.** Сначала выделить persistence-операции с сохранением транзакций, затем placement и worker-порты. Проверки: конкурирующие решения, квота, повторный import, восстановление после рестарта, очистка staging; результат на реальной БД и файловые побочные эффекты.
5. **Library / playback / catalog.** Разделить use cases и убрать DTO→HTTP Handler из персонализации. Сохранить HTTP-контракт, подписи stream URL и поведение X-Accel. Проверять персонализацию батча, реакции, history/state и fallback wave.
6. **Локализация UI и закрепление границ.** Разделить uploads feature. Добавить автоматическую проверку запрещённых зависимостей с точными исключениями для composition root и проекций. Отложить дробление auth до изменения соответствующей функциональности.
Каждый шаг — отдельное ревьюируемое изменение. Сначала выделение границ без изменения поведения, затем новые возможности. Generated-файлы обновлять штатными генераторами; публичный OpenAPI не дробить до появления измеримой проблемы и готового процесса сборки спецификации.
## Как проверить экономию
Взять сравнимые задачи до/после: изменение порога волны на обоих клиентах, изменение правила размещения upload, добавление поля настроек загрузок. Не сравнивать эти разные задачи между собой; фиксировать модель, режим рассуждения и применённые навыки.
Для каждой записывать input/cached input/output tokens, если доступны; отдельно число прочитанных файлов, объём вывода инструментов, повторные чтения, итерации исправлений, длительность проверок и затронутые модули. Сопоставлять распределение нескольких задач, а не один удачный запуск.
Немедленные меры: `rg` по символам, чтение диапазонов, ограниченный вывод, исключение generated/lock из общего исследования, целевые проверки на промежуточных шагах и необходимые полные проверки перед сдачей. Крупные patch/diff тоже читать сначала без generated, затем отдельно проверять генерацию.
Выбор workflow влияет независимо от кода: полный аудит полезен для такого исследования, но запуск всех его фаз на маленькой правке добавляет работы. Большой каталог навыков, системные инструкции и история чата также могут занимать контекст; их вклад и кэширование не измерены и не устраняются рефакторингом приложения.
## Tasks
Перенесено: 0. Новых задач: 8. Порядок — ожидаемая польза для локальности изменений; сроки и проценты экономии не оценивались.
- [x] 1. [CodeQuality] Добавить короткую карту работы агентов: области, точки входа, источники истины и целевые проверки; новые локальные AGENTS держать компактными — `README.md` line 36.
- [x] 2. [Logic] Привести архитектуру к реализации, явно отделить планы River/sqlc/internal specs — `docs/architecture.md` line 123.
- [x] 3. [SOLID] Сохранить Player как фасад, выделить persistence/media/analysis/audio с независимыми зависимостями и проверками поведения — `apps/web/src/player/player.ts` line 84.
- [x] 4. [SOLID] Вынести общую политику wave/feedback/refill в core, сохранив платформенные очереди и офлайн-адаптацию; покрыть смену сессии и retry — `apps/web/src/player/player.ts` line 255; `apps/mobile/src/player/controller.ts` line 375.
- [x] 5. [SOLID] Убрать SQL/pgx из uploads use cases, выделить атомарные persistence-операции и узкие порты библиотеки/сканера, сохранить квоты и статусные переходы — `services/api/internal/uploads/service.go` line 328.
- [x] 6. [SOLID] Разделить управление источниками, шаблоны и расписание сканов по обязанностям и зависимостям — `services/api/internal/library/service.go` line 49.
- [x] 7. [SOLID] Выделить персонализацию из playback Handler, убрать циклическую сборку DTO через замыкание, постепенно отделить playback use cases от HTTP — `services/api/cmd/api/main.go` line 157; `services/api/internal/playback/handler.go` line 37.
- [x] 8. [CodeQuality] Вынести uploads API/queries, диалоги и настройки в feature-модули с локальными тестами, оставить странице композицию — `apps/web/src/pages/admin/UploadQueuePage.tsx` line 30.
## Исправления — 2026-10-22
Реализованы все 8 задач по коду; внешняя проверка PostgreSQL и физического Android остаётся отдельным условием валидации. Исходные метрики и ссылки выше описывают состояние до исправлений.
1. Добавлены компактные корневой и локальные `AGENTS.md`; README ссылается на карту работы. Общая инструкция не загружает всю документацию.
2. Архитектура описывает реальные pgx-запросы, in-process scanner и статусные upload jobs; River/sqlc/internal YAML отделены от реализации. Добавлена актуальная карта компонентов.
3. Player сохраняет прежний интерфейс, а аудиограф/элементы, анализ, Media Session/Tauri и сохранение выделены в `engine.ts`, `analysis.ts`, `media.ts`, `persistence.ts`. Типы состояния отделены. Обработчики аудио и media освобождаются, отложенное сохранение отменяется при dispose. Player: 896 → 617 строк; crossfade и оркестрация очереди остаются в фасаде.
4. `WaveCoordinator` в core обслуживает пороги, feedback и версии запросов на обеих платформах. Поздние ответы/ошибки не меняют новую сессию; смена контекста сохраняет ещё не отправленный feedback. На Android добавлены проверки Controller после reset и ручного выбора списка, native queue остаётся владельцем воспроизведения.
5. SQL и pgx убраны из uploads service/worker. `persistence.go` предоставляет атомарные операции изменения метаданных, решений и завершения импорта; транзакционный механизм Transition сохранён. Выделен PlacementPlanner и порты SourceCatalog/RootResolver/ImportedScanner. Guarded UPDATE теперь сообщает конфликт, если статус успел измениться. Добавлены integration-проверки rollback решения, конкурирующих отказов, quota и повторного завершения импорта; без PostgreSQL они не исполнялись.
6. Library Service стал фасадом над SourceService, TemplateService, ScanService и Scheduler. Зависимости разделены интерфейсами. Результат health-check передаётся через канал, исключая общую запись из timeout и worker веток.
7. Personalizer отделён от HTTP Handler, DTO собирается напрямую без замыкания. Приём событий выделен в EventService; остальные playback-сценарии остаются в существующем handler для постепенного дальнейшего разделения. Unit-тесты проверяют батч персонализации, недоступные треки и ошибки чтения.
8. Страница очереди стала оболочкой из 3 строк; feature содержит экран (181 строка), отдельные диалоги, строки списка, настройки, API и queries. Ключи запросов и invalidation сохранены. Локальные тесты проверяют сохранение метаданных и сохранность формы при конфликте.
Дополнительно: `pnpm check:boundaries` и отдельный CI workflow проверяют введённые границы без сборки всего проекта. Проверка целевая, не заменяет полноценный анализ графа импортов.
### Проверки
- Web typecheck и production build — успешно.
- Web unit/component suite — 51 тест успешно; в том числе restore без autoplay, ошибки preload и новые диалоги.
- Playwright — 10 сценариев успешно: desktop и mobile layouts, очередь/диалоги, навигация и плеер.
- Core — 39 тестов успешно, включая 4 сценария WaveCoordinator.
- Android typecheck — успешно; unit-тесты — 5 успешно, включая 2 сценария Controller с заглушкой native-плеера.
- `go test ./...` и `go vet ./...` — успешно. Новые unit-тесты library/playback проходят.
- Integration build — успешно; `TestMigrationsUpDownUp`, `TestLibraryAndUploads`, `TestPlaylistPersistenceAndIsolation`, `TestUserJourney` явно **SKIP**: не задан `TEST_DATABASE_URL`, Docker daemon недоступен. Результат не подтверждает транзакции на реальной БД.
- Физический Android/background/offline→online не проверялся.
- Публичный OpenAPI и generated-код не менялись. Экономия токенов после рефакторинга ещё не измерена.