feat(spp_cel_domain): SQL CASE compiler, read-only smart-op lookup, translator cache tests (re-land from #76)

[gonzalesedwin1123]

Re-lands the spp_cel_domain portion of reverted PR #76 (revert: #271). Everything listed is contained in THIS PR's diff.

Summary

• SQL CASE compiler (to_sql_case, case_when/comparison grammar, ternary right-associativity fix in the CEL parser).
• Smart operator label lookup made read-only (removes a sudo().create() side effect during lookup).
• Tests for the translation cache (_make_cache_key, invalidate_translation_cache) plus test suites for CASE compilation and smart-op lookup.
• tests/__init__.py merges the new imports with the ones added by security PRs feat: SR DCI server/client end-to-end + CEL metric composition fix #223/security(dci): allowlist external predicate symbols (fields + metrics) #251/security(dci): restrict Data API pull to non-sensitive provider variables #257 (nothing overwritten).

Added on top of #76 (not in the original)

• 27 additional edge-path tests for the SQL CASE compiler (null handling, Or/Not connectives, uncompilable-operand fallbacks, age_years operator branches, defensive AST branches) raising codecov patch coverage from 67.8% to 99.3%.
• Version bump + HISTORY entry; README rendered from the updated fragment.

Verification

• ./spp t spp_cel_domain: 648 passed, 0 failed
• codecov: patch 99.3% (pass)

[gemini-code-assist]

Code Review

This pull request introduces the compilation of CEL ternary expressions into SQL CASE WHEN expressions via the new to_sql_case method, supported by new helper methods in SQLBuilder and a right-associative parsing fix in the CEL parser. Additionally, it updates the _smart_op_domain method to perform read-only lookups for gender labels instead of creating records on the fly, and adds comprehensive unit tests covering the new SQL compilation logic, read-only lookups, and translation cache helpers. There are no review comments to assess, and the changes appear solid and well-tested.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[codecov]

Codecov Report

❌ Patch coverage is 99.32886% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 68.90%. Comparing base (bf61488) to head (ce126f7).
⚠️ Report is 1 commits behind head on 19.0.

Files with missing lines	Patch %	Lines
spp_cel_domain/models/cel_translator.py	99.28%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             19.0     #275      +/-   ##
==========================================
- Coverage   74.86%   68.90%   -5.96%     
==========================================
  Files        1093      279     -814     
  Lines       63718    20393   -43325     
==========================================
- Hits        47701    14052   -33649     
+ Misses      16017     6341    -9676     

Flag	Coverage Δ
endpoint_route_handler	?
fastapi	?
spp_aggregation	?
spp_alerts	?
spp_analytics	93.13% <ø> (ø)
spp_api_v2	?
spp_api_v2_change_request	66.41% <ø> (ø)
spp_api_v2_cycles	70.65% <ø> (ø)
spp_api_v2_data	77.43% <ø> (ø)
spp_api_v2_entitlements	69.96% <ø> (ø)
spp_api_v2_gis	71.52% <ø> (ø)
spp_api_v2_products	?
spp_api_v2_programs	92.03% <ø> (ø)
spp_api_v2_service_points	?
spp_api_v2_simulation	71.12% <ø> (ø)
spp_api_v2_vocabulary	?
spp_approval	50.29% <ø> (ø)
spp_area	?
spp_area_hdx	?
spp_attachment_av_scan	?
spp_audit	?
spp_audit_programs	0.00% <ø> (ø)
spp_banking	?
spp_base_common	90.26% <ø> (ø)
spp_base_setting	?
spp_case_base	?
spp_case_cel	89.01% <ø> (ø)
spp_case_demo	?
spp_case_entitlements	97.61% <ø> (ø)
spp_case_graduation	?
spp_case_programs	97.14% <ø> (ø)
spp_case_registry	?
spp_case_session	?
spp_cel_domain	63.77% <99.32%> (+1.54%)	⬆️
spp_cel_event	85.23% <ø> (ø)
spp_cel_registry_search	0.00% <ø> (ø)
spp_cel_vocabulary	?
spp_change_request_v2	?
spp_claim_169	?
spp_cr_type_assign_program	?
spp_cr_types_advanced	?
spp_cr_types_base	?
spp_dci	?
spp_dci_client	?
spp_dci_client_dr	?
spp_dci_client_ibr	?
spp_dci_client_sr	?
spp_dci_compliance	?
spp_dci_demo	?
spp_dci_indicators	?
spp_dci_server	?
spp_dci_server_social	?
spp_demo	?
spp_demo_phl_luzon	?
spp_disability_registry	?
spp_drims	?
spp_drims_sl	?
spp_drims_sl_demo	?
spp_encryption	?
spp_farmer_registry	?
spp_farmer_registry_cr	?
spp_farmer_registry_demo	?
spp_farmer_registry_vocabularies	?
spp_gis	?
spp_gis_indicators	?
spp_gis_report	?
spp_graduation	?
spp_grm	?
spp_grm_case_link	?
spp_grm_demo	?
spp_hazard	?
spp_hazard_programs	?
spp_hxl_area	?
spp_import_match	?
spp_indicator	?
spp_irrigation	?
spp_land_record	?
spp_metric	?
spp_metric_service	?
spp_metrics_core	?
spp_metrics_services	?
spp_mis_demo_v2	?
spp_oauth	?
spp_program_geofence	?
spp_programs	65.27% <ø> (ø)
spp_registrant_gis	?
spp_registry	86.83% <ø> (ø)
spp_registry_group_hierarchy	?
spp_scoring	?
spp_scoring_programs	?
spp_security	66.66% <ø> (ø)
spp_service_points	?
spp_simulation	?
spp_starter_disability_registry	?
spp_starter_farmer_registry	?
spp_starter_social_registry	?
spp_starter_sp_mis	?
spp_statistic	?
spp_storage_backend	?
spp_studio	?
spp_studio_change_requests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
spp_cel_domain/__manifest__.py	0.00% <ø> (ø)
spp_cel_domain/models/cel_sql_builder.py	95.27% <100.00%> (+0.31%)	⬆️
spp_cel_domain/services/cel_parser.py	82.44% <100.00%> (ø)
spp_cel_domain/models/cel_translator.py	46.94% <99.28%> (+11.36%)	⬆️

... and 814 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kneckinator]

Code Review — PR #275: feat(spp_cel_domain) SQL CASE compiler, read-only smart-op lookup, translator cache tests

Reviewed the full gh pr diff plus surrounding source. Overall this is a clean, well-tested re-land. No SQL-injection or security defects found. The findings below are mostly edge-cases, one test-hygiene nit, and one verification gap on the shared parser change.

Overview of what the PR does

• SQL CASE compiler (cel_translator.py): new to_sql_case() + _ast_to_sql_expr() and helpers that compile a CEL expression (esp. ternaries) into an Odoo SQL value expression for use in SELECT columns, as opposed to translate() which builds WHERE-clause domains. Flattens nested ternaries into a single CASE ... WHEN ... ELSE ... END. Handles literals, idents/attrs, comparisons, And/Or/Not, null comparisons, and age_years() date arithmetic. Returns None for anything uncompilable (Python-fallback contract).
• SQLBuilder (cel_sql_builder.py): adds case_when() and comparison() builders with an operator whitelist.
• Parser (cel_parser.py): one-line fix making the ternary right-associative (expr(PRECEDENCE["QUESTION"] - 1)).
• Smart-op lookup (_smart_op_domain): removes a sudo().create() side effect — a missing gender label now resolves to a match-nothing domain [("id", "=", 0)] instead of seeding a vocabulary record.
• Tests: 3 new files (CASE compiler + defensive AST branches + SQLBuilder, smart-op read-only, translator cache), wired into tests/__init__.py.
• Housekeeping: version bump 19.0.2.0.0 → 19.0.2.1.0; changelog synced across HISTORY.md, README.rst, static/description/index.html.

Security — SQL injection & read-only enforcement (clean)

I traced every SQL construction point; no injection vectors:

• Values are parameterized: SQL("%s", node.value) for str/int/float (cel_translator.py:176-178); TRUE/FALSE/NULL are constant tokens.
• Identifiers go through SQL.identifier(alias) / SQL.identifier(field_name) (:199, :394).
• Operators are whitelisted before string interpolation in SQLBuilder.comparison() (cel_sql_builder.py:74-78) and via opmap in _compare_to_sql (cel_translator.py:279, :319) — the SQL("%s " + sql_op + " %s", ...) concatenation is safe because sql_op can only be a whitelisted literal.
• The age_years interval bound is int(cmp.right.value) passed as a parameter (cel_translator.py:356-357), so no injection and no format risk.
• Read-only smart-op: the sudo().create() branch is gone (cel_translator.py:1188-1191); the test asserts create() is never invoked and that a missing label yields match-nothing. Good.

Correctness

Parser ternary fix — correct. With the Pratt loop while lbp(cur) > rbp, the old expr(PRECEDENCE["QUESTION"]) = expr(1) stopped at the next ? (since 1 > 1 is false), producing left-associative (a?x:b)?y:c. The new - 1 = expr(0) lets the next ? bind, giving correct right-associative a ? x : (b ? y : c). Verified against expr() at cel_parser.py:531-539.

age_years SQL — matches the existing WHERE translation. _age_years_compare_to_sql (cel_translator.py:330-381) reproduces the exact operator mapping in _cmp_to_leaf (cel_translator.py:688-716): LT→birthdate > cutoff_n, LE→>= cutoff_n, GT→< cutoff_n, GE→<= cutoff_n, EQ→range (cutoff_{n+1}, cutoff_n], NE→the OR complement. This consistency is exactly what you want (SQL CASE result must agree with the domain filter). Note the LE/GE branches carry the same pre-existing mid-year boundary imprecision as _cmp_to_leaf (someone who is age n but past their birthday can fall outside birthdate >= cutoff_n for age <= n) — this is not a regression, and matching _cmp_to_leaf is the right call; flagging only for awareness.

