diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index cfe29ccf..9ab5894c 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,10 +1,11 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 231, issue: 111. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 232, issue: 112. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| | бета v1.79.0-beta.1 | [SHIP-REVIEW-v1.79.0-beta.1.md](SHIP-REVIEW-v1.79.0-beta.1.md) | пакетное ревью ship · — | ⚪ — | 0 | 0 | — | — | +| #739 | [SPEC-REVIEW-739-r1.md](SPEC-REVIEW-739-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #737 | [SPEC-REVIEW-737-r1.md](SPEC-REVIEW-737-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #732 | [CODE-REVIEW-732-r1.md](CODE-REVIEW-732-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #730 | [CODE-REVIEW-730-r1.md](CODE-REVIEW-730-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | текст сводки «повтор и ребейз не помогут» вводит в заблуждение именно в сценарии, котор… | `scripts/merge-candidate.mjs` | diff --git a/docs/reviews/SPEC-REVIEW-739-r1.md b/docs/reviews/SPEC-REVIEW-739-r1.md new file mode 100644 index 00000000..ad62088c --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-739-r1.md @@ -0,0 +1,159 @@ +# SPEC-REVIEW-739-r1 + +Issue: #739 · Этап: spec (PROCESS.md §2.4) · Трек: ask · Заход: r1 · блокирующих циклов 0/4 + +## Материал раунда + +- ТЗ: тело issue #739 (раздел `## ТЗ`), снято `gh issue view 739` в момент + ревью; issue открыт, метки `S4-spec-review`, `tech-debt`, `track:ask`. + Комментарий автора: оценка ценность 4/10 · сложность 3/10 · P3. +- Рабочая копия репозитория — `HEAD` на `3b9f25ea` (материал параллельного + code-review этапа, не связан с этой задачей по содержанию); для spec-этапа + код читался как текущее состояние `dev`, в которое #739 будет впадать после + merge `#725` (подтверждено: `235f60e6` — коммит #725 — предок `HEAD`). + +## Скоуп + +К1: в 2.5D с подложкой (`imagePlan: true`) переключение этажа не ищет заново +цвет бумаги карточки, если идентичность темы (`darkMode`, `default_theme`, +`default_dark_theme`, `theme`) и `_mode` не изменились, и рендерит карточку +один раз вместо двух. `revealActiveTab()` (`header-menu.ts`) и +`SummaryPanelPresentation.updated()` сознательно выведены из скоупа решением +по умолчанию («Решено по умолчанию», п.1) — это техническое решение +(§7.1), не продуктовый вопрос, и ревьюер с ним согласен (см. «Как +проверялось»). + +Обслуживает J1 (`docs/SCOPE.md`) — «show the whole home and what's +happening now» в режиме живого обзора по этажам; задача найдена при +реализации #725 (тот же перф-трек), не самостоятельная фича. + +## Как проверялось + +Ревьюер ≠ автор: issue и ТЗ прочитаны без устных пояснений. Вместо +доверия числам и строкам кода в тексте ТЗ каждое фактическое утверждение +сверено с реальным деревом: + +1. **Обязательные разделы §7.1** — все на месте: сценарий, что человек + увидит до/после, проблема, скоуп и не-скоуп, контракт поведения, + UX·данные·i18n·touch, граничные случаи (§2.6), критерии приёмки AC1–AC5 + с доказательством, план автотестов, риски, откат, release-артефакты, + плюс оба блока решений (решено по умолчанию / принято предположительно). +2. **Технический корень дефекта** прочитан в коде, не принят на слово: + - `src/iso-first-frame.ts` целиком — `isoPaperContext` (строки 18–28), + `prepare`/`sync`/`commitPaper` (45–86) — ключ `key` действительно + включает `space` первым элементом массива (строка 24), поэтому любое + переключение этажа меняет `key` и форсирует `commitPaper` заново + (строка 56, вызов `resolveThemePaper()`), что ровно описывает + найденный дефект. + - `src/houseplan-card.ts:4004` (`willUpdate` → `prepare(...)`) и `:4046` + (`updated` → `sync(...)` → `requestUpdate()` при `true`) — номера + строк и цепочка вызовов подтверждены построчно. + - `src/header-menu.ts:127,130` (`revealActiveTab`, `nav.clientWidth`) и + `src/summary-panel-presentation.ts:36–37,53` (`updated`, ранний выход + без `pending`, `getComputedStyle`) — номера строк и условия выхода + совпадают с текстом «Не-скоуп» дословно. + - `scripts/render-layout-read.mjs` прочитан целиком: гейт уже проверяет + весь `src/iso-*.ts` на принудительные чтения раскладки и не трогает + `updated()` карточки отдельно — вызов `_cssColor` из `updated()` + (разрешённое место) и предлагаемое кеширование цвета не меняют состав + гейта; фраза ТЗ «расширять на файл гейт нет смысла» для + summary-panel подтверждена чтением условия `pending` в коде. +3. **Существование артефактов, на которые ссылается план автотестов**: + `test/iso-stage6.test.mjs` (включая строку `a theme change hides the + stale classification`, :94 — тест #654, который ТЗ обещает оставить + зелёным), `demo/smoke_iso_first_frame.mjs`, + `demo/smoke_isometric_contract.mjs`, `demo/smoke_iso_flat_parity.mjs`, + `demo/smoke_iso_theme_walls.mjs`, `test/core-file-budget.test.mjs` + (строка бюджета `src/houseplan-card.ts` на месте) — все существуют, + план автотестов не ссылается на несуществующие файлы или функции. + Демо-дом: `demo/srv/demo.html:54,62` действительно задаёт `plan_url` для + `f1` и `garden` — подложки у обоих пространств, как заявлено. +4. **Конвенция `// private-ok: <причина>`**, которую AC2 использует для + счётчиков вызовов в smoke, существует и работает так, как описано + (`scripts/no-new-private-writes.mjs`, `docs/TESTING.md:249`) — формат + `// private-ok: счётчик проходов и поиска бумаги (#739)` пройдёт гейт. +5. **База измерений** — ветка `issue/725-layout-reads-and-fingerprint` + (коммит `071c74a5`, уже не резолвится — ожидаемо: #725 слит + squash-коммитом `235f60e6` в `dev`). Это не находка (§2.10 про + нерезолвящийся SHA относится к материалу предыдущего раунда ревью, но + тот же принцип применИм: дерево и блоб важнее хеша ветки, которая была + удалена после слияния); все утверждаемые номера строк и поведение + сверены с текущим `dev`-деревом и совпадают. +6. Трек подтверждён: `track:ask`, перф-класс (§5), файлы класса A + (`src/houseplan-card.ts`, `src/iso-first-frame.ts`) — ревью ТЗ для + `ask` обязательно (§2.4), что и происходит. + +Гейты (typecheck/test/build) не прогонялись — это ожидаемо на этапе spec: +зависимости и Chromium не ставились (#696), а артефакт этапа — ТЗ, не код. +Прогон гейтов на будущей ветке — обязанность code-review. + +## Находки + +Нет. High — 0, Medium — 0, Low — 0. + +Технических вопросов, ошибочно вынесенных владельцу, не найдено: оба блока +(«Решено по умолчанию», «Принято предположительно») корректно помечены как +не-продуктовые решения реализатора/ревьюера и явно допускают пересмотр без +эскалации — ровно то, что требует §7.1. + +## Что проверено и корректно + +- Все обязательные разделы §7.1 присутствуют и содержательны, не + шаблонные заглушки. +- Каждый AC (AC1–AC5) однозначен, имеет названный способ доказательства + (`unit`/`smoke`/`gate`/performance-workflow) и описывает проверяемое + состояние или счётчик, а не «работает корректно» общими словами. +- Защитный AC5 (гейт/мутант) содержит строку «чем краснеет» — мутант + `iso-paper-resolved-per-floor` с guard-тестом `test/iso-stage6.test.mjs` + (AC1), раздел «План автотестов» дополнительно называет, что именно + краснеет на текущем коде для AC1 и AC2 до фикса. +- Контракт поведения (К1) и граничные случаи (§2.6: async, данные, хост, + режимы, объём) покрывают переходы между темами, режимами, наличием/ + отсутствием подложки и сменой режима редактор↔View. +- Оценка производительности честно помечена как «оценка, а не обещание» + там, где измерение бенчмарком невозможно (CI-профили без подложки) — + это не нарушение §7.1 («догадка как факт»), так как оговорка на месте. +- Откат, release-артефакты (`User-Visible: no`, абзац `docs/ISOMETRIC.md`), + i18n/миграции («нет») и touch (киоск — блокирующая поверхность, вход не + меняется) закрыты явно. +- Фактические ссылки на код (номера строк, имена функций, поведение + гейта `render-layout-read.mjs`) проверены чтением и совпадают дословно — + редкий случай, когда ревью не нашло ни одного расхождения между ТЗ и + деревом. + +## Чего не проверял + +- Гейты `npx tsc --noEmit`, `npm test`, `npm run build` — не прогонялись: + не требуется на этапе spec (#696), код задачи ещё не написан. +- Сами измерения из раздела «Проблема»/«Не-скоуп» (CPU-профили, числа + `switchCycleMs`, `longTasks`) не переснимались — они сделаны временным + флагом харнесса вне репозитория и не воспроизводимы на этом материале; + проверена только непротиворечивость чисел с кодом и пометка «оценка, не + обещание» там, где это уместно. +- Мутация `iso-paper-resolved-per-floor` не существует в + `scripts/mutation-registry.mjs` на момент ревью — это ожидаемо: ТЗ + описывает её как часть будущей реализации (AC5), а не как уже + существующий гейт. +- Браузерные смоки/Full Performance не запускались — на этом треке и + этапе не требуются (нет кода, нет ветки). + +## Вердикт + +Зелёный. ТЗ выполнимо, каждый AC проверяем и привязан к существующим +файлам/инструментам, продуктовых вопросов не осталось открытыми, скоуп и +не-скоуп согласованы и обоснованы профилированием. Переход в «Готово к +разработке». + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `3b9f25ea659c` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `e94ca566360e026e50ecff7594279f9db87cfbbc` + ``` + git log --all --format='%H %T' | grep e94ca566360e + ``` +- Тело issue: `b4865b454fae1e35d41752478fda842157505fded49f39f20bbaff6fa2794b37` +- Вердикт конвейера: `green` · High 0 · маршрут `fix`