ci: report skill pairs that drive the same actions with no hand-off (advisory)

[xaleryb]

What this adds

tools/lint_skill_overlap.py, wired into validate.yml as two steps: a blocking self-test
and an advisory report.

Two skills driving the same tool with the same flags compete for the same request. That is
normal here and most of this catalog does it — as long as one of them says which is which.
vllm-xpu-bench and vllm-xpu-run name each other; torch-xpu-run and torch-xpu-profile
never have, and an agent holding both has nothing to route on. That is the only thing this
check looks for: undeclared adjacency, not duplication.

So it compares what each skill does rather than what it says — the commands, flags,
environment variables, API calls and endpoint paths in its code, from fenced blocks, inline
spans and bundled .sh/.py/.md — then subtracts every pair where a hand-off is written
down in either description.

containment = |A ∩ B| / min(|A|, |B|)

It does not block

A finding is annotated on the pull request and the merge proceeds. Which of two overlapping
skills should win — merge, split, or leave alone — is a judgement about the catalog that a
reviewer makes with the contributor, and a threshold that made it for them would be wrong in
both directions: blocking a legitimate composition pair, and passing the restatement in prose
it cannot see at all.

What does block is the detector's own --self-test. Every way this check breaks is silent —
a fence regex that stops matching scores every pair 0, which reads exactly like a catalog
with no duplication — so a detector that has stopped detecting is not a judgement call.

On this tree today the report is one line: torch-xpu-bench | torch-xpu-run, containment
0.3636 over 8 shared actions, undeclared in both directions, both sides imported so the
repair belongs upstream.

Proof it works, not just that it runs

16 assertions in --self-test against skills/ as committed, and --mutate M1…M10 so
each way of breaking it can be shown to turn red:

	broken on purpose	caught by
M1	PLATFORM = set()	threshold band, inherited-hand-off
M2	read Python fences as shell	element extraction
M3	delete the flag: regex	element extraction
M4 / M5	threshold 0.30 / 0.95	threshold band
M6 / M7	inject / delete a hand-off line	hand-off graph
M8	let an inherited hand-off excuse a verbatim copy	subsumption rule
M9	score only pairs whose names share a part	name-does-not-gate
M10	index hand-offs from half of each body	index ↔ scan agreement

Beyond the fixtures, seven candidate skills were written into the real tree and the real CI
command run against each: a byte-for-byte copy, a paraphrase, a narrow slice, a declared
duplicate, an imported drift, a genuinely different server, and a prose-only restatement.
The copy, the paraphrase, the slice and the declared duplicate all score containment 1.0000
and are reported; the different server is silent; every run exits 0.

Two defects in this design were found that way rather than by review, and are now fixtures:

• A copied file declares its own hand-off. A byte-for-byte copy of vllm-xpu-run
  carries that name in its own text, so the hand-off rule silenced it. At containment 1.0
  there is no division of labour for a hand-off to describe, so total subsumption is now
  reported even when declared (DECLARED_COPY, M8).
• Two calibration assertions could become jointly unsatisfiable as the catalog grows,
  leaving a self-test fixable only by editing the test. Replaced with one band read off the
  tree: ceiling < threshold ≤ 1.5 × ceiling.

The threshold is not a constant to defend

0.65 is the next 0.05 step above the highest-scoring judgeable pair in the tree
(dpnp-quickstart | dpnp-troubleshooting, 0.6154), so nothing already merged is reported.
That ceiling was measured to rise with catalog size — 0.36 at 12 skills, 0.56 at 20, 0.62 at
33, median over 40 random subsets of real content — so the self-test bounds the threshold to
1.0–1.5× of it and fails when the catalog grows into it, which is the signal to raise the
number. A verbatim copy scores 1.0 and is reported regardless of where the threshold sits.

Name and description: a reading order, not a filter

The cheapest imaginable signal is read, and deliberately given no authority. Measured on this
tree:

• an exact name collision is already impossible — validate_skills.py requires name to
  equal the directory name, and directories are unique
• a shared-name precondition would drop 4 of the 14 judgeable pairs, including
  linux-perf | performance-patterns — M9 keeps that honest
• the two axes agree on 2 pairs of 14 at the top of this tree
• the near-verbatim fixture shares 0.05 of its description with the skill it copies while
  sharing 1.0000 of its actions

So they order a short queue of the pairs the action axis cannot judge — the four skills here
with too little code to compare, and any prose-only contribution — printed as a reading order
that settles nothing. It costs 0.2 s at 300 skills, and it is the only affordable way to pick
the few pairs anything downstream that pays per pair should look at.

Cost

The hand-off detector is indexed rather than re-scanned per pair, which is where the cost
turned out to live:

skills	pairs	before	after
33	528	0.79 s	0.02 s
300	44 850	22.04 s	1.31 s
600	179 700	—	5.64 s

