feat(router): support causation_id: nil to mark events as chain roots#106
Conversation
Callers can supply a `:causation_id` and have it persisted on resulting events — but there was still no way to express "this event has no cause." `Keyword.get(opts, :causation_id)` returns `nil` whether the option is absent or explicitly `nil`, so the dispatcher fell back to `command_uuid` in both cases. Resolve `:causation_id` at the router boundary using `Keyword.fetch/2` so absence and explicit-`nil` can be distinguished: - opt absent -> fall back to `command_uuid` (legacy) - `causation_id: <uuid>` -> use the uuid (since straw-hat-team#103) - `causation_id: nil` -> use `nil` (new) Side benefit: the dispatcher's `to_execution_context/2` no longer carries the fallback `||` and reads `pipeline.causation_id` directly. Middleware that inspects `pipeline.causation_id` continues to see a uuid-or-nil, never a sentinel. Backwards compatible: - callers that never pass `:causation_id` get identical behaviour - callers that pass a uuid get the same behaviour as straw-hat-team#103's fix - the only new behaviour is opt-in via `causation_id: nil` `WHERE causation_id IS NULL` now identifies chain roots without a self-join against the events table — significantly cheaper on large event stores. - lib/commanded/commands/router.ex: resolve `:causation_id` with `Keyword.fetch/2`; document the new option behaviour on `dispatch/2` - lib/commanded/commands/dispatcher.ex: drop the `|| command_uuid` fallback; stop destructuring `command_uuid` (now unused here) - test/commands/correlation_causation_test.exs: assert `causation_id: nil` produces `nil` on the resulting event
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughRouter dispatch now resolves causation_id (absent → command_uuid, provided → used, explicit nil → preserved) and passes it into the dispatcher. The dispatcher stops falling back to command_uuid and uses payload.causation_id directly. A test verifies events persisted with explicit nil causation_id. ChangesCausation ID resolution at router boundary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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)
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 |
|
@apre I just realized, HOW DID I MISS THIS PR! I am so sorry! Your changes are already in, thank you so much regardless, my apologies for missing this one. Side note, are you using the fork? 👀 |
Hi @yordis ! I still wonder which version I should use (between this fork and the "original". It’s a hard choice and I still struggle on it |
|
https://github.com/straw-hat-team/commanded/blob/main/guides/explanations/fork-differences.md I try to document most of the changes there, definitely they are improvements and major breaking changes since I removed some components; clean up tests, make some stuff faster, and added features I tried to contribute upstream until, I got nowhere. |
|
Can you address CI failure 🙏🏻 ? |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/commanded/commands/router.ex (1)
207-222: 💤 Low valueConsider documenting the
causation_id: nilcase in this section.This new documentation section covers the
causation_id: <uuid>and omitted cases, but doesn't mention that passingcausation_id: nilexplicitly marks events as chain roots. While thedispatch/2options documentation (lines 467-473) does cover this, users reading only this section might miss the chain-root feature that is the primary addition in this PR.📝 Suggested addition
When `:causation_id` is not provided, the command's own `command_uuid` is used as the resulting events' `causation_id`. + +Pass `causation_id: nil` explicitly to mark the resulting events as chain +roots — they will be persisted with `causation_id` set to `NULL`. Process managers and event handlers that dispatch follow-up commands🤖 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 `@lib/commanded/commands/router.ex` around lines 207 - 222, Update the "Causation id" section to explicitly document the special-case when callers pass causation_id: nil: state that setting causation_id: nil on BankApp.dispatch/2 (or the referenced dispatch/2 options) explicitly marks the resulting events as chain roots (i.e., no causation_id will be propagated), include a short example showing :ok = BankApp.dispatch(command, causation_id: nil), and add a brief cross-reference to the existing dispatch/2 options paragraph so readers find the more detailed behavior described there.
🤖 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.
Nitpick comments:
In `@lib/commanded/commands/router.ex`:
- Around line 207-222: Update the "Causation id" section to explicitly document
the special-case when callers pass causation_id: nil: state that setting
causation_id: nil on BankApp.dispatch/2 (or the referenced dispatch/2 options)
explicitly marks the resulting events as chain roots (i.e., no causation_id will
be propagated), include a short example showing :ok = BankApp.dispatch(command,
causation_id: nil), and add a brief cross-reference to the existing dispatch/2
options paragraph so readers find the more detailed behavior described there.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6665eed8-778c-4716-be85-35b6906f8e43
📒 Files selected for processing (1)
lib/commanded/commands/router.ex
The added causation_id resolution pushed `__before_compile__`'s quote block past credo's `LongQuoteBlocks` threshold. Replace the multi-line explanation with a single-line comment — the case clauses are self-explanatory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Adrien Pré <apre@dxo.com>
|
DCO 😭 😭 😭 😭 😭 😭 😭 😭 😭 painful |
Callers can supply a
:causation_idand have it persisted on resulting events — but there was still no way to express "this event has no cause."Keyword.get(opts, :causation_id)returnsnilwhether the option is absent or explicitlynil, so the dispatcher fell back tocommand_uuidin both cases.Resolve
:causation_idat the router boundary usingKeyword.fetch/2so absence and explicit-nilcan be distinguished:command_uuid(legacy)causation_id: <uuid>-> use the uuid (since fix(dispatcher): honor caller-supplied :causation_id on persisted events #103)causation_id: nil-> usenil(new)Side benefit: the dispatcher's
to_execution_context/2no longer carries the fallback||and readspipeline.causation_iddirectly. Middleware that inspectspipeline.causation_idcontinues to see a uuid-or-nil, never a sentinel.Backwards compatible:
:causation_idget identical behaviourcausation_id: nilWHERE causation_id IS NULLnow identifies chain roots without a self-join against the events table — significantly cheaper on large event stores.:causation_idwithKeyword.fetch/2; document the new option behaviour ondispatch/2|| command_uuidfallback; stop destructuringcommand_uuid(now unused here)causation_id: nilproducesnilon the resulting event