[napetrov]

Summary

novelty routing used to leave a skill out of the field by default when its description is over 1024 characters, the Agent Skills spec limit. Harnesses do not agree on this: Claude Code 2.1.318 lists such a skill and cuts the description to about 1,536 characters, while pi.dev reportedly rejects it (anthropics/skills#881, #1622). So leaving the skill out measured every harness as the strictest one.

	Before	After
Only spec error is the description length	left out unless --include-invalid	kept with its full description; WARNING on stderr; field.spec_warnings in the JSON
Description length plus another spec error	left out	left out (unchanged)
Skill under test (--artifact) over the limit	no warning	WARNING on stderr and listed in field.spec_warnings
catalog scan name-conflict	counted such a copy as not listed	counts it as listed, the same as routing

Evidence

• New tests fail on base and pass on head; make ci-local: 15717 passed
• Fixtures that used a long description as their stand-in spec error now use a name/directory mismatch, so they still exercise a real error
• Probe: a 2471-char description in Claude Code 2.1.318 is listed and truncated, not missing (~$0.10)

Merge danger

• A run whose field used to drop an over-long skill now includes it, so its rates change. The warning and spec_warnings say why; re-run a saved baseline before comparing
• Not in this PR: no harness profiles (truncate or drop) yet

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

There are no active review issues requiring changes.

No open issues 🎉

What changed in this PR

0 new findings; nothing to fix.

This PR updates dokimi’s routing evaluation to retain skills whose only specification error is a description over 1024 characters, accommodating differences in how harnesses handle them.

Changes:

• Keeps full over-limit descriptions in routing fields while continuing to exclude skills with other specification errors by default.
• Adds stderr warnings and saved field.spec_warnings, and aligns catalog name-conflict detection with routing eligibility.
• Updates regression tests, documentation, and the changelog for the revised behavior.

File	Description
dokimi/​tests/​test_skill_repo_shapes.py	Updates routing expectations for over-limit descriptions.
dokimi/​tests/​test_novelty_routing_field.py	Tests retention, warnings, saved metadata, and other-error exclusion.
dokimi/​tests/​test_novelty_routing_all_vs_all.py	Updates invalid fixtures and expected field metadata.
dokimi/​tests/​test_catalog_validate.py	Tests name conflicts involving over-limit descriptions.
dokimi/​dokimi/​novelty/​routing.py	Retains length-only violations and generates warning records.
dokimi/​dokimi/​external/​validate.py	Uses shared eligibility rules for name conflicts.
dokimi/​dokimi/​external/​listing.py	Defines shared default listing eligibility.
dokimi/​dokimi/​commands/​novelty.py	Uses the shared field serializer and updates CLI help.
dokimi/​dokimi/​commands/​novelty_routing_inputs.py	Emits warnings and saves specification-warning metadata.
dokimi/​docs/​skill-novelty.md	Documents retention behavior and saved warnings.
dokimi/​docs/​external-content.md	Clarifies the catalog name-conflict exception.
dokimi/​changelog.d/​20261104_120000_napetrov_desc-over-limit-kept.md	Records the routing and catalog behavior changes.

_{Review effort: Balanced}

[exai-bot]

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

CI at 20ec659: 15 passing, 2 skipped.

Review details

• Commit reviewed: 20ec659 against merge base 040f86d
• Coverage: 12 files in 2 groups
• Engine: OpenCodeReview v1.13.5-exai.3.d7112371, us.openai.gpt-6.1-sol
• Usage: 276k tokens (204k from cache), 28s
• Tool calls: 11 (6 file_read, 3 code_search, 2 file_read_diff)
• External content listing and description validation: 6 files
• Novelty routing inputs, behavior, and documentation: 6 files

[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.79%. Comparing base (7fa98b9) to head (20ec659).
⚠️ Report is 2 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     #405   +/-   ##
=======================================
  Coverage   94.78%   94.79%           
=======================================
  Files         353      355    +2     
  Lines       46527    46606   +79     
  Branches     7967     7976    +9     
=======================================
+ Hits        44100    44179   +79     
  Misses       1540     1540           
  Partials      887      887           

Files with missing lines	Coverage Δ
dokimi/dokimi/commands/novelty.py	97.45% <100.00%> (-0.25%)	⬇️
dokimi/dokimi/commands/novelty_routing_inputs.py	88.15% <100.00%> (+1.20%)	⬆️
dokimi/dokimi/external/listing.py	100.00% <100.00%> (ø)
dokimi/dokimi/external/validate.py	96.46% <100.00%> (+<0.01%)	⬆️
dokimi/dokimi/novelty/routing.py	96.35% <100.00%> (+0.05%)	⬆️

... and 2 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 rule is one function (only_description_too_long), and the routing field, the explicit skill: competitors, the catalog path and catalog scan's name-conflict check all go through it. A description over 1024 characters with any other spec error still leaves the skill out. Two nits inline.

What I ran, on a Linux review host at 8db4eb4:

• make ci-local: all steps pass except ci.yml:test / Run tests. The whole suite ran to 100% with no test failure; the step then exited 1 on an unraisable ResourceWarning (unclosed scandir iterator) in pytest's temp-dir cleanup at session end. It happened under Python 3.13 and no test failed; the PR touches nothing that opens a directory scan. Changed-line coverage is 100% on all five source files.
• A mutation run: 16 mutants of only_description_too_long, in_field, competitor_left_out, _left_out_by_flag, _name_conflicts, over_limit_warnings and field_record against the four touched test files. 14 are killed (including all() to any(), dropping the guard in either path, and emptying the warning or spec_warnings). The survivors are the > to >= boundary (inline) and dropping bool(errors), which cannot be reached because both callers check state == "error" first.
• Reproductions with the LLM transport mocked: a long description plus a second error is left out; --include-invalid keeps both and warns; a byte-identical mirror warns once; the warning prints once per run.
• Commit has a Signed-off-by. No schemas/ file covers field.spec_warnings, so no schema bump is needed. The changelog fragment follows the README format.

Checked, no issues

• Imports: dokimi.novelty.routing, external.listing and external.validate each import on their own; listing.py imports SkillReview only under TYPE_CHECKING.
• Help and docs: the --include-invalid help text, skill-novelty.md and external-content.md agree with the code on the exception.

[napetrov]

Not reproduced on head: py3.12 suite with -W error::ResourceWarning passes (15722, 0 warnings); CI py3.11/3.14 green. Send the py3.13 traceback if it persists.

[vlad-perevezentsev]

Review at e343bb2. The fix is right: one predicate (only_description_too_long) keeps a skill whose only spec error is the length, in the catalog path, the skill: competitors and catalog scan's name-conflict check; any other error still leaves it out. The small gaps in the warning text, the skill under test and the changelog are in the line comments.

What I ran, at e343bb2:

• make ci-local --offline: 15,670 passed, 0 failed, 63 skipped; all steps pass.
• A lexical-only novelty routing run (no model call) with a 1139-character skill in the catalog and as --artifact: the catalog one is kept and warned about; the artifact itself is not in spec_warnings.

Checked, no issues

• Both nits from @antonwolfy: the 1024-character boundary is pinned by a test (652958a), and the validate.py docstring now names the exception (dc17ff8).
• --include-invalid: a long description plus a second error is still left out without it, and kept with it.
• Saved field: field_record moved to novelty_routing_inputs.py and both modes write spec_warnings; no schema covers field, so no schema version is needed.

[napetrov]

Thanks. All three line comments fixed in 83251e8 and the commits before it.

[xaleryb]

20ec659: merged main after #396. The docs, the routing docstring, external/listing.py and the changelog fragment now say catalog check where they said catalog scan.

[vlad-perevezentsev]

LGTM

I checked 20ec659. All three points from my last round are fixed: the warning says what each harness does at every over-limit length, the skill under test is warned about and saved in spec_warnings, and the changelog says to re-run a routing baseline.

Merge: safe now: it merges cleanly into current main, 0 commits behind. Once it is in, #394, #403 and #417 conflict with it (novelty.py, novelty_routing_inputs.py, validate.py, test_novelty_routing_field.py) and need a rebase.

What I ran, at 20ec659:

• make ci-local --offline: 15,873 passed, 0 failed, 63 skipped; all steps pass.

Checked, no issues

• Skill under test: load_routing_field prints its warning once and field_record puts it first in spec_warnings, named as the skill under test; --all-vs-all still lists every skill once, as a field skill. The new test fails without the gold argument.
• Merge of #396: the docs, the docstrings and the fragment now say catalog check.

[antonwolfy]

LGTM. The delta since my last review holds up: the 1024-character boundary is pinned by a test, the validate.py docstring names the exception, the skill under test is warned about and saved in field.spec_warnings, and the warning text and changelog now say what each harness does. Nothing new to fix.

What I ran, on a Linux review host at 20ec659:

• make ci-local: 30 passed, 0 failed, 7 skipped (the skips are jobs that do not run on a PR or need tools absent here). The test step and the changed-line coverage step both pass. The ResourceWarning at session end from my earlier run did not recur.
• A mutation run: 25 mutants of only_description_too_long, in_field, over_limit_warnings, competitor_left_out, _left_out_by_flag, field_record, the warning prints in load_routing_field and _name_conflicts. 23 are killed. The two survivors are the redundant bool(errors) guard (unreachable, both callers check state == "error" first) and a > DESCRIPTION_MAX + 1 threshold, which changes only whether a 1025-character skill gets its warning. The new tests against the base modules: 9 fail, as they should.
• Reproductions with no model call: 1024 characters is not warned, 1025 is, by character count and not bytes; a long description plus a second error is left out by default and kept with --include-invalid; a mirror warns once; the skill under test warns once and is listed once in spec_warnings; --all-vs-all saves spec_warnings for the field.
• Every non-merge commit has a Signed-off-by.

Checked, no issues

• Rule: the routing field, explicit skill: competitors, the catalog path and catalog check's name-conflict check all go through only_description_too_long.
• Repo rules: no schema covers field, so no version bump; cli-interface.json holds no help text, so the changed --include-invalid help needs no regeneration; the changelog fragment passes its check and the growth check is clean.
• Tests: the new tests use tmp_path and leave the tree clean.