Files
jellybit/openspec/changes/archive/2026-08-06-ingest-nits/design.md
T
av d081ef1d30 ingest: закрыты мелочи приёма — вырожденное имя, контракт Result, корреляция add
- имя раздачи нормализуется на границе разбора: вырожденное `-`
  (metainfo.NoName) даёт пустое имя, пробельное схлопывается — сентинел больше
  не доходит ни до контекста распознавания, ни до source_ref, ни до подсказки
  вывода имени
- контракт «на любом пути ошибки приёма результат нулевой» объявлен в ingest и
  удерживается структурно; три транспорта перестали обещать идентификатор,
  которого нет, и коррелируют отказ по request_id
- scoped-логгер загрузки ставится до вызова внешнего сервиса в семи командах
  воркера — записи об отказе qBittorrent и метабаз получили download_id
  и infohash; граница разбора bencode записана в docs/research
2026-08-06 18:20:12 +03:00

29 KiB
Raw Blame History

Context

Ревью приёма 2026-07-08 (Fable) оставило четыре нити — N1, N3, N4, N5. Они не связаны причиной, но связаны местом: все четыре живут на пути «источник → загрузка → добавление в qBittorrent» и все четыре читаются одним заходом. Изменение косметическое по последствиям и трогает пять пакетов, поэтому решения о месте правки важнее самих правок.

Текущее состояние, проверенное по коду:

  • internal/torrent/torrent.go:72DisplayName: meta.BestName(). Постановка задачи и первая редакция этого дизайна исходили из неверного факта — будто библиотека сама подставляет - безымянной раздаче. Проверено по исходникам anacrolix/torrent@v1.61.0: BestName() (metainfo/info.go:200-205) возвращает NameUtf8, иначе Name, иначе пустую строку; NoName = "-" (info.go:44) присваивается только в BuildFromFilePath при авторинге раздачи из вырожденного пути. Значит - доходит до нас лишь тогда, когда раздача объявила его полем name сама (та самая конвенция «имени нет», ради совместимости с которой библиотека константу и экспортирует). Раздача без поля name даёт пустое имя, и этот случай код уже отрабатывает верно. Потребителей DisplayName три: Info.Context() (строка названия), ingest.parse (source_ref, единственный, кто про - знает) и worker.sourceAddParts (подсказка имени для namer, то есть вход LLM).
  • internal/ingest/ingest.go — все выходы с ошибкой возвращают Result{} (строки 7475, 86, 106–107 на момент ревью). Комментарии internal/httpapi/httpapi.go:574 и internal/tgbot/bot.go:256 описывают прежний контракт «сбой после создания задачи → непустой DownloadID».
  • internal/qbt/qbt.go:246 — ветка Fails. пишет call.Failure со счётчиками urls/torrents. Логгер клиент берёт из ctx (logctx.FromOr). Из двух вызовов qbt.Add фоновый (worker.go:511) идёт со scoped-логгером (cctx, worker.go:415), а Retry (worker.go:1084) — с сырым ctx транспорта: Retry не присваивает ctx = w.scoped(…), в отличие от Delete (review.go:619), и трижды строит scoped-логгер одноразово прямо в аргументе записи. Первая редакция этого дизайна считала Retry единственной такой командой — ревью изменения показало, что их семь (см. «Что изменилось после ревью изменения»).
  • anacrolix/torrent v1.61.0: bencode/decode.go:17 задаёт DefaultDecodeMaxStrLen = 1<<27-1; parseStringLength (там же, :211) сверяет объявленную длину с этим потолком, а parseString (:250, :258) делает make([]byte, length) до чтения байтов. metainfo.Load (metainfo/metainfo.go:35-37) создаёт декодер без переопределения MaxStrLen, то есть с потолком по умолчанию.

Goals / Non-Goals

Goals:

  • Сентинел «без имени» не покидает пакет разбора — ни одному потребителю ниже по потоку не нужно про него знать.
  • Комментарии транспортов описывают контракт Ingest, который есть, и контракт подтверждён тестом, а не только чтением.
  • Запись о неуспешном добавлении в qBittorrent коррелируется с загрузкой на обоих путях добавления.
  • Знание о пределе аллокации bencode записано с провенансом и замером.

Non-Goals:

  • Не чиним поведение anacrolix/torrent — форк, replace и собственный разборщик bencode вне объёма и вне границы домена.
  • Не добавляем клиенту qbt знание о доменных сущностях (инфохэш, id загрузки) — см. решение 3.
  • Не трогаем files() и его фолбек на meta.BestName() для пути файла: это вырожденный случай (файл без пути и раздача без имени), к контексту распознавания отношения не имеющий.
  • Не переписываем Cancel/Dismiss под ту же форму, что Retry: внешних вызовов у них нет, требование identity их не касается, правка была бы churn'ом (записано в «Открытые вопросы»).

Decisions

1. N1 — нормализация имени стоит в torrent.Parse, а не в Context()

Задача формулировала фикс как «фильтровать - и в Context()». Это лечит симптом: DisplayName остаётся заражённым, а знание о вырожденном имени копируется в третье место. Потребителей у поля три, и третий (worker.sourceAddParts → подсказка имени для LLM) в постановке не назван, хотя страдает так же: - уезжает во вход вывода отображаемого имени.

Решение: нормализовать в ParseDisplayName пуст, если разобранное имя равно metainfo.NoName. Тогда Context() уже имеет проверку name != "" и не меняется вовсе, ingest.parse теряет ветку || ref == "-", а подсказка имени чинится без единой строки в worker.

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

Сравнение делаем с metainfo.NoName, а не со строковым литералом, по одной причине — библиотека экспортирует константу именно затем, чтобы на неё ссылались («By exposing it in the API we can check for references to this behaviour», metainfo/info.go:41-44). Аргумент «сломается компиляция, а не поведение» здесь неверен и снят: константа строковая, смена её значения компиляцию не ломает.

Тем же местом закрывается и вторая грязь в строке названия: имя — недоверенный вход, а контекст читается построчно, поэтому имя со встроенным переводом строки добавляет в контекст строку, выглядящую как синтезированный нами факт. Схлопывание разделителей строк уже применяется к комментарию торрента (oneLine), и распространить его на имя — одна строка. Инъекцией в промпт это не управляет и управлять не может: пользовательский текст контекста многострочен по замыслу и идёт в промпт как есть; правило устраняет несогласованность (комментарий схлопываем, имя нет), а не заводит защиту.

1а. Что изменилось после ревью предложения

Проход specs опроверг фактическое основание N1 оракулом — прогоном крафт-входов через metainfo.Load. Из этого следует три правки, уже внесённые выше и в дельта-спеку:

  • формулировка требования говорит про объявленное раздачей вырожденное имя, а не про сентинел, подставляемый библиотекой;
  • случай «раздача без поля name» назван отдельно и признан уже работающим — под него заводится сценарий и тест, потому что раньше он путался с первым (комментарий internal/torrent/torrent_test.go:224 «info без имени (NoName-сентинел)» и есть источник исходной ошибки ревью 2026-07-08);
  • критерий приёмки постановки «для безымянного торрента Context() не отдаёт -» выполняется до изменения. Это дефект критерия, а не задачи: проверяемое им поведение уже верно, а чинится соседнее. В докладе исход критерия назван честно, и заодно проверено то, что критерий имел в виду.

2. N3 — контракт объявляется в ingest, транспорты его не пересказывают

Комментарии в httpapi и tgbot разошлись с кодом потому, что каждый держал свою копию контракта. Правка «переписать оба комментария» повторяет ту же конструкцию и разойдётся снова.

Решение: контракт («на любом пути ошибки возвращается нулевой результат») объявляется один раз — в доке ingest.Ingest и ingest.Result — и подтверждается тестом в пакете ingest. Транспорты получают короткий комментарий-ссылку и передают в диагностику пустой идентификатор, а не заведомо пустой res.DownloadID: значение, которое доказуемо всегда пусто, прочитанное как «а вдруг непусто», и породило исходную нить.

Альтернатива «оставить res.DownloadID, поправить только текст» отвергнута: код продолжит утверждать обратное тому, что говорит комментарий.

Контракт держится структурой, а не перечнем веток (найдено проходом rubric): Ingest получает именованный возврат и один defer, обнуляющий результат при ненулевой ошибке. Иначе лекарство повторяет болезнь — сегодня три ветки возврата аккуратны, а завтра четвёртая вернёт полузаполненный результат, и тест, перечисляющий три известных класса отказа, этого не заметит. Тест сравнивает результат с нулевым значением целиком, а не по полю DownloadID.

Вызовов приёма на транспортах три, а не два (найдено проходом specs): REST handleAPIAdd, веб-форма handleUIAdd и tgbot.ingestAndReply. Третий в первой редакции задач отсутствовал — добавлен, иначе требование «транспорты не обещают идентификатора, которого нет» оказалось бы выполненным на две трети.

Корреляционный ключ у отказа приёма разный по транспортам, и требование называет это прямо: HTTP и веб-UI дают request_id, Telegram не даёт ничего (opErr при пустом идентификаторе возвращает текст без ключа). Это не регрессия — сегодня res.DownloadID уже всегда пуст, и наблюдаемое поведение Telegram не меняется. Но пробел реален, и заводить ли Telegram-транспорту собственный ключ — записанный вопрос, а не молчаливое «потом».

