diff --git a/README.md b/README.md index 184ab04..ce413da 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ You may also choose to customize or even disable some tests if you prefer it. Se | Command | Does | | --- | --- | -| `skillscope structural` | Skill folders, datasets, and markdown references. | +| `skillscope structural` | Skill folders, datasets, markdown references, and what the skills cost in an agent's startup listing. | | `skillscope routing` | Which skill fires, with several installed together. | | `skillscope behavioral` | What a skill does once it has fired. | | `skillscope select` | The CI plan for a change, as JSON. | diff --git a/docs/usage.md b/docs/usage.md index 68164ef..c23f067 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -116,6 +116,14 @@ agent that simply never uses the skill. So every `SKILL.md` is read first: | `description` | non-empty, at most 1024 characters | | body | at most 500 lines — past that it is reference material, and an agent reads it in full every time the skill loads | +One number is reported rather than asserted: what these skills add to the +listing an agent is given at the start of a session — a line per skill, its +name and its description — and what share of the default budget for that +listing they take. The budget belongs to the whole installed set rather than to +any one skill, so no per-skill bar can see it, and what a published catalog +shares it with is not visible from here. There is nothing to fail, so the run +prints the number and moves on. + A directory that holds no `SKILL.md` is simply not a skill, and is passed over without a word. Matching *no* skill at all is the case that is reported, since a run that graded nothing and called itself green is the one way this harness diff --git a/skillscope/cli.py b/skillscope/cli.py index f9d93b4..98ef6dd 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -72,7 +72,16 @@ from concurrent.futures import ThreadPoolExecutor from pathlib import Path -from . import behavior, config, datasets, deadline, references, routing, structure +from . import ( + behavior, + config, + datasets, + deadline, + listing, + references, + routing, + structure, +) from . import selection as select_module from .agent import check_api_reachable, enforce_model_policy @@ -195,6 +204,15 @@ def cmd_structural(args: argparse.Namespace) -> int: f"[evals] OK: {len(cases)} case(s) across {len(skills)} skill(s) " f"plus {len(datasets.load_shared_negatives())} shared negative(s)." ) + # Reported rather than graded. Every skill can be within the format's + # limits and the listing still overflow, because the budget belongs to the + # whole installed set rather than to any one skill, and what a published + # catalog shares that budget with is not visible from here. A run over a + # repo's prose alone has no listing to cost, so it says nothing. + cost = listing.cost() + if cost.skills: + print(f"[evals] listing: {listing.summary(cost)}") + local = sum(1 for reference in found if reference.is_local) external = references.external_urls(found) print( diff --git a/skillscope/listing.py b/skillscope/listing.py new file mode 100644 index 0000000..99ca9be --- /dev/null +++ b/skillscope/listing.py @@ -0,0 +1,154 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""What a repo's skills cost in the listing every agent reads at startup. + +Every enabled skill contributes one line to a listing the agent is given at +the start of a session: its name, and its description. That listing has a +budget, and the budget is a fraction of the context window rather than a fixed +number, because the listing is re-sent on every turn and so is paid for again +and again. + +When the listing does not fit, nothing fails. Names are kept and descriptions +are dropped, ordered by how recently and how often each skill was used, until +the rest fits. A skill with no description is still callable by name and can no +longer be matched against a prompt, because there is no longer any text to +match. Nobody is told, and a skill that was just installed has no usage history +at all, so it is first to lose its description and then never gets used. + +:mod:`structure` reads one skill at a time, which is the right shape for a +limit the format sets on each skill. This limit belongs to the whole set, so no +per-skill check can see it: every skill can be within the format's limits and +the listing still overflow. + +For a repo whose skills are installed elsewhere, that makes this a measure of +something the repo itself cannot observe. A published catalog is a guest in a +budget it does not own. It does not get the whole listing, it gets whatever is +left after the skills its installers already had, so the number worth watching +is the share it consumes rather than whether it fits on its own. + +The arithmetic mirrors the agent's, so the totals mean the same thing: + + budget = context window in tokens * bytes per token * fraction + entry cost = len(name) + 4 + min(len(description), per-description cap) + listing = sum(entry costs) + one separator between entries + +The defaults below are the shipped ones. Both the fraction and the +per-description cap are settable by whoever installs the skills, so an +overflowing listing has two honest answers: publish less, or ask installers to +spend more of every turn on it. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from . import config, structure + +# The shipped defaults. A reader is assumed to be on none of the settings that +# change them, because almost everyone is, and a report against a tuned budget +# would flatter a catalog that overflows for its actual audience. +BUDGET_FRACTION = 0.01 +BYTES_PER_TOKEN = 4 +DEFAULT_CONTEXT_TOKENS = 200_000 + +# A single description is truncated at this many characters in the listing, +# separately from the shared budget, so one very long one cannot take the room +# every other skill needs. Defensive, for a caller that costs a repo without +# running the structural gate first: that gate rejects anything past +# `structure.MAX_DESCRIPTION_LENGTH`, which is lower, so a description reaching +# here through the CLI is never clamped. +MAX_DESCRIPTION_IN_LISTING = 1536 + +# `- ` before the name and `: ` after it, in each line of the listing. +_ENTRY_OVERHEAD = 4 + + +def budget(context_tokens: int = DEFAULT_CONTEXT_TOKENS) -> int: + """Characters available to the whole listing at a given context size.""" + return max(1, int(context_tokens * BYTES_PER_TOKEN * BUDGET_FRACTION)) + + +@dataclass(frozen=True) +class Cost: + """What this repo's skills add to a listing, and what that is a share of.""" + + skills: int + characters: int + budget: int + unreadable: tuple[str, ...] + + @property + def share(self) -> float: + """The fraction of the default budget this repo alone consumes.""" + return self.characters / self.budget if self.budget else 0.0 + + +def cost(skills: list[str] | None = None) -> Cost: + """The listing cost of every skill in the repo. + + A skill whose frontmatter cannot be read is counted in ``unreadable`` + rather than as zero. :func:`structure.errors` is what reports it as a + fault; leaving it out of the total silently would make a broken repo look + cheaper than a working one. The CLI runs that gate first, so ``unreadable`` + is empty there, and populated only for a caller that skips it. + """ + cfg = config.active() + wanted = skills if skills is not None else sorted(cfg.skills) + + characters = 0 + counted = 0 + unreadable: list[str] = [] + for skill in wanted: + try: + text = (cfg.skill_path(skill) / structure.SKILL_FILE).read_text( + encoding="utf-8" + ) + except (OSError, UnicodeDecodeError): + unreadable.append(skill) + continue + + declared, _, _ = structure._frontmatter(text) # noqa: SLF001 + if declared is None: + unreadable.append(skill) + continue + + name = declared.get("name") + description = declared.get("description") + if not isinstance(name, str) or not isinstance(description, str): + unreadable.append(skill) + continue + + characters += ( + len(name) + + _ENTRY_OVERHEAD + + min(len(description), MAX_DESCRIPTION_IN_LISTING) + ) + counted += 1 + + # One separator between entries, so N entries carry N-1 of them. + characters += max(0, counted - 1) + return Cost( + skills=counted, + characters=characters, + budget=budget(), + unreadable=tuple(unreadable), + ) + + +def summary(measured: Cost) -> str: + """One line for a run that reports rather than gates. + + No threshold is applied. Where the line sits between "a catalog people can + install alongside their own skills" and "a catalog that takes the room" is + a judgement this harness has no standing to make for another repo, and a + number invented here would be argued with rather than watched. + """ + return ( + f"{measured.skills} skill(s) cost {measured.characters} character(s) " + f"in the startup listing, {measured.share:.0%} of the " + f"{measured.budget}-character budget at a " + f"{DEFAULT_CONTEXT_TOKENS:,}-token context window, " + "assuming the shipped defaults." + ) diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 11cdee6..ff1250c 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -42,6 +42,7 @@ credentials, datasets, deadline, + listing, references, routing, structure, @@ -1728,6 +1729,168 @@ def test_a_run_asked_for_docs_still_has_work_to_do(self) -> None: self.assertEqual(structure.errors(), []) +class TestListingCost(unittest.TestCase): + """What a repo's skills add to the listing an agent reads at startup. + + Every expected number here is worked out by hand from the entries the + fixture writes, rather than from the module's own constants. A total + derived the way `listing` derives it would agree just as readily with a + separator counted once too often, or an entry overhead of three, as with + the arithmetic the agent actually does -- and a misreported share is not + the kind of thing anyone notices by reading the line. + """ + + def setUp(self) -> None: + self.repo = Repo(self) + + def write(self, text: str, skill: str = "alpha") -> None: + (self.repo.root / skill / "SKILL.md").write_text(text, encoding="utf-8") + + def test_one_skill_costs_its_name_its_description_and_the_line_around_it(self) -> None: + # `alpha` is 5, `Aa` is 2, and `- ` before the name and `: ` after it + # are 4 more. One entry carries no separator, so 11 and not 12. + self.repo.skill("alpha", description="Aa") + self.repo.activate() + measured = listing.cost() + self.assertEqual(measured.skills, 1) + self.assertEqual(measured.characters, 11) + self.assertEqual(measured.unreadable, ()) + + def test_a_separator_sits_between_entries_and_not_after_the_last(self) -> None: + # 5 + 4 + 2 = 11, 4 + 4 + 6 = 14, 9 + 4 + 3 = 16, and two separators + # holding the three of them apart: 43. + self.repo.skill("alpha", description="Aa") + self.repo.skill("beta", description="Bbbbbb") + self.repo.skill("gamma-two", description="Ccc") + self.repo.activate() + measured = listing.cost() + self.assertEqual(measured.skills, 3) + self.assertEqual(measured.characters, 43) + + def test_a_repo_with_no_skills_costs_nothing(self) -> None: + self.repo.activate(docs="*.md") + measured = listing.cost() + self.assertEqual(measured.skills, 0) + self.assertEqual(measured.characters, 0) + + def test_a_skill_md_that_does_not_decode_is_named_rather_than_counted(self) -> None: + self.repo.skill("alpha", description="Aa") + self.repo.skill("broken") + (self.repo.root / "broken" / "SKILL.md").write_bytes(b"---\nname: \xff\xfe\n---\n") + self.repo.activate() + measured = listing.cost() + self.assertEqual(measured.unreadable, ("broken",)) + # Neither an entry nor a separator: what is left is alpha's 11 alone. + self.assertEqual(measured.skills, 1) + self.assertEqual(measured.characters, 11) + + def test_frontmatter_an_agent_cannot_load_is_named_rather_than_counted(self) -> None: + self.repo.skill("alpha", description="Aa") + self.repo.skill("broken") + self.write("# No frontmatter at all.\n", skill="broken") + self.repo.activate() + measured = listing.cost() + self.assertEqual(measured.unreadable, ("broken",)) + self.assertEqual(measured.characters, 11) + + def test_a_skill_with_no_description_to_cost_is_named_rather_than_counted(self) -> None: + # Counting it as name-plus-overhead would price a skill an agent can + # match nothing against as if it were carrying its share. + self.repo.skill("broken") + self.write("---\nname: broken\n---\n", skill="broken") + self.repo.activate() + measured = listing.cost() + self.assertEqual(measured.unreadable, ("broken",)) + self.assertEqual(measured.skills, 0) + self.assertEqual(measured.characters, 0) + + def test_a_description_is_counted_up_to_the_listing_cap_and_no_further(self) -> None: + # Defensive: the structural gate rejects a description past + # structure.MAX_DESCRIPTION_LENGTH, which is lower than this cap, so + # only a caller running without that gate ever reaches the clamp. Both + # halves of that claim are pinned here -- the cap's own value, and its + # sitting above the gate's -- since a cap read back off the module + # would agree with whatever the module happened to say, and the comment + # in `listing` explaining why the clamp is unreachable stops being true + # the moment the two cross. + self.assertEqual(listing.MAX_DESCRIPTION_IN_LISTING, 1536) + self.assertGreater( + listing.MAX_DESCRIPTION_IN_LISTING, structure.MAX_DESCRIPTION_LENGTH + ) + self.repo.skill("alpha") + self.repo.activate() + for length, expected in ( + (1535, 5 + 4 + 1535), + (1536, 5 + 4 + 1536), + (1586, 5 + 4 + 1536), + ): + with self.subTest(description=length): + self.write(f"---\nname: alpha\ndescription: {'d' * length}\n---\n") + self.assertEqual(listing.cost().characters, expected) + + +class TestListingBudget(unittest.TestCase): + """The budget the cost is a share of, and what a share means at the edges.""" + + def test_the_budget_is_a_fraction_of_the_window_in_characters(self) -> None: + # 200,000 tokens, 4 bytes each, 1% of them: 8,000 characters. + self.assertEqual(listing.budget(), 8000) + self.assertEqual(listing.budget(50_000), 2000) + + def test_a_window_too_small_to_hold_the_fraction_still_has_one_character(self) -> None: + # A budget of zero would be a division rather than a report. + self.assertEqual(listing.budget(1), 1) + + def test_share_is_the_fraction_of_the_budget_this_repo_alone_takes(self) -> None: + measured = listing.Cost(skills=3, characters=800, budget=8000, unreadable=()) + self.assertAlmostEqual(measured.share, 0.1) + + def test_a_cost_with_no_budget_to_share_reports_no_share(self) -> None: + measured = listing.Cost(skills=3, characters=800, budget=0, unreadable=()) + self.assertEqual(measured.share, 0.0) + + +class TestListingReport(unittest.TestCase): + """The one line `structural` prints about the listing, and when it does not.""" + + def structural(self, *argv) -> str: + """Everything `skillscope structural` printed over a clean repo.""" + args = cli.build_parser().parse_args(["structural", *argv]) + stdout = io.StringIO() + with contextlib.redirect_stdout(stdout): + self.assertEqual(cli.cmd_structural(args), 0) + return stdout.getvalue() + + def test_the_line_reports_the_count_the_cost_and_the_share(self) -> None: + measured = listing.Cost(skills=3, characters=800, budget=8000, unreadable=()) + self.assertEqual( + listing.summary(measured), + "3 skill(s) cost 800 character(s) in the startup listing, 10% of " + "the 8000-character budget at a 200,000-token context window, " + "assuming the shipped defaults.", + ) + + def test_the_share_is_rounded_to_a_whole_percent(self) -> None: + measured = listing.Cost(skills=1, characters=1234, budget=8000, unreadable=()) + self.assertIn("15% of the 8000-character budget", listing.summary(measured)) + + def test_a_run_reports_what_the_repos_skills_cost(self) -> None: + repo = Repo(self) + repo.skill("alpha", dataset=tier0_dataset("alpha"), description="Aa") + repo.activate() + self.assertIn( + "[evals] listing: 1 skill(s) cost 11 character(s)", self.structural() + ) + + def test_a_run_over_a_repos_prose_alone_has_no_listing_to_cost(self) -> None: + # --docs grades a repo that ships no skill, and a listing line there + # would be reporting on nothing. + repo = Repo(self) + (repo.root / "README.md").write_text("# Notes\n", encoding="utf-8") + repo.activate(docs="*.md") + self.assertNotIn("listing:", self.structural()) + + def targets(text: str) -> list[str]: """Every reference the extractor finds in one markdown document.""" with tempfile.TemporaryDirectory() as tmp: