From 3fe06f61dcb3a649ce6f546913b0e685b81f9a4d Mon Sep 17 00:00:00 2001 From: Shahovalov MIkhail Date: Wed, 27 May 2026 13:45:35 +0300 Subject: [PATCH] =?UTF-8?q?feat:=20assembly=5Fadd=5Fmate=20=E2=80=94=20?= =?UTF-8?q?=D1=83=D0=BF=D1=80=D0=BE=D1=87=D0=BD=D0=B5=D0=BD=D0=B8=D0=B5=20?= =?UTF-8?q?=D0=BF=D0=BE=20=D1=80=D0=B5=D0=B2=D1=8C=D1=8E=20Codex=20=D1=80?= =?UTF-8?q?=D0=B5=D0=B0=D0=BB=D0=B8=D0=B7=D0=B0=D1=86=D0=B8=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1: Valid==false → ошибка (откат), а не «успех с valid=false»; косвенно отсекает грани одного компонента (вырожденный мат). #6/#7: distance value строго > 0 (нулевой зазор = coincidence) + unit-кейс value=0 бросает. #8: негативный интеграционный тест — точка в пустоте → грань не найдена → ошибка. #2/#3/#4 оставлены по обоснованию (best-effort откол, Add??throw, первый IFace — консистентно с HoleService). #9 точечный release RCW отклонён (конвенция v2-2). Тесты: 98 unit + 58 integration = 156 зелёных. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../specs/2026-05-27-assembly-add-mate-design.md | 15 ++++++++++++++- src/Kompas.Mcp.Core/Assemblies/AssemblyService.cs | 6 +++++- .../Assemblies/AssemblyValidation.cs | 8 +++++--- tests/Kompas.Mcp.Tests/AssemblyValidationTests.cs | 3 ++- .../Integration/AssemblyMateTests.cs | 13 +++++++++++++ 5 files changed, 39 insertions(+), 6 deletions(-) diff --git a/docs/superpowers/specs/2026-05-27-assembly-add-mate-design.md b/docs/superpowers/specs/2026-05-27-assembly-add-mate-design.md index b0b8ea3..1646c7a 100644 --- a/docs/superpowers/specs/2026-05-27-assembly-add-mate-design.md +++ b/docs/superpowers/specs/2026-05-27-assembly-add-mate-design.md @@ -1,7 +1,20 @@ # Дизайн: сопряжение компонентов сборки (assembly_add_mate) через API7 **Дата:** 2026-05-27 -**Статус:** спайк зелёный; ревью Codex учтено (8 замечаний); реализуется +**Статус:** реализовано и проверено (2 интеграционных + 14 unit, всего 154); ревью Codex спека (8) и реализации (9) учтено + +## Правки по ревью реализации Codex (9 замечаний) + +- **#1 `Valid==false` → ошибка** (а не «успех с valid=false»): после Update+Rebuild проверяем + `mate.Valid`, иначе бросаем (срабатывает откат). `AddedMate.Valid` теперь всегда true при успехе. + Это же косвенно ловит грани одного компонента (#5) — мат выродится. +- **#6/#7 distance `value` > 0** (не ≥0): нулевой зазор вырождается в coincidence → + `RequireMateValue` требует строго `> 0`; unit-кейс `value=0` бросает. +- **#8 негативный интеграционный тест**: точка в пустоте → грань не найдена → ошибка. +- **#2 откат best-effort** оставлен намеренно (исходное исключение важнее ошибки отката; паттерн + `HoleService`). **#3** `mates.Add ?? throw` до try корректно (null = ничего не добавлено). + **#4** «первый IFace» оставлен (консистентно с HoleService/move_face). **#9** точечный release + RCW отклонён (конвенция v2-2). ## Решения по ревью Codex (важные правки к дизайну ниже) diff --git a/src/Kompas.Mcp.Core/Assemblies/AssemblyService.cs b/src/Kompas.Mcp.Core/Assemblies/AssemblyService.cs index 197855b..278e9cb 100644 --- a/src/Kompas.Mcp.Core/Assemblies/AssemblyService.cs +++ b/src/Kompas.Mcp.Core/Assemblies/AssemblyService.cs @@ -123,8 +123,12 @@ public sealed class AssemblyService "Update сопряжения вернул FALSE — связь не наложена (несовместимые грани/значение?)."); if (!top.RebuildModel(true)) throw new InvalidOperationException("RebuildModel вернул FALSE — сборка не перестроена после сопряжения."); + if (!mate.Valid) + throw new InvalidOperationException( + "Сопряжение построено, но вырождено (Valid=false) — вероятно, грани несовместимы " + + "или принадлежат одному компоненту. Проверьте выбор граней."); - return new AddedMate { Type = type, Valid = mate.Valid }; + return new AddedMate { Type = type, Valid = true }; } catch { diff --git a/src/Kompas.Mcp.Core/Assemblies/AssemblyValidation.cs b/src/Kompas.Mcp.Core/Assemblies/AssemblyValidation.cs index 68c44c7..c6bfd8c 100644 --- a/src/Kompas.Mcp.Core/Assemblies/AssemblyValidation.cs +++ b/src/Kompas.Mcp.Core/Assemblies/AssemblyValidation.cs @@ -38,13 +38,15 @@ public static class AssemblyValidation /// /// Проверить числовое значение сопряжения. Если значение требуется (, - /// напр. расстояние) — оно должно быть конечным и неотрицательным. + /// напр. расстояние) — оно должно быть конечным и строго положительным (зазор 0 вырождается в + /// совпадение — для него есть отдельный тип). /// public static void RequireMateValue(bool requiresValue, double value) { if (!requiresValue) return; // для типов без параметра значение игнорируется - if (!double.IsFinite(value) || value < 0) + if (!double.IsFinite(value) || value <= 0) throw new ArgumentOutOfRangeException(nameof(value), - "Значение сопряжения (расстояние) должно быть конечным и неотрицательным."); + "Значение сопряжения (расстояние) должно быть конечным и > 0 " + + "(для нулевого зазора используйте тип coincidence)."); } } diff --git a/tests/Kompas.Mcp.Tests/AssemblyValidationTests.cs b/tests/Kompas.Mcp.Tests/AssemblyValidationTests.cs index b646961..2e09238 100644 --- a/tests/Kompas.Mcp.Tests/AssemblyValidationTests.cs +++ b/tests/Kompas.Mcp.Tests/AssemblyValidationTests.cs @@ -77,10 +77,11 @@ public sealed class AssemblyValidationTests [InlineData(double.NaN)] [InlineData(double.PositiveInfinity)] [InlineData(-1)] + [InlineData(0)] // нулевой зазор вырождается в coincidence → ошибка public void RequireMateValue_throws_on_bad_required_value(double value) => Assert.Throws(() => AssemblyValidation.RequireMateValue(true, value)); [Fact] - public void RequireMateValue_accepts_nonnegative_required_value() + public void RequireMateValue_accepts_positive_required_value() => Assert.Null(Record.Exception(() => AssemblyValidation.RequireMateValue(true, 30))); } diff --git a/tests/Kompas.Mcp.Tests/Integration/AssemblyMateTests.cs b/tests/Kompas.Mcp.Tests/Integration/AssemblyMateTests.cs index d80a89f..174b57f 100644 --- a/tests/Kompas.Mcp.Tests/Integration/AssemblyMateTests.cs +++ b/tests/Kompas.Mcp.Tests/Integration/AssemblyMateTests.cs @@ -95,4 +95,17 @@ public sealed class AssemblyMateTests : IntegrationTestBase } finally { await _docs.CloseAsync(save: false); } } + + [Fact] + public async Task Mate_with_point_off_any_face_throws() + { + await SetupTwoCubesAsync(); + try + { + // Вторая точка — в пустоте (далеко от обоих кубов): грань не найдена → понятная ошибка. + await Assert.ThrowsAsync( + () => _assembly.AddMateAsync(MateType.Coincidence, 20, 10, 10, 1000, 1000, 1000, value: 0)); + } + finally { await _docs.CloseAsync(save: false); } + } }