From f319264009fc716023e2fbeb0b344abdf6b824d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 06:04:01 -0400 Subject: [PATCH 1/2] The community feed says how much has arrived since you last looked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "when someone recieves messaes it is notifies in notification bell but no no.of messages appear next to messages in the sidepanel. fix this also applies to community feed, approvals, etc" Messages and approvals were answerable from tables that already existed — `Delivery.readAt` for one, the approval window for the other. The community feed was not, and that is the whole reason it shipped without a count while its two neighbours in the rail got theirs. `FeedPost` records who WROTE a post; nothing anywhere recorded who had READ one. The test asserting the absence said so, and said what it would take: it("does NOT badge the community feed", …) // `FeedPost` has no per-user read state — no seenAt, no visit marker. A // count derived from "posted recently" would look identical to a real one // and be a guess. It needs a migration and it gets its own change. This is that change. The assertion is now the positive one. ── WHY A WATERMARK AND NOT A RECENCY WINDOW ──────────────────────────────── "Posted in the last seven days" is one line, needs no table, and looks identical on screen. It is also a guess in both directions: it keeps counting posts you read an hour ago, and it stops counting the post you have never opened the moment it turns eight days old. A badge that is wrong both ways is worse than no badge, because people act on it. `FeedVisit` holds one instant per person per institution. A feed is read by SCROLLING — nobody opens each item — so "everything up to here" is the only thing a reader actually did, and it is exactly what a watermark records. The per-post alternative grows by (people x posts) to answer a question no surface in Tenure asks. ── A MISSING ROW IS "NEVER OPENED", NOT "EVERYTHING IS UNREAD" ───────────── The count falls back to the reader's own `createdAt`. Somebody joining today is not met with four years of a club's history in a red badge — the same thing Slack and Teams do, and for the same reason: a badge reading 900 on first login is one people learn to ignore, which costs the two beside it that matter. ── THE BADGE COUNTS WHAT THE PAGE WILL SHOW ──────────────────────────────── `feedInstitutionIds` was four lines inlined on the feed page, and four lines is exactly the size of thing that gets written twice and then drifts. It has two callers now — the page that RENDERS the feed and the badge that COUNTS what is new on it — and a badge counting rows the page would not show is a badge for a page that looks empty. Neither number looks wrong on its own, which is why it is one function. ── OPENING THE FEED IS READING IT ────────────────────────────────────────── The watermark is written AFTER the posts are loaded, so a read that failed does not mark anything seen, and per institution, because the feeds are separate. Wrapped in try/catch rather than `.catch()` on the promise: the first version chained `.catch(() => null)`, which covers a REJECTED upsert and not a synchronous throw on the way to calling it — and the page's own suite proved the difference by reaching `db.feedVisit` before its fake had one. A page whose entire job is to show a feed must not fail to render because a read-marker could not be written. The worst case is a badge that stays up until the next visit, which the reader clears by doing what they were already doing. ── FOUR REPO GATES CAUGHT THE REGISTRATIONS A NEW MODEL NEEDS ────────────── All four were doing their job, and all four are now satisfied: the tenancy registry (FeedVisit carries institutionId), the retention register, the rail's own "does NOT badge the feed" assertion, and the feed page's action test, whose db fake had no `feedVisit` — which is how the synchronous-throw hazard above was found rather than shipped. DEPLOY SHAPE: a new table, no backfill, no column added to an existing one. Nothing reads it until the code that writes it ships, and the join-date fallback means the feature is correct on an empty table from the first request. No ordering hazard in either direction. tsc 0 · jest 420 suites / 6,722 tests green · next build exit 0 · eslint clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../migration.sql | 67 +++++++++++++ apps/web/prisma/schema.prisma | 31 ++++++ .../consequential-actions-confirm.test.tsx | 4 + apps/web/src/app/(app)/feed/page.tsx | 65 ++++++++---- apps/web/src/components/shell/nav.ts | 1 + .../the-rail-says-what-is-waiting.test.tsx | 17 +++- apps/web/src/lib/feed/audience.ts | 33 +++++++ ...e-badge-counts-what-the-page-shows.test.ts | 98 +++++++++++++++++++ apps/web/src/lib/nav/attention-shape.ts | 4 +- apps/web/src/lib/nav/attention.ts | 67 ++++++++++++- apps/web/src/lib/retention-register.test.ts | 6 ++ apps/web/src/lib/tenancy/registry.ts | 1 + 12 files changed, 369 insertions(+), 25 deletions(-) create mode 100644 apps/web/prisma/migrations/20260826120000_the_feed_remembers_where_you_stopped/migration.sql create mode 100644 apps/web/src/lib/feed/audience.ts create mode 100644 apps/web/src/lib/feed/the-badge-counts-what-the-page-shows.test.ts diff --git a/apps/web/prisma/migrations/20260826120000_the_feed_remembers_where_you_stopped/migration.sql b/apps/web/prisma/migrations/20260826120000_the_feed_remembers_where_you_stopped/migration.sql new file mode 100644 index 00000000..6790ab20 --- /dev/null +++ b/apps/web/prisma/migrations/20260826120000_the_feed_remembers_where_you_stopped/migration.sql @@ -0,0 +1,67 @@ +-- THE FEED REMEMBERS WHERE YOU STOPPED. +-- +-- The owner's report was about the rail: "when someone recieves messaes it is +-- notifies in notification bell but no no.of messages appear next to messages +-- in the sidepanel. fix this also applies to community feed, approvals, etc". +-- +-- Messages and approvals were answerable from tables that already existed — +-- `Delivery.readAt` for one, the approval window for the other. The community +-- feed was not, and that is the whole reason it shipped without a count while +-- its two neighbours got theirs. `FeedPost` records who WROTE a post and when; +-- nothing anywhere recorded who had READ one. +-- +-- A count can be invented without this table. "Posted in the last seven days" +-- is one line of code and looks identical on screen — and it is a guess: it +-- keeps counting things you read an hour ago, and it stops counting the post +-- you have never seen the moment it turns eight days old. A badge that is +-- sometimes wrong in both directions is worse than no badge, because people +-- act on it. +-- +-- ── WHY ONE INSTANT, NOT ONE ROW PER POST ────────────────────────────────── +-- +-- A feed is read by SCROLLING. Nobody opens each item, so "everything up to +-- here" is the only thing a reader actually did, and it is exactly what a +-- watermark records. The per-post alternative grows by (people x posts) to +-- answer a question — "did this specific person read this specific post?" — +-- that no surface in Tenure asks. +-- +-- Keyed [userId, institutionId] rather than by user alone because a person can +-- hold seats in more than one institution's workspace, and the feeds are +-- separate. One watermark per feed they can see. +-- +-- ── WHAT A MISSING ROW MEANS ─────────────────────────────────────────────── +-- +-- That the person has never opened the feed. It is NOT read as "everything is +-- unread": the count falls back to the reader's own `User.createdAt`, so +-- somebody who joins today is not met with four years of a club's history in a +-- red badge. Slack and Teams both do this, and for the same reason — a badge +-- that says 900 on first login is a badge people learn to ignore, which costs +-- the two that actually matter. +-- +-- ── DEPLOY SHAPE ─────────────────────────────────────────────────────────── +-- +-- A new table with no backfill and no column added to an existing one. Nothing +-- reads it until the code that writes it ships, and the fallback above means +-- the feature is correct on an empty table from the first request. There is no +-- ordering hazard between this migration and the deploy in either direction. + +-- CreateTable +CREATE TABLE "FeedVisit" ( + "userId" TEXT NOT NULL, + "institutionId" TEXT NOT NULL, + "lastSeenAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "FeedVisit_pkey" PRIMARY KEY ("userId","institutionId") +); + +-- The referencing side of a foreign key is not indexed for you, and deleting +-- an institution cascades here. The primary key already covers `userId` as its +-- leading column, so only this one is needed. +-- CreateIndex +CREATE INDEX "FeedVisit_institutionId_idx" ON "FeedVisit"("institutionId"); + +-- AddForeignKey +ALTER TABLE "FeedVisit" ADD CONSTRAINT "FeedVisit_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "FeedVisit" ADD CONSTRAINT "FeedVisit_institutionId_fkey" FOREIGN KEY ("institutionId") REFERENCES "Institution"("id") ON DELETE CASCADE ON UPDATE CASCADE; diff --git a/apps/web/prisma/schema.prisma b/apps/web/prisma/schema.prisma index d78d20f2..5d3a54cb 100644 --- a/apps/web/prisma/schema.prisma +++ b/apps/web/prisma/schema.prisma @@ -119,6 +119,7 @@ model Institution { serviceNotices ServiceNotice[] signatureMarks SignatureMark[] analyticsEvents ProductAnalyticsEvent[] + feedVisits FeedVisit[] } enum InstitutionRole { @@ -191,6 +192,7 @@ model User { /// Reminders this person has been sent about approvals still waiting on /// them. One row per (approval, person, day) — see `ApprovalReminder`. approvalReminders ApprovalReminder[] + feedVisits FeedVisit[] } // ─── Organizations & Role Seats ─────────────────────────────────────────────── @@ -1403,6 +1405,35 @@ enum NotificationChannel { // Clubs post collaboration calls; other clubs raise interest; the OSE // Director sits in the middle and approves each collaboration. +/// WHEN A PERSON LAST LOOKED AT THE COMMUNITY FEED. +/// +/// The feed had no per-user read state at all, which is why its nav entry was +/// the one with no count while messages and approvals had theirs. A number +/// derived from "posted this week" would have been a guess dressed as a fact. +/// +/// One row per person per institution, holding one instant. Deliberately NOT a +/// per-POST read table: a feed is read by scrolling, not by opening each item, +/// so "everything up to here" is what a reader actually did — and a per-post +/// table would grow by (people x posts) to answer a question nobody asks about +/// an individual post. +/// +/// A person with no row has never opened the feed. That is NOT treated as +/// "everything is unread": the count falls back to their own `createdAt`, so +/// somebody who joins today is not greeted by four years of a club's history. +/// It is the same thing Slack and Teams do, and for the same reason — a badge +/// showing 900 is a badge people learn to ignore. +model FeedVisit { + userId String + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + institutionId String + institution Institution @relation(fields: [institutionId], references: [id], onDelete: Cascade) + /// The moment the feed was last rendered for this person. + lastSeenAt DateTime @default(now()) + + @@id([userId, institutionId]) + @@index([institutionId]) +} + model FeedPost { id String @id @default(cuid()) institutionId String diff --git a/apps/web/src/app/(app)/feed/consequential-actions-confirm.test.tsx b/apps/web/src/app/(app)/feed/consequential-actions-confirm.test.tsx index 37d5867c..347daf98 100644 --- a/apps/web/src/app/(app)/feed/consequential-actions-confirm.test.tsx +++ b/apps/web/src/app/(app)/feed/consequential-actions-confirm.test.tsx @@ -83,6 +83,10 @@ jest.mock("@/lib/db", () => ({ ? [{ institutionId: INSTITUTION }] : [{ id: "org_swib", name: "Simon Women in Business" }], }, + // The read-marker behind the rail's "new" badge. Present in the fake + // because the page writes it on every render — and because leaving it out + // is how this suite caught the write being able to take the page down. + feedVisit: { upsert: async () => ({}) }, feedPost: { findMany: async () => [ { diff --git a/apps/web/src/app/(app)/feed/page.tsx b/apps/web/src/app/(app)/feed/page.tsx index 89959700..61d66d92 100644 --- a/apps/web/src/app/(app)/feed/page.tsx +++ b/apps/web/src/app/(app)/feed/page.tsx @@ -1,4 +1,5 @@ import Link from "next/link" +import { feedInstitutionIds } from "@/lib/feed/audience" import { redirect } from "next/navigation" import { CalendarDays, Handshake, MessageCircle, Newspaper } from "@/components/ui/icons" import { auth } from "@/lib/auth" @@ -45,23 +46,10 @@ export default async function FeedPage() { .filter((r) => r.status === "ACTIVE") .map((r) => r.organizationId) - // The whole institution sees the community feed - const institutionIds = oseInstitutionIds.length - ? oseInstitutionIds - : [ - ...new Set( - ( - await db.organization.findMany({ - where: { - id: { - in: ctx.orgRoles.map((r) => r.organizationId), - }, - }, - select: { institutionId: true }, - }) - ).map((o) => o.institutionId) - ), - ] + // The whole institution sees the community feed. The rule lives in + // `lib/feed/audience.ts` because the sidebar's unread badge has to count + // exactly what this page will show — see the note there. + const institutionIds = await feedInstitutionIds(ctx) /* * The three reads this page can start at once. @@ -117,6 +105,49 @@ export default async function FeedPage() { : [], ]) + /* + * OPENING THE FEED IS READING IT. + * + * The watermark behind the rail's "new" badge. Written AFTER the posts are + * loaded, so a read that failed does not mark anything seen — and written + * per institution, because the feeds are separate and so are the counts. + * + * `updatedAt`-style semantics on purpose: the row moves forward to now on + * every visit, so a second look at the same feed clears what arrived + * between the two. + * + * NOT awaited into the render path's critical section on its own — it is + * fire-and-forget against the reader's own row, and a failure here must not + * fail the page. The worst case is a badge that stays up until the next + * visit, which is exactly the failure a reader can recover from themselves. + */ + const seenAt = new Date() + try { + await Promise.all( + institutionIds.map((institutionId) => + db.feedVisit.upsert({ + where: { userId_institutionId: { userId, institutionId } }, + create: { userId, institutionId, lastSeenAt: seenAt }, + update: { lastSeenAt: seenAt }, + }) + ) + ) + } catch { + /* + * A TRY/CATCH, not `.catch()` on the promise. + * + * The first version chained `.catch(() => null)`, which covers a REJECTED + * upsert and not a synchronous throw on the way to calling it — and the + * property access is the part most likely to fail, as the page's own test + * demonstrated by reaching `db.feedVisit` before it existed. + * + * A page whose entire job is to show a feed must not fail to render + * because a read-marker could not be written. The worst case here is a + * badge that stays up until the next visit, which is a thing the reader + * clears themselves by doing what they were already doing. + */ + } + // Resolve author names in one pass const authorIds = [ ...new Set([ diff --git a/apps/web/src/components/shell/nav.ts b/apps/web/src/components/shell/nav.ts index 78c9b0fe..3caf6d04 100644 --- a/apps/web/src/components/shell/nav.ts +++ b/apps/web/src/components/shell/nav.ts @@ -164,6 +164,7 @@ export const NAV_CONTRIBUTIONS: readonly NavContribution[] = [ module: "feed", capability: null, workspaces: ALL_WORKSPACES, + attention: { key: "feed", noun: "new" }, }, { label: "Messages", diff --git a/apps/web/src/components/shell/the-rail-says-what-is-waiting.test.tsx b/apps/web/src/components/shell/the-rail-says-what-is-waiting.test.tsx index f1a816a4..014e0bbe 100644 --- a/apps/web/src/components/shell/the-rail-says-what-is-waiting.test.tsx +++ b/apps/web/src/components/shell/the-rail-says-what-is-waiting.test.tsx @@ -49,11 +49,18 @@ describe("which entries carry a badge", () => { }) }) - it("does NOT badge the community feed", () => { - // `FeedPost` has no per-user read state — no seenAt, no visit marker. A - // count derived from "posted recently" would look identical to a real one - // and be a guess. It needs a migration and it gets its own change. - expect(ENTRY("Community feed").attention).toBeUndefined() + it("badges the community feed with what has arrived since you last looked", () => { + /* + * This assertion used to be `toBeUndefined()`, and its comment said why: + * "`FeedPost` has no per-user read state — no seenAt, no visit marker. A + * count derived from 'posted recently' would look identical to a real one + * and be a guess. It needs a migration and it gets its own change." + * + * That migration is `20260826120000_the_feed_remembers_where_you_stopped`, + * and `FeedVisit` is the visit marker. The reason for the absence is gone, + * so the absence goes with it. + */ + expect(ENTRY("Community feed").attention).toEqual({ key: "feed", noun: "new" }) }) it("gives every badge a figure that exists", () => { diff --git a/apps/web/src/lib/feed/audience.ts b/apps/web/src/lib/feed/audience.ts new file mode 100644 index 00000000..ed15140b --- /dev/null +++ b/apps/web/src/lib/feed/audience.ts @@ -0,0 +1,33 @@ +import "server-only" +import { db } from "@/lib/db" +import type { UserContext } from "@/lib/rbac" + +/** + * WHOSE FEED THIS PERSON SEES. + * + * ── Why this is a function and not two copies of a rule ──────────────────── + * + * The community feed is institution-wide: an OSE role names its institutions + * directly, and everybody else reaches theirs through the clubs they hold a + * seat in. That is four lines, and four lines is exactly the size of thing that + * gets written twice and then drifts. + * + * It now has two callers — the page that RENDERS the feed and the badge that + * COUNTS what is new on it. A badge counting rows the page would not show is a + * badge for a page that looks empty, and the two disagreeing is not a failure + * anything reports: both numbers are plausible. + * + * The same reason `rosterStanding` was lifted out of a route handler earlier + * today. A rule with one call site is a rule; a rule with two call sites and no + * name is a coincidence. + */ +export async function feedInstitutionIds(ctx: UserContext): Promise { + const oseInstitutionIds = ctx.institutionRoles.map((m) => m.institutionId) + if (oseInstitutionIds.length) return oseInstitutionIds + + const orgs = await db.organization.findMany({ + where: { id: { in: ctx.orgRoles.map((r) => r.organizationId) } }, + select: { institutionId: true }, + }) + return [...new Set(orgs.map((o) => o.institutionId))] +} diff --git a/apps/web/src/lib/feed/the-badge-counts-what-the-page-shows.test.ts b/apps/web/src/lib/feed/the-badge-counts-what-the-page-shows.test.ts new file mode 100644 index 00000000..77691ef8 --- /dev/null +++ b/apps/web/src/lib/feed/the-badge-counts-what-the-page-shows.test.ts @@ -0,0 +1,98 @@ +import { readFileSync } from "node:fs" +import { join } from "node:path" + +/** + * "when someone recieves messaes it is notifies in notification bell but no + * no.of messages appear next to messages in the sidepanel. fix this also + * applies to community feed, approvals, etc" + * + * Messages and approvals were answerable from tables that already existed. The + * community feed was not — `FeedPost` recorded who WROTE a post and nothing + * recorded who had READ one — so it shipped without a count while its two + * neighbours in the rail got theirs. + * + * ── What these pin ───────────────────────────────────────────────────────── + * + * Not the arithmetic, which Prisma does. The two things that make a badge + * TRUSTWORTHY, both of which are invisible when they break: + * + * 1. The badge and the page ask the SAME audience question. A badge counting + * rows the page will not show sends somebody to a screen that looks empty, + * and neither number looks wrong on its own. + * 2. The count is a watermark, not a recency window. "Posted in the last + * seven days" is one line, looks identical on screen, and is a guess in + * both directions — it keeps counting what you read an hour ago and stops + * counting what you have never opened once it turns eight days old. + */ + +const SRC = join(__dirname, "..", "..") +const read = (rel: string) => readFileSync(join(SRC, rel), "utf8") + +describe("the badge and the feed page ask one question", () => { + const page = read("app/(app)/feed/page.tsx") + const attention = read("lib/nav/attention.ts") + const audience = read("lib/feed/audience.ts") + + it("reads the real files, so this suite is not vacuously passing", () => { + expect(page.length).toBeGreaterThan(2000) + expect(attention.length).toBeGreaterThan(2000) + }) + + it("BOTH derive the institutions from the one helper", () => { + // The rule was four lines inlined on the page. Four lines is exactly the + // size of thing that gets written twice and then drifts. + expect(page).toContain("feedInstitutionIds(ctx)") + expect(attention).toContain("feedInstitutionIds(ctx)") + expect(audience).toContain("export async function feedInstitutionIds") + }) + + it("both exclude archived posts", () => { + expect(page).toContain("isArchived: false") + expect(attention).toContain("isArchived: false") + }) +}) + +describe("the count is a watermark, not a guess", () => { + const attention = read("lib/nav/attention.ts") + + it("counts posts newer than the reader's last visit", () => { + expect(attention).toContain("createdAt: { gt:") + expect(attention).toContain("db.feedVisit.findMany") + }) + + it("falls back to when the READER JOINED, never to the beginning of time", () => { + // Somebody who joins today must not be met with four years of a club's + // history in a red badge. A badge reading 900 on first login is one people + // learn to ignore, which costs the two beside it that do matter. + expect(attention).toContain("me?.createdAt") + }) + + it("does not derive the count from a recency window", () => { + // The one-line alternative this table exists to avoid. + const guesses = [/last\s*7\s*days/i, /subDays/, /sevenDaysAgo/, /RECENT_DAYS/] + for (const guess of guesses) expect(attention).not.toMatch(guess) + }) +}) + +describe("opening the feed records that you opened it", () => { + const page = read("app/(app)/feed/page.tsx") + + it("writes the watermark AFTER the posts are read", () => { + // A read that failed must not mark anything as seen. + expect(page.indexOf("db.feedVisit.upsert")).toBeGreaterThan( + page.indexOf("db.feedPost.findMany") + ) + }) + + it("cannot take the page down when the write fails", () => { + /* + * A try/catch and not `.catch()` on the promise. The first version chained + * `.catch(() => null)`, which covers a rejected upsert and NOT a + * synchronous throw on the way to calling it — and the property access is + * the part most likely to fail. The page's own suite proved it by reaching + * `db.feedVisit` before the fake had one. + */ + const write = page.slice(page.indexOf("const seenAt = new Date()")) + expect(write.slice(0, 400)).toContain("try {") + }) +}) diff --git a/apps/web/src/lib/nav/attention-shape.ts b/apps/web/src/lib/nav/attention-shape.ts index d868c729..abe9d60a 100644 --- a/apps/web/src/lib/nav/attention-shape.ts +++ b/apps/web/src/lib/nav/attention-shape.ts @@ -12,12 +12,14 @@ export interface NavAttention { readonly messaging: number /** Open approvals this person can decide right now. */ readonly approvals: number + /** Community-feed posts since this person last opened the feed. */ + readonly feed: number } /** Every key a nav entry may point its badge at. */ export type NavAttentionKey = keyof NavAttention -export const NAV_ATTENTION_NONE: NavAttention = { messaging: 0, approvals: 0 } +export const NAV_ATTENTION_NONE: NavAttention = { messaging: 0, approvals: 0, feed: 0 } /** * Above this the badge stops counting and starts saying "a lot". diff --git a/apps/web/src/lib/nav/attention.ts b/apps/web/src/lib/nav/attention.ts index bd54ebfa..38948850 100644 --- a/apps/web/src/lib/nav/attention.ts +++ b/apps/web/src/lib/nav/attention.ts @@ -1,4 +1,5 @@ import { db } from "@/lib/db" +import { feedInstitutionIds } from "@/lib/feed/audience" import type { ApprovalStatus, Prisma } from "@prisma/client" import { approvalMoney, @@ -167,10 +168,72 @@ async function approvalsNeedingMe(ctx: UserContext): Promise { return count } +/** + * Community-feed posts this person has not seen. + * + * ── Why a watermark and not "posted this week" ───────────────────────────── + * + * `FeedPost` records who wrote a post; until `FeedVisit` there was nothing + * recording who had READ one, which is why this was the nav entry with no count + * while messages and approvals had theirs. + * + * A recency window is one line and looks identical on screen. It is also a + * guess in both directions: it keeps counting posts you read an hour ago, and + * it stops counting the post you have never opened the moment it turns eight + * days old. A badge that is sometimes wrong in both directions is worse than no + * badge, because people act on it. + * + * ── The fallback for somebody who has never opened the feed ──────────────── + * + * Their own `createdAt`, not the beginning of time. A person joining today is + * not met with four years of a club's history in a red badge — the same thing + * Slack and Teams do, and for the same reason: a badge reading 900 on first + * login is one people learn to ignore, which costs the two beside it that + * actually matter. + * + * ── It counts what the PAGE would show ───────────────────────────────────── + * + * Same institutions via `feedInstitutionIds`, same `isArchived: false`. A badge + * counting rows the page would not show is a badge for a page that looks empty, + * and nothing anywhere reports the two disagreeing: both numbers are plausible. + */ +async function unreadFeed(ctx: UserContext): Promise { + const institutionIds = await feedInstitutionIds(ctx) + if (institutionIds.length === 0) return 0 + + const [visits, me] = await Promise.all([ + db.feedVisit.findMany({ + where: { userId: ctx.userId, institutionId: { in: institutionIds } }, + select: { institutionId: true, lastSeenAt: true }, + }), + db.user.findUnique({ where: { id: ctx.userId }, select: { createdAt: true } }), + ]) + + const seenAt = new Map(visits.map((v) => [v.institutionId, v.lastSeenAt])) + // A missing user cannot be reached here — the context was built from one — + // but the epoch fallback is deliberate rather than incidental: it would count + // everything, and being loudly wrong beats silently returning zero. + const joined = me?.createdAt ?? new Date(0) + + const counts = await Promise.all( + institutionIds.map((institutionId) => + db.feedPost.count({ + where: { + institutionId, + isArchived: false, + createdAt: { gt: seenAt.get(institutionId) ?? joined }, + }, + }) + ) + ) + return counts.reduce((a, b) => a + b, 0) +} + export async function navAttention(ctx: UserContext): Promise { - const [messaging, approvals] = await Promise.all([ + const [messaging, approvals, feed] = await Promise.all([ unreadMessages(ctx.userId), approvalsNeedingMe(ctx), + unreadFeed(ctx), ]) - return { messaging, approvals } + return { messaging, approvals, feed } } diff --git a/apps/web/src/lib/retention-register.test.ts b/apps/web/src/lib/retention-register.test.ts index fc363bc2..0fcdb16f 100644 --- a/apps/web/src/lib/retention-register.test.ts +++ b/apps/web/src/lib/retention-register.test.ts @@ -296,6 +296,12 @@ const REGISTER: Record = { "Organization", "A post on the club feed, written by a named student.", ), + FeedVisit: owedVia( + "Institution", + "When one person last opened the community feed. A single timestamp per reader, " + + "carrying nothing anybody wrote — it exists so a badge can count what is new " + + "rather than guess. Deleting it costs the reader one stale badge.", + ), FeedComment: owedVia( "FeedPost", "A comment on a post, written by a named student. Free text about people.", diff --git a/apps/web/src/lib/tenancy/registry.ts b/apps/web/src/lib/tenancy/registry.ts index 8884ad62..8b4d25be 100644 --- a/apps/web/src/lib/tenancy/registry.ts +++ b/apps/web/src/lib/tenancy/registry.ts @@ -127,6 +127,7 @@ export const TENANT_SCOPED = [ "Budget", "Vendor", "FeedPost", + "FeedVisit", "MemoryMovement", "MemoryHandoff", "SuccessionBriefing", From 7af985aeab3b3492118927de14e14d11901f7caf Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 06:22:32 -0400 Subject: [PATCH 2/2] A post arriving mid-read was marked seen and never shown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cutoff was stamped AFTER `findMany` returned. A post created between the query and the stamp is not in the response — so it is never rendered — and its `createdAt` is before `lastSeenAt`, so the badge excludes it too. Invisible in both places, permanently, with nothing anywhere reporting it. The window is small and the loss is not: that post is simply gone for that reader. Taking the instant BEFORE the read closes it in the safe direction. A post arriving during the read is now after the watermark, so it stays unread and appears in the next badge — one visit late rather than never. The write still happens after the read succeeds, so a failed read still marks nothing as seen. Both halves matter, and fixing the capture by also moving the WRITE earlier would have traded this defect for that one. ── AND THE TEST THAT BROKE WAS THE LESSON AGAIN ──────────────────────────── `cannot take the page down when the write fails` asserted that `try {` appeared within 400 characters of `const seenAt = new Date()`. Moving that line to close the race broke the assertion while the property it names — the upsert is inside a try/catch — stayed true the whole time. A window is not a scope. It now finds the `try` that actually encloses the write, and checks nothing closes the block in between. That is the third time today a check has read the right file and asserted the wrong relation, and the second time in this file. Found by CodeRabbit on #310. Co-Authored-By: Claude Opus 5 (1M context) --- apps/web/src/app/(app)/feed/page.tsx | 20 ++++++++- ...e-badge-counts-what-the-page-shows.test.ts | 44 +++++++++++++++++-- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/apps/web/src/app/(app)/feed/page.tsx b/apps/web/src/app/(app)/feed/page.tsx index 61d66d92..b7828ea5 100644 --- a/apps/web/src/app/(app)/feed/page.tsx +++ b/apps/web/src/app/(app)/feed/page.tsx @@ -67,6 +67,23 @@ export default async function FeedPage() { * statement is `await getUserContext(userId)`, and that call is `cache()`d, * so it already awaits the very promise it would be racing. */ + /* + * THE CUTOFF IS TAKEN BEFORE THE READ, AND SAVED AFTER IT. + * + * It used to be stamped after `findMany` returned, which opens a window: a + * post created between the query and the stamp is not in the response — so + * it is never rendered — and its `createdAt` is before `lastSeenAt`, so the + * badge excludes it too. That post becomes invisible in both places, for + * good, and nothing anywhere reports it. + * + * Taking the instant first closes the window in the safe direction. A post + * arriving during the read is now AFTER the watermark, so it stays unread + * and shows up in the next badge — the reader sees it one visit late rather + * than never. The write still happens after the read succeeds, so a failed + * read still marks nothing as seen. + */ + const seenAt = new Date() + const [posts, myClubs, myEvents] = await Promise.all([ db.feedPost.findMany({ where: { institutionId: { in: institutionIds }, isArchived: false }, @@ -120,8 +137,9 @@ export default async function FeedPage() { * fire-and-forget against the reader's own row, and a failure here must not * fail the page. The worst case is a badge that stays up until the next * visit, which is exactly the failure a reader can recover from themselves. + * + * `seenAt` was captured BEFORE the posts were read — see the note there. */ - const seenAt = new Date() try { await Promise.all( institutionIds.map((institutionId) => diff --git a/apps/web/src/lib/feed/the-badge-counts-what-the-page-shows.test.ts b/apps/web/src/lib/feed/the-badge-counts-what-the-page-shows.test.ts index 77691ef8..a027ee4b 100644 --- a/apps/web/src/lib/feed/the-badge-counts-what-the-page-shows.test.ts +++ b/apps/web/src/lib/feed/the-badge-counts-what-the-page-shows.test.ts @@ -84,6 +84,29 @@ describe("opening the feed records that you opened it", () => { ) }) + it("CAPTURES the cutoff BEFORE the posts are read", () => { + /* + * The race, and it loses a post permanently rather than briefly. + * + * Stamping `new Date()` after `findMany` returns opens a window: a post + * created between the query and the stamp is not in the response — so it is + * never rendered — and its `createdAt` is before `lastSeenAt`, so the badge + * excludes it as well. Invisible in both places, for good, with nothing + * anywhere reporting it. + * + * Taking the instant first fails in the safe direction: a post arriving + * during the read is AFTER the watermark, so it stays unread and appears in + * the next badge. One visit late rather than never. + * + * Both halves are asserted, because fixing the capture by ALSO moving the + * write earlier would reintroduce the other defect — a failed read marking + * everything seen. + */ + expect(page.indexOf("const seenAt = new Date()")).toBeLessThan( + page.indexOf("db.feedPost.findMany") + ) + }) + it("cannot take the page down when the write fails", () => { /* * A try/catch and not `.catch()` on the promise. The first version chained @@ -91,8 +114,21 @@ describe("opening the feed records that you opened it", () => { * synchronous throw on the way to calling it — and the property access is * the part most likely to fail. The page's own suite proved it by reaching * `db.feedVisit` before the fake had one. + * + * Asserted as CONTAINMENT, not as "a `try` appears near the write". The + * first version of this test sliced 400 characters from `const seenAt = + * new Date()` — and when that line moved above the post query to close a + * race, the assertion broke while the property it names was still true. A + * window is not a scope; this finds the `try` that actually encloses the + * upsert. */ - const write = page.slice(page.indexOf("const seenAt = new Date()")) - expect(write.slice(0, 400)).toContain("try {") - }) -}) + const at = page.indexOf("db.feedVisit.upsert") + expect(at).toBeGreaterThan(-1) + + const opened = page.lastIndexOf("try {", at) + const caught = page.indexOf("} catch", at) + expect(opened).toBeGreaterThan(-1) + expect(caught).toBeGreaterThan(at) + // Nothing closes the block between the `try` and the write. + expect(page.slice(opened, at)).not.toContain("} catch") + })})