diff --git a/backend/omni/CHANGELOG.md b/backend/omni/CHANGELOG.md index 49a425a..818419c 100644 --- a/backend/omni/CHANGELOG.md +++ b/backend/omni/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Config can include other config files - OpenAPI tool registry now supports `tool_servers` and auto-registers all `operationId` endpoints from each OpenAPI spec as tools -### Chnaged +### Changed - Config to be used can be defined via CONFIG_PATH environment variable - Refactored tools contract to use request-aware registry methods: `get_tools(request)` and `run_tool(request, params)` diff --git a/backend/omni/config.sample.yaml b/backend/omni/config.sample.yaml index 72a583b..2257b6e 100644 --- a/backend/omni/config.sample.yaml +++ b/backend/omni/config.sample.yaml @@ -3,11 +3,9 @@ includes: modules: # Deactivate authentication completely for development purposes. - auth_oidc: - collision_strategy: drop - session: + auth_oidc: !drop + session: !override class: modai.modules.session.dev_mock_session.DevMockSessionModule - collision_strategy: replace config: user_id: "dev-user" email: "dev@example.com" diff --git a/backend/omni/docs/architecture/core.md b/backend/omni/docs/architecture/core.md index a4fb502..079b0ee 100644 --- a/backend/omni/docs/architecture/core.md +++ b/backend/omni/docs/architecture/core.md @@ -56,7 +56,6 @@ modules: class: modai.modules.health.simple_health_module.SimpleHealthModule my_module: class: modai.modules.example.ExampleModule - collision_strategy: merge # optional, see below config: some_key: some_value ``` @@ -89,19 +88,51 @@ B modules (loaded 2nd — overwrites A on collision) root modules (loaded last — highest precedence, always wins) ``` -##### `collision_strategy` — handling duplicate module names +##### `!override` and `!drop` — controlling merge behaviour -When the same module name appears in multiple config files, the -`collision_strategy` field on the **incoming** (later-loaded) module decides what -happens. Because root modules are always applied last as incoming, setting -`collision_strategy` on a **root module** controls how it interacts with a module -already loaded from an include. +By default, when the same module name (or nested key) appears in multiple config +files, values are **deep-merged**: dict keys are combined recursively and the +incoming value wins on shared keys; lists are **concatenated** (base items first, +then incoming items). -| Value | Behaviour | +Two YAML tags can change this behaviour at **any level** of the document: + +| Tag | Behaviour | |---|---| -| `merge` *(default)* | Deep-merges the incoming module into the existing one. Keys present only in the base (earlier-loaded) module are preserved. When the same key exists in both, the **incoming value wins**. Nested dicts are merged recursively with the same rule. | -| `replace` | The incoming module definition **completely replaces** the existing one. | -| `drop` | The incoming module is **not added**, and any existing module with the same name is **removed**. Modules with different names are unaffected. | +| *(no tag, default)* | Deep-merge. Dict keys merged recursively, incoming wins on shared keys. Lists concatenated (base + incoming). | +| `!override` | The tagged value **completely replaces** the base value at that level — no merge is performed at this level or below. | +| `!drop` | The tagged key (or module) is **removed** from the base and is not re-added. | + +Because the tags can appear on **any YAML node**, the granularity is flexible: +drop or override an entire module, a single config section, or a single list. + +**Examples:** + +```yaml +# Drop an entire module (remove it from the base, don't add this one) +modules: + auth_oidc: !drop + +# Override an entire module (replace, don't merge) +modules: + session: !override + class: modai.modules.session.dev_mock_session.DevMockSessionModule + config: + user_id: dev-user + +# Override only the config sub-tree (merge the module, but replace config) +modules: + my_module: + class: modai.modules.example.ExampleModule + config: !override + key: value + +# Drop a single nested key during merge +modules: + my_module: + config: + obsolete_key: !drop +``` diff --git a/backend/omni/src/modai/modules/startup_config/__tests__/test_config_loader.py b/backend/omni/src/modai/modules/startup_config/__tests__/test_config_loader.py index 5656bce..6a85916 100644 --- a/backend/omni/src/modai/modules/startup_config/__tests__/test_config_loader.py +++ b/backend/omni/src/modai/modules/startup_config/__tests__/test_config_loader.py @@ -87,21 +87,15 @@ def test_includes_merges_modules_from_included_file(tmp_path: Path): } -def test_includes_collision_strategy_replace(tmp_path: Path): - """collision_strategy=replace on a root module: root fully replaces the included one.""" +def test_includes_override_tag_replaces_module(tmp_path: Path): + """!override on a root module: root fully replaces the included one.""" main = tmp_path / "config.yaml" main.write_text( - yaml.dump( - { - "includes": [{"path": "extra.yaml"}], - "modules": { - "health": { - "class": "a.b.HealthRoot", - "collision_strategy": "replace", - } - }, - } - ) + "includes:\n" + " - path: extra.yaml\n" + "modules:\n" + " health: !override\n" + " class: a.b.HealthRoot\n" ) included = tmp_path / "extra.yaml" @@ -130,8 +124,8 @@ def test_includes_collision_strategy_replace(tmp_path: Path): } -def test_includes_collision_strategy_merge_default(tmp_path: Path): - """Without collision_strategy the default is merge: root wins on collision, include-only keys survive.""" +def test_includes_merge_default(tmp_path: Path): + """Default (no tag) is merge: root wins on collision, include-only keys survive.""" main = tmp_path / "config.yaml" main.write_text( yaml.dump( @@ -171,8 +165,10 @@ def test_includes_collision_strategy_merge_default(tmp_path: Path): } -def test_includes_collision_strategy_merge_explicit(tmp_path: Path): - """Explicit collision_strategy=merge on a root module behaves identically to the default.""" +def test_includes_merge_does_not_remove_existing_keys( + tmp_path: Path, +): + """merge never removes keys that already exist in the included module.""" main = tmp_path / "config.yaml" main.write_text( yaml.dump( @@ -181,7 +177,7 @@ def test_includes_collision_strategy_merge_explicit(tmp_path: Path): "modules": { "health": { "class": "a.b.HealthRoot", - "collision_strategy": "merge", + "config": {"root_section": {"value": 1}}, } }, } @@ -195,7 +191,7 @@ def test_includes_collision_strategy_merge_explicit(tmp_path: Path): "modules": { "health": { "class": "a.b.HealthIncluded", - "config": {"extra_key": "added"}, + "config": {"include_key": True}, } } } @@ -209,31 +205,70 @@ def test_includes_collision_strategy_merge_explicit(tmp_path: Path): "modules": { "health": { "class": "a.b.HealthRoot", - "config": {"extra_key": "added"}, + "config": { + "root_section": {"value": 1}, + "include_key": True, + }, } } } -def test_includes_collision_strategy_merge_does_not_remove_existing_keys( - tmp_path: Path, -): - """merge never removes keys that already exist in the included module.""" +def test_includes_override_tag_on_nested_key(tmp_path: Path): + """!override on a nested config key replaces that sub-tree without merging.""" main = tmp_path / "config.yaml" main.write_text( + "includes:\n" + " - path: extra.yaml\n" + "modules:\n" + " health:\n" + " class: a.b.HealthRoot\n" + " config: !override\n" + " new_key: new_value\n" + ) + + included = tmp_path / "extra.yaml" + included.write_text( yaml.dump( { - "includes": [{"path": "extra.yaml"}], "modules": { "health": { - "class": "a.b.HealthRoot", - "config": {"root_section": {"value": 1}}, + "class": "a.b.HealthIncluded", + "config": {"old_key": "old_value", "other_key": 42}, } - }, + } } ) ) + loader = YamlConfigModule(ModuleDependencies(), {"config_path": str(main)}) + config = loader.get_config() + + # config is fully replaced; old_key from include is gone + assert config == { + "modules": { + "health": { + "class": "a.b.HealthRoot", + "config": {"new_key": "new_value"}, + } + } + } + + +def test_includes_drop_tag_on_nested_key(tmp_path: Path): + """!drop on a nested key removes that key from the merged result.""" + main = tmp_path / "config.yaml" + main.write_text( + "includes:\n" + " - path: extra.yaml\n" + "modules:\n" + " health:\n" + " class: a.b.HealthRoot\n" + " config:\n" + " keep_key: kept\n" + " remove_key: !drop\n" + ) + included = tmp_path / "extra.yaml" included.write_text( yaml.dump( @@ -241,7 +276,7 @@ def test_includes_collision_strategy_merge_does_not_remove_existing_keys( "modules": { "health": { "class": "a.b.HealthIncluded", - "config": {"include_key": True}, + "config": {"keep_key": "base", "remove_key": "base_value"}, } } } @@ -255,11 +290,94 @@ def test_includes_collision_strategy_merge_does_not_remove_existing_keys( "modules": { "health": { "class": "a.b.HealthRoot", - "config": { - "root_section": {"value": 1}, - "include_key": True, + "config": {"keep_key": "kept"}, + } + } + } + + +def test_default_list_merge(tmp_path: Path): + """Lists are concatenated (base first, then incoming) by default.""" + main = tmp_path / "config.yaml" + main.write_text( + yaml.dump( + { + "includes": [{"path": "extra.yaml"}], + "modules": { + "svc": { + "class": "a.b.Svc", + "config": {"items": ["c", "d"]}, + } }, } + ) + ) + + included = tmp_path / "extra.yaml" + included.write_text( + yaml.dump( + { + "modules": { + "svc": { + "class": "a.b.Svc", + "config": {"items": ["a", "b"]}, + } + } + } + ) + ) + + loader = YamlConfigModule(ModuleDependencies(), {"config_path": str(main)}) + config = loader.get_config() + + assert config == { + "modules": { + "svc": { + "class": "a.b.Svc", + "config": {"items": ["a", "b", "c", "d"]}, + } + } + } + + +def test_override_tag_on_list_replaces_base_list(tmp_path: Path): + """!override on a list value replaces the base list instead of concatenating.""" + main = tmp_path / "config.yaml" + main.write_text( + "includes:\n" + " - path: extra.yaml\n" + "modules:\n" + " svc:\n" + " class: a.b.Svc\n" + " config:\n" + " items: !override\n" + " - c\n" + " - d\n" + ) + + included = tmp_path / "extra.yaml" + included.write_text( + yaml.dump( + { + "modules": { + "svc": { + "class": "a.b.Svc", + "config": {"items": ["a", "b"]}, + } + } + } + ) + ) + + loader = YamlConfigModule(ModuleDependencies(), {"config_path": str(main)}) + config = loader.get_config() + + assert config == { + "modules": { + "svc": { + "class": "a.b.Svc", + "config": {"items": ["c", "d"]}, + } } } @@ -367,19 +485,16 @@ def test_includes_nested_includes_are_rejected(tmp_path: Path): loader.get_config() -def test_includes_collision_strategy_drop_removes_base_and_incoming(tmp_path: Path): - """collision_strategy=drop on an incoming module removes both the base and incoming entries.""" +def test_includes_drop_tag_removes_base_and_incoming(tmp_path: Path): + """!drop on an incoming module removes both the base and incoming entries.""" main = tmp_path / "config.yaml" main.write_text( - yaml.dump( - { - "includes": [{"path": "extra.yaml"}], - "modules": { - "health": {"collision_strategy": "drop"}, - "other": {"class": "a.b.Other"}, - }, - } - ) + "includes:\n" + " - path: extra.yaml\n" + "modules:\n" + " health: !drop\n" + " other:\n" + " class: a.b.Other\n" ) included = tmp_path / "extra.yaml" @@ -400,19 +515,10 @@ def test_includes_collision_strategy_drop_removes_base_and_incoming(tmp_path: Pa assert config == {"modules": {"other": {"class": "a.b.Other"}}} -def test_includes_collision_strategy_drop_no_existing_module(tmp_path: Path): - """collision_strategy=drop on an incoming module with no prior base entry is simply not added.""" +def test_includes_drop_tag_no_existing_module(tmp_path: Path): + """!drop on an incoming module with no prior base entry is simply not added.""" main = tmp_path / "config.yaml" - main.write_text( - yaml.dump( - { - "modules": { - "health": {"collision_strategy": "drop"}, - "other": {"class": "a.b.Other"}, - }, - } - ) - ) + main.write_text("modules:\n health: !drop\n other:\n class: a.b.Other\n") loader = YamlConfigModule(ModuleDependencies(), {"config_path": str(main)}) config = loader.get_config() @@ -421,19 +527,16 @@ def test_includes_collision_strategy_drop_no_existing_module(tmp_path: Path): assert config == {"modules": {"other": {"class": "a.b.Other"}}} -def test_includes_collision_strategy_drop_successor_modules_unaffected(tmp_path: Path): - """Modules defined after the drop module in the same config are loaded normally.""" +def test_includes_drop_tag_successor_modules_unaffected(tmp_path: Path): + """Modules defined after the !drop module in the same config are loaded normally.""" main = tmp_path / "config.yaml" main.write_text( - yaml.dump( - { - "includes": [{"path": "extra.yaml"}], - "modules": { - "health": {"collision_strategy": "drop"}, - "successor": {"class": "a.b.Successor"}, - }, - } - ) + "includes:\n" + " - path: extra.yaml\n" + "modules:\n" + " health: !drop\n" + " successor:\n" + " class: a.b.Successor\n" ) included = tmp_path / "extra.yaml" diff --git a/backend/omni/src/modai/modules/startup_config/yaml_config_module.py b/backend/omni/src/modai/modules/startup_config/yaml_config_module.py index e810a76..42dd775 100644 --- a/backend/omni/src/modai/modules/startup_config/yaml_config_module.py +++ b/backend/omni/src/modai/modules/startup_config/yaml_config_module.py @@ -18,6 +18,46 @@ ## __init__ function signature. +class _OverrideValue: + """Sentinel produced by the ``!override`` YAML tag. + + The tagged value completely replaces the corresponding base value — no + deep-merge is performed at this level or below. + """ + + def __init__(self, value: Any): + self.value = value + + +class _DropValue: + """Sentinel produced by the ``!drop`` YAML tag. + + The tagged key (or module) is removed from the base and is not re-added. + """ + + +class _ModaiYamlLoader(yaml.SafeLoader): + """SafeLoader extended with ``!override`` and ``!drop`` tag support.""" + + +def _construct_override(loader: yaml.SafeLoader, node: yaml.Node) -> _OverrideValue: + if isinstance(node, yaml.MappingNode): + value = loader.construct_mapping(node, deep=True) + elif isinstance(node, yaml.SequenceNode): + value = loader.construct_sequence(node, deep=True) + else: + value = loader.construct_scalar(node) + return _OverrideValue(value) + + +def _construct_drop(loader: yaml.SafeLoader, node: yaml.Node) -> _DropValue: + return _DropValue() + + +_ModaiYamlLoader.add_constructor("!override", _construct_override) +_ModaiYamlLoader.add_constructor("!drop", _construct_drop) + + class YamlConfigModule(StartupConfig): """Default implementation of the StartupConfig module.""" @@ -52,8 +92,7 @@ def _load_config(self, config_path: Path) -> dict[str, Any]: 2. Each subsequent include is applied in order, top-to-bottom. Later includes overwrite earlier ones on ``merge`` collisions. 3. The root config's own modules are applied last and always win - (highest priority). The ``collision_strategy`` on a root module - controls how it merges with an already-loaded module from an include. + (highest priority). Example: root → [A, B] @@ -92,7 +131,7 @@ def _load_config(self, config_path: Path) -> dict[str, Any]: def _load_yaml_file(self, path: Path) -> dict[str, Any]: with open(path, "r") as file: - config = yaml.safe_load(file) + config = yaml.load(file, Loader=_ModaiYamlLoader) if config is None: raise ValueError(f"Config file is empty or invalid: {path}") return _resolve_env_vars(config) @@ -100,20 +139,23 @@ def _load_yaml_file(self, path: Path) -> dict[str, Any]: def _apply_config( self, base_modules: dict[str, Any], new_modules: dict[str, Any] ) -> dict[str, Any]: - """Merge *config_modules* onto *base_modules* and return the result. + """Merge *new_modules* onto *base_modules* and return the result. * Modules present only in *base_modules* are kept as-is. - * Modules present only in *config_modules* are added unconditionally. - * On a name collision the ``collision_strategy`` field on the **incoming** - (*config_modules*) entry decides: - - - ``drop`` – the incoming entry is not added, and any existing entry - with the same name in *base_modules* is also removed. Modules with - different names are not affected. - - ``replace`` – the incoming entry completely replaces the existing one. - - ``merge`` *(default)* – deep-merged; base-only keys are preserved, + * Modules present only in *new_modules* are added unconditionally. + * On a name collision the merge behaviour is controlled by YAML tags + placed on the **incoming** (*new_modules*) entry: + + - ``!drop`` – the incoming entry is not added, and any existing entry + with the same name in *base_modules* is also removed. + - ``!override`` – the incoming entry completely replaces the existing + one; no deep-merge is performed. + - *(no tag, default)* – deep-merged; base-only keys are preserved, shared keys are overwritten by the incoming value, nested dicts - recurse with the same rule. + recurse with the same rule, lists are concatenated (base + incoming). + + The ``!override`` and ``!drop`` tags may also be placed on any nested + key within a module definition for fine-grained control. Because the root config's modules are always passed last (see ``_load_config``), they act as the final incoming and always win. @@ -121,28 +163,26 @@ def _apply_config( result = dict(base_modules) for name, cfg in new_modules.items(): - cfg = cfg or {} - strategy = cfg.get("collision_strategy", "merge") - if strategy == "drop": + if isinstance(cfg, _DropValue): result.pop(name, None) continue + if isinstance(cfg, _OverrideValue): + result[name] = cfg.value + continue if name not in result: result[name] = cfg - elif strategy == "replace": - result[name] = cfg else: - result[name] = _deep_merge(result[name] or {}, cfg) - - # Strip the parsing-only key from every module in the result. - for cfg in result.values(): - if isinstance(cfg, dict): - cfg.pop("collision_strategy", None) + result[name] = _deep_merge(result[name] or {}, cfg or {}) return result def _resolve_env_vars(obj: Any) -> Any: - if isinstance(obj, dict): + if isinstance(obj, _OverrideValue): + return _OverrideValue(_resolve_env_vars(obj.value)) + elif isinstance(obj, _DropValue): + return obj + elif isinstance(obj, dict): return {k: _resolve_env_vars(v) for k, v in obj.items()} elif isinstance(obj, list): return [_resolve_env_vars(i) for i in obj] @@ -160,13 +200,25 @@ def _deep_merge(base: dict[str, Any], incoming: dict[str, Any]) -> dict[str, Any - Keys present only in *base* are preserved (nothing is removed). - Keys present only in *incoming* are added. - - When both sides share a key: if both values are dicts, the merge recurses; - otherwise the *incoming* value overwrites the base value. + - ``!drop`` on an incoming value removes the key from the result. + - ``!override`` on an incoming value uses it as-is (no further merging). + - When both sides share a key: dicts are merged recursively, lists are + concatenated (base first, then incoming), other values are replaced by + the incoming value. """ result = dict(base) for key, value in incoming.items(): - if key in result and isinstance(result[key], dict) and isinstance(value, dict): - result[key] = _deep_merge(result[key], value) + if isinstance(value, _DropValue): + result.pop(key, None) + elif isinstance(value, _OverrideValue): + result[key] = value.value + elif key in result: + if isinstance(result[key], dict) and isinstance(value, dict): + result[key] = _deep_merge(result[key], value) + elif isinstance(result[key], list) and isinstance(value, list): + result[key] = result[key] + value + else: + result[key] = value else: result[key] = value return result