mirror of https://github.com/garrytan/gstack.git
fix(gen-skill-docs): stop truncating first sentences containing an embedded period
The catalog-description splitter used the sentence regex `^([^.!?]*[.!?])(?:\s|$)` to find the lead (first) sentence. The `[^.!?]*` class is greedy and cannot backtrack across a period, so any first sentence containing an embedded terminator — "TODOS.md" / "CLAUDE.md", a URL, or a version number like "v1.45.0.0" — failed to match a real sentence boundary. When the match failed, the code silently fell back to a blind 20-word cut, producing leads that either stopped mid-token or ran past the true sentence end into the next clause. The fix rewrites the lead regex to `^((?:[^.!?]|[.!?](?!\s|$))*[.!?])(?:\s|$)`: a terminator that is NOT followed by whitespace/end-of-text (an embedded period) is consumed by the second, disjoint alternative and no longer ends the sentence, while a terminator followed by a boundary still closes the lead. The two alternatives cover disjoint inputs, so there is no ambiguity or catastrophic-backtracking risk. Adds regression coverage in test/catalog-trim.test.ts for embedded periods, URLs, and a >200-char first sentence with embedded periods (ellipsis path, not the word-count fallback). Re-renders the two skill descriptions that were previously mis-split: - diagram: lead now runs to "...and rendered SVG + PNG." instead of truncating at "you can open". - sync-gbrain: lead now stops at "...guidance in CLAUDE.md." instead of spilling the next clause in via the 20-word fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Stefan Andrei
parent
11de390be1
commit
a98b873882
|
|
@ -1,7 +1,7 @@
|
|||
---
|
||||
name: diagram
|
||||
version: 1.0.0
|
||||
description: "Turn an English description (or mermaid source) into a diagram triplet: the source, an editable .excalidraw file you can open (gstack)"
|
||||
description: "Turn an English description (or mermaid source) into a diagram triplet: the source, an editable .excalidraw file you can open on excalidraw.com, and rendered SVG + PNG. (gstack)"
|
||||
allowed-tools:
|
||||
- Bash
|
||||
- Read
|
||||
|
|
@ -21,9 +21,8 @@ triggers:
|
|||
|
||||
## When to invoke this skill
|
||||
|
||||
on excalidraw.com,
|
||||
and rendered SVG + PNG (clean mermaid style; the .excalidraw carries the
|
||||
hand-drawn aesthetic). Fully offline.
|
||||
The SVG/PNG use clean mermaid style; the
|
||||
.excalidraw carries the hand-drawn aesthetic. Fully offline.
|
||||
Use when asked to "make a diagram", "draw the architecture", "create a
|
||||
flowchart", "diagram this", or "visualize this flow".
|
||||
|
||||
|
|
|
|||
|
|
@ -4,8 +4,8 @@ version: 1.0.0
|
|||
description: |
|
||||
Turn an English description (or mermaid source) into a diagram triplet:
|
||||
the source, an editable .excalidraw file you can open on excalidraw.com,
|
||||
and rendered SVG + PNG (clean mermaid style; the .excalidraw carries the
|
||||
hand-drawn aesthetic). Fully offline.
|
||||
and rendered SVG + PNG. The SVG/PNG use clean mermaid style; the
|
||||
.excalidraw carries the hand-drawn aesthetic. Fully offline.
|
||||
Use when asked to "make a diagram", "draw the architecture", "create a
|
||||
flowchart", "diagram this", or "visualize this flow". (gstack)
|
||||
allowed-tools:
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ Conventions:
|
|||
- [/design-review](design-review/SKILL.md): Designer's eye QA: finds visual inconsistency, spacing issues, hierarchy problems, AI slop patterns, and slow interactions — then fixes them.
|
||||
- [/design-shotgun](design-shotgun/SKILL.md): Design shotgun: generate multiple AI design variants, open a comparison board, collect structured feedback, and iterate.
|
||||
- [/devex-review](devex-review/SKILL.md): Live developer experience audit.
|
||||
- [/diagram](diagram/SKILL.md): Turn an English description (or mermaid source) into a diagram triplet: the source, an editable .excalidraw file you can open on excalidraw.com, and rendered SVG + PNG (clean mermaid style; the .excalidraw carries the hand-drawn aesthetic).
|
||||
- [/diagram](diagram/SKILL.md): Turn an English description (or mermaid source) into a diagram triplet: the source, an editable .excalidraw file you can open on excalidraw.com, and rendered SVG + PNG.
|
||||
- [/document-generate](document-generate/SKILL.md): Generate missing documentation from scratch for a feature, module, or entire project.
|
||||
- [/document-release](document-release/SKILL.md): Post-ship documentation update.
|
||||
- [/freeze](freeze/SKILL.md): Restrict file edits to a specific directory for the session.
|
||||
|
|
|
|||
|
|
@ -317,12 +317,16 @@ export function splitCatalogDescription(description: string): CatalogParts {
|
|||
const hasGstackTag = /\(gstack\)/.test(working);
|
||||
if (hasGstackTag) working = working.replace(/\(gstack\)/, '').trim();
|
||||
|
||||
// Lead = first sentence (up to first period followed by space or end of string).
|
||||
// We tolerate sentences with embedded periods (URLs, "v1.45.0.0") by requiring
|
||||
// the period to be followed by whitespace OR end-of-text.
|
||||
// Lead = first sentence, ending at the first `.`/`!`/`?` that is followed by
|
||||
// whitespace or end-of-text. Terminator chars NOT followed by whitespace/end
|
||||
// (embedded periods in "TODOS.md", URLs, "v1.45.0.0") are consumed by the
|
||||
// second alternative `[.!?](?!\s|$)` and do NOT end the sentence. The two
|
||||
// alternatives are disjoint character classes, so there is no ambiguity and
|
||||
// no catastrophic-backtracking risk. If no terminator-followed-by-boundary
|
||||
// exists at all, we fall back to a 20-word cut below.
|
||||
// First normalize to single-line for sentence detection, then back out.
|
||||
const collapsed = working.replace(/\s+/g, ' ').trim();
|
||||
const sentenceMatch = collapsed.match(/^([^.!?]*[.!?])(?:\s|$)/);
|
||||
const sentenceMatch = collapsed.match(/^((?:[^.!?]|[.!?](?!\s|$))*[.!?])(?:\s|$)/);
|
||||
// sentenceLead is the FULL first sentence (no truncation). We compute routing
|
||||
// from this position, then optionally truncate the displayed lead afterwards.
|
||||
// Truncating first then computing routing was the v1.45.0.0 bug — when the
|
||||
|
|
|
|||
|
|
@ -79,8 +79,8 @@
|
|||
"voice_line": "Voice triggers (speech-to-text aliases): \"dx audit\", \"test the developer experience\", \"try the onboarding\", \"developer experience test\"."
|
||||
},
|
||||
"diagram": {
|
||||
"lead": "Turn an English description (or mermaid source) into a diagram triplet: the source, an editable .excalidraw file you can open",
|
||||
"routing": "on excalidraw.com,\nand rendered SVG + PNG (clean mermaid style; the .excalidraw carries the\nhand-drawn aesthetic). Fully offline.\nUse when asked to \"make a diagram\", \"draw the architecture\", \"create a\nflowchart\", \"diagram this\", or \"visualize this flow\".",
|
||||
"lead": "Turn an English description (or mermaid source) into a diagram triplet: the source, an editable .excalidraw file you can open on excalidraw.com, and rendered SVG + PNG.",
|
||||
"routing": "The SVG/PNG use clean mermaid style; the\n.excalidraw carries the hand-drawn aesthetic. Fully offline.\nUse when asked to \"make a diagram\", \"draw the architecture\", \"create a\nflowchart\", \"diagram this\", or \"visualize this flow\".",
|
||||
"voice_line": null
|
||||
},
|
||||
"document-generate": {
|
||||
|
|
@ -264,8 +264,8 @@
|
|||
"voice_line": null
|
||||
},
|
||||
"sync-gbrain": {
|
||||
"lead": "Keep gbrain current with this repo's code and refresh agent search guidance in CLAUDE.md. Wraps the gstack-gbrain-sync orchestrator with state",
|
||||
"routing": "probing, native code-surface registration, capability checks,\nand a verdict block. Re-runnable, idempotent. Use when: \"sync gbrain\",\n\"refresh gbrain\", \"re-index this repo\", \"gbrain search isn't finding\nthings\".",
|
||||
"lead": "Keep gbrain current with this repo's code and refresh agent search guidance in CLAUDE.md.",
|
||||
"routing": "Wraps the gstack-gbrain-sync orchestrator with\nstate probing, native code-surface registration, capability checks,\nand a verdict block. Re-runnable, idempotent. Use when: \"sync gbrain\",\n\"refresh gbrain\", \"re-index this repo\", \"gbrain search isn't finding\nthings\".",
|
||||
"voice_line": null
|
||||
},
|
||||
"unfreeze": {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
name: sync-gbrain
|
||||
preamble-tier: 2
|
||||
version: 1.0.0
|
||||
description: Keep gbrain current with this repo's code and refresh agent search guidance in CLAUDE.md. Wraps the gstack-gbrain-sync orchestrator with state (gstack)
|
||||
description: Keep gbrain current with this repo's code and refresh agent search guidance in CLAUDE.md. (gstack)
|
||||
triggers:
|
||||
- sync gbrain
|
||||
- refresh gbrain
|
||||
|
|
@ -23,7 +23,8 @@ allowed-tools:
|
|||
|
||||
## When to invoke this skill
|
||||
|
||||
probing, native code-surface registration, capability checks,
|
||||
Wraps the gstack-gbrain-sync orchestrator with
|
||||
state probing, native code-surface registration, capability checks,
|
||||
and a verdict block. Re-runnable, idempotent. Use when: "sync gbrain",
|
||||
"refresh gbrain", "re-index this repo", "gbrain search isn't finding
|
||||
things".
|
||||
|
|
|
|||
|
|
@ -95,31 +95,59 @@ describe('splitCatalogDescription', () => {
|
|||
expect(parts.routingProse).toBe('With routing prose afterward.');
|
||||
});
|
||||
|
||||
test('embedded-period descriptions: known limitation falls back to first-20-words', () => {
|
||||
// KNOWN LIMITATION: the sentence regex `^([^.!?]*[.!?])(?:\\s|$)` stops
|
||||
// at the FIRST `.`-then-non-whitespace because [^.!?]* is greedy and
|
||||
// can't backtrack past a non-period char. For "DESIGN.md and v1.45.0.0
|
||||
// in the lead. Use when..." the regex fails entirely and the lead falls
|
||||
// back to the first 20 words (~the whole short input).
|
||||
//
|
||||
// The real-world impact is small: descriptions like "DESIGN.md" or "v1.45"
|
||||
// appearing in the middle of the FIRST sentence are rare. When they do
|
||||
// occur, the lead simply becomes the full description (no body section
|
||||
// generated) — same as a description without a period. The trim CI gate
|
||||
// still keeps the per-skill size budget honest.
|
||||
//
|
||||
// If this gap matters later, replace the regex with a sentence tokenizer
|
||||
// (compromise.js / Intl.Segmenter) — until then we accept the fallback.
|
||||
test('REGRESSION: embedded-period first sentence splits at real boundary', () => {
|
||||
// The old regex `^([^.!?]*[.!?])(?:\s|$)` could not cross ANY period —
|
||||
// [^.!?]* stops at the first `.` even mid-token ("DESIGN.md"), the
|
||||
// boundary check then fails, and the whole match failed. The code
|
||||
// silently fell back to a 20-word cut mid-phrase (observed with a
|
||||
// description mentioning "TODOS.md"). The fixed regex
|
||||
// `^((?:[^.!?]|[.!?](?!\s|$))*[.!?])(?:\s|$)` consumes terminators NOT
|
||||
// followed by whitespace/end, so embedded periods no longer break it.
|
||||
const desc =
|
||||
'Skill that mentions DESIGN.md and v1.45.0.0 in the lead. ' +
|
||||
'Use when asked to do something.';
|
||||
const parts = splitCatalogDescription(desc);
|
||||
// Actual behavior: lead absorbs the whole input via the word-count fallback.
|
||||
expect(parts.lead.length).toBeGreaterThan(0);
|
||||
// routingProse may be empty when the fallback consumes everything.
|
||||
// The test exists to detect REGRESSIONS (lead becoming oddly short like
|
||||
// "Skill that mentions DESIGN.") not to assert ideal behavior.
|
||||
expect(parts.lead).toContain('Skill that mentions');
|
||||
expect(parts.lead).toBe('Skill that mentions DESIGN.md and v1.45.0.0 in the lead.');
|
||||
expect(parts.routingProse).toBe('Use when asked to do something.');
|
||||
});
|
||||
|
||||
test('REGRESSION: "TODOS.md backlog" style sentence keeps full lead + routing', () => {
|
||||
const desc =
|
||||
'Drive an approved plan to completion on a TODOS.md backlog. ' +
|
||||
'Use when asked to "autobuilder" or "run the build loop". ' +
|
||||
'Proactively suggest after a plan is approved. (gstack)';
|
||||
const parts = splitCatalogDescription(desc);
|
||||
expect(parts.lead).toBe('Drive an approved plan to completion on a TODOS.md backlog.');
|
||||
expect(parts.routingProse).toContain('Use when asked to "autobuilder"');
|
||||
expect(parts.routingProse).toContain('Proactively suggest');
|
||||
expect(parts.hasGstackTag).toBe(true);
|
||||
});
|
||||
|
||||
test('URL in first sentence does not end the lead early', () => {
|
||||
const desc =
|
||||
'See https://example.com/docs/v2 for the workflow. ' +
|
||||
'Use when asked to consult the docs.';
|
||||
const parts = splitCatalogDescription(desc);
|
||||
expect(parts.lead).toBe('See https://example.com/docs/v2 for the workflow.');
|
||||
expect(parts.routingProse).toBe('Use when asked to consult the docs.');
|
||||
});
|
||||
|
||||
test('>200 char first sentence WITH embedded periods still truncates with ellipsis and keeps routing', () => {
|
||||
const firstSentence =
|
||||
'Regenerate the iOS debug bridge against files like Bridge.swift and TODOS.md, ' +
|
||||
'walking every target listed in Project.xcodeproj while preserving v1.45.0.0 ' +
|
||||
'compatibility shims, custom entitlements, and the long tail of per-target ' +
|
||||
'build settings nobody remembers configuring.';
|
||||
expect(firstSentence.length).toBeGreaterThan(200);
|
||||
const desc = firstSentence + ' Use when asked to resync the bridge. (gstack)';
|
||||
const parts = splitCatalogDescription(desc);
|
||||
// Lead is the truncated first sentence (ellipsis path), not a 20-word cut.
|
||||
expect(parts.lead.endsWith('...')).toBe(true);
|
||||
expect(parts.lead.length).toBeLessThanOrEqual(200);
|
||||
expect(parts.lead).toContain('TODOS.md');
|
||||
// Routing prose survives intact.
|
||||
expect(parts.routingProse).toContain('Use when asked to resync the bridge.');
|
||||
expect(parts.hasGstackTag).toBe(true);
|
||||
});
|
||||
|
||||
test('description without a period uses first ~20 words as lead', () => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue