fix(native-chat): stop the picker cap from erasing whole skill scopes

The Chat UI slash picker sorted skills by scope (repo, home, bundled,
plugin) and then took the first 50. With more repo skills than the cap,
every home, bundled, and plugin skill was evicted from the unfiltered
list, so users saw a fraction of the skills their terminal session
offered (STA-4402, #14690).

Raise the limit to a safety valve against pathological catalogs and
budget it across scopes when it binds, so no scope can be erased.
Ranked (queried) results keep a plain slice so relevance order stands.

Measured: 0.29 ms/build at a real 121-skill catalog, 4.4 ms at a
synthetic 2000-skill catalog with the result still bounded at 500.
This commit is contained in:
Brennan Benson
2026-08-25 06:18:41 -07:00
parent 61c7b51c8c
commit 01361a8470
2 changed files with 106 additions and 2 deletions
@@ -277,6 +277,69 @@ describe('native skill and command picker', () => {
])
})
it('keeps lower-priority scopes visible when one scope fills the unfiltered list', () => {
const skills = [
...Array.from({ length: 61 }, (_, index) =>
skill({
name: `repo-skill-${String(index).padStart(3, '0')}`,
sourceKind: 'repo',
skillFilePath: `/repo/.agents/skills/repo-${index}/SKILL.md`
})
),
...Array.from({ length: 11 }, (_, index) =>
skill({
name: `home-skill-${String(index).padStart(3, '0')}`,
sourceKind: 'home',
skillFilePath: `/home/.claude/skills/home-${index}/SKILL.md`
})
)
]
const items = buildNativeChatPickerItems([], skills, '', '/')
const names = items.map((item) => item.name)
expect(names).toHaveLength(72)
expect(names).toContain('home-skill-000')
expect(names).toContain('home-skill-010')
expect(names).toContain('repo-skill-060')
})
it('budgets truncation across scopes so a huge scope cannot erase a smaller one', () => {
const skills = [
...Array.from({ length: 800 }, (_, index) =>
skill({
name: `repo-skill-${String(index).padStart(4, '0')}`,
sourceKind: 'repo',
skillFilePath: `/repo/.agents/skills/repo-${index}/SKILL.md`
})
),
...Array.from({ length: 3 }, (_, index) =>
skill({
name: `home-skill-${index}`,
sourceKind: 'home',
skillFilePath: `/home/.claude/skills/home-${index}/SKILL.md`
})
)
]
const names = buildNativeChatPickerItems([], skills, '', '/').map((item) => item.name)
expect(names).toHaveLength(500)
expect(names).toContain('home-skill-0')
expect(names).toContain('home-skill-1')
expect(names).toContain('home-skill-2')
})
it('keeps ranked order when a query narrows a catalog past the cap', () => {
const skills = Array.from({ length: 600 }, (_, index) =>
skill({
name: `deploy-${String(index).padStart(4, '0')}`,
sourceKind: 'repo',
skillFilePath: `/repo/.agents/skills/deploy-${index}/SKILL.md`
})
)
const names = buildNativeChatPickerItems([], skills, 'deploy-0001', '/').map(
(item) => item.name
)
expect(names[0]).toBe('deploy-0001')
})
it('keeps a long token-safe name intact for insertion instead of truncating it', () => {
const longName = `skill-${'x'.repeat(100)}`
const items = buildNativeChatPickerItems(
@@ -35,7 +35,10 @@ export type NativeChatSkillDiscoverySnapshot = {
errorKind?: 'unavailable' | 'timeout' | 'host' | 'unknown'
}
const PICKER_RESULT_LIMIT = 50
// Why: a safety valve against pathological catalogs, not a display policy. The
// menu scrolls, so a low cap only hid skills — and because rows are scope-sorted,
// one large scope silently erased every smaller one (#14690).
const PICKER_RESULT_LIMIT = 500
const SCOPE_PRIORITY: Record<SkillSourceKind, number> = {
repo: 0,
home: 1,
@@ -75,10 +78,48 @@ export function buildNativeChatPickerItems(
)
return [
...commandItems.slice(0, PICKER_RESULT_LIMIT),
...skillItems.slice(0, PICKER_RESULT_LIMIT)
// Ranked results are ordered by relevance, so a plain slice keeps the best
// matches; the unfiltered list is scope-ordered and needs a fair budget.
...(query
? skillItems.slice(0, PICKER_RESULT_LIMIT)
: limitSkillsAcrossScopes(skillItems, PICKER_RESULT_LIMIT))
]
}
/** Spread the cap across scopes so a large scope cannot evict smaller ones. */
function limitSkillsAcrossScopes(
skills: Extract<NativeChatPickerItem, { kind: 'skill' }>[],
limit: number
): Extract<NativeChatPickerItem, { kind: 'skill' }>[] {
if (skills.length <= limit) {
return skills
}
const byScope = new Map<SkillSourceKind, Extract<NativeChatPickerItem, { kind: 'skill' }>[]>()
for (const item of skills) {
const scope = item.sources[0].sourceKind
byScope.set(scope, [...(byScope.get(scope) ?? []), item])
}
const queues = [...byScope.values()]
const picked: Extract<NativeChatPickerItem, { kind: 'skill' }>[] = []
for (let depth = 0; picked.length < limit; depth += 1) {
let advanced = false
for (const queue of queues) {
if (picked.length >= limit) {
break
}
const next = queue[depth]
if (next) {
picked.push(next)
advanced = true
}
}
if (!advanced) {
break
}
}
return picked.sort(comparePickerSkills)
}
function mergeNativeChatSkills(
skills: readonly DiscoveredSkill[]
): Extract<NativeChatPickerItem, { kind: 'skill' }>[] {