From 9e64fc80eeaf38a6cb79638c0029d79c50303805 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 12:22:21 +0000 Subject: [PATCH] docs: review document for #754 Issue: #754 User-Visible: no --- docs/reviews/CODE-REVIEW-754-r1.md | 159 +++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-754-r1.md diff --git a/docs/reviews/CODE-REVIEW-754-r1.md b/docs/reviews/CODE-REVIEW-754-r1.md new file mode 100644 index 00000000..c862a338 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-754-r1.md @@ -0,0 +1,159 @@ +# CODE-REVIEW-754-r1 + +**Issue:** #754 «Выбор смоков и проверки фикстур: слепые места» +**Материал раунда:** `871a689ebf8faed2c45e328af60860bb22d2f31b` (ветка `issue/754-smoke-select-blind-spots`, поверх `origin/dev` `7ce3f662`, один коммит) +**Трек:** show, подтверждён владельцем в комментарии «Оценка» до начала работы +**Заход:** r1 · блокирующих циклов израсходовано 0 из 2 + +## Скоуп + +Три независимых слепых места харнесса (класс B, без продуктового кода): + +1. `scripts/smoke-select.mjs` — правка аргументов многострочного вызова (напр. + `runtime.resolveIsoOverlayFitEnvelope({` строкой выше изменённого аргумента) + раньше не давала символа при `--unified=0` и тонула в «неопределённости». + Теперь дифф берётся с контекстом `CALL_CONTEXT_LINES = 3`, `parseDiff` + ищет ближайшую незакрытую `(` выше по ханку (`enclosingCallee`), прямые + символы по-прежнему берутся только с изменённых строк. Новое поле + `callees` в JSON, пометка «(вызов)» в текстовом отчёте. + `scripts/task-packet.mjs` берёт для `selectSmokes` отдельный дифф с тем + же контекстом; `change-risk` остаётся на `--unified=0`. +2. `test/iso-overlay-fixture-types.test.mjs` + `test/iso-scene-render.test.mjs` + — проверка мёртвых полей фикстур (#732) расширена на вход + `resolveIsoOverlayFitEnvelope`: все 4 вызова в тестах идут через новый + хелпер `overlayFit` с типом `OverlayFitFixture`, который проверяется так + же, как `SCENE_BUILDERS`. +3. `demo/smoke_backdrop.mjs` — хелпер `mode()` больше не зовёт приватный + `_setMode` и не спит фиксированные 220 мс: он входит в режим через + `window.__hpTest.setMode` и ждёт тем же условием конца перехода, что + секция 6b (#715), вынесенным в общий `window.__backdropEnterMode`. + +`docs/TESTING.md` получил абзац про атрибуцию по вызову. `User-Visible: no`, +продукт (`src/**`) не тронут. + +## Как проверялось + +Зелёный Validate на этом SHA (https://github.com/Matysh/houseplan-card/actions/runs/36859266285) +подтверждает дешёвые гейты. По диффу и AC проверено дополнительно: + +| Гейт | Статус | Результат | +|---|---|---| +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | прогнан | `src/**/*.ts` не тронут → browser-smoke диффом не выбираются; это ожидаемо (диапазон правки — scripts/test/demo/docs) | +| `tsc -p tsconfig.test.json && fix-test-build.mjs && node --test test/smoke-select.test.mjs` | прогнан | 12/12 зелёных, включая 3 новых теста AC1 | +| то же для `test/iso-overlay-fixture-types.test.mjs` + `test/iso-scene-render.test.mjs` | прогнан | 34/34 зелёных | +| AC2 «умеет падать»: временно вернул `stageSize` в `IsoOverlayFitEnvelopeInput` (src/iso-scene-render.ts), пересобрал test-build | прогнан | тест `#732 AC2: тип фикстуры…` красный с `OverlayFitFixture отвергает stageSize…`; остальные 2 теста файла зелёные. Файл восстановлен (`cp` исходной копии), `git diff` по нему пуст | +| AC1 «умеет падать» без защиты: тест `#754 AC1: без строк контекста…` гоняет тот же `741-call-arguments.diff` с вырезанными строками контекста | прогнан (это один из новых unit-тестов, не отдельный прогон) | `callees=[]`, `unproven=true`, визуальный минимум — т.е. воспроизводит дотестовое поведение | +| `node demo/smoke_backdrop.mjs` (AC3, назван в тексте AC) | прогнан | `OK`. Потребовалась собственная сборка (`npm run build` + `node scripts/bundle-sync.mjs`): закоммиченный `dist/` в обычных коммитах законно отстаёт от `src/` (Release-трейлер — отдельная дисциплина, #657), поэтому фингерпринт committed-бандла не совпадал с текущим деревом. После прогона `dist/` и `demo/srv/assets` возвращены в исходное состояние (`git checkout -- dist/ && git clean -fd dist/`; `demo/srv/assets` не отслеживается git) | +| `HP_SMOKE_CHECKS=1 node demo/smoke_backdrop.mjs` | прогнан | 59 имён проверок — совпадает с заявленным в AC3 и в комментарии автора | +| `node scripts/no-new-private-writes.mjs --count` для `demo/smoke_backdrop.mjs` на HEAD и на `origin/dev` (подменой файла и восстановлением) | прогнан | HEAD: `0`, `origin/dev`: `1` — совпадает с заявленным | +| `node scripts/mutation-gate.mjs --check` | прогнан | «предупреждений mutation registry: 4» — совпадает с заявленным «как на dev» | +| Байт-сравнение `test/fixtures/smoke-select/741-call-arguments.diff` с `git show d5bdfde9196bc7ecf7a4cfefea04f4dd69559b0d` | прогнан | идентичны | + +### Чего не проверял + +- Полный `npx tsc --noEmit` / `npm test` / `npm run build` с трёхкопийной + сверкой бандла — не перегонял отдельно, положился на зелёный Validate на + этом же SHA (бюджет §4 не тратится на переповтор дешёвых гейтов). +- Историческую выборку «80 последних коммитов dev, +15 смоков, не больше 4 + на коммит» из раздела «Итог исследования» — не воспроизводил: это + исследовательская иллюстрация в ТЗ, а не формальный AC, и прогон несоразмерен + объёму задачи (track show, сложность 3/10). +- Точное повторение авторского прогона `smoke_backdrop` с «кадрами, замедленными + до 60 и 150 мс» — в репозитории нет закоммичённого инструмента для такого + throttling (ТЗ прямо говорит: «прототипы — во временных файлах, в ветку не + идут»), а писать собственный зонд для show-задачи с сложностью 3/10 было бы + несоразмерно. Вместо этого прочтением убедился, что перенесённая логика + (`window.__backdropEnterMode`) побайтово повторяет прежний `enter()` секции + 6b, чья устойчивость к медленным кадрам уже установлена #715, и подтвердил + функциональность прогоном на обычной скорости плюс сверкой числа и имён + проверок (HP_SMOKE_CHECKS) и метрики `no-new-private-writes`. +- `golden:verify`, `pytest tests_backend`, `npm run invariants`, + performance-профили — не применимо: нет метки `ci:golden`, нет правок + Python, нет изменений геометрии/рендера, AC не называет + performance-профиль. +- Поведение на 80 проверяемых исторических коммитах для скана «callees» — + см. выше, не требуется AC. + +## Находки + +Находок, блокирующих или требующих правки, не обнаружено. + +Разобрано отдельно (не находка, см. ниже «Что проверено и корректно»): +алгоритм `enclosingCallee` не «проваливается насквозь» через вложенный +объект-значение (`stageSize: { width, height }` — объект, присвоенный +свойству, а не прямой аргумент вызова) глубже одного уровня: такая строка +сама по себе осталась бы неопределённостью. Это не расходится ни с одним AC: +негативные случаи ТЗ перечисляют ровно «строка после `;`» и «строка в теле +блока», а не вложенные литералы-значения; в самом диффе-свидетеле #741 +символ всё равно попадает в `callees`, потому что другая изменённая строka +того же вызова резолвится напрямую (аргумент `stageSize: null` стоит сразу +после `(`/`,` на строке с открывающей скобкой вызова). AC1 требует +`callees === ['resolveIsoOverlayFitEnvelope']` — выполнено, тестом и живым +прогоном. Это задокументированный компромисс эвристики (риск в ТЗ: «ложная +атрибуция… даёт лишний смок, но не пропуск»), а не дефект. + +## Что проверено и корректно + +- **AC1.** `CALL_CONTEXT_LINES = 3`, `gitDiff` берёт дифф с этим контекстом + (`--unified=3`), `parseDiff`/`closeHunk`/`enclosingCallee` корректно + восстанавливают обе стороны ханка (old/new) из интерлива контекстных и + изменённых строк, ищут ближайшую незакрытую `(` по алгоритму скобочных + глубин, не трогая прямые символы на строках контекста. Прошёл пошаговую + трассировку вручную на диффе-свидетеле #741 (оба добавленных символа) и на + всех 5 встроенных негативных кейсах нового unit-теста (аргумент-литерал, + вложенный массив, строка после `;`, тело функции-колбэка, присваивание + объекта, прямой символ на изменённой строке) — поведение соответствует + описанному в комментариях коду и ожиданиям теста. CLI-тест подтверждает + `callees` и пометку «(вызов)» в текстовом отчёте. `task-packet.mjs` берёт + для выборки смоков отдельный дифф с тем же контекстом, `change-risk` + остаётся на `--unified=0` — ровно то, что требует ТЗ; отдельного unit-теста + на это у `task-packet.test.mjs` нет (мокается `collectInputs`), но это + механическая однострочная замена существующей, уже протестированной + `selectSmokes`, риск низкий. +- **AC2.** Хелпер `overlayFit`/`OverlayFitFixture` заведён и использован во + всех 4 местах вызова `resolveIsoOverlayFitEnvelope` в тестах; проверка #732 + судит их так же, как построители сцены (`CHECKED_CALLS`). Негативный + прогон (временный возврат `stageSize` в боевой тип) подтвердил, что защита + действительно краснеет — тест умеет падать, не только «не падает сейчас». +- **AC3.** `mode()` и `enter()` секции 6b делят один page-level хелпер + `window.__backdropEnterMode`, логика (класс `mode-`, отсутствие + `mode-transition`, отсутствие бегущих анимаций, дедлайн 5 с с именованной + ошибкой) идентична прежней реализации — перенос, не переписывание. + `_setMode` и фиксированная пауза убраны. Оракулы и оба набора проверок не + изменились: 59 имён совпадают, `no-new-private-writes` подтверждает 0 + вызовов фасада вместо 1 на `dev`. +- Трейлеры `Issue: #754` / `User-Visible: no` на единственном коммите — в + порядке; changelog не трогается, что и требуется при `no`. +- `docs/TESTING.md` обновлён абзацем ровно про добавленное поведение + (атрибуция по вызову, пометка «(вызов)»), расхождений с кодом нет. +- Диапазон правки не затрагивает ни одного файла `src/**`: подтверждено и + чтением диффа, и выводом `smoke-select` («Исполняемого frontend-диффа + нет»). + +## Маршрут + +Все шесть критериев §5 пройдены: сложность и риск ≤3 (сам харнесс, без +продукта), одна поверхность (харнесс выбора смоков и один демо-смок, не +несколько продуктовых диалогов), нет миграции конфига, нет нового +UX-контракта (`User-Visible: no`), нет влияния на производительность или +touch, ожидаемое поведение зафиксировано в обновлённом `docs/TESTING.md` и в +самом ТЗ. Трек show подтверждён решением владельца ещё на этапе оценки — +понижать/переквалифицировать нет оснований. + +## Вердикт + +Зелёный. High: 0, Medium: 0. + +--- + + + +## Материал раунда + +- Ветка: `issue/754-smoke-select-blind-spots`, коммит `871a689ebf8f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `260f78f16c56d841e92d349132ecc74070abce47` + ``` + git log --all --format='%H %T' | grep 260f78f16c56 + ``` +- Тело issue: `24db9bf79ba8f97df7f5605fe148a37381b9054227b220ba4d64e1108698803f` +- Вердикт конвейера: `green` · High 0 · маршрут `fix`