13 KiB
name: phase-review description: Ревью срезов и фаз проекта h-school по их же критериям приёмки — с дописыванием недостающих тестов, исправлением найденного и журналом проверенного в docs/phases//reviewed.md. Обязательно используй этот скилл, когда просят «сделай ревью», «проведи ревью», «проверь срез», «проверь непроверенные срезы», «что ещё не проверено», «review the slice» — и вообще при любой просьбе проверить, действительно ли сделанная фаза сделана. Не для ревью одного диффа или пул-реквеста: там нужен /code-review. Не для волны разработчиков — /phase-orch (он может запустить этот скилл как одного из агентов). Не для планирования нового среза — /slice-work.
Ревью срезов
Фазы в docs/phases/ помечаются ✅, когда их написали. Это заявка автора, а не
доказательство. Скилл превращает заявку в проверенный факт: берёт срез, сверяет каждое его
обещание с кодом и тестами, дописывает недостающее, чинит найденное и записывает результат,
чтобы одну и ту же работу не делать дважды.
Главное, что делает ревью здесь дешёвым: критерии приёмки уже написаны. У каждой фазы есть
«Задачи», «Тесты, без которых фаза не закрыта» и «Критерий готовности», а у проекта — список
инвариантов в AGENTS.md. Ничего не надо выдумывать, надо проверить.
Журнал
docs/phases/<slice>/reviewed.md — правда о проверке этого среза. Сводка ссылок —
короткий docs/phases/reviewed.md. Чужие журналы не читать. Если файла среза нет, создай
его и добавь строку в сводку.
Раздел на срез:
## Срез 4. Расписание
- **Фазы:** 14–17
- **Проверен на:** `3c54f98`, 2026-08-19
- **Пути:** `src/HSchool.Schedule`, `src/HSchool.Simulation/SchoolTimetables.cs`,
`src/HSchool.Server/Api/TimetableEndpoints.cs`, `tests/HSchool.Schedule.Tests`
- **Итог:** дописано 3 теста, исправлено 2 расхождения с дизайном, одно замечание оставлено
открытым (см. ниже)
Коммит в строке «Проверен на» — не украшение. Он делает журнал самопроверяющимся: срез,
проверенный на 3c54f98, перестаёт быть проверенным, как только его пути тронули дальше. Поэтому
«Пути» тоже обязательны — без них дрейф не поймать.
Как выбрать, что проверять
- Оглавление
docs/phases/README.mdи сводкаdocs/phases/reviewed.md— только чтобы выбрать срез. Дальше — индекс, фазы иreviewed.mdэтой папки, не остальные. - Прочитай журнал этого среза.
- Срез попадает в очередь, если:
- его нет в журнале; или
git log <коммит-из-журнала>..HEAD -- <пути среза>непустой — код менялся после проверки; это перепроверка, и в отчёте её надо называть именно так;- в журнале записано «проверен частично».
- Порядок — от раннего к позднему. Верхние срезы стоят на нижних: искать причину странного расписания, не проверив генерацию людей, значит искать не там.
Срезы в работе (есть ⬜ или 🔄, код уже пишется) проверять можно и нужно, но в журнале это отмечается как «в работе на момент проверки» — иначе следующий проход решит, что там всё закрыто.
Один срез за проход. Проверил → починил → записал в журнал → доложил → взялся за следующий. Если проходов будет несколько, пользователь увидит промежуточные результаты и сможет вмешаться, а не получит через час одну кучу правок в семи проектах.
/phase-orch мог отдать часть среза (этап A, фазы 29–31). Это всё ещё один проход: только
эти фазы, свой заголовок в журнале, ветка git branch review/<slug> main (не от HEAD), worktree
вне репо, дальше абсолютные пути этого worktree — журнал не коммитить в bug/… пользователя.
Слияние как у /phase-work (merge/lock общий с фазами и багами — чужой не снимать; --no-ff;
не пушь; основное дерево уже на main, не checkout bug/…). Чужой раздел reviewed.md не
затирай. Следующий срез сам не бери. Два ревьюера на один и тот же этап не садятся.
Что именно проверять
Идти сверху вниз, по каждому пункту фазы отдельно:
1. Обещания фазы. Для каждой задачи и каждого пункта «Тесты, без которых фаза не закрыта»
найди фактическое подтверждение: строчку кода, которая это делает, и тест, который это
проверяет. Галочка [x] подтверждением не является. Чаще всего расхождение выглядит так: код
написан, а теста из списка нет — либо тест есть, но проверяет соседнее.
2. Инварианты AGENTS.md, относящиеся к затронутым проектам. Особенно те, что нельзя
нарушить незаметно: протокол в трёх местах сразу, отсутствие HTTP и ASP.NET в Simulation и в
чистых библиотеках, фиксированный шаг вместо DateTime.Now, одна школа — один поток,
детерминированность от сида.
3. «Things that will bite you». Этот раздел AGENTS.md — список уже случившихся регрессий.
Проверить, что ни одна не вернулась, дешевле, чем поймать её второй раз.
4. Дизайн-док среза (docs/design/<slice>/, ссылка есть в индексе среза). Расхождение кода с
принятым решением — находка, даже если тесты зелёные. Но сначала спроси себя, не устарел ли док:
бывает, что решение сознательно поменяли, и тогда чинить надо документ.
Как проверять
Измеряй, а не рассуждай. Когда вопрос звучит как «а хорошо ли раскладываются часы» или «а сколько получается неполных семей» — не рассуждай о коде, напиши временный тест, который печатает реальные числа, посмотри на них и удали его. Час размышлений о том, как поведёт себя алгоритм, стоит дороже и ошибается чаще, чем один прогон, который печатает распределение.
Проверяй подозрение до того, как о нём докладывать. Половина находок «на глаз» рассыпается при первом же взгляде на соседний файл: параметр, который считался забытым, передаётся из вызывающего кода; поле, которого якобы нет в размере кадра, там есть. Проверенное подозрение — находка, непроверенное — шум, который пользователю придётся разбирать за тебя.
Что чинить
- Недостающие тесты дописывай в проект, который назначен политикой из
AGENTS.md(«Testing policy»). Тест генерации людей не место в тестах хоста. - Чини причину, а не симптом. Падающий тест не ослабляют и не удаляют ради зелёного прогона. Если тест действительно неправ — так и напиши в отчёте, с обоснованием, и меняй его осознанно.
- Держись границ среза. Найденное за его пределами — в отчёт строкой «замечено рядом», а не в диф. Ревью, которое походя переписало соседний проект, невозможно посмотреть глазами.
- Молча не меняй версию протокола, форму сейва и публичное поведение API. Это отдельное решение пользователя, даже когда оно очевидно правильное.
Какие тесты гонять
Сначала читай код и имена тестов — ревью это сверка обещаний, не ритуал прогона. Когда
дописал недостающий тест или починил найденное, гоняй только проект среза из Testing
policy в AGENTS.md, с --filter на новый класс. Клиентский Vitest — только если ревью
трогает src/HSchool.Client. HSchool.AppHost.Tests — только если срез про HTTP, сокет
или хост. Весь solution, парный npm test «для уверенности» и прогон в начале ревью —
не делать.
Чего ждать и что делать:
MSB3021/MSB3027, «блокирует этот файл» — у пользователя запущено приложение, оно держит DLL. Процесс не убивать. Проверь то, что можно проверить без сборки .NET (клиентские тесты, чтение кода, дизайн-доки), а в отчёте скажи прямо: .NET-часть не прогонялась, потому что запущено приложение. В журнале такой срез — «проверен частично».- Концы строк. В репозитории есть файлы и с CRLF, и с LF. Правка не должна переворачивать
файл целиком: сверься с
git diff --stat— внезапные «изменено 400 строк» в файле, где ты правил три, это оно. - Dev-сервер не запускать. Если нужно посмотреть на UI, пользуйся уже запущенным приложением пользователя; свой не поднимай и чужой не останавливай.
Отчёт
Коротко и по делу, в конце каждого среза:
- что проверено и чем это подтверждено;
- что дописано (тесты — списком, по одной строке);
- что исправлено и почему это была ошибка;
- что осталось под вопросом — с формулировкой, по которой можно принять решение;
- какой срез следующий в очереди.
Пустой отчёт — тоже результат: «срез 2 проверен, все восемь тестов из фазы 6 на месте, расхождений с дизайном нет» стоит написать явно. Это ровно та информация, ради которой затевался журнал.