Document filters.py scope; fix three edge-case pitfall gaps

Document the scope explicitly in the module docstring — what gets
parsed vs what passes through to the server untouched — since a
reviewer came away from the previous version wondering "are we only
handling OR?" (answer: yes for chunking, three forms for pitfall
detection, everything else passes through).

While adding the docs I swept more CQL-text shapes and found three
real gaps in the pitfall check:

1. Leading-dot decimals (``value > .5``, ``value > -.5``,
   ``value > .5e-3``) bypassed the numeric regex because it required
   ``\d+`` before the decimal point. Users writing fractions this way
   were getting server 500s instead of the client-side ValueError.
   Extend ``_NUM`` to also accept ``\.\d+``.

2. ``NOT IN`` / ``NOT BETWEEN`` misattributed the field. The regexes
   matched with ``field='NOT'`` and the error said
   ``against 'NOT' (``NOT IN (…)``)``, which is confusing. Add a
   ``(?!NOT\b)`` lookahead so ``NOT`` can't be captured as a field,
   and an optional ``(?P<negated>NOT\s+)?`` before the keyword so the
   error reports ``against 'value' (``value NOT IN (…)``)`` instead.

3. Consequence-free guard: ``NOTES = 'hello'`` (a real field name
   starting with "NOT") continues to pass — the ``(?!NOT\b)``
   lookahead uses a word boundary so only the bare keyword is
   excluded.

Tests: +18 cases on the raise/allow parametrize lists; +5 cases on a
new ``test_pitfall_error_names_real_field_not_NOT_keyword`` that
pins the attribution contract. 176 non-live tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: thodson-usgs

dataretrieval/waterdata/filters.py

@@ -7,7 +7,42 @@
 ``_NUMERIC_COMPARE_RE`` for why), and the ``chunked`` decorator that
 ``utils.py`` applies to its single-request fetch function.
 
-Isolation contract (rolling the feature back):
+Scope — what this module parses vs. what passes through
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The CQL-text filter the caller supplies is **forwarded verbatim** to
+the server as a ``filter=`` query parameter. This module doesn't
+parse CQL semantics; it inspects the string for exactly two
+purposes:
+
+1. **Chunking** (``_chunk_cql_or``) splits the filter on top-level
+   ``OR`` boundaries so the full URL can stay under the server's
+   ~8 KB limit. Only ``OR`` splits cleanly into independent
+   sub-requests whose result sets can be union'd back together by
+   ``pd.concat`` + dedup. Everything else — ``AND`` chains,
+   ``NOT``, ``LIKE``, ``IS NULL``, spatial / temporal predicates,
+   function calls — is sent as a single request. If such a filter
+   is long enough to trip the URL limit the caller gets the
+   server's ``414``; we don't try to rewrite it, because no rewrite
+   preserves set semantics for those shapes.
+
+2. **Pitfall detection** (``_check_numeric_filter_pitfall``) scans
+   for three patterns where the user almost certainly means a
+   numeric comparison but the server would do a lexicographic one
+   (every queryable is string-typed):
+
+   - ``<field> <op> <unquoted_num>`` (and the reverse)
+   - ``<field> [NOT] IN (<unquoted_num>, ...)``
+   - ``<field> [NOT] BETWEEN <unquoted_num> AND <unquoted_num>``
+
+   ``LIKE``, ``IS NULL``, function-call RHS (``COUNT(x) > 5``),
+   ``CAST`` expressions, and arithmetic (``value > 1 + 1`` —
+   partially caught on ``value > 1``) are not flagged. The first
+   three the server doesn't support anyway; the last is a minor
+   offense-text imprecision rather than a correctness issue.
+
+Isolation contract (rolling the feature back)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 - ``dataretrieval/waterdata/filters.py`` and
   ``tests/waterdata_filters_test.py`` can be deleted wholesale.
@@ -22,9 +57,11 @@
   getters.
 - ``__init__.py``: drop the ``FILTER_LANG`` re-export.
 
