# Fixed Gaps — Changelog

Every gap found and fixed during this engagement, in the order discovered, plus everything found
but **not** fixed (still open). Each fixed item links to the topic document with full detail.
Dates are approximate (same multi-day engagement); the point of this document is traceability, not
exact timestamps.

## Fixed

1. **`MwRisk.FeatureHydrator` Type-A extraction was blind to Jube-style PascalCase fields** —
   only recognized snake_case payload keys (`"amount"`, `"ip_address"`, etc.), never
   `CurrencyAmount`/`IP`/`CardPan`/etc. Fixed by adding a Jube-aware `get_jube/2` fallback ahead
   of every snake_case lookup.
2. **`GatewayWebWeb.Components.RuleBuilder` LiveComponent reset bug** — `update/2`
   unconditionally re-parsed the external `value` into local state on *every* parent re-render
   (e.g. clicking an unrelated UI toggle), silently discarding in-progress rule-expression edits
   before they were saved. Fixed with `assign_new/3` so external state is only seeded on first
   mount.
3. **Cross-model rule leakage** — `RuleCache` (activation), `GatewayRuleEngine` (gateway), and the
   abstraction rule loader all filtered by `tenant_id` only; every model's rules executed against
   every other model's `/verify` call. Fixed by adding `entity_model_id` filters to all three.
   Detail: [07-MULTI-MODEL-SCOPING.md](07-MULTI-MODEL-SCOPING.md).
4. **`RuleExpression`'s legacy single-line text parser had no `AND`/`OR` support** — a string
   like `"X > 5000 AND Y != \"USA\""` silently mis-tokenized into one bogus, almost-always-true
   condition. Fixed by splitting on whitespace-delimited `AND`/`OR` before tokenizing each
   segment. Detail: [02-RULE-EXPRESSION-ENGINE.md](02-RULE-EXPRESSION-ENGINE.md).
5. **No field-vs-field comparison** — `Payload.MerchantCountry != Payload.IssuerCountry` compared
   the left field against the *literal string* `"Payload.IssuerCountry"`, always true. Fixed with
   a namespace-prefix heuristic in `resolve_operand/2`.
6. **`sanction`/`tag`/`dict` rule-tree ops were parsed but never evaluated** — `evaluate/2`'s
   catch-all silently returned `false` for any op type beyond `and/or/not/cond`, so every
   sanctions/BIN/geography/country-risk rule using these ops could never fire, with no error
   surfaced anywhere. Implemented `eval_sanction/3`, `eval_tag/3`, `eval_dict/3`.
7. **`is_bypass` sanctions-decline polarity bug** — `"Bypass - Sanctions Exact Match"` had
   `is_bypass=true`, meaning a confirmed sanctions hit was short-circuiting to **approve**
   (`is_bypass=true` = "trusted fast-path, skip scoring and approve"). Fixed by making a matched
   `"sanction"`-op rule always decline regardless of `is_bypass`.
   Detail: [03-GATEWAY-RULES.md](03-GATEWAY-RULES.md).
8. **`risk_activation_watcher.id` missing `AUTO_INCREMENT`** — every watcher-record insert
   (triggered whenever a non-approve decision fires) crashed silently in an async task. Found
   only because fix #7/#3 made non-approve decisions actually occur for the first time in testing.
9. **`RedisTier.parse_float/1` crashed on whole-number `HINCRBYFLOAT` replies** — Redis returns
   `"1"` (no decimal point) for a whole-number result; `String.to_float/1` requires one and
   raises. Found only because fixing the entity-extraction blindness (#11 below) made
   `TtlCounter.increment` actually get called for entities that had never successfully written
   before. Fixed by switching to the tolerant `Float.parse/1`.
10. **`VelocityPipeline`/`FeatureHydrator` entity extraction was blind to Jube-style fields** —
    same root cause as #1, but in the velocity write/read paths (`card`/`merchant`/`ip_device`/etc.
    extraction only recognized snake_case candidates). Fixed with the same `get_jube/2` pattern.
11. **`TtlCounterCache`'s `@horizon_map` was missing common interval combinations** —
    `{"hours", 1}`, `{"minutes", 5/15/20}`, `{"hours", 24}` all fell back to the raw DB string
    (`"hours"`) instead of a canonical horizon (`"1h"`/`"1d"`), producing duplicate,
    inconsistently-named counters for the same logical window.
12. **`/verify` made fully config-driven for velocity, by explicit request** —
    `VelocityPipeline.update_sync/1` now runs synchronously after the decision is computed, so
    repeated `/verify` calls accumulate real TTL-counter/abstraction-journal history (a deliberate
    change to the dry-run contract for velocity specifically; case/notification side effects
    remain dry). Detail: [01-APIS-AND-FLOWS.md](01-APIS-AND-FLOWS.md).
