jellybit/docs/review.md

Ревью: настройка и журнал

Проектная часть конвейера ревью: чем jellybit отличается от абстрактного Go-сервиса и что здесь уже проскакивало. Устройство самого конвейера (метки, стадии, контракт находок) живёт в скилле, а не здесь.

Как настроен конвейер

Прежде чем задать вопрос, посмотри, не задан ли он уже машиной. Перечень механизированного — conventions/README.md → «Механизировано»; чем безусловно краснеет гейт и чего в нём намеренно нет — CLAUDE.md → «Гейт». Вопрос про уже проверенное вытесняет вопрос про непроверенное — места в прогоне столько же.

Severity не выводится проходом заново. Она стоит рядом с формулировкой инварианта в CLAUDE.md → «Инварианты», обратимость — там же в «Работа» → «Необратимое». Шкала ущерба берётся оттуда, а порядок ценностей — из security.md → «Что чувствительнее чего».

Типовые узлы

Рода узлов проекта и проверяемые свойства к каждому. Род, а не инвентарь пакетов: узел, которого ещё нет, но который проект заведёт, включён намеренно.

Клиент внешнего 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.

Операция с файловой системой (layout)

• целевой путь проверяется после filepath.Clean, на принадлежность библиотеке;
• существующая цель не перезаписывается ни при каком исходе;
• под paths.downloads нет ни одной операции записи или удаления;
• частичный сбой батча оставляет систему в состоянии, из которого повтор доводит начатое или откатывает целиком;
• удаление снимает только свои ссылки своего батча и не снимает последнюю копию.

Парсер недоверенного входа (magnet, torrent, парсер сообщения бота, разбор ответа LLM)

• вход враждебный по умолчанию: длина, вложенность, мусорные байты, пустота;
• разбор не паникует и не аллоцирует по числу из самого входа;
• невалидный вход даёт доменную ошибку, а не тихий дефолт;
• результат нормализуется на границе (lowercase hex, trim, ident.Parse).

Вызов LLM и разбор его ответа (recognize, naming.Derive)

Самый специфичный род узла в проекте: единственный, чей выход недетерминирован, недоверен и стоит денег одновременно.

• решение узла не является гейтом безопасности: инъекция в промпт считается состоявшейся, защита стоит ниже — на валидации целевого пути (security.md);
• самооценка модели не заменяет матч в метабазе — порог confidence стоит поверх матча дополнительным условием, а не вместо него;
• разбор ответа устойчив к лишним и недостающим полям, обрезке и не-JSON; негодный ответ даёт доменную ошибку, а не тихий дефолт;
• попытка стоит лимита и денег: число ретраев берётся из конфига, а фоновый цикл не запускает распознавание сам по себе повторно;
• тест не ходит в живой эндпоинт — провайдер за интерфейсом, ответ фикстурой; живые прогоны только за env-гейтом.

htmx-хендлер

• один партиал обслуживает страницу и фрагмент, ветвление по isHTMX;
• ошибка на htmx-пути — 200 плюс фрагмент, а не 4xx/5xx;
• страница деградирует без JS;
• поллинг самозавершается, когда наблюдать больше нечего.

Типовые ложноположительные

Находки, которые здесь выглядят убедительно и всегда неверны.

• «Веб-UI и REST без авторизации». Принятое решение под сегодняшний периметр — security.md. Дефектом станет только вместе с путём снаружи LAN.
• «Ошибка на htmx-пути возвращает 200». Так и задумано — conventions/web-ui.md.
• «Решение auto/review должно опираться на confidence модели». Неверно в форме «вместо матча»: авто только при подтверждённом матче в базе — ADR-2026-06-13-auto-link-requires-db-match. Самооценка LLM плохо откалибрована и поддаётся инъекции. Порог confidence при этом существует дополнительным блокирующим условием поверх матча — его наличие дефектом не является.
• «Копировать надёжнее, чем хардлинк» / «взять симлинк». Хардлинк — осознанный выбор ради неприкосновенности источника и недублирования диска, ADR-2026-06-13-hardlinks; copy — только фолбэк.
• «Целочисленный автоинкрементный ключ был бы проще». ULID — требование capability identity; AUTOINCREMENT вдобавок краснит гейт.
• «Не хватает метрик, трейсинга, health-эндпоинтов по каждой зависимости». «Минимум компонентов» — принцип проекта; глубокий healthcheck заведён задачей и ждёт своей очереди, а не является упущением.
• «Здесь нужен интерфейс, чтобы это можно было замокать». Единственная реализация за интерфейсом — обычно лишний слой; см. «Честный предел» ниже.
• «Нет ретрая у вызова в фоновом цикле». Тик повторится сам через poll_interval; ретрай внутри тика чаще вреден.
• «Оригинальное и локализованное названия дублируются — избыточность». original_title заполняется всегда и при неуверенности дублирует title — это контракт capability recognition, а не недосмотр.

Вопросы по темам

Форма: <тема>: <вопрос> (<провенанс>). Адресуется теме, а не имени прохода: проход переезжает между метками и упраздняется, тема переезд переживает. Задаёт вопрос тот, кто закрывает тему на текущем прогоне.

• security: можно ли, управляя только именами файлов в раздаче и текстом контекста, добиться целевого пути вне paths.movies/series — включая путь через юникод, длину сверх лимита ФС и коллизию после нормализации? (инвариант «целевой путь строго под библиотекой», security.md)
• security: есть ли последовательность команд, после которой снимается последняя копия данных — с учётом superseded-ссылок и гонки со сверкой? (инвариант «источник неприкосновенен»)
• security: что даёт крафт-магнет с чужим или подставным инфохэшем — присоединение к чужой активной загрузке, отравление владения? (открытая задача про идентичность инфохэшей)
• security: где признак «это наше» снимается с одной сущности, а действие применяется к другой — присутствие раздачи в qBittorrent против байтов на диске, запись в БД против файла, инфохэш против содержимого? (журнал, 2026-08-06: уборка своего торрента сносила чужие файлы)
• operations: что делает эта ветка, когда qBittorrent недоступен несколько минут подряд — сколько ERROR-строк в секунду и меняется ли состояние задач? (задача про ERROR-шторм фоновых циклов)
• operations: как это ведёт себя при сотне загрузок в базе и десятках тысяч file_link — есть ли запрос без индекса и полный проход по таблице? (задача про масштаб 100/1000, database.md → «Настройки»)
• operations: что остаётся на диске и в базе, если процесс убит посреди раскладки батча? (состояние linking и его восстановление)
• autotests: изменённые строки не просто исполнены тестом, а проверены — есть ли тест, который упал бы без этой правки? (diff-coverage в гейте меряет исполнение, отличить его от проверки машина не может)
• autotests: конкурентная часть правки проверена тестом, а не рассуждением? (-race без gcc уходит в SKIP, и тогда гонки не проверял никто — CLAUDE.md → «Гейт»)
• autotests: новый тест не ходит в живой qBittorrent, LLM, метабазу или Telegram — а если ходит, он за env-гейтом в *_integration_test.go? (CLAUDE.md → «Запреты»)
• conventions: логирующий чекпоинт один на операцию — или ошибка залогирована и возвращена вверх, где залогирована снова? (conventions/logging.md)
• conventions: новое поле конфига появилось в config.example.toml с описанием назначения, диапазона и единиц? (conventions/config.md)
• requirements: не завелось ли поведение, которого спека не заказывала — тихий дефолт, проглоченная ошибка, ретрай «на всякий случай», отброшенное поле?
• security: читается ли тело ответа внешнего сервиса целиком без предела — лимита на размер ответа LLM в проекте нет, и это единственный недоверенный канал, где предел не стоит (security.md → «Что вне модели»)
• operations: не удваивает ли новая ветка расход лимита метабаз и платного LLM — повтор, ретрай, «распознать заново» на том же входе? (кэша ответов нет, задача metadata-cache)
• operations: копится ли то, что заводит это изменение, без срока хранения — строки терминальных задач, сырые ответы, файлы? (авточистки в проекте нет, задача db-retention-cleanup)
• architecture: не появился ли второй способ делать то, что уже делается — второе место, где генерится время или id, второй парсер источника, вторая логика целевых имён мимо naming? (architecture.md → «Единые точки проекта»)

Триггеры метки

Уточняет умолчания конвейера, не отменяет их. Рабочее умолчание — medium; миграция схемы и публичный контракт метку не поднимают: их проверяют проходы, которые в medium и так есть. Списка три: два поднимают до large, по одному на ось, третий опускает до small.

Крупное здесь — про объём, сколько узлов и слоёв трогает изменение:

• перенос ответственности между worker, recognition, layout и store;
• новое состояние в графе переходов загрузки: оно тянет за собой воркер, спеку, отображение в веб-UI и боте и восстановление после рестарта;
• правка, идущая насквозь по цепочке приём → распознавание → раскладка;
• новый провайдер метабазы за существующим интерфейсом: клиент, поле конфига с образцом, слияние полей кандидата, ветка «провайдера нет».

Незнакомое здесь — про форму решения, которую предстоит нащупать по ходу:

• новый пакет internal/* или новая capability в openspec/specs/;

• новый транспорт приёма или уведомлений рядом с REST, веб-UI, ботом и CLI;

• заводится или меняется правило идентичности, слияния или разбора: ключ владения раздачей и сверка с qBittorrent (ident, internal/store, state-reconciliation); построение целевых путей и санитизация имён (internal/layout, naming); новый вид входа или новая ветка неоднозначности у разбора недоверенного — bencode, magnet, текст контекста, ответ LLM (internal/torrent, internal/magnet, internal/tgbot/parse.go); новый источник или новый победитель при конфликте в слиянии кандидата метабазы (internal/metadata, metadata-match); merge-раскладка при повторном добавлении раздачи.

• «Поведение, видимое снаружи» здесь включает тексты и карточки Telegram — для единственного пользователя это и есть интерфейс.

Место из перечня метку не поднимает — поднимает правило. Перечни выше отвечают «здесь такие правила водятся», а не «любая правка здесь идёт в large». Метку поднимает то, что даёт работу новому проходу: заводится ключ сравнения или меняется его состав; у разбора появляется новый вид входа или новая ветка неоднозначности; в слияние добавляется источник или меняется победитель при конфликте.

Мелкое здесь — опускает до small. Перечень закрытый, каждый пункт проверяется взглядом на дифф и дельта-спеку, любое сработавшее держит метку внизу:

• дельта-спека называет исход поимённо — сценарий уже говорит, что даёт вырожденный вход, и решать в коде нечего;
• новых сценариев в дельта-спеке нет — изменение уточняет уже описанное поведение, а не заказывает новое;
• правка сообщения, комментария, записи журнала, имени или теста в узле из перечней выше;
• сужение уже существующей нормализации без нового вида входа: вход остался тот же, изменился исход на одном его значении.

Отрицательный тест поверх перечня: что после мерджа не откатывается обратной правкой — миграция, формат на диске, публичный контракт, имя, — не small, каким бы маленьким ни был дифф.

Ориентир частоты. medium закрывает большинство задач, large рассчитана на 5–10% и приходится на крупную функциональность, а не на уборку: задача типа chore или fix, собранная из нитей прошлого ревью, идёт в small или medium, даже когда трогает файл из перечней выше. large чаще одной задачи на спринт означает ошибку в критерии, а не спринт из сложных задач.

Недоступно проверке

Не проверит ни один проход — принципиальная граница, по факту промаха не пересматривается.

• operations: история инцидентов на umbar и то, что уже ломалось в проде.
• operations: поведение таблицы SQLite под реальным объёмом и профилем нагрузки — реального профиля нет ни у кого, кроме сервера.
• architecture: завязка внешних потребителей (Jellyfin, закладки, чужие ссылки) на текущее поведение.
• architecture: суждение «этой функциональности не должно существовать».
• requirements: качество распознавания как таковое — правильно ли LLM определил фильм. Это вопрос тюнинга модели и промпта, а не ревью кода; размеченный корпус, по которому это можно было бы судить числом, решено не собирать (tasks/REJECTED.md, 2026-08-06).

Перестали проверять сознательно — пересматривается первым, как только что-то проскочило.

• conventions: идиоматичность Go — с 2026-08-04. Проектный проход idiom (поимённая сверка с положениями Effective Go, Go Code Review Comments, стайлгайдов Uber и Google) упразднён вместе с переездом конвейера в плагин (ADR-2026-08-04-review-pipeline-to-plugin); способные части переселены — в тему operations (эксперимент против поведения библиотеки и драйвера) и в architecture («не изобретаем ли то, что уже есть в библиотеке»). Различение «идиоматично против распространено» теперь не спрашивает никто. Класс обратимый: портит форму кода, не данные. Пересмотр — задача quality-review-agents.
• security, operations, architecture: на метках small и medium не проверяется ничто, требующее запуска, — построенных путей атаки, замеров и эксплуатационного постмортема там нет по устройству конвейера. Их даёт только large, а она приходится на 5–10% задач.

Журнал дефектов

Запись на каждый воспроизведённый дефект, сразу, а не ретроспективно: со временем теряется не факт, а причина непоймания. Проскочившие — эвал-сет для калибровки конвейера, выборка по пометке.

Форма:

ГГГГ-ММ-ДД — <краткое последствие> [проскочил|пойман]

• Где: путь:строка либо «конвейер, а не код»
• Симптом: как обнаружилось, кем и когда
• Причина: что на самом деле было не так
• Чем воспроизведён: тест, команда, замер — с числами
• Почему не поймали: только для проскочивших — какой проход обязан был найти и что ему помешало
• Что меняем: правило прохода, шаг гейта, конвенция, факт в документе проекта — либо «ничего, цена поимки выше цены дефекта»

Записи

Журнал заведён 2026-07-23 вместе с переработкой конвейера (ADR-2026-07-23-review-pipeline-generative); случаи до этой даты не восстанавливались — восстановленная постфактум причина непоймания недостоверна, а именно она и нужна.

2026-08-06 — уборка своего торрента после отмены сносит чужие файлы [проскочил]

• Где: internal/worker/worker.go:501-556 — гард :501-509, удаление :550. Норма — openspec/specs/download-tracking/spec.md, требование «Добавление пойманной загрузки в qBittorrent».
• Симптом: найден проходом adversary на ревью задачи dismiss-marker-lost (2026-08-06), не в эксплуатации. В проде не всплывал.
• Причина: гард «подтверждённое отсутствие непосредственно перед add» подтверждает отсутствие записи торрента в qBittorrent, но не отсутствие данных на диске. Пользователь, снявший раздачу из qBittorrent с сохранением файлов (download-tracking сама предписывает это как способ восстановления зависшей magnet-раздачи), и подавший тот же торрент заново, получает add, подхватывающий пред-существующие файлы. Отмена в окне между re-read и PromoteCatched даёт torrents/delete с deleteFiles=true по этим файлам. Исключение инварианта «источник неприкосновенен» покрывает «собственный торрент», а признак «своё» подменён на «торрента не было».
• Чем воспроизведён: тестом на фейковом клиенте qBittorrent во временном каталоге прогона (tmp/, не сохранён): Cancel в окне после add вызывает Delete(hashes, deleteFiles=true) при живом файле под paths.downloads, созданном до add. Тот же путь достижим через Dismiss — гейт Dismiss шире, а уборка срабатывает по состоянию (after.State != catched), а не по команде. Не прогонялся последний шаг — что боевой qBittorrent по deleteFiles=true физически сносит пред-существующий контент: в бой ходить запрещено, отсюда Confidence: medium.
• Почему не поймали: окно после add разбиралось как гонка (кто успел — отмена или промоушен) и проверялось на «не удалим ли чужой торрент». Вопрос «а если торрента нет, но данные есть» не задавал никто: ни один проход не спрашивал про асимметрию признака владения — признак снимается с одной сущности (запись в qBittorrent), а действие применяется к другой (байты на диске). Враждебный проход до этой задачи на данном коде не гонялся.
• Что меняем: в «Вопросы по темам» добавлен вопрос темы security про асимметрию признака владения. Сам дефект — задачей в беклоге, кандидат critical; спека state-reconciliation в том же изменении перестала утверждать, что уборка «данных пользователя не касается».

2026-08-06 — нормализация имени раздачи сама производила сентинел, который отбрасывала [пойман]

• Где: internal/torrent/torrent.go, функция displayName (введена изменением ingest-nits). Норма — openspec/specs/ingest/spec.md, требование «Вырожденное имя раздачи не считается именем».
• Симптом: найден на ревью изменения, до мерджа. В эксплуатации не был.
• Причина: сравнение с metainfo.NoName стояло до схлопывания пробельного. Имя " - " сравнение не проходило, а oneLine превращал его ровно в -, и вырожденное значение уезжало вниз по потоку всеми тремя путями: строкой названия в контексте распознавания, в source_ref (фолбек на имя присланного файла не срабатывал — строка непуста) и подсказкой вывода отображаемого имени. Против master это регрессия: там стояло strings.TrimSpace(...) перед сравнением с -, и фолбек работал.
• Чем воспроизведён: двумя независимыми падающими тестами — adversary прогнал приём на входах " - ", "-\n", "\t-", " -" и получил source_ref = "-" вместо Dune.torrent; reimpl принёс свой тест на разбор. В дереве остался табличный TestParseNoNameSentinelDropped.
• Почему не поймали раньше: ловить было нечему — дефект внесён этим же изменением и пойман тем же прогоном. Отмечено потому, что это эвал-сет наоборот: случай, где старшая метка окупилась. Три прохода из семи (specs, adversary, reimpl) нашли его независимо, и двое принесли оракул; проход code (конвенции) и гейт его не видели — порядок двух операций внутри функции не выражается ни правилом линтера, ни конвенцией.
• Что меняем: ничего в конвейере. Класс «нормализация и сравнение с константой идут в неверном порядке» дешевле ловить тестом на границе разбора, чем правилом; такой тест заведён. Наблюдение о самой библиотеке (что BestName() сентинел не синтезирует — исходное основание нити было неверным) записано в research/torrent-bencode-limits.md.