--self-test asserts the index agrees with scanning every pair, rather than trusting that it
does.

What it cannot do

Stated because the alternative is a check that gets trusted for more than it proves.

• A restatement in different words is out of reach. A prose-only skill has no actions to
  compare. Embeddings do not close this: on this catalog potion-base-8M scored a legitimate
  pair 0.9000 and a near-verbatim copy 0.8877 — it ranked the legitimate pair higher. The
  instrument that would work is a model reading both, which no keyless gate can have.
• The reported half and the actionable half barely overlap. A pair needs 8 shared actions
  on the smaller side, and the imported skills carry far more signature material than the ones
  authored here, so today the check's reach is a new contribution and the dpnp family.
• A hand-off line that is a lie buys silence. One sentence naming a neighbour suppresses
  the finding, and only a reviewer can tell whether the division of labour it claims is real.
• The extractor was tuned after seeing which pairs it reported. Each removal in PLATFORM
  is defensible as a shape rule rather than a fix for a specific pair, and the fixtures plus
  M1–M10 are what keep that tuning honest — but it is tuning, and it is recorded here rather
  than in a commit message nobody reads.

Not in this PR

Candidate-vs-catalog mode (against() exists in the code, unwired), delta-vs-base for sync
pull requests, a suppression file, and any model-based tier — a fork PR gets no secrets, so a
model tier cannot run where this one does.

Docs updated in the same change: CONTRIBUTING.md (local gate, and the not-blocking list)
and MAINTAINERS.md (CI table, tools table, and what the check cannot do).

Draft: opened for review of the shape before the numbers are argued.

[napetrov]

Reviewed at 8a51139. Ran it against skills/ as committed, plus injected skills, to check the claims rather than read them.

What already works, verified locally:

• --self-test: 16 assertions, all pass, 0.76 s. Every mutation M1–M10 is caught (exit 1). Making that a separate blocking step ahead of the advisory gate is the right shape — the gate can be advisory precisely because the detector's own liveness is not.
• The gate itself is 0.20 s over 435 scored pairs, no network, no dependencies.
• On today's tree it produces exactly one finding, torch-xpu-bench | torch-xpu-run at 0.3636, both imported → WARN. So it merges green today and does not start life noisy.
• The hand-off-index shortcut agrees with the naive all-pairs scan on all 528 pairs, and the containment-1.0 rule correctly refuses to let a copy excuse itself with an inherited hand-off. Both are asserted, not just claimed.

Five things to settle before merge. Only the first changes behaviour; the rest are calibration, reporting and scope.

1. A verbatim copy of an authored skill merges green. Measured: copy skills/dpnp-memory/ to skills/dpnp-memory-tuning/, reword the description so it does not name the original, and the run reports REVIEW overlap 1.0000 ... everything the smaller one does, the larger one already does — and exits 0, because the workflow passes --advisory. Containment 1.0 with no hand-off is not the judgement call --advisory exists to defer; it is the same category as the self-test. Detail inline.
2. --max-overlap 0.65 sits 0.035 above the lower edge of the band its own self-test enforces, and the self-test step is blocking. Detail inline.
3. --min-shared cannot reach below the hardcoded 5-action floor, and 93 of 528 pairs are never scored without that being visible in the summary. Detail inline.
4. --strict cannot be adopted as written: on a clean tree it exits 1 on a pair nobody here can fix. Detail inline.
5. One scope sentence in the module docstring, so a future reader does not read "0 findings" as "no duplication". The action axis is blind to duplicated prose by construction — that is a boundary worth stating, not a gap to close in this PR. Detail inline.

Exit-code matrix as measured on this tree, for the record:

flags	clean tree	with an authored 1.0 copy injected
--advisory	0	0
--advisory --strict	1	1
--strict	1	1
(none)	0	1

Nothing here is a redesign request — the action-signature extractor is the part of this that no off-the-shelf library replaces, and it should land.

[napetrov]

Containment 1.0 on an authored pair should block, even under --advisory.

Measured on this tree: cp -r skills/dpnp-memory skills/dpnp-memory-tuning, change name:, reword description: so it does not name the original. Result:

REVIEW overlap 1.0000 dpnp-memory | dpnp-memory-tuning: everything the smaller one
does, the larger one already does, so the hand-off cannot be what separates them.
$ echo $?
0

--advisory is well argued for the middle of the range — which of two overlapping skills wins really is a reviewer's call. But containment 1.0 with no hand-off is not in that range. The larger skill already does everything the smaller does; there is no division of labour to weigh, which is exactly the reasoning in this docstring. verdict() already separates that case (pair["containment"] >= 1.0 survives a declared hand-off); summarize() then throws the distinction away because --advisory short-circuits on needs_review as a whole.

Suggestion: --advisory suppresses failure for REVIEW below 1.0, and returns 1 for REVIEW at containment >= 1.0 (pair["authored"] only, so an upstream pair stays a WARN). Same argument that makes --self-test a blocking step: subsumption is not a judgement call.

