mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-07 06:59:46 +00:00
@@ -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-<m>`, отсутствие
|
||||
`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.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/754-smoke-select-blind-spots`, коммит `871a689ebf8f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `260f78f16c56d841e92d349132ecc74070abce47`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 260f78f16c56
|
||||
```
|
||||
- Тело issue: `24db9bf79ba8f97df7f5605fe148a37381b9054227b220ba4d64e1108698803f`
|
||||
- Вердикт конвейера: `green` · High 0 · маршрут `fix`
|
||||
Reference in New Issue
Block a user