fix: improve admin user search coverage

[ImLukeF]

Summary

Admin user search was only scanning a recent slice of users when a query was present. That meant an older existing account could be missing from the admin Users page even when the search text matched exactly.

This PR changes searched admin listings to scan the full ordered user set, while keeping the default non-search listing capped by limit. It also adds a regression test covering an older matching user outside the previous recent scan window.

Testing

Not run locally in this environment because bun is unavailable.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
clawhub	Ready	Preview, Comment	Apr 2, 2026 0:32am

[greptile-apps]

Greptile Summary

This PR fixes a correctness bug where admin user searches missed older users that fell outside the previous 500–5 000 user bounded scan window, replacing take(n) with collect() in queryUsersForAdminList for non-empty queries.

• The change affects both list and searchInternal (they share queryUsersForAdminList), but the two existing users.list tests that assert take(500) / take(2_000) were not updated — they will fail. The PR author notes tests were not run locally, so this was likely missed.

Confidence Score: 4/5

• Safe to merge after fixing the two stale users.list test assertions that will fail under the new collect-based search path.
• The logic change itself is correct and intentional. However, two tests in the unchanged users.list describe block still assert the old bounded-scan behaviour (take(500) / take(2_000)) and will fail because queryUsersForAdminList is shared. Since the author confirmed tests were not run, this slip is unsurprising but blocks a clean CI pass.
• convex/users.test.ts — lines 873–919 in the users.list describe block need to be updated to match the new collect()-based search path.

Comments Outside Diff (2)

1. convex/users.test.ts, line 873-919 (link)

   Stale users.list search tests will fail after this change

   queryUsersForAdminList is shared by both list and searchInternal, so the switch from take(n) to collect() on the search path affects both handlers. Two tests in the users.list describe block were not updated and now assert the old bounded-scan behaviour:

   • "uses bounded scan for search instead of full collect" (line 873): asserts expect(take).toHaveBeenCalledWith(500) and expect(collect).not.toHaveBeenCalled() — both will fail because collect() is now called instead.
   • "clamps large limit and search scan size" (line 900): asserts expect(take).toHaveBeenCalledWith(2_000) — will fail for the same reason.

   Both tests should be updated to mirror the pattern established in the new searchInternal test: assert collect was called once and take was not.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: convex/users.test.ts
   Line: 873-919
   
   Comment:
   **Stale `users.list` search tests will fail after this change**
   
   `queryUsersForAdminList` is shared by both `list` and `searchInternal`, so the switch from `take(n)` to `collect()` on the search path affects both handlers. Two tests in the `users.list` describe block were not updated and now assert the old bounded-scan behaviour:
   
   - *"uses bounded scan for search instead of full collect"* (line 873): asserts `expect(take).toHaveBeenCalledWith(500)` and `expect(collect).not.toHaveBeenCalled()` — both will fail because `collect()` is now called instead.
   - *"clamps large limit and search scan size"* (line 900): asserts `expect(take).toHaveBeenCalledWith(2_000)` — will fail for the same reason.
   
   Both tests should be updated to mirror the pattern established in the new `searchInternal` test: assert `collect` was called once and `take` was not.
   
   How can I resolve this? If you propose a fix, please make it concise.

2. convex/users.ts, line 28-29 (link)

   Dead constants after removing computeUserSearchScanLimit

   MAX_USER_SEARCH_SCAN and MIN_USER_SEARCH_SCAN are now unreferenced. The only consumer was the deleted computeUserSearchScanLimit function. They can be removed to keep the module clean.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: convex/users.ts
   Line: 28-29
   
   Comment:
   **Dead constants after removing `computeUserSearchScanLimit`**
   
   `MAX_USER_SEARCH_SCAN` and `MIN_USER_SEARCH_SCAN` are now unreferenced. The only consumer was the deleted `computeUserSearchScanLimit` function. They can be removed to keep the module clean.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: convex/users.test.ts
Line: 873-919

Comment:
**Stale `users.list` search tests will fail after this change**

`queryUsersForAdminList` is shared by both `list` and `searchInternal`, so the switch from `take(n)` to `collect()` on the search path affects both handlers. Two tests in the `users.list` describe block were not updated and now assert the old bounded-scan behaviour:

- *"uses bounded scan for search instead of full collect"* (line 873): asserts `expect(take).toHaveBeenCalledWith(500)` and `expect(collect).not.toHaveBeenCalled()` — both will fail because `collect()` is now called instead.
- *"clamps large limit and search scan size"* (line 900): asserts `expect(take).toHaveBeenCalledWith(2_000)` — will fail for the same reason.

Both tests should be updated to mirror the pattern established in the new `searchInternal` test: assert `collect` was called once and `take` was not.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: convex/users.ts
Line: 28-29

Comment:
**Dead constants after removing `computeUserSearchScanLimit`**

`MAX_USER_SEARCH_SCAN` and `MIN_USER_SEARCH_SCAN` are now unreferenced. The only consumer was the deleted `computeUserSearchScanLimit` function. They can be removed to keep the module clean.

```suggestion
const MAX_USER_LIST_LIMIT = 200;
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix admin user search coverage" | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR updates the admin user search behavior so that when a search query is provided it scans the full users dataset (rather than a bounded “recent users” window), while keeping the empty-query admin listing capped by limit. It also updates tests to prevent regressions where older matching users are missed.

Changes:

• Switch admin search path from bounded take(...) scanning to full-table collect() scanning when a search query is present.
• Keep empty-query listing behavior capped by limit.
• Update/add regression coverage to ensure older matching users are included in results.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
convex/users.ts	Changes admin list/search query behavior to collect() all users when searching.
convex/users.test.ts	Updates regression tests to assert full-list search behavior and preserve empty-query cap behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

MAX_USER_SEARCH_SCAN/MIN_USER_SEARCH_SCAN are now unused after removing computeUserSearchScanLimit. With noUnusedLocals: true in tsconfig, this will fail TypeScript compilation; remove these constants or reintroduce usage (e.g., as a cap for the search scan).

Suggested change

	const scannedUsers = await orderedUsers.collect();
	const scanLimit = clampInt(args.limit, MIN_USER_SEARCH_SCAN, MAX_USER_SEARCH_SCAN);
	const scannedUsers = await orderedUsers.take(scanLimit);

[Copilot]

Calling orderedUsers.collect() for every non-empty search performs an unbounded full-table scan of the users table. This will scale linearly with user count and may hit Convex execution/response limits as the dataset grows; consider adding a bounded/paginated scan strategy or a dedicated search/index-based approach instead of collecting all users at once.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 146e2bb3b1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Restore bounded scan for admin user search

Changing queryUsersForAdminList from a bounded take(...) to collect() makes every non-empty admin search read the entire users table, which can hit Convex query limits (document/bytes read caps, including the 32K-doc ceiling) as the user base grows. In that state, searchInternal/list with a search term will error instead of returning matches, so this is a functional regression plus a significant bandwidth/cost increase.

Useful? React with 👍 / 👎.