From 707ccd6f38a69dd646edc4393845921d84cb6ec1 Mon Sep 17 00:00:00 2001 From: thanhnguyentung95 <17350411+thanhnguyentung95@users.noreply.github.com> Date: Sun, 18 Oct 2026 09:02:43 +0700 Subject: [PATCH] perf(gen-skill-docs): stop emitting the "When to invoke" body section The Claude catalog trim moved each skill's routing prose ("Use when...", "Proactively invoke...", voice triggers) out of the frontmatter description into a "## When to invoke this skill" section at the top of the body. Claude Code reads only name + description to choose a skill and loads the body after the skill is invoked, so that text cannot route anything and costs tokens on every invocation. Keep the one-line trimmed description; drop the section. The routing prose stays in each template's description, which --catalog-mode=full and the other hosts still render. Tests that pinned the section now assert its absence, or check the template instead. --- scripts/gen-skill-docs.ts | 33 +++-------------------- test/catalog-mode-full.test.ts | 4 +-- test/catalog-trim.test.ts | 49 +++++----------------------------- test/cso-preserved.test.ts | 1 - test/gen-skill-docs.test.ts | 19 ++++++------- test/helpers/parity-harness.ts | 2 +- test/helpers/skill-fixture.ts | 6 +---- test/skill-e2e-retro.test.ts | 1 - test/skill-validation.test.ts | 17 ++++++------ 9 files changed, 31 insertions(+), 101 deletions(-) diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index e4718441e..34f3e0f82 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -271,8 +271,9 @@ export { extractVoiceTriggers, processVoiceTriggers }; // "(gstack)" tag. This pile is the always-loaded catalog surface — every // session pays for the full text. The catalog trim splits the description // into a one-line catalog entry (lead sentence + "(gstack)") that stays in -// the frontmatter, and a "## When to invoke" body section that holds the -// routing/voice triggers prose for in-skill discovery. +// the frontmatter and drops the routing/voice prose. The body is not a home +// for it: Claude Code loads a skill body only after the skill is invoked, so +// routing text there can no longer route anything. // // Opt-out: `--catalog-mode=full` keeps v1.44 behavior (no trim, full // description in frontmatter). Use when debugging routing regressions or @@ -358,20 +359,6 @@ export function buildTrimmedDescription(parts: CatalogParts): string { return `${lead}${suffix}`; } -/** Build the body section that holds the routing/voice prose. */ -export function buildWhenToInvokeSection(parts: CatalogParts): string { - const lines: string[] = ['## When to invoke this skill', '']; - if (parts.routingProse) { - lines.push(parts.routingProse); - lines.push(''); - } - if (parts.voiceLine) { - lines.push(parts.voiceLine); - lines.push(''); - } - return lines.join('\n'); -} - /** * Render a string as a YAML inline scalar value (the text after `key: `), * quoting only when a plain scalar would be invalid or ambiguous. @@ -398,9 +385,6 @@ export function toYamlInlineScalar(s: string): string { /** * Apply catalog trim to a SKILL.md body: * - shorten frontmatter `description:` to lead + (gstack) - * - insert "## When to invoke" body section AFTER the generated header - * (so it lands near the top of body content, where routing guidance - * belongs) * * Returns the rewritten content plus the extracted parts. */ @@ -445,16 +429,7 @@ export function applyCatalogTrim(content: string, skillName: string): { content: // pattern and silently corrupt the frontmatter. const newDescLine = `description: ${toYamlInlineScalar(newDesc)}\n`; const newFrontmatter = frontmatter.replace(descMatch[0], () => newDescLine); - let newContent = '---\n' + newFrontmatter + content.slice(fmEnd); - - // Insert body section after frontmatter (after the closing ---\n and any - // existing GENERATED header). We insert before the first non-comment line. - const bodyStart = newContent.indexOf('\n---\n') + 5; - const whenToInvoke = '\n' + buildWhenToInvokeSection(parts).trim() + '\n'; - // Skip past the generated header if present (it lives after frontmatter close) - const headerMatch = newContent.slice(bodyStart).match(/^(\s*\n)+/); - const insertAt = bodyStart + (headerMatch ? headerMatch[0].length : 0); - newContent = newContent.slice(0, insertAt) + whenToInvoke + '\n' + newContent.slice(insertAt); + const newContent = '---\n' + newFrontmatter + content.slice(fmEnd); return { content: newContent, parts }; } diff --git a/test/catalog-mode-full.test.ts b/test/catalog-mode-full.test.ts index 43d7b9b96..1f5432c0e 100644 --- a/test/catalog-mode-full.test.ts +++ b/test/catalog-mode-full.test.ts @@ -40,7 +40,7 @@ describe('catalog mode CLI behavior', () => { test('omitting the flag defaults to trim', () => { expect(frontmatter(defaultContent)).toMatch(/^description: "?Ship workflow:[^\n]*\(gstack\)"?$/m); expect(frontmatter(defaultContent)).not.toMatch(/Use when asked to/i); - expect(defaultContent).toContain('## When to invoke this skill'); + expect(defaultContent).not.toContain('## When to invoke this skill'); }); for (const form of ['equals', 'separate'] as const) { @@ -53,14 +53,12 @@ describe('catalog mode CLI behavior', () => { expect(fm).toMatch(/^description: \|\s*$/m); expect(fm).toMatch(/Use when asked to/i); expect(fm).not.toBe(frontmatter(defaultContent)); - expect(result.content.slice(fm.length)).not.toContain('## When to invoke this skill'); }); test(`${form} flag form accepts explicit trim and matches the default catalog`, () => { const result = render(args('trim')); expect(result.status, result.stderr).toBe(0); expect(frontmatter(result.content)).toBe(frontmatter(defaultContent)); - expect(result.content).toContain('## When to invoke this skill'); }); test(`${form} flag form rejects invalid modes before writing output`, () => { diff --git a/test/catalog-trim.test.ts b/test/catalog-trim.test.ts index 79380c341..db130d816 100644 --- a/test/catalog-trim.test.ts +++ b/test/catalog-trim.test.ts @@ -1,8 +1,7 @@ /** * Unit tests for catalog-trim helpers (gen-skill-docs.ts T4 functions). * - * splitCatalogDescription, buildTrimmedDescription, buildWhenToInvokeSection, - * applyCatalogTrim — these handle every skill's frontmatter rewrite at gen + * splitCatalogDescription, buildTrimmedDescription, applyCatalogTrim — these handle every skill's frontmatter rewrite at gen * time. Two bugs already shipped here: * * v1.45.0.0 design-consultation: when the first sentence exceeded 200 chars, @@ -21,7 +20,6 @@ import { describe, test, expect } from 'bun:test'; import { splitCatalogDescription, buildTrimmedDescription, - buildWhenToInvokeSection, applyCatalogTrim, toYamlInlineScalar, } from '../scripts/gen-skill-docs'; @@ -229,41 +227,6 @@ describe('buildTrimmedDescription', () => { }); }); -describe('buildWhenToInvokeSection', () => { - test('produces markdown H2 with routing prose and voice line', () => { - const out = buildWhenToInvokeSection({ - lead: 'Lead.', - routingProse: 'Use when asked to ship.', - voiceLine: 'Voice triggers (speech-to-text aliases): "ship it".', - hasGstackTag: true, - }); - expect(out).toContain('## When to invoke this skill'); - expect(out).toContain('Use when asked to ship.'); - expect(out).toContain('Voice triggers'); - }); - - test('omits routing block when routingProse is empty', () => { - const out = buildWhenToInvokeSection({ - lead: 'Lead.', - routingProse: '', - voiceLine: null, - hasGstackTag: true, - }); - expect(out).toContain('## When to invoke this skill'); - expect(out).not.toContain('Use when'); - }); - - test('emits even when only voice line is present', () => { - const out = buildWhenToInvokeSection({ - lead: 'Lead.', - routingProse: '', - voiceLine: 'Voice triggers: x.', - hasGstackTag: true, - }); - expect(out).toContain('Voice triggers: x.'); - }); -}); - describe('applyCatalogTrim', () => { const minimalSkill = `--- name: example @@ -280,7 +243,7 @@ preamble-tier: 2 Original body content here. `; - test('rewrites multi-line description into one-line + body section', () => { + test('rewrites multi-line description into one line and adds no body section', () => { const result = applyCatalogTrim(minimalSkill, 'example'); expect(result).not.toBeNull(); const { content, parts } = result!; @@ -288,10 +251,10 @@ Original body content here. // description with an interior colon ("Example skill:") is YAML-quoted, so // the value is wrapped in double quotes — tolerate the optional quotes. expect(content).toMatch(/^description: "?Example skill:[^\n]*\(gstack\)"?\n/m); - // Body has the When to invoke section - expect(content).toContain('## When to invoke this skill'); - expect(content).toContain('Use when asked to do an example task.'); - expect(content).toContain('Proactively suggest when'); + // The body gains no routing section: Claude Code reads a skill body only + // after invocation, so routing prose there routes nothing. + expect(content).not.toContain('## When to invoke this skill'); + expect(content).not.toContain('Use when asked to do an example task.'); // Original body still present expect(content).toContain('# Example body'); expect(content).toContain('Original body content here.'); diff --git a/test/cso-preserved.test.ts b/test/cso-preserved.test.ts index 04c25022e..5d45f7576 100644 --- a/test/cso-preserved.test.ts +++ b/test/cso-preserved.test.ts @@ -103,6 +103,5 @@ describe('CSO v3 always-loaded policy', () => { const description = frontmatter.match(/^description:\s+(.+)$/m)![1].trim(); expect(description.length).toBeLessThanOrEqual(200); expect(description).toContain('(gstack)'); - expect(controller()).toContain('## When to invoke this skill'); }); }); diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index 645b947fa..da11616e6 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -437,14 +437,13 @@ describe('gen-skill-docs', () => { }); test('benchmark routing is scoped to web page performance', () => { - const generated = fs.readFileSync(path.join(ROOT, 'benchmark', 'SKILL.md'), 'utf-8'); + // The template description is the routing source; the Claude catalog trim + // drops its tail from benchmark/SKILL.md. const template = fs.readFileSync(path.join(ROOT, 'benchmark', 'SKILL.md.tmpl'), 'utf-8'); - for (const content of [generated, template]) { - const useWhen = content.match(/Use when:[\s\S]*?(?=\n\n|\(gstack\))/)?.[0] ?? ''; - expect(useWhen).toContain('"web performance benchmark"'); - expect(useWhen).not.toMatch(/"benchmark"|"performance"/); - expect(content).toContain('/benchmark-models'); - } + const useWhen = template.match(/Use when:[\s\S]*?(?=\n\n|\(gstack\))/)?.[0] ?? ''; + expect(useWhen).toContain('"web performance benchmark"'); + expect(useWhen).not.toMatch(/"benchmark"|"performance"/); + expect(template).toContain('/benchmark-models'); const router = fs.readFileSync(path.join(ROOT, 'SKILL.md.tmpl'), 'utf-8'); expect(router).toContain('web performance regression → invoke `/benchmark`'); expect(router).toContain('model or skill benchmarks → invoke `/benchmark-models`'); @@ -4444,8 +4443,10 @@ describe('voice-triggers processing', () => { expect(processVoiceTriggers(content)).toBe(content); }); - test('generated CSO SKILL.md contains voice triggers in description', () => { - const content = fs.readFileSync(path.join(ROOT, 'cso', 'SKILL.md'), 'utf-8'); + test('CSO voice triggers fold into the full description', () => { + // The Claude catalog trim drops this line from cso/SKILL.md; full-mode and + // other-host renders still carry it. + const content = processVoiceTriggers(fs.readFileSync(path.join(ROOT, 'cso', 'SKILL.md.tmpl'), 'utf-8')); expect(content).toContain('"see-so"'); expect(content).toContain('Voice triggers (speech-to-text aliases):'); }); diff --git a/test/helpers/parity-harness.ts b/test/helpers/parity-harness.ts index 6c1caf4dd..c0382c8da 100644 --- a/test/helpers/parity-harness.ts +++ b/test/helpers/parity-harness.ts @@ -229,7 +229,7 @@ const CARVED_INVARIANTS: ParityInvariant[] = Object.values(CARVE_GUARDS).map((g) mustContain: g.mustContain, // CSO's helper trust boundary requires its private startup; demanding the // shared Preamble here would silently reintroduce conflicting policy. - mustHaveHeadings: g.skill === 'cso' ? ['## When to invoke'] : ['## Preamble', '## When to invoke'], + mustHaveHeadings: g.skill === 'cso' ? [] : ['## Preamble'], maxSizeRatio: g.maxSizeRatio ?? 1.05, })); diff --git a/test/helpers/skill-fixture.ts b/test/helpers/skill-fixture.ts index b18806459..ffece830f 100644 --- a/test/helpers/skill-fixture.ts +++ b/test/helpers/skill-fixture.ts @@ -39,7 +39,6 @@ import * as path from 'path'; /** /review E2E (sql-injection, enum-completeness, design-lite): the core * review workflow without the shared preamble, Review Army, or Fix-First. */ export const REVIEW_E2E_SECTIONS = [ - 'When to invoke this skill', 'Step 0: Detect platform and base branch', 'Step 1: Check branch', 'Step 2: Read the checklist', @@ -54,7 +53,6 @@ export const REVIEW_E2E_SECTIONS = [ * (delivery-audit test) + Step 4.5 specialist dispatch (quality score, * JSON findings schema, MULTI-SPECIALIST consensus, Red Team). */ export const REVIEW_ARMY_E2E_SECTIONS = [ - 'When to invoke this skill', 'Step 0: Detect platform and base branch', 'Step 1: Check branch', 'Step 1.5: Scope Drift Detection', @@ -72,7 +70,6 @@ export const REVIEW_ARMY_E2E_SECTIONS = [ * + the narrative report template. Global mode and Compare mode are not * exercised by the E2E tests and are dropped. */ export const RETRO_E2E_SECTIONS = [ - 'When to invoke this skill', 'Step 0: Detect platform and base branch', 'User-invocable', 'Arguments', @@ -86,8 +83,7 @@ export const RETRO_E2E_SECTIONS = [ /** codex-review-findings E2E against the Codex host variant * (.agents/skills/gstack-review/SKILL.md). Same core workflow as - * REVIEW_E2E_SECTIONS, minus "When to invoke this skill" (the Codex host - * adapter does not emit that section). */ + * REVIEW_E2E_SECTIONS. */ export const CODEX_REVIEW_E2E_SECTIONS = [ 'Step 0: Detect platform and base branch', 'Step 1: Check branch', diff --git a/test/skill-e2e-retro.test.ts b/test/skill-e2e-retro.test.ts index 29aa7e853..7d0324eee 100644 --- a/test/skill-e2e-retro.test.ts +++ b/test/skill-e2e-retro.test.ts @@ -23,7 +23,6 @@ const evalCollector = createEvalCollector('e2e-retro'); // still an extraction, not a full-file copy (sections ARE the minimal // on-demand units; same pattern as skill-e2e-review-army.test.ts). const RETRO_SKELETON_SECTIONS = [ - 'When to invoke this skill', 'Step 0: Detect platform and base branch', 'User-invocable', 'Arguments', diff --git a/test/skill-validation.test.ts b/test/skill-validation.test.ts index 4f8c73cde..3a003970f 100644 --- a/test/skill-validation.test.ts +++ b/test/skill-validation.test.ts @@ -1779,14 +1779,13 @@ describe('Skill trigger phrases', () => { ]; for (const skill of SKILLS_REQUIRING_TRIGGERS) { - test(`${skill}/SKILL.md has "Use when" trigger phrases`, () => { - const skillPath = path.join(ROOT, skill, 'SKILL.md'); + test(`${skill}/SKILL.md.tmpl has "Use when" trigger phrases`, () => { + const skillPath = path.join(ROOT, skill, 'SKILL.md.tmpl'); if (!fs.existsSync(skillPath)) return; const content = fs.readFileSync(skillPath, 'utf-8'); - // v1.45.0.0 catalog trim moved trigger prose out of frontmatter into a - // body "## When to invoke" section. Search the full file content, not - // just frontmatter. The trigger phrase must still appear somewhere in - // the skill so agents can match user requests to the skill. + // The catalog trim drops trigger prose from the Claude SKILL.md, so check + // the template description: --catalog-mode=full and the other hosts + // still route on it. expect(content).toMatch(/Use when/i); }); } @@ -1799,11 +1798,11 @@ describe('Skill trigger phrases', () => { ]; for (const skill of SKILLS_REQUIRING_PROACTIVE) { - test(`${skill}/SKILL.md has proactive routing phrase`, () => { - const skillPath = path.join(ROOT, skill, 'SKILL.md'); + test(`${skill}/SKILL.md.tmpl has proactive routing phrase`, () => { + const skillPath = path.join(ROOT, skill, 'SKILL.md.tmpl'); if (!fs.existsSync(skillPath)) return; const content = fs.readFileSync(skillPath, 'utf-8'); - // Same catalog-trim consideration — search the full file content. + // Same catalog-trim consideration — check the template description. expect(content).toMatch(/Proactively (suggest|invoke)/i); }); }