refactor(skills): using-scribe keeps the every-turn practices; moment-specific depth moves to reference files (#4398)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 55s
CI & Build / integration (push) Successful in 1m8s
CI & Build / Python tests (push) Successful in 1m44s
CI & Build / Build & push image (push) Successful in 21s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 55s
CI & Build / integration (push) Successful in 1m8s
CI & Build / Python tests (push) Successful in 1m44s
CI & Build / Build & push image (push) Successful in 21s
Anthropic's skill guidance: keep SKILL.md under 500 lines, split into reference files linked one level deep as it nears that. using-scribe was 478 and every new practice lands there. - SKILL.md 478 -> 317 lines. It keeps orientation, one copy, the reflexes, scope, the judge section, UI and the process-skill index, plus a "Read these when the moment comes" list naming each file with its moment. - projects.md: binding a non-git directory (.scribe) and project inception. - writing-records.md: where a new rule goes, lesson growth, and notes that carry their own check (reflex 10 keeps a pointer). - missed-retrieval.md: the record-before-dial route, verbatim. - Text moved, not rewritten, except for the seams and one cross-reference. Tests: - tests.helpers.skill_text reads SKILL.md plus its reference files. The ownership registry, the miss-route and the verification tests use it, so a topic stays owned by its skill whichever file holds it. - The force test scans every skill .md on its own, since each file is read on its own. - New test_skill_structure: SKILL.md <= 350 lines, every reference file is linked from SKILL.md, none links another, and one over 100 lines opens with Contents. Each guard is shown to fail. The plugin version is minted. That also clears 4fb53b8's red Plugin hooks lane, which failed only because PACKAGING.md changed without a mint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -475,3 +475,19 @@ async def rule_row(rule_id: int):
|
||||
|
||||
async with async_session() as s:
|
||||
return await s.get(Rule, rule_id)
|
||||
|
||||
|
||||
def skill_text(name: str) -> str:
|
||||
"""Everything a bundled skill states: its SKILL.md, then each reference file.
|
||||
|
||||
A skill keeps what matters on most turns in SKILL.md and moves what matters
|
||||
at one moment into sibling files it links (Agent Skills progressive
|
||||
disclosure, #4398). Both are the skill's own statement, so a test asking
|
||||
"does this skill still say X" reads them together rather than pinning X to
|
||||
whichever file it happens to sit in today.
|
||||
"""
|
||||
import pathlib
|
||||
|
||||
folder = pathlib.Path(__file__).resolve().parents[1] / "plugin" / "skills" / name
|
||||
refs = sorted(p for p in folder.glob("*.md") if p.name != "SKILL.md")
|
||||
return "\n\n".join(p.read_text() for p in [folder / "SKILL.md", *refs])
|
||||
|
||||
@@ -48,6 +48,8 @@ import pathlib
|
||||
import re
|
||||
from typing import NamedTuple
|
||||
|
||||
from tests.helpers import skill_text
|
||||
|
||||
ROOT = pathlib.Path(__file__).resolve().parents[1]
|
||||
|
||||
|
||||
@@ -75,7 +77,7 @@ def delivered_surfaces() -> dict[str, str]:
|
||||
The one definition of "delivered" for this module:
|
||||
- `instructions` — the MCP server's `_INSTRUCTIONS` (every MCP client)
|
||||
- `docstrings` — the MCP tool modules (tool descriptions, every client)
|
||||
- `skill:<name>` — each bundled Agent Skill
|
||||
- `skill:<name>` — each bundled Agent Skill, reference files included
|
||||
- `static` — the Claude Code adapter's static session context
|
||||
- `commands` — the Claude Code adapter's slash commands
|
||||
- `live` — the live session context the server builds
|
||||
@@ -90,8 +92,10 @@ def delivered_surfaces() -> dict[str, str]:
|
||||
"commands": "".join(p.read_text() for p in sorted((ROOT / "plugin/commands").glob("*.md"))),
|
||||
"live": _live_session_context_source(),
|
||||
}
|
||||
# A skill is SKILL.md plus the reference files it links (#4398): one
|
||||
# owner, however many files it is split across.
|
||||
for skill in sorted((ROOT / "plugin/skills").glob("*/SKILL.md")):
|
||||
surfaces[f"skill:{skill.parent.name}"] = skill.read_text()
|
||||
surfaces[f"skill:{skill.parent.name}"] = skill_text(skill.parent.name)
|
||||
return {label: _norm(text) for label, text in surfaces.items()}
|
||||
|
||||
|
||||
|
||||
@@ -45,11 +45,12 @@ def _all_surfaces() -> list[tuple[str, str]]:
|
||||
the push channel to the operator installing the plugin, and telling a human
|
||||
what the hook does is not the same act as telling an agent it need not pull.
|
||||
The boundary is "does a session read this", which is skills (loaded by
|
||||
description match), the hook-injected static context, and the MCP server's
|
||||
description match, and each reference file a skill links is read on its
|
||||
own, so it is its own surface — #4398), the hook-injected static context, and the MCP server's
|
||||
own instructions.
|
||||
"""
|
||||
found = [(str(p.relative_to(ROOT)), p.read_text())
|
||||
for p in (ROOT / "plugin" / "skills").rglob("SKILL.md")]
|
||||
for p in (ROOT / "plugin" / "skills").rglob("*.md")]
|
||||
found += [(str(p.relative_to(ROOT)), p.read_text())
|
||||
for p in (ROOT / "plugin" / "hooks").glob("*.md")]
|
||||
server = ROOT / "src" / "scribe" / "mcp" / "server.py"
|
||||
|
||||
@@ -41,7 +41,6 @@ import pathlib
|
||||
import re
|
||||
|
||||
ROOT = pathlib.Path(__file__).resolve().parents[1]
|
||||
SKILL = ROOT / "plugin/skills/using-scribe/SKILL.md"
|
||||
|
||||
# The three tools that shipped with no route to them. Named together because
|
||||
# the gap was all three at once, and a partial fix would leave the loop broken
|
||||
@@ -51,7 +50,10 @@ TUNING_TOOLS = ("retrieval_telemetry", "update_rule", "retrieval_surfaces",
|
||||
|
||||
|
||||
def _skill() -> str:
|
||||
return SKILL.read_text()
|
||||
# The route lives in missed-retrieval.md, a reference file of using-scribe
|
||||
# (#4398); the skill is SKILL.md and its references read together.
|
||||
from tests.helpers import skill_text
|
||||
return skill_text("using-scribe")
|
||||
|
||||
|
||||
def _instructions() -> str:
|
||||
|
||||
@@ -0,0 +1,109 @@
|
||||
"""Every bundled skill keeps the shape Agent Skills can load well (#4398).
|
||||
|
||||
WHY THIS EXISTS
|
||||
|
||||
Anthropic's skill authoring guidance
|
||||
(platform.claude.com/docs/en/agents-and-tools/agent-skills/best-practices):
|
||||
keep the SKILL.md body under 500 lines, split into reference files as it
|
||||
nears that, and link each reference file ONE level deep from SKILL.md —
|
||||
Claude may only preview a file reached through another reference file.
|
||||
|
||||
using-scribe reached 478 lines because every new practice lands there, the
|
||||
same squeeze `_INSTRUCTIONS` was in before #4389. It was split: what matters
|
||||
on most turns stays in SKILL.md, what matters at one moment (placing a rule,
|
||||
a missed retrieval, starting a project) moved to files SKILL.md names with
|
||||
the moment to open them. These tests keep that shape from quietly undoing
|
||||
itself.
|
||||
|
||||
WHAT IT PINS
|
||||
|
||||
1. A BUDGET BELOW THE GUIDELINE. 500 is where loading degrades; the budget
|
||||
sits under it so the next addition is a choice about what moves out, not
|
||||
a squeeze past the line.
|
||||
2. EVERY REFERENCE FILE IS LINKED FROM SKILL.md. A file nothing links is
|
||||
never read — it states guidance that reaches nobody.
|
||||
3. NO REFERENCE FILE LINKS ANOTHER. One level deep, per the guidance.
|
||||
4. A LONG REFERENCE FILE OPENS WITH ITS CONTENTS, so a partial read still
|
||||
shows what the file covers.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import pathlib
|
||||
import re
|
||||
|
||||
SKILLS = pathlib.Path(__file__).resolve().parents[1] / "plugin" / "skills"
|
||||
|
||||
SKILL_LINE_BUDGET = 350
|
||||
TOC_AFTER_LINES = 100
|
||||
_MD_LINK = re.compile(r"\]\(([^)#\s]+\.md)\)")
|
||||
|
||||
|
||||
def _skill_dirs() -> list[pathlib.Path]:
|
||||
return sorted(p.parent for p in SKILLS.glob("*/SKILL.md"))
|
||||
|
||||
|
||||
def _refs(folder: pathlib.Path) -> list[pathlib.Path]:
|
||||
return sorted(p for p in folder.glob("*.md") if p.name != "SKILL.md")
|
||||
|
||||
|
||||
def over_budget(texts: dict[str, str], budget: int) -> list[str]:
|
||||
return [f"{k}: {len(t.splitlines())} lines" for k, t in texts.items()
|
||||
if len(t.splitlines()) > budget]
|
||||
|
||||
|
||||
def unlinked(skill_md: str, ref_names: list[str]) -> list[str]:
|
||||
linked = set(_MD_LINK.findall(skill_md))
|
||||
return [n for n in ref_names if n not in linked]
|
||||
|
||||
|
||||
def nested(refs: dict[str, str]) -> list[str]:
|
||||
return [f"{name} → {target}" for name, text in refs.items()
|
||||
for target in _MD_LINK.findall(text)]
|
||||
|
||||
|
||||
def missing_contents(refs: dict[str, str], after: int) -> list[str]:
|
||||
return [name for name, text in refs.items()
|
||||
if len(text.splitlines()) > after and "## Contents" not in text]
|
||||
|
||||
|
||||
def test_every_skill_md_fits_its_budget():
|
||||
found = over_budget({d.name: (d / "SKILL.md").read_text() for d in _skill_dirs()},
|
||||
SKILL_LINE_BUDGET)
|
||||
assert not found, (
|
||||
f"SKILL.md over {SKILL_LINE_BUDGET} lines: {found}. Move what matters at "
|
||||
f"one moment into a reference file SKILL.md links with that moment "
|
||||
f"(#4398), rather than raising the budget toward 500."
|
||||
)
|
||||
|
||||
|
||||
def test_every_reference_file_is_linked_from_skill_md():
|
||||
found = [f"{d.name}/{n}" for d in _skill_dirs()
|
||||
for n in unlinked((d / "SKILL.md").read_text(), [p.name for p in _refs(d)])]
|
||||
assert not found, (
|
||||
f"reference files no SKILL.md links: {found}. Nothing reads a file "
|
||||
f"nothing names — link it from SKILL.md with the moment to open it."
|
||||
)
|
||||
|
||||
|
||||
def test_reference_files_are_one_level_deep():
|
||||
found = [f"{d.name}/{x}" for d in _skill_dirs()
|
||||
for x in nested({p.name: p.read_text() for p in _refs(d)})]
|
||||
assert not found, (
|
||||
f"reference files linking other files: {found}. Link each from SKILL.md "
|
||||
f"directly; a file reached through another may only be previewed."
|
||||
)
|
||||
|
||||
|
||||
def test_long_reference_files_open_with_contents():
|
||||
found = [f"{d.name}/{n}" for d in _skill_dirs()
|
||||
for n in missing_contents({p.name: p.read_text() for p in _refs(d)},
|
||||
TOC_AFTER_LINES)]
|
||||
assert not found, f"reference files over {TOC_AFTER_LINES} lines with no '## Contents': {found}"
|
||||
|
||||
|
||||
def test_the_guards_can_fail():
|
||||
"""Rule 167: each guard bites on the failure it exists for."""
|
||||
assert over_budget({"s": "x\n" * 400}, SKILL_LINE_BUDGET) == ["s: 400 lines"]
|
||||
assert unlinked("see [a](a.md)", ["a.md", "b.md"]) == ["b.md"]
|
||||
assert nested({"a.md": "then [b](b.md)"}) == ["a.md → b.md"]
|
||||
assert missing_contents({"a.md": "x\n" * 150}, TOC_AFTER_LINES) == ["a.md"]
|
||||
@@ -91,12 +91,11 @@ def test_the_skill_carries_the_test_a_writer_can_actually_apply():
|
||||
read while deciding what to write. The one-question form has to be in the
|
||||
second place too, or the guidance only reaches callers who already opened
|
||||
the tool."""
|
||||
import pathlib
|
||||
from tests.helpers import skill_text
|
||||
|
||||
skill = pathlib.Path(__file__).resolve().parents[1] / (
|
||||
"plugin/skills/using-scribe/SKILL.md"
|
||||
)
|
||||
text = " ".join(skill.read_text().split())
|
||||
# Reference files included: the full statement sits in writing-records.md
|
||||
# and SKILL.md keeps the pointer (#4398).
|
||||
text = " ".join(skill_text("using-scribe").split())
|
||||
assert "could this note become false without anyone editing it" in text.lower(), (
|
||||
"the using-scribe skill no longer carries the one-question test. That "
|
||||
"question is what makes the distinction applicable rather than merely "
|
||||
|
||||
Reference in New Issue
Block a user