Files
transcriber/docs/review.md
T
av bd6001cd8d имя файла отправителя убрано из журнала приёма
- расширение приводится к перечню известных форматов прежде метки метрики:
  страница метрик открыта, и хвост имени уезжал на неё дословно
- проверки приёма перехватывают все три потока журнала и читают реестр метрик,
  каждая падает при снятии того, что сторожит
2026-08-11 16:37:58 +03:00

269 lines
22 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Ревью: настройка и журнал
## Как настроен конвейер
Конвейер ревью прогонялся один раз — 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).
### Типовые ложноположительные
- **«Воркер глотает ошибку `NoopJobError`».** Не дефект: этот тип означает «задач
в этом состоянии нет», и `internal/controller/worker/worker.go` намеренно не
логирует его и не считает в метрику. Настоящий дефект рядом другой — проверка
идёт приведением типа и сломается при первой же обёртке; он уже записан в
[conventions/errors.md](conventions/errors.md).
- **«Захват задачи не в транзакции — гонка двух воркеров».** По построению её
нет: три воркера читают три разных состояния, и одну строку они не делят.
Механика захвата и её слабые места — [database.md](database.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).
- `security`: не уходит ли значение, пришедшее снаружи, меткой метрики — страница
метрик отдаётся без проверки отправителя, и метка это поверхность пошире
журнала (журнал, запись 2026-08-11 про хвост имени).
- `architecture`: не появился ли второй путь приёма мимо
`createTranscribeJob` — сегодня через него идут оба входа
([architecture.md](architecture.md), «Единые точки проекта»).
- `architecture`: не поехало ли поведение в `architecture.md` вместо спеки —
заведена одна capability (`openspec/specs/intake`), поведение прочих узлов
живёт в обзоре под маркерами долга, и соблазн дописать туда ещё максимальный.
- `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/service/transcribe.go`, метки `file_extension` у размера
принятой записи и `source_format` у длительности конвертации
- **Симптом:** имя `запись.тайное-слово` клало `тайное-слово` меткой метрики, а
`GET /metrics` отдаётся без проверки отправителя. Тем же каналом множеством значений
метки распоряжался анонимный отправитель
- **Причина:** расширение берётся из имени отправителя дословно (`filepath.Ext`)
и употреблялось меткой без приведения. Канал старше задачи, которая его нашла
- **Чем воспроизведён:** прогон `filepath.Ext` на именах вида
`запись.тайное-слово`, `Разговор с Петровым 11.08`, затем чтение реестра
метрик после приёма — метка несла хвост дословно
- **Почему не поймали:** метрику никто не считал выходом приватного значения.
Тема `security` смотрела журнал, ответ и пути на диске; вопроса про метку в
перечне вопросов не было, и ни один проход её не открывал. Поймали три прохода
разом на задаче, которая закрывала соседний канал
- **Что меняем:** вопрос про метку добавлен в «Вопросы по темам»; правило
приведения нормировано спекой `intake` и записано
[решением](adr/ADR-2026-08-11-known-format-label.md)
## 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)