[napetrov]

Summary

A SKILL.md that is plain Markdown with no frontmatter now gets its own error code, not-a-skill-no-frontmatter. The message carries a suggested frontmatter block: the description is the paragraph after the file's H1 (the first paragraph when there is no H1). The name is the directory's below the scanned root, where the loader checks it against the directory; at the root, or when the directory name is not a valid name, it is the H1 in kebab-case. The catalog summary reports these files apart from broken skills ("N error, of them M plain Markdown with no frontmatter"). This code cannot be passed to --ignore.

A file whose --- block starts after a blank line is a broken skill, not prose, and keeps the loader's existing error.

Evidence

• GitSkills corpus: 165K SKILL.md files have no frontmatter (141K of them start with #), against 39K with broken YAML.
• tests/test_catalog_no_frontmatter.py (25 test functions, 62 cases); full suite, ruff and mypy pass. Self-review of this head: 1 pass, none blocking.

Merge danger

Conflicts with the other PRs in this series on FINDING_CODES and the docs. The JSON rows of these files change code from the loader error to the new one, so a --resume over old rows keeps the old code.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Hint generation can abort catalog scans on invalid date-like names and incorrectly extract metadata from fenced code because fence markers and lengths are not matched.

2 open issues

• Use matching fence markers when extracting metadata hints · New
• Quote YAML names to avoid invalid-date parsing errors · New

What changed in this PR

2 new must-fix findings.

This PR distinguishes plain-Markdown SKILL.md files from malformed skills in catalog validation and provides suggested frontmatter to help users convert them.

Changes:

• Adds not-a-skill-no-frontmatter with a name and description derived from Markdown content.
• Counts these files separately in catalog summaries and rejects ignoring the new error code.
• Adds focused tests, documentation, and a changelog entry.

File	Description
dokimi/​tests/​test_catalog_no_frontmatter.py	Tests classification, hints, summaries, JSON output, and ignore rejection.
dokimi/​dokimi/​external/​validate.py	Classifies unloaded Markdown files using the new finding code.
dokimi/​dokimi/​external/​no_frontmatter.py	Generates suggested frontmatter from headings and prose.
dokimi/​dokimi/​commands/​catalog.py	Updates catalog summaries and rejects ignoring the new code.
dokimi/​dokimi/​commands/​catalog_scan.py	Tracks plain-Markdown findings in per-skill scan summaries.
dokimi/​docs/​external-content.md	Documents the new finding and summary count.
dokimi/​changelog.d/​20261109_170000_napetrov_no-frontmatter.md	Announces the new catalog diagnostics and hints.

_{Review effort: Balanced}

[exai-bot]

OpenCodeReview: Commit c70579a was not reviewed. This PR has reached its review limit (6 reviews).

CI at `c70579a`: 8 still running, 9 passing, 2 skipped

Check	Status
CodeQL (python)	still running
External catalog smoke test (uxlfoundation/skills)	still running
Linux Python 3.11	still running
Linux Python 3.14	still running
Linux Wheels Python 3.11	still running
Linux Wheels Python 3.14	still running
Static checks (ruff, schema, zizmor, licenses, pre-commit, bandit, secrets, changelog, schema com...	still running
mypy	still running

To review this commit: re-request review from exai-bot in the Reviewers list.

<details><summary>Last review (56c6f9b): no findings</summary>

Open threads from the bot: 0

</details>

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 97.11538% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.73%. Comparing base (f12057b) to head (c70579a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
dokimi/dokimi/external/validate.py	91.42%	1 Missing and 2 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #558      +/-   ##
==========================================
+ Coverage   95.71%   95.73%   +0.01%     
==========================================
  Files         415      416       +1     
  Lines       55346    55461     +115     
  Branches     9514     9536      +22     
==========================================
+ Hits        52977    53093     +116     
+ Misses       1466     1465       -1     
  Partials      903      903              

Files with missing lines	Coverage Δ
dokimi/dokimi/commands/catalog.py	99.11% <100.00%> (+0.01%)	⬆️
dokimi/dokimi/commands/catalog_scan.py	95.81% <100.00%> (+0.18%)	⬆️
dokimi/dokimi/external/catalog_review.py	94.87% <ø> (ø)
dokimi/dokimi/external/no_frontmatter.py	100.00% <100.00%> (ø)
dokimi/dokimi/external/validate.py	97.39% <91.42%> (-0.68%)	⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[vlad-perevezentsev]

Review at 7a679d0. The new finding and the summary count work, and the load error still fails the scan. The hint needs work before it helps: its suggested name gives a name-mismatch once pasted into a catalog skill, and the text output breaks the row layout. Details are in the line comments. The branch also needs a rebase: it conflicts with current main in catalog.py, catalog_scan.py and validate.py (#417).

Agreed with @exai-bot on the intro paragraph before the H1; I added the README badge case in its thread.

What I ran, at 7a679d0:

• make ci-local --offline: tests 17,898 passed, 0 failed, 72 skipped; one FAIL, schema_compat on arms.v1.json, which this PR does not touch (it comes from main moving on since the base).
• catalog check on a scratch catalog with four plain-Markdown skills (H1 first, badge before H1, indented ---, H1 inside a paragraph), then again with the suggested block pasted into one of them.

Checked, no issues

• Exit code and --ignore: a plain-Markdown SKILL.md still fails the scan as a load error, and --ignore not-a-skill-no-frontmatter is refused like frontmatter-parse.
• Callers of the review: novelty routing skips unloaded entries, and catalog eval inventory only adds the new finding next to skill-load.
• Changelog and docs: the fragment and external-content.md match the behaviour; the new file has the SPDX header.

Comments on lines outside the diff:

• dokimi/dokimi/commands/catalog_scan.py:106: The finding's message holds newlines, so the text output prints ---, name:, description:, --- at column 0 between the skill rows.

Every other finding line is indented by two spaces, and on a corpus scan this block repeats for every plain-Markdown file, so the rows can no longer be read line by line.

Suggestion: indent the continuation lines of a message in print_skill_row (e.g. message.replace("\n", "\n ")), or print the block on one line.

[napetrov]

Re: comment by xaleryb

Nothing is left for the author.
Fixed in b7fa886: e773c3c and 16b5d9f are in the PR.

[vlad-perevezentsev]

LGTM

I checked b7fa886. Everything from my earlier rounds still holds, and the new commits fix the points @vchamarthi and @xaleryb raised: a --- rule before prose, an H1 in a comment, a setext H1, an indented code block, quoted or flow YAML keys, and an unclosed block.

Merge: safe now: it merges cleanly into current main, 10 commits behind, and main has not changed its files since. Once it is in, #454 conflicts with it and needs a rebase.

Disagreed with @vchamarthi on the two unverified notes. The inventory now has not-a-skill-no-frontmatter next to skill-load, which only adds the hint. The --resume gap applies to every new check, not only this one, so it is better as its own issue.

What I ran, at b7fa886:

• make ci-local --offline: 18,993 passed, 0 failed, 72 skipped; every step passes.
• The hint on a few more layouts: a setext H1 with a paragraph, a tab-indented code block before the paragraph, an HTML paragraph, a list after the H1. Each gives the expected name and description.

Checked, no issues

• Broken skills vs prose: a --- block with a YAML key (plain, quoted, flow or list) keeps the loader's error. A --- rule followed by prose gets the hint, and so does an unclosed block whose first line is prose.
• Hint: the H1 search and the paragraph both skip HTML comments. A setext H1 is the title. The 64-character limit is tested for a name from the H1 and for one from the directory.
• Resume: a resumed finding with no code no longer raises.

[xaleryb]

• Re-checked b7fa886 merged with main d2b5a91, Python 3.13. Re-ran the original input of every earlier point (ours, vchamarthi's, antonwolfy's, vlad-perevezentsev's and the bots'), and each one passes.
• Pushed 2 commits (b7fa886..58a6fdd):
  • 4c63e3e merges main. The conflicts in external-content.md, catalog.py, catalog_scan.py and validate.py sat next to fix(validate): read the when cue of each language in description-no-when #454's description-when-not-checked; both sides are kept: the counts line shows N ok (M not checked) and the plain-Markdown note, and multi-line messages stay indented.
  • 58a6fdd drops a note reference from the test module docstring that the repository does not define.
• On the merged tree: full suite (19795 passed), ruff, mypy, bandit and docs check --strict pass.

[xaleryb]

Re-checked b7fa886 + 4c63e3e + 58a6fdd merged with main (Python 3.13): full suite, ruff, mypy, bandit, growth, comment-block and docs check --strict pass, and the original input of every earlier point (ours, vchamarthi's, antonwolfy's, vlad-perevezentsev's and the bots') passes. Since then only main (#737) was merged in; the PR's own diff is unchanged, and CI on 364d8da is green.

[xaleryb]

@vchamarthi every point of your changes-requested review is fixed at the head, and its original input was re-run and passes (details in the comments above). Could you re-review so the changes-requested state clears?

[antonwolfy]

The merge waits for the author on two notes from @vchamarthi's last round: whether the catalog eval inventory should carry the new finding next to skill-load, and --resume over rows written before this change. Neither is answered yet. My own review finds no bug or risk: the classification, the hint and the summary count hold up, and every earlier point is fixed at this head. Four nits inline, all on edge cases of the classifier and the hint. None changes an exit code: each variant is still a failing error row.

What I ran, on a Linux review host at 364d8da:

• make ci-local: 31 passed, 1 failed, 14 skipped; 19,813 tests passed, 19 skipped; changed-line coverage gate passes. The one failure is the test step, where 1 test and 2 fixtures timed out at 60 s in tests/test_import_architecture.py and tests/test_audit_test_quality.py, which the PR does not touch; both files pass alone (225 passed)
• Reproductions for the classifier findings, with the transport offline.
• A mutation run on the new tests: 106 mutants, 40 of which pass the new test file; the ones that matter are in the test comment.

Checked, no issues

• Commits: all 38 non-merge commits have a Signed-off-by.
• --ignore: not-a-skill-no-frontmatter is refused and named in the docs list.
• Hint YAML: about 40,000 random inputs plus odd layouts (fences, setext, CRLF, BOM, surrogates, NUL) always give a block that yaml.safe_load reads, with a spec-valid name of at most 64 characters and a description of at most 304.
• Pasted hint: below the root with a valid directory name, pasting the block gives no name-mismatch.
• Summary count: the same in the whole-catalog, --per-skill and --jsonl --resume paths.

[napetrov]

Re: review by vchamarthi

Waiting on a decision before I change this: should the catalog eval inventory carry not-a-skill-no-frontmatter beside skill-load, or drop it there? My recommendation: drop it in the inventory, since one plain file would count two errors and the finding already shows in catalog check.

Waiting on a decision before I change this: should --resume match on the dokimi version too, so rows from before this change are re-reviewed, or re-review only the rows that lack the finding? My recommendation: add the dokimi version to the key.

[napetrov]

Re: review by antonwolfy

The merge waits on two notes from vchamarthi's review
Waiting on a decision before I change this: the same two decisions as #1 and #2 above. The four inline nits are fixed in their threads.

[napetrov]

Re: comment by xaleryb

Pushed 2 commits (b7fa886..58a6fdd)
A status note with no request in it, so there is nothing to change for it.

[napetrov]

Re: comment by xaleryb

every point of your changes-requested review is fixed at the head
Not yet: #1 and #2 of the review by vchamarthi are open, waiting on a decision. The re-review can follow once they are decided.

[napetrov]

Round on c4f27c6 (merged with intel/main f12057b):

• Fixed: f9ni in 63ff037, f9no in 3b25b70 (bounds in 7c4a02f, 0077698, fccb593, c4f27c6), f9nr in a8c2aa1; main merged in da1f8fe.
• Open, waiting on a decision: late-frontmatter window (f9nl), the eval inventory and --resume notes (vchamarthi Make dokimi public #1, Make dokimi public #2). Each has its question in its thread or section.

[antonwolfy]

The merge waits for the author on two notes from @vchamarthi's last round: whether the catalog eval inventory should carry the new finding next to skill-load, and --resume over rows written before this change. Both were handed back as decisions and are not answered. The comment removal added since my last review holds up, and one nit is inline. My earlier thread on the late-frontmatter window stays open as a nit.

What I ran, on a Linux review host at c4f27c6:

• make ci-local: 32 passed, 1 failed, 14 skipped; 20,505 tests passed, 19 skipped; changed-line coverage gate passes. The one failure is the test step, where test_a_fixed_finding_leaves_the_baseline_too in tests/test_audit_test_quality.py timed out at 60 s in setup. The PR does not touch that file, and it passes alone (172 passed)
• Reproductions for the comment and code-span handling, with the transport offline: comments before, inside and after text, unclosed and multi-line comments, markers inside code spans, and about 20,000 random inputs.
• A mutation run on the new tests: 48 mutants, 23 of which pass the new test file; the ones that matter are in the test comment.

Still open from my earlier review:

• dokimi/dokimi/external/validate.py:367: late frontmatter is recognised only within the first 20 lines and only when --- is directly followed by name: or description:. A 22-line <!-- ... --> header before a full block, and # Deck, ---, title: x, name: deck, description: Use when x., ---, both still get not-a-skill-no-frontmatter with a description built from the YAML lines. Your recommendation to keep the window is fine for the code, but external-content.md does not state it. Say there that frontmatter after another line, or after line 20, or opening with another key, is plain Markdown, and that a block of blank lines counts as frontmatter. That closes the thread.

@vchamarthi's two questions wait for the author, on the inventory and on --resume.

Checked, no issues

• Comment removal: <!-- a --> Real. gives Real., a mid-line comment keeps the text around it, a multi-line comment over a blank line is removed, an unclosed line-start comment runs to the end, and a stray --> stays.
• Code spans: <!-- inside a backtick span is kept, and a comment after an unmatched backtick is still removed.
• Hint YAML: about 20,000 random inputs of backticks, comment markers, fences, CRLF, BOM, NUL and quotes always give a block that yaml.safe_load reads, with string name and description.
• Blank-line block: ---, a blank line, --- keeps the loader's error.
• Commits: all 45 non-merge commits have a Signed-off-by.
• Repo rules: the changelog fragment, the SPDX header, the docs nav entry and the growth baseline are in order.

[antonwolfy]

I pushed a commit per nit, up to d1a8ea0, which fix the two nits from my review:

• fcd55c7: dokimi/docs/external-content.md:244: states that a --- block after another line, after line 20, or opening with a key other than name or description is plain Markdown, and that a block of blank lines counts as frontmatter.
• d1a8ea0: dokimi/tests/test_catalog_no_frontmatter.py:317: adds a test for the unclosed line-start comment, the blank line inside a line-start comment, the code span before a later -->, and the 1024-character span bound on both sides.

make ci-local at d1a8ea0: 33 passed, 0 failed, 14 skipped.

Not approving yet. Still open: @vchamarthi on whether the catalog eval inventory should carry not-a-skill-no-frontmatter beside skill-load; @vchamarthi on whether --resume should match on the dokimi version so rows written before this change are re-reviewed.

[vchamarthi]

Pushed 1 commit to feat/not-a-skill-no-frontmatter (d1a8ea0..c70579a).

• A block whose first line opens a flow collection is frontmatter; a Markdown link [text](url) stays prose.
• YAML_KEY_RE accepts a key that starts with a digit.
• A block whose first line opens a flow collection is frontmatter; a Markdown link ([text](url), [text][ref], [ref]: url) stays prose.
• _has_late_frontmatter looks for name: or description: anywhere in the block after a ---, not only on the next line.
• The docs describe which --- blocks keep the loader's error and which are plain Markdown.

Threads: 1 fixed, 0 answered as not a defect.
Run: the full suite with main merged in, 20569 passed.

Seen in files outside this PR, not changed here:

• dokimi/dokimi/external/catalog_eval_inventory.py:148: evaluate_catalog on skills/deck/SKILL.md holding # Deck Builder then Builds decks. gives two error problems, skill-load and not-a-skill-no-frontmatter; main gives one.

The PR description needs these corrections:

• "A file whose --- block starts after a blank line is a broken skill, not prose, and keeps the loader's existing error." should read: A file with a --- block that opens with a YAML key or flow collection, or holds name: or description: in the first 20 lines, is a broken skill, not prose, and keeps the loader's existing error.

Questions for @napetrov

1. Should a --resume row written without its findings count as plain Markdown in the summary? (dokimi/dokimi/commands/catalog_scan.py:264)
--per-skill --jsonl rows.jsonl, rows' findings set to [], then --resume: summary prints 1 error, a fresh scan prints 1 error, of them 1 plain Markdown. _load_prior keeps rows by skill_sha256, so the finding is never recomputed; the PR text already notes the old-row limit.

• A: Keep as documented: old rows keep the old code until rescanned.
• B: Drop prior rows whose error row has no findings, so they are rescanned.