Files
jellybit/openspec/changes/archive/2026-08-06-ingest-nits/review/checkpoint-2-deep.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

30 KiB
Raw Blame History

Чекпоинт 2 — ревью изменения после apply, профиль deep

  • Change: ingest-nits
  • База диффа: master (5c79fdfffe9e2ddc78b7ebf9f4cdc3aa83560b2b); изменения на ветке task/ingest-nits не закоммичены.
  • Профиль: deep. Режим прогона: по графу.
  • Гейт: ЗЕЛЁНЫЙ — 12/12 шагов OK, ни одного SKIP/WARN; -race реально прогнан; diff-coverage 81 % (7 изменённых строк не покрыты — они и стали находками F1/F2).
  • Дата: 2026-08-06.

Запущенные проходы и исход

Состав сверен с профилем deep: все семь стадий запускались, расхождений с профилем нет.

Проход Исход
review-gate отработал: 2 находки (обе major, пробелы покрытия изменённых строк)
review-specs отработал: 5 находок + 3 границы спеки
review-code отработал: 0 находок (нарушений записанных конвенций нет), 1 promote-кандидат
review-adversary отработал: 4 находки + 1 гипотеза; фаззинг torrent.Parse+Context() 6.1 млн исполнений — паник нет; утечек экранирования нет
review-ops отработал: 5 находок (3 с замерами); гипотезы «ERROR-шторм», «сирота при отмене ctx», «kill посреди раскладки» сняты проверкой
review-reimpl отработал: 3 находки + перечень мест, где код лучше независимой реализации
review-architecture отработал: 2 находки + 3 пункта «дешевле переделать до мерджа»; новых понятий change не вводит

На входе — 21 находка + 1 гипотеза + 3 «дешевле до мерджа» + 1 promote-кандидат. После дедупликации по причине — 16 причин. В основном списке — 6 пунктов (потолок 7 соблюдён); остальное — в гипотезах, promote и урожае, ничего не выброшено молча.

Дедупликация: S2 = A1 = R1 (один дефект порядка нормализации, найден тремя проходами независимо — оракул есть, это повысило приоритет, не confidence); S1 = R2 = AR1 (ложность требования identity; перечни команд расходились — 6 против 3, точный перечень установлен триажем по коду: 6); G1 + S5 (один пробел — отказ приёма на HTTP-границе без оракула).

Отчёт чекпоинта 1 (checkpoint-1-design.md) учтён: находки, закрытые там инлайн, повторно не поднимались; урожай и записанные вопросы не дублируются.


Блокирует мердж

B1. Имя, схлопывающееся в -, проходит нормализацию: source_ref и контекст получают сентинел, а фолбек на имя файла регрессировал против master

  • Файл: internal/torrent/torrent.go:94-99
  • Severity: major (сломано требование дельта-спеки этого же change — «Вырожденное имя раздачи не считается именем», сценарий «Вырожденное имя даёт source_ref из имени файла»)
  • Confidence: high
  • Оракул: падающий тест — прогнан триажем: tmp/_reimpl/probe/probe_test.go даёт name " - " → DisplayName "-", "-\n" → "-", "\t-" → "-" (ожидалось ""); Context() первой строкой несёт -. Второй независимый падающий тест — у прохода adversary. Регрессия: на master internal/ingest делал TrimSpace + сравнение с - (git diff по ingest.go:198-205), и фолбек на имя присланного файла срабатывал; теперь source_ref становится -.
  • Причина: в displayName сравнение с metainfo.NoName стоит до oneLine, а oneLine (strings.Fields) схлопывает " - ", "-\n", "\t-" ровно в -.
  • Последствие: молчаливое — ошибки нет, просто карточка и source_ref несут -, контекст распознавания получает строку-мусор вместо отсутствия строки названия. Ровно тот класс входа, который change обещал закрыть.
  • Предложение: в displayName сначала oneLine, потом сравнение с metainfo.NoName (порядок независимой реализации reimpl); тест-входы " - ", "-\n", "\t-" в internal/torrent/torrent_test.go; в дельта-спеке зафиксировать порядок (нормализовать → сравнить).
  • Найдено проходами: specs (S2), adversary (A1), reimpl (R1)
  • Действие: инлайн

