feat(tests): add a static skill quality gate with a shrink-only baseline by knihit · Pull Request #154 · aws-solutions/konductor

knihit · 2026-10-31T23:40:03Z

Description

Stack: 5 of 9 in the skills stack. Base: feat/claude-preload-docs. Depends on #120 and the three Claude preload PRs below it. The four skill-slimming PRs above it lower this gate's baseline.

Tests / CI:

New pytest gate tests/test_skill_quality.py, backed by a small pure-Python module tests/skill_quality.py and a checked-in ratchet tests/skill-quality-baseline.json. It checks every skills/*/SKILL.md for: name matches directory; a description with an activation condition and a length window; line and byte ceilings; link and references/ integrity, including reference-style links and skipping links in inline code; code fences that are never closed, and fences nested in list items.
The baseline holds one exact ceiling per over-threshold dimension per skill. The test fails on growth past a ceiling, on a new or newly fat skill without an entry, and on a stale entry that no longer matches a shrunk skill, so the file can only move down.
--print-baseline regenerates the file and refuses a baseline that would grow; an unreadable baseline fails with --allow-growth too.
.github/workflows/validate-pr.yml: one new step runs --check-ratchet against the merge-base, rejecting a PR whose baseline has a raised or added entry.
Runs under the existing make test-py; no new Make target and no dependency beyond PyYAML.

Docs:

AGENTS.md: one bullet pointing at the checker for the rule list and the thresholds; renaming an oversized skill cannot pass the ratchet.
CHANGELOG.md: [Unreleased] bullet. No VERSION change.

Type of Change

Infrastructure / pipeline
Documentation
Other: pytest gate and baseline

Testing

npm test: not run (no JS changed).
cd cli && make test passes (no cli/ change)
Gate on this branch's head, and on the top of the stack (feat/slim-cypress-test-implementation, which contains it): make build && make synth, cd cli && make test (1,836 Rust tests on the top, 0 failed), make test-skills-sh, uvx --with pyyaml pytest -q tests/ (468 passed on the top), make claude-plugin-check. All pass.
tests/skill_quality.py --check-ratchet passes, and --print-baseline matches the checked-in file.
New code files carry the SPDX header on line 1

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own changes
I have commented my code where necessary
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added or updated tests that prove my change works

Notes for Reviewers

Review: adversarial review in rounds until 0 CRITICAL and 0 IMPORTANT findings remained (5 rounds). The one IMPORTANT, an unclosed code fence that turned off the link and path checks for the rest of a file, now fails the check. Every finding has a fix commit.
An entry passes at or within one tolerance unit below its ceiling (50 lines, 1 KiB, or 50 description characters). A skill that shrinks by more fails until its entry is lowered, so a later PR cannot regrow it to the old ceiling with no diff to the baseline. The failure message carries the value to paste.
This branch's history includes feat(claude): trim each agent's skill preload and warn on oversized preloads #137's original commits, merged earlier; every feat(claude): trim each agent's skill preload and warn on oversized preloads #137 file here matches the docs PR below, so the diff shows only this PR's 6 files.

…reloads Claude Code injects the full SKILL.md of every name in an agent's clientConfig.claudeCli.skills into each subagent it spawns, on every turn. Each agent now preloads only what it needs before its first turn (constraints, plus verification or the agent's primary checker where the prompt mandates it); every other declared skill stays loadable through the Skill tool. dependencies.skills.skillNames is unchanged everywhere. - synth warns (konductor: warning:) when an agent's preload passes 32,768 B or Claude Code's 32-name cap, naming the dropped names. - synth -v prints a "claude preload:" line per agent; --json carries a claude_preload array. Both use the same measuring function as the lint. - The Claude agent file now ends with a "Skills you can load" block listing declared skills that are not preloaded. - k-product-manager, k-tpm and k-quality-assurance prompts no longer say their skills load automatically. - delegation-protocol asks for skills by exact skills/<name> identifier. - User guide and both HTML bundles: Skills is the declared count, with a separate preloaded count; check-guide-facts.py asserts both. - New e2e test synth_claude_preload_e2e.rs fails if any agent in this repository exceeds the budget or cap. Preload bytes per spawned subagent (synth -v, before = origin/main 7428dd9 synthed with this binary, after = this commit): agent before B names after B names k-architect 556,663 38 7,816 2 k-browser 25,899 4 25,899 4 k-developer 247,874 27 15,119 4 k-media-analyzer 2,708 1 2,708 1 k-product-manager 174,132 16 2,708 1 k-quality-assurance 260,121 12 7,816 2 k-researcher 55,255 8 6,340 2 k-tpm 112,859 14 2,708 1 konductor 141,380 13 13,063 2 konductor-cmux-orchestrator 114,097 10 2,708 1 konductor-mux-orchestrator 116,410 10 2,708 1 total 1,807,398 153 89,593 21 Verification run on this commit: - make build && make synth: pass; synth prints no preload warning (main's tree printed 9). - cargo test (cli/konductor-rs): 1800 passed, 0 failed. - make -C cli lint (clippy -D warnings, fmt --check): pass. - make test-skills-sh: pass. - make claude-plugin-check: pass. - make guide-check: 1 failure, the same one origin/main has today (`config` listed in WITHDRAWN_COMMANDS); no new failures. - dist/kiro-cli-v2 and dist/kiro-v3 differ from main's synth only in the k-product-manager, k-tpm and k-quality-assurance agent files and the delegation-protocol skill.

…or specs The Claude Code orchestrator follows its spec and never loads delegation-protocol, so the sentence asking for skills by exact skills/<name> identifier did not reach it there. It now sits on rule 4 of konductor, konductor-mux-orchestrator and konductor-cmux-orchestrator, and delegation-protocol's REQUIRED SKILLS line is back to main's text. Verification run on this commit: - make build && make synth: pass, no preload warning. - cd cli && make test: pass (cargo test 1800 passed, 0 failed; __dump_schema smoke check passed). - make -C cli lint: pass. - make test-skills-sh: 85 passed, 0 failed. - make test-py: 223 passed. - make claude-plugin-check: pass. - npm test: 119 of 136 pass; the 17 failures are identical on main (missing generated scripts/brand-config/lib/constants.js). - make guide-check: 1 failure, the same one main has (`config` in WITHDRAWN_COMMANDS).

…t/claude-preload-trim # Conflicts: # cli/konductor-rs/src/cli/synth/parse_canonical.rs

tests/test_skill_quality.py (run by make test-py) checks every skills/*/SKILL.md for: frontmatter name matching the directory; a description with an activation condition (trigger clause, condition word, scope clause, or a behavior-summary verb, which only warns); description length 60 to 512 chars and at least 8 words; SKILL.md lines and bytes; inline link and bare-path integrity; and references/ shape, orphan and read-cue rules. Logic lives in tests/skill_quality.py, which also offers --scorecard, --print-baseline and --check-ratchet. Thresholds: 500 lines, 32768 bytes, 512 description chars. Tolerance on the shrink side: 50 lines, 1024 bytes, 50 chars. The baseline tests/skill-quality-baseline.json has 15 entries (14 skills over 500 lines, 10 over 32 KiB, sop-state-management over bytes only). A new or grown skill fails; a slimmed skill must lower or delete its entry. validate-pr.yml gains a step that rejects a baseline raised or added relative to the merge-base. AGENTS.md documents the references/ rule. Verified: - make build && make synth: pass (11 agents, 82 skills, 19 SOPs) - cd cli && make test: pass (1832 passed, 0 failed) - make test-skills-sh: pass (10 passed, 0 failed, 1 excluded as before) - uvx --with pyyaml pytest -q tests/: 448 passed, 2 warnings (verb-proxy-only descriptions: app-discovery, dom-inspection) - make claude-plugin-check: pass

Moves this branch from the old #137 head (4b4b6c9) onto the split 137c head (319c033). Conflicts: CHANGELOG.md, the three orchestrator specs, cli/README.md, synth/claude.rs, synth/parse_canonical.rs, tests/synth_claude_preload_e2e.rs, docs/index.html, docs/site/user-guide.html, appendix/contributing.md, sync-html-content.py, check-guide-facts.py. Every #137 file takes the 137c version. CHANGELOG.md keeps this branch's [Unreleased] "Added" bullet and 137c's preload bullet ("warns past 32 KiB, fails past 32 names"). git diff feat/claude-preload-docs HEAD lists only this branch's 6 files. Gate (run from the worktree root on the merge result): - make build && make synth: pass (11 agents, 82 skills, 19 SOPs, no preload warnings) - cd cli && make test: pass (1833 passed, 0 failed) - make test-skills-sh: pass (10 passed, 0 failed, 1 excluded) - uvx --with pyyaml pytest -q tests/: pass (448 passed, 2 warnings) - make claude-plugin-check: pass - skill_quality.py --check-ratchet: 137c has no baseline (CI skips); vs e10c923's baseline: pass

An unclosed fence blanked every later line, so the link and bare-path checks saw nothing below it and passed. The quality gate now reports the line where the open fence starts. Ran: uvx --with pyyaml pytest -q tests/test_skill_quality.py (226 passed).

The trigger verb must now start a clause, 'read' no longer counts as a trigger verb, and a bare 'if' is no longer a condition word. Phrases such as 'Reference to use for ...' and 'what to do if ...' now fail. The docstring calls the patterns heuristics. Every shipped skill still passes. Ran: uvx --with pyyaml pytest -q tests/test_skill_quality.py (230 passed).

The link check now blanks inline code spans before matching, so a quoted '[n](x.md)' is no longer reported. It also resolves reference definitions ('[n]: x.md'), and it reads a target with one level of balanced parentheses as a whole. Ran: uvx --with pyyaml pytest -q tests/test_skill_quality.py (235 passed).

Regenerating the baseline used to write whatever was measured, so a grown or new oversized skill got a raised or added entry and make test-py passed locally. --print-baseline now compares against the committed baseline and exits 1 with the ratchet lines, unless --allow-growth is passed after a threshold change. It also refuses to run when the committed baseline is unreadable, which is what a direct shell redirect onto the file causes. Ran: uvx --with pyyaml pytest -q tests/test_skill_quality.py (238 passed); --print-baseline output matches the committed baseline.

…chet A rename deletes one baseline entry and adds another, and the ratchet rejects the added one. The baseline comment and AGENTS.md now say to bring the skill under every threshold before renaming it. The baseline file was regenerated for the new comment; no entry changed. Ran: uvx --with pyyaml pytest -q tests/test_skill_quality.py (238 passed).

The checkout has no ref, so on pull_request the working tree is the test merge commit and base-branch slimming is already in it. The comment now says the merge-base is used for consistency with the dist/ and CHANGELOG steps. Comment-only change. Ran: parsed validate-pr.yml with PyYAML (ok).

…esholds AGENTS.md hardcoded the size thresholds and left out the name, description, activation and fence checks. It now names the docstring of tests/skill_quality.py as the full list, lists the main failures, and says the thresholds live in tests/skill-quality-baseline.json. Ran: docs-only change; checked the added lines for dashes (none).

Gate on this tree (K1 with every review fix), from the worktree root: - make build && make synth: pass (no preload warnings) - cd cli && make test: pass (1833 passed, 0 failed) - make test-skills-sh: pass (10 scripts, 85 passed, 0 failed, 1 excluded) - uvx --with pyyaml pytest -q tests/: pass (461 passed, 2 warnings) - make claude-plugin-check: pass - skill_quality.py --check-ratchet vs e10c923 baseline: pass (137c has no baseline file, so CI would skip)

"Notes on how to read ..." passed the activation check as a behavior summary. The verb proxy now skips a first word followed by on, of, for, about, in, to, from, with, at or by, so that probe fails. No shipped skill description matches the new exclusion. Ran: uvx --with pyyaml pytest -q tests/test_skill_quality.py (238 passed); skill_quality.py --check-ratchet against the K1 baseline (pass).

--print-baseline --allow-growth redirected onto the baseline truncated it, fell back to the default thresholds and wrote them back, silently undoing a lowered threshold. An existing but unreadable baseline now fails with the temp-file hint whatever the flag, and the docstring shows the temp-file form for --allow-growth. Verified: uvx --with pyyaml pytest -q tests/test_skill_quality.py (239 passed; the new test fails on the old code path).

Verified: text only; no other line of the bullet changed.

The fence patterns allowed at most 3 leading spaces, so a fence inside a list item (often 4 or more) was scanned as prose and its sample links and paths were reported. Any indent now opens a fence, and a closer may sit at most 3 spaces deeper than its opener. Verified: uvx --with pyyaml pytest -q tests/test_skill_quality.py (241 passed); the list-nested fixture fails on the old patterns.

…nted code The round-1 fence fix let any indent open a fence. In CommonMark a line indented 4 or more spaces outside a list item is indented code, so a bare " ```" there opened a fence that ran to the next real closer and hid the prose in between, including a broken link, from the link check. A fence indented 4 or more spaces now opens only when the lines above place it in a list item (walking up through smaller indents to a list marker). Two fixtures cover the top-level case (expects a link finding) and a fence under a wrapped list item (still a fence). Gate on this tree (after merging feat/claude-preload-docs), from the worktree root: - make build && make synth: pass (no warnings) - cd cli && make test: pass (1836 passed, 0 failed) - make test-skills-sh: pass (10 scripts, 85 passed) - uvx --with pyyaml pytest -q tests/: pass (466 passed, 2 warnings) - make claude-plugin-check: pass - skill_quality.py --check-ratchet vs e10c923 baseline: pass (137c has no baseline file); --print-baseline matches the committed baseline

…hrough The walk that decides whether a deep-indented fence marker sits in a list item read the blanked lines, where a whole fenced block is ''. A fence at column 0 ends a list in CommonMark, but the walk skipped its blank lines, reached the earlier "- item" and opened a fence that hid the prose below it, broken links included. The walk now reads a parallel copy in which each fenced line is a placeholder at the opener's indent, so a top-level fence stops it. New fixture: a list, a top-level fence, then a bare " ```" and a broken link, which must report a link finding (fails on the old code). uvx --with pyyaml pytest -q tests/test_skill_quality.py: 244 passed.

…s content Inside a list item the checker opened a fence at any indent. CommonMark measures the 4-space indented-code rule from the item's content column too, so a bare "```" 6 or more spaces under "- Step" is indented code, and opening a fence there hid the prose below it from the link check. The list walk now returns the item's content column (marker indent plus marker width plus its spacing), and a deeper marker opens a fence only within 3 spaces of it. The wrapped-list fixture used a 6-space fence, which is code; it now uses 4 spaces under "- Step one,". New fixture: a marker 10 spaces under "- Step" and a broken link below it, which must report a link finding (fails on the old code). Gate on this tree (after merging feat/claude-preload-docs), from the worktree root: - make build && make synth: pass (no warnings) - cd cli && make test: pass (1836 passed, 0 failed) - make test-skills-sh: pass (10 scripts, 85 passed) - uvx --with pyyaml pytest -q tests/: pass (468 passed, 2 warnings); one earlier run stalled past 76% and was rerun, twice clean - make claude-plugin-check: pass (only the existing ICON_MISSING warning) - skill_quality.py --check-ratchet vs e10c923 baseline: pass (137c has no baseline file); --print-baseline matches the committed baseline

Nihit Kasabwala added 24 commits October 29, 2026 23:12

Merge remote-tracking branch 'origin/feat/agent-skill-index' into fea…

4b4b6c9

…t/claude-preload-trim # Conflicts: # cli/konductor-rs/src/cli/synth/parse_canonical.rs

Merge branch 'feat/claude-preload-docs' into feat/skill-quality-check

5cbae63

docs(agents): rewrap the skill quality bullet at 100 columns

27f3b10

Verified: text only; no other line of the bullet changed.

Merge branch 'feat/claude-preload-docs' into feat/skill-quality-check

395bcb2

Merge branch 'feat/claude-preload-docs' into feat/skill-quality-check

bbedf1f

Merge branch 'feat/claude-preload-docs' into feat/skill-quality-check

50b673d

knihit added this pull request to stack #159 October 31, 2026 23:40

Navigation Menu

Repository navigation

feat(tests): add a static skill quality gate with a shrink-only baseline - #154

feat(tests): add a static skill quality gate with a shrink-only baseline#154
knihit wants to merge 24 commits into
feat/claude-preload-docsfrom
feat/skill-quality-check

knihit commented Oct 31, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

knihit commented Oct 31, 2026

Description

Type of Change

Testing

Checklist

Notes for Reviewers

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant