Files
transcriber/docs/review.md
T
av 09228f23d8 Go обновлён до 1.26, а расхождение версий теперь роняет гейт
- шаг go-version в task gate сверяет объявленную версию в go.mod, Dockerfile,
  CLAUDE.md и README.md; судит по репозиторию, go не зовёт, docker и сети не
  требует
- заведена capability toolchain: до сих пор спеки нормировали только поведение
  сервиса, теперь и инструмент сборки. Причина и цена — в двух ADR
- закрыт дефект 2026-08-12: образ на golang:1.24-alpine разошёлся с go.mod и
  перестал собираться, а восемь шагов гейта и шесть проходов ревью были зелёными
2026-08-12 10:57:50 +03:00

29 KiB
Raw Blame History

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

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

Конвейер ревью прогонялся один раз — 2026-08-11, на изменении fix-http-handler-tests; его триаж лежит в openspec/changes/archive/2026-08-11-fix-http-handler-tests/review/triage.md. Разделы ниже заполнены наперёд по коду и правятся по итогам прогонов: «Типовые ложноположительные» первым прогоном уже пользовались.

Типовые узлы

Рода узлов проекта и проверяемые свойства к каждому.

Шаг конвейера (FindAndRunConversionJob, FindAndRunTranscribeJob, FindAndRunTranscribeCheckJob):

  • отличает «задач нет» от отказа и не считает первое ошибкой;
  • при отказе на середине оставляет задачу в состоянии, из которого повтор корректен, либо переводит в failed осознанно;
  • не теряет ссылку на файл: job.FileID переставляется только после того, как запись о новом файле создана;
  • повтор шага на той же задаче не создаёт лишних файлов и записей;
  • отвечает пользователю ровно один раз.

Транспорт (internal/controller/tg, internal/controller/http):

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

Клиент внешнего сервиса (adapter/recognizer/yandex, adapter/telegram):

  • имеет таймаут и не виснет, когда внешний сервис не отвечает;
  • не кладёт секрет в URL и не даёт ему утечь через ошибку транспорта;
  • различает «сервис ответил отказом» и «сервис недоступен»;
  • вырожденный ответ (пустой, усечённый, без ожидаемого поля) не превращает в успех молча.

Репозиторий SQLite (adapter/repo/sqlite):

  • список колонок совпадает во всех четырёх запросах файла;
  • NULL в колонке разбирается в указатель, а не роняет Scan;
  • захват задачи не выдаёт одну строку двум вызывающим;
  • ошибка драйвера транслируется в доменную у источника.

Обёртка над внешним процессом (adapter/converter/ffmpeg, adapter/metaviewer/ffmpeg):

  • отсутствие программы в PATH отличается от отказа обработки;
  • вход, пришедший от пользователя, не попадает в аргументы командной строки неразобранным;
  • пустой или частично записанный выходной файл считается отказом;
  • процесс не висит вечно.

Любой узел — сверх свойств своего рода:

  • изменённое место покрыто хоть одним проходящим тестом. Тест, который никогда не был зелёным, обнуляет сигнал всего пакета: настоящий отказ в нём становится неотличим от привычного шума (журнал, запись 2026-08-10);
  • проверка способна упасть. Утверждение, разбирающее ответ в ту же структуру, чьи теги и составляют проверяемый контракт, меняется вместе с ним и никогда не ловит поломку; такое судят по сырому виду ответа. Признак ищется мутацией: сломай проверяемое свойство и убедись, что тест краснеет (журнал, запись 2026-08-11);
  • то же и об оракуле критерия приёмки, не только о тесте. Критерий, чей единственный оракул — молчание линтера, годится ровно тогда, когда линтер краснеет на всех негодных реализациях; проверяется той же мутацией. Прецедент: «отказ Close не теряется молча» принимался молчанием errcheck, а тот пропускал _ = conn.Close() — реализацию, теряющую отказ целиком (журнал, запись 2026-08-11 про недостижимую норму; закрыто решением);
  • требование без сценария не имеет оракула и потому не может быть нарушено заметно. Норма, которую нечем уронить, расходится с кодом молча — и расходится тем вернее, чем убедительнее написана (журнал, запись 2026-08-11).

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

  • «Воркер глотает ошибку NoopJobError». Не дефект: этот тип означает «задач в этом состоянии нет», и internal/controller/worker/worker.go намеренно не логирует его и не считает в метрику. Норма записана требованием pipeline.

    Оговорка, и она тут главная: ложноположительным считается только само молчание воркера. Проверка формы узнавания ложноположительной не является: приведение типа на этом месте — настоящий дефект, закрытый 2026-08-11 задачей errors-as-instead-of-typecast. Появилось снова — это регрессия, и выбрасывать её как известную нельзя.

  • «Захват задачи не в транзакции — гонка двух воркеров». По построению её нет: три воркера читают три разных состояния, и одну строку они не делят. Механика захвата и её слабые места — database.md, «Представление данных». Находка становится настоящей ровно тогда, когда появится второй экземпляр процесса или второй воркер на то же состояние.

  • «Файлы и объекты не удаляются, диск растёт». Факт верный и записан в database.md; срок хранения не задан сознательно, задачи на него нет. Новой находкой это не считается, пока не измерен рост.

  • «HTTP API открыт без аутентификации». Известно и записано первой строкой security.md. Находкой считается только новая поверхность, выставленная наружу, а не повторение этого факта.

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