B2. При архивации нормативная спека identity станет ложной: шесть команд воркера зовут внешний сервис до scoped-логгера

  • Файл: internal/worker/review.go:410/417, 442/445, 465/468, 796, 849; internal/worker/review.go:734→796; openspec/changes/ingest-nits/specs/identity/spec.md:12-17; openspec/changes/ingest-nits/proposal.md:36-38
  • Severity: major
  • Confidence: high
  • Оракул: поимённое построение путей вызовов, проверено триажем по коду:
    1. RelinkensureSourceReady (review.go:410) → torrentByInfohashqbt.Torrents до ctx = w.scoped(...) (:417);
    2. Rerecognize:442 до :445;
    3. Refine:465 до :468;
    4. ChooseCandidatechooseCandidateLocked (:796) — recognizer.Directormetadata (HTTP) с необогащённым ctx (команда вообще не делает ctx = w.scoped, только точечные logctx.From(w.scoped(...)) на своих записях);
    5. AddManualSource — тот же путь через :738;
    6. SetProviderIDDirector на :849 с необогащённым ctx. Клиенты metadata логируют через logctx.FromOr(ctx, ...) (internal/metadata/http.go:78, tvdb.go:101,128) — записи уходят без download_id/infohash. Требование дельты (identity/spec.md:13-15): «операция … делающая вызов внешнего сервиса, SHALL положить scoped-логгер … до такого вызова — включая команды, пришедшие с транспорта». proposal.md:36-38 утверждает, что Retry — «единственная команда воркера», не кладущая scoped-логгер: ложь, уедет в архив. Остальные команды (Apply, Undo, Delete, Defer, RefreshDisplayName) проверены — они требование соблюдают; Cancel/Dismiss внешних вызовов не делают.
  • Последствие: класс «молчание» — сбой внешнего сервиса с этих шести путей даёт запись, которую фильтр по id загрузки не находит; спека, ставшая нормативной, утверждает обратное, и следующая задача будет строиться на ложном тексте.
  • Найдено проходами: specs (S1), reimpl (R2), architecture (AR1)
  • Действие: развилка. Вопрос: как разрешить расхождение требования identity с кодом?
    • (а) сузить требование до фактически закрытого пути (Retry + фоновый цикл), убрав «включая команды, пришедшие с транспорта» из нормы в цель. Цена: спека честна, но шесть команд остаются без корреляции внешних вызовов, вопрос вернётся на следующем ревью identity.
    • (б) починить шесть команд в этом change: Relink/Rerecognize/Refine — перенести ctx = w.scoped(...) выше ensureSourceReady; ChooseCandidate/AddManualSource/SetProviderID — обогатить ctx на входе (форма Retry/Delete). Цена: ~6 однотипных мелких правок + тесты по образцу нового probe-хендлера torrent_add_test.go; scope растёт умеренно, инвариантов не трогает.
    • (в) = (б) сейчас + отдельная задача в беклог на обёртку w.withDownload(...), схлопывающую однородный пролог ~18 команд и делающую пропуск невозможным по построению (рекомендация architecture). Рекомендация проходов — (в). В любом варианте proposal.md («единственная команда») требует правки — при (б)/(в) формулировкой «был не единственным», при (а) — снятием претензии.

Стоит исправить сейчас

