Files
houseplan-card/docs/reviews/SPEC-REVIEW-273-r2.md
T
2026-08-23 17:10:22 +00:00

15 KiB
Raw Blame History

SPEC-REVIEW-273-r2

  • Issue: #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 делта не создала. Продуктовых вопросов владельцу нет.