From 63c4763fcded2b6315fef9c61a3ae5415eff2c2a Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 21:44:16 -0700 Subject: [PATCH 01/15] feat(novelty): --competitors-from-catalog, read through the catalog manifest --catalog already took the competitors from a skill directory; this adds the name the flag is asked for under, and makes the catalog reader the one catalog scan uses. A skills.yaml at the root now decides which skills are competitors. A declared skill that cannot be loaded is skipped with a warning instead of making the whole field unreadable. Signed-off-by: Nikolay Petrov --- ...30000_napetrov_routing-catalog-manifest.md | 7 +++++ dokimi/docs/skill-novelty-walkthrough.md | 6 ++-- dokimi/docs/skill-novelty.md | 6 ++-- dokimi/dokimi/commands/novelty.py | 11 ++++++-- dokimi/dokimi/novelty/routing.py | 23 +++++++++++---- dokimi/tests/test_novelty_routing.py | 28 +++++++++++++++++++ 6 files changed, 68 insertions(+), 13 deletions(-) create mode 100644 dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md diff --git a/dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md b/dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md new file mode 100644 index 00000000..86c152b9 --- /dev/null +++ b/dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md @@ -0,0 +1,7 @@ +### Added + +- **`novelty routing --competitors-from-catalog `** is the same flag as `--catalog`, so + the name a harness-facing script expects works. The catalog's `skills.yaml`, when present, + now decides which skills are competitors, as it does for `catalog scan`; without one, every + `SKILL.md` under the directory counts as before. A declared skill that cannot be loaded is + skipped with a warning instead of making the field unreadable. diff --git a/dokimi/docs/skill-novelty-walkthrough.md b/dokimi/docs/skill-novelty-walkthrough.md index 6497f0f0..9e830e59 100644 --- a/dokimi/docs/skill-novelty-walkthrough.md +++ b/dokimi/docs/skill-novelty-walkthrough.md @@ -39,9 +39,9 @@ python cli.py novelty routing --artifact skill:skills/my-skill \ --queries queries.json ``` -The competitors are the skills a harness lists next to this one: `--catalog ` takes -every other `SKILL.md` under the directory, 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 +The competitors are the skills a harness lists next to this one: `--catalog ` (or +`--competitors-from-catalog `) takes every other skill the catalog lists, 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 diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 5dac13bb..456e4410 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -590,8 +590,10 @@ python cli.py novelty routing --artifact skill:path/to/skill \ --queries queries.json [--model M] ``` -`--catalog ` takes the competitors from a skill directory instead: every `SKILL.md` -under it except the artifact itself. +`--catalog ` (also `--competitors-from-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. A +declared skill that cannot be loaded is skipped with a warning. Two numbers, reported side by side and never merged: diff --git a/dokimi/dokimi/commands/novelty.py b/dokimi/dokimi/commands/novelty.py index adbdc943..0f574896 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -286,8 +286,11 @@ def card(ref: str) -> tuple[Candidate, str]: gold, body = card(args.artifact) competitors = [card(ref)[0] for ref in args.competitors or []] if args.catalog: + notes: list[str] = [] catalog = catalog_competitors(args.catalog, - {gold.name, *(c.name for c in competitors)}) + {gold.name, *(c.name for c in competitors)}, notes) + for note in notes: + print(f"WARNING: {note}", file=sys.stderr) 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, " @@ -582,9 +585,11 @@ def register(sub, positive_int) -> 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, + routing_p.add_argument("--catalog", "--competitors-from-catalog", default=None, + dest="catalog", help="Directory of skills to take the competitors from: every " - "SKILL.md under it except the artifact itself, i.e. the " + "skill the catalog lists (skills.yaml when present, else every " + "SKILL.md under it) except the artifact itself, i.e. the " "siblings a harness lists next to it. Combines with " "--competitors") routing_p.add_argument("--queries", default=None, diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 36d13045..d94ce0d6 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -339,20 +339,33 @@ 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) -> 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. Raises: - ValueError: ``root`` is not a directory, or a skill under it cannot be loaded. + ValueError: ``root`` is not a directory, or a skill under it cannot be loaded (with + no manifest every ``SKILL.md`` is read, and one that is broken still raises). """ - from dokimi.skills import discover_skills + from dokimi.external.catalog import read_skill_manifest 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] + candidates: list[Candidate] = [] + for entry in read_skill_manifest(Path(root)): + 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 not in exclude: + candidates.append(Candidate(name=skill.name, description=skill.description)) + return candidates def check_field(gold: Candidate, competitors: list[Candidate]) -> None: diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index cd7ea078..1261be0e 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -473,3 +473,31 @@ def test_catalog_competitors_with_name_in_competitors_excludes_it_from_catalog(t assert names.count("perf") == 0, "perf should be excluded from catalog" assert set(names) == {"xpu", "cache"}, f"Expected xpu and cache, got {names}" + + +def _catalog_skill(root, name, description): + skill_dir = root / "skills" / name + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: {description}\n---\n\n# B\n", encoding="utf-8") + + +def test_the_catalog_is_the_skills_yaml_manifest_when_there_is_one(tmp_path): + # A manifest decides which skills the catalog has: a SKILL.md it does not list is not a + # sibling, and a listed skill with no directory is reported rather than aborting the run. + root = tmp_path / "catalog" + _catalog_skill(root, "xpu", "profile a slow GPU kernel") + _catalog_skill(root, "db", "tune a slow database query") + _catalog_skill(root, "unlisted", "profile a slow thing") + (root / "skills.yaml").write_text("- name: xpu\n- name: db\n- name: ghost\n", + encoding="utf-8") + notes: list[str] = [] + names = [c.name for c in catalog_competitors(str(root), {"perf"}, notes)] + assert names == ["xpu", "db"] + assert len(notes) == 1 and "ghost" in notes[0] + + +def test_competitors_from_catalog_is_the_same_flag_as_catalog(tmp_path): + args = _parse("novelty", "routing", "--artifact", _skill(tmp_path, "a", "d"), + "--competitors-from-catalog", str(tmp_path)) + assert args.catalog == str(tmp_path) From 7226ea450395d0904d32b553fe8e75ce4f8b6b07 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 21:46:08 -0700 Subject: [PATCH 02/15] fix(novelty): routing skips a non-SKILL.md competitor with a warning A shell glob over a skills directory passes every file beside the skills, and the first helper script (activation-rule.mjs) aborted the whole routing run. A skill: competitor that is not a SKILL.md, or a directory without one, is now left out of the field with a warning. The artifact under test is still refused, and a text: competitor is unchanged. Signed-off-by: Nikolay Petrov --- ..._131000_napetrov_routing-skip-non-skill.md | 7 ++++ dokimi/docs/skill-novelty.md | 2 ++ dokimi/dokimi/commands/novelty.py | 9 +++++- dokimi/dokimi/novelty/routing.py | 18 +++++++++++ dokimi/tests/test_novelty_routing.py | 32 +++++++++++++++++++ 5 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 dokimi/changelog.d/20261021_131000_napetrov_routing-skip-non-skill.md diff --git a/dokimi/changelog.d/20261021_131000_napetrov_routing-skip-non-skill.md b/dokimi/changelog.d/20261021_131000_napetrov_routing-skip-non-skill.md new file mode 100644 index 00000000..f96d23a2 --- /dev/null +++ b/dokimi/changelog.d/20261021_131000_napetrov_routing-skip-non-skill.md @@ -0,0 +1,7 @@ +### Changed + +- **`novelty routing` skips a non-`SKILL.md` competitor with a warning** — a + `skill:` competitor that names a file other than `SKILL.md`, or a directory without one, + is left out of the field and reported on stderr. It used to abort the whole run, so a + shell glob over a skills directory (which also matches helper scripts) could not be + passed. The artifact under test is still refused. diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 456e4410..813ed447 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -594,6 +594,8 @@ python cli.py novelty routing --artifact skill:path/to/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. A declared skill that cannot be loaded is skipped with a warning. +A `skill:` competitor that is not a `SKILL.md`, or a directory without one, is skipped with a +warning, so a shell glob over a skills directory can be passed as it is. Two numbers, reported side by side and never merged: diff --git a/dokimi/dokimi/commands/novelty.py b/dokimi/dokimi/commands/novelty.py index 0f574896..2079c9ec 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -274,6 +274,7 @@ def cmd_novelty_routing(args: argparse.Namespace) -> None: load_candidate, load_queries, model_routing, + non_skill_reason, ) def card(ref: str) -> tuple[Candidate, str]: @@ -284,7 +285,13 @@ def card(ref: str) -> tuple[Candidate, str]: try: gold, body = card(args.artifact) - competitors = [card(ref)[0] for ref in args.competitors or []] + competitors = [] + for ref in args.competitors or []: + reason = non_skill_reason(ref) + if reason: + print(f"WARNING: skipped competitor {ref}: {reason}", file=sys.stderr) + continue + competitors.append(card(ref)[0]) if args.catalog: notes: list[str] = [] catalog = catalog_competitors(args.catalog, diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index d94ce0d6..8d4d8e0a 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -319,6 +319,24 @@ 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 + + class RoutingInputError(ValueError): """A competitor field or query file in which selection is not a measurement.""" diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index 1261be0e..a21b1fc9 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -501,3 +501,35 @@ def test_competitors_from_catalog_is_the_same_flag_as_catalog(tmp_path): args = _parse("novelty", "routing", "--artifact", _skill(tmp_path, "a", "d"), "--competitors-from-catalog", str(tmp_path)) assert args.catalog == str(tmp_path) + + +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) From 0698e2b02284ceda1f627703c2e4f009a8cee7ec Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 21:48:13 -0700 Subject: [PATCH 03/15] feat(novelty): routing leaves disable-model-invocation skills out of the field No harness lists a skill that sets disable-model-invocation, so the model cannot select it, and a routing card for one measured a choice that does not exist. Such skills are now left out of a --catalog and of --competitors, with a warning on stderr. --include-disabled keeps them. A disabled artifact is still routed, with a warning. The flag is read with the truthy set repos.budget already uses. Signed-off-by: Nikolay Petrov --- ...00_napetrov_routing-model-hidden-skills.md | 8 +++ dokimi/docs/skill-novelty.md | 3 + dokimi/dokimi/commands/novelty.py | 24 ++++++- dokimi/dokimi/novelty/routing.py | 36 ++++++++-- dokimi/tests/test_novelty_routing.py | 67 +++++++++++++++++++ 5 files changed, 132 insertions(+), 6 deletions(-) create mode 100644 dokimi/changelog.d/20261021_132000_napetrov_routing-model-hidden-skills.md diff --git a/dokimi/changelog.d/20261021_132000_napetrov_routing-model-hidden-skills.md b/dokimi/changelog.d/20261021_132000_napetrov_routing-model-hidden-skills.md new file mode 100644 index 00000000..c7dec363 --- /dev/null +++ b/dokimi/changelog.d/20261021_132000_napetrov_routing-model-hidden-skills.md @@ -0,0 +1,8 @@ +### Changed + +- **`novelty routing` leaves `disable-model-invocation` skills out of the field** — no + harness lists such a skill, so the model cannot select it, and a card for it scored a + choice that does not exist (`ultracave` and `megacave` in the caveman repo). They are + left out of a `--catalog` and of `--competitors`, each with a warning on stderr, unless + `--include-disabled` is passed. A disabled artifact is still routed, with a warning that + its rate is not a harness rate. diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 813ed447..507ae65c 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -596,6 +596,9 @@ at the root decides what the catalog lists; without one, every `SKILL.md` under declared skill that cannot be loaded is skipped with a warning. A `skill:` competitor that is not a `SKILL.md`, or a directory without one, is skipped with a warning, so a shell glob over a skills directory can be passed as it is. +A skill that sets `disable-model-invocation` is left out of the field, both from a catalog +and as `--competitors`: no harness lists it, so the model cannot select it. Pass +`--include-disabled` to keep it. The same flag goes on a disabled artifact warning. Two numbers, reported side by side and never merged: diff --git a/dokimi/dokimi/commands/novelty.py b/dokimi/dokimi/commands/novelty.py index 2079c9ec..ecd49b27 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -273,8 +273,10 @@ def cmd_novelty_routing(args: argparse.Namespace) -> None: lexical_routing, load_candidate, load_queries, + disables_model_invocation, model_routing, non_skill_reason, + skill_file, ) def card(ref: str) -> tuple[Candidate, str]: @@ -285,17 +287,30 @@ def card(ref: str) -> tuple[Candidate, str]: try: gold, body = 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) competitors = [] for ref in args.competitors or []: reason = non_skill_reason(ref) if reason: print(f"WARNING: skipped competitor {ref}: {reason}", file=sys.stderr) continue - competitors.append(card(ref)[0]) + candidate = 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) + continue + competitors.append(candidate) if args.catalog: notes: list[str] = [] catalog = catalog_competitors(args.catalog, - {gold.name, *(c.name for c in competitors)}, notes) + {gold.name, *(c.name for c in competitors)}, notes, + include_disabled=args.include_disabled) for note in notes: print(f"WARNING: {note}", file=sys.stderr) no_description = [c.name for c in catalog if not c.description] @@ -599,6 +614,11 @@ def register(sub, positive_int) -> None: "SKILL.md under it) 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/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 8d4d8e0a..6a9a2bf8 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -39,6 +39,7 @@ from functools import partial from pathlib import Path +from dokimi.utils import parse_frontmatter from .client import LLMClient, parse_json_object @@ -337,6 +338,25 @@ def non_skill_reason(ref: str) -> str | None: 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.""" @@ -358,13 +378,15 @@ def load_candidate(ref: str) -> tuple[Candidate, str, str | None]: def catalog_competitors(root: str, exclude: set[str], - notes: list[str] | None = None) -> list[Candidate]: + 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. + ``notes`` (when given) rather than aborting the run. A skill with + ``disable-model-invocation`` is left out too, unless ``include_disabled``. Raises: ValueError: ``root`` is not a directory, or a skill under it cannot be loaded (with @@ -381,8 +403,14 @@ def catalog_competitors(root: str, exclude: set[str], if notes is not None: notes.append(f"skipped catalog entry {entry.name!r}: {entry.error}") continue - if skill.name not in exclude: - candidates.append(Candidate(name=skill.name, description=skill.description)) + 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 + candidates.append(Candidate(name=skill.name, description=skill.description)) return candidates diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index a21b1fc9..fac61f55 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -16,6 +16,7 @@ RoutingInputError, catalog_competitors, check_field, + disables_model_invocation, headings_as_queries, lexical_routing, load_queries, @@ -533,3 +534,69 @@ def test_a_non_skill_artifact_is_still_refused(tmp_path): "--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_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), +]) +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 From 763625cac81f29fd96adb70d511181830fcf3278 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 21:49:48 -0700 Subject: [PATCH 04/15] fix(novelty): routing counts a mirrored catalog skill once A plugin copy of a skills/ tree is a byte-identical SKILL.md under another path with the same name. The catalog read both, so the field named the skill twice and the run was refused. Catalog entries are now grouped by the SHA-256 of SKILL.md; the copy nearest the catalog root stands for the card, and the others are named in the warnings. Signed-off-by: Nikolay Petrov --- ...1_133000_napetrov_routing-mirror-dedupe.md | 6 ++++ dokimi/docs/skill-novelty.md | 3 ++ dokimi/dokimi/novelty/routing.py | 25 +++++++++++++-- dokimi/tests/test_novelty_routing.py | 31 +++++++++++++++++++ 4 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 dokimi/changelog.d/20261021_133000_napetrov_routing-mirror-dedupe.md diff --git a/dokimi/changelog.d/20261021_133000_napetrov_routing-mirror-dedupe.md b/dokimi/changelog.d/20261021_133000_napetrov_routing-mirror-dedupe.md new file mode 100644 index 00000000..7956763b --- /dev/null +++ b/dokimi/changelog.d/20261021_133000_napetrov_routing-mirror-dedupe.md @@ -0,0 +1,6 @@ +### Fixed + +- **`novelty routing --catalog` counts a mirrored skill once** — a `SKILL.md` that is + byte-identical under two paths (a plugin copy of the `skills/` tree) used to put the same + name in the field twice, and the run was refused as naming a competitor more than once. + The copy nearest the catalog root stands for the card; the others are named on stderr. diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 507ae65c..67b1f476 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -599,6 +599,9 @@ warning, so a shell glob over a skills directory can be passed as it is. A skill that sets `disable-model-invocation` is left out of the field, both from a catalog and as `--competitors`: no harness lists it, so the model cannot select it. Pass `--include-disabled` to keep it. The same flag goes on a disabled artifact warning. +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: diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 6a9a2bf8..0b8b23e3 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -34,11 +34,13 @@ import math import random import re +import hashlib from collections import Counter from dataclasses import dataclass from functools import partial from pathlib import Path +from dokimi.skills import Skill from dokimi.utils import parse_frontmatter from .client import LLMClient, parse_json_object @@ -386,7 +388,9 @@ def catalog_competitors(root: str, exclude: set[str], 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``. + ``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 (with @@ -396,7 +400,7 @@ def catalog_competitors(root: str, exclude: set[str], if not Path(root).is_dir(): raise ValueError(f"--catalog is not a directory: {root}") - candidates: list[Candidate] = [] + eligible: list[tuple[Skill, str]] = [] for entry in read_skill_manifest(Path(root)): skill = entry.skill if skill is None or not entry.loaded: @@ -410,6 +414,23 @@ def catalog_competitors(root: str, exclude: set[str], 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())) + + # The copy with the fewest path parts stands for each byte-identical group; ties go to + # the path that sorts first, so the choice does not depend on directory order. + keeper: dict[str, Skill] = {} + for skill, digest in eligible: + best = keeper.get(digest) + if best is None or (len(skill.path.parts), skill.path.as_posix()) < ( + len(best.path.parts), best.path.as_posix()): + keeper[digest] = skill + candidates: list[Candidate] = [] + for skill, digest in eligible: + if keeper[digest] is not skill: + if notes is not None: + notes.append(f"left out catalog skill {skill.name!r} at {skill.path}: " + f"identical to {keeper[digest].path}") + continue candidates.append(Candidate(name=skill.name, description=skill.description)) return candidates diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index fac61f55..1c92211b 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -600,3 +600,34 @@ def test_the_disable_flag_is_read_the_way_the_repo_budget_reads_it(tmp_path, lin 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] From 135470958fbe43e2acc417f55899a5de07171974 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 21:57:38 -0700 Subject: [PATCH 05/15] feat(novelty): routing --all-vs-all, a confusion matrix and attractor table Every skill with queries becomes the gold in turn and its queries are routed against the whole field. The output names the skills picked for other skills' queries (attractors), with how many of those queries each took and from which skills. Lexical arm always; the model arm when --model is given. Takes --queries-by-skill and --out, and no --artifact. The --artifact flag is now optional for a single run. Signed-off-by: Nikolay Petrov --- ...1021_134000_napetrov_routing-all-vs-all.md | 7 + dokimi/docs/skill-novelty.md | 13 +- dokimi/dokimi/commands/novelty.py | 160 +++++++++++++----- dokimi/dokimi/novelty/routing.py | 123 ++++++++++++++ dokimi/tests/test_novelty_routing.py | 118 +++++++++++++ 5 files changed, 378 insertions(+), 43 deletions(-) create mode 100644 dokimi/changelog.d/20261021_134000_napetrov_routing-all-vs-all.md diff --git a/dokimi/changelog.d/20261021_134000_napetrov_routing-all-vs-all.md b/dokimi/changelog.d/20261021_134000_napetrov_routing-all-vs-all.md new file mode 100644 index 00000000..29843570 --- /dev/null +++ b/dokimi/changelog.d/20261021_134000_napetrov_routing-all-vs-all.md @@ -0,0 +1,7 @@ +### Added + +- **`novelty routing --all-vs-all`** routes every skill that has queries against the whole + field, in turn, and reports a confusion matrix plus an attractor table: the skills picked + for other skills' queries, with how many of those queries they took and from whom. Takes + `--queries-by-skill ` and `--out`, and no `--artifact`. The lexical arm always runs; + `--model` adds the model arm. diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 67b1f476..fbe1f689 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -622,7 +622,18 @@ and the figure would be a structural 1.0. The lexical arm also reports `n_querie next to `n_queries_scored`, since a query with no content words is dropped rather than counted as a miss. -Three properties worth knowing before quoting either: +**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. The output has a confusion matrix (gold to picked to count, with `(none)` for +a query no card shares a word with, or an abstention or failed call in the model arm), 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`). It takes no +`--artifact`. 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. + +Three properties worth knowing before quoting either: before quoting either: - **Competitors are mandatory.** The existing `skill-agent:` arm offers one skill, which makes selection a binary did-it-load question. The real failure is picking the wrong diff --git a/dokimi/dokimi/commands/novelty.py b/dokimi/dokimi/commands/novelty.py index ecd49b27..175b5307 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -262,64 +262,131 @@ 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]) -> list: + """The field ``--competitors`` and ``--catalog`` name, without the names in ``exclude``. + + Raises: + FileNotFoundError, ValueError: A reference or the catalog cannot be loaded. + """ + from dokimi.novelty.routing import ( + catalog_competitors, + disables_model_invocation, + non_skill_reason, + skill_file, + ) + + competitors = [] + for ref in args.competitors or []: + reason = non_skill_reason(ref) + if reason: + print(f"WARNING: skipped competitor {ref}: {reason}", file=sys.stderr) + 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) + 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) + no_description = [c.name for c in catalog if not c.description] + if no_description: + print(f"WARNING: {len(no_description)} catalog skill(s) have no description, " + f"so their card is a name only: {', '.join(sorted(no_description))}", + file=sys.stderr) + print(f"--catalog {args.catalog}: {len(catalog)} competitors", file=sys.stderr) + competitors += catalog + return competitors + + +def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None: + """Every skill in the field is the gold in turn: confusion matrix and attractors.""" + from dokimi.novelty.routing import RoutingInputError, all_vs_all, load_queries_by_skill + + if args.artifact: + raise SystemExit("--all-vs-all takes no --artifact: every skill in the field is the " + "gold in turn") + 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 ") + try: + field = _routing_field(args, set()) + except (FileNotFoundError, ValueError) as exc: + raise SystemExit(f"cannot load the field: {exc}") from None + try: + queries_by_skill = load_queries_by_skill(args.queries_by_skill) + selector = _client(args.model, args.provider, "selector", args) if args.model else None + 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) + + 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} + out.write_text(json.dumps(payload, indent=2), encoding="utf-8") + 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") + for arm in ("lexical", "model"): + if arm not in result: + continue + rows = result[arm]["attractors"][:5] + if not rows: + print(f"{arm}: no skill was picked for another skill's query") + for row in rows: + print(f"{arm}: {row['skill']} picked for {row['wrongly_picked']} of " + f"{row['sibling_queries']} sibling queries (from {', '.join(row['from'])})") + + def cmd_novelty_routing(args: argparse.Namespace) -> None: """Can this artifact be selected against its competitors? Lexical floor, model on top.""" from dokimi.novelty.routing import ( - Candidate, RoutingInputError, - catalog_competitors, check_field, + disables_model_invocation, headings_as_queries, lexical_routing, - load_candidate, load_queries, - disables_model_invocation, model_routing, - non_skill_reason, 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) + 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) - competitors = [] - for ref in args.competitors or []: - reason = non_skill_reason(ref) - if reason: - print(f"WARNING: skipped competitor {ref}: {reason}", file=sys.stderr) - continue - candidate = 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) - continue - competitors.append(candidate) - if args.catalog: - notes: list[str] = [] - catalog = catalog_competitors(args.catalog, - {gold.name, *(c.name for c in competitors)}, notes, - include_disabled=args.include_disabled) - for note in notes: - print(f"WARNING: {note}", file=sys.stderr) - 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 + competitors = _routing_field(args, {gold.name}) except (FileNotFoundError, ValueError) as exc: raise SystemExit(f"cannot load an artifact: {exc}") from None # Checked before the query file is read, so the message is about the real problem. @@ -601,8 +668,17 @@ 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 " diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 0b8b23e3..4de0554a 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -487,3 +487,126 @@ def load_queries(path: str) -> list[str]: raise RoutingInputError(f"{path}: queries must be a list of strings, " "or one query per line") return [q for q in listed if q.strip()] + + +NO_PICK = "(none)" + + +def load_queries_by_skill(path: str) -> dict[str, list[str]]: + """Queries per skill, from a JSON object ``{"": ["request", ...]}``. + + Each request is one the named skill should be selected for, written independently of + it. Blank requests are dropped, and a skill left with none is dropped with them. + + Raises: + RoutingInputError: The file is not JSON, is not an object, or a value is not a list + of strings. + """ + try: + data = json.loads(Path(path).read_text(encoding="utf-8-sig")) + except json.JSONDecodeError as exc: + raise RoutingInputError(f"{path}: not JSON: {exc}") from None + if not isinstance(data, dict): + raise RoutingInputError(f"{path}: expected an object of skill name to query list") + by_skill: dict[str, list[str]] = {} + for name, listed in data.items(): + if not isinstance(listed, list) or not all(isinstance(q, str) for q in listed): + raise RoutingInputError(f"{path}: the queries for {name!r} must be a list of strings") + queries = [q for q in listed if q.strip()] + if queries: + by_skill[name] = queries + return by_skill + + +def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str]], *, + client: LLMClient | None = None, seed: int = 0) -> dict: + """Each skill's queries, routed against the whole field: a confusion matrix and attractors. + + Every skill that has queries is the gold in turn. Its queries go to the whole field, and + the pick is recorded. The lexical arm always runs (free); the model arm runs when + ``client`` is given. A query no card shares a word with is ``(none)``, not the first card: + the tie-break in ``lexical_routing`` would otherwise name a card that matched nothing. + + An attractor is a skill picked for queries written for other skills. ``wrongly_picked`` + counts those queries, ``sibling_queries`` is how many queries the other skills have, and + ``from`` names the skills whose queries it took. + + Returns: + ``n_skills``, ``n_queries``, and per arm (``lexical``, ``model``): ``confusion`` (gold + skill to picked skill to count, ``(none)`` included), ``own_rate`` per gold skill, and + ``attractors`` ordered by ``wrongly_picked``. The model arm also records ``seed``. + + 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] + if len(candidates) < 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)) + if not queries_by_skill: + raise RoutingInputError("no skill in the field has queries to route") + + bm25 = BM25([tokenize(c.card) for c in candidates]) + lexical: dict[str, Counter] = {} + for name, queries in queries_by_skill.items(): + picks: Counter = Counter() + for query in queries: + tokens = tokenize(query) + order = bm25.rank(tokens) if tokens else [] + best = order[0] if order else None + if best is None or bm25.score(tokens, best) <= 0: + picks[NO_PICK] += 1 + else: + picks[candidates[best].name] += 1 + lexical[name] = picks + result: dict = { + "n_skills": len(candidates), + "n_queries": sum(len(q) for q in queries_by_skill.values()), + "lexical": _attractor_table(lexical, queries_by_skill, names), + } + if client is not None: + model: dict[str, Counter] = {} + for index, name in enumerate(names): + if name not in queries_by_skill: + continue + rows = model_routing(candidates, queries_by_skill[name], client=client, gold=index, + seed=seed + index)["per_query"] + model[name] = Counter(row.get("picked") or NO_PICK for row in rows) + result["model"] = dict(_attractor_table(model, queries_by_skill, names), seed=seed) + return result + + +def _attractor_table(picks_by_gold: dict[str, Counter], queries_by_skill: dict[str, list[str]], + names: list[str]) -> dict: + """The confusion matrix, own rate and attractor table for one arm of :func:`all_vs_all`.""" + attractors: list[dict] = [] + for name in names: + wrong = {gold: picks[name] for gold, picks in picks_by_gold.items() + if gold != name and picks[name]} + if not wrong: + continue + sibling_queries = sum(len(queries_by_skill[gold]) for gold in picks_by_gold + if gold != name) + attractors.append({ + "skill": name, + "wrongly_picked": sum(wrong.values()), + "sibling_queries": sibling_queries, + "rate": round(sum(wrong.values()) / sibling_queries, 4) if sibling_queries else None, + "from": sorted(wrong, key=lambda gold: (-wrong[gold], gold)), + }) + attractors.sort(key=lambda row: (-row["wrongly_picked"], row["skill"])) + return { + "confusion": {gold: dict(sorted(picks.items())) + for gold, picks in sorted(picks_by_gold.items())}, + "own_rate": {gold: round(picks[gold] / len(queries_by_skill[gold]), 4) + for gold, picks in sorted(picks_by_gold.items())}, + "attractors": attractors, + "note": (f"{NO_PICK} counts a query no card shares a word with; in the model arm it " + "also counts an abstention and a failed call."), + } diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index 1c92211b..68c8bc85 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -14,12 +14,14 @@ from dokimi.novelty.routing import ( Candidate, RoutingInputError, + all_vs_all, catalog_competitors, check_field, disables_model_invocation, headings_as_queries, lexical_routing, load_queries, + load_queries_by_skill, model_routing, ) @@ -631,3 +633,119 @@ def test_the_mirror_kept_is_the_copy_nearest_the_catalog_root(tmp_path): 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] + + +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("field, queries, message", [ + ([PERF], {"linux-perf": ["q"]}, "at least two skills"), + ([PERF, PERF], {"linux-perf": ["q"]}, "more than once"), + ([PERF, TBB], {"mkl-advisor": ["q"]}, "not in the field: `mkl-advisor`"), + ([PERF, TBB], {}, "no skill in the field has queries"), +]) +def test_all_vs_all_refuses_a_field_where_it_would_not_measure_selection(field, queries, + message): + with pytest.raises(RoutingInputError, match=message): + all_vs_all(field, queries) + + +def test_a_queries_file_names_skills_by_object_and_drops_blank_requests(tmp_path): + path = _write_queries(tmp_path, {"linux-perf": [" ", "profile it"], "db": [" "]}) + assert load_queries_by_skill(str(path)) == {"linux-perf": ["profile it"]} + + +@pytest.mark.parametrize("payload", ['["a list"]', '{"linux-perf": "one string"}', + '{"linux-perf": [1]}']) +def test_a_queries_file_that_is_not_an_object_of_string_lists_is_refused(tmp_path, payload): + path = tmp_path / "q.json" + path.write_text(payload, encoding="utf-8") + with pytest.raises(RoutingInputError): + load_queries_by_skill(str(path)) + + +def test_all_vs_all_writes_the_matrix_and_prints_the_attractors(tmp_path, capsys): + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + _catalog_skill(root, "db", "tune a slow database query with an index") + _catalog_skill(root, "hub", "general advice for slow programs and slow queries") + queries = _write_queries(tmp_path, { + "perf": ["my slow program needs advice", "profile a slow program"], + "db": ["my database query is slow", "add an index to speed up a query"], + }) + out = tmp_path / "all_vs_all.json" + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(out)) + args.func(args) + payload = json.loads(out.read_text(encoding="utf-8")) + assert payload["mode"] == "all_vs_all" and payload["n_candidates"] == 3 + assert payload["result"]["lexical"]["confusion"]["db"]["db"] == 2 + assert payload["result"]["lexical"]["confusion"]["perf"] == {"perf": 1, "hub": 1} + stdout = capsys.readouterr().out + assert "all-vs-all: 3 skills in the field, 4 queries" in stdout + assert "lexical: hub picked for 1 of 4 sibling queries (from perf)" in stdout + + +def test_all_vs_all_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) From 3ab7f1b868d75aeec915e6f4d2f63debcc2d2808 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 22:11:42 -0700 Subject: [PATCH 06/15] fix(novelty): routing --catalog names a same-name clash; all-vs-all --model counts its calls --competitors-from-catalog was an alias of --catalog, which is already on main. Drop the alias. The changelog and docs now describe the --catalog change itself: a skills.yaml at the catalog root decides which skills are competitors. Two catalog skills with one name and different content are refused, naming both paths, instead of falling through to check_field's "pass each competitor once". With --model, all-vs-all prints the number of model calls (one per query) to stderr before it makes them, and the docs note the cost. Signed-off-by: Nikolay Petrov --- ...30000_napetrov_routing-catalog-manifest.md | 14 ++- dokimi/docs/skill-novelty-walkthrough.md | 8 +- dokimi/docs/skill-novelty.md | 19 ++- dokimi/dokimi/commands/novelty.py | 15 ++- dokimi/dokimi/novelty/routing.py | 17 +++ dokimi/tests/test_novelty_routing.py | 108 +++++++++++++++++- 6 files changed, 159 insertions(+), 22 deletions(-) diff --git a/dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md b/dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md index 86c152b9..53e0e44c 100644 --- a/dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md +++ b/dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md @@ -1,7 +1,9 @@ -### Added +### Changed -- **`novelty routing --competitors-from-catalog `** is the same flag as `--catalog`, so - the name a harness-facing script expects works. The catalog's `skills.yaml`, when present, - now decides which skills are competitors, as it does for `catalog scan`; without one, every - `SKILL.md` under the directory counts as before. A declared skill that cannot be loaded is - skipped with a warning instead of making the field unreadable. +- **`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/docs/skill-novelty-walkthrough.md b/dokimi/docs/skill-novelty-walkthrough.md index 9e830e59..e77c6c84 100644 --- a/dokimi/docs/skill-novelty-walkthrough.md +++ b/dokimi/docs/skill-novelty-walkthrough.md @@ -39,10 +39,10 @@ python cli.py novelty routing --artifact skill:skills/my-skill \ --queries queries.json ``` -The competitors are the skills a harness lists next to this one: `--catalog ` (or -`--competitors-from-catalog `) takes every other skill the catalog lists, 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. +The competitors are the skills a harness lists next to this one: `--catalog ` takes +every other skill the catalog lists, 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 fbe1f689..8b52f3db 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -590,10 +590,11 @@ python cli.py novelty routing --artifact skill:path/to/skill \ --queries queries.json [--model M] ``` -`--catalog ` (also `--competitors-from-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. A -declared skill that cannot be loaded is skipped with a warning. +`--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. A declared skill that cannot be loaded is skipped +with a warning. Two catalog skills with the same name and different content are refused, with +both paths named: rename one, or delete the copy you do not mean to route against. A `skill:` competitor that is not a `SKILL.md`, or a directory without one, is skipped with a warning, so a shell glob over a skills directory can be passed as it is. A skill that sets `disable-model-invocation` is left out of the field, both from a catalog @@ -633,6 +634,16 @@ own rate per skill, and an attractor table: a skill picked for other skills' que `--artifact`. It is refused with fewer than two skills, a name repeated in the field, a query file that names a skill outside the field, or no queries at all. +**Cost of `--model` in all-vs-all.** The model arm makes one selector call per query, across +every skill in the field: the call count is the total number of requests in `queries.json`, and +a failed call still counts. Each call sends the whole field's cards, so a large field costs +more per call as well as in calls. Before the first call the run prints that count to stderr +(`all-vs-all: --model makes N model calls, one per query, ...`). The run does not pause for it +and routing has no `--max-usd` cap, so count the queries first and stop the run (Ctrl-C) if the +number is more than you meant to spend. 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: 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 175b5307..a7c3b000 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -330,11 +330,21 @@ def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None: raise SystemExit("--all-vs-all needs --out ") try: field = _routing_field(args, set()) + except RoutingInputError as exc: + raise SystemExit(str(exc)) from None except (FileNotFoundError, ValueError) as exc: raise SystemExit(f"cannot load the field: {exc}") from None try: queries_by_skill = load_queries_by_skill(args.queries_by_skill) selector = _client(args.model, args.provider, "selector", args) if args.model else None + if selector is not None: + # One selector call per query, each sending every card in the field. + calls = sum(len(queries) for queries in queries_by_skill.values()) + print( + f"all-vs-all: --model makes {calls} model calls, one per query, over " + f"{len(field)} cards each", + file=sys.stderr, + ) result = all_vs_all(field, queries_by_skill, client=selector, seed=args.seed) except RoutingInputError as exc: raise SystemExit(str(exc)) from None @@ -387,6 +397,8 @@ def cmd_novelty_routing(args: argparse.Namespace) -> None: "it and the model cannot select it; its routing rate is not a harness rate", file=sys.stderr) competitors = _routing_field(args, {gold.name}) + except RoutingInputError as exc: + raise SystemExit(str(exc)) from None except (FileNotFoundError, ValueError) as exc: raise SystemExit(f"cannot load an artifact: {exc}") from None # Checked before the query file is read, so the message is about the real problem. @@ -683,8 +695,7 @@ def register(sub, positive_int) -> 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", "--competitors-from-catalog", default=None, - dest="catalog", + routing_p.add_argument("--catalog", default=None, help="Directory of skills to take the competitors from: every " "skill the catalog lists (skills.yaml when present, else every " "SKILL.md under it) except the artifact itself, i.e. the " diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 4de0554a..3e01bf78 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -395,6 +395,8 @@ def catalog_competitors(root: str, exclude: set[str], Raises: ValueError: ``root`` is not a directory, or a skill under it cannot be loaded (with no manifest every ``SKILL.md`` is read, and one that is broken still raises). + RoutingInputError: Two skills in the field share a name with different content; the + message names both paths. """ from dokimi.external.catalog import read_skill_manifest @@ -416,6 +418,21 @@ def catalog_competitors(root: str, exclude: set[str], continue eligible.append((skill, hashlib.sha256(skill.path.read_bytes()).hexdigest())) + # Two skills with one name and different bytes cannot both stand for the card, and + # 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" + ) + # The copy with the fewest path parts stands for each byte-identical group; ties go to # the path that sorts first, so the choice does not depend on directory order. keeper: dict[str, Skill] = {} diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index 68c8bc85..be8600ae 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -500,12 +500,6 @@ def test_the_catalog_is_the_skills_yaml_manifest_when_there_is_one(tmp_path): assert len(notes) == 1 and "ghost" in notes[0] -def test_competitors_from_catalog_is_the_same_flag_as_catalog(tmp_path): - args = _parse("novelty", "routing", "--artifact", _skill(tmp_path, "a", "d"), - "--competitors-from-catalog", str(tmp_path)) - assert args.catalog == str(tmp_path) - - 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. @@ -635,6 +629,43 @@ def test_the_mirror_kept_is_the_copy_nearest_the_catalog_root(tmp_path): assert "plugins" in notes[0] and "identical to" 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 _write_queries(tmp_path, by_skill): path = tmp_path / "queries_by_skill.json" path.write_text(json.dumps(by_skill), encoding="utf-8") @@ -737,6 +768,71 @@ def test_all_vs_all_writes_the_matrix_and_prints_the_attractors(tmp_path, capsys 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", From 70b7a72547fb1e67dfc6e49c06c2cb56ec24fc74 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Thu, 22 Oct 2026 12:26:32 +0000 Subject: [PATCH 07/15] docs(novelty): fix two garbled sentences in the routing section Signed-off-by: Anton Volkov --- dokimi/docs/skill-novelty.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 8b52f3db..e4720f9e 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -599,7 +599,7 @@ A `skill:` competitor that is not a `SKILL.md`, or a directory without one, is s warning, so a shell glob over a skills directory can be passed as it is. A skill that sets `disable-model-invocation` is left out of the field, both from a catalog and as `--competitors`: no harness lists it, so the model cannot select it. Pass -`--include-disabled` to keep it. The same flag goes on a disabled artifact warning. +`--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. @@ -644,7 +644,7 @@ number is more than you meant to spend. A lexical-only run (no `--model`) makes 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: before quoting either: +Three properties worth knowing before quoting either: - **Competitors are mandatory.** The existing `skill-agent:` arm offers one skill, which makes selection a binary did-it-load question. The real failure is picking the wrong From 0c98097c61bbc321e8962d6924a6baa28e501e7e Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Thu, 22 Oct 2026 08:06:38 -0700 Subject: [PATCH 08/15] fix(novelty): all-vs-all ties, failed model calls, unreadable queries file, max-usd doc Rank with the gold last so a lexical tie is no win; count n_failed and n_abstained in the model arm, warn on failures and exit non-zero when every call failed; catch OSError/UnicodeDecodeError on --queries-by-skill; say routing has --max-usd; add tests for the surviving mutants. Signed-off-by: Nikolay Petrov --- dokimi/docs/skill-novelty.md | 5 +- dokimi/dokimi/commands/novelty.py | 15 ++- dokimi/dokimi/novelty/routing.py | 21 +++- dokimi/tests/test_novelty_routing.py | 155 +++++++++++++++++++++++++++ 4 files changed, 187 insertions(+), 9 deletions(-) diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index e4720f9e..54a302dc 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -638,9 +638,8 @@ file that names a skill outside the field, or no queries at all. every skill in the field: the call count is the total number of requests in `queries.json`, and a failed call still counts. Each call sends the whole field's cards, so a large field costs more per call as well as in calls. Before the first call the run prints that count to stderr -(`all-vs-all: --model makes N model calls, one per query, ...`). The run does not pause for it -and routing has no `--max-usd` cap, so count the queries first and stop the run (Ctrl-C) if the -number is more than you meant to spend. A lexical-only run (no `--model`) makes no model calls. +(`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. diff --git a/dokimi/dokimi/commands/novelty.py b/dokimi/dokimi/commands/novelty.py index a7c3b000..5c722ba9 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -335,7 +335,10 @@ def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None: except (FileNotFoundError, ValueError) as exc: raise SystemExit(f"cannot load the field: {exc}") from None try: - queries_by_skill = load_queries_by_skill(args.queries_by_skill) + try: + queries_by_skill = load_queries_by_skill(args.queries_by_skill) + except (OSError, UnicodeDecodeError) as exc: + raise SystemExit(f"cannot read the queries: {exc}") from None selector = _client(args.model, args.provider, "selector", args) if args.model else None if selector is not None: # One selector call per query, each sending every card in the field. @@ -353,11 +356,21 @@ def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None: print(f"WARNING: {len(without)} skill(s) in the field have no queries, so they only " f"compete: {', '.join(sorted(without))}", file=sys.stderr) + model_arm = result.get("model") + failed = model_arm["n_failed"] if model_arm else 0 + if failed: + print(f"WARNING: {failed} model call(s) failed (first error: " + f"{model_arm['first_error']}); a failed call is counted as (none) in the model " + "arm, so its rates are lower bounds", 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} out.write_text(json.dumps(payload, indent=2), encoding="utf-8") + if model_arm and failed and failed >= result["n_queries"]: + raise SystemExit(f"every model call failed ({model_arm['first_error']}); the partial " + f"result 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") diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 3e01bf78..654845ae 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -551,7 +551,7 @@ def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str Returns: ``n_skills``, ``n_queries``, and per arm (``lexical``, ``model``): ``confusion`` (gold skill to picked skill to count, ``(none)`` included), ``own_rate`` per gold skill, and - ``attractors`` ordered by ``wrongly_picked``. The model arm also records ``seed``. + ``attractors`` ordered by ``wrongly_picked``. The model arm also records ``seed``, ``n_failed``, ``n_abstained`` and ``first_error``. Raises: RoutingInputError: Fewer than two skills in the field; a name repeated in the field; a @@ -575,7 +575,9 @@ def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str picks: Counter = Counter() for query in queries: tokens = tokenize(query) - order = bm25.rank(tokens) if tokens else [] + # The gold sorts after every card it ties with, as in lexical_routing: a tie is + # never a win for the card that happens to come first in the field. + order = bm25.rank(tokens, last=names.index(name)) if tokens else [] best = order[0] if order else None if best is None or bm25.score(tokens, best) <= 0: picks[NO_PICK] += 1 @@ -589,13 +591,22 @@ def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str } if client is not None: model: dict[str, Counter] = {} + n_failed = n_abstained = 0 + first_error = None for index, name in enumerate(names): if name not in queries_by_skill: continue - rows = model_routing(candidates, queries_by_skill[name], client=client, gold=index, - seed=seed + index)["per_query"] + routed = model_routing(candidates, queries_by_skill[name], client=client, + gold=index, seed=seed + index) + rows = routed["per_query"] + n_failed += routed["n_failed"] + n_abstained += routed["n_abstained"] + if first_error is None: + first_error = next((r["error"] for r in rows if r.get("error")), None) model[name] = Counter(row.get("picked") or NO_PICK for row in rows) - result["model"] = dict(_attractor_table(model, queries_by_skill, names), seed=seed) + result["model"] = dict(_attractor_table(model, queries_by_skill, names), seed=seed, + n_failed=n_failed, n_abstained=n_abstained, + first_error=first_error) return result diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index be8600ae..b1ef9d5d 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -577,6 +577,19 @@ def test_an_explicit_disabled_competitor_is_skipped_with_a_warning(tmp_path, cap 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") @@ -591,6 +604,10 @@ def test_a_disabled_artifact_is_still_routed_but_warned_about(tmp_path, capsys): @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" @@ -627,6 +644,22 @@ def test_the_mirror_kept_is_the_copy_nearest_the_catalog_root(tmp_path): 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"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( @@ -845,3 +878,125 @@ def test_a_single_routing_run_still_needs_an_artifact(tmp_path): args = _parse("novelty", "routing", "--out", str(tmp_path / "o.json")) with pytest.raises(SystemExit, match="--all-vs-all for the whole field"): args.func(args) + + +def _two_identical_cards(order): + cards = {"a": Candidate(name="a", description="format json output"), + "b": Candidate(name="b", description="format json output")} + return [cards[n] for n in order] + + +@pytest.mark.parametrize("order", [("a", "b"), ("b", "a")]) +def test_two_identical_cards_tie_and_neither_is_scored_a_win(order): + result = all_vs_all(_two_identical_cards(order), + {"a": ["format json"], "b": ["format json"]})["lexical"] + assert result["own_rate"] == {"a": 0.0, "b": 0.0} + assert result["confusion"] == {"a": {"b": 1}, "b": {"a": 1}} + + +def test_a_three_way_tie_is_not_a_perfect_pick(): + field = [Candidate(name=f"s{i}", description="common words") for i in range(3)] + lexical = all_vs_all(field, {"s0": ["common"]})["lexical"] + assert lexical["own_rate"] == {"s0": 0.0} + + +def test_attractors_are_ranked_by_count_and_rated_over_the_other_skills_queries(): + # hub takes 2 of perf's 2 queries and 1 of db's 3; sink takes 1 of perf's. + field = [Candidate(name="perf", description="profile binary"), + Candidate(name="db", description="index query"), + Candidate(name="hub", description="slow advice"), + Candidate(name="sink", description="gpu offload")] + queries = {"perf": ["slow one", "slow two", "gpu thing"], + "db": ["slow query", "index query", "index query again"]} + lexical = all_vs_all(field, queries)["lexical"] + hub, sink = lexical["attractors"][:2] + assert hub == {"skill": "hub", "wrongly_picked": 3, "sibling_queries": 6, "rate": 0.5, + "from": ["perf", "db"]} + assert sink["skill"] == "sink" and sink["wrongly_picked"] == 1 + # hub's own queries are not sibling queries of hub: only the other golds count. + assert all(row["skill"] != "perf" or row["sibling_queries"] == 3 + for row in lexical["attractors"]) + + +def _positional_client(pick_for): + def transport(prompt: str, system: str | None): + picked = pick_for(prompt) + return json.dumps({"pick": picked}), None + return LLMClient(model="m", role="selector", transport=transport) + + +def test_a_selector_that_abstains_is_none_and_counted_in_the_model_arm(): + model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=_positional_client(lambda p: 0))["model"] + assert model["confusion"] == {"linux-perf": {"(none)": 2}, "mkl-advisor": {"(none)": 2}} + assert model["n_abstained"] == 4 and model["n_failed"] == 0 + assert model["attractors"] == [] + + +def test_the_model_arm_seeds_each_gold_apart_and_hands_model_routing_the_gold(monkeypatch): + import dokimi.novelty.routing as routing + + seen = [] + real = routing.model_routing + + def spy(candidates, queries, **kwargs): + seen.append((kwargs["gold"], kwargs["seed"])) + return real(candidates, queries, **kwargs) + + monkeypatch.setattr(routing, "model_routing", spy) + all_vs_all(ALL_FIELD, ALL_QUERIES, client=_positional_client(lambda p: 1), seed=10) + assert seen == [(0, 10), (2, 12)] + + +def test_the_model_arm_follows_each_call_not_one_fixed_card(): + calls = iter([1, 2, 3, 1]) + model = all_vs_all(ALL_FIELD, ALL_QUERIES, + client=_positional_client(lambda p: next(calls)), seed=3)["model"] + assert sum(sum(row.values()) for row in model["confusion"].values()) == 4 + assert len({name for row in model["confusion"].values() for name in row}) > 1 + + +def test_a_transport_that_raises_is_counted_not_reported_as_a_clean_zero(): + def transport(prompt: str, system: str | None): + raise RuntimeError("boom") + + client = LLMClient(model="m", role="selector", transport=transport) + model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=client)["model"] + assert model["n_failed"] == 4 and model["n_abstained"] == 0 + assert "boom" in model["first_error"] + + +def test_all_vs_all_exits_nonzero_when_every_model_call_fails(tmp_path, capsys, monkeypatch): + import dokimi.commands.novelty as novelty_cmd + + def transport(prompt: str, system: str | None): + raise RuntimeError("boom") + + monkeypatch.setattr(novelty_cmd, "_client", lambda model, provider, role, args: LLMClient( + model=model, provider=provider, role=role, transport=transport)) + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + _catalog_skill(root, "db", "tune a slow database query with an index") + queries = _write_queries(tmp_path, {"perf": ["profile a slow program"], + "db": ["add an index"]}) + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json"), + "--model", "m") + with pytest.raises(SystemExit, match="every model call 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 + + +@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) From 2696119427ce40fff6c53b731c2986a1ab590538 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Thu, 22 Oct 2026 08:36:11 -0700 Subject: [PATCH 09/15] fix(novelty): narrow the model arm for mypy in all-vs-all Signed-off-by: Nikolay Petrov --- dokimi/dokimi/commands/novelty.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dokimi/dokimi/commands/novelty.py b/dokimi/dokimi/commands/novelty.py index 5c722ba9..ebc87ef4 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -356,8 +356,8 @@ def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None: print(f"WARNING: {len(without)} skill(s) in the field have no queries, so they only " f"compete: {', '.join(sorted(without))}", file=sys.stderr) - model_arm = result.get("model") - failed = model_arm["n_failed"] if model_arm else 0 + model_arm = result.get("model") or {} + failed = model_arm.get("n_failed", 0) if failed: print(f"WARNING: {failed} model call(s) failed (first error: " f"{model_arm['first_error']}); a failed call is counted as (none) in the model " From 8097254f84130d39fd23cfac5c69fdb41c48ff45 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Thu, 22 Oct 2026 11:42:56 -0700 Subject: [PATCH 10/15] test(novelty): all-vs-all stops at the --max-usd cap the docs point to Signed-off-by: Nikolay Petrov --- dokimi/tests/test_novelty_routing.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index b1ef9d5d..13522ae4 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -988,6 +988,33 @@ def transport(prompt: str, system: str | None): 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" From c570e6cd1b269441a88c4ad00b77f045da813d21 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Thu, 22 Oct 2026 14:00:51 -0700 Subject: [PATCH 11/15] refactor(novelty): split all-vs-all routing and its tests under the growth caps Signed-off-by: Nikolay Petrov --- dokimi/dokimi/commands/novelty.py | 26 +- dokimi/dokimi/novelty/routing.py | 114 ++++-- dokimi/tests/test_novelty_routing.py | 332 ---------------- .../tests/test_novelty_routing_all_vs_all.py | 367 ++++++++++++++++++ 4 files changed, 459 insertions(+), 380 deletions(-) create mode 100644 dokimi/tests/test_novelty_routing_all_vs_all.py diff --git a/dokimi/dokimi/commands/novelty.py b/dokimi/dokimi/commands/novelty.py index ebc87ef4..60271a7f 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -320,14 +320,7 @@ def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None: """Every skill in the field is the gold in turn: confusion matrix and attractors.""" from dokimi.novelty.routing import RoutingInputError, all_vs_all, load_queries_by_skill - if args.artifact: - raise SystemExit("--all-vs-all takes no --artifact: every skill in the field is the " - "gold in turn") - 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 ") + _check_all_vs_all_args(args) try: field = _routing_field(args, set()) except RoutingInputError as exc: @@ -374,6 +367,23 @@ def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None: 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 _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 not args.queries_by_skill: + raise SystemExit("--all-vs-all needs --queries-by-skill : a JSON object of skill " + "name to the requests that should select it") + if not args.out: + raise SystemExit("--all-vs-all needs --out ") + + +def _print_attractors(result: dict) -> None: + """The top five attractors of each arm, one line each.""" for arm in ("lexical", "model"): if arm not in result: continue diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 654845ae..ce357966 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -398,10 +398,27 @@ def catalog_competitors(root: str, exclude: set[str], RoutingInputError: Two skills in the field share a name with different content; the message names both paths. """ - from dokimi.external.catalog import read_skill_manifest - if not Path(root).is_dir(): raise ValueError(f"--catalog is not a directory: {root}") + eligible = _eligible_catalog_skills(root, exclude, notes, include_disabled) + _refuse_conflicting_names(eligible) + keeper = _keepers_by_digest(eligible) + candidates: list[Candidate] = [] + for skill, digest in eligible: + if keeper[digest] is not skill: + if notes is not None: + notes.append(f"left out catalog skill {skill.name!r} at {skill.path}: " + f"identical to {keeper[digest].path}") + continue + candidates.append(Candidate(name=skill.name, description=skill.description)) + return candidates + + +def _eligible_catalog_skills(root: str, exclude: set[str], notes: list[str] | None, + include_disabled: bool) -> list[tuple[Skill, str]]: + """Each loadable, non-excluded catalog skill with the sha256 of its ``SKILL.md``.""" + from dokimi.external.catalog import read_skill_manifest + eligible: list[tuple[Skill, str]] = [] for entry in read_skill_manifest(Path(root)): skill = entry.skill @@ -417,9 +434,14 @@ def catalog_competitors(root: str, exclude: set[str], "disable-model-invocation (--include-disabled keeps it)") continue eligible.append((skill, hashlib.sha256(skill.path.read_bytes()).hexdigest())) + return eligible + - # Two skills with one name and different bytes cannot both stand for the card, and - # check_field would only say "pass each competitor once", which is not the problem. +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)) @@ -433,23 +455,19 @@ def catalog_competitors(root: str, exclude: set[str], "different content; dedupe or rename" ) - # The copy with the fewest path parts stands for each byte-identical group; ties go to - # the path that sorts first, so the choice does not depend on directory order. + +def _keepers_by_digest(eligible: list[tuple[Skill, str]]) -> dict[str, Skill]: + """The copy with the fewest path parts stands for each byte-identical group. + + Ties go to the path that sorts first, so the choice does not depend on directory order. + """ keeper: dict[str, Skill] = {} for skill, digest in eligible: best = keeper.get(digest) if best is None or (len(skill.path.parts), skill.path.as_posix()) < ( len(best.path.parts), best.path.as_posix()): keeper[digest] = skill - candidates: list[Candidate] = [] - for skill, digest in eligible: - if keeper[digest] is not skill: - if notes is not None: - notes.append(f"left out catalog skill {skill.name!r} at {skill.path}: " - f"identical to {keeper[digest].path}") - continue - candidates.append(Candidate(name=skill.name, description=skill.description)) - return candidates + return keeper def check_field(gold: Candidate, competitors: list[Candidate]) -> None: @@ -558,7 +576,21 @@ def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str query file that names a skill outside the field; no skill with any queries. """ names = [c.name for c in candidates] - if len(candidates) < 2: + _check_all_vs_all_input(names, queries_by_skill) + result: dict = { + "n_skills": len(candidates), + "n_queries": sum(len(q) for q in queries_by_skill.values()), + "lexical": _attractor_table(_lexical_picks(candidates, queries_by_skill, names), + queries_by_skill, names), + } + if client is not None: + result["model"] = _model_arm(candidates, queries_by_skill, names, client, seed) + return result + + +def _check_all_vs_all_input(names: list[str], queries_by_skill: dict[str, list[str]]) -> None: + """Refuse a field or query file :func:`all_vs_all` cannot measure.""" + if len(names) < 2: raise RoutingInputError("all-vs-all needs at least two skills in the field") if len(set(names)) != len(names): raise RoutingInputError("all-vs-all: the field names a skill more than once") @@ -569,6 +601,10 @@ def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str if not queries_by_skill: raise RoutingInputError("no skill in the field has queries to route") + +def _lexical_picks(candidates: list[Candidate], queries_by_skill: dict[str, list[str]], + names: list[str]) -> dict[str, Counter]: + """The lexical arm of :func:`all_vs_all`: picks per gold skill.""" bm25 = BM25([tokenize(c.card) for c in candidates]) lexical: dict[str, Counter] = {} for name, queries in queries_by_skill.items(): @@ -584,30 +620,28 @@ def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str else: picks[candidates[best].name] += 1 lexical[name] = picks - result: dict = { - "n_skills": len(candidates), - "n_queries": sum(len(q) for q in queries_by_skill.values()), - "lexical": _attractor_table(lexical, queries_by_skill, names), - } - if client is not None: - model: dict[str, Counter] = {} - n_failed = n_abstained = 0 - first_error = None - for index, name in enumerate(names): - if name not in queries_by_skill: - continue - routed = model_routing(candidates, queries_by_skill[name], client=client, - gold=index, seed=seed + index) - rows = routed["per_query"] - n_failed += routed["n_failed"] - n_abstained += routed["n_abstained"] - if first_error is None: - first_error = next((r["error"] for r in rows if r.get("error")), None) - model[name] = Counter(row.get("picked") or NO_PICK for row in rows) - result["model"] = dict(_attractor_table(model, queries_by_skill, names), seed=seed, - n_failed=n_failed, n_abstained=n_abstained, - first_error=first_error) - return result + return lexical + + +def _model_arm(candidates: list[Candidate], queries_by_skill: dict[str, list[str]], + names: list[str], client: LLMClient, seed: int) -> dict: + """The model arm of :func:`all_vs_all`: attractor table plus failure counts.""" + model: dict[str, Counter] = {} + n_failed = n_abstained = 0 + first_error = None + for index, name in enumerate(names): + if name not in queries_by_skill: + continue + routed = model_routing(candidates, queries_by_skill[name], client=client, + gold=index, seed=seed + index) + rows = routed["per_query"] + n_failed += routed["n_failed"] + n_abstained += routed["n_abstained"] + if first_error is None: + first_error = next((r["error"] for r in rows if r.get("error")), None) + model[name] = Counter(row.get("picked") or NO_PICK for row in rows) + return dict(_attractor_table(model, queries_by_skill, names), seed=seed, + n_failed=n_failed, n_abstained=n_abstained, first_error=first_error) def _attractor_table(picks_by_gold: dict[str, Counter], queries_by_skill: dict[str, list[str]], diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index 13522ae4..234e1168 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -14,14 +14,12 @@ from dokimi.novelty.routing import ( Candidate, RoutingInputError, - all_vs_all, catalog_competitors, check_field, disables_model_invocation, headings_as_queries, lexical_routing, load_queries, - load_queries_by_skill, model_routing, ) @@ -697,333 +695,3 @@ def test_two_catalog_skills_with_one_name_and_different_content_are_refused_by_n 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 _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("field, queries, message", [ - ([PERF], {"linux-perf": ["q"]}, "at least two skills"), - ([PERF, PERF], {"linux-perf": ["q"]}, "more than once"), - ([PERF, TBB], {"mkl-advisor": ["q"]}, "not in the field: `mkl-advisor`"), - ([PERF, TBB], {}, "no skill in the field has queries"), -]) -def test_all_vs_all_refuses_a_field_where_it_would_not_measure_selection(field, queries, - message): - with pytest.raises(RoutingInputError, match=message): - all_vs_all(field, queries) - - -def test_a_queries_file_names_skills_by_object_and_drops_blank_requests(tmp_path): - path = _write_queries(tmp_path, {"linux-perf": [" ", "profile it"], "db": [" "]}) - assert load_queries_by_skill(str(path)) == {"linux-perf": ["profile it"]} - - -@pytest.mark.parametrize("payload", ['["a list"]', '{"linux-perf": "one string"}', - '{"linux-perf": [1]}']) -def test_a_queries_file_that_is_not_an_object_of_string_lists_is_refused(tmp_path, payload): - path = tmp_path / "q.json" - path.write_text(payload, encoding="utf-8") - with pytest.raises(RoutingInputError): - load_queries_by_skill(str(path)) - - -def test_all_vs_all_writes_the_matrix_and_prints_the_attractors(tmp_path, capsys): - root = tmp_path / "catalog" - _catalog_skill(root, "perf", "profile a slow binary with perf record and report") - _catalog_skill(root, "db", "tune a slow database query with an index") - _catalog_skill(root, "hub", "general advice for slow programs and slow queries") - queries = _write_queries(tmp_path, { - "perf": ["my slow program needs advice", "profile a slow program"], - "db": ["my database query is slow", "add an index to speed up a query"], - }) - out = tmp_path / "all_vs_all.json" - args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), - "--queries-by-skill", str(queries), "--out", str(out)) - args.func(args) - payload = json.loads(out.read_text(encoding="utf-8")) - assert payload["mode"] == "all_vs_all" and payload["n_candidates"] == 3 - assert payload["result"]["lexical"]["confusion"]["db"]["db"] == 2 - assert payload["result"]["lexical"]["confusion"]["perf"] == {"perf": 1, "hub": 1} - stdout = capsys.readouterr().out - assert "all-vs-all: 3 skills in the field, 4 queries" in stdout - assert "lexical: hub picked for 1 of 4 sibling queries (from perf)" in stdout - - -def test_all_vs_all_with_model_names_its_model_calls_on_stderr(tmp_path, capsys, monkeypatch): - # One selector call per query: 2 + 2 queries, so 4 calls, each over the 3 cards. - import dokimi.commands.novelty as novelty_cmd - - def transport(prompt: str, system: str | None): - return json.dumps({"pick": 1}), None - - monkeypatch.setattr( - novelty_cmd, - "_client", - lambda model, provider, role, args: LLMClient( - model=model, provider=provider, role=role, transport=transport - ), - ) - root = tmp_path / "catalog" - _catalog_skill(root, "perf", "profile a slow binary with perf record and report") - _catalog_skill(root, "db", "tune a slow database query with an index") - _catalog_skill(root, "hub", "general advice for slow programs and slow queries") - queries = _write_queries( - tmp_path, - { - "perf": ["my slow program needs advice", "profile a slow program"], - "db": ["my database query is slow", "add an index to speed up a query"], - }, - ) - args = _parse( - "novelty", - "routing", - "--all-vs-all", - "--catalog", - str(root), - "--queries-by-skill", - str(queries), - "--out", - str(tmp_path / "o.json"), - "--model", - "m", - ) - args.func(args) - assert ( - "all-vs-all: --model makes 4 model calls, one per query, over 3 cards each" - in capsys.readouterr().err - ) - - -def test_all_vs_all_without_model_says_nothing_about_model_calls(tmp_path, capsys): - root = tmp_path / "catalog" - _catalog_skill(root, "perf", "profile a slow binary with perf record and report") - _catalog_skill(root, "db", "tune a slow database query with an index") - queries = _write_queries(tmp_path, {"perf": ["profile a slow program"]}) - args = _parse( - "novelty", - "routing", - "--all-vs-all", - "--catalog", - str(root), - "--queries-by-skill", - str(queries), - "--out", - str(tmp_path / "o.json"), - ) - args.func(args) - assert "model calls" not in capsys.readouterr().err - - -def test_all_vs_all_takes_no_artifact(tmp_path): - queries = _write_queries(tmp_path, {"linux-perf": ["q"]}) - args = _parse("novelty", "routing", "--all-vs-all", "--artifact", "skill:/x", - "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) - with pytest.raises(SystemExit, match="takes no --artifact"): - args.func(args) - - -def test_a_single_routing_run_still_needs_an_artifact(tmp_path): - args = _parse("novelty", "routing", "--out", str(tmp_path / "o.json")) - with pytest.raises(SystemExit, match="--all-vs-all for the whole field"): - args.func(args) - - -def _two_identical_cards(order): - cards = {"a": Candidate(name="a", description="format json output"), - "b": Candidate(name="b", description="format json output")} - return [cards[n] for n in order] - - -@pytest.mark.parametrize("order", [("a", "b"), ("b", "a")]) -def test_two_identical_cards_tie_and_neither_is_scored_a_win(order): - result = all_vs_all(_two_identical_cards(order), - {"a": ["format json"], "b": ["format json"]})["lexical"] - assert result["own_rate"] == {"a": 0.0, "b": 0.0} - assert result["confusion"] == {"a": {"b": 1}, "b": {"a": 1}} - - -def test_a_three_way_tie_is_not_a_perfect_pick(): - field = [Candidate(name=f"s{i}", description="common words") for i in range(3)] - lexical = all_vs_all(field, {"s0": ["common"]})["lexical"] - assert lexical["own_rate"] == {"s0": 0.0} - - -def test_attractors_are_ranked_by_count_and_rated_over_the_other_skills_queries(): - # hub takes 2 of perf's 2 queries and 1 of db's 3; sink takes 1 of perf's. - field = [Candidate(name="perf", description="profile binary"), - Candidate(name="db", description="index query"), - Candidate(name="hub", description="slow advice"), - Candidate(name="sink", description="gpu offload")] - queries = {"perf": ["slow one", "slow two", "gpu thing"], - "db": ["slow query", "index query", "index query again"]} - lexical = all_vs_all(field, queries)["lexical"] - hub, sink = lexical["attractors"][:2] - assert hub == {"skill": "hub", "wrongly_picked": 3, "sibling_queries": 6, "rate": 0.5, - "from": ["perf", "db"]} - assert sink["skill"] == "sink" and sink["wrongly_picked"] == 1 - # hub's own queries are not sibling queries of hub: only the other golds count. - assert all(row["skill"] != "perf" or row["sibling_queries"] == 3 - for row in lexical["attractors"]) - - -def _positional_client(pick_for): - def transport(prompt: str, system: str | None): - picked = pick_for(prompt) - return json.dumps({"pick": picked}), None - return LLMClient(model="m", role="selector", transport=transport) - - -def test_a_selector_that_abstains_is_none_and_counted_in_the_model_arm(): - model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=_positional_client(lambda p: 0))["model"] - assert model["confusion"] == {"linux-perf": {"(none)": 2}, "mkl-advisor": {"(none)": 2}} - assert model["n_abstained"] == 4 and model["n_failed"] == 0 - assert model["attractors"] == [] - - -def test_the_model_arm_seeds_each_gold_apart_and_hands_model_routing_the_gold(monkeypatch): - import dokimi.novelty.routing as routing - - seen = [] - real = routing.model_routing - - def spy(candidates, queries, **kwargs): - seen.append((kwargs["gold"], kwargs["seed"])) - return real(candidates, queries, **kwargs) - - monkeypatch.setattr(routing, "model_routing", spy) - all_vs_all(ALL_FIELD, ALL_QUERIES, client=_positional_client(lambda p: 1), seed=10) - assert seen == [(0, 10), (2, 12)] - - -def test_the_model_arm_follows_each_call_not_one_fixed_card(): - calls = iter([1, 2, 3, 1]) - model = all_vs_all(ALL_FIELD, ALL_QUERIES, - client=_positional_client(lambda p: next(calls)), seed=3)["model"] - assert sum(sum(row.values()) for row in model["confusion"].values()) == 4 - assert len({name for row in model["confusion"].values() for name in row}) > 1 - - -def test_a_transport_that_raises_is_counted_not_reported_as_a_clean_zero(): - def transport(prompt: str, system: str | None): - raise RuntimeError("boom") - - client = LLMClient(model="m", role="selector", transport=transport) - model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=client)["model"] - assert model["n_failed"] == 4 and model["n_abstained"] == 0 - assert "boom" in model["first_error"] - - -def test_all_vs_all_exits_nonzero_when_every_model_call_fails(tmp_path, capsys, monkeypatch): - import dokimi.commands.novelty as novelty_cmd - - def transport(prompt: str, system: str | None): - raise RuntimeError("boom") - - monkeypatch.setattr(novelty_cmd, "_client", lambda model, provider, role, args: LLMClient( - model=model, provider=provider, role=role, transport=transport)) - root = tmp_path / "catalog" - _catalog_skill(root, "perf", "profile a slow binary with perf record and report") - _catalog_skill(root, "db", "tune a slow database query with an index") - queries = _write_queries(tmp_path, {"perf": ["profile a slow program"], - "db": ["add an index"]}) - args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), - "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json"), - "--model", "m") - with pytest.raises(SystemExit, match="every model call 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) diff --git a/dokimi/tests/test_novelty_routing_all_vs_all.py b/dokimi/tests/test_novelty_routing_all_vs_all.py new file mode 100644 index 00000000..c0d18940 --- /dev/null +++ b/dokimi/tests/test_novelty_routing_all_vs_all.py @@ -0,0 +1,367 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""All-vs-all routing: every skill in the field is the gold in turn.""" + +from __future__ import annotations + +import json + +import pytest + +from dokimi.cli import build_parser +from dokimi.novelty.client import LLMClient +from dokimi.novelty.routing import ( + Candidate, + RoutingInputError, + all_vs_all, + load_queries_by_skill, +) + +PERF = Candidate(name="linux-perf", + description="Profile a Linux binary with perf: record, report, annotate") +TBB = Candidate(name="onetbb-quickstart", + description="Parallelize C++ loops with oneTBB task groups") +MKL = Candidate(name="mkl-advisor", + description="Choose oneMKL BLAS routines for dense linear algebra") + + +def _parse(*argv): + return build_parser().parse_args(list(argv)) + + +def _catalog_skill(root, name, description): + skill_dir = root / "skills" / name + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: {description}\n---\n\n# B\n", encoding="utf-8") + + +def _write_queries(tmp_path, by_skill): + path = tmp_path / "queries_by_skill.json" + path.write_text(json.dumps(by_skill), encoding="utf-8") + return path + + +# hub's card shares "slow" with the others' queries, so it takes them. Lexically it picks +# linux-perf's two queries and nothing of mkl-advisor's; the model arm (below) picks it for all. +HUB = Candidate(name="hub", description="slow programs and slow queries advice") +ALL_FIELD = [PERF, HUB, MKL] +ALL_QUERIES = { + "linux-perf": ["my slow program needs advice", "profile a slow program"], + "mkl-advisor": ["choose blas routines for dense linear algebra", "zzz unrelated words"], +} + + +def test_all_vs_all_names_the_skill_that_takes_the_queries_of_others(): + confusion = all_vs_all(ALL_FIELD, ALL_QUERIES)["lexical"]["confusion"] + assert confusion["linux-perf"] == {"hub": 2} + + +def test_an_attractor_row_says_how_many_sibling_queries_it_took_and_from_whom(): + result = all_vs_all(ALL_FIELD, ALL_QUERIES) + assert result["n_skills"] == 3 and result["n_queries"] == 4 + (row,) = result["lexical"]["attractors"] + assert row == {"skill": "hub", "wrongly_picked": 2, "sibling_queries": 4, "rate": 0.5, + "from": ["linux-perf"]} + + +def test_a_query_no_card_shares_a_word_with_is_none_not_the_first_card(): + # linux-perf is first in the field, so the lexical tie-break would name it here. + lexical = all_vs_all(ALL_FIELD, ALL_QUERIES)["lexical"] + assert lexical["confusion"]["mkl-advisor"] == {"mkl-advisor": 1, "(none)": 1} + assert lexical["own_rate"]["mkl-advisor"] == 0.5 + + +def test_the_model_arm_is_reported_only_when_a_client_is_given(): + assert "model" not in all_vs_all(ALL_FIELD, ALL_QUERIES) + + +def test_the_model_arm_attractor_is_the_skill_the_selector_always_picks(): + def transport(prompt: str, system: str | None): + lines = prompt.split("TOOLS:", 1)[1].strip().splitlines() + return json.dumps({"pick": next(i + 1 for i, line in enumerate(lines) + if "hub" in line)}), None + + client = LLMClient(model="m", role="selector", transport=transport) + model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=client, seed=7)["model"] + assert model["seed"] == 7 + (row,) = model["attractors"] + assert (row["skill"], row["wrongly_picked"], row["sibling_queries"]) == ("hub", 4, 4) + assert row["from"] == ["linux-perf", "mkl-advisor"] + + +@pytest.mark.parametrize("field, queries, message", [ + ([PERF], {"linux-perf": ["q"]}, "at least two skills"), + ([PERF, PERF], {"linux-perf": ["q"]}, "more than once"), + ([PERF, TBB], {"mkl-advisor": ["q"]}, "not in the field: `mkl-advisor`"), + ([PERF, TBB], {}, "no skill in the field has queries"), +]) +def test_all_vs_all_refuses_a_field_where_it_would_not_measure_selection(field, queries, + message): + with pytest.raises(RoutingInputError, match=message): + all_vs_all(field, queries) + + +def test_a_queries_file_names_skills_by_object_and_drops_blank_requests(tmp_path): + path = _write_queries(tmp_path, {"linux-perf": [" ", "profile it"], "db": [" "]}) + assert load_queries_by_skill(str(path)) == {"linux-perf": ["profile it"]} + + +@pytest.mark.parametrize("payload", ['["a list"]', '{"linux-perf": "one string"}', + '{"linux-perf": [1]}']) +def test_a_queries_file_that_is_not_an_object_of_string_lists_is_refused(tmp_path, payload): + path = tmp_path / "q.json" + path.write_text(payload, encoding="utf-8") + with pytest.raises(RoutingInputError): + load_queries_by_skill(str(path)) + + +def test_all_vs_all_writes_the_matrix_and_prints_the_attractors(tmp_path, capsys): + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + _catalog_skill(root, "db", "tune a slow database query with an index") + _catalog_skill(root, "hub", "general advice for slow programs and slow queries") + queries = _write_queries(tmp_path, { + "perf": ["my slow program needs advice", "profile a slow program"], + "db": ["my database query is slow", "add an index to speed up a query"], + }) + out = tmp_path / "all_vs_all.json" + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(out)) + args.func(args) + payload = json.loads(out.read_text(encoding="utf-8")) + assert payload["mode"] == "all_vs_all" and payload["n_candidates"] == 3 + assert payload["result"]["lexical"]["confusion"]["db"]["db"] == 2 + assert payload["result"]["lexical"]["confusion"]["perf"] == {"perf": 1, "hub": 1} + stdout = capsys.readouterr().out + assert "all-vs-all: 3 skills in the field, 4 queries" in stdout + assert "lexical: hub picked for 1 of 4 sibling queries (from perf)" in stdout + + +def test_all_vs_all_with_model_names_its_model_calls_on_stderr(tmp_path, capsys, monkeypatch): + # One selector call per query: 2 + 2 queries, so 4 calls, each over the 3 cards. + import dokimi.commands.novelty as novelty_cmd + + def transport(prompt: str, system: str | None): + return json.dumps({"pick": 1}), None + + monkeypatch.setattr( + novelty_cmd, + "_client", + lambda model, provider, role, args: LLMClient( + model=model, provider=provider, role=role, transport=transport + ), + ) + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + _catalog_skill(root, "db", "tune a slow database query with an index") + _catalog_skill(root, "hub", "general advice for slow programs and slow queries") + queries = _write_queries( + tmp_path, + { + "perf": ["my slow program needs advice", "profile a slow program"], + "db": ["my database query is slow", "add an index to speed up a query"], + }, + ) + args = _parse( + "novelty", + "routing", + "--all-vs-all", + "--catalog", + str(root), + "--queries-by-skill", + str(queries), + "--out", + str(tmp_path / "o.json"), + "--model", + "m", + ) + args.func(args) + assert ( + "all-vs-all: --model makes 4 model calls, one per query, over 3 cards each" + in capsys.readouterr().err + ) + + +def test_all_vs_all_without_model_says_nothing_about_model_calls(tmp_path, capsys): + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + _catalog_skill(root, "db", "tune a slow database query with an index") + queries = _write_queries(tmp_path, {"perf": ["profile a slow program"]}) + args = _parse( + "novelty", + "routing", + "--all-vs-all", + "--catalog", + str(root), + "--queries-by-skill", + str(queries), + "--out", + str(tmp_path / "o.json"), + ) + args.func(args) + assert "model calls" not in capsys.readouterr().err + + +def test_all_vs_all_takes_no_artifact(tmp_path): + queries = _write_queries(tmp_path, {"linux-perf": ["q"]}) + args = _parse("novelty", "routing", "--all-vs-all", "--artifact", "skill:/x", + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) + with pytest.raises(SystemExit, match="takes no --artifact"): + args.func(args) + + +def test_a_single_routing_run_still_needs_an_artifact(tmp_path): + args = _parse("novelty", "routing", "--out", str(tmp_path / "o.json")) + with pytest.raises(SystemExit, match="--all-vs-all for the whole field"): + args.func(args) + + +def _two_identical_cards(order): + cards = {"a": Candidate(name="a", description="format json output"), + "b": Candidate(name="b", description="format json output")} + return [cards[n] for n in order] + + +@pytest.mark.parametrize("order", [("a", "b"), ("b", "a")]) +def test_two_identical_cards_tie_and_neither_is_scored_a_win(order): + result = all_vs_all(_two_identical_cards(order), + {"a": ["format json"], "b": ["format json"]})["lexical"] + assert result["own_rate"] == {"a": 0.0, "b": 0.0} + assert result["confusion"] == {"a": {"b": 1}, "b": {"a": 1}} + + +def test_a_three_way_tie_is_not_a_perfect_pick(): + field = [Candidate(name=f"s{i}", description="common words") for i in range(3)] + lexical = all_vs_all(field, {"s0": ["common"]})["lexical"] + assert lexical["own_rate"] == {"s0": 0.0} + + +def test_attractors_are_ranked_by_count_and_rated_over_the_other_skills_queries(): + # hub takes 2 of perf's 2 queries and 1 of db's 3; sink takes 1 of perf's. + field = [Candidate(name="perf", description="profile binary"), + Candidate(name="db", description="index query"), + Candidate(name="hub", description="slow advice"), + Candidate(name="sink", description="gpu offload")] + queries = {"perf": ["slow one", "slow two", "gpu thing"], + "db": ["slow query", "index query", "index query again"]} + lexical = all_vs_all(field, queries)["lexical"] + hub, sink = lexical["attractors"][:2] + assert hub == {"skill": "hub", "wrongly_picked": 3, "sibling_queries": 6, "rate": 0.5, + "from": ["perf", "db"]} + assert sink["skill"] == "sink" and sink["wrongly_picked"] == 1 + # hub's own queries are not sibling queries of hub: only the other golds count. + assert all(row["skill"] != "perf" or row["sibling_queries"] == 3 + for row in lexical["attractors"]) + + +def _positional_client(pick_for): + def transport(prompt: str, system: str | None): + picked = pick_for(prompt) + return json.dumps({"pick": picked}), None + return LLMClient(model="m", role="selector", transport=transport) + + +def test_a_selector_that_abstains_is_none_and_counted_in_the_model_arm(): + model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=_positional_client(lambda p: 0))["model"] + assert model["confusion"] == {"linux-perf": {"(none)": 2}, "mkl-advisor": {"(none)": 2}} + assert model["n_abstained"] == 4 and model["n_failed"] == 0 + assert model["attractors"] == [] + + +def test_the_model_arm_seeds_each_gold_apart_and_hands_model_routing_the_gold(monkeypatch): + import dokimi.novelty.routing as routing + + seen = [] + real = routing.model_routing + + def spy(candidates, queries, **kwargs): + seen.append((kwargs["gold"], kwargs["seed"])) + return real(candidates, queries, **kwargs) + + monkeypatch.setattr(routing, "model_routing", spy) + all_vs_all(ALL_FIELD, ALL_QUERIES, client=_positional_client(lambda p: 1), seed=10) + assert seen == [(0, 10), (2, 12)] + + +def test_the_model_arm_follows_each_call_not_one_fixed_card(): + calls = iter([1, 2, 3, 1]) + model = all_vs_all(ALL_FIELD, ALL_QUERIES, + client=_positional_client(lambda p: next(calls)), seed=3)["model"] + assert sum(sum(row.values()) for row in model["confusion"].values()) == 4 + assert len({name for row in model["confusion"].values() for name in row}) > 1 + + +def test_a_transport_that_raises_is_counted_not_reported_as_a_clean_zero(): + def transport(prompt: str, system: str | None): + raise RuntimeError("boom") + + client = LLMClient(model="m", role="selector", transport=transport) + model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=client)["model"] + assert model["n_failed"] == 4 and model["n_abstained"] == 0 + assert "boom" in model["first_error"] + + +def test_all_vs_all_exits_nonzero_when_every_model_call_fails(tmp_path, capsys, monkeypatch): + import dokimi.commands.novelty as novelty_cmd + + def transport(prompt: str, system: str | None): + raise RuntimeError("boom") + + monkeypatch.setattr(novelty_cmd, "_client", lambda model, provider, role, args: LLMClient( + model=model, provider=provider, role=role, transport=transport)) + root = tmp_path / "catalog" + _catalog_skill(root, "perf", "profile a slow binary with perf record and report") + _catalog_skill(root, "db", "tune a slow database query with an index") + queries = _write_queries(tmp_path, {"perf": ["profile a slow program"], + "db": ["add an index"]}) + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json"), + "--model", "m") + with pytest.raises(SystemExit, match="every model call 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) From fdb540948678f6cac4867587cc8bab1ad3c6096d Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Thu, 22 Oct 2026 14:01:15 -0700 Subject: [PATCH 12/15] chore(growth): raise commands/novelty.py for the W4 flag Signed-off-by: Nikolay Petrov --- dokimi/scripts/growth_baseline.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dokimi/scripts/growth_baseline.json b/dokimi/scripts/growth_baseline.json index 2b0f5fe8..b6344912 100644 --- a/dokimi/scripts/growth_baseline.json +++ b/dokimi/scripts/growth_baseline.json @@ -1,6 +1,6 @@ { "files": { - "dokimi/commands/novelty.py": {"lines": 827}, + "dokimi/commands/novelty.py": {"lines": 969, "rationale": "W4 adds routing --all-vs-all; file split tracked separately to avoid conflicts across the W-series"}, "dokimi/harnesses/benchflow.py": {"lines": 869}, "dokimi/harnesses/docker_solver.py": {"lines": 927, "rationale": "pre-existing when the gate was introduced"}, "dokimi/llm.py": {"lines": 1112}, From d992d4645d892c9b33942dab18b690b3b3e235bd Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Thu, 22 Oct 2026 22:38:07 -0700 Subject: [PATCH 13/15] fix(novelty): all-vs-all model arm records which model made it Single-mode routing.json keeps the selector model; the all-vs-all model arm did not, so files from two different models were indistinguishable. Signed-off-by: Alexander Rybkin --- dokimi/docs/skill-novelty.md | 3 ++- dokimi/dokimi/novelty/routing.py | 5 +++-- dokimi/tests/test_novelty_routing_all_vs_all.py | 7 +++++++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 399a8d17..a7146a77 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -632,7 +632,8 @@ skill's own text. The output has a confusion matrix (gold to picked to count, wi a query no card shares a word with, or an abstention or failed call in the model arm), 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`). It takes no -`--artifact`. It is refused with fewer than two skills, a name repeated in the field, a query +`--artifact`. 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 diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index ce357966..29c80e77 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -569,7 +569,8 @@ def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str Returns: ``n_skills``, ``n_queries``, and per arm (``lexical``, ``model``): ``confusion`` (gold skill to picked skill to count, ``(none)`` included), ``own_rate`` per gold skill, and - ``attractors`` ordered by ``wrongly_picked``. The model arm also records ``seed``, ``n_failed``, ``n_abstained`` and ``first_error``. + ``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 @@ -640,7 +641,7 @@ def _model_arm(candidates: list[Candidate], queries_by_skill: dict[str, list[str if first_error is None: first_error = next((r["error"] for r in rows if r.get("error")), None) model[name] = Counter(row.get("picked") or NO_PICK for row in rows) - return dict(_attractor_table(model, queries_by_skill, names), seed=seed, + return dict(_attractor_table(model, queries_by_skill, names), model=client.model, seed=seed, n_failed=n_failed, n_abstained=n_abstained, first_error=first_error) diff --git a/dokimi/tests/test_novelty_routing_all_vs_all.py b/dokimi/tests/test_novelty_routing_all_vs_all.py index c0d18940..ae6bafe7 100644 --- a/dokimi/tests/test_novelty_routing_all_vs_all.py +++ b/dokimi/tests/test_novelty_routing_all_vs_all.py @@ -91,6 +91,13 @@ def transport(prompt: str, system: str | None): 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"), From adbe542720a37763d94976bcaa7df97a2412a14b Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Thu, 22 Oct 2026 22:38:20 -0700 Subject: [PATCH 14/15] test(novelty): pin the attractor rate for a skill that has queries of its own The existing guard never ran: in its fixture no attractor has queries, so "rate over all queries" and "own queries not excluded" mutants passed. Signed-off-by: Alexander Rybkin --- dokimi/tests/test_novelty_routing_all_vs_all.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/dokimi/tests/test_novelty_routing_all_vs_all.py b/dokimi/tests/test_novelty_routing_all_vs_all.py index ae6bafe7..61ab8829 100644 --- a/dokimi/tests/test_novelty_routing_all_vs_all.py +++ b/dokimi/tests/test_novelty_routing_all_vs_all.py @@ -258,9 +258,16 @@ def test_attractors_are_ranked_by_count_and_rated_over_the_other_skills_queries( 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 - # hub's own queries are not sibling queries of hub: only the other golds count. - assert all(row["skill"] != "perf" or row["sibling_queries"] == 3 - for row in lexical["attractors"]) + + +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): From 5e27afa302063d3beb42e4536fc53e8a7b331c11 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Fri, 23 Oct 2026 12:50:46 -0700 Subject: [PATCH 15/15] fix(novelty): all-vs-all scores only scored queries, ties are order-free, partial failures exit non-zero Rates exclude unscored queries (no content word, failed model call) and report null when a skill has none scored. A tie among other cards is (tie). Repeated skill keys in the queries file are refused, the not-in-the-field error names disable-model-invocation, --queries is refused with --all-vs-all, and routing results record the field rule. Signed-off-by: Nikolay Petrov --- ...1021_134000_napetrov_routing-all-vs-all.md | 4 + dokimi/docs/skill-novelty.md | 21 ++-- dokimi/dokimi/commands/novelty.py | 39 ++++-- dokimi/dokimi/novelty/routing.py | 105 +++++++++++----- dokimi/scripts/growth_baseline.json | 2 +- .../tests/test_novelty_routing_all_vs_all.py | 112 +++++++++++++++++- 6 files changed, 239 insertions(+), 44 deletions(-) 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 index 29843570..935bfe87 100644 --- a/dokimi/changelog.d/20261021_134000_napetrov_routing-all-vs-all.md +++ b/dokimi/changelog.d/20261021_134000_napetrov_routing-all-vs-all.md @@ -5,3 +5,7 @@ for other skills' queries, with how many of those queries they took and from whom. Takes `--queries-by-skill ` and `--out`, and no `--artifact`. The lexical arm always runs; `--model` adds the model arm. +- `--all-vs-all` rates are over the scored queries (`n_scored`, `n_unscored`; `null` own rate + when none was scored); a lexical tie among other cards is `(tie)`; a failed model call + exits non-zero; a repeated skill key in the queries file is refused; routing results record + the field rule under `field`. diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 36f9bf5d..52dde155 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -628,13 +628,20 @@ counted as a miss. --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. The output has a confusion matrix (gold to picked to count, with `(none)` for -a query no card shares a word with, or an abstention or failed call in the model arm), 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`). It takes no -`--artifact`. 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. +skill's own text. A skill named twice in the file is refused. The output has a confusion +matrix (gold to picked to count, with `(none)` for a query no card shares a word with or, in +the model arm, an abstention, and `(tie)` for a query on which two or more other cards tie for +the top lexical score, so no field order decides it), an own rate per skill, and an attractor +table: a skill picked for other skills' queries, with `wrongly_picked` of `sibling_queries` and +the skills whose queries it took (`from`). Every rate is over the queries that were scored: +`n_scored` and `n_unscored` per skill say how many, a query with no content word or a failed +model call is unscored, and a skill with none scored has an own rate of `null`, not 0.0. A run +with any failed model call writes the result and exits non-zero. The saved file records the +field rule under `field` (`include_disabled`, the `competitors` and what was `left_out`); a +single-artifact run records it too. It takes no `--artifact`, `--queries` or +`--queries-from-headings`. The model arm also records the selector `model`, so two runs with +different models can be told apart. It is refused with fewer than two skills, a name repeated +in the field, a query file that names a skill outside the field, or no queries at all. **Cost of `--model` in all-vs-all.** The model arm makes one selector call per query, across every skill in the field: the call count is the total number of requests in `queries.json`, and diff --git a/dokimi/dokimi/commands/novelty.py b/dokimi/dokimi/commands/novelty.py index e196928d..387ae33d 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -272,9 +272,12 @@ def _routing_card(ref: str): return candidate, body -def _routing_field(args: argparse.Namespace, exclude: set[str]) -> list: +def _routing_field(args: argparse.Namespace, exclude: set[str], + left_out: list[str] | None = None) -> list: """The field ``--competitors`` and ``--catalog`` name, without the names in ``exclude``. + What was left out of the field, and why, is appended to ``left_out`` when given. + Raises: FileNotFoundError, ValueError: A reference or the catalog cannot be loaded. """ @@ -290,6 +293,8 @@ def _routing_field(args: argparse.Namespace, exclude: set[str]) -> list: 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) @@ -297,6 +302,8 @@ def _routing_field(args: argparse.Namespace, exclude: set[str]) -> list: 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: @@ -306,6 +313,8 @@ def _routing_field(args: argparse.Namespace, exclude: set[str]) -> list: 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, " @@ -321,8 +330,9 @@ def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None: from dokimi.novelty.routing import RoutingInputError, all_vs_all, load_queries_by_skill _check_all_vs_all_args(args) + left_out: list[str] = [] try: - field = _routing_field(args, set()) + field = _routing_field(args, set(), left_out) except RoutingInputError as exc: raise SystemExit(str(exc)) from None except (FileNotFoundError, ValueError) as exc: @@ -353,28 +363,39 @@ def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None: failed = model_arm.get("n_failed", 0) if failed: print(f"WARNING: {failed} model call(s) failed (first error: " - f"{model_arm['first_error']}); a failed call is counted as (none) in the model " - "arm, so its rates are lower bounds", file=sys.stderr) + 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 model_arm and failed and failed >= result["n_queries"]: - raise SystemExit(f"every model call failed ({model_arm['first_error']}); the partial " - f"result is in {out}") + if failed: + raise SystemExit(f"{failed} of {result['n_queries']} model call(s) failed " + f"({model_arm['first_error']}); the partial result, with those queries " + f"left out of every rate, is in {out}") print(f"OK Saved all-vs-all routing: {out}") print(f"all-vs-all: {len(field)} skills in the field, {result['n_queries']} queries over " f"{len(queries_by_skill)} skills with queries") _print_attractors(result) +def _field_record(args: argparse.Namespace, competitors: list, left_out: list[str]) -> dict: + """The field rule a routing result was measured under, saved next to the result.""" + return {"include_disabled": bool(args.include_disabled), + "competitors": [c.name for c in competitors], "left_out": left_out} + + def _check_all_vs_all_args(args: argparse.Namespace) -> None: """Refuse the flag combinations ``--all-vs-all`` cannot run with.""" if args.artifact: raise SystemExit("--all-vs-all takes no --artifact: every skill in the field is the " "gold in turn") + if args.queries or args.queries_from_headings: + raise SystemExit("--all-vs-all takes --queries-by-skill, not --queries or " + "--queries-from-headings") if not args.queries_by_skill: raise SystemExit("--all-vs-all needs --queries-by-skill : a JSON object of skill " "name to the requests that should select it") @@ -419,7 +440,8 @@ def cmd_novelty_routing(args: argparse.Namespace) -> None: 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) - competitors = _routing_field(args, {gold.name}) + left_out: list[str] = [] + competitors = _routing_field(args, {gold.name}, left_out) except RoutingInputError as exc: raise SystemExit(str(exc)) from None except (FileNotFoundError, ValueError) as exc: @@ -460,6 +482,7 @@ def cmd_novelty_routing(args: argparse.Namespace) -> None: "status": "ok", "n_candidates": len(candidates), "queries_self_derived": self_derived, + "field": _field_record(args, competitors, left_out), "lexical": lexical, } if args.model: diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 0afcc6fe..78d1c574 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -551,6 +551,19 @@ def load_queries(path: str) -> list[str]: 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]]: @@ -560,11 +573,12 @@ def load_queries_by_skill(path: str) -> dict[str, list[str]]: it. Blank requests are dropped, and a skill left with none is dropped with them. Raises: - RoutingInputError: The file is not JSON, is not an object, or a value is not a list - of strings. + RoutingInputError: The file is not JSON, is not an object, names a skill twice, or a + value is not a list of strings. """ try: - data = json.loads(Path(path).read_text(encoding="utf-8-sig")) + data = json.loads(Path(path).read_text(encoding="utf-8-sig"), + object_pairs_hook=_no_repeated_key(path)) except json.JSONDecodeError as exc: raise RoutingInputError(f"{path}: not JSON: {exc}") from None if not isinstance(data, dict): @@ -594,8 +608,9 @@ def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str Returns: ``n_skills``, ``n_queries``, and per arm (``lexical``, ``model``): ``confusion`` (gold - skill to picked skill to count, ``(none)`` included), ``own_rate`` per gold skill, and - ``attractors`` ordered by ``wrongly_picked``. The model arm also records ``model``, + 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: @@ -607,8 +622,8 @@ def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str result: dict = { "n_skills": len(candidates), "n_queries": sum(len(q) for q in queries_by_skill.values()), - "lexical": _attractor_table(_lexical_picks(candidates, queries_by_skill, names), - queries_by_skill, names), + "lexical": _attractor_table(*_lexical_picks(candidates, queries_by_skill, names), + names), } if client is not None: result["model"] = _model_arm(candidates, queries_by_skill, names, client, seed) @@ -624,36 +639,59 @@ def _check_all_vs_all_input(names: list[str], queries_by_skill: dict[str, list[s 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)) + + ", ".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]) -> dict[str, Counter]: - """The lexical arm of :func:`all_vs_all`: picks per gold skill.""" + 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) - # The gold sorts after every card it ties with, as in lexical_routing: a tie is - # never a win for the card that happens to come first in the field. - order = bm25.rank(tokens, last=names.index(name)) if tokens else [] - best = order[0] if order else None - if best is None or bm25.score(tokens, best) <= 0: - picks[NO_PICK] += 1 - else: - picks[candidates[best].name] += 1 + 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 + return lexical, unscored def _model_arm(candidates: list[Candidate], queries_by_skill: dict[str, list[str]], names: list[str], client: LLMClient, seed: int) -> dict: """The model arm of :func:`all_vs_all`: attractor table plus failure counts.""" model: dict[str, Counter] = {} + unscored: dict[str, int] = {} n_failed = n_abstained = 0 first_error = None for index, name in enumerate(names): @@ -666,22 +704,30 @@ def _model_arm(candidates: list[Candidate], queries_by_skill: dict[str, list[str n_abstained += routed["n_abstained"] if first_error is None: first_error = next((r["error"] for r in rows if r.get("error")), None) - model[name] = Counter(row.get("picked") or NO_PICK for row in rows) - return dict(_attractor_table(model, queries_by_skill, names), model=client.model, seed=seed, + # A failed call measured nothing: it is not a pick, so it is left out of every rate. + model[name] = Counter(row.get("picked") or NO_PICK for row in rows if not row.get("error")) + if routed["n_failed"]: + unscored[name] = routed["n_failed"] + return dict(_attractor_table(model, unscored, names), model=client.model, seed=seed, n_failed=n_failed, n_abstained=n_abstained, first_error=first_error) -def _attractor_table(picks_by_gold: dict[str, Counter], queries_by_skill: dict[str, list[str]], +def _attractor_table(picks_by_gold: dict[str, Counter], unscored: dict[str, int], names: list[str]) -> dict: - """The confusion matrix, own rate and attractor table for one arm of :func:`all_vs_all`.""" + """The confusion matrix, own rate and attractor table for one arm of :func:`all_vs_all`. + + Every rate is over the queries that were scored: ``n_scored`` per gold skill, with + ``n_unscored`` beside it. A skill with none scored has an ``own_rate`` of ``None``, which + is not the 0.0 of a skill the arm really never picked. + """ + scored = {gold: sum(picks.values()) for gold, picks in picks_by_gold.items()} attractors: list[dict] = [] for name in names: wrong = {gold: picks[name] for gold, picks in picks_by_gold.items() if gold != name and picks[name]} if not wrong: continue - sibling_queries = sum(len(queries_by_skill[gold]) for gold in picks_by_gold - if gold != name) + sibling_queries = sum(n for gold, n in scored.items() if gold != name) attractors.append({ "skill": name, "wrongly_picked": sum(wrong.values()), @@ -693,9 +739,14 @@ def _attractor_table(picks_by_gold: dict[str, Counter], queries_by_skill: dict[s return { "confusion": {gold: dict(sorted(picks.items())) for gold, picks in sorted(picks_by_gold.items())}, - "own_rate": {gold: round(picks[gold] / len(queries_by_skill[gold]), 4) + "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 " - "also counts an abstention and a failed call."), + f"also counts an abstention. {TIE} counts a query on which two or more other " + "cards tie for the top score, so no field order decides it. A query with no " + "content word, and a failed model call, is not scored: it is in neither the " + "confusion matrix nor any rate (see n_unscored)."), } diff --git a/dokimi/scripts/growth_baseline.json b/dokimi/scripts/growth_baseline.json index bc150b39..0df02ed1 100644 --- a/dokimi/scripts/growth_baseline.json +++ b/dokimi/scripts/growth_baseline.json @@ -1,6 +1,6 @@ { "files": { - "dokimi/commands/novelty.py": {"lines": 1005, "rationale": "routing catalog field, disabled/mirrored skills, all-vs-all in novelty CLI; file split tracked separately"}, + "dokimi/commands/novelty.py": {"lines": 1028, "rationale": "routing catalog field, disabled/mirrored skills, all-vs-all, field record"}, "dokimi/harnesses/benchflow.py": {"lines": 869}, "dokimi/harnesses/docker_solver.py": {"lines": 927, "rationale": "pre-existing when the gate was introduced"}, "dokimi/llm.py": {"lines": 1118, "rationale": "grew on main in #180 (replay a stored retry) after #93 seeded this baseline; split it in its own PR"}, diff --git a/dokimi/tests/test_novelty_routing_all_vs_all.py b/dokimi/tests/test_novelty_routing_all_vs_all.py index 61ab8829..64d16185 100644 --- a/dokimi/tests/test_novelty_routing_all_vs_all.py +++ b/dokimi/tests/test_novelty_routing_all_vs_all.py @@ -333,7 +333,7 @@ def transport(prompt: str, system: str | None): 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="every model call failed"): + 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 @@ -379,3 +379,113 @@ def test_an_unreadable_queries_file_is_a_message_not_a_traceback(tmp_path, make) "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) with pytest.raises(SystemExit, match="cannot read the queries"): args.func(args) + + +def _cat(root, names_descriptions): + for name, description in names_descriptions.items(): + _catalog_skill(root, name, description) + + +def test_a_partial_model_failure_exits_non_zero_and_leaves_failed_queries_out( + tmp_path, capsys, monkeypatch): + import dokimi.commands.novelty as novelty_cmd + + answered = [] + + def transport(prompt: str, system: str | None): + if answered: + raise RuntimeError("rate limited") + answered.append(1) + return '{"pick": 1}', None + + monkeypatch.setattr(novelty_cmd, "_client", lambda model, provider, role, args: LLMClient( + model=model, provider=provider, role=role, transport=transport)) + root = tmp_path / "catalog" + _cat(root, {"perf": "profile a slow binary with perf", "db": "tune a slow database query"}) + queries = _write_queries(tmp_path, {"db": ["add an index"], "perf": ["profile it", "perf it"]}) + out = tmp_path / "o.json" + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(out), "--model", "m") + with pytest.raises(SystemExit, match="2 of 3 model call"): + args.func(args) + assert "OK Saved" not in capsys.readouterr().out + model = json.loads(out.read_text(encoding="utf-8"))["result"]["model"] + assert model["n_failed"] == 2 + # perf's two queries both failed: no rate, not a measured 0.0. + assert model["own_rate"]["perf"] is None and model["n_scored"]["perf"] == 0 + assert model["n_unscored"] == {"perf": 2} + + +def test_a_query_with_no_content_word_is_not_scored_as_a_miss(): + field = [Candidate("alpha", "convert a json file to yaml"), + Candidate("beta", "tune a slow database query")] + result = all_vs_all(field, {"alpha": ["convert my json file to yaml", + "json to yaml please", "the of and"]}) + lexical = result["lexical"] + assert lexical["own_rate"]["alpha"] == 1.0 + assert lexical["n_scored"] == {"alpha": 2} and lexical["n_unscored"] == {"alpha": 1} + + +@pytest.mark.parametrize("order", [(0, 1, 2), (2, 1, 0), (1, 2, 0)]) +def test_a_tie_among_other_cards_is_a_tie_in_any_field_order(order): + cards = [Candidate("fft", "fast transforms"), Candidate("linalg", "fast transforms"), + Candidate("random", "fast transforms"), + Candidate("gold", "something unrelated entirely")] + field = [cards[i] for i in order] + [cards[3]] + lexical = all_vs_all(field, {"gold": ["fast transforms"]})["lexical"] + assert lexical["confusion"]["gold"] == {"(tie)": 1} + assert lexical["attractors"] == [] + + +def test_the_saved_payload_records_the_field_rule(tmp_path): + root = tmp_path / "catalog" + _cat(root, {"perf": "profile a slow binary", "db": "tune a slow database query"}) + hidden = root / "skills" / "ghost" + hidden.mkdir(parents=True) + (hidden / "SKILL.md").write_text( + "---\nname: ghost\ndescription: x\ndisable-model-invocation: true\n---\n\n# B\n", + encoding="utf-8") + queries = _write_queries(tmp_path, {"perf": ["profile a binary"]}) + out = tmp_path / "o.json" + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), + "--queries-by-skill", str(queries), "--out", str(out)) + args.func(args) + field = json.loads(out.read_text(encoding="utf-8"))["field"] + assert field["include_disabled"] is False + assert sorted(field["competitors"]) == ["db", "perf"] + assert len(field["left_out"]) == 1 and "ghost" in field["left_out"][0] + + +def test_a_skill_named_twice_in_the_queries_file_is_refused(tmp_path): + path = tmp_path / "q.json" + path.write_text('{"a": ["profile"], "a": ["slow"]}', encoding="utf-8") + with pytest.raises(RoutingInputError, match="skill 'a' appears more than once"): + load_queries_by_skill(str(path)) + + +def test_the_not_in_the_field_error_names_the_disable_flag_and_its_override(): + with pytest.raises(RoutingInputError, match="disable-model-invocation.*--include-disabled"): + all_vs_all([PERF, MKL], {"ghost": ["x y"]}) + + +@pytest.mark.parametrize("flag", [["--queries", "q.txt"], ["--queries-from-headings"]]) +def test_all_vs_all_refuses_the_single_artifact_query_flags(tmp_path, flag): + queries = _write_queries(tmp_path, {"perf": ["profile"]}) + args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(tmp_path), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json"), *flag) + with pytest.raises(SystemExit, match="takes --queries-by-skill, not --queries"): + args.func(args) + + +def test_a_single_artifact_routing_run_records_the_field_rule_too(tmp_path): + root = tmp_path / "catalog" + _cat(root, {"perf": "profile a slow binary", "db": "tune a slow database query"}) + queries = tmp_path / "q.txt" + queries.write_text("profile a binary\n", encoding="utf-8") + out = tmp_path / "r.json" + args = _parse("novelty", "routing", "--artifact", f"skill:{root / 'skills' / 'perf'}", + "--catalog", str(root), "--include-disabled", "--queries", str(queries), + "--out", str(out)) + args.func(args) + field = json.loads(out.read_text(encoding="utf-8"))["field"] + assert field == {"include_disabled": True, "competitors": ["db"], "left_out": []}