feat: CEL ↔ DCI bridge (spp_cel_dci_bridge) + OpenG2P preset (spp_dci_openg2p)#199
Draft
gonzalesedwin1123 wants to merge 54 commits into
Draft
feat: CEL ↔ DCI bridge (spp_cel_dci_bridge) + OpenG2P preset (spp_dci_openg2p)#199gonzalesedwin1123 wants to merge 54 commits into
gonzalesedwin1123 wants to merge 54 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.85% +0.16%
==========================================
Files 942 976 +34
Lines 55470 56347 +877
==========================================
+ Hits 39763 40487 +724
- Misses 15707 15860 +153
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.
…ibility
When an operator clicks "Import Eligible" or "Enroll Eligible" on a
Program (not a Cycle), the eligibility manager compiles
`has_disability == true` to `metric('has_disability', me) == true`.
The executor finds the cache empty, falls back to a legacy Python path,
and crashes:
AttributeError: 'spp.indicator' object has no attribute 'evaluate'
Root cause: cycle_manager_base.py already calls
_precompute_cycle_cached_variables before each eligibility check. The
program-level flow has no equivalent pre-fetch — the SQL fast path is
never available, the broken Python fallback always fires.
Fix: inherit spp.program.membership.manager.default and override
_prepare_eligible_domain to call cache_mgr.precompute_cached_variables
on the candidate cohort before the CEL compile runs. After pre-fetch
the cache is fresh, the executor takes the SQL fast path, and the
broken legacy path is never touched.
Cohort definition matches the base domain that
spp_programs/models/cel/eligibility_cel.py applies before the CEL filter
(is_registrant=True, is_group respects target_type, disabled=False).
When membership is passed (cycle / verify flows), the cohort is
narrowed to those partners — same scaling profile as the cycle manager.
Tests cover: no CEL expression -> no pre-warm; CEL expression with
matching cohort -> pre-warm fires; empty cohort -> short-circuit;
pre-warm failure -> caught and logged, eligibility evaluation continues.
No sudo on the cohort search: respects operator record rules, matching
cycle_manager_base behaviour.
Adds spp_programs to the bridge's manifest depends (we now inherit
spp.program.membership.manager.default from that module).
ADR-017's Phase 4 migration removed the .evaluate() / .enqueue_refresh_from_domain() methods from spp.indicator, but the model itself is still registered when spp_indicators is installed. The executor's _exec_metric branch detects the model via `if "spp.indicator" not in self.env` and falls through when present — then crashes calling `svc.evaluate(...)` because the method is gone. This is hit by every code path that compiles a `metric()` reference against an incomplete cache: CEL builder live preview, validation, ad-hoc compile. Guard: check hasattr(svc, "evaluate") before invoking. If absent, treat the same as the model-not-installed branch — warn, return [], let the caller continue. Preview shows "0 matching" instead of crashing; callers that need real results pre-warm the cache via spp.data.cache.manager.precompute_cached_variables (spp_programs/managers/cycle_manager_base.py and spp_cel_dci_bridge/models/eligibility_manager.py already do this). Caught by spp_cel_dci_bridge integration: pre-warm only fires from the eligibility manager flow, not the CEL widget's /spp_cel/validate live preview. The widget hits this every keystroke / Save in the Advanced Builder.
…iable
The DCI binding override on spp_studio.var_has_disability set source_type
and the DCI fields, but left the variable in DRAFT state (spp_studio's
default for new variables). Symptom observed in the demo:
- Variable shown as source_type=field, state=Draft, Field Missing
- Pre-warm iterates cached variables but the field-based has_disability
quietly returns empty (the local field doesn't exist when
spp_disability_registry isn't installed)
- No DCI call. No audit row. No cache row.
Two fixes:
1. Data XML override now sets state='active' AND active=True explicitly
so first install pushes the variable through the Draft → Active
transition.
2. post_init_hook tracks state + active in _EXPECTED_BINDING_FIELDS so
any drift (e.g., from a later spp_studio upgrade or manual edit
reverting state to draft) gets corrected on every install/upgrade
of the preset.
Also cleans up the hook's docstring to spell out the three reasons it
exists alongside the data XML: noupdate semantics, state machine
transitions, and as a safety net for failed XML loads.
Tests assert state='active' and active=True after both initial install
and post-reset re-assert. The drift-simulation test now also resets
state='draft' to verify the hook activates it back.
… role
Replaces the FR-as-DR pretense with OpenG2P playing its proper Social
Registry role (ADR-024 federated demo topology). Disability data moves
to a separate OpenSPP-DR instance in subsequent phases.
Wire-format changes (per OpenG2P-provided sample):
- query_type = expression with namespaced URI ns:org:QueryType:expression
- query.value = nested {expression:{query:{search_text:{$eq:<id>}}}} shape
- reg_type and reg_record_type both literal "Individual"
- consent and authorize blocks attached to every search_criteria
Routing changes:
- Data source registry_type DR -> SR
- Dispatcher override moves from _handler_dr to _handler_sr
- Bridge's _handler_sr stub still raises DCIConfigurationError when vendor
is cleared, preserving the silent-failure guard
Service changes:
- OpenG2PFRService replaced by OpenG2PSocialService
- New surface: get_partner_record(partner) -> dict | None (raw reg_record)
- No vendor-specific synthesis; bridge dispatcher extracts via
variable.dci_attribute_path
Manifest no longer depends on spp_dci_client_dr (SR path doesn't need it).
ADR-024 federated topology: the OpenG2P preset owns Social Registry variables, not disability. Disability is sourced from a separate OpenSPP-DR instance, which will be bound by spp_dci_openspp_dr (Phase 6). Drops the var_has_disability override (no XML record, no post_init_hook re-assertion) so the variable falls back to its spp_studio default. The DR preset will rebind it when installed. Adds two new SR-bound CEL variables, each pre-bound to the OpenG2P SR provider with semantic vendor-neutral accessors (ADR-023 §1a): - is_poor (dci_attribute_path=is_poor) - has_dependent_under_school_age (dci_attribute_path=has_dependent_under_school_age) post_init_hook is generalised to iterate a _PRESET_VARIABLES table so new SR variables can be added without touching the drift-correction loop.
…h handler Replaces spp_dci_server's 501 stub at the disability search endpoint with a working DR implementation. Installs on the OpenSPP-DR instance in the federated topology (ADR-024); SP instances see no change. Components: - DisabilitySearchService: parse SearchRequest -> look up partner by spp.registry.id.value -> return wire-format reg_record with has_disability/disability_certified/disability_percentage keys - disability_search_router: signed DCI envelope, mirrors the shape of spp_dci_server.routers.search.search_registry - fastapi.endpoint override: filters the parent's stub router out of _get_fastapi_routers and substitutes the real one — FastAPI matches routes by registration order, so filtering is required to win the /disability/registry/sync/search path collision - UIN vocab code seed for the system id-type vocabulary (only path through which a code can be added) Wire-format details: - Query parsing handles both idtype-value (upstream flat shape) and expression (OpenG2P nested search_text shape). - "Not found" surfaces as status=rjct with REG-ERR-001 / REGISTER_NOT_FOUND (the SPDCI SearchStatusReasonCode enum has no canonical not-found code; the OpenG2P convention is the only widely understood pattern). - Disability fields are read defensively (getattr with False/None defaults) so the module doesn't strict-depend on the partner-fields module.
Mirrors spp_dci_openg2p's structure but for the DR side of the federated demo topology (ADR-024). Configures a DCI data source targeting a sibling OpenSPP-DR container, a CEL provider, and rebinds spp_studio.var_has_disability to source from the DR provider. The vendor field used to live in spp_dci_openg2p only — hoisting it into spp_cel_dci_bridge with an empty selection lets each preset register its own vendor via selection_add. Both presets now compile in any combination without depending on each other. Why a vendor override exists for the DR path: upstream DRService reads disability fields from `data` directly, but the SPDCI spec puts records at `data.reg_records[0]`. OpenSPPDRService takes ownership of the response unwrap until DRService is fixed upstream. Default base_url=http://openspp-dr:8069 lines up with the docker-compose service name introduced in the next phase.
Second OpenSPP container that plays the Disability Registry role in
the federated demo topology (ADR-024). The SP container reaches it
in-network at http://openspp-dr:8069; the host can reach each instance
on a different port (SP=8069, DR=8070).
Both containers share the existing db service but use distinct
databases (openspp vs openspp_dr) so neither sees the other's data.
Launch:
docker compose \
-f docker-compose.yml \
-f docker-compose.dr.yml \
--profile ui --profile dr up -d
Module wiring is left to operators via ODOO_INIT_MODULES /
ODOO_DR_INIT_MODULES env vars, defaulting to spp_dci_server_disability
on the DR side.
Edwin's workflow keeps ./spp + docker-compose.yml as the SP control surface. The DR file should not modify SP services or share their lifecycle — it's a separate container the operator launches and tears down independently. Changes: - Project name set to "openspp-dr" so it doesn't merge with the SP's default "openspp2" project. - Drops the dr profile and the SP file's volume/build inheritance — the DR uses its own openspp_dr_data volume and reuses the existing openspp-dev image without rebuilding. - Joins the SP project's network as an external network (openspp2_openspp by default, OPENSPP_NETWORK overrides) so SP and DR containers can still resolve each other by service name. - container_name + hostname pinned to openspp-dr so the SP's preset (base_url http://openspp-dr:8069) reaches it predictably. New launch flow: ./spp start # SP as before docker compose -f docker-compose.dr.yml up -d # DR alongside
…ails When both spp_dci_openg2p and spp_dci_openspp_dr install on the same SP database, their independent UIN seeds collide on UNIQUE(vocabulary_id, code) in the system urn:openspp:vocab:id-type vocabulary, raising ParseError "Code 'UIN' already exists" on the second preset's install. In the federated topology this collision is unavoidable — the SP needs both presets to drive SR (OpenG2P) and DR (OpenSPP-DR) routing. The OpenG2P preset's seed stays as the canonical source for SP-side UIN; this preset relies on it. Changes: - Drop data/openspp_dr_id_types.xml from the manifest and delete the file. The previous xmlid spp_dci_openspp_dr.id_type_uin_sp is gone. - Tests previously calling env.ref on that xmlid now use a new get_or_create_uin_code helper (tests/common.py) that calls spp.vocabulary.code.get_or_create_local — the ADR-016 supported path for adding codes to system vocabularies at runtime. The helper makes the tests self-contained whether or not spp_dci_openg2p is also installed in the test DB. - test_install drops the two assertions that depended on the xmlid; test_service_priority_first_is_uin replaces them by checking the IDENTIFIER_PRIORITY tuple's first entry directly. For a SP database that wants to use only spp_dci_openspp_dr without spp_dci_openg2p (not the demo scenario but a sensible deployment), operators can seed UIN manually or call the same get_or_create_local helper from a deployment script.
DisabilitySearchService was reading is_person_with_disability, disability_certified, and disability_percentage from res.partner. None of those fields exist on res.partner — the actual disability data model in spp_disability_registry exposes has_disability (Boolean related to current approved assessment) plus severity/review metadata. The result: every DR response returned has_disability=False regardless of the partner's actual state, defeating the SP-side eligibility check. Rewrites the reg_record builder to read: - has_disability (boolean, real field) - disability_severity_code (vocabulary code via severity_id) - disability_review_category (selection) - disability_next_review (date ISO string) Drops disability_certified and disability_percentage entirely — neither has a counterpart on the data model. Tests: - setUpClass stamps has_disability via raw UPDATE since the field is computed-stored and skipping the assessment chain is simpler in isolation than creating a full approved assessment graph. - test_reg_record_carries_wire_format_keys now asserts the new key set instead of just absence of legacy names.
…_api/v1 The fastapi.endpoint record in spp_dci_server registers the DCI app under root_path=/dci_api/v1, not /dci as I'd assumed. With the wrong prefix, the SP's data source was POSTing to /dci/disability/... which Odoo's website dispatcher 404s on (HTML response, "Server: Werkzeug"). Corrects: - spp_dci_openspp_dr/data/openspp_dr_data_source.xml — search_endpoint - Service docstring, tests, both readme files in both modules Operators with an existing data source need to manually update the search_endpoint field on their openspp_dr record (noupdate=1 means the XML change won't apply on upgrade).
The Phase 6 hoist of the `vendor` selection field from spp_dci_openg2p to spp_cel_dci_bridge made the field available on every spp.dci.data.source record, but no view extension surfaced it on the form or list. Operators had no way to set vendor through the UI — manual psql or developer-mode field editing was the only path. Adds two inherited views in spp_cel_dci_bridge/views/dci_data_source_views.xml: - Form: inserts the vendor field after auth_type (the natural neighbor; both are "which adapter does the bridge use" decisions). - Tree: inserts a vendor column after registry_type with optional=show. Both presets' selection_add entries (openg2p, openspp) appear in the resulting dropdown automatically.
…er requests The DCI FastAPI endpoint runs as base.public_user (spp_dci_server's fastapi_endpoint_data.xml). When a DCI search request arrives, DisabilitySearchService._find_partner_by_identifier raises AccessError trying to read spp.registry.id — public has no Registry permissions. Authentication is upstream (DCI signature + bearer token middleware), so once the request reaches the service the sender is trusted. The service queries reg_ids and the matched partner via sudo() to bypass the public-user ACL, mirroring the pattern in spp_dci_server/routers/search.py for signing-key reads. No tests change — test setUpClass runs as admin so ACLs were never exercised; the bug only surfaces against the live endpoint.
![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.
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