Repository navigation
feat: DelegationIntentBridge completes delegation chain end-to-end [OMN-7604] - #1180
Merged
Merged
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdded multiple delegation-related event/topic constants, implemented a DelegationIntentBridge to execute routing/inference/quality-gate intents and publish envelopes to the event bus, and added end-to-end tests exercising the full delegation pipeline and idempotency. Changes
Sequence DiagramsequenceDiagram
participant Orchestrator as Orchestrator
participant Bridge as DelegationIntentBridge
participant LLM as LLM/Reducer
participant EventBus as Event Bus
participant Tests as Test Assertions
Orchestrator->>Bridge: ModelRoutingIntent
Bridge->>LLM: routing_delta(intent.payload)
LLM-->>Bridge: ModelRoutingDecision
Bridge->>EventBus: publish_envelope(TOPIC_DELEGATION_ROUTING_DECISION)
EventBus-->>Tests: recorded
Orchestrator->>Bridge: ModelInferenceIntent
Bridge->>LLM: llm_caller.call(intent)
LLM-->>Bridge: ModelInferenceResponseData
Bridge->>EventBus: publish_envelope(TOPIC_DELEGATION_INFERENCE_RESPONSE)
EventBus-->>Tests: recorded
Orchestrator->>Bridge: ModelQualityGateIntent
Bridge->>LLM: quality_gate_delta(intent.payload)
LLM-->>Bridge: ModelQualityGateResult
Bridge->>EventBus: publish_envelope(TOPIC_DELEGATION_QUALITY_GATE_RESULT)
EventBus-->>Tests: recorded
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/omnibase_infra/event_bus/topic_constants.py`:
- Around line 492-495: The TOPIC_DELEGATION_INFERENCE_RESPONSE constant is using
a hardcoded wire string; replace it with the topic value from the contract
loader / generated contract symbols (do not hardcode "onex.evt..."). Update
TOPIC_DELEGATION_INFERENCE_RESPONSE to reference the contract-provided name (for
example via the contract loader API or generated symbol such as
ContractTopics.INFERENCE_RESPONSE or contracts.get_topic("inference-response"))
so the value comes from the contract definitions rather than a literal string.
In
`@src/omnibase_infra/nodes/node_delegation_orchestrator/delegation_intent_bridge.py`:
- Around line 128-130: Replace the plain RuntimeError raised when
self._llm_caller is None with an infra exception created via
ModelInfraErrorContext.with_correlation(...): use the intent correlation id
(e.g., self._intent.correlation_id) and call the context factory to create/raise
an infra-level exception (keeping the same descriptive message) instead of
raising RuntimeError; locate the check around self._llm_caller in
DelegationIntentBridge/delegation_intent_bridge.py and replace the raise with
ModelInfraErrorContext.with_correlation(self._intent.correlation_id).<create_or_raise_infra_exception>(message)
so the error carries infra context and correlation propagation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f047969d-e6c3-4e81-a6f2-e9ec62291878
⛔ Files ignored due to path filters (1)
src/omnibase_infra/enums/generated/enum_omnibase_infra_topic.pyis excluded by!**/generated/**
📒 Files selected for processing (3)
src/omnibase_infra/event_bus/topic_constants.pysrc/omnibase_infra/nodes/node_delegation_orchestrator/delegation_intent_bridge.pytests/unit/delegation/test_delegation_chain_e2e.py
jonahgabriel
enabled auto-merge
April 7, 2026 07:34
jonahgabriel
added this pull request to the merge queue
Apr 7, 2026
jonahgabriel
changed the title
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Apr 7, 2026
jonahgabriel
force-pushed
the
jonah/fix-delegation-chain-inmemory
branch
from
April 7, 2026 09:42
f250865 to
b49fd3d
Compare
jonahgabriel
enabled auto-merge
April 7, 2026 09:45
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/omnibase_infra/nodes/node_delegation_orchestrator/delegation_intent_bridge.py (1)
177-191: Consider extracting lazy imports to module level.The
datetimeandModelEventEnvelopeimports inside_publishare lazy imports. While this works, these imports are unlikely to cause circular dependency issues and could be moved to the module level for consistency with the rest of the file.♻️ Optional: Move imports to module level
Add to the existing imports section (lines 23-62):
from datetime import UTC, datetime from omnibase_core.models.events.model_event_envelope import ModelEventEnvelopeThen simplify
_publish:async def _publish(self, model: BaseModel, topic: str) -> None: """Publish a Pydantic model as an event envelope to the bus.""" - from datetime import UTC, datetime - - from omnibase_core.models.events.model_event_envelope import ( - ModelEventEnvelope, - ) - correlation_id: UUID | None = getattr(model, "correlation_id", None)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/omnibase_infra/nodes/node_delegation_orchestrator/delegation_intent_bridge.py` around lines 177 - 191, Move the lazy imports out of the _publish method to the module top: import UTC and datetime from datetime and import ModelEventEnvelope from omnibase_core.models.events.model_event_envelope at the file-level, then remove the in-method imports in _publish so it simply constructs ModelEventEnvelope(payload=model, correlation_id=..., envelope_timestamp=datetime.now(UTC)) and calls self._event_bus.publish_envelope; this keeps behavior identical but centralizes imports and matches the rest of the file.tests/unit/delegation/test_delegation_chain_e2e.py (2)
86-88: Fix type annotation: fixture returnsUUID, notobject.The return type annotation
objectis overly generic. Sinceuuid4()returnsUUID, the annotation should reflect this for type safety and IDE support.♻️ Proposed fix
+ from uuid import UUID + `@pytest.fixture` - def correlation_id(self) -> object: + def correlation_id(self) -> UUID: return uuid4()Note:
UUIDis already imported at line 25, so only the annotation change is needed:`@pytest.fixture` - def correlation_id(self) -> object: + def correlation_id(self) -> UUID: return uuid4()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/delegation/test_delegation_chain_e2e.py` around lines 86 - 88, The fixture correlation_id currently annotates its return as object but it returns uuid4(), so change the return type annotation of the correlation_id fixture from object to UUID (UUID is already imported) to accurately reflect the returned value and improve type safety and IDE support.
90-98: Update parameter type to match corrected fixture return type.If
correlation_idfixture return type is corrected toUUID, this parameter annotation should also be updated for consistency.♻️ Proposed fix
`@pytest.fixture` - def delegation_request(self, correlation_id: object) -> ModelDelegationRequest: + def delegation_request(self, correlation_id: UUID) -> ModelDelegationRequest: return ModelDelegationRequest(Note:
UUIDtype is already imported at line 25.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/delegation/test_delegation_chain_e2e.py` around lines 90 - 98, The delegation_request pytest fixture currently types its correlation_id parameter as object but the corrected correlation_id fixture returns a UUID; update the delegation_request signature to use UUID instead of object (i.e., change the parameter annotation on the delegation_request fixture to UUID) so the fixture type matches ModelDelegationRequest and existing imports; verify ModelDelegationRequest(...) remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/omnibase_infra/nodes/node_delegation_orchestrator/delegation_intent_bridge.py`:
- Around line 177-191: Move the lazy imports out of the _publish method to the
module top: import UTC and datetime from datetime and import ModelEventEnvelope
from omnibase_core.models.events.model_event_envelope at the file-level, then
remove the in-method imports in _publish so it simply constructs
ModelEventEnvelope(payload=model, correlation_id=...,
envelope_timestamp=datetime.now(UTC)) and calls
self._event_bus.publish_envelope; this keeps behavior identical but centralizes
imports and matches the rest of the file.
In `@tests/unit/delegation/test_delegation_chain_e2e.py`:
- Around line 86-88: The fixture correlation_id currently annotates its return
as object but it returns uuid4(), so change the return type annotation of the
correlation_id fixture from object to UUID (UUID is already imported) to
accurately reflect the returned value and improve type safety and IDE support.
- Around line 90-98: The delegation_request pytest fixture currently types its
correlation_id parameter as object but the corrected correlation_id fixture
returns a UUID; update the delegation_request signature to use UUID instead of
object (i.e., change the parameter annotation on the delegation_request fixture
to UUID) so the fixture type matches ModelDelegationRequest and existing
imports; verify ModelDelegationRequest(...) remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59e2079f-769a-49cf-8e72-11e67298fe4b
⛔ Files ignored due to path filters (2)
src/omnibase_infra/enums/generated/enum_omnibase_infra_topic.pyis excluded by!**/generated/**uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
src/omnibase_infra/event_bus/topic_constants.pysrc/omnibase_infra/nodes/node_delegation_orchestrator/delegation_intent_bridge.pytests/unit/contracts/test_protocol_ownership.pytests/unit/delegation/test_delegation_chain_e2e.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/unit/contracts/test_protocol_ownership.py
- src/omnibase_infra/event_bus/topic_constants.py
jonahgabriel
added this pull request to the merge queue
Apr 7, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Apr 7, 2026
jonahgabriel
force-pushed
the
jonah/fix-delegation-chain-inmemory
branch
from
April 7, 2026 10:16
b49fd3d to
668c9cb
Compare
jonahgabriel
enabled auto-merge
April 7, 2026 10:20
There was a problem hiding this comment.
🧹 Nitpick comments (4)
tests/unit/delegation/test_delegation_chain_e2e.py (4)
217-240: Cover allhandle_output_event()branches.This only exercises the
ModelRoutingIntentpath. A dispatch regression in theModelInferenceIntentorModelQualityGateIntentbranch would still pass, so the generic router is not fully pinned down yet.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/delegation/test_delegation_chain_e2e.py` around lines 217 - 240, The test test_handle_output_event_routes_correctly only exercises the ModelRoutingIntent branch of DelegationIntentBridge.handle_output_event; extend it to cover ModelInferenceIntent and ModelQualityGateIntent branches by constructing or obtaining intents of those types (e.g., via handler methods or by instantiating ModelInferenceIntent and ModelQualityGateIntent with minimal valid payloads), calling bridge.handle_output_event(intent) for each, and asserting the returned value/type is the expected result for each branch (similar to the existing assertion for ModelRoutingIntent) so all dispatch paths of handle_output_event are exercised.
284-305: Verify idempotency at the event-bus boundary too.Right now this only proves the second
handle_delegation_request()call returns no new intents. It does not prove the full chain avoids duplicate publishes for the same correlation ID. Comparing event-history counts before and after the duplicate request would lock that down.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/delegation/test_delegation_chain_e2e.py` around lines 284 - 305, The test only asserts handler.handle_delegation_request(delegation_request) is idempotent but doesn't verify the EventBusInmemory didn't receive duplicate publishes; update test_chain_idempotent_duplicate_request to capture the event-bus state (e.g., inspect EventBusInmemory's event history or published events) after starting the bus and after the first handle_delegation_request, then call handler.handle_delegation_request a second time and assert the event history length (or the set of correlation IDs) did not increase and contains only the original event for that correlation ID; reference EventBusInmemory, DelegationIntentBridge, and handler.handle_delegation_request to locate and implement the check.
162-175: Assert the terminal outputs explicitly.Line 164’s
>= 1check is too loose for this path. If the compatibility event or baseline-comparison request stops being emitted, this test still passes because the later bus assertions only cover the bridge’s intermediate topics. Please pin down the exact terminal outputs this flow is expected to produce.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/delegation/test_delegation_chain_e2e.py` around lines 162 - 175, The events length assertion is too loose—replace the generic assert len(events) >= 1 with explicit checks that handler.handle_gate_result(gate_result) returns the exact expected terminal events (e.g., delegation-completed, compatibility event, baseline-comparison request) by asserting len(events) equals the expected count and that the returned events contain the specific event types/identifiers; also keep the workflow state assertion (workflow.state == EnumDelegationState.COMPLETED) and ensure the bus history (await bus.get_event_history) still asserts all three topics TOPIC_DELEGATION_ROUTING_DECISION, TOPIC_DELEGATION_INFERENCE_RESPONSE, and TOPIC_DELEGATION_QUALITY_GATE_RESULT are present in topics_published.
243-282: Finish the topic-routing check with the quality-gate publish.This test stops after
TOPIC_DELEGATION_INFERENCE_RESPONSE. If the bridge starts publishingModelQualityGateIntentresults to the wrong topic, this suite would miss it even though that mapping is part of the new chain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/delegation/test_delegation_chain_e2e.py` around lines 243 - 282, The test stops after asserting TOPIC_DELEGATION_INFERENCE_RESPONSE and misses validating the final quality-gate publish; extend test_bridge_publishes_to_correct_topics to continue the chain by: deserialize the published inference event (use ModelInferenceResponse.model_validate with _extract_payload on inference_history[0]), pass that response to handler.handle_inference_response(...) to get quality gate intents, call await bridge.handle_quality_gate_intent(quality_gate_intents[0]) and then assert that bus.get_event_history(topic=TOPIC_DELEGATION_QUALITY_GATE) returns one event. This uses the existing DelegationIntentBridge, handler.handle_inference_response, and TOPIC_DELEGATION_QUALITY_GATE symbols to locate the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/delegation/test_delegation_chain_e2e.py`:
- Around line 217-240: The test test_handle_output_event_routes_correctly only
exercises the ModelRoutingIntent branch of
DelegationIntentBridge.handle_output_event; extend it to cover
ModelInferenceIntent and ModelQualityGateIntent branches by constructing or
obtaining intents of those types (e.g., via handler methods or by instantiating
ModelInferenceIntent and ModelQualityGateIntent with minimal valid payloads),
calling bridge.handle_output_event(intent) for each, and asserting the returned
value/type is the expected result for each branch (similar to the existing
assertion for ModelRoutingIntent) so all dispatch paths of handle_output_event
are exercised.
- Around line 284-305: The test only asserts
handler.handle_delegation_request(delegation_request) is idempotent but doesn't
verify the EventBusInmemory didn't receive duplicate publishes; update
test_chain_idempotent_duplicate_request to capture the event-bus state (e.g.,
inspect EventBusInmemory's event history or published events) after starting the
bus and after the first handle_delegation_request, then call
handler.handle_delegation_request a second time and assert the event history
length (or the set of correlation IDs) did not increase and contains only the
original event for that correlation ID; reference EventBusInmemory,
DelegationIntentBridge, and handler.handle_delegation_request to locate and
implement the check.
- Around line 162-175: The events length assertion is too loose—replace the
generic assert len(events) >= 1 with explicit checks that
handler.handle_gate_result(gate_result) returns the exact expected terminal
events (e.g., delegation-completed, compatibility event, baseline-comparison
request) by asserting len(events) equals the expected count and that the
returned events contain the specific event types/identifiers; also keep the
workflow state assertion (workflow.state == EnumDelegationState.COMPLETED) and
ensure the bus history (await bus.get_event_history) still asserts all three
topics TOPIC_DELEGATION_ROUTING_DECISION, TOPIC_DELEGATION_INFERENCE_RESPONSE,
and TOPIC_DELEGATION_QUALITY_GATE_RESULT are present in topics_published.
- Around line 243-282: The test stops after asserting
TOPIC_DELEGATION_INFERENCE_RESPONSE and misses validating the final quality-gate
publish; extend test_bridge_publishes_to_correct_topics to continue the chain
by: deserialize the published inference event (use
ModelInferenceResponse.model_validate with _extract_payload on
inference_history[0]), pass that response to
handler.handle_inference_response(...) to get quality gate intents, call await
bridge.handle_quality_gate_intent(quality_gate_intents[0]) and then assert that
bus.get_event_history(topic=TOPIC_DELEGATION_QUALITY_GATE) returns one event.
This uses the existing DelegationIntentBridge,
handler.handle_inference_response, and TOPIC_DELEGATION_QUALITY_GATE symbols to
locate the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca8c35d8-a419-48eb-821f-6718ba8d70c6
⛔ Files ignored due to path filters (1)
src/omnibase_infra/enums/generated/enum_omnibase_infra_topic.pyis excluded by!**/generated/**
📒 Files selected for processing (4)
src/omnibase_infra/event_bus/topic_constants.pysrc/omnibase_infra/nodes/node_delegation_orchestrator/delegation_intent_bridge.pytests/unit/contracts/test_protocol_ownership.pytests/unit/delegation/test_delegation_chain_e2e.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/unit/contracts/test_protocol_ownership.py
- src/omnibase_infra/nodes/node_delegation_orchestrator/delegation_intent_bridge.py
Run generate_topic_enums.py to update generated enum files with new delegation routing, quality gate, baseline comparison, and task delegated topics added in this branch.
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
2 participants
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
DelegationIntentBridgethat executes intents emitted by the delegation orchestrator dispatchers (routing, inference, quality gate) and publishes results back to the event busTOPIC_DELEGATION_INFERENCE_RESPONSEtopic constant for LLM inference responsesMockLlmCallerfor testing delegation chain without real LLM endpointsEventBusInmemoryProblem
The delegation orchestrator dispatchers emit intents (
ModelRoutingIntent,ModelInferenceIntent,ModelQualityGateIntent) asoutput_eventsinModelDispatchResult. TheDispatchResultApplierpublishes these to the event bus, but nothing on the receiving side executes them or feeds results back. The chain stalls at RECEIVED because:delta()withModelRoutingIntentpayloadsModelInferenceIntentdelta()withModelQualityGateIntentpayloadsFix
The
DelegationIntentBridgebridges this gap by:routing_delta,quality_gate_delta) or LLM effectTest plan
test_full_chain_completes_with_passing_gate— full chain: request -> route -> infer -> gate pass -> COMPLETEDtest_full_chain_fails_with_refusal— full chain with LLM refusal -> gate fail -> FAILEDtest_handle_output_event_routes_correctly— generic dispatch routes to correct handlertest_bridge_publishes_to_correct_topics— verifies events land on correct bus topicstest_chain_idempotent_duplicate_request— duplicate requests are idempotentSummary by CodeRabbit
New Features
Tests