feat: switch /skills and homepage to listPublicPageV4

[sethconvex]

Summary

• Root cause fix: getPage() ignores indexFields at runtime — always needs schema. Fixed by passing schema directly.
• Second fix: targetMaxRows ignored when endIndexKey provided — switched to absoluteMaxRows.
• /skills browse page and homepage now use listPublicPageV4 (cacheable deterministic cursors)
• /skillsv4 staging route verified in production, then removed
• Guard against hasMore=true with nextCursor=null edge case

Test plan

• Visit /skills — verify skills load, pagination, sort/filter/search all work
• Visit homepage — verify popular skills section loads
• Test pagination: load multiple pages, change sort, verify no duplicates

🤖 Generated with Claude Code

[vercel]

@sethconvex is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

[sethconvex]

@codex review

[greptile-apps]

Greptile Summary

This PR ships two correctness fixes to listPublicPageV4 and adds a staged-release path so the V4 paginator can be validated in production without touching the existing /skills (V3) route.

Key changes:

• convex/skills.ts: Passes schema directly to getPage() instead of the hand-maintained DIGEST_INDEX_FIELDS map, which was silently ignored at runtime (the library always derives fields from the schema). Also switches targetMaxRows → absoluteMaxRows so the fetch-size limit is respected when endIndexKey is present. Both fixes are correct and well-motivated.
• src/routes/skillsv4.tsx: New /skillsv4 route — a direct copy of /skills wired to the V4 query via useV4: true. Route structure and search validation are correct.
• src/routes/skills/-useSkillsBrowseModel.ts: V4 branch cleanly added behind the useV4 flag. One defensive concern: if the server ever returns { hasMore: true, nextCursor: null }, the client sets listCursor to null but canLoadMore to true, causing the next "load more" to re-fetch page 1 and replace accumulated results instead of appending.
• src/routes/test-v4.tsx: Debug page for raw V4 API testing, deployed publicly with no auth guard. The route logs verbose internal details to the console and lets users query any Convex URL. Since /test-v4 is intended to be short-lived, it should either be gated behind an admin check or removed promptly once V4 is verified.

Confidence Score: 3/5

• Safe to merge for staging, but the publicly accessible /test-v4 debug route and the defensive pagination edge case in the client model should be addressed before or shortly after merging.
• The Convex-side fixes are correct and straightforward. The staged release approach is sound. The score is reduced for two reasons: (1) /test-v4 has no auth guard and is accessible to all users in production, which is an unintended exposure for a temporary debug tool; (2) the V4 client path in useSkillsBrowseModel can silently reset the accumulated list if hasMore=true but nextCursor=null, though this edge case is unlikely to occur in practice given the server-side cursor logic.
• src/routes/test-v4.tsx (no auth guard) and src/routes/skills/-useSkillsBrowseModel.ts (defensive pagination guard for null cursor).

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/routes/skills/-useSkillsBrowseModel.ts
Line: 85-87

Comment:
**List reset when `hasMore=true` but `nextCursor=null`**

If the server returns `{ hasMore: true, nextCursor: null }` (an edge case possible when both fetch rounds return no accepted items — e.g., `highlightedOnly=true` with no matches and an unexpectedly empty `indexKeys` array from `getPage`), `listCursor` is set to `null` but `listStatus` is set to `'idle'`, which means `canLoadMore` is `true`.

The next "load more" call invokes `fetchPage(null, generation)`, where `cursor` is `null` (falsy). This triggers the **replace** branch of `setListResults`:

```ts
setListResults((prev) => (cursor ? [...prev, ...result.page] : result.page))
//                          ^ cursor is null → replaces, not appends
```

The accumulated results are silently reset to page 1, which is incorrect behavior for pagination. A defensive guard would prevent this:

```suggestion
          if (generation !== fetchGeneration.current) return
          setListResults((prev) => (cursor ? [...prev, ...result.page] : result.page))
          const canAdvance = result.hasMore && result.nextCursor != null
          setListCursor(canAdvance ? result.nextCursor : null)
          setListStatus(canAdvance ? 'idle' : 'done')
```

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

---

This is a comment left during a code review.
Path: src/routes/test-v4.tsx
Line: 7-13

Comment:
**Debug page is publicly accessible in production**

The `/test-v4` route has no `beforeLoad` auth guard. Any user who discovers the URL can visit it in production. The page:
- Exposes a user-editable Convex URL field, letting anyone point the debug UI at arbitrary Convex deployments.
- Logs verbose internal query details to the browser console (`[test-v4]` prefixed logs).
- Displays raw JSON API responses, revealing internal field names and data shape.

Since `listPublicPageV4` is a public query, this doesn't expose private data — but it does expose implementation details and is an unpolished surface for end users to stumble on. At minimum, consider redirecting non-admin users, consistent with how `/management` gates access via `isAdmin`:

```ts
beforeLoad: ({ context }) => {
  if (!context.isAdmin) throw redirect({ to: '/' })
},
```

Or, add a note in the PR to remove this route once V4 is verified rather than leaving it in indefinitely.

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

---

This is a comment left during a code review.
Path: src/routes/test-v4.tsx
Line: 50-51

Comment:
**New `ConvexHttpClient` instantiated on every fetch**

`new ConvexHttpClient(url)` is constructed inside `fetchPage`, so a fresh client is created on every query call. This bypasses any connection pooling or keep-alive the client may provide. For a debug tool this is fine, but if you expect heavy use during testing, consider memoizing the client with `useMemo` keyed on `url`:

```ts
const client = useMemo(() => new ConvexHttpClient(url), [url])
```

And remove it from the `fetchPage` closure.

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

_{Last reviewed commit: b3dd6c1}

[greptile-apps]

List reset when hasMore=true but nextCursor=null

If the server returns { hasMore: true, nextCursor: null } (an edge case possible when both fetch rounds return no accepted items — e.g., highlightedOnly=true with no matches and an unexpectedly empty indexKeys array from getPage), listCursor is set to null but listStatus is set to 'idle', which means canLoadMore is true.

The next "load more" call invokes fetchPage(null, generation), where cursor is null (falsy). This triggers the replace branch of setListResults:

setListResults((prev) => (cursor ? [...prev, ...result.page] : result.page))
//                          ^ cursor is null → replaces, not appends

The accumulated results are silently reset to page 1, which is incorrect behavior for pagination. A defensive guard would prevent this:

Suggested change

	setListResults((prev) => (cursor ? [...prev, ...result.page] : result.page))
	setListCursor(result.hasMore ? (result.nextCursor ?? null) : null)
	setListStatus(result.hasMore ? 'idle' : 'done')
	if (generation !== fetchGeneration.current) return
	setListResults((prev) => (cursor ? [...prev, ...result.page] : result.page))
	const canAdvance = result.hasMore && result.nextCursor != null
	setListCursor(canAdvance ? result.nextCursor : null)
	setListStatus(canAdvance ? 'idle' : 'done')

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/routes/skills/-useSkillsBrowseModel.ts
Line: 85-87

Comment:
**List reset when `hasMore=true` but `nextCursor=null`**

If the server returns `{ hasMore: true, nextCursor: null }` (an edge case possible when both fetch rounds return no accepted items — e.g., `highlightedOnly=true` with no matches and an unexpectedly empty `indexKeys` array from `getPage`), `listCursor` is set to `null` but `listStatus` is set to `'idle'`, which means `canLoadMore` is `true`.

The next "load more" call invokes `fetchPage(null, generation)`, where `cursor` is `null` (falsy). This triggers the **replace** branch of `setListResults`:

```ts
setListResults((prev) => (cursor ? [...prev, ...result.page] : result.page))
//                          ^ cursor is null → replaces, not appends
```

The accumulated results are silently reset to page 1, which is incorrect behavior for pagination. A defensive guard would prevent this:

```suggestion
          if (generation !== fetchGeneration.current) return
          setListResults((prev) => (cursor ? [...prev, ...result.page] : result.page))
          const canAdvance = result.hasMore && result.nextCursor != null
          setListCursor(canAdvance ? result.nextCursor : null)
          setListStatus(canAdvance ? 'idle' : 'done')
```

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

[greptile-apps]

Debug page is publicly accessible in production

The /test-v4 route has no beforeLoad auth guard. Any user who discovers the URL can visit it in production. The page:

• Exposes a user-editable Convex URL field, letting anyone point the debug UI at arbitrary Convex deployments.
• Logs verbose internal query details to the browser console ([test-v4] prefixed logs).
• Displays raw JSON API responses, revealing internal field names and data shape.

Since listPublicPageV4 is a public query, this doesn't expose private data — but it does expose implementation details and is an unpolished surface for end users to stumble on. At minimum, consider redirecting non-admin users, consistent with how /management gates access via isAdmin:

beforeLoad: ({ context }) => {
  if (!context.isAdmin) throw redirect({ to: '/' })
},

Or, add a note in the PR to remove this route once V4 is verified rather than leaving it in indefinitely.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/routes/test-v4.tsx
Line: 7-13

Comment:
**Debug page is publicly accessible in production**

The `/test-v4` route has no `beforeLoad` auth guard. Any user who discovers the URL can visit it in production. The page:
- Exposes a user-editable Convex URL field, letting anyone point the debug UI at arbitrary Convex deployments.
- Logs verbose internal query details to the browser console (`[test-v4]` prefixed logs).
- Displays raw JSON API responses, revealing internal field names and data shape.

Since `listPublicPageV4` is a public query, this doesn't expose private data — but it does expose implementation details and is an unpolished surface for end users to stumble on. At minimum, consider redirecting non-admin users, consistent with how `/management` gates access via `isAdmin`:

```ts
beforeLoad: ({ context }) => {
  if (!context.isAdmin) throw redirect({ to: '/' })
},
```

Or, add a note in the PR to remove this route once V4 is verified rather than leaving it in indefinitely.

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

[greptile-apps]

New ConvexHttpClient instantiated on every fetch

new ConvexHttpClient(url) is constructed inside fetchPage, so a fresh client is created on every query call. This bypasses any connection pooling or keep-alive the client may provide. For a debug tool this is fine, but if you expect heavy use during testing, consider memoizing the client with useMemo keyed on url:

const client = useMemo(() => new ConvexHttpClient(url), [url])

And remove it from the fetchPage closure.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/routes/test-v4.tsx
Line: 50-51

Comment:
**New `ConvexHttpClient` instantiated on every fetch**

`new ConvexHttpClient(url)` is constructed inside `fetchPage`, so a fresh client is created on every query call. This bypasses any connection pooling or keep-alive the client may provide. For a debug tool this is fine, but if you expect heavy use during testing, consider memoizing the client with `useMemo` keyed on `url`:

```ts
const client = useMemo(() => new ConvexHttpClient(url), [url])
```

And remove it from the `fetchPage` closure.

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

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🎉

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".