diff --git a/docs/reviews/SPEC-REVIEW-316-r2.md b/docs/reviews/SPEC-REVIEW-316-r2.md new file mode 100644 index 00000000..9221cff5 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-316-r2.md @@ -0,0 +1,237 @@ +# SPEC-REVIEW-316-r2 + +Issue: #316 · ТЗ: `docs/specs/316-opening-host-auto-resolution.md` · +заход: r2 · блокирующих циклов израсходовано 1 из 4 (входящее значение, до +этого вердикта) · SHA материала ревью r2: `6ca2ce8b` +(`docs: spec #316 revision 2 — registry/compatibility/goldens sections, +reachable AC3 fixture`). Материал r1: SHA `36f735ae`, документ +`docs/reviews/SPEC-REVIEW-316-r1.md`. + +## Скоуп раунда + +Разбор — по дельте (PROCESS.md §2.9): `git diff 36f735ae..HEAD -- +docs/specs/316-opening-host-auto-resolution.md`. Дельта локальна — это +прямой ответ автора на M1/M2/L1 ревью r1 (новые §5 «Затронутые поверхности и +артефакты», §6 «Принято предположительно», перенумерация §5→§7 «Риски» и +§6→§8 «Откат», замена термина и фикстуры AC3), нормативные правила §3 и AC +не переписаны. Оснований для полного разбора (ребейз, смена контракта, +новая подсистема, объём дельты сопоставим с исходной задачей) нет — дельта +это ровно правка по находкам r1, и по объёму (67 добавленных / 9 удалённых +строк из ТЗ на 109 строк) меньше исходной задачи. + +Комментарии issue #316 проверены полностью (`gh issue view 316 --json +comments`): после S4-решения владельца и вердикта r1 новых +продуктовых директив нет — комментарий автора «Ревизия 2 ТЗ (6ca2ce8b)» +описывает исключительно техническую правку по находкам r1, открытых +продуктовых вопросов не появилось. + +## Как проверялось + +1. Найден вердикт r1 в комментариях issue (2026-08-26T15:09:36Z) и SHA + материала (`36f735ae`, назван в документе r1 — не пришлось его + реконструировать). +2. `git diff 36f735ae..HEAD -- docs/specs/316-opening-host-auto-resolution.md` + — построчная сверка дельты с текстом находок M1/M2/L1. +3. По каждой находке r1 — проверка не заявления автора, а конкретной строки + документа, которая её закрывает (таблица ниже), и, где утверждение + опирается на код или другой документ, — сверка с реальным состоянием + репозитория на HEAD: + - `src/wall-segment-model.ts:588–615` (`resolveRoomOpeningHost`, + `eligible`) и `src/wall-thickness.ts:1718–1771` + (`atomicPolyForRoom`) — проверка, что новая фикстура AC3 (стык двух + контурных атомов одной стены на границе смены толщины) действительно + достижима: `wallBreaks`/`space.walls` с разными `cm` вносят точку + разрыва в `atomicPolyForRoom` (строки 1743–1746), значит два + атома одной стены с общей вершиной на границе толщины — реальный + код-путь; `demo/smoke_wall_thickness_transition.mjs` подтверждает, что + такой разрыв уже используется в тестах (для другого сценария, но тем + же механизмом). Короткий проём в центре стыка попадает в допуск `t` + обоих атомов (`t >= -EPS && t <= 1+EPS` при `half` в пределах + `GRID_STEP_N*0.02`) — `eligible` истинен для обоих. Фикстура реальна. + - `scripts/config-field-registry.mjs:291–303` (запись + `spaces[].openings[].host=wall`) — текст `default`/`migration`, + процитированный в §5.1 ТЗ, совпадает с файлом дословно. + - `docs/CONFIG-COMPATIBILITY.md` — прочитан целиком (`§Stable wall + identity`, `§Canonical zero-thickness walls — model v9 (#306)`, + `§Independent-wall opening host (#132)`) для проверки, на какой именно + абзац ссылается новый пункт §5.1 ТЗ — см. находку M3 ниже. + - `demo/golden/harness.mjs` (`coincidentPartition` фикстура, + строки ~85–112) и `demo/golden/matrix.mjs:169–186` + (`coincident-partition-virtual-dark`) и + `demo/golden/baselines/baselines-index.json:13` — проверка claim'а + §5.3 «существующие сцены... в них нет комбинации span+проём» — + см. находку M4 ниже. + - `docs/specs/316-opening-host-auto-resolution.md` §5.2 (i18n) — + сверено с r1 «Как проверялось» п.7 (ключи `toast.zero_wall_migration_blocked`, + `wall_model.reason.opening-host` в `src/i18n/{ru,en}.json` не тронуты) — + наследуется, дельта i18n-ключей не касается. +4. AC1, AC2, AC4–AC6 дельтой не задеты (текст не менялся) — не + перепроверялись повторно, унаследованы из r1 (см. раздел ниже). AC3 + изменился текстуально — перепроверен полностью (см. п.3 выше). +5. `grep -n "инертн" docs/specs/316-opening-host-auto-resolution.md` — пусто, + термин заменён везде, коллизии с `docs/UX-MODES.md`/`smoke_inert_openings.mjs` + больше нет. + +Гейты `typecheck`/`test`/`build`/`check-docs` не прогонялись: диапазон +изменений — `docs/specs/316-*.md` (плюс сам документ ревью r1, тоже +документация), продуктового кода нет, класс C. Это не пропуск гейта — как +и в r1, гейтов для класса C на стадии ТЗ не предусмотрено. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где видно | +|---|---|---| +| M1 / i18n не назван | Явный раздел «Без изменений», с именами обоих ключей и обоснованием (fail-closed путь §2 остаётся) | §5.2, строки 123–127 | +| M1 / release-артефакты не названы | Раздел с CHANGELOG RU+EN, требованием новой golden-сцены и обязательством `golden:verify`, явный «без правок» для USER-GUIDE | §5.3, строки 129–137 | +| M1 / затронутые файлы/модули не перечислены | Список из 7 файлов/поверхностей с указанием, что именно меняется в каждом | §5.1, строки 100–121 | +| M1 / perf и touch не названы | Явные абзацы «нет» с обоснованием (одна доп. проходка O(atoms×openings) вне рендера; миграция — не жест) | §5.4, строки 139–144 | +| M1 / `docs/CONFIG-COMPATIBILITY.md` не упомянут | Пункт с планом правки реестра и документа | §5.1, строки 109–118 — **но см. новую находку M3**: указан не тот абзац `docs/CONFIG-COMPATIBILITY.md` | +| M1 / нет раздела «принято предположительно» | Новый раздел с 3 пунктами (tie-break, семантика деградации 3.3, имя состояния) | §6, строки 146–153 | +| M2 / AC3 недостижимая фикстура | Фикстура заменена на стык двух контурных атомов одной стены на границе толщины — проверено реальным код-путём (см. «Как проверялось» п.3) | AC3, строки 84–88 | +| L1 / коллизия термина «инертный» | Термин заменён на «непривязанный (unhosted)» везде, включая §3.3 и §7 «Риски» | grep по документу — 0 совпадений | + +Все находки r1 закрыты по существу, не декларативно. Дополнительно к +закрытию M1 автор сам заметил и исправил нумерацию разделов (5→7 «Риски», +6→8 «Откат») — корректно, ссылок на старые номера в остальном тексте нет. + +## Унаследовано из r1 + +Без повторной проверки приняты выводы `docs/reviews/SPEC-REVIEW-316-r1.md` +(SHA `36f735ae`) по частям документа, которые дельта r2 не затронула: + +- §1 «Сценарий» и §2 «Границы» — не изменились; цепочка причин и сверка с + #306 §8.2 приняты как проверенные в r1. +- Правило 3.1 (проём удерживает стену) — текст не менялся; заимствование + критерия `eligible()` и обоснование physического смысла приняты из r1. +- Правило 3.3 нормативная часть (кроме переименования термина) — рендер по + x/y для безхостовых проёмов, подтверждённый в r1 чтением `space-render.ts:285` + и `plan-geometry-preflight.ts:224/276`, принят без повторного чтения кода. +- AC1, AC2, AC4, AC5, AC6 — текст не менялся, приняты как «сформулированы + однозначно, со способом доказательства» из вывода r1. +- Откат (текст §8, бывший §6) — не менялся, принят как «конкретен и + проверяем» из r1. + +## Находки + +Обе находки этого раунда — новые: они возникли из текста, добавленного в +r2 при закрытии M1, а не из чего-то, что r1 пропустил. + +### M3 (Medium, в скоупе) — §5.1 указывает не тот абзац `docs/CONFIG-COMPATIBILITY.md` + +ТЗ (строки 115–118): «`docs/CONFIG-COMPATIBILITY.md` — новый подраздел о +непривязанном контурном проёме... прежний абзац «missing/invalid host … +fail dark» уточняется ссылкой». + +Проверено чтением `docs/CONFIG-COMPATIBILITY.md` целиком: фраза +«A missing/invalid host is not re-associated automatically: current +renderers fail dark» (строки 201–202) находится в разделе +**«Independent-wall opening host (#132)»** (строки 188–222) — этот раздел +целиком про `host.kind === 'partition'` (независимая кладка/партиции), +код-путь, который #316 прямо не затрагивает: `resolveRoomOpeningHost` +(строка 591) и `hostRoomOpenings` (строка 620) явно пропускают +`opening.host?.kind === 'partition'` — это тот же факт, которым сам автор +обосновал замену фикстуры AC3 (M2 r1). + +Абзац, который #316 действительно делает неточным, — в разделе +**«Canonical zero-thickness walls — model v9 (#306)»** (строки 81–100), +конкретно строка 94: «...verifies that no opening would acquire a zero +host, and then removes both legacy fields in one transaction. A conflict +rejects the complete candidate» — это прямое описание текущего +fail-closed поведения, которое §3.1–3.4 заменяют на детерминированное +авто-разрешение. Именно этот абзац перестаёт быть верным после реализации +#316, и именно его, а не абзац про партиции, нужно уточнить в том же +коммите, что и код (правило 11 PROCESS.md). + +**Чем грозит, если не поправить:** автор реализации, следуя ТЗ буквально, +отредактирует раздел #132 (независимые стены — не тот код-путь) и оставит +нетронутым реально устаревающий абзац раздела #306/v9 — формальное +соответствие «документация в том же коммите» будет выполнено, а по факту +контракт-документ останется противоречить новому поведению. + +**Почему Medium, а не High:** ошибка ссылки, а не решения — нормативные +правила §3 не затронуты, откат/AC не сломаны, правится одной строкой (замена +цитаты и номера раздела) без пересмотра §3. + +### M4 (Medium, в скоупе) — §5.3 неточно утверждает отсутствие span+проём в существующих golden-сценах + +ТЗ (строки 132–135): «существующие сцены не должны измениться (в них нет +комбинации span+проём) — verify обязан подтвердить». + +Проверено чтением `demo/golden/harness.mjs` (ветка `coincidentPartition`, +`state === 'virtual'`) и `demo/golden/matrix.mjs:186` +(`coincident-partition-virtual-dark`): фикстура явно строит `open_spans` +поверх позиции, где стоит `space.openings[0]` (с удалённым `host`, +`x:0.504166667, y:0.5`, `open_spans[0].a/b` — тот же `x`). Сцена +присутствует в принятой матрице (`baselines-index.json:13`, +`demo/golden/baselines/coincident-partition-virtual-dark.png`) — то есть +комбинация «open_span + проём в той же позиции» в текущих golden-сценах +**есть**, вопреки формулировке ТЗ. + +Не берусь утверждать, что рендер этой сцены изменится: `coincidentPartition` +тестирует независимую кладку (партиции), а не контурные атомы комнаты — +не исключено, что правило 3.1 (единственное, что меняет видимую +геометрию) на эту сцену не влияет ровно по той же причине, что и в M2/M3 +(независимая кладка вне `resolveRoomOpeningHost`/`buildAtoms` +контурных атомов). Находка не в этом: **ТЗ формулирует как факт то, что не +проверено и, как минимум по одному конкретному контрпримеру, неточно** — +это ровно то, что процесс требует помечать как предположение, а не +утверждение (см. вводный промпт: «утверждение о поведении, которого нет ни +в одном документе... и не помечено как предположение — замечание»). + +**Практическое смягчение:** тот же абзац уже требует `golden:verify` на +стадии кода, так что расхождение будет поймано автоматически независимо от +точности claim'а — риска тихой регрессии нет. Находка — про точность ТЗ +как источника для код-ревью следующего этапа (код-ревью #316 будет читать +именно эту фразу, чтобы решить, какие сцены смотреть предметно, а не +только доверять `golden:verify`). + +**Почему Medium, а не Low:** ТЗ прямо ссылается на конкретный (неверный) +факт, а не на общую оценку риска; чинится заменой формулировки на честную +(«есть минимум одна существующая сцена с похожей комбинацией — +`coincident-partition-virtual-dark`; ожидание — без изменений, т.к. это +независимая кладка вне контурных атомов §3.1; `golden:verify` обязателен и +для неё»), без пересмотра §3. + +## Что проверено и корректно + +- Все восемь пунктов M1 (i18n, release-артефакты, затронутые файлы, + perf/touch, ссылка на `docs/CONFIG-COMPATIBILITY.md` по факту наличия, + раздел «принято предположительно») закрыты по существу — см. таблицу + «Закрытие раунда r1». +- Новая фикстура AC3 — реально достижима внутри `resolveRoomOpeningHost`, + подтверждено чтением `atomicPolyForRoom` и существующего смока + `smoke_wall_thickness_transition.mjs`, использующего тот же механизм + разрыва атома на границе толщины (M2 закрыта корректно). +- Термин «инертный» полностью заменён на «непривязанный (unhosted)» — новых + коллизий с `docs/UX-MODES.md` не создано (L1 закрыта). +- Текст `scripts/config-field-registry.mjs`, процитированный в §5.1, + дословно совпадает с файлом на HEAD. +- Раздел «Принято предположительно» (§6) корректно отделяет + непродуктовые технические решения автора от нормативных правил §3 — не + создаёт новых открытых продуктовых вопросов владельцу. +- Перенумерация разделов (5→7, 6→8) внутренне согласована, висячих ссылок + на старые номера не осталось. + +## Чего не проверял + +- Не перепроверял AC1, AC2, AC4–AC6 и правила 3.1/3.3 (нормативная часть) + по коду заново — текст не менялся дельтой r2, наследуется из r1 (см. + «Унаследовано из r1»). +- Не прогонял гейты typecheck/test/build/check-docs/смоки/golden — класс C, + продуктового кода ещё нет, как и в r1. +- Не проверял #319 — вне скоупа, как и в r1. +- Не проверял осуществимость `atomicPolyForRoom`-механизма для реализации + правил 3.1–3.3 в целом (только для новой фикстуры AC3, точечно) — общая + реализуемость остаётся задачей код-ревью, не ТЗ-ревью. +- Не проверял остальные golden-сцены на предмет других возможных + span+проём комбинаций сверх найденной `coincident-partition-virtual-dark` + — одного контрпримера достаточно, чтобы находка M4 была обоснована; + исчерпывающий список сцен для реальной проверки — задача `golden:verify` + на стадии кода, не ТЗ-ревью. + +## Вывод + +High-находок нет. M3 и M4 — в скоупе задачи, новые в этом раунде (не +пропуски r1), чинятся точечной правкой формулировок в §5.1/§5.3 без +пересмотра нормативных правил §3 и без изменения AC. Вердикт — жёлтый, +документ возвращается автору на ревизию 3.