feat: assembly_add_mate — упрочнение по ревью Codex реализации
#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) <noreply@anthropic.com>
This commit is contained in:
@@ -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 (важные правки к дизайну ниже)
|
||||
|
||||
|
||||
@@ -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
|
||||
{
|
||||
|
||||
@@ -38,13 +38,15 @@ public static class AssemblyValidation
|
||||
|
||||
/// <summary>
|
||||
/// Проверить числовое значение сопряжения. Если значение требуется (<paramref name="requiresValue"/>,
|
||||
/// напр. расстояние) — оно должно быть конечным и неотрицательным.
|
||||
/// напр. расстояние) — оно должно быть конечным и строго положительным (зазор 0 вырождается в
|
||||
/// совпадение — для него есть отдельный тип).
|
||||
/// </summary>
|
||||
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).");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<ArgumentOutOfRangeException>(() => 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)));
|
||||
}
|
||||
|
||||
@@ -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<InvalidOperationException>(
|
||||
() => _assembly.AddMateAsync(MateType.Coincidence, 20, 10, 10, 1000, 1000, 1000, value: 0));
|
||||
}
|
||||
finally { await _docs.CloseAsync(save: false); }
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user