feat: CEL ↔ DCI bridge (spp_cel_dci_bridge) + OpenG2P preset (spp_dci_openg2p)#199
Draft
gonzalesedwin1123 wants to merge 68 commits into
Draft
feat: CEL ↔ DCI bridge (spp_cel_dci_bridge) + OpenG2P preset (spp_dci_openg2p)#199gonzalesedwin1123 wants to merge 68 commits into
gonzalesedwin1123 wants to merge 68 commits into
Conversation
Empty module skeleton for the CEL <-> DCI external-fetch bridge described in ADR-023. Installs cleanly; subsequent commits add schema extensions, dispatcher, registry-type handlers, cache-manager override, and tests.
Add dci_data_source_id + is_dci_backed on spp.data.provider, and dci_attribute_path + external_failure_policy on spp.cel.variable. Constraint ensures DCI-backed externals declare an attribute path. Inherit form/list views to expose the new fields. Validates ADR-023 schema additions (additive only, no removals).
spp.cel.dci.dispatcher routes fetch_values_for_variable() by the DCI
data source's registry_type to per-type handlers (DR, CRVS, IBR, SR, FR).
Handlers return {} in this step; subsequent commits implement DR (step 4),
CRVS and IBR (steps 9-10).
Tests verify routing logic, graceful empty returns for missing/inactive
setup, UserError on unknown registry types, and the nested-attribute
path extraction helper used by handlers.
_handler_dr instantiates DRService per subject, extracts the configured
dci_attribute_path from the response payload, and returns {subject_id: value}.
Subjects without DR records, without resolvable identifiers, or that error
during the fetch are omitted (not raised) so a single bad subject can't
fail the batch. Failure policy (step 6) decides what null means.
Tests cover happy path, false values, nested attribute extraction, empty
responses, missing identifiers, multi-subject batches, and per-subject
error tolerance. Uses MagicMock to patch DCIClient — matching the
established mocking pattern in spp_dci_client_dr/tests.
spp_dci_client_dr is declared as a hard dependency for v1; the runtime
ImportError guard remains so future deployments without DR can refactor.
Inherit spp.data.cache.manager and override _compute_variable_values:
when source_type='external' and provider.is_dci_backed, call the
dispatcher; otherwise super(). This fills the documented blind spot in
spp_cel_domain/models/data_evaluator.py:108-116 ("Values must be pushed
via API or scoring run") for the DCI case.
The cycle pre-fetch path (cycle_manager_base._precompute_cycle_cached_variables)
flows through this override unchanged — the rest of the eligibility
plumbing requires no edits.
Tests verify routing, super-fallthrough for non-DCI externals,
non-interference with source_type='field', the end-to-end precompute path
writes to spp.data.value, and dispatcher errors yield {} not raise.
Three policies as defined in ADR-023 §8: - null (default): swallow errors; missing entries leave CEL evaluating against null - last_known: surface most recent non-null spp.data.value row, regardless of expiry - fail: propagate exception as UserError; eligibility check aborts last_known queries spp.data.value sorted by recorded_at desc, takes the first non-null payload per subject, and logs a warning per fallback so operators see what's degraded. Per-subject errors inside the handler loop continue to fall under whichever policy applies; only the wholesale dispatcher exception triggers the fail re-raise in v1. Tests cover all three policies for both wholesale and partial failures, plus the partial-success case where some subjects get live values and others get last-known fallbacks.
New lightweight model spp.dci.fetch.audit records one row per subject per fetch attempt: provider, data source, registry type, variable, subject, outcome (ok/not_found/error), error message, elapsed ms, user. Decided in ADR-023 §6.4 to ship a dedicated model rather than reuse spp.audit.log, which is CRUD-shaped and would require synthetic audit rules to record non-CRUD events. DR handler now wraps each subject fetch with start/stop timing and calls _record_audit with the appropriate result. Audit writes go through sudo so background workers can record regardless of user context; audit failures are caught so they can't poison a fetch. Read access granted to all internal users; write to spp admin only.
`isinstance(True, int)` is True in Python — bool is a subclass of int.
The previous _metric_cmp_supported and _metric_inselect_sql checked int|float
first, routing boolean rhs through the numeric SQL path. That generates:
AND (CASE WHEN jsonb_typeof(...) = 'object' THEN (...)::numeric ... END) = true
which postgres rejects with "operator does not exist: numeric = boolean".
Fix: check isinstance(rhs, bool) BEFORE the int|float branch in both methods,
and emit ::boolean casts plus a boolean rhs comparison. Affects any cached
variable with value_type=boolean queried via `var == true/false` in CEL —
including the new DCI-backed has_disability variable used by spp_cel_dci_bridge.
Exercises the full chain: precompute_cached_variables() -> cache manager override -> dispatcher -> DR handler -> mocked DCIClient -> spp.data.value rows -> CEL service.compile_expression() -> SQL fast path against spp_data_value -> domain that filters to the right partners. The cache manager override now fills missing subjects with explicit None after policy is applied. This keeps the cache complete across the queried cohort, which is what the executor needs to use the metric SQL fast path (have == base) instead of falling back to Python evaluation that requires spp.indicator. CEL semantics line up: a subject with value=null fails `has_disability == true` filtering, which is the right answer when the external registry returned no data. Tests cover the happy path (all subjects have DR records) and the partial-results case (only some subjects matched). Several earlier failure-policy tests had to be updated for the new contract: "missing subject" now appears in the result as None rather than being absent.
…ormalizer CRVS and IBR handlers follow the DR pattern: loop subjects, call the service, extract via dci_attribute_path, record audit. Each tolerates its respective DCI client module being uninstalled via try/ImportError. CRVS's verify_birth(id_type, id_value) needs the partner's identifier resolved first; added _first_identifier helper that reads from reg_ids. IBR's check_duplication(partner) takes a partner directly; IBRService also has a different constructor signature ((data_source, env) instead of (env, data_source_code=...)) — handled in the handler. The three DCI services use inconsistent registry_type strings: DR -> "DR" CRVS -> "ns:org:RegistryType:Civil" IBR -> "ibr" Dispatcher now normalizes via _REGISTRY_TYPE_ALIASES before key lookup so a deployment's source value (URI or short code, any of the legacy shapes) routes to the right handler. Upstream cleanup of the field is tracked separately.
Three data/ records ship the OpenG2P wiring:
- spp.dci.data.source 'openg2p_dr' (DR registry, base_url placeholder,
auth_type='none' — admins configure OAuth2 after install so no
secrets land in source control)
- spp.data.provider 'openg2p_dr' linked to the source
- In-place override of spp_studio.var_has_disability: switches the
semantic 'has_disability' CEL accessor from source_type='field'
(local res.partner.is_person_with_disability) to source_type='external'
routed through the OpenG2P DCI provider, cache_strategy=ttl, ttl=300s
for demo visibility, failure_policy=null
Installing the preset declaratively states "OpenG2P is the authority
for disability status in this deployment." Existing CEL rules that
reference `has_disability == true` continue to work — they now evaluate
against the cached DCI value instead of the local field.
The CEL accessor stays semantic (vendor-neutral per ADR-023 §1a). The
OpenG2P-ness lives only in the data-source and provider records.
Repointing at a different DCI Disability Registry is a configuration
change on the data source, never a CEL change.
Smoke tests confirm the three records exist, are correctly linked,
and the accessor names contain no vendor strings.
- DESCRIPTION/USAGE/CONFIGURE markdown fragments for both modules - Auto-generated README.rst + pyproject.toml + static/description (OCA hook) - ruff-format applied to all changed Python - Add # nosemgrep justification on the audit sudo() call (background workers without spp admin rights must still write audit rows)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 19.0 #199 +/- ##
==========================================
+ Coverage 71.68% 71.89% +0.21%
==========================================
Files 942 980 +38
Lines 55470 56722 +1252
==========================================
+ Hits 39763 40780 +1017
- Misses 15707 15942 +235
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the spp_cel_dci_bridge module, which integrates OpenSPP's CEL expression engine with external DCI registries (DR, CRVS, IBR). It allows CEL variables to fetch and cache external values for use in SQL-based eligibility filters. The PR also includes an OpenG2P preset module and updates the CEL executor to support boolean comparisons in SQL. Review feedback highlights performance concerns regarding historical cache retrieval and audit log creation, suggesting the use of DISTINCT ON and batched database writes to optimize these operations.
_record_audit() escalated to sudo() to write the audit row, which made the user_id field's default lambda resolve self.env.user against the sudoed env — recording every fetch as user_root. This defeated the audit's compliance purpose (we recorded WHAT but lost WHO triggered it). Capture self.env.uid into acting_user_id BEFORE sudo() and pass it explicitly. Audit rows now record the operator who triggered enrollment. Test reworked to drive the dispatcher via .with_user(officer) where officer is a non-admin internal user, then assert the row records that officer rather than user_root.
…urationError
Previously, configuration errors silently degraded into "no one is
eligible":
- spp_dci_client_dr not installed -> ImportError branch returned {}
- _handler_sr / _handler_fr stubs returned {}
- Unknown registry_type raised UserError that the cache manager could
swallow under null policy
_compute_dci_values would then fill every subject with None, the cache
looked fresh, and the eligibility CEL filter silently excluded everyone.
This is the worst kind of compliance bug — silently wrong, indistinguishable
from "no one is disabled."
Introduce DCIConfigurationError (subclass of UserError so existing
catch-blocks still work). Raise it from:
- The three ImportError branches (_handler_dr/_handler_crvs/_handler_ibr)
- The two v1 stub handlers (_handler_sr, _handler_fr)
- The unknown-registry-type dispatch failure
_compute_dci_values now distinguishes configuration errors from runtime
errors: configuration errors propagate unconditionally regardless of
external_failure_policy. Runtime errors (transient network failures,
registry returns 500) continue to follow the policy.
Tests:
- Dispatcher raises DCIConfigurationError for SR/FR/unknown
- Cache manager lets DCIConfigurationError propagate under all three
failure policies (null/last_known/fail)
spp_studio.var_has_disability lacks noupdate=1 in its declaring module, so a future `-u spp_studio` (run as part of any unrelated upgrade) will silently reset the variable back to source_type='field', breaking the demo deployment with no error. The preset's own noupdate=1 only protects against re-applying THIS module's data file, not upstream resets. post_init_hook re-asserts the DCI binding (source_type='external', external_provider_id, dci_attribute_path, ttl, failure_policy) after every install/upgrade. Odoo upgrade ordering guarantees this fires after spp_studio's data files have loaded, so any silent reset gets undone here. Idempotent: when the binding is already correct, the hook short-circuits without writing. Tests: - Simulated spp_studio reset then ran hook: binding restored - Idempotency check: hook on clean state is a no-op
action_dci_fetch_audit was previously unreachable from any menu. Two entries because two operator personas need the log: - DCI > Activity Logs > DCI Fetch Audit (DCI ops persona) - CEL Domain > Data Management > DCI Fetch Audit (CEL ops persona) Both gated to spp_admin since audit data is sensitive.
…n provider form Previously inserted dci_data_source_id inside the parent "Connection" group on the data provider form, right next to base_url and auth_type. This was confusing: when DCI routing is active, those parent fields are runtime-ignored (the linked DCI Data Source has its own URL and auth), but the form let operators edit them as if they mattered. New layout: - New "DCI Integration" notebook page (first, before "Authentication") - Hosts dci_data_source_id with no_create/no_quick_create options - Info alert appears when is_dci_backed, explaining that the legacy Base URL/Auth fields are ignored at runtime - Legacy base_url and auth_type become readonly when DCI-backed
Previously, dci_attribute_path and external_failure_policy showed for any external-source variable, including non-DCI providers (REST APIs, scoring services) where they have no meaning. Add external_provider_is_dci_backed as a related field on spp.cel.variable so the view can gate visibility through the provider's flag. Tighten both fields' invisible= conditions to require it. Also: mark dci_attribute_path required= when the provider is DCI-backed. Previously the constraint at _check_dci_attribute_path() raised ValidationError only on save; now the form renders the red asterisk and Odoo's client-side check catches it before save.
The audit list displayed subject_id as a bare integer with no resolution to the partner. Compliance reviewers looking at "subject_id 4271" had no way to trace it back to a person. Add subject_ref as a computed Reference field that resolves (subject_model, subject_id) to a partner record. Not stored — falsy when the partner has been deleted since the fetch — but the immutable subject_id snapshot is preserved as the historical truth. The list shows subject_ref by default; subject_id is hidden in the list (toggle-able) and remains the canonical search field for historical investigations. Tests: - subject_ref resolves to the current partner record - subject_ref is False when subject_id points to a missing partner, but the integer subject_id is preserved
_augment_with_last_known previously Python-filtered a search() result
to pick the latest non-null row per subject. That works at demo scale
but is O(history × cohort) — a deployment with daily TTL refresh over
6 months × 1k subjects fetches ~180k rows just to surface the latest
1k. Reported by gemini-code-assist on the PR.
Replace with a single SQL query using DISTINCT ON (subject_id)
ORDER BY subject_id, recorded_at DESC, id DESC. JSON null is filtered
at the SQL layer so historical {"value": null} rows aren't surfaced
as "last known."
Behavior is unchanged for the existing tests (single row per subject,
no rows, null rows). Added a new test that exercises multiple history
rows per subject to lock in the "most recent wins" semantic that the
prior Python loop also produced.
Defer Gemini's other suggestion (batch the per-subject audit create()
calls) to a separate follow-up — the durability tradeoff vs. throughput
deserves its own design discussion.
…om CI Three issues caught by CI that the local --files run had missed because it didn't exercise --all-files semantics: 1. oca-checks-odoo-module: spp_dci_openg2p/security/ir.model.access.csv was empty (preset module defines no custom models, no ACL needed). Removed the file and the now-empty security/ directory rather than adding it to the manifest's data list — there's nothing to ACL. 2. ruff E501: dci_fetch_audit.py:50 subject_ref help= string was 189 chars on one line. Split into a parenthesised multi-line string. 3. ruff-format: tests/test_failure_policy.py and models/data_cache_manager.py carried minor formatting drift that --files didn't catch. Reformatted. All 56 spp_cel_dci_bridge tests and 6 spp_dci_openg2p tests still pass.
…/safety paths Codecov flagged 41 missing lines on the PR. Most of the gap is in parallel-structure code paths that DR has tests for but CRVS and IBR don't. Added focused tests for: CRVS handler: - Per-subject exception swallow + audit error row - Empty registry response -> not_found audit - Successful response with missing dci_attribute_path -> not_found IBR handler (same shape, patches check_duplication directly since the inner search_by_id is swallowed by the IBR service itself): - Per-subject exception - Missing attribute path Dispatcher: - _record_audit's outer try/except: if the audit write itself fails, the fetch must still complete and return values post_init_hook safety branches (spp_dci_openg2p): - spp_studio.var_has_disability missing -> log warning, no raise - openg2p_dr_provider missing -> log error, no raise CEL executor boolean SQL: - `has_disability != true` exercises the boolean `!=` operator in the metric SQL fast path (symmetric with `==` but a separate emission branch in _metric_inselect_sql) Test count: 56 -> 62 on bridge, 6 -> 8 on preset. Total 70 tests. The remaining uncovered lines are the two __manifest__.py files (Odoo manifests are evaluated as dicts at module discovery, not imported as Python — coverage cannot reach them) plus a handful of branches in edge-case I/O paths that aren't worth dedicated tests.
…olicy pylint_odoo W8113 (attribute-string-redundant). I missed this one during the earlier sweep (caught it only on external_provider_is_dci_backed). Auto-label from the field name is functionally equivalent.
Three tests imported `patch` inside the method body even though it was already imported at the top of the file (or trivially could be): - test_audit_logging.py:135 - test_audit_write_failure_does_not_break_fetch - test_crvs_ibr_handlers.py:174 - test_ibr_handler_swallows_per_subject_error - test_install.py:95,107 - the two new safety-branch tests Promote `patch` to a top-level import in test_install.py and remove the inner reimports. Behaviour identical; pylint_odoo W0404 cleared.
Live test against partner-registry.play.openg2p.org revealed two protocol
quirks vs. upstream spp_dci_client:
1. idtype-value query shape: OpenG2P expects nested
{type: "idtype-value", value: {id_type, id_value}} but upstream emits
{type: <id_type>, value: <id_value>}. Server rejects upstream form
with rjct.search_criteria.invalid.
2. Required reg_record_type on search_criteria, omitted by upstream's
SearchCriteria Pydantic model.
This adapter (ADR-023 §6 Option C path) absorbs both:
- spp.dci.data.source gets a `vendor` Selection field; preset's data
source ships with vendor='openg2p'.
- OpenG2PDCIClient subclasses DCIClient and overrides _parse_query
(nested shape) + _build_search_envelope (inject reg_record_type,
re-sign).
- OpenG2PFRService mirrors DRService's surface but queries the FR
registry (reg_type=Social, reg_record_type=Farmer) and unwraps
data.reg_records[] to extract the first record.
- Bridge dispatcher inherits and overrides _handler_dr: when source
has vendor='openg2p', route to OpenG2PFRService instead of DRService.
FR-as-DR pretense for the demo: presence of any farmer record for
a partner -> has_disability=True. CEL surface stays `has_disability == true`.
The demo audience sees a real DCI round-trip; behind the scenes we're
querying the Farmer Registry because OpenG2P hasn't published their
Disability Registry yet.
Migration plan (in readme/CONFIGURE.md) when real DR arrives: clear the
`vendor` field on the data source. Bridge falls back to upstream
DRService. No code or CEL rule changes.
Tests: 25 in preset (was 8), 62 in bridge unchanged. Covers query
shape regression, reg_record_type injection, response unwrap, vendor
routing, fallback when vendor is cleared.
OpenG2PFRService.IDENTIFIER_PRIORITY = ("UIN", "DRN", "NATIONAL_ID", "NID")
expects matching vocabulary codes on the urn:openspp:vocab:id-type
vocabulary so the registrant form's Identity tab can pick them as
ID Type. spp_vocabulary ships only lowercase generic codes (national_id,
passport, ...), so without this seed an operator could not pick UIN
when adding a reg_id to a test partner — the dispatcher would fall back
through the priority list and find nothing.
Adds spp_dci_openg2p/data/openg2p_id_types.xml seeding `UIN` (uppercase
to match SPDCI wire convention). Vocabulary is referenced via
spp_vocabulary.vocab_id_type — adds spp_vocabulary to module deps.
Test updates: two test classes used to create a fresh `UIN` code in
setUpClass; that now collides with the preset's seed. Switched both to
env.ref('spp_dci_openg2p.id_type_uin'). New tests assert the seed exists
and matches the service's IDENTIFIER_PRIORITY first entry.
If OpenG2P later returns records under DRN / NATIONAL_ID / NID, follow
the same pattern in openg2p_id_types.xml.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… var
Verified against partner-nsr.play.openg2p.org on 2026-05-15: OpenG2P's
SR reg_record exposes neither is_poor nor has_dependent_under_school_age
as top-level fields. The closest poverty signal is `income_level`
(string: "low" / "medium" / "high"); no signal for under-school-age
dependents exists at all.
Changes:
- var_is_poor: value_type boolean -> string; dci_attribute_path
is_poor -> income_level. CEL rules now read `is_poor == "low"` rather
than `== true`. Variable name kept semantic.
- var_has_dependent_under_school_age: parked state=inactive,
active=False. Record stays registered as a deferred-feature
placeholder so revival is a config-only change once OpenG2P exposes
the data (or once we wire a secondary household-search call).
Implementation:
- post_init_hook's _PRESET_VARIABLES table extended to carry per-variable
value_type and state. Hook now re-asserts both the active is_poor
binding AND the inactive placeholder on every -i/-u, preventing UI
edits from silently re-activating the deferred variable.
- _EXPECTED_BINDING_FIELDS gains value_type so type drift is caught.
- Tests updated: test_var_is_poor_bound_to_dci_provider asserts the new
string/income_level pair; test_var_has_dependent_under_school_age_
parked_inactive replaces the prior active-state assertion;
test_post_init_hook_parks_deferred_variable_inactive locks the
hook's drag-back-to-inactive behaviour.
- Dispatcher routing test now mocks {"income_level": "low", ...} and
asserts the dispatcher surfaces the raw string to CEL.
Docs: CONFIGURE.md gains a "Deferred features" table documenting why
has_dependent_under_school_age is inactive and the path to revive it.
…tation Audits ADR-023, ADR-024, the federated demo plan, and every module's readme fragments against the as-shipped code. Corrects stale facts and adds operational guidance discovered during end-to-end demo wiring on 2026-05-15. Key corrections: - ADR statuses: Proposed -> Accepted on both ADR-023 (bridge shipped) and ADR-024 (federated demo wired end-to-end). ADR-024 gains a resolution block for the original Open Items: is_poor binds to income_level (string compare); has_dependent_under_school_age is permanently deferred (no field on OpenG2P's per-individual record). - Federated demo plan: prepended a "Post-shipment deltas" section capturing every place the body drifted from implementation — OpenG2P host (partner-nsr vs partner-registry), DR endpoint path (/dci_api/v1 prefix), DR-side dev-mode flags, real DR field names, UIN seed ownership, standalone DR docker-compose project. - spp_dci_server_disability DESCRIPTION/CONFIGURE: replaced the fabricated wire-format JSON sample and Disability-fields table. Actual fields read are has_disability (Boolean from assessment chain), disability_severity_code, disability_review_category, disability_next_review — NOT is_person_with_disability / disability_certified / disability_percentage. Added the sudo() rationale and the dev-mode flag table. - spp_dci_openspp_dr CONFIGURE: documented both required dev-mode flags (dci.allow_unsigned_requests + dci.bypass_bearer_auth) and the UIN-seed dependency on spp_dci_openg2p (no longer ships its own UIN). - spp_dci_openg2p CONFIGURE: called out the partner-nsr host override that operators must apply manually (noupdate=1 means the XML default cannot be rewritten on upgrade). Added vendor field selection guidance. - spp_cel_dci_bridge DESCRIPTION/USAGE: documented the hoisted vendor field, the dci_data_source_views.xml, and the eager pre-warm behaviour with the inactive-variable opt-out. - spp_cel_dci_bridge/readme/USAGE.md: cel code-fence retitled to plain text (Pygments has no cel lexer; was blocking README.rst regeneration). Operator-facing example uses CEL && operator. Regenerated README.rst and static/description/index.html for every preset whose readme fragment changed (oca-gen-addon-readme).
Matches the whool-based packaging stub every other spp_* module ships. Auto-generated; included to keep the new modules consistent with the rest of the repo's Python-packaging convention.
Creates 4 demo personas (Maria Widow, Kim Lee, Priya Rivera, Noah Rivera)
on both sides of the federated topology. UINs match real OpenG2P SR
seeds (IND-NSR-0001/0002/0003/0007) so the SP-side is_poor lookup
returns a live income_level. DR side gets approved disability
assessments for the two flagged personas.
NOT A MODULE. Production module installs create zero registrants —
this is an out-of-band seed script run ONLY before a demo. Designed to
be deleted along with its partner records after the demo.
Usage:
docker compose exec openspp-dev odoo shell -d openspp --no-http \
< scripts/demo/setup_federated_demo.py
docker compose -f docker-compose.dr.yml exec openspp-dr \
odoo shell -d openspp_dr --no-http \
< scripts/demo/setup_federated_demo.py
Idempotent on rerun. Cleanup recipe included in the script docstring.
Was creating 4 partners; OpenG2P has 15 seeded records (IND-NSR-0001.. IND-NSR-0015). Wiring every UIN gives the demo a complete eligibility matrix: - 4 partners ENROLLED (poor + disabled across both registries) - 3 partners fail has_disability (poor on SR, no DR assessment) - 4 partners fail is_poor (disabled on DR, non-low income on SR) - 4 partners fail both Names mirror OpenG2P's actual seed values (Alex Rivera, Morgan Cole, Taylor Brooks, etc.) so the federation story stays honest — an SP audit row tagged "Alex Rivera" matches what OpenG2P returns on probe. Cleanup recipe updated to iterate the full 15-UIN range.
Two operational tweaks for tonight's dry-run: 1. RENAME instead of skip when a UIN reg_id already exists on the partner. Edwin's existing IND-NSR-0001 partner is already attached to programs (memberships, change requests) so unlink would orphan that state. Writing name/given_name/family_name in-place rebrands the existing record while preserving its FK relationships. 2. SP-side only: add every demo partner as a draft membership of spp.program record id=DEMO_PROGRAM_ID (default 1). This lets the operator demo Enroll Eligible directly — no need to walk through the change-request flow to register members first (a colleague demonstrates that on a separate instance). Memberships start in state='draft'; eligibility evaluation flips them based on the CEL rule. DEMO_PROGRAM_ID is a named constant at the top of the script so operators with a non-1 program id can override before running.
Resets the 15 demo memberships on program id=1 back to state='draft' and wipes the DCI value cache for the demo partners. Lets the operator re-run Enroll Eligible multiple times during a presentation without manually resetting each membership through the UI. WIPE_DCI_CACHE constant at the top toggles whether the script also flushes the cache (default True). With cache wiped, the next eligibility click fires fresh DCI queries to OpenG2P SR and OpenSPP-DR — useful when the demo audience should see the live round-trip in the SP log. Same scripts/demo/ pattern as setup_spdci_demo.py — out-of-band, not a module, not shipped to production.
…ions
The CEL executor's top-level compile_and_preview took only the FIRST
override_domain from metrics_info, breaking compound expressions of the
form `metric('a', me) == X and metric('b', me) == Y`. Each MetricCompare's
SQL fast path successfully built its subquery clause and pushed it to
metrics_info, but only the first was used in the final domain — silently
dropping the AND'd second clause.
Symptom in tonight's SPDCI dry-run: rule
`has_disability == true && is_poor == "low"` evaluated as just
`has_disability == true`, so 8 partners enrolled (all with DR
assessments) instead of the expected 4 (intersection with low-income).
Fix: collect ALL override_domains from metrics_info and AND them on
final_domain. Single-metric case is unchanged (one clause is identical
to "take the first").
Known limitation: an OR of metric clauses still mis-composes — the per-
clause SQL subqueries cannot be UNION'd through Odoo domain syntax, so
ids materialization in _exec_metric would be needed for correctness.
Not addressed here; OR-of-metrics is uncommon in eligibility rules and
not in scope for the federated demo.
573 spp_cel_domain tests + 67 spp_cel_dci_bridge tests still pass.
Single-page reference covering: - The 15-persona demo matrix with each registrant's UIN, OpenG2P income_level, OpenSPP-DR has_disability, and expected eligibility verdict — the four ENROLLED rows are highlighted. - ASCII topology diagram showing SP, DR, and OpenG2P SR with the DCI search-sync flows between them. - Glossary of every acronym used in the demo (SPDCI, DCI, search-sync, SR, DR, CRVS, IBR, FR, CEL, CEL accessor, metric() call, spp.dci.data.source / data.provider / dci.dispatcher / data.value / dci.fetch.audit, vendor adapter, MOSIP, eSignet, OpenG2P). - Cross-references to ADRs and demo scripts. Located alongside the seed/reset scripts in scripts/demo/ so the operator's presentation kit is in one place.
Sweep of all touched files against the project's pre-commit suite: - ruff-format + prettier formatting (consistent line-length and multi-line wrapping across services, tests, demo scripts, view XMLs). - C8107 translation-required: wrap user-facing UserError / ValidationError messages in self.env._() so they participate in Odoo's translation pipeline. Applied in OpenG2PSocialService and OpenSPPDRService. - Updated the stale top-of-file docstring on spp_dci_server_disability/services/disability_search_service.py to describe the actual fields read (has_disability boolean from the approved assessment + severity/review-category/next-review) instead of the deleted is_person_with_disability/disability_certified/ disability_percentage names. - Repositioned the sudo() nosemgrep markers in disability_search_service._find_partner_by_identifier so the odoo-sudo-on-sensitive-models rule also recognizes them. Added an inline authorization-context comment block explaining why sudo() is the right call here (upstream signature/bearer middleware is the real auth boundary; the service surface is read-only). All 705 tests across the five touched modules still pass (spp_cel_domain 573, spp_cel_dci_bridge 67, spp_dci_openg2p 34, spp_dci_openspp_dr 20, spp_dci_server_disability 11). Pre-commit overall returns exit 0.
The previous nosemgrep marker landed on the closing-paren line after ruff-format wrapped the return statement across three lines, so semgrep's per-line suppression didn't apply to line 236 where the actual self.env["res.partner"].sudo() call lives. CI semgrep flagged both odoo-sudo-on-sensitive-models (critical) and odoo-sudo-without-context (warning) on this line. Fix: extract the sudoed env reference to a named local on the same line as its nosemgrep marker; the .browse() call moves to its own unmarked statement (no sudo() on it). Functionally identical; this is purely about the marker placement.
Three classes of fixes:
1. scripts/lint/check_naming.py: anchor the deprecated g2p_/g2p. import
rule to a path-segment boundary so it doesn't false-positive against
`openg2p_*` (the OpenG2P platform's distinct namespace). Previous
substring check ("g2p_" in module_name) flagged
`.openg2p_dci_client` as deprecated even though it's the local
module in spp_dci_openg2p.
2. scripts/demo/{setup,reset}_spdci_demo.py: add file-scope linter
directives. These are interactive Odoo-shell scripts — env is
injected at runtime (so ruff's F821 is wrong), print() is the
right output channel for an operator at a REPL (so pylint_odoo's
W8116 is wrong), and the summary loop in setup intentionally
unpacks the full row (B007). Directives:
ruff: noqa: F821, B007
pylint: disable=print-used
CI was failing on:
- Semgrep OSS (already fixed in 166155e — nosemgrep marker
placement on the actual sudo() line)
- pre-commit's full-repo run flagging this branch's check_naming.py
false positive and the demo scripts' linter mismatches
Local `pre-commit run --all-files` now returns exit 0. All 705 tests
across the five touched modules still pass.
Captures every IND-NSR-0001..IND-NSR-0015 record as returned by
partner-nsr.play.openg2p.org on 2026-05-15. Columns include:
- identity: uin, given_name, surname, sex, birth_date
- SR data: marital_status, employment_status, occupation,
income_level, education_level
- disability: openg2p_is_disabled, openg2p_self_id_disability
(informational — the demo's has_disability comes from
the OpenSPP-DR, not OpenG2P)
- household: household_id, relationship_to_head,
citizenship_category, displacement_status
- demo: demo_dr_has_disability (will the script approve a DR
assessment for this UIN?), demo_expected_verdict
(eligibility outcome under
has_disability == true && is_poor == "low")
Useful for the SPDCI presentation — drop into a spreadsheet or
slide table to show the source-of-truth data and the expected
demo outcomes.
Keeps uin, given_name, surname, sex, birth_date, marital_status, employment_status, occupation — the presentation-ready subset. The SR-specific (income_level, education_level), disability, household, and demo-annotation columns are documented elsewhere in SPDCI_DEMO_BRIEFING.md and don't need to clutter the audience-facing CSV.
End-to-end technical narrative for explaining how
`has_disability == true && is_poor == "low"` triggers two federated
DCI calls and composes the eligibility decision.
Eleven steps, code-path references at each layer:
1. Enroll Eligible button -> eligibility manager
2. Pre-warm fans out every active DCI-backed CEL variable
3. Dispatcher routes by registry_type + vendor adapter
4. Vendor service builds and POSTs a DCI envelope
5. Remote registry answers (OpenSPP-DR or OpenG2P SR)
6. Service unwraps reg_records[0], dispatcher extracts via
dci_attribute_path, audit row written
7. Cache write to spp.data.value
8. CEL parser -> translator -> plan (AND[MetricCompare, MetricCompare])
9. Executor builds per-clause SQL subqueries
10. PostgreSQL composes the final WHERE
11. Memberships flip to enrolled/not_eligible
Plus ASCII overview diagram, wire-format JSON envelope snippets for
both DR and SR sides, a "why this matters for SPDCI" framing block,
and the demo verification commands.
…trant ingest
Replaces the manual setup_spdci_demo.py script with a UI flow under
Registry → Import from External Registry (DCI). The wizard fires
DCI search-sync requests against the configured OpenG2P SR data
source, previews matched records, and imports the selected ones as
res.partner + spp.registry.id rows on the SP — optionally enrolling
them into a target program in one step.
Discovery semantics: SPDCI search-sync is lookup-only (no "list all
registrants" operation), so the wizard offers two practical modes:
- Range sweep: contiguous identifier range (e.g., IND-NSR-0001..
IND-NSR-0015). Works against the OpenG2P playground.
- Identifier list: paste/type identifiers one per line. Matches the
production-shaped workflow where the SR operator hands over a
partner list out of band.
Scope: captures only the bare minimum partner fields (given_name,
surname, sex, birth_date) plus a UIN reg_id. Eligibility rules
continue to read income_level etc. on demand via the CEL ↔ DCI
bridge — this wizard is NOT a full SR replica.
Implementation:
- spp.dci.sr.import.wizard (TransientModel) with three-state form
(configure → preview → done)
- spp.dci.sr.import.wizard.line for preview rows; carries
already_exists + existing_partner_id so the operator can see
which UINs are already on the SP
- View renders all states on one form, gated by `invisible="state
!= 'X'"` per pane
- Menuitem under spp_registry.spp_main_menu_root, sequence=90
Tests:
- 10 new tests in test_sr_import_wizard.py covering identifier
collection (range padding, list dedup/comments, empty-input
validation), preview (matched/not_found/error/already_exists),
import (selected only, skip-existing, auto-enroll), and
back-to-configure state reset.
- Module total: 44 tests passing.
Module manifest gains spp_registry as a dependency for the menu
parent xmlid; security/ir.model.access.csv added for the two
TransientModel records.
… to "Social Registry" Operators see neutral terminology in the SR-import wizard and the DCI configuration screens; vendor identity stays in technical slugs (xml ids, vendor='openg2p' dispatcher key, code='openg2p_dr', Python class names) so the routing layer is unaffected. Touched UI strings only: data source / data provider names, CEL variable labels and descriptions, UIN id-type definition, wizard form title, and the Source Registry field's help tooltip.
…zard The wizard's data_source_id domain filtered on the short alias 'SR', which only exists in the dispatcher's URI-to-alias routing map — not as a stored field value. spp.dci.data.source.registry_type is a Selection over URIs from spp_dci.schemas.constants.RegistryType, so the filter never matched and the Source Registry dropdown rendered empty. Switch the domain, the _default_data_source search, and the seeded data XML to 'ns:org:RegistryType:Social' so the dropdown picks up the Social Registry source on fresh installs and the operator sees the configured source pre-filled.
Captures the leaf modules to install on each side (SP: spp_dci_openspp_dr + spp_dci_openg2p; DR: spp_dci_server_disability), the dependency tree they pull in, the full down/wipe/re-init flow, post-install wiring (DR base_url, demo seeding, optional auth bypass), and a sanity-check table — so the federated demo can be rebuilt from scratch without rediscovering the steps.
The next demo run wipes only the SP; the DR keeps its database, its 8 approved disability assessments, and its DCI-server bypass flags from prior runs. Rewrite the reset section to stop/wipe only the SP container + filestore, leave docker-compose.dr.yml alone, drop the DR-side reseed step from the post-install wiring, and note explicitly that the DR config carries over.
So an operator running both the SP and the DR side-by-side during the SPDCI demo can tell the two OpenSPP UIs apart at a glance — SP keeps the default OpenSPP blue from spp_base_common, the DR shows Material green-800 in the top navbar and green-900 in the apps-sidebar hover and active states. The SCSS overrides ship with the DR-only module, so no styling bleeds onto SP installs.
Presenter-oriented runbook that ties each click and screen in the live SPDCI federated-eligibility demo to its talking points and expected audience reaction. Includes a pre-demo state table, ten scene breakdowns (frame → empty SP → SR import → CEL rule → enroll → audit drill-down → live SR verify → failure-mode recap → architecture → roadmap), a 5-minute lightning cut, pre-canned Q&A anchors (including the marital_status filtering question we probed today), and a pre-flight checklist so the host can verify environment health before the audience walks in. Complements the existing briefing sheet (narrative + personas + glossary), CEL-to-DCI internals walkthrough, and modules/reset reference.
…theme
The first pass missed three places where the OpenSPP blue still leaked
through on the DR's backend:
- .mk_apps_sidebar_panel (sidebar base background — muk_web_appsbar
paints this from $mk-appbar-background, which spp_base_common
overrides to brand blue).
- .o_main_navbar .o_menu_sections .o_nav_entry /
.o_main_navbar .o_menu_sections .dropdown-toggle (per-section
menu-entry backgrounds — theme_openspp_muk uses descendant
selectors that the existing direct-child override didn't match).
- .btn-primary / .btn-primary:hover (action buttons).
After the fix, the DR backend is green throughout the chrome operators
look at during the demo while the SP stays on default OpenSPP blue.
Leftover from commit 50c9b59 which moved the data source's registry_type to the URI form 'ns:org:RegistryType:Social'. Two tests still asserted the short alias 'SR' and started failing once the DR-side install pulled the dispatcher routing test set in. Update both to assert the canonical URI.
… install Operators running a fresh DR deploy used to hit a 401 on the SP's first lookup because the bearer-token middleware enforces auth by default, and the parameter keys are easy to mistype in the System Parameters UI. Add a post_init_hook that creates `dci.bypass_bearer_auth=true` and `dci.allow_unsigned_requests=true` on initial install only, with a loud WARNING in the boot log so the demo-mode bypass is obvious. Existing values are respected on upgrade — operators who later flip either flag to 'false' for production keep their choice.
…aft flow
Replaces the SR-import wizard's mirror-to-DR path with a DCI-native
write operation, and lets the SR's self-reported disability claim
surface to the DR's assessor backlog as draft assessments without
shortcutting the clinical workflow.
--- DCI register-individual endpoint (DR side) ---
spp_dci_server_disability:
- schemas.py: RegisterIndividualItem / RegisterRequest / RegisterResponse(Item).
Custom action name 'register-individual' since the standard DCI MessageAction
enum has no generic 'create-individual' operation; envelope shape mirrors
the search side (transaction_id + per-item reference_id + per-item status).
- services/disability_register_service.py: idempotent upsert by UIN reg_id.
New UIN -> create partner + reg_id ('created'). Existing UIN, refresh=False
-> 'skipped' (partner untouched). Existing UIN, refresh=True -> partner
identity rewritten ('updated'). Failures surface as per-item rjct rows
rather than rolling back the whole transaction.
- routers/disability_router.py: POST /sync/register endpoint, same signature +
bearer middleware as /sync/search, response envelope follows the same
'succ' / 'rjct' / 'part' overall-status convention.
- Manifest: depends on spp_disability_registry (the register service now
creates spp.disability.assessment records).
--- SR self-report -> draft DR assessment ---
spp_dci_server_disability:
- schemas: RegisterIndividualItem.is_disabled bool, RegisterResponseItem
.draft_assessment_created bool.
- service: when is_disabled=true AND the registrant has no prior assessment
(any state), create a DRAFT spp.disability.assessment with WG fields LEFT
BLANK and post an SR-provenance chatter message. The clinical separation
holds — has_disability stays false until an assessor conducts the
Washington Group interview and approves. Idempotent: re-runs don't pile
on additional drafts behind the assessor's back.
--- DCI client + wizard refactor (SP side) ---
spp_dci_openspp_dr:
- openspp_dr_service.py: new register_individuals() method using
DCIClient._build_envelope('register-individual', ...) and ._make_request().
Coerces Odoo's False-for-empty-Char convention to Pydantic-friendly None
on the wire.
spp_dci_openg2p:
- wizards/sr_import_wizard.py: drop the four XML-RPC fields (URL / db /
user / password), add a single dr_data_source_id Many2one to the same DR
DCI source the bridge already uses for has_disability lookups. mirror_to_dr
collects items across the SP loop and fires ONE batched DCI envelope at
the end. Forwards sr_is_disabled from the SR payload onto the wire.
Surfaces DR per-item counts and draft-assessment count in the done summary.
- views/sr_import_wizard_views.xml: replace XML-RPC fields with the single
DCI source Many2one; new "SR claim" column in the preview grid.
- Manifest: depends on spp_dci_openspp_dr (the wizard's mirror path now
calls OpenSPPDRService).
--- Tests ---
spp_dci_server_disability: 7 new tests covering upsert semantics (new /
exist / refresh on-off) AND draft-creation semantics (is_disabled true/false
combined with prior-assessment yes/no, including the existing-partner-but-no-
prior-assessment edge case).
spp_dci_openg2p: mirror tests rewritten to mock OpenSPPDRService.register_
individuals instead of xmlrpc.client.ServerProxy. New test covers the
refresh-flag plumbing through the wire.
Why a custom action vs. SPDCI standard: DCI has no generic 'create-individual'
operation today (ENROLLMENT is program-enrollment, not registrant onboarding).
The action name and envelope shape stay close enough to the search side that
the same middleware, signing, audit, and dispatcher infrastructure works
unchanged. Same trust boundary as the read side — no XML-RPC admin
credentials on the SP, no second auth surface to manage.
| set either parameter to 'false' (or anything else), upgrading the module | ||
| leaves their choice alone. Only first installs get the demo defaults. | ||
| """ | ||
| Param = env["ir.config_parameter"].sudo() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enables CEL eligibility rules of the form
has_disability == trueto fetchvalues from external DCI registries (OpenG2P or any compliant DR), cache
them in
spp.data.value, and resolve via the existing CEL metric SQL fastpath during program enrollment. See ADR-023 (in
.claude-shared/docs/architecture/decisions/)for the full design.
Two new modules:
spp_cel_dci_bridge— registry-type-agnostic infrastructure. Overridesspp.data.cache.manager._compute_variable_valuesto route DCI-backedexternal CEL variables through a dispatcher that picks the right DCI
service (DR / CRVS / IBR) by
registry_type. Schema extensions onspp.data.provider(dci_data_source_id) andspp.cel.variable(
dci_attribute_path,external_failure_policy). New audit modelspp.dci.fetch.auditrecords one row per subject per fetch.spp_dci_openg2p— permanent OpenG2P vendor preset. Config-only inv1 (no Python). Ships three
data/records: DCI data source, CEL dataprovider, and an in-place override of
spp_studio.var_has_disabilitythat repoints the semantic
has_disabilityaccessor at the OpenG2Pprovider. CEL accessor name stays vendor-neutral (ADR-023 §1a).
One upstream fix bundled in for unblock:
spp_cel_domain.cel_executorhandles boolean RHS in the metric SQL fast path (previously
True/Falsewent through the numeric branch because
boolis a subclass ofint,producing broken SQL
numeric = boolean).Architecture (demo flow)
Test plan
./scripts/test_single_module.sh spp_cel_dci_bridge— 47/47 passing./scripts/test_single_module.sh spp_dci_openg2p— 4/4 passingpre-commit run --files— clean (ruff/format/semgrep/bandit/pylint)Notable design choices
has_disability == true), not function call_compute_variable_valuesis the single integration pointspp.dci.fetch.auditmodel, notspp.audit.logreusefailpolicy opts in_demo, not_client_)spp_studio.var_has_disabilityin place rather than creating a parallel variable (single source-of-truth for the semantic accessor)Out of scope (tracked in ADR-023 §"Future Work")
Files
git log 19.0..HEAD --onelinefor the staged commit sequence