fix: reduce souls browse overfetch

[ImLukeF]

Summary

• add the missing souls.by_active_updated index so browse queries can target active souls directly
• update souls.list browse mode to read limit active rows instead of limit * 5 rows filtered in JS
• add focused regression coverage for the browse-path query shape
• rename a shadowed local in src/routes/cli/auth.tsx so lint passes cleanly

Why

The previous browse listing path over-read soul documents and then filtered soft-deleted rows in application code. That meant requests like /souls with limit=500 could read up to 2500 rows to return 500 visible items.

This patch keeps the behavior the same while making the browse path ask Convex for active rows directly.

Verification

• bun run lint
• bun x vitest run convex/souls.test.ts src/routes/cli/-auth.test.ts
• bun run build

Notes

• This supersedes the approach in fix: use indexed query for soul list to avoid over-fetching #1531 with a smaller patch that includes the missing schema index.
• bunx convex codegen is blocked in this shell because no CONVEX_DEPLOYMENT is configured here.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
clawhub	Ready	Preview, Comment	Apr 12, 2026 10:51am

[Copilot]

Pull request overview

This PR reduces over-fetching in the /souls browse listing by adding and using an index that targets active (non-soft-deleted) souls directly, and adds a focused regression test to ensure the optimized query shape is preserved.

Changes:

• Add souls.by_active_updated compound index (softDeletedAt, updatedAt) to support efficient active-only browsing.
• Update souls.list browse mode to use the new index and take(limit) instead of take(limit * 5) with JS-side filtering/slicing.
• Add a regression test asserting the browse query uses the active index and respects the requested limit; rename a shadowed local in the CLI auth route.

Reviewed changes

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

File	Description
src/routes/cli/auth.tsx	Renames a shadowed local variable used for redirect URL construction to satisfy linting.
convex/souls.ts	Switches browse listing to an active-only indexed query and removes JS-side overfetch filtering.
convex/souls.test.ts	Adds regression coverage to assert browse listing uses the active index and only takes limit.
convex/schema.ts	Adds the missing by_active_updated index to the souls table schema.

---

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

[greptile-apps]

Greptile Summary

This PR fixes the souls.list browse path to use the new by_active_updated compound index (["softDeletedAt", "updatedAt"]) so Convex returns only active souls directly, replacing the previous take(limit * 5) over-fetch and JS filter.

• The .order(\"desc\") on the new index sorts by updatedAt (the second index field after softDeletedAt is equality-filtered), not by _creationTime as the old scan did — a silent ordering change that contradicts the PR's claim of identical behaviour.

Confidence Score: 4/5

Safe to merge once the sort-order change is acknowledged or corrected — no data loss or security risk, but browsing order changes silently.

One P1 finding: the browse listing's sort order silently changes from _creationTime to updatedAt. This is a real behavioural difference that the PR description claims doesn't exist. If the change is intentional (aligning with listPublicPage), confirming that and documenting it would make this ready to merge at 5/5.

convex/souls.ts — browse path ordering and owner-path over-fetch

Comments Outside Diff (1)

1. convex/souls.ts, line 127-137 (link)

   Owner-scoped path still over-fetches

   The ownerUserId branch still reads limit * 5 rows and filters soft-deleted ones in JS — the exact pattern this PR fixes for the browse path. A compound index like ["ownerUserId", "softDeletedAt", "updatedAt"] would let this branch take limit rows directly too. Out of scope here, but worth a follow-up.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: convex/souls.ts
   Line: 127-137
   
   Comment:
   **Owner-scoped path still over-fetches**
   
   The `ownerUserId` branch still reads `limit * 5` rows and filters soft-deleted ones in JS — the exact pattern this PR fixes for the browse path. A compound index like `["ownerUserId", "softDeletedAt", "updatedAt"]` would let this branch take `limit` rows directly too. Out of scope here, but worth a follow-up.
   
   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/souls.ts
Line: 139-143

Comment:
**Silent sort-order change: `_creationTime` → `updatedAt`**

The old scan (no index, `.order("desc")`) sorted by `_creationTime` — Convex's default ordering. The new `by_active_updated` index is `["softDeletedAt", "updatedAt"]`, so after the equality constraint on `softDeletedAt` the remaining sort key is `updatedAt`. The browse listing will now return souls ordered by most-recently-*updated* rather than most-recently-*created*, which is a silent behavioral change. The PR description says "keeps the behavior the same," but the sort order is different. If this is intentional (aligning with `listPublicPage`'s `by_updated` ordering), please call it out explicitly in the description.

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/souls.ts
Line: 127-137

Comment:
**Owner-scoped path still over-fetches**

The `ownerUserId` branch still reads `limit * 5` rows and filters soft-deleted ones in JS — the exact pattern this PR fixes for the browse path. A compound index like `["ownerUserId", "softDeletedAt", "updatedAt"]` would let this branch take `limit` rows directly too. Out of scope here, but worth a follow-up.

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

_{Reviews (1): Last reviewed commit: "fix: reduce souls browse overfetch" | Re-trigger Greptile}

[greptile-apps]

Silent sort-order change: _creationTime → updatedAt

The old scan (no index, .order("desc")) sorted by _creationTime — Convex's default ordering. The new by_active_updated index is ["softDeletedAt", "updatedAt"], so after the equality constraint on softDeletedAt the remaining sort key is updatedAt. The browse listing will now return souls ordered by most-recently-updated rather than most-recently-created, which is a silent behavioral change. The PR description says "keeps the behavior the same," but the sort order is different. If this is intentional (aligning with listPublicPage's by_updated ordering), please call it out explicitly in the description.

Prompt To Fix With AI

This is a comment left during a code review.
Path: convex/souls.ts
Line: 139-143

Comment:
**Silent sort-order change: `_creationTime` → `updatedAt`**

The old scan (no index, `.order("desc")`) sorted by `_creationTime` — Convex's default ordering. The new `by_active_updated` index is `["softDeletedAt", "updatedAt"]`, so after the equality constraint on `softDeletedAt` the remaining sort key is `updatedAt`. The browse listing will now return souls ordered by most-recently-*updated* rather than most-recently-*created*, which is a silent behavioral change. The PR description says "keeps the behavior the same," but the sort order is different. If this is intentional (aligning with `listPublicPage`'s `by_updated` ordering), please call it out explicitly in the description.

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

[ImLukeF]

Thanks. I checked the ordering concern before merging.

For /souls, the route fetches the browse set and then applies client-side sorting, so the server-side browse order is not what drives that screen. Separately, the paginated public listing path already uses by_updated, so using the active+updated index here is acceptable and consistent with the broader soul listing direction.

I’m treating the owner-branch overfetch as follow-up scope, not a blocker for this patch.