13. **Real velocity computation implemented for previously-stubbed abstraction function types**
    (2 Distinct Count, 5 Median, 6 Kurtosis, 7 Skew, 8 StdDev, 11 Mode, 12 Same Count, 14 Max, 15
    Min, 16 Since) — all unified into one generic journal-based mechanism, also fixing the
    "implemented but broken" types (1 Count, 3 Sum, 4 Average, 13 Actual Value) whose lookup keys
    never matched real data. Detail: [05-ABSTRACTION-RULES-AND-VELOCITY.md](05-ABSTRACTION-RULES-AND-VELOCITY.md).
14. **`review_status` approval workflow was completely unenforced** — across gateway, activation,
    *and* abstraction rules, the database column existed but no Ecto schema declared the field at
    all, so no query could ever filter on it. `pending_review`/`draft`/`rejected` rules executed
    identically to approved ones. Fixed by adding the field to all three schemas and a
    `review_status == "approved"` filter to all three loader queries.
15. **No arithmetic support in `RuleExpression`** — `StdDevAnomalyReview`'s
    `Abstraction.StdDev30DayForAccountId > Abstraction.AvgAmount30DayForAccountId * 3` never fired
    even when the math was true, because the right-hand side was treated as one literal
    (nonexistent) field path. Added single-binary-operator arithmetic (`+ - * /`).
16. **`risk_ttl_counters` configuration didn't actually drive `VelocityPipeline`** — three layered
    issues: (a) `TtlCounterCache` grouped all of a model's counters under one bucket keyed by the
    *model's* `entity_type`, not each row's own entity; (b) `VelocityPipeline` never read
    `get_specs/2` at all, only wrote a hardcoded metric list; (c) no column existed to express a
    counter's firing condition. All three fixed. Detail: [06-TTL-COUNTERS.md](06-TTL-COUNTERS.md).
17. **`data_name`/`data_value` swapped relative to Jube** — the UI's "Sum field" wrote to
    `data_value`; the engine read `data_name`. Confirmed against Jube source that `data_value` is
    correct; fixed the engine to match.
18. **Jube-style arbitrary-field grouping added** — counters whose name doesn't follow the legacy
    `"<entity> <horizon> <metric>"` convention (e.g. one literally named `"IP"`) now group by
    `data_name` directly, resolving the value from the payload field of that name — additive, zero
    regression to the legacy convention.
19. **Cross-model leakage in TTL counters** — found via the same `"IP"` counter (#18) showing
    another model's `ip_device` data in its response. Two compounding causes: the cache wasn't
    model-scoped, and even after scoping it, a hardcoded fallback-horizon mechanism in
    `FeatureHydrator` read from a Redis key with no model component at all. Both fixed.
20. **Cross-model leakage in the abstraction velocity journal** — the same root cause as #19, one
    layer over: `abs_account`/`abs_card`/`abs_ip` journal entity names had no model component, so
    `Volume1DayUSDForAccountId`-style rules summed a *different* model's transaction history for
    the same `AccountId`. Fixed by suffixing entity names with `model_id`.
21. **O1 closed — `MwRisk.SanctionsChecker.check_payload/1` was blind to Jube fields.** Same root
    cause as #1/#10/#11, in the early Step-0 sanctions check this time. Added a Jube-aware
    `@jube_name_fields` list (`BeneficiaryName`, `PayerName`, etc.) plus a `BillingFirstName`
    + `BillingLastName` combiner. Verified end-to-end: the very first test payload of this whole
    engagement (`"Robert Mugabe"` via billing name fields against model 257) now correctly
    declines via `Sanction.hit`.
22. **O7 closed — `rule_expression`/threshold-column ambiguity made visible in the UI.**
    `ActivationEngine.rule_fires?/2` still silently prioritizes `rule_expression` over
    `feature_key`/`operator`/`threshold_value` on the same row (a structural fix was judged too
    risky — could break existing rules with both fields legitimately populated). Instead, the
    Activation Rules detail panel now shows an explicit warning banner whenever a rule has both,
    naming the inert values and pointing at "Delete expression" — confirmed visually in the admin
    UI.
23. **O6 closed — automated regression test for cross-model leakage, all four subsystems.** Added
    `apps/mw_risk/test/mw_risk/multi_model_scoping_test.exs`, seeding two distinct models in the
    same tenant and asserting `GatewayRuleEngine.check/1`, `Risks.list_active_abstraction_rules/1`,
    `RuleCache.get_rules/2`, and `TtlCounterCache.get_specs/3` all never cross model boundaries,
    plus a `review_status` exclusion check for each. Initially `RuleCache`/`TtlCounterCache`
    couldn't be tested at all — running `mix test` scoped to just the `mw_risk` app crashed at
    boot (`MwRisk.TtlCounterCache.init/1` subscribes to `MwCore.PubSub`, but that registry is owned
    by `infra_cache`'s own supervisor, which `mw_risk` never declared as a dependency — it only
    worked in production because `infra_cache` happened to be started as a sibling app in the full
    umbrella boot). Fixed by adding `{:infra_cache, in_umbrella: true}` to `mw_risk`'s `mix.exs` —
    an explicit fix for a real undeclared-dependency bug, not just a test workaround. The test
    itself uses shared sandbox mode so the GenServers (which query the DB from their own process)
    see the same transaction-scoped data the test inserts.
24. **O9 fully closed — all 8 unconfigured abstraction rules.** Found that `AbstractionEngine`
    never read the `rule_expression` column at all, even though it existed on the schema and was
    already populated for `CurrencyRiskGate`/`AmlRiskGate` — which is *why* those two were also
    stuck at `0.0` despite looking configured. Wired `evaluate_rule/4` to fall back to
    `RuleExpression.evaluate/3` when there's no usable `search_key`, mirroring `ActivationEngine`'s
    existing precedence. This fixed, in one change:
    - `HighValueFlag` → `Payload.AmountUSD > 1000`
    - `InternationalFlag` → `Payload.MerchantCountry != Payload.IssuerCountry`
    - `NightTimeFlag` → `Payload.LocalHour >= 22 OR Payload.LocalHour <= 5`
    - `CurrencyRiskGate`/`AmlRiskGate` (bonus — no config change needed, just the engine fix)

    `NewMerchantFlag` configured separately via the existing `function_type=12` (Same Count,
    `search_key=Payload.AccountId`, `function_key=Payload.MerchantId`). Verified the polarity is
    `== 1` (not `0`) means "first transaction with this merchant" — the synthetic in-flight event
    used for `/verify`'s dry-run semantics (#13) always counts itself, so a genuinely new
    relationship reads `1`, not `0`; confirmed by sending the same account+merchant twice and
    seeing `1 → 2`.

    The remaining 4 (`AmountZScore`, `AmountPercentile`, `VelocityScore`, `GeographicAnomalyScore`)
    needed formulas over multiple stats, not just config — proposed concrete formulas for review
    (not guessed at silently) and, once confirmed, added four new mw-core-specific `function_type`
    codes (101-104, deliberately outside Jube's 1-16 range):
    - **101 — Z-Score**: `(current - mean) / stddev`; `0.0` when stddev is `0`.
    - **102 — Percentile**: fraction of window values `<=` current, as 0-100.
    - **103 — Velocity score**: `min(count / cap, 1.0)`, cap defaults to `10` (via `offset_value`).
    - **104 — Geographic anomaly**: `1 - (count matching current value / total count)`.

    Verifying `GeographicAnomalyScore` live surfaced a real, separate bug: `MerchantCountry` was
    never added to `@event_field_map`/`current_event_data/1` (read side) or `VelocityPipeline`'s
    journal-write event map at all — meaning this field could never be read from any event, real or
    synthetic, regardless of the formula above it. Without this fix, `GeographicAnomalyScore` would
    have silently returned `1.0` unconditionally (or `0.0` on an empty window) no matter what the
    actual history was. Fixed on both sides; confirmed live with 3 same-country transactions
    followed by a 4th in a new country, producing `0.0 → 0.0 → 0.0 → 0.8`.
25. **O3 closed — `risk_velocity_pipeline` and `risk_scoring_enabled` flags.** Both were unset
    everywhere, so real-transaction-path scoring never ran at all (independent of and prior to the
    O2 schema mismatch). Set both to `true` in `config/dev.exs`, with a comment explaining this is a
    deliberate choice to exercise the path despite the known O2 mismatch, and explicitly not to
    carry into `prod.exs` until O2 is separately addressed.
26. **O5 closed — `review_status` enforcement for TTL counters.** `risk_ttl_counters` never had the
    column at all (gateway/activation/abstraction rules got it via untracked SQL prior to this
    migration history). Added a proper migration (`20261009000001_add_review_status_to_ttl_counters.exs`),
    the schema fields, and a `review_status == "approved"` filter in
    `Risks.list_active_ttl_counters/2` — same pattern as the other three rule types.
27. **O8 closed — chained arithmetic in `RuleExpression`.** Extended `parse_arithmetic/2` from a
    single-operator regex to a small tokenizer + two-pass reducer (`*`/`/` first, then `+`/`-`,
    left-to-right within each level), so `A * 2 + B - C` now evaluates with standard precedence.
    Still value-side only, matching the existing convention. Verified directly against the exact
    shape `StdDevAnomalyReview` uses, plus new chained cases.

## Open (found, not fixed — by scope decision or not yet addressed)

| # | Gap | Status |
|---|---|---|
| O1 | `MwRisk.SanctionsChecker.check_payload/1` blind to Jube-PascalCase payloads. | ✅ **Closed** (#21) — added `@jube_name_fields` + a `BillingFirstName`/`BillingLastName` combiner. |
| O2 | `/api/v1/transactions` uses a different, snake_case canonical payload schema, incompatible with the PascalCase Jube-style fields the rule engine reads. | Still open — explicitly confirmed out of scope by the user. See [01-APIS-AND-FLOWS.md](01-APIS-AND-FLOWS.md). |
| O3 | `risk_velocity_pipeline`/`risk_scoring_enabled` feature flags (gating real-transaction-path scoring + velocity writes) were never set anywhere in config — dormant by default, independent of O2. | ✅ **Closed** (#25) — both set to `true` in `config/dev.exs`, deliberately, accepting the O2 schema mismatch on that path until O2 is separately addressed. Not carried into `prod.exs`. |
| O4 | TTL counter storage schema not migrated to Jube's one-key-per-counter-GUID model. | Still open — deliberately deferred, confirmed not required for correctness. See [06-TTL-COUNTERS.md](06-TTL-COUNTERS.md). |
| O5 | `review_status` not enforced for `risk_ttl_counters` (the column doesn't even exist on that table). | ✅ **Closed** (#26) — migration + schema fields + query filter added, matching the other three rule types. |
| O6 | No structural guard preventing a *future* cache/feature from reintroducing cross-model leakage. | ✅ **Closed** (#23) — automated test now covers all four subsystems (gateway/activation/abstraction rules + TTL counters). Note: this is a regression guard for these four call paths, not a structural guarantee that a brand-new fifth subsystem will remember to scope itself — that broader gap (a shared `ModelScopedCache` convention/lint) remains open, see [07-MULTI-MODEL-SCOPING.md](07-MULTI-MODEL-SCOPING.md). |
| O7 | Activation rules: `rule_expression`, if set, silently makes `feature_key`/`operator`/`threshold_value` on the same row inert, with no UI warning. | ✅ **Closed** (#22) — visible warning banner added to the detail panel; the precedence itself is unchanged (judged too risky to alter). |
| O8 | `RuleExpression` arithmetic is single-binary-operator only (no `A * 2 + B` chaining). | ✅ **Closed** (#27) — extended to a two-pass reducer with standard `*`/`/` then `+`/`-` precedence. |
| O9 | 8 abstraction rules (`AmountZScore`, `AmountPercentile`, `VelocityScore`, `InternationalFlag`, `HighValueFlag`, `NightTimeFlag`, `NewMerchantFlag`, `GeographicAnomalyScore`) have `function_type: NULL` — never configured. | ✅ **Closed** (#24) — all 8 configured; the 4 needing new formulas got new mw-core-specific `function_type` codes (101-104), proposed for review before implementation. |
| O10 | No async+callback request pattern (Jube's `/Async` + `/Callback/{guid}`). | Still open — not requested. |
| O11 | Jube's "no gateway match = skip entire pipeline" semantics not replicated. | Still open — confirmed intentional divergence, not a bug. See [03-GATEWAY-RULES.md](03-GATEWAY-RULES.md). |
| O12 | `AbstractionEngine` never read the `rule_expression` column at all (separate from O9) — this is *why* `CurrencyRiskGate`/`AmlRiskGate` were stuck at `0.0` despite looking configured. | ✅ **Closed** (#24) — fixed as part of wiring the rule_expression fallback for O9; both now evaluate correctly. |
| O13 | `MerchantCountry` missing from `@event_field_map`/`current_event_data/1` and `VelocityPipeline`'s journal-write event map — found while implementing O9's `GeographicAnomalyScore`. | ✅ **Closed** (#24) — added to both the read and write side. |

Remaining genuinely open: **O2** (out of scope, explicit user decision), **O4** (deferred, explicit
user decision — "Option B"), **O10**/**O11** (not gaps — confirmed intentional non-requirements),
and the broader structural-guard half of **O6** (no convention preventing a *future* subsystem from
omitting model-scoping in the first place).

## How these were found

Almost none of these were found by static code reading alone. The pattern throughout this
engagement was: **construct a real test payload, call `/verify`, and check whether the actual
response matched the rule's documented intent.** Several fixes (the watcher `AUTO_INCREMENT`
crash, the `RedisTier.parse_float` crash, the `data_name`/`data_value` swap, both cross-model
leakage instances) were only discovered as *side effects* of fixing something else and then
re-testing — reinforcing that this class of system needs behavioral verification, not just code
review, to catch silent rule-evaluation gaps.
