From 8d5277dedb164018d193ad77c840d6df8583f083 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Mon, 14 Sep 2026 18:35:53 +1000 Subject: [PATCH] perf(collections): rank only the collection's repositories Collection detail ranked every Skill in the registry and ran a correlated repo Skill count per row before joining the collection: 1.69M rows read per call, 291.6M over 173 calls between 7 and 14 Sep 2026. The manifest and index handlers ranked the whole table too, at about 64K rows per call. The rank partitions by repository, so limiting the CTE to the collection's repositories leaves every rank unchanged. On production data collection 150 returns identical rows at 637 rows read, manifest at 471, index at 2,238. Claude-Session: https://claude.ai/code/session_014N85FDJLggfBs3BHJXT7GZ --- .../by-author/[login]/[slug]/index.get.ts | 18 ++- .../by-author/[login]/[slug]/manifest.get.ts | 4 + server/api/collections/index.get.ts | 6 +- test/unit/collection-detail.test.ts | 110 ++++++++++++++++++ 4 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 test/unit/collection-detail.test.ts diff --git a/server/api/collections/by-author/[login]/[slug]/index.get.ts b/server/api/collections/by-author/[login]/[slug]/index.get.ts index a8362435..9b835e33 100644 --- a/server/api/collections/by-author/[login]/[slug]/index.get.ts +++ b/server/api/collections/by-author/[login]/[slug]/index.get.ts @@ -24,13 +24,13 @@ export default defineApiHandler({ if (!collection) throw createError({ statusCode: 404, message: 'Collection not found' }) + // The CTE is limited to the collection's repositories. Unfiltered, it ranked + // every Skill in the registry and ran the correlated repo count for each, + // about 1.69M rows read per call. `rn` partitions by repository, so the + // filter leaves every rank unchanged. const skillsRes = await platform.db.prepare( `WITH ranked_skills AS ( SELECT s.owner, s.repo, s.name, s.display_name, - (SELECT COUNT(*) FROM skills repo_skills - WHERE repo_skills.owner = s.owner - AND repo_skills.repo = s.repo - AND repo_skills.source_resolved = 1) AS repo_skill_count, ROW_NUMBER() OVER ( PARTITION BY s.owner, s.repo ORDER BY s.modified_at DESC, s.name ASC @@ -39,9 +39,15 @@ export default defineApiHandler({ WHERE (r.broken_since IS NULL OR r.broken_since > unixepoch() - 604800) AND s.source_resolved = 1 AND s.rendered_status = 'ok' + AND (s.owner, s.repo) IN ( + SELECT owner, repo FROM collection_skills_v2 WHERE collection_id = ?1 + ) ) SELECT cs.position, cs.owner, cs.repo, rs.name, rs.display_name, cs.reason, - rs.repo_skill_count + (SELECT COUNT(*) FROM skills repo_skills + WHERE repo_skills.owner = cs.owner + AND repo_skills.repo = cs.repo + AND repo_skills.source_resolved = 1) AS repo_skill_count FROM collection_skills_v2 cs JOIN ranked_skills rs ON rs.owner = cs.owner @@ -50,7 +56,7 @@ export default defineApiHandler({ (cs.name IS NOT NULL AND rs.name = cs.name) OR (cs.name IS NULL AND rs.rn = 1) ) - WHERE cs.collection_id = ? + WHERE cs.collection_id = ?1 ORDER BY cs.position ASC`, ).bind(collection.id).all() diff --git a/server/api/collections/by-author/[login]/[slug]/manifest.get.ts b/server/api/collections/by-author/[login]/[slug]/manifest.get.ts index a836bd01..711e3d8e 100644 --- a/server/api/collections/by-author/[login]/[slug]/manifest.get.ts +++ b/server/api/collections/by-author/[login]/[slug]/manifest.get.ts @@ -41,6 +41,10 @@ export default defineApiHandler({ WHERE (r.broken_since IS NULL OR r.broken_since > unixepoch() - 604800) AND s.source_resolved = 1 AND s.rendered_status = 'ok' + -- Rank only the collection's repositories; \`rn\` is per repository. + AND (s.owner, s.repo) IN ( + SELECT owner, repo FROM collection_skills_v2 WHERE collection_id = ?1 + ) ) SELECT cs.owner, cs.repo, rs.name, cs.position, rs.target_package diff --git a/server/api/collections/index.get.ts b/server/api/collections/index.get.ts index 59da17ab..eb7d2b01 100644 --- a/server/api/collections/index.get.ts +++ b/server/api/collections/index.get.ts @@ -103,6 +103,10 @@ async function loadSkillLabels(db: D1Database, rows: CollectionIndexRow[]) { WHERE (r.broken_since IS NULL OR r.broken_since > unixepoch() - 604800) AND s.source_resolved = 1 AND s.rendered_status = 'ok' + -- Rank only the listed collections' repositories; \`rn\` is per repository. + AND (s.owner, s.repo) IN ( + SELECT owner, repo FROM collection_skills_v2 WHERE collection_id IN (${placeholders}) + ) ) SELECT cs.collection_id, cs.owner, cs.repo, rs.name, rs.display_name FROM collection_skills_v2 cs @@ -115,7 +119,7 @@ async function loadSkillLabels(db: D1Database, rows: CollectionIndexRow[]) { ) WHERE cs.collection_id IN (${placeholders}) ORDER BY cs.collection_id ASC, cs.position ASC`, - ).bind(...ids).all() + ).bind(...ids, ...ids).all() for (const skill of skillsRes.results ?? []) { const labels = skillsByCollection.get(skill.collection_id) ?? [] diff --git a/test/unit/collection-detail.test.ts b/test/unit/collection-detail.test.ts new file mode 100644 index 00000000..47b2feff --- /dev/null +++ b/test/unit/collection-detail.test.ts @@ -0,0 +1,110 @@ +import type { EventHandler, H3Event } from 'h3' +import Database from 'better-sqlite3' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +describe('collection detail endpoint', () => { + let sqlite: Database.Database + let event: H3Event + let detailHandler: EventHandler + + beforeEach(async () => { + vi.resetModules() + vi.stubGlobal('defineEventHandler', (handler: EventHandler) => handler) + sqlite = new Database(':memory:') + sqlite.exec(` + CREATE TABLE users (id INTEGER PRIMARY KEY, login TEXT NOT NULL, name TEXT, avatar TEXT); + CREATE TABLE collections_v2 ( + id INTEGER PRIMARY KEY, author_user_id INTEGER, slug TEXT NOT NULL, name TEXT NOT NULL, + preamble TEXT, featured INTEGER DEFAULT 0, created_at INTEGER, updated_at INTEGER, deleted_at INTEGER + ); + CREATE TABLE collection_skills_v2 ( + collection_id INTEGER NOT NULL, position INTEGER NOT NULL, + owner TEXT NOT NULL, repo TEXT NOT NULL, name TEXT, reason TEXT + ); + CREATE TABLE skills ( + owner TEXT NOT NULL, repo TEXT NOT NULL, name TEXT NOT NULL, display_name TEXT, + modified_at INTEGER, source_resolved INTEGER NOT NULL DEFAULT 0, rendered_status TEXT + ); + CREATE TABLE repos (owner TEXT NOT NULL, repo TEXT NOT NULL, broken_since INTEGER); + + INSERT INTO users VALUES (1, 'harlan', 'Harlan', NULL); + INSERT INTO collections_v2 VALUES (1, 1, 'stack', 'Stack', NULL, 0, 1, 1, NULL); + INSERT INTO collections_v2 VALUES (2, 1, 'other', 'Other', NULL, 0, 1, 1, NULL); + -- Collection 1 names one Skill and one whole repository. + INSERT INTO collection_skills_v2 VALUES (1, 1, 'acme', 'kit', 'older', 'named'); + INSERT INTO collection_skills_v2 VALUES (1, 2, 'solo', 'hub', NULL, 'repo'); + INSERT INTO collection_skills_v2 VALUES (2, 1, 'elsewhere', 'repo', 'x', NULL); + INSERT INTO skills VALUES ('acme', 'kit', 'older', 'Older', 100, 1, 'ok'); + INSERT INTO skills VALUES ('acme', 'kit', 'newer', 'Newer', 200, 1, 'ok'); + INSERT INTO skills VALUES ('solo', 'hub', 'stale', 'Stale', 100, 1, 'ok'); + INSERT INTO skills VALUES ('solo', 'hub', 'fresh', 'Fresh', 300, 1, 'ok'); + INSERT INTO skills VALUES ('solo', 'hub', 'unresolved', 'Unresolved', 900, 0, 'ok'); + INSERT INTO skills VALUES ('elsewhere', 'repo', 'x', 'X', 999, 1, 'ok'); + INSERT INTO repos VALUES ('acme', 'kit', NULL); + INSERT INTO repos VALUES ('solo', 'hub', NULL); + INSERT INTO repos VALUES ('elsewhere', 'repo', NULL); + `) + + event = { + method: 'GET', + context: { + platform: { db: wrapSqlite(sqlite) }, + params: { login: 'harlan', slug: 'stack' }, + }, + node: { req: { headers: {} } }, + } as unknown as H3Event + vi.stubGlobal('getUserSession', () => Promise.resolve(null)) + vi.stubGlobal('getRouterParam', (_event: H3Event, key: string) => event.context.params?.[key]) + detailHandler = (await import('../../server/api/collections/by-author/[login]/[slug]/index.get')).default + }) + + afterEach(() => { + sqlite.close() + vi.unstubAllGlobals() + }) + + it('resolves named Skills and repository entries with resolved repo counts', async () => { + const result = await detailHandler(event) as { skills: Array> } + + expect(result.skills.map(skill => ({ + position: skill.position, + owner: skill.owner, + repo: skill.repo, + name: skill.name, + displayName: skill.displayName, + registryPath: skill.registryPath, + }))).toEqual([ + { position: 1, owner: 'acme', repo: 'kit', name: 'older', displayName: 'Older', registryPath: '/gh/acme/kit/older' }, + { position: 2, owner: 'solo', repo: 'hub', name: 'fresh', displayName: 'Fresh', registryPath: '/gh/solo/hub/fresh' }, + ]) + }) +}) + +function wrapSqlite(sqlite: Database.Database) { + return { + prepare(sql: string) { + return { + bind(...params: unknown[]) { + const prepared = expandNumberedPlaceholders(sql, params) + return { + async all() { + return { results: sqlite.prepare(prepared.sql).all(...prepared.params) as T[] } + }, + async first() { + return (sqlite.prepare(prepared.sql).get(...prepared.params) as T | undefined) ?? null + }, + } + }, + } + }, + } +} + +function expandNumberedPlaceholders(sql: string, params: unknown[]): { sql: string, params: unknown[] } { + const expanded: unknown[] = [] + const nextSql = sql.replace(/\?(\d+)/g, (_, raw: string) => { + expanded.push(params[Number(raw) - 1]) + return '?' + }) + return expanded.length ? { sql: nextSql, params: expanded } : { sql, params } +}