diff --git a/docs/reviews/SPEC-REVIEW-290-r2.md b/docs/reviews/SPEC-REVIEW-290-r2.md new file mode 100644 index 00000000..7619b210 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-290-r2.md @@ -0,0 +1,182 @@ +# SPEC-REVIEW-290-r2 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/290 +- **Этап:** spec (PROCESS.md §2.4) +- **Заход:** r2 · блокирующих циклов израсходовано 1 из 4 (не увеличивается — + зелёный вердикт бюджет §4 не тратит, #227) +- **Ревьюируемый артефакт:** `docs/specs/290-near-axis-authoring-and-repair.md`, + ветка `issue/290-near-axis-authoring-repair`, коммит `dc9f9a6c` +- **Предыдущий раунд:** `docs/reviews/SPEC-REVIEW-290-r1.md`, вердикт жёлтый, + получен на коммите `dcdb7565` +- **Вердикт:** зелёный + +## Скоуп ревью (PROCESS.md §2.10) + +Второй раунд — разбор по дельте, а не заново. SHA предыдущего вердикта названо +в самом документе r1 (`dcdb7565`), так что находка «SHA не назван» не +применяется. + +Дельта: `git diff dcdb7565..dc9f9a6c` (два коммита: `089d1663` — сам документ +r1, `dc9f9a6c` — правки автора): + +``` +docs/reviews/SPEC-REVIEW-290-r1.md | 265 ++++++++++++++ +docs/specs/290-near-axis-authoring-and-repair.md | 46 ++- +``` + +Только один файл — предмет ревью, `docs/specs/290-near-axis-authoring-and-repair.md` +(класс C). Продуктовый код не тронут (ожидаемо: этап `spec`). Изменения в самой +спеке: + +1. AC4: ссылка на fixture заменена с «minimized fixture из #284» на + `test/fixtures/279-near-orthogonal-junction.json`; +2. новый раздел **AC10 «Реальные планы и видимое снижение»** (старый AC10 + «Локальные гейты» стал AC11, в который добавлена строка `npm run invariants`); +3. новые разделы **10. «Риски и меры»** и **11. «Откат»**; +4. чисто механический сдвиг номеров разделов 10→12, 11→13, 12→14 без изменения + их содержимого (сверено `git diff` — эти три блока идентичны кроме заголовка). + +Это ровно три находки r1 (M1, M2, L1) и ничего сверх них — делта локальна, +подсистема не новая, контракт поведения не меняется, ребейза на `dev` не было. +Полный повторный разбор продуктовой рамки, вопросов владельцу и AC1–AC9/AC6 +(инвариант, который дельта не задевает) не требуется; тем не менее ниже я +перечитал файл целиком, чтобы проверить, что вставки не разошлись по +нумерации/перекрёстным ссылкам с остальным текстом (единственный дёшевый +способ подтвердить, что дельта действительно локальна, а не выглядит такой). + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **M1.** Нет обязательных разделов §7.1 «Риски» и «Откат» | Добавлены раздел «10. Риски и меры» (4 пункта: ложная ось при auto-straightening, потеря owner/metadata при lossy Optimize, каскадный уступ, расхождение classifier с renderer #279 — каждый со ссылкой на AC-меру) и раздел «11. Откат» (чистый revert коммита, schema не меняется, подтверждённая геометрия не требует downgrade-миграции) | `docs/specs/290-near-axis-authoring-and-repair.md:282-300` | +| **M2.** AC6 issue («`npm run invariants` на обоих реальных планах, видимое снижение near-orthogonal count») не перенесён; `scripts/model-invariants.mjs` не назван явно | Новый **AC10** требует `npm run invariants` на `real-plan-first-floor.json` и `real-plan-second-floor.json` до/после Confirm, строгое уменьшение near-orthogonal node count на плане с repair candidates, точную строку отчёта Optimize и повторный прогон с нулём исправлений; AC11 («Локальные гейты», старый AC10) теперь явно перечисляет `npm run invariants` | `docs/specs/290-near-axis-authoring-and-repair.md:241-253` (AC10), `:260` (AC11, строка `npm run invariants`) | +| **L1.** AC4 приписывает fixture issue #284, хотя она — уже закоммиченная `test/fixtures/279-near-orthogonal-junction.json` | Ссылка исправлена на `test/fixtures/279-near-orthogonal-junction.json` | `docs/specs/290-near-axis-authoring-and-repair.md:192-193` | + +Все три закрыты правкой того же файла, без нового issue — в точности то, что +требует жёлтый вердикт в скоупе задачи (PROCESS.md §2.4, #202). + +## Унаследовано из r1 + +Без повторной проверки принято то, что дельта не задевает — весь продуктовый +разбор из `docs/reviews/SPEC-REVIEW-290-r1.md` (коммит `dcdb7565`): + +- сценарий и продуктовая рамка по J6 `docs/SCOPE.md`; +- закрытие всех трёх продуктовых вопросов владельцу дословно (раздел 2 ТЗ); +- единый допуск `0.25°` и его источник (раздел 4), включая обоснование, что + `NEAR_AXIS_MAX_DEGREES` — это то же числовое значение, что и существующий + `MULTI_WALL_NEAR_ORTHOGONAL_MAX_DEGREES`, а не тот же экспортируемый символ + (принято как «assumed, change freely», не продуктовое решение); +- негативный контракт для диагоналей (AC6 ТЗ, не изменён этой дельтой) и + соответствующие мутанты AC9 (не изменены); +- совместимость с `docs/RESIZE.md` (раздел 5.2, не изменён) и + `docs/WALL-THICKNESS.md` (раздел 6.1, не изменён); +- touch/производительность (раздел 9, не изменён) и соответствие + `docs/TOUCH-SUPPORT.md`; +- отсутствие догадок, выданных за факт, среди технических утверждений о уже + существующем поведении; +- i18n на уровне ТЗ (файлы названы, ключи не требуются на этом этапе — сверено + с #277/#279) и место/имя файла спеки. + +Раздел «Чего не проверял» r1 (гейты не запускались, реальная достижимость +performance-бюджетов, реализуемость мутантов, реестр `config-field-registry.mjs`) +остаётся в силе без изменений: дельта этого раунда не касается ни одного из +этих пунктов. + +## Как проверялось (этот раунд) + +1. Получен текст вердикта r1 и SHA `dcdb7565` из `docs/reviews/SPEC-REVIEW-290-r1.md` + и из комментария `claude` от 2026-08-24T10:48:47Z в issue #290. +2. `git diff dcdb7565..dc9f9a6c -- docs/specs/290-near-axis-authoring-and-repair.md` + — построчно, для каждой находки r1 найдена конкретная закрывающая строка (см. + таблицу выше), а не заявление автора («Правки r1 внесены... AC10 теперь + требует...», комментарий Matysh от 2026-08-24T10:57:10Z) принято за исходную + гипотезу, но не за доказательство. +3. Прочитан файл `docs/specs/290-near-axis-authoring-and-repair.md` целиком + (350 строк) — проверено, что перенумерация разделов 10→12/11→13/12→14 + механическая (содержимое идентично) и что все перекрёстные ссылки на AC + (`grep -n "AC[0-9]"`) внутри файла остались согласованными (AC1, AC2, AC5, + AC6, AC7, AC10 в разделе «Риски» — все существуют и соответствуют описанию). +4. Проверена фактическая обоснованность нового AC10, а не только его текст: + `grep` по `test/fixtures/real-plan-second-floor.json` подтвердил, что узел + ребра из тела issue #290 (`(-1.670833333, 2.95)` / `(-0.354166667, 2.954166667)`) + буквально присутствует в этой фикстуре (строки 29-108) — значит «план с + repair candidates» в AC10 не голословен, второй реальный план действительно + несёт описанный в issue дефект, и AC10 не окажется невыполнимым/vacuous. + Также подтверждено, что оба файла (`real-plan-first-floor.json`, + `real-plan-second-floor.json`) уже отслеживаются в репозитории (коммиты + `d5659478`, `523190d8`) и уже используются `test/model-invariants.test.mjs` + — то есть AC10 ссылается на существующую, а не гипотетическую инфраструктуру. + Проверено также, что `npm run invariants` в `package.json` реально запускает + `scripts/model-invariants.mjs` — команда, названная в AC11, разрешима. +5. Проверено, что новый раздел AC10 не противоречит принятому владельцем ответу + на Q2 (issue, комментарий 2026-08-24T10:32:01Z): строка отчёта «Выпрямлено + стен: N; максимальное перемещение: X» воспроизведена в AC10 дословно. +6. Гейты (`typecheck`/`test`/`build`/`check-docs`) не запускались: диапазон + этого раунда — документация класса C, продуктовый код (`src/**`) не + изменён, гейты к этапу `spec` не относятся (то же решение, что в r1). + +## Находки + +Ни одной. Все три находки r1 закрыты предметно (таблица выше); дельта не +вносит новых High/Medium/Low: перенумерация разделов механическая, новый AC10 +проверен на предмет обоснованности данных (не голословен), формулировка «Риски» +и «Откат» соразмерна риску задачи (auto-straightening без modifier bypass) и +по формату соответствует соседним ТЗ той же геометрической линии (`#277 §12.1`, +`#279 §5`, на которые сам документ r1 ссылался как на образец). + +## Что проверено и признано корректным + +- Раздел «10. Риски и меры» называет содержательные риски (ложная ось у + границы допуска, потеря owner/metadata при lossy Optimize, каскадный уступ, + расхождение classifier/renderer), и у каждого — конкретная AC-мера, а не + общая фраза; это именно то, чего не хватало по M1. +- Раздел «11. Откат» корректно ограничен тем, что применимо к задаче без + Labs-флага и без изменения схемы: чистый revert коммита, старые данные + читаемы без миграции. Согласуется с `AGENTS.md` («Labs может менять только + презентацию»): эта задача меняет контракт поведения authoring/Optimize, а не + презентационный эксперимент, поэтому Labs-флаг как механизм откат здесь и не + должен упоминаться. +- AC10 корректно разводит два разных доказательства: внешний аудит + (`npm run invariants`, до/после, строгое уменьшение near-orthogonal count) и + видимую пользователю строку Optimize (принятый текст из ответа владельца). + Это не то же самое, что показывать сырое число near-orthogonal-узлов в UI + Optimize — и не должно быть: владелец в Q2 согласовал именно строку + «Выпрямлено стен: N; максимальное перемещение: X», а не отображение + аудиторской метрики. +- AC4 теперь называет реально существующий tracked-файл, а не issue, которая + эту fixture не производила — L1 закрыт полностью, не просто переформулирован. +- AC11 «Локальные гейты» — правки не сломали список, `npm run invariants` + добавлен в верное место (список локальных гейтов, а не отдельно). + +## Чего не проверял + +- **Не проверял AC1–AC9, AC6 (совместимость), раздел 5–7 («Authoring + contract», «Explicit Optimize repair», «Scope»)** — дельта их не касается, + наследуются из r1 без повторной проверки (раздел выше). +- **Не проверял first-floor фикстуру на отсутствие ложных срабатываний** — + AC10 требует строгого уменьшения только «на плане с repair candidates» + (second floor); AC10 не формулирует явно, что count на «чистом» first floor + должен остаться нулевым/неизменным после Confirm. Это не новая находка (Low + не заводится): AC10 всё равно требует нулевой exit-код `invariants` на обоих + планах и до, и после, что уже страхует от регрессии на чистом плане; явного + утверждения «count на first floor не растёт» в тексте просто нет. Оставляю + как наблюдение, не как Low — граница между «этого достаточно» и «стоило бы + явнее» здесь не продуктовая, а редакторская, и не стоит правки отдельным + циклом. +- **Не запускал гейты** (`typecheck`/`test`/`build`/`check-docs`, + `npm run invariants`) — как и в r1, диапазон изменений это класс C + (документация), продуктового кода и тестовых фикстур в этом диффе нет, гейты + относятся к этапу `code`, не к `spec`. +- **Не проверял docs/CONFIG-COMPATIBILITY.md реестр** — унаследовано из r1 + без изменений (schema не менялась и в этом раунде тоже). + +## Вывод + +Оба Medium из r1 закрыты предметно, каждое — конкретной строкой текста, а не +заявлением автора; Low закрыт полностью. Дельта локальна (один файл +документации, 46 строк), не задевает продуктовую рамку, вопросы владельцу, +контракт поведения или AC, не подтронутые правкой, поэтому они наследуются без +повторной проверки. Новых находок дельта не вносит — в частности, обоснованность +нового AC10 проверена по факту (реальная fixture действительно несёт описанный +в issue дефект), а не принята на слово. Вердикт: зелёный, ТЗ готово к +«Готово к разработке».