NULL handling — correct. field == null/null == field → IS NULL, != → IS NOT NULL, and ordering comparisons against null (>, <) correctly return None for Python fallback (cel_translator.py:298-316, tests test_null_ordering_comparison_returns_none).

Minor edge cases (all in dormant code — see downstream section):

1. No field-existence validation in the Ident/Attr → column path (cel_translator.py:198-199, :392-394). A typo'd bare field name compiles to a quoted-but-nonexistent column and would error at query execution rather than returning None for fallback. Can't inject (quoted), just a robustness gap.
2. CASE-branch result-type homogeneity isn't checked, e.g. cond ? "a" : 5 compiles to CASE ... THEN 'a' ELSE 5 END and would raise a Postgres type-mismatch at runtime. Real CEL ternaries are usually homogeneous; low risk.

Test coverage (strong, one hygiene nit)

• Good breadth: happy paths, nested/flattened ternary, all six age_years operators + non-literal/uncompilable-field guards, both null sides, And/Or/Not incl. uncompilable-operand fallbacks, bare-r, list literal, nested attr, plus a defensive-branch class exercising AST shapes the parser can't emit. case_when/comparison unit tests cover valid ops, invalid op, and == normalization. Cache tests cover deterministic/config-sensitive keys, the unsortable-config fallback, roundtrip+invalidate, and FIFO eviction.
• str(SQL) in the assertions works because odoo.tools.sql.SQL has no __str__ and its __repr__ (sql.py:154-155) embeds both code and param reprs — so substrings like "18 years" are present. Fragile against Odoo internals but fine for now.
• Nit (Ken's "clean test output" standard): test_invalid_expression_returns_none feeds "???invalid!!!", which trips the SyntaxError branch in to_sql_case and emits a real _logger.warning("CEL to SQL compilation failed ...") (cel_translator.py:149-151). The warning is neither captured nor asserted. Prefer wrapping it in self.assertLogs(...) so the expected warning is asserted and test output stays clean.

Downstream / in-repo impact

I grepped the repo (ignoring ken/ scratch copies and .claude/worktrees/ which are duplicate trees, not real consumers):

• New public methods have zero callers anywhere — to_sql_case, case_when, and comparison are not invoked outside the new tests, not even inside spp_cel_domain's executor. This is additive/dormant API: the SQL CASE feature is a building block that isn't wired into any execution path yet, so its real-world behavior (the type/field edge cases above) is exercised only by unit tests, never by an integration query. Not a defect, but worth knowing the coverage is white-box only.
• Two subclasses inherit spp.cel.translator: spp_cel_vocabulary/models/cel_vocabulary_translator.py:28 and spp_cel_event/models/cel_event_translator.py:44. Neither (nor spp_studio, spp_approval, spp_case_cel, spp_grm_cel) overrides any changed method (_smart_op_domain, led, _cmp_to_leaf, to_sql_case, case_when, comparison), so they inherit the new behavior transparently. The read-only smart-op change is safe for them.
• The parser change is shared (services/cel_parser.py is used by every CEL module). The ternary right-associativity fix changes how any chained ternary parses across the whole CEL ecosystem. No static source in the downstream modules relies on chained ternaries, but the PR's stated verification only ran ./spp t spp_cel_domain. Recommend running the downstream suites (spp_cel_event, spp_cel_vocabulary, spp_studio, spp_approval) to confirm nothing depended on the old left-associative behavior in data/tests.

Caching correctness (context — impl unchanged by this PR)

The PR only adds tests for the cache; the cache implementation itself is pre-existing. For completeness: _make_cache_key keys on (expression, model, cfg_hash) (cel_translator.py:40-62) and _translation_cache is a process-global module-level dict with no DB/company component. Since a cached plan can embed DB-resolved IDs (e.g. smart-op gender ids), a multi-DB deployment sharing a worker process could in principle serve a plan computed against another database. This is pre-existing, not introduced here — noting it only because the review scope asked about cache staleness. The new tests do call invalidate_translation_cache() in setUp/addCleanup, which is good isolation.

Conventions

• Version bump and changelog are consistent across __manifest__.py, HISTORY.md, README.rst, and static/description/index.html.
• Style, docstrings, @tagged("post_install", "-at_install"), and license headers match the surrounding module. # noqa: C901 used consistently with existing complex methods.

Summary

Solid, safe, well-tested re-land. No blocking issues.

• Should address: capture the expected warning in test_invalid_expression_returns_none (assertLogs) to keep test output clean; run the downstream CEL suites to validate the shared parser change.
• Consider: noting that to_sql_case/case_when/comparison are currently unreferenced (dormant); the type-homogeneity and field-existence edges will matter once a caller exists.
• No change needed: SQL injection posture, read-only smart-op fix, and the parser right-associativity fix are all correct.