Email Threading — CSR Inbox

Status: Draft — reviewed (CEO + Design + Eng, dual-voice), design only, no implementation Author: Claude (AI pair), prompted by David Boone Date: 2026-07-22 Branch: claude/email-threading-plan-revision · PR base: staging Triggered by: PR #2514 — VMS silent order-loss incident (merged)

Update 2026-09-16. The app now goes past this doc’s UI scope. With the org flag inbox_conversation_view_v1 on, one Conversation tab in the cockpit detail pane replaces the Email and Thread tabs. It shows every inbound message on the item’s thread with its body and triage outcome, archive and trash activity, and internal notes with @mentions. §1 called rebuilding the conversation in-app low-differentiation work, and §2 left CSR notes and @mentions out of scope. Those sections record the July reasoning. Notes live in their own inbox_notes table and do not reuse the CommentsThread entry schema. The Layer 1 data model in §3 is unchanged. The code is CockpitConversation.tsx in apps/webapp/src/components/inbox/cockpit/, served by GET /api/work-items/:id/conversation.

0. Correction from the first draft of this doc

The first pass of this doc conflated two unrelated things: an actual email thread (what Outlook/Superhuman call a conversation — the same mail chain, recognized as one thing) and the app’s internal CommentsThread UI (a Claude Design handoff from PR #871, never implemented, for CSR-authored comments and system events like submitted/erp_rejected). Those are different features. This doc is now about the first one only: email threading is a data-structure problem, not a UI feature to bolt onto internal comments.

Verified against production data before writing this — both providers, not just VMS’s M365. On M365, a single Microsoft Graph conversationId genuinely spans a real reply chain — e.g. subject RE: PO 12267 has 5 distinct messages, all sharing one thread_id, 5 separate email_events rows, zero linkage between them today. Separately, RE: OPOR WK 28 - VISUAL MARKING SYSTEMS INC is one continuous mail thread (one thread_id) running 2026-07-16 → 2026-07-20, touching multiple different PO numbers as the conversation evolves.

Gmail has the identical problem, at larger scale, for paying customers today. Cleveland Kitchen and Genfit are both Gmail-based (not M365), and a quick query against real threads (last 30 days) turns up the same shape, worse:

org	subject	messages	distinct `thread_id`s
Cleveland Kitchen	`Re: Updated PO / SO for Sams Club Riverside`	22	1
Cleveland Kitchen	`Re: Pickle testing for Factor`	20	1
Cleveland Kitchen	`Re: [EXT] Re: PO 604100`	14	1
Cleveland Kitchen	`Re: Cleveland Kitchen Pick-up Schedule (7/16-7/22)`	14	1
Cleveland Kitchen	`Re: Cleveland Kitchen Pick-up Schedule (7/9-7/15)`	12	1
Genfit	`Re: Request for Updated Sales Tax Exemption Certificate`	15	11

CK’s real threads run to 22 messages, all one thread_id, all fully disconnected email_events rows today — a bigger version of the exact VMS problem. apps/temporal-worker/activities/gmail.py:335 already captures Gmail’s threadId into the same email_events.thread_id column, the same way microsoft_graph.py:897 does for Graph’s conversationId — the data capture is already provider-symmetric; this was never an M365-only fix. (Cloudflare — the third value in the email_provider enum — is not: see §3.)

The Genfit row above (15 messages, 11 distinct threads) is most likely the expected, correct kind of split, not a bug — some senders (automated report/reminder systems) send a fresh message each time with no In-Reply-To/References chain, so Gmail doesn’t thread them, which would be the same shape as VMS’s EDI gateway sending a threadless email per PO-change (see #2514). This is not yet confirmed — see Open Questions (§6); an earlier draft of this doc stated it as settled fact while also listing it as an open question, which was a contradiction, not a hedge.

Two facts from this data matter for the design: one mail thread can span many POs, and one PO’s story can span many mail threads. Email threading and PO-level business linking are genuinely different groupings, not two names for the same thing.

Review summary (2026-07-22)

This doc went through a full CEO + Design + Eng review (/autoplan, each phase run with two independent voices — a fresh Claude subagent and Codex — against the actual code, not just this doc’s prose). Both voices independently converged on the same core critiques in every phase, which is the main reason this revision looks materially different from the first draft:

Two citations in the original draft were fabricated and have been removed: m365-email-pull-config (cited in §3, does not exist anywhere in the repo) and docs/feature-flag-policy-erp-writes.md Rule #4 (that file has no numbered rules, and its actual scope doesn’t cover a read-side, non-ERP-write change like this one).
The proposed ingest-time stamping mechanism has no working data path today. email_events has no email_pull_connection_id column, and the real mailbox-connection id (input_data.connection_id in the email-pull workflow) is never forwarded to the activities that persist email_events rows (save_email_order, save_email_rfq, save_skipped_email). This isn’t a missing column, it’s a missing plumbing path — see §3.
Phase 2 as originally written (collapse every multi-message thread into one row) would have broken real invariants, not just been a rough draft: the app’s Claim/Snooze/Archive/Trash actions all target exactly one work_items.id, and the inbox groups rows by a single uxState per group. A thread with multiple independently-actionable orders (the “OPOR WK 28” shape from §0) collapsed into one row has no defined owner, no defined claim target, and risks hiding an unreviewed order behind a reviewed one — arguably the same failure class as the VMS incident, in a new place. Both design reviewers and both engineering reviewers converged on this independently. Phase 2 is reworked in §4.
Layer 2 (deferred in the first draft as “later, no schedule”) is the actual root-cause fix for the VMS incident that triggered this doc — see §2’s revised framing.

Full findings, including migration-risk precedent and deployment-risk specifics, are folded into §3 and §5 below rather than kept in a separate review artifact, so the plan and its review stay in one place.

1. Problem

email_events.thread_id is populated correctly at ingestion — apps/temporal-worker/activities/microsoft_graph.py:897 sets it straight from Microsoft Graph’s conversationId for every M365 message, and apps/temporal-worker/activities/gmail.py:335 does the same for Gmail’s threadId. The data already knows which emails are the same conversation. Nothing downstream reads it: not the inbox list, not the detail pane, not triage, not work_items. So a 5-message reply chain becomes 5 fully disconnected database rows, and if more than one of them gets promoted to a work item, a CSR sees the same conversation as multiple unrelated cards.

This is a data-structure gap, not a missing UI widget. Fixing it in the UI alone (e.g. “add a badge if two cards share a subject”) would be guessing at what a real mail client already solves correctly at the model layer: group by conversation identity, once, at ingestion.

What this doc is not trying to do: clone Outlook/Gmail/Superhuman’s conversation view. Both providers already render the full conversation correctly (and MS Graph messages carry a working webLink deep link into Outlook Web; Gmail thread URLs are constructible from threadId alone) — re- building that in-app is low-differentiation engineering effort. The actual gap only this app can fill is showing which messages in the conversation this system has already acted on — which one became a review, which were skipped, which are still pending triage. §4 scopes Phase 2 to that.

2. Two layers, not one

Layer 1 — Email Thread (this doc’s actual scope). A real mail conversation. Keyed on the provider’s own thread identity (conversationId for Graph, threadId for Gmail). Purely mechanical, no business logic: it answers “is this message part of a conversation I’ve already seen,” nothing more. This is the fix for “we see it twice.”

Layer 2 — PO/order linking (separate, not designed here, but not “unscheduled” either). A business-level concept: “these emails are about the same purchase order,” which may span multiple Layer-1 threads (VMS’s EDI gateway) or may be a subset of one long Layer-1 thread that also discusses other POs (the OPOR WK 28 case above). This is the actual root-cause fix for the VMS incident (#2514) — that incident’s failure mode was threadless EDI emails about the same PO, which Layer 1 alone does not connect. Framing it as a distant, unscheduled follow-up (as the first draft did) undersells that: Layer 1 ships real value for CK/Genfit’s long-thread clutter, but if anyone reads this doc as “the fix for #2514,” they’d be wrong. Recommend scheduling Layer 2’s design as the next doc after Layer 1 ships, not an indefinite someday.

Explicitly not in scope here: the internal CommentsThread UI (PR #871, still unimplemented). That’s CSR-to-CSR collaboration on a work item — comments, @mentions, status_changed/submitted events. It has nothing to do with grouping the customer’s actual email messages. Don’t reuse its entry schema for this; they can both exist and even appear near each other in the UI later, but they model different things.

3. Data model (Layer 1)

email_threads
  id                       uuid pk
  clerk_organization_id    text not null
  email_pull_connection_id uuid not null references email_pull_connections(id)
  provider                 text not null            -- 'microsoft' | 'gmail' (see Cloudflare note below)
  provider_thread_id       text not null             -- raw conversationId / Gmail threadId
  subject_normalized       text                      -- Re:/Fwd:/[EXTERNAL] stripped, for display
  message_count            integer not null default 0
  first_message_at         timestamptz
  last_message_at          timestamptz
  created_at, updated_at

  unique (email_pull_connection_id, provider_thread_id)

email_events.email_thread_id — nullable FK to email_threads.id.

Scoped to email_pull_connection_id, not clerk_organization_id. A provider conversationId/threadId is only unique within one mailbox; two different connections for the same org (or, in principle, colliding raw ids across orgs) must never merge into one thread. This repo has already shipped one incident from conflating different “connection id” spaces in the same code path (_agent_traits()’s docstring in gemini.py:1862 documents PR #2514 — 92+ lost POs from exactly this class of bug) — name this column email_pull_connection_id everywhere, never bare connection_id, and do not reuse alias.get("connection_id") anywhere near this feature: that value is actually erp_connection_id, a different id space entirely.

Prerequisite: the plumbing to populate this doesn’t exist yet

This is the one blocking gap found in review, confirmed independently by two reviewers reading the actual code: email_events has no email_pull_connection_id column today, and the real mailbox-connection id never reaches the activities that persist email_events rows. input_data.connection_id (the actual email_pull_connections.id) is available in apps/temporal-worker/workflows/email_processing.py and passed into fetch_microsoft_message/fetch_gmail_message_for_connection, but it is not forwarded to save_email_order, save_email_rfq, or save_skipped_email in apps/temporal-worker/activities/database.py — those three activities have no parameter for it today. Phase 1 must add:

email_events.email_pull_connection_id (nullable at first — see backfill note below).
A new parameter on save_email_order/save_email_rfq/save_skipped_email carrying input_data.connection_id through from the workflow.
The email_threads upsert + email_events.email_thread_id stamp, inside those same three save activities (single transaction with the insert, not a separate follow-up write).

Populated at ingest, not at triage. Threading is a property of the message, not of the triage decision — a skip-decided reply is exactly as much “the same conversation” as a process-decided one, so all three save paths need this, not just the order-producing one.

Atomicity, not read-then-write. message_count/last_message_at maintenance must be a single atomic SQL statement (e.g. ON CONFLICT ... DO UPDATE SET message_count = email_threads.message_count + 1) inside the same transaction as the email_events insert. Temporal activities are at-least-once — a read-modify-write in application code will double-count on retry. This codebase has already shipped a dedup fix for the adjacent “message ingested twice via webhook + poll” shape of bug; don’t reintroduce the same class here.

Deployment ordering. The new activity parameter changes an activity signature. Deploy the activity-side change (new optional parameter, default behavior unchanged) before the workflow-side change that starts passing it, so a rolling deploy never has new workflow code calling old-signature workers.

Migration/backfill

Before backfilling email_pull_connection_id into historical rows, audit how many existing email_events rows can actually be mapped to a connection with confidence (legacy webhook-path rows and any Cloudflare-routed mail may not be cleanly attributable). Do this audit before deciding on not null — this repo has already hit exactly this failure mode once: pdf_documents.r2_key’s global unique constraint was permanently unappliable in prod because of 123 pre-existing violating rows, discovered only after the migration was written (see PR #2213 / the revert of #2211). Keep email_pull_connection_id nullable until the audit confirms coverage; rows that can’t be confidently scoped stay unthreaded rather than guessing.

Cloudflare provider is excluded from Layer 1

email_provider has a third value, cloudflare — the current pipeline’s non-pull ingestion path. It has no conversation/thread API to source a thread_id from, so Cloudflare-routed mail has no threading story under this design and is excluded, not silently unsupported. If Cloudflare-sourced order volume grows enough to matter, that’s a separate, later scoping question.

Table vs. computed view — prove the need before building

Both CEO-phase reviewers independently questioned whether a new mutable table is justified at CK’s actual scale (22 messages is the largest real thread found) versus a GROUP BY (email_pull_connection_id, provider_thread_id) query or a periodically-refreshed materialized view — either of which would have no upsert-on-ingest logic, no counter-drift failure class, and no second source of truth to keep in sync with work_items. Recommendation: keep the table (Phase 2 needs a real join target for per-message data anyway, and an explicit FK is more legible than a materialized-view refresh story) but run an EXPLAIN ANALYZE against CK/Genfit’s real email_events volume before implementation starts, to confirm a live aggregate genuinely wouldn’t be fast enough. If it would be, the lighter-weight version is the right call — this is a real open decision, not settled by this doc.

work_items change: today work_items.email_event_id is 1:1 (work_items_email_event_unique). Add work_items.email_thread_id (nullable FK, denormalized from the linked email_events row, indexed) so the inbox can group/query by thread without a join through email_events on every list render.

4. UX (Layer 1 only)

Reworked after review. The first draft’s “collapse every multi-message thread into one row + full conversation view” broke real invariants in this app, independently flagged by both design reviewers and both engineering reviewers: every mutating action (Claim, Snooze, Archive, Trash) targets exactly one work_items.id, and the inbox groups rows by a single uxState per group (RowGroup, groupItemsByState in cockpit/lib.ts). A thread with more than one independently actionable work item (the “OPOR WK 28” shape from §0 — one long thread touching multiple distinct POs over time) has no defined Claim target and no defined uxState group if collapsed — and collapsing it risks visually hiding an unreviewed order behind a reviewed one, which is close to the same failure shape as the VMS incident, just moved into the UI layer instead of the triage layer.

The rule this revision uses instead:

A thread collapses into one row only when it maps to exactly one actionable work item (i.e., at most one message in the thread is process-classified). That row’s Claim/Snooze/Archive/Trash target that one work item, unchanged from today. A small, muted badge shows total message count for context (a chat-bubble icon and the count, “4”) — informational only, never the row’s primary identity, and never the row’s SLA/age clock (the row’s age stays anchored to the actionable work item’s own receivedAt, not “most recent activity in thread” — a stale order shouldn’t look fresh because someone replied about something unrelated in the same thread).
A thread with more than one actionable work item does not collapse. Each stays its own row, exactly as today. This is the “don’t hide an order” guarantee; it costs some row-count reduction on the rare multi-order threads, in exchange for never risking a silently-buried order.
Row identity stays the customer/vendor (item.company, unchanged from today), never “most recent message’s sender” — the latest message in a thread is often the CSR’s own reply or a CC’d broker, not the customer.

Thread context (new surface — not a full conversation renderer, not the CommentsThread component): add a compact “Thread” section to the existing CockpitDetail/CockpitItemPreview pane, alongside the current Email/ Document tabs — not a new top-level UI. Shows: message count, a chronological compact list of the other messages in the thread (sender, timestamp, process/skip status, one-line snippet), and a deep link out to the provider (Graph webLink for M365, the constructed Gmail thread URL) for anyone who wants to read full message bodies in their own mail client rather than a re-built renderer in-app.

Scope: Cockpit view only for Phase 2. This app has three independent row surfaces over the same InboxItem model — CockpitRow (the DESIGN.md- canonical two-pane view), ConservativeRow (dense table), and KanbanCard (drag-drop board, already visually packed with vendor/PO/value/assignee/SLA). Phase 2 targets Cockpit only; Conservative and Kanban are explicitly deferred, not silently unhandled — a message-count badge doesn’t have an obvious slot on a Kanban card without cutting something else, and that’s a design decision for a later pass, not a gap in this one.

States (unspecified in the first draft — named here):

Thread metadata loading: skeleton, matching the existing CockpitItemPreview loading pattern rather than inventing a new one.
Thread fetch error: inline retry, same treatment as the existing preview error state.
Message pending triage: a message can exist in a thread before triage decides skip/process (§3’s “populated at ingest, not at triage”). Render it grayed, no status badge, no false promise of a review link — never omit it from the list, since omitting it is exactly the kind of silent gap this whole doc exists to close.
Live update while a thread is open: the existing 30s reconcile poll (ReconcileEntry in types.ts) doesn’t carry email_thread_id or a message count today. Phase 2 does not wire live thread updates into that poll — an open conversation is a static snapshot until refreshed. State this explicitly as a v1 limitation rather than leaving it ambiguous.

Accessibility / responsive (minimum bar, not exhaustive): the thread badge and any expand/collapse control need real keyboard behavior (not just mouse), an accessible name (“4 messages in this thread,” not a bare number), and must not reduce the existing row’s touch target. On mobile (<768px, matching the cockpit’s existing breakpoint), the Thread section lives in the same full-screen detail subview the app already uses for mobile, not a nested expandable list inside a row.

5. Rollout

This change (ingest-time stamping, read-side UI) doesn’t fall under docs/feature-flag-policy-erp-writes.md’s actual scope (ERP writes, quote/ approval, extraction, agentic validation, triage decision logic, customer/ item matching) — an earlier draft cited a “Rule #4” from that file which doesn’t exist. A flag is still reasonable as an operational kill switch given this touches the ingest path for every email, not as a compliance requirement. If used, reuse _agent_traits()’s identity/trait shape (added in #2514) for any per-connection targeting — don’t reintroduce that bug on a new flag.

Phase 1a (prerequisite, backend only): thread the real email_pull_connection_id through the three save activities (§3’s “Prerequisite” section). Ship this before anything else — nothing else in this doc works without it. Deploy activity-side first, workflow-side second (see Deployment ordering in §3).
Phase 1b (backend, shadow, both providers together): email_threads upsert + email_thread_id stamping at ingest, atomic counters, both M365 and Gmail in the same pass (both already write the same email_events.thread_id column from their native conversation id, per §0). No UI change. Data audit + nullable backfill per §3’s migration note.
Phase 2 (UI, Cockpit only, pilot orgs): collapsed rows for single- actionable threads + Thread context section + provider deep link, per the reworked §4. Pilot on whichever connection’s Phase 1b shadow data is cleanest to verify against, not gated by provider — CK’s real 12-22 message threads are arguably a better stress test than VMS’s shorter ones.
Phase 3: all orgs, Cockpit view.
Next doc, scheduled (not “someday”): Layer 2 (PO-level cross-thread linking) — the actual #2514 root-cause fix. Start once Phase 1b’s shadow data shows the real shape of VMS’s threadless-EDI volume.

6. Open questions

Genfit’s Re: Request for Updated Sales Tax Exemption Certificate case (15 messages, 11 threads) needs to be confirmed as “genuinely separate messages” (no In-Reply-To/References), not a Gmail-side quirk worth handling specially — confirm during Phase 1b shadow verification, before Phase 2 ships for Gmail orgs. (Not settled by this doc — see §0.)
Table vs. computed view (§3): run the EXPLAIN ANALYZE before committing to the migration.
Backfill volume: how far back should the one-time backfill reach for existing orgs (Genfit/CK have been live far longer than VMS)? Full history vs. a bounded window is a cost/value call for Phase 1b, not a design blocker.
Data-audit scope (§3): what fraction of historical email_events rows can actually be attributed to a connection with confidence? Determines whether email_pull_connection_id can ever become not null, or stays nullable permanently for pre-threading-era rows.