fix(services): listInbox 的 unreadCount 数总未读,不再只数取回窗口 (#6363)

.changeset/notification-unread-count-true-total.md

@@ -0,0 +1,44 @@
+---
+"@objectstack/service-messaging": patch
+---
+
+fix(services): `unreadCount` counts the TOTAL unread, not the returned window (#6363)
+
+`ListNotificationsResponseSchema.unreadCount` is published into the API
+reference as **"Total number of unread notifications"** — a `.describe()`, so it
+is the documentation shipped to every consumer of
+`GET /api/v1/notifications`. It was counted inside `rows.map(...)` in
+`MessagingService.listInbox`, i.e. over the rows that `limit` had already
+truncated, so the badge saturated at the window size forever.
+
+Measured on a real stack (sqlite-wasm + ObjectQL + service-messaging + hono +
+dispatcher) with 60 unread messages:
+
+| request | `notifications[]` | `unreadCount` (before) | `unreadCount` (after) |
+|:---|---:|---:|---:|
+| no `limit` | 50 | **50** | **60** |
+| `?limit=10` | 10 | **10** | **60** |
+
+The declaration was right and the implementation was wrong, so the
+implementation moved (maintainer ruling, 2026-08-07). Every consumer that
+renders `unreadCount` as a bell badge now gets the number it asked for; nothing
+had to learn an implementation detail to read the field correctly.
+
+**The list itself is unchanged.** `notifications[]` is still the window —
+`limit` rows, default 50, hard cap 200, newest first. The two bounds were
+conflated, not shared.
+
+Read-state lives on `sys_notification_receipt`, not on the inbox row
+(ADR-0030), so the total is a reverse join rather than a `count()`. It is
+computed only when the window came back **saturated** (`rows.length === limit`)
+— a short window is already the whole matching set, so the common inbox costs
+exactly what it cost before. When the window does saturate, the extra work is
+one projection read of a single column (`notification_id`) under the same
+`where`, no `orderBy` and no `limit`: the same order as the receipt scan
+`listInbox` already performs unconditionally, and exact under a `type` filter
+and for rows carrying no `notification_id`.
+
+Two related behaviours are unchanged and now pinned: a `read` filter narrows
+the list and never the badge (asking for the read half does not mean zero
+unread), and a `type` filter narrows both (the count answers the query that was
+asked).

packages/runtime/src/notification-schema-conformance.integration.test.ts

@@ -216,36 +216,48 @@ describe('[#5792] the notification wire bodies conform to the schemas the catalo
   });
 
   // ═══════════════════════════════════════════════════════════════════════════
-  // Declared, not delivered — recorded, NOT endorsed
+  // The gaps the double assertion cannot see — one now closed, one still open
   // ═══════════════════════════════════════════════════════════════════════════
   //
-  // The two assertions above are 3/3 green for this family. These two facts are
-  // real inconsistencies that BOTH assertions are structurally blind to, and
-  // that is the point worth writing down for #3877's Stage D ratchet:
+  // The two assertions above are 3/3 green for this family. These two facts
+  // were real inconsistencies that BOTH assertions are structurally blind to,
+  // and that is the point worth writing down for #3877's Stage D ratchet:
   //
   //   * `unreadCount` is a `number` whether it counts the total or the window,
   //     so a VALUE assertion cannot see a wrong semantic;
   //   * `cursor` is `optional`, so "no producer ever emits it" is a legal
   //     parse and the KEY assertion (⊆, not =) cannot see it either.
   //
-  // Pinned as the measured behaviour of `origin/main`, with the issues that own
-  // the judgement call. Whichever way #6361 / #6363 are ruled, these two
-  // assertions are the ones that must flip — which is why they are here rather
-  // than left for the next reader to rediscover.
+  // Both were pinned here as the measured behaviour of `origin/main`, on the
+  // note that whichever way #6361 / #6363 were ruled, these assertions are the
+  // ones that must flip. #6363 has been ruled (2026-08-07, Option A: make the
+  // declaration true) and its assertion has flipped — it now pins the fix, over
+  // the wire, which is the only place the whole stack is in play. The `cursor`
+  // half is unchanged: it is one capability's two halves and is being retired
+  // with #6361, so it stays pinned as measured until that lands.
+  //
+  // The Stage D input stands either way, and is if anything sharper now: the
+  // ratchet still cannot see EITHER fact. Both had to be written by hand, and
+  // the fix below would have been just as invisible to it as the defect was.
   describe('[#6361 / #6363] the gaps the double assertion cannot see', () => {
-    it('[#6363] `unreadCount` counts the RETURNED WINDOW, not the total the schema describes', async () => {
+    it('[#6363] `unreadCount` is the TOTAL the schema describes, and survives a smaller window', async () => {
       const all = await getJson(GAP_USER, '/api/v1/notifications');
       expect(all.unreadCount, 'fixture must leave more than one unread for this to mean anything')
         .toBeGreaterThan(1);
 
       const windowed = await getJson(GAP_USER, '/api/v1/notifications?limit=1');
 
+      // The LIST is still windowed — that half never changed.
       expect(windowed.notifications).toHaveLength(1);
-      // Declared: 'Total number of unread notifications'. Delivered: the unread
-      // count within the fetched window. See #6363.
-      expect(windowed.unreadCount).toBe(1);
-      expect(windowed.unreadCount).not.toBe(all.unreadCount);
-      // …and it still parses, which is exactly the blind spot.
+      // The BADGE is not: declared 'Total number of unread notifications', and
+      // now delivered as one. Before #6363 this read `1` — the window's size,
+      // which is what a user with more unread than the page size was told
+      // forever. The parse was green either way; only this assertion can tell.
+      expect(windowed.unreadCount).toBe(all.unreadCount);
+      // Stated as the property rather than only as an equality, so the pin
+      // cannot go quietly vacuous if a future fixture change flattens both
+      // sides: the count MUST be able to exceed the window it came back in.
+      expect(windowed.unreadCount).toBeGreaterThan(windowed.notifications.length);
       expect(ListNotificationsResponseSchema.safeParse(windowed).success).toBe(true);
     });
 

packages/services/service-messaging/src/messaging-service.test.ts

@@ -592,3 +592,204 @@ describe('MessagingService — inbox read API (ADR-0030)', () => {
         expect(await svc.listInbox('')).toEqual({ notifications: [], unreadCount: 0 });
     });
 });
+
+/**
+ * `n` inbox rows for one user, oldest first. `created_at` carries a padded
+ * millisecond index so the fake engine's lexicographic `desc` sort is the real
+ * newest-first order for any `n` — the window tests below all depend on
+ * knowing exactly WHICH rows a truncated window holds.
+ */
+function seedInbox(
+    userId: string,
+    n: number,
+    topicAt: (i: number) => string = () => 'task.assigned',
+): Array<Record<string, unknown>> {
+    return Array.from({ length: n }, (_, i) => ({
+        id: `m${i + 1}`,
+        user_id: userId,
+        notification_id: `n${i + 1}`,
+        topic: topicAt(i),
+        title: `Notification ${i + 1}`,
+        body_md: 'body',
+        created_at: `2026-01-01T00:00:00.${String(i).padStart(3, '0')}Z`,
+    }));
+}
+
+/** An inbox receipt in a read state, for the message `seedInbox` numbered `i`. */
+function readReceipt(userId: string, i: number): Record<string, unknown> {
+    return { id: `r${i}`, notification_id: `n${i}`, user_id: userId, channel: 'inbox', state: 'read' };
+}
+
+/**
+ * Record every `find` an existing engine is asked to run, in order. Wraps the
+ * double rather than declaring another one: the cost claims below are about how
+ * many reads `listInbox` issues, which is only observable at the call site.
+ */
+function recordFinds(engine: any): Array<{ object: string; query: any }> {
+    const calls: Array<{ object: string; query: any }> = [];
+    const real = engine.find.bind(engine);
+    engine.find = async (object: string, query: any = {}) => {
+        calls.push({ object, query });
+        return real(object, query);
+    };
+    return calls;
+}
+
+/**
+ * [#6363] `ListNotificationsResponseSchema.unreadCount` is published into the
+ * API reference as "Total number of unread notifications". It was counted
+ * inside `rows.map(...)`, i.e. over the `limit`-truncated window, so the badge
+ * saturated at the window size forever: measured on a real stack with 60
+ * unread, the route answered `unreadCount: 50` unfiltered and `10` at
+ * `?limit=10`. Maintainer ruling (2026-08-07, Option A): make the declaration
+ * true — count the total — and leave the list itself windowed.
+ *
+ * The fixtures below are the issue's measured shape (60 unread, `limit=10`).
+ */
+describe('[#6363] listInbox — unreadCount is the TOTAL unread, not the fetched window', () => {
+    const logger = silentLogger();
+
+    it('counts every unread message when the window truncates the inbox', async () => {
+        const engine = inboxEngine({ inbox: seedInbox('u1', 60) });
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        // No `limit`: the default clamp windows the LIST at 50 — unchanged.
+        const unfiltered = await svc.listInbox('u1');
+        expect(unfiltered.notifications).toHaveLength(50);
+        expect(unfiltered.unreadCount).toBe(60); // was 50 — the window's size
+
+        // `?limit=10`: the list shrinks with the window, the badge does not.
+        const windowed = await svc.listInbox('u1', { limit: 10 });
+        expect(windowed.notifications).toHaveLength(10);
+        expect(windowed.unreadCount).toBe(60); // was 10 — the window's size
+    });
+
+    it('subtracts read-state across the whole inbox, not just inside the window', async () => {
+        // The 20 read messages are the OLDEST, so none of them is inside a
+        // newest-first `limit=10` window: a window-scoped count cannot see
+        // them, and a total that ignored receipts would answer 60.
+        const engine = inboxEngine({
+            inbox: seedInbox('u1', 60),
+            receipts: Array.from({ length: 20 }, (_, i) => readReceipt('u1', i + 1)),
+        });
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        const res = await svc.listInbox('u1', { limit: 10 });
+        expect(res.notifications).toHaveLength(10);
+        expect(res.notifications.every((n) => n.read === false)).toBe(true); // window is all-unread
+        expect(res.unreadCount).toBe(40);
+    });
+
+    it('counts rows carrying no notification_id — never receipted, so never read', async () => {
+        const inbox = seedInbox('u1', 60);
+        // A synthetic/legacy row with no event id keys no receipt at all.
+        for (const row of inbox.slice(0, 5)) row.notification_id = null;
+        const engine = inboxEngine({ inbox });
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        expect((await svc.listInbox('u1', { limit: 10 })).unreadCount).toBe(60);
+    });
+
+    it('answers the `type` filter it was asked, over the whole inbox', async () => {
+        // 60 messages alternating between two topics ⇒ 30 of each.
+        const engine = inboxEngine({
+            inbox: seedInbox('u1', 60, (i) => (i % 2 === 0 ? 'deal.won' : 'task.assigned')),
+        });
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        const res = await svc.listInbox('u1', { type: 'deal.won', limit: 10 });
+        expect(res.notifications).toHaveLength(10);
+        expect(res.notifications.every((n) => n.type === 'deal.won')).toBe(true);
+        expect(res.unreadCount).toBe(30);
+    });
+
+    it('counts only the addressed user, at any window size', async () => {
+        const engine = inboxEngine({ inbox: [...seedInbox('u1', 60), ...seedInbox('u2', 7)] });
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        expect((await svc.listInbox('u1', { limit: 10 })).unreadCount).toBe(60);
+        expect((await svc.listInbox('u2', { limit: 10 })).unreadCount).toBe(7);
+    });
+
+    it('the `read` filter narrows the list and never the badge', async () => {
+        const engine = inboxEngine({
+            inbox: seedInbox('u1', 60),
+            receipts: Array.from({ length: 20 }, (_, i) => readReceipt('u1', i + 1)),
+        });
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        // Asking for the READ half does not mean the unread badge is zero.
+        const readOnly = await svc.listInbox('u1', { read: true, limit: 10 });
+        expect(readOnly.notifications).toEqual([]); // the newest 10 are all unread
+        expect(readOnly.unreadCount).toBe(40);
+
+        const unreadOnly = await svc.listInbox('u1', { read: false, limit: 10 });
+        expect(unreadOnly.notifications).toHaveLength(10);
+        expect(unreadOnly.unreadCount).toBe(40);
+    });
+
+    it('a window that came back SHORT costs no second read', async () => {
+        // Nothing was truncated, so the window count already IS the total and
+        // the reverse join would re-read what the first `find` returned.
+        const engine = inboxEngine({ inbox: seedInbox('u1', 3) });
+        const calls = recordFinds(engine);
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        const res = await svc.listInbox('u1');
+        expect(res.unreadCount).toBe(3);
+        expect(calls.filter((c) => c.object === 'sys_inbox_message')).toHaveLength(1);
+    });
+
+    it('the total read is one narrow projection, unwindowed, over the same predicate', async () => {
+        const engine = inboxEngine({ inbox: seedInbox('u1', 60) });
+        const calls = recordFinds(engine);
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        await svc.listInbox('u1', { type: 'task.assigned', limit: 10 });
+
+        const inboxReads = calls.filter((c) => c.object === 'sys_inbox_message');
+        expect(inboxReads).toHaveLength(2);
+        const [windowRead, totalRead] = inboxReads;
+        expect(windowRead.query.limit).toBe(10);
+        // Same predicate as the windowed read — the count answers the same
+        // question, minus the window — and one column, so the extra work stays
+        // the same order as the receipt scan `listInbox` already performs.
+        expect(totalRead.query.where).toEqual(windowRead.query.where);
+        expect(totalRead.query.limit).toBeUndefined();
+        expect(totalRead.query.fields).toEqual(['notification_id']);
+    });
+
+    it('a failing total read is NOT swallowed into a window-sized answer', async () => {
+        // The receipt read degrades (different object, may be absent from a
+        // minimal stack); this one re-reads the object whose `find` just
+        // succeeded, so a failure is a data-layer outage — not a licence to
+        // quietly re-tell the window-sized lie.
+        const engine = inboxEngine({ inbox: seedInbox('u1', 60) });
+        const real = engine.find.bind(engine);
+        engine.find = async (object: string, query: any = {}) => {
+            if (object === 'sys_inbox_message' && query.fields) throw new Error('connection lost');
+            return real(object, query);
+        };
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        await expect(svc.listInbox('u1', { limit: 10 })).rejects.toThrow('connection lost');
+    });
+
+    /* ------------------------------------------------------------------ */
+    /*  The other half of the ruling: the LIST window is unchanged.        */
+    /* ------------------------------------------------------------------ */
+
+    it('the list keeps its window: default 50, hard cap 200, floor 1, newest first', async () => {
+        const engine = inboxEngine({ inbox: seedInbox('u1', 250) });
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        const dflt = await svc.listInbox('u1');
+        expect(dflt.notifications).toHaveLength(50);
+        expect(dflt.notifications[0].id).toBe('n250'); // newest first, still
+        expect(dflt.unreadCount).toBe(250);
+
+        expect((await svc.listInbox('u1', { limit: 500 })).notifications).toHaveLength(200);
+        expect((await svc.listInbox('u1', { limit: 0 })).notifications).toHaveLength(1);
+        expect((await svc.listInbox('u1', { limit: 120 })).notifications).toHaveLength(120);
+    });
+});