Skip to content

fix: use indexed query for soul list to avoid over-fetching #1531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
goulonghui wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: use indexed query for soul list to avoid over-fetching #1531

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions convex/souls.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,24 +125,27 @@ export const list = query({
const limit = args.limit ?? 24;
const ownerUserId = args.ownerUserId;
if (ownerUserId) {
// No compound index on (ownerUserId, softDeletedAt) exists, so use
// the by_owner index and let Convex filter out deleted rows before
// truncating. This avoids the previous take(limit*5) over-fetch.
const entries = await ctx.db
.query("souls")
.withIndex("by_owner", (q) => q.eq("ownerUserId", ownerUserId))
.order("desc")
.take(limit * 5);
.filter((q) => q.eq(q.field("softDeletedAt"), undefined))
.take(limit);
return entries
.filter((soul) => !soul.softDeletedAt)
.slice(0, limit)
.map((soul) => toPublicSoul(soul))
.filter((soul): soul is NonNullable<typeof soul> => Boolean(soul));
}
// Use the by_active_updated index to skip soft-deleted rows at the
Copy link
Copy Markdown
Contributor

@vercel vercel bot Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The souls.ts query uses .withIndex("by_active_updated", ...) but the souls table schema is missing this index definition, which will cause a Convex runtime error.

Fix on Vercel

// index level instead of reading 5× documents and filtering in JS.
const entries = await ctx.db
.query("souls")
.withIndex("by_active_updated", (q) => q.eq("softDeletedAt", undefined))
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Query souls with an existing index

The new browse path in souls.list now calls withIndex("by_active_updated", ...), but the souls schema only defines by_slug, by_owner, by_owner_publisher, and by_updated (see convex/schema.ts). In environments where this commit is deployed as-is, any souls.list call without ownerUserId will fail at runtime with an unknown-index error instead of returning results.

Useful? React with 👍 / 👎.

.order("desc")
Comment on lines 144 to 146
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 Missing by_active_updated index on souls table

The browse path references .withIndex("by_active_updated", ...), but this index does not exist on the souls table. Looking at convex/schema.ts, the souls table (defined around line 366) only declares four indexes: by_slug, by_owner, by_owner_publisher, and by_updated. The by_active_updated compound index (["softDeletedAt", "updatedAt"]) is defined on skillSearchDigest, packages, and packageSearchDigest — not on souls.

Because Convex generates typed index accessors at build time, this will fail TypeScript type-checking and npx convex deploy will reject the code entirely. The by_active_updated index needs to be added to the souls table in convex/schema.ts:

// convex/schema.ts — add to the souls defineTable chain
const souls = defineTable({ ... })
  .index("by_slug", ["slug"])
  .index("by_owner", ["ownerUserId"])
  .index("by_owner_publisher", ["ownerPublisherId"])
  .index("by_updated", ["updatedAt"])
  .index("by_active_updated", ["softDeletedAt", "updatedAt"]); // add this
Prompt To Fix With AI 
This is a comment left during a code review.
Path: convex/souls.ts
Line: 144-146

Comment:
**Missing `by_active_updated` index on `souls` table**

The browse path references `.withIndex("by_active_updated", ...)`, but this index does not exist on the `souls` table. Looking at `convex/schema.ts`, the `souls` table (defined around line 366) only declares four indexes: `by_slug`, `by_owner`, `by_owner_publisher`, and `by_updated`. The `by_active_updated` compound index (`["softDeletedAt", "updatedAt"]`) is defined on `skillSearchDigest`, `packages`, and `packageSearchDigest` — not on `souls`.

Because Convex generates typed index accessors at build time, this will fail TypeScript type-checking and `npx convex deploy` will reject the code entirely. The `by_active_updated` index needs to be added to the `souls` table in `convex/schema.ts`:

```typescript
// convex/schema.ts — add to the souls defineTable chain
const souls = defineTable({ ... })
  .index("by_slug", ["slug"])
  .index("by_owner", ["ownerUserId"])
  .index("by_owner_publisher", ["ownerPublisherId"])
  .index("by_updated", ["updatedAt"])
  .index("by_active_updated", ["softDeletedAt", "updatedAt"]); // add this
```

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

.take(limit * 5);
.take(limit);
return entries
.filter((soul) => !soul.softDeletedAt)
.slice(0, limit)
.map((soul) => toPublicSoul(soul))
.filter((soul): soul is NonNullable<typeof soul> => Boolean(soul));
},
Expand Down