Repository navigation
feat(OMN-10794): wire HandlerComplianceLoop into HandlerDelegationWorkflow - #1560
Merged
Merged
Conversation
…kflow Wave 3 / Task 5 of the tokens-to-compliance epic. Adds optional ``output_schema_key`` and ``compliance_budget`` fields to ModelDelegationRequest. When set, HandlerDelegationWorkflow.handle_inference_response invokes HandlerComplianceLoop per attempt: * compliant or budget ABORT → record the attempt, transition ROUTED → INFERENCE_COMPLETED, forward to the quality gate (terminal event carries the running tokens_to_compliance + compliance_attempts) * non-compliant + budget CONTINUE → emit a fresh ModelInferenceIntent carrying the repair prompt and stay in ROUTED (self-loop), incrementing compliance_attempts for the next iteration DelegationWorkflowState gains ``compliance_attempts`` and ``accumulated_tokens``; the FSM transition table allows ROUTED → ROUTED for the repair re-prompt path (both in code _VALID_TRANSITIONS and in contract.yaml). Bumps node contract version to 0.3.0 and updates the FSM transition documentation. The compliance counters are populated onto the terminal ModelDelegationResult and ModelTaskDelegatedEvent so the omnimarket projection (OMN-10793) and the omniclaude sqlite_adapter (OMN-10789) can write them to delegation_events. Backwards compatibility: legacy callers that omit ``output_schema_key`` get the existing single-attempt path unchanged. Their terminal event carries ``compliance_attempts=1`` and ``tokens_to_compliance`` equal to that single attempt's total_tokens — semantically equivalent to first-try success. Tests: 9 new unit tests prove first-try compliance, repair-on-failure with self-loop, two-attempt token accumulation, budget ABORT path, and FSM self-loop legality. 32 existing orchestrator tests still pass. mypy strict clean. pre-commit clean. Evidence-Source: OCC#905 Evidence-Ticket: OMN-10794
jonahgabriel
enabled auto-merge (squash)
May 10, 2026 10:24
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements a compliance-loop feature for the HandlerDelegationWorkflow orchestrator, allowing schema validation and repair re-prompts during LLM inference. The request model gains optional ChangesOMN-10794: Compliance-Loop Wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/omnibase_infra/nodes/node_delegation_orchestrator/handlers/handler_delegation_workflow.py`:
- Around line 368-387: The handler currently asserts workflow.routing_decision
is not None which breaks cases where handle_invocation_command leaves a ROUTED
workflow without a routing_decision; remove that unconditional assert and
instead guard usage: only require routing_decision when you actually need it
(e.g., before calling codepaths that depend on it such as _evaluate_compliance
or any routing-specific logic), or update _evaluate_compliance to accept a None
routing_decision and handle it safely; ensure unexpected/replayed inference
responses for correlation IDs are ignored (return early) when routing_decision
is missing rather than crashing.
In
`@src/omnibase_infra/nodes/node_delegation_orchestrator/models/model_delegation_request.py`:
- Around line 73-89: Add a model-level validator on ModelDelegationRequest to
enforce the invariant between output_schema_key and compliance_budget: use
pydantic's `@root_validator` (or `@model_validator` for pydantic v2) to check that
if output_schema_key is not None then compliance_budget must also be not None
(and optionally reject compliance_budget being set while output_schema_key is
None), and raise a ValueError with a clear message when the invariant is
violated so invalid combinations are rejected at validation time rather than
causing a runtime assertion in the handler.
🪄 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: 304d5c4d-ed95-47ff-893f-e862786ed4ce
📒 Files selected for processing (5)
contracts/OMN-10794.yamlsrc/omnibase_infra/nodes/node_delegation_orchestrator/contract.yamlsrc/omnibase_infra/nodes/node_delegation_orchestrator/handlers/handler_delegation_workflow.pysrc/omnibase_infra/nodes/node_delegation_orchestrator/models/model_delegation_request.pytests/unit/nodes/node_delegation_orchestrator/test_compliance_loop_wiring.py
…p config Two CodeRabbit findings + Integration Test Coverage gate: 1. ModelDelegationRequest: add @model_validator(mode='after') rejecting output_schema_key set without compliance_budget. The compliance loop's evaluator requires both — catching this at model construction prevents a downstream assertion crash in the workflow handler when the orchestrator first sees the inference response. (CodeRabbit #2) 2. tests/integration/delegation/test_compliance_loop_wiring_integration.py: exercise HandlerDelegationWorkflow's compliance-loop path against unmocked omnimarket schema-registry / schema-repair / budget-policy handlers and prove the cross-package contract holds end-to-end (3 tests). Closes the Integration Test Coverage gap. Note: CodeRabbit finding #1 (don't require routing_decision for every ROUTED workflow in handle_inference_response) is already addressed by the existing 'if workflow.routing_decision is None: return []' guard at line 389. Evidence-Source: OCC#907 Evidence-Ticket: OMN-10794
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
1 participant
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.
Evidence-Source: OCC#907
Evidence-Ticket: OMN-10794
Summary
Wave 3 / Task 5 of the tokens-to-compliance epic. Wires the OMN-10792
HandlerComplianceLoopinto the delegation orchestrator so that delegations opting in viaoutput_schema_keyget per-attempt schema validation, repair-prompt re-issue, and accumulatedtokens_to_compliance/compliance_attemptson the terminal event.Changes
ModelDelegationRequest(additive, optional)output_schema_key: str | None— when set, activates the loop. Resolves to a schema in omnimarket's output schema registry.compliance_budget: ModelBudgetLimits | None— token / cost / time ceilings the loop enforces between attempts.HandlerDelegationWorkflow.handle_inference_response(split paths)Two paths inside the same handler:
output_schema_key is None) — accept the response on the first attempt and forward to the quality gate. Behavior unchanged.output_schema_key is set) — callHandlerComplianceLoop.evaluate(...):ModelInferenceIntentcarrying the repair prompt and stay inROUTED(self-loop), incrementcompliance_attemptsfor the next iterationDelegationWorkflowStateNew fields:
compliance_attempts: int = 0,accumulated_tokens: int = 0. The orchestrator owns the loop counters; the loop handler is per-attempt-stateless.FSM
ROUTED → ROUTEDself-loopAdded to
_VALID_TRANSITIONSandcontract.yamltransition table. Triggered by repair re-prompts from the loop.Terminal event population
handle_gate_resultnow populatestokens_to_complianceandcompliance_attemptson bothModelDelegationResultand the backwards-compatibleModelTaskDelegatedEvent. Defaults preserve legacy semantics: 1 attempt, total tokens of that single attempt.Contract bump
node_delegation_orchestrator/contract.yaml→ 0.3.0 (was 0.2.0 from OMN-10792). Updated description documents the new opt-in loop wiring; FSM section adds the self-loop transition.Tests
tests/unit/nodes/node_delegation_orchestrator/test_compliance_loop_wiring.py:--strictclean.Why this is a runtime change (not pure-compute)
The wiring runs in the orchestrator's hot path. Downstream consumers (
HandlerProjectionDelegationfrom OMN-10793,sqlite_adapterfrom OMN-10789) are already wired fortokens_to_compliance+compliance_attemptswith safe defaults, so a rolling restart is the only deploy step. No schema migration, no data migration.Files
src/omnibase_infra/nodes/node_delegation_orchestrator/handlers/handler_delegation_workflow.pysrc/omnibase_infra/nodes/node_delegation_orchestrator/models/model_delegation_request.pysrc/omnibase_infra/nodes/node_delegation_orchestrator/contract.yamltests/unit/nodes/node_delegation_orchestrator/test_compliance_loop_wiring.pycontracts/OMN-10794.yamlSummary by CodeRabbit