fix(domains): read an integral float code column as integer text

CHANGELOG.md

@@ -7,6 +7,17 @@ adheres to [Semantic Versioning](https://semver.org/).
 ## [Unreleased]
 
 ### Fixed
+- A domain regex rule no longer fails a valid code because its column is
+  `float64`. A numeric code column with one blank cell loads from CSV as
+  `float64`, so `str()` renders `10000266` as `"10000266.0"` and the trailing
+  `0` reads as an extra digit — every row of a perfectly valid column then
+  failed a digit-only pattern and the domain trust score dropped with it
+  (GS1-008 `[0-9]{8}` and FIN-008 `[A-Za-z0-9]{4,12}` were both affected).
+  `retail/validator.py` already carried `_integral_float_text` for exactly this
+  case on the GTIN checks; it now lives in `domains.base` as
+  `integral_float_text` and the shared rule engine applies it to every regex
+  rule. Genuine decimals, non-finite floats, text and integers are returned
+  unchanged, so a real violation is still reported.
 - `fd.clean_excel` now preserves zero padding, as `fd.clean_csv` already did.
   `pandas.read_excel` infers types exactly as `read_csv` does, so a cell that
   the workbook stored as the **text** `"02134"` arrived as the integer `2134`

src/freshdata/domains/base.py

@@ -14,12 +14,14 @@
 
 from __future__ import annotations
 
+import math
 import re
 from abc import ABC, abstractmethod
 from collections.abc import Callable, Iterable, Mapping, Sequence
 from dataclasses import dataclass, field
 from typing import Any
 
+import numpy as np
 import pandas as pd
 
 from .._numeric import safe_to_numeric
@@ -40,6 +42,24 @@
 SEVERITY_WEIGHT: dict[str, float] = {"error": 1.0, "warning": 0.25, "info": 0.05}
 
 
+
+def integral_float_text(value: Any) -> Any:
+    """Render an integral float cell as integer text (``10000266.0`` -> ``"10000266"``).
+
+    A numeric code column with a blank cell loads from CSV as float64, and its
+    ``str()`` form carries a ``.0`` suffix whose ``0`` reads as an extra digit.
+    Every other value is returned unchanged, so a genuine decimal keeps its
+    fractional part.
+    """
+    if (
+        isinstance(value, (float, np.floating))
+        and math.isfinite(value)
+        and float(value).is_integer()
+    ):
+        return str(int(value))
+    return value
+
+
 class DomainError(ValueError):
     """Base class for domain-pack errors."""
 
@@ -560,7 +580,10 @@ def _check_regex(self, df: pd.DataFrame, mapping: ColumnMapping, rule: Rule) ->
         pattern = str(rule.params["pattern"])
         series = df[col]
         present = series.notna()
-        as_text = series.astype("string")
+        # A numeric code column with one blank cell loads from CSV as float64,
+        # so str() renders 10000266 as "10000266.0" and a digit-only pattern
+        # rejects every row. Render integral floats as integer text first.
+        as_text = series.map(integral_float_text).astype("string")
         matches = as_text.str.fullmatch(pattern)
         bad = present & ~matches.fillna(False)
         return df.index[bad].tolist()

src/freshdata/domains/retail/validator.py

@@ -10,17 +10,15 @@
 from __future__ import annotations
 
 import json
-import math
 import re
 from functools import lru_cache
 from pathlib import Path
 from typing import Any
 
-import numpy as np
 import pandas as pd
 
 from ..._numeric import safe_to_numeric
-from ..base import ColumnMapping, ConfigDrivenValidator, Rule, RuleResult
+from ..base import ColumnMapping, ConfigDrivenValidator, Rule, RuleResult, integral_float_text
 
 _PACK_DIR = Path(__file__).resolve().parent
 _BUNDLED_DIR = _PACK_DIR.parent / "bundled"
@@ -60,20 +58,9 @@ def _gtin_well_formed(text: str) -> bool:
     return text.isdigit() and len(text) in _GTIN_LENGTHS
 
 
-def _integral_float_text(value: Any) -> Any:
-    """Render an integral float cell as integer text (``4012345678901.0`` -> ``"4012345678901"``).
-
-    A GTIN column with a blank cell loads from CSV as float64; its ``str()`` form
-    carries a ``.0`` suffix whose ``0`` would otherwise be read as an extra digit.
-    Every other value is returned unchanged.
-    """
-    if (
-        isinstance(value, (float, np.floating))
-        and math.isfinite(value)
-        and float(value).is_integer()
-    ):
-        return str(int(value))
-    return value
+#: The GTIN path has always needed this; it now lives in ``domains.base`` so the
+#: shared rule engine applies the same rendering to every regex rule.
+_integral_float_text = integral_float_text
 
 
 class RetailValidator(ConfigDrivenValidator):

tests/test_domain_regex_float_text.py

@@ -0,0 +1,119 @@
+"""A regex rule must not fail a valid code because the column is float64.
+
+A numeric code column with one blank cell loads from CSV as ``float64``, so
+``str()`` renders ``10000266`` as ``"10000266.0"`` and the trailing ``0`` reads
+as an extra digit. Every row of a perfectly valid column then fails a
+digit-only pattern, and the domain trust score drops accordingly.
+
+The repository had already settled this: ``retail/validator.py`` carried
+``_integral_float_text`` for exactly the GTIN case, and its docstring described
+this same CSV-blank-cell scenario. Only the GTIN checks used it -- the shared
+rule engine's ``_check_regex`` did not -- so the helper now lives in
+``domains.base`` and every regex rule gets the same rendering.
+
+Both built-in regex rules were affected: GS1-008 (``[0-9]{8}``) and FIN-008
+(``[A-Za-z0-9]{4,12}``).
+"""
+
+from __future__ import annotations
+
+import numpy as np
+import pandas as pd
+import pytest
+
+from freshdata.domains import run_domain
+from freshdata.domains.base import integral_float_text
+
+VALID_BRICKS = [10000266, 10000267, 10000268, 10000269]
+
+
+def _report(result):
+    report = result[1] if isinstance(result, tuple) else result
+    return getattr(report, "report", report)
+
+
+def _retail_frame(codes) -> pd.DataFrame:
+    return pd.DataFrame(
+        {
+            "gtin": [
+                "04012345678901",
+                "04012345678902",
+                "04012345678903",
+                "04012345678904",
+            ],
+            "description": ["a", "b", "c", "d"],
+            "gpc_brick_code": codes,
+        }
+    )
+
+
+def _rule(report, rule_id):
+    return next((r for r in report.results if r.rule_id == rule_id), None)
+
+
+def test_a_blank_cell_does_not_invalidate_every_other_code():
+    """The regression: one blank made all four valid codes fail GS1-008."""
+    floats = [float(c) for c in VALID_BRICKS[:3]] + [np.nan]
+    report = _report(run_domain(_retail_frame(floats), "retail"))
+    rule = _rule(report, "GS1-008")
+    assert rule.passed, "a float64 column of valid codes must still pass"
+    assert not rule.violation_rows
+
+
+def test_the_float_and_integer_columns_agree():
+    """Same codes, two dtypes, same verdict and same trust score."""
+    as_int = _report(run_domain(_retail_frame(list(VALID_BRICKS)), "retail"))
+    floats = [float(c) for c in VALID_BRICKS[:3]] + [np.nan]
+    as_float = _report(run_domain(_retail_frame(floats), "retail"))
+    assert _rule(as_int, "GS1-008").passed == _rule(as_float, "GS1-008").passed is True
+    assert as_int.domain_trust_score == as_float.domain_trust_score
+
+
+def test_genuinely_invalid_codes_are_still_caught():
+    """The fix must not buy its pass rate with a false negative."""
+    report = _report(run_domain(_retail_frame([1234, 5678, 91011, 121314]), "retail"))
+    rule = _rule(report, "GS1-008")
+    assert not rule.passed
+    assert len(rule.violation_rows) == 4
+
+
+def test_the_finance_regex_rule_benefits_too():
+    """FIN-008 uses the same shared check, so it had the same defect."""
+    frame = pd.DataFrame(
+        {
+            "txn_id": ["t1", "t2", "t3", "t4"],
+            "amount": [10.0, 20.0, 30.0, 40.0],
+            "currency": ["USD", "USD", "USD", "USD"],
+            "account_code": [10000266.0, 10000267.0, 10000268.0, np.nan],
+        }
+    )
+    rule = _rule(_report(run_domain(frame, "finance")), "FIN-008")
+    assert rule is not None, "FIN-008 should be evaluated for this frame"
+    assert rule.passed, "a float64 account_code of valid codes must pass"
+
+
+# -- the helper itself -------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+    ("value", "expected"),
+    [
+        (10000266.0, "10000266"),   # the case that mattered
+        (-42.0, "-42"),
+        (12.5, 12.5),               # a real decimal keeps its fraction
+        (0.1, 0.1),
+        ("0250", "0250"),           # text is untouched, padding survives
+        (7, 7),                     # ints are already fine
+        (None, None),
+    ],
+)
+def test_only_integral_floats_are_rewritten(value, expected):
+    assert integral_float_text(value) == expected
+
+
+@pytest.mark.parametrize("value", [np.nan, np.inf, -np.inf])
+def test_non_finite_floats_pass_through(value):
+    """``int(nan)`` would raise; these must be returned unchanged."""
+    result = integral_float_text(value)
+    assert isinstance(result, float)
+    assert (np.isnan(result) and np.isnan(value)) or result == value

