Skip to content

Commit fcac976

Browse files
committed
Fix provider TTL overrides and CEL preview cache
1 parent 2cdc776 commit fcac976

28 files changed

+248
-71
lines changed

spp_cel_domain/EXTERNAL_METRICS_SPEC_V2.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ Modules
3434

3535
- `openspp_metrics` (new core)
3636
- Owns feature store, provider registry, push/pull APIs, invalidation, admin dashboards.
37-
- Exposes Odoo models: `openspp.metric.registry`, `openspp.function.registry`, `openspp.feature.store`.
37+
- Exposes Odoo models: `openspp.indicator.registry`, `openspp.function.registry`, `openspp.feature.store`.
3838
- `cel_domain` (existing)
3939
- Soft‑depends at runtime; uses the registries to evaluate metrics in CEL expressions.
4040
- Reuse elsewhere
@@ -126,7 +126,7 @@ Preview vs Evaluate
126126

127127
6.1 Registration (soft dependency)
128128

129-
- Providers register metrics in `post_init_hook` only if `openspp.metric.registry` exists.
129+
- Providers register metrics in `post_init_hook` only if `openspp.indicator.registry` exists.
130130
- Example
131131

132132
```
@@ -135,10 +135,10 @@ from odoo import api, SUPERUSER_ID
135135
136136
def post_init_hook(cr, registry):
137137
env = api.Environment(cr, SUPERUSER_ID, {})
138-
if 'openspp.metric.registry' not in env:
138+
if 'openspp.indicator.registry' not in env:
139139
return
140140
from .providers.attendance import AttendanceProvider
141-
env['openspp.metric.registry'].register(
141+
env['openspp.indicator.registry'].register(
142142
name='education.attendance_pct',
143143
handler=AttendanceProvider(),
144144
return_type='number',
@@ -431,12 +431,12 @@ This repository now ships Phase 1 fully and key Phase 2 items:
431431

432432
- New addon `openspp_metrics`
433433

434-
- Feature store model/table: `openspp.feature.value`.
434+
- Feature store model/table: `openspp.indicator.value`.
435435
- Columns now include: `metric`, `provider`, `subject_model`, `subject_id`, `period_key`, `value_json`,
436436
`value_type`, `params_hash`, `coverage`, `as_of`, `fetched_at`, `expires_at`, `source`, `error_code`,
437437
`error_message`, `updated_at`, `company_id`.
438438
- Unique key: `(metric, provider, subject_model, subject_id, period_key, params_hash, company_id)`.
439-
- Registry: `openspp.metric.registry` (Python‑backed) + `register_static()` for deterministic startup.
439+
- Registry: `openspp.indicator.registry` (Python‑backed) + `register_static()` for deterministic startup.
440440
- Service: `openspp.metrics.evaluate()` with cache_only/refresh/fallback, ID mapping chain, and stats
441441
(requested/hits/misses/fresh/coverage).
442442
- HTTP: `POST /api/metrics/push`, `POST /api/metrics/invalidate` (X‑Api‑Key or admin session).

spp_cel_domain/METRICS_SQL_OPTIMIZATION_SPEC_V2.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ Queue details (existing API)
162162

163163
### 5) Provider/params resolution
164164

165-
- Provider label comes from `openspp.metric.registry.get(metric)['provider']` if present; fallback to
165+
- Provider label comes from `openspp.indicator.registry.get(metric)['provider']` if present; fallback to
166166
`metric`.
167167
- `params_hash` should be `""` for CEL V2 unless the CEL translator passes explicit params; keep code ready to
168168
accept it.
@@ -211,7 +211,7 @@ File: `cel_domain/models/cel_executor.py`
211211

212212
2. Add helper `_metric_registry_info(metric) -> (provider, return_type)`
213213

214-
- Query `openspp.metric.registry.get(metric)`; default provider to metric name.
214+
- Query `openspp.indicator.registry.get(metric)`; default provider to metric name.
215215

216216
3. Add helper
217217
`_metric_cache_status_sql(model, base_domain, metric, period_key, provider, params_hash) -> dict`

spp_cel_domain/models/cel_executor.py

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,10 +236,23 @@ def _exec_exists(self, p: ExistsThrough) -> list[int]:
236236
dom = self._and_domains(dom, mem_dom)
237237
if child_subplan is not None:
238238
child_domain, requires_exec_child = self._plan_to_domain(p.child_model, child_subplan)
239+
try:
240+
self._logger.info(
241+
"[CEL EXISTS] child_subplan model=%s plan=%s requires_exec=%s",
242+
getattr(child_subplan, "model", None),
243+
getattr(child_subplan, "domain", None),
244+
requires_exec_child,
245+
)
246+
except Exception:
247+
pass
239248
if requires_exec_child:
240249
child_ids = self._execute_plan(p.child_model, child_subplan)
241250
else:
242251
child_domain = self._ensure_domain_list(child_domain)
252+
try:
253+
self._logger.info("[CEL EXISTS] applying child domain=%s on model=%s", child_domain, p.child_model)
254+
except Exception:
255+
pass
243256
child_ids = self.env[p.child_model].search(child_domain).ids
244257
child_ids = [int(i) for i in child_ids if i]
245258
if not child_ids:
@@ -302,8 +315,28 @@ def _exec_count(self, p: CountThrough) -> list[int]: # noqa: C901
302315
child_ids = self._execute_plan(p.child_model, child_subplan)
303316
else:
304317
child_domain = self._ensure_domain_list(child_domain)
318+
try:
319+
self._logger.info("[CEL EXISTS] applying child domain=%s on model=%s", child_domain, p.child_model)
320+
except Exception:
321+
pass
305322
child_ids = self.env[p.child_model].search(child_domain).ids
306323
child_ids = [int(i) for i in child_ids if i]
324+
try:
325+
sample_labels = []
326+
if child_ids:
327+
records = self.env[p.child_model].browse(child_ids[:5])
328+
sample_labels = [
329+
getattr(rec, "name", None) or getattr(rec, "display_name", None) for rec in records
330+
]
331+
self._logger.info(
332+
"[CEL EXISTS] child filter results child_model=%s domain=%s ids=%s sample=%s",
333+
p.child_model,
334+
child_domain,
335+
child_ids[:10],
336+
sample_labels,
337+
)
338+
except Exception:
339+
pass
307340
if not child_ids:
308341
# No matching children; counts are zero for all candidate parents
309342
if not candidate_parents and parent_model_name:
@@ -386,7 +419,7 @@ def _exec_metric(self, model: str, p: MetricCompare, metrics_info: list[dict[str
386419
# Resolve flags
387420
ICP = self.env["ir.config_parameter"].sudo()
388421
enable_sql = bool(int(ICP.get_param("cel.enable_sql_metrics", "1")))
389-
preview_cache_only = bool(int(ICP.get_param("cel.preview_cache_only", "1")))
422+
preview_cache_only = bool(int(ICP.get_param("cel.preview_cache_only", "0")))
390423
async_threshold = int(ICP.get_param("cel.async_threshold", "50000") or 50000)
391424
allow_any_provider = self._allow_any_provider_fallback()
392425
# Provider resolution
@@ -483,12 +516,17 @@ def _exec_metric(self, model: str, p: MetricCompare, metrics_info: list[dict[str
483516
if total_requested:
484517
stats_total["requested"] = total_requested
485518
stats_total["coverage"] = len(aggregated_values) / float(base_count or 1)
519+
incomplete_cache = eval_mode == "cache_only" and total_requested and len(aggregated_values) < total_requested
486520
if eval_mode == "cache_only":
487521
path_flag = "cache_only"
488522
else:
489523
path_flag = "python" if status.get("status") != "fresh" else "cache"
524+
stats_total.update({"path": path_flag})
525+
if incomplete_cache:
526+
if metrics_info is not None:
527+
metrics_info.append(stats_total)
528+
return []
490529
if metrics_info is not None:
491-
stats_total.update({"path": path_flag})
492530
metrics_info.append(stats_total)
493531
res = []
494532
for sid, val in aggregated_values.items():
@@ -498,7 +536,7 @@ def _exec_metric(self, model: str, p: MetricCompare, metrics_info: list[dict[str
498536
return res
499537

500538
def _metric_registry_info(self, metric: str) -> tuple[str, str]:
501-
info = self.env["openspp.metric.registry"].get(metric) or {}
539+
info = self.env["openspp.indicator.registry"].get(metric) or {}
502540
provider = info.get("provider") or metric
503541
return_type = info.get("return_type") or "json"
504542
return provider, return_type

spp_cel_domain/models/cel_translator.py

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,37 @@ def _smart_op_domain(self, field: str, op: str, right: Any, model_name: str) ->
648648
break
649649
resolved_ids: list[int] = []
650650
if comodel and isinstance(right, str):
651+
if "value" in self.env[comodel]._fields:
652+
direct = (
653+
self.env[comodel]
654+
.with_context(active_test=False)
655+
.search([("value", "=", right.capitalize())], limit=None)
656+
)
657+
if not direct and right.lower() in {"male", "female"}:
658+
code_defaults = {"male": "M", "female": "F"}
659+
direct = (
660+
self.env[comodel]
661+
.with_context(active_test=False)
662+
.sudo()
663+
.create(
664+
{
665+
"value": right.capitalize(),
666+
"code": code_defaults[right.lower()],
667+
}
668+
)
669+
)
670+
if direct:
671+
resolved_ids = direct.ids
672+
if op in ("=", "=="):
673+
if len(resolved_ids) == 1:
674+
return [(field, "=", resolved_ids[0])]
675+
return [(field, "in", resolved_ids)]
676+
if op == "in":
677+
return [(field, "in", resolved_ids)]
678+
if op == "ilike":
679+
base_domain = [(field, "in", resolved_ids)]
680+
return expression.OR([base_domain, [(f"{field}.{label_field}", "ilike", right)]])
681+
651682
name_clauses: list[list[Any]] = []
652683
for attr in ("value", "name", "code"):
653684
if attr in self.env[comodel]._fields:
@@ -658,6 +689,17 @@ def _smart_op_domain(self, field: str, op: str, right: Any, model_name: str) ->
658689
else:
659690
lookup_domain = [("name", "ilike", right)]
660691
matches = self.env[comodel].with_context(active_test=False).search(lookup_domain, limit=None)
692+
try:
693+
_logger.info(
694+
"[CEL TRANSLATOR] smart-op lookup model=%s field=%s op=%s value=%s matches=%s",
695+
model_name,
696+
field,
697+
op,
698+
right,
699+
[(m.id, getattr(m, "value", None), getattr(m, "name", None)) for m in matches],
700+
)
701+
except Exception:
702+
pass
661703
if matches:
662704
target = right.casefold()
663705
exact = matches.filtered(
@@ -666,7 +708,7 @@ def _smart_op_domain(self, field: str, op: str, right: Any, model_name: str) ->
666708
for attr in ("value", "name", "code")
667709
)
668710
)
669-
resolved_ids = exact.ids or []
711+
resolved_ids = exact.ids if exact else []
670712
if resolved_ids:
671713
if op in ("=", "=="):
672714
if len(resolved_ids) == 1:

spp_cel_domain/tests/test_aggregators.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def setUp(self):
1818
M.create({"group": self.g1.id, "individual": self.m2.id, "is_ended": False})
1919
M.create({"group": self.g2.id, "individual": self.m3.id, "is_ended": False})
2020
# Push metric values for September
21-
FV = self.env["openspp.feature.value"]
21+
FV = self.env["openspp.indicator.value"]
2222
# G1 avg = (90 + 100) / 2 = 95, coverage=1.0
2323
FV.sudo().upsert_values(
2424
[

spp_cel_domain/tests/test_cycles.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def _ids(self, expr):
4343

4444
def test_last_first_previous_next(self):
4545
# Push values for each cycle to exercise cycle helpers with metrics
46-
FV = self.env["openspp.feature.value"]
46+
FV = self.env["openspp.indicator.value"]
4747
FV.sudo().upsert_values(
4848
[
4949
{

spp_cel_domain/tests/test_metrics_integration.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ def test_push_and_filter_by_metric(self):
2323
# p3 missing
2424
],
2525
}
26-
self.env["openspp.feature.value"].sudo().upsert_values(
26+
self.env["openspp.indicator.value"].sudo().upsert_values(
2727
[
2828
{
2929
"metric": payload["metric"],
@@ -59,7 +59,7 @@ def compute_batch(self, env, ctx, subject_ids):
5959
counts = {r["group"][0]: r["group_count"] for r in rows if r.get("group")}
6060
return {int(sid): int(counts.get(sid, 0)) for sid in subject_ids}
6161

62-
self.env["openspp.metric.registry"].register(
62+
self.env["openspp.indicator.registry"].register(
6363
name="test_household.size",
6464
handler=_TestHouseholdSizeProvider(),
6565
return_type="number",
@@ -78,7 +78,7 @@ def compute_batch(self, env, ctx, subject_ids):
7878
Membership.create({"group": group.id, "individual": ind2.id, "is_ended": False})
7979

8080
# Seed the value (cache path) to make the test robust across environments
81-
self.env["openspp.feature.value"].sudo().upsert_values(
81+
self.env["openspp.indicator.value"].sudo().upsert_values(
8282
[
8383
{
8484
"metric": "test_household.size",

spp_cel_domain/tests/test_metrics_namespaced.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ def setUp(self):
1111

1212
def test_namespaced_metric_compare_individuals(self):
1313
# Seed cached values without provider (provider-agnostic push)
14-
FV = self.env["openspp.feature.value"]
14+
FV = self.env["openspp.indicator.value"]
1515
FV.sudo().upsert_values(
1616
[
1717
{
@@ -51,7 +51,7 @@ def test_namespaced_metric_aggregator_groups(self):
5151
M = self.env["g2p.group.membership"]
5252
M.create({"group": G.id, "individual": c1.id, "is_ended": False})
5353
M.create({"group": G.id, "individual": c2.id, "is_ended": False})
54-
FV = self.env["openspp.feature.value"]
54+
FV = self.env["openspp.indicator.value"]
5555
# Values 90 and 100 → avg 95
5656
FV.sudo().upsert_values(
5757
[

spp_cel_domain/tests/test_metrics_sql_fastpath.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def __init__(self, test_case):
3232
def compute_batch(self, env, ctx, subject_ids):
3333
return {sid: self._test.seed_values.get(sid) for sid in subject_ids if sid in self._test.seed_values}
3434

35-
self.env["openspp.metric.registry"].register(
35+
self.env["openspp.indicator.registry"].register(
3636
self.metric,
3737
_FakeProvider(self),
3838
return_type="number",
@@ -63,7 +63,7 @@ def _seed_cache(self, rows):
6363
}
6464
for sid, val in rows
6565
]
66-
self.env["openspp.feature.value"].sudo().upsert_values(rows_payload)
66+
self.env["openspp.indicator.value"].sudo().upsert_values(rows_payload)
6767
self.seed_values = {sid: val for sid, val in rows}
6868

6969
def test_sql_fast_path_numeric(self):
@@ -109,7 +109,7 @@ def test_evaluate_large_cohort_enqueues_refresh(self):
109109
plan, _ = Translator.translate("res.partner", expr, cfg)
110110
calls = {}
111111
# Patch enqueue_refresh to capture calls without relying on queue_job runtime
112-
OpensppMetricsService = import_module("odoo.addons.spp_indicators.models.service").OpensppMetricsService
112+
OpensppIndicatorService = import_module("odoo.addons.spp_indicators.models.service").OpensppIndicatorService
113113

114114
def _fake_enqueue(self_, metric, subject_model, subject_ids, period_key, *, chunk_size=2000):
115115
calls["metric"] = metric
@@ -118,7 +118,7 @@ def _fake_enqueue(self_, metric, subject_model, subject_ids, period_key, *, chun
118118
calls["count"] = len(subject_ids)
119119
return 1
120120

121-
with patch.object(OpensppMetricsService, "enqueue_refresh", _fake_enqueue):
121+
with patch.object(OpensppIndicatorService, "enqueue_refresh", _fake_enqueue):
122122
metrics_info = []
123123
_ = (
124124
self.env["cel.executor"]
@@ -195,7 +195,7 @@ def test_preflight_status_transitions(self):
195195
assert status_fresh["status"] == "fresh"
196196

197197
# incomplete: remove one row
198-
self.env["openspp.feature.value"].sudo().search(
198+
self.env["openspp.indicator.value"].sudo().search(
199199
[
200200
("metric", "=", self.metric),
201201
("subject_id", "=", self.p_ko.id),
@@ -228,7 +228,7 @@ def test_small_cohort_sync_refresh(self):
228228
self._seed_cache([(self.p_ok.id, 82), (self.p_ko.id, 84)])
229229
cfg = {"root_model": "res.partner", "base_domain": [("email", "ilike", "fastpath@test.local")]}
230230
expr = f'metric("{self.metric}", me, "{self.period}") >= 80'
231-
FV = self.env["openspp.feature.value"].sudo()
231+
FV = self.env["openspp.indicator.value"].sudo()
232232
assert (
233233
FV.search_count(
234234
[

spp_cel_domain/tests/test_provider_config_overrides.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class _MiniHHProvider:
1818
def compute_batch(self, env, ctx, subject_ids):
1919
return {int(s): 1 for s in subject_ids}
2020

21-
self.env["openspp.metric.registry"].register(
21+
self.env["openspp.indicator.registry"].register(
2222
name="test_household.size",
2323
handler=_MiniHHProvider(),
2424
return_type="number",
@@ -43,7 +43,7 @@ def compute_batch(self, env, ctx, subject_ids):
4343
svc.evaluate("test_household.size", "res.partner", [hh.id], "current", mode="refresh")
4444

4545
# Validate expires_at is close (<< 1 minute), and company_id is set
46-
row = self.env["openspp.feature.value"].search(
46+
row = self.env["openspp.indicator.value"].search(
4747
[
4848
("metric", "=", "test_household.size"),
4949
("subject_model", "=", "res.partner"),

0 commit comments

Comments
 (0)