WP4 Phase 3: Skill Discovery Enhancement (Validation & Indexing)#40
WP4 Phase 3: Skill Discovery Enhancement (Validation & Indexing)#40Steffen025 merged 6 commits intodevfrom
Conversation
Enhanced skill discovery system for WP3 hierarchical structure: ## GenerateSkillIndex.ts (Enhanced) - Added category tracking for hierarchical skills - Added isHierarchical flag per skill - Added categoryMap showing skills per category - Updated stats: categories, flatSkills, hierarchicalSkills - Better console output with category breakdown ## ValidateSkillStructure.ts (New) - Validates skill directory structure - Checks for: frontmatter, duplicates, nesting depth - Verifies category SKILL.md files exist - Reports errors and warnings - Exit code 0 on success, 1 on errors ## NPM Scripts Added ## Testing Results - Found 48 skills (9 categories, 16 flat, 32 hierarchical) - Identified 7 issues (known problems): * Telos/USMetrics duplicate naming * Documents sub-skills (3-level nesting) * PAI missing frontmatter (core skill) - Token savings: ~80% with deferred loading Related: WP4 Phase 3 - Skill Discovery Enhancement
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDie PR fügt Kategorie-Metadaten (category, isHierarchical), Zählwerte und eine categoryMap zum Skill-Index hinzu, erweitert GenerateSkillIndex für Kategorien/Duplikate, ergänzt das neue ValidateSkillStructure-Skript zur Strukturprüfung, liefert einen aktualisierten Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer/CI
participant Validator as ValidateSkillStructure.ts
participant Indexer as GenerateSkillIndex.ts
participant FS as FileSystem
Dev->>Validator: start `skills:validate`
Validator->>FS: list skills directories (skip hidden/node_modules)
FS-->>Validator: files & paths
Validator->>Validator: validate frontmatter, names, lengths, categories, duplicates
Validator-->>Dev: emit errors/warnings, exit (0/1)
alt no errors
Dev->>Indexer: start `skills:index` (or `skills:check`)
Indexer->>FS: read skill files and paths
Indexer->>Indexer: determine isHierarchical, category, build categoryMap & counts, skip duplicates
Indexer->>FS: write `.opencode/skills/skill-index.json`
Indexer-->>Dev: success + stats
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Consolidates remaining work into 2 substantive PRs: - PR #3 (WP5): Algorithm v3.7.0 & Core System (~2000 lines, 25 files) - PR #4 (WP6): Installer, Migration & Release (~800 lines, 20 files) Recommend merging PR #40 as 'WP4 Complete' containing all integration work (Phases 1-3 combined). Total: 4 PRs for entire v3.0 (not 8+) Related: WP4 consolidation, PR strategy
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.opencode/skills/PAI/Tools/GenerateSkillIndex.ts (2)
242-245:⚠️ Potential issue | 🟠 MajorNamenskollision überschreibt Skills stillschweigend
Bei gleichem
skill.name.toLowerCase()wird der vorherige Eintrag ersetzt,totalSkillsaber trotzdem erhöht. Das führt zu inkonsistenten Indexdaten.🔧 Vorschlag
const key = skill.name.toLowerCase(); + if (index.skills[key]) { + console.warn( + `⚠️ Duplicate skill name "${skill.name}" at ${skill.path}; keeping first entry at ${index.skills[key].path}` + ); + continue; + } index.skills[key] = skill; index.totalSkills++;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts around lines 242 - 245, Das Problem: beim Arbeiten in GenerateSkillIndex.ts wird bei identischem skill.name.toLowerCase() der bestehende Eintrag in index.skills überschrieben, während index.totalSkills trotzdem inkrementiert wird, was den Index inkonsistent macht. Fix: vor dem Setzen von index.skills[key] prüfen, ob key bereits existiert; wenn ja, entweder den neuen Skill überspringen oder ihn unter einem deterministischen alternativen Schlüssel (z.B. Suffix) ablegen, und in beiden Fällen index.totalSkills nur dann erhöhen, wenn tatsächlich ein neuer Eintrag hinzugefügt wurde; außerdem optional eine Warnung/Log über die Namenskollision mit Referenz auf skill.name und key schreiben.
109-117:⚠️ Potential issue | 🟠 MajorFrontmatter-Parser verliert
descriptionbei gefaltetem YAML (>)Bei
description: >landet faktisch nur">"im Index. Das verschlechtertfullDescriptionundtriggers(sichtbar z. B. an leeren Triggern trotz vorhandener Beschreibung).🔧 Vorschlag
- const descMatch = frontmatter.match(/^description:\s*\|?\s*([\s\S]*?)(?=\n[a-z]+:|$)/m); + const descMatch = frontmatter.match( + /^description:\s*(?:[>|]\s*)?([\s\S]*?)(?=\n[a-zA-Z_][\w-]*:|$)/m + );As per coding guidelines: SKILL.md should have YAML frontmatter with name, description, and category context, plus body sections describing usage, customization, and workflows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts around lines 109 - 117, Der aktuelle Regex für description in GenerateSkillIndex.ts erkennt gefaltetes YAML (description: >) nicht richtig, deshalb landet nur ">" im Index; fix das, indem du die Frontmatter-Parsing-Logik für description erweiterst: entweder (preferred) parse das gesamte frontmatter mit einem YAML-Parser (z.B. js-yaml) und nimm das Feld description direkt, oder passe die Regex an /^description:\s*[>|]?\s*([\s\S]*?)(?=\n[a-z]+:|$)/m, erkenne ob das Präfix '>' war und normalisiere gefaltete Texte (ersetze Zeilenumbrüche durch Leerzeichen, trims/merges Leerzeilen) bevor du description in die Variable setzt (Referenzen: frontmatter, descMatch, description in GenerateSkillIndex.ts).
🧹 Nitpick comments (1)
.opencode/skills/PAI/Tools/ValidateSkillStructure.ts (1)
46-46: Kategoriefehler wird mehrfach gemeldet (Output-Rauschen)Fehlt eine Category-
SKILL.md, wird derselbe Fehler pro Sub-Skill erneut erzeugt. Ein deduplizierendes Set macht die Ausgabe deutlich klarer.♻️ Vorschlag
- const categories = new Set<string>(); + const categories = new Set<string>(); + const missingCategorySkill = new Set<string>(); ... const categoryPath = join(SKILLS_DIR, pathParts[0]); const categorySkillPath = join(categoryPath, 'SKILL.md'); - if (!existsSync(categorySkillPath)) { + if (!existsSync(categorySkillPath) && !missingCategorySkill.has(pathParts[0])) { + missingCategorySkill.add(pathParts[0]); issues.push({ type: 'error', path: categoryPath, message: `Missing category SKILL.md for "${pathParts[0]}"`, }); }Also applies to: 89-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/skills/PAI/Tools/ValidateSkillStructure.ts at line 46, The validator currently emits the same "missing category SKILL.md" error for each sub-skill; use a deduplicating Set to emit the category-missing error only once per category by tracking reported categories (use the existing categories Set<string> defined as categories) and check categories.has(category) before pushing/logging the error, then add category to the Set after reporting; apply the same deduplication logic to the other duplicate-reporting block referenced around lines 89-98 (use the same or a separate reported set there) so each missing-category message is emitted just once per category.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts:
- Around line 199-200: The logic that classifies a skill as hierarchical uses
isHierarchical = pathParts.length >= 3 which incorrectly treats deeper nested
paths (length > 3) as the same two-level category/skill shape; change the
predicate to isHierarchical = pathParts.length === 3 and update the subsequent
handling around category and categoryMap population in GenerateSkillIndex.ts
(references: isHierarchical, pathParts, category, categoryMap) so that only
exactly three-part paths are mapped to the simple category/skill structure, and
implement separate handling for pathParts.length > 3 (either build true nested
entries or skip mapping into the flat categoryMap) to preserve accurate
hierarchy.
In @.opencode/skills/PAI/Tools/ValidateSkillStructure.ts:
- Around line 107-117: The code calls scanDirectory(fullPath, depth + 1) twice
for the branch where a directory has no SKILL.md at depth===0, causing duplicate
recursion and duplicate issues; update the logic in scanDirectory so that when
you detect "no SKILL.md" and you already call await scanDirectory(fullPath,
depth + 1) for the top-level category case, you do not fall through to the
unconditional recursive call — either remove the later unconditional call or add
an explicit else/continue/return so scanDirectory(fullPath, depth + 1) is
invoked only once for that path (references: function scanDirectory, variables
depth and fullPath, check for SKILL.md).
- Around line 203-216: The current frontmatter check uses a single-line regex
(descMatch) that misses block-style YAML descriptions (description: | or >);
replace the regex-based detection with proper YAML parsing of the frontmatter
(e.g., use an existing YAML parser like js-yaml to safeLoad/safeParse the
frontmatter) and then read the parsed.description value (handle strings and
folded/multiline values) before checking for the "USE WHEN" substring; update
the warning pushes that reference descMatch to use the parsed description and
keep the same issue shape (type, path: relativePath, message) so multi-line
descriptions are correctly detected.
- Around line 61-63: Der aktuelle catch-Block in ValidateSkillStructure.ts (die
catch um die Symlink-Auflösung) schluckt defekte Symlinks komplett und verwirft
damit strukturelle Fehler; stattdessen fange die Ausnahme im catch und melde sie
explizit: entweder wirf eine neue aussagekräftige Error mit dem betroffenen Pfad
und der Original-Fehlermeldung oder füge einen Eintrag in die bestehende
Fehlerliste (z. B. errors.push) mit Pfad, Typ "BrokenSymlink" und originaler
Fehlermeldung hinzu; passe den catch-Block um die Original-Exception zu erhalten
(catch (err)) und verwende die relevanten Symbole/Variablen aus dieser Datei
(die catch-Block um die Symlink-Auflösung in ValidateSkillStructure) beim
Erstellen der Fehlermeldung oder beim Push in die Fehlerstruktur.
---
Outside diff comments:
In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts:
- Around line 242-245: Das Problem: beim Arbeiten in GenerateSkillIndex.ts wird
bei identischem skill.name.toLowerCase() der bestehende Eintrag in index.skills
überschrieben, während index.totalSkills trotzdem inkrementiert wird, was den
Index inkonsistent macht. Fix: vor dem Setzen von index.skills[key] prüfen, ob
key bereits existiert; wenn ja, entweder den neuen Skill überspringen oder ihn
unter einem deterministischen alternativen Schlüssel (z.B. Suffix) ablegen, und
in beiden Fällen index.totalSkills nur dann erhöhen, wenn tatsächlich ein neuer
Eintrag hinzugefügt wurde; außerdem optional eine Warnung/Log über die
Namenskollision mit Referenz auf skill.name und key schreiben.
- Around line 109-117: Der aktuelle Regex für description in
GenerateSkillIndex.ts erkennt gefaltetes YAML (description: >) nicht richtig,
deshalb landet nur ">" im Index; fix das, indem du die Frontmatter-Parsing-Logik
für description erweiterst: entweder (preferred) parse das gesamte frontmatter
mit einem YAML-Parser (z.B. js-yaml) und nimm das Feld description direkt, oder
passe die Regex an /^description:\s*[>|]?\s*([\s\S]*?)(?=\n[a-z]+:|$)/m, erkenne
ob das Präfix '>' war und normalisiere gefaltete Texte (ersetze Zeilenumbrüche
durch Leerzeichen, trims/merges Leerzeilen) bevor du description in die Variable
setzt (Referenzen: frontmatter, descMatch, description in
GenerateSkillIndex.ts).
---
Nitpick comments:
In @.opencode/skills/PAI/Tools/ValidateSkillStructure.ts:
- Line 46: The validator currently emits the same "missing category SKILL.md"
error for each sub-skill; use a deduplicating Set to emit the category-missing
error only once per category by tracking reported categories (use the existing
categories Set<string> defined as categories) and check categories.has(category)
before pushing/logging the error, then add category to the Set after reporting;
apply the same deduplication logic to the other duplicate-reporting block
referenced around lines 89-98 (use the same or a separate reported set there) so
each missing-category message is emitted just once per category.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4afb398d-8061-45bf-a8e2-e849c010db33
📒 Files selected for processing (4)
.opencode/skills/PAI/Tools/GenerateSkillIndex.ts.opencode/skills/PAI/Tools/ValidateSkillStructure.ts.opencode/skills/skill-index.jsonpackage.json
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/epic/OPTIMIZED-PR-PLAN.md (1)
1-6: Frontmatter für wichtiges Strategiedokument hinzufügen.Dieses Dokument definiert die v3.0 Release-Strategie und sollte gemäß Coding Guidelines Frontmatter für Metadaten und Dataview-Queries enthalten.
📋 Vorgeschlagenes Frontmatter
+--- +title: "PAI-OpenCode v3.0 - Optimierter PR-Plan" +date: 2026-03-05 +status: active +version: "3.0" +type: planning +tags: [release, strategy, pr-plan, wp3, wp4, wp5, wp6] +--- + # PAI-OpenCode v3.0 - Optimierter PR-PlanAs per coding guidelines: "Frontmatter for Dataview queries where appropriate" und "Frontmatter for important documents" in
docs/**files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/epic/OPTIMIZED-PR-PLAN.md` around lines 1 - 6, Add YAML frontmatter to the top of the document (above the "# PAI-OpenCode v3.0 - Optimierter PR-Plan" header) containing metadata fields such as title, version (v3.0), status, authors, date, and tags, and include a Dataview-compatible query block in the frontmatter or immediately after it for any cross-document indexing; update the document's header to remain untouched and ensure keys are clearly named (e.g., title, version, status, authors, date, tags, dataview) so tools parsing frontmatter/Dataview can consume the metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/epic/OPTIMIZED-PR-PLAN.md`:
- Around line 47-60: The fenced code block that shows the "PAI-Algorithm
Migration" tree is missing a language identifier which triggers markdownlint
MD040; update the opening fence for that block (the fenced code block containing
the PAI-Algorithm Migration tree) to include a language token such as "text" or
"md" (e.g., change ``` to ```text) so the snippet adheres to Markdown best
practices and satisfies the linter.
- Around line 74-85: The fenced code block containing the "Final Delivery:" tree
in OPTIMIZED-PR-PLAN.md is missing a language identifier (MD040); update that
fenced block (the triple-backtick block around the tree listing) to include a
language tag like "text" (e.g., ```text) so markdownlint passes and the snippet
renders consistently; ensure you only add the language identifier to the
existing block and do not change the block contents or surrounding text.
---
Nitpick comments:
In `@docs/epic/OPTIMIZED-PR-PLAN.md`:
- Around line 1-6: Add YAML frontmatter to the top of the document (above the "#
PAI-OpenCode v3.0 - Optimierter PR-Plan" header) containing metadata fields such
as title, version (v3.0), status, authors, date, and tags, and include a
Dataview-compatible query block in the frontmatter or immediately after it for
any cross-document indexing; update the document's header to remain untouched
and ensure keys are clearly named (e.g., title, version, status, authors, date,
tags, dataview) so tools parsing frontmatter/Dataview can consume the metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c8d2bf12-7b20-40d1-98a4-a1e18d1eeca4
📒 Files selected for processing (1)
docs/epic/OPTIMIZED-PR-PLAN.md
GenerateSkillIndex.ts: - Fix hierarchical detection: === 3 instead of >= 3 - Add duplicate skill detection with warning - Improve description regex for folded/literal YAML ValidateSkillStructure.ts: - Fix double recursion bug (add continue) - Report broken symlinks as errors - Add deduplication for missing category errors - Improve description check for multi-line YAML OPTIMIZED-PR-PLAN.md: - Add YAML frontmatter - Add code block language identifiers
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
docs/epic/OPTIMIZED-PR-PLAN.md (1)
51-95: Bitte ergänze ein Mermaid-Diagramm in einklappbarer Sektion.Die ASCII-Übersichten sind gut, aber für die Detaildarstellung der PR-Abhängigkeiten (WP5/WP6) fehlt ein Mermaid-Block in
<details>...</details>.As per coding guidelines: "Mermaid diagrams in collapsible sections for detail".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/epic/OPTIMIZED-PR-PLAN.md` around lines 51 - 95, Add a collapsible Mermaid diagram under the PR `#3` / PR `#4` sections by inserting a <details> block with a <summary> and a fenced mermaid code block that visually shows dependencies between "PR `#3`: WP5 - Algorithm v3.7.0 & Core System (GROSS)" and "PR `#4`: WP6 - Installer, Migration & Release (MITTEL)" (reference these exact headings), linking nodes for PAI/Algorithm/v3.7.0.md, PAI/Tools/, PAI-Install/, Tools/migration-v2-to-v3.ts and Tests/validation/Final integration tests so reviewers can expand to see PR dependencies; ensure the mermaid block follows the existing ASCII tree and obeys the repo guideline "Mermaid diagrams in collapsible sections for detail"..opencode/skills/PAI/Tools/GenerateSkillIndex.ts (2)
209-217: Hierarchische Erkennung korrekt behoben.Die Bedingung
pathParts.length === 3ist nun korrekt für die Category/Skill/SKILL.md-Struktur. Zu tiefe Verschachtelung (>3 Teile) wird hier stillschweigend als nicht-hierarchisch behandelt, währendValidateSkillStructure.tsdies als Fehler meldet.Für bessere Nachvollziehbarkeit könnte eine Warnung bei der Index-Generierung hinzugefügt werden:
💡 Optionale Warnung für tiefe Verschachtelung
const isHierarchical = pathParts.length === 3; const category = isHierarchical ? pathParts[0] : null; + + if (pathParts.length > 3) { + console.warn(`⚠️ Invalid nesting depth for ${relativePath} (expected Category/Skill/SKILL.md)`); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts around lines 209 - 217, The current detection sets isHierarchical based on pathParts.length === 3 but silently treats deeper nesting as non-hierarchical; update GenerateSkillIndex.ts to emit a clear warning when pathParts.length > 3 (e.g., use the module's existing logger or console.warn) including the filePath and computed pathParts so the maintainer can trace the unexpected deep nesting; keep the existing isHierarchical and category logic (variables relativePath, pathParts, isHierarchical, category) unchanged, just add the additional warning branch before or after computing isHierarchical to report the too-deep structure while relying on ValidateSkillStructure.ts to surface it as an error.
107-133: Multi-Line YAML-Parsing konsistent mit ValidateSkillStructure.ts.Die Implementierung ist identisch zur Validierungsdatei. Für konsistente Wartung könnte eine gemeinsame Hilfsfunktion in einer separaten Datei erwogen werden, ist aber für den aktuellen Umfang nicht kritisch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts around lines 107 - 133, The YAML description parsing block (variables descMatch, indicator, rawDesc and the resulting description) is duplicated from ValidateSkillStructure.ts; extract it into a shared helper like parseFrontmatterDescription(frontmatter: string): string and replace the inline logic in GenerateSkillIndex.ts (and the validator) to call that helper, keeping behavior for '>' folded, '|' literal and plain styles identical and returning the trimmed string; ensure the helper is exported and imported where used and update references so no duplicated parsing logic remains..opencode/skills/PAI/Tools/ValidateSkillStructure.ts (1)
211-234: Multi-Line YAML-Parsing funktioniert, aber Einschränkungen beachten.Die Regex-basierte Lösung für mehrzeilige YAML-Beschreibungen (
|und>) ist eine Verbesserung, hat aber Einschränkungen:
- Einrückung wird nicht YAML-konform verarbeitet:
.trim()auf jeder Zeile entfernt Einrückungen, was bei Literal-Blöcken (|) die ursprüngliche Formatierung verliert.- Feld-Namen-Lookahead:
(?=\n[a-zA-Z_]+:|$)erkennt keine YAML-Felder mit Bindestrich (z.B.some-field:).Für den aktuellen Validierungszweck (Prüfung auf "USE WHEN") ist dies ausreichend. Für robustere Parsing-Anforderungen wäre ein YAML-Parser wie
js-yamlempfehlenswert.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/skills/PAI/Tools/ValidateSkillStructure.ts around lines 211 - 234, The current frontmatter parsing incorrectly strips indentation for literal blocks and fails to recognize field names with hyphens; update the lookahead in the regex (change (?=\n[a-zA-Z_]+:|$) to allow hyphens, e.g. (?=\n[0-9A-Za-z_-]+:|$) or a more permissive token) and change the literal ('|') branch handling so it does not call .trim() on each line—preserve leading indentation (e.g., remove only a trailing newline or use rawDesc.replace(/\n$/, '') ) when indicator.includes('|') while leaving the folded ('>') logic as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.opencode/skills/PAI/Tools/ValidateSkillStructure.ts:
- Around line 245-252: The check uses content.length (the entire file) but the
message refers to the SKILL.md body; update the logic to strip or parse
frontmatter and measure only the body length before pushing the warning. Locate
the validation block that uses the content variable and issues.push (and
relativePath) in ValidateSkillStructure.ts, extract the markdown body (e.g.,
remove leading/trailing frontmatter between '---' markers or use the existing
frontmatter parser if available), then replace the content.length check with
body.length < 50 and keep the same issues.push warning using relativePath.
In `@docs/epic/OPTIMIZED-PR-PLAN.md`:
- Line 23: Replace the phrase "statt 1 substantiellem" (line containing
"**Problem:** WP4 wurde in 3 kleine PRs aufgeteilt statt 1 substantiellem.")
with "statt einem substanziellen PR" to correct grammar and clarity, and make
the sentence read naturally; also normalize the language at the sentence
containing "v3.0 wird in insgesamt 4 PRs ausgeliefert." (line ~158) to match the
document language—update that line to the German variant "v3.0 wird in insgesamt
4 PRs ausgeliefert." or convert the surrounding text to English if you prefer
English consistency so the wording and casing are uniform across the document.
---
Nitpick comments:
In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts:
- Around line 209-217: The current detection sets isHierarchical based on
pathParts.length === 3 but silently treats deeper nesting as non-hierarchical;
update GenerateSkillIndex.ts to emit a clear warning when pathParts.length > 3
(e.g., use the module's existing logger or console.warn) including the filePath
and computed pathParts so the maintainer can trace the unexpected deep nesting;
keep the existing isHierarchical and category logic (variables relativePath,
pathParts, isHierarchical, category) unchanged, just add the additional warning
branch before or after computing isHierarchical to report the too-deep structure
while relying on ValidateSkillStructure.ts to surface it as an error.
- Around line 107-133: The YAML description parsing block (variables descMatch,
indicator, rawDesc and the resulting description) is duplicated from
ValidateSkillStructure.ts; extract it into a shared helper like
parseFrontmatterDescription(frontmatter: string): string and replace the inline
logic in GenerateSkillIndex.ts (and the validator) to call that helper, keeping
behavior for '>' folded, '|' literal and plain styles identical and returning
the trimmed string; ensure the helper is exported and imported where used and
update references so no duplicated parsing logic remains.
In @.opencode/skills/PAI/Tools/ValidateSkillStructure.ts:
- Around line 211-234: The current frontmatter parsing incorrectly strips
indentation for literal blocks and fails to recognize field names with hyphens;
update the lookahead in the regex (change (?=\n[a-zA-Z_]+:|$) to allow hyphens,
e.g. (?=\n[0-9A-Za-z_-]+:|$) or a more permissive token) and change the literal
('|') branch handling so it does not call .trim() on each line—preserve leading
indentation (e.g., remove only a trailing newline or use rawDesc.replace(/\n$/,
'') ) when indicator.includes('|') while leaving the folded ('>') logic as-is.
In `@docs/epic/OPTIMIZED-PR-PLAN.md`:
- Around line 51-95: Add a collapsible Mermaid diagram under the PR `#3` / PR `#4`
sections by inserting a <details> block with a <summary> and a fenced mermaid
code block that visually shows dependencies between "PR `#3`: WP5 - Algorithm
v3.7.0 & Core System (GROSS)" and "PR `#4`: WP6 - Installer, Migration & Release
(MITTEL)" (reference these exact headings), linking nodes for
PAI/Algorithm/v3.7.0.md, PAI/Tools/, PAI-Install/, Tools/migration-v2-to-v3.ts
and Tests/validation/Final integration tests so reviewers can expand to see PR
dependencies; ensure the mermaid block follows the existing ASCII tree and obeys
the repo guideline "Mermaid diagrams in collapsible sections for detail".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a22d6637-c1c5-400b-adeb-3f623326c296
📒 Files selected for processing (3)
.opencode/skills/PAI/Tools/GenerateSkillIndex.ts.opencode/skills/PAI/Tools/ValidateSkillStructure.tsdocs/epic/OPTIMIZED-PR-PLAN.md
docs/epic/OPTIMIZED-PR-PLAN.md
Outdated
| | WP3 | Category Structure | 1 PR (#37) | ✅ Gut - 881 Files, substanziell | | ||
| | WP4 | Integration | 3 PRs (#38-#40) | ⚠️ Zu granular - nur ~100 Zeilen total | | ||
|
|
||
| **Problem:** WP4 wurde in 3 kleine PRs aufgeteilt statt 1 substantiellem. |
There was a problem hiding this comment.
Kleine Sprachkorrekturen für Konsistenz und Lesbarkeit.
- Line 23: „statt einem substanziellen (PR)“ statt „statt 1 substantiellem“
- Line 158: Einheitliche Sprache (Deutsch oder Englisch), z. B. „v3.0 wird in insgesamt 4 PRs ausgeliefert.“
Also applies to: 158-158
🧰 Tools
🪛 LanguageTool
[grammar] ~23-~23: Passe das Symbol an
Context: ...ar - nur ~100 Zeilen total | Problem: WP4 wurde in 3 kleine PRs aufgeteilt st...
(QB_NEW_DE_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~23-~23: Hier könnte ein Fehler sein.
Context: ...** WP4 wurde in 3 kleine PRs aufgeteilt statt 1 substantiellem. --- ## Optimierter ...
(QB_NEW_DE)
[grammar] ~23-~23: Korrigiere den Fehler
Context: ...urde in 3 kleine PRs aufgeteilt statt 1 substantiellem. --- ## Optimierter Plan: 4 PRs bis v...
(QB_NEW_DE_OTHER_ERROR_IDS_REPLACEMENT_CONFUSION_SUBSTANTIELLEM_SUBSTANZIELLEM)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/epic/OPTIMIZED-PR-PLAN.md` at line 23, Replace the phrase "statt 1
substantiellem" (line containing "**Problem:** WP4 wurde in 3 kleine PRs
aufgeteilt statt 1 substantiellem.") with "statt einem substanziellen PR" to
correct grammar and clarity, and make the sentence read naturally; also
normalize the language at the sentence containing "v3.0 wird in insgesamt 4 PRs
ausgeliefert." (line ~158) to match the document language—update that line to
the German variant "v3.0 wird in insgesamt 4 PRs ausgeliefert." or convert the
surrounding text to English if you prefer English consistency so the wording and
casing are uniform across the document.
ValidateSkillStructure.ts: - Fix body length check: measure only body (excluding frontmatter), not entire file - Fix YAML regex: allow hyphens in field names ([0-9A-Za-z_-]+) - Preserve indentation for literal blocks (|): use rawDesc.replace(/\n$/, '') GenerateSkillIndex.ts: - Add warning for deep nesting (>3 parts) - Fix YAML regex: allow hyphens in field names - Preserve indentation for literal blocks OPTIMIZED-PR-PLAN.md: - Fix grammar: 'statt 1 substantiellem' → 'statt einem substanziellen PR' - Add collapsible Mermaid diagram showing PR #3 → PR #4 dependencies
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
docs/epic/OPTIMIZED-PR-PLAN.md (1)
185-185:⚠️ Potential issue | 🟡 MinorSprache am Dokumentende vereinheitlichen (DE statt EN).
Auf Line 185 wechselt der Text zu Englisch („released in 4 PRs total.“), obwohl das Dokument sonst Deutsch ist.
🔧 Vorschlag
-**Fertig.** v3.0 released in 4 PRs total. +**Fertig.** v3.0 wird in insgesamt 4 PRs ausgeliefert.As per coding guidelines, "
**/*.md: German language: Use generic masculine (Teilnehmer, nicht Teilnehmer*innen)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/epic/OPTIMIZED-PR-PLAN.md` at line 185, Die Abschlusszeile enthält Englisch ("v3.0 released in 4 PRs total.") in einem ansonsten deutschen Dokument; replace that fragment in the line containing "Fertig. v3.0 released in 4 PRs total." with a German phrasing such as "v3.0 in 4 Pull Requests insgesamt veröffentlicht." and ensure the final sentence follows the repo's German language guideline (use German terms and generic masculine where applicable).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.opencode/skills/PAI/Tools/ValidateSkillStructure.ts:
- Around line 58-73: The code treats symlinks differently but still gates
recursion on entry.isDirectory(), so valid symlinked directories are skipped;
change the logic in ValidateSkillStructure.ts to resolve and store the directory
status (e.g., const isDirectory = entry.isSymbolicLink() ? stats.isDirectory() :
entry.isDirectory()) after the stat(fullPath) call and then use that isDirectory
variable in the subsequent recursion check instead of calling
entry.isDirectory(); follow the same pattern used in GenerateSkillIndex.ts so
symlinked directories are recursively validated and broken symlinks still emit
the existing error.
In @.opencode/skills/skill-index.json:
- Around line 979-980: The skill "worldthreatmodelharness" currently has an
empty triggers array in the index and therefore lacks a usable "USE WHEN"
signal; open the skill's SKILL.md for worldthreatmodelharness, add concise,
actionable "USE WHEN" trigger lines (e.g., when asked to analyze long-term
adversarial threats, create multi-horizon world models, forecast
geopolitical/ideological risks, or produce time-phased threat scenarios) that
match PAI Skills format, update the skill metadata to include those triggers,
and then regenerate the skills index so the new triggers populate the triggers
array in the skill-index.json.
In `@docs/epic/OPTIMIZED-PR-PLAN.md`:
- Around line 121-123: Die Mermaid-Diagrammpfeile sind vermutlich invertiert:
ändere die Beziehung so, dass das abhängige PR auf die PR verweist, von dem es
abhängt (z.B. ersetze `PR3 -->|depends on| PR4` durch `PR4 -->|depends on| PR3`)
und prüfe analogere Einträge wie `C --> E` und `D --> I`, damit die
Pfeilrichtungen die im Plan beschriebene Ausführungsreihenfolge widerspiegeln.
---
Duplicate comments:
In `@docs/epic/OPTIMIZED-PR-PLAN.md`:
- Line 185: Die Abschlusszeile enthält Englisch ("v3.0 released in 4 PRs
total.") in einem ansonsten deutschen Dokument; replace that fragment in the
line containing "Fertig. v3.0 released in 4 PRs total." with a German phrasing
such as "v3.0 in 4 Pull Requests insgesamt veröffentlicht." and ensure the final
sentence follows the repo's German language guideline (use German terms and
generic masculine where applicable).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33eeb3e9-58b2-4944-9ca2-03dd573d67b6
📒 Files selected for processing (4)
.opencode/skills/PAI/Tools/GenerateSkillIndex.ts.opencode/skills/PAI/Tools/ValidateSkillStructure.ts.opencode/skills/skill-index.jsondocs/epic/OPTIMIZED-PR-PLAN.md
ValidateSkillStructure.ts: - Fix symlink handling to resolve and use isDirectory status - Improve description parsing for multiline YAML (> and |) - Use position-based extraction for folded/literal YAML GenerateSkillIndex.ts: - Rewrite parseFrontmatter to handle multiline YAML descriptions - Fix folded YAML: newlines become spaces, strip indentation - Fix literal YAML: preserve content, remove common indentation - WorldThreatModelHarness now correctly extracts 21 triggers OPTIMIZED-PR-PLAN.md: - Fix Mermaid diagram arrow direction - Fix language consistency in final line skill-index.json: Regenerated with proper trigger extraction
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.opencode/skills/PAI/Tools/GenerateSkillIndex.ts (1)
236-242:⚠️ Potential issue | 🟠 MajorTiefe Verschachtelung wird aktuell als
flatin die Statistik aufgenommen.In Line 236–242 wird bei
pathParts.length > 3nur gewarnt; danach wird der Skill implizit als nicht-hierarchisch behandelt und in Line 311 alsflatgezählt. Das verfälschtflatSkillsund die Semantik voncategory.🔧 Vorschlag
if (pathParts.length > 3) { console.warn(`⚠️ Deep nesting detected at ${filePath} (${pathParts.length} levels). Only 2 levels (Category/Skill) are supported.`); + return null; // Ungültige Struktur nicht als flat indexieren }As per coding guidelines: "category=null indicates flat skills, non-null category indicates hierarchical placement."
Also applies to: 301-312
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts around lines 236 - 242, Deeply nested skill paths (pathParts.length > 3) are currently only warned about and then treated as flat, which incorrectly sets category=null and inflates flatSkills; update the classification so any pathParts.length >= 3 is treated as hierarchical: set isHierarchical = pathParts.length >= 3 and category = pathParts[0] (still emit a console.warn for >3), and ensure the later counting logic that updates flatSkills (the block referencing flatSkills and hierarchical counts) uses this isHierarchical flag so deep-nested entries are not counted as flat.
🧹 Nitpick comments (1)
.opencode/skills/PAI/Tools/GenerateSkillIndex.ts (1)
280-323: Deterministische Sortierung reduziert unnötige Diff-Churn im generierten Index.
readdir-/Traversal-Reihenfolge ist nicht garantiert; dadurch können Reihenfolge undcategoryMap-Einträge zwischen Läufen schwanken.♻️ Vorschlag
const skillFiles = await findSkillFiles(SKILLS_DIR); + skillFiles.sort((a, b) => a.localeCompare(b)); ... index.categories = categories.size; + + for (const category of Object.keys(index.categoryMap)) { + index.categoryMap[category].sort((a, b) => a.localeCompare(b)); + }Also applies to: 347-353
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts around lines 280 - 323, The generated index is non-deterministic because directory traversal and insertion order vary; before iterating skillFiles (the loop that calls parseSkillFile), sort skillFiles deterministically (e.g., lexicographically), and after building the index ensure deterministic ordering by creating sorted outputs: sort the categories set, sort each index.categoryMap[category] array (or insert in order), and produce a new index.skills object with keys iterated in sorted order before calling writeFile(OUTPUT_FILE, ...). Update the logic around skillFiles, index.categoryMap, categories and the final writeFile step so the JSON output is stable across runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts:
- Around line 230-232: The current logic building relativePath and splitting
with split('/') is platform-dependent and breaks on Windows; replace the
string-based manipulation by computing the relative path via
path.relative(SKILLS_DIR, filePath) (importing Node's path), then split using
path.sep (e.g., const pathParts = relative.split(path.sep)) and use that
pathParts for isHierarchical; update references to relativePath/pathParts
accordingly so leading separators are handled correctly on all platforms (use
path.relative to avoid manual replace).
In `@docs/epic/OPTIMIZED-PR-PLAN.md`:
- Line 165: Korrigiere den Tippfehler in der Tabellenüberschrift: ersetze die
Zeichenfolge "Altes Plan" durch "Alter Plan" (Tabelle-Header mit "| Kriterium |
Altes Plan (8 PRs) | Neuer Plan (4 PRs) |") so die Grammatik stimmt; suche nach
genau dieser Header-Zeile und aktualisiere nur diesen Text.
---
Duplicate comments:
In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts:
- Around line 236-242: Deeply nested skill paths (pathParts.length > 3) are
currently only warned about and then treated as flat, which incorrectly sets
category=null and inflates flatSkills; update the classification so any
pathParts.length >= 3 is treated as hierarchical: set isHierarchical =
pathParts.length >= 3 and category = pathParts[0] (still emit a console.warn for
>3), and ensure the later counting logic that updates flatSkills (the block
referencing flatSkills and hierarchical counts) uses this isHierarchical flag so
deep-nested entries are not counted as flat.
---
Nitpick comments:
In @.opencode/skills/PAI/Tools/GenerateSkillIndex.ts:
- Around line 280-323: The generated index is non-deterministic because
directory traversal and insertion order vary; before iterating skillFiles (the
loop that calls parseSkillFile), sort skillFiles deterministically (e.g.,
lexicographically), and after building the index ensure deterministic ordering
by creating sorted outputs: sort the categories set, sort each
index.categoryMap[category] array (or insert in order), and produce a new
index.skills object with keys iterated in sorted order before calling
writeFile(OUTPUT_FILE, ...). Update the logic around skillFiles,
index.categoryMap, categories and the final writeFile step so the JSON output is
stable across runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6716f070-a7e2-4d1d-b7da-1d354e489550
📒 Files selected for processing (4)
.opencode/skills/PAI/Tools/GenerateSkillIndex.ts.opencode/skills/PAI/Tools/ValidateSkillStructure.ts.opencode/skills/skill-index.jsondocs/epic/OPTIMIZED-PR-PLAN.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .opencode/skills/PAI/Tools/ValidateSkillStructure.ts
GenerateSkillIndex.ts: - Use path.relative() and path.sep for cross-platform path handling - Fix hierarchical detection: >= 3 parts (not === 3), treat deep nesting as hierarchical - Add deterministic sorting: skillFiles, categories, categoryMap, skills keys - Sort category breakdown output alphabetically OPTIMIZED-PR-PLAN.md: - Fix typo: 'Altes Plan' → 'Alter Plan' skill-index.json: - Regenerated with deterministic sorting (alphabetical order)
Der alte Plan war irreführend: - Er durchnummerierte PRs neu (PR #1 = WP3, etc.) - Ignorierte dass WP1-WP4 bereits vollständig erledigt sind - Beschrieb WP5 falsch (Algorithm bereits in WP1 erledigt) Korrigierte Darstellung: - WP1 (Algorithm v3.7.0): PR #35, #36 ✅ - WP2 (Context Modernization): PR #34 ✅ - WP3 (Category Structure): PR #37 ✅ - WP4 (Integration & Validation): PR #38, #39, #40 ✅ - WP5 (Core PAI System): Noch offen - .opencode/PAI/ Verzeichnis - WP6 (Installer & Migration): Noch offen - Final delivery Ergebnis: Nur noch 2 PRs bis v3.0 statt 4! Zeigt korrekt: - Was fehlt (Core PAI Struktur, fehlende Tools, Installer) - Dass .opencode/PAI/ neu erstellt werden muss - Dass .opencode/skills/PAI/ reduziert wird
Summary
WP4 Phase 3: Enhanced skill discovery system with validation and indexing tools.
Changes
1. Enhanced GenerateSkillIndex.ts
Updated to support WP3 hierarchical structure:
New Fields:
category: Category name or null for flat skillsisHierarchical: Boolean flagcategoryMap: Object mapping categories to skill arraysNew Stats:
categories: Number of categories foundflatSkills: Skills at root levelhierarchicalSkills: Skills in categoriesExample Output:
2. New ValidateSkillStructure.ts
Comprehensive validation tool that checks:
Errors:
Warnings:
Usage:
Test Results:
3. NPM Scripts
Added to package.json:
{ "skills:index": "bun run GenerateSkillIndex.ts", "skills:validate": "bun run ValidateSkillStructure.ts", "skills:check": "runs both sequentially" }Usage:
Files Changed
.opencode/skills/PAI/Tools/GenerateSkillIndex.ts(+enhancements).opencode/skills/PAI/Tools/ValidateSkillStructure.ts(+new, 283 lines).opencode/skills/skill-index.json(+generated index)package.json(+3 scripts)Benefits
✅ Validation: Catch structure errors before they cause issues
✅ Discovery: Fast JSON index for skill lookups
✅ Insights: Understand skill organization (categories vs flat)
✅ Maintenance: Easy to run validation before commits
Related
Testing
Next Steps
WP4 Phase 4 will be final integration testing.
Summary by CodeRabbit
New Features
Chores
Documentation