tests/test_domain_validation_lane.py

@@ -30,11 +30,13 @@
 behaviour cannot drift unnoticed and a deliberate fix has one obvious place to
 update:
 
-* ``test_regex_check_misreads_an_integral_float_column`` — S2: the shared
-  ``_check_regex`` stringifies a float64 column as ``"10000266.0"``, so FIN-008
-  and GS1-008 raise false findings for the ordinary "CSV column with one blank
-  cell" case. The retail pack already solves exactly this for GTIN with
-  ``_integral_float_text``; the base engine does not use it.
+* ``test_regex_check_reads_an_integral_float_column_correctly`` — was an S2
+  finding: the shared ``_check_regex`` stringified a float64 column as
+  ``"10000266.0"``, so FIN-008 and GS1-008 raised false findings for the
+  ordinary "CSV column with one blank cell" case. **Fixed** by moving the
+  retail pack's ``_integral_float_text`` into ``domains.base`` as
+  ``integral_float_text`` and applying it to every regex rule; this test now
+  asserts the repaired behaviour.
 * ``test_balanced_tolerance_never_admits_a_one_cent_imbalance`` — S3: FIN-006
   declares ``tolerance: 0.01`` but compares raw binary floats, so a one-cent
   rounding difference is always above tolerance.
@@ -1098,20 +1100,21 @@ def test_reported_row_labels_are_translated_back_but_stay_ambiguous(self):
 class TestSharedCheckEngine:
     """Defects that live in ``ConfigDrivenValidator``, so they hit several packs."""
 
