diff --git a/dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md b/dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md new file mode 100644 index 00000000..53e0e44c --- /dev/null +++ b/dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md @@ -0,0 +1,9 @@ +### Changed + +- **`novelty routing --catalog ` reads the catalog through its `skills.yaml`** — when the + catalog root has a `skills.yaml`, that manifest decides which skills are competitors, as it + does for `catalog scan`, so a `SKILL.md` it does not list is no longer a sibling. Without a + manifest, every `SKILL.md` under the directory counts as before. A declared skill that cannot + be loaded is skipped with a warning on stderr instead of making the whole field unreadable. + Two catalog skills with the same name and different content are refused, naming both paths, + rather than the run failing on a duplicate competitor name. diff --git a/dokimi/changelog.d/20261021_131000_napetrov_routing-skip-non-skill.md b/dokimi/changelog.d/20261021_131000_napetrov_routing-skip-non-skill.md new file mode 100644 index 00000000..f96d23a2 --- /dev/null +++ b/dokimi/changelog.d/20261021_131000_napetrov_routing-skip-non-skill.md @@ -0,0 +1,7 @@ +### Changed + +- **`novelty routing` skips a non-`SKILL.md` competitor with a warning** — a + `skill:` competitor that names a file other than `SKILL.md`, or a directory without one, + is left out of the field and reported on stderr. It used to abort the whole run, so a + shell glob over a skills directory (which also matches helper scripts) could not be + passed. The artifact under test is still refused. diff --git a/dokimi/changelog.d/20261021_132000_napetrov_routing-model-hidden-skills.md b/dokimi/changelog.d/20261021_132000_napetrov_routing-model-hidden-skills.md new file mode 100644 index 00000000..c7dec363 --- /dev/null +++ b/dokimi/changelog.d/20261021_132000_napetrov_routing-model-hidden-skills.md @@ -0,0 +1,8 @@ +### Changed + +- **`novelty routing` leaves `disable-model-invocation` skills out of the field** — no + harness lists such a skill, so the model cannot select it, and a card for it scored a + choice that does not exist (`ultracave` and `megacave` in the caveman repo). They are + left out of a `--catalog` and of `--competitors`, each with a warning on stderr, unless + `--include-disabled` is passed. A disabled artifact is still routed, with a warning that + its rate is not a harness rate. diff --git a/dokimi/changelog.d/20261021_133000_napetrov_routing-mirror-dedupe.md b/dokimi/changelog.d/20261021_133000_napetrov_routing-mirror-dedupe.md new file mode 100644 index 00000000..7956763b --- /dev/null +++ b/dokimi/changelog.d/20261021_133000_napetrov_routing-mirror-dedupe.md @@ -0,0 +1,6 @@ +### Fixed + +- **`novelty routing --catalog` counts a mirrored skill once** — a `SKILL.md` that is + byte-identical under two paths (a plugin copy of the `skills/` tree) used to put the same + name in the field twice, and the run was refused as naming a competitor more than once. + The copy nearest the catalog root stands for the card; the others are named on stderr. diff --git a/dokimi/changelog.d/20261021_134000_napetrov_routing-all-vs-all.md b/dokimi/changelog.d/20261021_134000_napetrov_routing-all-vs-all.md new file mode 100644 index 00000000..935bfe87 --- /dev/null +++ b/dokimi/changelog.d/20261021_134000_napetrov_routing-all-vs-all.md @@ -0,0 +1,11 @@ +### Added + +- **`novelty routing --all-vs-all`** routes every skill that has queries against the whole + field, in turn, and reports a confusion matrix plus an attractor table: the skills picked + for other skills' queries, with how many of those queries they took and from whom. Takes + `--queries-by-skill ` and `--out`, and no `--artifact`. The lexical arm always runs; + `--model` adds the model arm. +- `--all-vs-all` rates are over the scored queries (`n_scored`, `n_unscored`; `null` own rate + when none was scored); a lexical tie among other cards is `(tie)`; a failed model call + exits non-zero; a repeated skill key in the queries file is refused; routing results record + the field rule under `field`. diff --git a/dokimi/docs/skill-novelty-walkthrough.md b/dokimi/docs/skill-novelty-walkthrough.md index 0e94c594..c3d9b87e 100644 --- a/dokimi/docs/skill-novelty-walkthrough.md +++ b/dokimi/docs/skill-novelty-walkthrough.md @@ -41,10 +41,9 @@ python cli.py novelty routing --artifact skill:skills/my-skill \ ``` The competitors are the skills a harness lists next to this one: `--catalog ` takes -every other `SKILL.md` under the directory (a `template/` copy and a byte-identical mirror -that loads under the same skill name are left out), and `--competitors` adds or names them one by -one. With no real siblings at hand, skip this step and say so; a field of competitors made -up for the run measures nothing. +every other skill the catalog lists (a `template/` copy is left out), and `--competitors` adds or +names them one by one. With no real siblings at hand, skip this step and say so; a field of +competitors made up for the run measures nothing. Read `recall@1` against the competitors, not alone. Measured on the study corpus, every `dpnp-*` skill scored 0.0 against its siblings, and `linux-perf` scored 0.0 recall@1 on diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index f0ef48df..48164676 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -591,9 +591,19 @@ python cli.py novelty routing --artifact skill:path/to/skill \ --queries queries.json [--model M] ``` -`--catalog ` takes the competitors from a skill directory instead: every `SKILL.md` -under it except the artifact itself (a `template/` copy and a byte-identical mirror that -loads under the same skill name are left out). +`--catalog ` takes the competitors from a skill directory instead: every skill the catalog +lists except the artifact itself. A `skills.yaml` at the root decides what the catalog lists; +without one, every `SKILL.md` under it counts, except a `template/` copy. A declared skill +that cannot be loaded is skipped with a warning. Two catalog skills with the same name and different content are refused, with +both paths named: rename one, or delete the copy you do not mean to route against. +A `skill:` competitor that is not a `SKILL.md`, or a directory without one, is skipped with a +warning, so a shell glob over a skills directory can be passed as it is. +A skill that sets `disable-model-invocation` is left out of the field, both from a catalog +and as `--competitors`: no harness lists it, so the model cannot select it. Pass +`--include-disabled` to keep it. A disabled artifact is still routed, with a warning; the flag does not change that. +A catalog that holds the same `SKILL.md` under two paths (a plugin copy of its `skills/` +tree) counts it once: the copy nearest the catalog root stands for the card, and the +others are named on stderr. Two numbers, reported side by side and never merged: @@ -614,6 +624,34 @@ and the figure would be a structural 1.0. The lexical arm also reports `n_querie next to `n_queries_scored`, since a query with no content words is dropped rather than counted as a miss. +**All skills against each other.** `novelty routing --all-vs-all --catalog +--queries-by-skill queries.json --out routing.json` (add `--model M` for the model arm) makes +every skill that has queries the gold in turn, routed against the whole field. The file is a +JSON object of skill name to the requests that should select it, each written without the +skill's own text. A skill named twice in the file is refused. The output has a confusion +matrix (gold to picked to count, with `(none)` for a query no card shares a word with or, in +the model arm, an abstention, and `(tie)` for a query on which two or more other cards tie for +the top lexical score, so no field order decides it), an own rate per skill, and an attractor +table: a skill picked for other skills' queries, with `wrongly_picked` of `sibling_queries` and +the skills whose queries it took (`from`). Every rate is over the queries that were scored: +`n_scored` and `n_unscored` per skill say how many, a query with no content word or a failed +model call is unscored, and a skill with none scored has an own rate of `null`, not 0.0. A run +with any failed model call writes the result and exits non-zero. The saved file records the +field rule under `field` (`include_disabled`, the `competitors` and what was `left_out`); a +single-artifact run records it too. It takes no `--artifact`, `--queries` or +`--queries-from-headings`. The model arm also records the selector `model`, so two runs with +different models can be told apart. It is refused with fewer than two skills, a name repeated +in the field, a query file that names a skill outside the field, or no queries at all. + +**Cost of `--model` in all-vs-all.** The model arm makes one selector call per query, across +every skill in the field: the call count is the total number of requests in `queries.json`, and +a failed call still counts. Each call sends the whole field's cards, so a large field costs +more per call as well as in calls. Before the first call the run prints that count to stderr +(`all-vs-all: --model makes N model calls, one per query, ...`). The run does not pause for it, +so count the queries first, or pass `--max-usd` to stop the run at a spend limit. A lexical-only run (no `--model`) makes no model calls. +A stored response in `DOKIMI_RESPONSE_CACHE` is replayed without a call, so a rerun can make +fewer calls than the printed count. + Three properties worth knowing before quoting either: - **Competitors are mandatory.** The existing `skill-agent:` arm offers one skill, which diff --git a/dokimi/dokimi/commands/novelty.py b/dokimi/dokimi/commands/novelty.py index 6afb318f..99b07d17 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -262,39 +262,188 @@ def cmd_novelty_items(args: argparse.Namespace) -> None: f"{agreement['n_atoms_multi_kind']} atoms asked by several kinds") +def _routing_card(ref: str): + """An artifact or competitor reference as the selector sees it: ``(candidate, body)``.""" + from dokimi.novelty.routing import load_candidate + + candidate, body, warning = load_candidate(ref) + if warning: + print(f"WARNING: {warning}", file=sys.stderr) + return candidate, body + + +def _routing_field(args: argparse.Namespace, exclude: set[str], + left_out: list[str] | None = None) -> list: + """The field ``--competitors`` and ``--catalog`` name, without the names in ``exclude``. + + What was left out of the field, and why, is appended to ``left_out`` when given. + + Raises: + FileNotFoundError, ValueError: A reference or the catalog cannot be loaded. + """ + from dokimi.novelty.routing import ( + catalog_competitors, + disables_model_invocation, + non_skill_reason, + skill_file, + ) + + competitors = [] + for ref in args.competitors or []: + reason = non_skill_reason(ref) + if reason: + print(f"WARNING: skipped competitor {ref}: {reason}", file=sys.stderr) + if left_out is not None: + left_out.append(f"{ref}: {reason}") + continue + candidate = _routing_card(ref)[0] + ref_file = skill_file(ref) + if (ref_file and not args.include_disabled + and disables_model_invocation(ref_file)): + print(f"WARNING: skipped competitor {ref}: disable-model-invocation " + "(--include-disabled keeps it)", file=sys.stderr) + if left_out is not None: + left_out.append(f"{ref}: disable-model-invocation") + continue + competitors.append(candidate) + if args.catalog: + notes: list[str] = [] + catalog = catalog_competitors(args.catalog, + exclude | {c.name for c in competitors}, notes, + include_disabled=args.include_disabled) + for note in notes: + print(f"WARNING: {note}", file=sys.stderr) + if left_out is not None: + left_out.extend(notes) + no_description = [c.name for c in catalog if not c.description] + if no_description: + print(f"WARNING: {len(no_description)} catalog skill(s) have no description, " + f"so their card is a name only: {', '.join(sorted(no_description))}", + file=sys.stderr) + print(f"--catalog {args.catalog}: {len(catalog)} competitors", file=sys.stderr) + competitors += catalog + return competitors + + +def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None: + """Every skill in the field is the gold in turn: confusion matrix and attractors.""" + from dokimi.novelty.routing import RoutingInputError, all_vs_all, load_queries_by_skill + + _check_all_vs_all_args(args) + left_out: list[str] = [] + try: + field = _routing_field(args, set(), left_out) + except RoutingInputError as exc: + raise SystemExit(str(exc)) from None + except (FileNotFoundError, ValueError) as exc: + raise SystemExit(f"cannot load the field: {exc}") from None + try: + try: + queries_by_skill = load_queries_by_skill(args.queries_by_skill) + except (OSError, UnicodeDecodeError) as exc: + raise SystemExit(f"cannot read the queries: {exc}") from None + selector = _client(args.model, args.provider, "selector", args) if args.model else None + if selector is not None: + # One selector call per query, each sending every card in the field. + calls = sum(len(queries) for queries in queries_by_skill.values()) + print( + f"all-vs-all: --model makes {calls} model calls, one per query, over " + f"{len(field)} cards each", + file=sys.stderr, + ) + result = all_vs_all(field, queries_by_skill, client=selector, seed=args.seed) + except RoutingInputError as exc: + raise SystemExit(str(exc)) from None + without = [c.name for c in field if c.name not in queries_by_skill] + if without: + print(f"WARNING: {len(without)} skill(s) in the field have no queries, so they only " + f"compete: {', '.join(sorted(without))}", file=sys.stderr) + + model_arm = result.get("model") or {} + failed = model_arm.get("n_failed", 0) + if failed: + print(f"WARNING: {failed} model call(s) failed (first error: " + f"{model_arm['first_error']}); those queries are left out of " + "every rate in the model arm", file=sys.stderr) + + out = Path(args.out) + out.parent.mkdir(parents=True, exist_ok=True) + payload = {"mode": "all_vs_all", "queries_by_skill": args.queries_by_skill, + "n_candidates": len(field), "result": result} + payload["field"] = _field_record(args, field, left_out) + out.write_text(json.dumps(payload, indent=2), encoding="utf-8") + if failed: + raise SystemExit(f"{failed} of {result['n_queries']} model call(s) failed " + f"({model_arm['first_error']}); the partial result, with those queries " + f"left out of every rate, is in {out}") + print(f"OK Saved all-vs-all routing: {out}") + print(f"all-vs-all: {len(field)} skills in the field, {result['n_queries']} queries over " + f"{len(queries_by_skill)} skills with queries") + _print_attractors(result) + + +def _field_record(args: argparse.Namespace, competitors: list, left_out: list[str]) -> dict: + """The field rule a routing result was measured under, saved next to the result.""" + return {"include_disabled": bool(args.include_disabled), + "competitors": [c.name for c in competitors], "left_out": left_out} + + +def _check_all_vs_all_args(args: argparse.Namespace) -> None: + """Refuse the flag combinations ``--all-vs-all`` cannot run with.""" + if args.artifact: + raise SystemExit("--all-vs-all takes no --artifact: every skill in the field is the " + "gold in turn") + if args.queries or args.queries_from_headings: + raise SystemExit("--all-vs-all takes --queries-by-skill, not --queries or " + "--queries-from-headings") + if not args.queries_by_skill: + raise SystemExit("--all-vs-all needs --queries-by-skill : a JSON object of skill " + "name to the requests that should select it") + if not args.out: + raise SystemExit("--all-vs-all needs --out ") + + +def _print_attractors(result: dict) -> None: + """The top five attractors of each arm, one line each.""" + for arm in ("lexical", "model"): + if arm not in result: + continue + rows = result[arm]["attractors"][:5] + if not rows: + print(f"{arm}: no skill was picked for another skill's query") + for row in rows: + print(f"{arm}: {row['skill']} picked for {row['wrongly_picked']} of " + f"{row['sibling_queries']} sibling queries (from {', '.join(row['from'])})") + + def cmd_novelty_routing(args: argparse.Namespace) -> None: """Can this artifact be selected against its competitors? Lexical floor, model on top.""" from dokimi.novelty.routing import ( - Candidate, RoutingInputError, - catalog_competitors, check_field, + disables_model_invocation, headings_as_queries, lexical_routing, - load_candidate, load_queries, model_routing, + skill_file, ) - def card(ref: str) -> tuple[Candidate, str]: - candidate, body, warning = load_candidate(ref) - if warning: - print(f"WARNING: {warning}", file=sys.stderr) - return candidate, body - + if args.all_vs_all: + return _cmd_routing_all_vs_all(args) + if not args.artifact: + raise SystemExit("pass --artifact , or --all-vs-all for the whole field") try: - gold, body = card(args.artifact) - competitors = [card(ref)[0] for ref in args.competitors or []] - if args.catalog: - catalog = catalog_competitors(args.catalog, - {gold.name, *(c.name for c in competitors)}) - no_description = [c.name for c in catalog if not c.description] - if no_description: - print(f"WARNING: {len(no_description)} catalog skill(s) have no description, " - f"so their card is a name only: {', '.join(sorted(no_description))}", - file=sys.stderr) - print(f"--catalog {args.catalog}: {len(catalog)} competitors", file=sys.stderr) - competitors += catalog + gold, body = _routing_card(args.artifact) + artifact_file = skill_file(args.artifact) + if artifact_file and disables_model_invocation(artifact_file): + print(f"WARNING: {args.artifact} sets disable-model-invocation, so no harness lists " + "it and the model cannot select it; its routing rate is not a harness rate", + file=sys.stderr) + left_out: list[str] = [] + competitors = _routing_field(args, {gold.name}, left_out) + except RoutingInputError as exc: + raise SystemExit(str(exc)) from None except (FileNotFoundError, ValueError) as exc: raise SystemExit(f"cannot load an artifact: {exc}") from None # Checked before the query file is read, so the message is about the real problem. @@ -333,6 +482,7 @@ def card(ref: str) -> tuple[Candidate, str]: "status": "ok", "n_candidates": len(candidates), "queries_self_derived": self_derived, + "field": _field_record(args, competitors, left_out), "lexical": lexical, } if args.model: @@ -612,18 +762,33 @@ def register(sub, positive_int) -> None: routing_p = novelty_sub.add_parser( "routing", help="Can the artifact be selected against competitors? (name+description)" ) - routing_p.add_argument("--artifact", required=True, - help="skill: or text: under test") + routing_p.add_argument("--artifact", default=None, + help="skill: or text: under test (required unless " + "--all-vs-all)") + routing_p.add_argument("--all-vs-all", action="store_true", dest="all_vs_all", + help="Route every skill that has queries against the whole field, " + "in turn: a confusion matrix and the attractors, the skills " + "picked for queries written for another skill. Takes " + "--queries-by-skill and --out, and no --artifact") + routing_p.add_argument("--queries-by-skill", default=None, dest="queries_by_skill", + help="With --all-vs-all: a JSON object of skill name to the list of " + "requests that should select that skill") routing_p.add_argument("--competitors", nargs="+", default=None, help="Other artifacts the selector sees. Selection among one " "candidate is not routing: the sibling confusion is the " "failure mode (every `dpnp-*` skill scored 0.0 lexically)") routing_p.add_argument("--catalog", default=None, help="Directory of skills to take the competitors from: every " - "SKILL.md under it except the artifact itself (a template/ " - "copy and a byte-identical mirror that loads under the same " - "skill name are left out), i.e. the siblings a harness " - "lists next to it. Combines with --competitors") + "skill the catalog lists (skills.yaml when present, else every " + "SKILL.md under it; a template/ copy is left out) except " + "the artifact itself, i.e. the " + "siblings a harness lists next to it. Combines with " + "--competitors") + routing_p.add_argument("--include-disabled", action="store_true", dest="include_disabled", + help="Keep skills that set disable-model-invocation in the field. " + "Off by default: no harness lists them, so the model cannot " + "select them and a card for one measures a choice that does " + "not exist") routing_p.add_argument("--queries", default=None, help="JSON list (or one per line) of user-shaped requests that " "should select this artifact, written independently of it") diff --git a/dokimi/dokimi/external/catalog.py b/dokimi/dokimi/external/catalog.py index b8b22148..4c9877c5 100644 --- a/dokimi/dokimi/external/catalog.py +++ b/dokimi/dokimi/external/catalog.py @@ -93,8 +93,12 @@ def loaded(self) -> bool: return self.task is not None and not self.skipped and not self.failed -def read_skill_manifest(root: Path) -> list[SkillCatalogEntry]: - """Read skills under *root*, preferring a ``skills.yaml`` manifest.""" +def read_skill_manifest(root: Path, notes: list[str] | None = None) -> list[SkillCatalogEntry]: + """Read skills under *root*, preferring a ``skills.yaml`` manifest. + + *notes* receives one line per template or mirror the filesystem scan leaves out (a + manifest lists its skills, so it has none). + """ root = Path(root) manifest_path = root / "skills.yaml" @@ -107,7 +111,7 @@ def read_skill_manifest(root: Path) -> list[SkillCatalogEntry]: source_root=root, skill=skill, ) - for skill in discover_skills(root) + for skill in discover_skills(root, notes=notes) ] import yaml diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 2ec7cf0b..888609c3 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -34,11 +34,14 @@ import math import random import re +import hashlib from collections import Counter from dataclasses import dataclass from functools import partial from pathlib import Path +from dokimi.skills import Skill +from dokimi.utils import parse_frontmatter from .client import LLMClient, parse_json_object @@ -345,6 +348,43 @@ def headings_as_queries(body: str, *, limit: int = 40) -> list[str]: return [h for h in headings if len(tokenize(h)) >= 2][:limit] +def non_skill_reason(ref: str) -> str | None: + """Why a ``skill:`` reference names something other than a skill, or ``None``. + + A shell glob over a skills directory hands the field every file beside the skills (a + ``.mjs`` helper, a ``registry.json``), and a directory with no ``SKILL.md`` in it is not a + skill either. Only ``skill:`` references are checked: a ``text:`` or plain path is a text + artifact, which is a legitimate competitor. + """ + if not ref.startswith("skill:"): + return None + path = Path(ref.removeprefix("skill:")) + if path.is_file() and path.name != "SKILL.md": + return f"{path.name} is not a SKILL.md" + if path.is_dir() and not (path / "SKILL.md").is_file(): + return "the directory has no SKILL.md" + return None + + +def skill_file(ref: str) -> Path | None: + """The ``SKILL.md`` a ``skill:`` reference names, or ``None`` for any other reference.""" + if not ref.startswith("skill:"): + return None + path = Path(ref.removeprefix("skill:")) + return path / "SKILL.md" if path.is_dir() else path + + +def disables_model_invocation(skill_md: Path) -> bool: + """Whether a ``SKILL.md`` sets ``disable-model-invocation``, so no harness lists it. + + The model cannot select such a skill, so a card for it measures a choice that does not + exist. The truthy spellings are the ones ``repos.budget`` reads, so the two agree. + """ + meta, _body = parse_frontmatter(skill_md.read_text(encoding="utf-8")) + return str(meta.get("disable-model-invocation", "")).strip().lower() in ( + "1", "true", "yes", "on") + + class RoutingInputError(ValueError): """A competitor field or query file in which selection is not a measurement.""" @@ -365,20 +405,95 @@ def load_candidate(ref: str) -> tuple[Candidate, str, str | None]: return Candidate(name=name or ref, description=description), body, warning -def catalog_competitors(root: str, exclude: set[str]) -> list[Candidate]: +def catalog_competitors(root: str, exclude: set[str], + notes: list[str] | None = None, *, + include_disabled: bool = False) -> list[Candidate]: """Every skill under ``root`` whose name is not in ``exclude``. The field a harness would list next to the artifact, rather than one the caller picks. + Skills come from the catalog reader, so a ``skills.yaml`` manifest decides which skills + the catalog has. A declared skill that cannot be loaded is left out and described in + ``notes`` (when given) rather than aborting the run. A skill with + ``disable-model-invocation`` is left out too, unless ``include_disabled``. A mirror, a + byte-identical ``SKILL.md`` under another path (a plugin copy of the ``skills/`` tree), is + left out too: the copy nearest the catalog root stands for the card. Raises: - ValueError: ``root`` is not a directory, or a skill under it cannot be loaded. + ValueError: ``root`` is not a directory, or a skill under it cannot be loaded (with + no manifest every ``SKILL.md`` is read, and one that is broken still raises). + RoutingInputError: Two skills in the field share a name with different content; the + message names both paths. """ - from dokimi.skills import discover_skills - if not Path(root).is_dir(): raise ValueError(f"--catalog is not a directory: {root}") - return [Candidate(name=s.name, description=s.description) - for s in discover_skills(root) if s.name not in exclude] + eligible = _eligible_catalog_skills(root, exclude, notes, include_disabled) + _refuse_conflicting_names(eligible) + keeper = _keepers_by_digest(eligible) + candidates: list[Candidate] = [] + for skill, digest in eligible: + if keeper[digest] is not skill: + if notes is not None: + notes.append(f"left out catalog skill {skill.name!r} at {skill.path}: " + f"identical to {keeper[digest].path}") + continue + candidates.append(Candidate(name=skill.name, description=skill.description)) + return candidates + + +def _eligible_catalog_skills(root: str, exclude: set[str], notes: list[str] | None, + include_disabled: bool) -> list[tuple[Skill, str]]: + """Each loadable, non-excluded catalog skill with the sha256 of its ``SKILL.md``.""" + from dokimi.external.catalog import read_skill_manifest + + eligible: list[tuple[Skill, str]] = [] + for entry in read_skill_manifest(Path(root), notes): + skill = entry.skill + if skill is None or not entry.loaded: + if notes is not None: + notes.append(f"skipped catalog entry {entry.name!r}: {entry.error}") + continue + if skill.name in exclude: + continue + if not include_disabled and disables_model_invocation(skill.path): + if notes is not None: + notes.append(f"left out catalog skill {skill.name!r}: " + "disable-model-invocation (--include-disabled keeps it)") + continue + eligible.append((skill, hashlib.sha256(skill.path.read_bytes()).hexdigest())) + return eligible + + +def _refuse_conflicting_names(eligible: list[tuple[Skill, str]]) -> None: + """Two skills with one name and different bytes cannot both stand for the card. + + check_field would only say "pass each competitor once", which is not the problem. + """ + copies_by_name: dict[str, list[tuple[str, str]]] = {} + for skill, digest in eligible: + copies_by_name.setdefault(skill.name, []).append((skill.path.as_posix(), digest)) + for name in sorted(copies_by_name): + copies = sorted(copies_by_name[name]) + first_path, first_digest = copies[0] + other = next((path for path, digest in copies if digest != first_digest), None) + if other is not None: + raise RoutingInputError( + f"catalog has {name} at {first_path} and {other} with " + "different content; dedupe or rename" + ) + + +def _keepers_by_digest(eligible: list[tuple[Skill, str]]) -> dict[str, Skill]: + """The copy with the fewest path parts stands for each byte-identical group. + + Ties go to the path that sorts first, so the choice does not depend on directory order. + """ + keeper: dict[str, Skill] = {} + for skill, digest in eligible: + best = keeper.get(digest) + if best is None or (len(skill.path.parts), skill.path.as_posix()) < ( + len(best.path.parts), best.path.as_posix()): + keeper[digest] = skill + return keeper def check_field(gold: Candidate, competitors: list[Candidate]) -> None: @@ -433,3 +548,205 @@ def load_queries(path: str) -> list[str]: raise RoutingInputError(f"{path}: queries must be a list of strings, " "or one query per line") return [q for q in listed if q.strip()] + + +NO_PICK = "(none)" +TIE = "(tie)" + + +def _no_repeated_key(path: str): + """A ``json`` object hook that refuses a key written twice, which ``dict`` would merge.""" + def build(pairs: list[tuple[str, object]]) -> dict: + seen: set[str] = set() + for key, _value in pairs: + if key in seen: + raise RoutingInputError(f"{path}: skill {key!r} appears more than once") + seen.add(key) + return dict(pairs) + return build + + +def load_queries_by_skill(path: str) -> dict[str, list[str]]: + """Queries per skill, from a JSON object ``{"": ["request", ...]}``. + + Each request is one the named skill should be selected for, written independently of + it. Blank requests are dropped, and a skill left with none is dropped with them. + + Raises: + RoutingInputError: The file is not JSON, is not an object, names a skill twice, or a + value is not a list of strings. + """ + try: + data = json.loads(Path(path).read_text(encoding="utf-8-sig"), + object_pairs_hook=_no_repeated_key(path)) + except json.JSONDecodeError as exc: + raise RoutingInputError(f"{path}: not JSON: {exc}") from None + if not isinstance(data, dict): + raise RoutingInputError(f"{path}: expected an object of skill name to query list") + by_skill: dict[str, list[str]] = {} + for name, listed in data.items(): + if not isinstance(listed, list) or not all(isinstance(q, str) for q in listed): + raise RoutingInputError(f"{path}: the queries for {name!r} must be a list of strings") + queries = [q for q in listed if q.strip()] + if queries: + by_skill[name] = queries + return by_skill + + +def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str]], *, + client: LLMClient | None = None, seed: int = 0) -> dict: + """Each skill's queries, routed against the whole field: a confusion matrix and attractors. + + Every skill that has queries is the gold in turn. Its queries go to the whole field, and + the pick is recorded. The lexical arm always runs (free); the model arm runs when + ``client`` is given. A query no card shares a word with is ``(none)``, not the first card: + the tie-break in ``lexical_routing`` would otherwise name a card that matched nothing. + + An attractor is a skill picked for queries written for other skills. ``wrongly_picked`` + counts those queries, ``sibling_queries`` is how many queries the other skills have, and + ``from`` names the skills whose queries it took. + + Returns: + ``n_skills``, ``n_queries``, and per arm (``lexical``, ``model``): ``confusion`` (gold + skill to picked skill to count, ``(none)`` and ``(tie)`` included), ``own_rate`` per gold + skill (``None`` when none of its queries was scored), ``n_scored`` and ``n_unscored`` + per gold skill, and ``attractors`` ordered by ``wrongly_picked``. The model arm also records ``model``, + ``seed``, ``n_failed``, ``n_abstained`` and ``first_error``. + + Raises: + RoutingInputError: Fewer than two skills in the field; a name repeated in the field; a + query file that names a skill outside the field; no skill with any queries. + """ + names = [c.name for c in candidates] + _check_all_vs_all_input(names, queries_by_skill) + result: dict = { + "n_skills": len(candidates), + "n_queries": sum(len(q) for q in queries_by_skill.values()), + "lexical": _attractor_table(*_lexical_picks(candidates, queries_by_skill, names), + names), + } + if client is not None: + result["model"] = _model_arm(candidates, queries_by_skill, names, client, seed) + return result + + +def _check_all_vs_all_input(names: list[str], queries_by_skill: dict[str, list[str]]) -> None: + """Refuse a field or query file :func:`all_vs_all` cannot measure.""" + if len(names) < 2: + raise RoutingInputError("all-vs-all needs at least two skills in the field") + if len(set(names)) != len(names): + raise RoutingInputError("all-vs-all: the field names a skill more than once") + unknown = sorted(set(queries_by_skill) - set(names)) + if unknown: + raise RoutingInputError("the queries name skills that are not in the field: " + + ", ".join(f"`{n}`" for n in unknown) + + "; a skill that sets disable-model-invocation is left out " + "unless you pass --include-disabled") + if not queries_by_skill: + raise RoutingInputError("no skill in the field has queries to route") + + +def _lexical_pick(bm25: BM25, tokens: list[str], gold: int, candidates: list[Candidate]) -> str: + """The card the lexical arm picks for one query written for ``candidates[gold]``. + + The gold is a pick only when it scores strictly above every other card. Two or more other + cards tied at the top are ``(tie)``, not whichever comes first in the field, so the result + does not depend on the order of the field. One other card tied with the gold is that card: + the gold sorts after what it ties with, as in ``lexical_routing``. + """ + scores = [bm25.score(tokens, i) for i in range(len(candidates))] + top = max(scores) + if top <= 0: + return NO_PICK + at_top = [i for i, score in enumerate(scores) if math.isclose(score, top, rel_tol=1e-12)] + others = [i for i in at_top if i != gold] + if not others: + return candidates[gold].name + return candidates[others[0]].name if len(others) == 1 else TIE + + +def _lexical_picks(candidates: list[Candidate], queries_by_skill: dict[str, list[str]], + names: list[str]) -> tuple[dict[str, Counter], dict[str, int]]: + """The lexical arm of :func:`all_vs_all`: picks per gold skill, and the unscored counts. + + A query with no content word cannot be scored; it is counted in the second dict and left + out of the picks, as ``lexical_routing`` leaves it out of its rates. + """ + bm25 = BM25([tokenize(c.card) for c in candidates]) + lexical: dict[str, Counter] = {} + unscored: dict[str, int] = {} + for name, queries in queries_by_skill.items(): + picks: Counter = Counter() + for query in queries: + tokens = tokenize(query) + if not tokens: + unscored[name] = unscored.get(name, 0) + 1 + continue + picks[_lexical_pick(bm25, tokens, names.index(name), candidates)] += 1 + lexical[name] = picks + return lexical, unscored + + +def _model_arm(candidates: list[Candidate], queries_by_skill: dict[str, list[str]], + names: list[str], client: LLMClient, seed: int) -> dict: + """The model arm of :func:`all_vs_all`: attractor table plus failure counts.""" + model: dict[str, Counter] = {} + unscored: dict[str, int] = {} + n_failed = n_abstained = 0 + first_error = None + for index, name in enumerate(names): + if name not in queries_by_skill: + continue + routed = model_routing(candidates, queries_by_skill[name], client=client, + gold=index, seed=seed + index) + rows = routed["per_query"] + n_failed += routed["n_failed"] + n_abstained += routed["n_abstained"] + if first_error is None: + first_error = next((r["error"] for r in rows if r.get("error")), None) + # A failed call measured nothing: it is not a pick, so it is left out of every rate. + model[name] = Counter(row.get("picked") or NO_PICK for row in rows if not row.get("error")) + if routed["n_failed"]: + unscored[name] = routed["n_failed"] + return dict(_attractor_table(model, unscored, names), model=client.model, seed=seed, + n_failed=n_failed, n_abstained=n_abstained, first_error=first_error) + + +def _attractor_table(picks_by_gold: dict[str, Counter], unscored: dict[str, int], + names: list[str]) -> dict: + """The confusion matrix, own rate and attractor table for one arm of :func:`all_vs_all`. + + Every rate is over the queries that were scored: ``n_scored`` per gold skill, with + ``n_unscored`` beside it. A skill with none scored has an ``own_rate`` of ``None``, which + is not the 0.0 of a skill the arm really never picked. + """ + scored = {gold: sum(picks.values()) for gold, picks in picks_by_gold.items()} + attractors: list[dict] = [] + for name in names: + wrong = {gold: picks[name] for gold, picks in picks_by_gold.items() + if gold != name and picks[name]} + if not wrong: + continue + sibling_queries = sum(n for gold, n in scored.items() if gold != name) + attractors.append({ + "skill": name, + "wrongly_picked": sum(wrong.values()), + "sibling_queries": sibling_queries, + "rate": round(sum(wrong.values()) / sibling_queries, 4) if sibling_queries else None, + "from": sorted(wrong, key=lambda gold: (-wrong[gold], gold)), + }) + attractors.sort(key=lambda row: (-row["wrongly_picked"], row["skill"])) + return { + "confusion": {gold: dict(sorted(picks.items())) + for gold, picks in sorted(picks_by_gold.items())}, + "own_rate": {gold: round(picks[gold] / scored[gold], 4) if scored[gold] else None + for gold, picks in sorted(picks_by_gold.items())}, + "n_scored": dict(sorted(scored.items())), + "n_unscored": dict(sorted(unscored.items())), + "attractors": attractors, + "note": (f"{NO_PICK} counts a query no card shares a word with; in the model arm it " + f"also counts an abstention. {TIE} counts a query on which two or more other " + "cards tie for the top score, so no field order decides it. A query with no " + "content word, and a failed model call, is not scored: it is in neither the " + "confusion matrix nor any rate (see n_unscored)."), + } diff --git a/dokimi/scripts/growth_baseline.json b/dokimi/scripts/growth_baseline.json index c0456236..31b9f8c8 100644 --- a/dokimi/scripts/growth_baseline.json +++ b/dokimi/scripts/growth_baseline.json @@ -1,6 +1,6 @@ { "files": { - "dokimi/commands/novelty.py": {"lines": 864, "rationale": "W3 adds routing no_queries handling, W10 a --catalog help line; file split tracked separately to avoid conflicts across the W-series"}, + "dokimi/commands/novelty.py": {"lines": 1029, "rationale": "routing catalog field, disabled/mirrored skills, all-vs-all, field record in novelty CLI; file split tracked separately"}, "dokimi/harnesses/benchflow.py": {"lines": 869}, "dokimi/harnesses/docker_solver.py": {"lines": 927, "rationale": "pre-existing when the gate was introduced"}, "dokimi/llm.py": {"lines": 1118, "rationale": "grew on main in #180 (replay a stored retry) after #93 seeded this baseline; split it in its own PR"}, diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index 18c6ac60..e90b9fd6 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -16,6 +16,7 @@ RoutingInputError, catalog_competitors, check_field, + disables_model_invocation, headings_as_queries, lexical_routing, load_queries, @@ -509,6 +510,227 @@ def test_catalog_competitors_with_name_in_competitors_excludes_it_from_catalog(t assert set(names) == {"xpu", "cache"}, f"Expected xpu and cache, got {names}" + +def _catalog_skill(root, name, description): + skill_dir = root / "skills" / name + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: {description}\n---\n\n# B\n", encoding="utf-8") + + +def test_the_catalog_is_the_skills_yaml_manifest_when_there_is_one(tmp_path): + # A manifest decides which skills the catalog has: a SKILL.md it does not list is not a + # sibling, and a listed skill with no directory is reported rather than aborting the run. + root = tmp_path / "catalog" + _catalog_skill(root, "xpu", "profile a slow GPU kernel") + _catalog_skill(root, "db", "tune a slow database query") + _catalog_skill(root, "unlisted", "profile a slow thing") + (root / "skills.yaml").write_text("- name: xpu\n- name: db\n- name: ghost\n", + encoding="utf-8") + notes: list[str] = [] + names = [c.name for c in catalog_competitors(str(root), {"perf"}, notes)] + assert names == ["xpu", "db"] + assert len(notes) == 1 and "ghost" in notes[0] + + +def test_a_non_skill_file_among_competitors_is_skipped_with_a_warning(tmp_path, capsys): + # A shell glob over a skills directory passes the helper scripts and an empty directory + # too; that used to abort the whole run on the first .mjs file. + helper = tmp_path / "activation-rule.mjs" + helper.write_text("export {}\n", encoding="utf-8") + empty = tmp_path / "generated" + empty.mkdir() + queries = tmp_path / "q.json" + queries.write_text(json.dumps(["profile a binary"]), encoding="utf-8") + out = tmp_path / "routing.json" + args = _parse("novelty", "routing", + "--artifact", _skill(tmp_path, "a", "profile a binary with perf"), + "--competitors", f"skill:{helper}", f"skill:{empty}", + _skill(tmp_path, "b", "tune a database"), + "--queries", str(queries), "--out", str(out)) + args.func(args) + assert json.loads(out.read_text(encoding="utf-8"))["n_candidates"] == 2 + err = capsys.readouterr().err + assert "skipped competitor" in err and "activation-rule.mjs is not a SKILL.md" in err + assert "the directory has no SKILL.md" in err + + +def test_a_non_skill_artifact_is_still_refused(tmp_path): + helper = tmp_path / "activation-rule.mjs" + helper.write_text("export {}\n", encoding="utf-8") + args = _parse("novelty", "routing", "--artifact", f"skill:{helper}", + "--competitors", _skill(tmp_path, "b", "tune a database"), + "--queries", "x") + with pytest.raises(SystemExit, match="cannot load an artifact"): + args.func(args) + + +def _model_hidden_skill(root, name, description): + skill_dir = root / name + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: {description}\n" + "disable-model-invocation: true\n---\n\n# B\n", encoding="utf-8") + return f"skill:{skill_dir}" + + +def test_a_disable_model_invocation_skill_is_not_a_competitor_unless_asked(tmp_path, capsys): + # ultracave and megacave set the flag: no harness lists them, so a card for them scores a + # choice the model cannot make. + catalog = tmp_path / "skills" + catalog.mkdir() + gold = _skill(catalog, "perf", "profile a slow binary with perf") + _skill(catalog, "db", "tune a slow database query") + _model_hidden_skill(catalog, "ultra", "compress output hard") + queries = tmp_path / "q.json" + queries.write_text(json.dumps(["my binary is slow"]), encoding="utf-8") + out = tmp_path / "routing.json" + args = _parse("novelty", "routing", "--artifact", gold, "--catalog", str(catalog), + "--queries", str(queries), "--out", str(out)) + args.func(args) + assert json.loads(out.read_text(encoding="utf-8"))["n_candidates"] == 2 + assert "left out catalog skill 'ultra'" in capsys.readouterr().err + + kept = tmp_path / "kept.json" + args = _parse("novelty", "routing", "--artifact", gold, "--catalog", str(catalog), + "--queries", str(queries), "--out", str(kept), "--include-disabled") + args.func(args) + assert json.loads(kept.read_text(encoding="utf-8"))["n_candidates"] == 3 + + +def test_an_explicit_disabled_competitor_is_skipped_with_a_warning(tmp_path, capsys): + queries = tmp_path / "q.json" + queries.write_text(json.dumps(["profile a binary"]), encoding="utf-8") + hidden = _model_hidden_skill(tmp_path, "ultra", "compress output hard") + args = _parse("novelty", "routing", "--artifact", _skill(tmp_path, "a", "profile a binary"), + "--competitors", hidden, _skill(tmp_path, "b", "tune a database"), + "--queries", str(queries), "--out", str(tmp_path / "r.json")) + args.func(args) + assert json.loads((tmp_path / "r.json").read_text(encoding="utf-8"))["n_candidates"] == 2 + assert f"skipped competitor {hidden}: disable-model-invocation" in capsys.readouterr().err + + +def test_include_disabled_keeps_a_disabled_skill_named_as_a_competitor(tmp_path, capsys): + queries = tmp_path / "q.json" + queries.write_text(json.dumps(["profile a binary"]), encoding="utf-8") + hidden = _model_hidden_skill(tmp_path, "ultra", "compress output hard") + out = tmp_path / "r.json" + args = _parse("novelty", "routing", "--artifact", _skill(tmp_path, "a", "profile a binary"), + "--competitors", hidden, _skill(tmp_path, "b", "tune a database"), + "--queries", str(queries), "--out", str(out), "--include-disabled") + args.func(args) + assert json.loads(out.read_text(encoding="utf-8"))["n_candidates"] == 3 + assert "skipped competitor" not in capsys.readouterr().err + + +def test_a_disabled_artifact_is_still_routed_but_warned_about(tmp_path, capsys): + queries = tmp_path / "q.json" + queries.write_text(json.dumps(["profile a binary"]), encoding="utf-8") + artifact = _model_hidden_skill(tmp_path, "ultra", "compress output hard") + args = _parse("novelty", "routing", "--artifact", artifact, + "--competitors", _skill(tmp_path, "b", "tune a database"), + "--queries", str(queries), "--out", str(tmp_path / "r.json")) + args.func(args) + assert "the model cannot select it" in capsys.readouterr().err + + +@pytest.mark.parametrize("line, hidden", [ + ("disable-model-invocation: true", True), ("disable-model-invocation: false", False), + ("disable-model-invocation: yes", True), ("description: x", False), + # Unquoted yes/on parse to True; the quoted and numeric spellings reach the string branch. + ("disable-model-invocation: 'yes'", True), ("disable-model-invocation: 'on'", True), + ("disable-model-invocation: 1", True), ("disable-model-invocation: 0", False), + ("disable-model-invocation: 'TRUE'", True), ("disable-model-invocation: 'no'", False), +]) +def test_the_disable_flag_is_read_the_way_the_repo_budget_reads_it(tmp_path, line, hidden): + path = tmp_path / "SKILL.md" + path.write_text(f"---\nname: x\ndescription: x\n{line}\n---\n\nbody\n", encoding="utf-8") + assert disables_model_invocation(path) is hidden + + +def test_a_mirror_of_a_catalog_skill_is_counted_once(tmp_path, capsys): + # A plugin copy of the skills/ tree: byte-identical SKILL.md, same name. It used to make + # the field name db twice and refuse the run. + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf") + _catalog_skill(root, "db", "tune a slow database query") + mirror = root / "plugins" / "x" / "skills" / "db" + mirror.mkdir(parents=True) + (mirror / "SKILL.md").write_bytes((root / "skills" / "db" / "SKILL.md").read_bytes()) + queries = tmp_path / "q.json" + queries.write_text(json.dumps(["my binary is slow"]), encoding="utf-8") + out = tmp_path / "routing.json" + args = _parse("novelty", "routing", "--artifact", f"skill:{root / 'skills' / 'perf'}", + "--catalog", str(root), "--queries", str(queries), "--out", str(out)) + args.func(args) + assert json.loads(out.read_text(encoding="utf-8"))["n_candidates"] == 2 + err = capsys.readouterr().err + assert "identical to" in err and "skills" in err + + +def test_the_mirror_kept_is_the_copy_nearest_the_catalog_root(tmp_path): + root = tmp_path / "catalog" + _catalog_skill(root, "db", "tune a slow database query") + deep = root / "plugins" / "x" / "skills" / "db" + deep.mkdir(parents=True) + (deep / "SKILL.md").write_bytes((root / "skills" / "db" / "SKILL.md").read_bytes()) + notes: list[str] = [] + assert [c.name for c in catalog_competitors(str(root), set(), notes)] == ["db"] + assert "plugins" in notes[0] and "identical to" in notes[0] + # The note names the copy that was kept, not only the one left out. + assert notes[0].endswith(f"identical to {root / 'skills' / 'db' / 'SKILL.md'}") + + +def test_mirrors_at_one_depth_keep_the_path_that_sorts_first(tmp_path): + root = tmp_path / "catalog" + for branch in ("b", "a"): + copy = root / branch / "skills" / "db" + copy.mkdir(parents=True) + (copy / "SKILL.md").write_text( + "---\nname: db\ndescription: tune a slow database query\n---\n\n# B\n", + encoding="utf-8") + notes: list[str] = [] + assert [c.name for c in catalog_competitors(str(root), set(), notes)] == ["db"] + assert notes[0].endswith(f"identical to {root / 'a' / 'skills' / 'db' / 'SKILL.md'}") + assert f"left out {root / 'b' / 'skills' / 'db' / 'SKILL.md'}" in notes[0] + + +def test_two_catalog_skills_with_one_name_and_different_content_are_refused_by_name( + tmp_path, capsys +): + # Not "pass each competitor once": the catalog holds two different cards under one name, + # and the refusal names both copies rather than the duplicate-competitor check. + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf") + _catalog_skill(root, "db", "tune a slow database query") + fork = root / "plugins" / "x" / "skills" / "db" + fork.mkdir(parents=True) + (fork / "SKILL.md").write_text( + "---\nname: db\ndescription: a different card\n---\n\n# B\n", encoding="utf-8" + ) + queries = tmp_path / "q.json" + queries.write_text(json.dumps(["my binary is slow"]), encoding="utf-8") + out = tmp_path / "routing.json" + args = _parse( + "novelty", + "routing", + "--artifact", + f"skill:{root / 'skills' / 'perf'}", + "--catalog", + str(root), + "--queries", + str(queries), + "--out", + str(out), + ) + with pytest.raises(SystemExit) as refused: + args.func(args) + message = str(refused.value) + assert message.startswith("catalog has db at ") and "dedupe or rename" in message + assert str(root / "skills" / "db" / "SKILL.md") in message + assert str(fork / "SKILL.md") in message + assert "more than once" not in message and not out.exists() + def test_headings_inside_code_only_refuse_the_run_and_say_why(tmp_path): out = tmp_path / "routing.json" args = _parse("novelty", "routing", diff --git a/dokimi/tests/test_novelty_routing_all_vs_all.py b/dokimi/tests/test_novelty_routing_all_vs_all.py new file mode 100644 index 00000000..64d16185 --- /dev/null +++ b/dokimi/tests/test_novelty_routing_all_vs_all.py @@ -0,0 +1,491 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""All-vs-all routing: every skill in the field is the gold in turn.""" + +from __future__ import annotations + +import json + +import pytest + +from dokimi.cli import build_parser +from dokimi.novelty.client import LLMClient +from dokimi.novelty.routing import ( + Candidate, + RoutingInputError, + all_vs_all, + load_queries_by_skill, +) + +PERF = Candidate(name="linux-perf", + description="Profile a Linux binary with perf: record, report, annotate") +TBB = Candidate(name="onetbb-quickstart", + description="Parallelize C++ loops with oneTBB task groups") +MKL = Candidate(name="mkl-advisor", + description="Choose oneMKL BLAS routines for dense linear algebra") + + +def _parse(*argv): + return build_parser().parse_args(list(argv)) + + +def _catalog_skill(root, name, description): + skill_dir = root / "skills" / name + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: {description}\n---\n\n# B\n", encoding="utf-8") + + +def _write_queries(tmp_path, by_skill): + path = tmp_path / "queries_by_skill.json" + path.write_text(json.dumps(by_skill), encoding="utf-8") + return path + + +# hub's card shares "slow" with the others' queries, so it takes them. Lexically it picks +# linux-perf's two queries and nothing of mkl-advisor's; the model arm (below) picks it for all. +HUB = Candidate(name="hub", description="slow programs and slow queries advice") +ALL_FIELD = [PERF, HUB, MKL] +ALL_QUERIES = { + "linux-perf": ["my slow program needs advice", "profile a slow program"], + "mkl-advisor": ["choose blas routines for dense linear algebra", "zzz unrelated words"], +} + + +def test_all_vs_all_names_the_skill_that_takes_the_queries_of_others(): + confusion = all_vs_all(ALL_FIELD, ALL_QUERIES)["lexical"]["confusion"] + assert confusion["linux-perf"] == {"hub": 2} + + +def test_an_attractor_row_says_how_many_sibling_queries_it_took_and_from_whom(): + result = all_vs_all(ALL_FIELD, ALL_QUERIES) + assert result["n_skills"] == 3 and result["n_queries"] == 4 + (row,) = result["lexical"]["attractors"] + assert row == {"skill": "hub", "wrongly_picked": 2, "sibling_queries": 4, "rate": 0.5, + "from": ["linux-perf"]} + + +def test_a_query_no_card_shares_a_word_with_is_none_not_the_first_card(): + # linux-perf is first in the field, so the lexical tie-break would name it here. + lexical = all_vs_all(ALL_FIELD, ALL_QUERIES)["lexical"] + assert lexical["confusion"]["mkl-advisor"] == {"mkl-advisor": 1, "(none)": 1} + assert lexical["own_rate"]["mkl-advisor"] == 0.5 + + +def test_the_model_arm_is_reported_only_when_a_client_is_given(): + assert "model" not in all_vs_all(ALL_FIELD, ALL_QUERIES) + + +def test_the_model_arm_attractor_is_the_skill_the_selector_always_picks(): + def transport(prompt: str, system: str | None): + lines = prompt.split("TOOLS:", 1)[1].strip().splitlines() + return json.dumps({"pick": next(i + 1 for i, line in enumerate(lines) + if "hub" in line)}), None + + client = LLMClient(model="m", role="selector", transport=transport) + model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=client, seed=7)["model"] + assert model["seed"] == 7 + (row,) = model["attractors"] + assert (row["skill"], row["wrongly_picked"], row["sibling_queries"]) == ("hub", 4, 4) + assert row["from"] == ["linux-perf", "mkl-advisor"] + + +@pytest.mark.parametrize("name", ["model-a", "model-b"]) +def test_the_model_arm_names_the_selector_model_that_made_it(name): + client = LLMClient(model=name, role="selector", + transport=lambda prompt, system: (json.dumps({"pick": 1}), None)) + assert all_vs_all(ALL_FIELD, ALL_QUERIES, client=client)["model"]["model"] == name + + +@pytest.mark.parametrize("field, queries, message", [ + ([PERF], {"linux-perf": ["q"]}, "at least two skills"), + ([PERF, PERF], {"linux-perf": ["q"]}, "more than once"), + ([PERF, TBB], {"mkl-advisor": ["q"]}, "not in the field: `mkl-advisor`"), + ([PERF, TBB], {}, "no skill in the field has queries"), +]) +def test_all_vs_all_refuses_a_field_where_it_would_not_measure_selection(field, queries, + message): + with pytest.raises(RoutingInputError, match=message): + all_vs_all(field, queries) + + +def test_a_queries_file_names_skills_by_object_and_drops_blank_requests(tmp_path): + path = _write_queries(tmp_path, {"linux-perf": [" ", "profile it"], "db": [" "]}) + assert load_queries_by_skill(str(path)) == {"linux-perf": ["profile it"]} + + +@pytest.mark.parametrize("payload", ['["a list"]', '{"linux-perf": "one string"}', + '{"linux-perf": [1]}']) +def test_a_queries_file_that_is_not_an_object_of_string_lists_is_refused(tmp_path, payload): + path = tmp_path / "q.json" + path.write_text(payload, encoding="utf-8") + with pytest.raises(RoutingInputError): + load_queries_by_skill(str(path)) + + +def test_all_vs_all_writes_the_matrix_and_prints_the_attractors(tmp_path, capsys): + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + _catalog_skill(root, "db", "tune a slow database query with an index") + _catalog_skill(root, "hub", "general advice for slow programs and slow queries") + queries = _write_queries(tmp_path, { + "perf": ["my slow program needs advice", "profile a slow program"], + "db": ["my database query is slow", "add an index to speed up a query"], + }) + out = tmp_path / "all_vs_all.json" + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(out)) + args.func(args) + payload = json.loads(out.read_text(encoding="utf-8")) + assert payload["mode"] == "all_vs_all" and payload["n_candidates"] == 3 + assert payload["result"]["lexical"]["confusion"]["db"]["db"] == 2 + assert payload["result"]["lexical"]["confusion"]["perf"] == {"perf": 1, "hub": 1} + stdout = capsys.readouterr().out + assert "all-vs-all: 3 skills in the field, 4 queries" in stdout + assert "lexical: hub picked for 1 of 4 sibling queries (from perf)" in stdout + + +def test_all_vs_all_with_model_names_its_model_calls_on_stderr(tmp_path, capsys, monkeypatch): + # One selector call per query: 2 + 2 queries, so 4 calls, each over the 3 cards. + import dokimi.commands.novelty as novelty_cmd + + def transport(prompt: str, system: str | None): + return json.dumps({"pick": 1}), None + + monkeypatch.setattr( + novelty_cmd, + "_client", + lambda model, provider, role, args: LLMClient( + model=model, provider=provider, role=role, transport=transport + ), + ) + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + _catalog_skill(root, "db", "tune a slow database query with an index") + _catalog_skill(root, "hub", "general advice for slow programs and slow queries") + queries = _write_queries( + tmp_path, + { + "perf": ["my slow program needs advice", "profile a slow program"], + "db": ["my database query is slow", "add an index to speed up a query"], + }, + ) + args = _parse( + "novelty", + "routing", + "--all-vs-all", + "--catalog", + str(root), + "--queries-by-skill", + str(queries), + "--out", + str(tmp_path / "o.json"), + "--model", + "m", + ) + args.func(args) + assert ( + "all-vs-all: --model makes 4 model calls, one per query, over 3 cards each" + in capsys.readouterr().err + ) + + +def test_all_vs_all_without_model_says_nothing_about_model_calls(tmp_path, capsys): + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + _catalog_skill(root, "db", "tune a slow database query with an index") + queries = _write_queries(tmp_path, {"perf": ["profile a slow program"]}) + args = _parse( + "novelty", + "routing", + "--all-vs-all", + "--catalog", + str(root), + "--queries-by-skill", + str(queries), + "--out", + str(tmp_path / "o.json"), + ) + args.func(args) + assert "model calls" not in capsys.readouterr().err + + +def test_all_vs_all_takes_no_artifact(tmp_path): + queries = _write_queries(tmp_path, {"linux-perf": ["q"]}) + args = _parse("novelty", "routing", "--all-vs-all", "--artifact", "skill:/x", + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) + with pytest.raises(SystemExit, match="takes no --artifact"): + args.func(args) + + +def test_a_single_routing_run_still_needs_an_artifact(tmp_path): + args = _parse("novelty", "routing", "--out", str(tmp_path / "o.json")) + with pytest.raises(SystemExit, match="--all-vs-all for the whole field"): + args.func(args) + + +def _two_identical_cards(order): + cards = {"a": Candidate(name="a", description="format json output"), + "b": Candidate(name="b", description="format json output")} + return [cards[n] for n in order] + + +@pytest.mark.parametrize("order", [("a", "b"), ("b", "a")]) +def test_two_identical_cards_tie_and_neither_is_scored_a_win(order): + result = all_vs_all(_two_identical_cards(order), + {"a": ["format json"], "b": ["format json"]})["lexical"] + assert result["own_rate"] == {"a": 0.0, "b": 0.0} + assert result["confusion"] == {"a": {"b": 1}, "b": {"a": 1}} + + +def test_a_three_way_tie_is_not_a_perfect_pick(): + field = [Candidate(name=f"s{i}", description="common words") for i in range(3)] + lexical = all_vs_all(field, {"s0": ["common"]})["lexical"] + assert lexical["own_rate"] == {"s0": 0.0} + + +def test_attractors_are_ranked_by_count_and_rated_over_the_other_skills_queries(): + # hub takes 2 of perf's 2 queries and 1 of db's 3; sink takes 1 of perf's. + field = [Candidate(name="perf", description="profile binary"), + Candidate(name="db", description="index query"), + Candidate(name="hub", description="slow advice"), + Candidate(name="sink", description="gpu offload")] + queries = {"perf": ["slow one", "slow two", "gpu thing"], + "db": ["slow query", "index query", "index query again"]} + lexical = all_vs_all(field, queries)["lexical"] + hub, sink = lexical["attractors"][:2] + assert hub == {"skill": "hub", "wrongly_picked": 3, "sibling_queries": 6, "rate": 0.5, + "from": ["perf", "db"]} + assert sink["skill"] == "sink" and sink["wrongly_picked"] == 1 + + +def test_an_attractor_with_queries_of_its_own_is_rated_over_the_other_skills_queries_only(): + # perf takes db's one query; perf's own three queries are not sibling queries of perf. + field = [Candidate(name="perf", description="profile binary slow"), + Candidate(name="db", description="index query")] + queries = {"perf": ["profile binary", "profile run", "binary size"], "db": ["slow"]} + assert all_vs_all(field, queries)["lexical"]["attractors"] == [ + {"skill": "perf", "wrongly_picked": 1, "sibling_queries": 1, "rate": 1.0, + "from": ["db"]}] + + +def _positional_client(pick_for): + def transport(prompt: str, system: str | None): + picked = pick_for(prompt) + return json.dumps({"pick": picked}), None + return LLMClient(model="m", role="selector", transport=transport) + + +def test_a_selector_that_abstains_is_none_and_counted_in_the_model_arm(): + model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=_positional_client(lambda p: 0))["model"] + assert model["confusion"] == {"linux-perf": {"(none)": 2}, "mkl-advisor": {"(none)": 2}} + assert model["n_abstained"] == 4 and model["n_failed"] == 0 + assert model["attractors"] == [] + + +def test_the_model_arm_seeds_each_gold_apart_and_hands_model_routing_the_gold(monkeypatch): + import dokimi.novelty.routing as routing + + seen = [] + real = routing.model_routing + + def spy(candidates, queries, **kwargs): + seen.append((kwargs["gold"], kwargs["seed"])) + return real(candidates, queries, **kwargs) + + monkeypatch.setattr(routing, "model_routing", spy) + all_vs_all(ALL_FIELD, ALL_QUERIES, client=_positional_client(lambda p: 1), seed=10) + assert seen == [(0, 10), (2, 12)] + + +def test_the_model_arm_follows_each_call_not_one_fixed_card(): + calls = iter([1, 2, 3, 1]) + model = all_vs_all(ALL_FIELD, ALL_QUERIES, + client=_positional_client(lambda p: next(calls)), seed=3)["model"] + assert sum(sum(row.values()) for row in model["confusion"].values()) == 4 + assert len({name for row in model["confusion"].values() for name in row}) > 1 + + +def test_a_transport_that_raises_is_counted_not_reported_as_a_clean_zero(): + def transport(prompt: str, system: str | None): + raise RuntimeError("boom") + + client = LLMClient(model="m", role="selector", transport=transport) + model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=client)["model"] + assert model["n_failed"] == 4 and model["n_abstained"] == 0 + assert "boom" in model["first_error"] + + +def test_all_vs_all_exits_nonzero_when_every_model_call_fails(tmp_path, capsys, monkeypatch): + import dokimi.commands.novelty as novelty_cmd + + def transport(prompt: str, system: str | None): + raise RuntimeError("boom") + + monkeypatch.setattr(novelty_cmd, "_client", lambda model, provider, role, args: LLMClient( + model=model, provider=provider, role=role, transport=transport)) + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + _catalog_skill(root, "db", "tune a slow database query with an index") + queries = _write_queries(tmp_path, {"perf": ["profile a slow program"], + "db": ["add an index"]}) + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json"), + "--model", "m") + with pytest.raises(SystemExit, match="2 of 2 model call.s. failed"): + args.func(args) + captured = capsys.readouterr() + assert "OK Saved" not in captured.out + assert "2 model call(s) failed" in captured.err and "boom" in captured.err + + +def test_all_vs_all_stops_at_the_max_usd_cap_the_docs_point_to(tmp_path, monkeypatch): + import dokimi.commands.novelty as novelty_cmd + from dokimi.novelty.client import SpendCap + from dokimi.metrics.usage import UsageRecord + + calls: list[str] = [] + + def transport(prompt: str, system: str | None = None): + calls.append(prompt) + return "none", UsageRecord(cost_usd=0.6) + + cap = SpendCap(limit_usd=1.0) + monkeypatch.setattr(novelty_cmd, "_client", lambda model, provider, role, args: LLMClient( + model=model, provider=provider, role=role, transport=transport, cap=cap)) + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + _catalog_skill(root, "db", "tune a slow database query with an index") + queries = _write_queries(tmp_path, {"perf": ["profile a slow program", "find hot functions"], + "db": ["add an index"]}) + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json"), + "--model", "m", "--max-usd", "1") + with pytest.raises(SystemExit, match=r"\$1\.20 of \$1\.00"): + args.func(args) + assert len(calls) == 2 # the third request was refused before it reached the provider + + +@pytest.mark.parametrize("make", ["missing", "not_utf8"]) +def test_an_unreadable_queries_file_is_a_message_not_a_traceback(tmp_path, make): + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary") + _catalog_skill(root, "db", "tune a slow database query") + queries = tmp_path / "q.json" + if make == "not_utf8": + queries.write_bytes(b"\xff\xfe\x00bad") + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) + with pytest.raises(SystemExit, match="cannot read the queries"): + args.func(args) + + +def _cat(root, names_descriptions): + for name, description in names_descriptions.items(): + _catalog_skill(root, name, description) + + +def test_a_partial_model_failure_exits_non_zero_and_leaves_failed_queries_out( + tmp_path, capsys, monkeypatch): + import dokimi.commands.novelty as novelty_cmd + + answered = [] + + def transport(prompt: str, system: str | None): + if answered: + raise RuntimeError("rate limited") + answered.append(1) + return '{"pick": 1}', None + + monkeypatch.setattr(novelty_cmd, "_client", lambda model, provider, role, args: LLMClient( + model=model, provider=provider, role=role, transport=transport)) + root = tmp_path / "catalog" + _cat(root, {"perf": "profile a slow binary with perf", "db": "tune a slow database query"}) + queries = _write_queries(tmp_path, {"db": ["add an index"], "perf": ["profile it", "perf it"]}) + out = tmp_path / "o.json" + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(out), "--model", "m") + with pytest.raises(SystemExit, match="2 of 3 model call"): + args.func(args) + assert "OK Saved" not in capsys.readouterr().out + model = json.loads(out.read_text(encoding="utf-8"))["result"]["model"] + assert model["n_failed"] == 2 + # perf's two queries both failed: no rate, not a measured 0.0. + assert model["own_rate"]["perf"] is None and model["n_scored"]["perf"] == 0 + assert model["n_unscored"] == {"perf": 2} + + +def test_a_query_with_no_content_word_is_not_scored_as_a_miss(): + field = [Candidate("alpha", "convert a json file to yaml"), + Candidate("beta", "tune a slow database query")] + result = all_vs_all(field, {"alpha": ["convert my json file to yaml", + "json to yaml please", "the of and"]}) + lexical = result["lexical"] + assert lexical["own_rate"]["alpha"] == 1.0 + assert lexical["n_scored"] == {"alpha": 2} and lexical["n_unscored"] == {"alpha": 1} + + +@pytest.mark.parametrize("order", [(0, 1, 2), (2, 1, 0), (1, 2, 0)]) +def test_a_tie_among_other_cards_is_a_tie_in_any_field_order(order): + cards = [Candidate("fft", "fast transforms"), Candidate("linalg", "fast transforms"), + Candidate("random", "fast transforms"), + Candidate("gold", "something unrelated entirely")] + field = [cards[i] for i in order] + [cards[3]] + lexical = all_vs_all(field, {"gold": ["fast transforms"]})["lexical"] + assert lexical["confusion"]["gold"] == {"(tie)": 1} + assert lexical["attractors"] == [] + + +def test_the_saved_payload_records_the_field_rule(tmp_path): + root = tmp_path / "catalog" + _cat(root, {"perf": "profile a slow binary", "db": "tune a slow database query"}) + hidden = root / "skills" / "ghost" + hidden.mkdir(parents=True) + (hidden / "SKILL.md").write_text( + "---\nname: ghost\ndescription: x\ndisable-model-invocation: true\n---\n\n# B\n", + encoding="utf-8") + queries = _write_queries(tmp_path, {"perf": ["profile a binary"]}) + out = tmp_path / "o.json" + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(out)) + args.func(args) + field = json.loads(out.read_text(encoding="utf-8"))["field"] + assert field["include_disabled"] is False + assert sorted(field["competitors"]) == ["db", "perf"] + assert len(field["left_out"]) == 1 and "ghost" in field["left_out"][0] + + +def test_a_skill_named_twice_in_the_queries_file_is_refused(tmp_path): + path = tmp_path / "q.json" + path.write_text('{"a": ["profile"], "a": ["slow"]}', encoding="utf-8") + with pytest.raises(RoutingInputError, match="skill 'a' appears more than once"): + load_queries_by_skill(str(path)) + + +def test_the_not_in_the_field_error_names_the_disable_flag_and_its_override(): + with pytest.raises(RoutingInputError, match="disable-model-invocation.*--include-disabled"): + all_vs_all([PERF, MKL], {"ghost": ["x y"]}) + + +@pytest.mark.parametrize("flag", [["--queries", "q.txt"], ["--queries-from-headings"]]) +def test_all_vs_all_refuses_the_single_artifact_query_flags(tmp_path, flag): + queries = _write_queries(tmp_path, {"perf": ["profile"]}) + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(tmp_path), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json"), *flag) + with pytest.raises(SystemExit, match="takes --queries-by-skill, not --queries"): + args.func(args) + + +def test_a_single_artifact_routing_run_records_the_field_rule_too(tmp_path): + root = tmp_path / "catalog" + _cat(root, {"perf": "profile a slow binary", "db": "tune a slow database query"}) + queries = tmp_path / "q.txt" + queries.write_text("profile a binary\n", encoding="utf-8") + out = tmp_path / "r.json" + args = _parse("novelty", "routing", "--artifact", f"skill:{root / 'skills' / 'perf'}", + "--catalog", str(root), "--include-disabled", "--queries", str(queries), + "--out", str(out)) + args.func(args) + field = json.loads(out.read_text(encoding="utf-8"))["field"] + assert field == {"include_disabled": True, "competitors": ["db"], "left_out": []}