chore: cover topic and named queue patterns in skills#7
Draft
anthonyiscoding wants to merge 1 commit intomainfrom
Draft
chore: cover topic and named queue patterns in skills#7anthonyiscoding wants to merge 1 commit intomainfrom
anthonyiscoding wants to merge 1 commit intomainfrom
Conversation
Document both queue modes side-by-side in core skills and align reference implementations so topic-based enqueue flows and named queue dispatch examples are both clear and consistent.
📝 WalkthroughWalkthroughThe pull request transitions the event dissemination architecture from pubsub-style fan-out to queue/topic-based delivery. Documentation and code examples are updated to reflect the shift: commands emit events via Changes
Sequence Diagram(s)sequenceDiagram
participant Cmd as Command Handler
participant Queue as Queue/Topic
participant Proj as Projection<br/>(Consumer)
participant ReadDB as Read DB
rect rgba(100, 200, 100, 0.5)
Note over Cmd,ReadDB: New: Queue/Topic-Based
Cmd->>Queue: enqueue(topic='orders.created',<br/>data)
activate Queue
Queue->>Proj: trigger queue handler<br/>(type: 'queue',<br/>topic: 'orders.created')
deactivate Queue
activate Proj
Proj->>ReadDB: Update projection
Proj-->>Proj: messageReceiptId returned
deactivate Proj
end
sequenceDiagram
participant Alert as Alert Logic
participant Queue as Named Queue
participant Notif as Notification<br/>Handler
participant Slack as Slack API
rect rgba(100, 150, 200, 0.5)
Note over Alert,Slack: Alert Fan-Out: Enqueue to Named Queue
Alert->>Queue: TriggerAction.Enqueue<br/>({queue: 'alerts-notify'})
activate Queue
Queue->>Notif: Trigger handler<br/>(type: 'queue',<br/>queue: 'alerts-notify')
deactivate Queue
activate Notif
Notif->>Slack: Send low-stock notification
deactivate Notif
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
references/functions-and-triggers.js (1)
89-100:⚠️ Potential issue | 🟠 MajorDon't emit
orders.completedbefore fulfillment starts.
orders::createonly validates the request and queuesorders::fulfill, so consumers on this topic will observe a completion that has not happened yet. Rename this toorders.created/orders.accepted, or move the emit into the actual fulfillment completion path.💡 Possible fix
Consumer side:
-iii.registerFunction({ id: 'notifications::on-order-complete' }, async (data) => { +iii.registerFunction({ id: 'notifications::on-order-created' }, async (data) => { const logger = new Logger() - logger.info('Order completed event received', { orderId: data.order_id }) + logger.info('Order created event received', { orderId: data.order_id }) return { processed: true } }) iii.registerTrigger({ type: 'queue', - function_id: 'notifications::on-order-complete', - config: { topic: 'orders.completed' }, + function_id: 'notifications::on-order-created', + config: { topic: 'orders.created' }, })Producer side:
await iii.trigger({ function_id: 'enqueue', payload: { - topic: 'orders.completed', + topic: 'orders.created', data: { order_id: data.order_id }, }, })Also applies to: 119-126
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@references/functions-and-triggers.js` around lines 89 - 100, The trigger is listening to the topic 'orders.completed' but the producer only validates and queues fulfillment (orders::create → orders::fulfill), so consumers will see a completion event before fulfillment actually finishes; update the trigger configuration (the iii.registerTrigger call that references function_id 'notifications::on-order-complete') to subscribe to a more accurate topic such as 'orders.created' or 'orders.accepted', or instead move emission of 'orders.completed' from the create path into the actual fulfillment completion path (where orders::fulfill finishes) so the topic name matches the real lifecycle event.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@references/event-driven-cqrs.js`:
- Around line 154-157: notify::low-stock-alert races with proj::catalog-on-sell
because both independently consume inventory.item-sold and the alert handler can
read inventory-read before the projection decrement has been written; fix by
ensuring the alert receives the post-sale stock value or by emitting/triggering
the low-stock notification from the projection update path after
proj::catalog-on-sell finishes writing inventory-read. Specifically, either
include the updated stock count on the inventory.item-sold event (so
notify::low-stock-alert can make a correct decision) or invoke
notify::low-stock-alert (or publish a dedicated low-stock topic) from inside the
proj::catalog-on-sell projection write routine once the decrement is persisted.
- Around line 51-55: The appendEvent() followed by multiple iii.trigger({
function_id: 'enqueue', ... }) calls is non-atomic: appendEvent() can succeed
while one or more enqueue fan-out calls fail, leaving event-log ahead of
projections. Fix by adopting an outbox/replay-marker pattern: make appendEvent()
persist the event plus an outbox entry or advance a replay marker in the same
atomic operation/transaction, then have a separate reliable dispatcher read the
outbox/replay marker and call iii.trigger('enqueue', ...) (or enqueue) only
after the write is committed; alternatively, enqueue the fan-out work into the
same durable store (outbox table) and have a background process perform
iii.trigger() from that outbox with retry/at-least-once semantics so projections
and event-log stay consistent. Ensure code changes touch appendEvent(), the
outbox/replay marker persistence, and the dispatcher that invokes
iii.trigger/enqueue.
---
Outside diff comments:
In `@references/functions-and-triggers.js`:
- Around line 89-100: The trigger is listening to the topic 'orders.completed'
but the producer only validates and queues fulfillment (orders::create →
orders::fulfill), so consumers will see a completion event before fulfillment
actually finishes; update the trigger configuration (the iii.registerTrigger
call that references function_id 'notifications::on-order-complete') to
subscribe to a more accurate topic such as 'orders.created' or
'orders.accepted', or instead move emission of 'orders.completed' from the
create path into the actual fulfillment completion path (where orders::fulfill
finishes) so the topic name matches the real lifecycle event.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7204104-ef4f-46a6-af09-305ea6607fbe
📒 Files selected for processing (6)
event-driven-cqrs/SKILL.mdfunctions-and-triggers/SKILL.mdqueue-processing/SKILL.mdreferences/event-driven-cqrs.jsreferences/functions-and-triggers.jsreferences/queue-processing.js
Comment on lines
+51
to
+55
| // Emit domain event for all topic queue consumers to process | ||
| await iii.trigger({ | ||
| function_id: 'enqueue', | ||
| payload: { topic: 'inventory.item-added', data: event }, | ||
| }) |
There was a problem hiding this comment.
Event-log append and fan-out are not atomic.
appendEvent() completes before each enqueue call, so a failure here leaves event-log ahead of the projections. Because the query endpoints read projections rather than event-log, the system stays inconsistent until you replay manually. Consider persisting an outbox/replay marker with the append.
Also applies to: 82-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@references/event-driven-cqrs.js` around lines 51 - 55, The appendEvent()
followed by multiple iii.trigger({ function_id: 'enqueue', ... }) calls is
non-atomic: appendEvent() can succeed while one or more enqueue fan-out calls
fail, leaving event-log ahead of projections. Fix by adopting an
outbox/replay-marker pattern: make appendEvent() persist the event plus an
outbox entry or advance a replay marker in the same atomic
operation/transaction, then have a separate reliable dispatcher read the
outbox/replay marker and call iii.trigger('enqueue', ...) (or enqueue) only
after the write is committed; alternatively, enqueue the fan-out work into the
same durable store (outbox table) and have a background process perform
iii.trigger() from that outbox with retry/at-least-once semantics so projections
and event-log stay consistent. Ensure code changes touch appendEvent(), the
outbox/replay marker persistence, and the dispatcher that invokes
iii.trigger/enqueue.
Comment on lines
+154
to
+157
| // Projections consume domain events independently via queue topic triggers | ||
| iii.registerTrigger({ type: 'queue', function_id: 'proj::catalog-on-add', config: { topic: 'inventory.item-added' } }) | ||
| iii.registerTrigger({ type: 'queue', function_id: 'proj::catalog-on-sell', config: { topic: 'inventory.item-sold' } }) | ||
| iii.registerTrigger({ type: 'queue', function_id: 'proj::sales-analytics', config: { topic: 'inventory.item-sold' } }) |
There was a problem hiding this comment.
Low-stock alerts race the sell projection.
notify::low-stock-alert and proj::catalog-on-sell consume inventory.item-sold independently, so this handler can read inventory-read before the decrement lands and miss threshold crossings. Put the post-sale stock on the ItemSold event, or trigger the alert from the code path that writes the updated projection.
💡 Possible direction
const event = {
type: 'ItemSold',
sku,
+ name: item.name,
quantity,
revenue: quantity * item.price,
+ remaining_stock: item.stock - quantity,
timestamp: new Date().toISOString(),
}
iii.registerFunction({ id: 'notify::low-stock-alert' }, async (event) => {
- const item = await iii.trigger({ function_id: 'state::get', payload: { scope: 'inventory-read', key: event.sku } })
- if (item && item.stock <= 5) {
+ if (event.remaining_stock <= 5) {
await iii.trigger({
function_id: 'notify::slack-low-stock',
- payload: { sku: event.sku, name: item.name, remaining: item.stock },
+ payload: { sku: event.sku, name: event.name, remaining: event.remaining_stock },
action: TriggerAction.Enqueue({ queue: 'alerts-notify' }),
})
}
})Also applies to: 162-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@references/event-driven-cqrs.js` around lines 154 - 157,
notify::low-stock-alert races with proj::catalog-on-sell because both
independently consume inventory.item-sold and the alert handler can read
inventory-read before the projection decrement has been written; fix by ensuring
the alert receives the post-sale stock value or by emitting/triggering the
low-stock notification from the projection update path after
proj::catalog-on-sell finishes writing inventory-read. Specifically, either
include the updated stock count on the inventory.item-sold event (so
notify::low-stock-alert can make a correct decision) or invoke
notify::low-stock-alert (or publish a dedicated low-stock topic) from inside the
proj::catalog-on-sell projection write routine once the decrement is persisted.
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.
Document both queue modes side-by-side in core skills and align reference implementations so topic-based enqueue flows and named queue dispatch examples are both clear and consistent.
Summary by CodeRabbit