-    def test_regex_check_misreads_an_integral_float_column(self):
-        """FINDING (S2): ``_check_regex`` stringifies float64 with a ``.0`` suffix.
+    def test_regex_check_reads_an_integral_float_column_correctly(self):
+        """A float64 code column must not raise false findings.
 
-        A numeric code column loaded from CSV becomes float64 as soon as one cell
-        is blank, and ``Series.astype("string")`` then renders ``10000266`` as
-        ``"10000266.0"``, which no digit-only pattern can match. Both regex rules
-        in the repo are affected: GS1-008 (``[0-9]{8}``) and FIN-008
-        (``[A-Za-z0-9]{4,12}``). Both are warning severity, so ``passed`` does not
-        flip, but every row raises a false finding and the trust score drops.
+        This test was written to pin the defect. A numeric code column loaded
+        from CSV becomes float64 as soon as one cell is blank, and
+        ``Series.astype("string")`` then rendered ``10000266`` as
+        ``"10000266.0"``, which no digit-only pattern can match. Both regex
+        rules were affected: GS1-008 (``[0-9]{8}``) and FIN-008
+        (``[A-Za-z0-9]{4,12}``).
 
-        The repo already solves exactly this problem for GTIN — see
+        The repo had already settled the intended behaviour for GTIN in
         ``retail/validator.py::_integral_float_text``, whose docstring describes
-        the same CSV-blank-cell scenario — so the intended behaviour is settled;
-        the shared engine simply does not apply it.
+        the same CSV-blank-cell scenario; the shared engine simply did not apply
+        it. The helper now lives in ``domains.base`` and every regex rule uses
+        it, so the assertions below are the repaired behaviour.
         """
         as_object = pd.DataFrame({
             "gtin": ["00012345678905", "00012345678905"],
@@ -1128,8 +1131,10 @@ def test_regex_check_misreads_an_integral_float_column(self):
         })
         assert with_blank_cell["gpc_brick_code"].dtype == "float64"
         _, floaty = run_domain(with_blank_cell, "retail")
-        assert _result(floaty, "GS1-008").violation_rows == [0]   # false positive
-        assert floaty.trust_score == 0.875                        # 1 - 0.25 * (1/2)
+        assert _result(floaty, "GS1-008").violation_rows == []
+        assert floaty.trust_score == clean.trust_score, (
+            "the same codes must score the same whatever the column dtype"
+        )
 
         # The same engine path, the same false positive, in the finance pack.
         ledger = pd.DataFrame({
@@ -1139,7 +1144,7 @@ def test_regex_check_misreads_an_integral_float_column(self):
         })
         assert ledger["account_code"].dtype == "float64"
         _, finance = run_domain(ledger, "finance")
-        assert _result(finance, "FIN-008").violation_rows == [0]
+        assert _result(finance, "FIN-008").violation_rows == []
         # ...and it does not happen when the column stays an integer.
         ledger["account_code"] = [1000, 2000]
         _, integral = run_domain(ledger, "finance")