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

[JohnMcLear]

Summary

• thm reported on a 3.1.0 deployment: clicking the empty pads chip on the admin pads page only filtered the current 12-row page slice, not the full pad universe. Same bug applied to active / recent / stale.
• Cause: PadPage.tsx:81–88 ran filteredResults = pads.results.filter(...) on the already-paginated payload from padLoad. Filter chip → pagination order was inverted.
• Fix: push the filter into PadSearchQuery and apply it server-side in the /settings socket's padLoad handler, before offset/limit. total then reflects the filtered universe and the pagination footer makes sense.

Server-side refactor

The old handler had a 4-way if/else if (query.sortBy === …) that duplicated the hydrate-then-sort loop per sort key. Folded into one pipeline:

1. Pattern filter on names
2. Hydrate metadata for the candidate universe only if a non-all filter or a non-padName sort is requested (preserves the existing fast path for the default browse)
3. Apply filter chip on hydrated rows
4. Sort + slice → total becomes the filtered total

Client-side

• Drop filteredResults client-side filter
• Chip click writes filter into searchParams (so it round-trips through the debounced refetch) and resets currentPage to 0
• Older clients that don't send filter keep working — server defaults to all

Compatibility

• PadSearchQuery.filter is optional. Older clients (e.g. cached admin SPA) emit without it; server treats missing/unknown as all. No protocol break.

Known limitation (out of scope)

The stats cards (totalUsers / activeCount / emptyCount) sum from the current page slice — a pre-existing limitation; making them reflect the global universe needs a separate aggregate endpoint.

Test plan

• tsc --noEmit clean (server + admin)
• New backend spec tests/backend/specs/admin/padLoadFilter.ts — 5 passing locally on Node 25:
  • filter:"empty" returns only revisionNumber===0 from the full set
  • filter:"empty" with limit=2 still reports total=5 (the regression thm hit)
  • filter omitted → falls back to all
  • filter:"all" equals no-filter behaviour
  • filter:"active" excludes pads with userCount === 0
• CI: full backend + admin frontend playwright
• Manual: chip click on a >12-pad deployment, verify pagination spans the filtered set

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Apply admin pads filter server-side before pagination

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Move filter chip logic server-side before pagination to fix regression
  - Filter now applied in /settings socket before offset/limit slice
  - total reflects filtered universe, pagination footer now correct
• Refactor duplicated sort-by logic into single unified pipeline
  - Consolidate 4-way if/else into one comparator switch
  - Optimize metadata hydration only when needed (filter or non-name sort)
• Client-side filter chip now writes to searchParams for round-trip
  - Chip click resets currentPage to 0 and triggers debounced refetch
  - Older clients without filter field gracefully default to 'all'
• Add comprehensive backend test suite for filter regression
  - 5 test cases covering empty/active/omitted/all/recent scenarios

Diagram

flowchart LR
  A["Client: Chip Click"] -->|"filter in searchParams"| B["Server: padLoad Handler"]
  B -->|"1. Pattern Filter"| C["Candidate Names"]
  C -->|"2. Decide Hydration"| D{"Filter or Non-Name Sort?"}
  D -->|"Yes"| E["Hydrate All Metadata"]
  D -->|"No"| F["Fast Path: Name Sort Only"]
  E -->|"3. Apply Filter Chip"| G["Filtered Metadata"]
  G -->|"4. Sort + Slice"| H["Results with Correct Total"]
  F -->|"Sort + Slice"| H
  H -->|"Pagination Footer Reflects Filtered Universe"| I["Client: Display Results"]

Loading

File Changes

1. admin/src/utils/PadSearch.ts ✨ Enhancement +3/-0

Add filter field to PadSearchQuery type

• Add PadFilter type definition ('all' | 'active' | 'recent' | 'empty' | 'stale')
• Add optional filter?: PadFilter field to PadSearchQuery type
• Enables client to send filter chip selection to server

admin/src/utils/PadSearch.ts

---

2. src/node/types/PadSearchQuery.ts ✨ Enhancement +9/-0

Define PadFilter type and add to query schema

• Define PadFilter type and PAD_FILTERS constant array for validation
• Add optional filter?: PadFilter field to PadSearchQuery with documentation
• Explains why filter must be server-side (pagination ordering issue)

src/node/types/PadSearchQuery.ts

---

3. src/node/hooks/express/adminsettings.ts 🐞 Bug fix +88/-114

Refactor padLoad to apply filter before pagination

• Refactor padLoad handler from 4-way if/else into unified pipeline
• Implement 7-stage processing: pattern filter → hydration decision → metadata load → filter chip →
 total calculation → offset/limit clamp → sort/slice
• Add lazy hydration optimization: only load full metadata if filter chip or non-name sort requested
• Apply filter chip server-side before pagination so total reflects filtered universe
• Support backward compatibility: missing filter field defaults to 'all'

src/node/hooks/express/adminsettings.ts

---

View more (2)

4. src/tests/backend/specs/admin/padLoadFilter.ts 🧪 Tests +200/-0

Add comprehensive backend filter regression tests

• New regression test suite with 5 test cases for filter chip functionality
• Tests verify filter:empty returns only revisionNumber===0 pads from full set
• Regression test: filter:empty with limit=2 still reports correct total (thm's bug)
• Tests cover omitted filter, filter:all, and filter:active edge cases
• Includes admin socket setup with authentication and probe mechanism

src/tests/backend/specs/admin/padLoadFilter.ts

---

5. admin/src/pages/PadPage.tsx 🐞 Bug fix +26/-28

Remove client-side filter, use server-side via searchParams

• Remove client-side filteredResults computation (server now filters)
• Move filter state from local useState to searchParams for round-trip through server
• Chip click now writes filter to searchParams and resets currentPage to 0
• Rename filteredResults to visibleResults to clarify it's the server-filtered page slice
• Remove isRecent and isStale helper functions (now server-side only)
• Update stats cards to reflect current page only (pre-existing limitation documented)

admin/src/pages/PadPage.tsx

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. Stale query overwrites filter ✓ Resolved 🐞 Bug ≡ Correctness

Description

PadPage writes filter into searchParams, but multiple updates use
setSearchParams({...searchParams, ...}) with a captured (stale) searchParams. A pending
debounced pattern update can run after a chip click/sort/page change and overwrite the newer
filter/offset, causing the server to re-fetch the wrong universe.

Code

admin/src/pages/PadPage.tsx[R62-69]

+  // 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})
+  }

Evidence

setFilter sets filter and offset by spreading searchParams, and the debounced search term
handler also spreads searchParams after a delay. The debounce implementation schedules the
callback later, making stale overwrites possible when users interact quickly (type then click chip).

admin/src/pages/PadPage.tsx[62-116]
admin/src/utils/useDebounce.ts[6-22]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`searchParams` is updated via object spread from a render-time snapshot (e.g., in `setFilter`), while pattern changes are applied via a delayed debounce callback that also spreads from its own captured snapshot. Because `filter` now lives inside `searchParams`, the delayed update can unintentionally revert a chip selection (and/or pagination offset) and trigger a wrong `padLoad` request.
### Issue Context
This became user-visible due to moving the filter chip state into `searchParams` to round-trip through the server.
### Fix Focus Areas
- Use functional updaters for **every** `setSearchParams` call so updates merge against the latest state.
- Consider resetting pagination (offset + `currentPage`) when `pattern`, `sortBy`, or `filter` changes.
- file/path[start_line-end_line]
- admin/src/pages/PadPage.tsx[62-116]
- admin/src/pages/PadPage.tsx[269-287]
- admin/src/pages/PadPage.tsx[430-446]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. padLoad filter lacks docs 📘 Rule violation ⚙ Maintainability

Description

The admin /settings socket padLoad query now supports a new optional PadSearchQuery.filter
field, which changes the effective API contract for the event. No corresponding documentation update
is included, making the new query capability undiscoverable for maintainers/clients.

Code

src/node/types/PadSearchQuery.ts[R11-15]

+  // 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;

Evidence

PR Compliance ID 3 requires documentation updates when modifying an API. The PR adds a new `filter?:
PadFilter field to the PadSearchQuery type used by the admin /settings socket padLoad`
handler, but no docs are updated in this PR to describe the new field and allowed values.

src/node/types/PadSearchQuery.ts[11-15]
src/node/hooks/express/adminsettings.ts[120-170]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The admin `/settings` socket `padLoad` message now accepts an optional `filter` field (`all|active|recent|empty|stale`), but this API change is not documented.
## Issue Context
This PR adds `filter?: PadFilter` to `PadSearchQuery` and applies it server-side before pagination. Without docs, future maintainers (and any non-admin UI consumers) won't know the supported field/values or behavior.
## Fix Focus Areas
- admin/README.md[1-66]
- src/node/types/PadSearchQuery.ts[11-15]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Unbounded pad hydration ✓ Resolved 🐞 Bug ☼ Reliability

Description

The /settings socket padLoad handler hydrates metadata for every candidate pad using
Promise.all(candidateNames.map(loadMeta)) when filter !== 'all' or sortBy !== 'padName'. On
large deployments this can trigger thousands of concurrent padManager.getPad() + DB reads, risking
resource saturation and request timeouts.

Code

src/node/hooks/express/adminsettings.ts[R145-152]

+      // 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;
+      };

Evidence

candidateNames is derived from listAllPads() (the full pad universe), and hydrateAll() runs
Promise.all over it. loadMeta() calls padManager.getPad(), which initializes and loads Pad
state, and pad.getLastEdit() reads revision timestamp from the DB.

src/node/hooks/express/adminsettings.ts[110-152]
src/node/db/PadManager.ts[109-144]
src/node/db/Pad.ts[352-357]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`hydrateAll()` uses `Promise.all()` across the entire candidate pad list, which can create unbounded parallelism. Each `loadMeta()` calls `padManager.getPad()` and `pad.getLastEdit()`, which hit the database; doing this for thousands of pads concurrently can overload the DB/event loop.
### Issue Context
This path is taken whenever a non-name sort is requested (including the current default `lastEdited`) or any filter chip other than `all` is selected.
### Fix Focus Areas
- Replace `Promise.all(candidateNames.map(loadMeta))` with a concurrency-limited mapper (e.g., `p-limit`, `async.mapLimit`, or a custom pool) tuned for your DB.
- Keep behavior identical (same outputs), but bound in-flight `getPad`/`getLastEdit` calls.
- file/path[start_line-end_line]
- src/node/hooks/express/adminsettings.ts[135-152]
- src/node/db/PadManager.ts[109-144]
- src/node/db/Pad.ts[352-357]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Test admin user persists ✓ Resolved 🐞 Bug ☼ Reliability

Description

The new backend spec adds settings.users['test-admin'], but the suite teardown only reassigns
settings.users back to the previously saved reference, which does not remove the injected key if
settings.users was already an object. This can leak credentials/state into later backend specs.

Code

src/tests/backend/specs/admin/padLoadFilter.ts[R19-31]

+const adminSocket = async () => {
+  settings.users = settings.users || {};
+  settings.users['test-admin'] = {password: 'test-admin-password', is_admin: true};
+  const savedRequireAuthentication = settings.requireAuthentication;
+  settings.requireAuthentication = true;
+  let res: any;
+  try {
+    res = await (common.agent as any)
+        .get('/admin/')
+        .auth('test-admin', 'test-admin-password');
+  } finally {
+    settings.requireAuthentication = savedRequireAuthentication;
+  }

Evidence

The helper creates/updates settings.users and assigns test-admin. The suite saves `savedUsers =
settings.users` before probing, but that is a reference to the same object that later gets mutated;
reassigning it in after() does not undo the inserted property.

src/tests/backend/specs/admin/padLoadFilter.ts[19-31]
src/tests/backend/specs/admin/padLoadFilter.ts[100-146]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test mutates the global Settings singleton by inserting a `test-admin` user. The `after()` hook restores `settings.users` by reassigning the same object reference saved earlier, so the inserted key remains if the original `settings.users` object existed.
### Issue Context
This can cause cross-test contamination in backend suites that share the same Node process.
### Fix Focus Areas
- Snapshot `settings.users` via deep clone (or at least shallow clone) before mutation, then restore from the clone.
- Alternatively, explicitly `delete settings.users['test-admin']` in `after()` (and restore `requireAuthentication` as you already do).
- file/path[start_line-end_line]
- src/tests/backend/specs/admin/padLoadFilter.ts[19-31]
- src/tests/backend/specs/admin/padLoadFilter.ts[100-146]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[JohnMcLear]

Qodo's remaining Rule violation #2: padLoad filter lacks docs is stale — the docs section (Socket.io: padLoad query shape table with the new filter field) was added to admin/README.md at L67–88 in commit 5646ea3. Qodo seems to be re-evaluating the original [1-66] line range and not picking up the appended section. Bugs #1 and #3 are correctly marked ✓ Resolved.

Windows-with-Plugins (24) failure is the pre-existing Node 24 + Windows post-suite hard-kill flake — process exits 255 after the last completed test (cookie identity: same-author second socket kicks the first, pre-existing) with no test assertion failure. Same flake hit #7797 earlier and passed on rerun. Re-running now.