fix(native-chat): make the ranked-order test discriminating, split the command cap

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.
This commit is contained in:
Brennan Benson
2026-08-25 06:40:55 -07:00
parent 01361a8470
commit 88f27c9be7
2 changed files with 29 additions and 16 deletions
@@ -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', () => {
@@ -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<SkillSourceKind, number> = {
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))
]
}