Skip to content

fix(eda): align event context modeling - #56

Merged
yordis merged 1 commit into
mainfrom
yordis/fix-eda-event-context-modeling
May 26, 2026
Merged

fix(eda): align event context modeling#56
yordis merged 1 commit into
mainfrom
yordis/fix-eda-event-context-modeling

Conversation

@yordis

@yordis yordis commented May 26, 2026

Copy link
Copy Markdown
Member
  • Event guidance needs a clearer boundary between immutable record metadata, event context, and domain payload.
  • Causality and occurrence time guidance needs to preserve auditability without forcing generic context into business payload fields.
  • Schema identity guidance needs one canonical discriminator so event routing and decoding cannot drift across duplicate version fields.

@cursor

cursor Bot commented May 26, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Documentation-only change to an EDA naming skill; no runtime, auth, or data-path code is modified.

Overview
Updates the event-name skill so reviewers treat persisted events as three layers—record metadata, event context, and domain payload—instead of lumping envelope facts into payload naming rules.

Who / when / causality: Generic actor_id, occurred_at, correlation, and causation belong in typed event context at append time; payload _by / _at fields are only for domain facts (e.g. approved_by, effective_at). The checklist and output now ask for payload vs context classification and migration allowances when context is duplicated in payload.

Schema identity: Replaces “no version in the event name, use schema_version” with event_type as the single canonical discriminator for routing/decoding, guidance on when schema_version is still needed, incompatible revisions (OrderPlacedV2), and stream namespace vs event schema revision. Adds stream names as storage addresses (compact paths; don’t embed full type URLs or tenant/env in event_type).

New review rules: Actor context enforcement at the append boundary, ownership vs actor_id, and deterministic handler/replay testing with immutable RecordedEvent fixtures.

Reviewed by Cursor Bugbot for commit 6db3e83. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@yordis, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 43 minutes and 12 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cb1146bc-c64c-4ad6-a2f6-06f99cd69db5

📥 Commits

Reviewing files that changed from the base of the PR and between ad5af4d and 6db3e83.

📒 Files selected for processing (1)
  • plugins/trogonstack-eda/skills/event-name/SKILL.md

Walkthrough

This PR updates the event-name skill documentation to revise event-naming and event-payload-field conventions. Changes clarify three-layer separation (metadata/envelope/payload), strengthen event-type discrimination, rework What/Who/When fielding to rely on context fields for routing and audit, expand record-layer validation and enforcement guidance, and consolidate supporting conventions and review checklist items.

Changes

Event naming and payload field conventions

Layer / File(s) Summary
Core principles and three-layer separation
plugins/trogonstack-eda/skills/event-name/SKILL.md
The document now explicitly distinguishes three layers (record metadata, generic event context/envelope, and domain payload) and tightens rules on what belongs in each layer.
Event type discrimination and naming conventions
plugins/trogonstack-eda/skills/event-name/SKILL.md
Added guidance on using a single canonical event_type discriminator, differentiating stream version/namespace from event schema revision, keeping stream names as storage addresses, and adjusted temporal-field guidance to prefer occurred_at in context while maintaining consistent _at vs _on suffix usage.
What/Who/When fielding principle
plugins/trogonstack-eda/skills/event-name/SKILL.md
Reworked to standardize "what happened" as event_type/typed name, place "who" and "when" as context fields, and restrict _by/_at usage in payload to when actor/time are themselves domain facts.
Event record layers and enforcement
plugins/trogonstack-eda/skills/event-name/SKILL.md
Expanded section with explicit separation of record metadata, event context/envelope, and domain payload; added field validation tests, typed context enforcement, ownership/authorization expectations, deterministic handler inputs, and currency requirements.
Field conventions and payload shapes
plugins/trogonstack-eda/skills/event-name/SKILL.md
Reinforced plural naming for collection fields, introduced "no polymorphic payloads" rule requiring type-driven changes to be split into separate events, and updated numbering for PII and casing conventions.
Review checklist consolidation
plugins/trogonstack-eda/skills/event-name/SKILL.md
Renumbered and extended checklist items to reinforce discriminator/schema-identity rules, typed-context capture of Who/When, record/context/payload separation, non-duplication of metadata, compact stream naming, authorization validation, replay testing, field casing, and payload vs context classification for disputed fields.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • TrogonStack/agentskills#44: Introduced the event-name skill documentation that this PR refines and updates with revised conventions.

