From 88f27c9be738020d9a2e53b0f2f298a44bfc0c1d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 25 Aug 2026 06:40:55 -0700 Subject: [PATCH] fix(native-chat): make the ranked-order test discriminating, split the command cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1 (both lanes independently) found the ranked-order test was vacuous: its query matched exactly one of 600 items, so replacing the ranked branch with the scope budget still passed. Rebuild it so 600 prefix matches in one scope compete with substring matches in another — the assertion now fails when the ranked branch is swapped for the scope budget. Also give commands their own limit so the skill safety valve no longer widens the command cap from 50 as an unintended side effect. --- .../native-chat-composer-state.test.ts | 35 ++++++++++++------- .../native-chat/native-chat-picker-items.ts | 10 +++--- 2 files changed, 29 insertions(+), 16 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 68998093624..713f36d2957 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 @@ -326,18 +326,29 @@ describe('native skill and command picker', () => { 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 queried catalog in rank order rather than spreading it across scopes', () => { + // 600 prefix matches (rank 1) in repo vs 10 substring matches (rank 2) in home: + // rank order fills the cap with repo rows, a scope budget would interleave home rows. + 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` + }) + ), + ...Array.from({ length: 10 }, (_, index) => + skill({ + name: `alpha-deploy-${index}`, + sourceKind: 'home', + skillFilePath: `/home/.claude/skills/alpha-deploy-${index}/SKILL.md` + }) + ) + ] + const names = buildNativeChatPickerItems([], skills, 'deploy', '/').map((item) => item.name) + expect(names).toHaveLength(500) + expect(names.every((name) => name.startsWith('deploy-'))).toBe(true) + expect(names).not.toContain('alpha-deploy-0') }) it('keeps a long token-safe name intact for insertion instead of truncating it', () => { 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 cf42d7e9f94..467d9013566 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 @@ -38,7 +38,9 @@ export type NativeChatSkillDiscoverySnapshot = { // 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 SKILL_RESULT_LIMIT = 500 +/** Commands are a small curated catalog; keep their original bound. */ +const COMMAND_RESULT_LIMIT = 50 const SCOPE_PRIORITY: Record = { repo: 0, home: 1, @@ -77,12 +79,12 @@ export function buildNativeChatPickerItems( query ) return [ - ...commandItems.slice(0, PICKER_RESULT_LIMIT), + ...commandItems.slice(0, COMMAND_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)) + ? skillItems.slice(0, SKILL_RESULT_LIMIT) + : limitSkillsAcrossScopes(skillItems, SKILL_RESULT_LIMIT)) ] }