Skip to content

Commit 2d00628

Browse files
committed
fix: add explicit decimal grounding equality
1 parent 061d650 commit 2d00628

7 files changed

Lines changed: 136 additions & 5 deletions

File tree

actionrail/sdk/grounding/match.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,22 @@ def _compare_dt(actual_dt, op: str, threshold) -> bool:
6868
"eq": actual_dt == threshold, "ne": actual_dt != threshold}.get(op, False)
6969

7070

71-
def _compare(actual, op: str, expected) -> bool:
71+
def _compare(actual, op: str, expected, value_type: str | None = None) -> bool:
7272
if op in ("set", "empty"):
7373
is_empty = actual is None or str(actual) == ""
7474
return is_empty if op == "empty" else not is_empty
7575
if op == "in":
7676
return str(actual) in [s.strip() for s in str(expected).split(",")]
7777
if op == "contains":
7878
return str(expected) in str(actual)
79+
if value_type == "decimal":
80+
a, e = finite_decimal(actual), finite_decimal(expected)
81+
if a is None or e is None:
82+
return False
83+
if op == "eq":
84+
return a == e
85+
if op == "ne":
86+
return a != e
7987
if op in ("gt", "gte", "lt", "lte"):
8088
a, e = finite_decimal(actual), finite_decimal(expected)
8189
if a is None or e is None:
@@ -117,7 +125,7 @@ def _eval_column(m: dict, record: dict, ctx: dict, args: dict, value) -> str | N
117125
expected, shown = m.get("value"), repr(m.get("value"))
118126
else:
119127
expected, shown = None, ""
120-
ok = _compare(actual, op, expected)
128+
ok = _compare(actual, op, expected, m.get("type"))
121129
if ok:
122130
return None
123131
return f"{col}={actual} fails: {col} {_OP_LABEL.get(op, op)} {shown}".strip()

actionrail/sdk/rule_validation.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import math
1212
import re
1313

14+
from .numeric import finite_decimal
1415
from .policy import parse_policy
1516

1617
_COLUMN_OPERATORS = {"eq", "ne", "gt", "gte", "lt", "lte", "contains", "in", "set", "empty"}
@@ -143,7 +144,7 @@ def _validate_condition(
143144

144145
errors = _unknown_fields(
145146
condition,
146-
{"column", "op", "ctx", "arg", "value", "now"},
147+
{"column", "op", "ctx", "arg", "value", "now", "type"},
147148
tool=tool,
148149
arg=arg,
149150
field=field,
@@ -159,6 +160,17 @@ def _validate_condition(
159160
))
160161
return errors
161162

163+
value_type = condition.get("type")
164+
if value_type is not None:
165+
if value_type != "decimal":
166+
errors.append(_issue(
167+
tool, arg, field, "condition_type", "Condition type must be decimal when specified."
168+
))
169+
elif operator not in {"eq", "ne"}:
170+
errors.append(_issue(
171+
tool, arg, field, "condition_type_operator", "Decimal type is supported only with eq or ne."
172+
))
173+
162174
targets = [name for name in ("ctx", "arg", "value", "now") if name in condition]
163175
if operator in _NO_TARGET_OPERATORS:
164176
if targets:
@@ -188,6 +200,10 @@ def _validate_condition(
188200
isinstance(target_value, float) and not math.isfinite(target_value)
189201
):
190202
errors.append(_issue(tool, arg, field, "condition_value", "Comparison value must be a finite scalar."))
203+
if value_type == "decimal" and finite_decimal(target_value) is None:
204+
errors.append(_issue(
205+
tool, arg, field, "condition_decimal_value", "Decimal comparison value must be finite numeric data."
206+
))
191207
if target == "now":
192208
if operator not in _TIME_OPERATORS:
193209
errors.append(_issue(tool, arg, field, "time_operator", "Time comparisons require eq, ne, gt, gte, lt, or lte."))

docs/docs/rules/match-conditions.mdx

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,21 @@ Another proposed tool argument:
5252
arg: amount
5353
```
5454

55+
Exact decimal equality for a money field must be declared explicitly:
56+
57+
```yaml
58+
- column: amount_due
59+
op: eq
60+
arg: amount
61+
type: decimal
62+
```
63+
64+
`type: decimal` is valid with `eq` and `ne`. Both operands are parsed by the
65+
same exact, finite `Decimal` path used by ActionRail policies. It treats
66+
`1250`, `1250.0`, and `"1250.00"` as the same number without rounding, while
67+
invalid, Boolean, NaN, and infinite inputs fail closed. Do not apply this type
68+
to identifiers, account numbers, or scopes where formatting is meaningful.
69+
5570
The current time plus an optional offset:
5671

5772
```yaml
@@ -66,8 +81,8 @@ Time offsets support `s`, `m`, `h`, `d`, and `w`, such as `-15m`, `+1h`, or `-30
6681

6782
| Operator | Meaning | Comparison behavior |
6883
| --- | --- | --- |
69-
| `eq` | equal | String equality; default. |
70-
| `ne` | not equal | String inequality. |
84+
| `eq` | equal | String equality by default; exact numeric equality with `type: decimal`. |
85+
| `ne` | not equal | String inequality by default; exact numeric inequality with `type: decimal`. |
7186
| `gt` | greater than | Numeric conversion. |
7287
| `gte` | greater than or equal | Numeric conversion. |
7388
| `lt` | less than | Numeric conversion. |
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Numeric grounding connector audit
2+
3+
This audit separates the benchmark fixture defect from shipped connector risk.
4+
It covers the SQLite, PostgreSQL, MySQL, HTTP, and MCP grounding sources.
5+
6+
## Finding
7+
8+
Every shipped connector delegates returned-record conditions to
9+
`actionrail.sdk.grounding.match.evaluate`. Before this repair, its `eq` and `ne`
10+
operators compared `str(actual)` with `str(expected)`. Numeric values returned
11+
as `1250.0` therefore differed from proposed values serialized as `1250`.
12+
13+
| Connector | Proposed value transport | Returned-record comparison | Exposure before repair |
14+
| --- | --- | --- | --- |
15+
| SQLite | Bound query parameter | Shared `evaluate` matcher | Yes for `eq`/`ne` column conditions |
16+
| PostgreSQL | Bound query parameter | Shared `evaluate` matcher | Yes for `eq`/`ne` column conditions |
17+
| MySQL | Bound query parameter | Shared `evaluate` matcher | Yes for `eq`/`ne` column conditions |
18+
| HTTP | Path/request interpolation | Shared `evaluate` matcher | Yes for JSON numeric `eq`/`ne` conditions |
19+
| MCP | Structured tool arguments | Shared `evaluate` matcher | Yes for structured numeric `eq`/`ne` conditions |
20+
21+
Queries that directly compare a bound `:value` with a database column use the
22+
database's own equality semantics. That path is not the defect. The exposed path
23+
is a returned record subsequently compared by ActionRail's shared matcher.
24+
25+
## Repair contract
26+
27+
Money fields opt into `type: decimal` on an `eq` or `ne` column condition. Both
28+
operands use the existing `finite_decimal()` implementation shared with policy
29+
evaluation. Comparison is exact and never rounds through `float`.
30+
31+
Plain `eq` and `ne` remain string comparisons. Numeric-looking identifiers,
32+
accounts, and scopes therefore retain their original formatting semantics.
33+
Invalid, Boolean, non-finite, or excessively large decimal inputs fail closed.
34+
35+
Database schema inference is intentionally not used as an authority: arbitrary
36+
queries may contain casts, aliases, expressions, and views. Connector metadata
37+
may support future configuration validation, but the rule's explicit field
38+
contract controls security behavior.
39+
40+
## Regression coverage
41+
42+
Tests cover integer/float equivalence, zero, cents, numeric strings, sub-cent
43+
inequality, non-finite values, identifier isolation, rule validation, shared
44+
record evaluation, and SQLite connector behavior. Because all five connectors
45+
call the same matcher, the shared tests exercise the production comparison path
46+
used by each connector.

tests/test_operators.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,33 @@ def test_compare(actual, op, expected, result):
3232
assert _compare(actual, op, expected) is result
3333

3434

35+
@pytest.mark.parametrize(
36+
("actual", "expected", "result"),
37+
[
38+
(1250, 1250.0, True),
39+
(0, 0.0, True),
40+
("86.40", 86.4, True),
41+
(1250.001, "1250.00", False),
42+
("00123", "123", True),
43+
(float("nan"), 0, False),
44+
(float("inf"), 0, False),
45+
(True, 1, False),
46+
],
47+
)
48+
def test_decimal_equality_uses_shared_exact_numeric_path(actual, expected, result):
49+
assert _compare(actual, "eq", expected, "decimal") is result
50+
51+
52+
def test_numeric_contract_is_explicit_and_does_not_leak_to_identifiers():
53+
assert _compare("00123", "eq", "123") is False
54+
assert _compare("1250", "eq", 1250) is True
55+
assert _compare("1250", "eq", 1250, "decimal") is True
56+
57+
58+
def test_decimal_inequality_fails_closed_for_invalid_values():
59+
assert _compare("not-a-number", "ne", 0, "decimal") is False
60+
61+
3562
def test_parse_offset():
3663
assert _parse_offset("") == timedelta(0)
3764
assert _parse_offset("0") == timedelta(0)
@@ -71,6 +98,13 @@ def test_evaluate_operand_free_column_ops():
7198
assert evaluate(1, {"note": ""}, empty, {}, {}, "A").grounded
7299

73100

101+
def test_evaluate_decimal_field_contract():
102+
check = {"match": [{"column": "amount", "op": "eq", "arg": "amount", "type": "decimal"}]}
103+
assert evaluate(1, {"amount": 1250.0}, check, {}, {"amount": 1250}, "invoice").grounded
104+
assert evaluate(1, {"amount": 86.4}, check, {}, {"amount": "86.40"}, "invoice").grounded
105+
assert not evaluate(1, {"amount": 1250.0}, check, {}, {"amount": 1250.001}, "invoice").grounded
106+
107+
74108
def test_compare_dt():
75109
ref = datetime(2026, 1, 1, tzinfo=timezone.utc)
76110
assert _compare_dt(None, "gt", ref) is False # unparseable actual -> safe False

tests/test_rule_validation.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,9 @@ def test_retry_shape_and_types_are_fail_closed(retry, code):
164164
({"column": "owner", "value": float("inf")}, "condition_value"),
165165
({"column": "created_at", "op": "contains", "now": "-30d"}, "time_operator"),
166166
({"column": "created_at", "op": "gte", "now": "yesterday"}, "time_offset"),
167+
({"column": "amount", "op": "eq", "arg": "amount", "type": "number"}, "condition_type"),
168+
({"column": "amount", "op": "gte", "arg": "amount", "type": "decimal"}, "condition_type_operator"),
169+
({"column": "amount", "op": "eq", "value": "not-a-number", "type": "decimal"}, "condition_decimal_value"),
167170
])
168171
def test_column_conditions_reject_invalid_shapes_and_targets(condition, code):
169172
rules = _rules({"source": "billing", "match": [condition]})
@@ -177,6 +180,7 @@ def test_column_conditions_reject_invalid_shapes_and_targets(condition, code):
177180
{"column": "created_at", "op": "gte", "now": 0},
178181
{"column": "created_at", "op": "lt", "now": "+1h"},
179182
{"column": "owner", "value": None},
183+
{"column": "amount", "op": "eq", "value": "86.40", "type": "decimal"},
180184
])
181185
def test_valid_target_variants_are_accepted(condition):
182186
rules = _rules({"source": "billing", "match": [condition]})

tests/test_source.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,14 @@ def test_match_another_arg(conn):
5454
assert conn.ground("A", chk, {}, {"amount": 500}).grounded is False
5555

5656

57+
def test_exact_decimal_match_another_arg(conn):
58+
chk = {"query": "SELECT bal FROM accts WHERE id=:value",
59+
"match": [{"column": "bal", "op": "eq", "arg": "amount", "type": "decimal"}]}
60+
assert conn.ground("A", chk, {}, {"amount": 100}).grounded is True
61+
assert conn.ground("A", chk, {}, {"amount": "100.00"}).grounded is True
62+
assert conn.ground("A", chk, {}, {"amount": 100.001}).grounded is False
63+
64+
5765
def test_match_now(conn):
5866
q = "SELECT exp FROM accts WHERE id=:value"
5967
# exp is +10 days: later than now, earlier than now+30d

0 commit comments

Comments
 (0)