Skip to content

Commit 9f2f833

Browse files
ale-rinaldiclaude
andauthored
fix(session): accept value matching any band on multi-RANGE DATA (#16)
validateAgainstRange used to read only Data.Range (the single-band field kept for back-compat) and refuse anything outside it. For DATA declarations with several <RANGE> children — piecewise-valid bands with different step sizes, e.g. VLineaPrimario_1 declaring [50..499 step 1] / [500..4990 step 10] / [5000..49900 step 100] / [50000..500000 step 1000] — the validator effectively saw only the last band and rejected legitimate values from any other. The catalog parser already populates Data.Ranges with every <RANGE> child (kept the back-compat Data.Range = first band on the side). Switch the validator to consult Data.Ranges when populated, fall back to the single Range otherwise, and accept the value if it sits inside *any* band — both bounds and step alignment checked per-band. The error path now distinguishes single-band (preserves the original "out of catalog range [Min, Max]" wording for callers/operators used to it) from multi-band (lists every band so the operator can see what's actually allowed). Tests cover: single-band accept/reject/step-violation; multi-band accept inside each band, reject below/above, reject when value is inside one band's bounds but violates its step; non-numeric TIPOs skipped; absent ranges no-op. Closes #2 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c15cef3 commit 9f2f833

2 files changed

Lines changed: 152 additions & 8 deletions

File tree

pkg/session/range_internal_test.go

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package session
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/gridsociety/mythy/pkg/catalog"
8+
)
9+
10+
func TestValidateAgainstRangeSingleBand(t *testing.T) {
11+
d := &catalog.Data{
12+
Name: "test",
13+
Tipo: "ULONG",
14+
Range: &catalog.DataRange{Min: 100, Max: 200, Step: 5},
15+
}
16+
cases := []struct {
17+
val int64
18+
wantErr string // empty = expect nil
19+
}{
20+
{100, ""}, // lower bound
21+
{200, ""}, // upper bound
22+
{155, ""}, // (155 - 100) % 5 == 0
23+
{99, "out of catalog range [100, 200]"}, // below
24+
{201, "out of catalog range [100, 200]"}, // above
25+
{102, "violates step=5"}, // wrong step
26+
}
27+
for _, c := range cases {
28+
err := validateAgainstRange(c.val, d)
29+
switch {
30+
case c.wantErr == "" && err != nil:
31+
t.Errorf("value=%d: unexpected error: %v", c.val, err)
32+
case c.wantErr != "" && err == nil:
33+
t.Errorf("value=%d: expected error containing %q, got nil", c.val, c.wantErr)
34+
case c.wantErr != "" && err != nil && !strings.Contains(err.Error(), c.wantErr):
35+
t.Errorf("value=%d: error %q does not contain %q", c.val, err.Error(), c.wantErr)
36+
}
37+
}
38+
}
39+
40+
func TestValidateAgainstRangeMultiBand(t *testing.T) {
41+
// Regression for #2: a DATA with multiple <RANGE> children defines
42+
// piecewise-valid bands with different step sizes. The validator
43+
// must accept a value matching any band, not just the last one.
44+
// Bands mirror NV10P-EA0-u's VLineaPrimario_1 in miniature.
45+
d := &catalog.Data{
46+
Name: "VLineaPrimario_1",
47+
Tipo: "ULONG",
48+
Range: &catalog.DataRange{Min: 50000, Max: 500000, Step: 1000}, // last band, kept for back-compat
49+
Ranges: []*catalog.DataRange{
50+
{Min: 50, Max: 499, Step: 1},
51+
{Min: 500, Max: 4990, Step: 10},
52+
{Min: 5000, Max: 49900, Step: 100},
53+
{Min: 50000, Max: 500000, Step: 1000},
54+
},
55+
}
56+
57+
t.Run("accepted in each band", func(t *testing.T) {
58+
ok := []int64{50, 400, 499, 500, 4990, 5000, 49900, 50000, 500000}
59+
for _, v := range ok {
60+
if err := validateAgainstRange(v, d); err != nil {
61+
t.Errorf("value=%d should be accepted (#2 reproducer for band-1 cases), got %v", v, err)
62+
}
63+
}
64+
})
65+
66+
t.Run("rejected below all bands", func(t *testing.T) {
67+
err := validateAgainstRange(int64(49), d)
68+
if err == nil {
69+
t.Fatal("expected rejection")
70+
}
71+
if !strings.Contains(err.Error(), "not in any of catalog bands") {
72+
t.Errorf("error %q should mention multi-band", err.Error())
73+
}
74+
// Error message must enumerate every band so the operator sees
75+
// what's actually allowed.
76+
for _, want := range []string{"[50,499 step 1]", "[500,4990 step 10]",
77+
"[5000,49900 step 100]", "[50000,500000 step 1000]"} {
78+
if !strings.Contains(err.Error(), want) {
79+
t.Errorf("error %q missing band %q", err.Error(), want)
80+
}
81+
}
82+
})
83+
84+
t.Run("rejected above all bands", func(t *testing.T) {
85+
if err := validateAgainstRange(int64(500001), d); err == nil {
86+
t.Error("expected rejection for value above last band")
87+
}
88+
})
89+
90+
t.Run("rejected in a gap (none of the bands match step)", func(t *testing.T) {
91+
// 401 is in band 1's [50,499] but band 1 has step=1 so 401 IS valid.
92+
// Try 502: in band 2's [500,4990] but step=10 → (502-500)%10=2 → rejected.
93+
// 502 also doesn't fit band 1 [50,499]. So it should be rejected.
94+
if err := validateAgainstRange(int64(502), d); err == nil {
95+
t.Error("502 is in band-2 bounds but violates its step=10; expected rejection")
96+
}
97+
})
98+
}
99+
100+
func TestValidateAgainstRangeNonNumericSkipped(t *testing.T) {
101+
// STRING and ENUM TIPOs are validated elsewhere; validateAgainstRange
102+
// must bail out before trying to read Range numerics.
103+
for _, tipo := range []string{"STRING", "ENUM", "ENUM_BYTE", "ENUM_LONG"} {
104+
d := &catalog.Data{
105+
Name: "x",
106+
Tipo: tipo,
107+
Range: &catalog.DataRange{Min: 1, Max: 2, Step: 1},
108+
}
109+
if err := validateAgainstRange("anything", d); err != nil {
110+
t.Errorf("TIPO=%s: expected skip, got %v", tipo, err)
111+
}
112+
}
113+
}
114+
115+
func TestValidateAgainstRangeNoRangesNoOp(t *testing.T) {
116+
// DATA without any <RANGE> child must not reject anything; the
117+
// type-width fallback in encodeForWrite is the only guardrail.
118+
d := &catalog.Data{Name: "x", Tipo: "ULONG"}
119+
if err := validateAgainstRange(int64(123456789), d); err != nil {
120+
t.Errorf("no Range/Ranges: expected nil, got %v", err)
121+
}
122+
}

pkg/session/set.go

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -268,25 +268,47 @@ func asAnyInt(value any) (int, error) {
268268
// before the value is encoded. Numeric ranges only — STRING / ENUM
269269
// types validate elsewhere. Audit I6.
270270
func validateAgainstRange(value any, d *catalog.Data) error {
271-
r := d.Range
272-
if r == nil {
273-
return nil
274-
}
275271
// Skip non-numeric TIPOs; the RANGE on STRING DATA is for character
276272
// count, not a comma-triple, so DataRange is nil there anyway.
277273
switch d.Tipo {
278274
case "STRING", "ENUM", "ENUM_BYTE", "ENUM_LONG":
279275
return nil
280276
}
277+
// Multi-band DATA (e.g. VLineaPrimario_1): the catalog declares
278+
// several <RANGE> children describing disjoint bands with
279+
// different step sizes. Accept the value if it matches any band.
280+
ranges := d.Ranges
281+
if len(ranges) == 0 && d.Range != nil {
282+
ranges = []*catalog.DataRange{d.Range}
283+
}
284+
if len(ranges) == 0 {
285+
return nil
286+
}
281287
n, err := asAnyInt(value)
282288
if err != nil {
283289
return fmt.Errorf("set %s: %w", d.Name, err)
284290
}
285-
if int64(n) < r.Min || int64(n) > r.Max {
286-
return fmt.Errorf("set %s: %d out of catalog range [%d, %d]", d.Name, n, r.Min, r.Max)
291+
v := int64(n)
292+
for _, r := range ranges {
293+
if v < r.Min || v > r.Max {
294+
continue
295+
}
296+
if r.Step > 1 && (v-r.Min)%r.Step != 0 {
297+
continue
298+
}
299+
return nil
287300
}
288-
if r.Step > 1 && (int64(n)-r.Min)%r.Step != 0 {
301+
// Build a helpful error message listing all bands.
302+
if len(ranges) == 1 {
303+
r := ranges[0]
304+
if v < r.Min || v > r.Max {
305+
return fmt.Errorf("set %s: %d out of catalog range [%d, %d]", d.Name, n, r.Min, r.Max)
306+
}
289307
return fmt.Errorf("set %s: %d violates step=%d (offsets from %d allowed)", d.Name, n, r.Step, r.Min)
290308
}
291-
return nil
309+
parts := make([]string, 0, len(ranges))
310+
for _, r := range ranges {
311+
parts = append(parts, fmt.Sprintf("[%d,%d step %d]", r.Min, r.Max, r.Step))
312+
}
313+
return fmt.Errorf("set %s: %d not in any of catalog bands %s", d.Name, n, strings.Join(parts, " "))
292314
}

0 commit comments

Comments
 (0)