Skip to content

Commit ce918cf

Browse files
committed
feat(skill): deepen compile pipeline + design-consistency cleanup
Two rounds of review fixes on top of the skill factory. Design consistency (from PR-level review): - workspace: restore_iteration now saves the current skill before overwriting, matching skill new's reversibility invariant — a user who edits files in chat then rolls back keeps those edits as a new iteration - generator: validate_skill moved inside Generator.run() so /skill new in chat gets the same quality gate as openkb skill new (was CLI-only) - skill/__init__: single-source path helpers (skills_root / skill_dir / skill_workspace_dir) and frontmatter parser (extract_frontmatter / extract_description). Removes duplicated path construction across 6+ sites and four divergent frontmatter implementations - cli/chat: route every "output/skills/<name>" construction through the helper; add missing "Run openkb init first." hint to skill validate / skill eval; capitalize /skill autocomplete; fix stale comment Skills domain quality (the compile pipeline can actually distill now): - creator/tools: skill agent gets the query agent's deep-retrieval toolset — get_page_content for page-range source reads on PageIndex docs, get_image for figures. query_wiki demoted to narrow follow-ups only; primary traversal is direct file reads - skill_create.md: rewritten end-to-end. Working method now mirrors query's 6-step strategy (survey -> summaries -> sources via full_text pointer -> concepts -> draft -> review-and-revise). Required output adds a "Core decision rules" section (>=5 if-X-then-Y rules) and a "Known gaps" section. Description-writing rules optimize for trigger predicate (situations, keywords, exclusions), not topic accuracy - evaluator: was a tautology (generator and grader both saw only the description -> a LOREM IPSUM body could pass 100%). Now the generator reads body + reference excerpts so prompts reflect actual claims, and a second grader (grade_coverage) checks whether the body has substance to support what the description promises. CLI surfaces both Trigger accuracy and Body coverage - validator: foreign-wikilink gate. SKILL.md and references/*.md cannot contain [[concepts/]] / [[summaries/]] / [[sources/]] — those point at the producer's wiki, which doesn't ship; on the consumer's machine they are dead links plus wasted context tokens. Only [[references/<slug>]] (which ships with the skill) is valid - prompt's source-use rules: paraphrase, short quotes <=40 words, no bulk copying; provenance audit is the producer's responsibility at compile time, not something to ship in the artifact Tests: +9 new (rollback preservation, body-coverage grader, foreign wikilink detection in body and references). 504 passing.
1 parent 1679c8c commit ce918cf

15 files changed

Lines changed: 833 additions & 206 deletions

openkb/agent/chat.py

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ def _bottom_toolbar(session: ChatSession) -> FormattedText:
215215
("/list", "List all documents"),
216216
("/lint", "Lint the knowledge base"),
217217
("/add", "Add a document or directory"),
218-
("/skill", "compile a skill (try `/skill new <name> \"intent\"`)"),
218+
("/skill", "Compile a skill (try `/skill new <name> \"intent\"`)"),
219219
]
220220

221221

@@ -530,7 +530,8 @@ async def _handle_slash_skill(arg: str, kb_dir: Path, style: Style) -> None:
530530
_fmt(style, ("class:error", f"[ERROR] {err}\n"))
531531
return
532532