Форма: <тема>: <вопрос> (<провенанс>).

  • operations: пережил ли шаг конвейера отмену контекста на середине — воркеры получают ctx, но ни один шаг его внутрь не передаёт (чтение worker.go и transcribe.go, 2026-08-10).
  • operations: появился ли таймаут у обращения к Telegram, S3 и SpeechKit — ни у одного из них таймаута нет (чтение tg.go, s3.go, speechkit.go, 2026-08-10).
  • operations: не удвоилась ли запись об одном сбое — шаг логирует ошибку и возвращает её воркеру, который логирует снова (чтение transcribe.go, 2026-08-10).
  • security: не попал ли в лог текст расшифровки, имя файла пользователя или URL с токеном бота (запрет в security.md и conventions/logging.md).
  • security: не строится ли путь на диске или ключ объекта из значения, пришедшего снаружи, — расширение файла сегодня берётся из имени отправителя (чтение service/transcribe.go, 2026-08-10).
  • security: не уходит ли значение, пришедшее снаружи, меткой метрики — страница метрик отдаётся без проверки отправителя, и метка это поверхность пошире журнала (журнал, запись 2026-08-11 про хвост имени).
  • architecture: не появился ли второй путь приёма мимо createTranscribeJob — сегодня через него идут оба входа (architecture.md, «Единые точки проекта»).
  • architecture: не поехало ли поведение в architecture.md вместо спеки — заведены две capability (openspec/specs/intake и openspec/specs/pipeline), и каждая описана частично. Поведение прочих узлов живёт в обзоре под маркерами долга, а соблазн дописать туда ещё — самый большой.
  • conventions: новая колонка правится во всех четырёх местах репозитория (CLAUDE.md, «Инварианты»).
  • autotests: покрыт ли изменённый шаг конвейера хоть одним тестом — сегодня тестов два файла, и оба мимо конвейера.

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

Проектная конкретизация правила выбора метки. Умолчание — medium.

Крупное здесь (поднимает до large, ось объёма):

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

Незнакомое здесь (поднимает до large, ось формы решения):

  • вход через OIDC и разграничение доступа: как связаны пользователь Telegram и пользователь приложения, до начала работы назвать нельзя;
  • всё, что делается на выбранном фреймворке впервые: форма решения нащупывается по ходу, пока конвенция веб-UI пуста;
  • установка на телефон: service worker перехватывает запросы, и что он кэширует, до работы назвать нельзя;
  • работа с записями в несколько часов: потолки внешних сервисов не замерены, форма решения зависит от замера;
  • приём дорожки из видео и форматов, которых ffmpeg не берёт текущей командой;
  • всё, что требует записи в research/ прежде, чем начать.

Мелкое здесь (опускает до small):

  • правка текста, который видит пользователь Telegram;
  • новая метрика в internal/metrics;
  • правка config.dist.toml и умолчаний defaultConfig() без нового поля;
  • правка документов канона.

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

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

Не проверит ни один проход:

  • operations: поведение внешних сервисов под нагрузкой и на границах — SpeechKit и Object Storage поднять в тесте нечем;
  • operations: реальный профиль нагрузки. Проект работает на единицах записей в день, и утверждения о росте остаются условиями, а не замерами;
  • security: стойкость ffmpeg к вредоносному входу — разбор чужого формата отдан внешней программе, и она вне нашей границы.

