Repository navigation
feat(runtime): implement central message type registry [OMN-937] - #67
jonahgabriel merged 4 commits into
Conversation
Implement message type registry that maps message types to handler implementations and enforces topic category constraints for the ONEX runtime dispatch infrastructure. Key features: - Message type to handler mapping with fan-out support - Topic category constraints (commands/events/intents) - Startup-time validation with fail-fast behavior - Domain ownership enforcement (onex.<domain>.* patterns) - Cross-domain consumption requires explicit opt-in - Runtime rejects topic/message-type domain mismatches - Freeze-after-init pattern for thread-safe access New files: - runtime/registry/model_domain_constraint.py - runtime/registry/model_message_type_entry.py - runtime/registry/protocol_message_type_registry.py - runtime/registry/registry_message_type.py - 73 unit tests covering all functionality
WalkthroughCentralizes JSON type aliases into a new types package and introduces a Message Type Registry subsystem: protocol, immutable models (ModelDomainConstraint, ModelMessageTypeEntry), and a concrete MessageTypeRegistry implementation with domain/category validation, freeze-after-init lifecycle, and comprehensive tests. Changes
sequenceDiagram
participant Client as Client/Handler
participant Registry as MessageTypeRegistry
participant Entry as ModelMessageTypeEntry
participant Domain as ModelDomainConstraint
rect rgb(200,220,255)
Note over Client,Registry: Registration phase (single-threaded)
Client->>Registry: register_message_type(entry)
Registry->>Entry: validate entry fields
Entry->>Domain: validate domain/category constraints
Domain-->>Entry: result (ok/err)
Registry->>Registry: merge handlers / update indexes
end
rect rgb(200,255,220)
Note over Client,Registry: Freeze phase
Client->>Registry: freeze()
Registry->>Registry: lock registry (is_frozen = true)
end
rect rgb(255,240,200)
Note over Client,Registry: Query phase (concurrent-safe)
Client->>Registry: get_handlers(message_type, category, topic_domain)
Registry->>Entry: retrieve entry
Entry->>Domain: validate consumption (category + domain)
Domain-->>Registry: allowed
Registry-->>Client: list of handler_ids
end
rect rgb(255,220,220)
Note over Client,Registry: Startup validation
Client->>Registry: validate_startup(available_handlers)
Registry->>Registry: check missing handlers, domain consistency
Registry-->>Client: list of error strings
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Poem
Comment |
Code Review - Central Message Type Registry [OMN-937]Overall Assessment ✅This is high-quality infrastructure code that follows ONEX patterns extremely well. The implementation demonstrates:
Recommendation: Approve with minor suggestions below. Code Quality ✅ ExcellentStrengths
Potential Issues 🔍 Minor1. Domain Extraction Regex Ambiguity (Low Priority)Location: The regex pattern may not handle all edge cases correctly: _DOMAIN_PATTERN = re.compile(
r"^(?:onex\.)?(?P<env>[a-zA-Z0-9_-]+)\.(?P<domain>[a-zA-Z0-9_-]+)\.",
re.IGNORECASE,
)Issue: For
But the extraction logic at lines 180-184 checks if Suggestion: Consider renaming groups for clarity or simplifying the regex: # Option 1: Simpler pattern with clearer logic
_DOMAIN_PATTERN = re.compile(
r"^(?:onex|[a-zA-Z0-9_-]+)\.(?P<domain>[a-zA-Z0-9_-]+)\.",
re.IGNORECASE,
)
# Then just: return match.group('domain')
# Option 2: Two separate patterns for clarity
_ONEX_PATTERN = re.compile(r"^onex\.(?P<domain>[a-zA-Z0-9_-]+)\.")
_ENV_PATTERN = re.compile(r"^[a-zA-Z0-9_-]+\.(?P<domain>[a-zA-Z0-9_-]+)\.")Impact: Low - current implementation works, but clearer code reduces maintenance burden. 2. Missing Validation: Empty
|
| File | Assessment | Notes |
|---|---|---|
registry_message_type.py |
✅ Excellent | Core implementation is solid, minor regex clarity suggestion |
model_message_type_entry.py |
✅ Excellent | Add empty category validation |
model_domain_constraint.py |
✅ Perfect | No issues |
protocol_message_type_registry.py |
✅ Perfect | Clean protocol definition |
__init__.py (module) |
✅ Good | Proper exports |
| Test files | ✅ Excellent | Comprehensive coverage |
Final Verdict ✅ APPROVE
This PR is production-ready with only minor suggestions that could be addressed in follow-up work if desired. The implementation:
- Meets all acceptance criteria
- Follows ONEX patterns religiously
- Has excellent test coverage
- Includes clear documentation
- Demonstrates strong software engineering practices
Outstanding work on this infrastructure component! 🚀
Related Tickets
As noted in the PR description, this unblocks:
- ✅ OMN-941 (B2) Handler Output Model (already done)
- Future runtime integration work (B3+)
Consider verifying integration with MessageDispatchEngine in a follow-up PR or integration test suite.
- Add validation for empty allowed_categories (must have at least one) - Fix None/empty/whitespace handling in extract_domain_from_topic - Clarify regex patterns with separate ONEX and env-aware patterns - Add 7 new tests for edge cases - Bump INFRA_MAX_UNIONS threshold (451→455) pending OMN-983 refactor Fixes: empty categories, None handling, regex ambiguity
PR Review: Central Message Type Registry [OMN-937]Overall Assessment ✅This is a high-quality implementation that introduces the central message type registry for ONEX runtime dispatch infrastructure. The code demonstrates excellent adherence to ONEX patterns, comprehensive test coverage, and thoughtful design. I found only minor suggestions for improvement. Strengths 💪1. Exceptional Documentation
2. Strong Type Safety
3. ONEX Compliance
4. Comprehensive Test Coverage
5. Robust Error Handling
Code Quality ObservationsThread Safety ✅The freeze-after-init pattern is correctly implemented: # registration_message_type.py:356-362
with self._lock:
if self._frozen:
raise ModelOnexError(
message="Cannot register message type: MessageTypeRegistry is frozen...",
error_code=EnumCoreErrorCode.INVALID_STATE,
)Observation: The
Domain Extraction Logic ✅The
Test coverage: 14 tests covering all edge cases (test_registry_message_type.py:22-85) Immutability Pattern ✅Models use # model_message_type_entry.py:232-250
def with_additional_handler(self, handler_id: str) -> "ModelMessageTypeEntry":
"""Create a new entry with an additional handler ID."""
if handler_id in self.handler_ids:
return self
new_handlers = tuple(list(self.handler_ids) + [handler_id])
return self.model_copy(update={"handler_ids": new_handlers})Suggestions for Improvement1. Type Annotation Consistency (Minor)Current (registry_message_type.py:142): def extract_domain_from_topic(topic: str | None) -> str | None:Observation: This is already using the preferred PEP 604 syntax ( However, I notice the function accepts
Rationale: Accepting 2. Validation Error Aggregation (Enhancement Opportunity)Current (registry_message_type.py:796-810): if available_handler_ids is not None:
missing_handlers = self._handler_references - available_handler_ids
if missing_handlers:
for handler_id in sorted(missing_handlers):
# Find which message types reference this handler
referencing_types = [
mt
for mt, entry in self._entries.items()
if handler_id in entry.handler_ids
]
errors.append(
f"Handler '{handler_id}' is referenced by message types "
f"{referencing_types} but is not registered..."
)Observation: This iterates Suggestion: Build a reverse index during registration: # In __init__
self._handler_to_types: dict[str, list[str]] = defaultdict(list)
# In register_message_type
for handler_id in entry.handler_ids:
self._handler_to_types[handler_id].append(message_type)Impact: Low - validation happens once at startup. Current implementation is acceptable. This is a future optimization if registries grow very large. 3. Consider Caching Domain Extraction (Performance)Current (registry_message_type.py:886): domain = extract_domain_from_topic(topic)Observation: Domain extraction uses regex matching. If Suggestion: from functools import lru_cache
@lru_cache(maxsize=256)
def extract_domain_from_topic_cached(topic: str) -> str | None:
# ImplementationImpact: Depends on usage pattern. If validation is infrequent, current implementation is fine. Not a blocker. 4. Error Message Truncation (UX Enhancement)Current (registry_message_type.py:567-574): entry = self._entries.get(message_type)
if entry is None:
registered = sorted(self._entries.keys())[:10]
suffix = "..." if len(self._entries) > 10 else ""
raise MessageTypeRegistryError(
f"No handler mapping for message type '{message_type}'. "
f"Registered types: {registered}{suffix}",
...
)Observation: Shows first 10 registered types alphabetically. Consider showing closest matches using fuzzy string matching (e.g., Levenshtein distance). Example: import difflib
closest = difflib.get_close_matches(message_type, self._entries.keys(), n=5)
if closest:
hint = f"Did you mean: {closest}?"Impact: Better developer experience. Nice to have, not required. Security Considerations ✅1. No Sensitive Data Exposure
2. Input Validation
3. Immutability
Performance Considerations ✅Indexing StrategyThe registry maintains indexes for efficient queries: # registry_message_type.py:310-311
self._category_index: dict[EnumMessageCategory, list[str]] = defaultdict(list)
self._domain_index: dict[str, list[str]] = defaultdict(list)Analysis:
No concerns - appropriate data structures for the use case. Testing Quality ✅Coverage Breakdown
Total: 73 tests as claimed in PR description ✅ Edge Cases Covered
Excellent test discipline. ONEX Pattern Compliance Checklist
RecommendationsMust Fix (Blocking)None - No blocking issues found. Should Consider (Pre-merge)
Nice to Have (Post-merge)
ConclusionThis PR demonstrates exemplary code quality and is ready to merge pending CI verification. The implementation:
Recommendation: APPROVE ✅ Great work on this implementation! The freeze-after-init pattern, domain constraint validation, and fail-fast behavior will provide a solid foundation for the runtime dispatch infrastructure. Files Reviewed
|
… [OMN-937] - Create centralized json_types.py module with JsonPrimitive, JsonValue, EnvelopeDict, and ResultDict type aliases - Deduplicate union definitions from model_dispatch_result.py and protocol_types.py (now import from centralized types) - Update INFRA_MAX_UNIONS from 455 to 450 to match threshold - Union count reduced from 455 to 449 (6 unions saved)
Code Review: Central Message Type Registry (OMN-937)✅ Overall AssessmentThis is excellent work. The implementation demonstrates strong adherence to ONEX patterns and provides a critical piece of runtime infrastructure. The code quality is high with comprehensive documentation, robust validation, and extensive test coverage. 🎯 Strengths1. Architecture & Design ⭐⭐⭐⭐⭐
2. ONEX Compliance ⭐⭐⭐⭐⭐
3. Code Quality ⭐⭐⭐⭐⭐
|
🔍 Key Technical AnalysisDomain Extraction Logic (registry_message_type.py:142-203)
Thread Safety Implementation (registry_message_type.py:315-518)
Validation & Error Handling (registry_message_type.py:753-840)
Security & Domain Isolation
|
🐛 RecommendationsMinor Improvements (Non-Blocking)
Nice to Have (Future Work)
🧪 Test Coverage Assessment: 5/5 ⭐⭐⭐⭐⭐73 comprehensive tests covering:
Minor gaps identified above are low priority and non-blocking. 📊 Performance AnalysisDocumented Performance Characteristics:
Analysis:
No performance concerns identified. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/omnibase_infra/runtime/registry/model_message_type_entry.py (1)
84-164: LGTM, with a minor consideration on metadata immutability.The field definitions use appropriate types with proper constraints. The use of
tuple[str, ...]forhandler_idsandfrozenset[EnumMessageCategory]forallowed_categoriesensures immutability.Note: The
metadata: dict[str, str] | Nonefield uses a mutabledict. While the frozen model prevents field reassignment, external code with a reference to the dict could mutate it. If strict immutability is required, consider usingtypes.MappingProxyTypeor documenting that callers should not mutate the returned dict. This is a minor consideration given the overall design.src/omnibase_infra/runtime/registry/registry_message_type.py (1)
997-1001: Consider documenting the__contains__behavioral difference before/after freeze.The
inoperator has subtly different semantics:
- Pre-freeze: Checks if message type exists in
_entries(includes disabled entries)- Post-freeze: Calls
has_message_type()which returnsFalsefor disabled entriesThis asymmetry is intentional (to avoid calling
_require_frozenduring registration), but could surprise users. Consider adding a docstring note or unifying the behavior.🔎 Option: Unify behavior to always check enabled status
def __contains__(self, message_type: str) -> bool: """Check if message type is registered using 'in' operator.""" - if not self._frozen: - return message_type in self._entries - return self.has_message_type(message_type) + entry = self._entries.get(message_type) + return entry is not None and entry.enabled
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (14)
src/omnibase_infra/models/dispatch/model_dispatch_result.py(1 hunks)src/omnibase_infra/models/types/__init__.py(1 hunks)src/omnibase_infra/models/types/json_types.py(1 hunks)src/omnibase_infra/nodes/node_registry_effect/v1_0_0/protocol_types.py(2 hunks)src/omnibase_infra/runtime/__init__.py(2 hunks)src/omnibase_infra/runtime/registry/__init__.py(1 hunks)src/omnibase_infra/runtime/registry/model_domain_constraint.py(1 hunks)src/omnibase_infra/runtime/registry/model_message_type_entry.py(1 hunks)src/omnibase_infra/runtime/registry/protocol_message_type_registry.py(1 hunks)src/omnibase_infra/runtime/registry/registry_message_type.py(1 hunks)tests/unit/runtime/registry/__init__.py(1 hunks)tests/unit/runtime/registry/test_model_domain_constraint.py(1 hunks)tests/unit/runtime/registry/test_model_message_type_entry.py(1 hunks)tests/unit/runtime/registry/test_registry_message_type.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: All data structures must be proper Pydantic models - never use Any types. Use specific types instead.
Use X | None (PEP 604) union syntax for nullable types instead of Optional[X]
Use ProtocolConfigurationError for configuration validation failures, SecretResolutionError for credential resolution failures, InfraConnectionError for connection failures, InfraTimeoutError for operation timeouts, InfraAuthenticationError for auth failures, and InfraUnavailableError for resource unavailable
Use EnumMessageCategory (EVENT, COMMAND, INTENT) for message routing and topic parsing. Use EnumNodeOutputType (EVENT, COMMAND, INTENT, PROJECTION) for execution shape and handler return type validation. PROJECTION only valid for REDUCER nodes
Do not use Any types anywhere in the codebase - always use specific types. If type is not known at definition time, use object as the type parameter instead
Use duck typing through protocols instead of isinstance checks. Protocol resolution based on structural matching, not type checking
Files:
tests/unit/runtime/registry/test_registry_message_type.pysrc/omnibase_infra/models/types/__init__.pysrc/omnibase_infra/runtime/registry/protocol_message_type_registry.pysrc/omnibase_infra/runtime/registry/model_domain_constraint.pysrc/omnibase_infra/runtime/__init__.pysrc/omnibase_infra/models/types/json_types.pysrc/omnibase_infra/runtime/registry/__init__.pysrc/omnibase_infra/runtime/registry/model_message_type_entry.pysrc/omnibase_infra/nodes/node_registry_effect/v1_0_0/protocol_types.pytests/unit/runtime/registry/test_model_message_type_entry.pysrc/omnibase_infra/models/dispatch/model_dispatch_result.pytests/unit/runtime/registry/__init__.pysrc/omnibase_infra/runtime/registry/registry_message_type.pytests/unit/runtime/registry/test_model_domain_constraint.py
**/*protocol*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use protocol_.py or protocols.py file naming with Protocol class naming for protocol definitions. Single protocol uses protocol_.py, domain-grouped protocols use protocols.py
Files:
src/omnibase_infra/runtime/registry/protocol_message_type_registry.pysrc/omnibase_infra/nodes/node_registry_effect/v1_0_0/protocol_types.py
**/*model_*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*model_*.py: Use model_.py file naming with Model class naming for data model files
Each Python file must contain exactly one Model* class for data structure definitions
Files:
src/omnibase_infra/runtime/registry/model_domain_constraint.pysrc/omnibase_infra/runtime/registry/model_message_type_entry.pytests/unit/runtime/registry/test_model_message_type_entry.pysrc/omnibase_infra/models/dispatch/model_dispatch_result.pytests/unit/runtime/registry/test_model_domain_constraint.py
**/*dispatch*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use ModelEventEnvelope[object] instead of Any for generic dispatcher parameters when type is not known at definition time
Files:
src/omnibase_infra/models/dispatch/model_dispatch_result.py
**/registry_*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use registry_.py file naming with Registry class naming for standalone registries in domain directories
Files:
src/omnibase_infra/runtime/registry/registry_message_type.py
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T19:53:07.665Z
Learning: Applies to **/*.py : Use EnumMessageCategory (EVENT, COMMAND, INTENT) for message routing and topic parsing. Use EnumNodeOutputType (EVENT, COMMAND, INTENT, PROJECTION) for execution shape and handler return type validation. PROJECTION only valid for REDUCER nodes
📚 Learning: 2025-12-08T00:48:30.737Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_spi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T00:48:30.737Z
Learning: Import `omnibase_core` models and types only for type hints and runtime usage - follow the SPI → Core dependency direction
Applied to files:
src/omnibase_infra/models/types/__init__.pysrc/omnibase_infra/runtime/__init__.py
📚 Learning: 2025-12-08T00:48:30.737Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_spi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T00:48:30.737Z
Learning: Applies to src/omnibase_spi/**/*.py : SPI modules may import from `omnibase_core` for type hints and model runtime usage (allowed and required)
Applied to files:
src/omnibase_infra/models/types/__init__.pysrc/omnibase_infra/runtime/__init__.py
📚 Learning: 2025-11-24T17:22:32.195Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-24T17:22:32.195Z
Learning: Applies to **/protocols/protocol_*.py : Use Protocol from typing module for all interface definitions; never use ABC (Abstract Base Classes) for service interfaces
Applied to files:
src/omnibase_infra/runtime/registry/protocol_message_type_registry.pysrc/omnibase_infra/nodes/node_registry_effect/v1_0_0/protocol_types.py
📚 Learning: 2025-11-24T17:24:41.687Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T17:24:41.687Z
Learning: Applies to **/protocols/protocol_*.py : Use Protocol for tool interfaces and plugin APIs based on method shape (structural typing), not Pydantic models with inheritance
Applied to files:
src/omnibase_infra/runtime/registry/protocol_message_type_registry.py
📚 Learning: 2025-11-24T16:33:32.747Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T16:33:32.747Z
Learning: Use Protocol for interface definitions when implementations may live outside core codebase; use Pydantic models only for base classes with shared logic
Applied to files:
src/omnibase_infra/runtime/registry/protocol_message_type_registry.py
📚 Learning: 2025-12-08T00:48:30.737Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_spi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T00:48:30.737Z
Learning: Applies to src/omnibase_spi/protocols/**/*.py : All public protocols must be decorated with `runtime_checkable`
Applied to files:
src/omnibase_infra/runtime/registry/protocol_message_type_registry.pysrc/omnibase_infra/runtime/__init__.py
📚 Learning: 2025-11-24T16:33:32.747Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T16:33:32.747Z
Learning: Applies to **/models/model_contract_*.py : Contract model files must follow the naming pattern `model_contract_<domain>.py` and be located in `*/models/` directories
Applied to files:
src/omnibase_infra/runtime/registry/model_domain_constraint.py
📚 Learning: 2025-11-24T17:24:41.687Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T17:24:41.687Z
Learning: Applies to **/models/model_contract_*.py : Contract-backed model files must follow the naming pattern `model_contract_<domain>.py` and be located in `*/models/` directories
Applied to files:
src/omnibase_infra/runtime/registry/model_domain_constraint.py
📚 Learning: 2025-11-24T17:24:41.687Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T17:24:41.687Z
Learning: Applies to src/omnibase/constants/*_constants.py : Constants files must follow the naming pattern `<domain>_constants.py` and be located in `src/omnibase/constants/`
Applied to files:
src/omnibase_infra/runtime/registry/model_domain_constraint.py
📚 Learning: 2025-11-24T16:33:32.747Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T16:33:32.747Z
Learning: Applies to src/omnibase/constants/*_constants.py : Constants files must follow the naming pattern `<domain>_constants.py` and be located in `src/omnibase/constants/` directory
Applied to files:
src/omnibase_infra/runtime/registry/model_domain_constraint.py
📚 Learning: 2025-11-28T18:58:53.781Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Organize models under `src/omnibase_core/models/` by domain including: base, cli, common, config, core, contracts, discovery, health, infrastructure, logging, metadata, nodes, operations, results, security, service, tools, validation, and workflows
Applied to files:
src/omnibase_infra/runtime/registry/model_domain_constraint.py
📚 Learning: 2025-11-24T16:33:32.747Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T16:33:32.747Z
Learning: Applies to **/{models,protocols}/{model_*,protocol_*}.py : Avoid using Any, dict, or primitive types in model and protocol definitions; use strongest typing possible
Applied to files:
src/omnibase_infra/models/types/json_types.pysrc/omnibase_infra/nodes/node_registry_effect/v1_0_0/protocol_types.py
📚 Learning: 2025-11-24T17:24:41.687Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T17:24:41.687Z
Learning: Applies to **/protocols/protocol_*.py : Avoid using Any, dict, or primitive types in protocol signatures; use the strongest typing possible with Pydantic models
Applied to files:
src/omnibase_infra/models/types/json_types.pysrc/omnibase_infra/nodes/node_registry_effect/v1_0_0/protocol_types.py
📚 Learning: 2025-12-20T19:53:07.665Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T19:53:07.665Z
Learning: Applies to **/registry_*.py : Use registry_<purpose>.py file naming with Registry<Purpose> class naming for standalone registries in domain directories
Applied to files:
src/omnibase_infra/runtime/registry/__init__.py
📚 Learning: 2025-12-08T00:48:30.737Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_spi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T00:48:30.737Z
Learning: Applies to src/omnibase_spi/protocols/nodes/*.py : Use Protocol naming convention `Protocol{Type}Node` for node protocols (e.g., `ProtocolComputeNode`, `ProtocolEffectNode`)
Applied to files:
src/omnibase_infra/nodes/node_registry_effect/v1_0_0/protocol_types.py
📚 Learning: 2025-11-24T16:33:32.747Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T16:33:32.747Z
Learning: Applies to **/protocols/protocol_*.py : All protocol method signatures must use Pydantic models exclusively, never primitives or dicts
Applied to files:
src/omnibase_infra/nodes/node_registry_effect/v1_0_0/protocol_types.py
📚 Learning: 2025-11-24T16:33:51.604Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T16:33:51.604Z
Learning: Applies to tests/unit/models/**/test_model_*.py : Model tests must achieve 100% coverage and test instantiation, inheritance, serialization, deserialization, JSON serialization, roundtrip serialization, equality, hashing, string representation, repr, attributes, validation, metadata, data creation, copying, and immutability
Applied to files:
tests/unit/runtime/registry/test_model_message_type_entry.pytests/unit/runtime/registry/test_model_domain_constraint.py
📚 Learning: 2025-11-28T18:58:53.781Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Applies to tests/**/*.py : Write comprehensive test coverage following the test structure under `tests/unit/` organized by subsystem (enums, models, mixins, utils)
Applied to files:
tests/unit/runtime/registry/test_model_message_type_entry.py
📚 Learning: 2025-12-20T19:53:07.665Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T19:53:07.665Z
Learning: Applies to **/*dispatch*.py : Use ModelEventEnvelope[object] instead of Any for generic dispatcher parameters when type is not known at definition time
Applied to files:
src/omnibase_infra/models/dispatch/model_dispatch_result.py
📚 Learning: 2025-11-28T18:58:53.781Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Applies to **/*.py : Use `EnumCoreErrorCode` with `ModelOnexError` for proper error code usage
Applied to files:
src/omnibase_infra/models/dispatch/model_dispatch_result.py
📚 Learning: 2025-11-24T17:24:54.193Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T17:24:54.193Z
Learning: Applies to **/*test*.py : Use registry=None in test harness to force registry resolver usage instead of manually creating registry instances
Applied to files:
tests/unit/runtime/registry/__init__.py
📚 Learning: 2025-12-20T19:53:07.665Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T19:53:07.665Z
Learning: Applies to **/*.py : Use EnumMessageCategory (EVENT, COMMAND, INTENT) for message routing and topic parsing. Use EnumNodeOutputType (EVENT, COMMAND, INTENT, PROJECTION) for execution shape and handler return type validation. PROJECTION only valid for REDUCER nodes
Applied to files:
src/omnibase_infra/runtime/registry/registry_message_type.py
🧬 Code graph analysis (7)
src/omnibase_infra/runtime/registry/protocol_message_type_registry.py (3)
src/omnibase_infra/enums/enum_message_category.py (1)
EnumMessageCategory(34-192)src/omnibase_infra/runtime/registry/model_message_type_entry.py (1)
ModelMessageTypeEntry(33-270)src/omnibase_infra/runtime/registry/registry_message_type.py (10)
register_message_type(321-432)freeze(487-518)get_handlers(524-604)get_entry(634-650)has_message_type(652-669)list_message_types(671-717)list_domains(719-732)validate_startup(753-840)is_frozen(928-937)entry_count(940-949)
src/omnibase_infra/runtime/__init__.py (3)
src/omnibase_infra/runtime/registry/registry_message_type.py (2)
MessageTypeRegistry(211-1028)MessageTypeRegistryError(68-119)src/omnibase_infra/runtime/registry/model_domain_constraint.py (1)
ModelDomainConstraint(32-197)src/omnibase_infra/runtime/registry/model_message_type_entry.py (1)
ModelMessageTypeEntry(33-270)
src/omnibase_infra/runtime/registry/__init__.py (4)
src/omnibase_infra/runtime/registry/model_domain_constraint.py (1)
ModelDomainConstraint(32-197)src/omnibase_infra/runtime/registry/model_message_type_entry.py (1)
ModelMessageTypeEntry(33-270)src/omnibase_infra/runtime/registry/protocol_message_type_registry.py (1)
ProtocolMessageTypeRegistry(31-303)src/omnibase_infra/runtime/registry/registry_message_type.py (2)
MessageTypeRegistry(211-1028)MessageTypeRegistryError(68-119)
src/omnibase_infra/runtime/registry/model_message_type_entry.py (2)
src/omnibase_infra/enums/enum_message_category.py (1)
EnumMessageCategory(34-192)src/omnibase_infra/runtime/registry/model_domain_constraint.py (1)
ModelDomainConstraint(32-197)
tests/unit/runtime/registry/test_model_message_type_entry.py (2)
src/omnibase_infra/enums/enum_message_category.py (1)
EnumMessageCategory(34-192)tests/unit/runtime/registry/test_model_domain_constraint.py (1)
test_immutable(103-107)
src/omnibase_infra/runtime/registry/registry_message_type.py (4)
src/omnibase_infra/enums/enum_message_category.py (2)
EnumMessageCategory(34-192)from_topic(121-159)src/omnibase_infra/errors/infra_errors.py (1)
RuntimeHostError(35-100)src/omnibase_infra/runtime/registry/model_domain_constraint.py (2)
ModelDomainConstraint(32-197)validate_consumption(150-197)src/omnibase_infra/runtime/registry/model_message_type_entry.py (3)
ModelMessageTypeEntry(33-270)with_additional_handler(226-256)validate_category(190-224)
tests/unit/runtime/registry/test_model_domain_constraint.py (1)
src/omnibase_infra/runtime/registry/model_domain_constraint.py (3)
ModelDomainConstraint(32-197)can_consume_from(117-148)validate_consumption(150-197)
🔇 Additional comments (35)
src/omnibase_infra/nodes/node_registry_effect/v1_0_0/protocol_types.py (1)
5-24: LGTM! Type centralization reduces duplication.The refactoring to import JSON type aliases from the centralized
omnibase_infra.models.typesmodule eliminates duplication while maintaining backward compatibility through re-exports. The updated docstring clearly documents the change.src/omnibase_infra/models/dispatch/model_dispatch_result.py (1)
59-59: LGTM! Consistent type centralization.The import change aligns with the broader refactoring effort to centralize JSON type definitions. The
__all__export on line 381 maintains backward compatibility for consumers of this module.tests/unit/runtime/registry/test_model_domain_constraint.py (1)
12-128: LGTM! Comprehensive test coverage for domain constraints.The test suite thoroughly validates ModelDomainConstraint behavior including:
- Domain ownership and cross-domain consumption rules
- Validation with detailed error messages
- Immutability guarantees
- Default values and edge cases
The test structure is clear and follows best practices.
src/omnibase_infra/runtime/__init__.py (1)
96-102: LGTM! Proper exposure of new registry components.The imports correctly expose the new message type registry infrastructure at the package level, making them accessible via
omnibase_infra.runtime. The components are appropriately grouped in__all__(lines 164-169) with a clear OMN-937 reference.src/omnibase_infra/models/types/__init__.py (1)
1-21: LGTM! Clean package structure for centralized types.The package initializer follows standard Python patterns for re-exporting types. The module docstring clearly explains the purpose of reducing union duplication across the codebase.
tests/unit/runtime/registry/test_model_message_type_entry.py (1)
18-261: LGTM! Excellent test coverage for ModelMessageTypeEntry.The test suite comprehensively validates:
- Basic instantiation with required fields
- Fan-out support via multiple handlers
- Multi-category message types
- Category validation with detailed error messages
- Immutable update patterns (
with_additional_handler,with_enabled)- Duplicate handler prevention
- All Pydantic validation constraints (required fields, min/max lengths)
- Edge case: empty allowed_categories correctly rejected with clear error (lines 242-261)
The test structure is well-organized with separate classes for functional and validation tests.
src/omnibase_infra/models/types/json_types.py (1)
28-34: PEP 695 syntax is correctly supported by the project's Python 3.12+ baseline requirement.The code uses PEP 695
typestatement syntax for recursive type definitions (JsonValue). This aligns with the project's minimum Python version of 3.12, which introduced support for this feature. No action needed.src/omnibase_infra/runtime/registry/__init__.py (1)
1-61: LGTM!The module init file is well-structured with comprehensive documentation covering core components, key features, thread safety patterns, and related issues. The exports are properly organized and the
__all__list correctly exposes all public API entities. Based on learnings, the registry naming conventions are correctly followed.src/omnibase_infra/runtime/registry/model_domain_constraint.py (3)
32-82: LGTM!The
ModelDomainConstraintclass follows coding guidelines: propermodel_<name>.pyfile naming withModel<Name>class naming, single class per file, frozen/immutable configuration for thread safety, and specific types instead ofAny. The field definitions with appropriate constraints and defaults are well-designed.
117-148: LGTM!The
can_consume_frommethod has correct short-circuit logic prioritizingallow_all_domains, thenowning_domainmatch, thenallowed_cross_domainslookup. The implementation matches the documented behavior in the docstring.
150-198: LGTM!The
validate_consumptionmethod correctly delegates tocan_consume_fromand provides detailed, actionable error messages that differ based on therequire_explicit_opt_inflag. The return type uses proper PEP 604 union syntax.src/omnibase_infra/runtime/registry/protocol_message_type_registry.py (4)
30-78: LGTM!The protocol class is correctly decorated with
@runtime_checkableas required by learnings, usesProtocolinstead of ABC per guidelines, and follows theprotocol_<name>.pyfile naming withProtocol<Name>class naming conventions. The comprehensive docstrings document thread safety requirements and the freeze-after-init pattern clearly.
83-164: LGTM!The registration and query methods are well-documented with comprehensive docstrings covering arguments, return values, error conditions, and thread safety requirements. The method signatures align with the implementation.
166-246: LGTM!The lookup and listing methods follow consistent patterns with proper optional parameter handling and clear documentation. Return types use PEP 604 union syntax correctly.
277-306: LGTM!The properties
is_frozenandentry_countare correctly defined with proper return types and documentation.src/omnibase_infra/runtime/registry/model_message_type_entry.py (4)
33-82: LGTM!The
ModelMessageTypeEntryclass follows coding guidelines with proper file and class naming conventions, frozen configuration for thread safety, and comprehensive docstrings documenting fan-out support and thread safety guarantees.
110-136: LGTM!The field validator correctly ensures
allowed_categoriesis non-empty with a clear, actionable error message explaining why a message type must be allowed in at least one category to be routable.
166-224: LGTM!The
supports_categorymethod is a clean membership check, andvalidate_categorycorrectly returns a tuple with a descriptive error message when validation fails. The error message includes the allowed categories for debugging.
226-271: LGTM!The
with_additional_handlermethod correctly implements idempotent fan-out registration, returningselfif the handler is already present. Thewith_enabledmethod properly usesmodel_copyfor immutable updates on the frozen model.tests/unit/runtime/registry/test_registry_message_type.py (8)
1-20: LGTM!The test file has well-organized imports and follows pytest conventions. The import of
extract_domain_from_topicindicates this is a public utility function exported from the registry module.
22-85: LGTM!The
TestExtractDomainFromTopicclass provides comprehensive test coverage for the domain extraction utility, including edge cases for ONEX Kafka format, environment-aware format, empty/whitespace inputs, special characters, case sensitivity, and fallback behavior. Thetype: ignore[arg-type]on line 40 is appropriately used for testing None input behavior.
87-223: LGTM!The
TestMessageTypeRegistryRegistrationclass thoroughly tests the registration functionality including basic registration, convenience methods, fan-out with multiple handlers, freeze enforcement, null rejection, and conflict detection for category and domain constraints. The tests properly verify error message content.
225-242: LGTM!The freeze tests correctly verify basic freeze functionality and idempotent behavior.
244-444: LGTM!The
TestMessageTypeRegistryQueriesclass has excellent coverage with a well-designed fixture and tests for all query methods including success cases, error cases (not found, category/domain mismatch, pre-freeze access), and filtering capabilities. The tests properly verify error message content.
446-551: LGTM!The
TestMessageTypeRegistryValidationclass covers startup validation with and without handler sets, detection of missing handlers, and topic-message type validation including category and domain mismatch scenarios. Error message verification is thorough.
553-603: LGTM!The cross-domain consumption tests correctly verify that explicit opt-in via
allowed_cross_domainsenables cross-domain access, while the default behavior blocks it. This aligns with the domain isolation design principle.
605-748: LGTM!The property tests correctly verify counting behavior for entries, handlers (unique), and domains (unique). The dunder method tests verify
__len__,__contains__,__str__, and__repr__implementations produce expected output formats.src/omnibase_infra/runtime/registry/registry_message_type.py (8)
1-61: LGTM!Module structure is well-organized with comprehensive documentation covering design principles, thread safety guarantees, and performance characteristics. The
__all__export list is properly defined and imports are appropriate.
68-119: LGTM!The error class properly extends
RuntimeHostErrorwith registry-specific context fields (message_type, domain, category). The category value conversion ensures proper serialization.
142-204: Well-designed domain extraction with comprehensive edge case handling.The priority ordering (ONEX format first, then ENV format, then fallback) is correctly reasoned. The fallback logic on lines 199-201 intentionally handles edge cases like "a.b" topics that don't have trailing segments.
281-433: Solid registration logic with correct fan-out merging.The registration flow correctly:
- Validates entry is not None before acquiring lock
- Checks frozen state under lock protection
- Validates constraint consistency before merging handlers
- Updates indexes only for new registrations (existing message types already indexed)
- Tracks all handler references for startup validation
434-518: LGTM!
register_simpleprovides a clean convenience API with sensible defaults.freeze()is correctly idempotent with thread-safe lock protection.
524-748: Query methods are well-designed with consistent semantics.Key strengths:
- All queries enforce frozen state for thread safety
- Error messages include helpful context (first 10 registered types)
enabledstatus is consistently respected across all lookup methods- Return values are copies/new collections, preserving internal immutability
753-922: Comprehensive startup validation with clear error messages.The validation methods cover:
- Missing handler references (when available_handler_ids provided)
- Handler-less entries and empty domain constraints (defensive checks even though Pydantic validators would catch these during registration)
- Topic-to-message-type compatibility with detailed error reasons
The fail-fast design ensures configuration issues surface before message processing begins.
1003-1028: LGTM!The
__str__and__repr__implementations provide appropriate levels of detail for debugging while limiting output size for large registries.
| # ========================================================================= | ||
| # Validation Methods | ||
| # ========================================================================= | ||
|
|
||
| def validate_startup(self) -> list[str]: | ||
| """ | ||
| Perform startup-time validation and return any errors. | ||
|
|
||
| Validates registry consistency: | ||
| - All handler references are valid | ||
| - No conflicting category constraints | ||
| - Domain constraints are properly configured | ||
|
|
||
| This method should be called after freeze() to ensure fail-fast | ||
| behavior before consumers start processing messages. | ||
|
|
||
| Returns: | ||
| List of validation error messages. Empty list if valid. | ||
|
|
||
| Raises: | ||
| ModelOnexError: If registry is not frozen (INVALID_STATE) | ||
|
|
||
| Thread Safety: | ||
| Must be safe for concurrent access after freeze(). | ||
|
|
||
| .. versionadded:: 0.5.0 | ||
| """ | ||
| ... |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for validate_topic_message_type in the codebase
rg -n "validate_topic_message_type" --type=py -C2Repository: OmniNode-ai/omnibase_infra
Length of output: 4562
🏁 Script executed:
#!/bin/bash
# Look at the protocol file to see what methods are defined
cat -n src/omnibase_infra/runtime/registry/protocol_message_type_registry.py | head -300Repository: OmniNode-ai/omnibase_infra
Length of output: 11163
🏁 Script executed:
#!/bin/bash
# Check if validate_topic_message_type is in the protocol file
grep -n "validate_topic_message_type" src/omnibase_infra/runtime/registry/protocol_message_type_registry.pyRepository: OmniNode-ai/omnibase_infra
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check how validate_topic_message_type is being called in the codebase
rg "validate_topic_message_type" --type=py -B2 -A2 | head -100Repository: OmniNode-ai/omnibase_infra
Length of output: 4382
🏁 Script executed:
#!/bin/bash
# Check if ProtocolMessageTypeRegistry is used in type hints anywhere with validate_topic_message_type
rg "ProtocolMessageTypeRegistry" --type=py -A3 | grep -A3 "validate_topic_message_type"Repository: OmniNode-ai/omnibase_infra
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check where ProtocolMessageTypeRegistry is actually used
rg "ProtocolMessageTypeRegistry" --type=py -C3Repository: OmniNode-ai/omnibase_infra
Length of output: 5768
🏁 Script executed:
#!/bin/bash
# Check the full implementation of validate_topic_message_type in registry_message_type.py
sed -n '842,920p' src/omnibase_infra/runtime/registry/registry_message_type.pyRepository: OmniNode-ai/omnibase_infra
Length of output: 2555
🏁 Script executed:
#!/bin/bash
# Check if the method is used in other parts of the codebase or if it's part of documented public API
rg "def.*registry.*ProtocolMessageTypeRegistry|registry:.*ProtocolMessageTypeRegistry" --type=py -C2Repository: OmniNode-ai/omnibase_infra
Length of output: 541
Add validate_topic_message_type to the protocol definition.
The MessageTypeRegistry implementation includes a public method validate_topic_message_type(topic: str, message_type: str) -> tuple[bool, str | None] that validates whether a message type is allowed on a given topic. This method is thoroughly tested and documented, but is not defined in the ProtocolMessageTypeRegistry protocol. If this is part of the intended public API contract for all registry implementations, it should be added to the protocol to ensure consistency across implementations.
🤖 Prompt for AI Agents
In src/omnibase_infra/runtime/registry/protocol_message_type_registry.py around
lines 248 to 275, the ProtocolMessageTypeRegistry is missing the public API
method validate_topic_message_type(topic: str, message_type: str) -> tuple[bool,
str | None]; add this abstract method declaration to the protocol with the
correct typing signature and a short docstring describing that it returns
(allowed: bool, error_message: Optional[str]) so all implementations (like
MessageTypeRegistry) must implement it and static type checkers will enforce the
contract.
🎨 ONEX Pattern Compliance Checklist
Pattern Compliance: 100% ✅ 🚀 Integration & DependenciesUnblocks
Dependencies
Migration PathNo breaking changes. This is new infrastructure that extends existing runtime capabilities. 🏆 Final Verdict: APPROVED ✅Must Fix Before MergeNone - Code is production-ready. Overall Scores
Overall: 5/5 ⭐⭐⭐⭐⭐ 💬 SummaryThis PR represents exemplary work that demonstrates:
The implementation is production-ready and sets a high bar for infrastructure code quality in the ONEX ecosystem. The central message type registry provides the critical mapping layer between message types and handlers with robust domain isolation, fail-fast validation, and thread-safe operation. Excellent work! This is a foundational piece of infrastructure that will serve the ONEX runtime well. 🚀 |
…-937] - Add Design Note to extract_domain_from_topic() docstring explaining critical regex pattern ordering (ONEX before env-aware) - Add inline comments with worked example of ambiguous topic handling - Add test_register_duplicate_handler_idempotent for handler deduplication - Add test_completely_invalid_topic_formats for malformed topics - Add test_edge_case_empty_segment_topics for consecutive dots - Add test_special_character_topics for unicode and special chars
PR Review: Central Message Type Registry [OMN-937]SummaryThis PR implements a well-architected central message type registry that maps message types to handler implementations with comprehensive domain ownership and category constraint enforcement. The implementation demonstrates excellent adherence to ONEX principles and follows infrastructure best practices. ✅ Strengths1. Excellent ONEX Compliance
2. Thread Safety Design
3. Comprehensive Test Coverage
4. Security & Error Handling
5. Code Quality
🔍 Code Quality ObservationsPattern Precedence Documentation (registry_message_type.py:178-226)Excellent: The Empty Category Validation (model_message_type_entry.py:110-136)Well done: The Handler Deduplication (registry_message_type.py:437-443)Clean implementation: The fan-out registration properly deduplicates handlers using the 🎯 Potential Improvements (Non-Blocking)1. Domain Extraction Edge Case (Low Priority)File: The fallback case in Impact: Low - validated by tests and documented behavior 2. Lock-Free Query Documentation (Future Enhancement)File: The Impact: Documentation enhancement only - code is already correct 🔒 Security ConsiderationsDomain Isolation (Excellent)The registry enforces domain isolation by default with explicit opt-in for cross-domain consumption:
No Secrets Exposure
📊 Performance ConsiderationsLookup Performance (Excellent)
Memory Efficiency
🧪 Test Coverage AssessmentCoverage: Excellent (73 tests across 3 test files) Test file breakdown:
Well-tested scenarios:
📋 ONEX Guidelines Compliance
🎉 ConclusionThis is high-quality production-ready code that demonstrates excellent engineering practices:
The code quality exceeds expectations for a foundational infrastructure component. The pattern precedence documentation and edge case handling show mature engineering judgment. ✅ RecommendationApprove and merge. This PR is ready for production. The minor suggestions above are enhancements for future consideration, not blockers. The implementation is solid, well-tested, and follows all ONEX conventions. Reviewed by: Claude Code Assistant |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/omnibase_infra/runtime/registry/registry_message_type.py (2)
854-859: Consider removing redundant handler check.The check
len(entry.handler_ids) == 0at line 856 can never be true becauseModelMessageTypeEntry.handler_idshas amin_length=1validation constraint. This is defensive but could be misleading since the validation would fail at registration time.🔎 Proposed removal
- # Validate that enabled entries have at least one handler - for message_type, entry in self._entries.items(): - if entry.enabled and len(entry.handler_ids) == 0: - errors.append( - f"Message type '{message_type}' is enabled but has no handlers." - )This validation is already enforced by the Pydantic model's
min_length=1constraint onhandler_ids.
1039-1043: Subtle behavioral difference in__contains__before/after freeze.Before freeze,
inchecks raw dict membership. After freeze, it useshas_message_typewhich also checksentry.enabled. This means a disabled entry returnsTruebefore freeze butFalseafter freeze. Consider documenting this or making behavior consistent.🔎 Option: Make behavior consistent
def __contains__(self, message_type: str) -> bool: """Check if message type is registered using 'in' operator.""" - if not self._frozen: - return message_type in self._entries - return self.has_message_type(message_type) + # Check registration only (not enabled status) for consistency + return message_type in self._entriesOr document the current behavior explicitly in the docstring.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (2)
src/omnibase_infra/runtime/registry/registry_message_type.py(1 hunks)tests/unit/runtime/registry/test_registry_message_type.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: All data structures must be proper Pydantic models - never use Any types. Use specific types instead.
Use X | None (PEP 604) union syntax for nullable types instead of Optional[X]
Use ProtocolConfigurationError for configuration validation failures, SecretResolutionError for credential resolution failures, InfraConnectionError for connection failures, InfraTimeoutError for operation timeouts, InfraAuthenticationError for auth failures, and InfraUnavailableError for resource unavailable
Use EnumMessageCategory (EVENT, COMMAND, INTENT) for message routing and topic parsing. Use EnumNodeOutputType (EVENT, COMMAND, INTENT, PROJECTION) for execution shape and handler return type validation. PROJECTION only valid for REDUCER nodes
Do not use Any types anywhere in the codebase - always use specific types. If type is not known at definition time, use object as the type parameter instead
Use duck typing through protocols instead of isinstance checks. Protocol resolution based on structural matching, not type checking
Files:
tests/unit/runtime/registry/test_registry_message_type.pysrc/omnibase_infra/runtime/registry/registry_message_type.py
**/registry_*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use registry_.py file naming with Registry class naming for standalone registries in domain directories
Files:
src/omnibase_infra/runtime/registry/registry_message_type.py
🧠 Learnings (1)
📚 Learning: 2025-12-20T19:53:07.665Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T19:53:07.665Z
Learning: Applies to **/*.py : Use EnumMessageCategory (EVENT, COMMAND, INTENT) for message routing and topic parsing. Use EnumNodeOutputType (EVENT, COMMAND, INTENT, PROJECTION) for execution shape and handler return type validation. PROJECTION only valid for REDUCER nodes
Applied to files:
src/omnibase_infra/runtime/registry/registry_message_type.py
🧬 Code graph analysis (1)
src/omnibase_infra/runtime/registry/registry_message_type.py (4)
src/omnibase_infra/enums/enum_message_category.py (2)
EnumMessageCategory(34-192)from_topic(121-159)src/omnibase_infra/errors/infra_errors.py (1)
RuntimeHostError(35-100)src/omnibase_infra/runtime/registry/model_domain_constraint.py (2)
ModelDomainConstraint(32-197)validate_consumption(150-197)src/omnibase_infra/runtime/registry/model_message_type_entry.py (3)
ModelMessageTypeEntry(33-270)with_additional_handler(226-256)validate_category(190-224)
🔇 Additional comments (14)
tests/unit/runtime/registry/test_registry_message_type.py (5)
22-180: Comprehensive edge case coverage for domain extraction.Excellent test coverage documenting expected behavior for malformed topic strings. The edge case tests (lines 86-180) serve as valuable documentation for how the function handles unexpected inputs without crashing.
182-362: Thorough registration tests with conflict detection.Good coverage of fan-out pattern, idempotent registration, and constraint conflict detection. The tests verify that conflicting category and domain constraints are properly rejected with descriptive error messages.
383-583: Well-structured query tests with fixture pattern.The
populated_registryfixture provides consistent test data, and the tests cover success paths, error handling (not found, category mismatch, domain mismatch), and various filtering combinations. Good verification of pre-freeze query failures.
585-690: Solid validation test coverage.Tests verify startup validation with missing handlers and topic-message validation for category/domain mismatches. The fail-fast behavior is properly tested.
692-887: Complete coverage of cross-domain, properties, and dunder methods.Cross-domain tests verify explicit opt-in requirement. Property tests correctly verify that handler_count and domain_count track unique values. Dunder method tests ensure string representations include key state information.
src/omnibase_infra/runtime/registry/registry_message_type.py (9)
68-119: Well-structured error class extending RuntimeHostError.Correctly extends
RuntimeHostErrorper coding guidelines for infrastructure errors. Uses PEP 604 union syntax (str | None) and properly propagates contextual fields for debugging.
142-245: Robust domain extraction with proper pattern precedence.The ONEX pattern correctly takes precedence over the environment-aware pattern. The fallback to
parts[1]at lines 241-243 handles edge cases gracefully. The extensive documentation of pattern matching order (lines 177-196, 206-226) is thorough.
323-358: Clean initialization with proper data structures.Uses appropriate types:
dictfor entries,setfor unique tracking, anddefaultdict(list)for efficient category/domain indexing. The lock protects registration operations.
363-475: Correct fan-out merge logic with constraint validation.The registration correctly validates that subsequent registrations for the same message type have matching category and domain constraints before merging handlers. The
with_additional_handlerpattern preserves immutability of the entry model.
529-560: Idempotent freeze with proper locking.The freeze method correctly uses the lock for thread safety and is idempotent by design. Good logging of registry state at freeze time.
566-646: Comprehensive validation in get_handlers.Properly validates: frozen state, entry existence, enabled status, category constraint, and domain constraint. The error message at lines 609-616 helpfully shows registered types for debugging.
713-759: Efficient filtering with set intersection.The
list_message_typesmethod correctly uses set intersection when both category and domain filters are provided, and filters to enabled entries only.
884-963: Solid topic-message type validation.Correctly extracts both category and domain from the topic string and validates against registered constraints. Error messages are descriptive and actionable.
1-60: Well-documented module with clear design principles.Excellent module-level docstring explaining the freeze-after-init pattern, thread safety model, performance characteristics, and related tickets. The
__all__export list is explicit and correct.
1 participant
Summary
Implements the central message type registry (B1a) that maps message types to handler implementations and enforces topic category constraints for the ONEX runtime dispatch infrastructure.
Depends on: OMN-934 (B1) Runtime Dispatch Engine ✅ Done
Unblocks: OMN-941 (B2) Handler Output Model (already done), future runtime integration work
Key Features
Files Added
runtime/registry/__init__.pyruntime/registry/model_domain_constraint.pyruntime/registry/model_message_type_entry.pyruntime/registry/protocol_message_type_registry.pyruntime/registry/registry_message_type.pyTest Coverage
Acceptance Criteria
Test plan
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.