feat(merge_configs): expose public merge_configs(*dicts) API per §3.1 deep-merge algorithm #19
No reviewers
Labels
No labels
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Blocks
#11 feat(merge_configs): expose public merge_configs(*dicts) API per §3.1 deep-merge algorithm
cleveragents/cleveractors-core
Reference
cleveragents/cleveractors-core!19
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/merge-configs-api"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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 itsbasearg, only accepts two dicts, and is not publicly importable.What Was Done
New module:
src/cleveractors/config_utils.pymerge_configs(*dicts: dict[str, Any]) -> dict[str, Any]— variadic, immutable, public.TypeErrorimmediately (fail-fast per CONTRIBUTING.md)._apply_merge()internal helper:dict→ deep-merged (via explicit stack, not recursion).list→ existing + deep copy of new.{}.copy.deepcopyused for all stored values).logger.debug()calls on all three merge branches (dict merge, list append, value replacement) for operational traceability.RecursionErroron deeply nested configs.deepcopysecurity note, O(k²·m) list concatenation complexity.Export:
src/cleveractors/__init__.pymerge_configsadded to the module import and__all__.from cleveractors import merge_configs.Tests
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.robot/config.robotincluding an end-to-end pipeline test that loads YAML viaConfigurationManagerand merges an overlay throughmerge_configs.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
CONTRIBUTORS.mdwith contributor entry per CONTRIBUTING.md.CHANGELOG.mdunder[Unreleased] > Added.Quality Gates
ruff checkpyright(strict)config_utils.py: 100%)Known Limitations
config_utils.pynow uses plain/minimal docstrings consistent with the rest of the codebase.Design Notes
_merge_configsinstance method onReactiveConfigParseris unchanged to avoid breaking the existing file-parsing path._apply_mergehelper is intentionally private (module-level_prefix) — onlymerge_configsis part of the public contract._apply_mergewith explicit stack-based iteration and addedlogger.debug()calls across all merge branches.import sysfromrobot/CleverActorsLib.py.teardownmethod from benchmarks.Closes #11
Implement a new module-level merge_configs() function in src/cleveractors/config_utils.py that exposes the §3.1 deep-merge algorithm as a public, importable API. Motivation: The internal ReactiveConfigParser._merge_configs() method mutates its base dict in place and only accepts two dicts. The CleverThis router (wave 2 of cleveragents/cleveragents-webapp#275) needs a public merge_configs() to combine platform base configs with actor configs from the database without side-effects. Implementation: - merge_configs(*dicts) accumulates into a fresh dict, applying each overlay using the §3.1 rules: * Key absent → add via deep copy. * Both mappings → deep-merge recursively (_apply_merge helper). * Both sequences → extend (existing + deep copy of new). * Otherwise → replace with deep copy of new value. - Zero-argument call returns {}. - Inputs are never mutated (copy.deepcopy used for all stored values). - Internal _merge_configs on ReactiveConfigParser is unchanged to avoid breaking the existing parse path. Exports: merge_configs added to cleveractors/__init__.py import and __all__ so consumers can use 'from cleveractors import merge_configs'. Tests: - 11 Behave BDD scenarios in features/merge_configs.feature covering zero-args, single-dict copy, absent-key insertion, scalar override, recursive mapping merge, sequence append, three-dict chained merge, deep nesting, input mutation guard, and type-mismatch replacement. - 4 Robot Framework integration tests in robot/config.robot verifying the same cases at the API boundary. - 5 ASV benchmarks in benchmarks/merge_configs_benchmark.py for flat, nested, sequence, three-way, and zero-arg cases. Coverage: 97% (config_utils.py: 100%). Quality gates: ruff lint ✓, pyright strict ✓ (0 errors), all BDD/Robot tests ✓. ISSUES CLOSED: #115825a643f5fe37c4cdb3fe37c4cdb3bf991b32c2bf991b32c2ba439b3920ba439b39207f5cd145a6Note 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_configsmethod 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 theReactiveConfigParser, 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.
Design Note:
merge_configsvs_merge_configsA natural question when reviewing this PR is: why add a new
merge_configsfunction instead of reusing or removing the existingReactiveConfigParser._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)basedictbase[key] = value)copy.deepcopy(value)Noneinputif new is None: return)TypeError_merge_configsis designed to be called in a loop insideparse_files()wherecombined_configis 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:
extendvs. concatenationBoth produce the same final content. The difference is that
extendmutates 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_configsis a parser-internal tool — an instance method called only byparse_files(), operating on a mutable accumulator it owns. ItsNoneguard exists becauseyaml.safe_loadlegitimately returnsNoneon an empty or comment-only YAML file, which is a normal condition during file loading.merge_configsis 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 swallowingNone.Reusing
_merge_configsas 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 inconfig_utils.pyis 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.
Reviewed by a human, some comments, but no major objection
@ -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: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:"Deep-merging key %r (both sides are mappings)""Appending %d items to list at key %r""Replacing value for key %r: %s -> %s"with the old and new type namesThe 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.@ -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)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_mergefrom 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. Awhile 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_configsloop inparse_files(). All 1676 unit tests and 53 integration tests pass with identical results.7f5cd145a6ebbb167c99ebbb167c998ebdcda1778ebdcda177f2359c3604f2359c36046b80be2117