refactor!: carry out the simplification audit across gates, tests, plugins and docs #135
@@ -7,7 +7,7 @@ description: >
|
|||||||
fixes -> agent-author.
|
fixes -> agent-author.
|
||||||
allowed-tools: Bash Read
|
allowed-tools: Bash Read
|
||||||
metadata:
|
metadata:
|
||||||
version: "1.0.2"
|
version: "1.0.3"
|
||||||
category: factory
|
category: factory
|
||||||
source_keys:
|
source_keys:
|
||||||
- agentskills-home
|
- agentskills-home
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
extends: existence
|
extends: existence
|
||||||
message: "Composition or architecture note in a description: '%s' — a description carries a trigger, one capability clause and a boundary clause only; move this to README.md"
|
message: "Composition or architecture note in a description: '%s' — a description carries a trigger, one capability clause and a boundary clause only; move this to the body or a references/ file"
|
||||||
level: error
|
level: error
|
||||||
scope: text.frontmatter.description
|
scope: text.frontmatter.description
|
||||||
ignorecase: true
|
ignorecase: true
|
||||||
|
|||||||
@@ -55,7 +55,8 @@ A model-invoked description carries exactly three things:
|
|||||||
3. **Boundary clause.** Compressed form: `Not <thing> -> <skill-name>.` The target must resolve to
|
3. **Boundary clause.** Compressed form: `Not <thing> -> <skill-name>.` The target must resolve to
|
||||||
a real skill directory or agent file in the authoring source.
|
a real skill directory or agent file in the authoring source.
|
||||||
|
|
||||||
Everything else belongs in the body or in the plugin's `README.md`.
|
Everything else belongs in the body or in a `references/` file. Not a `README.md`: `plugins/gitea/`
|
||||||
|
and `plugins/lint/` both ship agents this file governs and neither has one.
|
||||||
|
|
||||||
## Indirect triggers — conditional, never blanket
|
## Indirect triggers — conditional, never blanket
|
||||||
|
|
||||||
|
|||||||
@@ -732,8 +732,41 @@ def boundary_targets(description):
|
|||||||
return sorted({name for name, _, _ in _extract(description)})
|
return sorted({name for name, _, _ in _extract(description)})
|
||||||
|
|
||||||
|
|
||||||
def _arrow_targets(description):
|
def _clause_end(description, pos):
|
||||||
"""Names extracted from ARROW notation specifically.
|
"""Where CLAUSE_BODY stops scanning forward from `pos`.
|
||||||
|
|
||||||
|
The same two stops the class itself encodes: a `;`, or a `.` that is not
|
||||||
|
followed by a non-space character (a sentence end rather than a dot inside
|
||||||
|
`AGENTS.md`).
|
||||||
|
"""
|
||||||
|
for index in range(pos, len(description)):
|
||||||
|
char = description[index]
|
||||||
|
if char == ';':
|
||||||
|
return index
|
||||||
|
if char == '.' and not description[index + 1:index + 2].strip():
|
||||||
|
return index
|
||||||
|
return len(description)
|
||||||
|
|
||||||
|
|
||||||
|
def _arrow_clause_spans(description):
|
||||||
|
"""(start, end) for EACH ADR-0020 arrow clause, one span per clause.
|
||||||
|
|
||||||
|
A clause runs from its `Not` to whichever comes first: the start of the
|
||||||
|
NEXT arrow clause, or the end of the clause body. Bounding on the next
|
||||||
|
clause is what keeps two clauses joined by a comma inside one sentence
|
||||||
|
apart — a sentence-scoped span would merge them and let the second clause's
|
||||||
|
target vouch for the first.
|
||||||
|
"""
|
||||||
|
starts = [match.start() for match in BOUNDARY_ARROW.finditer(description)]
|
||||||
|
spans = []
|
||||||
|
for index, start in enumerate(starts):
|
||||||
|
limit = starts[index + 1] if index + 1 < len(starts) else len(description)
|
||||||
|
spans.append((start, min(limit, _clause_end(description, start))))
|
||||||
|
return spans
|
||||||
|
|
||||||
|
|
||||||
|
def _arrow_clause_parses(clause):
|
||||||
|
"""True when either arrow extractor reads a target out of ONE clause.
|
||||||
|
|
||||||
Kept apart from boundary_targets() because the arrow form is the one shape
|
Kept apart from boundary_targets() because the arrow form is the one shape
|
||||||
that ALWAYS names a target: ADR-0020's `Not <thing> -> <name>`. A clause
|
that ALWAYS names a target: ADR-0020's `Not <thing> -> <name>`. A clause
|
||||||
@@ -741,15 +774,11 @@ def _arrow_targets(description):
|
|||||||
that deserves its own message, and telling it apart needs the arrow targets
|
that deserves its own message, and telling it apart needs the arrow targets
|
||||||
alone rather than every target in the description.
|
alone rather than every target in the description.
|
||||||
"""
|
"""
|
||||||
out = []
|
for match in ARROW_MARKED.finditer(clause):
|
||||||
for sentence in SENTENCE_SPLIT.split(description):
|
|
||||||
for match in ARROW_MARKED.finditer(sentence):
|
|
||||||
name, _, _ = _first(match)
|
name, _, _ = _first(match)
|
||||||
if name:
|
if name:
|
||||||
out.append(name)
|
return True
|
||||||
for match in ARROW_BOUNDARY.finditer(sentence):
|
return bool(ARROW_BOUNDARY.search(clause))
|
||||||
out.append(match.group(1))
|
|
||||||
return out
|
|
||||||
|
|
||||||
|
|
||||||
def boundary_clause_status(description):
|
def boundary_clause_status(description):
|
||||||
@@ -761,16 +790,28 @@ def boundary_clause_status(description):
|
|||||||
three of them reworded a correct clause to satisfy a regex instead.
|
three of them reworded a correct clause to satisfy a regex instead.
|
||||||
|
|
||||||
'unparsed' is the narrow, certain case: an ADR-0020 arrow clause was
|
'unparsed' is the narrow, certain case: an ADR-0020 arrow clause was
|
||||||
detected and NO target came out of it. The arrow form always names one, so
|
detected and NO target came out of IT. The arrow form always names one, so
|
||||||
zero targets means the name is written in a shape the extractor cannot see
|
zero targets means the name is written in a shape the extractor cannot see
|
||||||
— a single-word bare target (`Not X -> forge`, which has to be written
|
— a single-word bare target (`Not X -> forge`, which has to be written
|
||||||
`` `forge` `` or `/forge`) is the live example, since single-word names are
|
`` `forge` `` or `/forge`) is the live example, since single-word names are
|
||||||
deliberately not matchable bare.
|
deliberately not matchable bare.
|
||||||
|
|
||||||
|
The test is PER CLAUSE, and that is the whole point of the span walk. Both
|
||||||
|
operands used to take the whole description, so ONE arrow clause that
|
||||||
|
parsed suppressed the diagnostic for every other clause beside it: a
|
||||||
|
backticked hyphenated target wrapped across a line break inside a `>`
|
||||||
|
folded scalar — `` `git-`` / ``commits` `` — went unchecked with no ERROR
|
||||||
|
and no SUGGESTION, while the same wrap written bare was reported correctly.
|
||||||
|
26 of this corpus's 38 skill descriptions carry more than one arrow clause,
|
||||||
|
so the suppression covered most of it. This is the issue #100 regression
|
||||||
|
class, and a whole-description test cannot see it by construction.
|
||||||
|
|
||||||
A PROSE clause yielding no target is NOT reported: "Do not use for anything
|
A PROSE clause yielding no target is NOT reported: "Do not use for anything
|
||||||
else" is a complete and legitimate boundary clause that names nowhere to go.
|
else" is a complete and legitimate boundary clause that names nowhere to go.
|
||||||
"""
|
"""
|
||||||
if BOUNDARY_ARROW.search(description) and not _arrow_targets(description):
|
spans = _arrow_clause_spans(description)
|
||||||
|
if spans and not all(_arrow_clause_parses(description[start:end])
|
||||||
|
for start, end in spans):
|
||||||
return 'unparsed'
|
return 'unparsed'
|
||||||
if has_boundary_clause(description):
|
if has_boundary_clause(description):
|
||||||
return 'present'
|
return 'present'
|
||||||
|
|||||||
@@ -38,7 +38,7 @@ set -euo pipefail
|
|||||||
# Divergence 2: a path-shaped argument that does not exist is a hard error
|
# Divergence 2: a path-shaped argument that does not exist is a hard error
|
||||||
# (exit 2). Bare vale drops it, falls back to reading stdin, and prints
|
# (exit 2). Bare vale drops it, falls back to reading stdin, and prints
|
||||||
# `0 errors ... in stdin` with exit 0 — a typo'd target is then indistinguishable
|
# `0 errors ... in stdin` with exit 0 — a typo'd target is then indistinguishable
|
||||||
# from a clean run. Both audit skills treat a `0 files` report as NOT RUN rather
|
# from a clean run. factory-audit treats a `0 files` report as NOT RUN rather
|
||||||
# than clean, and `in stdin` does not match that guard, so the silent form would
|
# than clean, and `in stdin` does not match that guard, so the silent form would
|
||||||
# read as "prefilter clean" and skip the LLM fallback. Erroring is the only way
|
# read as "prefilter clean" and skip the LLM fallback. Erroring is the only way
|
||||||
# to keep that guard honest. Linting prose piped on stdin is therefore
|
# to keep that guard honest. Linting prose piped on stdin is therefore
|
||||||
|
|||||||
@@ -945,6 +945,48 @@ make_hand_invoked_skill() {
|
|||||||
assert_output --partial "no target could be read"
|
assert_output --partial "no target could be read"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# ADR-0020 — the unparsed diagnostic is PER CLAUSE, not per description
|
||||||
|
#
|
||||||
|
# boundary_clause_status() used to test `BOUNDARY_ARROW.search(description) and
|
||||||
|
# not _arrow_targets(description)`. Both operands took the WHOLE description,
|
||||||
|
# so ONE arrow clause that parsed suppressed the diagnostic for every other
|
||||||
|
# clause beside it.
|
||||||
|
#
|
||||||
|
# The shape that hides there is a backticked hyphenated target wrapped across
|
||||||
|
# the line break of a `>` folded scalar: the fold turns `` `fixture-sibling- ``
|
||||||
|
# / `` skill` `` into `fixture-sibling- skill`, which no extractor can read.
|
||||||
|
# Written BARE the same wrap is reported correctly, so the two spellings
|
||||||
|
# disagreed. 26 of this repo's 38 skill descriptions carry more than one arrow
|
||||||
|
# clause, which is how wide the suppression was. This is the #100 regression
|
||||||
|
# class: no ERROR, no SUGGESTION, exit 0.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
@test "ADR-0020: an unparsed arrow clause is reported even when a sibling clause parses" {
|
||||||
|
local skill
|
||||||
|
skill="$(make_fixture_tree "$TMPDIR/tree" "my-skill")"
|
||||||
|
# Written by hand rather than through make_sized_skill: the `>` folded
|
||||||
|
# scalar and the wrap INSIDE the backticks are the fixture. The second
|
||||||
|
# clause parses and resolves against the fixture sibling, and that is what
|
||||||
|
# used to silence the first.
|
||||||
|
cat > "$skill/SKILL.md" <<'EOF'
|
||||||
|
---
|
||||||
|
name: my-skill
|
||||||
|
description: >
|
||||||
|
Use when doing the thing. Not the other thing -> `fixture-sibling-
|
||||||
|
skill`. Not a third thing -> `fixture-sibling-skill`.
|
||||||
|
metadata:
|
||||||
|
version: "1.0.0"
|
||||||
|
---
|
||||||
|
|
||||||
|
word word word word word word word word word word
|
||||||
|
EOF
|
||||||
|
run bash "$SCRIPT" "$skill"
|
||||||
|
assert_success
|
||||||
|
assert_output --partial "no target could be read"
|
||||||
|
refute_output --partial "has no boundary clause"
|
||||||
|
}
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Encoding, write side: sys.stdout/stderr.reconfigure(encoding='utf-8')
|
# Encoding, write side: sys.stdout/stderr.reconfigure(encoding='utf-8')
|
||||||
#
|
#
|
||||||
|
|||||||
@@ -442,6 +442,24 @@ fi
|
|||||||
# terminal and therefore danglable. The issue #99 retrofit cut that composition
|
# terminal and therefore danglable. The issue #99 retrofit cut that composition
|
||||||
# sentence and the dangling target went with it, so the set is down to one.
|
# sentence and the dangling target went with it, so the set is down to one.
|
||||||
#
|
#
|
||||||
|
# Correction (2026-09-20): the historical text carried NO backticks. `7801589^`
|
||||||
|
# has gitea-issues' description as "Composes gitea-labels-\n milestones for all
|
||||||
|
# label inference/resolution and milestone lookup", bare, so the token was read
|
||||||
|
# by the route-verb path — `Composes` is a ROUTE_VERB and the name matched
|
||||||
|
# NAME_HYPH — and not by the backtick sweep. Everything the paragraph above says
|
||||||
|
# about the fold and the trailing hyphen holds; only the spelling is wrong.
|
||||||
|
#
|
||||||
|
# The spelling is the load-bearing part, because the two are not equally
|
||||||
|
# visible. Backticked, that same wrap reaches every extractor as
|
||||||
|
# `` `gitea-labels- milestones` ``, which none of them can read: the opening
|
||||||
|
# backtick blocks the bare NAME_HYPH alternative and the space inside blocks the
|
||||||
|
# backticked one. Bare, it was extracted and reported all along, which is the
|
||||||
|
# only reason this dangling target was ever measured. In an ARROW clause the
|
||||||
|
# backticked wrap was silent until the 2026-09-20 fix to
|
||||||
|
# boundary_clause_status() in the shared resolver made the unparsed diagnostic
|
||||||
|
# per clause: before it, one sibling clause that parsed suppressed the finding
|
||||||
|
# for the whole description.
|
||||||
|
#
|
||||||
# `neuledge-context` was the last one. The issue #99 wave-3 retrofit deleted that
|
# `neuledge-context` was the last one. The issue #99 wave-3 retrofit deleted that
|
||||||
# boundary clause outright — commit `6146120` had already deleted the skill it
|
# boundary clause outright — commit `6146120` had already deleted the skill it
|
||||||
# named, and nothing has owned MCP-server installation since — so the corpus
|
# named, and nothing has owned MCP-server installation since — so the corpus
|
||||||
|
|||||||
Reference in New Issue
Block a user