3. N4 — корреляция приходит из ctx, полей клиенту не добавляем

Задача предлагала «логировать хеши/первые байты». Клиент qbt инфохэша не знает: на magnet-ветке у него ссылка, на torrent-ветке — байты, и вычислить хеш он мог бы только собственным разбором источника, а разбор источника — единая точка проекта (internal/magnet, internal/torrent). Поле Infohash в AddRequest завело бы второй канал корреляции рядом со scoped-логгером и жило бы только ради лога.

Настоящая причина того, что оператор не находит запись: Retry не кладёт scoped-логгер загрузки в ctx. Фоновый путь добавления это делает и запись Fails. там уже несёт download_id и infohash; путь retry — нет. То есть N4 — не «клиент мало пишет», а «один вызывающий не выполнил обязанность, которую выполняют остальные».

Решение: Retry присваивает ctx = w.scoped(ctx, capReview, id, d.PrimaryInfohash()) сразу после чтения загрузки, а одноразовые w.scoped(…) и ручные w.log.… внутри него схлопываются в logctx.From(ctx). Обязанность вызывающего закрепляется требованием в identityограниченным вызовом внешнего сервиса, а не «первым вызовом, способным что-либо записать» (найдено проходами architecture и specs): в широкой формулировке требование объявляло бы нарушителями Cancel и Dismiss, которые этот же change сознательно не трогает, и нормативная спека стала бы ложной в момент архивации.

Побочный эффект присвоения назван заранее, чтобы на чекпоинте 2 он не читался регрессией: поля capability/download_id/infohash появятся также у qbt.Torrents из torrentByInfohash, у logCmd и у предупреждений внутри метода. Это и есть цель. Двойной download_id в logCmd (из логгера и явным аргументом) — существующее поведение пути Delete, прецедент есть, чистить его здесь не будем.

Тест записи о Fails. проверяет не только наличие полей, но и отсутствие лишнего: значение отправленного magnet с узнаваемым passkey в записи не встречается ни в одном поле (найдено проходом rubric). Инвариант «секреты не попадают в логи» — major, а add — единственный вызов qbt, через который magnet-URI приватного трекера физически проходит.

«Первые байты источника» в лог не идут: для magnet это часть URI (шум, а на приватном трекере — потенциально чувствительный passkey), для torrent — начало bencode, диагностической ценности не несущее.

Причину отказа qBittorrent мы при этом не получаем и не выдумываем: Fails. причины не несёт, и никакого «это был дубль» из него не выводится. Запись честно остаётся «отказ без причины», но становится привязанной к загрузке.

4. N5 — исход пункта запись, а не код

Пункт про аллокации bencode правке не подлежит: это чужая библиотека, поведение ограничено (потолок ~128 MiB) и завершается ошибкой, а не порчей данных. Записываем наблюдение в docs/research/ с провенансом (версия, файл:строка) и замером, снятым воспроизводимой командой во временном каталоге, а не оценкой из головы. Требование каталога разведки — «каждый вывод с числами и командой, которой получен» — выполняется буквально.

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

Записка получает условие устаревания («перепроверить при обновлении anacrolix/torrent»): наблюдение о коде зависимости протухает от бампа go.mod, в отличие от наблюдения о формате провода, которое живёт своей жизнью. Вводная docs/research/README.md расширяется на полстроки, чтобы каталог честно включал и такие наблюдения (найдено проходом architecture). Туда же уезжает второе наблюдение о той же библиотеке — про BestName/NoName: цена нити N1 ровно в том, что этого наблюдения не было записано нигде.

Risks / Trade-offs

  • Нормализация DisplayName меняет вход namer для безымянных раздач → раньше в подсказку уходил -, теперь пусто. Это улучшение (вход LLM чище), но формально смена поведения. Митигация: путь покрыт тестом приёма на безымянной раздаче; namer пустую подсказку и так обрабатывает (magnet без dn даёт то же самое).
  • Scoped-логгер в Retry добавляет поля к записям, которых там раньше не было → тесты, сверяющие записи журнала на пути retry, могут разойтись. Митигация: гейт (test, flaky) ловит это детерминированно.
  • Замер аллокаций bencode делается на синтетическом входе → он показывает поведение декодера, а не профиль реального .torrent. Митигация: в записке прямо сказано, что число получено на крафт-входе, и приведена команда.
  • Пустой идентификатор в диагностике транспорта → если контракт Ingest когда-нибудь начнёт возвращать id на ошибке, транспорты его не покажут. Митигация: контракт закреплён спекой и тестом, менять его придётся осознанно и вместе с транспортами.

Migration Plan

Миграции нет: схема БД, конфигурация, HTTP-контракт и тексты Telegram не меняются. Нумерованных артефактов изменение не добавляет. Откат — обычный revert коммита.

Что изменилось после ревью изменения (чекпоинт 2)

Профиль deep, семь проходов. Отработано инлайн:

  • Порядок в displayName был неверен — сравнение с сентинелом стояло до схлопывания, и имя " - " превращалось в - уже после проверки. Три прохода (specs, adversary, reimpl) нашли это независимо, два принесли падающий тест; adversary показал, что это регрессия против master, где фолбек source_ref на имя файла срабатывал. Порядок исправлен и закреплён в спеке.
  • Команд без scoped-логгера оказалось семь, а не одна. Кроме Retry, внешний сервис до постановки логгера зовут Relink, Rerecognize, Refine (qBittorrent через ensureSourceReady) и ChooseCandidate, AddManualSource, SetProviderID (метабаза через recognizer.Director). Развилка разрешена в пользу починки всех семи: узкая альтернатива требовала сузить требование identity до одного пути, и тогда нормативная спека фиксировала бы слепоту как норму. Конструкция, снимающая класс целиком (обёртка входа команд), — записанный вопрос ниже.
  • Добавлены недостающие оракулы: отказ приёма на обоих HTTP-транспортах коррелируется по request_id; best-effort ветки Retry исполняются тестами.
  • Записка разведки получила текст программы замера и разграничение TotalAlloc против RSS: замер ops показал, что физическая память при деградационном пути не расходуется вовсе, и вывод записки от этого становится сильнее, а не слабее.

Open Questions

  • Заводить ли обёртку входа команд воркера, делающую пропуск scoped-логгера невозможным? Пролог всех ~18 публичных команд однороден: defer logCmdw.mu.Lock → чтение загрузки → проверка состояния → ctx = w.scoped(…). Последний шаг держится дисциплиной, и дисциплина уже дала семь промахов из ~дюжины — то есть это не гипотеза, а измеренная частота. Варианты: (а) обёртка w.withDownload(ctx, op, capability, id, allowedStates, fn) — пропуск невозможен по построению, цена: правка всех команд разом и потеря гибкости там, где capability меняется по ходу метода (review.go дважды пере-скопливает под capFileLayout); (б) тест-перебор публичных команд, краснеющий на команде без scope, — дешевле и не трогает продовый код, но ловит только известную форму нарушения; (в) оставить дисциплину. Пока решения нет, стоит (в) — все известные нарушения починены. Рекомендация — (б) отдельной задачей: механизация без переписывания, ровно по процедуре промоута «находка → конвенция → правило».
  • Нужен ли Telegram-транспорту собственный корреляционный ключ у отказа приёма? Сегодня opErr при пустом идентификаторе возвращает текст без ключа, а конвенция (docs/conventions/errors.md, «Граница и трансляция») требует к сообщению корреляционный ключdownload_id либо request_id. На пути приёма из Telegram нет ни того, ни другого, и это главный сценарий паспорта. Варианты: (а) завести идентификатор запроса на входе обработчика апдейта (или взять update_id) и класть его в scoped-логгер и в текст отказа — цена в том, что рядом с существующей корреляцией по download_id/infohash появляется второй канал, ровно тот класс, который решение 3 отвергло для qbt; (б) не заводить ключ, а поднять запись отказа разбора с DEBUG до INFO с infohash, и тогда искать по инфохэшу — дешевле, но у пользователя в руках всё равно ничего нет; (в) оставить как есть. Пока решения нет, стоит (в): изменение поведения Telegram не меняет и регрессии не вносит. Рекомендация — (б) отдельной задачей: она закрывает наблюдаемость, не заводя второго канала корреляции. Пробел существует до этого изменения и им не создан.
  • Приводить ли Cancel и Dismiss к той же форме, что Retry и Delete (присвоение ctx = w.scoped(…) в начале команды вместо одноразового построения логгера в аргументе записи)? Варианты: (а) привести — правило «команда воркера скопливает ctx один раз» становится однородным, цена — правка двух методов без наблюдаемого эффекта, риск разойтись с тестами журнала; (б) не трогать — у этих команд нет внешних вызовов, поэтому наблюдаемой разницы нет, цена — форма остаётся неоднородной, и следующий вызывающий скопирует ту из двух, что попалась. Пока решения нет, стоит (б): изменение закрывает N4 и не расширяется. Рекомендация — (а) отдельной задачей-гигиеной, когда в Cancel/Dismiss появится первый внешний вызов; раньше этого момента правка не окупается.