Перестали проверять сознательно:

  • autotests: разбор вывода настоящего ffprobe. Проверки приёма звали его до 2026-08-11 — правда, звали так, что он всегда отказывал, — а теперь получают длительность от подставного источника. Своего теста у adapter/metaviewer/ffmpeg нет; решение и его цена — в adr/ADR-2026-08-11-stub-adapters-in-tests.md.

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

Верхняя запись найдена конвейером ревью на первом же его прогоне, вторая — прогоном гейта при заведении канона 2026-08-10, две нижние восстановлены по истории git тогда же. Три нижние помечены проскочил: ревью тогда не было, и поймать их было некому. У восстановленных нет поля «Чем воспроизведён», и выдумывать его задним числом нельзя.

2026-08-12 — образ не собирался, и этого не увидел никто [проскочил]

Что сломалось. go mod tidy поднял директиву go в go.mod до 1.25.0 — её требует PocketBase, — а Dockerfile продолжал собирать на golang:1.24-alpine с GOTOOLCHAIN=local. task image упал бы на шаге сборки: выкладки задачи pocketbase-storage не существовало бы вовсе.

Почему не поймали. Все шесть проходов ревью и весь гейт видели зелёное: go build ./... идёт на хостовом Go, а образ не собирает ни один шаг гейта. Расхождение выглядело согласованным ещё и потому, что CLAUDE.md и README.md обещали Go 1.24 — то есть три места из четырёх говорили одно и то же, и неверными были именно они.

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

Чем чинится на будущее. Сборка образа гейтом не проверяется намеренно — дорого. Дешёвая замена: шаг, сверяющий версию сборщика в Dockerfile с директивой go в go.mod. Строкой сравнения, без docker. Заведено урожаем ревью.

Закрыто задачей go-1-26-upgrade 2026-08-12: шаг go-version в task gate (scripts/check-go-version.sh). Сверяются четыре места, а не два, — go.mod, Dockerfile, CLAUDE.md, README.md: в этом дефекте трое из четырёх врали согласованно, и парная сверка не увидела бы документ, разошедшийся с согласованным кодом. Норма — capability toolchain.

2026-08-11 — норма требовала от сервиса недостижимого [пойман ревью]

  • Где: дельта-спека pipeline задачи errors-as-instead-of-typecast, абзац об отказе шага
  • Симптом: требование гласило «отказ MUST быть записан ровно один раз единственной логирующей точкой». Сервис пишет дважды — сначала шаг конвейера, следом воркер, — то есть норма не выполнялась бы с первого дня, а после архивации стала бы посылкой для следующих задач
  • Причина: дефект родился при починке соседнего. Первая редакция назначала логирующей точкой воркера и фиксировала уровень ERROR, чем закрепляла контрактом долг conventions/logging.md. Правка по этой находке ушла в противоположную крайность: вместо «норма молчит о числе записей» получилось «норма требует одной». Двойная запись — записанный системный долг, и обе редакции с ним расходились, только в разные стороны
  • Чем воспроизведён: прогоном пробы через go test -overlay: один отказ хранилища даёт две записи — Failed to find and acquire job из шага и Worker error из воркера
  • Почему не поймали раньше: требование не имело сценария, а значит и оракула — упасть ему было нечем. Ревью дизайна абзац читало, но код с ним не сверяло: кода тогда не существовало. Поймал проход specs на ревью кода, направлением code → spec, и поймал прогоном, а не чтением
  • Что меняем: норма говорит только проверяемое сегодня — отказ виден владельцу и засчитан в счётчик; число записей и уровень названы долгом с адресом. В критерии приёмки добавлена строка: норма не объявляет обязательным недостижимое — ни в ту, ни в другую сторону

2026-08-11 — хвост имени отправителя уезжал на открытую страницу метрик [пойман ревью]

  • Где: internal/service/transcribe.go, метки file_extension у размера принятой записи и source_format у длительности конвертации
  • Симптом: имя запись.тайное-слово клало тайное-слово меткой метрики, а GET /metrics отдаётся без проверки отправителя. Тем же каналом множеством значений метки распоряжался анонимный отправитель
  • Причина: расширение берётся из имени отправителя дословно (filepath.Ext) и употреблялось меткой без приведения. Канал старше задачи, которая его нашла
  • Чем воспроизведён: прогон filepath.Ext на именах вида запись.тайное-слово, Разговор с Петровым 11.08, затем чтение реестра метрик после приёма — метка несла хвост дословно
  • Почему не поймали: метрику никто не считал выходом приватного значения. Тема security смотрела журнал, ответ и пути на диске; вопроса про метку в перечне вопросов не было, и ни один проход её не открывал. Поймали три прохода разом на задаче, которая закрывала соседний канал
  • Что меняем: вопрос про метку добавлен в «Вопросы по темам»; правило приведения нормировано спекой intake и записано решением

