fix(spp_mis_demo_v2): make the SP-MIS demo back up what its programs claim

spp_cel_domain/README.rst

@@ -142,6 +142,44 @@ Dependencies
 Changelog
 =========
 
+19.0.2.1.2
+~~~~~~~~~~
+
+- feat(translator): arithmetic containing an aggregate is now evaluated
+  instead of discarded.
+  ``(child_count + elderly_count) / max(1, working_age_count) >= 1.5``
+  cannot be expressed as an Odoo domain, and the translator resolved the
+  whole left-hand side to the field ``id``, so
+  ``dependency_ratio >= 1.5`` compiled to ``('id', '>=', 1.5)`` and
+  matched every record. Such comparisons now run per candidate, with
+  each aggregate leaf resolved once into a per-parent map so the cost
+  stays a few queries rather than one per record. Supported inside
+  arithmetic: ``+ - * / %``, unary minus,
+  ``max``/``min``/``abs``/``round``, aggregate counts, literals and
+  numeric fields (#956)
+- fix(translator): an expression that cannot be resolved to a field now
+  raises instead of silently comparing on ``id``. That fallback turned
+  every unsupported form into a match against the primary key, which for
+  an eligibility rule means matching everyone -- a worse outcome than
+  refusing to compile (#956)
+
+19.0.2.1.1
+~~~~~~~~~~
+
+- fix(translator): ``members.count(predicate)`` now honours its
+  predicate when compared. Both call styles are valid -- a single
+  argument is the predicate with ``m`` implicit, two arguments are an
+  explicit loop variable and predicate -- but the comparison path read
+  the first argument as the loop variable either way and substituted a
+  ``True`` predicate when there was no second one.
+  ``members.count(pred) > n`` therefore counted every member, silently
+  and without error, so every aggregate count variable (``child_count``,
+  ``elderly_count``, ``working_age_count``) returned the household size
+  and any program targeting on one matched every household. ``exists()``
+  was unaffected, which is why variables built on it kept working. Note
+  an aggregate count nested inside arithmetic, as ``dependency_ratio``
+  is, still loses its predicate -- a separate path (#955)
+
 19.0.2.1.0
 ~~~~~~~~~~
 

spp_cel_domain/__manifest__.py

@@ -2,7 +2,7 @@
 {
     "name": "CEL Domain Query Builder",
     "summary": "Write simple CEL-like expressions to filter records (OpenSPP/OpenG2P friendly)",
-    "version": "19.0.2.1.0",
+    "version": "19.0.2.1.2",
     "license": "LGPL-3",
     "development_status": "Production/Stable",
     "author": "OpenSPP.org, OpenSPP Community",

spp_cel_domain/models/cel_executor.py

@@ -9,11 +9,13 @@
 from odoo.tools.sql import SQL
 
 from ..exceptions import CELMetricsUnavailableError
+from ..services import cel_parser as P
 from .cel_queryplan import (
     AND,
     NOT,
     OR,
     AggMetricCompare,
+    ArithmeticCompare,
     CountThrough,
     CoverageRequire,
     ExistsThrough,
@@ -658,7 +660,7 @@ def _plan_to_domain(self, model: str, plan: Any) -> tuple[list[Any], bool]:
             if e:
                 return [], True
             return ["!", *d], False
-        if isinstance(plan, ExistsThrough | CountThrough | FieldAggregateThrough):
+        if isinstance(plan, ExistsThrough | CountThrough | FieldAggregateThrough | ArithmeticCompare):
             return [], True
         return [], True
 
@@ -708,6 +710,164 @@ def _ensure_domain_list(self, domain: list[Any]) -> list[Any]:
             return normalized
         return [domain]
 
+    # ── Arithmetic over aggregates ──────────────────────────────────────
+    # No Odoo domain can express `(child_count + elderly_count) / max(1,
+    # working_age_count) >= 1.5`, so it is evaluated here. Each aggregate leaf
+    # is resolved once into a {parent_id: value} map, then the arithmetic runs
+    # per candidate against those maps: a handful of queries rather than one
+    # per record, but still a scan of the candidate set.
+
+    _ARITH_OPS = {
+        "ADD": lambda a, b: a + b,
+        "SUB": lambda a, b: a - b,
+        "MUL": lambda a, b: a * b,
+        "DIV": lambda a, b: a / b if b else 0.0,
+        "MOD": lambda a, b: a % b if b else 0.0,
+    }
+    _COMPARE_OPS = {
+        "=": lambda a, b: a == b,
+        "==": lambda a, b: a == b,
+        "!=": lambda a, b: a != b,
+        ">": lambda a, b: a > b,
+        ">=": lambda a, b: a >= b,
+        "<": lambda a, b: a < b,
+        "<=": lambda a, b: a <= b,
+    }
+    _ARITH_FUNCS = {
+        "max": max,
+        "min": min,
+        "abs": lambda *a: abs(a[0]),
+        "round": lambda *a: round(*a),
+    }
+
+    def _is_aggregate_call(self, node: Any, cfg: dict) -> bool:
+        """A `<collection>.count(...)` call on a relation symbol."""
+        if not isinstance(node, P.Call) or not isinstance(node.func, P.Attr):
+            return False
+        if not isinstance(node.func.obj, P.Ident):
+            return False
+        sym = (cfg.get("symbols") or {}).get(node.func.obj.name) or {}
+        return sym.get("relation") == "rel" and node.func.name == "count"
+
+    def _collect_aggregate_calls(self, node: Any, cfg: dict, found: list | None = None) -> list:
+        found = [] if found is None else found
+        if self._is_aggregate_call(node, cfg):
+            found.append(node)
+            return found
+        for attr in ("left", "right", "expr", "obj"):
+            child = getattr(node, attr, None)
+            if child is not None:
+                self._collect_aggregate_calls(child, cfg, found)
+        for arg in getattr(node, "args", None) or []:
+            self._collect_aggregate_calls(arg, cfg, found)
+        return found
+
+    def _aggregate_count_map(self, node: Any, cfg: dict) -> dict[int, int]:
+        """{parent_id: matching child count} for one `collection.count(pred)`."""
+        coll_name = node.func.obj.name
+        sym = (cfg.get("symbols") or {}).get(coll_name) or {}
+        through_model = sym["through"]
+        parent_field = sym["parent"]
+        link_field = sym.get("link_to") or sym.get("link_field") or sym.get("link") or "id"
+        child_model = sym.get("child_model") or "res.partner"
+
+        # Both call styles, same discrimination the translator makes.
+        args = list(node.args or [])
+        if len(args) == 1:
+            first = args[0]
+            if isinstance(first, P.Ident) and first.name == "m":
+                pred = P.Literal(True)
+            else:
+                pred = first
+        elif len(args) >= 2:
+            pred = args[1]
+        else:
+            pred = P.Literal(True)
+
+        translator = self.env["spp.cel.translator"]
+        child_plan, _explain = translator._to_plan(
+            child_model, pred, cfg, {"m": {"kind": "rel_var", "sym": sym, "model": child_model}}
+        )
+        child_ids = self._execute_plan(child_model, child_plan)
+
+        counts: dict[int, int] = {}
+        if not child_ids and not isinstance(pred, P.Literal):
+            return counts
+        domain = list(sym.get("default_domain") or [])
+        if link_field == "id":
+            domain = domain + [("id", "in", child_ids)]
+        else:
+            domain = domain + [(link_field, "in", child_ids)]
+        for group in self.env[through_model].read_group(domain, [parent_field], [parent_field]):
+            parent = group.get(parent_field)
+            parent_id = parent[0] if isinstance(parent, tuple | list) else parent
+            if parent_id:
+                counts[parent_id] = group.get("__count") or group.get(f"{parent_field}_count") or 0
+        return counts
+
+    def _eval_arith(self, node: Any, record: Any, agg_maps: dict, cfg: dict):
+        """Evaluate an arithmetic node for one record."""
+        if self._is_aggregate_call(node, cfg):
+            return agg_maps.get(id(node), {}).get(record.id, 0)
+        if isinstance(node, P.Literal):
+            return node.value
+        if isinstance(node, P.Neg):
+            return -self._eval_arith(node.expr, record, agg_maps, cfg)
+        if isinstance(node, P.BinOp):
+            left = self._eval_arith(node.left, record, agg_maps, cfg)
+            right = self._eval_arith(node.right, record, agg_maps, cfg)
+            handler = self._ARITH_OPS.get(node.op)
+            if handler is None:
+                raise NotImplementedError(f"arithmetic operator {node.op} is not supported")
+            return handler(left, right)
+        if isinstance(node, P.Call) and isinstance(node.func, P.Ident):
+            func = self._ARITH_FUNCS.get(node.func.name)
+            if func is None:
+                raise NotImplementedError(f"function {node.func.name}() is not supported inside arithmetic")
+            return func(*[self._eval_arith(a, record, agg_maps, cfg) for a in node.args or []])
+        if isinstance(node, P.Ident):
+            if node.name in ("r", "me"):
+                return record
+            value = record[node.name] if node.name in record._fields else None
+            return value if value not in (None, False) else 0
+        if isinstance(node, P.Attr):
+            obj = self._eval_arith(node.obj, record, agg_maps, cfg)
+            if hasattr(obj, "_fields"):
+                value = obj[node.name] if node.name in obj._fields else None
+                return value if value not in (None, False) else 0
+            raise NotImplementedError(f"cannot read {node.name} inside arithmetic")
+        raise NotImplementedError(f"{type(node).__name__} is not supported inside arithmetic")
+
+    def _execute_arithmetic(self, plan: ArithmeticCompare) -> list[int]:
+        cfg = plan.cfg or {}
+        compare = self._COMPARE_OPS.get(plan.op)
+        if compare is None:
+            raise NotImplementedError(f"comparison {plan.op} is not supported for arithmetic")
+
+        agg_maps = {}
+        for call in self._collect_aggregate_calls(plan.expr, cfg):
+            agg_maps[id(call)] = self._aggregate_count_map(call, cfg)
+
+        candidates = self.env[plan.model].search(self._ensure_domain_list(cfg.get("base_domain") or []))
+        self._logger.info(
+            "[CEL] arithmetic comparison evaluated in Python over %d candidate(s)",
+            len(candidates),
+        )
+        matched = []
+        for record in candidates:
+            try:
+                value = self._eval_arith(plan.expr, record, agg_maps, cfg)
+            except NotImplementedError:
+                raise
+            except Exception:
+                continue
+            try:
+                if compare(value, plan.rhs):
+                    matched.append(record.id)
+            except TypeError:
+                continue
+        return matched
+
     # Execute
     def _execute_plan(
         self,
@@ -718,6 +878,8 @@ def _execute_plan(
     ) -> list[int]:  # noqa: C901
         if isinstance(plan, LeafDomain):
             return self.env[plan.model].search(plan.domain).ids
+        if isinstance(plan, ArithmeticCompare):
+            return self._execute_arithmetic(plan)
         if isinstance(plan, AND):
             # intersection
             id_sets = [set(self._execute_plan(model, p, metrics_info)) for p in flatten_and(plan.nodes)]

spp_cel_domain/models/cel_queryplan.py

@@ -116,6 +116,33 @@ class AggMetricCompare:
     default_domain: list | None = None
 
 
+@dataclass
+class ArithmeticCompare:
+    """Compare an arithmetic expression over aggregates against a value.
+
+    `dependency_ratio >= 1.5` expands to
+    `(child_count + elderly_count) / max(1, working_age_count) >= 1.5`, which
+    no Odoo domain can express: the value depends on counting related records
+    per parent and then doing arithmetic on the results.
+
+    Before this node existed such an expression fell through the translator's
+    field resolution to the literal field `id`, producing `('id', '>=', 1.5)`
+    and quietly matching every record.
+
+    `expr` is the parsed left-hand side. The executor collects its aggregate
+    leaves, resolves each to a {parent_id: value} map with one grouped read
+    apiece, then evaluates the arithmetic per candidate. That keeps the cost
+    at a few queries rather than one per record, but it is still a scan of the
+    candidate set: no SQL fast path applies.
+    """
+
+    model: str
+    expr: Any
+    op: str
+    rhs: Any
+    cfg: dict | None = None
+
+
 def flatten_and(nodes: list[Any]) -> list[Any]:
     out = []
     for n in nodes:

spp_cel_domain/models/cel_translator.py

@@ -15,6 +15,7 @@
     NOT,
     OR,
     AggMetricCompare,
+    ArithmeticCompare,
     CountThrough,
     CoverageRequire,
     ExistsThrough,
@@ -727,8 +728,30 @@ def _cmp_to_leaf(self, model: str, cmp: P.Compare, cfg: dict[str, Any], ctx: dic
                 if sym and sym.get("relation") == "rel":
                     if len(cmp.left.args) < 1:
                         raise NotImplementedError(f"{coll_name}.count(var, pred?) requires at least 1 argument")
-                    var = cmp.left.args[0]
-                    pred = cmp.left.args[1] if len(cmp.left.args) > 1 else P.Literal(True)
+                    # Both call styles are supported, and telling them apart
+                    # matters: a single argument is the predicate with `m`
+                    # implicit (ADR-008), not a loop variable. Reading args[0]
+                    # as the variable unconditionally made
+                    # `members.count(pred) > n` discard its predicate and count
+                    # every member, so every aggregate count variable
+                    # (child_count, elderly_count, working_age_count, and
+                    # dependency_ratio derived from them) silently returned the
+                    # household size and matched everything. The boolean-context
+                    # branch above already distinguishes the two; this one did
+                    # not.
+                    if len(cmp.left.args) == 1:
+                        first_arg = cmp.left.args[0]
+                        if isinstance(first_arg, P.Ident) and first_arg.name == "m":
+                            # `members.count(m) > n`: no predicate, count all.
+                            var = first_arg
+                            pred = P.Literal(True)
+                        else:
+                            var = P.Ident("m")
+                            pred = first_arg
+                    else:
+                        # Legacy style: members.count(var, pred)
+                        var = cmp.left.args[0]
+                        pred = cmp.left.args[1]
                     child_model = self._symbol_child_model(sym)
                     subctx = dict(ctx)
                     if isinstance(var, P.Ident):
@@ -1005,6 +1028,18 @@ def _flatten_attr(a):
                 f"{agg.upper()} over {coll_name} of METRIC({metric_name}) {op} {rhs}",
             )
 
+        # Arithmetic on the left, possibly over aggregates: no domain can
+        # express it, so hand it to the executor to evaluate per record. Left
+        # to fall through, `_resolve_field` would return the field `id` and the
+        # comparison would silently match everything -- which is how
+        # `dependency_ratio >= 1.5` came to match every household.
+        if isinstance(cmp.left, P.BinOp | P.Neg):
+            rhs = self._eval_literal(cmp.right, ctx)
+            return (
+                ArithmeticCompare(model, cmp.left, opmap[cmp.op], rhs, cfg),
+                f"ARITHMETIC {opmap[cmp.op]} {rhs}",
+            )
+
         # Normal comparison
         left_field, left_model = self._resolve_field(model, cmp.left, cfg, ctx)
         # normalize aliases
@@ -1286,7 +1321,17 @@ def _resolve_field(self, model: str, expr: Any, cfg: dict[str, Any], ctx: dict[s
             return expr.name, model
         if isinstance(expr, P.Literal):
             return expr.value, model
-        return "id", model
+        # Anything else cannot be resolved to a field. Returning "id" here, as
+        # this used to, turned every unsupported expression into a comparison
+        # on the primary key: `dependency_ratio >= 1.5` became
+        # `('id', '>=', 1.5)` and matched every record, with nothing logged and
+        # no error raised. For eligibility rules that failure mode is worse
+        # than no answer, so it is loud now.
+        raise NotImplementedError(
+            f"Cannot resolve {type(expr).__name__} to a field on {model}. "
+            f"Arithmetic over aggregates is handled separately; if you are "
+            f"comparing something else, it is not supported yet."
+        )
 
     def _symbol_child_model(self, sym: dict[str, Any]) -> str:
         return sym.get("child_model") or "res.partner"

spp_cel_domain/readme/HISTORY.md

@@ -1,3 +1,12 @@
+### 19.0.2.1.2
+
+- feat(translator): arithmetic containing an aggregate is now evaluated instead of discarded. `(child_count + elderly_count) / max(1, working_age_count) >= 1.5` cannot be expressed as an Odoo domain, and the translator resolved the whole left-hand side to the field `id`, so `dependency_ratio >= 1.5` compiled to `('id', '>=', 1.5)` and matched every record. Such comparisons now run per candidate, with each aggregate leaf resolved once into a per-parent map so the cost stays a few queries rather than one per record. Supported inside arithmetic: `+ - * / %`, unary minus, `max`/`min`/`abs`/`round`, aggregate counts, literals and numeric fields (#956)
+- fix(translator): an expression that cannot be resolved to a field now raises instead of silently comparing on `id`. That fallback turned every unsupported form into a match against the primary key, which for an eligibility rule means matching everyone -- a worse outcome than refusing to compile (#956)
+
+### 19.0.2.1.1
+
+- fix(translator): `members.count(predicate)` now honours its predicate when compared. Both call styles are valid -- a single argument is the predicate with `m` implicit, two arguments are an explicit loop variable and predicate -- but the comparison path read the first argument as the loop variable either way and substituted a `True` predicate when there was no second one. `members.count(pred) > n` therefore counted every member, silently and without error, so every aggregate count variable (`child_count`, `elderly_count`, `working_age_count`) returned the household size and any program targeting on one matched every household. `exists()` was unaffected, which is why variables built on it kept working. Note an aggregate count nested inside arithmetic, as `dependency_ratio` is, still loses its predicate -- a separate path (#955)
+
 ### 19.0.2.1.0
 
 - feat(sql): compile CEL ternary expressions to SQL CASE via `to_sql_case`, with `case_when`/`comparison` builders and a right-associative ternary parsing fix