av
6c04c801c9
- проверки больше не зовут ffprobe и не меняют рабочий каталог процесса; добавлены случаи на отказ чтения метаданных и на отсутствие поля audio - заведена спека intake на приём по HTTP, ADR о подставных адаптерах, запись в журнал ревью о проверке, которая не могла упасть - go test снят из объявленных долгов CLAUDE.md, послабление errcheck для _test.go в .golangci.yml убрано
241 lines
19 KiB
Markdown
241 lines
19 KiB
Markdown
# Ревью: настройка и журнал
|
||
|
||
## Как настроен конвейер
|
||
|
||
Конвейера ревью в проекте пока нет: плагин не подключён, ни одного прогона не
|
||
было. Раздел заполнен наперёд по коду — он и служит настройкой первому прогону.
|
||
|
||
### Типовые узлы
|
||
|
||
Рода узлов проекта и проверяемые свойства к каждому.
|
||
|
||
**Шаг конвейера** (`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).
|
||
|
||
### Типовые ложноположительные
|
||
|
||
- **«Воркер глотает ошибку `NoopJobError`».** Не дефект: этот тип означает «задач
|
||
в этом состоянии нет», и `internal/controller/worker/worker.go` намеренно не
|
||
логирует его и не считает в метрику. Настоящий дефект рядом другой — проверка
|
||
идёт приведением типа и сломается при первой же обёртке; он уже записан в
|
||
[conventions/errors.md](conventions/errors.md).
|
||
- **«Захват задачи не в транзакции — гонка двух воркеров».** По построению её
|
||
нет: три воркера читают три разных состояния, и одну строку они не делят.
|
||
Находка становится настоящей ровно тогда, когда появится второй экземпляр
|
||
процесса или второй воркер на то же состояние.
|
||
- **«Файлы и объекты не удаляются, диск растёт».** Факт верный и записан в
|
||
[database.md](database.md); срок хранения не задан сознательно, задачи на него нет.
|
||
Новой находкой это не считается, пока не измерен рост.
|
||
- **«HTTP API открыт без аутентификации».** Известно и записано первой строкой
|
||
[security.md](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](security.md) и
|
||
[conventions/logging.md](conventions/logging.md)).
|
||
- `security`: не строится ли путь на диске или ключ объекта из значения,
|
||
пришедшего снаружи, — расширение файла сегодня берётся из имени отправителя
|
||
(чтение `service/transcribe.go`, 2026-08-10).
|
||
- `architecture`: не появился ли второй путь приёма мимо
|
||
`createTranscribeJob` — сегодня через него идут оба входа
|
||
([architecture.md](architecture.md), «Единые точки проекта»).
|
||
- `architecture`: не поехало ли поведение в `architecture.md` вместо спеки —
|
||
спек ещё нет, и соблазн описать поведение в обзоре максимальный.
|
||
- `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](adr/ADR-2026-08-11-stub-adapters-in-tests.md).
|
||
|
||
## Журнал дефектов
|
||
|
||
Верхняя запись найдена конвейером ревью на первом же его прогоне, вторая —
|
||
прогоном гейта при заведении канона 2026-08-10, две нижние восстановлены по
|
||
истории git тогда же. Три нижние помечены `проскочил`: ревью тогда не было, и
|
||
поймать их было некому. У восстановленных нет поля «Чем воспроизведён», и
|
||
выдумывать его задним числом нельзя.
|
||
|
||
## 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](../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](database.md)
|