2026-08-11 — проверка приёма не могла упасть [пойман ревью]

  • Где: internal/controller/http/transcribe_test.go, случай успеха приёма
  • Симптом: тест не поймал ни одного настоящего дефекта приёма, хотя был зелёным и выглядел содержательным
  • Причина: две штуки одного рода. Тест разбирал ответ в CreateTranscribeJobResponse — ту самую структуру, чьи теги json и составляют публичный контракт: переименование тега меняло и проверяемое, и ожидаемое разом. И заведение задачи тест подтверждал только эхом ответа, а не чтением базы
  • Чем воспроизведён: мутацией. Замена тега на json:"jobId" и удаление s.jobRepo.Create(job) из internal/service/transcribe.go — тесты в обоих случаях оставались зелёными; после правки обе мутации их роняют
  • Почему не поймали: проверки писались тем же заходом, что и правились, а «зелено» на новом тесте читается как подтверждение. Поймал проход specs ревью кода, и поймал ровно тем, что добыл оракул мутацией, а не рассуждением
  • Что меняем: успех судится по сырому JSON и по строке в базе. В типовые узлы, «Любой узел», добавлено свойство «проверка способна упасть» с указанием на мутацию как способ его проверить

2026-08-10 — тесты http-обработчика ни разу не были зелёными [проскочил]

  • Где: internal/controller/http/transcribe_test.go
  • Симптом: go test ./... падает четырьмя случаями; обнаружено первым же прогоном гейта при заведении канона
  • Причина: тест требует testdata/sample.m4a, которого в репозитории нет и не могло быть — .gitignore содержит *.m4a. Остальные случаи записывают в файл строку test audio content и ждут 201, а обработчик зовёт настоящий ffprobe, который такой вход отвергает
  • Чем воспроизведён: go test ./internal/controller/http/ — четыре отказа, из них один по отсутствию файла и три по коду 500 вместо 201
  • Почему не поймали: гейта не было вовсе, а go test руками, судя по результату, не гоняли ни разу с коммита 87d8b05
  • Что меняем: заведена задача http-handler-tests-never-green; в гейт добавлен шаг go test ./..., и красный тест теперь виден. Настоящий остаток шире: тест, который никогда не проходил, обнуляет сигнал всего пакета — в типовые узлы добавлено свойство «покрыт хоть одним проходящим тестом», а в вопросы темы autotests — вопрос про изменённый шаг конвейера
  • Закрыт 2026-08-11: проверки переписаны, go test ./... зелёный и из списка объявленных долгов в CLAUDE.md снят

2025-10-23 — пустой ответ вместо текста расшифровки [проскочил]

  • Где: internal/service/transcribe.go, ветка завершения задачи
  • Симптом: пользователь Telegram получал пустое сообщение вместо текста
  • Причина: SpeechKit возвращал операцию успешной, но с пустым текстом, и задача завершалась этим пустым значением
  • Чем воспроизведён: восстановлено по коммиту ec637c0, оракула нет
  • Почему не поймали: конвейера ревью не существовало
  • Что меняем: уже сделано — пустой текст подменяется фразой «на записи нет текста». Настоящий остаток в другом: свойство «вырожденный ответ внешнего сервиса не превращается в успех молча» вынесено в типовой узел «клиент внешнего сервиса» выше

2025-08-17 — длинная расшифровка не доходила до пользователя [проскочил]

  • Где: internal/adapter/telegram/sender.go
  • Симптом: отправка текста длиннее предела сообщения Telegram завершалась ошибкой целиком, пользователь не получал ничего
  • Причина: предел длины сообщения на стороне Telegram не учитывался
  • Чем воспроизведён: восстановлено по коммиту 822e168, который тем же заходом завёл internal/adapter/telegram/split_test.go
  • Почему не поймали: конвейера ревью не существовало
  • Что меняем: уже сделано — деление по словам с пределом 4000 символов, число записано в database.md