Poem

🐰 A skill doc grows with wisdom earned,
Three layers dance where once they churned,
Event-type speaks, context listens bright,
What/Who/When split—now that's just right! ✨
The checklist hops through every care.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly addresses the main change: clarifying event context modeling in EDA documentation, which aligns with the substantial revisions to event-naming and event-payload-field conventions in the skill markdown.
Description check ✅ Passed The description is directly related to the changeset, clearly articulating three key improvements: boundary clarification between record metadata/context/payload, auditability preservation, and schema identity canonicalization.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch yordis/fix-eda-event-context-modeling

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yordis
yordis force-pushed the yordis/fix-eda-event-context-modeling branch 3 times, most recently from e8385e9 to ad5af4d Compare May 26, 2026 05:00

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
plugins/trogonstack-eda/skills/event-name/SKILL.md (1)

296-297: ⚡ Quick win

Clarify the temporal suffix exception to avoid checklist contradictions.

Line 296 allows scheduled_for, but Line 478 enforces only _at/_on. Please explicitly document the exception (or remove scheduled_for) so reviewers apply one rule consistently.

Also applies to: 478-478

🤖 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 `@plugins/trogonstack-eda/skills/event-name/SKILL.md` around lines 296 - 297,
The docs conflict: Line referencing allowed names (mentions scheduled_for)
contradicts the rule enforcing only `_at`/`_on`; update SKILL.md so the
temporal-suffix rule is unambiguous by either (A) explicitly adding
`scheduled_for` as an accepted exception alongside the payload-business-time
examples (`effective_at`, `expires_at`, `starts_at`) and note it is allowed only
in payload, or (B) removing `scheduled_for` from the earlier example and
replacing it with a `_at` form (e.g., `scheduled_at`) so both the line that
lists examples and the rule that enforces only `_at`/`_on` are consistent; make
the change wherever `scheduled_for`, `_at`, or `_on` are referenced so reviewers
apply a single rule consistently.
🤖 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 `@plugins/trogonstack-eda/skills/event-name/SKILL.md`:
- Around line 296-297: The docs conflict: Line referencing allowed names
(mentions scheduled_for) contradicts the rule enforcing only `_at`/`_on`; update
SKILL.md so the temporal-suffix rule is unambiguous by either (A) explicitly
adding `scheduled_for` as an accepted exception alongside the
payload-business-time examples (`effective_at`, `expires_at`, `starts_at`) and
note it is allowed only in payload, or (B) removing `scheduled_for` from the
earlier example and replacing it with a `_at` form (e.g., `scheduled_at`) so
both the line that lists examples and the rule that enforces only `_at`/`_on`
are consistent; make the change wherever `scheduled_for`, `_at`, or `_on` are
referenced so reviewers apply a single rule consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f88fe09c-6e21-4904-b8b2-0421e5aaccdd

📥 Commits

Reviewing files that changed from the base of the PR and between 060dc57 and ad5af4d.

📒 Files selected for processing (1)
  • plugins/trogonstack-eda/skills/event-name/SKILL.md

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis force-pushed the yordis/fix-eda-event-context-modeling branch from ad5af4d to 6db3e83 Compare May 26, 2026 05:17
@yordis
yordis merged commit f6e8d20 into main May 26, 2026
4 checks passed
@yordis
yordis deleted the yordis/fix-eda-event-context-modeling branch May 26, 2026 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant