Skip to content

fix: suppress success digest bot notifications#2054

Merged
yottahmd merged 1 commit intomainfrom
fix/suppress-success-digest-notifications
Apr 29, 2026
Merged

fix: suppress success digest bot notifications#2054
yottahmd merged 1 commit intomainfrom
fix/suppress-success-digest-notifications

Conversation

@yottahmd
Copy link
Copy Markdown
Collaborator

@yottahmd yottahmd commented Apr 29, 2026 •
edited by coderabbitai Bot
Loading

Summary

  • suppress success-class bot notification batches before they reach Slack, Telegram, or Discord transports
  • keep success events in the shared notification state machine so they still clear stale pending wait alerts
  • extend monitor and bot tests to verify successful completions are acknowledged silently while urgent notifications still deliver normally

Why

  • successful DAG completion digests were noisy in chat and did not require operator action
  • the shared notification monitor was still useful for deduplication and state cleanup, so the change should remove the outward message without breaking delivery semantics for urgent statuses
  • the regression coverage needs to pin down that success events no longer open threads, append chat messages, or send direct notifications

Validation

  • go test ./internal/service/chatbridge ./internal/service/slack ./internal/service/telegram ./internal/service/discord ./internal/cmn/config

Summary by CodeRabbit

  • Bug Fixes
    • Successful DAG run completions are now properly suppressed and no longer generate chat notifications or digests in Slack and Telegram.
    • Successful runs are still tracked as delivered and seen by the system, preventing duplicate alert notifications.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c3b5b1bb-f1a8-4035-a1c7-74ceb7070b53

📥 Commits

Reviewing files that changed from the base of the PR and between 405cc68 and 685ca8f.

📒 Files selected for processing (7)
  • internal/cmn/schema/config.schema.json
  • internal/service/chatbridge/monitor.go
  • internal/service/chatbridge/monitor_state_test.go
  • internal/service/chatbridge/monitor_test.go
  • internal/service/slack/bot_test.go
  • internal/service/telegram/bot_test.go
  • internal/service/telegram/monitor_test.go
✨ 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/suppress-success-digest-notifications

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
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c3b5b1bb-f1a8-4035-a1c7-74ceb7070b53

📥 Commits

Reviewing files that changed from the base of the PR and between 405cc68 and 685ca8f.

📒 Files selected for processing (7)
  • internal/cmn/schema/config.schema.json
  • internal/service/chatbridge/monitor.go
  • internal/service/chatbridge/monitor_state_test.go
  • internal/service/chatbridge/monitor_test.go
  • internal/service/slack/bot_test.go
  • internal/service/telegram/bot_test.go
  • internal/service/telegram/monitor_test.go
📝 Walkthrough

Walkthrough

The changes update schema documentation, add special handling in the monitor to suppress notifications for successful DAG run completions, and update tests across multiple bot services to validate success-suppression behavior instead of digest delivery mechanics.

Changes

Cohort / File(s) Summary
Schema Documentation
internal/cmn/schema/config.schema.json		 Updated descriptions for TelegramBotDef.interested_event_types and SlackBotDef.interested_event_types to clarify that dag.run.succeeded suppresses stale pending alerts without sending chat messages.
Monitor Control Flow
internal/service/chatbridge/monitor.go		 Added early-return logic in flushPendingBatch to acknowledge and skip external flush for NotificationClassSuccessDigest batches, preventing chat-digest posting.
Monitor Tests
internal/service/chatbridge/monitor_state_test.go, internal/service/chatbridge/monitor_test.go		 Updated DAG run event emissions to use failure semantics (core.Failed, eventstore.TypeDAGRunFailed) across existing tests; added new test validating successful completions are delivered without transport flush.
Bot Service Tests
internal/service/slack/bot_test.go, internal/service/telegram/bot_test.go, internal/service/telegram/monitor_test.go		 Rewrote success-digest tests to validate suppression and marking behavior; removed checks for digest message creation and thread posts; updated test names and DAG run status semantics from success to failure in one test case.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 summarizes the main change: suppressing success digest bot notifications. It directly reflects the primary intent of the pull request.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/suppress-success-digest-notifications

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
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

@yottahmd yottahmd merged commit 4e06220 into main Apr 29, 2026
10 checks passed
@yottahmd yottahmd deleted the fix/suppress-success-digest-notifications branch April 29, 2026 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@yottahmd