Notifications never clear on resolution: 93% of unread access-request pings and 78% of unread practicaToReview pings point at already-decided work
Found 2026-09-12 analysing Elazar's notification panel: three "Nueva solicitud de acceso" entries, of which TWO were already approved with users created (#51 Ferley, approved in 14h; #52 Leonardo, approved after 11d) and only ONE is genuinely pending (#54 Micaela, 2026-09-06). Elazar read the panel as three people waiting. It is not. THE DEFECT: a Mars notification is created on an EVENT and is cleared only by a HUMAN CLICK. Nothing clears it when the thing it points at is RESOLVED. So the bell is an append-only event log being read as a work queue, and the unread badge counts work that is already done. MEASURED (queries below, run against prod 2026-09-12): accessRequestSubmitted : 41 unread, of which 33 approved + 5 rejected = 38 STALE (93%). Only 3 unread point at a pending request - and those 3 are ONE request fanned to 3 admins. practicaToReview : 2402 unread joinable, of which 1837 aprobado + 33 rectificar + 8 desaprobado = 1878 STALE (78%). Only 524 still noRevisado. Fleet totals: 6502 notification rows, 6078 unread (93.5%). SELECT n.type, ar.status, count(*) FROM notifications n LEFT JOIN "accessRequests" ar ON ar.id::text = n."relatedId" WHERE n.type='accessRequestSubmitted' AND n."readAt" IS NULL GROUP BY 1,2; SELECT lo.value, count(*) FROM notifications n JOIN practicas p ON p.id::text = n."relatedId" JOIN "lookupOptions" lo ON lo.id = p."approvalStateOptionId" WHERE n.type='practicaToReview' AND n."readAt" IS NULL GROUP BY 1; THE PRECEDENT EXISTS BUT IS SCOPED WRONG TO COPY BLINDLY. src/lib/notifications.ts:146 markComentarioNotificationsRead (bwi#309) is the only resolution-driven clear in the codebase, and it is WHERE "userId"=$1 AND "relatedId"=$3 - PER-VIEWER, which is correct for a comentario because that ping is personal. accessRequestSubmitted is NOT personal: it is a SHARED WORK-QUEUE ITEM fanned to up to 4 admins (measured: SELECT "relatedId", count(*) ... GROUP BY 1 returns 4,4,3,2,1 recipients), and when ANY ONE admin approves it is resolved FOR ALL OF THEM. A per-viewer clear copied from bwi#309 would clear 1 of 4 rows and leave the bug at 3/4 scale, with nothing failing. THE DISCRIMINATOR, and it is what picks the WHERE clause for every type: PERSONAL PING vs SHARED QUEUE ITEM. shared (clear per-ENTITY, across all recipients): accessRequestSubmitted, practicaToReview, supportTicketCreated personal (clear per-VIEWER): practicaComentario (already done), practicaReviewed, practicaRectificar, practicaAyudanteAsignada, supportTicketStaffReply Do not implement a single helper that takes the WHERE clause as a parameter and let the call site choose - the call site choosing is how the wrong scope gets picked silently. Two named helpers. ORPHANS NEED A STORY OR THEY STAY UNREAD FOREVER. notifications."relatedId" is TEXT with NO foreign key (schema.md:344; only userId and notificationTypeOptionId are FKs). Measured: exactly 1 unread row (practicaToReview) whose relatedId resolves to no practica, accessRequest or supportTicket. Small today, unbounded by construction - a clear-on-resolution design that joins to the target table silently skips every orphan. MARS-294 DOES NOT CLOSE THIS, AND WOULD HIDE IT. That row proposes retention/pruning over notifications. A purge drops the stale rows and makes the badge look correct while the missing-clear defect survives untouched - the badge would then re-inflate at exactly the rate it does now. Instruments that fail toward reassurance accumulate; do not let a pruning job be accepted as the remedy for this. THE INSTRUMENT THAT IS ALREADY CORRECT, and the reason this is P3 and not P2: "vHealthAccessRequestsAging" returns EXACTLY ONE row today (Micaela, 5 days, pending_over_3_days) with zero false positives, because it reads STATE (status='pending') rather than EVENTS. The general form worth keeping: A WORK QUEUE MUST BE DERIVED FROM STATE, NEVER ACCUMULATED FROM EVENTS. Where both exist, the state-derived one is the queue and the event log is history. Consider whether /administrar should surface the aging view rather than the bell for this class of work. OPERATIONAL CAUTION FOR WHOEVER TOUCHES THIS FIRST: do NOT use "Marcar todas como leidas" as a cleanup. markAllAsRead (src/lib/notifications.ts:116) is WHERE "userId"=$1 AND "readAt" IS NULL, indiscriminate - it clears Micaela's ping, the only true signal, along with the 38 false ones. NOT IMPLEMENTED. Elazar asked for analysis + a suggested solution, not a change.
Activity
-
coder03-mars-ca