diff --git a/docs/reviews/CODE-REVIEW-425-r1.md b/docs/reviews/CODE-REVIEW-425-r1.md new file mode 100644 index 00000000..0abc4add --- /dev/null +++ b/docs/reviews/CODE-REVIEW-425-r1.md @@ -0,0 +1,169 @@ +# CODE-REVIEW-425-r1 + +- Issue: https://github.com/Matysh/houseplan-card/issues/425 +- Ветка: `issue/425-core-file-budget`, материал: `git diff origin/dev...HEAD` на SHA `d4ecace6` +- ТЗ: `docs/specs/425-core-file-budget.md` (полный трек), спек-ревью зелёное на r2 + (`docs/reviews/SPEC-REVIEW-425-r2.md`) +- Заход: r1 · блокирующих циклов израсходовано 0 из 4 + +## Скоуп + +Диапазон `origin/dev..HEAD` — 5 коммитов: + +``` +4eae3398 docs: specify the core file budget gate (#425) +3df0f164 docs: review document for #425 +84b5f63c docs: address the core budget spec review (#425) +f399ad54 docs: review document for #425 +d4ecace6 test: cap the two frontend cores with a ratchet (#425) +``` + +Единственный коммит с продуктовым воздействием на гейты — `d4ecace6`: + +``` +docs/TESTING.md | 14 ++++++ +scripts/mutation-gate.mjs | 13 +++++ +test/core-file-budget.test.mjs | 109 ++++++++++++++++++++++++++++++ +``` + +Остальные — спек-документы и ревью-документы предыдущего этапа (класс C), к +код-ревью не относятся содержательно. Диф не трогает `src/**`, +`custom_components/**`, `demo/**` — класс B (гейты и инструменты), issue +переиспользован как разрешено таблицей классов. `User-Visible: no` на каждом +коммите корректно: продуктовый код не менялся. + +## Как проверялось + +| Гейт | Команда | Результат | +|---|---|---| +| typecheck | `npx tsc --noEmit` | зелёный, 0 ошибок | +| unit (полный) | `npm test` | 1778 тестов, 1777 pass, 1 skip (`issue 281 private exact fixture...` — существующий условный skip, не связан с #425), 0 fail | +| build + сверка копии | `npm run build && cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | бандл собран, побайтовое совпадение; копия `demo/srv/assets/**` не коммитится (#255) и не сравнивается | +| целевой тест | `node --test test/core-file-budget.test.mjs` | 7/7 pass | +| мутант (штатный раннер) | `node scripts/mutation-gate.mjs --id=core-budget-ignores-growth` | `ok core-budget-ignores-growth: тест покраснел, как обязан` — поймано 1 из 1 | +| проверка якорей мутанта | `node scripts/mutation-gate.mjs --check` | `ok core-budget-ignores-growth` — патч применим к текущему коду | +| выбор браузерных смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет (`src/**/*.ts` не тронут). Browser-smoke этим диффом не выбираются — выбирать нечего.» | +| process-gate (офлайн) | `node scripts/process-gate.mjs --base origin/dev --head HEAD` | «гейт пройден, предупреждений 0» | +| фактические размеры ядер | `node -e "…split('\n').length"` на обоих файлах | `houseplan-card.ts` = 13659, `houseplan-editor-runtime.ts` = 14323 — совпадает с `CAPS` в тесте | + +### Чего не проверял и почему + +- `node scripts/check-docs.mjs` — не запускал: диф не трогает `src/**`, условие + «любой diff по `src/**` требует check-docs» не выполнено. +- Полный прогон `node scripts/mutation-gate.mjs` (все мутанты) — не гонял: + дорогой прогон, задача трогает ровно один новый мутант, который проверен + адресно через `--id`. Попутно замечено (не находка этой задачи): полный + прогон в этом окружении падает на несвязанном бэкенд-мутанте из-за + отсутствующего `pytest` (`No module named pytest`) — окружение без Python- + венды, не регрессия #425. +- `golden:verify`, `smoke_*.mjs`, `pytest tests_backend`, performance-профили — + диф не меняет визуал, DOM, Python-код или производительность; AC их не + называют. `smoke-select.mjs` независимо подтвердил «выбирать нечего». +- `node scripts/model-invariants.mjs` — диф не касается геометрии, толщины стен, + `layout`, `marker.space`, `open_spans`. +- Ручное тестирование в браузере — задача не создаёт видимого поведения + (`User-Visible: no`, продуктовый код не тронут), пункт неприменим по + определению задачи. + +## Разбор по AC + +- **AC1** (рост ядра выше потолка роняет `npm test` с числом). Доказано + исполнением, не чтением: мутант `core-budget-ignores-growth` + (`scripts/mutation-gate.mjs:936-948`) добавляет 400 строк в + `src/houseplan-card.ts` перед `export class HouseplanCard`, что больше люфта + (250). Прогон `node scripts/mutation-gate.mjs --id=core-budget-ignores-growth` + дал «тест покраснел, как обязан» — тест реально умеет падать, а не только + умеет проходить. Текст сообщения называет файл, потолок и превышение + (`test/core-file-budget.test.mjs:41-47`), проверено юнитом + «рост выше потолка становится нарушением с числом» (строки 67-72). +- **AC2** (уменьшение больше чем на 250 строк тоже роняет тест, с требованием + опустить потолок). Доказано юнитом «заметное уменьшение требует опустить + потолок» (`:74-79`): проверяет `kind==='shrank'`, `under===300`, текст + содержит «Опустите потолок». Чтением подтверждено, что тот же путь исполнения + используется и на реальных ядрах (`measure` вызывается один раз в верхнем + тесте, `coreBudgetViolations` — чистая функция без ветвления по источнику + данных). +- **AC3** (изменение в пределах люфта тест не трогает). Доказано юнитами + «изменение в пределах люфта не трогает никого» и «границы включительно» + (`:81-92`) — отдельно проверены точки `cap`, `cap-slack`, `cap+1`, `cap-slack-1`, + то есть обе границы включительно и оба соседних шага исключительно. +- **AC4** (потолки — числа в тесте, не вычисление от текущего размера). + Доказательство по ТЗ — «ревью кода плюс юнит на то, что функция принимает + потолки аргументом». Юнит «потолки — числа в этом файле…» (`:99-109`) + парсит блок `const CAPS` из исходника теста и проверяет regex-отсутствие + `measure|readFileSync|process.env` внутри блока — то есть механически + исключает как раз тот способ обмана, который ТЗ называет неприемлемым + (потолок, вычисленный от текущего размера). Прочитано чтением: `CAPS` + (`:21-24`) — действительно литерал из двух чисел, `coreBudgetViolations` + (`:33-57`) получает `caps` и `slack` параметрами и не читает файлы и `env` + сама; чтение файлов инкапсулировано в отдельной `measure` (`:59`), вызываемой + только в интеграционном тесте верхнего уровня. Проверено чтением, не + исполнением, в части «архитектурного» инварианта (что функция не может + прочитать файл сама), исполнением — в части regex-проверки текста. +- **AC5** (гейт судит только два ядра, остальные файлы не ограничены). Доказано + юнитом «потолки заданы для двух ядер и ни для чего больше» (`:94-97`), + сверяющим точный список ключей `CAPS`. Чтением подтверждено, что других мест + в `src/**`, ссылающихся на `CAPS`/`coreBudgetViolations`, нет (единственный + потребитель — сам файл теста). + +Все пять AC доказаны: AC1 — мутантом, исполненным лично (не на слово автора); +AC2/AC3/AC5 — юнитами, прогнанными лично; AC4 — комбинацией юнита и чтения кода +с явной пометкой, какая часть доказана как. + +## Соответствие ТЗ и процессу + +- Числа `CAPS` (`13659`, `14323`) совпадают с фактическим размером файлов на + этом SHA, измеренным независимо (`split('\n').length`), что и есть мера, + описанная в комментарии над `CAPS` (`:14-16`) и совпадающая с мерой в + `measure()` (`:59`) — единственный источник числа, нет второго измерения той + же величины другим способом (правило «одно число — один источник» неприменимо + буквально, т.к. величина инженерная и не видна пользователю, но принцип + соблюдён: потолок и измерение используют одну и ту же функцию подсчёта строк). +- `docs/TESTING.md` дополнен разделом с объяснением, что делать при красном + гейте — соответствует Release-артефактам ТЗ (только `TESTING.md`, + changelog не требуется при `User-Visible: no`). +- Расхождение чисел в теле issue (таблица «dev: 13658») с `CAPS` («13659») — + не дефект: таблица в issue использует `wc -l` (мера issue), тест — мера + `split('\n').length`; автор явно проговорил это расхождение в хендофф- + комментарии и держит одну меру в коде, что и требовалось. Само число в + тексте issue — историческая иллюстрация, не контрактное значение. +- Скоуп ТЗ («один тест с потолками и храповиком, мутант, строка в + `docs/TESTING.md`») выдержан ровно, попутных правок нет. +- SCOPE.md: задача инженерная, `User-Visible: no`, не обязана закрывать + Core user job — прецедент (#337 bundle-budget, #85 mutation-gate) уже принят + ревью ТЗ на r1, не переоткрываю. + +## Находки + +Нет находок High или Medium. Low не обнаружено сверх уже снятого на этапе ТЗ. + +Одна стилистическая заметка, не являющаяся дефектом (не заводится ни как Low, +ни как issue): пункт `docs/TESTING.md:3593` оформлен чек-боксом `- [ ]`, хотя +проверка полностью автоматическая и уже входит в `npm test` — соседние +чек-боксы в этом файле смешивают ручные и автоматические проверки, так что +формат не расходится с текущей конвенцией документа. + +## Материал раунда + +- Ветка: `issue/425-core-file-budget` +- SHA на момент вывода: `d4ecace6` (сверено непосредственно перед подведением + итогов, см. `git rev-parse HEAD` в начале сессии — совпадает с `HEAD` + диапазона `origin/dev..HEAD`, использованного во всех командах выше) +- Дерево материала: полный диф `origin/dev...HEAD`, 6 файлов изменено (см. + таблицу выше) + +--- + + + +## Материал раунда + +- Ветка: `issue/425-core-file-budget`, коммит `d4ecace64c51` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `226386b38b1cb661fcb45b2c8f01a568b954a70a` + ``` + git log --all --format='%H %T' | grep 226386b38b1c + ``` +- ТЗ `docs/specs/425-core-file-budget.md`, блоб `0469a1eec26759469cf469c87136556935a1f629` + ``` + git log --all --find-object=0469a1eec26759469cf469c87136556935a1f629 -- docs/specs/425-core-file-budget.md + ```