Skip to content

perf: batch tag resolution to reduce action→query round-trips #112

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
mkrokosz wants to merge 2 commits into
base: main
Choose a base branch
from
+221 −63
Open

perf: batch tag resolution to reduce action→query round-trips #112

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f96bee
perf: batch tag resolution to reduce action→query round-trips
mkrokosz Feb 3, 2026
2be0a5e
fix: add null guard and short-circuit for empty tags
mkrokosz Feb 3, 2026
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
80 changes: 77 additions & 3 deletions convex/httpApiV1.handlers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe('httpApiV1 handlers', () => {
expect(json.match.version).toBe('1.0.0')
})

it('lists skills with resolved tags', async () => {
it('lists skills with resolved tags using batch query', async () => {
const runQuery = vi.fn(async (_query: unknown, args: Record<string, unknown>) => {
if ('cursor' in args || 'limit' in args) {
return {
Expand All @@ -145,7 +145,10 @@ describe('httpApiV1 handlers', () => {
nextCursor: null,
}
}
if ('versionId' in args) return { version: '1.0.0' }
// Batch query: versionIds (plural)
if ('versionIds' in args) {
return [{ _id: 'versions:1', version: '1.0.0', softDeletedAt: undefined }]
}
return null
})
const runMutation = vi.fn().mockResolvedValue(okRate())
Expand All @@ -158,6 +161,74 @@ describe('httpApiV1 handlers', () => {
expect(json.items[0].tags.latest).toBe('1.0.0')
})

it('batches tag resolution across multiple skills into single query', async () => {
const runQuery = vi.fn(async (_query: unknown, args: Record<string, unknown>) => {
if ('cursor' in args || 'limit' in args) {
return {
items: [
{
skill: {
_id: 'skills:1',
slug: 'skill-a',
displayName: 'Skill A',
summary: 's',
tags: { latest: 'versions:1', stable: 'versions:2' },
stats: { downloads: 0, stars: 0, versions: 2, comments: 0 },
createdAt: 1,
updatedAt: 2,
},
latestVersion: { version: '2.0.0', createdAt: 3, changelog: 'c' },
},
{
skill: {
_id: 'skills:2',
slug: 'skill-b',
displayName: 'Skill B',
summary: 's',
tags: { latest: 'versions:3' },
stats: { downloads: 0, stars: 0, versions: 1, comments: 0 },
createdAt: 1,
updatedAt: 2,
},
latestVersion: { version: '1.0.0', createdAt: 3, changelog: 'c' },
},
],
nextCursor: null,
}
}
// Batch query should receive all version IDs from all skills
if ('versionIds' in args) {
const ids = args.versionIds as string[]
expect(ids).toHaveLength(3)
expect(ids).toContain('versions:1')
expect(ids).toContain('versions:2')
expect(ids).toContain('versions:3')
return [
{ _id: 'versions:1', version: '2.0.0', softDeletedAt: undefined },
{ _id: 'versions:2', version: '1.0.0', softDeletedAt: undefined },
{ _id: 'versions:3', version: '1.0.0', softDeletedAt: undefined },
]
}
return null
})
const runMutation = vi.fn().mockResolvedValue(okRate())
const response = await __handlers.listSkillsV1Handler(
makeCtx({ runQuery, runMutation }),
new Request('https://example.com/api/v1/skills'),
)
expect(response.status).toBe(200)
const json = await response.json()
// Verify tags are correctly resolved for each skill
expect(json.items[0].tags.latest).toBe('2.0.0')
expect(json.items[0].tags.stable).toBe('1.0.0')
expect(json.items[1].tags.latest).toBe('1.0.0')
// Verify batch query was called exactly once (not per-tag)
const batchCalls = runQuery.mock.calls.filter(
([, args]) => args && 'versionIds' in (args as Record<string, unknown>),
)
expect(batchCalls).toHaveLength(1)
})

it('lists skills supports sort aliases', async () => {
const checks: Array<[string, string]> = [
['rating', 'stars'],
Expand Down Expand Up @@ -216,7 +287,10 @@ describe('httpApiV1 handlers', () => {
owner: { handle: 'p', displayName: 'Peter', image: null },
}
}
if ('versionId' in args) return { version: '1.0.0' }
// Batch query for tag resolution
if ('versionIds' in args) {
return [{ _id: 'versions:1', version: '1.0.0', softDeletedAt: undefined }]
}
return null
})
const runMutation = vi.fn().mockResolvedValue(okRate())
Expand Down
176 changes: 116 additions & 60 deletions convex/httpApiV1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,28 +204,29 @@ async function listSkillsV1Handler(ctx: ActionCtx, request: Request) {
sort,
})) as ListSkillsResult

const items = await Promise.all(
result.items.map(async (item) => {
const tags = await resolveTags(ctx, item.skill.tags)
return {
slug: item.skill.slug,
displayName: item.skill.displayName,
summary: item.skill.summary ?? null,
tags,
stats: item.skill.stats,
createdAt: item.skill.createdAt,
updatedAt: item.skill.updatedAt,
latestVersion: item.latestVersion
? {
version: item.latestVersion.version,
createdAt: item.latestVersion.createdAt,
changelog: item.latestVersion.changelog,
}
: null,
}
}),
// Batch resolve all tags in a single query instead of N queries
const resolvedTagsList = await resolveTagsBatch(
ctx,
result.items.map((item) => item.skill.tags),
)

const items = result.items.map((item, idx) => ({
slug: item.skill.slug,
displayName: item.skill.displayName,
summary: item.skill.summary ?? null,
tags: resolvedTagsList[idx],
stats: item.skill.stats,
createdAt: item.skill.createdAt,
updatedAt: item.skill.updatedAt,
latestVersion: item.latestVersion
? {
version: item.latestVersion.version,
createdAt: item.latestVersion.createdAt,
changelog: item.latestVersion.changelog,
}
: null,
}))

return json({ items, nextCursor: result.nextCursor ?? null }, 200, rate.headers)
}

Expand All @@ -245,7 +246,7 @@ async function skillsGetRouterV1Handler(ctx: ActionCtx, request: Request) {
const result = (await ctx.runQuery(api.skills.getBySlug, { slug })) as GetBySlugResult
if (!result?.skill) return text('Skill not found', 404, rate.headers)

const tags = await resolveTags(ctx, result.skill.tags)
const [tags] = await resolveTagsBatch(ctx, [result.skill.tags])
return json(
{
skill: {
Expand Down Expand Up @@ -706,32 +707,86 @@ function parsePublishBody(body: unknown) {
}
}

async function resolveSoulTags(
/**
* Batch resolve soul version tags to version strings.
* Collects all version IDs, fetches them in a single query, then maps back.
* Reduces N sequential queries to 1 batch query.
*/
async function resolveSoulTagsBatch(
ctx: ActionCtx,
tags: Record<string, Id<'soulVersions'>>,
): Promise<Record<string, string>> {
const resolved: Record<string, string> = {}
for (const [tag, versionId] of Object.entries(tags)) {
const version = await ctx.runQuery(api.souls.getVersionById, { versionId })
if (version && !version.softDeletedAt) {
resolved[tag] = version.version
tagsList: Array<Record<string, Id<'soulVersions'>>>,
): Promise<Array<Record<string, string>>> {
// Collect all unique version IDs
const allVersionIds = new Set<Id<'soulVersions'>>()
for (const tags of tagsList) {
for (const versionId of Object.values(tags)) {
allVersionIds.add(versionId)
}
}
return resolved

// Single batch query
const versions = await ctx.runQuery(api.souls.getVersionsByIds, {
versionIds: [...allVersionIds],
})

// Build lookup map: versionId -> version string
const versionMap = new Map<Id<'soulVersions'>, string>()
for (const v of versions) {
if (v && !v.softDeletedAt) {
versionMap.set(v._id, v.version)
}
}

// Resolve each tags record
return tagsList.map((tags) => {
const resolved: Record<string, string> = {}
for (const [tag, versionId] of Object.entries(tags)) {
const version = versionMap.get(versionId)
if (version) resolved[tag] = version
}
return resolved
})
}

async function resolveTags(
/**
* Batch resolve skill version tags to version strings.
* Collects all version IDs, fetches them in a single query, then maps back.
* Reduces N sequential queries to 1 batch query.
*/
async function resolveTagsBatch(
ctx: ActionCtx,
tags: Record<string, Id<'skillVersions'>>,
): Promise<Record<string, string>> {
const resolved: Record<string, string> = {}
for (const [tag, versionId] of Object.entries(tags)) {
const version = await ctx.runQuery(api.skills.getVersionById, { versionId })
if (version && !version.softDeletedAt) {
resolved[tag] = version.version
tagsList: Array<Record<string, Id<'skillVersions'>>>,
): Promise<Array<Record<string, string>>> {
// Collect all unique version IDs
const allVersionIds = new Set<Id<'skillVersions'>>()
for (const tags of tagsList) {
for (const versionId of Object.values(tags)) {
allVersionIds.add(versionId)
}
}
return resolved

// Single batch query
const versions = await ctx.runQuery(api.skills.getVersionsByIds, {
versionIds: [...allVersionIds],
})

// Build lookup map: versionId -> version string
const versionMap = new Map<Id<'skillVersions'>, string>()
for (const v of versions) {
if (v && !v.softDeletedAt) {
versionMap.set(v._id, v.version)
}
}

// Resolve each tags record
return tagsList.map((tags) => {
const resolved: Record<string, string> = {}
for (const [tag, versionId] of Object.entries(tags)) {
const version = versionMap.get(versionId)
if (version) resolved[tag] = version
}
return resolved
})
}

async function applyRateLimit(
Expand Down Expand Up @@ -914,28 +969,29 @@ async function listSoulsV1Handler(ctx: ActionCtx, request: Request) {
cursor,
})) as ListSoulsResult

const items = await Promise.all(
result.items.map(async (item) => {
const tags = await resolveSoulTags(ctx, item.soul.tags)
return {
slug: item.soul.slug,
displayName: item.soul.displayName,
summary: item.soul.summary ?? null,
tags,
stats: item.soul.stats,
createdAt: item.soul.createdAt,
updatedAt: item.soul.updatedAt,
latestVersion: item.latestVersion
? {
version: item.latestVersion.version,
createdAt: item.latestVersion.createdAt,
changelog: item.latestVersion.changelog,
}
: null,
}
}),
// Batch resolve all tags in a single query instead of N queries
const resolvedTagsList = await resolveSoulTagsBatch(
ctx,
result.items.map((item) => item.soul.tags),
)

const items = result.items.map((item, idx) => ({
slug: item.soul.slug,
displayName: item.soul.displayName,
summary: item.soul.summary ?? null,
tags: resolvedTagsList[idx],
stats: item.soul.stats,
createdAt: item.soul.createdAt,
updatedAt: item.soul.updatedAt,
latestVersion: item.latestVersion
? {
version: item.latestVersion.version,
createdAt: item.latestVersion.createdAt,
changelog: item.latestVersion.changelog,
}
: null,
}))

return json({ items, nextCursor: result.nextCursor ?? null }, 200, rate.headers)
}

Expand All @@ -955,7 +1011,7 @@ async function soulsGetRouterV1Handler(ctx: ActionCtx, request: Request) {
const result = (await ctx.runQuery(api.souls.getBySlug, { slug })) as GetSoulBySlugResult
if (!result?.soul) return text('Soul not found', 404, rate.headers)

const tags = await resolveSoulTags(ctx, result.soul.tags)
const [tags] = await resolveSoulTagsBatch(ctx, [result.soul.tags])
return json(
{
soul: {
Expand Down
8 changes: 8 additions & 0 deletions convex/skills.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,14 @@ export const getVersionById = query({
handler: async (ctx, args) => ctx.db.get(args.versionId),
})

export const getVersionsByIds = query({
args: { versionIds: v.array(v.id('skillVersions')) },
handler: async (ctx, args) => {
const versions = await Promise.all(args.versionIds.map((id) => ctx.db.get(id)))
return versions.filter((v): v is NonNullable<typeof v> => v !== null)
},
})
Comment on lines +1128 to +1134
Copy link

@greptile-apps greptile-apps bot Feb 3, 2026

Choose a reason for hiding this comment

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

[P2] getVersionsByIds uses Promise.all(versionIds.map(ctx.db.get)), which will issue one db.get per ID anyway. This reduces action→query round-trips (good), but inside the query it can still be a large fan-out (up to all tags across the page). If versionIds can be large, consider adding a conservative cap/deduplication at the query boundary, or using an index-based fetch strategy if available.

Also appears in convex/souls.ts:148-154.

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

Comment:
[P2] `getVersionsByIds` uses `Promise.all(versionIds.map(ctx.db.get))`, which will issue one `db.get` per ID anyway. This reduces action→query round-trips (good), but inside the query it can still be a large fan-out (up to all tags across the page). If `versionIds` can be large, consider adding a conservative cap/deduplication at the query boundary, or using an index-based fetch strategy if available.

Also appears in `convex/souls.ts:148-154`.

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


export const getVersionByIdInternal = internalQuery({
args: { versionId: v.id('skillVersions') },
handler: async (ctx, args) => ctx.db.get(args.versionId),
Expand Down
8 changes: 8 additions & 0 deletions convex/souls.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,14 @@ export const getVersionById = query({
handler: async (ctx, args) => ctx.db.get(args.versionId),
})

export const getVersionsByIds = query({
args: { versionIds: v.array(v.id('soulVersions')) },
handler: async (ctx, args) => {
const versions = await Promise.all(args.versionIds.map((id) => ctx.db.get(id)))
return versions.filter((v): v is NonNullable<typeof v> => v !== null)
},
})

export const getVersionByIdInternal = internalQuery({
args: { versionId: v.id('soulVersions') },
handler: async (ctx, args) => ctx.db.get(args.versionId),
Expand Down