Skip to content

OUT-3825 | Grouped content composer (pure function)#1303

Merged
arpandhakal merged 5 commits into
OUT-3821from
OUT-3825
Jun 10, 2026
Merged

OUT-3825 | Grouped content composer (pure function)#1303
arpandhakal merged 5 commits into
OUT-3821from
OUT-3825

Conversation

@arpandhakal

Copy link
Copy Markdown
Collaborator

What

Adds composeGroupedEmail(events) — the render-agnostic step that turns one recipient's buffered window of GroupedEmailEvents into the grouped-email structure. Pure function: no DB, no Copilot, no rendering, no brand prefix. The render adapter (OUT-3826) consumes its output.

Linear: OUT-3825

Stacked on #1301 (OUT-3821 schema) — base is OUT-3821. Retarget to feature/grouped-emails once #1301 merges.

Shape

composeGroupedEmail(events) -> {
  totalEventCount: number
  sections: { eventType, count, taskNames: string[], overflowCount: number }[]
}
  • Sections in fixed order ASSIGNED → SHARED → COMMENT → COMPLETED; empty sections omitted. (COMPLETED stays in the order but is dormant for CU in M2 — it'll light up for IU in M3 with no composer change.)
  • Per section: events sorted by createdAt then title; up to 3 distinct task names listed; overflowCount = max(0, distinctTasks - 3) drives the renderer's "+N other tasks".
  • totalEventCount = every buffered event (the N in "You have N new task updates"); section counts sum to it.

Design note: recipient vs. event grouping

The composer operates on one recipient's window — recipient-level grouping happens upstream via the windowKey claim (OUT-3823/3824). It deliberately never reads recipient IDs; sections-by-type is only the layout inside that single per-recipient email.

Design note: multiple comments on one task

Each comment is a separate buffered event, so it counts individually toward totalEventCount and the COMMENT section count (e.g. "3 comments were added"), but the task name is listed once in the bullets. That's why section count (events) can exceed the number of task names shown — intentional, and covered by a test.

Testing

  • 9 unit tests, 100% coverage — section ordering, empty-section omission, all-empty → { totalEventCount: 0, sections: [] } (caller's skip-send signal), the N count, count-sums-to-N, 3-cap/overflow math, createdAt-then-title sort, and the multi-comment-one-task case.
  • tsc --noEmit ✅ · eslint ✅ · prettier ✅

🤖 Generated with Claude Code

Add composeGroupedEmail(events) -> { totalEventCount, sections[] }, the
render-agnostic step that turns one recipient's buffered window of events
into the grouped email structure.

- Sections in fixed order ASSIGNED -> SHARED -> COMMENT -> COMPLETED;
  empty sections omitted
- Per section: sort by createdAt then title, list up to 3 distinct task
  names, overflowCount = max(0, distinctTasks - 3) for "+N other tasks"
- totalEventCount = all buffered events (the N in the subject); section
  counts sum to it
- No I/O, no rendering, no brand prefix — pure function

Multiple comments on one task count as separate events (toward N) but the
task name is listed once. 100% unit coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jun 9, 2026

Copy link
Copy Markdown

OUT-3825

@vercel

vercel Bot commented Jun 9, 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 10, 2026 9:15am

Request Review

@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown

Greptile Summary

Adds composeGroupedEmail(events) — a pure, render-agnostic function that turns one recipient's buffered GroupedEmailEvent window into a typed GroupedEmailContent structure consumed by the upcoming render adapter. No DB, Copilot, or side-effects; 9 unit tests at stated 100% coverage.

  • Section composition: events are filtered per SECTION_ORDER (ASSIGNED → SHARED → COMMENT → COMPLETED), sorted by createdAt then title, deduplicated by taskId, capped at 3 task names with an overflowCount for the renderer's "+N other tasks" copy.
  • totalEventCount: correctly reflects every buffered event (not distinct tasks), matching the intended "You have N new task updates" headline.
  • Schema compatibility: GroupedEmailEventInput is a Pick of the GroupedEmailEvent Prisma model; taskTitleSnapshot is non-nullable (String @db.VarChar(255)), so no null-safety gap.

Confidence Score: 4/5

Safe to merge; the composer is a pure function with no side-effects and solid test coverage for its primary paths.

The implementation is clean and the core logic is correct. The one design nuance worth watching is that distinctTaskNames always surfaces the oldest taskTitleSnapshot for a repeated taskId (because events are sorted ascending before deduplication). In the rare case where a task is renamed during a notification window and generates multiple buffered events, the stale name would appear in the email. This is a narrow edge-case rather than a structural flaw, but it isn't covered by any test.

Both files are straightforward; groupedEmail.composer.ts lines 33–41 (distinctTaskNames) deserve a second look regarding title-snapshot selection order.

Important Files Changed

Filename Overview
src/app/api/notification/groupedEmail.composer.ts New pure function that composes a grouped-email structure from buffered events; logic is correct, but distinctTaskNames always picks the oldest title snapshot for a repeated task, which silently stales names for renamed tasks.
src/app/api/notification/groupedEmail.composer.test.ts 9-case unit test suite with good coverage of section ordering, overflow math, and multi-comment deduplication; module-level seq counter is not reset between cases, and there is no test for the renamed-task (title divergence) scenario.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["composeGroupedEmail(events)"] --> B["Iterate SECTION_ORDER\n(ASSIGNED → SHARED → COMMENT → COMPLETED)"]
    B --> C["buildSection(eventType, events)"]
    C --> D["filter by eventType"]
    D --> E{"any events?"}
    E -- No --> F["return null\n(section omitted)"]
    E -- Yes --> G["sort by createdAt ↑ then title ↑"]
    G --> H["distinctTaskNames(sorted)\n(dedup by taskId, picks oldest snapshot)"]
    H --> I["slice(0, 3) → taskNames\nMath.max(0, distinct-3) → overflowCount"]
    I --> J["GroupedEmailSection"]
    J --> K["filter nulls → sections[]"]
    K --> L["{ totalEventCount: events.length, sections }"]
Loading

Reviews (1): Last reviewed commit: "OUT-3825 | Grouped content composer (pur..." | Re-trigger Greptile

Comment on lines +33 to +41
const seen = new Set<string>()
const names: string[] = []
for (const event of events) {
if (seen.has(event.taskId)) continue
seen.add(event.taskId)
names.push(event.taskTitleSnapshot)
}
return names
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 distinctTaskNames shows the oldest title snapshot when a task is renamed mid-window

distinctTaskNames iterates events already sorted ascending by createdAt, so for a given taskId it always picks the first occurrence — the oldest snapshot. If a task is renamed between two buffered events in the same window (e.g., two COMMENT events where the title changed), the email will display the stale name rather than the most recent snapshot. Passing the events sorted descending (or reducing to last-seen title) would surface the latest snapshot instead.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for addressing it! 👍

import { GroupedEmailEventType } from '@prisma/client'
import { composeGroupedEmail, GroupedEmailEventInput, MAX_TASK_NAMES_PER_SECTION } from './groupedEmail.composer'

let seq = 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The module-level seq counter is never reset between test cases, so each it block inherits whichever value seq reached at the end of the previous test. While no current test relies on specific sequential values, adding or reordering tests could produce surprising taskId/taskTitleSnapshot values. A beforeEach reset would make each case self-contained.

Suggested change
let seq = 0
let seq = 0
beforeEach(() => {
seq = 0
})

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the fix!

@arpandhakal arpandhakal self-assigned this Jun 10, 2026
- distinctTaskNames now keeps the latest taskTitleSnapshot for a repeated
  taskId (last-wins), so a task renamed mid-window shows its current name
  rather than the oldest snapshot. First-seen ordering preserved. Only
  affects repeated taskIds (e.g. multiple comments on one task).
- Reset the module-level seq counter in beforeEach so each test is
  self-contained.
- Add a test for the renamed-task scenario.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 10, 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

@arpandhakal

Copy link
Copy Markdown
Collaborator Author

Thanks @greptile-apps — both P2s addressed in 432cb7f2:

1. Stale title snapshot for a renamed task. distinctTaskNames now keeps the latest taskTitleSnapshot for a repeated taskId (last-wins, since events are already sorted ascending by createdAt), while preserving first-seen ordering. So a task renamed mid-window shows its current name. This only affects repeated taskIds — i.e. multiple comments on one task; ASSIGNED/SHARED are deduped to a single row per task at the DB level, so they only ever have one snapshot. Added a test covering the rename case.

2. Test seq not reset. Added beforeEach(() => { seq = 0 }) so each case is self-contained regardless of order.

10 tests, still 100% coverage; tsc/eslint/prettier clean.

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown

Both fixes look correct.

The distinctTaskNames implementation is clean — the order array preserves insertion order (first-seen) while latestTitle map accumulates last-seen title per taskId. Since sectionEvents is already sorted ascending by createdAt before being passed in, iterating in that order guarantees the final .set() for each taskId is the newest snapshot. The rename test correctly exercises this with two events on the same task_a at different timestamps.

The beforeEach(() => { seq = 0 }) reset is exactly right — no test now leaks sequence state to any other.

Both P2s fully addressed. LGTM. ✅

Comment thread src/app/api/notification/groupedEmail.composer.test.ts Outdated
Comment thread src/app/api/notification/groupedEmail.composer.test.ts Outdated
Comment thread src/app/api/notification/groupedEmail.composer.ts
arpandhakal and others added 2 commits June 10, 2026 14:53
Per review: scope seq, the event factory, sectionFor, and the seq reset
inside describe('composeGroupedEmail') so they don't sit at module level.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Plain-text render (markdown dependency OUT-3830 dropped for M2) + email-only
Copilot send, mirroring the reminder pipeline.

- renderGroupedEmail(content) -> { subject, header, title, body }: section
  headings per type (assigned/shared/comment/completed) with correct
  pluralization, task names as dashed quoted lines, "+N other tasks"
  overflow, sections separated by blank lines
- subject "You have N new task updates" (no brand prefix; Copilot prepends
  "{Brand} portal:"), header "Catch up on task activity", CTA "View all tasks"
- sendGroupedEmail(): email-only deliveryTargets (in-product stays
  immediate), IU sender (resolved by the dispatcher), workspace-scoped token,
  mirrors send-reminder-email.ts

17 unit tests (PRD sample, overflow, pluralization, every section heading,
empty body, email-only payload shape, error propagation).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@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

OUT-3826 | Render + send adapter for the grouped email
@arpandhakal arpandhakal merged commit 1839be1 into OUT-3821 Jun 10, 2026
2 of 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