feat(merge_configs): expose public merge_configs(*dicts) API per §3.1 deep-merge algorithm #19

2026-06-03T09:07:44Z

hurui200320 commented

2026-06-03 09:07:44 +00:00

Summary

Implements the public merge_configs(*dicts) function per Actor Configuration Standard §3.1, as required by Wave 2 of the CleverActors integration epic (cleveragents/cleveragents-webapp#275).

The CleverThis router needs to combine platform base configs with per-actor configs stored in the database (injecting default system prompts, org-level LLM settings, per-tenant overrides) without mutating either input. The existing internal ReactiveConfigParser._merge_configs() method was unsuitable: it mutates its base arg, only accepts two dicts, and is not publicly importable.

What Was Done

New module: `src/cleveractors/config_utils.py`

merge_configs(*dicts: dict[str, Any]) -> dict[str, Any] — variadic, immutable, public.
Runtime type guard — non-dict arguments raise TypeError immediately (fail-fast per CONTRIBUTING.md).
Implements §3.1 semantics via _apply_merge() internal helper:
- Key absent from accumulated result → deep-copied and added.
- Both values are dict → deep-merged (via explicit stack, not recursion).
- Both values are list → existing + deep copy of new.
- Otherwise → deep copy of new value replaces existing.
Zero-argument call returns {}.
All inputs are never mutated (copy.deepcopy used for all stored values).
Debug logging — logger.debug() calls on all three merge branches (dict merge, list append, value replacement) for operational traceability.
Non-recursive implementation — explicit LIFO stack replaces recursion, eliminating risk of RecursionError on deeply nested configs.
Documented caveats — circular reference warning, deepcopy security note, O(k²·m) list concatenation complexity.

Export: `src/cleveractors/init.py`

merge_configs added to the module import and __all__.
Consumers can now do from cleveractors import merge_configs.

Tests

BDD (Behave): features/merge_configs.feature — 20 scenarios covering: zero-args, single-dict deep copy (including nested identity check), empty-dict transparency (both positions), TypeError on None (as second arg, first arg, and only arg), absent-key insertion, scalar override, recursive mapping merge, sequence append, lists inside nested dicts, three-dict chained merge, deep nesting, input mutation guard, result-to-input mutation independence, list→scalar and dict→scalar type-mismatch replacements, and scalar→list and scalar→dict reverse type-mismatch replacements.
Integration (Robot Framework): 5 test cases in robot/config.robot including an end-to-end pipeline test that loads YAML via ConfigurationManager and merges an overlay through merge_configs.
Benchmarks (ASV): benchmarks/merge_configs_benchmark.py — flat merge, nested merge, sequence append, three-way merge (50 keys each with overlap), and zero-arg baseline.

Project Files

Added CONTRIBUTORS.md with contributor entry per CONTRIBUTING.md.
Added entry to CHANGELOG.md under [Unreleased] > Added.

Quality Gates

Gate	Result
`ruff check`	✅ All checks passed
`pyright` (strict)	✅ 0 errors, 0 warnings (1 pre-existing info for optional dep)
Behave unit tests	✅ 1681/1681 scenarios pass (20 new)
Robot integration tests	✅ 54/54 tests pass (5 new)
Coverage	✅ 96.9% (`config_utils.py`: 100%)

Known Limitations

Docstring style: config_utils.py now uses plain/minimal docstrings consistent with the rest of the codebase.
Circular references: Input dicts with self-referential cycles are not supported (documented in module docstring).

Design Notes

The internal _merge_configs instance method on ReactiveConfigParser is unchanged to avoid breaking the existing file-parsing path.
No new external dependencies introduced.
The _apply_merge helper is intentionally private (module-level _ prefix) — only merge_configs is part of the public contract.
Replaced recursive _apply_merge with explicit stack-based iteration and added logger.debug() calls across all merge branches.
Removed unused import sys from robot/CleverActorsLib.py.
Removed empty teardown method from benchmarks.
Mutation guard step in BDD now uses dynamic path discovery instead of hardcoded keys.

Closes #11

## Summary Implements the public `merge_configs(*dicts)` function per Actor Configuration Standard §3.1, as required by Wave 2 of the CleverActors integration epic (cleveragents/cleveragents-webapp#275). The CleverThis router needs to combine platform base configs with per-actor configs stored in the database (injecting default system prompts, org-level LLM settings, per-tenant overrides) without mutating either input. The existing internal `ReactiveConfigParser._merge_configs()` method was unsuitable: it mutates its `base` arg, only accepts two dicts, and is not publicly importable. ## What Was Done ### New module: `src/cleveractors/config_utils.py` - `merge_configs(*dicts: dict[str, Any]) -> dict[str, Any]` — variadic, immutable, public. - **Runtime type guard** — non-dict arguments raise `TypeError` immediately (fail-fast per CONTRIBUTING.md). - Implements §3.1 semantics via `_apply_merge()` internal helper: - Key absent from accumulated result → deep-copied and added. - Both values are `dict` → deep-merged (via explicit stack, not recursion). - Both values are `list` → existing + deep copy of new. - Otherwise → deep copy of new value replaces existing. - Zero-argument call returns `{}`. - All inputs are never mutated (`copy.deepcopy` used for all stored values). - **Debug logging** — `logger.debug()` calls on all three merge branches (dict merge, list append, value replacement) for operational traceability. - **Non-recursive implementation** — explicit LIFO stack replaces recursion, eliminating risk of `RecursionError` on deeply nested configs. - **Documented caveats** — circular reference warning, `deepcopy` security note, O(k²·m) list concatenation complexity. ### Export: `src/cleveractors/__init__.py` - `merge_configs` added to the module import and `__all__`. - Consumers can now do `from cleveractors import merge_configs`. ### Tests - **BDD (Behave):** `features/merge_configs.feature` — 20 scenarios covering: zero-args, single-dict deep copy (including nested identity check), empty-dict transparency (both positions), TypeError on None (as second arg, first arg, and only arg), absent-key insertion, scalar override, recursive mapping merge, sequence append, lists inside nested dicts, three-dict chained merge, deep nesting, input mutation guard, result-to-input mutation independence, list→scalar and dict→scalar type-mismatch replacements, and scalar→list and scalar→dict reverse type-mismatch replacements. - **Integration (Robot Framework):** 5 test cases in `robot/config.robot` including an end-to-end pipeline test that loads YAML via `ConfigurationManager` and merges an overlay through `merge_configs`. - **Benchmarks (ASV):** `benchmarks/merge_configs_benchmark.py` — flat merge, nested merge, sequence append, three-way merge (50 keys each with overlap), and zero-arg baseline. ### Project Files - Added `CONTRIBUTORS.md` with contributor entry per CONTRIBUTING.md. - Added entry to `CHANGELOG.md` under `[Unreleased] > Added`. ## Quality Gates | Gate | Result | |------|--------| | `ruff check` | ✅ All checks passed | | `pyright` (strict) | ✅ 0 errors, 0 warnings (1 pre-existing info for optional dep) | | Behave unit tests | ✅ 1681/1681 scenarios pass (20 new) | | Robot integration tests | ✅ 54/54 tests pass (5 new) | | Coverage | ✅ 96.9% (`config_utils.py`: 100%) | ## Known Limitations - **Docstring style:** `config_utils.py` now uses plain/minimal docstrings consistent with the rest of the codebase. - **Circular references:** Input dicts with self-referential cycles are not supported (documented in module docstring). ## Design Notes - The internal `_merge_configs` instance method on `ReactiveConfigParser` is unchanged to avoid breaking the existing file-parsing path. - No new external dependencies introduced. - The `_apply_merge` helper is intentionally private (module-level `_` prefix) — only `merge_configs` is part of the public contract. - Replaced recursive `_apply_merge` with explicit stack-based iteration and added `logger.debug()` calls across all merge branches. - Removed unused `import sys` from `robot/CleverActorsLib.py`. - Removed empty `teardown` method from benchmarks. - Mutation guard step in BDD now uses dynamic path discovery instead of hardcoded keys. Closes #11

hurui200320 added the

Type

Feature

label 2026-06-03 09:07:50 +00:00

hurui200320 referenced this pull request

2026-06-03 09:08:22 +00:00

feat(merge_configs): expose public merge_configs(*dicts) API per §3.1 deep-merge algorithm #11

hurui200320 added a new dependency 2026-06-03 09:08:50 +00:00

#11 feat(merge_configs): expose public merge_configs(*dicts) API per §3.1 deep-merge algorithm

hurui200320 force-pushed feature/merge-configs-api from 5825a643f5 to fe37c4cdb3

2026-06-03 09:10:15 +00:00

Compare

hurui200320 force-pushed feature/merge-configs-api from fe37c4cdb3 to bf991b32c2

2026-06-03 10:04:21 +00:00

Compare

hurui200320 referenced this pull request

2026-06-03 10:24:50 +00:00

feat(merge_configs): expose public merge_configs(*dicts) API per §3.1 deep-merge algorithm #11

hurui200320 requested review from CoreRasurae 2026-06-03 10:44:30 +00:00

hurui200320 force-pushed feature/merge-configs-api from bf991b32c2 to ba439b3920

2026-06-03 10:51:00 +00:00

Compare

hurui200320 force-pushed feature/merge-configs-api from ba439b3920 to 7f5cd145a6

2026-06-03 10:53:17 +00:00

Compare

hurui200320 commented

2026-06-03 11:04:02 +00:00

Note on the implementation: The ticket description mentioned a method _merge_configs, however, this PR didn't remove or reuse it despite they implement the same merge rule. The reason is: the existing _merge_configs method will perform merge in-place, which means the input will be changed, and it's an internal method to the file based actor yaml parser. The new method is a public method and is targeting the general use. In the long run, I'd prefer to have the library get rid of the ReactiveConfigParser, and provide a pydantic model for the config, and a series of method to process it: for example, the client will read the yaml files either from disk, HTTP request or database, then call this merge method to merge into one dict, then call validate to verify the config, etc.

To not break the cleveragents-core, I didn't touch the existing merge method and it's processing path.

Note on the implementation: The ticket description mentioned a method `_merge_configs`, however, this PR didn't remove or reuse it despite they implement the same merge rule. The reason is: the existing `_merge_configs` method will perform merge in-place, which means the input will be changed, and it's an internal method to the file based actor yaml parser. The new method is a public method and is targeting the general use. In the long run, I'd prefer to have the library get rid of the `ReactiveConfigParser`, and provide a pydantic model for the config, and a series of method to process it: for example, the client will read the yaml files either from disk, HTTP request or database, then call this merge method to merge into one dict, then call validate to verify the config, etc. To not break the cleveragents-core, I didn't touch the existing merge method and it's processing path.

hurui200320 commented

2026-06-03 11:05:59 +00:00

Design Note: `merge_configs` vs `_merge_configs`

A natural question when reviewing this PR is: why add a new merge_configs function instead of reusing or removing the existing ReactiveConfigParser._merge_configs() instance method? They implement the same §3.1 rules, but with two meaningful behavioural differences that make them unsuitable to share an implementation.

The Two Differences

1. Mutation vs. Immutability

	`_merge_configs` (internal)	`merge_configs` (public)
`base` dict	Mutated in-place	Never touched
Values stored	Assigned by reference (`base[key] = value`)	Always `copy.deepcopy(value)`
`None` input	Silently returns (`if new is None: return`)	Raises `TypeError`

_merge_configs is designed to be called in a loop inside parse_files() where combined_config is an internal accumulator that the parser owns. Mutating it in-place is safe there. The public function cannot make that assumption — callers own their dicts and must not see them modified.

2. List append: `extend` vs. concatenation

# _merge_configs — mutates the existing list in-place
base[key].extend(value)

# merge_configs — creates a fresh list
base[key] = existing + copy.deepcopy(new_value)

Both produce the same final content. The difference is that extend mutates the list already sitting in the accumulator, while the new version creates a new list. For the public API, this ensures the result is fully independent from all inputs.

Why Not Reuse or Remove `_merge_configs`?

The two functions serve different contracts:

_merge_configs is a parser-internal tool — an instance method called only by parse_files(), operating on a mutable accumulator it owns. Its None guard exists because yaml.safe_load legitimately returns None on an empty or comment-only YAML file, which is a normal condition during file loading.
merge_configs is a public API — it must be safe to call from any external code, must never mutate caller-owned dicts, and should fail loudly on bad input rather than silently swallowing None.

Reusing _merge_configs as the backing implementation would have required either changing its mutation semantics (breaking the existing parse path) or wrapping it with an awkward deep-copy layer. A clean, dedicated function in config_utils.py is the correct separation.

In short: same algorithm, different ownership model. Both are intentionally kept.

Note posted by Rui's AI agent (Claude Sonnet 4.6) during self-QA of this PR.

## Design Note: `merge_configs` vs `_merge_configs` A natural question when reviewing this PR is: why add a new `merge_configs` function instead of reusing or removing the existing `ReactiveConfigParser._merge_configs()` instance method? They implement the same §3.1 rules, but with two meaningful behavioural differences that make them unsuitable to share an implementation. ### The Two Differences #### 1. Mutation vs. Immutability | | `_merge_configs` (internal) | `merge_configs` (public) | |---|---|---| | **`base` dict** | Mutated in-place | Never touched | | **Values stored** | Assigned by reference (`base[key] = value`) | Always `copy.deepcopy(value)` | | **`None` input** | Silently returns (`if new is None: return`) | Raises `TypeError` | `_merge_configs` is designed to be called in a loop inside `parse_files()` where `combined_config` is an internal accumulator that the parser owns. Mutating it in-place is safe there. The public function cannot make that assumption — callers own their dicts and must not see them modified. #### 2. List append: `extend` vs. concatenation ```python # _merge_configs — mutates the existing list in-place base[key].extend(value) # merge_configs — creates a fresh list base[key] = existing + copy.deepcopy(new_value) ``` Both produce the same final content. The difference is that `extend` mutates the list already sitting in the accumulator, while the new version creates a new list. For the public API, this ensures the result is fully independent from all inputs. ### Why Not Reuse or Remove `_merge_configs`? The two functions serve **different contracts**: - **`_merge_configs`** is a parser-internal tool — an instance method called only by `parse_files()`, operating on a mutable accumulator it owns. Its `None` guard exists because `yaml.safe_load` legitimately returns `None` on an empty or comment-only YAML file, which is a normal condition during file loading. - **`merge_configs`** is a public API — it must be safe to call from any external code, must never mutate caller-owned dicts, and should fail loudly on bad input rather than silently swallowing `None`. Reusing `_merge_configs` as the backing implementation would have required either changing its mutation semantics (breaking the existing parse path) or wrapping it with an awkward deep-copy layer. A clean, dedicated function in `config_utils.py` is the correct separation. **In short: same algorithm, different ownership model. Both are intentionally kept.** --- *Note posted by Rui's AI agent (Claude Sonnet 4.6) during self-QA of this PR.*

CoreRasurae reviewed 2026-06-03 11:40:16 +00:00

CoreRasurae left a comment

Reviewed by a human, some comments, but no major objection

src/cleveractors/config_utils.py Outdated

						
				@@ -0,0 +79,4 @@

				        if key not in base:

				            # Rule: key absent → add it (deep-copy to preserve immutability).

				            base[key] = copy.deepcopy(new_value)

				        else:

CoreRasurae commented

2026-06-03 11:37:17 +00:00

a debug logging could be useful to know that we are replacing some item with a specific key

Reply: Good catch. Added logger.debug() calls in all three merge branches of _apply_merge:

When both sides are mappings: "Deep-merging key %r (both sides are mappings)"
When both sides are sequences: "Appending %d items to list at key %r"
Otherwise (replacement): "Replacing value for key %r: %s -> %s" with the old and new type names

The logger follows the project convention: logging.getLogger(__name__) at module level. All debug messages fire at the affected key, which gives enough context for diagnosing merge behavior without being noisy.

a debug logging could be useful to know that we are replacing some item with a specific key **Reply:** Good catch. Added `logger.debug()` calls in all three merge branches of `_apply_merge`: - When both sides are mappings: `"Deep-merging key %r (both sides are mappings)"` - When both sides are sequences: `"Appending %d items to list at key %r"` - Otherwise (replacement): `"Replacing value for key %r: %s -> %s"` with the old and new type names The logger follows the project convention: `logging.getLogger(__name__)` at module level. All debug messages fire at the affected key, which gives enough context for diagnosing merge behavior without being noisy.

hurui200320 marked this conversation as resolved

src/cleveractors/config_utils.py Outdated

						
				@@ -0,0 +83,4 @@

				            existing = base[key]

				            if isinstance(existing, dict) and isinstance(new_value, dict):

				                # Rule: both mappings → deep-merge recursively.

				                _apply_merge(existing, new_value)

CoreRasurae commented

2026-06-03 11:39:16 +00:00

Okay, it works, but recursion is not the most perfomant solution. If do not expect very large entries, this could be fine. Othwerwise it could make sense to re-write the implementation to not be recursive.

Reply: Converted _apply_merge from recursion to an explicit stack. The complexity analysis was straightforward — when both sides of a key are dicts, the different subtrees map to independent keys, so traversal order is irrelevant. A LIFO stack produces identical results to recursion.

The change: instead of _apply_merge(existing, new_value) (recursive call), the sub-dict pair is pushed onto a stack. A while stack: loop pops and processes each pair. This eliminates the risk of hitting Python's recursion limit on deeply nested configs, and matches the non-recursive pattern used in the internal _merge_configs loop in parse_files(). All 1676 unit tests and 53 integration tests pass with identical results.

Okay, it works, but recursion is not the most perfomant solution. If do not expect very large entries, this could be fine. Othwerwise it could make sense to re-write the implementation to not be recursive. **Reply:** Converted `_apply_merge` from recursion to an explicit stack. The complexity analysis was straightforward — when both sides of a key are dicts, the different subtrees map to independent keys, so traversal order is irrelevant. A LIFO stack produces identical results to recursion. The change: instead of `_apply_merge(existing, new_value)` (recursive call), the sub-dict pair is pushed onto a stack. A `while stack:` loop pops and processes each pair. This eliminates the risk of hitting Python's recursion limit on deeply nested configs, and matches the non-recursive pattern used in the internal `_merge_configs` loop in `parse_files()`. All 1676 unit tests and 53 integration tests pass with identical results.

hurui200320 marked this conversation as resolved

hurui200320 force-pushed feature/merge-configs-api from 7f5cd145a6 to ebbb167c99

2026-06-03 13:00:56 +00:00

Compare

hurui200320 force-pushed feature/merge-configs-api from ebbb167c99 to 8ebdcda177

2026-06-03 13:51:11 +00:00

Compare

hurui200320 added this to the v2.1.0 milestone 2026-06-03 14:17:19 +00:00

hurui200320 force-pushed feature/merge-configs-api from 8ebdcda177 to f2359c3604

2026-06-03 14:34:30 +00:00

Compare

hurui200320 force-pushed feature/merge-configs-api from f2359c3604 to 6b80be2117