diff --git a/CLAUDE.md b/CLAUDE.md index c785e7b..be89f5f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,7 +45,7 @@ CI quality gate (`.github/workflows/quality-gate.yml`) calls the shared reusable ### Layering - **`orchestrator/worker/worker.py`** — entrypoint (`python -m orchestrator.worker.worker`). Builds a single `Activities` instance, connects to Temporal, starts Prometheus (`HTTP_METRICS_PORT`) and Temporal SDK metrics (`HTTP_SDK_METRICS_PORT`) servers, then runs three `Worker`s concurrently via `asyncio.gather`, each on its own task queue: `orchestrator-queue`, `alerts-queue`, `reports-queue`. `prepare_worker.py` derives the queue name from the workflow class name (`camel_to_snake`) and reads per-queue concurrency/poller tuning from env vars prefixed with the workflow name (e.g. `ORCHESTRATOR_MAX_CONCURRENT_ACTIVITIES`). -- **`orchestrator/workflows/`** — Temporal workflow definitions: `orchestrator.py` (pipeline/slot/schedule orchestration), `alerts.py` (real-time ERROR notifications), `reports.py` (scheduled all-level reports). `alerts.py` and `reports.py` both delegate to the same two subworkflows in `workflows/subworkflows/`: `load_notification_package.py` (load notifications + receiver configs from Mongo, incremental via Redis timestamp) and `process_notifications.py` (build HTML email, send, log to Postgres). The difference between alerts and reports is which filter activity they call (`filter_notification_alerts` vs `filter_notification_reports`) and their base data filter (`level: ERROR` by default vs all levels) — `load_notification_package.py`'s `_compute_query_time_filter` can widen either beyond that default to the union of active receiver groups' own `levels` (SIENTIAPDE-2011); the ERROR-only default only holds when no active group narrows further. `load_latest_data`'s Mongo query is capped at `NOTIFICATION_QUEUE_BATCH_SIZE` documents per run (default 5000), sorted by `timestamp` ascending so a capped run always returns the oldest pending batch first (SIENTIAPDE-2013) — a hard cap with no drain loop, since the incremental Redis cursor picks up any remainder on the next run. +- **`orchestrator/workflows/`** — Temporal workflow definitions: `orchestrator.py` (pipeline/slot/schedule orchestration), `alerts.py` (real-time ERROR notifications), `reports.py` (scheduled all-level reports). `alerts.py` and `reports.py` both delegate to the same two subworkflows in `workflows/subworkflows/`: `load_notification_package.py` (load notifications + receiver configs from Mongo, incremental via Redis timestamp) and `process_notifications.py` (build HTML email, send, log to Postgres). The difference between alerts and reports is which filter activity they call (`filter_notification_alerts` vs `filter_notification_reports`) and their base data filter (`level: ERROR` by default vs all levels) — `load_notification_package.py`'s `_compute_query_time_filter` can widen either beyond that default to the union of active receiver groups' own `levels` (SIENTIAPDE-2011); the ERROR-only default only holds when no active group narrows further. Receiver groups also support a `model_ids` allow-list (SIENTIAPDE-2149), independent of the pre-existing `ignore_models_id` deny-list — both apply together (AND) when a group configures both, and `model_ids` follows the same union-at-query-time / precise-per-group-slice split as `levels`. `load_latest_data`'s Mongo query is capped at `NOTIFICATION_QUEUE_BATCH_SIZE` documents per run (default 5000), sorted by `timestamp` ascending so a capped run always returns the oldest pending batch first (SIENTIAPDE-2013) — a hard cap with no drain loop, since the incremental Redis cursor picks up any remainder on the next run. - **`orchestrator/activities/activities.py`** — `Activities` is a single class combining every activity mixin via multiple inheritance: `TemporalManager, SlotManager, Formatters, MongoDB, Email, Postgres` (Postgres comes from `sientia_do.temporal.activities.postgres`, the external `sientia-dataops-library` dependency). One instance is constructed in `worker.py` and its bound methods are handed to each `Worker` as the activity list — activities are plain methods on this shared object, not separate classes. Each mixin's `__init__` takes its own config dict plus shared `logger`, `notification_handler`, `metrics_controller`. `Activities.shutdown()` closes every mixin's connections. - **`orchestrator/utils/`** — `connectors_config.py` builds each config dict from env vars (Temporal/Redis/MongoDB/Postgres/Email); `email_builder.py` renders Jinja2 templates (`utils/templates/email_template.html`, `general_template.html`); `orchestrator_functions.py` transforms pipeline documents into pipeline-type-specific configs (`scouter`, `pi_web_api_scouter`, `predictions_batch`, `minimal_retrain`, `drift`, `simple_metrics`); `converters.py` handles Temporal schedule frequency parsing. diff --git a/e2e/helpers.py b/e2e/helpers.py index ebc27d6..451148b 100644 --- a/e2e/helpers.py +++ b/e2e/helpers.py @@ -347,6 +347,7 @@ def default_receiver_group( levels: list[str] | None = None, contents: list[str] | None = None, ignore_models_id: list[str] | None = None, + model_ids: list[str] | None = None, ) -> dict[str, Any]: """Minimal active receiver_groups document.""" return { @@ -356,6 +357,7 @@ def default_receiver_group( 'levels': levels or ['ERROR', 'WARNING', 'INFO'], 'contents': contents or ['core_alerts', 'persistent_alerts', 'reports'], 'ignore_models_id': ignore_models_id or [], + 'model_ids': model_ids or [], } diff --git a/e2e/scenarios.md b/e2e/scenarios.md index 6492fb9..bd66ac4 100644 --- a/e2e/scenarios.md +++ b/e2e/scenarios.md @@ -68,6 +68,12 @@ the query-time union/intersection across active groups (SIENTIAPDE-2011, see `levels`/`ignore_models_id`, even when the query-time filter widened the fetch to satisfy another active group's broader configuration. +### A.2.2 Model_id allow-list + +Receiver group `model_ids` (SIENTIAPDE-2149) honored: a group configuring it only +receives notifications from those models, even when another active group with +no such restriction widens the query-time fetch to include every model. + ### A.3.1 Empty queue No notifications → no SMTP, no Postgres row. @@ -99,6 +105,7 @@ No notifications → no SMTP, no Postgres row. - No prior Redis timestamp → all matching notifications returned, max timestamp stored. - Prior timestamp → only newer notifications returned. - Query-time filter (SIENTIAPDE-2011) → union of active groups' `levels`, intersection of their `ignore_models_id`; a model ignored by only one of several active groups is not excluded from the fetch. +- Model_id allow-list union at query time (SIENTIAPDE-2149) → when every active group sets `model_ids`, the fetch is restricted to the union of every group's allow-list; a model outside that union is never fetched at all, even for a group that would otherwise want it via a different axis. - Empty Mongo → no Redis timestamp write. - Batch size cap (SIENTIAPDE-2013) → with 5 pending notifications and a batch size of 2, each run returns exactly the next-oldest 2 (sorted ascending by timestamp), and the Redis cursor advances only to what was actually returned, so three successive runs drain the backlog in order instead of skipping anything. - Batch size 0 means unlimited (SIENTIAPDE-2013) → the configured default (`0`) does not cap the query; 12 pending notifications all come back in one run, verified against a real MongoDB instance rather than assumed from pymongo's documented convention. diff --git a/e2e/test_alerts_main_workflow.py b/e2e/test_alerts_main_workflow.py index 27c3ead..dad0645 100644 --- a/e2e/test_alerts_main_workflow.py +++ b/e2e/test_alerts_main_workflow.py @@ -182,6 +182,73 @@ async def test_scenario_a_2_1_per_group_levels_and_ignore_models_id( assert 'model-x' not in combined +@pytest.mark.e2e +@pytest.mark.asyncio +async def test_scenario_a_2_2_model_ids_allow_list( + temporal_env: WorkflowEnvironment, + orchestrator_worker, + stub_workers, + mongo_uri: str, + smtp_server, +): + """ + A.2.2 (SIENTIAPDE-2149): a group configuring `model_ids` only receives + notifications from those models, on top of the existing query-time/per-group + filtering (SIENTIAPDE-2011/2012). + + group-allow (model_ids=['model-a']) coexists with group-everything (no + model_ids restriction). Because the query-time filter only restricts to an + allow-list union when EVERY active group has one, group-everything's lack + of a model_ids restriction means the query itself fetches both models -- + the per-group filter added by this ticket is what narrows group-allow back + down to model-a only. + """ + seed_receiver_groups( + mongo_uri, + E2E_DATABASE, + [ + default_receiver_group( + group_name='group-allow', contents=['core_alerts'], model_ids=['model-a'] + ), + default_receiver_group(group_name='group-everything', contents=['core_alerts']), + ], + ) + seed_notifications( + mongo_uri, + E2E_DATABASE, + [ + default_notification( + notification_id='err-a', level='ERROR', model_name='model-a', model_id='model-a' + ), + default_notification( + notification_id='err-b', + level='ERROR', + model_name='model-b', + model_id='model-b', + timestamp=datetime(2024, 6, 1, 12, 1, 0, tzinfo=UTC), + ), + ], + ) + + await start_and_await_workflow( + temporal_env.client, + Alerts.run, + load_scenario_input('alerts_happy_path.json'), + make_workflow_id('alerts-2149'), + ) + + # One email per group -- group-allow got only err-a, group-everything got both. + assert len(smtp_server.messages) == 2 + bodies = [extract_html_body(message) for message in smtp_server.messages] + + # Distinguish the two emails by content rather than order: only + # group-everything's email mentions model-b. + allow_body = next(b for b in bodies if 'model-b' not in b) + everything_body = next(b for b in bodies if 'model-b' in b) + assert 'model-a' in allow_body + assert 'model-a' in everything_body + + @pytest.mark.e2e @pytest.mark.asyncio async def test_scenario_a_3_1_empty_queue( diff --git a/e2e/test_subworkflow_load_notification_package.py b/e2e/test_subworkflow_load_notification_package.py index 0eb2bf8..d44cf1a 100644 --- a/e2e/test_subworkflow_load_notification_package.py +++ b/e2e/test_subworkflow_load_notification_package.py @@ -205,6 +205,57 @@ async def test_load_package_query_time_filter_union_and_intersection( assert ids == {'error-included', 'warning-included', 'modely-included'} +@pytest.mark.e2e +@pytest.mark.asyncio +async def test_load_package_query_time_filter_model_ids_union( + temporal_env: WorkflowEnvironment, + orchestrator_worker, + stub_workers, + mongo_uri: str, + redis_client: Redis, +): + """ + SIENTIAPDE-2149: query-time filter unions active groups' `model_ids` + allow-list when every active group sets one, excluding any model outside + that union from the fetch entirely -- not just from a single group's + per-group slice. + + group_a wants only model-a, group_b wants only model-b -- both restrict + levels to ERROR so only the model_ids axis is under test. model-c is + outside the union of both allow-lists and must never be fetched. + """ + seed_receiver_groups( + mongo_uri, + E2E_DATABASE, + [ + default_receiver_group(group_name='group_a', levels=['ERROR'], model_ids=['model-a']), + default_receiver_group(group_name='group_b', levels=['ERROR'], model_ids=['model-b']), + ], + ) + seed_notifications( + mongo_uri, + E2E_DATABASE, + [ + default_notification(notification_id='a-included', level='ERROR', model_id='model-a'), + default_notification(notification_id='b-included', level='ERROR', model_id='model-b'), + default_notification(notification_id='c-excluded', level='ERROR', model_id='model-c'), + ], + ) + + input_data = load_scenario_input('subworkflow_load_notification_package.json') + _build_metadata(input_data) + + result = await start_and_await_workflow( + temporal_env.client, + LoadNotificationPackage.run, + input_data, + make_workflow_id('load-pkg-model-ids-union'), + ) + + ids = {n['notification_id'] for n in result['notification_package']} + assert ids == {'a-included', 'b-included'} + + @pytest.mark.e2e @pytest.mark.asyncio async def test_load_package_empty_mongo_no_redis_write( diff --git a/orchestrator/activities/formatters.py b/orchestrator/activities/formatters.py index 66c69af..f33b4db 100644 --- a/orchestrator/activities/formatters.py +++ b/orchestrator/activities/formatters.py @@ -826,6 +826,7 @@ class Formatters(SientiaMonitoring): - ignore (list, optional): Notification IDs to exclude - levels (list, optional): Notification levels to include - ignore_models_id (list, optional): Model IDs to exclude + - model_ids (list, optional): Model IDs to allow (SIENTIAPDE-2149) Returns: dict[str, Any]: Filtered receiver groups with their notifications, keyed by group_name. @@ -851,6 +852,7 @@ class Formatters(SientiaMonitoring): ignore_list = receiver_group.get('ignore', []) levels_list = receiver_group.get('levels') ignore_models_id_list = receiver_group.get('ignore_models_id') or [] + model_ids_list = receiver_group.get('model_ids') for notification in notification_package: alert_type = 'reports' @@ -868,6 +870,7 @@ class Formatters(SientiaMonitoring): not ignore_models_id_list or notification.get('model_id') not in ignore_models_id_list ) + and (not model_ids_list or notification.get('model_id') in model_ids_list) ): receiver_groups[group_name]['notifications'].append(notification) already_added_keys.append(key) diff --git a/orchestrator/activities/slot_manager.py b/orchestrator/activities/slot_manager.py index 7182551..3be01b6 100644 --- a/orchestrator/activities/slot_manager.py +++ b/orchestrator/activities/slot_manager.py @@ -398,6 +398,7 @@ class SlotManager(SientiaMonitoring): - ignore (list, optional): Notification IDs to exclude - levels (list, optional): Notification levels to include - ignore_models_id (list, optional): Model IDs to exclude + - model_ids (list, optional): Model IDs to allow (SIENTIAPDE-2149) - notification_ttl (int): Seconds before considering notification persistent Returns: @@ -450,6 +451,7 @@ class SlotManager(SientiaMonitoring): ignore_list = receiver_group.get('ignore', []) levels_list = receiver_group.get('levels') ignore_models_id_list = receiver_group.get('ignore_models_id') or [] + model_ids_list = receiver_group.get('model_ids') notifications = [] for notification in notification_package: @@ -462,8 +464,8 @@ class SlotManager(SientiaMonitoring): and self._check_keys_and_notif_id( notification_id, ignore_list, key, already_added_keys ) - and self._check_levels_and_ignore_models_id( - notification, levels_list, ignore_models_id_list + and self._check_levels_and_model_filters( + notification, levels_list, ignore_models_id_list, model_ids_list ) ): notifications.append(notification) @@ -493,23 +495,29 @@ class SlotManager(SientiaMonitoring): return 'do_nothing' @staticmethod - def _check_levels_and_ignore_models_id(notification, levels_list, ignore_models_id_list): + def _check_levels_and_model_filters( + notification, levels_list, ignore_models_id_list, model_ids_list + ): """ - Check if the notification's level is in the levels_list and its model_id is not in the ignore_models_id_list. + Check the notification's level and model_id against a group's filters. Args: notification (dict): The notification to check. levels_list (list, optional): List of levels to include. If None, all levels are included. ignore_models_id_list (list, optional): List of model_ids to exclude. If None, no models are excluded. + model_ids_list (list, optional): List of model_ids to allow (SIENTIAPDE-2149). + If None/empty, all models are allowed. Independent of ignore_models_id_list -- + both apply together when both are configured. Returns: bool: True if the notification passes the checks, False otherwise. """ level_check = not levels_list or notification.get('level') in levels_list - model_id_check = ( + ignore_models_id_check = ( not ignore_models_id_list or notification.get('model_id') not in ignore_models_id_list ) - return level_check and model_id_check + model_ids_check = not model_ids_list or notification.get('model_id') in model_ids_list + return level_check and ignore_models_id_check and model_ids_check @staticmethod def _check_keys_and_notif_id( diff --git a/orchestrator/workflows/subworkflows/load_notification_package.py b/orchestrator/workflows/subworkflows/load_notification_package.py index 614899c..f9b37da 100644 --- a/orchestrator/workflows/subworkflows/load_notification_package.py +++ b/orchestrator/workflows/subworkflows/load_notification_package.py @@ -137,6 +137,12 @@ class LoadNotificationPackage: is only safe to drop from the fetch if EVERY active group ignores it. A group with no `ignore_models_id` is treated as "ignores nothing" (empty set), which naturally zeroes out the intersection via set math. + - model_ids (SIENTIAPDE-2149): union of every group's `model_ids` allow-list, + mirroring `levels`' own union-or-nothing logic. If ANY active group has no + restriction (missing/empty `model_ids`), that group wants every model, so + the query can't restrict to an allow-list at all. `model_ids` and + `ignore_models_id` are independent axes and can both narrow the same + `model_id` field at once (Mongo supports multiple operators on one field). """ if not sending_configs: return default_filter @@ -160,7 +166,21 @@ class LoadNotificationPackage: ] models_intersection = set.intersection(*sending_configs_ignore_models_id) + sending_configs_model_ids = [group.get('model_ids') for group in sending_configs] + + model_id_filter: dict[str, Any] = {} + + if all(sending_configs_model_ids): + model_ids_union: set[str] = set() + for model_ids in sending_configs_model_ids: + if model_ids: + model_ids_union.update(model_ids) + model_id_filter['$in'] = sorted(model_ids_union) + if models_intersection: - query_filter['model_id'] = {'$nin': sorted(models_intersection)} + model_id_filter['$nin'] = sorted(models_intersection) + + if model_id_filter: + query_filter['model_id'] = model_id_filter return query_filter diff --git a/tests/orchestrator/activities/test_formatters.py b/tests/orchestrator/activities/test_formatters.py index 578d335..a7aba4a 100644 --- a/tests/orchestrator/activities/test_formatters.py +++ b/tests/orchestrator/activities/test_formatters.py @@ -852,6 +852,123 @@ def test_filter_notification_reports_ignore_models_id_restriction(formatters): } +def test_filter_notification_reports_model_ids_allow_list_restriction(formatters): + # SIENTIAPDE-2149: a group with `model_ids` set only receives notifications + # from those models, unlike `ignore_models_id` which is a deny-list. + input_data = { + **metadata, + 'notification_package': [ + { + 'trigger': 'test_trigger_1', + 'notification_id': 'test_notification_id_1', + 'model_id': 'model_x', + }, + { + 'trigger': 'test_trigger_2', + 'notification_id': 'test_notification_id_2', + 'model_id': 'model_y', + }, + ], + 'sending_configs': [ + { + 'group_name': 'test_group_1', + 'contents': ['reports'], + 'model_ids': ['model_x'], + }, + ], + } + + response = formatters.filter_notification_reports(input_data) + + assert response == { + 'test_group_1': { + 'group_name': 'test_group_1', + 'contents': ['reports'], + 'model_ids': ['model_x'], + 'notifications': [ + { + 'trigger': 'test_trigger_1', + 'notification_id': 'test_notification_id_1', + 'model_id': 'model_x', + }, + ], + }, + } + + +def test_filter_notification_reports_model_ids_and_ignore_models_id_together(formatters): + # SIENTIAPDE-2149: the allow-list and deny-list apply together (AND), not + # as alternatives -- a model must pass both checks. + input_data = { + **metadata, + 'notification_package': [ + { + 'trigger': 'test_trigger_1', + 'notification_id': 'test_notification_id_1', + 'model_id': 'model_x', + }, + { + 'trigger': 'test_trigger_2', + 'notification_id': 'test_notification_id_2', + 'model_id': 'model_y', + }, + { + 'trigger': 'test_trigger_3', + 'notification_id': 'test_notification_id_3', + 'model_id': 'model_z', + }, + ], + 'sending_configs': [ + { + 'group_name': 'test_group_1', + 'contents': ['reports'], + 'model_ids': ['model_x', 'model_y'], + 'ignore_models_id': ['model_x'], + }, + ], + } + + response = formatters.filter_notification_reports(input_data) + + assert response == { + 'test_group_1': { + 'group_name': 'test_group_1', + 'contents': ['reports'], + 'model_ids': ['model_x', 'model_y'], + 'ignore_models_id': ['model_x'], + 'notifications': [ + { + 'trigger': 'test_trigger_2', + 'notification_id': 'test_notification_id_2', + 'model_id': 'model_y', + }, + ], + }, + } + + +def test_filter_notification_reports_model_ids_explicit_none_does_not_raise(formatters): + # Same defensive concern as ignore_models_id: receiver_groups has no schema + # validation, so model_ids can be explicitly null instead of omitted. + input_data = { + **metadata, + 'notification_package': [ + { + 'trigger': 'test_trigger_1', + 'notification_id': 'test_notification_id_1', + 'model_id': 'model_x', + }, + ], + 'sending_configs': [ + {'group_name': 'test_group_1', 'contents': ['reports'], 'model_ids': None}, + ], + } + + response = formatters.filter_notification_reports(input_data) + + assert response['test_group_1']['notifications'] == input_data['notification_package'] + + def test_filter_notification_reports_no_levels_or_ignore_models_id_passes_everything( formatters, ): diff --git a/tests/orchestrator/activities/test_slot_manager.py b/tests/orchestrator/activities/test_slot_manager.py index 25aa9b5..d92282d 100644 --- a/tests/orchestrator/activities/test_slot_manager.py +++ b/tests/orchestrator/activities/test_slot_manager.py @@ -481,6 +481,137 @@ def test_filter_notification_alerts_ignore_models_id_restriction(slot_manager): } +def test_filter_notification_alerts_model_ids_allow_list_restriction(slot_manager): + # SIENTIAPDE-2149: a group with `model_ids` set only receives notifications + # from those models, unlike `ignore_models_id` which is a deny-list. + slot_manager.redis_repository.get = MagicMock(side_effect=[None, None]) + + input_data = { + **metadata, + 'notification_package': [ + { + 'trigger': 'test_trigger_1', + 'notification_id': 'test_notification_id_1', + 'model_id': 'model_x', + }, + { + 'trigger': 'test_trigger_2', + 'notification_id': 'test_notification_id_2', + 'model_id': 'model_y', + }, + ], + 'sending_configs': [ + { + 'group_name': 'test_group_1', + 'contents': ['core_alerts'], + 'model_ids': ['model_x'], + }, + ], + 'notification_ttl': 300, + 'mail_type': 'test_mail_type', + } + + response = slot_manager.filter_notification_alerts(input_data) + + assert response == { + 'test_group_1': { + 'group_name': 'test_group_1', + 'contents': ['core_alerts'], + 'model_ids': ['model_x'], + 'notifications': [ + { + 'trigger': 'test_trigger_1', + 'notification_id': 'test_notification_id_1', + 'model_id': 'model_x', + }, + ], + }, + } + + +def test_filter_notification_alerts_model_ids_and_ignore_models_id_together(slot_manager): + # SIENTIAPDE-2149: the allow-list and deny-list apply together (AND), not + # as alternatives -- a model must pass both checks. + slot_manager.redis_repository.get = MagicMock(side_effect=[None, None, None]) + + input_data = { + **metadata, + 'notification_package': [ + { + 'trigger': 'test_trigger_1', + 'notification_id': 'test_notification_id_1', + 'model_id': 'model_x', + }, + { + 'trigger': 'test_trigger_2', + 'notification_id': 'test_notification_id_2', + 'model_id': 'model_y', + }, + { + 'trigger': 'test_trigger_3', + 'notification_id': 'test_notification_id_3', + 'model_id': 'model_z', + }, + ], + 'sending_configs': [ + { + 'group_name': 'test_group_1', + 'contents': ['core_alerts'], + 'model_ids': ['model_x', 'model_y'], + 'ignore_models_id': ['model_x'], + }, + ], + 'notification_ttl': 300, + 'mail_type': 'test_mail_type', + } + + response = slot_manager.filter_notification_alerts(input_data) + + # model_x: allowed but ignored -> excluded. model_y: allowed and not + # ignored -> included. model_z: not in the allow-list -> excluded. + assert response == { + 'test_group_1': { + 'group_name': 'test_group_1', + 'contents': ['core_alerts'], + 'model_ids': ['model_x', 'model_y'], + 'ignore_models_id': ['model_x'], + 'notifications': [ + { + 'trigger': 'test_trigger_2', + 'notification_id': 'test_notification_id_2', + 'model_id': 'model_y', + }, + ], + }, + } + + +def test_filter_notification_alerts_model_ids_explicit_none_does_not_raise(slot_manager): + # Same defensive concern as ignore_models_id: receiver_groups has no schema + # validation, so model_ids can be explicitly null instead of omitted. + slot_manager.redis_repository.get = MagicMock(side_effect=[None]) + + input_data = { + **metadata, + 'notification_package': [ + { + 'trigger': 'test_trigger_1', + 'notification_id': 'test_notification_id_1', + 'model_id': 'model_x', + }, + ], + 'sending_configs': [ + {'group_name': 'test_group_1', 'contents': ['core_alerts'], 'model_ids': None}, + ], + 'notification_ttl': 300, + 'mail_type': 'test_mail_type', + } + + response = slot_manager.filter_notification_alerts(input_data) + + assert response['test_group_1']['notifications'] == input_data['notification_package'] + + def test_filter_notification_alerts_no_levels_or_ignore_models_id_passes_everything( slot_manager, ): diff --git a/tests/orchestrator/workflows/subworkflows/test_load_notification_package.py b/tests/orchestrator/workflows/subworkflows/test_load_notification_package.py index 3266f70..f5865c4 100644 --- a/tests/orchestrator/workflows/subworkflows/test_load_notification_package.py +++ b/tests/orchestrator/workflows/subworkflows/test_load_notification_package.py @@ -237,6 +237,65 @@ def test_compute_query_time_filter_model_ignored_by_only_one_group_not_excluded( assert 'model_id' not in result +def test_compute_query_time_filter_model_ids_union_when_all_groups_restrict( + load_notification_package, +): + # SIENTIAPDE-2149: every active group sets a model_ids allow-list -> the + # union applies at query time, mirroring how `levels` already behaves. + sending_configs = [ + {'group_name': 'group_a', 'model_ids': ['model_a']}, + {'group_name': 'group_b', 'model_ids': ['model_b']}, + ] + default_filter = {} + + result = load_notification_package._compute_query_time_filter(sending_configs, default_filter) + + assert result == {'model_id': {'$in': ['model_a', 'model_b']}} + + +def test_compute_query_time_filter_model_ids_no_restriction_when_any_group_missing( + load_notification_package, +): + # One active group has no model_ids restriction -> it wants every model, + # so the query can't restrict to an allow-list at all. + sending_configs = [ + {'group_name': 'group_a', 'model_ids': ['model_a']}, + {'group_name': 'group_b'}, + ] + default_filter = {} + + result = load_notification_package._compute_query_time_filter(sending_configs, default_filter) + + assert 'model_id' not in result + + +def test_compute_query_time_filter_model_ids_union_and_ignore_models_id_intersection_together( + load_notification_package, +): + # SIENTIAPDE-2149: the allow-list union ($in) and the ignore-list + # intersection ($nin) are independent axes and can both narrow the same + # model_id field at once. + sending_configs = [ + { + 'group_name': 'group_a', + 'model_ids': ['model_a', 'model_b'], + 'ignore_models_id': ['model_a'], + }, + { + 'group_name': 'group_b', + 'model_ids': ['model_b', 'model_c'], + 'ignore_models_id': ['model_a'], + }, + ] + default_filter = {} + + result = load_notification_package._compute_query_time_filter(sending_configs, default_filter) + + assert result == { + 'model_id': {'$in': ['model_a', 'model_b', 'model_c'], '$nin': ['model_a']}, + } + + def test_compute_query_time_filter_union_levels_and_intersection_models( load_notification_package, ):