mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
@@ -0,0 +1,213 @@
|
||||
# CODE-REVIEW-699-r1
|
||||
|
||||
Issue: #699 · этап: code · трек: show · заход: r1 · блокирующих циклов до этого раунда: 0/2
|
||||
|
||||
Материал раунда: `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD`,
|
||||
вершина ветки `issue/699-ratchet-bands` = `709c5b8a38f4ac16f7ce18593ef0a40d672742cb`.
|
||||
Validate на этом SHA — success (run 36486854256). Ребейз на dev не делался
|
||||
(трек show, #696): dev впереди на 14 коммитов, слияние без конфликта
|
||||
(`git merge-tree`, подтверждено автором); материал — ветка как есть.
|
||||
|
||||
Предыдущего раунда ревью по этой задаче не было: заход `3dd032d7` (первая
|
||||
попытка) упал на Validate до того, как ревью запускалось («ревью не
|
||||
запускалось», код никто не читал, цикл не потрачен). Второй заход `709c5b8a`
|
||||
чинит ровно то, что назвал прогон, и это первый заход, дошедший до ревьюера —
|
||||
поэтому разбор ниже полный, а не по дельте (§2.10 к этой задаче неприменим).
|
||||
|
||||
## Скоуп
|
||||
|
||||
Инфраструктурная задача (процесс/гейты), файлов класса A нет. Меняет правило
|
||||
двусторонних храповиков с нулевым запасом на полосу над потолком последней
|
||||
беты: строки двух ядер (`test/core-file-budget.test.mjs`, полоса 50), gzip
|
||||
initial View и трёх ленивых графов (`scripts/bundle-budget.mjs`, полоса
|
||||
2000 Б), шесть чисел связности монолита (`scripts/monolith-metrics.mjs`,
|
||||
`scripts/unused-locals-gate.mjs`, `METRIC_BANDS`), и переводит лимит 200
|
||||
браузерных мутантов из жёсткого в ориентир (`mutation-browser-policy.mjs`,
|
||||
`mutation-registry-check.mjs`). Второй коммит правит найденный Validate дефект:
|
||||
CLI `bundle-budget.mjs` проверял абсолютный бюджет раньше потолка беты, из-за
|
||||
чего рост сверх полосы (`ceiling + band` выше `INITIAL_VIEW_GZIP_BUDGET`)
|
||||
маскировался бюджетом и не был наблюдаем. Новый файл `scripts/ratchets.mjs` —
|
||||
инструмент релиз-менеджера: `report [--warn]` (факт против потолков,
|
||||
используется в `release:prerelease`) и `tighten` (потолки := факт, ручной шаг
|
||||
на кандидате беты).
|
||||
|
||||
Владелец принял оба открытых вопроса ТЗ (issue-комментарий 2026-09-28): полоса
|
||||
ядра = 50 строк, лимит браузерных мутантов становится ориентиром. Оба решения
|
||||
корректно реализованы теми же числами, что названы в issue.
|
||||
|
||||
User-Visible: no на обоих коммитах, верно — `src/**` не тронут, продуктовое
|
||||
поведение не меняется. Трейлеры `Issue: #699` и `User-Visible: no` на месте на
|
||||
обоих коммитах.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дешёвые гейты подтверждены зелёным Validate на этом SHA (`typecheck`,
|
||||
`npm test`, `npm run build` + bundle-policy) — не перегонялись повторно.
|
||||
Поверх этого, по диффу:
|
||||
|
||||
| Гейт | Прогнан | Результат |
|
||||
|---|---|---|
|
||||
| `node --test test/process-digests.test.mjs` | да | 5/5 ok — конспект `docs/process/REVIEWER.md` не разошёлся с PROCESS.md (диф этой задачи REVIEWER.md не трогает) |
|
||||
| `node --test test/ratchets.test.mjs test/core-file-budget.test.mjs test/monolith-metrics.test.mjs test/mutation-gate.test.mjs test/bundle-assets.test.mjs` | да | 125/125 ok |
|
||||
| `node scripts/ratchets.mjs report` (на закоммиченном `dist/`) | да | числа совпали один в один с таблицей из комментария автора: оба ядра и пять чисел монолита `равны факту`, четыре графа `рыхлые` на 550–960 Б |
|
||||
| `node scripts/ratchets.mjs tighten` — в изолированном `git worktree` от этого SHA, с тем же `dist/` | да (ручная проверка ревьюером, исполнением) | опустил ровно четыре рыхлых потолка и `bundleBytes` до факта, ядра и точные метрики не тронул (файл не переписан, если число не изменилось); повторный `report` после этого показал «равен факту» по всем строкам |
|
||||
| `node scripts/mutation-gate.mjs --check` | да | exit 0, без FAIL; `browser guards: 200/200`; 3 предсуществующих WARN не относятся к #699 (`#650`, `${…}` в паттернах) |
|
||||
| `grep` литералов монолита в `test/ratchets.test.mjs` + `node --test test/monolith-text-anchors.test.mjs` | да | новый тест не задевает замороженный список `FROZEN_TEXT_ANCHOR_TESTS` (#624 AC4) — подтверждает, что первый упавший прогон `gate:small` автора («мой тест называл ядро строкой») действительно исправлен |
|
||||
| чтение `scripts/ratchets.mjs`, `scripts/bundle-budget.mjs`, `scripts/monolith-metrics.mjs`, `scripts/unused-locals-gate.mjs`, `scripts/mutation-registry.mjs`, `scripts/mutation-registry-check.mjs`, `scripts/release-prerelease.mjs`, `PROCESS.md`, `docs/TESTING.md` целиком | да | см. находки и раздел «проверено корректно» |
|
||||
|
||||
**Не прогонялось и почему:** полный дифф-мутационный прогон (`mutation-gate.mjs`
|
||||
с реальными патчами, 186 мутантов по заявлению автора) — трек show, Validate
|
||||
уже зелёный на этом SHA, а структурная проверка реестра (`--check`) чистая;
|
||||
браузерные смоки — тело issue их не называет, правка не трогает `src/**`,
|
||||
`node scripts/smoke-select.mjs` не запускался, так как нет диффа по фронтенду,
|
||||
который он мог бы сопоставить со смоками; `golden:verify`, `pytest
|
||||
tests_backend`, `npm run invariants`, performance-профили — ни один не
|
||||
применим (нет диффа по рендеру/Python/геометрии, AC их не называет).
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе, чинится в этой же задаче)
|
||||
|
||||
**M1. Новый шаг «опустить потолки на бете» не попал в единственный канонический
|
||||
release-runbook.** `docs/DEVELOPMENT.md` прямым текстом объявляет себя
|
||||
единственным домом release-механики («This section is the only home of the
|
||||
release mechanics»), и раздел «Primary prerelease path» — чек-лист, который
|
||||
реально читает релиз-менеджер перед публикацией кандидата (синхронизировать
|
||||
версии, changelog, `npm run bundle:release`, `RELEASE-NOTES.md`). В этом
|
||||
чек-листе нет ни слова про `node scripts/ratchets.mjs tighten`, хотя весь
|
||||
смысл #699 — в том, что «вторая сторона храповика» переехала именно сюда
|
||||
(`PROCESS.md` §8, `scripts/ratchets.mjs`: «закоммитить вместе с кандидатом
|
||||
беты»). Единственное место, где инструмент реально упомянут релиз-менеджеру —
|
||||
`release-prerelease.mjs`, вызывающий `ratchets.mjs report --warn` **внутри
|
||||
самой команды публикации**, то есть уже после того, как кандидат зафиксирован,
|
||||
провалидирован и запрошена публикация. К этому моменту откатывать «один
|
||||
коммит вместе с кандидатом» уже поздно — нужен отдельный коммит, а чек-лист
|
||||
подготовки кандидата про это ничего не говорит.
|
||||
|
||||
*Воспроизведение:* `grep -n "ratchets" docs/DEVELOPMENT.md` — пусто; раздел
|
||||
«Primary prerelease path» (`docs/DEVELOPMENT.md:516-560`) не содержит этого
|
||||
шага. `release-prerelease.mjs:511` вызывает `ratchets.mjs report --warn`
|
||||
только внутри `if (invokedDirectly)`, после `releaseView()` — то есть в момент
|
||||
публикации, не подготовки.
|
||||
|
||||
*Почему это не мелочь:* без записи в реальном runbook шаг будет забываться
|
||||
всегда, а не иногда — предупреждение видно только тому, кто уже нажал
|
||||
публикацию, и не гейтится ничем (`--warn` всегда возвращает 0). Потолки
|
||||
останутся рыхлыми навсегда, и половина заявленной пользы задачи (полоса
|
||||
считается от факта беты, а не от старой точки) не реализуется на практике.
|
||||
Это не гипотетический сценарий: на этом самом SHA `report` уже показывает
|
||||
четыре рыхлых графа и `bundleBytes` — если следующая бета выйдет без ручного
|
||||
запуска `tighten`, отсчёт полосы продолжится от чисел годичной давности.
|
||||
|
||||
*Что нужно:* добавить в чек-лист «Prepare the candidate as usual»
|
||||
(`docs/DEVELOPMENT.md`, раздел «Primary prerelease path») шаг
|
||||
`node scripts/ratchets.mjs tighten` перед коммитом кандидата — рядом с
|
||||
`npm run bundle:release`, до пуша и Validate.
|
||||
|
||||
### Low (снято ревьюером)
|
||||
|
||||
**L1. Оркестрация `tighten` (ветка `command === 'tighten'` в
|
||||
`scripts/ratchets.mjs`) не покрыта автотестом** — юниты покрывают только
|
||||
чистые функции (`readCoreCaps`/`rewriteCoreCaps`, `readConst`/`rewriteConst`,
|
||||
`ratchetState`, `ratchetRows`), а сама склейка (фильтрация строк по `kind`,
|
||||
сборка `coreFacts`/`baseline`, порядок трёх `writeFileSync`) — нет. Снимаю
|
||||
находку без возврата в задачу: проверил исполнением сам — в изолированном
|
||||
`git worktree` от `709c5b8a` с тем же `dist/`, что и в дереве, `tighten`
|
||||
опустил все четыре рыхлых графа и `bundleBytes`, не тронул уже точные ядра и
|
||||
метрики монолита, и результат подтверждён повторным `report` (все строки —
|
||||
«равен факту»). Это инструмент релиз-менеджера, запускаемый вручную раз в
|
||||
бету, с диффом трёх файлов, который человек обязан просмотреть перед
|
||||
коммитом (по инструкции инструмента) — риск непойманной регрессии невысокий и
|
||||
несоразмерен цене автотеста, пишущего в реальные файлы гейтов. Если в
|
||||
следующей бете `tighten` даст неверные числа — это будет видно в `git diff`
|
||||
до коммита.
|
||||
|
||||
## Проверено и корректно
|
||||
|
||||
- Полоса включительна с обеих сторон корректно и симметрично на всех трёх
|
||||
видах храповика: границы `cap+band`/`cap+band+1` для ядра, gzip-графов
|
||||
(initial View и ленивых) и чисел монолита проверены отдельными тестами по
|
||||
каждой стороне (`core-file-budget.test.mjs`, `bundle-assets.test.mjs`,
|
||||
`monolith-metrics.test.mjs`) — прогнаны, зелёные.
|
||||
- Снижение больше не красит ветку нигде из трёх мест
|
||||
(`initialViewCeilingViolation`/`lazyGraphCeilingViolation` больше не имеют
|
||||
ветки `kind: 'shrank'`; `compareWithBaseline`/`decide` печатают `info`, а не
|
||||
`FAIL`) — старый текст «Опустите потолок» вычищен из кода и тестов везде,
|
||||
где раньше проверялась двусторонность (проверено `grep` по всему дереву,
|
||||
ни одного мёртвого упоминания старой ветки).
|
||||
- Реордер в CLI `bundle-budget.mjs` (потолок беты проверяется до
|
||||
`assertBundleBudget`) не создаёт второй источник числа: `result` из
|
||||
`assertBundleBudget` и `manifest.initialViewGzipBytes`, использованный для
|
||||
проверки потолка, — один и тот же непреобразованный byte-count из манифеста
|
||||
(прочитано в исходнике: `assertBundleBudget` возвращает поля манифеста
|
||||
насквозь, без пересчёта). Правка действительно чинит баг, названный в
|
||||
сообщении Validate (`initial-view-ceiling-unplugged`): построил ветку в
|
||||
изолированном чтении — старый порядок (бюджет раньше потолка) при
|
||||
`ceiling+band` выше `INITIAL_VIEW_GZIP_BUDGET` действительно маскирует
|
||||
проверку потолка бюджетом.
|
||||
- Мутанты реестра (`scripts/mutation-registry.mjs`): 5 новых
|
||||
(`core-band-ignored`, `initial-view-band-below-ceiling-again`,
|
||||
`monolith-band-exact-again`, `monolith-shrink-fails-branch-again`,
|
||||
`ratchet-report-calls-band-tight`) и один перенацеленный
|
||||
(`monolith-metrics-baseline-strict`) — якоря найдены ровно по одному разу
|
||||
(`mutation-gate.mjs --check` зелёный), guard-паттерны совпадают с
|
||||
существующими именами тестов, `because` объясняет, какую регрессию мутант
|
||||
ловит. Мутант, из-за которого автор чинил `bundle-budget.mjs` во втором
|
||||
коммите (`initial-view-ceiling-unplugged`), обновлён на новое место якоря и
|
||||
по-прежнему ловится тем же тестом.
|
||||
- `mutation-registry-check.mjs`: превышение ориентира браузерных мутантов
|
||||
теперь инкрементирует `warned`, а не `stale` — код выхода определяется
|
||||
только `stale`, значит превышение больше не блокирует гейт, при этом
|
||||
строка предупреждения печатается и называет способ обоснования — ровно то,
|
||||
что решил владелец.
|
||||
- Первый провал автора на `gate:small` (`#624 AC4`, «мой тест называл ядро
|
||||
строкой») действительно исправлен: `test/ratchets.test.mjs` не содержит
|
||||
литералов `houseplan-card.ts`/`houseplan-editor-runtime.ts`/
|
||||
`houseplan-source.mjs`, `test/monolith-text-anchors.test.mjs` зелёный,
|
||||
список `FROZEN_TEXT_ANCHOR_TESTS` не тронут.
|
||||
- `PROCESS.md` §3 и §8, `docs/TESTING.md` описывают новое поведение
|
||||
согласованно с кодом: полоса, кто её опускает, где стена (`
|
||||
INITIAL_VIEW_GZIP_BUDGET` осталась абсолютной, это подтверждено и в CLI, и
|
||||
в тексте).
|
||||
- Число «потолок initial View» видно в тексте PROCESS.md/TESTING.md/ratchets.mjs
|
||||
и в самом коде ровно с одним источником — `INITIAL_VIEW_GZIP_CEILING` в
|
||||
`bundle-budget.mjs`; `ratchets.mjs` читает его оттуда же регэкспом, не
|
||||
дублирует.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полный дифф-мутационный прогон `mutation-gate.mjs` с реальными патчами (186
|
||||
мутантов по заявлению автора, 6 шардов) — не перегонял; ограничился
|
||||
структурной проверкой `--check` и точечными юнитами по изменённым файлам.
|
||||
Трек show и зелёный Validate на этом SHA делают повторный полный прогон
|
||||
избыточным для ревью.
|
||||
- Браузерные смоки — не выбирались и не гонялись: диф не трогает `src/**` и
|
||||
ничего в demo/, `smoke-select.mjs` не запускался за отсутствием диффа,
|
||||
который он мог бы сопоставить.
|
||||
- `golden:verify`, `pytest tests_backend`, `npm run invariants`,
|
||||
performance-профили — не применимы к этому диффу (нет рендера, Python,
|
||||
геометрии; AC их не называет).
|
||||
- Не проверял, действительно ли `release:prerelease` печатает `--warn` в
|
||||
реальном GitHub Actions прогоне публикации (только прочитал код и юнит
|
||||
`test/ratchets.test.mjs`, который проверяет точную форму вызова текстом —
|
||||
не исполнением всей команды `release:prerelease`).
|
||||
|
||||
## Вердикт
|
||||
|
||||
Один Medium в скоупе (M1) без High → жёлтый, возврат автору. Один Low (L1)
|
||||
снят ревьюером с записью — проверен исполнением, дополнительной правки не
|
||||
требует.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/699-ratchet-bands`, коммит `709c5b8a38f4` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `a18812c473f68a3fca17fb0b1b9df7909a219339`
|
||||
```
|
||||
git log --all --format='%H %T' | grep a18812c473f6
|
||||
```
|
||||
- Тело issue: `18b6f2dce7c2f914e9c4d61715360cb529fc34c87cbad12c2a646990fefcb724`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user