# Enhancement backlog

Captured proposals — not yet triaged, not scheduled. Each entry has enough
context to pick up cold. Delete an entry once it's done or explicitly rejected.

---

## E1 — Denormalize `company_id` onto the time-series tables

**Where:** `inference.inference_event`,
`inference_detectobject_predicted`, `inference_runtimestat`.

**What:** Add a nullable `company_id INT` column to each of the three
Timescale hypertables. Populate it at write time (from the writer's
`TenantContext.company_id`). Add a supporting index —
`(company_id, time_sec)` — so tenant + time-range queries prune chunks and
then hit a b-tree.

**Why:** Today, tenant scoping on inference events requires a 4-table join:
`inference_event.request_id → job.request_id → device.location_id →
location.company_id`. Every analytics endpoint pays this cost, on every
read. That's the driver of most of the SQL in
`services/analytics_app/src/web/events.py`, and it's fragile (see E2).

**Trade-offs:**
- Cheap on reads (one WHERE clause).
- Adds a column to keep in sync at write time — writers need the tenant
  in scope, which they already have (they came in through `require_tenant`).
- Backfill migration is non-trivial: existing rows have to be joined
  back through `job` to hydrate the new column. Do it in batches during
  a maintenance window.
- Downstream code that ignores `company_id` (raw dumps, ETL) is
  unaffected — the column can be nullable at first.

**Migration sketch:**
1. Alembic migration: `ALTER TABLE ... ADD COLUMN company_id INT NULL`
   on all three tables. Add `(company_id, time_sec)` index.
2. Update analytics writers (`cim` event ingest,
   MLC callback into `inference_event`) to set `company_id` from
   `ctx.company_id`.
3. Backfill script: `UPDATE ... SET company_id = (SELECT l.company_id
   FROM job j JOIN device d ON d.id = j.device_id
   JOIN location l ON l.id = d.location_id
   WHERE j.request_id = e.request_id LIMIT 1)`
   run in `time_sec` batches per chunk.
4. Once backfill is done, rewrite analytics reads in
   `services/analytics_app/src/web/events.py` to filter directly and
   drop the joins.

**Related:** E2 (makes the backfill query in step 3 cleaner if
`request_id` becomes a real key).

---

## E2 — Turn `inference_event.request_id` into a real join key

**Where:** `inference.inference_event.request_id` (String),
`inference_detectobject_predicted.request_id`,
`inference_runtimestat.request_id` — all currently plain `VARCHAR(256)`
with no FK. Referenced from `dori.job.request_id` (also `VARCHAR(500)`).

**What:** Two options, in ascending order of invasiveness:

a. **Add a validating check constraint / index.** Enforce that every
   `inference_event.request_id` matches a row in `job.request_id`
   at write time, via a trigger or a unique-index-backed lookup. Doesn't
   remove the string join but makes format drift fail loudly.

b. **Add a surrogate `d_job_id BIGINT` column.** At write time, look up
   the `job.id` from `request_id` and store it. Then FK
   `inference_event.d_job_id → job.id`. Reads join on integers,
   not strings; format changes to `request_id` become harmless.

**Why:** Today, the join is a plain string equality with no FK
enforcement. If the `request_id` format ever changes (writer regenerates,
new prefix, whitespace, case) the join silently returns zero rows
instead of erroring. Every analytics query breaks quietly.
This is landmine #10 in `config_app_data_model.html`.

**Trade-offs:**
- (a) is a small migration but leaves the string join in place.
- (b) is a bigger migration (new column, backfill, all queries updated)
  but gives you integer joins and referential integrity.
- Both require thinking about ordering: the writer for `inference_event`
  may fire before `job` is fully committed — hard-FK would need
  a deferred constraint or a two-phase write.

**Suggested:** start with (a) — cheap enough to just do — and defer (b)
until we're rewriting the analytics reads anyway (see E1 step 4).

---

## E3 — Enforce a `time_sec` range on every hypertable read

**Where:** Every SQL against `inference.*` in
`services/analytics_app/src/web/events.py` (and any future readers).

**What:** Make it structurally hard to write a hypertable query without
a time range. Two implementation angles:

a. **Runtime guard in the DB layer.** Add a helper
   `analytics_query(model, *, since, until, ...)` in
   `common/dori_utils/analytics.py` that wraps every
   `select(DInferenceEvent)` and errors if `since`/`until` are missing
   or wider than a configured cap (e.g. 30 days). Existing endpoints
   already accept `from_ts`/`to_ts` — they'd just route through this
   helper.

b. **Static lint.** Add a ruff/AST rule to
   `unit_tests/`/`ci` that scans for `select(DInferenceEvent)` /
   `select(DInferenceDetectobjectPredicted)` etc. and requires
   `.where(...time_sec...)` in the same expression. Fails CI on
   naked reads.

**Why:** Timescale prunes chunks by the time column. A query that
filters only on `id` (or any non-time column) has to touch every chunk
— potentially thousands, for a well-populated table. Since
`inference_event`'s PK is `(id, time_sec)`, plain
`db.get(DInferenceEvent, some_id)` doesn't hit an index. Today, nothing
prevents this from being written.

**Trade-offs:**
- (a) works for handwritten queries but not for `db.get(...)`. Would
  need a wrapping repository layer to fully cover.
- (b) covers all callsites but doesn't catch dynamically-built queries.
- Combining (a) + (b) — a helper for the common case, a lint for the
  guardrail — is the strongest position.

**Suggested:** ship (a) first as a helper + convert existing
analytics endpoints to use it. Add (b) later if we still see raw
`select()` slipping in.

---

## Cross-references

- `config_app_data_model.html` — the data-model doc; the section
  "Time-series" and landmines #4, #5, #10 motivate all three of the
  above.
- `common/dori_model/dIdoModel.py` — the ORM for the affected tables.
- `services/analytics_app/src/web/events.py` — the primary reader that
  benefits from E1 and E3.

---

## E4 — Transactional outbox dispatch (producer moves into the app writer)

**Priority: NEXT.** This item now subsumes the former **E5** ("move
event-notification logic out of the Postgres trigger"): the writer-side of the
outbox *is* the producer-in-writer move, so the two are one task, not two. Do
not implement E5 standalone — on its own it only relocates `pg_notify` from the
trigger to app code, keeps the dual-write atomicity hole, and adds a bypass
risk. The value is the outbox + producer-in-writer done **together**, below.

**Where:** the event notification path — producer side: the writers of
`inference.inference_event`
(`services/analytics_app/src/utils/inference_ingest.py` for customer events;
`services/monitor/src/utils/mlc_check.py` + `.../service/system_event_service.py`
for system events); the trigger they currently lean on
(`alembic/sql/create_notify_trigger.std.sql`); relay/consumer side:
`services/cim` dispatcher on topic `dori.alerts.dispatch`, with
`pg_notify('ui_event')` kept for the DIM websocket path.

**What:** Make the **app writer** the producer and add a durable **outbox**:
1. New table `dori.event_outbox (id bigint identity, event_id uuid,
   aggregate_key text, topic text, payload jsonb, status text default 'pending',
   attempts int, next_attempt_at timestamptz, created_at, dispatched_at,
   last_error)`, partial index `(next_attempt_at) WHERE status='pending'`. ORM
   `DEventOutbox` next to `DInferenceEvent`.
2. The writer, **in the same transaction** as the `inference_event` insert
   (`INSERT … RETURNING id`), decides routing in Python (the former E5 move):
   if the event needs an alert (`event_type.is_trigger`/route) it inserts an
   *enriched* outbox row (topic `dori.alerts.dispatch`, `aggregate_key='mlc:<id>'`,
   `alert_id = event_id` as the idempotency key), then `NOTIFY outbox` as a
   contentless wake nudge; one `commit` makes event + outbox row atomic. The
   ephemeral UI path stays a separate `pg_notify('ui_event', payload)` when
   `is_notification`.
3. A **relay** (`run_outbox_relay`, LISTEN `outbox` + fallback poll) claims a
   batch `… WHERE status='pending' AND next_attempt_at<=now() ORDER BY id
   FOR UPDATE SKIP LOCKED LIMIT 100`, publishes each `payload` to its `topic`,
   marks `dispatched`; on failure bumps `attempts` + exponential-backoff
   `next_attempt_at`, dead-letters (`status='dead'`) after N tries.
4. Sweep dispatched rows older than a few days.

**Why:** `pg_notify` alone is at-most-once — if the consumer is down
(deploy/crash) the notification is silently lost, there's an ~8 KB payload cap
(why the trigger truncates event_data to 512), and no consumer groups/replay.
For critical alerts (MLC_DOWN → ops) a drop during a deploy is unacceptable. The
outbox makes alert delivery durable + at-least-once with retries. Moving the
producer into the writer (former E5) also makes routing testable/observable,
stops every schema rename from re-applying the trigger (014→016→017), and drops
the per-insert read-amplification (the writer already holds request/device/
company ids, so the trigger's 4 lookups/row largely vanish).

**Trade-offs:**
- One INSERT + one batched read-back on the dispatch path. Cheap: the claim is
  ≤100 rows/query, dwarfed by the downstream HTTP/WS/SMS work.
- At-least-once ⇒ consumers must be idempotent (dedupe on `alert_id`=event_id).
  Already partly covered: CIM `check_message_log_exists` cooloff dedups customer
  messages; duplicate WS pushes are harmless.
- The trigger's one virtue is firing no matter *who* inserts. Once the producer
  is in the writer, every writer must route through the shared enqueue helper or
  it silently skips notifications — you have 3 writers + a test today. Mitigate
  with one ingest path + a test asserting nothing else inserts into
  `inference_event`.
- Per-aggregate ordering (never deliver MLC_UP before MLC_DOWN): `ORDER BY id`
  plus a single relay worker, or shard workers by `hash(aggregate_key)`.

**Rollout (monitor MLC up/down as the first, lowest-risk slice):**
1. Migration: `event_outbox` table + index. 2. `DEventOutbox` model.
3. `run_outbox_relay` (SKIP LOCKED drain → `dori.alerts.dispatch`), wired into
   monitor's lifespan or a small standalone service. 4. Swap monitor's
   `_insert_system_event` → `_emit_system_event` (event + conditional outbox,
   one commit, `NOTIFY outbox`); this also replaces the `_watch_transitions`
   **dual write** (`producer.send` with no DB row behind it). 5. Point the
   `dori.alerts.dispatch` consumer at it; add dedupe on `alert_id`. 6. Extend
   the same helper to the `inference_ingest.py` customer-event writer. 7. Only
   after the relay path is verified end-to-end: drop the trigger for those
   events (keep `pg_notify('ui_event')` where the UI needs live status).

**Relates to:** the is_message/is_notification routing cleanup (the outbox's
"needs alert" gate comes straight from those flags); E7 (contact points + routes
consume the same `dori.alerts.dispatch` stream).

---

## E5 — (merged into E4)

Former "move event-notification logic out of the Postgres trigger." Folded into
**E4** above — moving the producer into the app writer is the writer-side of the
outbox and is not worth doing on its own. Kept as a heading only so earlier
references ("E5 — remove trigger") still resolve.

---

## E6 — Remove remaining `event_config` references (cleanup)

**Where:** the `d_event_config` / `event_config` table and its `DEventConfig`
model are **already gone** (dropped in `013_drop_more_unused_tables`; not in the
live DB, no active code). What remains are dead references to purge:
- `system_test/dash_sys_test/Apis_function.py` + `Falcon_company_creation_script.py`
  — comments mentioning the removed `event_config` endpoint/table.
- `system_test/test_reports/dori_dash_schema_checklist.html` — a stale generated
  report that still embeds the old pre-rename schema (regenerates on next run).
- `alembic/sql/create_dori_views.sql` — a leftover comment referencing it.
- The historical migrations `011`/`013` keep their references by design
  (immutable history) — **do not touch those.**

**What:** Grep `event_config` across the repo and delete/clean the non-historical
references above; regenerate the stale schema-checklist report.

**Why:** Pure hygiene — leaves no misleading mentions of a table that no longer
exists. Zero functional impact (confirmed: 0 rows, 0 columns, 0 active code
paths touch it).

**Trade-offs:** trivial; just don't edit the immutable migrations.

---

## E7 — Notification redesign: contact points + routing policies

**Where:** `dori.subscriber` + `dori.subscriber_event` (today's model),
`dori.event_type.is_message` / `is_notification`, CIM dispatch
(`services/cim/src/service/event_service.py`), DIM listener. Pairs with E4
(outbox relay) and E5 (remove trigger).

**What:** Replace `subscriber` + `subscriber_event` + the `is_message` flag with
the proven **contact-point + route** model (Grafana/Alertmanager-style):

1. `dori.contact_point (id, company_id, name, type, config jsonb, is_active)` —
   a reusable destination, one address, one type. `type ∈ EMAIL|SMS|WEBHOOK|
   MQTT|SLACK|…`; `config` holds `{"email":…}` / `{"phone":…}` /
   `{"url":…,"auth":…}` / `{"topic":…}`. Addresses defined **once**, reused by
   many routes (fixes today's split: email/phone on subscriber, webhook_url on
   subscription; and the person-less "subscriber" row for webhooks).
2. `dori.notification_route (id, company_id, match_event_type, scope_device_id,
   scope_location_id, contact_point_id, cooloff_seconds, is_active, template_id)`
   — one rule: "this event (+scope) in this company → this contact point."
3. `event_type` keeps **only** `is_notification` (→ WebSocket/UI). **`is_message`
   is removed** — "does it alert" becomes "does an active route match
   `(event_type, company, scope)`." The route IS the config (no flag to keep in
   sync with a subscription).
4. **Sender registry** `{type → sender}` replaces the hardcoded
   `if message_type == 'EMAIL'/'SMS'/…` chain, so a new channel (Slack/Teams/
   voice) is a plugin, not a dispatch edit.

**Routing semantics (the clean separation):**
- WebSocket push ⇔ `is_notification=true`.
- Alert ⇔ ≥1 active route matches. "Alert only, no UI" = `is_notification=false`
  + a route.
- Selectivity preserved: the producer **gates at enqueue** — write the outbox
  row + nudge only if `is_notification OR EXISTS(active route for
  event_type+company)`. So the relay is woken for the **same set** of events
  today's `pg_notify` fires on — never every event. (Optional: cache a
  `has_routes` boolean, maintained on route change, to keep the common
  "no routes" case a flag-read instead of an EXISTS.)
- `pg_notify` wakes the **relay**, not DIM. Relay → DIM/WebSocket only when
  `is_notification`; relay → senders when routes match. DIM becomes one consumer,
  not the dispatcher.
- **System events are not special**: the Dori root company (id 1) has contact
  points + routes like any company; retire the `process_system_event`
  DORI_ADMIN-SMS-only path.

**Why:** today's model has (a) the destination address split across two tables,
(b) double-config (`is_message` flag AND a subscription both required —
footgun), (c) hardcoded senders, (d) system events special-cased, (e) no clean
per-company precision on "does it alert" (the flag is global). Contact points +
routes fix all five and are the pattern you converge on anyway once you add more
channels, severities, escalation, or self-serve customer config.

**Trade-offs:**
- Bigger schema change than the "4 incremental fixes" (unify address, drop the
  double-config, unify system events, add a sender registry). If you'll stay at
  SMS/email/webhook/MQTT with a few recipients/company, the incremental fixes
  may be enough; do the full redesign when extensibility is on the horizon
  (retrofitting later is more painful).
- Per-event route `EXISTS` check replaces a free flag read (cheap on the events
  table; cache a boolean if it ever matters).
- At-least-once + idempotency still required (keyed on `(route_id, event_id)`),
  shared with E4.

**Migration (incremental, no big-bang):**
1. Add `contact_point` + `notification_route`; backfill — each
   `subscriber.{email,phone}` / `subscriber_event.webhook_url` → a contact point;
   each `subscriber_event` → a route.
2. Point CIM dispatch at the new tables via the sender registry.
3. Keep `subscriber`/`subscriber_event` as a compatibility view (or dual-write)
   until the ICD/COD UI is migrated, then drop them + `is_message`.
4. Fold in `is_message`→route and system-events→Dori-routes in the same change.