From 86f6f5b52d4501422811b8e1357b65787b2ce650 Mon Sep 17 00:00:00 2001 From: Matt Jones <47545907+SoundMatt@users.noreply.github.com> Date: Tue, 5 May 2026 10:29:13 -0700 Subject: [PATCH 1/2] model: use `is None` instead of falsy check in check_default_min_max `check_default_min_max` returned early when `not self.default` was truthy. Python treats 0, False, "", and [] as falsy, so a numeric signal with `default=0` and a positive `min` (or default=0 and a negative `max`) silently bypassed the range check. The validation intended to catch invalid defaults was a no-op for those values. Switch to an explicit `self.default is None` check. The downstream loop already handles list/scalar shapes correctly; with this fix, default=0 is run through the comparison and will trigger ValidationError when out of range. Adds three regression cases to tests/test_model.py covering default=0 with various min/max bounds (the primary bug case, plus a sanity case for default=0 within bounds). Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com> --- src/vss_tools/model.py | 7 ++++++- tests/test_model.py | 6 ++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/vss_tools/model.py b/src/vss_tools/model.py index 30a3665c..79c23809 100644 --- a/src/vss_tools/model.py +++ b/src/vss_tools/model.py @@ -260,7 +260,12 @@ def check_min_max_valid_datatype(self) -> Self: return self def check_default_min_max(self) -> Self: - if not self.default: + # Use an explicit None check rather than a falsy check: `not self.default` + # treats default=0 as missing, so a numeric signal with default=0 and + # min>0 (or default=0 and max<0) silently passed validation. Same shape + # for default=False / default=[] / default="", though those mostly bite + # for the numeric case. + if self.default is None: return self values = [self.default] if isinstance(self.default, list): diff --git a/tests/test_model.py b/tests/test_model.py index 8655aa9e..d11a5c2b 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -31,6 +31,12 @@ ({"datatype": "uint8", "max": 100, "default": 90}, True), ({"datatype": "uint8", "min": 10, "default": 5}, False), ({"datatype": "uint8", "min": 10, "default": 10}, True), + # Regression: `default=0` (a falsy but valid numeric default) used to + # bypass min/max validation entirely because `check_default_min_max` + # tested `if not self.default:` instead of `if self.default is None:`. + ({"datatype": "uint8", "min": 1, "default": 0}, False), + ({"datatype": "int8", "max": -1, "default": 0}, False), + ({"datatype": "uint8", "min": 0, "default": 0}, True), ({"datatype": "uint8", "default": 300}, False), ({"datatype": "uint8", "default": 200}, True), ({"datatype": "boolean", "default": True}, True), From dea2b3fceb224233f5e7401a59167f7de9d05636 Mon Sep 17 00:00:00 2001 From: Matt Jones <47545907+SoundMatt@users.noreply.github.com> Date: Fri, 22 May 2026 09:41:46 -0700 Subject: [PATCH 2/2] model: remove verbose comments per review feedback Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com> --- src/vss_tools/model.py | 5 ----- tests/test_model.py | 3 --- 2 files changed, 8 deletions(-) diff --git a/src/vss_tools/model.py b/src/vss_tools/model.py index 79c23809..c69fa9b6 100644 --- a/src/vss_tools/model.py +++ b/src/vss_tools/model.py @@ -260,11 +260,6 @@ def check_min_max_valid_datatype(self) -> Self: return self def check_default_min_max(self) -> Self: - # Use an explicit None check rather than a falsy check: `not self.default` - # treats default=0 as missing, so a numeric signal with default=0 and - # min>0 (or default=0 and max<0) silently passed validation. Same shape - # for default=False / default=[] / default="", though those mostly bite - # for the numeric case. if self.default is None: return self values = [self.default] diff --git a/tests/test_model.py b/tests/test_model.py index d11a5c2b..ddd79b37 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -31,9 +31,6 @@ ({"datatype": "uint8", "max": 100, "default": 90}, True), ({"datatype": "uint8", "min": 10, "default": 5}, False), ({"datatype": "uint8", "min": 10, "default": 10}, True), - # Regression: `default=0` (a falsy but valid numeric default) used to - # bypass min/max validation entirely because `check_default_min_max` - # tested `if not self.default:` instead of `if self.default is None:`. ({"datatype": "uint8", "min": 1, "default": 0}, False), ({"datatype": "int8", "max": -1, "default": 0}, False), ({"datatype": "uint8", "min": 0, "default": 0}, True),