Repository navigation
evidence(OMN-12409): OCC contract + receipts for EFFECT result_applier fix - #2985
Conversation
…r fix Central evidence for omnibase_infra PR #2086 which fixes the auto-wiring gap where EFFECT/REDUCER handler returned models were silently dropped because no result_applier was wired for those contract types. Receipts: dod-omnibase-infra-pr-2086, dod-unit-tests-pass, dod-deploy-runtime-wiring, dod-occ-pr-self.
📝 WalkthroughWalkthroughAdds the change-control contract manifest ChangesOMN-12409 Change Control Artifact
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
drift/dod_receipts/OMN-12409/dod-unit-tests-pass/command.yaml (1)
6-7: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider tightening the grep pattern for test results.
The current pattern
grep -q 'passed'is permissive and could match output like "7 failed, 1 passed" or even "passed" in error messages. Consider usinggrep -q '7 passed'to ensure all expected tests passed.♻️ Suggested improvement
-check_value: "uv run pytest tests/unit/runtime/test_effect_result_applier_wiring.py -q 2>&1 | grep -q - 'passed'" +check_value: "uv run pytest tests/unit/runtime/test_effect_result_applier_wiring.py -q 2>&1 | grep -q + '7 passed'"🤖 Prompt for 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. In `@drift/dod_receipts/OMN-12409/dod-unit-tests-pass/command.yaml` around lines 6 - 7, The grep pattern in the check_value field uses a permissive match for 'passed' which could incorrectly pass when there are failed tests mixed in the output (e.g., "7 failed, 1 passed"). Tighten the grep pattern to specifically check for the exact expected test count, such as changing the pattern from grep -q 'passed' to grep -q '7 passed' to ensure all tests actually passed and no failures occurred.
🤖 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 `@drift/dod_receipts/OMN-12409/dod-unit-tests-pass/command.yaml`:
- Around line 11-13: In the command.yaml file, update the commit_sha field from
its current value "5ff13e180" to the correct head commit sha of PR `#2086` which
is "e721252cf43065d6f645a53634ca5a075444809a" (or the abbreviated form
"e721252cf"). Keep the branch name and pr_number unchanged as they are already
correct.
---
Nitpick comments:
In `@drift/dod_receipts/OMN-12409/dod-unit-tests-pass/command.yaml`:
- Around line 6-7: The grep pattern in the check_value field uses a permissive
match for 'passed' which could incorrectly pass when there are failed tests
mixed in the output (e.g., "7 failed, 1 passed"). Tighten the grep pattern to
specifically check for the exact expected test count, such as changing the
pattern from grep -q 'passed' to grep -q '7 passed' to ensure all tests actually
passed and no failures occurred.
🪄 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: 7128a4c4-fc38-436c-ada4-64475a0d72cb
📒 Files selected for processing (5)
contracts/OMN-12409.yamldrift/dod_receipts/OMN-12409/dod-deploy-runtime-wiring/command.yamldrift/dod_receipts/OMN-12409/dod-occ-pr-self/command.yamldrift/dod_receipts/OMN-12409/dod-omnibase-infra-pr-2086/command.yamldrift/dod_receipts/OMN-12409/dod-unit-tests-pass/command.yaml
CodeRabbit (thread ccfd73e825d9bd708f4e370f): commit_sha in receipts referenced 5ff13e180 but the current head of infra PR #2086 is e721252cf. Updated all three receipts (dod-unit-tests-pass, dod-omnibase-infra-pr-2086, dod-deploy-runtime-wiring) to the correct SHA.
OMN-12409 — EFFECT result_applier wiring gap
Central OCC evidence for omnibase_infra PR #2086.
Problem fixed
EFFECT and REDUCER nodes whose handlers return a model destined for a published_events topic had their output silently dropped. The kernel's auto-wiring path only registered result_applier for a hard-coded set of ORCHESTRATOR contracts.
Fix (infra PR #2086)
After filtered_manifest is built in service_kernel.py, scan every contract. For each contract with a non-empty published_events map not already in auto_wiring_result_appliers, build a DispatchResultApplier from the contract's discovered contract_path. ORCHESTRATOR pre-registrations are not overwritten.
Evidence
Evidence-Ticket: OMN-12409
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
Documentation