Repository navigation
fix(OMN-12489): read delegation projection from analytics DB - #1011
Conversation
|
Warning Review limit reached
More reviews will be available in 37 minutes and 28 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR reorders published topics and switches handler routing to payload-type matching, replaces index-based topic constants with suffix-resolved lookups (and tests), and makes the projection API server prefer the analytics DB URL with a fallback to the contract DB. ChangesHandler Routing and Event Configuration
Analytics Database URL Selection
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 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 unit tests (beta)
Comment |
✅ Architectural Review — PASSEDErrors: 0 What this checks
No architectural violations found. Powered by node_pr_review_bot |
✅ PR Review Bot — CLEANFindings: 0
Verdict semantics
Powered by node_pr_review_bot — FSM pipeline with multi-model review fan-out |
|
| Verdict | Meaning | Blocks merge? |
|---|---|---|
passed |
No critical/error findings | No |
blocked |
CRITICAL or ERROR findings found | Yes |
degraded |
All models unavailable (infra) | No (pilot) |
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)
![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
🤖 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/omnimarket/nodes/node_llm_delegation_call_effect/contract.yaml`:
- Around line 96-101: The publish_topics order in contract.yaml no longer
matches the positional mapping used in HandlerLlmDelegationCall; update the
handler (HandlerLlmDelegationCall in
src/omnimarket/nodes/node_llm_delegation_call_effect/handlers/handler_llm_delegation_call.py)
to resolve topics by name instead of relying on _publish[0..3]: read the
publish_topics array from the node contract/metadata and look up each required
topic string (the values corresponding to TOPIC_DELEGATION_CALL_COMPLETED,
TOPIC_DELEGATION_CALL_ESCALATION_TRIGGERED, TOPIC_DELEGATION_ALL_TIERS_FAILED,
TOPIC_DELEGATION_MODEL_DEGRADED) by matching their topic names, assign them into
the handler's publish mapping by key, and use those named entries everywhere
instead of using fixed numeric indices; alternatively, revert contract.yaml to
the original ordering so _publish[0..3] remain correct.
🪄 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: e77c24c0-9fb1-4bc8-80fa-446e78b3b78d
📒 Files selected for processing (3)
src/omnimarket/nodes/node_llm_delegation_call_effect/contract.yamlsrc/omnimarket/projection/api_server.pytests/test_projection_api_server.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/nodes/node_llm_delegation_call_effect/test_handler_llm_delegation_call.py (1)
124-131: 💤 Low valueThese assertions are tautological — they restate the resolution invariant.
Each
TOPIC_DELEGATION_*constant is produced by_single_contract_topic(..., suffix), which returns a value that already satisfiesendswith(suffix). Soassert TOPIC_DELEGATION_CALL_COMPLETED.endswith("delegation-call-completed.v1")can never fail unless the module already raised at import. The only assertion with real coverage here is the ordering check onpublish_topics[0]. Consider asserting each resolved constant is actually present incontract["event_bus"]["publish_topics"]to verify resolution selected a declared topic.♻️ Make the assertions verify resolution against the contract
publish_topics = contract["event_bus"]["publish_topics"] assert publish_topics[0].endswith("inference-response.v1") -assert TOPIC_DELEGATION_CALL_COMPLETED.endswith("delegation-call-completed.v1") -assert TOPIC_DELEGATION_ESCALATION_TRIGGERED.endswith( - "delegation-escalation-triggered.v1" -) -assert TOPIC_DELEGATION_ALL_TIERS_FAILED.endswith( - "delegation-all-tiers-failed.v1" -) -assert TOPIC_DELEGATION_MODEL_DEGRADED.endswith("delegation-model-degraded.v1") +for resolved in ( + TOPIC_DELEGATION_CALL_COMPLETED, + TOPIC_DELEGATION_ESCALATION_TRIGGERED, + TOPIC_DELEGATION_ALL_TIERS_FAILED, + TOPIC_DELEGATION_MODEL_DEGRADED, +): + assert resolved in publish_topics🤖 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 `@tests/unit/nodes/node_llm_delegation_call_effect/test_handler_llm_delegation_call.py` around lines 124 - 131, The current assertions on TOPIC_DELEGATION_CALL_COMPLETED, TOPIC_DELEGATION_ESCALATION_TRIGGERED, TOPIC_DELEGATION_ALL_TIERS_FAILED, and TOPIC_DELEGATION_MODEL_DEGRADED are tautological because they mirror `_single_contract_topic(..., suffix)` behavior; instead assert that each resolved constant is actually present in the contract's publish topics (e.g., contract["event_bus"]["publish_topics"] or the local publish_topics variable used in test_handler_llm_delegation_call) to verify resolution selected a declared topic, while keeping the existing ordering assertion for publish_topics[0]; locate the constants and the publish_topics/contract usage in the test to make these presence assertions.
🤖 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/omnimarket/nodes/node_llm_delegation_call_effect/handlers/handler_llm_delegation_call.py`:
- Around line 62-99: The deploy-gate is rejecting the PR because it expects
runtime-evidence in a specific location/format; ensure the evidence for
OMN-12489 is placed exactly where the gate requires — typically as a commit
trailer (e.g., add `Evidence-Ticket: OMN-12489` and `Evidence-Source: OCC#2010`
to the latest commit message) or as the specific PR label/metadata the gate
checks (e.g., add the required `Evidence-Ticket` label or
`deploy:evidence=OMN-12489` label); update the PR/commit accordingly and re-run
the gate, and if it still fails paste the exact deploy-gate error output so we
can match the expected format (this is unrelated to code symbols like
_single_contract_topic or TOPIC_DELEGATION_EXECUTE which are fine).
---
Nitpick comments:
In
`@tests/unit/nodes/node_llm_delegation_call_effect/test_handler_llm_delegation_call.py`:
- Around line 124-131: The current assertions on
TOPIC_DELEGATION_CALL_COMPLETED, TOPIC_DELEGATION_ESCALATION_TRIGGERED,
TOPIC_DELEGATION_ALL_TIERS_FAILED, and TOPIC_DELEGATION_MODEL_DEGRADED are
tautological because they mirror `_single_contract_topic(..., suffix)` behavior;
instead assert that each resolved constant is actually present in the contract's
publish topics (e.g., contract["event_bus"]["publish_topics"] or the local
publish_topics variable used in test_handler_llm_delegation_call) to verify
resolution selected a declared topic, while keeping the existing ordering
assertion for publish_topics[0]; locate the constants and the
publish_topics/contract usage in the test to make these presence assertions.
🪄 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: 378c6f2d-039b-4f75-9899-fe42b7788053
📒 Files selected for processing (2)
src/omnimarket/nodes/node_llm_delegation_call_effect/handlers/handler_llm_delegation_call.pytests/unit/nodes/node_llm_delegation_call_effect/test_handler_llm_delegation_call.py
309919c to
8fed1c9
Compare
1 participant
Summary
Runtime evidence
Verification
Summary by CodeRabbit
Configuration Updates
Infrastructure Improvements
Bug Fixes
Tests
Ticket: OMN-12489
Evidence-Ticket: OMN-12489
Evidence-Source: OCC#2022