mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 22:14:25 +01:00
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.
This commit is contained in:
1 parent
8f103baa71
commit
707ccd6f38
9 files changed
+31
-101
No files matched your search
@@ -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 };
|
||||
}
|
||||
|
||||
@@ -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`, () => {
|
||||
|
||||
@@ -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.');
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -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):');
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
}));
|
||||
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user