diff --git a/convex/httpApiV1.handlers.test.ts b/convex/httpApiV1.handlers.test.ts index 5864b85..99894c5 100644 --- a/convex/httpApiV1.handlers.test.ts +++ b/convex/httpApiV1.handlers.test.ts @@ -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) => { if ('cursor' in args || 'limit' in args) { return { @@ -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()) @@ -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) => { + 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), + ) + expect(batchCalls).toHaveLength(1) + }) + it('lists skills supports sort aliases', async () => { const checks: Array<[string, string]> = [ ['rating', 'stars'], @@ -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()) diff --git a/convex/httpApiV1.ts b/convex/httpApiV1.ts index 69e5c9e..276ba83 100644 --- a/convex/httpApiV1.ts +++ b/convex/httpApiV1.ts @@ -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) } @@ -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: { @@ -706,32 +707,98 @@ 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>, -): Promise> { - const resolved: Record = {} - 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>>, +): Promise>> { + // Collect all unique version IDs + const allVersionIds = new Set>() + for (const tags of tagsList) { + for (const versionId of Object.values(tags)) { + allVersionIds.add(versionId) } } - return resolved + + // Short-circuit if no tags to resolve + if (allVersionIds.size === 0) { + return tagsList.map(() => ({})) + } + + // Single batch query + const versions = + (await ctx.runQuery(api.souls.getVersionsByIds, { + versionIds: [...allVersionIds], + })) ?? [] + + // Build lookup map: versionId -> version string + const versionMap = new Map, 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 = {} + 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>, -): Promise> { - const resolved: Record = {} - 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>>, +): Promise>> { + // Collect all unique version IDs + const allVersionIds = new Set>() + for (const tags of tagsList) { + for (const versionId of Object.values(tags)) { + allVersionIds.add(versionId) } } - return resolved + + // Short-circuit if no tags to resolve + if (allVersionIds.size === 0) { + return tagsList.map(() => ({})) + } + + // Single batch query + const versions = + (await ctx.runQuery(api.skills.getVersionsByIds, { + versionIds: [...allVersionIds], + })) ?? [] + + // Build lookup map: versionId -> version string + const versionMap = new Map, 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 = {} + for (const [tag, versionId] of Object.entries(tags)) { + const version = versionMap.get(versionId) + if (version) resolved[tag] = version + } + return resolved + }) } async function applyRateLimit( @@ -914,28 +981,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) } @@ -955,7 +1023,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: { diff --git a/convex/skills.ts b/convex/skills.ts index 44be105..fddcace 100644 --- a/convex/skills.ts +++ b/convex/skills.ts @@ -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 => v !== null) + }, +}) + export const getVersionByIdInternal = internalQuery({ args: { versionId: v.id('skillVersions') }, handler: async (ctx, args) => ctx.db.get(args.versionId), diff --git a/convex/souls.ts b/convex/souls.ts index e0e3ebd..0e0c1a3 100644 --- a/convex/souls.ts +++ b/convex/souls.ts @@ -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 => v !== null) + }, +}) + export const getVersionByIdInternal = internalQuery({ args: { versionId: v.id('soulVersions') }, handler: async (ctx, args) => ctx.db.get(args.versionId),