perf(skills): index source ownership counts per discovery (#20259)

Co-authored-by: m4air <m4air@Mac.localdomain>

config/scripts/skill-source-inventory-benchmark.mjs

@@ -0,0 +1,149 @@
+import assert from 'node:assert/strict'
+import { readFileSync } from 'node:fs'
+import { performance } from 'node:perf_hooks'
+import { transform } from 'esbuild'
+import { buildCounterbalancedSchedule } from './counterbalanced-benchmark-schedule.mjs'
+
+const path = 'src/renderer/src/components/skills/skill-source-inventory.ts'
+const arms = {}
+for (const [name, source] of [
+  ['baseline', readFileSync(0, 'utf8')],
+  ['indexed', readFileSync(path, 'utf8')]
+]) {
+  const { code } = await transform(source, { loader: 'ts', format: 'esm' })
+  const loaded = await import(`data:text/javascript;base64,${Buffer.from(code).toString('base64')}`)
+  assert.equal(typeof loaded.summarizeSkillSources, 'function', 'Pipe the baseline module on stdin')
+  arms[name] = loaded.summarizeSkillSources
+}
+
+function verify(result) {
+  const expected = arms.baseline(result)
+  const actual = arms.indexed(result)
+  assert.deepEqual(actual, expected)
+  actual.forEach((entry, index) => assert.equal(entry.source, result.sources[index]))
+  return expected
+}
+
+let seed = 20260911
+function random(max) {
+  seed = (Math.imul(seed, 1664525) + 1013904223) >>> 0
+  return Math.floor((seed / 0x100000000) * max)
+}
+const paths = [
+  '/home/ada/.agents/skills',
+  '/repo/.agents/skills',
+  '/REPO/.agents/skills',
+  '/remote/folder/.claude/skills',
+  'C:\\Users\\Ada\\.codex\\skills',
+  '\\\\wsl$\\Ubuntu\\home\\ada\\.agents\\skills',
+  '',
+  '/not-listed'
+]
+verify(null)
+for (let trial = 0; trial < 5000; trial++) {
+  const sources = Array.from({ length: random(25) }, (_, index) =>
+    Object.freeze({
+      id: `${index}`,
+      path: paths[random(paths.length - 1)],
+      exists: Boolean(random(2)),
+      skippedReason: [undefined, 'missing', 'remote-repo', 'unavailable'][random(4)]
+    })
+  )
+  const skills = []
+  const count = random(100)
+  for (let index = 0; index < count; index++) {
+    skills.push(
+      skills.length && random(4) === 0
+        ? skills[random(skills.length)]
+        : Object.freeze({
+            rootPath: paths[random(paths.length)],
+            rootPaths: random(3)
+              ? Object.freeze(Array.from({ length: random(15) }, () => paths[random(paths.length)]))
+              : undefined
+          })
+    )
+  }
+  verify(Object.freeze({ sources: Object.freeze(sources), skills: Object.freeze(skills) }))
+}
+console.log(
+  JSON.stringify({
+    differentialCases: 5001,
+    node: process.version,
+    platform: process.platform,
+    arch: process.arch
+  })
+)
+
+function workload(sourceCount, skillCount, rootsPerSkill) {
+  const paths = Array.from(
+    { length: sourceCount || 1 },
+    (_, index) => `/repo-${index}/.agents/skills`
+  )
+  return {
+    sources: paths.slice(0, sourceCount).map((path, index) => ({
+      id: `${index}`,
+      path,
+      exists: index % 3 !== 0,
+      skippedReason: index % 5 ? 'missing' : 'unavailable'
+    })),
+    skills: Array.from({ length: skillCount }, (_, index) => ({
+      rootPath: paths[index % paths.length],
+      rootPaths: Array.from(
+        { length: rootsPerSkill },
+        (_, rootIndex) => paths[(index + rootIndex) % paths.length]
+      )
+    }))
+  }
+}
+
+function median(values) {
+  const sorted = [...values].sort((a, b) => a - b)
+  return (sorted[3] + sorted[4]) / 2
+}
+
+for (const [sourceCount, skillCount, rootsPerSkill] of [
+  [0, 1000, 1],
+  [1, 1000, 0],
+  [1, 1000, 1],
+  [17, 0, 0],
+  [17, 20, 1],
+  [17, 200, 1],
+  [24, 1000, 3],
+  [87, 1000, 3],
+  [367, 5000, 3],
+  [17, 200, 17]
+]) {
+  const input = workload(sourceCount, skillCount, rootsPerSkill)
+  const expected = verify(input)
+  const samples = { baseline: [], indexed: [] }
+  const repeats = Math.max(
+    5,
+    Math.floor(200000 / (Math.max(1, sourceCount) * Math.max(1, skillCount)))
+  )
+  for (const run of Object.values(arms)) {
+    for (let warmup = 0; warmup < Math.min(100, repeats); warmup++) {
+      run(input)
+    }
+  }
+  for (const pair of buildCounterbalancedSchedule(8, 'baseline', 'indexed')) {
+    for (const arm of pair) {
+      const start = performance.now()
+      let result
+      for (let repeat = 0; repeat < repeats; repeat++) {
+        result = arms[arm](input)
+      }
+      samples[arm].push((performance.now() - start) / repeats)
+      assert.deepEqual(result, expected)
+    }
+  }
+  console.log(
+    JSON.stringify({
+      sourceCount,
+      skillCount,
+      rootsPerSkill,
+      medianMs: Object.fromEntries(
+        Object.entries(samples).map(([arm, values]) => [arm, median(values)])
+      )
+    })
+  )
+}

src/renderer/src/components/skills/skill-source-inventory.test.ts

@@ -41,6 +41,61 @@ function result(overrides: Partial<SkillDiscoveryResult> = {}): SkillDiscoveryRe
 }
 
 describe('summarizeSkillSources', () => {
+  it('counts each skill once per root, including a primary root omitted from rootPaths', () => {
+    const shared = skill({ rootPaths: ['/other', '/other', '/unknown'] })
+    const home = source()
+    const other = source({ path: '/other' })
+    const entries = summarizeSkillSources(
+      result({
+        sources: [home, other, other, source({ path: '/OTHER' })],
+        skills: [shared, shared, skill({ rootPaths: [home.path, home.path] })]
+      })
+    )
+    expect(entries.map((entry) => entry.skillCount)).toEqual([3, 2, 2, 0])
+    expect(entries[0].source).toBe(home)
+    expect(entries[1].source).toBe(other)
+    expect(entries[2].source).toBe(other)
+  })
+
+  it('does not scan every skill again for each source', () => {
+    let rootReads = 0
+    const skills = Array.from({ length: 1000 }, () => ({
+      ...skill(),
+      get rootPath() {
+        rootReads++
+        return '/home/dev/.agents/skills'
+      }
+    }))
+    const sources = Array.from({ length: 87 }, (_, index) => source({ id: `${index}` }))
+    const entries = summarizeSkillSources(result({ skills, sources }))
+    expect(entries.every((entry) => entry.skillCount === 1000)).toBe(true)
+    expect(rootReads).toBeLessThanOrEqual(skills.length)
+  })
+
+  it('does not inspect skills without sources', () => {
+    const unused = {
+      ...skill(),
+      get rootPath(): string {
+        throw new Error('No source needs a count')
+      }
+    }
+    expect(summarizeSkillSources(null)).toEqual([])
+    expect(summarizeSkillSources(result({ skills: [unused] }))).toEqual([])
+  })
+
+  it('accepts frozen ownership lists and inputs without changing them', () => {
+    const home = Object.freeze(source())
+    const item = skill({ rootPaths: [home.path, '/co-owner', home.path] })
+    Object.freeze(item.rootPaths)
+    Object.freeze(item)
+    const discovery = result({ sources: [home, source({ path: '/co-owner' })], skills: [item] })
+    Object.freeze(discovery.sources)
+    Object.freeze(discovery.skills)
+    Object.freeze(discovery)
+    expect(summarizeSkillSources(discovery).map((entry) => entry.skillCount)).toEqual([1, 1])
+    expect(item.rootPaths).toEqual([home.path, '/co-owner', home.path])
+  })
+
   it('counts a symlinked skill under every root that reached it', () => {
     const shared = source({ id: 'repo', path: '/repo/.agents/skills', sourceKind: 'repo' })
     const entries = summarizeSkillSources(

src/renderer/src/components/skills/skill-source-inventory.ts

@@ -13,8 +13,6 @@ export type SkillSourceInventoryEntry = {
 }
 
 function ownsSkill(source: SkillDiscoverySource, skill: DiscoveredSkill): boolean {
-  // Why: a symlinked skill is deduped to one row but keeps every root that
-  // reached it, so counting only `rootPath` would zero out the co-owning roots.
   return skill.rootPath === source.path || (skill.rootPaths?.includes(source.path) ?? false)
 }
 
@@ -37,12 +35,38 @@ function sourceStatus(source: SkillDiscoverySource): SkillSourceStatus {
 export function summarizeSkillSources(
   result: SkillDiscoveryResult | null
 ): SkillSourceInventoryEntry[] {
-  if (!result) {
+  if (!result || result.sources.length === 0) {
     return []
   }
+  // With no repeated skill traversal, the direct count needs no index.
+  if (result.sources.length === 1 || result.skills.length === 0) {
+    return result.sources.map((source) => ({
+      source,
+      skillCount: result.skills.filter((skill) => ownsSkill(source, skill)).length,
+      status: sourceStatus(source)
+    }))
+  }
+  const counts = new Map(
+    result.sources.map((source) => [source.path, { count: 0, lastSkillIndex: -1 }])
+  )
+  const countRoot = (rootPath: string, skillIndex: number): void => {
+    const count = counts.get(rootPath)
+    // Symlinked skills can name one owning root more than once.
+    if (count && count.lastSkillIndex !== skillIndex) {
+      count.count++
+      count.lastSkillIndex = skillIndex
+    }
+  }
+  for (let index = 0; index < result.skills.length; index++) {
+    const skill = result.skills[index]
+    countRoot(skill.rootPath, index)
+    for (const rootPath of skill.rootPaths ?? []) {
+      countRoot(rootPath, index)
+    }
+  }
   return result.sources.map((source) => ({
     source,
-    skillCount: result.skills.filter((skill) => ownsSkill(source, skill)).length,
+    skillCount: counts.get(source.path)?.count ?? 0,
     status: sourceStatus(source)
   }))
 }