-Exports:
-  - ``FILTER_LANG`` — type alias used by ``api.py`` and re-exported.
-  - ``chunked`` — decorator used by ``utils.py`` on its fetch helper.
+Exports
+~~~~~~~
+
+- ``FILTER_LANG`` — type alias used by ``api.py`` and re-exported.
+- ``chunked`` — decorator used by ``utils.py`` on its fetch helper.
 
 Everything else in this module is private (leading underscore).
 """
@@ -93,8 +130,12 @@
 # numeric>`` is a bug — quote the literal or drop the comparison and
 # filter in pandas.
 
-# Unquoted numeric literal: integer, decimal, or scientific notation.
-_NUM = r"-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?"
+# Unquoted numeric literal: integer, decimal (with or without leading
+# zero), or scientific notation. ``\d+(?:\.\d+)?`` covers ``1``,
+# ``1.5``; ``\.\d+`` covers the leading-dot form ``.5`` that users
+# sometimes write as a fraction. Trailing-dot ``5.`` is deliberately
+# not accepted (not a common numeric spelling and not required CQL).
+_NUM = r"-?(?:\d+(?:\.\d+)?|\.\d+)(?:[eE][+-]?\d+)?"
 _IDENT = r"[A-Za-z_]\w*"
 _OP = r">=|<=|<>|!=|==|=|>|<"
 
@@ -113,20 +154,23 @@
     re.VERBOSE,
 )
 
-# ``<field> IN (<numeric>, ...)`` — same footgun as simple comparison
-# but using the list form. Caught separately because ``IN`` isn't one
-# of the comparison operators in ``_OP``. We only need to see one
-# unquoted numeric inside the parentheses to know the user intends
-# numeric membership.
+# ``<field> [NOT] IN (<numeric>, ...)`` — same footgun as a simple
+# comparison but using the list form. Caught separately because ``IN``
+# isn't one of the comparison operators in ``_OP``. The ``(?!NOT\b)``
+# lookahead keeps the bare keyword ``NOT`` from being captured as the
+# field when the caller writes ``value NOT IN (…)``; the optional
+# ``(?:NOT\s+)?`` after the field captures the negation so the error
+# message can report the offending form accurately.
 _IN_NUMERIC_RE = re.compile(
-    rf"\b(?P<field>{_IDENT})\s+IN\s*\(\s*{_NUM}",
+    rf"\b(?!NOT\b)(?P<field>{_IDENT})\s+(?P<negated>NOT\s+)?IN\s*\(\s*{_NUM}",
     re.IGNORECASE,
 )
 
-# ``<field> BETWEEN <numeric> AND <numeric>`` — range form of the same
-# footgun.
+# ``<field> [NOT] BETWEEN <numeric> AND <numeric>`` — range form with
+# the same ``NOT`` handling as above.
 _BETWEEN_NUMERIC_RE = re.compile(
-    rf"\b(?P<field>{_IDENT})\s+BETWEEN\s+{_NUM}\s+AND\s+{_NUM}\b",
+    rf"\b(?!NOT\b)(?P<field>{_IDENT})\s+(?P<negated>NOT\s+)?"
+    rf"BETWEEN\s+{_NUM}\s+AND\s+{_NUM}\b",
     re.IGNORECASE,
 )
 
@@ -323,12 +367,14 @@ def _check_numeric_filter_pitfall(filter_expr: str) -> None:
     membership = _IN_NUMERIC_RE.search(masked)
     if membership:
         field = membership.group("field")
-        _raise_pitfall(field, f"{field} IN (…)")
+        op = "NOT IN" if membership.group("negated") else "IN"
+        _raise_pitfall(field, f"{field} {op} (…)")
 
     between = _BETWEEN_NUMERIC_RE.search(masked)
     if between:
         field = between.group("field")
-        _raise_pitfall(field, f"{field} BETWEEN …")
+        op = "NOT BETWEEN" if between.group("negated") else "BETWEEN"
+        _raise_pitfall(field, f"{field} {op} …")
 
 
 def _raise_pitfall(field: str, offense: str) -> None:

tests/waterdata_filters_test.py

@@ -480,13 +480,23 @@ def fake_construct_api_requests(**kwargs):
         "value > 1e5",
         "value >= 2.5E+3",
         "value < 1.5e-3",
+        # Leading-dot decimals (``.5`` is a fraction, not a typo)
+        "value > .5",
+        "value >= -.5",
+        "value < .5e-3",
         # ``IN`` list form — same footgun, common pattern for codes
         "parameter_code IN (60, 61)",
         "value IN (10, 20, 30)",
         "statistic_id in (11)",  # case-insensitive, single-element
+        # ``NOT IN`` with numbers — same footgun via negation
+        "value NOT IN (1, 2, 3)",
+        "parameter_code not in (60, 61)",
         # ``BETWEEN`` range form — same footgun
         "value BETWEEN 5 AND 10",
         "channel_flow between 100 and 500",
+        # ``NOT BETWEEN`` with numbers
+        "value NOT BETWEEN 0 AND 100",
+        "channel_flow not between 50 and 150",
         # Composite expressions
         "time >= '2023-01-01T00:00:00Z' AND value >= 1000",
         "value > 1000 OR value < 0",
@@ -529,6 +539,10 @@ def test_check_numeric_filter_pitfall_raises(expr):
         "parameter_code = '00060' AND statistic_id = '00011'",
         # CQL escape-quote (``O''Reilly``) within a quoted literal
         "name = 'O''Reilly 1000'",
+        # Identifiers that start with "NOT" (e.g. ``NOTES``) must not be
+        # mistakenly treated as the CQL negation keyword
+        "NOTES = 'hello'",
+        "NOTE_VAL LIKE 'anything%'",
     ],
 )
 def test_check_numeric_filter_pitfall_allows(expr):
@@ -537,6 +551,27 @@ def test_check_numeric_filter_pitfall_allows(expr):
     _check_numeric_filter_pitfall(expr)  # must not raise
 
 
+@pytest.mark.parametrize(
+    "expr,field,op",
+    [
+        ("value NOT IN (1, 2)", "value", "NOT IN"),
+        ("parameter_code NOT IN (60, 61)", "parameter_code", "NOT IN"),
+        ("value IN (1, 2)", "value", "IN"),
+        ("value NOT BETWEEN 0 AND 10", "value", "NOT BETWEEN"),
+        ("channel_flow between 100 and 500", "channel_flow", "BETWEEN"),
+    ],
+)
+def test_pitfall_error_names_real_field_not_NOT_keyword(expr, field, op):
+    """The CQL keyword ``NOT`` must not be reported as the offending field
+    — the error should identify the actual column and include ``NOT`` as
+    part of the operator form so the caller knows what to quote."""
+    with pytest.raises(ValueError) as exc:
+        _check_numeric_filter_pitfall(expr)
+    msg = str(exc.value)
+    assert f"against {field!r}" in msg, msg
+    assert op.upper() in msg.upper(), msg
+
+
 def test_get_continuous_surfaces_pitfall_to_caller():
     """End-to-end: the check runs at the ``get_continuous`` boundary,
     not as a deep internal-only protection, so callers see the error