docs: review document for #273

Issue: #273
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-23 17:10:22 +00:00
parent e2de0dbcc4
commit 3e8733ad27
+77
View File
@@ -0,0 +1,77 @@
# SPEC-REVIEW-273-r2
- Issue: [#273](https://github.com/Matysh/houseplan-card/issues/273) — Optimize сохраняет sub-grid островок толщины у T-узла: 22→15→22
- Этап: ТЗ на ревью (`S4-spec-review`), заход **r2**, блокирующих циклов израсходовано 1 из 4
- Артефакт ТЗ: `docs/specs/273-optimize-topology-island.md`
- Проверено на SHA: `e2de0dbcc4f09b0a3c7085db4fbf0961f7e3d970` (текущий HEAD ветки `issue/273-optimize-topology-island`)
- Предыдущий раунд: r1, вердикт жёлтый, документ `docs/reviews/SPEC-REVIEW-273-r1.md`, проверен на SHA `b5fbe633a60f8a2091552204fc7ff9e3da67fb06`
- Лёгкий/короткий трек: нет (`small`/`trivial` не назначены; лейблы — `bug`, `P1`, `S4-spec-review`) — ТЗ обязано жить файлом, что соблюдено
- Вердикт: **зелёный**
## Скоуп раунда
`git diff b5fbe633..e2de0dbc -- .` показывает, что между SHA r1 и текущим HEAD изменился **только** `docs/specs/273-optimize-topology-island.md` (плюс коммит r1-ревью `docs/reviews/SPEC-REVIEW-273-r1.md`, который является артефактом самого процесса, не предметом проверки). Никакой продуктовый код, `AGENTS.md`, `PROCESS.md`, `docs/SCOPE.md`, канонические документы подсистемы (`docs/WALL-THICKNESS.md` и др.) не менялись. Тело issue #273 после хендоффа автора на r1 не редактировалось; новых сообщений владельца между r1 и r2, кроме хендоффа «ТЗ обновлено по ревью r1», нет.
Раунд признан локальным (не полным разбором) по критерию из инструкции: дельта не задевает ребейз, не меняет контракт поведения, не касается новой подсистемы, объём дельты (34 строки, один файл) несопоставим с объёмом исходной задачи. Разбор в этом раунде — по дельте: закрытие двух находок r1 плюс проверка, что сама правка не создала новых противоречий в документе.
## Как проверялось
1. Восстановлен вердикт и SHA r1 из `docs/reviews/SPEC-REVIEW-273-r1.md` и из истории комментариев issue (`gh issue view 273 --json comments`) — SHA r1 в тексте вердикта был назван прямо (`b5fbe633a60f8a2091552204fc7ff9e3da67fb06`), в отличие от типового случая «SHA не назван», здесь автор его указал в хендоффе.
2. Дельта объявлена и получена: `git diff b5fbe63..e2de0db -- docs/specs/273-optimize-topology-island.md` (34 строки) и `git diff b5fbe63..e2de0db --stat` (подтверждение, что больше ничего не менялось).
3. По каждой находке r1 (Medium + Low) построчно сверено, чем именно она закрыта в дельте — таблица ниже.
4. Проверена техническая точность нового текста §14 п.4 («optimizer helper получает только room profile и `open_spans`, а не `space.partitions`/drafts») — это то же самое техническое утверждение, которое я уже проверял в r1 чтением `src/plan-optimizer.ts:99-200` (сборка `nodes` только из `roomPoly()` и `openCuts`) и `src/wall-thickness.ts:2311-2313` (комментарий «Independent partitions/columns are deliberately not accepted here»). Поскольку сам код не менялся между r1 и r2 (см. п.1 «Скоуп раунда»), это наследуется без повторного чтения кода — только сверено, что новая формулировка в ТЗ не противоречит уже проверенному факту.
5. Проверено, что новая формулировка §6.2/§6.3 не противоречит остальному документу: единственные упоминания «partition/draft» в файле теперь — только в §14 п.4 (assumption), других следов удалённой категории в §5 «Не входит», §12 «Риски», AC3/AC4 не осталось (grep по файлу).
6. Проверено, что все AC (AC1–AC8) теперь содержат явную строку «Доказательство:» с названием механизма (unit-файл/smoke/mutation-script) — это закрывает Low-находку r1.
7. Продуктовых вопросов владельцу в дельте нет — правка чисто техническая (сужение заявленного контракта + документирование admission), новых развилок поведения не появилось.
Гейты кода в этом раунде не гоняются: продуктовый код не менялся, изменился только spec-документ — на этапе ТЗ это ожидаемо, не упущение.
## Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
| **Medium** — §6.2/§6.3 обещали блокировку при совпадении synthetic endpoint с independent partition/draft axis boundary, но ни AC, ни код это не покрывают | Автор выбрал вариант 1 из трёх предложенных: убрал пункт из нормативного контракта и перенёс его в §14 явным техническим допущением | `docs/specs/273-optimize-topology-island.md:137-138` — из §6.2 убрано «endpoint другого physical axis», заменено на «вторым room-topology endpoint»; `:151-158` — из §6.3 полностью убрана строка «второй endpoint совпадает с отдельной room/partition/draft axis boundary»; `:337-342` — новый §14 п.4 прямо называет допущение: helper видит только room profile и `open_spans`, совпадение с partition/draft не классифицируется, меняет только `cm` central span на уже доказанную соседями толщину, identity-aware защита не заявляется этой задачей |
| **Low** — AC3/AC4/AC5/AC7 не называли явно способ доказательства по шаблону §2.5 | Строка «**Доказательство:** …» добавлена во все ACs, у которых её не было (в т.ч. и AC2/AC6/AC8, которые r1 уже засчитал как достаточные по тексту прозы) | `:209-210` (AC2), `:218-219` (AC3), `:231-232` (AC4), `:240-241` (AC5), `:254` (AC6), `:261-262` (AC7), `:270` (AC8) |
Обе находки закрыты вариантом, предложенным самим ревью r1 (пункт 1 списка «правка на выбор автора»), а не спором и не эскалацией — по правилам раунда это устраняет цикл без обращения к владельцу.
## Проверка точности закрытия (не просто «слова совпали»)
- Новая формулировка §6.2 («второй endpoint не является room vertex, endpoint opening/open span, или вторым room-topology endpoint») технически корректно описывает то же самое: `isTopologyNode()` — это объединение room-vertex и open-cut checks, так что «room-topology endpoint» как третий пункт покрывает первые два; редундантность есть, но она не создаёт логической ошибки и не ослабляет guard — второй endpoint всё ещё обязан быть вне полного множества topology nodes. Не нахожу здесь дефекта, достойного находки.
- §14 п.4 не выдаёт технический факт за решённое поведение — он явно помечен как «Принятое техническое предположение», что и требовалось по итогам r1: раздел прямо называет, что коллизия «не классифицируется» и «не заявляется этой задачей», не обещая скрытой защиты.
- AC4 попутно уточнён («offset/perpendicular/parallel coincidence **с room profiles/open cuts**») — это устраняет тень неоднозначности: без уточнения читатель мог бы думать, что этот тест покрывает и partition-коллизию. Уточнение корректно сужает формулировку синхронно с §6.3.
- Переупорядочение пунктов §14 (бывший п.4 про #271 стал п.5, бывший п.5 про отсутствие продуктовых вопросов стал п.6) — механическое смещение, семантика не изменена.
Новых находок delta не создала: правка строго сужает заявленный контракт до того, что реализация фактически может доказать, и документирует ранее недекларированное ограничение как предположение, а не как факт.
## Унаследовано из r1
Всё содержание документа, не затронутое дельтой, принимается без повторной проверки со ссылкой на `docs/reviews/SPEC-REVIEW-273-r1.md`, полученный на SHA `b5fbe633a60f8a2091552204fc7ff9e3da67fb06`:
- соответствие job **J6** из `docs/SCOPE.md` и обоснование сценария (§1 ТЗ);
- полнота обязательных разделов §7.1 PROCESS.md (сценарий, до/после, причина, скоуп/не-скоуп, контракт §6.1 «Базовые условия #198», UX/accessibility/touch/security/perf, данные и совместимость, i18n, риски, откат, release-артефакты);
- построчное соответствие «Обязательные регресс-тесты» из тела issue → AC1–AC9 (таблица в r1);
- корректность порога `0.5 × GRID_PITCH` и арифметики из приведённого в issue экспорта;
- корректность терминологии «opening/open-span» = `openCuts`/`open_spans`, не спутана с дверными/оконными маркерами (проверено r1 по `test/plan-optimizer.test.mjs:236-266`);
- отсутствие продуктовых вопросов владельцу (единственная развилка была решена контрактом §6.1–6.3 уже в r1; правка r2 её не трогает, кроме сужения одного пункта);
- реалистичность release-артефактов и rollback (§11, §13) — не описывают несуществующую миграцию;
- проверка «одно число — один источник» (PROCESS.md): новых пользовательских чисел не вводится, `wallsMerged` переиспользуется как есть.
§6.1 «Базовые условия #198» и §6.4 «Результат» не входят в дельту r2 и не переоценивались повторно.
## Что проверено и корректно (дополнительно к наследию)
- `git diff --stat` подтверждает: между r1 и r2 не менялись `AGENTS.md`, `PROCESS.md`, `docs/SCOPE.md`, `docs/WALL-THICKNESS.md` и любой продуктовый код — предпосылка «делать разбор по дельте» выполнена, а не предположена.
- SHA r1 был явно назван автором в хендоффе — не пришлось восстанавливать его из истории коммитов как отдельную находку.
- Дельта не расширяет and не меняет AC-контракт по существу (не добавляет и не убирает тестируемое поведение из позитивного сценария AC1–AC2), поэтому повторная проверка соответствия AC1–AC9 регресс-требованиям issue не нужна: только AC3/AC4 упомянуты в дельте, и оба сужены в сторону уже покрытого негативного случая, а не расширены на новый.
## Чего не проверял
- Не перечитывал заново код `src/plan-optimizer.ts`/`src/wall-thickness.ts` — он не менялся с r1, техническая точность §14 п.4 наследуется из уже проведённой в r1 проверки (см. «Как проверялось», п.4).
- Не запускал `npm test`/`typecheck`/`build`/`check-docs.mjs` — продуктовый код не менялся, гонять их не над чем; это ожидаемо на этапе ТЗ, не упущение.
- Не проверял golden/smoke/performance/mutation-gate — они относятся к этапу код-ревью, здесь код ещё не написан.
- Не проверял частотность partition-коллизии на реальных планах (тот же пробел, что в r1) — теперь это явное предположение §14 п.4, а не непроверенное заявление, поэтому вопрос закрыт как «принято осознанно», а не как открытый риск.
## Итог
Обе находки r1 (Medium и Low) закрыты корректно и по существу, а не только текстуально. Новых High/Medium делта не создала. Продуктовых вопросов владельцу нет.