Skip to content

OUT-3915 | Delete entries on GroupedEmailEvents table after emails are successfully sent - #1351

Merged
arpandhakal merged 4 commits into
feature/grouped-emailsfrom
OUT-3915-delete-grouped-email-events-after-flush
Jun 26, 2026
Merged

arpandhakal merged 4 commits into
feature/grouped-emailsfrom
OUT-3915-delete-grouped-email-events-after-flush

Conversation

@arpandhakal

Copy link
Copy Markdown
Collaborator

Summary

  • Adds a DELETE FROM GroupedEmailEvents WHERE windowKey = $windowKey at the end of flushGroupedEmailRun, executed only on full success
  • Wrapped in try/catch so a transient delete failure logs the error without triggering an unnecessary Trigger.dev retry (rows are already marked sentAt so they'll never be re-sent)
  • markRecipientSent is preserved — still needed for partial-failure retry safety mid-loop

Why

GroupedEmailEvents rows serve as a 5-minute buffer and are not needed after the window is flushed. Previously they accumulated indefinitely with sentAt set, bulking up the table with data that was never read again.

Behaviour

  • Success: all rows for the window are deleted after the loop completes
  • Failure (send throws): exception propagates before deleteWindowRows is reached — rows stay intact for Trigger.dev retry
  • Delete fails: caught, logged, run still returns successfully — rows remain with sentAt set (harmless, ignored by future queries)

Test plan

  • Updated integration tests to assert totalCount === 0 (full row deletion) after successful runs rather than checking unsent count
  • Retry test explicitly asserts rows survive a failed run (totalCount === 2) and are deleted after the successful retry (totalCount === 0)
  • Updated stale test descriptions that referenced "marks rows as sent" to reflect deletion

🤖 Generated with Claude Code

…flush (OUT-3915)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jun 26, 2026

Copy link
Copy Markdown

OUT-3915

@vercel

vercel Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tasks-app Ready Ready Preview, Comment Jun 26, 2026 11:44am

Request Review

@vercel

vercel Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Deployment failed with the following error:

Deploying Serverless Functions to multiple regions is restricted to the Pro and Enterprise plans.

Learn More: https://vercel.link/multiple-function-regions

@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a deleteWindowRows cleanup step to flushGroupedEmailRun that removes all sentAt IS NOT NULL rows for a window after the send loop completes successfully. The delete is wrapped in a try/catch so a transient DB failure logs the error without triggering a Trigger.dev retry — rows remaining with sentAt set are harmless since future reads filter on sentAt IS NULL.

  • deleteWindowRows is correctly scoped to sentAt IS NOT NULL, which preserves any late-arriving rows that were inserted after readUnsentWindowEvents ran but before the delete fires.
  • Integration tests are updated throughout to assert totalCount === 0 (full deletion) rather than checking the unsent count, and the retry test correctly verifies rows survive a failed run and are fully cleaned up after a successful retry.

Confidence Score: 5/5

Safe to merge — the cleanup logic is correctly scoped, failure modes are handled gracefully, and the updated tests exercise the full delete lifecycle.

The delete is gated on sentAt IS NOT NULL, so late-arriving rows for an already-flushing window are not touched. The try/catch around deleteWindowRows ensures a transient DB failure on cleanup never causes an unnecessary retry. The markRecipientSent call in the loop still fires before the delete, so the partial-failure retry path remains intact. Integration tests confirm deletion after success and row preservation after failure.

No files require special attention.

Important Files Changed

Filename Overview
src/jobs/notifications/flush-grouped-email.ts Adds deleteWindowRows (scoped to sentAt IS NOT NULL) called after the send loop, wrapped in a try/catch that logs but does not re-throw — correctly preserving late-arriving rows and not blocking successful completion on cleanup failure.
src/jobs/notifications/flush-grouped-email.integration.test.ts Helper renamed from unsentCount to totalCount (query drops the sentAt IS NULL filter) and all test assertions updated to verify full row deletion; retry test correctly exercises the preserve-on-failure / delete-on-success lifecycle.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant T as Trigger.dev
    participant F as flushGroupedEmailRun
    participant DB as GroupedEmailEvents
    participant C as CopilotAPI

    T->>F: trigger(payload)
    F->>DB: readUnsentWindowEvents (sentAt IS NULL)
    DB-->>F: rows[]

    alt "rows.length === 0"
        F-->>T: "{ skipped: true }"
    else rows exist
        loop for each recipient group
            F->>C: sendGroupedEmail / sendIndividualEmail
            C-->>F: success
            F->>DB: "markRecipientSent (SET sentAt = now())"
        end

        F->>DB: deleteWindowRows (WHERE sentAt IS NOT NULL)
        alt delete succeeds
            DB-->>F: rows removed
        else delete fails
            F->>F: logger.error (swallow, no retry)
            Note over DB: rows remain with sentAt set (harmless)
        end

        F-->>T: "{ recipients, sent, sentGrouped, sentIndividual }"
    end

    Note over T,DB: On send failure: exception propagates before markRecipientSent and deleteWindowRows. Rows stay with sentAt=NULL for retry.
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant T as Trigger.dev
    participant F as flushGroupedEmailRun
    participant DB as GroupedEmailEvents
    participant C as CopilotAPI

    T->>F: trigger(payload)
    F->>DB: readUnsentWindowEvents (sentAt IS NULL)
    DB-->>F: rows[]

    alt "rows.length === 0"
        F-->>T: "{ skipped: true }"
    else rows exist
        loop for each recipient group
            F->>C: sendGroupedEmail / sendIndividualEmail
            C-->>F: success
            F->>DB: "markRecipientSent (SET sentAt = now())"
        end

        F->>DB: deleteWindowRows (WHERE sentAt IS NOT NULL)
        alt delete succeeds
            DB-->>F: rows removed
        else delete fails
            F->>F: logger.error (swallow, no retry)
            Note over DB: rows remain with sentAt set (harmless)
        end

        F-->>T: "{ recipients, sent, sentGrouped, sentIndividual }"
    end

    Note over T,DB: On send failure: exception propagates before markRecipientSent and deleteWindowRows. Rows stay with sentAt=NULL for retry.
Loading

Reviews (2): Last reviewed commit: "fix(notifications): scope window DELETE ..." | Re-trigger Greptile

Comment on lines +43 to +44
const deleteWindowRows = (db: ReturnType<typeof DBClient.getInstance>, windowKey: string) =>
db.$executeRaw`DELETE FROM "GroupedEmailEvents" WHERE "windowKey" = ${windowKey}`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Broad DELETE may silently drop unread rows

deleteWindowRows deletes every row matching windowKey, regardless of sentAt. The flush reads rows once at startup (readUnsentWindowEvents), then spends time in the send loop. Any GroupedEmailEvents row inserted for this same windowKey after that initial read — but before the DELETE — is silently destroyed without being sent.

This is a latent risk if the upstream event-creation path can assign new events to an already-flushing window (e.g., a delayed webhook or a retry that reassigns the same windowKey within the send-loop window). Adding a sentAt IS NOT NULL guard to deleteWindowRows would make the cleanup strictly scoped to rows that were successfully processed and avoid touching anything that arrived late.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. Fixed by scoping the DELETE to sentAt IS NOT NULL — only rows that were successfully processed by the loop get cleaned up. Any row that arrives after readUnsentWindowEvents but before the delete will have sentAt IS NULL and be preserved.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@arpandhakal

Copy link
Copy Markdown
Collaborator Author

@greptile can you re review.

@priosshrsth priosshrsth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

… adding deleteWindowRows

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@arpandhakal
arpandhakal merged commit 20bf74e into feature/grouped-emails Jun 26, 2026
3 checks passed
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.

2 participants