feat(strategies): Base.merge for StrategyRegistry with cross-registry validation - #520
Merged
Conversation
… validation Downstream packages routinely hold two independently-built registries (e.g. a solve registry plus CTFlows' flow registry) and want one introspection entry point. Until now the only way to combine them was to reach into the struct and raw-merge the internal Dicts directly, which skips every validation create_registry enforces within one call. Base.merge(a::StrategyRegistry, b::StrategyRegistry...) now runs the equivalent cross-registry checks: - every strategy id() must be unique across all input registries - a parameter id() shared by more than one input must resolve to the same parameter type in each - no id() may be used as a strategy id in one registry and a parameter id in another When the same family is registered in more than one input, their strategy lists are unioned (still subject to the checks above), which is what makes merge useful for a downstream package extending a family across two independently-built registries rather than only combining disjoint ones. Extends Base.merge rather than introducing an exported Strategies.merge, per the issue's naming note, to avoid the same shadowing problem that made downstream packages move away from @reexport. Fixes #517 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Base.merge(a::StrategyRegistry, b::StrategyRegistry...), running the equivalent ofcreate_registry's cross-registry checks instead of a rawDictmerge, which silently let a colliding ID resolve to whichever entry won.id()is unique across all input registries;id()shared by more than one input registry resolves to the same parameter type in each;id()is used as a strategy ID in one registry and a parameter ID in another.mergeuseful for a downstream package extending a family across two independently-built registries (e.g. a solve registry plus a flow registry), not just combining registries with disjoint families.Base.mergerather than introducing an exportedStrategies.merge, per the issue's naming note, avoiding the shadowing problem that made downstream packages move away from@reexport.Fixes #517
Test plan
test/suite/strategies/test_registry_merge.jl(33 tests), organized as Unit / Integration / Contract / Error:describe/showexercised through the merged registry, confirming a unioned family renders under a single headersuite/strategies(1013 tests) green🤖 Generated with Claude Code