Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
9 changes: 9 additions & 0 deletions convex/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,15 @@ const skills = defineTable({
.index('by_stats_installs_all_time', ['statsInstallsAllTime', 'updatedAt'])
.index('by_batch', ['batch'])
.index('by_active_updated', ['softDeletedAt', 'updatedAt'])
.index('by_active_created', ['softDeletedAt', 'createdAt'])
.index('by_active_name', ['softDeletedAt', 'displayName'])
.index('by_active_stats_downloads', ['softDeletedAt', 'statsDownloads', 'updatedAt'])
.index('by_active_stats_stars', ['softDeletedAt', 'statsStars', 'updatedAt'])
.index('by_active_stats_installs_all_time', [
'softDeletedAt',
'statsInstallsAllTime',
'updatedAt',
])

const souls = defineTable({
slug: v.string(),
Expand Down
32 changes: 28 additions & 4 deletions convex/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ const MAX_PUBLIC_LIST_LIMIT = 200
const MAX_LIST_BULK_LIMIT = 200
const MAX_LIST_TAKE = 1000

const SORT_INDEXES = {
newest: 'by_active_created',
updated: 'by_active_updated',
name: 'by_active_name',
downloads: 'by_active_stats_downloads',
stars: 'by_active_stats_stars',
installs: 'by_active_stats_installs_all_time',
} as const

function isSkillVersionId(
value: Id<'skillVersions'> | null | undefined,
): value is Id<'skillVersions'> {
Expand Down Expand Up @@ -162,7 +171,8 @@ async function hardDeleteSkill(
if (related._id === skill._id) continue
if (related.canonicalSkillId === skill._id || related.forkOf?.skillId === skill._id) {
await ctx.db.patch(related._id, {
canonicalSkillId: related.canonicalSkillId === skill._id ? undefined : related.canonicalSkillId,
canonicalSkillId:
related.canonicalSkillId === skill._id ? undefined : related.canonicalSkillId,
forkOf: related.forkOf?.skillId === skill._id ? undefined : related.forkOf,
updatedAt: now,
})
Expand Down Expand Up @@ -720,14 +730,28 @@ export const listPublicPage = query({
export const listPublicPageV2 = query({
args: {
paginationOpts: paginationOptsValidator,
sort: v.optional(
v.union(
v.literal('newest'),
v.literal('updated'),
v.literal('downloads'),
v.literal('installs'),
v.literal('stars'),
v.literal('name'),
),
),
dir: v.optional(v.union(v.literal('asc'), v.literal('desc'))),
},
handler: async (ctx, args) => {
// Use the new index to filter out soft-deleted skills at query time.
const sort = args.sort ?? 'newest'
const dir = args.dir ?? (sort === 'name' ? 'asc' : 'desc')

// Use the index to filter out soft-deleted skills at query time.
// softDeletedAt === undefined means active (non-deleted) skills only.
const result = await paginator(ctx.db, schema)
.query('skills')
.withIndex('by_active_updated', (q) => q.eq('softDeletedAt', undefined))
.order('desc')
.withIndex(SORT_INDEXES[sort], (q) => q.eq('softDeletedAt', undefined))
.order(dir)
.paginate(args.paginationOpts)
Comment on lines 1042 to 1067
Copy link

Choose a reason for hiding this comment

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

[P1] listPublicPageV2 can return a different order than the client expects when sorting by stats, because the index tie-breaker fields don’t match the client-side comparator.

For downloads/stars/installs, the index is statsX, updatedAt but the UI’s results.sort only compares the primary stat (and doesn’t fall back to updatedAt / slug). This means within equal-stat groups you’ll get a stable-but-different ordering depending on whether results came from server pagination vs the client search path.

This matters when switching between search and non-search views or when many items share the same stat value (often 0). Consider aligning the server index/order with the UI’s full comparator (e.g., add deterministic secondary keys) or updating the client comparator to match the server ordering.

Also appears in: src/routes/skills/index.tsx:169-189

Prompt To Fix With AI
This is a comment left during a code review.
Path: convex/skills.ts
Line: 730:755

Comment:
[P1] `listPublicPageV2` can return a different order than the client expects when sorting by stats, because the index tie-breaker fields don’t match the client-side comparator.

For `downloads`/`stars`/`installs`, the index is `statsX, updatedAt` but the UI’s `results.sort` only compares the primary stat (and doesn’t fall back to `updatedAt` / slug). This means within equal-stat groups you’ll get a stable-but-different ordering depending on whether results came from server pagination vs the client search path.

This matters when switching between search and non-search views or when many items share the same stat value (often 0). Consider aligning the server index/order with the UI’s full comparator (e.g., add deterministic secondary keys) or updating the client comparator to match the server ordering.

Also appears in: `src/routes/skills/index.tsx:169-189`

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


// Build the public skill entries (fetch latestVersion + ownerHandle)
Expand Down
24 changes: 21 additions & 3 deletions convex/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,25 @@
{
"extends": "../tsconfig.json",
/* This TypeScript project config describes the environment that
* Convex functions run in and is used to typecheck them.
* You can modify it, but some settings are required to use Convex.
*/
"compilerOptions": {
/* These settings are not required by Convex and can be modified. */
"allowJs": true,
"strict": true,
"moduleResolution": "Bundler",
"skipLibCheck": true
}
"jsx": "react-jsx",
"skipLibCheck": true,
"allowSyntheticDefaultImports": true,

/* These compiler options are required by Convex */
"target": "ESNext",
"lib": ["ES2022", "dom"],
"forceConsistentCasingInFileNames": true,
"module": "ESNext",
"isolatedModules": true,
"noEmit": true
},
"include": ["./**/*"],
"exclude": ["./_generated"]
}
4 changes: 2 additions & 2 deletions src/__tests__/skills-index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ describe('SkillsIndex', () => {

it('requests the first skills page', () => {
render(<SkillsIndex />)
// usePaginatedQuery should be called with the API endpoint and empty args
// usePaginatedQuery should be called with the API endpoint and sort/dir args
expect(usePaginatedQueryMock).toHaveBeenCalledWith(
expect.anything(),
{},
{ sort: 'newest', dir: 'desc' },
{ initialNumItems: 25 },
)
})
Expand Down
2 changes: 1 addition & 1 deletion src/lib/og.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getOnlyCrabsSiteUrl, getClawHubSiteUrl } from './site'
import { getClawHubSiteUrl, getOnlyCrabsSiteUrl } from './site'

type SkillMetaSource = {
slug: string
Expand Down
2 changes: 1 addition & 1 deletion src/lib/site.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { afterEach, describe, expect, it, vi } from 'vitest'
import {
detectSiteMode,
detectSiteModeFromUrl,
getClawHubSiteUrl,
getOnlyCrabsHost,
getOnlyCrabsSiteUrl,
getClawHubSiteUrl,
getSiteDescription,
getSiteMode,
getSiteName,
Expand Down
7 changes: 5 additions & 2 deletions src/routes/skills/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export function SkillsIndex() {
results: paginatedResults,
status: paginationStatus,
loadMore: loadMorePaginated,
} = usePaginatedQuery(api.skills.listPublicPageV2, hasQuery ? 'skip' : {}, {
} = usePaginatedQuery(api.skills.listPublicPageV2, hasQuery ? 'skip' : { sort, dir }, {
initialNumItems: pageSize,
})

Expand Down Expand Up @@ -161,6 +161,9 @@ export function SkillsIndex() {
)

const sorted = useMemo(() => {
if (!hasQuery) {
return filtered
}
const multiplier = dir === 'asc' ? 1 : -1
const results = [...filtered]
results.sort((a, b) => {
Expand All @@ -186,7 +189,7 @@ export function SkillsIndex() {
}
})
return results
}, [dir, filtered, sort])
}, [dir, filtered, hasQuery, sort])

const isLoadingSkills = hasQuery ? isSearching && searchResults.length === 0 : isLoadingList
const canLoadMore = hasQuery
Expand Down