diff --git a/docs/reviews/CODE-REVIEW-500-r3.md b/docs/reviews/CODE-REVIEW-500-r3.md new file mode 100644 index 00000000..4e624bdc --- /dev/null +++ b/docs/reviews/CODE-REVIEW-500-r3.md @@ -0,0 +1,174 @@ +# CODE-REVIEW-500-r3 + +- **Issue:** #500 — Архитектура: выделить одну границу владения config/adoption +- **Материал:** `be6d57e96f425ceefa7ce5888785ac522e277b26` (ветка `issue/500-config-adoption-boundary`); рабочая копия уже была на этом SHA +- **Заход:** r3 · блокирующих циклов израсходовано 2/4 до этого раунда + +## Почему разбор по дельте, а не заново + +Материал и SHA предыдущего раунда объявлены в `docs/reviews/CODE-REVIEW-500-r2.md`: `93c6c7551b3a7f3df1901e69888611b1a7375ac4`. Этот SHA +живой и является прямым предком текущего `HEAD` (`git merge-base --is-ancestor 93c6c755 be6d57e9` → да; цепочка `93c6c755 → 5acf04fa +(docs: review document for #500) → be6d57e9` линейна, без ребейза). Дельта: + +``` +git diff 93c6c755..be6d57e9 --stat + demo/smoke_post_write_adoption.mjs | 7 ++ + docs/reviews/CODE-REVIEW-500-r2.md | 228 +++++++++++++++++++++++++++++++++++++ (документ предыдущего раунда, не код) + scripts/mutation-gate.mjs | 14 +++ +``` + +Один коммит реализации (`be6d57e9`) поверх doc-коммита `5acf04fa`. Продуктовый код (`src/**`) дельта не трогает вообще — только `demo/**` +(смок) и `scripts/**` (реестр мутантов), то есть class B по AGENTS.md. Ребейза на ушедший вперёд `dev` не было, контракт поведения не +менялся, новая подсистема не затронута, объём дельты (21 добавленная строка) на порядки меньше исходной задачи — все условия §2.10 для +разбора по дельте выполнены; полный разбор не требуется. + +## Скоуп дельты + +Единственная находка r2 (Medium-1: `demo/smoke_post_write_adoption.mjs` детерминированно красный на материале ревью, +`onboardingDeleteRefusedAdoptsNothing: expected true, got false`, причина — чужая отложенная запись `_saveConfigDebounced`/ +`_persistLayout` из предыдущего сценария доживает до следующего). Ответ автора («Ответ на код-ревью r2 — заход 3»): `reset()` смока +теперь отменяет обе отложенные записи в начале, плюс добавлен мутант `post-write-tail-runs-on-refused-gate` с гардом на этот смок, чтобы +witness больше не мог оставаться незапущенным CI незамеченным (ровно то, что произошло на r1→r2: witness был красным целый раунд, и ни +один гейт CI его не поймал, потому что ни один мутант не называл его своим гардом). + +## Как проверялось + +**Дешёвые гейты — подтверждены Validate на точном материале ревью, не перегонял отдельно.** +https://github.com/Matysh/houseplan-card/actions/runs/34476048247, `headSha` = `be6d57e96f425ceefa7ce5888785ac522e277b26` (сверено +`gh run view --json headSha`) — `success`. В этом прогоне джоб «Мутанты по диффу» **не был `skipped`** (в отличие от r2): это ревью-кандидат +(метка `S7-code-review` запускает Validate с мутантами по диффу на самом материале, PROCESS.md §2.7/AGENTS.md), поэтому прогон покрывает не +только `tsc --noEmit`/`npm test`/`npm run build`+bundle-sync, но и мутанты. Прочитал логи всех 6 шардов (`gh run view --job --log`), +не только факт green: + +``` +Шард 3/6, лог 102867009880: + ok чистый прогон: node demo/smoke_post_write_adoption.mjs + ok post-write-tail-runs-on-refused-gate: тест покраснел, как обязан +``` + +То есть CI сам подтвердил и чистый прогон смока (зелёный), и что новый мутант, снимающий ранний `return` в `_undoPlanOptimization`, красит +именно этот смок — то есть witness теперь реально запускается конвейером и реально ловит регрессию, чего не было в r1/r2. + +Дополнительно к CI-логам прогнал сам (Chromium в этой среде есть, `npm ci` не требовался): + +``` +npm run build && npm run bundle:sync → дерево чистое (git status --short пуст) +node demo/smoke_post_write_adoption.mjs × 3 подряд → OK, все 36 проверок true (все три прогона идентичны) +``` + +**Негативная проба (тест умеет падать) — воспроизвёл сам, не полагаясь на слово автора.** Временно убрал ровно те две строки +(`card._saveConfigDebounced.cancel(); card._persistLayout.cancel();`) из `reset()` в рабочей копии, прогнал смок, вернул файл на место: + +``` +FAILED (1): + - onboardingDeleteRefusedAdoptsNothing: expected true, got false +``` + +Дословно совпадает с воспроизведением, описанным в находке r2 — причина найдена верно, фикс минимален и точно закрывает названный +механизм, а не маскирует симптом. `git status --short` пуст после отката правки — рабочая копия не оставлена мутированной. + +**Мутант `post-write-tail-runs-on-refused-gate` (`scripts/mutation-gate.mjs`)** — цель патча (`src/houseplan-editor-runtime.ts:9578`, +`if (adopted.status !== 'adopted') return; // asset wait: the scheduled reload owns the tail`) уникальна в `src/**` +(`grep -rn "adopted.status !== 'adopted'" src/houseplan-editor-runtime.ts src/houseplan-onboarding-runtime.ts` даёt 4 совпадения, но +искомая строка с комментарием — ровно одна); `find`-строка мутанта совпадает с кодом дословно. Гард — тот же смок, что и находка; +структурная валидность записи (порядок, отсутствие дублей) подтверждена зелёными `test/mutation-gate.test.mjs`/`test/mutation-gate- +report.test.mjs` в том же CI-прогоне. + +**Что не прогонял и почему.** `npm run invariants` — дельта не касается геометрии/толщины/ссылок, только тестовую гигиену и реестр +мутантов. `python -m pytest tests_backend` — backend не в дельте. `golden:verify`/`performance_smoke` — визуал не меняется, дельта не +рендер. `node scripts/check-docs.mjs` отдельно не гонял — дельта не трогает `src/**` (только `demo/**`/`scripts/**`), отпечаток +скриншотов от неё не протухает; в Validate-прогоне `S7` docs-job не входит в набор джобов ревью-кандидата (полный `docs`-джоб — часть +предполётных проверок, прошёл в этом же прогоне success). Полный `smoke-select` (48 прямых совпадений по прошлому раунду) не прогонял +заново — дельта меняет один-единственный файл смока и реестр мутантов, остальные 47 совпадений её не касаются: это не изменение +семантики читаемых полей, а изоляция сценариев внутри одного смока. + +## Закрытие раунда r2 + +| Находка r2 | Чем закрыта | Где это видно | +|---|---|---| +| Medium-1 (единственный автоматический свидетель AC4/M1, `demo/smoke_post_write_adoption.mjs`, детерминированно красный на материале ревью из-за не отменённых отложенных записей в `reset()`) | Коммит `be6d57e9` добавляет `card._saveConfigDebounced.cancel(); card._persistLayout.cancel();` в начало `reset()` | `demo/smoke_post_write_adoption.mjs:86-87`; подтверждено CI (`Мутанты по диффу 3/6`: чистый прогон ok) и лично (3/3 прогона OK, негативная проба красная на снятой правке) | +| Сопутствующая дыра (witness не был зарегистрирован ни одним мутантом — CI не гонял его ни разу за весь r1/r2) | Новый мутант `post-write-tail-runs-on-refused-gate` с гардом на этот смок | `scripts/mutation-gate.mjs` (+14 строк); подтверждено CI-логом: `post-write-tail-runs-on-refused-gate: тест покраснел, как обязан` | + +## AC → что изменилось в этом раунде + +Только AC4 затронут дельтой (единственный незакрытый пункт из r2 — «код верен, свидетель красный»). Остальные AC дельта не задевает +(дельта не трогает `src/**`). + +| AC | Статус | Как проверено в этом раунде | +|---|---|---| +| AC4 (post-write гейт + ревизии из re-read, включая отказную ветку) | **Подтверждено полностью** | Единственный автоматический свидетель (`demo/smoke_post_write_adoption.mjs`) теперь зелёный и на материале ревью в CI, и в трёх личных прогонах; негативная проба лично воспроизведена и совпадает с диагнозом r2; новый мутант закрывает дыру «witness никогда не запускался» | + +## Унаследовано из r2 (дельта не задевает — продуктовый код `src/**` не менялся) + +- AC1 (идентичность пишет только модуль), AC2 (семь путей через `adoptAuthoritativeGated`) — код `src/**` не тронут дельтой r2→r3; + документ r2 перепроверил их чтением и тестом `config-adoption-ownership.test.mjs`. Документ: `docs/reviews/CODE-REVIEW-500-r2.md`, + материал `93c6c7551b3a`. +- AC3 (поведенческая нейтральность, профиль `post-write` без пост-шагов) — не задето, тот же материал r2. +- AC5 (тёплый старт round-trip), AC6 (optimistic rollback) — код `config-adoption.ts` не в дельте; r2 подтвердил мутантами + `config-adoption-rollback-ignores-rev` и повторным прогоном round-trip юнитов; в этом раунде мутант перепроверен CI на новом SHA (лог + шарда, `тест покраснел, как обязан`) без изменения содержания. +- AC7 (бюджеты) — `houseplan-card.ts`/бандл не менялись дельтой (diff-статистика: только `demo/**`+`scripts/**`); факты r2 (13605 строк, + 299 774 Б) остаются в силе, бюджет не пересчитывался, так как нечему. +- AC8 (документация) — `docs/ARCHITECTURE.md` не в дельте. +- Ревизии `space/delete` из `config/get`/`layout/get`, а не из ответа `delete` — продуктовая логика не в дельте, r2 подтвердил grep'ом. +- M1-фикс r1 (четыре post-write вызывающих симметрично пропускают хвост при отказе гейта) — продуктовый код не в дельте r2→r3; r2 + перепроверил чтением построчно (`houseplan-editor-runtime.ts:8694,9575,9736`, `houseplan-onboarding-runtime.ts:480`), это не + переоткрывается. +- Материал/содержание задачи в целом (одна граница владения, единая гейт-последовательность, `plan-optimize-write`/`space-copy`/vacuum- + писатели, сужение `SummaryPanelHost`/host-портов) — покрыто полным разбором r1 и повторным полным разбором r2 (ребейз на `dev` принёс + инфраструктуру #518, не тронувшую #500), эта дельта их не касается. + +## Что проверено и корректно (эта дельта) + +- Фикс `reset()` изолирует сценарии смока так же, как уже делает `demo/smoke_danger_confirmation.mjs:196` — согласованный с остальной + кодовой базой паттерн, не разовый хак. +- Единственная затронутая строка продуктового кода — цель нового мутанта, не сам продуктовый код; дельта не меняет поведение карточки. +- Новый мутант зарегистрирован единожды, без дублей, структура записи проходит `test/mutation-gate.test.mjs`/`test/mutation-gate- + report.test.mjs`. +- Trailers коммита `be6d57e9`: `Issue: #500`, `User-Visible: no` — верно (правка только `demo/**`+`scripts/**`, поведение карточки не + меняется); changelog не тронут, согласовано. + +## Чего не проверял и почему + +- Полный набор `smoke-select` (48 совпадений) — дельта не меняет семантику чтения ни одного поля за пределами `smoke_post_write_ + adoption.mjs`; полный набор — предрелизный гейт (§8). +- `npm run invariants`, `python -m pytest tests_backend`, `golden:verify`, `performance_smoke` — дельта не задевает геометрию, backend + или рендер. +- Полный повторный аудит продуктового кода #500 (`config-adoption.ts`, семь вызывающих, host-делегаты) — не задет дельтой, унаследован + из r1/r2 (см. раздел выше), не пересматривался заново. +- Собственный повторный прогон `tsc`/`npm test`/`npm run build` с нуля — не дублировал: Validate на точном материале ревью зелёный + (headSha сверен), включая на этот раз и мутанты по диффу; сверх этого лично прогнал сам смок (3×) и негативную пробу. + +## Материал раунда + +- SHA материала: `be6d57e96f425ceefa7ce5888785ac522e277b26` +- Дерево: рабочая копия была на этом SHA весь разбор; после build/bundle-sync и временной негативной пробы дерево возвращено в исходное + состояние (`git status --short` пуст) +- Предыдущий код-ревью: `docs/reviews/CODE-REVIEW-500-r2.md`, материал `93c6c7551b3a7f3df1901e69888611b1a7375ac4`, вердикт + `yellow · High 0 · Medium 1` +- ТЗ: `docs/specs/500-config-adoption-boundary.md`, зелёное ревью `docs/reviews/SPEC-REVIEW-500-r3.md` на `622470ff`; дельта r2→r3 файл + ТЗ не трогает + +## Вердикт + +Единственная находка r2 (Medium-1) закрыта содержательно и подтверждена независимо: CI поймал новый мутант на смоке-свидетеле, я лично +прогнал смок трижды (зелёный) и воспроизвёл красную пробу без фикса (совпадает дословно с диагнозом r2). Новых High/Medium/Low в дельте +не найдено. AC4 — последний ранее не полностью закрытый пункт — подтверждён полностью. Зелёный. + +--- + + + +## Материал раунда + +- Ветка: `issue/500-config-adoption-boundary`, коммит `be6d57e96f42` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `b756dee77c425861ccf68d44c06aa57a65336a78` + ``` + git log --all --format='%H %T' | grep b756dee77c42 + ``` +- Тело issue: `d4a68d8c756302bcc7b8ec4bf3a7f77ffc88848e51f2b054d58736bfda431ec5` +- ТЗ `docs/specs/500-config-adoption-boundary.md`, блоб `b7b4f02c6ca5a57092a360f1f664656b62ed968c` + ``` + git log --all --find-object=b7b4f02c6ca5a57092a360f1f664656b62ed968c -- docs/specs/500-config-adoption-boundary.md + ``` +- Вердикт конвейера: `green` · High 0