From d012708ce2a7f2a26ba58b3ce4e975b39e481867 Mon Sep 17 00:00:00 2001 From: Brandon Rose Date: Wed, 22 Jul 2026 10:50:00 -0500 Subject: [PATCH 1/3] Discover examples for remotely-loaded skills A skill loaded from a URL exposed no examples at all. `_discover_examples` scanned the `examples/` directory, which only works for a local skill -- the remote branch was a comment saying examples "must be declared in the frontmatter (not yet implemented)". So a remote skill gave the agent instructions and reference files, and silently dropped every worked example the author shipped. That is the tier the agent most wants before writing code, and the loss is invisible: the prompt reports `has_code_examples: false` and nothing indicates that the examples exist but could not be seen. Take them from the body instead of the filesystem. Skill authors already publish an example listing as markdown links -- a bulleted `[examples/x.py] (examples/x.py) - what it shows` is the convention -- and `_build_skill_ integration` already has the parsed body in hand. Titles come from the link text, descriptions from the text following the link, and content is still fetched on demand, so discovery costs no additional requests. Local skills keep scanning the directory, which stays authoritative. Verified against two published skills served from raw.githubusercontent.com: all eight examples are now discovered with their descriptions intact, and loading one fetches correctly from the skill's base URL. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01555XSwMPRxBZFAEsvubUMz --- src/beaker_notebook/lib/integrations/skill.py | 58 ++++++- .../skills/test_skill_provider.py | 154 ++++++++++++++++++ 2 files changed, 207 insertions(+), 5 deletions(-) diff --git a/src/beaker_notebook/lib/integrations/skill.py b/src/beaker_notebook/lib/integrations/skill.py index 254a71be..5220908c 100644 --- a/src/beaker_notebook/lib/integrations/skill.py +++ b/src/beaker_notebook/lib/integrations/skill.py @@ -168,6 +168,36 @@ def extract_file_references(body: str) -> list[str]: return list(dict.fromkeys(references)) +# A markdown link into examples/, plus whatever text follows it on the line. +# Skill authors conventionally list examples as +# - [examples/foo.py](examples/foo.py) — what it demonstrates +# so the trailing text is the natural description. +_EXAMPLE_LINK_PATTERN = re.compile(r'\[([^\]]*)\]\((examples/[^)\s]+)\)([^\n]*)') + + +def extract_example_references(body: str) -> list[tuple[str, str, str]]: + """Extract examples declared as markdown links in the body. + + Used for remote skills, where the ``examples/`` directory cannot be listed. + Local skills scan the directory instead, which stays authoritative. + + Returns ``(relative_path, title, description)`` per example, deduplicated on + path and in document order. ``relative_path`` is relative to the skill root + (i.e. it retains the ``examples/`` prefix). + """ + examples: dict[str, tuple[str, str, str]] = {} + for match in _EXAMPLE_LINK_PATTERN.finditer(body): + link_text, path, trailing = match.group(1).strip(), match.group(2).strip(), match.group(3) + if path in examples: + continue + # Authors usually write the path as the link text; a filename reads + # better as a title than a repeated path does. + title = link_text if link_text and link_text != path else Path(path).name + description = trailing.strip().lstrip("-–—:*").strip() + examples[path] = (path, title, description) + return list(examples.values()) + + def parse_example_md(content: str) -> tuple[str, str]: """Extract (title, description) from an example markdown file. @@ -468,7 +498,7 @@ def _build_skill_integration( relative_path=ref_path, )) - example_resources = cls._discover_examples(skill, source_type, base_path, base_url) + example_resources = cls._discover_examples(skill, source_type, base_path, base_url, body=body) skill.add_resources([metadata_resource, instructions_resource] + file_resources + example_resources) return skill @@ -480,12 +510,18 @@ def _discover_examples( source_type: str, base_path: Optional[str] = None, base_url: Optional[str] = None, + body: str = "", ) -> list[SkillExampleResource]: - """Discover example files from the skill's examples/ directory. + """Discover a skill's examples. + + Local skills scan the ``examples/`` subdirectory, reading only enough of + each file to pull out a title and description. - 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. + Remote skills cannot list a directory over HTTP, so their examples are + taken from the ``examples/`` markdown links in the body -- the listing + skill authors already write. Titles and descriptions come from the link + text and the text following it, so discovery costs no extra requests; + content is still fetched on demand. """ resources = [] if source_type == "local" and base_path: @@ -504,6 +540,18 @@ def _discover_examples( )) except Exception: logger.exception("Failed to parse example: %s", example_path) + elif source_type == "remote" and base_url: + for relative_path, title, description in extract_example_references(body): + # Keep the path below examples/ as the filename so nested + # examples round-trip through load_skill_examples correctly. + filename = relative_path[len("examples/"):] + resources.append(SkillExampleResource( + integration=skill.uuid, + filename=filename, + title=title, + description=description, + content=None, # Loaded on demand (tier 3) + )) return resources # --- Abstract method implementations --- diff --git a/tests/integrations/skills/test_skill_provider.py b/tests/integrations/skills/test_skill_provider.py index 581cf26a..d7c13603 100644 --- a/tests/integrations/skills/test_skill_provider.py +++ b/tests/integrations/skills/test_skill_provider.py @@ -12,6 +12,7 @@ SkillIntegrationProvider, parse_skill_md, parse_example_md, + extract_example_references, extract_file_references, ) from beaker_notebook.lib.integrations.types import ( @@ -218,6 +219,58 @@ def test_examples_paths_excluded(self): assert "references/guide.md" in refs +# --------------------------------------------------------------------------- +# extract_example_references +# --------------------------------------------------------------------------- + +class TestExtractExampleReferences: + def test_conventional_listing(self): + """The form skill authors actually write: a bulleted link plus a gloss.""" + body = ( + "## Runnable examples\n\n" + "- [examples/basic_fetch.py](examples/basic_fetch.py) — obs + hindcast fetch\n" + "- [examples/s2s.py](examples/s2s.py) — sub-seasonal issuance\n" + ) + assert extract_example_references(body) == [ + ("examples/basic_fetch.py", "basic_fetch.py", "obs + hindcast fetch"), + ("examples/s2s.py", "s2s.py", "sub-seasonal issuance"), + ] + + def test_descriptive_link_text_becomes_the_title(self): + body = "- [End-to-end forecast](examples/e2e.py) — optimize then verify\n" + assert extract_example_references(body) == [ + ("examples/e2e.py", "End-to-end forecast", "optimize then verify"), + ] + + def test_separators_and_bullets_stripped_from_description(self): + for separator in ("-", "–", "—", ":"): + body = f"- [x](examples/a.py) {separator} does a thing\n" + assert extract_example_references(body)[0][2] == "does a thing" + + def test_link_without_description(self): + assert extract_example_references("[x](examples/a.py)\n") == [ + ("examples/a.py", "x", ""), + ] + + def test_deduplicated_in_document_order(self): + body = ( + "- [a](examples/a.py) — first\n" + "- [b](examples/b.py) — second\n" + "See [a](examples/a.py) again.\n" + ) + paths = [path for path, _, _ in extract_example_references(body)] + assert paths == ["examples/a.py", "examples/b.py"] + + def test_non_example_links_ignored(self): + body = "[guide](references/guide.md) and [site](https://example.com/examples/x.py)\n" + assert extract_example_references(body) == [] + + def test_nested_path_retained(self): + """Paths keep the examples/ prefix so they can be fetched back.""" + body = "[deep](examples/advanced/tuning.py) — tuning\n" + assert extract_example_references(body)[0][0] == "examples/advanced/tuning.py" + + # --------------------------------------------------------------------------- # SkillIntegrationProvider — local loading # --------------------------------------------------------------------------- @@ -610,6 +663,107 @@ 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_descriptions_come_from_the_listing(self): + 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"].description == "the simplest case" + assert by_name["advanced.py"].description == "every knob turned" + + 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)] + + # --------------------------------------------------------------------------- # parse_example_md # --------------------------------------------------------------------------- From c8fe864aeb08eab8e004e59ac033063991af46c2 Mon Sep 17 00:00:00 2001 From: Brandon Rose Date: Wed, 22 Jul 2026 11:39:21 -0500 Subject: [PATCH 2/3] Extract examples through extract_file_references (review feedback) Per review: drop the examples/ guard in extract_file_references rather than adding a second regex pass over the same body, and dedupe the two example sources where they meet. Extraction is now one uniform pass; the caller decides what a path becomes. _build_skill_integration routes examples/ paths to SkillExampleResources and everything else to SkillFileResources, so examples still do not double as reference files. A bare examples/ directory link names no file and is dropped rather than becoming an unloadable resource. The backtick pattern gained examples/ for the same consistency. _discover_examples now takes the referenced paths and merges them with the directory scan, keyed on filename, scan winning -- it carries a real parsed title and description where a referenced path yields only a filename. Losing the link glosses costs little: the agent must call load_skill_instructions, which returns the whole body including the author's own example listing, before it can ask for an example by name. Those descriptions were duplicated context. This also fixes local skills, which the first version did not. The scan globs *.md, so a skill shipping examples/*.py was invisible whether loaded locally or remotely. Both paths now see them. 487 passed. Verified against the two published skills: 4 examples each, none leaking into file resources, content still fetched on demand. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01555XSwMPRxBZFAEsvubUMz --- src/beaker_notebook/lib/integrations/skill.py | 132 +++++++-------- .../skills/test_skill_provider.py | 150 +++++++++++------- 2 files changed, 163 insertions(+), 119 deletions(-) diff --git a/src/beaker_notebook/lib/integrations/skill.py b/src/beaker_notebook/lib/integrations/skill.py index 5220908c..1bd59e0c 100644 --- a/src/beaker_notebook/lib/integrations/skill.py +++ b/src/beaker_notebook/lib/integrations/skill.py @@ -141,11 +141,22 @@ 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 = [] @@ -153,14 +164,16 @@ def extract_file_references(body: str) -> list[str]: 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)) @@ -168,36 +181,6 @@ def extract_file_references(body: str) -> list[str]: return list(dict.fromkeys(references)) -# A markdown link into examples/, plus whatever text follows it on the line. -# Skill authors conventionally list examples as -# - [examples/foo.py](examples/foo.py) — what it demonstrates -# so the trailing text is the natural description. -_EXAMPLE_LINK_PATTERN = re.compile(r'\[([^\]]*)\]\((examples/[^)\s]+)\)([^\n]*)') - - -def extract_example_references(body: str) -> list[tuple[str, str, str]]: - """Extract examples declared as markdown links in the body. - - Used for remote skills, where the ``examples/`` directory cannot be listed. - Local skills scan the directory instead, which stays authoritative. - - Returns ``(relative_path, title, description)`` per example, deduplicated on - path and in document order. ``relative_path`` is relative to the skill root - (i.e. it retains the ``examples/`` prefix). - """ - examples: dict[str, tuple[str, str, str]] = {} - for match in _EXAMPLE_LINK_PATTERN.finditer(body): - link_text, path, trailing = match.group(1).strip(), match.group(2).strip(), match.group(3) - if path in examples: - continue - # Authors usually write the path as the link text; a filename reads - # better as a title than a repeated path does. - title = link_text if link_text and link_text != path else Path(path).name - description = trailing.strip().lstrip("-–—:*").strip() - examples[path] = (path, title, description) - return list(examples.values()) - - def parse_example_md(content: str) -> tuple[str, str]: """Extract (title, description) from an example markdown file. @@ -490,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, - )) + 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, body=body) + 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 @@ -510,20 +502,31 @@ def _discover_examples( source_type: str, base_path: Optional[str] = None, base_url: Optional[str] = None, - body: str = "", + referenced: "Optional[list[str]]" = None, ) -> list[SkillExampleResource]: - """Discover a skill's examples. + """Discover a skill's examples, from the directory and from the body. - Local skills scan the ``examples/`` subdirectory, reading only enough of - each file to pull out a title and description. + Two sources, in precedence order: - Remote skills cannot list a directory over HTTP, so their examples are - taken from the ``examples/`` markdown links in the body -- the listing - skill authors already write. Titles and descriptions come from the link - text and the text following it, so discovery costs no extra requests; - content is still fetched on demand. + 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(): @@ -538,20 +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) - elif source_type == "remote" and base_url: - for relative_path, title, description in extract_example_references(body): - # Keep the path below examples/ as the filename so nested - # examples round-trip through load_skill_examples correctly. - filename = relative_path[len("examples/"):] - resources.append(SkillExampleResource( - integration=skill.uuid, - filename=filename, - title=title, - description=description, - content=None, # Loaded on demand (tier 3) - )) + + 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 --- diff --git a/tests/integrations/skills/test_skill_provider.py b/tests/integrations/skills/test_skill_provider.py index d7c13603..ad115094 100644 --- a/tests/integrations/skills/test_skill_provider.py +++ b/tests/integrations/skills/test_skill_provider.py @@ -12,7 +12,6 @@ SkillIntegrationProvider, parse_skill_md, parse_example_md, - extract_example_references, extract_file_references, ) from beaker_notebook.lib.integrations.types import ( @@ -207,7 +206,12 @@ 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" @@ -215,60 +219,12 @@ def test_examples_paths_excluded(self): ) 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 - -# --------------------------------------------------------------------------- -# extract_example_references -# --------------------------------------------------------------------------- - -class TestExtractExampleReferences: - def test_conventional_listing(self): - """The form skill authors actually write: a bulleted link plus a gloss.""" - body = ( - "## Runnable examples\n\n" - "- [examples/basic_fetch.py](examples/basic_fetch.py) — obs + hindcast fetch\n" - "- [examples/s2s.py](examples/s2s.py) — sub-seasonal issuance\n" - ) - assert extract_example_references(body) == [ - ("examples/basic_fetch.py", "basic_fetch.py", "obs + hindcast fetch"), - ("examples/s2s.py", "s2s.py", "sub-seasonal issuance"), - ] - - def test_descriptive_link_text_becomes_the_title(self): - body = "- [End-to-end forecast](examples/e2e.py) — optimize then verify\n" - assert extract_example_references(body) == [ - ("examples/e2e.py", "End-to-end forecast", "optimize then verify"), - ] - - def test_separators_and_bullets_stripped_from_description(self): - for separator in ("-", "–", "—", ":"): - body = f"- [x](examples/a.py) {separator} does a thing\n" - assert extract_example_references(body)[0][2] == "does a thing" - - def test_link_without_description(self): - assert extract_example_references("[x](examples/a.py)\n") == [ - ("examples/a.py", "x", ""), - ] - - def test_deduplicated_in_document_order(self): - body = ( - "- [a](examples/a.py) — first\n" - "- [b](examples/b.py) — second\n" - "See [a](examples/a.py) again.\n" - ) - paths = [path for path, _, _ in extract_example_references(body)] - assert paths == ["examples/a.py", "examples/b.py"] - - def test_non_example_links_ignored(self): - body = "[guide](references/guide.md) and [site](https://example.com/examples/x.py)\n" - assert extract_example_references(body) == [] - - def test_nested_path_retained(self): - """Paths keep the examples/ prefix so they can be fetched back.""" - body = "[deep](examples/advanced/tuning.py) — tuning\n" - assert extract_example_references(body)[0][0] == "examples/advanced/tuning.py" + def test_backtick_example_paths(self): + refs = extract_file_references("Run `examples/quickstart.py` to start.") + assert "examples/quickstart.py" in refs # --------------------------------------------------------------------------- @@ -704,15 +660,18 @@ def test_examples_discovered_from_body(self): 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_descriptions_come_from_the_listing(self): + 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"].description == "the simplest case" - assert by_name["advanced.py"].description == "every knob turned" + 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.""" @@ -763,6 +722,83 @@ def test_skill_without_example_links_has_none(self): 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 From 044058af03243095951a1f338b0777f23671263c Mon Sep 17 00:00:00 2001 From: Brandon Rose Date: Wed, 22 Jul 2026 11:48:22 -0500 Subject: [PATCH 3/3] Don't repeat an example's filename as its title in the listing An example discovered from a reference in the body carries no title beyond its own filename, so the listing rendered "- **basic_fetch.py**: basic_fetch.py" -- which reads as a bug and spends tokens saying nothing. Omit the title when it adds nothing to the filename. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01555XSwMPRxBZFAEsvubUMz --- src/beaker_notebook/lib/integrations/skill.py | 7 ++++++- tests/integrations/skills/test_skill_tools.py | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/beaker_notebook/lib/integrations/skill.py b/src/beaker_notebook/lib/integrations/skill.py index 1bd59e0c..74e31e21 100644 --- a/src/beaker_notebook/lib/integrations/skill.py +++ b/src/beaker_notebook/lib/integrations/skill.py @@ -718,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 += ( diff --git a/tests/integrations/skills/test_skill_tools.py b/tests/integrations/skills/test_skill_tools.py index a57ca320..a8fcbd55 100644 --- a/tests/integrations/skills/test_skill_tools.py +++ b/tests/integrations/skills/test_skill_tools.py @@ -9,6 +9,7 @@ import pytest from beaker_notebook.lib.integrations.skill import SkillIntegrationProvider +from beaker_notebook.lib.integrations.types import SkillExampleResource SKILL_MD = textwrap.dedent("""\ @@ -87,6 +88,26 @@ async def test_load_skill_instructions_returns_body(provider): assert "basic.md" in result +async def test_example_listing_does_not_repeat_the_filename_as_its_title(provider): + """An example found via a body reference has no title but its filename. + + Rendering `- **basic.py**: basic.py` reads as a bug and spends tokens + saying nothing, so the title is omitted when it adds nothing. + """ + skill = provider._find_skill_by_slug("my-skill") + for resource in skill.resources.values(): + if isinstance(resource, SkillExampleResource): + resource.title = resource.filename + resource.description = "" + + result = await SkillIntegrationProvider.load_skill_instructions( + provider, skill_slug="my-skill", agent=_agent_ref(), + ) + listing = result.split("## Available Code Examples")[-1] + assert "- **basic.md**" in listing + assert "basic.md**: basic.md" not in listing + + async def test_load_skill_instructions_dedupes_per_session(provider): agent = _agent_ref("session-A") await SkillIntegrationProvider.load_skill_instructions(provider, skill_slug="my-skill", agent=agent)