fix: complete the predicate event taxonomy across SDKs - #574
Conversation
Java gates only at enqueue and only pass/reject, but a webhook row written by another SDK filters on the wire name, and an unknown token silently never matches. Declaring all four keeps such a subscription portable.
Reserved here: it means a dispatch-time predicate cancelled an already enqueued job, which only Python produces. Declaring it lets a Python-authored webhook subscription load and validate against this SDK.
Reserved here: a Cancel at enqueue raises PredicateRejectedError and emits PREDICATE_REJECTED, so nothing drops an enqueue silently. Declaring it lets a Node-authored webhook subscription round-trip.
Python counts by outcome sentinel, not decision kind, so the mapping was wrong: its denied and cancelled both land on rejected here, and it has no enqueue-time skip at all.
The page predated predicate.skipped/deferred landing: it claimed 26 wire names, listed only predicate.rejected, and showed PredicateEvent without reason or delayMs.
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR aligns predicate event names and compatibility handling across Java, Node, and Python SDKs. It updates event mappings, tests, predicate payload documentation, webhook filtering descriptions, and cross-SDK lifecycle documentation. ChangesPredicate event contract
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://cdn.github.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 `@sdks/java/src/main/java/org/byteveda/taskito/events/EventName.java`:
- Around line 74-78: Update the shared predicate-event descriptions to preserve
dispatch-time semantics: in
sdks/java/src/main/java/org/byteveda/taskito/events/EventName.java lines 74-78,
make PREDICATE_DEFERRED phase-neutral or explicitly include dispatch-time
deferral; in docs/content/docs/java/guides/extensibility/events.mdx lines
140-146, replace “a deferred enqueue” with wording covering both enqueue- and
dispatch-time deferral; and in
docs/content/docs/node/guides/extensibility/webhooks.mdx line 76, limit the
missing-jobId statement to enqueue-time predicate events because dispatch-time
events may include jobId.
🪄 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 Plus
Run ID: d60fe035-3ab0-45ab-9e1a-59c77412e113
📒 Files selected for processing (12)
docs/content/docs/java/guides/extensibility/events.mdxdocs/content/docs/java/guides/extensibility/webhooks.mdxdocs/content/docs/node/guides/extensibility/events.mdxdocs/content/docs/node/guides/extensibility/webhooks.mdxdocs/content/docs/python/guides/extensibility/events.mdxsdks/java/src/main/java/org/byteveda/taskito/events/EventName.javasdks/java/src/test/java/org/byteveda/taskito/events/EventNameTest.javasdks/node/src/events.tssdks/node/src/predicates/metrics.tssdks/node/test/core/webhooks.test.tssdks/python/taskito/events.pysdks/python/tests/observability/test_events.py
Python emits it at dispatch as well as enqueue, carrying job_id, so a contract-level description scoped to enqueue was wrong. The Node delivery-log note now says why its predicate events have no jobId rather than claiming none ever do.
2 participants
Closes #570. Closes #571. Unblocks #535.
The decision
#570 asked to settle on one wire name for
predicate.skippedvspredicate.cancelled. The premise doesn't hold — they denote different phases, not the same outcome:predicate.rejectedpredicate.rejectedpredicate.rejectedpredicate.skippedpredicate.deferredpredicate.deferredpredicate.cancelledA
Cancelat enqueue in Python emitspredicate.rejectedand raisesPredicateRejectedError(sdks/python/taskito/mixins/predicates.py:276);predicate.cancelledfires only at dispatch and carriesjob_id. Renaming either would have merged two distinct events and broken existing Node subscribers.The actual defect
Each SDK's subscription API is typed on its own enum, so a wire name that SDK doesn't declare is unsubscribable there —
Webhook.Builder.on(EventName...)in Java,WebhookInput.events: EventName[]in Node,add_webhook(events: list[EventType])in Python. Java'ssubscribedToswallows unknown stored tokens (sdks/java/.../webhooks/WebhookManager.java:288), so a subscription written by Node silently never matches.Fixed by completing the contract to 29 names in all three SDKs, each marked emitted-here or reserved (the pattern Java already used for
worker.unhealthy):PREDICATE_DEFERRED,PREDICATE_SKIPPED,PREDICATE_CANCELLED(all reserved — gates are pass/reject only)predicate.cancelled(reserved — no dispatch-time gates)PREDICATE_SKIPPED(reserved — enqueueCancelrejects rather than skips)Also corrects
sdks/node/src/predicates/metrics.ts, which claimed Python uses "cancelledforskipped". Python counts by outcome sentinel, not decision kind, so itsdeniedandcancelledboth land on Node'srejected, and it has no enqueue-time skip.Docs (#571)
The Node events page predated
predicate.skipped/deferredlanding in #543: it claimed 26 wire names, listed onlypredicate.rejected, and showedPredicateEventwithoutreason/delayMs. Java and Python pages document their reserved entries.Verification
webhooks,predicates,queue-events,webhookStoreContract)ruffandmypycleanspotlessCheckandcheckstyleMaingreenJava tests were not executed:
:compileTestJavais disabled in the local environment (Skipping task ':compileTestJava' as task onlyIf 'Task is enabled' is false), reproducible on a cleanmasterworktree, so it predates this branch.EventNameTestis updated to assert 29 and needs CI to confirm.Summary by CodeRabbit
New Features
predicate.cancelled.Documentation