[napetrov]

Summary

One invented repo per shape, each run through catalog scan and novelty routing --catalog:

catalog/                                  shape: byte_identical_mirror
├── skills/widget-polish/SKILL.md
├── plugins/kit/skills/widget-polish/SKILL.md   (byte-identical copy)
└── skills/door-check/SKILL.md
  scan    -> 3 rows, one with duplicate_of set, exit 0
  routing -> field = [door-check, widget-polish], exit 0

Thirteen shapes in tests/skill_repo_shapes.py: mirrors (identical and drifted), same name in two directories, disable-model-invocation, template/, vendored directories, a zero-skill repo, plugins/*/skills nesting, an over-long description, a name that is not the directory name, a title-case name, links that leave the skill directory, and a skill with no headings.

tests/test_skill_repo_shapes.py checks two things. A sweep runs both commands, lexical and with a stub selector, over every shape and asserts that nothing raises. The expectation table asserts the exit codes and the routing field each shape should get.

Evidence

Shapes that fail on intel/main are strict xfails. Each fails on the bug itself, checked with --runxfail:

• drifted_mirror, same_name_distinct_dirs: routing exits 2 with catalog has widget-polish at … and … with different content; dedupe or rename. Marked fixed by fix/rescan-catalog.
• zero_skill: catalog scan exits 0 with no skills and no note. Marked fixed by fix/rescan-catalog.
• description_too_long, name_not_dir, title_case_name: the skill that fails the spec check still appears in the routing field. Marked fixed by fix/rescan-routing.

Not xfailed: the all-vs-all disabled-skill case. intel/main already names --include-disabled in that refusal (tests/test_novelty_routing_all_vs_all.py covers the message), so it is a plain regression test here.

Local checks on 793bc74f:

• ruff check .: passed
• mypy dokimi: Success: no issues found in 303 source files
• pytest -n auto -q: 12032 passed, 27 skipped, 6 xfailed in 66.20s
• Self-review (pr-review local, two passes on 793bc74f): no blocking: finding. The three plain-pass rows are the intended xfails; the duplicate-keying difference in review_skills vs routing is pre-existing and left for a follow-up.

Merge Danger

Door: two-way. Tests and a changelog fragment only; a revert removes them and nothing else.

Blast Radius: none. No package code, data or CLI behaviour changes. When the fix/rescan-catalog and fix/rescan-routing fixes land, their six strict xfails must be removed in the same PR. Otherwise they fail as XPASS.

Checklist

• Tests added/updated and pytest passes
• ruff, mypy, and schema checks pass (ruff and mypy run; schema check not run, no schema changed)
• Docs updated (or N/A): N/A, no user-facing behaviour changed
• Self-reviewed per "Before opening a PR" in AGENTS.md
• No ad-hoc run artifacts committed
• New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

🤖 Generated with Claude Code

[exai-bot]

✅ OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s).

Review details

• Commit reviewed: f71a36c against merge base 05f9d62
• Coverage: 2 files in 2 groups
• Engine: OpenCodeReview v1.13.3, us.openai.gpt-6.1-sol
• Usage: 101k tokens (49k from cache), 18s
• Tool calls: 5 (4 file_read, 1 code_search)
• dokimi/tests/skill_repo_shapes.py: 1 file
• dokimi/tests/test_skill_repo_shapes.py: 1 file

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.06%. Comparing base (05f9d62) to head (f71a36c).
⚠️ Report is 1 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #327      +/-   ##
==========================================
+ Coverage   94.05%   94.06%   +0.01%     
==========================================
  Files         297      297              
  Lines       38018    38018              
  Branches     6424     6424              
==========================================
+ Hits        35756    35762       +6     
+ Misses       1453     1449       -4     
+ Partials      809      807       -2     

see 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[antonwolfy]

LGTM. The thirteen shapes are well chosen, the stub selector really replaces the model client, and all six strict xfails fail on the bug they name. Three nits inline.

What I ran, on a Linux review host at 793bc74:

• make ci-local: 30 passed, 0 failed, 7 skipped; 12000 passed, 59 skipped, 6 xfailed; changed-line coverage step passed.
• The six xfails re-run with --runxfail: each fails on the stated dimension (routing exit 2 "different content; dedupe or rename", zero-skill scan exit 0, over-long / mismatched / title-case skill kept in the field).
• Mutations of the code under test: dropping routing dedupe, duplicate_of, disabled-skill exclusion, or the no-queries refusal each fails a test. Flipping the mirror sort key in external/validate.py passes the whole file (see inline).

Checked, no issues

• Stub selector: novelty_cmd._client is looked up as a module global, so the monkeypatch takes effect; no real client is built.
• Shapes: template_dir is excluded by directory name, the .venv half of vendored_dir is a real guard, and strict scan exits 1 for the three spec-error shapes.
• Rules: SPDX headers, writes only under tmp_path, growth ratchet, DCO sign-off on the one commit.

[Copilot]

🟡 Changes recommended

A critical test assertion fails on Windows because serialized paths use backslashes while the expected suffix uses forward slashes, even when mirror detection is correct.

1 open issue

• Use platform-independent path comparison in mirror assertion · New

What changed in this PR

1 new finding.

This PR adds regression coverage for catalog scanning and novelty routing across synthetic skill-repository layouts, without changing package behavior.

Changes:

• Defines 13 repository shapes covering mirrors, invalid metadata, excluded directories, and edge cases.
• Exercises scan modes and lexical/stub-model routing, with explicit exit-code and competitor expectations.
• Tracks six known failing shapes with strict expected failures.

File	Description
dokimi/​tests/​test_skill_repo_shapes.py	Adds command sweeps, expected outcomes, and focused regression assertions.
dokimi/​tests/​skill_repo_shapes.py	Defines synthetic repository layouts and shared probe fixtures.

_{Review effort: Balanced}

[antonwolfy]

LGTM. I pushed a commit per nit, up to f71a36c, which fix the three nits from my review:

• e667713: dokimi/changelog.d/20261031_120000_napetrov_skill-repo-shapes.md: deleted the changelog fragment.
• 30906cf: dokimi/tests/test_skill_repo_shapes.py:191: after the count, assert the duplicate row's path is the plugins/kit/skills/widget-polish copy and its duplicate_of is skills/widget-polish.
• 07c1b3c: dokimi/tests/test_skill_repo_shapes.py:171: the strict xfail marks now carry raises=AssertionError.

The review bot then flagged lines of those commits; fixed as well:

• f71a36c: dokimi/tests/test_skill_repo_shapes.py:193: the mirror path is now compared as a Path against the expected path under root, not as a forward-slash suffix.

make ci-local at f71a36c: 30 passed, 0 failed, 7 skipped.