533-
target = kb_dir / "output" / "skills" / name
533+
from openkb.skill import skill_dir
534+
target = skill_dir(kb_dir, name)
534535
if target.exists():
535536
_fmt(style, ("class:error",
536537
f"[ERROR] output/skills/{name}/ already exists. Remove it first "
@@ -544,19 +545,31 @@ async def _handle_slash_skill(arg: str, kb_dir: Path, style: Style) -> None:
544545

545546
from openkb.skill.generator import Generator
546547
_fmt(style, ("class:slash.help", f"Compiling skill '{name}'...\n"))
548+
gen = Generator(
549+
target_type="skill",
550+
name=name,
551+
intent=intent,
552+
kb_dir=kb_dir,
553+
model=model,
554+
)
547555
try:
548-
gen = Generator(
549-
target_type="skill",
550-
name=name,
551-
intent=intent,
552-
kb_dir=kb_dir,
553-
model=model,
554-
)
555556
await gen.run()
556557
except RuntimeError as exc:
557558
_fmt(style, ("class:error", f"[ERROR] {exc}\n"))
558559
return
559560

561+
# Surface validation issues from Generator.run (same gate as CLI).
562+
result = gen.validation
563+
if result is not None and (result.errors or result.warnings):
564+
_fmt(style, ("class:error", "[WARN] Validation found issues:\n"))
565+
for err in result.errors:
566+
_fmt(style, ("class:error", f" ERROR: {err}\n"))
567+
for warn in result.warnings:
568+
_fmt(style, ("class:error", f" WARN: {warn}\n"))
569+
_fmt(style, ("class:slash.help",
570+
f"Run `openkb skill validate {name}` to re-check, or "
571+
f"`openkb skill rollback {name}` to revert.\n"))
572+
560573
_fmt(style, ("class:slash.ok", f"Saved: output/skills/{name}/\n"))
561574
_fmt(style, ("class:slash.help",
562575
f"Iterate: ask follow-up questions in this chat and the agent can "

openkb/cli.py

Lines changed: 51 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,9 @@ def _preflight_skill_new(kb_dir: Path, name: str) -> str | None:
206206

207207
def _clear_existing_skill_dir(kb_dir: Path, name: str) -> None:
208208
"""Delete an existing ``<kb>/output/skills/<name>/`` directory."""
209-
target = kb_dir / "output" / "skills" / name
209+
from openkb.skill import skill_dir
210+
211+
target = skill_dir(kb_dir, name)
210212
if target.exists():
211213
shutil.rmtree(target)
212214

@@ -1465,10 +1467,11 @@ def skill_new(ctx, name, intent, yes_flag):
14651467
# When overwriting, we don't destroy the old skill — we copy it
14661468
# into <kb>/output/skills/<name>-workspace/iteration-N/ first, so
14671469
# the user can roll back via `openkb skill rollback`. See
1468-
# ``openkb/skill_workspace.py``.
1470+
# ``openkb/skill/workspace.py``.
1471+
from openkb.skill import skill_dir
14691472
from openkb.skill.workspace import save_iteration, write_diff
14701473

1471-
target = kb_dir / "output" / "skills" / name
1474+
target = skill_dir(kb_dir, name)
14721475
saved_iteration: Path | None = None
14731476
if target.exists():
14741477
if yes_flag:
@@ -1491,17 +1494,18 @@ def skill_new(ctx, name, intent, yes_flag):
14911494
)
14921495
ctx.exit(1)
14931496

1494-
# Run the generator
1497+
# Run the generator. Generator.run handles compile -> validate ->
1498+
# marketplace publish, so both CLI and chat get the same quality gate.
14951499
from openkb.skill.generator import Generator
14961500
click.echo(f"Compiling skill '{name}'...")
1501+
gen = Generator(
1502+
target_type="skill",
1503+
name=name,
1504+
intent=intent,
1505+
kb_dir=kb_dir,
1506+
model=model,
1507+
)
14971508
try:
1498-
gen = Generator(
1499-
target_type="skill",
1500-
name=name,
1501-
intent=intent,
1502-
kb_dir=kb_dir,
1503-
model=model,
1504-
)
15051509
asyncio.run(gen.run())
15061510
except RuntimeError as exc:
15071511
click.echo(f"[ERROR] {exc}", err=True)
@@ -1517,12 +1521,10 @@ def skill_new(ctx, name, intent, yes_flag):
15171521
"diff generation failed: %s", exc, exc_info=True
15181522
)
15191523

1520-
# Auto-validate the freshly compiled skill. Surface issues but don't
1521-
# block — files are on disk and the user can fix or rollback.
1522-
from openkb.skill.validator import validate_skill
1523-
skill_dir = kb_dir / "output" / "skills" / name
1524-
result = validate_skill(skill_dir)
1525-
if result.errors or result.warnings:
1524+
# Surface validation issues. Don't block — files are on disk and
1525+
# the user can fix or rollback.
1526+
result = gen.validation
1527+
if result is not None and (result.errors or result.warnings):
15261528
click.echo("\n[WARN] Validation found issues:")
15271529
for err in result.errors:
15281530
click.echo(f" ERROR: {err}")
@@ -1581,7 +1583,8 @@ def skill_history(ctx, name):
15811583
stamp = "-"
15821584
click.echo(f" {n} {rel} {stamp}")
15831585

1584-
current = kb_dir / "output" / "skills" / name
1586+
from openkb.skill import skill_dir
1587+
current = skill_dir(kb_dir, name)
15851588
if current.is_dir():
15861589
rel_curr = current.relative_to(kb_dir)
15871590
click.echo(f"\n Current: {rel_curr}/")
@@ -1642,7 +1645,8 @@ def skill_rollback(ctx, name, to_n, yes_flag):
16421645
)
16431646
ctx.exit(1)
16441647

1645-
current = kb_dir / "output" / "skills" / name
1648+
from openkb.skill import skill_dir
1649+
current = skill_dir(kb_dir, name)
16461650
if current.exists():
16471651
prompt = (
16481652
f"This will overwrite output/skills/{name}/ with {target_label}. Continue?"
@@ -1681,27 +1685,28 @@ def skill_rollback(ctx, name, to_n, yes_flag):
16811685
@click.pass_context
16821686
def skill_validate(ctx, name, strict):
16831687
"""Validate one skill (by name) or all compiled skills in this KB."""
1688+
from openkb.skill import skill_dir, skills_root
16841689
from openkb.skill.validator import validate_skill
16851690

16861691
kb_dir = _find_kb_dir(ctx.obj.get("kb_dir_override"))
16871692
if kb_dir is None:
1688-
click.echo("No knowledge base found.", err=True)
1693+
click.echo("No knowledge base found. Run `openkb init` first.", err=True)
16891694
ctx.exit(1)
16901695

1691-
skills_root = kb_dir / "output" / "skills"
1692-
if not skills_root.is_dir():
1696+
root = skills_root(kb_dir)
1697+
if not root.is_dir():
16931698
click.echo("No skills found. Compile one with `openkb skill new`.")
16941699
return
16951700

16961701
if name:
1697-
target = skills_root / name
1702+
target = skill_dir(kb_dir, name)
16981703
if not target.is_dir():
16991704
click.echo(f"[ERROR] Skill '{name}' not found.", err=True)
17001705
ctx.exit(1)
17011706
targets = [target]
17021707
else:
17031708
targets = sorted(
1704-
d for d in skills_root.iterdir()
1709+
d for d in root.iterdir()
17051710
if d.is_dir() and not d.name.endswith("-workspace")
17061711
)
17071712

@@ -1748,12 +1753,14 @@ def skill_eval(ctx, name, save_flag, eval_set_path, count):
17481753
run_eval, save_eval_set, load_eval_set, EvalPrompt,
17491754
)
17501755

1756+
from openkb.skill import skill_dir as _skill_dir
1757+
17511758
kb_dir = _find_kb_dir(ctx.obj.get("kb_dir_override"))
17521759
if kb_dir is None:
1753-
click.echo("No knowledge base found.", err=True)
1760+
click.echo("No knowledge base found. Run `openkb init` first.", err=True)
17541761
ctx.exit(1)
17551762

1756-
skill_dir = kb_dir / "output" / "skills" / name
1763+
skill_dir = _skill_dir(kb_dir, name)
17571764
if not skill_dir.is_dir():
17581765
click.echo(f"[ERROR] Skill '{name}' not found.", err=True)
17591766
ctx.exit(1)
@@ -1783,16 +1790,29 @@ def skill_eval(ctx, name, save_flag, eval_set_path, count):
17831790

17841791
click.echo(f"\nEval set: {result.total} prompts")
17851792
click.echo(
1786-
f"Pass rate: {result.passed}/{result.total} "
1787-
f"({result.pass_rate * 100:.0f}%)"
1793+
f"Trigger accuracy: {result.passed}/{result.total} "
1794+
f"({result.pass_rate * 100:.0f}%) "
1795+
f"— does the description fire on the right questions?"
1796+
)
1797+
click.echo(
1798+
f"Body coverage: {result.coverage_passed}/{result.trigger_questions} "
1799+
f"({result.coverage_rate * 100:.0f}%) "
1800+
f"— does SKILL.md actually support what the description promises?"
17881801
)
17891802

17901803
if result.misses:
1791-
click.echo(f"\nMisses ({len(result.misses)}):")
1804+
click.echo(f"\nTrigger misses ({len(result.misses)}):")
17921805
for miss in result.misses:
17931806
click.echo(f" - {miss.label} {miss.prompt.question}")
1794-
else:
1795-
click.echo("\nAll prompts graded correctly.")
1807+
1808+
if result.coverage_misses:
1809+
click.echo(f"\nCoverage gaps ({len(result.coverage_misses)}):")
1810+
for gap in result.coverage_misses:
1811+
tail = f" — {gap.reason}" if gap.reason else ""
1812+
click.echo(f" - {gap.prompt.question}{tail}")
1813+
1814+
if not result.misses and not result.coverage_misses:
1815+
click.echo("\nAll prompts graded correctly with full body support.")
17961816

17971817
if save_flag and eval_set is None:
17981818
path = save_eval_set(kb_dir, name, result.prompts)

0 commit comments

Comments
 (0)