refactor: address code review findings from simplify pass

- Consolidate 4 repeated get_effective_rule_for_user() calls into
  single _get_effective_rule() lookup per compute_aggregation() call
- Revert simulation to call metric services directly: distribution
  has no access control, fairness needs target_type-aware base_domain
- Fix stale README documenting removed privacy service methods
- Rename Python classes Statistic->Indicator, StatisticContext->
  IndicatorContext to match model names
- Update _description fields to match new naming

Author: jeremi

spp_analytics/models/service_aggregation.py

@@ -20,7 +20,7 @@ class AggregationService(models.AbstractModel):
     """
 
     _name = "spp.analytics.service"
-    _description = "Aggregation Service"
+    _description = "Analytics Service"
 
     MAX_GROUP_BY_DIMENSIONS = 3
 
@@ -62,16 +62,19 @@ def compute_aggregation(
         # Resolve scope
         scope_record = self._resolve_scope(scope)
 
+        # Resolve user access context once (single DB lookup)
+        rule = self._get_effective_rule()
+
         # Validate group_by dimensions
         group_by = group_by or []
-        self._validate_group_by(group_by)
+        self._validate_group_by(group_by, rule)
 
-        # Determine access level from user permissions
-        access_level = self._determine_access_level()
-        k_threshold = self._get_k_threshold()
+        # Determine access level and k-threshold from the resolved rule
+        access_level = self._access_level_from_rule(rule)
+        k_threshold = self._k_threshold_from_rule(rule)
 
         # Check scope is allowed for user
-        self._check_scope_allowed(scope)
+        self._check_scope_allowed(scope, rule)
 
         # Check cache if enabled
         cache_service = self.env["spp.analytics.cache"]
@@ -136,11 +139,23 @@ def _resolve_scope(self, scope):
         # Assume it's already a record
         return scope
 
-    def _validate_group_by(self, group_by):
+    def _get_effective_rule(self, user=None):
+        """
+        Look up the effective access rule for a user (single DB query).
+
+        :param user: res.users record (defaults to current user)
+        :returns: access rule record or None
+        """
+        user = user or self.env.user
+        # Use sudo() to read access rules - this is an internal security check
+        return self.env["spp.analytics.access.rule"].sudo().get_effective_rule_for_user(user)  # nosemgrep: odoo-sudo-without-context  # noqa: E501  # fmt: skip
+
+    def _validate_group_by(self, group_by, rule=None):
         """
         Validate group_by dimensions.
 
         :param group_by: List of dimension names
+        :param rule: Pre-resolved access rule (or None)
         :raises: ValidationError if invalid
         """
         if len(group_by) > self.MAX_GROUP_BY_DIMENSIONS:
@@ -155,52 +170,42 @@ def _validate_group_by(self, group_by):
                 raise ValidationError(_("Unknown dimension: %s") % dim_name)
 
         # Check access rule dimension restrictions
-        user = self.env.user
-        # Use sudo() to read access rules - this is an internal security check
-        rule = self.env["spp.analytics.access.rule"].sudo().get_effective_rule_for_user(user)  # nosemgrep: odoo-sudo-without-context  # noqa: E501  # fmt: skip
         if rule and group_by:
             rule.check_dimensions_allowed(group_by)
 
-    def _determine_access_level(self, user=None):
+    def _access_level_from_rule(self, rule):
         """
-        Determine access level from user permissions.
+        Extract access level from a pre-resolved rule.
 
-        :param user: res.users record (defaults to current user)
+        :param rule: access rule record or None
         :returns: "aggregate" or "individual"
         :rtype: str
         """
-        user = user or self.env.user
-        rule = self.env["spp.analytics.access.rule"].sudo().get_effective_rule_for_user(user)  # nosemgrep: odoo-sudo-without-context  # noqa: E501  # fmt: skip
         if rule:
             return rule.access_level
         # Default to aggregate-only for safety
         return "aggregate"
 
-    def _get_k_threshold(self, user=None):
+    def _k_threshold_from_rule(self, rule):
         """
-        Get k-anonymity threshold for user.
+        Extract k-anonymity threshold from a pre-resolved rule.
 
-        :param user: res.users record (defaults to current user)
+        :param rule: access rule record or None
         :returns: k threshold value
         :rtype: int
         """
-        user = user or self.env.user
-        rule = self.env["spp.analytics.access.rule"].sudo().get_effective_rule_for_user(user)  # nosemgrep: odoo-sudo-without-context  # noqa: E501  # fmt: skip
         if rule:
             return rule.minimum_k_anonymity
         return self.env["spp.metric.privacy"].DEFAULT_K_THRESHOLD
 
-    def _check_scope_allowed(self, scope):
+    def _check_scope_allowed(self, scope, rule=None):
         """
         Check if scope is allowed for current user.
 
         :param scope: Scope record or dict
+        :param rule: Pre-resolved access rule (or None)
         :raises: AccessError if not allowed
         """
-        user = self.env.user
-        # Use sudo() to read access rules - this is an internal security check
-        rule = self.env["spp.analytics.access.rule"].sudo().get_effective_rule_for_user(user)  # nosemgrep: odoo-sudo-without-context  # noqa: E501  # fmt: skip
-
         if not rule:
             # No explicit rule - allow with defaults
             return

spp_indicator/models/statistic.py

@@ -10,22 +10,22 @@
 _logger = logging.getLogger(__name__)
 
 
-class Statistic(models.Model):
-    """A publishable statistic based on a CEL variable.
+class Indicator(models.Model):
+    """A publishable indicator based on a CEL variable.
 
-    Statistics separate concerns:
+    Indicators separate concerns:
     - CEL Variable: "What data and how to compute it"
-    - Statistic: "Where to publish and how to present it"
+    - Indicator: "Where to publish and how to present it"
 
-    A single CEL variable can be published as multiple statistics
+    A single CEL variable can be published as multiple indicators
     with different presentations for different contexts.
 
     Inherits from spp.metric.base for common metric fields
     (name, label, description, category_id, sequence, etc.)
     """
 
     _name = "spp.indicator"
-    _description = "Publishable Statistic"
+    _description = "Publishable Indicator"
     _inherit = ["spp.metric.base"]
     _order = "category_id, sequence, name"
 

spp_indicator/models/statistic_context.py

@@ -4,16 +4,16 @@
 from odoo import fields, models
 
 
-class StatisticContext(models.Model):
-    """Context-specific presentation configuration for a statistic.
+class IndicatorContext(models.Model):
+    """Context-specific presentation configuration for an indicator.
 
     Allows overriding default presentation settings for specific contexts
-    (GIS, dashboard, API, reports). For example, a statistic might use
+    (GIS, dashboard, API, reports). For example, an indicator might use
     a different label or grouping in the GIS context vs. dashboard.
     """
 
     _name = "spp.indicator.context"
-    _description = "Statistic Context Configuration"
+    _description = "Indicator Context Configuration"
     _order = "statistic_id, context"
 
     statistic_id = fields.Many2one(

spp_metric_service/README.md

@@ -198,8 +198,7 @@ Enforces k-anonymity privacy protection on aggregation results.
 
 ```python
 enforce(result, k_threshold=None, access_level="aggregate")
-validate_access_level(user=None)
-get_k_threshold(user=None, context=None)
+suppress_value(value, count, k_threshold=None, stat_config=None)
 ```
 
 **Features:**

spp_simulation/services/simulation_service.py

@@ -65,17 +65,19 @@ def execute_simulation(self, scenario):
             amounts = self._apply_budget_strategy(scenario, amounts)
 
             # Step 4: Distribution stats
-            # Route through analytics service for consistent access control
-            analytics_service = self.env["spp.analytics.service"]
-            distribution_data = analytics_service.compute_distribution(amounts)
+            distribution_service = self.env["spp.metric.distribution"]
+            distribution_data = distribution_service.compute_distribution(amounts)
             gini = distribution_data.get("gini_coefficient", 0.0)
 
             # Step 5: Fairness analysis
-            # Route through analytics service with explicit scope
-            from odoo.addons.spp_analytics.services import build_explicit_scope
-
-            scope = build_explicit_scope(beneficiary_ids)
-            fairness_data = analytics_service.compute_fairness(scope)
+            fairness_service = self.env["spp.metric.fairness"]
+            # Derive base domain from target type so fairness compares against
+            # the correct population (groups vs individuals)
+            profile = "registry_groups" if scenario.target_type == "group" else "registry_individuals"
+            registry = self.env["spp.cel.registry"]
+            cfg = registry.load_profile(profile)
+            base_domain = cfg.get("base_domain", [])
+            fairness_data = fairness_service.compute_fairness(beneficiary_ids, base_domain)
             equity_score = fairness_data.get("equity_score", 100.0)
             has_disparity = fairness_data.get("has_disparity", False)