fix(admin/pads): apply filter chip server-side, before pagination

JohnMcLear · claude · JohnMcLear · commit bdae1d638986 · 2026-05-17T19:12:01.000+01:00
Before: PadPage's filter chip (`active`/`recent`/`empty`/`stale`) ran
on the client AFTER the 12-row page slice was already on screen. On a
deployment with hundreds of pads it produced obviously wrong results
— click "empty pads" on page 1 with 100 empties and only the 0–12
empties within the current page passed the filter. thm reported this
on a 3.1.0 deployment.

Move the filter into `PadSearchQuery` so the `/settings` socket can
apply it before slicing:

  1. pattern filter on names (cheap)
  2. hydrate metadata for the matching pad universe iff a non-`all`
     filter is set or a non-`padName` sort is requested
  3. apply filter chip on the hydrated set
  4. sort + slice → `total` reflects the filtered universe so the
     pagination footer makes sense

The original handler also had a 4-way `if/else if` that duplicated the
hydrate-and-sort loop per `sortBy`. Folded those into one pipeline
with a single comparator switch.

Client side, `PadPage.tsx`:
- drop the client-side `filteredResults` filter (server already filters)
- chip click writes `filter` into searchParams (debounced refetch) and
  resets `currentPage` to 0
- older clients that don't send `filter` keep working — server defaults
  to `all`

Stats cards (totalUsers/activeCount/emptyCount) still count the visible
page only — that's a pre-existing UI limitation tracked separately.

Closes the regression thm reported.

Test plan
- `tsc --noEmit` clean (server + admin)
- New backend spec `padLoadFilter.ts` exercises filter:empty with
  small `limit` to lock in the bug-fix, plus all/active/omitted cases
- `5 passing` locally on Node 25

Co-Authored-By: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/admin/src/pages/PadPage.tsx b/admin/src/pages/PadPage.tsx
@@ -1,20 +1,16 @@
 import {Trans, useTranslation} from "react-i18next";
 import {useEffect, useMemo, useState} from "react";
 import {useStore} from "../store/store.ts";
-import {PadSearchQuery, PadSearchResult} from "../utils/PadSearch.ts";
+import {PadFilter, PadSearchQuery, PadSearchResult} from "../utils/PadSearch.ts";
 import {useDebounce} from "../utils/useDebounce.ts";
 import * as Dialog from "@radix-ui/react-dialog";
 import {ChevronLeft, ChevronRight, Eye, Trash2, FileStack, PlusIcon, Search, X, RefreshCw, History} from "lucide-react";
 import {useForm} from "react-hook-form";
 import type {TFunction} from "i18next";
 
 type PadCreateProps = { padName: string }
-type FilterId = 'all' | 'active' | 'recent' | 'empty' | 'stale'
 
-const PAD_FILTER_IDS: FilterId[] = ['all', 'active', 'recent', 'empty', 'stale']
-
-const isRecent = (ts: number) => (Date.now() - ts) < 86_400_000 * 7
-const isStale  = (ts: number) => (Date.now() - ts) > 86_400_000 * 365
+const PAD_FILTER_IDS: PadFilter[] = ['all', 'active', 'recent', 'empty', 'stale']
 
 function relativeTime(t: TFunction, ts: number): string {
   const d = (Date.now() - ts) / 1000
@@ -58,12 +54,19 @@ function sanitizeLocale(lng?: string): string {
 export const PadPage = () => {
   const settingsSocket = useStore(state => state.settingsSocket)
   const [searchParams, setSearchParams] = useState<PadSearchQuery>({
-    offset: 0, limit: 12, pattern: '', sortBy: 'lastEdited', ascending: false,
+    offset: 0, limit: 12, pattern: '', sortBy: 'lastEdited', ascending: false, filter: 'all',
   })
   const {t, i18n} = useTranslation()
   const locale = sanitizeLocale(i18n.resolvedLanguage ?? i18n.language)
   const [searchTerm, setSearchTerm] = useState('')
-  const [filter, setFilter] = useState<FilterId>('all')
+  // Read filter off searchParams so chip changes round-trip through
+  // the server (`filter` is applied before pagination there). Clicking
+  // a chip used to filter only the current 12-row page slice.
+  const filter: PadFilter = searchParams.filter ?? 'all'
+  const setFilter = (f: PadFilter) => {
+    setCurrentPage(0)
+    setSearchParams({...searchParams, filter: f, offset: 0})
+  }
   const [selected, setSelected] = useState<Set<string>>(new Set())
   const pads = useStore(state => state.pads)
   const [currentPage, setCurrentPage] = useState(0)
@@ -78,28 +81,23 @@ export const PadPage = () => {
     [pads, searchParams.limit]
   )
 
-  const filteredResults = useMemo(() => {
-    const r = pads?.results ?? []
-    if (filter === 'active') return r.filter(p => p.userCount > 0)
-    if (filter === 'recent') return r.filter(p => isRecent(p.lastEdited))
-    if (filter === 'empty')  return r.filter(p => p.revisionNumber === 0)
-    if (filter === 'stale')  return r.filter(p => isStale(p.lastEdited))
-    return r
-  }, [pads, filter])
-
-  const totalUsers  = useMemo(() => (pads?.results ?? []).reduce((s, p) => s + p.userCount, 0), [pads])
-  const activeCount = useMemo(() => (pads?.results ?? []).filter(p => p.userCount > 0).length, [pads])
-  const emptyCount  = useMemo(() => (pads?.results ?? []).filter(p => p.revisionNumber === 0).length, [pads])
+  // The server applies `filter` before paginating; the page payload is
+  // already the filtered slice. The stats cards still reflect the
+  // current page (pre-existing behaviour) — making them global would
+  // require a separate aggregate query.
+  const visibleResults = pads?.results ?? []
+  const totalUsers  = useMemo(() => visibleResults.reduce((s, p) => s + p.userCount, 0), [pads])
+  const activeCount = useMemo(() => visibleResults.filter(p => p.userCount > 0).length, [pads])
+  const emptyCount  = useMemo(() => visibleResults.filter(p => p.revisionNumber === 0).length, [pads])
   const lastActivity = useMemo(() => {
-    const r = pads?.results ?? []
-    return r.length ? Math.max(...r.map(p => p.lastEdited)) : null
+    return visibleResults.length ? Math.max(...visibleResults.map(p => p.lastEdited)) : null
   }, [pads])
 
-  const allSelected = filteredResults.length > 0 && filteredResults.every(p => selected.has(p.padName))
+  const allSelected = visibleResults.length > 0 && visibleResults.every(p => selected.has(p.padName))
   const toggleAll = () => {
     const s = new Set(selected)
-    if (allSelected) filteredResults.forEach(p => s.delete(p.padName))
-    else filteredResults.forEach(p => s.add(p.padName))
+    if (allSelected) visibleResults.forEach(p => s.delete(p.padName))
+    else visibleResults.forEach(p => s.add(p.padName))
     setSelected(s)
   }
   const toggleOne = (name: string) => {
@@ -250,7 +248,7 @@ export const PadPage = () => {
       <section className="pm-section">
         <div className="pm-section-header">
           <h2><Trans i18nKey="admin_pads.all_pads"/></h2>
-          <span className="pm-count-badge">{filteredResults.length}</span>
+          <span className="pm-count-badge">{visibleResults.length}</span>
           <div className="pm-spacer"/>
           <div className="pm-toolbar">
             <div className="pm-search">
@@ -330,7 +328,7 @@ export const PadPage = () => {
           </div>
         )}
 
-        {filteredResults.length > 0 ? (
+        {visibleResults.length > 0 ? (
           <div className="pm-table-wrap">
             <table>
               <thead>
@@ -348,7 +346,7 @@ export const PadPage = () => {
                 </tr>
               </thead>
               <tbody>
-                {filteredResults.map(pad => {
+                {visibleResults.map(pad => {
                   const isEmpty = pad.revisionNumber === 0
                   const isSel = selected.has(pad.padName)
                   return (
diff --git a/admin/src/utils/PadSearch.ts b/admin/src/utils/PadSearch.ts
@@ -1,9 +1,12 @@
+export type PadFilter = 'all' | 'active' | 'recent' | 'empty' | 'stale';
+
 export type PadSearchQuery = {
   pattern: string;
   offset: number;
   limit: number;
   ascending: boolean;
   sortBy: string;
+  filter?: PadFilter;
 }
 
 
diff --git a/src/node/hooks/express/adminsettings.ts b/src/node/hooks/express/adminsettings.ts
@@ -1,7 +1,7 @@
 'use strict';
 
 
-import {PadQueryResult, PadSearchQuery} from "../../types/PadSearchQuery";
+import {PAD_FILTERS, PadFilter, PadQueryResult, PadSearchQuery} from "../../types/PadSearchQuery";
 import log4js from 'log4js';
 
 const fsp = require('fs').promises;
@@ -110,137 +110,111 @@ exports.socketio = (hookName: string, {io}: any) => {
     socket.on('padLoad', async (query: PadSearchQuery) => {
       const {padIDs} = await padManager.listAllPads();
 
-      const data: {
-        total: number,
-        results?: PadQueryResult[]
-      } = {
-        total: padIDs.length,
-      };
-      let result: string[] = padIDs;
-      let maxResult;
-
-      // Filter out matches
+      // ── 1. Pattern filter (cheap, by name only) ─────────────────────
+      let candidateNames: string[] = padIDs;
       if (query.pattern) {
-        result = result.filter((padName: string) => padName.includes(query.pattern));
+        candidateNames = candidateNames.filter(
+            (padName: string) => padName.includes(query.pattern));
       }
 
-      data.total = result.length;
+      // ── 2. Resolve filter chip ──────────────────────────────────────
+      // PadPage sends a chip id; "all" (default) means no additional
+      // filtering. We accept missing values from older clients gracefully.
+      const filter: PadFilter =
+          (query.filter && PAD_FILTERS.includes(query.filter)) ? query.filter : 'all';
+
+      // ── 3. Decide whether we need full metadata for every candidate ──
+      // The fast path — name-sort with no filter chip — only needs to
+      // hydrate metadata for the 12-row page slice. Any other path
+      // (filter chip OR non-name sort) requires every candidate's revs
+      // / users / lastEdited up front so we can sort and slice against
+      // the right universe. The expensive call is `padManager.getPad`;
+      // user counts come from an in-memory map.
+      const needsFullScan = filter !== 'all' || query.sortBy !== 'padName';
+
+      const loadMeta = async (padName: string): Promise<PadQueryResult> => {
+        const pad = await padManager.getPad(padName);
+        return {
+          padName,
+          lastEdited: await pad.getLastEdit(),
+          userCount: api.padUsersCount(padName).padUsersCount,
+          revisionNumber: pad.getHeadRevisionNumber(),
+        };
+      };
+
+      // Lazily lifted so we don't load every pad twice on the fast path.
+      let hydrated: PadQueryResult[] | null = null;
+      const hydrateAll = async () => {
+        if (hydrated == null) {
+          hydrated = await Promise.all(candidateNames.map(loadMeta));
+        }
+        return hydrated;
+      };
+
+      // ── 4. Filter chip — applied to hydrated metadata ────────────────
+      // Bucket boundaries match the client chips in PadPage.tsx so the
+      // counts on the stats cards keep meaning the same thing. Compute
+      // `now` once per request so a pad doesn't slip between buckets
+      // mid-loop.
+      const now = Date.now();
+      const isRecent = (lastEdited: number) => now - lastEdited < 86_400_000 * 7;
+      const isStale  = (lastEdited: number) => now - lastEdited > 86_400_000 * 365;
+      const matchesFilter = (m: PadQueryResult) => {
+        switch (filter) {
+          case 'active': return m.userCount > 0;
+          case 'recent': return isRecent(Number(m.lastEdited));
+          case 'empty':  return m.revisionNumber === 0;
+          case 'stale':  return isStale(Number(m.lastEdited));
+          default:       return true;
+        }
+      };
 
-      maxResult = result.length - 1;
-      if (maxResult < 0) {
-        maxResult = 0;
+      // ── 5. Total — i.e. the count the pagination footer reflects ────
+      // For the fast path this is just the pattern-filtered name list;
+      // for full-scan we report the post-chip total.
+      let totalNames: string[] | null = needsFullScan ? null : candidateNames;
+      let postFilterMetas: PadQueryResult[] | null = null;
+      if (needsFullScan) {
+        postFilterMetas = (await hydrateAll()).filter(matchesFilter);
       }
+      const total = needsFullScan ? postFilterMetas!.length : totalNames!.length;
 
-      // Reset to default values if out of bounds
+      // ── 6. Clamp offset/limit ──────────────────────────────────────
+      const maxOffset = Math.max(total - 1, 0);
       if (query.offset && query.offset < 0) {
         query.offset = 0;
-      } else if (query.offset > maxResult) {
-        query.offset = maxResult;
+      } else if (query.offset > maxOffset) {
+        query.offset = maxOffset;
       }
-
       if (query.limit && query.limit < 0) {
-       // Too small
         query.limit = 0;
       } else if (query.limit > queryPadLimit) {
-       // Too big
         query.limit = queryPadLimit;
       }
 
-
-      if (query.sortBy === 'padName') {
-        result = result.sort((a, b) => {
-          if (a < b) return query.ascending ? -1 : 1;
-          if (a > b) return query.ascending ? 1 : -1;
-          return 0;
-        }).slice(query.offset, query.offset + query.limit);
-
-        data.results = await Promise.all(result.map(async (padName: string) => {
-          const pad = await padManager.getPad(padName);
-          const revisionNumber = pad.getHeadRevisionNumber()
-          const userCount = api.padUsersCount(padName).padUsersCount;
-          const lastEdited = await pad.getLastEdit();
-
-          return {
-            padName,
-            lastEdited,
-            userCount,
-            revisionNumber
+      // ── 7. Sort + slice ────────────────────────────────────────────
+      const dir = query.ascending ? 1 : -1;
+      const cmpStr = (a: string, b: string) => a < b ? -dir : a > b ? dir : 0;
+      const cmpNum = (a: number, b: number) => a < b ? -dir : a > b ? dir : 0;
+
+      let results: PadQueryResult[];
+      if (needsFullScan) {
+        const sorted = postFilterMetas!.sort((a, b) => {
+          switch (query.sortBy) {
+            case 'padName':        return cmpStr(a.padName, b.padName);
+            case 'revisionNumber': return cmpNum(a.revisionNumber, b.revisionNumber);
+            case 'userCount':      return cmpNum(a.userCount, b.userCount);
+            case 'lastEdited':     return cmpStr(String(a.lastEdited), String(b.lastEdited));
+            default:               return 0;
           }
-        }));
-      } else if (query.sortBy === "revisionNumber") {
-        const currentWinners: PadQueryResult[] = []
-        const padMapping = [] as {padId: string, revisionNumber: number}[]
-        for (let res of result) {
-          const pad = await padManager.getPad(res);
-          const revisionNumber = pad.getHeadRevisionNumber()
-          padMapping.push({padId: res, revisionNumber})
-        }
-        padMapping.sort((a, b) => {
-          if (a.revisionNumber < b.revisionNumber) return query.ascending ? -1 : 1;
-          if (a.revisionNumber > b.revisionNumber) return query.ascending ? 1 : -1;
-          return 0;
-        })
-
-       for (const padRetrieval of padMapping.slice(query.offset, query.offset + query.limit)) {
-        let pad = await padManager.getPad(padRetrieval.padId);
-        currentWinners.push({
-         padName: padRetrieval.padId,
-         lastEdited: await pad.getLastEdit(),
-         userCount: api.padUsersCount(pad.padName).padUsersCount,
-         revisionNumber: padRetrieval.revisionNumber
-        })
-       }
-
-       data.results = currentWinners;
-      } else if (query.sortBy === "userCount") {
-       const currentWinners: PadQueryResult[] = []
-       const padMapping = [] as {padId: string, userCount: number}[]
-       for (let res of result) {
-        const userCount = api.padUsersCount(res).padUsersCount
-        padMapping.push({padId: res, userCount})
-       }
-       padMapping.sort((a, b) => {
-        if (a.userCount < b.userCount) return query.ascending ? -1 : 1;
-        if (a.userCount > b.userCount) return query.ascending ? 1 : -1;
-        return 0;
-       })
-
-       for (const padRetrieval of padMapping.slice(query.offset, query.offset + query.limit)) {
-        let pad = await padManager.getPad(padRetrieval.padId);
-        currentWinners.push({
-         padName: padRetrieval.padId,
-         lastEdited: await pad.getLastEdit(),
-         userCount: padRetrieval.userCount,
-         revisionNumber: pad.getHeadRevisionNumber()
-        })
-       }
-       data.results = currentWinners;
-      } else if (query.sortBy === "lastEdited") {
-       const currentWinners: PadQueryResult[] = []
-       const padMapping = [] as {padId: string, lastEdited: string}[]
-       for (let res of result) {
-        const pad = await padManager.getPad(res);
-        const lastEdited = await pad.getLastEdit();
-        padMapping.push({padId: res, lastEdited})
-       }
-       padMapping.sort((a, b) => {
-        if (a.lastEdited < b.lastEdited) return query.ascending ? -1 : 1;
-        if (a.lastEdited > b.lastEdited) return query.ascending ? 1 : -1;
-        return 0;
-       })
-
-       for (const padRetrieval of padMapping.slice(query.offset, query.offset + query.limit)) {
-        let pad = await padManager.getPad(padRetrieval.padId);
-        currentWinners.push({
-         padName: padRetrieval.padId,
-         lastEdited: padRetrieval.lastEdited,
-         userCount: api.padUsersCount(pad.padName).padUsersCount,
-         revisionNumber: pad.getHeadRevisionNumber()
-        })
-       }
-       data.results = currentWinners;
+        });
+        results = sorted.slice(query.offset, query.offset + query.limit);
+      } else {
+        const sliceNames = totalNames!.sort(cmpStr).slice(query.offset, query.offset + query.limit);
+        results = await Promise.all(sliceNames.map(loadMeta));
       }
 
+      const data: {total: number, results?: PadQueryResult[]} = {total, results};
       socket.emit('results:padLoad', data);
     })
 
diff --git a/src/node/types/PadSearchQuery.ts b/src/node/types/PadSearchQuery.ts
@@ -1,9 +1,18 @@
+export type PadFilter = "all" | "active" | "recent" | "empty" | "stale";
+
+export const PAD_FILTERS: PadFilter[] = ["all", "active", "recent", "empty", "stale"];
+
 export type PadSearchQuery = {
   pattern: string;
   offset: number;
   limit: number;
   ascending: boolean;
   sortBy: "padName" | "lastEdited" | "userCount" | "revisionNumber";
+  // Filter chip. Defaults to "all". Applied server-side so pagination
+  // reflects the filtered universe — without this, the chip filters only
+  // the current page slice and "0 empty pads" can appear on page 1 while
+  // page 2 has nothing but empties.
+  filter?: PadFilter;
 }
 
 
diff --git a/src/tests/backend/specs/admin/padLoadFilter.ts b/src/tests/backend/specs/admin/padLoadFilter.ts
