From 302f6d0c19fa35bd8d72b18545ef674de92d96f3 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Sun, 9 Aug 2026 17:23:51 +0000 Subject: [PATCH] fix(lint): tighten the SKILL.md word ceiling to 2770 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MAX_WORDS=2900 was calibrated to the corpus median density and carried no margin: at the densest observed 7.22 chars/word (~1.81 tokens/word) it permits ~5,240 tokens against the 5,000 it proxies for. 2770 holds the worst observed density under the ceiling. The largest SKILL.md is 2,489 words, so the change costs nothing today — 281 words of margin — and the header comment now argues the new calibration rather than swapping the digits. Both enforcement points move together, and a new test asserts they agree, since a SKILL.md passing its own audit while the commit hook blocks it is the disagreement this pair exists to prevent. CONTEXT.md is deliberately left ungated: it is 2,816 words, and gating it would block the build. Recorded here so the omission reads as a decision rather than an oversight. skill-audit's manual-fallback path listed only the line ceiling, so an agent taking that path passed an oversized SKILL.md the hook then rejected. The word ceiling is now named alongside it. agent-audit is deliberately unchanged: the size hook scopes to SKILL.md only and agent-audit's validate.sh has no word gate, so claiming it there would be false. The Vale research doc still showed the MDX {/* vale off */} form under a Markdown heading, contradicting CONTEXT.md and vale-run's troubleshooting reference — that form suppresses nothing in plain .md. Fixed in both places it appeared. tests/run-tests.sh used mapfile (bash 4.0+) with unguarded array expansion, though AGENTS.md tells contributors to run it and macOS ships bash 3.2. It now collects via a while-read loop over process substitution and guards every expansion. The newline-delimited find|sort pipeline is kept rather than -print0 with sort -z, whose BSD portability is the weaker link, and which matches mapfile -t's previous behaviour exactly. Refs: #85 ADR: 0013 --- CONTEXT.md | 2 +- ...013-vale-harness-scope-and-rule-sources.md | 9 +++--- .../kyberforge/skills/skill-audit/SKILL.md | 2 +- .../skills/skill-audit/scripts/validate.sh | 6 +++- .../research/docs/vale/troubleshooting.md | 19 ++++++++++-- scripts/skill-size-check.sh | 18 +++++++----- tests/run-tests.sh | 18 +++++++++--- tests/test-skill-size-check.sh | 29 +++++++++++++++++-- 8 files changed, 80 insertions(+), 23 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index b386d21..b77c576 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -74,7 +74,7 @@ Wiring Vale as a deterministic prefilter for `skill-audit`/`agent-audit`'s Descr Both skills' Step 1, and the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks, call each copy's own `scripts/vale-wrap.sh` rather than `vale` directly — a workaround for a confirmed Vale 3.15.2 limitation (see `vale-config`'s Gotchas): `text.frontmatter.description` silently stops matching on most — not all — multi-line descriptions. Verified by reproduction, not assumed: `>` folded scalars, plain (unquoted) continuation lines, and single- or double-quoted multi-line scalars all yield 0 alerts and exit 0 on a deliberately-bad fixture, while a `|` literal block spanning the same 2+ lines lints normally (alerts fire, exit 1). The wrapper flattens those three broken forms to one physical line in a scratch copy (padding with blank lines so every other line number is unchanged) before handing off to real `vale`; `|` literal blocks and single-line descriptions pass through untouched, already linting correctly. The plain and quoted forms previously passed silently — unflattened and unmatched — so a bad description in either sailed through the prefilter. Handed no `--config` at all, the wrapper falls back to its own sibling `assets/vale/.vale.ini`, located from `${BASH_SOURCE[0]}` rather than from the cwd — which is why both manifests' `entry:` is now the bare script path with no argument after it. pre-commit prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]), *cmd[1:])`), so every later argument resolves against the *consuming* repo's root: a `--config` in `.pre-commit-hooks.yaml` pointed at a path no consumer has and hard-failed every external run with `E100 [--config] Runtime error`. `.pre-commit-config.yaml` drops the argument too, deliberately keeping the two entries identical — the local `repo: local` hook resolved its `--config` correctly only because the consuming repo *was* this repo, and that divergence is why three review rounds exercised a path no external consumer takes and missed the defect. An explicit `--config` still wins, in all three argv forms (`--config X`, `--config=/abs`, `--config=rel`), and a relative one still resolves against the caller's cwd, matching bare `vale`, not the repo root. Both audit skills' Step 1 now passes no `--config` either: it resolves the script relative to the skill's own directory so the call works from an installed plugin cache, but a relative `--config` alongside it would still resolve against the cwd, yielding `E100 Runtime error ... does not exist` and exit 2 — which both skills' fallback misreads as "vale unavailable" and silently downgrades to full LLM judgment. `tests/test-vale-wrap.sh` regression-tests this against skill-audit's copy specifically (its fixtures are all `SKILL.md`-shaped, and only skill-audit's `.vale.ini` has that glob section). Each `.vale.ini`'s section globs are path-agnostic (`[**/SKILL.md]` for skill-audit's copy; `[**/agents/*.md]`/`[**/*.agent.md]` for agent-audit's) and do no scoping on their own: Vale's `*` crosses `/`. Scoping comes from each pre-commit hook's own `files:` regex and from the audit skills passing one explicit file per invocation. The two manifests scope differently on purpose: this repo's `.pre-commit-config.yaml` pins its own layout — `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` for `-skill`, `^plugins/[^/]+/agents/[^/]+\.md$` for `-agent` — while the shipped `.pre-commit-hooks.yaml` stays layout-agnostic for external consumers whose skills live anywhere, using `(^|/)SKILL\.md$` and `(^|/)agents/[^/]+\.md$|\.agent\.md$`. Both manifests split the prefilter into two hooks precisely because one combined hook pointed at only one copy would silently 0-file-skip the other file type. A `SKILL.md` outside `plugins/` (e.g. project-scope `.claude/skills/foo/SKILL.md`) still matches `[**/SKILL.md]` and gets linted normally — the globs constrain filename shape, not location. Vale reports 0 files only when the path it is handed matches no glob section at all: a differently-named file, or a directory argument holding nothing that matches. That run prints `✔ 0 errors ... in 0 files.` and exits 0, indistinguishable from a clean pass, so both audits treat a 0-file Vale run as NOT RUN and fall back to full LLM judgment. -This scope expands per ADR-0013: one cherry-picked low-noise `write-good`/`alex` rule landed in `styles/Kyberforge`, `Kyberforge.SentenceOpenerThereIs` (22 held-out hits, both in-corpus hits clean rewrites, zero suppressions). A second, `Kyberforge.VagueQualifier`, was cherry-picked and then deleted: 2 hits across the 41 skill/agent files, one marginal and one an unfixable false positive (`caveman/SKILL.md` quotes `of course` as an example of filler — a mention, not a use) that forced the repo's only Vale suppression comments. Also new is a sibling pre-commit hook, `skill-size-check` (`scripts/skill-size-check.sh`), enforcing agentskills.io's `SKILL.md` ceiling as two blocking gates: `MAX_LINES=500` and `MAX_WORDS=2900` (a word-count proxy for the 5,000-token limit, calibrated from this repo's measured ~1.6-1.7 tokens per word). Both are inclusive, and `skill-audit/scripts/validate.sh` checks the same pair on the same terms, so a `SKILL.md` can no longer pass its own audit yet be blocked by the commit hook. Scoped to `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` only, same as `vale-audit-prefilter-skill`, so it never lints `docs/research/examples/` reference skills. It's also exposed in the root-level `.pre-commit-hooks.yaml` as `kyberforge-skill-size-check` — it has no external asset dependency, so it needed no relocation, only exposure to external consumers. File scope (`SKILL.md` + agent files) and enforcement model (rules land directly in `styles/Kyberforge`, blocking immediately, no trial tier) stay unchanged; governance.md/CONTROLS.md were evaluated and excluded as rule sources (nothing prose-pattern-matchable to mine). House convention: banned phrasing that must be mentioned rather than used goes in backticks or a fenced code block — Vale skips code spans and fences, so no suppression is needed; inline `` (HTML-comment form; the MDX `{/* */}` form does not work in plain Markdown) is the fallback only where backticking is impossible. +This scope expands per ADR-0013: one cherry-picked low-noise `write-good`/`alex` rule landed in `styles/Kyberforge`, `Kyberforge.SentenceOpenerThereIs` (22 held-out hits, both in-corpus hits clean rewrites, zero suppressions). A second, `Kyberforge.VagueQualifier`, was cherry-picked and then deleted: 2 hits across the 41 skill/agent files, one marginal and one an unfixable false positive (`caveman/SKILL.md` quotes `of course` as an example of filler — a mention, not a use) that forced the repo's only Vale suppression comments. Also new is a sibling pre-commit hook, `skill-size-check` (`scripts/skill-size-check.sh`), enforcing agentskills.io's `SKILL.md` ceiling as two blocking gates: `MAX_LINES=500` and `MAX_WORDS=2770` (a word-count proxy for the 5,000-token limit, calibrated to the densest prose measured in this repo — 1.81 tokens per word — so even a worst-case `SKILL.md` at the ceiling stays under 5,000 tokens). Both are inclusive, and `skill-audit/scripts/validate.sh` checks the same pair on the same terms, so a `SKILL.md` can no longer pass its own audit yet be blocked by the commit hook. Scoped to `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` only, same as `vale-audit-prefilter-skill`, so it never lints `docs/research/examples/` reference skills. It's also exposed in the root-level `.pre-commit-hooks.yaml` as `kyberforge-skill-size-check` — it has no external asset dependency, so it needed no relocation, only exposure to external consumers. File scope (`SKILL.md` + agent files) and enforcement model (rules land directly in `styles/Kyberforge`, blocking immediately, no trial tier) stay unchanged; governance.md/CONTROLS.md were evaluated and excluded as rule sources (nothing prose-pattern-matchable to mine). House convention: banned phrasing that must be mentioned rather than used goes in backticks or a fenced code block — Vale skips code spans and fences, so no suppression is needed; inline `` (HTML-comment form; the MDX `{/* */}` form does not work in plain Markdown) is the fallback only where backticking is impossible. ### LESSONS.md Long-loop feedback log for patterns observed across sessions. Three or more entries on the same pattern graduate to the relevant standing file (e.g. a coding convention, a governance rule). Updated by the session-handoff skill or directly by the human. Lives at the repo root. diff --git a/docs/adr/0013-vale-harness-scope-and-rule-sources.md b/docs/adr/0013-vale-harness-scope-and-rule-sources.md index 1774b89..2dfb67d 100644 --- a/docs/adr/0013-vale-harness-scope-and-rule-sources.md +++ b/docs/adr/0013-vale-harness-scope-and-rule-sources.md @@ -96,10 +96,11 @@ every rule to `level: error` is what actually implements this decision. "cherry-picked rules" is one rule, not two. - A new pre-commit hook, `skill-size-check` (`scripts/skill-size-check.sh`), enforces the 500-line/5,000-token `SKILL.md` ceiling, sibling to `skill-frontmatter`. Both halves of that - ceiling are blocking gates, not just the line count: `MAX_LINES=500`, and `MAX_WORDS=2900` as a - word-count proxy for the 5,000-token limit (calibrated from this repo's measured ~1.6-1.7 tokens - per word — `wc -w` is not BPE tokenization). Either one exceeded fails the hook. Both are - inclusive: a file at exactly 500 lines or exactly 2,900 words passes, and only one past a ceiling + ceiling are blocking gates, not just the line count: `MAX_LINES=500`, and `MAX_WORDS=2770` as a + word-count proxy for the 5,000-token limit (calibrated to the densest prose this repo measured, + 1.81 tokens per word, so a worst-case `SKILL.md` at the ceiling still lands under 5,000 tokens — + `wc -w` is not BPE tokenization). Either one exceeded fails the hook. Both are + inclusive: a file at exactly 500 lines or exactly 2,770 words passes, and only one past a ceiling fails. `skill-audit/scripts/validate.sh` enforces the same pair on the same inclusive terms, so the audit and the commit hook cannot disagree about whether a given `SKILL.md` is over size. - `styles/KyberforgeTrial/` and `.vale.trial.ini` were deliberately not created — noted here so a diff --git a/plugins/kyberforge/skills/skill-audit/SKILL.md b/plugins/kyberforge/skills/skill-audit/SKILL.md index 6f71299..e09ba5d 100644 --- a/plugins/kyberforge/skills/skill-audit/SKILL.md +++ b/plugins/kyberforge/skills/skill-audit/SKILL.md @@ -37,7 +37,7 @@ bash scripts/validate-provenance.sh scripts/vale-wrap.sh /SKILL.md ``` -Note any structural FAILs — they will appear in the report as a `### Structure` dimension. If the script cannot execute (python3 unavailable, Bash denied, or permission error), perform structural checks manually: name format, name matches directory, description length ≤1024 chars, SKILL.md ≤500 lines, no unfilled `FILL IN:` placeholders, scripts executable and free of interactive prompts. +Note any structural FAILs — they will appear in the report as a `### Structure` dimension. If the script cannot execute (python3 unavailable, Bash denied, or permission error), perform structural checks manually: name format, name matches directory, description length ≤1024 chars, SKILL.md ≤500 lines and ≤2770 words (the word count is a proxy for the ~5,000-token ceiling, and blocks a commit exactly like the line count does), no unfilled `FILL IN:` placeholders, scripts executable and free of interactive prompts. Note any Provenance FAILs and INFO findings from `validate-provenance.sh` — they surface in the report as a `### Provenance` dimension (separate from `### Structure`). The script embeds full FAIL/INFO format with Why and Fix per finding; surface them verbatim. diff --git a/plugins/kyberforge/skills/skill-audit/scripts/validate.sh b/plugins/kyberforge/skills/skill-audit/scripts/validate.sh index e9ac95b..0c0473e 100755 --- a/plugins/kyberforge/skills/skill-audit/scripts/validate.sh +++ b/plugins/kyberforge/skills/skill-audit/scripts/validate.sh @@ -140,7 +140,11 @@ else: # no single source to share. Keep the two in sync by hand: if they drift, this # audit will report a skill ready to ship that the commit hook then rejects. MAX_LINES = 500 -MAX_WORDS = 2900 # word-count proxy for the ~5,000-token ceiling +# Word-count proxy for the ~5,000-token ceiling, calibrated to the densest +# prose in the corpus (7.22 chars/word): 2770 words is ~20,000 characters, +# ~5,000 tokens at 4 characters per token. See skill-size-check.sh's header +# for the full measurement. +MAX_WORDS = 2770 line_count = len(content.splitlines()) if line_count <= MAX_LINES: diff --git a/plugins/lint/docs/research/docs/vale/troubleshooting.md b/plugins/lint/docs/research/docs/vale/troubleshooting.md index b5cd1ca..e75a817 100644 --- a/plugins/lint/docs/research/docs/vale/troubleshooting.md +++ b/plugins/lint/docs/research/docs/vale/troubleshooting.md @@ -8,7 +8,14 @@ source_keys: Vale supports inline markup comments to disable checks for a section of content. Syntax varies by format: -Markdown/MDX: +Markdown — HTML comments; the MDX `{/* */}` form suppresses nothing in a plain `.md` file: +```markdown + +This text will be ignored. + +``` + +MDX: ```mdx {/* vale off */} This text will be ignored. @@ -24,7 +31,15 @@ This text will be ignored. ## Disabling a Specific Rule for Specific Matches -Rather than disabling all checks, target one rule and specific known-exception strings, then re-enable: +Rather than disabling all checks, target one rule and specific known-exception strings, then re-enable. Same per-format comment syntax as above — Markdown: + +```markdown + +This is some text ACT test + +``` + +MDX: ```mdx {/* vale Style.Redundancy["ACT test","OTHER"] = NO */} diff --git a/scripts/skill-size-check.sh b/scripts/skill-size-check.sh index 3f877b4..a1cf3bc 100755 --- a/scripts/skill-size-check.sh +++ b/scripts/skill-size-check.sh @@ -20,13 +20,15 @@ set -euo pipefail # ~4-characters-per-token English approximation that is 1.49 / 1.70 / 1.69 / # 1.81 tokens per word. # -# MAX_WORDS=2900 is therefore calibrated to the corpus MEDIAN, not to its worst -# case, and carries no margin: 2900 x 1.70 = ~4,900 tokens for a median-density -# file, but 2900 x 1.81 = ~5,240 tokens for the densest file in the corpus. So -# what this gate actually guarantees is "under 5,000 tokens for a file of -# typical prose density"; a prose-dense SKILL.md can sit at exactly MAX_WORDS -# and still be a few hundred tokens over the agentskills.io ceiling. Holding -# the worst observed ratio under 5,000 tokens would need MAX_WORDS ~2770. +# MAX_WORDS=2770 is therefore calibrated to the corpus WORST case rather than +# its median: 2770 words at the densest observed 7.22 chars/word is ~20,000 +# characters, or ~5,000 tokens at the 4-characters-per-token approximation. So +# what this gate guarantees is "under 5,000 tokens even for the densest prose +# the corpus has produced" — the earlier median-calibrated MAX_WORDS=2900 let +# such a file sit at exactly the ceiling and still spend ~5,240 tokens. A +# median-density file at 2770 words spends ~4,700 tokens, so typical prose +# gives up ~130 words of headroom to close that gap. The largest SKILL.md in +# the repo is 2,489 words, so no current file is affected. # # It is a one-sided proxy in the useful direction — nothing under the word # ceiling is wildly over the token ceiling — but it is not exact BPE @@ -34,7 +36,7 @@ set -euo pipefail # any of these numbers as still current. MAX_LINES=500 -MAX_WORDS=2900 +MAX_WORDS=2770 FAIL=0 for f in "$@"; do diff --git a/tests/run-tests.sh b/tests/run-tests.sh index 1f3c067..1e73765 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -35,14 +35,24 @@ fi run_bats -mapfile -t SCRIPTS < <( +# Collected with a `while read` loop rather than `mapfile` — macOS ships +# /bin/bash 3.2, which has no `mapfile`. Process substitution (not a pipe) +# keeps the loop in this shell so the appends survive. `sort` is still fed +# newline-delimited output, exactly as before. +SCRIPTS=() +while IFS= read -r script; do + SCRIPTS+=("$script") +done < <( find "$SEARCH_ROOT" -name "test-*.sh" \ -not -path "*/.git/*" \ -not -path "*/.claude/worktrees/*" \ | sort ) -for script in "${SCRIPTS[@]}"; do +# bash before 4.4 treats "${arr[@]}" on an empty array as unbound under +# `set -u`, so every array expansion here uses the ${arr[@]+"${arr[@]}"} guard, +# including the SKIPPED/FAILED loops already fenced by a count check. +for script in ${SCRIPTS[@]+"${SCRIPTS[@]}"}; do rel="${script#"$SEARCH_ROOT/"}" echo "=== $rel ===" rc=0 @@ -60,13 +70,13 @@ done echo "=== Summary: $PASSED passed, ${#SKIPPED[@]} skipped, ${#FAILED[@]} failed ===" if [[ ${#SKIPPED[@]} -gt 0 ]]; then echo "Skipped scripts:" - for s in "${SKIPPED[@]}"; do + for s in ${SKIPPED[@]+"${SKIPPED[@]}"}; do echo " $s" done fi if [[ ${#FAILED[@]} -gt 0 ]]; then echo "Failed scripts:" - for s in "${FAILED[@]}"; do + for s in ${FAILED[@]+"${FAILED[@]}"}; do echo " $s" done exit 1 diff --git a/tests/test-skill-size-check.sh b/tests/test-skill-size-check.sh index 32133db..b62aaf4 100755 --- a/tests/test-skill-size-check.sh +++ b/tests/test-skill-size-check.sh @@ -1,12 +1,14 @@ #!/usr/bin/env bash # Regression test for scripts/skill-size-check.sh: enforces agentskills.io's # 500-line/5,000-token SKILL.md size ceiling. The token half is enforced via a -# word-count proxy (MAX_WORDS, currently 2900) — 5,000 is the token ceiling, -# 2,900 is the word budget the script derives from it. +# word-count proxy (MAX_WORDS, currently 2770) — 5,000 is the token ceiling, +# 2,770 is the word budget the script derives from it at the corpus's densest +# measured prose. set -euo pipefail REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" SCRIPT="$REPO_ROOT/scripts/skill-size-check.sh" +VALIDATE="$REPO_ROOT/plugins/kyberforge/skills/skill-audit/scripts/validate.sh" PASS=0 FAIL=0 @@ -67,6 +69,29 @@ fi MAX_WORDS="$(grep -oE '^MAX_WORDS=[0-9]+' "$SCRIPT" | cut -d= -f2)" MAX_LINES="$(grep -oE '^MAX_LINES=[0-9]+' "$SCRIPT" | cut -d= -f2)" +# The audit (skill-audit/scripts/validate.sh) duplicates both ceilings, because +# a cache-installed plugin's scripts cannot read files outside the plugin +# directory. Nothing but this assertion stops the copies drifting, and drift +# means a SKILL.md passes its own audit and is then rejected by the commit hook. +echo "" +echo "--- the hook and skill-audit's validate.sh agree on both ceilings ---" +if [[ ! -f "$VALIDATE" ]]; then + fail "skill-audit validate.sh not found at $VALIDATE" +else + V_MAX_WORDS="$(grep -oE '^MAX_WORDS = [0-9]+' "$VALIDATE" | grep -oE '[0-9]+')" + V_MAX_LINES="$(grep -oE '^MAX_LINES = [0-9]+' "$VALIDATE" | grep -oE '[0-9]+')" + if [[ "$V_MAX_WORDS" == "$MAX_WORDS" ]]; then + pass "both enforce MAX_WORDS=$MAX_WORDS" + else + fail "MAX_WORDS drift: hook says $MAX_WORDS, validate.sh says ${V_MAX_WORDS:-}" + fi + if [[ "$V_MAX_LINES" == "$MAX_LINES" ]]; then + pass "both enforce MAX_LINES=$MAX_LINES" + else + fail "MAX_LINES drift: hook says $MAX_LINES, validate.sh says ${V_MAX_LINES:-}" + fi +fi + # make_line_fixture builds a file with an exact total line count (frontmatter # included), independent of word count, for the line-boundary tests. make_line_fixture() {