From 99244e2d185e4c62695e236504dda438f56eb410 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 22:15:09 +0000 Subject: [PATCH 1/2] perf(app-shell): scope the inbox receipt read to the listed messages (objectui#7392) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inbox poll re-read `sys_notification_receipt` in full on every tick — filtered by `user_id` + `channel:'inbox'`, `$top: 200`, no cursor — beside the `sys_inbox_message` read's `$top: 20`, at 10s foregrounded and 60s hidden. `mergeInboxRows` uses the receipts as a lookup keyed by `notification_id` while mapping over the MESSAGE rows, so every receipt outside that window of 20 was fetched, indexed and dropped, six times a minute. The read now names those messages' notification ids as an `$in` comparand and is skipped outright when the window lists nothing a receipt could belong to. `$top` becomes the id count, which is an exact bound rather than a guessed headroom: the object declares its key `{ fields: ['notification_id', 'user_id', 'channel'], unique: true }`, so at most one row per named id can match. No number the user sees moves. The bell badge is `unreadTopics + pendingApprovalsCount` and `unreadTopics` folds this feed's rows, which are one per listed message — bounded by the message read's `$top: 20` since #4225 gave the bell and Home one feed, never by the receipt set. Home's `unreadTopicCount` folds the same rows the same way (#4329). The receipts only ever supplied read-state to rows the message query had already chosen. The two reads are sequential where they were a `Promise.all`: the receipt query cannot be written until the message read says which notifications are in the window. One extra round trip per tick on a background poll, against ~180 discarded rows saved on each of them. The #7249 cadence pin goes red on this change and that red is correct: it asserts the receipt read is exactly as frequent as the message read, on a fixture whose inbox was empty — a window where no receipt read is now due. Its fixture now lists rows so the equality is exercised where it means something, the equality itself is unchanged, and the empty window is pinned separately. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- .changeset/inbox-receipt-window-scope.md | 18 ++ .../sharedInboxFeed.cadence-7249.test.tsx | 71 +++- ...sharedInboxFeed.receiptScope-7392.test.tsx | 306 ++++++++++++++++++ .../app-shell/src/hooks/sharedUserFeeds.ts | 100 ++++-- 4 files changed, 473 insertions(+), 22 deletions(-) create mode 100644 .changeset/inbox-receipt-window-scope.md create mode 100644 packages/app-shell/src/hooks/__tests__/sharedInboxFeed.receiptScope-7392.test.tsx diff --git a/.changeset/inbox-receipt-window-scope.md b/.changeset/inbox-receipt-window-scope.md new file mode 100644 index 0000000000..543f3046db --- /dev/null +++ b/.changeset/inbox-receipt-window-scope.md @@ -0,0 +1,18 @@ +--- +'@object-ui/app-shell': patch +--- + +inbox poll: read the receipts of the listed messages, not the user's whole receipt history + +Every inbox tick re-read `sys_notification_receipt` in full — filtered by +`user_id` + `channel:'inbox'`, `$top: 200`, no cursor — beside the +`sys_inbox_message` read's `$top: 20`, at 10s foregrounded and 60s hidden. The +join only ever used the receipts of the messages that read listed, so the rest +were fetched, indexed and dropped on every tick. + +The receipt read now names those messages' notification ids (`$in`) and is +skipped entirely when the window lists nothing a receipt could belong to. No +number the user sees moves: the bell badge's inbox addend folds the rows this +feed produces, which are one per listed message, so it was never derived from +the receipt set's size. Ceiling and answer are now the same size, which the +unique `(notification_id, user_id, channel)` key makes exact. diff --git a/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.cadence-7249.test.tsx b/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.cadence-7249.test.tsx index 805c80a649..9d65b6b159 100644 --- a/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.cadence-7249.test.tsx +++ b/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.cadence-7249.test.tsx @@ -61,6 +61,21 @@ * - give `useHomeInbox` a second scheduler of its own and * "one feed's cadence, not one per consumer" goes RED while the single- * consumer case stays green — a second scheduler is not a faster one. + * + * ## Later — objectui#7392 narrowed that receipt read's PAYLOAD + * + * The `top=200` receipt read named above is the shape #7249 measured, and is + * left standing as the record of that measurement. objectui#7392 replaced it + * with a read scoped to the notification ids the message read just listed, and + * with that made it CONDITIONAL: a window listing nothing joinable issues no + * receipt read at all. + * + * The subject of this file is the CADENCE, which that change does not touch — + * but its fixture now lists rows, because "the receipt read is exactly as + * frequent as the message read" is not assertable on a window where no receipt + * read is due. The equality is unchanged; what changed is that it is now + * exercised on a window that has something to join, with the empty window + * pinned separately below. */ import '@testing-library/jest-dom/vitest'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; @@ -72,10 +87,42 @@ vi.mock('@object-ui/auth', async (importOriginal) => ({ useAuth: () => ({ user: userFixture }), })); +/** + * The `mine` window this fixture lists — two messages, each carrying the + * `notification_id` its receipt is keyed by (ADR-0030). + * + * Rows rather than an empty array because of objectui#7392: the receipt read + * is now issued only when the window lists something a receipt could belong + * to, so an empty inbox is the one window where counting receipt reads against + * message reads asserts nothing. + */ +const INBOX_ROWS: Array> = [ + { + id: 'ibx_1', + user_id: 'u1', + notification_id: 'ntf_1', + topic: 'collab.assignment', + title: 'Assigned to you: Ship it', + created_at: '2026-09-02T09:00:00Z', + }, + { + id: 'ibx_2', + user_id: 'u1', + notification_id: 'ntf_2', + topic: 'approval.request', + title: 'Approval reminder: INV-1008', + created_at: '2026-09-01T09:00:00Z', + }, +]; + +/** What the fake inbox answers with — emptied by the case that pins that path. */ +let inboxRows: Array> = INBOX_ROWS; + const findCalls: Array<{ object: string }> = []; const fakeAdapter = { find: (object: string) => { findCalls.push({ object }); + if (object === 'sys_inbox_message') return Promise.resolve({ data: inboxRows }); return Promise.resolve({ data: [] }); }, getClient: () => undefined, @@ -117,6 +164,7 @@ function setHidden(hidden: boolean): void { beforeEach(() => { vi.useFakeTimers(); userFixture = { id: 'u1' }; + inboxRows = INBOX_ROWS; findCalls.length = 0; setHidden(false); // Module-scoped stores outlive any one render tree. @@ -149,10 +197,31 @@ describe('objectui#7249 — the inbox feed holds ONE declared cadence', () => { expect(inboxReads()).toBe(READS_IN_WINDOW); // The join travels with the rows: the receipt read is issued by the same // runner, so it can neither lag the message read nor double it. The card's - // `top=200` receipt read is exactly as frequent as the pair, no more. + // receipt read is exactly as frequent as the pair, no more — the claim is + // the same one #7249 made, now asserted on a window that lists something a + // receipt can belong to (objectui#7392 made the read conditional on that, + // and the case below is where the empty window is pinned). expect(receiptReads()).toBe(inboxReads()); }); + it('issues NO receipt read on a window with nothing to join (objectui#7392)', async () => { + // The other half of the equality above. The receipt read exists to decorate + // the listed messages with read-state; with no messages listed there is + // nothing it could decorate, and every row it would return is one + // `mergeInboxRows` drops. So the correct count here is zero, per tick, + // forever — not "as frequent as the message read". + inboxRows = []; + + renderHook(() => useSharedInboxFeed()); + await settle(); + await advance(WINDOW_MS); + + // Same window, same cadence — the message read is untouched … + expect(inboxReads()).toBe(READS_IN_WINDOW); + // … and not one receipt read went out behind it. + expect(receiptReads()).toBe(0); + }); + it('is one FEED cadence, not one per consumer', async () => { // Three consumers on one page must cost what one costs. This is the claim // the mount-time dedupe pins cannot make: they stop at the first tick, diff --git a/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.receiptScope-7392.test.tsx b/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.receiptScope-7392.test.tsx new file mode 100644 index 0000000000..3fd2457c2c --- /dev/null +++ b/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.receiptScope-7392.test.tsx @@ -0,0 +1,306 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#7392 — the inbox tick asks for the receipts it can USE, not the + * user's whole receipt history. + * + * ## What was measured, and what it cost + * + * Every inbox tick issued `sys_notification_receipt` filtered by `user_id` + + * `channel:'inbox'` with `$top: 200` and no cursor of any kind, beside the + * `sys_inbox_message` read's `$top: 20` — at 10 s foregrounded, 60 s hidden. + * `mergeInboxRows` then used the receipts as a lookup table keyed by + * `notification_id` while mapping over the MESSAGE rows, so a receipt outside + * that window of 20 was fetched, indexed, and dropped. Two orders of magnitude + * of payload, re-fetched in full six times a minute, to decorate at most 20 + * rows. + * + * ## The one way this could have gone silently wrong — and why it does not + * + * The receipt set feeds the bell's badge, and a receipt is not a "read" marker: + * `delivered` is a receipt and is NOT read (`READ_STATES`). So "read only the + * receipts of the listed messages" invites the failure of turning an unread + * TOTAL into "unread among the newest 20" with no error and no red test. + * + * It does not, and the reason is upstream of this change. The badge is + * `unreadTopics + pendingApprovalsCount` (`InboxPopover`), and `unreadTopics` + * folds the rows this feed produces — `mergeInboxRows`' output, which is one + * row per `sys_inbox_message` row and therefore bounded by that read's + * `$top: 20`, never by the receipt set. Home's `unreadTopicCount` (#4329) + * folds the same rows with the same function. The inbox addend has been + * window-sized since #4225 gave both surfaces one feed; the receipts only ever + * supplied read-state to rows the MESSAGE query had already chosen. Narrowing + * them to those same rows' ids therefore changes no number — which is what the + * `badge` case below asserts against a fixture holding 180 receipts that the + * narrowing drops. + * + * ## Why the fake backend honours the filter + * + * A fake that ignores `$filter` answers every query with the same rows, so the + * "after" row count would echo the fixture rather than measure the read. This + * one applies `user_id`, `channel` and `$in` and then `$top`, so the number of + * rows a tick delivers is a reading of the query the feed actually wrote. + * + * ## Reverse verification (direction predicted BEFORE running, measured in the PR) + * + * - restore the old read (`{ user_id, channel }`, `$top: 200`, unconditional) + * and the three payload cases go RED — the delivered row count returns to + * the whole history and the `$in` comparand is gone — while the `badge` and + * `read-state` cases stay GREEN. That asymmetry is the whole finding: the + * old read was wider, not more correct. + * - drop the `notification_id` clause but keep `$top: ids.length` and the + * payload count case still goes RED with a different number, so the case is + * reading the FILTER and not just the ceiling. + */ +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { act, renderHook } from '@testing-library/react'; + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1' } }), +})); + +/** The `mine` window the message read lists — `$top: 20`, newest first. */ +const WINDOW_SIZE = 20; +/** How many notifications this user has accumulated receipts for, all time. */ +const HISTORY_SIZE = 200; +/** Receipt states that count as READ — the feed's `READ_STATES`, restated. */ +const READ_STATES = new Set(['read', 'clicked', 'dismissed']); + +const notifId = (i: number) => `ntf_${String(i).padStart(3, '0')}`; + +/** + * The newest `WINDOW_SIZE` messages. `notification_id` descends with + * `created_at`, so the window is `ntf_001` … `ntf_020`. + */ +const WINDOW_ROWS: Array> = Array.from({ length: WINDOW_SIZE }, (_, i) => ({ + id: `ibx_${i + 1}`, + user_id: 'u1', + notification_id: notifId(i + 1), + topic: i % 2 === 0 ? 'collab.assignment' : 'approval.request', + title: `Message ${i + 1}`, + created_at: `2026-09-${String(WINDOW_SIZE - i).padStart(2, '0')}T09:00:00Z`, +})); + +/** + * Every receipt this user holds on the inbox channel — one per notification, + * as the object's `{ fields: ['notification_id', 'user_id', 'channel'], unique: + * true }` key allows. Every third is READ; the rest are `delivered`, which is a + * receipt that is NOT read. + * + * Rows for another user and for another channel sit alongside them so the two + * equality clauses of the filter stay load-bearing: a query that dropped them + * would over-answer, and the row counts below would say so. + */ +const RECEIPT_STORE: Array> = [ + ...Array.from({ length: HISTORY_SIZE }, (_, i) => ({ + id: `rcp_${i + 1}`, + notification_id: notifId(i + 1), + user_id: 'u1', + channel: 'inbox', + state: (i + 1) % 3 === 0 ? 'read' : 'delivered', + })), + { id: 'rcp_other_user', notification_id: notifId(1), user_id: 'u2', channel: 'inbox', state: 'read' }, + { id: 'rcp_other_chan', notification_id: notifId(2), user_id: 'u1', channel: 'email', state: 'read' }, +]; + +/** Which window notifications the store says are read — the expected answer. */ +const READ_IN_WINDOW = new Set( + RECEIPT_STORE.filter( + (r) => + r.user_id === 'u1' && + r.channel === 'inbox' && + READ_STATES.has(String(r.state)) && + WINDOW_ROWS.some((m) => m.notification_id === r.notification_id), + ).map((r) => String(r.notification_id)), +); + +interface ReceiptQuery { + $filter?: Record; + $top?: number; +} + +/** Every `sys_notification_receipt` read, with the rows it answered with. */ +const receiptReads: Array<{ query: ReceiptQuery; rowsDelivered: number }> = []; +let inboxRows: Array> = WINDOW_ROWS; + +/** + * Applies the filter the way the data layer does: equality for a scalar + * comparand, membership for `{ $in: [...] }` (`convertFiltersToAST` lowers that + * spelling to `in`), then `$top` as a ceiling. + */ +function answerReceipts(query: ReceiptQuery): Array> { + const filter = query.$filter ?? {}; + const matched = RECEIPT_STORE.filter((row) => + Object.entries(filter).every(([field, comparand]) => { + const actual = row[field]; + if (comparand && typeof comparand === 'object' && '$in' in comparand) { + const members = (comparand as { $in: unknown }).$in; + return Array.isArray(members) && members.some((m) => m === actual); + } + return comparand === actual; + }), + ); + return typeof query.$top === 'number' ? matched.slice(0, query.$top) : matched; +} + +const fakeAdapter = { + find: (object: string, query: unknown) => { + if (object === 'sys_inbox_message') return Promise.resolve({ data: inboxRows }); + if (object === 'sys_notification_receipt') { + const data = answerReceipts((query ?? {}) as ReceiptQuery); + receiptReads.push({ query: (query ?? {}) as ReceiptQuery, rowsDelivered: data.length }); + return Promise.resolve({ data }); + } + return Promise.resolve({ data: [] }); + }, + getClient: () => undefined, +}; + +vi.mock('../../providers/AdapterProvider', () => ({ useAdapter: () => fakeAdapter })); + +import { useSharedInboxFeed, __resetSharedUserFeeds } from '../sharedUserFeeds'; +import { groupNotifications } from '../../layout/inboxGrouping'; + +const settle = () => act(async () => { await vi.advanceTimersByTimeAsync(0); }); + +/** Mount the feed and let its first tick complete. */ +async function firstTick() { + const view = renderHook(() => useSharedInboxFeed()); + await settle(); + return view; +} + +beforeEach(() => { + vi.useFakeTimers(); + receiptReads.length = 0; + inboxRows = WINDOW_ROWS; + __resetSharedUserFeeds(); + // Approvals degrade to 0 (404): the inbox is this suite's only subject. + vi.stubGlobal('fetch', vi.fn(() => Promise.resolve(new Response('{}', { status: 404 })))); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.useRealTimers(); +}); + +describe('objectui#7392 — the receipt read is scoped to the listed messages', () => { + it('names the window\'s notification ids as the comparand, and bounds $top by them', async () => { + await firstTick(); + + expect(receiptReads).toHaveLength(1); + const { query } = receiptReads[0]; + expect(query.$filter).toEqual({ + user_id: 'u1', + channel: 'inbox', + notification_id: { $in: WINDOW_ROWS.map((m) => m.notification_id) }, + }); + // The key `(notification_id, user_id, channel)` is unique, so the ids the + // filter names ARE the ceiling — an exact bound, not a guessed headroom. + expect(query.$top).toBe(WINDOW_SIZE); + expect(query.$top).not.toBe(HISTORY_SIZE); + }); + + it('delivers the window\'s receipts instead of the whole history — the card\'s closing criterion', async () => { + await firstTick(); + + // What the read this replaces asked for, measured on the same fixture: its + // filter was `user_id` + `channel` alone, ceilinged at 200. + const wouldHaveDelivered = answerReceipts({ + $filter: { user_id: 'u1', channel: 'inbox' }, + $top: HISTORY_SIZE, + }).length; + expect(wouldHaveDelivered).toBe(HISTORY_SIZE); + + // What this one delivers. The gap is the finding; the control above is + // what stops a shrunken fixture from reading as a narrowed query. + expect(receiptReads[0].rowsDelivered).toBe(WINDOW_SIZE); + }); + + it('keeps every read-state the old, wider read carried', async () => { + const { result } = await firstTick(); + + const rows = result.current.value; + expect(rows).toHaveLength(WINDOW_SIZE); + // Not "some row is read": the whole window, row by row, against the store. + expect(rows.map((r) => `${r.notification_id}:${r.is_read}`)).toEqual( + WINDOW_ROWS.map( + (m) => `${m.notification_id}:${READ_IN_WINDOW.has(String(m.notification_id))}`, + ), + ); + // The fixture must contain both answers, or the assertion above is vacuous. + expect(READ_IN_WINDOW.size).toBeGreaterThan(0); + expect(READ_IN_WINDOW.size).toBeLessThan(WINDOW_SIZE); + }); + + it('leaves the badge\'s inbox addend exactly where it was', async () => { + const { result } = await firstTick(); + + // `unreadTopics` as `InboxPopover` computes it — the badge's first addend, + // and the same fold `useHomeInbox.unreadTopicCount` applies (#4329). + const unreadTopics = groupNotifications(result.current.value).reduce( + (n, g) => n + (g.unreadCount > 0 ? 1 : 0), + 0, + ); + const expectedTopics = groupNotifications( + WINDOW_ROWS.map((m) => ({ + id: String(m.id), + notification_id: String(m.notification_id), + type: String(m.topic), + title: String(m.title), + is_read: READ_IN_WINDOW.has(String(m.notification_id)), + created_at: String(m.created_at), + })), + ).reduce((n, g) => n + (g.unreadCount > 0 ? 1 : 0), 0); + + expect(unreadTopics).toBe(expectedTopics); + // And the receipts the narrowing dropped could not have moved it: none of + // them belongs to a message this window lists. + const dropped = HISTORY_SIZE - receiptReads[0].rowsDelivered; + expect(dropped).toBe(HISTORY_SIZE - WINDOW_SIZE); + expect( + RECEIPT_STORE.filter( + (r) => + r.user_id === 'u1' && + r.channel === 'inbox' && + !WINDOW_ROWS.some((m) => m.notification_id === r.notification_id), + ), + ).toHaveLength(dropped); + }); + + it('asks once per notification when two messages share one notification event', async () => { + inboxRows = [ + { id: 'ibx_a', user_id: 'u1', notification_id: notifId(1), topic: 't', title: 'A', created_at: '2026-09-20T09:00:00Z' }, + { id: 'ibx_b', user_id: 'u1', notification_id: notifId(1), topic: 't', title: 'B', created_at: '2026-09-19T09:00:00Z' }, + ]; + + await firstTick(); + + // The ids are a query comparand, not a count: a repeat would widen the + // request's text for an answer the unique key already bounds at one row. + expect(receiptReads[0].query.$filter?.notification_id).toEqual({ $in: [notifId(1)] }); + expect(receiptReads[0].query.$top).toBe(1); + }); + + it('issues no receipt read when the window lists nothing joinable', async () => { + // Two ways to have nothing to join, both real: an empty inbox, and rows + // that carry no `notification_id` (legacy / synthetic — never receipted, + // therefore always unread, which `mergeInboxRows` already answers alone). + inboxRows = []; + await firstTick(); + expect(receiptReads).toHaveLength(0); + + __resetSharedUserFeeds(); + inboxRows = [ + { id: 'ibx_legacy', user_id: 'u1', topic: 'legacy', title: 'No event id', created_at: '2026-09-20T09:00:00Z' }, + ]; + const { result } = await firstTick(); + + expect(receiptReads).toHaveLength(0); + // …and the row still arrives, unread, rather than being dropped with the read. + expect(result.current.value.map((r) => r.is_read)).toEqual([false]); + }); +}); diff --git a/packages/app-shell/src/hooks/sharedUserFeeds.ts b/packages/app-shell/src/hooks/sharedUserFeeds.ts index 3acb9dee18..0f393bf965 100644 --- a/packages/app-shell/src/hooks/sharedUserFeeds.ts +++ b/packages/app-shell/src/hooks/sharedUserFeeds.ts @@ -741,6 +741,31 @@ function mergeInboxRows(rows: unknown[], receipts: unknown[]): InboxNotification }); } +/** + * The notification ids of the listed messages — the only receipts this feed + * can USE, which is why they are the only ones it now asks for (objectui#7392). + * + * {@link mergeInboxRows} maps over the MESSAGE rows and looks each one's + * receipt up by `notification_id`; a receipt belonging to anything else is + * fetched and dropped on the floor. Asking for exactly these ids narrows the + * PAYLOAD, not the answer. + * + * De-duplicated, because the ids are a query comparand rather than a count. + * Rows with a blank or absent `notification_id` are skipped: read-state is + * keyed by that id, so such a row can never be receipted and is always unread + * — the same rule the merge applies on the other side of the join. + */ +function listedNotificationIds(rows: readonly unknown[]): string[] { + const ids = new Set(); + for (const raw of rows) { + const nid = (raw as Record | null)?.notification_id; + if (nid == null) continue; + const id = String(nid); + if (id) ids.add(id); + } + return [...ids]; +} + /** * The signed-in user's 20 most recent in-app inbox messages, joined with their * read-state receipts (ADR-0030 L5, the `mine` materialization). @@ -748,9 +773,35 @@ function mergeInboxRows(rows: unknown[], receipts: unknown[]): InboxNotification * Two scoped reads, joined client-side, polled at 10s while the tab is * foregrounded — the bell's cadence, now the store's: * - `sys_inbox_message` filtered by `user_id`, newest first, `$top: 20`. - * - `sys_notification_receipt` filtered by `user_id` + `channel:'inbox'`. - * Best-effort: if receipts are unavailable the inbox still renders - * (everything shows unread) rather than erroring. + * - `sys_notification_receipt` filtered by `user_id` + `channel:'inbox'`, + * narrowed to the notification ids that message read just listed + * (objectui#7392). Best-effort: if receipts are unavailable the inbox + * still renders (everything shows unread) rather than erroring. + * + * The receipt read used to ask for `$top: 200` of the user's receipts whatever + * was listed, and `mergeInboxRows` then dropped every row that did not belong + * to one of the 20 — a steady-state payload two orders of magnitude wider than + * the answer it decorated, re-fetched in full at the 10s foreground cadence. + * Its `$top` is now the id count, and that bound is EXACT rather than a guess: + * `sys_notification_receipt` declares its key `{ fields: ['notification_id', + * 'user_id', 'channel'], unique: true }`, so this filter can match at most one + * row per id it names. + * + * ⚠️ The two reads are SEQUENTIAL where they used to be a `Promise.all`: the + * receipt query cannot be written until the message read says which + * notifications are in the window. That is one extra round trip per tick, on a + * background poll, against ~200 rows saved on each of them. + * + * ⛔ What this does NOT change is the bell's unread number, and that is the + * one thing a narrowing here could have broken silently. The badge is + * `unreadTopics + pendingApprovalsCount` (`InboxPopover`), and `unreadTopics` + * folds THIS feed's rows — which are `mergeInboxRows`' output, one per + * `sys_inbox_message` row. So the inbox addend has been "unread within the + * `$top: 20` window" since #4225 gave the two surfaces one feed; the receipt + * set never contributed a row to it, only a read-state to rows the message + * query had already chosen. Home's `unreadTopicCount` folds the same rows the + * same way (#4329). A receipt outside the window changed no number before this + * change and changes none after it. * * This is the SUPERSET both consumers cut from. The bell lists all 20 and * badges the unread topics; Home's action centre takes the unread ones, newest @@ -775,25 +826,32 @@ export function useSharedInboxFeed(): SharedFeedSnapshot { return useSharedFeed(inboxFeed, key, async ({ markUnavailable, markFailed }) => { if (!dataSource || !userId) return undefined; try { - const [inboxRes, receiptRes] = await Promise.all([ - Promise.resolve( - dataSource.find('sys_inbox_message', { - $filter: { user_id: userId }, - $orderby: { created_at: 'desc' }, - $top: 20, - }) as Promise<{ data?: unknown[] }>, - ), - Promise.resolve( - dataSource.find('sys_notification_receipt', { - $filter: { user_id: userId, channel: 'inbox' }, - $top: 200, - }) as Promise<{ data?: unknown[] }>, - ).catch(() => ({ data: [] as unknown[] })), - ]); - return mergeInboxRows( - Array.isArray(inboxRes?.data) ? inboxRes.data : [], - Array.isArray(receiptRes?.data) ? receiptRes.data : [], + const inboxRes = await Promise.resolve( + dataSource.find('sys_inbox_message', { + $filter: { user_id: userId }, + $orderby: { created_at: 'desc' }, + $top: 20, + }) as Promise<{ data?: unknown[] }>, ); + const rows = Array.isArray(inboxRes?.data) ? inboxRes.data : []; + const notificationIds = listedNotificationIds(rows); + // Nothing in this window can carry a receipt ⇒ no receipt read at all. + // Every row the old query would have returned here is one the merge + // discards, so the cheapest correct request is the one not sent. + const receiptRes = + notificationIds.length === 0 + ? { data: [] as unknown[] } + : await Promise.resolve( + dataSource.find('sys_notification_receipt', { + $filter: { + user_id: userId, + channel: 'inbox', + notification_id: { $in: notificationIds }, + }, + $top: notificationIds.length, + }) as Promise<{ data?: unknown[] }>, + ).catch(() => ({ data: [] as unknown[] })); + return mergeInboxRows(rows, Array.isArray(receiptRes?.data) ? receiptRes.data : []); } catch (err: unknown) { // No inbox object ⇒ no messaging pipeline in this deployment, so nothing // is waiting: an answer. A denial / outage / malformed reply is not. From 46f693d7ae340860551dff83bd2300124f28e721 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 22:34:33 +0000 Subject: [PATCH 2/2] test(app-shell): record the MEASURED reverse verification, not the prediction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pin shipped with the direction it predicted. Running it turned in a different count — 6 failed, 5 passed across this file and the #7249 pin, not the three-red/two-green that was written down — and the gap is the informative part: the `badge` case's `unreadTopics` equality passed under the old, wider read too, so what went red there is the row arithmetic beside it, not the number a narrowing could have moved. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- ...sharedInboxFeed.receiptScope-7392.test.tsx | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.receiptScope-7392.test.tsx b/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.receiptScope-7392.test.tsx index 3fd2457c2c..46bfa85f54 100644 --- a/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.receiptScope-7392.test.tsx +++ b/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.receiptScope-7392.test.tsx @@ -42,16 +42,28 @@ * one applies `user_id`, `channel` and `$in` and then `$top`, so the number of * rows a tick delivers is a reading of the query the feed actually wrote. * - * ## Reverse verification (direction predicted BEFORE running, measured in the PR) + * ## Reverse verification — direction predicted first, then MEASURED * - * - restore the old read (`{ user_id, channel }`, `$top: 200`, unconditional) - * and the three payload cases go RED — the delivered row count returns to - * the whole history and the `$in` comparand is gone — while the `badge` and - * `read-state` cases stay GREEN. That asymmetry is the whole finding: the - * old read was wider, not more correct. - * - drop the `notification_id` clause but keep `$top: ids.length` and the - * payload count case still goes RED with a different number, so the case is - * reading the FILTER and not just the ceiling. + * Predicted: restoring the old read (`{ user_id, channel }`, `$top: 200`, + * unconditional) turns the payload cases RED and leaves `badge` and + * `read-state` GREEN. Measured, with that read put back on the committed fix + * and the tree restored afterwards by blob hash: **6 failed, 5 passed** across + * this file and the #7249 cadence pin. The prediction was right about the + * direction and wrong about the count, and the gap is the interesting part: + * + * - `read-state` stayed GREEN — the only case in this file that did. The old + * read carried the same answer; it carried more rows to get there. That is + * the behaviour-preservation half, and it holds under BOTH reads. + * - `badge` went red, but NOT on the badge. Its `unreadTopics` equality — the + * assertion that would catch an unread total quietly becoming a window — is + * evaluated first and passed under the old read too. What failed after it + * is the row arithmetic beside it (`expected +0 to be 180`: nothing was + * dropped, because nothing was narrowed). ⇒ the number is invariant and the + * payload is not, asserted in one case, which is why the two live together. + * - the three payload cases and the new empty-window case in + * `sharedInboxFeed.cadence-7249` went red as predicted, the last of them + * with `expected 7 to be +0` — one receipt read per tick for a window with + * nothing to join. */ import '@testing-library/jest-dom/vitest'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';