From 76c7eb03704f23891ae6b763b3bf11c2c509f8d2 Mon Sep 17 00:00:00 2001 From: John Lybeck Date: Mon, 14 Sep 2026 09:45:54 +0200 Subject: [PATCH 1/2] Record what produced a run, not just what it scored A report says which prompts passed and nothing about what was grading them. `model` in the meta block is the alias the caller asked for, and the target behind `opus` moves; the CLI that discovers and activates a skill updates on its own schedule and is not recorded at all. Two runs a week apart can disagree and the artifacts hold nothing that separates a skill getting worse from a dependency that changed. Three fields, taken where they are already free: the engine as a named constant, the CLI's own version, and the model the API actually served, read out of the preflight call that every graded run already pays for. The preflight reports usage per model rather than naming the one that ran, and bills auxiliary work to a second model in the same block, so the alias that was asked for is what picks the right entry. Signed-off-by: John Lybeck --- skillscope/agent.py | 86 +++++++++++++++++++++++++++++++++++++++++++++ skillscope/cli.py | 26 +++++++++++++- 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/skillscope/agent.py b/skillscope/agent.py index 53bce6d..aad48fc 100644 --- a/skillscope/agent.py +++ b/skillscope/agent.py @@ -39,6 +39,7 @@ import sys import tempfile from dataclasses import dataclass +from functools import lru_cache from pathlib import Path from . import datasets, deadline @@ -94,6 +95,88 @@ def claude_env() -> dict[str, str]: return env +# What `--model opus` actually resolved to, learned from the preflight call. +# Ambient like the active deadline: every engine wants it in its report and +# none of them should pay for a second call to find it out. +_RESOLVED_MODEL: str | None = None + + +@lru_cache(maxsize=1) +def cli_version() -> str | None: + """The `claude` CLI's own version, or None if it cannot be read. + + Part of a run's provenance. The CLI is what discovers and activates a + skill, so a routing score is only comparable to another taken on the same + build. `claude --version` prints `2.1.270 (Claude Code)`; the number is + the part worth recording. + """ + claude_bin = shutil.which("claude") + if not claude_bin: + return None + try: + proc = subprocess.run( + [claude_bin, "--version"], capture_output=True, text=True, + encoding="utf-8", timeout=30, env=claude_env(), + ) + except (OSError, subprocess.SubprocessError): + return None + fields = (proc.stdout or "").split() + return fields[0] if proc.returncode == 0 and fields else None + + +def resolved_model() -> str | None: + """The model the API actually served, or None if nothing has asked it yet. + + `opus` and `sonnet` are aliases whose target moves, so the alias a caller + passed does not identify the thing that produced a score. This is None + when the preflight was skipped, which is honest: nothing has spoken to the + API, so nothing knows. + """ + return _RESOLVED_MODEL + + +def _model_from_result(stdout: str, alias: str | None) -> str | None: + """Which model answered, read out of a `--output-format json` result. + + The result reports usage per model rather than naming the one that ran, + and the CLI bills auxiliary work to a different model in the same block: a + preflight that asked for `sonnet` can come back with a session-title model + listed beside it. The alias is what disambiguates, since `sonnet` resolves + to a name containing `sonnet`. With no alias to match, the entry that did + the most work is the best available answer. + """ + try: + payload = json.loads(stdout) + except (TypeError, ValueError): + return None + usage = payload.get("modelUsage") + if not isinstance(usage, dict): + return None + + # `canonicalModel` is the name the provider settled on; the key is the one + # the CLI asked under. Prefer the former, fall back to the latter. + served: dict[str, int] = {} + for name, detail in usage.items(): + if not isinstance(name, str) or not name: + continue + tokens = 0 + if isinstance(detail, dict): + if isinstance(detail.get("canonicalModel"), str) and detail["canonicalModel"]: + name = detail["canonicalModel"] + if isinstance(detail.get("outputTokens"), int): + tokens = detail["outputTokens"] + served[name] = max(served.get(name, 0), tokens) + + if not served: + return None + if alias: + needle = alias.lower() + for name in served: + if needle in name.lower(): + return name + return max(served, key=lambda name: served[name]) + + def check_api_reachable(model: str | None = DEFAULT_MODEL, timeout: float = 60) -> tuple[bool, str]: """Preflight: confirm the `claude` CLI can actually reach the API. @@ -129,6 +212,9 @@ def check_api_reachable(model: str | None = DEFAULT_MODEL, timeout: float = 60) if proc.returncode != 0: detail = (proc.stderr or proc.stdout or f"exit code {proc.returncode}").strip() return False, detail[:500] + + global _RESOLVED_MODEL + _RESOLVED_MODEL = _model_from_result(proc.stdout or "", model) return True, "ok" diff --git a/skillscope/cli.py b/skillscope/cli.py index f9d93b4..985fe2b 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -72,7 +72,7 @@ from concurrent.futures import ThreadPoolExecutor from pathlib import Path -from . import behavior, config, datasets, deadline, references, routing, structure +from . import agent, behavior, config, datasets, deadline, references, routing, structure from . import selection as select_module from .agent import check_api_reachable, enforce_model_policy @@ -81,6 +81,28 @@ RUNS_DIRNAME = Path(".skillscope") / "runs" +# The only engine today, named rather than implied. A reader should not have to +# date a report to work out what graded it. +ENGINE = "legacy" + + +def _provenance() -> dict: + """What produced a run's numbers, as opposed to what those numbers say. + + Two scores are comparable only if the same engine, the same agent build + and the same model produced both. `model` in the meta block is the alias + the caller asked for, which is not enough on its own: the target moves + underneath it, as does the CLI that does the discovering. A report that + records neither cannot tell a skill that got worse from a dependency that + changed. + """ + return { + "engine": ENGINE, + "agent_cli_version": agent.cli_version(), + "model_resolved": agent.resolved_model(), + } + + def _selected_skills(names: str) -> list[str]: available = datasets.skills_with_datasets() if not names: @@ -397,6 +419,7 @@ def cmd_routing(args: argparse.Namespace) -> int: list(routing_set), { "model": args.model, + **_provenance(), "effort": args.effort, "skills": list(routing_set), "extended": args.extended, @@ -447,6 +470,7 @@ def cmd_behavioral(args: argparse.Namespace) -> int: outcomes, { "model": args.model, + **_provenance(), "effort": args.effort, "skills": skills, "extended": args.extended, From 86d8d16134d8deb82553c16dc3263724459ad59f Mon Sep 17 00:00:00 2001 From: John Lybeck Date: Mon, 14 Sep 2026 10:09:07 +0200 Subject: [PATCH 2/2] Test the provenance a report records about itself The extraction is the part that can be quietly wrong. A preflight result reports usage per model rather than naming the one that ran, and the CLI bills auxiliary work to a second model in the same block, so reading the first entry reports a model the run never asked for. That is not a hypothetical: it is what the first version did against a live API. Eleven cases covering which entry an alias claims, the canonical name winning over the key it was billed under, the fallback when no alias was given, output that names no model at all, an absent CLI, and the three fields arriving in every report. Signed-off-by: John Lybeck --- tests/test_skillscope.py | 114 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 11cdee6..c02be37 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -2501,5 +2501,119 @@ def test_an_empty_room_leaves_only_the_shared_pool(self) -> None: self.assertTrue(all(case.skill is None for case in cases)) +class TestARunRecordsWhatProducedIt(unittest.TestCase): + """A score is only comparable to another if the same things produced both. + + `model` in a report is the alias the caller asked for, and what `opus` + points at moves. The CLI that discovers and activates a skill updates on + its own schedule. Without those written down, two runs that disagree hold + nothing that separates a skill getting worse from a dependency that + changed, so these fields are the report's own provenance and the tests + here are about them being right rather than merely present. + """ + + def setUp(self) -> None: + self.addCleanup(agent.cli_version.cache_clear) + agent.cli_version.cache_clear() + # The resolved model is module state, set by whichever preflight ran + # last. Put back whatever the process already had. + previous = agent._RESOLVED_MODEL + self.addCleanup(setattr, agent, "_RESOLVED_MODEL", previous) + + @staticmethod + def usage(*models: tuple[str, int]) -> str: + return json.dumps( + {"modelUsage": {name: {"outputTokens": out} for name, out in models}} + ) + + def test_the_alias_picks_its_own_entry_out_of_a_shared_usage_block(self) -> None: + # The CLI bills auxiliary work to a model the run never asked for: a + # preflight for `sonnet` comes back with a session-title model listed + # beside it. Taking the first entry reports that one. + blob = self.usage(("gpt-5.5", 9), ("claude-sonnet-5", 4)) + self.assertEqual(agent._model_from_result(blob, "sonnet"), "claude-sonnet-5") + + def test_the_canonical_name_wins_over_the_key_it_was_billed_under(self) -> None: + blob = json.dumps( + { + "modelUsage": { + "opus": {"canonicalModel": "claude-opus-5-20260722", "outputTokens": 4} + } + } + ) + self.assertEqual( + agent._model_from_result(blob, "opus"), "claude-opus-5-20260722" + ) + + def test_with_no_alias_the_entry_that_did_the_work_is_the_answer(self) -> None: + blob = self.usage(("a-title-model", 2), ("the-one-that-answered", 400)) + self.assertEqual( + agent._model_from_result(blob, None), "the-one-that-answered" + ) + + def test_output_that_says_nothing_about_a_model_resolves_to_nothing(self) -> None: + for stdout in ("", "not json at all", "{}", '{"modelUsage": []}', + '{"modelUsage": {}}'): + with self.subTest(stdout=stdout): + self.assertIsNone(agent._model_from_result(stdout, "opus")) + + def test_a_reachable_api_leaves_behind_the_model_it_served(self) -> None: + served = self.usage(("claude-opus-5-20260722", 4)) + with mock.patch.object(agent.shutil, "which", return_value="/usr/bin/claude"), \ + mock.patch.object( + agent.subprocess, + "run", + return_value=subprocess.CompletedProcess([], 0, served, ""), + ): + agent._RESOLVED_MODEL = None + self.assertEqual(agent.check_api_reachable("opus"), (True, "ok")) + self.assertEqual(agent.resolved_model(), "claude-opus-5-20260722") + + def test_a_preflight_nobody_ran_reports_no_model_rather_than_a_guess(self) -> None: + # `--skip-preflight` means nothing has spoken to the API. Reporting the + # alias here would state as fact something no call confirmed. + agent._RESOLVED_MODEL = None + self.assertIsNone(agent.resolved_model()) + + def test_the_cli_version_is_the_number_not_the_product_name(self) -> None: + with mock.patch.object(agent.shutil, "which", return_value="/usr/bin/claude"), \ + mock.patch.object( + agent.subprocess, + "run", + return_value=subprocess.CompletedProcess( + [], 0, "2.1.270 (Claude Code)\n", "" + ), + ): + self.assertEqual(agent.cli_version(), "2.1.270") + + def test_no_cli_on_the_path_is_reported_as_unknown_not_as_a_crash(self) -> None: + # Structural runs never touch an agent, so an absent CLI must not be + # what stops a report being written. + with mock.patch.object(agent.shutil, "which", return_value=None): + self.assertIsNone(agent.cli_version()) + + def test_a_cli_that_fails_to_answer_is_also_unknown(self) -> None: + with mock.patch.object(agent.shutil, "which", return_value="/usr/bin/claude"), \ + mock.patch.object( + agent.subprocess, "run", side_effect=OSError("boom") + ): + self.assertIsNone(agent.cli_version()) + + def test_every_report_carries_the_three_provenance_fields(self) -> None: + with mock.patch.object(agent, "cli_version", return_value="2.1.270"), \ + mock.patch.object(agent, "resolved_model", return_value="claude-opus-5"): + self.assertEqual( + cli._provenance(), + { + "engine": cli.ENGINE, + "agent_cli_version": "2.1.270", + "model_resolved": "claude-opus-5", + }, + ) + + def test_the_engine_is_named_so_a_reader_never_has_to_date_the_report(self) -> None: + self.assertEqual(cli.ENGINE, "legacy") + + if __name__ == "__main__": unittest.main(verbosity=2)