av
42d5b73a04
- Раскладка docs/ приведена к канону 2: заведены passport/architecture/ database/security/review и research; docs/specs, drafts, backlog, review/ и BRIEF.md разобраны и удалены, беклог переехал в docs/tasks (34 задачи, 6 целей, слаги на английский). - Нарративы specs удалены как дубли openspec-спек после поимённой сверки; остаток заведён задачами (редактор маппинга ревью, крайние случаи именования), отказ от сущности title промоутнут в ADR. - Проектные копии агентов и скиллов ревью/пайплайна удалены в пользу плагинов av-dev-pm и av-dev-pipeline; в task gate добавлен шаг canon вместо er-schema.
202 lines
16 KiB
Markdown
202 lines
16 KiB
Markdown
# Ревью: настройка и журнал
|
||
|
||
Проектная часть конвейера ревью: чем jellybit отличается от абстрактного
|
||
Go-сервиса и что здесь уже проскакивало. Устройство самого конвейера (профили,
|
||
стадии, контракт находок) живёт в скилле, а не здесь.
|
||
|
||
## Как настроен конвейер
|
||
|
||
### Типовые узлы
|
||
|
||
Рода узлов проекта и проверяемые свойства к каждому. Род, а не инвентарь
|
||
пакетов: узел, которого ещё нет, но который проект заведёт, включён намеренно.
|
||
|
||
**Клиент внешнего HTTP-сервиса** (`qbt`, `llm`, `metadata`, `jellyfin`, `tgbot`)
|
||
|
||
- у каждого исходящего вызова свой таймаут из конфига, не дефолт транспорта;
|
||
- `context` доходит до запроса и отменяет его, а не игнорируется;
|
||
- ошибка зависимости отличима от ошибки нашей логики на приёме результата;
|
||
- секреты (пароль, ключ, токен) не попадают ни в лог, ни в текст ошибки;
|
||
- недоступность **опциональной** зависимости (метабаза, Jellyfin, Telegram) не
|
||
двигает состояние загрузки и не краснеет ERROR-ом в фоновом цикле.
|
||
|
||
**Тик воркера и переход состояния**
|
||
|
||
- переход легален по декларативному графу, а не «просто присвоили `state`»;
|
||
- работа идёт под per-download блокировкой; два транспорта не гонятся;
|
||
- тик идемпотентен: повтор на том же состоянии не порождает второго эффекта;
|
||
- отмена контекста на середине не оставляет полуприменённого состояния;
|
||
- новое промежуточное состояние имеет выход **и** предохранитель по времени.
|
||
|
||
**Репозиторий `store`**
|
||
|
||
- запрос параметризован, время только через `store.Now()`, id через `ident`;
|
||
- многошаговое изменение — в одной write-транзакции (`_txlock=immediate`);
|
||
- инвариант, не выражаемый схемой («одна активная загрузка на infohash»),
|
||
держится guarded-методом, а не проверкой в вызывающем коде;
|
||
- миграция forward-only и сопровождается правкой [database.md](database.md).
|
||
|
||
**Операция с файловой системой** (`layout`)
|
||
|
||
- целевой путь проверяется **после** `filepath.Clean`, на принадлежность
|
||
библиотеке;
|
||
- существующая цель не перезаписывается ни при каком исходе;
|
||
- под `paths.downloads` нет ни одной операции записи или удаления;
|
||
- частичный сбой батча оставляет систему в состоянии, из которого повтор
|
||
доводит начатое или откатывает целиком;
|
||
- удаление снимает только свои ссылки своего батча и не снимает последнюю копию.
|
||
|
||
**Парсер недоверенного входа** (`magnet`, `torrent`, парсер сообщения бота,
|
||
разбор ответа LLM)
|
||
|
||
- вход враждебный по умолчанию: длина, вложенность, мусорные байты, пустота;
|
||
- разбор не паникует и не аллоцирует по числу из самого входа;
|
||
- невалидный вход даёт доменную ошибку, а не тихий дефолт;
|
||
- результат нормализуется на границе (lowercase hex, trim, `ident.Parse`).
|
||
|
||
**htmx-хендлер**
|
||
|
||
- один партиал обслуживает страницу и фрагмент, ветвление по `isHTMX`;
|
||
- ошибка на htmx-пути — 200 плюс фрагмент, а не 4xx/5xx;
|
||
- страница деградирует без JS;
|
||
- поллинг самозавершается, когда наблюдать больше нечего.
|
||
|
||
### Типовые ложноположительные
|
||
|
||
Находки, которые здесь выглядят убедительно и всегда неверны.
|
||
|
||
- **«Веб-UI и REST без авторизации».** Принятое решение под сегодняшний
|
||
периметр — [security.md](security.md). Дефектом станет только вместе с путём
|
||
снаружи LAN.
|
||
- **«Ошибка на htmx-пути возвращает 200».** Так и задумано —
|
||
[conventions/web-ui.md](conventions/web-ui.md).
|
||
- **«Решение auto/review должно опираться на `confidence` модели».** Наоборот:
|
||
авто только при подтверждённом матче в базе —
|
||
[ADR-2026-06-13-auto-link-requires-db-match](adr/ADR-2026-06-13-auto-link-requires-db-match.md).
|
||
Самооценка LLM плохо откалибрована и поддаётся инъекции.
|
||
- **«Копировать надёжнее, чем хардлинк» / «взять симлинк».** Хардлинк —
|
||
осознанный выбор ради неприкосновенности источника и недублирования диска,
|
||
[ADR-2026-06-13-hardlinks](adr/ADR-2026-06-13-hardlinks.md); copy — только
|
||
фолбэк.
|
||
- **«Целочисленный автоинкрементный ключ был бы проще».** ULID — требование
|
||
capability `identity`; `AUTOINCREMENT` вдобавок краснит гейт.
|
||
- **«Не хватает метрик, трейсинга, health-эндпоинтов по каждой зависимости».**
|
||
«Минимум компонентов» — принцип проекта; глубокий healthcheck заведён задачей
|
||
и ждёт своей очереди, а не является упущением.
|
||
- **«Здесь нужен интерфейс, чтобы это можно было замокать».** Единственная
|
||
реализация за интерфейсом — обычно лишний слой; см. «Честный предел» ниже.
|
||
- **«Нет ретрая у вызова в фоновом цикле».** Тик повторится сам через
|
||
`poll_interval`; ретрай внутри тика чаще вреден.
|
||
- **«Оригинальное и локализованное названия дублируются — избыточность».**
|
||
`original_title` заполняется всегда и при неуверенности дублирует `title` —
|
||
это контракт capability `recognition`, а не недосмотр.
|
||
|
||
### Вопросы к проходам
|
||
|
||
Форма: `<имя прохода>: <вопрос> (<провенанс>)`. Журнал дефектов пока пуст,
|
||
поэтому провенанс у всех пунктов — инвариант или ADR, а не пойманный случай;
|
||
по мере накопления журнала список должен смещаться в сторону реальных промахов.
|
||
|
||
- `adversary`: можно ли, управляя только именами файлов в раздаче и текстом
|
||
контекста, добиться целевого пути вне `paths.movies`/`series` — включая путь
|
||
через юникод, длину сверх лимита ФС и коллизию после нормализации?
|
||
(инвариант «целевой путь строго под библиотекой», [security.md](security.md))
|
||
- `adversary`: есть ли последовательность команд, после которой снимается
|
||
**последняя** копия данных — с учётом `superseded`-ссылок и гонки со сверкой?
|
||
(инвариант «источник неприкосновенен»)
|
||
- `adversary`: что даёт крафт-магнет с чужим или подставным инфохэшем —
|
||
присоединение к чужой активной загрузке, отравление владения?
|
||
(открытая задача про идентичность инфохэшей)
|
||
- `ops`: что делает эта ветка, когда qBittorrent недоступен несколько минут
|
||
подряд — сколько ERROR-строк в секунду и меняется ли состояние задач?
|
||
(задача про ERROR-шторм фоновых циклов)
|
||
- `ops`: как это ведёт себя при сотне загрузок в базе и десятках тысяч
|
||
`file_link` — есть ли запрос без индекса и полный проход по таблице?
|
||
(задача про масштаб 100/1000, [database.md](database.md) → «Настройки»)
|
||
- `ops`: что остаётся на диске и в базе, если процесс убит посреди раскладки
|
||
батча? (состояние `linking` и его восстановление)
|
||
- `code`: логирующий чекпоинт один на операцию — или ошибка залогирована и
|
||
возвращена вверх, где залогирована снова?
|
||
([conventions/logging.md](conventions/logging.md))
|
||
- `code`: новое поле конфига появилось в `config.example.toml` с описанием
|
||
назначения, диапазона и единиц? ([conventions/config.md](conventions/config.md))
|
||
- `specs`: не завелось ли поведение, которого спека не заказывала — тихий
|
||
дефолт, проглоченная ошибка, ретрай «на всякий случай», отброшенное поле?
|
||
- `architecture`: не появился ли второй способ делать то, что уже делается —
|
||
второе место, где генерится время или id, второй парсер источника, вторая
|
||
логика целевых имён мимо `naming`?
|
||
([architecture.md](architecture.md) → «Единые точки проекта»)
|
||
|
||
### Триггеры профиля
|
||
|
||
Уточняет умолчания конвейера, не отменяет их.
|
||
|
||
- **`deep`** — есть миграция в `internal/store/migrations/`; появляется новый
|
||
пакет `internal/*`; меняется сигнатура публичной команды воркера; трогается
|
||
раскладка файлов, построение целевых путей или удаление ссылок; трогается
|
||
разбор недоверенного входа.
|
||
- **`standard`** — меняется поведение, видимое снаружи: REST-эндпоинт,
|
||
htmx-путь, набор или семантика состояний загрузки, формат сообщения бота,
|
||
поле конфига.
|
||
- **`quick`** — всё остальное: локальный багфикс, документация, тесты.
|
||
- **Независимая реализация (`reimpl`)** запускается, когда узел одновременно
|
||
новый и имеет внешний оракул в виде спеки: новый парсер, новый провайдер за
|
||
существующим интерфейсом, новая стадия конвейера распознавания.
|
||
- «Поведение, видимое снаружи» здесь включает **тексты и карточки Telegram** —
|
||
для единственного пользователя это и есть интерфейс.
|
||
|
||
### Недоступно проверке
|
||
|
||
**Не проверит ни один проход** — принципиальная граница, по факту промаха не
|
||
пересматривается.
|
||
|
||
- История инцидентов на umbar и то, что уже ломалось в проде.
|
||
- Поведение таблицы SQLite под реальным объёмом и профилем нагрузки: реального
|
||
профиля нет ни у кого, кроме сервера.
|
||
- Завязка внешних потребителей (Jellyfin, закладки, чужие ссылки) на текущее
|
||
поведение.
|
||
- Качество распознавания как таковое: правильно ли LLM определил фильм — вопрос
|
||
eval-харнеса и корпуса кейсов, а не ревью кода.
|
||
- Суждение «этой функциональности не должно существовать».
|
||
|
||
**Перестали проверять сознательно** — пересматривается первым, как только
|
||
что-то проскочило.
|
||
|
||
- **Идиоматичность Go — с 2026-08-04.** Проектный проход `idiom` (поимённая
|
||
сверка с положениями Effective Go, Go Code Review Comments, стайлгайдов Uber
|
||
и Google) удалён вместе с проектными копиями агентов при переезде на плагин
|
||
`av-dev-pipeline`, который этот проход упразднил. Способные части переселены:
|
||
эксперимент против поведения библиотеки и драйвера — в `ops`, «не изобретаем
|
||
ли то, что уже есть в библиотеке» — в `architecture`. **Различение
|
||
«идиоматично против распространено» теперь не спрашивает никто.** Класс
|
||
обратимый: портит форму кода, не данные. Пересмотр — задача
|
||
`quality-review-agents`.
|
||
|
||
## Журнал дефектов
|
||
|
||
Запись на каждый воспроизведённый дефект, **сразу**, а не ретроспективно: со
|
||
временем теряется не факт, а причина непоймания. Проскочившие — эвал-сет для
|
||
калибровки конвейера, выборка по пометке.
|
||
|
||
Форма:
|
||
|
||
<!-- копия: журнал-дефектов-форма из av-dev-pipeline/skills/review-pipeline/references/review-journal.md -->
|
||
## ГГГГ-ММ-ДД — <краткое последствие> [проскочил|пойман]
|
||
|
||
- **Где:** путь:строка либо «конвейер, а не код»
|
||
- **Симптом:** как обнаружилось, кем и когда
|
||
- **Причина:** что на самом деле было не так
|
||
- **Чем воспроизведён:** тест, команда, замер — с числами
|
||
- **Почему не поймали:** только для проскочивших — какой проход обязан был найти
|
||
и что ему помешало
|
||
- **Что меняем:** правило прохода, шаг гейта, конвенция, факт в документе
|
||
проекта — либо «ничего, цена поимки выше цены дефекта»
|
||
<!-- /копия: журнал-дефектов-форма -->
|
||
|
||
### Записи
|
||
|
||
Пока пусто. Журнал заведён 2026-07-23 вместе с переработкой конвейера
|
||
([ADR-2026-07-23-review-pipeline-generative](adr/ADR-2026-07-23-review-pipeline-generative.md));
|
||
случаи до этой даты не восстанавливались — восстановленная постфактум причина
|
||
непоймания недостоверна, а именно она и нужна.
|