Skip to content

fix: address PR #180 review with proper typed Pydantic models #186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jonahgabriel merged 28 commits into from
Dec 15, 2025
Merged

fix: address PR #180 review with proper typed Pydantic models #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9cf09ff
feat: add new models, contracts, and documentation [OMN-675]
jonahgabriel Dec 13, 2025
3e77ae7
fix: address PR #180 review feedback for models and contracts
jonahgabriel Dec 13, 2025
0a16f9b
fix: address remaining PR #180 review feedback
jonahgabriel Dec 13, 2025
17aefde
Merge branch 'main' into feat/new-models-contracts-docs
jonahgabriel Dec 13, 2025
bbc779d
fix: address PR #180 review with proper typed Pydantic models
jonahgabriel Dec 13, 2025
667402a
merge: resolve conflicts with main branch
jonahgabriel Dec 13, 2025
eca3257
docs: add comprehensive docstrings for typed Pydantic models
jonahgabriel Dec 13, 2025
45262ee
fix: remove duplicate ModelGraphNodeInputs class from model_typed_met…
jonahgabriel Dec 14, 2025
258a79a
fix: correct fingerprint version format and mixin default type in tests
jonahgabriel Dec 14, 2025
6e5b3a2
merge: resolve conflicts with main branch
jonahgabriel Dec 14, 2025
6f406c7
fix(models): address PR #186 review feedback
jonahgabriel Dec 14, 2025
730dd28
fix(models): add security constraints and O(1) caching [PR #186]
jonahgabriel Dec 14, 2025
175479e
fix(models): address PR #180 review feedback
jonahgabriel Dec 14, 2025
3316403
Merge remote-tracking branch 'origin/main' into fix/pr-180-review-fee…
jonahgabriel Dec 14, 2025
33bef28
fix(contracts): remove 'v' prefix from fingerprint versions
jonahgabriel Dec 14, 2025
bb70fca
Merge branch 'main' into fix/pr-180-review-feedback
jonahgabriel Dec 14, 2025
1323642
fix(models): address CodeRabbit PR #186 review feedback
jonahgabriel Dec 14, 2025
39108cc
fix(models): use Self return types, ModelOnexError, remove dict round…
jonahgabriel Dec 14, 2025
376ff24
fix(models): address additional PR #186 review feedback
jonahgabriel Dec 14, 2025
8b6b442
feat(events): add runtime event models for event_bus_wiring contract
jonahgabriel Dec 14, 2025
15bcf71
fix(models): convert model_typed_metadata.py to re-export module
jonahgabriel Dec 15, 2025
b7e657f
fix(events): address PR #186 review feedback for runtime event models
jonahgabriel Dec 15, 2025
8eb0211
fix(models): address PR #186 review feedback - comprehensive fixes
jonahgabriel Dec 15, 2025
9eea791
docs(CLAUDE.md): fix from_attributes=True model paths and expand list
jonahgabriel Dec 15, 2025
d0a5548
fix(models): fix CI test failures for introspection and contract tests
jonahgabriel Dec 15, 2025
9470975
fix(models): address PR #186 review feedback - nitpicks and improvements
jonahgabriel Dec 15, 2025
85f0882
fix(models): address PR #186 review feedback - release ready improvem…
jonahgabriel Dec 15, 2025
dd1c7a5
fix(models): fix Pydantic validation errors for config_schema and typ…
jonahgabriel Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,12 +797,19 @@ class ModelSemVer(BaseModel):
patch: int
```

**Models using this pattern**:
**Key models using this pattern** (29 total in codebase):
- `ModelSemVer` - Semantic versioning (`models/primitives/model_semver.py`)
- `ModelWorkflowNode` - Workflow nodes (`models/workflow/model_workflow_node.py`)
- `ModelExecutionGraph` - Execution graphs (`models/graph/model_execution_graph.py`)
- `ModelWorkflowDefinitionMetadata` - Workflow metadata (`models/workflow/model_workflow_definition_metadata.py`)
- `ModelServiceMetadata` - Service metadata (`models/service/model_service_metadata.py`)
- `ModelWorkflowNode` - Workflow nodes (`models/contracts/subcontracts/model_workflow_node.py`)
- `ModelExecutionGraph` - Execution graphs (`models/contracts/subcontracts/model_execution_graph.py`)
- `ModelWorkflowDefinitionMetadata` - Workflow metadata (`models/contracts/subcontracts/model_workflow_definition_metadata.py`)
- `ModelServiceMetadata` - Service metadata (`models/container/model_service_metadata.py`)
- `ModelContractBase` - Base contract model (`models/contracts/model_contract_base.py`)
- `ModelFSMStateSnapshot` - FSM state snapshots (`models/fsm/model_fsm_state_snapshot.py`)
- Various metadata models in `models/common/` (effect, reducer, tool, node capabilities metadata)
- Event models in `models/events/` (runtime event base, wiring error info, node graph info)
- GitHub workflow configuration models in `models/configuration/`

To find all models using this pattern: `grep -r "from_attributes.*True" src/omnibase_core/models/`

**When NOT to use**:
- Models that are never nested in other Pydantic models
Expand Down
35 changes: 35 additions & 0 deletions contracts/runtime/event_bus_wiring_effect.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,17 +221,52 @@ health_check:
#
# These handlers are implemented by the NodeEventBusWiringEffect class or provided by the
# runtime's built-in subscription handler executor. See docs/architecture/SUBSCRIPTION_HANDLERS.md.
#
# TODO: Implement subscription handler registration in the runtime:
# 1. Create SubscriptionHandlerRegistry to map handler names to implementations
# 2. Implement wire_all_subscriptions in NodeEventBusWiringEffect or runtime executor
# 3. Implement wire_single_subscription for hot-reload support
# 4. Implement remove_subscription for graceful node removal
# 5. Add handler validation during contract loading to fail-fast on missing handlers
subscriptions:
# Implementation Status: PLANNED
# These handlers are contract-declared but not yet runtime-registered.
# The runtime will validate handler availability during contract loading
# and emit warnings for unregistered handlers in non-strict mode.
#
# Tracking: Planned for v1.2.0 - SubscriptionHandlerRegistry implementation
# Status: Handler implementation pending (not yet runtime-registered)
topics:
- topic: onex.runtime.node_graph.ready
description: Triggered when node graph is fully instantiated
handler: wire_all_subscriptions
handler_status: planned # NOT_IMPLEMENTED - requires SubscriptionHandlerRegistry
handler_implementation:
class: NodeEventBusWiringEffect
method: _handle_node_graph_ready
input_schema: ModelNodeGraphReadyEvent
output_schema: ModelWiringResultEvent
idempotent: true
- topic: onex.runtime.node.registered
description: Handle new node registration for hot reload
handler: wire_single_subscription
handler_status: planned # NOT_IMPLEMENTED - requires SubscriptionHandlerRegistry
handler_implementation:
class: NodeEventBusWiringEffect
method: _handle_node_registered
input_schema: ModelNodeRegisteredEvent
output_schema: ModelSubscriptionCreatedEvent
idempotent: true
- topic: onex.runtime.node.unregistered
description: Handle node removal
handler: remove_subscription
handler_status: planned # NOT_IMPLEMENTED - requires SubscriptionHandlerRegistry
handler_implementation:
class: NodeEventBusWiringEffect
method: _handle_node_unregistered
input_schema: ModelNodeUnregisteredEvent
output_schema: ModelSubscriptionRemovedEvent
idempotent: true
Comment on lines 239 to +269

@coderabbitai coderabbitai Bot Dec 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify that all referenced event schema models are defined in the codebase.

# Search for model definitions of event-related schemas referenced in the YAML
echo "=== Searching for event model definitions ==="
rg -n 'class\s+(ModelNodeGraphReadyEvent|ModelNodeRegisteredEvent|ModelNodeUnregisteredEvent|ModelWiringResultEvent|ModelSubscriptionCreatedEvent|ModelSubscriptionRemovedEvent|ModelRuntimeReadyEvent|ModelSubscriptionFailedEvent|ModelWiringErrorEvent)' --type py

echo ""
echo "=== Checking if these models are exported from __init__.py or __all__ ==="
rg -n '__all__\s*=' src/omnibase_core/models/ -A 20 | grep -E '(ModelNodeGraphReadyEvent|ModelNodeRegisteredEvent|ModelNodeUnregisteredEvent|ModelWiringResultEvent|ModelSubscriptionCreatedEvent|ModelSubscriptionRemovedEvent|ModelRuntimeReadyEvent|ModelSubscriptionFailedEvent|ModelWiringErrorEvent)' || echo "No matches found in __all__ exports"

echo ""
echo "=== Listing all model files to understand structure ==="
find src/omnibase_core/models -name "*.py" -type f | head -20

Repository: OmniNode-ai/omnibase_core

Length of output: 1361

🏁 Script executed:

#!/bin/bash
# Broader search for event-related models
echo "=== Searching for any Event model definitions ==="
rg -n 'class\s+Model.*Event' --type py src/omnibase_core/models/

echo ""
echo "=== Checking runtime-related model directories ==="
find src/omnibase_core/models -type d -name "*runtime*" -o -name "*event*" -o -name "*node*"

echo ""
echo "=== Searching for files containing 'runtime' or 'event' in models ==="
find src/omnibase_core/models -name "*.py" -type f | xargs grep -l "runtime\|event" 2>/dev/null | head -20

echo ""
echo "=== Checking if referenced models are imported from elsewhere ==="
rg -n 'ModelNodeGraphReadyEvent|ModelRuntimeReadyEvent' --type py

Repository: OmniNode-ai/omnibase_core

Length of output: 20913

🏁 Script executed:

#!/bin/bash
# Check runtime directory contents
echo "=== Contents of src/omnibase_core/models/runtime ==="
ls -la src/omnibase_core/models/runtime/

echo ""
echo "=== Files in runtime directory ==="
find src/omnibase_core/models/runtime -name "*.py" -type f

echo ""
echo "=== Final confirmation - search for the exact model names ==="
rg "ModelNodeGraphReadyEvent|ModelNodeRegisteredEvent|ModelNodeUnregisteredEvent|ModelWiringResultEvent|ModelSubscriptionCreatedEvent|ModelSubscriptionRemovedEvent|ModelRuntimeReadyEvent|ModelSubscriptionFailedEvent|ModelWiringErrorEvent" --type py

Repository: OmniNode-ai/omnibase_core

Length of output: 575

Create the missing event schema models referenced in the handler implementations.

The YAML references nine event schema models that do not exist in the codebase:

  • Subscription handlers: ModelNodeGraphReadyEvent, ModelNodeRegisteredEvent, ModelNodeUnregisteredEvent, ModelWiringResultEvent, ModelSubscriptionCreatedEvent, ModelSubscriptionRemovedEvent
  • Publication handlers: ModelRuntimeReadyEvent, ModelSubscriptionFailedEvent, ModelWiringErrorEvent

These models must be defined and exported before the contract can be validated at runtime. Define them in src/omnibase_core/models/runtime/ or another appropriate location and ensure they are properly exported from the models package.


# Events published by this node
publications:
Expand Down
3 changes: 2 additions & 1 deletion contracts/runtime/node_graph_reducer.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ state_transitions:
- cleanup_graph_state
exit_actions: []
is_terminal: true
is_recoverable: false
metadata:
ui_color: '#6B7280'
icon: stop-circle
Expand All @@ -109,7 +110,7 @@ state_transitions:
description: Node registry is loaded and ready, begin wiring nodes
conditions:
- condition_name: registry_loaded
expression: node_registry is_not_empty
expression: node_registry min_length 1
required: true
description: Node registry must contain at least one node definition
version: {major: 1, minor: 0, patch: 0}
Expand Down
5 changes: 3 additions & 2 deletions docs/architecture/CONTAINER_TYPES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# Container Types in omnibase_core

**Status**: ✅ Complete
**Version**: 0.2.0
**Last Updated**: 2025-11-03
**Version**: 0.4.0
**Last Updated**: 2025-12-13

---

Expand Down Expand Up @@ -421,4 +421,5 @@ async def test_service_resolution():
---

**Version History**:
- **v0.4.0** (2025-12-13): Updated documentation metadata, refined examples
- v0.1.0 (2025-10-30) - Initial documentation created to clarify container type distinction
2 changes: 1 addition & 1 deletion docs/ci/DEPRECATION_WARNINGS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ filterwarnings = [
"default::DeprecationWarning",
"default::PendingDeprecationWarning",
# Suppress specific unavoidable warning from dependency
# TODO(OMN-XXX): Remove when library-name >= X.Y.Z
# TODO(YOUR-TICKET): Remove when library-name >= X.Y.Z
"ignore:specific warning message:DeprecationWarning:library_name",
]
```
Expand Down
6 changes: 3 additions & 3 deletions docs/conventions/NAMING_CONVENTIONS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ event_handling = ModelEventHandlingSubcontract(
```

**Components:**
- `semver`: Semantic version **without 'v' prefix** (e.g., `1.0.0`, `0.4.0-beta.1`)
- `semver`: Semantic version **without 'v' prefix** (e.g., `1.0.0`, `0.3.6-beta.1`)
- `:`: Separator (single colon)
- `hash`: First N hex characters of SHA256 (default: 12)

Expand All @@ -378,7 +378,7 @@ event_handling = ModelEventHandlingSubcontract(

```text
1.0.0:8fa1e2b4c9d1
0.4.0:abcdef123456
0.3.6:abcdef123456
2.1.3-beta.1:deadbeef0000
```

Expand Down Expand Up @@ -470,5 +470,5 @@ models/

---

**Last Updated**: 2025-12-12
**Last Updated**: 2025-12-14
**Project**: omnibase_core v0.4.0
4 changes: 2 additions & 2 deletions docs/conventions/PYDANTIC_BEST_PRACTICES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,5 +360,5 @@ Need from_attributes?

---

**Last Updated**: 2025-12-12
**Project**: omnibase_core v0.4.0
**Last Updated**: 2025-12-14
**Project**: omnibase_core v0.3.6
6 changes: 3 additions & 3 deletions docs/guides/DECLARATIVE_NODE_IMPORT_RULES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ from omnibase_core.decorators import allow_dict_any
class NodeLegacyProcessor(NodeCompute):
@allow_dict_any(
reason="External API returns untyped JSON",
ticket="OMN-XXX",
ticket="YOUR-TICKET",
migration_plan="Convert to typed model in v0.5.0"
)
def parse_external_response(self, response: dict[str, Any]) -> ModelParsedResponse:
Expand All @@ -437,7 +437,7 @@ class NodeLegacyProcessor(NodeCompute):
This exemption is required because:
1. External API does not provide OpenAPI schema
2. Response structure varies by endpoint version
3. Typed wrapper being developed under OMN-XXX
3. Typed wrapper being developed under separate ticket
"""
# Implementation
...
Expand All @@ -453,7 +453,7 @@ class NodeLegacyProcessor(NodeCompute):
For legacy code during migration:

```python
# ONEX_EXCLUDE: Legacy bridge code, migration tracked in OMN-XXX
# ONEX_EXCLUDE: Legacy bridge code, migration tracked in YOUR-TICKET
from typing import Any # noqa: ONEX001

def legacy_bridge(data: Any) -> Any: # noqa: ONEX001
Expand Down
8 changes: 4 additions & 4 deletions docs/guides/ENUM_NODE_KIND_MIGRATION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# EnumNodeKind Migration Guide

> **Version**: 0.2.0
> **Last Updated**: 2025-12-03
> **Version**: 0.4.0
> **Last Updated**: 2025-12-13
> **Related**: [CLAUDE.md](../../CLAUDE.md), [ONEX Four-Node Architecture](../architecture/ONEX_FOUR_NODE_ARCHITECTURE.md)

---
Expand Down Expand Up @@ -266,5 +266,5 @@ def get_node_kind(cls, node_type: EnumNodeType) -> EnumNodeKind:

---

**Last Updated**: 2025-12-03
**Version**: 0.2.0
**Last Updated**: 2025-12-13
**Version**: 0.4.0
4 changes: 2 additions & 2 deletions docs/testing/CI_TEST_STRATEGY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,6 @@ For operational guidance on monitoring CI health, detecting performance regressi

---

**Last Updated**: 2025-11-01
**Test Suite Version**: 0.2.0 (12,198 tests)
**Last Updated**: 2025-12-13
**Test Suite Version**: 0.4.0 (12,198 tests)
**CI Strategy Version**: v2.1 (20 splits + conditional coverage + monitoring)
Loading