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 000000000..53e0e44c6 --- /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 000000000..f96d23a27 --- /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 000000000..c7dec3631 --- /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 000000000..de87206de --- /dev/null +++ b/dokimi/changelog.d/20261021_133000_napetrov_routing-mirror-dedupe.md @@ -0,0 +1,6 @@ +### Fixed + +- **`novelty routing --catalog` with a `skills.yaml` counts a mirrored skill once** — a catalog + that lists the same `SKILL.md` bytes under two paths (a plugin copy of the `skills/` tree) + used to put the 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 000000000..4bbb5209a --- /dev/null +++ b/dokimi/changelog.d/20261021_134000_napetrov_routing-all-vs-all.md @@ -0,0 +1,14 @@ +### 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` or `--repeats`. 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 query with no content word is unscored in both arms and not sent + to the model); 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`. +- `--all-vs-all` names skills with the same description as one tied group (`tied_groups`), not + as each other's attractor: a pick between them is in the confusion matrix only. diff --git a/dokimi/docs/skill-novelty-walkthrough.md b/dokimi/docs/skill-novelty-walkthrough.md index 89644dffb..5ad273f62 100644 --- a/dokimi/docs/skill-novelty-walkthrough.md +++ b/dokimi/docs/skill-novelty-walkthrough.md @@ -41,12 +41,10 @@ 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 vendored and VCS directories such as -`.git`, `node_modules`, `site-packages` and virtualenvs are not walked), 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 and vendored or VCS directories +such as `.git` are left out, and a byte-identical copy under another path counts once), 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 fcae011ee..2ac08ef34 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -600,10 +600,23 @@ python cli.py novelty routing --artifact skill:path/to/skill \ --queries queries.json [--model M [--repeats N]] ``` -`--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, and vendored and VCS directories such as `.git`, -`node_modules`, `site-packages` and virtualenvs are not walked). +`--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 and vendored or VCS +directories such as `.git` and `node_modules`, which are not walked. A byte-identical copy under +another path counts once. 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. A shell glob must be expanded first, with the prefix on each path +(`skill:dir/a/SKILL.md skill:dir/b/SKILL.md`): an unexpanded `skill:dir/*` is refused as a missing +path. +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: @@ -631,20 +644,50 @@ Two numbers, reported side by side and never merged: queries x repeats, `n_abstained` and `n_failed` count calls, and each `per_query` row carries its `repeat` index. -A `--model` run also writes a `cost` block (calls, input and output tokens, cache read and +A single-artifact `--model` run also writes a `cost` block (calls, input and output tokens, cache read and write tokens, and `usd`, which is `null` when any call this run paid for had no known price) and prints the same totals on stdout. Cache tokens are reported separately from input tokens, and depending on the provider they may or may not be counted inside `input_tokens`. A run without `--model` makes no calls and has no `cost` block. `usd` is also `null` when a call returned no usage, and a call that raised is not counted. A call replayed from the response cache is counted in `replayed_calls` and left out of the token and `usd` totals, since this run did not pay for it; -`llm_calls` still counts it. +`llm_calls` still counts it. `--all-vs-all` writes no `cost` block. `recall_at_3` comes back `null` with three or fewer candidates, where every rank is `<= 3` and the figure would be a structural 1.0. The lexical arm also reports `n_queries_given` 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`). Skills whose descriptions have the same words (case and punctuation aside) are a tied group +(`tied_groups`, named once on the console): a pick between them is in the confusion matrix but +is no attractor, and a tied skill's own rate is lower by the picks inside its group. 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`, +`--queries-from-headings` or `--repeats` (one model call per query). 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 number of queries in `queries.json` that have a +content word (one without is unscored in both arms and not sent), 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 c468ca18a..c22fdf419 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -279,6 +279,192 @@ 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: + OSError, ValueError, RecursionError: 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.all_vs_all import all_vs_all, load_queries_by_skill + from dokimi.novelty.routing import RoutingInputError, tokenize + + _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 (OSError, ValueError, RecursionError) 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 call per query with a content word, each sending every card in the field. + calls = sum(bool(tokenize(q)) for qs in queries_by_skill.values() for q in qs) + 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) + _warn_blind_skills(result, queries_by_skill) + + model_arm = result.get("model") or {} + failed = model_arm.get("n_failed", 0) + sent = sum(model_arm.get("n_scored", {}).values()) + failed # contentless ones are not sent + 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 {sent} 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 args.repeats > 1: + raise SystemExit("--all-vs-all takes no --repeats: it makes one call per query") + 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 _warn_blind_skills(result: dict, queries_by_skill: dict[str, list[str]]) -> None: + """Warn when the lexical arm scores none of a skill's queries: its own rate is then None.""" + lexical = result.get("lexical") + if not lexical: + return + blind = [name for name in sorted(queries_by_skill) if lexical["n_scored"].get(name) == 0] + if blind: + print(f"WARNING: the lexical arm cannot score any query of {', '.join(blind)} " + "(no content word in them), so their lexical own_rate is None", file=sys.stderr) + + +def _print_attractors(result: dict) -> None: + """The top five attractors of each arm, one line each, and what the arm left out. + + A tie and an unscored query name no pick, so a "no skill was picked" line beside them + would read as a clean result; the line after the attractors says how many there were. + Skills with the same description words are one tied group, named once, not each other's + attractor. + """ + from dokimi.novelty.all_vs_all import TIE + + for group in result["tied_groups"]: + print(f"tied: {', '.join(group)} have the same description; one group, not attractors " + "of each other") + for arm in ("lexical", "model"): + if arm not in result: + continue + section = result[arm] + rows = section["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'])})") + tied = sum(picks.get(TIE, 0) for picks in section["confusion"].values()) + unscored = sum(section["n_unscored"].values()) + if tied or unscored: + print(f"{arm}: {tied} tied and {unscored} unscored queries are not counted as picks") + + def cmd_novelty_routing(args: argparse.Namespace) -> None: """Can this artifact be selected against its competitors? Lexical floor, model on top.""" if args.repeats > 1 and not args.model: @@ -287,36 +473,31 @@ def cmd_novelty_routing(args: argparse.Namespace) -> None: "measures nothing" ) 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 (OSError, ValueError, RecursionError) 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. @@ -355,6 +536,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: @@ -658,18 +840,35 @@ 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, a byte-" - "identical mirror that loads under the same skill name, and " - "vendored or VCS directories like .git are left out), i.e. the " - "siblings a harness lists next to it. Combines with --competitors") + help="Directory of skills to take the competitors from: every " + "skill the catalog lists (skills.yaml when present, else every " + "SKILL.md under it; a template/ copy and vendored or VCS " + "directories like .git are left out, and a byte-identical " + "copy under another path counts once) 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 d423c499d..0eb4c8083 100644 --- a/dokimi/dokimi/external/catalog.py +++ b/dokimi/dokimi/external/catalog.py @@ -94,17 +94,20 @@ def loaded(self) -> bool: return self.task is not None and not self.skipped and not self.failed -def _scan_skill_files(root: Path) -> list[SkillCatalogEntry]: +def _scan_skill_files(root: Path, notes: list[str] | None = None) -> list[SkillCatalogEntry]: """One entry per ``SKILL.md`` under *root*, as the manifest path gives. A skill that does not load is an error row, so one broken skill does not abort the scan. A ``SKILL.md`` under a ``template``/``templates`` directory is a scaffold and is left - out, as :func:`dokimi.skills.discover_skills` leaves it out; one under a vendored or - VCS directory is a dependency's copy and is not walked, as there too. + out, as :func:`dokimi.skills.discover_skills` leaves it out; *notes* gets a line for it. + One under a vendored or VCS directory is a dependency's copy and is not walked, as there too. + Byte-identical copies are kept here: ``catalog scan`` reports them. """ found: list[SkillCatalogEntry] = [] for skill_file in walk_skill_files(root): if TEMPLATE_DIR_NAMES.intersection(skill_file.relative_to(root).parts[:-1]): + if notes is not None: + notes.append(f"left out {skill_file}: a template, not a skill") continue try: skill = load_skill(skill_file) @@ -168,13 +171,17 @@ def _manifest_items(manifest_path: Path) -> tuple[list, bool]: return items, list_shape -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 the filesystem scan leaves out (a manifest lists + its skills, so it has none). + """ root = Path(root) manifest_path = root / "skills.yaml" if not manifest_path.is_file(): - return _scan_skill_files(root) + return _scan_skill_files(root, notes) items, list_shape = _manifest_items(manifest_path) entries: list[SkillCatalogEntry] = [] diff --git a/dokimi/dokimi/novelty/all_vs_all.py b/dokimi/dokimi/novelty/all_vs_all.py new file mode 100644 index 000000000..f26267c92 --- /dev/null +++ b/dokimi/dokimi/novelty/all_vs_all.py @@ -0,0 +1,260 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""All-vs-all routing: every skill's queries routed against the whole field. + +Each skill with queries is the gold in turn, and its queries go to the whole field. The result +is a confusion matrix, each skill's own rate, and the attractors: skills picked for queries +written for other skills. :mod:`dokimi.novelty.routing` holds the single-artifact measurement +and the field it routes against. +""" + +from __future__ import annotations + +import json +import math +from collections import Counter +from pathlib import Path + +from .client import LLMClient +from .routing import BM25, Candidate, RoutingInputError, model_routing, tokenize + +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 nested too deeply, 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 + except RecursionError: + raise RoutingInputError(f"{path}: nested too deeply to read") 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. Skills with the same description are a + tied group (``tied_groups``): a pick between them is in the confusion matrix but is no + attractor, and their queries are no sibling queries of each other. + + Returns: + ``n_skills``, ``n_queries``, ``tied_groups``, 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()), + "tied_groups": _tied_groups(candidates), + "lexical": _attractor_table(*_lexical_picks(candidates, queries_by_skill, names), + names, _tied_groups(candidates)), + } + if client is not None: + result["model"] = _model_arm(candidates, queries_by_skill, names, client, seed) + return result + + +def _tied_groups(candidates: list[Candidate]) -> list[list[str]]: + """Groups of two or more skills whose descriptions have the same words, in field order. + + The words are the lexical arm's tokens, so case and punctuation do not split a group. Such + skills tie on a query that names neither of them when their names have the same number of + words (otherwise the shorter card wins), so neither is the other's attractor: the group is + reported once instead. + """ + by_words: dict[tuple[str, ...], list[str]] = {} + for candidate in candidates: + words = tuple(sorted(tokenize(candidate.description))) + if words: + by_words.setdefault(words, []).append(candidate.name) + return [group for group in by_words.values() if len(group) > 1] + + +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. + + A query with no content word is not sent: it is unscored here as in the lexical arm, so + both arms' rates cover the same queries and no call is spent on it. + """ + 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 + queries = [q for q in queries_by_skill[name] if tokenize(q)] + if len(queries) < len(queries_by_skill[name]): + unscored[name] = len(queries_by_skill[name]) - len(queries) + if not queries: + model[name] = Counter() + continue + routed = model_routing(candidates, queries, 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] = unscored.get(name, 0) + routed["n_failed"] + return dict(_attractor_table(model, unscored, names, _tied_groups(candidates)), + 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], tied_groups: list[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. A pick between skills of one tied + group is no attractor, and their queries are not sibling queries of each other. + """ + tied = {name: set(group) for group in tied_groups for name in group} + + def same_group(gold: str, name: str) -> bool: + return gold == name or name in tied.get(gold, ()) + + 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 not same_group(gold, name) and picks[name]} + if not wrong: + continue + sibling_queries = sum(n for gold, n in scored.items() if not same_group(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/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index e18c04245..021ba4475 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -30,6 +30,7 @@ from __future__ import annotations +import hashlib import json import math import random @@ -39,6 +40,8 @@ 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 @@ -281,7 +284,7 @@ def model_routing( prompt, accept=partial(_is_pick_reply, n_cards=len(order)), draw=repeat, ) except Exception as exc: # noqa: BLE001 - one failed query must not lose the rest - row = {**base, "picked": None, "error": str(exc)[:200]} + row = {**base, "picked": None, "error": str(exc)[:200] or type(exc).__name__} else: # A reply that is not a usable pick is the model declining to route, not a # transport failure: it stays in the denominator like an explicit abstention, @@ -422,6 +425,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.""" @@ -442,20 +482,98 @@ 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 its ``skills.yaml`` is invalid (with no + manifest every ``SKILL.md`` is read, and a broken one is skipped with a note). + 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_copy(eligible) + candidates: list[Candidate] = [] + for skill, digest in eligible: + if keeper[(skill.name, 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[(skill.name, 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_copy(eligible: list[tuple[Skill, str]]) -> dict[tuple[str, str], Skill]: + """The copy with the fewest path parts stands for each group of identical copies. + + A group is one name with one digest: two skills with identical text but different names + (``skills/a`` and ``skills/b`` without a frontmatter name) are two cards, not copies. + Ties go to the path that sorts first, so the choice does not depend on directory order. + """ + keeper: dict[tuple[str, str], Skill] = {} + for skill, digest in eligible: + group = (skill.name, digest) + best = keeper.get(group) + if best is None or (len(skill.path.parts), skill.path.as_posix()) < ( + len(best.path.parts), best.path.as_posix()): + keeper[group] = skill + return keeper def check_field(gold: Candidate, competitors: list[Candidate]) -> None: diff --git a/dokimi/scripts/growth_baseline.json b/dokimi/scripts/growth_baseline.json index 96f66b2b4..b0823eec7 100644 --- a/dokimi/scripts/growth_baseline.json +++ b/dokimi/scripts/growth_baseline.json @@ -1,6 +1,6 @@ { "files": { - "dokimi/commands/novelty.py": {"lines": 993, "rationale": "estimate-only projection (expected_usd, arm scaling, extrapolation flag) in novelty CLI, on top of main's W-series and #173 lines, plus W1 routing cost output (#164); file split tracked separately"}, + "dokimi/commands/novelty.py": {"lines": 1192, "rationale": "estimate-only projection and W1 routing cost output (#164) from main, plus the #172 routing field, --all-vs-all, tied/unscored caveat and blind-skill warning, --repeats refused under --all-vs-all; file split tracked separately"}, "dokimi/harnesses/benchflow.py": {"lines": 869}, "dokimi/harnesses/docker_solver.py": {"lines": 899, "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_cli.py b/dokimi/tests/test_novelty_cli.py index 8eb6e478e..62b7f1f5f 100644 --- a/dokimi/tests/test_novelty_cli.py +++ b/dokimi/tests/test_novelty_cli.py @@ -6,6 +6,7 @@ from __future__ import annotations import json +import os import re import pytest @@ -877,6 +878,34 @@ def test_a_command_given_a_frontmatter_nested_too_deeply_exits_with_a_message(tm assert "RecursionError" not in str(exc.value) +def test_all_vs_all_names_a_competitor_nested_too_deeply_and_exits_with_a_message(tmp_path): + queries = tmp_path / "q.json" + queries.write_text('{"deep": ["a request"]}', encoding="utf-8") + args = _parse("novelty", "routing", "--all-vs-all", "--competitors", _deep_skill(tmp_path), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) + with pytest.raises(SystemExit, match="maximum recursion") as exc: + args.func(args) + assert "RecursionError" not in str(exc.value) + + +@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="root reads any file") +def test_all_vs_all_names_an_unreadable_competitor_and_exits_with_a_message(tmp_path): + root = tmp_path / "locked" + root.mkdir() + skill = root / "SKILL.md" + skill.write_text("---\nname: locked\ndescription: x\n---\n\nBody.\n", encoding="utf-8") + skill.chmod(0) + queries = tmp_path / "q.json" + queries.write_text('{"locked": ["a request"]}', encoding="utf-8") + args = _parse("novelty", "routing", "--all-vs-all", "--competitors", f"skill:{root}", + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) + try: + with pytest.raises(SystemExit, match="cannot load the field"): + args.func(args) + finally: + skill.chmod(0o644) + + def test_item_analysis_accepts_a_probe_file_that_records_no_digest(tmp_path): from dokimi.novelty.items import load_probe_cells 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 000000000..2c3f5b2e6 --- /dev/null +++ b/dokimi/tests/test_novelty_routing_all_vs_all.py @@ -0,0 +1,742 @@ +# 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.all_vs_all import all_vs_all, load_queries_by_skill +from dokimi.novelty.routing import Candidate, RoutingInputError + +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 _recursion_on_load(monkeypatch): + """Make json.loads hit the depth limit: whether a deep file does depends on the Python.""" + def too_deep(*args, **kwargs): + raise RecursionError("maximum recursion depth exceeded") + monkeypatch.setattr(json, "loads", too_deep) + + +def test_a_queries_file_nested_too_deeply_is_refused_by_name(tmp_path, monkeypatch): + path = tmp_path / "q.json" + path.write_text('{"linux-perf": []}', encoding="utf-8") + _recursion_on_load(monkeypatch) + with pytest.raises(RoutingInputError, match="nested too deeply"): + load_queries_by_skill(str(path)) + + +def test_a_queries_file_nested_too_deeply_exits_with_a_message(tmp_path, monkeypatch): + path = tmp_path / "q.json" + path.write_text('{"linux-perf": []}', encoding="utf-8") + _recursion_on_load(monkeypatch) + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(path), "--out", str(tmp_path / "o.json")) + with pytest.raises(SystemExit, match="nested too deeply") as exc: + args.func(args) + assert "RecursionError" not in str(exc.value) + + +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}} + + +@pytest.mark.parametrize("order", [("a", "b"), ("b", "a")]) +def test_two_identical_cards_are_one_tied_group_and_no_attractor(order): + result = all_vs_all(_two_identical_cards(order), + {"a": ["format json"], "b": ["format json"]}) + assert result["tied_groups"] == [list(order)] + assert result["lexical"]["attractors"] == [] + + +def test_an_attractor_outside_the_tied_group_is_still_reported(): + field = [Candidate(name="a", description="format json output"), + Candidate(name="b", description="format json output"), + Candidate(name="c", description="profile binary")] + result = all_vs_all(field, {"a": ["profile"], "c": ["profile"]}) + assert result["tied_groups"] == [["a", "b"]] + assert result["lexical"]["attractors"] == [{ + "skill": "c", "wrongly_picked": 1, "sibling_queries": 1, "rate": 1.0, "from": ["a"], + }] + + +def test_a_tied_group_member_is_a_real_attractor_for_an_outside_query_only(): + # A query naming jsonfmt breaks the tie, so jsonfmt is picked for c's query: an attractor. + # jsonpretty's queries land on jsonfmt, its group-mate, so they are neither an attraction + # nor sibling queries: jsonfmt's sibling count is c's one query, not c's plus jsonpretty's two. + field = [Candidate(name="jsonfmt", description="format json output"), + Candidate(name="jsonpretty", description="format json output"), + Candidate(name="c", description="profile binary")] + result = all_vs_all(field, {"c": ["jsonfmt format"], + "jsonpretty": ["format json", "format json"]}) + assert result["lexical"]["attractors"] == [{ + "skill": "jsonfmt", "wrongly_picked": 1, "sibling_queries": 1, "rate": 1.0, + "from": ["c"], + }] + + +def test_two_empty_descriptions_are_not_a_tied_group(): + field = [Candidate(name="a", description=""), Candidate(name="b", description=" ")] + assert all_vs_all(field, {"a": ["alpha"]})["tied_groups"] == [] + + +def test_the_model_arm_does_not_count_a_pick_of_the_group_mate(): + 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 "jsonfmt" in line)}), None + + field = [Candidate(name="jsonfmt", description="format json output"), + Candidate(name="jsonpretty", description="format json output"), + Candidate(name="c", description="profile binary")] + client = LLMClient(model="m", role="selector", transport=transport) + model = all_vs_all(field, {"c": ["profile"], "jsonpretty": ["format json"]}, + client=client)["model"] + assert [(row["skill"], row["from"]) for row in model["attractors"]] == [("jsonfmt", ["c"])] + + +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.all_vs_all as all_vs_all_module + + seen = [] + real = all_vs_all_module.model_routing + + def spy(candidates, queries, **kwargs): + seen.append((kwargs["gold"], kwargs["seed"])) + return real(candidates, queries, **kwargs) + + monkeypatch.setattr(all_vs_all_module, "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_a_transport_that_raises_with_no_message_is_still_counted_as_failed(): + def transport(prompt: str, system: str | None): + raise RuntimeError() + + 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 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_all_vs_all_refuses_repeats_it_would_not_make(tmp_path, monkeypatch): + # all_vs_all makes one call per query, so --repeats 3 would be dropped without a word. + import dokimi.commands.novelty as novelty_cmd + + calls = [] + monkeypatch.setattr(novelty_cmd, "_client", lambda model, provider, role, args: LLMClient( + model=model, provider=provider, role=role, + transport=lambda prompt, system: calls.append(prompt) or (json.dumps({"pick": 1}), None))) + root = tmp_path / "catalog" + _cat(root, {"perf": "profile a slow binary", "db": "tune a slow database query"}) + queries = _write_queries(tmp_path, {"perf": ["profile my slow binary"]}) + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json"), + "--model", "m", "--repeats", "3") + with pytest.raises(SystemExit, match="takes no --repeats"): + args.func(args) + assert calls == [] and not (tmp_path / "o.json").exists() + + +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": []} + + +def test_console_says_how_many_tied_and_unscored_queries_are_not_counted_as_picks( + tmp_path, capsys): + # A tie and a query with no content word are the two cases where no pick is named. The + # summary must not read "no skill was picked" without saying what was left out beside it. + root = tmp_path / "catalog" + _catalog_skill(root, "fft", "fast transforms") + _catalog_skill(root, "linalg", "fast transforms") + _catalog_skill(root, "gold", "something unrelated entirely") + queries = _write_queries(tmp_path, {"gold": ["fast transforms", "the of and"]}) + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) + args.func(args) + stdout = capsys.readouterr().out + assert "lexical: no skill was picked for another skill's query" in stdout + assert "lexical: 1 tied and 1 unscored queries are not counted as picks" in stdout + + +def test_console_warns_when_the_lexical_arm_cannot_score_any_query_of_a_skill(tmp_path, capsys): + root = tmp_path / "catalog" + _catalog_skill(root, "fft", "fast transforms") + _catalog_skill(root, "gold", "something unrelated entirely") + queries = _write_queries(tmp_path, {"gold": ["the of and", "a an the"]}) + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) + args.func(args) + err = capsys.readouterr().err + assert "the lexical arm cannot score any query of gold" in err + assert "no content word" in err + + +def test_two_contentless_queries_of_one_skill_are_two_unscored_and_no_rate(): + field = [Candidate("alpha", "convert a json file to yaml"), + Candidate("beta", "tune a slow database query")] + lexical = all_vs_all(field, {"alpha": ["the of and", "a an the"]})["lexical"] + assert lexical["n_unscored"] == {"alpha": 2} + assert lexical["n_scored"] == {"alpha": 0} + assert lexical["own_rate"]["alpha"] is None + + +def _run_console(tmp_path, capsys, skills, queries_by_skill): + root = tmp_path / "catalog" + for name, description in skills.items(): + _catalog_skill(root, name, description) + queries = _write_queries(tmp_path, queries_by_skill) + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) + args.func(args) + return capsys.readouterr().out + + +def test_console_counts_unscored_queries_when_nothing_tied(tmp_path, capsys): + stdout = _run_console(tmp_path, capsys, + {"fft": "fast transforms", "gold": "something unrelated entirely"}, + {"gold": ["something unrelated", "the of and"]}) + assert "lexical: 0 tied and 1 unscored queries are not counted as picks" in stdout + + +def test_console_counts_tied_queries_when_nothing_is_unscored(tmp_path, capsys): + stdout = _run_console(tmp_path, capsys, + {"fft": "fast transforms", "linalg": "fast transforms", + "gold": "something unrelated entirely"}, + {"gold": ["fast transforms", "transforms fast"]}) + assert "lexical: 2 tied and 0 unscored queries are not counted as picks" in stdout + + +def test_console_names_a_tied_group_once_and_not_as_attractors(tmp_path, capsys): + stdout = _run_console(tmp_path, capsys, + {"fft": "fast transforms", "linalg": "fast transforms", + "gold": "something unrelated entirely"}, + {"fft": ["fast transforms"], "gold": ["something unrelated"]}) + assert ("tied: fft, linalg have the same description; one group, not attractors of each " + "other") in stdout + assert "sibling queries" not in stdout + + +def test_console_sums_unscored_queries_over_every_skill(tmp_path, capsys): + stdout = _run_console(tmp_path, capsys, + {"fft": "fast transforms", "gold": "something unrelated entirely"}, + {"gold": ["the of and", "a an the", "something unrelated"], + "fft": ["the and", "fast transforms"]}) + assert "lexical: 0 tied and 3 unscored queries are not counted as picks" in stdout + + +@pytest.mark.parametrize("order", [(0, 1), (1, 0)]) +def test_exactly_two_other_cards_tied_at_the_top_are_a_tie(order): + cards = [Candidate("fft", "fast transforms"), Candidate("linalg", "fast transforms")] + field = [cards[i] for i in order] + [Candidate("gold", "something unrelated entirely")] + lexical = all_vs_all(field, {"gold": ["fast transforms"]})["lexical"] + assert lexical["confusion"]["gold"] == {"(tie)": 1} + assert lexical["attractors"] == [] + + +def test_the_model_arm_does_not_send_a_query_with_no_content_word(): + prompts: list[str] = [] + + def transport(prompt: str, system: str | None): + prompts.append(prompt) + return '{"pick": 1}', None + + field = [Candidate("alpha", "convert a json file to yaml"), + Candidate("beta", "tune a slow database query")] + client = LLMClient(model="m", provider="p", role="selector", transport=transport) + result = all_vs_all(field, {"alpha": ["the of and", "convert json"], + "beta": ["a an the"]}, client=client) + model, lexical = result["model"], result["lexical"] + assert len(prompts) == 1 + assert model["n_scored"] == lexical["n_scored"] == {"alpha": 1, "beta": 0} + assert model["n_unscored"] == lexical["n_unscored"] == {"alpha": 1, "beta": 1} + assert model["own_rate"]["beta"] is None and model["n_failed"] == 0 + + +def test_a_failed_call_adds_to_the_contentless_queries_of_the_same_skill(): + def transport(prompt: str, system: str | None): + raise RuntimeError("boom") + + field = [Candidate("alpha", "convert a json file to yaml"), + Candidate("beta", "tune a slow database query")] + client = LLMClient(model="m", provider="p", role="selector", transport=transport) + model = all_vs_all(field, {"alpha": ["the of and", "convert json"]}, client=client)["model"] + assert model["n_unscored"] == {"alpha": 2} + assert model["n_scored"] == {"alpha": 0} + + +def test_the_failed_call_count_is_over_the_calls_sent_not_every_query( + tmp_path, monkeypatch, capsys): + 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") + _catalog_skill(root, "db", "tune a slow database query") + queries = _write_queries(tmp_path, {"db": ["add an index", "the of and"]}) + 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="^1 of 1 model call"): + args.func(args) + assert "makes 1 model calls" in capsys.readouterr().err diff --git a/dokimi/tests/test_novelty_routing_field.py b/dokimi/tests/test_novelty_routing_field.py new file mode 100644 index 000000000..2de05770d --- /dev/null +++ b/dokimi/tests/test_novelty_routing_field.py @@ -0,0 +1,288 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""The routing field: which catalog skills compete, and how disabled and copied skills are kept out.""" + +from __future__ import annotations + +import json + +import pytest + +from dokimi.cli import build_parser +from dokimi.novelty.routing import ( + RoutingInputError, + catalog_competitors, + disables_model_invocation, +) + + +def _parse(*argv): + return build_parser().parse_args(list(argv)) + + +def _skill(tmp_path, name, description, body="# B\n\n## A section here\n"): + root = tmp_path / name + root.mkdir() + (root / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: {description}\n---\n\n{body}", encoding="utf-8") + return f"skill:{root}" + + +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"catalog skill 'db' at {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_two_differently_named_skills_with_identical_text_both_stay_in_the_field(tmp_path): + # Without a frontmatter name the name is the directory's, so identical text under skills/a + # and skills/b is two skills, not a mirror: both are siblings the field must show. + text = "---\ndescription: tune a slow database query\n---\n\n# B\n" + for name in ("a", "b"): + (tmp_path / "skills" / name).mkdir(parents=True) + (tmp_path / "skills" / name / "SKILL.md").write_text(text, encoding="utf-8") + notes: list[str] = [] + names = [c.name for c in catalog_competitors(str(tmp_path), set(), notes)] + assert names == ["a", "b"] + assert not any("identical to" in note for note in notes) + + +def test_two_manifest_entries_with_identical_text_and_different_names_both_stay(tmp_path): + # The same rule for a skills.yaml that lists two skills pointing at identical bytes. + text = "---\ndescription: tune a slow database query\n---\n\n# B\n" + for name in ("a", "b"): + (tmp_path / "skills" / name).mkdir(parents=True) + (tmp_path / "skills" / name / "SKILL.md").write_text(text, encoding="utf-8") + (tmp_path / "skills.yaml").write_text("- name: a\n- name: b\n", encoding="utf-8") + notes: list[str] = [] + names = [c.name for c in catalog_competitors(str(tmp_path), set(), notes)] + assert names == ["a", "b"] + assert not any("identical to" in note for note in notes) + + +def test_two_same_name_copies_that_differ_only_late_in_the_body_are_refused(tmp_path): + # The copies share the frontmatter and the first 100+ bytes, so a digest of a prefix would + # call them mirrors; the whole file decides, and they clash. + head = "---\nname: db\ndescription: tune a slow database query\n---\n\n" + "x" * 120 + for branch, tail in (("a", "\nfirst ending\n"), ("b", "\nsecond ending\n")): + copy = tmp_path / branch / "skills" / "db" + copy.mkdir(parents=True) + (copy / "SKILL.md").write_text(head + tail, encoding="utf-8") + with pytest.raises(RoutingInputError, match="catalog has db at .* different content"): + catalog_competitors(str(tmp_path), set(), [])