diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 6231a54e..6c7b65df 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,6 +1,6 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 205, issue: 100. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 206, issue: 100. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| @@ -33,6 +33,7 @@ | #695 | [CODE-REVIEW-695-r1.md](CODE-REVIEW-695-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | «инфраструктура без метки трека» не читается как track:show нигде в коде | `PROCESS.md` `.github/workflows/_process.yml` `scripts/task-packet.mjs` `_process.yml` `task-packet.mjs` `docs/process/AUTHOR.md` `test/task-packet.test.mjs` | | #695 | [CODE-REVIEW-695-r2.md](CODE-REVIEW-695-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — | | #694 | [SPEC-REVIEW-694-r1.md](SPEC-REVIEW-694-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 0 | 1. Отсутствуют разделы «Сценарий» и «Что человек увидит до и после», обязательные по §7…; 1. UX, модель данных/миграция, i18n и touch-влияние не названы явно «нет» отдельными ст… | `docs/SCOPE.md` | +| #694 | [SPEC-REVIEW-694-r2.md](SPEC-REVIEW-694-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — | | #692 | [CODE-REVIEW-692-r1.md](CODE-REVIEW-692-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #691 | [SPEC-REVIEW-691-r1.md](SPEC-REVIEW-691-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | направление edge-swipe не квантифицировано и не отмечено как допущение | `src/houseplan-card.ts` | | #691 | [SPEC-REVIEW-691-r2.md](SPEC-REVIEW-691-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-694-r2.md b/docs/reviews/SPEC-REVIEW-694-r2.md new file mode 100644 index 00000000..a75e1f21 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-694-r2.md @@ -0,0 +1,184 @@ +# SPEC-REVIEW-694-r2 — «Исследовать и устранить performance-регрессии v1.78 относительно v1.77» + +Issue: [#694](https://github.com/Matysh/houseplan-card/issues/694) +Этап: spec (§2.4) +Трек: `ask` (производительность, метка `track:ask`) +Заход: r2 · блокирующих циклов израсходовано 1 из 4 (зелёный вердикт цикла не образует, §4/§10.4) + +## Вердикт + +**Зелёный.** High: 0. Medium в скоупе: 0. Medium вне скоупа: 0. Low: 0. + +## Скоуп разбора (по дельте, §2.10) + +Предыдущий раунд — [SPEC-REVIEW-694-r1.md](../../docs/reviews/SPEC-REVIEW-694-r1.md), +жёлтый, 3 Medium в скоупе, 1 Low снята ревьюером. Материал r1 — тело issue на +момент вынесения вердикта r1 (детач `dev`@`84ed38e3d5d511687de48e8ca4dae714f82efe2f`, +дерево `51385b4bd49434fbfe510a14c8824a1c48b01d0b`, блоб тела +`3e716989f90524ce41441b1b6ac157229af3aec237befdc309a8e08b66ad6816`). + +Дельта объявлена не как дифф файла (ТЗ живёт в теле issue, не в файле +`docs/specs/`), а как сравнение снимков тела issue через GraphQL +`userContentEdits` (снимки правок, которые REST `timeline`/`events` не отдают +для body-правок): + +``` +gh api graphql -f query='{ repository(owner:"Matysh", name:"houseplan-card") { + issue(number:694) { userContentEdits(first:20) { nodes { editedAt diff } } } } }' +``` + +Три снимка: `2026-09-28T18:37:07Z` (исходное тело при заведении issue), +`2026-09-30T20:20:10Z` (тело на момент вынесения вердикта r1, подтверждено +совпадением его hash с материалом r1 выше) и `2026-09-30T20:50:14Z` (тело после +правки по r1, `diff` этого снимка совпадает с текущим телом issue байт-в-байт +кроме завершающего перевода строки). Дельта между снимком r1 и текущим телом — +`diff /tmp/edit_1.txt /tmp/issue694_body.txt` (сохранены локально из +GraphQL-ответа), она и разобрана ниже по каждой находке. + +Дополнительно проверено, не ушёл ли `dev` вперёд настолько, чтобы затронуть +факты, проверенные в r1 чтением кода (П1–П3 — не только план, а точные +утверждения о коде на момент r1): + +``` +git log --oneline 84ed38e3..a5a73d15 +``` + +Шесть коммитов после материала r1; из них только `75745185` (#714, удаление +мёртвого поиска раскладки оверлеев #651) трогает продуктовый код — +`src/iso-overlays.ts`, `src/iso-scene-render.ts`, 16 строк в +`src/houseplan-card.ts` (сигнатуры `_isoOverlayScene`/`_overlaysForSpace`, +поле `data-hp-iso-nudged` захардкожено в `'false'`). Прочитан весь +`git show 75745185 -- src/houseplan-card.ts`: ни одна из изменённых строк не +пересекается с `_pointerHoverObserver`/`inDeviceLayer` (П1, `connectedCallback`, +район строки 2542–2567 по r1), `src/stairs-view.ts` (П2) и +`src/i18n/language-runtime.ts` (П3) этим коммитом не затронуты вовсе — commit +stat подтверждает (`git show --stat 75745185`). Остальные пять коммитов — +документация ревью и процесс-фиксы (#704, #705, #718), продуктового кода не +трогают. Разбор кода П1–П3 из r1 остаётся в силе без повторного чтения — +раздел «Унаследовано из r1» ниже. + +Проверены заново только те AC/разделы, которых касается сама дельта: +комплектность §7.1 (разделы «Сценарий», «Что человек увидит», «Риски», +«Откат») и формулировка AC4. AC1–AC3, AC5, «Не входит», «Принятые +предположения» текстуально не менялись (см. дифф выше) — не перепроверялись +повторно как факты кода, унаследованы из r1. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Medium 1: отсутствуют обязательные по §7.1 разделы «Сценарий» и «Что человек увидит до и после» | Оба раздела добавлены в `## ТЗ` целиком, текстом, близким к предложенному ревьюером r1 | Тело issue, `### Сценарий` (строки 65–67) и `### Что человек увидит до и после` (69–72); персона «домочадцы» и поверхность «настенный планшет в киоске и телефон» совпадают с `docs/SCOPE.md` (Household members / Guests), явка — переключение вкладок этажей (J1) | +| Medium 2: отсутствуют разделы «Риски» и «Откат» (оба обязательны §7.1, «Откат» — также DoR §2.5) | Добавлен `### Риски` (4 пункта: запас `core-file-budget`, возможная остаточная краснота лестниц, риск пропуска узла в П1, риск внешнего сброса `lang` в П3) и `### Откат` (revert коммита, без данных/миграций; пользовательского переключателя нет и не нужно) | Тело issue, `### Риски` (127–132) и `### Откат` (134–137) | +| Medium 3: AC4 допускает исключение без названного способа подтвердить, что редность вызвана именно лестницами | AC4 переформулирован: явно назван метод из двух измерений в одном прогоне на одном раннере — счёт/время long task по каждому переключению этажа показывает, что превышение приходится на заходы на этаж 1, и тот же профиль на фикстуре без лестниц проходит бюджет против `v1.77.0` | Тело issue, `### Критерии приёмки`, AC4 (110–115) | +| Low 1: UX/модель данных/i18n/touch не названы явно «нет» | Не правилось — ревьюер r1 явно снял находку («доработка не обязательна»), повторно не поднимается | SPEC-REVIEW-694-r1.md, раздел Low | + +Все три Medium закрыты текстом в самом теле issue (не заявлением автора в +комментарии) — дифф снимков тела выше показывает точное место правки для +каждой находки. + +## Унаследовано из r1 (без повторной проверки) + +Материал: `docs/reviews/SPEC-REVIEW-694-r1.md`, детач `dev`@`84ed38e3d5d5`. + +- Чтение кода П1–П3 построчно (`src/houseplan-card.ts:2542–2567` + `_pointerHoverObserver`/`inDeviceLayer`; `src/stairs-view.ts:63–77` + `renderLayer`; `src/i18n/language-runtime.ts:105–120` `languageRenderGate`) — + подтверждено r1, дельта этих файлов не касается (проверено выше: + `git show --stat 75745185`, прочие коммиты диапазона документацию/процесс). +- `test/core-file-budget.test.mjs:52` = 12896 для `src/houseplan-card.ts` — + бюджет не менялся; факт длины файла даже улучшился (12891 строка сейчас + против 12895 на материале r1, см. «Что проверено» ниже) — это не находка, + просто запас чуть вырос. +- Существование `demo/smoke_stairs.mjs`, `demo/smoke_device_hit_capsules.mjs`, + `demo/smoke_french_locale.mjs`, `test/device-hit-owner*.test.mjs`, имён + performance-профилей и `longTask.countP95` в `demo/performance/evaluate.mjs` — + не перепроверялось повторно, `demo/performance/**` дельтой не затронут. +- Однозначность и falsifiability AC1, AC2, AC3, AC5 — текст этих AC не менялся + (см. дифф снимков), вердикт r1 «однозначны, доказуемы» остаётся в силе. +- Вывод «трек `ask` обоснован», «открытых продуктовых вопросов в тексте ТЗ не + осталось», «"Не входит" мотивирует каждый пункт» — текст этих разделов не + менялся дельтой, кроме перечня `#725` вместо перечисления функций (см. ниже). + +## Что проверено заново и корректно + +- **Оба продуктовых раздела (§7.1) содержательны, а не формальная отписка.** + «Сценарий» называет персону словами `docs/SCOPE.md` («домочадцы» — + Household members) и поверхность (настенный планшет в киоске, телефон); + «Что человек увидит» — без терминов реализации, симметрично описывает до/после + и честно называет допустимое исключение (лестницы) тем же языком, что и + Риск 2 и AC4 — без противоречия между разделами. +- **AC4 теперь исполним и не зависит от недоказуемого утверждения.** Метод + атрибуции, который r1 требовал назвать, уже опирается на существующий код: + `demo/fixtures/large-house.mjs:211` — `makeLargeHouseFixture({ includeStairs = true } = {})` + уже параметризован флагом отключения лестниц (`floor === 0 ? { stairs: … } : {}`), + то есть «фикстура без лестниц» из AC4 — не гипотеза, а один вызов + существующей функции с `includeStairs: false`. Единственное, чего сейчас нет — + проброс этого флага как CLI/раннер-опции в `demo/benchmark_large_house.mjs` + (проверено: файл принимает `--samples`, `--warmups`, `--profile`, + `--allow-stage2-base`, но не флаг лестниц) — это техническая реализация внутри + задачи, а не пробел контракта: ТЗ явно оставляет выбор («вариант фикстуры или + флаг раннера») автору. +- **`#725` — не придуманная ссылка.** Раздел «Не входит» в правленом ТЗ заменил + перечисление прочих кандидатов профилирования на ссылку `#725`; issue + существует (`S1-new`, «Производительность: принудительные layout и пересборка + отпечатка конфигурации на каждом рендере»), и по названию соответствует + списку, который раньше был инлайн-текстом. Ссылка не расширяет и не сужает + скоуп текущей задачи — она лишь выносит источник для уже названных, не + входящих сюда кандидатов. +- **DoR (§2.5) теперь проходим по всем пунктам буквально**, включая «откат» и + «риски перечислены» — оба ранее отсутствовавших пункта закрыты, остальные + были закрыты уже в r1. +- **Дельта дева между раундами не задевает предмет этой задачи.** Единственный + продуктовый коммит диапазона (#714) работает в изометрической раскладке + оверлеев, не в `_pointerHoverObserver`, `stairs-view.ts` или + `language-runtime.ts`; построчно сверено выше. +- **Числа не разошлись.** `viewToggle` действительно отсутствует в + `demo/performance/budgets-isometric-smoke.json` и + `budgets-isometric-stage3-dense.json` — соответствует утверждению «Уже + сделано: #720» в ТЗ (метрика убрана из бюджетов решением владельца, а не + тихо ослаблена). + +## Чего не проверял + +- Не перечитывал заново П1–П3 построчно на текущем `dev`@`a5a73d15` — дельта их + не касается (см. «Скоуп разбора»), это унаследовано из r1, а не пропущено. +- Не гонял `npx tsc --noEmit` / `npm test` / `npm run build` — ветка + `issue/694-…` не заведена (проверено: `git branch -a` и `git ls-remote + --heads origin` не находят ветку), продуктового диффа нет, гонять гейты не + над чем; зелёный Validate на `a5a73d15`, упомянутый в промпте, относится к + материалу code-review, не к этому этапу. +- Не проверял технико-экономическую точность самих чисел в таблицах + комментариев (проценты регрессий, медианы по бетам) — это предмет r1 и + предыдущих циклов (#692), дельта текущего раунда их не меняет. +- Не запускал `node scripts/smoke-select.mjs` — на этапе ТЗ нет диффа `base`/ + `head` для сравнения, инструмент неприменим до кода. +- Не оценивал, будет ли реализация флага отключения лестниц в + `benchmark_large_house.mjs` тривиальной сверх того, что уже показал код + `makeLargeHouseFixture` — это вопрос кода будущей реализации, не ТЗ. + +## Материал раунда + +Issue #694, тело на момент вынесения вердикта r2 (полный текст, включая +разделы «Сценарий», «Что человек увидит до и после», «Риски», «Откат» и +переформулированный AC4, добавленные правкой от 2026-09-30T20:50:14Z по +итогам r1). Рабочая копия — detached HEAD на +`dev`@`a5a73d1511ae4c09277e16a41c076bfd5eb1fd8e`, продуктовые изменения по +задаче отсутствуют (ветка `issue/694-…` не заведена); код дерева `dev` читался +только для проверки, что дельта между материалом r1 и текущим `dev` не задевает +П1–П3 (см. «Скоуп разбора»), а не как материал стадии code-review. +sha256 тела issue (снимок, использованный при выводе вердикта r2, вычислен +локально из `gh issue view --json body`): `a740d41c81932b63b6170c409df8faee87c9c6a22121fbe303edae5dafcfe84d`. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `a5a73d1511ae` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `5a1bea4a8cb02a6fc04a3a8742cb8b57e57b7c51` + ``` + git log --all --format='%H %T' | grep 5a1bea4a8cb0 + ``` +- Тело issue: `02419f4111e3d31c9b6850dd135fcd78c7d57892ba54c752a6088ccb070a5a05` +- Вердикт конвейера: `green` · High 0