Skip to content

fix: chronological Discord PR-open notification order - #43

Merged
shubham5080 merged 2 commits into
mainfrom
fix/notify-chronological-order
Aug 4, 2026
Merged

fix: chronological Discord PR-open notification order#43
shubham5080 merged 2 commits into
mainfrom
fix/notify-chronological-order

Conversation

@shubham5080

@shubham5080 shubham5080 commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

  • Batch run-once could post newer PR-open channel notices before older ones (GitHub list order is newest-first).
  • Sort a copy of contribution events by created_at only inside the notification pass.
  • Storage, cursor advance, and ingestion order are unchanged.

Test plan

  • pytest tests/test_notifications.py (includes new oldest-first batch test)
  • Confirm CI Python Tests green

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes

    • Corrected notification ordering so contribution events are posted from oldest to newest.
    • Ensured event ordering is consistent when multiple events share the same creation time.
  • Tests

    • Added coverage confirming that pull request notifications are delivered chronologically, regardless of input order.

Batch syncs were posting PR-open channel notices in GitHub API order
(newest first). Sort a copy by created_at only for the notify pass so
storage and cursors stay unchanged.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@shubham5080, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 54 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

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.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 064e78e0-be44-4544-9492-ee03b6f1cca3

📥 Commits

Reviewing files that changed from the base of the PR and between 8716dd2 and 5e51d35.

📒 Files selected for processing (2)
  • src/ghdcbot/engine/orchestrator.py
  • tests/test_notifications.py

Walkthrough

The notification engine now sorts contribution events by creation time before Discord processing. Deterministic tie-breakers define equal-time ordering. The original contribution list remains unchanged. A regression test verifies oldest-first ordering for batch pull request notifications.

Changes

Notification ordering

Layer / File(s) Summary
Deterministic event sorting
src/ghdcbot/engine/orchestrator.py
The orchestrator adds _notification_event_sort_key with creation time and deterministic tie-breakers.
Sorted processing and regression coverage
src/ghdcbot/engine/orchestrator.py, tests/test_notifications.py
Notification processing iterates over a sorted copy. The test verifies that PR #41 is posted before PR #42 when input events arrive newest-first.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Poem

A rabbit sorted events in a row,
From oldest PR to newest flow.
Tie-breakers kept the order bright,
Tests watched each message land right.
Thump, thump—notifications go!

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: chronological ordering of Discord pull request open notifications.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/notify-chronological-order

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.

@github-actions github-actions Bot added size/M and removed size/M labels Aug 4, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/ghdcbot/engine/orchestrator.py`:
- Around line 273-286: The _notification_event_sort_key function needs a
deterministic final tie-breaker for events sharing all current key fields.
Append the canonical event identifier, such as payload review_id, to the
returned tuple so equal-time pr_reviewed events with different reviews sort
independently of ingestion order.

In `@tests/test_notifications.py`:
- Around line 1355-1368: Update the test around
_send_notifications_for_new_events to pass a named input list containing newer
and older events, then assert after the call that the list retains the same
element identities and ingestion order. Keep the existing message-order
assertions unchanged.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7e92ea1b-b54a-412a-b6c3-7e88c7af6c7c

📥 Commits

Reviewing files that changed from the base of the PR and between 3b09fa8 and 8716dd2.

📒 Files selected for processing (2)
  • src/ghdcbot/engine/orchestrator.py
  • tests/test_notifications.py

Comment thread src/ghdcbot/engine/orchestrator.py
Comment thread tests/test_notifications.py
Append review_id to the sort key for equal-time pr_reviewed events, and
assert the notify pass leaves the caller's contributions order untouched.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions Bot added size/M and removed size/M labels Aug 4, 2026
@shubham5080
shubham5080 merged commit 937ab5e into main Aug 4, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant