From 01361a8470a449cb8998f3d8d1167014c1af12ee Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 25 Aug 2026 06:18:41 -0700 Subject: [PATCH] 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. --- .../native-chat-composer-state.test.ts | 63 +++++++++++++++++++ .../native-chat/native-chat-picker-items.ts | 45 ++++++++++++- 2 files changed, 106 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts b/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts index df9fa3de5a1..68998093624 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts @@ -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( diff --git a/src/renderer/src/components/native-chat/native-chat-picker-items.ts b/src/renderer/src/components/native-chat/native-chat-picker-items.ts index 6938c2078a4..cf42d7e9f94 100644 --- a/src/renderer/src/components/native-chat/native-chat-picker-items.ts +++ b/src/renderer/src/components/native-chat/native-chat-picker-items.ts @@ -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 = { 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[], + limit: number +): Extract[] { + if (skills.length <= limit) { + return skills + } + const byScope = new Map[]>() + 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[] = [] + 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[] {