fix(code-metrics): ground the principles skill in its primaries and add a skill-quality Next-section check - #4098
Conversation
…dd a skill-quality Next-section check code-metrics 0.1.9: the principles skill's Halstead split-file answer and eval 1 said difficulty should not move on a split; the formula carries no explicit length term but both factors change per half, so the entry and the eval now say it legitimately moves in either direction. The ISO/IEC 5055 section 8.2.115 percentage-of-file reading is labelled as the plugin's, with a four-part verification record, and the files name ISO/IEC 5055:2021 as the ISO publication of ASCQM v1.0. The quick guide answers "which measure should I look at" with an intent-keyed tree grounded in McCabe, NIST SP 500-235, Campbell, and the ISO patterns, gains a duplication entry, and routes the report vocabulary to the report-schema reference. The no-verdict rule is stated once, the ADR citation by number is dropped, the reference files carry present-tense facts instead of research narrative, and a Next section is added. literature.md gains a coverage-and-test-effectiveness section citing the contested primaries with DOIs. skill-quality 0.21.3: check 26 reads the Next section, INFO when absent and WARN when present but misplaced or malformed, with five test cases. Closes #4067 Closes #4066 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3rGcaMrtBbnQrKCkW6J1t
…rinciples-rs0sp0 # Conflicts: # docs/CATALOG.md # plugins/code-metrics/.claude-plugin/plugin.json # plugins/code-metrics/CHANGELOG.md # plugins/code-metrics/scripts/config-defaults.json # plugins/code-metrics/skills/principles/reference/thresholds.md # plugins/skill-quality/.claude-plugin/plugin.json # plugins/skill-quality/CHANGELOG.md # plugins/skill-quality/scripts/check-skill.sh # plugins/skill-quality/skills/check/SKILL.md
audit-size/SKILL.md and principles/reference/thresholds.md now carry the file_lines provenance string from config-defaults.json verbatim, which the audit-size test requires. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3rGcaMrtBbnQrKCkW6J1t
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 4m 27s —— View job Claude finished reviewing this PR
Ran the code-review lane's criteria (correctness and maintainability, scoped to changed files, security findings left to the security lane) against the full diff. This PR is well-grounded overall — the citation-verification work is careful and the new skill-quality check (27) is logically sound (placement check, shape check, bullet-count check, and the joined-continuation-line bullet judging in the awk block all hold up). Three findings, two posted inline, one cross-file:
All three are documentation/reference-file consistency gaps rather than executable bugs — nothing in |
|
Claude finished @kyle-sexton's task in 53s —— View job Security review complete
Result: no security issues found. This PR is documentation/reference content (ISO citation grounding, literature annotations, threshold provenance) across the I reviewed the new shell code in
No CRITICAL / IMPORTANT / SUGGESTION findings to report. |
|
Last security-reviewed head: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
The size.file_lines row now matches the provenance sentence in config-defaults.json, audit-size/SKILL.md, and thresholds.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3rGcaMrtBbnQrKCkW6J1t
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3rGcaMrtBbnQrKCkW6J1t
…quick guide Difficulty carries no length term, but a split changes each part's distinct operators and operand reuse, so per-file difficulty legitimately moves on a split. The reference file now says so alongside SKILL.md and literature.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3rGcaMrtBbnQrKCkW6J1t
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9a931c54a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…asing Check 27 accepted a single-shape section whose first line merely mentioned a /plugin:skill token, so a Skill-tool instruction with an installed-ness gate and a fallback clause passed as a mention. The single shape now has to open with the invocation, and both shapes are scanned whole for Skill-tool, installed, fallback, and otherwise phrasing. Two test cases cover the prose-first chain and a bullet fallback; every repo skill with a Next section still passes with no new warning. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3rGcaMrtBbnQrKCkW6J1t
…n prose across 36 skills (#4108) No related issue: this closes the one decision the audit behind #4072 reclassified to a human, resolved by re-reading the sources; no issue tracks it. ## Summary Anthropic's [skill-authoring best-practices page](https://platform.claude.com/docs/en/agents-and-tools/agent-skills/best-practices#writing-effective-descriptions) rules out first and second person in a skill description. Its two Avoid examples are "I can help you process Excel files" and "You can use this to process Excel files", and its reason is that the text is injected into the system prompt. Its own effective examples, the Claude Code skills page's examples, and the bundled skill-creator's frontmatter are all imperative ("Extract text and tables from PDF files", "Fix a GitHub issue", "Create new skills..."), so imperative and third-person singular both conform and this marketplace's imperative descriptions are not a deviation. The real gap was description prose that addresses the reader. A scripted scan that strips quoted trigger phrases and hyphenated or slash-joined identifiers found it in 36 skills across 24 plugins. ## Fix - **36 skill descriptions** (adhd, bugs, claude-config, claude-ops, code-metrics, discipline, discovery, docs-hygiene, education, firecrawl, improvement, instruction-placement, kindle-dedrm, mutation-testing, overengineering, playgrounds, plugin-quality, prototype, session-flow, songwriting, source-control, tdd): each clause that addressed the reader now names the user, the session, the assistant, or the repository. Examples: "so you can act on it" becomes "so the reader can act on it"; "a permission rule you cannot locate" becomes "a permission rule whose source is unknown"; "books you own" becomes "books the user owns"; "synthesize where we stand" becomes "synthesize where the session stands". Quoted trigger phrases (user utterances such as 'audit my .claude folder'), backticked tokens, slash tokens, and skill names are byte-identical. No description grew by more than 20 codepoints or crossed 1,024; two that sat near the cap got shorter. - **One clause stays as it was.** The native-surface presence gate ("resolves in your session") in the two claude-ops descriptions (`audit-install-state`, `audit-skill-visibility`) is a literal token the native-overlap registry self-check matches against every baked store row, so `validate-plugins.sh` fails when it is reworded (that is what turned `7cbf30ab8` red). Commit `af0e7b274` restores the token in both and the claude-ops changelog entry says why. Changing the token itself touches the checker, its tests, twelve skills, the seam-phrasing convention, and the store evidence, and is tracked as #4112. - **Two plugin manifests** (kindle-dedrm, session-flow) carried the same wording in their own descriptions and follow; `docs/CATALOG.md` regenerated. - **playbooks 0.11.2**: `skill-authoring/reference/authoring-guidance.md` states the rule as the page defines it, with the Avoid examples, the system-prompt reason, and the finding that imperative conforms, and its Record cites the three imperative example surfaces with a recheck trigger for the page rewriting its examples. `reference/authoring-checklist.md` gains the matching judgment row. The previous text called the fleet's imperative voice a deviation to leave alone, which the sources do not support. - Each of the 24 touched plugins gets a patch bump and a CHANGELOG entry naming its rewritten skills. Main moved four times while this was open; the merge commits `2cc1102c0`, `f082ed2f8`, `7cbf30ab8`, and `0b10ccae7` restack this change's entries above the newer main entries: claude-ops to 0.54.1 (above #4099, #4107, and #4092), code-metrics to 0.2.3 (above #4098), source-control to 0.55.76 (above #4106). Every other plugin's bump is unchanged. ## Verification Run on Linux at head `0b10ccae7`: - Voice scan over all 258 skill descriptions (quoted phrases and identifiers stripped, then first and second person pronouns matched): 36 hits before; after, only the two claude-ops descriptions that carry the checked gate token (3 occurrences of that token, nothing else). An uppercase variant scan finds 0. - `scripts/validate-plugins.sh`: passes; the native-overlap self-check reports degraded (stale-but-honest advisories, the same as main), not broken. - `scripts/check-changed-skills.sh origin/main`: 35 skills checked, 0 failed, no dropped-trigger-keyword WARN on any skill (check 3 compares every trigger phrase against the base ref). - `check-changelog-parity.sh --check`, `--check-bump origin/main`, `--check-order`: pass across all 93 changelogs. `generate-catalog.mjs --check`: in sync. - `markdownlint-cli2` over every changed markdown file: 0 issues. `typos` over the added lines: clean. `check-purged-em-dashes.sh`: no em dashes. - Diff shape: 36 SKILL.md files touched on their description line only, 23 plugin.json version bumps plus the two manifest description edits, 23 CHANGELOG.md entries, the two playbooks spokes, and the catalog. No skill body, script, or test changed. - `scripts/affected-tests.sh --run`: no suites selected (every changed file is a recorded no-suite class). - CI: `ci-status` and every lane green on `45af55489` (draft) and on `2cc1102c0` (draft and after the ready flip). Codex review on `2cc1102c0`: completed, no findings. `7cbf30ab8` went red on `test-linux` (the gate-token self-check above); `af0e7b274` carries the fix and `0b10ccae7` merges the latest main on top. ## Related - Follow-up to #4069 and #4072. The two fresh-context validators behind #4072 both reclassified the description-voice row to a human decision on the premise that imperative and third person are two voices; re-reading the page's examples shows they are one conforming class, so the decision resolves on evidence rather than taste. The reasoning and the scan live in the untracked `.work/skill-authoring-best-practices/INTERVIEW-ACCEPTED.md` ("Audit resolution", row I1.2) of the authoring checkout. - #4112 tracks retokening the presence gate so the last two descriptions lose their second person too. - #4070 and #4071 remain the deferred follow-ups from #4069. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01MSGD4WGhUXepHq3LTmpQML --------- Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Closes #4067
Closes #4066
Summary
The
code-metrics:principlesskill was exercised through the Skill tool on this repository and every load-bearing citation checked against a primary. Seven findings came back (#4067): one factual overstatement baked into an eval, an ISO clause reading presented as the spec's words, a missing## Next, research narrative in two reference files, an ADR cited by number, no duplication entry, and a non-answer to the skill's most likely first question. The skill-quality gate passed the skill anyway because it has no check for the successor section (#4066). This PR fixes both, as code-metrics 0.2.2 and skill-quality 0.22.3, in one change set because the two fixes were found together and are each small.Fix
code-metrics
principles:(n1/2) * (N2/n2)carries no explicit length term, but both factors change per half on a split (a radon run gave 1.667 for the whole against 1.000 and 1.800 for the halves), so the entry, the measures.md paragraph, literature.md, and the eval now say per-file difficulty legitimately moves in either direction.reference/config.md, and the provenance strings inconfig-defaults.json. The files name ISO/IEC 5055:2021 as the ISO publication of ASCQM v1.0, with v1.1 identical for the cited clauses, and record that the document's informative §6.3 Table 1 rows carry different defaults (1000 lines, 10%) from its detection patterns (5%, 90%).report-schema.mdfor the report vocabulary.## Nextadded in the sibling skills' shape. McCabe's "testable and maintainable" framing and Campbell's verbatim switch rule are carried in measures.md and literature.md; literature.md gains a coverage-and-test-effectiveness section citing the five primaries with DOIs.file_linesprovenance sentence inconfig-defaults.jsonis carried verbatim byaudit-size/SKILL.md,principles/reference/thresholds.md, and thereference/config.mdrow, which the audit-size test requires for the first two.skill-quality
check:## Nextsection: INFO when absent (226 of 258 skills here have none, and the rule leaves "has a successor" to the author), WARN when present but after## Gotchas, last in the file, or in neither the one-invocation nor the two-to-four-bullet shape. The single shape has to open with the invocation, and both shapes are read whole for Skill-tool, installed, fallback, and otherwise phrasing, so an operative chain dressed as a successor also warns. Seven cases incheck-skill.test.sh; the gate stays advisory. Wording in the skill description, manifest, and generated catalog moves from twenty-six to twenty-seven checks.Every technical claim the skill now asserts was verified against fetched primaries by a dispatched research run and a separate fresh-context verifier, with Crossref confirmation of all seven paper citations; the record is in #4067's Evidence section.
Verification
check-skill.sh principles: PASS, 0 errors, 0 warnings (was FAIL on the report-schema routing row until it was cited through${CLAUDE_PLUGIN_ROOT}, the sibling skills' form).check-skill.sh audit-size: PASS.check-skill.sh check: PASS, 0 errors, 1 pre-existing line-count warning.check-skill.test.sh: all assertions passed, including the seven## Nextcases and main's new check-26 cases after the merge.audit-size.test.sh: 13 cases, 0 failed.## Nextsection (32) passes the tightened check with no warning, and every other code-metrics skill passes with no## Nextwarning.check-evals-quality.shon the principles evals: PASS.check-code-metrics-config-reference.py: config.md documents all 25 keys.check-changelog-parity.shin--check,--check-bump origin/main, and--check-preserved origin/main: clean.generate-catalog.mjs --check: in sync.markdownlint-cli2over every changed markdown file: 0 issues.check-purged-em-dashes.sh: 383 declared paths, no em dashes.shellcheckandshfmt -don both shell files: clean. All edited JSON parses.scripts/affected-tests.sh --runon the merged tree: every selected Bash suite passed except two.audit-size.test.shfailed on the provenance sentence, which a later commit fixes (re-run above).plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.shfails its two process-budget cases becausePS4does not reachbash -xin this container at all (PS4='+${BASHPID}+ ' bash -x -c trueprints a bare+ true); this PR touches no claude-ops file, the failure reproduces identically on a standalone re-run, fix(implementation): state the current subagent model resolution order #4091 recorded the same environmental failure with CI green, and CI on main is green. The 18 Python and Node suites the Bash runner lists as NOT RUN include four code-metrics suites (test_report.py,test_resolve_config.py,test_setup_apply.py,test_check_code_metrics_config_reference.py), run through pytest: 77 passed.Related
🤖 Generated with Claude Code
https://claude.ai/code/session_01T3rGcaMrtBbnQrKCkW6J1t
Generated by Claude Code