perf(d1): avoid catalog scans for admissions

layers/registry/server/utils/trending-admission.ts

@@ -279,16 +279,40 @@ export async function listAdmittedSkills(
   opts: { board: TrendingBoardName, page: number, indexable: ReadonlyArray<{ owner: string, repo: string, name: string }> },
 ): Promise<{ rows: AdmittedSkillRow[], total: number }> {
   const indexable = new Set(opts.indexable.map(skill => `${skill.owner}/${skill.repo}/${skill.name}`))
+  // Drive the small admitted set, then look up each Skill and Repository.
+  // Reordered joins scanned every resolved Skill before checking admission.
   const result = await db.prepare(`
-    SELECT s.owner, s.repo, s.name, s.description, r.stars, a.admitted_at,
-      (SELECT COUNT(*) FROM skills c WHERE c.owner = s.owner AND c.repo = s.repo AND c.source_resolved = 1) AS repo_skill_count
+    SELECT s.owner, s.repo, s.name, s.description, r.stars, a.admitted_at
     FROM skill_trending_admissions a
-    JOIN skills s ON s.owner = a.owner AND s.repo = a.repo AND s.name = a.name
-    JOIN repos r ON r.owner = s.owner AND r.repo = s.repo
+    CROSS JOIN skills s ON s.owner = a.owner AND s.repo = a.repo AND s.name = a.name
+    CROSS JOIN repos r ON r.owner = s.owner AND r.repo = s.repo
     WHERE a.first_board = ?1 AND s.source_resolved = 1
     ORDER BY a.admitted_at DESC, s.owner, s.repo, s.name
-  `).bind(opts.board).all<AdmittedSkillRow>()
+  `).bind(opts.board).all<Omit<AdmittedSkillRow, 'repo_skill_count'>>()
   const rows = (result.results ?? []).filter(row => indexable.has(`${row.owner}/${row.repo}/${row.name}`))
   const start = (opts.page - 1) * ADMITTED_PAGE_SIZE
-  return { rows: rows.slice(start, start + ADMITTED_PAGE_SIZE), total: rows.length }
+  const page = rows.slice(start, start + ADMITTED_PAGE_SIZE)
+  if (page.length === 0)
+    return { rows: [], total: rows.length }
+
+  const repositories = [...new Map(page.map(row => [
+    `${row.owner}/${row.repo}`,
+    { owner: row.owner, repo: row.repo },
+  ])).values()]
+  // Count once per returned Repository, including unadmitted resolved siblings.
+  // One JSON binding keeps a full page inside D1's parameter limit.
+  const counts = await db.prepare(`
+    SELECT json_extract(requested.value, '$.owner') AS owner,
+      json_extract(requested.value, '$.repo') AS repo,
+      (SELECT COUNT(*) FROM skills c
+       WHERE c.owner = json_extract(requested.value, '$.owner')
+         AND c.repo = json_extract(requested.value, '$.repo')
+         AND c.source_resolved = 1) AS repo_skill_count
+    FROM json_each(?1) requested
+  `).bind(JSON.stringify(repositories)).all<Pick<AdmittedSkillRow, 'owner' | 'repo' | 'repo_skill_count'>>()
+  const countByRepository = new Map(counts.results.map(row => [`${row.owner}/${row.repo}`, row.repo_skill_count]))
+  return {
+    rows: page.map(row => ({ ...row, repo_skill_count: countByRepository.get(`${row.owner}/${row.repo}`)! })),
+    total: rows.length,
+  }
 }

test/unit/admitted-skills-cost.test.ts

@@ -0,0 +1,108 @@
+import { describe, expect, it } from 'vitest'
+import { listAdmittedSkills } from '../../layers/registry/server/utils/trending-admission'
+import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite'
+
+describe('admitted Skill paging cost', () => {
+  it('counts all resolved siblings only for repositories on the returned page', async () => {
+    const harness = createSqliteD1([])
+    const { raw } = harness
+    let resolutionReads = 0
+    raw.function('tracked_resolution', { deterministic: true }, (_owner: unknown, _repo: unknown, _name: unknown) => {
+      resolutionReads++
+      return 1
+    })
+    raw.exec(`
+      CREATE TABLE repos (owner TEXT, repo TEXT, stars INTEGER, PRIMARY KEY (owner, repo));
+      CREATE TABLE skills (
+        owner TEXT, repo TEXT, name TEXT, description TEXT,
+        source_resolved INTEGER GENERATED ALWAYS AS (tracked_resolution(owner, repo, name)) VIRTUAL,
+        PRIMARY KEY (owner, repo, name)
+      );
+      CREATE TABLE skill_trending_admissions (
+        owner TEXT, repo TEXT, name TEXT, first_board TEXT, admitted_at INTEGER,
+        PRIMARY KEY (owner, repo, name)
+      );
+      INSERT INTO repos VALUES ('acme', 'tools', 10);
+    `)
+    const insertSkill = raw.prepare('INSERT INTO skills (owner, repo, name, description) VALUES (\'acme\', \'tools\', ?, \'Description\')')
+    const admit = raw.prepare('INSERT INTO skill_trending_admissions VALUES (\'acme\', \'tools\', ?, \'week\', ?)')
+    for (let index = 0; index < 300; index++) {
+      const name = `skill-${String(index).padStart(3, '0')}`
+      insertSkill.run(name)
+      if (index < 65)
+        admit.run(name, 1000 + index)
+    }
+    resolutionReads = 0
+    try {
+      const result = await listAdmittedSkills(harness.db, {
+        board: 'week',
+        page: 1,
+        indexable: ['skill-000', 'skill-001'].map(name => ({ owner: 'acme', repo: 'tools', name })),
+      })
+      expect(result).toEqual({
+        rows: [
+          { owner: 'acme', repo: 'tools', name: 'skill-001', description: 'Description', stars: 10, admitted_at: 1001, repo_skill_count: 300 },
+          { owner: 'acme', repo: 'tools', name: 'skill-000', description: 'Description', stars: 10, admitted_at: 1000, repo_skill_count: 300 },
+        ],
+        total: 2,
+      })
+      // One eligibility pass and one sibling count suffice for this page.
+      // Virtual source metadata makes the database reads observable.
+      expect(resolutionReads).toBeLessThanOrEqual(400)
+    }
+    finally {
+      raw.close()
+    }
+  })
+
+  it('pages tied admissions after filtering and retains full sibling counts', async () => {
+    const harness = createSqliteD1(allMigrations())
+    const { raw } = harness
+    const insertRepo = raw.prepare('INSERT INTO repos (owner, repo, stars) VALUES (?, ?, 10)')
+    const insertSkill = raw.prepare(`INSERT INTO skills
+      (owner, repo, name, slug, display_name, description, source_resolved)
+      VALUES (?, ?, ?, ?, ?, 'Description', ?)`)
+    const admit = raw.prepare(`INSERT INTO skill_trending_admissions
+      (owner, repo, name, first_board, admitted_at) VALUES (?, ?, ?, ?, ?)`)
+    const indexable: Array<{ owner: string, repo: string, name: string }> = []
+    const seed = (owner: string, repo: string, name: string, resolved = 1) =>
+      insertSkill.run(owner, repo, name, `${owner}/${repo}/${name}`, name, resolved)
+    for (let index = 0; index < 55; index++) {
+      const owner = `owner-${String(index).padStart(3, '0')}`
+      const repo = 'tools'
+      const name = 'admitted'
+      insertRepo.run(owner, repo)
+      seed(owner, repo, name)
+      admit.run(owner, repo, name, 'week', 1000)
+      indexable.push({ owner, repo, name })
+    }
+    // These newer rows must not consume page positions.
+    insertRepo.run('excluded', 'tools')
+    seed('excluded', 'tools', 'not-indexable')
+    admit.run('excluded', 'tools', 'not-indexable', 'week', 2000)
+    seed('excluded', 'tools', 'unresolved', 0)
+    admit.run('excluded', 'tools', 'unresolved', 'week', 2000)
+    indexable.push({ owner: 'excluded', repo: 'tools', name: 'unresolved' })
+    seed('excluded', 'tools', 'other-board')
+    admit.run('excluded', 'tools', 'other-board', 'month', 2000)
+    indexable.push({ owner: 'excluded', repo: 'tools', name: 'other-board' })
+    // Siblings outside admissions still decide the canonical repository URL.
+    seed('owner-050', 'tools', 'unadmitted-sibling')
+    seed('owner-050', 'tools', 'unresolved-sibling', 0)
+    try {
+      const first = await listAdmittedSkills(harness.db, { board: 'week', page: 1, indexable })
+      const second = await listAdmittedSkills(harness.db, { board: 'week', page: 2, indexable })
+      const empty = await listAdmittedSkills(harness.db, { board: 'week', page: 3, indexable })
+      expect(first.rows.map(row => row.owner)).toEqual(indexable.slice(0, 50).map(row => row.owner))
+      expect(first.rows.every(row => row.repo_skill_count === 1)).toBe(true)
+      expect(first.total).toBe(55)
+      expect(second.rows.map(row => row.owner)).toEqual(indexable.slice(50, 55).map(row => row.owner))
+      expect(second.rows.map(row => row.repo_skill_count)).toEqual([2, 1, 1, 1, 1])
+      expect(second.total).toBe(55)
+      expect(empty).toEqual({ rows: [], total: 55 })
+    }
+    finally {
+      harness.close()
+    }
+  })
+})