diff --git a/dokimi/changelog.d/20261113_150000_heading-query-artifact.md b/dokimi/changelog.d/20261113_150000_heading-query-artifact.md new file mode 100644 index 000000000..418087b91 --- /dev/null +++ b/dokimi/changelog.d/20261113_150000_heading-query-artifact.md @@ -0,0 +1,3 @@ +### Fixed + +- **The dpnp 0.0 recall@1 is no longer presented as a finding** — the novelty docs, the `--competitors` help and the `note` of every lexical routing result said every `dpnp-*` skill scored 0.0 against its siblings. That figure came from queries drawn from the skills' section headings, which are one template across the family, so it measured the template, not the descriptions. They now say so and point to queries written independently of the skills. `catalog routing` warns on stderr when half or more of the queries in `--queries-by-skill` are listed for more than one skill, before the estimate and before any model call, since the same text is then the gold of several skills and the result reflects the queries, not the descriptions. diff --git a/dokimi/changelog.d/20261113_160000_shared-queries-in-result.md b/dokimi/changelog.d/20261113_160000_shared-queries-in-result.md new file mode 100644 index 000000000..703e91d78 --- /dev/null +++ b/dokimi/changelog.d/20261113_160000_shared-queries-in-result.md @@ -0,0 +1,3 @@ +### Added + +- **The `catalog routing` result records `shared_queries`** — `{"shared": 9, "total": 9}`: how many of the queries with a content word are listed for more than one skill. The shared-query warning went to stderr only, so a file saved with `--out` from a run on queries drawn from one template read later as a normal own-rate table. Files saved before this change lack the key. Two queries are the same when the lexical scorer reads them as the same content words: case, spacing, trailing punctuation and word order do not tell `Quick Start` from `Quick Start.` or `start quick`, while `Straße` and `STRASSE` stay two queries. diff --git a/dokimi/docs/skill-novelty-walkthrough.md b/dokimi/docs/skill-novelty-walkthrough.md index e0ec89c83..a0e8889c6 100644 --- a/dokimi/docs/skill-novelty-walkthrough.md +++ b/dokimi/docs/skill-novelty-walkthrough.md @@ -46,8 +46,9 @@ such as `.git` are left out, and a byte-identical copy under another path counts `--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 +Read `recall@1` against the competitors, not alone. Sibling skills built from one template +have identical heading queries, so heading-query recall@1 across such a family measures the +template, not the descriptions. Measured on the study corpus, `linux-perf` scored 0.0 recall@1 on five independently written requests — losing three of them to an `xpu-*` skill phrased the way a user phrases the problem. If that is your result, **fix the description before anything else**; `skill novelty analyze` holds the frontmatter fixed and will not see this. diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 2b3f8d39a..737963f02 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -804,7 +804,7 @@ with any failed model call writes the result and exits non-zero. The saved file field rule under `field` (`include_disabled`, `include_invalid`, `keep_copies_under`, the `competitors` and what was `left_out`, which also names each skill left out for a conflicting name or a spec error, and `spec_warnings`, one `{skill, length, limit, message}` per kept skill whose description is over 1024 characters); a single-artifact run -records it too. An all-vs-all run also lists in `field.dropped_queries` each skill whose queries it dropped, as +records it too. An all-vs-all result also records `shared_queries` (`{shared, total}`: how many of the queries with a content word are listed for more than one skill), so a file saved from a run on shared queries says so. An all-vs-all run also lists in `field.dropped_queries` each skill whose queries it dropped, as `{skill, reasons, keep_flags, queries}`, one record per skill, with each reason `disabled`, `invalid` or `conflict`. A query for a catalog skill that failed to load refuses the run and names the load error. It refuses `--artifact`, `--queries` and `--queries-from-headings`: it takes `--queries-by-skill` only. `--estimate-only` prints the calls and the list-price cost of the whole run as JSON, in the shape of a single-artifact estimate, with no client built and @@ -862,8 +862,9 @@ 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 - *sibling*: measured lexically over the 33-skill corpus, every `dpnp-*` skill scored - **0.0** recall@1 against its siblings. + *sibling*. Siblings built from one template have identical heading queries, so heading-query + recall@1 at the family level measures the template; check sibling confusion with queries + written independently of the skills. - **The queries must come from outside the artifact**, or it defines both the problem scope and its own relevance. `--queries-from-headings` exists for triage, warns, and stamps `queries_self_derived: true` into the result; corpus-wide those self-derived queries gave diff --git a/dokimi/dokimi/commands/novelty_routing_report.py b/dokimi/dokimi/commands/novelty_routing_report.py index 37962b66b..7dc1dfe8c 100644 --- a/dokimi/dokimi/commands/novelty_routing_report.py +++ b/dokimi/dokimi/commands/novelty_routing_report.py @@ -279,6 +279,7 @@ def drop_queries_left_out(queries_by_skill: dict[str, list[str]], field: list[Ca other name is more likely a misspelling. Refused too when no query is left to route. Returns the kept queries and one record per dropped skill, for the saved field record: ``{"skill", "reasons", "keep_flags", "queries"}``, so ``queries`` sums to the dropped count. + Warns on stderr of the kept queries several skills share, before any estimate or model call. """ names = {c.name for c in field} # a copy that failed to load may differ from the one that loaded, so the name is not measured @@ -309,6 +310,7 @@ def drop_queries_left_out(queries_by_skill: dict[str, list[str]], field: list[Ca why = ("" if not queries_by_skill else ": the queries name only skills the field leaves " f"out ({_left_out_list(_left_out_only(queried, field))})") raise InputError(f"no skill in the field has queries to route{why}") + warn_shared_queries(kept) return kept, records @@ -661,6 +663,14 @@ def warn_blind_skills(result: dict, queries_by_skill: dict[str, list[str]]) -> N "(no content word in them), so their lexical own_rate is None", file=sys.stderr) +def warn_shared_queries(queries_by_skill: dict[str, list[str]]) -> None: + """Warn on stderr when half or more of the queries are listed for several skills.""" + from dokimi.novelty.query_overlap import shared_queries_warning + + if shared := shared_queries_warning(queries_by_skill): + print(f"WARNING: {shared}", file=sys.stderr) + + def field_record(args: argparse.Namespace, competitors: list[Candidate], left_out: list[str], dropped_queries: list[dict] | None = None, gold: Candidate | None = None) -> dict: """The field rule a routing result was measured under, saved next to the result. diff --git a/dokimi/dokimi/commands/routing_parser.py b/dokimi/dokimi/commands/routing_parser.py index 3da8d35a6..142ce0744 100644 --- a/dokimi/dokimi/commands/routing_parser.py +++ b/dokimi/dokimi/commands/routing_parser.py @@ -60,8 +60,8 @@ def _add_routing_options(routing_p, positive_int, refused: bool = False) -> None """ routing_p.add_argument("--competitors", nargs="+", default=None, type=_skill_ref, help="Other artifacts the selector sees. One candidate is not " - "routing: sibling confusion is the failure mode (every " - "`dpnp-*` skill scored 0.0 lexically). A bare path is a skill " + "routing: sibling confusion is the failure mode (check it " + "with independently written queries). A bare path is a skill " "(its directory or SKILL.md); text:PATH is also accepted") routing_p.add_argument("--include-disabled", action="store_true", dest="include_disabled", help="Keep skills with disable-model-invocation in the field. Off by " diff --git a/dokimi/dokimi/novelty/all_vs_all.py b/dokimi/dokimi/novelty/all_vs_all.py index b657a34be..75e5674ec 100644 --- a/dokimi/dokimi/novelty/all_vs_all.py +++ b/dokimi/dokimi/novelty/all_vs_all.py @@ -17,6 +17,7 @@ from pathlib import Path from .client import LLMClient +from .query_overlap import shared_query_counts from .routing import ( BM25, Candidate, @@ -88,7 +89,9 @@ def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str attractor, and their queries are no sibling queries of each other. Returns: - ``n_skills``, ``n_queries``, ``tied_groups``, and per arm (``lexical``, ``model``): + ``n_skills``, ``n_queries``, ``shared_queries`` (``shared``: how many queries, of the + ``total`` that have a content word, are listed for two or more skills; when most are, the + result reflects the queries, not the descriptions), ``tied_groups``, and per arm (``lexical``, ``model``): ``confusion`` (gold skill to picked skill to count, ``(none)`` and ``(tie)`` included), ``own_rate`` per gold skill (``None`` when none of its queries was scored), ``n_scored`` and ``n_unscored`` per gold skill, and ``attractors`` ordered by ``wrongly_picked``. The @@ -107,9 +110,11 @@ def all_vs_all(candidates: list[Candidate], queries_by_skill: dict[str, list[str """ names = [c.name for c in candidates] _check_all_vs_all_input(names, queries_by_skill) + shared, scored = shared_query_counts(queries_by_skill) result: dict = { "n_skills": len(candidates), "n_queries": sum(len(q) for q in queries_by_skill.values()), + "shared_queries": {"shared": shared, "total": scored}, "tied_groups": _tied_groups(candidates), "lexical": _attractor_table(*_lexical_picks(candidates, queries_by_skill, names), names, _tied_groups(candidates)), diff --git a/dokimi/dokimi/novelty/query_overlap.py b/dokimi/dokimi/novelty/query_overlap.py new file mode 100644 index 000000000..d8dcbd3a2 --- /dev/null +++ b/dokimi/dokimi/novelty/query_overlap.py @@ -0,0 +1,51 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Queries that several skills share: a field-wide run on them says nothing about descriptions.""" + +from __future__ import annotations + +from dokimi.novelty.routing import tokenize + +# Share of the given queries listed for more than one skill at which a run is warned. +SHARED_QUERY_WARN_SHARE = 0.5 + + +def _key(query: str) -> tuple[str, ...]: + return tuple(sorted(tokenize(query))) + + +def shared_query_counts(queries_by_skill: dict[str, list[str]]) -> tuple[int, int]: + """``(shared, total)``: the queries listed for two or more skills, and all the queries. + + A query counts as shared when the lexical scorer reads it as the same content words (case, + spacing, punctuation and word order aside; a repeated word still counts) as a query of another skill. Queries with no content word are + not scored by either arm and are in neither count. + """ + scored = {name: [q for q in queries if tokenize(q)] for name, queries in queries_by_skill.items()} + owners: dict[tuple[str, ...], set[str]] = {} + for name, queries in scored.items(): + for query in queries: + owners.setdefault(_key(query), set()).add(name) + total = sum(len(queries) for queries in scored.values()) + shared = sum(1 for queries in scored.values() for query in queries + if len(owners[_key(query)]) > 1) + return shared, total + + +def shared_queries_warning(queries_by_skill: dict[str, list[str]]) -> str | None: + """Warning text when half or more of the queries are shared by several skills, else ``None``. + + Sibling skills built from one template have identical heading queries, so the same text is + the gold of several skills and the result reflects the template, not the descriptions. + """ + shared, total = shared_query_counts(queries_by_skill) + if len(queries_by_skill) < 2 or not total: + return None + share = shared / total + if share < SHARED_QUERY_WARN_SHARE: + return None + return (f"{shared} of {total} queries ({share:.0%}) are listed for more than one skill, as " + "when sibling skills built from one template give the same heading queries: the same " + "text is the gold of several skills, so the result reflects the queries, not the " + "descriptions; use queries written independently for each skill") diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index e417f775f..5435a6538 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -14,9 +14,9 @@ * **Lexical** — BM25 over each candidate's `name + description`, which is the text a tool schema shows. Free, deterministic, and a floor: measured over the 33-skill study corpus it ranked the right skill first for only 0.225 of queries drawn from the skills' own - section headings, and every `dpnp-*` sibling scored 0.0. A description that cannot beat - BM25 among its siblings is not discriminating; one that can has not yet been shown to be - picked by a model. + section headings. Siblings from one template (`dpnp-*`) share heading queries, so for a + family that figure measures the template. A description that cannot beat BM25 among its + siblings on independent queries is not discriminating; one that can is not yet shown picked. * **Model** — ask a model to choose from the same cards. This is the number people mean by "routing", and it is an **upper bound on harness behaviour**: a harness that truncates or omits descriptions in its own skill listing selects on less than this measurement shows, @@ -196,8 +196,8 @@ def lexical_routing( "per_query": per_query, "note": ( "A floor, and free. Over the 33-skill study corpus BM25 put the right skill " - "first for 0.225 of queries drawn from the skills' own headings, and 0.0 for " - "every `dpnp-*` sibling: descriptions discriminate a domain, not a sibling." + "first for 0.225 of queries drawn from the skills' own headings. Template siblings " + "share heading queries: check sibling confusion with independently written ones." ), } diff --git a/dokimi/tests/test_novelty_routing_shared_queries.py b/dokimi/tests/test_novelty_routing_shared_queries.py new file mode 100644 index 000000000..2d728e35e --- /dev/null +++ b/dokimi/tests/test_novelty_routing_shared_queries.py @@ -0,0 +1,133 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Heading queries shared by sibling skills: the warning, and a lexical note that claims no finding.""" + +from __future__ import annotations + +import json + +from dokimi.cli import build_parser +from dokimi.commands.novelty_routing_report import warn_shared_queries +from dokimi.novelty.all_vs_all import all_vs_all +from dokimi.novelty.query_overlap import ( + SHARED_QUERY_WARN_SHARE, + shared_queries_warning, + shared_query_counts, +) +from dokimi.novelty.routing import Candidate, headings_as_queries, lexical_routing + +TEMPLATE = "## Quick Start\n\n## Implementation Guide\n\n## Gotchas & Limitations\n" + + +def test_sibling_skills_on_one_template_give_identical_heading_queries(): + queries = headings_as_queries(TEMPLATE) + warning = shared_queries_warning({"dpnp-a": queries, "dpnp-b": queries, "dpnp-c": queries}) + assert warning is not None + assert "9 of 9 queries (100%)" in warning + assert "same rank" not in warning + assert "independent" in warning + + +def test_case_and_spacing_do_not_hide_a_shared_query(): + warning = shared_queries_warning({"a": ["Quick Start"], "b": ["quick start"]}) + assert warning is not None + assert "2 of 2" in warning + + +def test_trailing_punctuation_does_not_hide_a_shared_query(): + assert shared_query_counts({"a": ["Quick Start"], "b": ["Quick Start."]}) == (2, 2) + + +def test_word_order_does_not_hide_a_shared_query(): + assert shared_query_counts({"a": ["quick start"], "b": ["start quick"]}) == (2, 2) + + +def test_a_repeated_word_keeps_a_query_apart_from_the_single_word(): + assert shared_query_counts({"a": ["start start"], "b": ["start"]}) == (0, 2) + + +def test_queries_the_scorer_tokenizes_differently_are_not_shared(): + assert shared_query_counts({"a": ["Straße"], "b": ["STRASSE"]}) == (0, 2) + + +def test_the_threshold_is_half_of_the_queries_listed_for_several_skills(): + assert SHARED_QUERY_WARN_SHARE == 0.5 + at = {"a": ["shared one", "own a1"], "b": ["shared one", "own b1"]} + below = {"a": ["shared one", "own a1", "own a2"], "b": ["shared one", "own b1", "own b2"]} + assert shared_queries_warning(at) is not None # 2 of 4 + assert shared_queries_warning(below) is None # 2 of 6 + + +def test_queries_written_per_skill_draw_no_warning(): + assert shared_queries_warning({"a": ["profile a binary"], "b": ["slice an array"]}) is None + + +def test_one_skill_or_no_queries_draw_no_warning(): + assert shared_queries_warning({"a": ["x y", "x y"]}) is None + assert shared_queries_warning({"a": [], "b": []}) is None + + +def test_the_warning_does_not_claim_equal_ranks_the_run_does_not_give(): + cands = [Candidate("a", "alpha"), Candidate("b", "gamma")] + queries = {"a": ["alpha"], "b": ["alpha"]} + rates = all_vs_all(cands, queries)["lexical"]["own_rate"] + assert rates["a"] != rates["b"] + assert "same rank" not in shared_queries_warning(queries) + + +def test_queries_with_no_content_word_are_not_counted(): + queries = {"a": ["the", "and", "profile a binary"], "b": ["the", "and", "slice an array"]} + assert shared_queries_warning(queries) is None + + +def test_the_warning_goes_to_stderr(capsys): + shared = headings_as_queries(TEMPLATE) + warn_shared_queries({"dpnp-a": shared, "dpnp-b": shared}) + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err.startswith("WARNING: 6 of 6 queries (100%)") + + +def test_the_lexical_note_does_not_present_the_family_zero_as_a_finding(): + result = lexical_routing([Candidate("a", "alpha beta"), Candidate("b", "gamma")], + ["alpha beta"]) + assert "0.0 for every" not in result["note"] + assert "independently" in result["note"] + + +def _run_all_vs_all(tmp_path, *extra): + root = tmp_path / "catalog" + for name in ("a", "b"): + skill_dir = root / "skills" / name + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: tune {name} things\n---\n\n# B\n", encoding="utf-8") + queries = tmp_path / "q.json" + queries.write_text(json.dumps({"a": ["quick start"], "b": ["quick start"]}), encoding="utf-8") + args = build_parser().parse_args( + ["catalog", "routing", str(root), + "--queries-by-skill", str(queries), "--out", str(tmp_path / "out.json"), *extra]) + args.func(args) + + +def test_all_vs_all_result_records_the_shared_query_counts(): + cands = [Candidate("a", "alpha"), Candidate("b", "beta")] + queries = {"a": ["alpha one", "the", "own first"], "b": ["Alpha one", "own second"]} + assert all_vs_all(cands, queries)["shared_queries"] == {"shared": 2, "total": 4} + + +def test_a_saved_all_vs_all_file_carries_the_shared_query_counts(tmp_path): + _run_all_vs_all(tmp_path) + saved = json.loads((tmp_path / "out.json").read_text(encoding="utf-8")) + assert saved["result"]["shared_queries"] == {"shared": 2, "total": 2} + + +def test_all_vs_all_warns_of_shared_queries_on_stderr(tmp_path, capsys): + _run_all_vs_all(tmp_path) + assert "WARNING: 2 of 2 queries (100%)" in capsys.readouterr().err + + +def test_estimate_only_warns_of_shared_queries_before_returning(tmp_path, capsys): + _run_all_vs_all(tmp_path, "--estimate-only") + assert "WARNING: 2 of 2 queries (100%)" in capsys.readouterr().err