You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Dime Ultrareview cycle-1 on PR #241 (fix/device-validation-level-hardening), tracked finding T3 — surfaced by the simplification agent.
Finding
PR #241 collapses the two duplicated `Enum.IsDefined` setter throw blocks on `DeviceValidationLevel` and `InputValidationLevel` into a shared `ThrowIfUndefined` helper (finding L4), but the throw message text still hard-codes the valid-arms list twice:
`"DeviceValidationLevel must be one of Ignore (0), Warning (1), Remove (2), Strict (3)."`
`"InputValidationLevel must be one of Ignore (0), Warning (1), Remove (2), Strict (3)."`
The list `Ignore (0), Warning (1), Remove (2), Strict (3)` is a shared vocabulary (both enums share ordinals 0..3 by contract — that's what makes the load-time mirror in `AgentConfiguration.Normalize` safe). Extracting it to a shared string constant would keep the two setters (and any future third axis) DRY.
Rationale for defer
The existing exception-shape pins (see `DeviceValidationLevelMigrationTests.DeviceValidationLevel_Setter_Exception_Carries_ParamName_And_ActualValue` and its `InputValidationLevel` sibling) assert `Message` contains the enum name — extracting the valid-arms list into a constant is safe as long as the enum name prefix stays. The tests would need review to confirm the assertion shapes survive.
Origin
Dime Ultrareview cycle-1 on PR #241 (
fix/device-validation-level-hardening), tracked finding T3 — surfaced by the simplification agent.Finding
PR #241 collapses the two duplicated `Enum.IsDefined` setter throw blocks on `DeviceValidationLevel` and `InputValidationLevel` into a shared `ThrowIfUndefined` helper (finding L4), but the throw message text still hard-codes the valid-arms list twice:
The list `Ignore (0), Warning (1), Remove (2), Strict (3)` is a shared vocabulary (both enums share ordinals 0..3 by contract — that's what makes the load-time mirror in `AgentConfiguration.Normalize` safe). Extracting it to a shared string constant would keep the two setters (and any future third axis) DRY.
Rationale for defer
Suggested design (for the follow-up PR)
Add a shared constant near `AgentConfiguration.DeviceValidationLevelDefault`:
```csharp
private const string ValidationLevelValidArmsSuffix =
" must be one of Ignore (0), Warning (1), Remove (2), Strict (3).";
```
Then the setter calls become:
```csharp
ThrowIfUndefined(value, nameof(DeviceValidationLevel) + ValidationLevelValidArmsSuffix);
ThrowIfUndefined(value, nameof(InputValidationLevel) + ValidationLevelValidArmsSuffix);
```
Refs: dime cycle-1 T3 (simplification) — see PR #241's L4 refactor.