Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 78 additions & 17 deletions src/beaker_notebook/lib/integrations/skill.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,26 +141,39 @@ def parse_skill_md(content: str) -> tuple[dict, str]:
return frontmatter, body


#: Directory, relative to the skill root, whose contents are examples rather
#: than plain reference files. Referenced paths under it become
#: SkillExampleResources; see `_build_skill_integration`.
EXAMPLES_DIR = "examples/"


def extract_file_references(body: str) -> list[str]:
"""Extract relative file paths referenced in the markdown body.

Detects both markdown links [text](path) and backtick-quoted paths
like `references/some_file.md` or `scripts/run.py`.

Paths under ``examples/`` are included. The caller decides what a path
becomes -- `_build_skill_integration` routes them to SkillExampleResources
rather than SkillFileResources -- so this stays a single, uniform pass over
the body. A bare ``examples/`` directory link names no file and is dropped.
"""
references = []

# Markdown links: [text](path)
link_pattern = re.compile(r'\[(?:[^\]]*)\]\(([^)]+)\)')
for match in link_pattern.finditer(body):
path = match.group(1)
if not path.startswith(("http://", "https://", "#", "mailto:")):
# Skip examples/ paths — they are handled as SkillExampleResources
if not path.startswith("examples/") and path != "examples/":
references.append(path)
if path.startswith(("http://", "https://", "#", "mailto:")):
continue
# A link to the directory itself, not to a file in it.
if path.rstrip("/") == EXAMPLES_DIR.rstrip("/"):
continue
references.append(path)

# Backtick-quoted file paths: `some/path.ext`
# Match paths that contain a / and end with a file extension
backtick_pattern = re.compile(r'`((?:references|scripts|assets)/[^`]+)`')
backtick_pattern = re.compile(r'`((?:references|scripts|assets|examples)/[^`]+)`')
for match in backtick_pattern.finditer(body):
references.append(match.group(1))

Expand Down Expand Up @@ -460,15 +473,24 @@ def _build_skill_integration(
content=body,
)

# One pass over the body; the prefix decides which kind of resource a
# referenced path becomes. Examples have their own tier and their own
# loading tool, so they must not also appear as plain files.
file_resources = []
referenced_examples = []
for ref_path in extract_file_references(body):
file_resources.append(SkillFileResource(
integration=skill.uuid,
name=Path(ref_path).name,
relative_path=ref_path,
))

example_resources = cls._discover_examples(skill, source_type, base_path, base_url)
if ref_path.startswith(EXAMPLES_DIR):
referenced_examples.append(ref_path)
else:
file_resources.append(SkillFileResource(
integration=skill.uuid,
name=Path(ref_path).name,
relative_path=ref_path,
))

example_resources = cls._discover_examples(
skill, source_type, base_path, base_url, referenced=referenced_examples
)

skill.add_resources([metadata_resource, instructions_resource] + file_resources + example_resources)
return skill
Expand All @@ -480,14 +502,31 @@ def _discover_examples(
source_type: str,
base_path: Optional[str] = None,
base_url: Optional[str] = None,
referenced: "Optional[list[str]]" = None,
) -> list[SkillExampleResource]:
"""Discover example files from the skill's examples/ directory.
"""Discover a skill's examples, from the directory and from the body.

Two sources, in precedence order:

For local skills, scans the examples/ subdirectory. For remote skills,
examples must be declared in the frontmatter (not yet implemented).
Only reads the first few lines of each file to extract title and description.
1. **Local directory scan.** Reads enough of each ``examples/*.md`` file
to pull out a title and description. Only possible for local skills.
2. **Paths referenced in SKILL.md**, passed in as ``referenced``. This is
the only source available to a remote skill, where the directory
cannot be listed over HTTP, and it also picks up local examples the
``*.md`` glob misses -- a skill shipping ``examples/*.py`` was
previously invisible whether it was local or remote.

The scan wins on conflict because it carries a real parsed title and
description; a referenced path yields only its filename. That costs
little, because the agent must call ``load_skill_instructions`` -- which
returns the whole body, author's example listing included -- before it
can ask for an example by name.

Content is never fetched here; it stays tier 3, loaded on demand.
"""
resources = []
seen: set[str] = set()

if source_type == "local" and base_path:
examples_dir = Path(base_path) / "examples"
if examples_dir.is_dir():
Expand All @@ -502,8 +541,25 @@ def _discover_examples(
description=description,
content=None, # Loaded on demand (tier 3)
))
seen.add(example_path.name)
except Exception:
logger.exception("Failed to parse example: %s", example_path)

for relative_path in referenced or []:
# Keep the path below examples/ as the filename so nested examples
# round-trip through load_skill_examples and _fetch_file_content.
filename = relative_path[len(EXAMPLES_DIR):]
if not filename or filename in seen:
continue
seen.add(filename)
resources.append(SkillExampleResource(
integration=skill.uuid,
filename=filename,
title=filename,
description="",
content=None, # Loaded on demand (tier 3)
))

return resources

# --- Abstract method implementations ---
Expand Down Expand Up @@ -662,7 +718,12 @@ async def load_skill_instructions(self, skill_slug: str, agent: AgentRef) -> str
if examples:
result += "\n\n## Available Code Examples\n"
for ex in examples:
result += f"\n- **{ex.filename}**: {ex.title}"
result += f"\n- **{ex.filename}**"
# An example discovered from a reference in the body has no
# title beyond its own filename; printing it twice reads as a
# bug and spends tokens saying nothing.
if ex.title and ex.title != ex.filename:
result += f": {ex.title}"
if ex.description:
result += f"\n {ex.description}"
result += (
Expand Down
194 changes: 192 additions & 2 deletions tests/integrations/skills/test_skill_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,17 +206,26 @@ def test_mixed_references(self):
assert "references/patterns.md" in refs
assert "scripts/run.py" in refs

def test_examples_paths_excluded(self):
def test_example_paths_included_but_bare_directory_link_dropped(self):
"""examples/ files are extracted here; the caller routes them onward.

A link to the directory itself names no file, so it is dropped rather
than becoming a resource that cannot be loaded.
"""
body = (
"See [examples/](examples/) for working examples.\n"
"Also see [examples/basic.md](examples/basic.md) for a quick start.\n"
"And [the guide](references/guide.md) for docs."
)
refs = extract_file_references(body)
assert "examples/" not in refs
assert "examples/basic.md" not in refs
assert "examples/basic.md" in refs
assert "references/guide.md" in refs

def test_backtick_example_paths(self):
refs = extract_file_references("Run `examples/quickstart.py` to start.")
assert "examples/quickstart.py" in refs


# ---------------------------------------------------------------------------
# SkillIntegrationProvider — local loading
Expand Down Expand Up @@ -610,6 +619,187 @@ def test_url_with_trailing_slash(self):
mock_get.assert_called_once_with("https://example.com/repo/main/SKILL.md", timeout=30)


# ---------------------------------------------------------------------------
# Remote example discovery
# ---------------------------------------------------------------------------

REMOTE_SKILL_WITH_EXAMPLES_MD = textwrap.dedent("""\
---
name: remote-skill
description: A skill served over HTTP.
---

# Remote skill

See [the guide](references/guide.md) for details.

## Runnable examples

- [examples/basic.py](examples/basic.py) — the simplest case
- [examples/advanced.py](examples/advanced.py) — every knob turned
""")


def _load_remote_skill_with_examples():
mock_response = MagicMock()
mock_response.text = REMOTE_SKILL_WITH_EXAMPLES_MD
mock_response.raise_for_status = MagicMock()
with patch("beaker_notebook.lib.integrations.skill.requests.get", return_value=mock_response):
return SkillIntegrationProvider._load_remote_skill("https://example.com/skill/")


class TestRemoteExampleDiscovery:
"""A remote skill's examples come from its own body.

Listing a directory over HTTP is not possible, so before this the agent saw
no examples at all for any remotely-loaded skill.
"""

def test_examples_discovered_from_body(self):
skill = _load_remote_skill_with_examples()
examples = [r for r in skill.resources.values() if isinstance(r, SkillExampleResource)]
assert {e.filename for e in examples} == {"basic.py", "advanced.py"}

def test_filename_is_the_handle(self):
"""The filename is what load_skill_examples is called with."""
skill = _load_remote_skill_with_examples()
by_name = {
r.filename: r
for r in skill.resources.values()
if isinstance(r, SkillExampleResource)
}
assert by_name["basic.py"].title == "basic.py"
# The author's own gloss already reached the agent with the body, which
# load_skill_instructions returns in full before examples can be asked for.
assert by_name["basic.py"].description == ""

def test_content_is_not_fetched_during_discovery(self):
"""Discovery must stay free; content is tier 3, loaded on demand."""
skill = _load_remote_skill_with_examples()
examples = [r for r in skill.resources.values() if isinstance(r, SkillExampleResource)]
assert examples and all(e.content is None for e in examples)

def test_only_one_request_is_made(self):
mock_response = MagicMock()
mock_response.text = REMOTE_SKILL_WITH_EXAMPLES_MD
mock_response.raise_for_status = MagicMock()
with patch(
"beaker_notebook.lib.integrations.skill.requests.get", return_value=mock_response
) as mock_get:
SkillIntegrationProvider._load_remote_skill("https://example.com/skill/")
mock_get.assert_called_once()

def test_example_content_fetches_from_the_base_url(self):
skill = _load_remote_skill_with_examples()
provider = _make_provider([])
mock_response = MagicMock()
mock_response.text = "print('hi')"
mock_response.raise_for_status = MagicMock()
with patch(
"beaker_notebook.lib.integrations.skill.requests.get", return_value=mock_response
) as mock_get:
content = provider._fetch_file_content(skill, "examples/basic.py")
mock_get.assert_called_once_with(
"https://example.com/skill/examples/basic.py", timeout=30
)
assert content == "print('hi')"

def test_examples_are_not_also_file_resources(self):
"""Examples have their own tier; they must not double as reference files."""
skill = _load_remote_skill_with_examples()
file_paths = {
r.relative_path
for r in skill.resources.values()
if isinstance(r, SkillFileResource)
}
assert file_paths == {"references/guide.md"}

def test_skill_without_example_links_has_none(self):
mock_response = MagicMock()
mock_response.text = MINIMAL_SKILL_MD
mock_response.raise_for_status = MagicMock()
with patch("beaker_notebook.lib.integrations.skill.requests.get", return_value=mock_response):
skill = SkillIntegrationProvider._load_remote_skill("https://example.com/skill/")
assert not [r for r in skill.resources.values() if isinstance(r, SkillExampleResource)]

def test_nested_example_path_round_trips(self):
"""A nested path keeps enough of itself to be fetched back."""
md = MINIMAL_SKILL_MD.rstrip() + "\n\n[deep](examples/advanced/tuning.py)\n"
resp = MagicMock(text=md, raise_for_status=MagicMock())
with patch("beaker_notebook.lib.integrations.skill.requests.get", return_value=resp):
skill = SkillIntegrationProvider._load_remote_skill("https://example.com/skill/")
example = next(
r for r in skill.resources.values() if isinstance(r, SkillExampleResource)
)
assert example.filename == "advanced/tuning.py"

provider = _make_provider([])
fetched = MagicMock(text="x = 1", raise_for_status=MagicMock())
with patch(
"beaker_notebook.lib.integrations.skill.requests.get", return_value=fetched
) as mock_get:
provider._fetch_file_content(skill, f"examples/{example.filename}")
mock_get.assert_called_once_with(
"https://example.com/skill/examples/advanced/tuning.py", timeout=30
)


# ---------------------------------------------------------------------------
# Example discovery: directory scan and body references combined
# ---------------------------------------------------------------------------

class TestExampleSourcesAreDeduped:
"""Local skills draw examples from two sources, which must not collide."""

def test_directory_scan_wins_over_body_reference(self, skill_dir: Path):
"""The scan carries a real parsed title; a bare reference does not."""
skill_md = skill_dir / "SKILL.md"
skill_md.write_text(
skill_md.read_text()
+ "\n\nSee [examples/basic_usage.md](examples/basic_usage.md) too.\n"
)
skill = SkillIntegrationProvider._load_local_skill(str(skill_dir))
matching = [
r for r in skill.resources.values()
if isinstance(r, SkillExampleResource) and r.filename == "basic_usage.md"
]
assert len(matching) == 1, "example listed twice"
assert matching[0].title == "Basic usage of the full skill"

def test_body_reference_adds_examples_the_glob_misses(self, skill_dir: Path):
"""A skill shipping examples/*.py was previously invisible entirely.

_discover_examples only globs *.md, so a .py example is found solely
because SKILL.md points at it.
"""
(skill_dir / "examples" / "script_example.py").write_text("print('hi')")
skill_md = skill_dir / "SKILL.md"
skill_md.write_text(
skill_md.read_text()
+ "\n\n- [examples/script_example.py](examples/script_example.py) - a script\n"
)
skill = SkillIntegrationProvider._load_local_skill(str(skill_dir))
names = {
r.filename for r in skill.resources.values()
if isinstance(r, SkillExampleResource)
}
assert names == {"basic_usage.md", "advanced_usage.md", "script_example.py"}

def test_referenced_examples_are_not_also_file_resources(self, skill_dir: Path):
"""Examples have their own tier and must not double as reference files."""
skill_md = skill_dir / "SKILL.md"
skill_md.write_text(
skill_md.read_text() + "\n\n[x](examples/basic_usage.md)\n"
)
skill = SkillIntegrationProvider._load_local_skill(str(skill_dir))
file_paths = {
r.relative_path for r in skill.resources.values()
if isinstance(r, SkillFileResource)
}
assert not any(p.startswith("examples/") for p in file_paths)



# ---------------------------------------------------------------------------
# parse_example_md
# ---------------------------------------------------------------------------
Expand Down
Loading