F1. Сценарий «Отказ приёма на HTTP-границе» остался без оракула: ветка ошибки веб-формы не исполняется ни одним тестом

  • Файл: internal/httpapi/httpapi.go:437 (изменённая строка); дельта-спека ingest, сценарий «Отказ приёма на HTTP-границе»
  • Severity: major
  • Confidence: high
  • Оракул: grep request_id internal/httpapi/*_test.go — пусто; ни один тест не делает POST /ui/downloads (форма добавления): проверено триажем — тесты ходят только в /ui/downloads/{id}/.... REST-ошибки приёма покрыты (httpapi_test.go:138,154), но request_id в ответе не проверяет и REST. Асимметрия: Telegram-путь получил новый тест, веб-форма — нет.
  • Последствие: THEN-обещание дельта-спеки (ответ несёт request_id, а не идентификатор загрузки) не проверяет никто; регрессия на этой строке будет молчаливой.
  • Предложение: тест на handleUIAdd с fakeIngestor.err (редирект с ошибкой, без download_id) + проверка request_id в теле/заголовке отказа на обоих HTTP-транспортах.
  • Найдено проходами: gate (G1), specs (S5)
  • Действие: инлайн

F2. Best-effort-ветки Retry (откат активации, сбросы базиса и счётчика) переписаны диффом и не исполняются тестами

  • Файл: internal/worker/worker.go:1085, 1093, 1103, 1111
  • Severity: major
  • Confidence: high
  • Оракул: diff-coverage гейта — 4 из 7 непокрытых изменённых строк; fakeStore не умеет ронять SetDownloadState/SetRetriedAt/SetSourceMissCount. Строки в диффе: w.log.Error(...) заменён на logctx.From(ctx)... (проверено git diff).
  • Последствие: пути отката «активация прошла, add не удался» держат инвариант «задача не качается без раздачи в qBittorrent» — и не проверяются; молчаливая регрессия отката оставила бы задачу в downloading без источника.
  • Предложение: научить fakeStore инъекции ошибок этих трёх сеттеров; тесты: откат возвращает прежнее состояние/код/сообщение, сбой best-effort-сеттеров не проваливает состоявшийся retry (только WARN).
  • Найдено проходом: gate (G2)
  • Действие: инлайн

F3. Записка research обещает несуществующий текст замера и переоценивает риск: мерился TotalAlloc, а физический RSS почти не растёт

  • Файл: docs/research/torrent-bencode-limits.md:22-26, 49-53
  • Severity: minor
  • Confidence: high
  • Оракул: (1) записка обещает «текст — в этом change, openspec/changes/archive/ingest-nits/» — в каталоге change программы нет (проверено листингом; в tmp/ лежит только bencode-alloc.out), нарушено правило воспроизводимости docs/research/README.md; (2) замер ops: make([]byte, 128MiB) без касания страниц → +800 КБ RSS; деградационный путь (make → проваленный io.ReadFull на 36-байтовом входе) страниц не касается; concurrency=1000 → пик RSS 7 352 КБ. Вывод «N одновременных дают до N × 128 MiB» относится к виртуальным аллокациям, не к памяти процесса.
  • Последствие: будущий читатель примет решение (лимиты, семафор приёма) по завышенной оценке риска; невоспроизводимость обесценивает записку как наблюдение.
  • Предложение: вложить текст программы в записку (или в change) и дописать разграничение TotalAlloc vs RSS с числами замера ops — вывод «принято как есть» усиливается, а не смягчается.
  • Найдено проходами: adversary (A4), ops (O4)
  • Действие: инлайн

F4. Дельта-спека ingest в двух местах описывает не то поведение, которое реализовано и принято

  • Файл: openspec/changes/ingest-nits/specs/ingest/spec.md:28-30, 32-35
  • Severity: minor
  • Confidence: high
  • Оракул: (1) спека: нормализация схлопывает «любые разделители строк и краевые пробелы»; код: oneLine = strings.Join(strings.Fields(s), " ") (torrent.go:234-236) — любые пробельные последовательности, включая внутренние табы/пробелы. Поведение кода корректно — неверен текст спеки. (2) спека: пути файлов «не выводятся из имени»; код: files() при пустом пути делает p = meta.BestName() (torrent.go:114-116) — сырое имя. Решение не нормализовать пути остаётся верным (они только сигнал, целевые пути строятся из ответа qBittorrent) — ложно только основание.
  • Последствие: нормативный текст разойдётся с кодом в момент архивации; при B1 спека всё равно правится — дешевле сделать одним заходом.
  • Предложение: (1) сформулировать правило как «любые пробельные последовательности схлопываются в один пробел»; (2) заменить основание исключения путей на честное (путь может совпасть с именем при пустом fi.Path; не нормализуются, потому что служат только сигналом).
  • Найдено проходом: specs (S3, S4)
  • Действие: инлайн

Гипотезы без доказательства

  • naming.sanitize не режет разделители пути, а выведенное имя уходит в qBittorrent параметром rename (adversary; заявлен major, confidence medium — остаётся гипотезой). Неизвестно, применяет ли qBittorrent rename к папке на диске — если да, крафт-имя могло бы дать запись по произвольному относительному пути под paths.downloads (тень инварианта «источник неприкосновенен»). Оракула нет и не будет в конвейере: ходить в живой qBittorrent запрещено (CLAUDE.md → «Запреты»). Диффом не внесено (Retry namer не зовёт — имя выводится при первом добавлении). Предложение: задача в беклог — интеграционный тест за env-гейтом (*_integration_test.go) на поведение rename c / и .. в имени; до ответа — дешёвый пояс: резать разделители пути в подсказке имени.

Promote candidates

  • docs/conventions/errors.md: корреляционный ключ для транспорта без понятия запроса. Конвенция описывает трансляцию ошибок через request_id, но Telegram запроса не имеет — правило «у каждого транспорта назван свой корреляционный ключ публичного канала» просится в конвенцию (найдено code; пересекается с Open Question 1 в design.md и promote-кандидатом (в) чекпоинта 1 — при заведении объединить).
  • docs/conventions/logging.md: команда воркера не дублирует поля scoped-логгера. После ctx = w.scoped(...) отложенный logCmd(ctx, cmd, id, err) пишет download_id и из логгера, и явным аргументом — дублирующийся ключ в JSON-записи command failed (найдено adversary, A3; путь retry — в диффе, та же пара давно у Delete). Как находка это nit без записанной конвенции — потому promote: правило + возможная механизация тестом-перебором команд (родственно promote (а) чекпоинта 1).

Урожай

Отложено: вне диффа, ниже порога либо требует отдельной задачи. Формат: формулировка — оракул — провенанс.

  1. recognizePending при недоступном qBittorrent не обрывается по первому сбою: каждая completed-задача уезжает в review с диагностикой про qBittorrent — тест с фейковым недоступным qbt и 2+ задачами — ops (O1), вне диффа.
  2. SupersedeForeignLinksSCAN file_link без индекса под w.mu: EXPLAIN QUERY PLAN + замер ~24 мс на 200k строк, растёт линейно, ретеншена нет — ops (O2), вне диффа; задача: индекс или ретеншен.
  3. Retry держит глобальный w.mu через оба внешних вызова; таймаут qbt-клиента (30 с дефолт) не настраивается конфигом: замер — параллельный Cancel другой загрузки ждал 282 мс при qbt 300 мс — ops (O3), диффом не внесено (присвоение передвинуто внутри той же секции); связать с задачей на w.withDownload из B2-(в) — per-download блокировка там же.
  4. source_ref не ограничен по длине: .torrent 7 МиБ с именем 7 МиБ даёт source_ref 7 340 032 байта при соседе capContext 16 КиБ; читается SELECT * на каждом тике — замер adversary (A2); строка переписана диффом, но поведение не новое; задача: кап по образцу capContext.
  5. Пустые source_ref+display_name: веб-UI рисует пустой заголовок карточки, Telegram — #id: чтение кода (httpapi/download.go:71-81 vs tgbot/render.go:187-205) — ops (O5), вне диффа; связан с записью урожая чекпоинта 1 «пустой source_ref возможен» — закрывать одной задачей.
  6. Два рукодельных slog-хендлера родились в одном change (captureHandler в internal/qbt/qbt_test.go, probeHandler в internal/worker/torrent_add_test.go), уже разошлись возможностями — architecture (AR2); порог превращения в находку — третий потребитель.
  7. Нормализация комментария осталась в Context(), имени — в Parse, хотя спека подаёт правило как общее; сегодня Info.Comment вне пакета никто не читает — reimpl (R3, confidence low); при следующей правке пакета — Comment: oneLine(mi.Comment) в Parse либо оговорка в спеке.
  8. Доккомментарии (по одной строке, «дешевле до мерджа» от architecture; оркестратор может сделать заодно с B2/F4): ingest.Ingest цитирует требование по русскому имени — при переименовании требования ссылка оборвётся молча; magnet.Info.DisplayName не оговаривает, что оно НЕ нормализуется, в отличие от torrent.Info.DisplayName.
  9. Приоритет NameUtf8 над Name не описан спекой — граница спеки — specs; при случае — строкой в требование нормализации.
  10. Запись отказа приёма — DEBUG, на боевом INFO не пишется вовсе, а требование отправляет искать диагностику Telegram-отказа по этой записи — specs; уже записано вопросом 1 в design.md → Open Questions с рекомендацией поднять до INFO отдельной задачей — не дублировать.

Отсев вкусовщины: типовых generative-находок (переименования, перестановки, «вынести в файл», обобщение частного случая) в выводах не оказалось; единственный кандидат — A3 (дублирующийся ключ) — переведён в promote, т.к. записанной конвенции под nit нет. Ни одна находка не попала под «Типовые ложноположительные» docs/review.md — раздел просмотрен поимённо (никто не предлагал авторизацию, интерфейсы под моки, ретраи в тике и т.п.).

Границы покрытия

Прогон. Профиль deep, режим «по графу», чекпоинт 2. Запускались все семь проходов профиля (перечень с исходами — в сводке); не запускался никакой — состав полный. На чекпоинте 1 (профиль design) шли review-specs, review-rubric, review-architecture; rubric на чекпоинте 2 не запускается по построению профиля.

Что каждый запущенный проход не мог проверить в принципе (из charter'ов):

  • gate — только механизируемое с объективным оракулом; осмысленность тестов и спек не видит.
  • specs — сверяет спеку с кодом; поведение, отсутствующее в обоих, не найдёт.
  • code — только записанные конвенции; «хорошо ли это» вне них не судит.
  • adversary — не ходил в живой qBittorrent (запрет CLAUDE.md) — гипотеза про rename осталась гипотезой; фаззинг ограничен бюджетом 45 с; целевой путь из имени раздачи не строится, поэтому главный вопрос раздела «Вопросы к проходам» (выход за библиотеку через имена) на этом диффе не применим.
  • ops — реального профиля нагрузки umbar нет ни у кого; замеры синтетические (200k строк, concurrency=1000), боевые числа могут отличаться.
  • reimpl — переписывал только затронутые нити; расхождение вне них не обнаруживает.
  • architecture — судит по карте и диффу, код не исполняет.
  • Триаж — ничего нового не ищет по определению; пропуск любого прохода — его пропуск; здесь пропусков состава нет.

Осталось целиком на человеке. Два списка из docs/review.md → «Недоступно проверке», раздельно.

Не проверит ни один проход (принципиальная граница):

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

Перестали проверять сознательно (пересматривается первым при промахе):

  • идиоматичность Go — с 2026-08-04, проход idiom упразднён при переезде на плагин av-dev-pipeline; различение «идиоматично против распространено» не спрашивает никто; пересмотр — задача quality-review-agents.

Плюс общее для любого прогона: поведение под реальным потоком; поведение внешних систем в их боевых версиях (в этом чекпоинте конкретно — реакция qBittorrent на rename с разделителями пути и физическое поведение deleteFiles=true); история инцидентов.

Документы проекта. Всех нужных хватило: CLAUDE.md с разделом инвариантов (severity брались оттуда, а не выводились), docs/review.md с «Типовыми ложноположительными» (отсев шёл по ним) и обоими подразделами «Недоступно проверке», конвенции, docs/research/, дельта-спеки change. Одна оговорка: журнал дефектов содержит единственную запись (2026-08-06, про уборку торрента) — оракулов-прецедентов для классов находок этого чекпоинта в нём нет, подтверждение «такое здесь уже воспроизводилось» было недоступно; все оракулы добывались тестами и замерами прогона.

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