[napetrov]

The threshold is 0.035 above the edge of a band the blocking step enforces.

--self-test asserts ceiling < budget <= 1.5 * ceiling where ceiling is the highest judgeable containment in the tree. Measured band today: (0.6154, 0.9231], ceiling set by dpnp-quickstart | dpnp-troubleshooting at 0.6154.

--max-overlap 0.60 -> self-test FAIL   (margin 0.97x)
--max-overlap 0.62 -> ok
--max-overlap 0.65 -> ok               <- what the workflow passes, margin 1.06x
--max-overlap 0.92 -> ok
--max-overlap 0.93 -> self-test FAIL   (margin 1.51x)

So 0.65 leaves 0.035 of headroom below and 0.27 unused above. The dpnp-* family is 8 skills over one API surface; the ninth that reuses dpnp.array/asnumpy/arange raises ceiling past 0.65 and the blocking self-test step fails on a PR that has nothing to do with this linter — and the failure is on the workflow file, not on the skill the author touched.

Two suggestions, either is fine:

• move the number to the middle of the band (~0.75) so ordinary catalog growth does not trip it; or
• keep 0.65 and make the self-test failure self-servicing: print the exact replacement value (set --max-overlap to 0.75), since the assertion already computes ceiling and knows the legal range.

Same value also appears in the module docstring usage line and MAINTAINERS.md; the self-test's "workflow names the threshold exactly once" check covers the workflow but not those copies.

[napetrov]

Two floors, and --min-shared cannot reach past this one.

--min-shared is a flag with a default of 8; this smaller < 5 is a second, invisible floor at a different granularity (signature size, not shared count). Measured:

--min-shared 3 -> 435 scored pair(s)
--min-shared 5 -> 435 scored pair(s)
--min-shared 8 -> 435 scored pair(s)

435 of 528, identically. So 93 pairs are never scored, and lowering --min-shared below 5 silently does nothing. Nothing in the summary line says so — 435 scored pair(s) reads as a total, not as a remainder.

To be fair, --self-test does name the affected skills (skills below the min-shared reach are named) and the where actions cannot judge queue surfaces some of these pairs at gate time, which is more than most linters do. Two small things would close it:

• name the constant (MIN_SIGNATURE = 5) with the reason next to it, and either clamp --min-shared to it or reject --min-shared < MIN_SIGNATURE in build_parser(), so the flag cannot lie about its reach;
• add the count to the summary: 435 scored, 93 pair(s) below the 5-action floor. It costs one number and stops "0 findings" from meaning "nothing was compared" for the thin end of the catalog.

Worth noting signature() handles bundled files correctly — linux-perf yields 123 actions because its references/*.md are read. The floor only bites genuinely thin skills, which is the intent; it just needs to be counted out loud.

[napetrov]

--strict as written cannot be turned on.

Measured on this tree with no injections:

$ python3 tools/lint_skill_overlap.py --max-overlap 0.65 --min-shared 8 --strict; echo $?
OK 33 skill(s), 435 scored pair(s), 1 undeclared overlap(s) reported, worst
torch-xpu-bench | torch-xpu-run at 0.3636, none over 0.65.
1

The single undeclared pair is torch-xpu-bench | torch-xpu-run, both imported — the report itself says (upstream - both are imported) and the fix is a description edit in someone else's repo. So --strict fails on something no PR here can resolve, which makes it unusable as a follow-on step rather than a stricter mode.

undeclared counts WARN and REVIEW alike; needs_review already filters on pair["authored"]. Scoping --strict the same way (args.strict and any(p["authored"] for p in undeclared)) makes it adoptable, and keeps the upstream pair visible as a WARN.

Also: --advisory --strict exits 1 — --strict wins. That is defensible, but --advisory's help says "report and exit 0" unconditionally. Four words there ("unless --strict is given") prevents someone reading the help and concluding the pair is contradictory.

[napetrov]

One sentence of scope in the docstring.

The docstring is unusually good at saying what the check does; it does not say what it cannot see. Concretely, on this tree the pair with the most near-duplicate prose is invisible to this check:

pair	duplicated block	containment here	shared actions
torch-xpu-bench | vllm-xpu-bench	the Env vars table, already drifted in wording between the two copies	0.0833	3
torch-xpu-profile | vllm-xpu-profile	same	0.1250	2
dpnp-io | dpnp-memory	shared links table	0.3000	6

That is correct behaviour, not a bug: the two skills drive different tools, so an action-signature axis should score them low. But it means "0 findings above 0.65" says nothing about text maintained in two places, and the summary line does not hint at the difference. A sentence like "scope: shared actions, not shared text — two skills can duplicate a whole section and score near zero here" keeps the next reader from over-reading a clean run.

No change to the code, and I would not hold the PR on it.