Skip to content

Commit ff0ab78

Browse files
authored
feat(#907): bounded per-cycle edit budget for skill edits (SkillOpt) (#918)
SkillOpt (arXiv:2605.23904) treats a skill's system-prompt text as an optimized parameter and argues unbounded full-rewrites are unstable, proposing an "edit budget" to bound add/delete/replace changes per cycle. Extends scripts/evolution_skill_lint.py (not a new script) with a second, independent gate alongside the existing dead-script-wiring check: - check_skill_edit_budget / find_edit_budget_violations: pure functions that flag a skills/**/SKILL.md edit when changed lines exceed a proportional churn ratio (default 50% of the file's pre-edit line count), exempting brand-new skills and tiny files (< 20 lines) where the ratio isn't meaningful. - lint_skill_edit_budget + git IO helpers: diffs the working tree against the merge-base with a ref (default origin/main), mirroring the exact git convention already documented in evolution-implementation's landability-gate step (git diff against `git merge-base HEAD origin/main`, not `base...HEAD`, so uncommitted work is included). - New CLI mode: `python scripts/evolution_skill_lint.py --skill-edit-budget [--base <ref>]`. The existing no-flag static lint (lint_repo) is unchanged and still runs by default. Wires the gate into the documented pipeline: evolution-implementation's SKILL.md gets a new Step 2b, between lint/format and the landability gate, telling the agent to run the new flag whenever a PR touches a SKILL.md. Tests: 11 new cases in tests/scripts/test_evolution_skill_integrity.py (TestCheckSkillEditBudget, TestFindEditBudgetViolations) covering the budget boundary, ratio/min-lines overrides, and malformed-input safety. Only the pure functions are unit-tested; the git-diff IO boundary is exercised manually (matches this file's/evolution_merge_gate.py's existing pure-function-only test convention). Out of scope (larger SkillOpt proposal, needs an LLM eval harness): held-out validation sets, a rejection buffer, cross-model validation. Closes #907
1 parent d646f57 commit ff0ab78

3 files changed

Lines changed: 276 additions & 0 deletions

File tree

scripts/evolution_skill_lint.py

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,27 @@
2323
``scripts/evolution_watchdog.sh`` named in #101's warning is correctly ignored).
2424
2525
Pure functions + explicit IO boundary so it is import-safe and unit-testable.
26+
27+
Part 2 — bounded skill edits (#907, SkillOpt)
28+
----------------------------------------------
29+
SkillOpt (arXiv:2605.23904) treats a skill's system-prompt text as an optimized
30+
parameter and argues for an "edit budget": bounded add/delete/replace changes per
31+
cycle instead of unstable full rewrites. This adds that ONE deterministic,
32+
CI-testable piece of the proposal — a per-cycle churn cap on ``skills/**/SKILL.md``
33+
— without the rest of SkillOpt (held-out validation sets, a rejection buffer,
34+
cross-model validation), which need an LLM-driven eval harness and are out of
35+
scope for a mechanical gate. ``check_skill_edit_budget`` / ``find_edit_budget_violations``
36+
are the pure core (unit-tested with synthetic diff stats); ``lint_skill_edit_budget``
37+
is the git-diff IO boundary, run via ``--skill-edit-budget`` (not part of the
38+
default ``lint_repo()`` static check, since it is inherently relative to a base
39+
ref — see ``main()``).
2640
"""
2741

2842
from __future__ import annotations
2943

44+
import os
3045
import re
46+
import subprocess
3147
import sys
3248
from pathlib import Path
3349
from typing import Any, Dict, List, Optional
@@ -51,6 +67,72 @@ def skill_ref_to_name(skill_ref: str) -> str:
5167
return skill_ref.replace("/", "-")
5268

5369

70+
# ── Bounded skill edits (#907 — SkillOpt) ──────────────────────────────────────
71+
# "Edit budget": cap the FRACTION of a skill's pre-change lines a single cycle
72+
# may touch (added + removed), not an absolute line count — a 20-line and a
73+
# 2000-line skill are held to the same proportional standard. Below
74+
# MIN_SKILL_LINES_FOR_BUDGET the ratio is not meaningful (a brand-new skill, or
75+
# one tiny enough that its first real edit always looks like ~100% churn), so
76+
# such files are exempt — this is a "no wholesale rewrite" check, not a
77+
# "no big skill" check. Overridable via EVOLUTION_SKILL_EDIT_BUDGET_RATIO for
78+
# the same reason DEFAULT_MAX_LINES in evolution_merge_gate.py is
79+
# env-overridable via EVOLUTION_MERGE_MAX_LINES.
80+
DEFAULT_EDIT_BUDGET_RATIO = 0.5
81+
MIN_SKILL_LINES_FOR_BUDGET = 20
82+
83+
84+
def check_skill_edit_budget(
85+
path: str,
86+
before_lines: int,
87+
added: int,
88+
removed: int,
89+
ratio: float = DEFAULT_EDIT_BUDGET_RATIO,
90+
min_lines: int = MIN_SKILL_LINES_FOR_BUDGET,
91+
) -> Optional[Dict[str, str]]:
92+
"""Pure check for one changed ``SKILL.md``. ``before_lines`` is the file's
93+
line count at the base ref; ``added``/``removed`` are the diff's numstat
94+
counts. A brand-new file has ``before_lines == 0`` and is exempt (creation
95+
is not a "rewrite"); so is any existing skill under ``min_lines`` (the
96+
ratio isn't meaningful yet). Returns a violation dict, or ``None`` if the
97+
edit is within budget."""
98+
if before_lines < min_lines:
99+
return None
100+
frac = (added + removed) / before_lines
101+
if frac <= ratio:
102+
return None
103+
return {
104+
"path": path,
105+
"kind": "edit_budget_exceeded",
106+
"detail": (
107+
f"{added + removed} changed lines ({frac:.0%}) of a {before_lines}-line "
108+
f"skill exceed the {ratio:.0%} per-cycle edit budget — split into "
109+
f"smaller incremental edits instead of a wholesale rewrite"
110+
),
111+
}
112+
113+
114+
def find_edit_budget_violations(
115+
diffs: List[Dict[str, Any]],
116+
ratio: float = DEFAULT_EDIT_BUDGET_RATIO,
117+
min_lines: int = MIN_SKILL_LINES_FOR_BUDGET,
118+
) -> List[Dict[str, str]]:
119+
"""Pure core, mirrors ``find_violations``. ``diffs`` = [{path, before_lines,
120+
added, removed}, ...] for changed ``skills/**/SKILL.md`` files. No git IO."""
121+
out: List[Dict[str, str]] = []
122+
for d in diffs:
123+
v = check_skill_edit_budget(
124+
path=str(d.get("path") or ""),
125+
before_lines=int(d.get("before_lines") or 0),
126+
added=int(d.get("added") or 0),
127+
removed=int(d.get("removed") or 0),
128+
ratio=ratio,
129+
min_lines=min_lines,
130+
)
131+
if v:
132+
out.append(v)
133+
return out
134+
135+
54136
def find_violations(
55137
stages: List[Dict[str, Any]],
56138
skill_texts: Dict[str, str],
@@ -144,7 +226,115 @@ def lint_repo(repo_root: Optional[Path] = None) -> List[Dict[str, str]]:
144226
return find_violations(stages, skill_texts, existing)
145227

146228

229+
def _git_show_line_count(repo_root: Path, ref: str, path: str) -> int:
230+
"""Line count of ``path`` at ``ref``, or 0 if it doesn't exist there (a
231+
brand-new file) or git fails for any reason. Never raises."""
232+
try:
233+
proc = subprocess.run(
234+
["git", "show", f"{ref}:{path}"],
235+
cwd=repo_root,
236+
capture_output=True,
237+
text=True,
238+
check=False,
239+
)
240+
except OSError:
241+
return 0
242+
if proc.returncode != 0:
243+
return 0
244+
return len(proc.stdout.splitlines())
245+
246+
247+
def _git_merge_base(repo_root: Path, base_ref: str) -> Optional[str]:
248+
"""``git merge-base HEAD base_ref``, or ``None`` if it can't be resolved."""
249+
try:
250+
proc = subprocess.run(
251+
["git", "merge-base", "HEAD", base_ref],
252+
cwd=repo_root,
253+
capture_output=True,
254+
text=True,
255+
check=False,
256+
)
257+
except OSError:
258+
return None
259+
if proc.returncode != 0:
260+
return None
261+
return proc.stdout.strip() or None
262+
263+
264+
def _git_skill_md_diffs(repo_root: Path, base_ref: str) -> List[Dict[str, Any]]:
265+
"""Real git IO: numstat for changed ``skills/**/SKILL.md`` files vs the
266+
merge-base of ``HEAD`` and ``base_ref``, plus each file's pre-change line
267+
count. Diffed against the WORKING TREE (not just HEAD) so this can run
268+
pre-commit — same convention as the evolution-implementation skill's
269+
Step 3 landability gate (``git diff --shortstat "$(git merge-base HEAD
270+
origin/main)"``). Best-effort — any git failure yields an empty list (a
271+
broken/shallow checkout must never crash the gate, only skip it)."""
272+
merge_base = _git_merge_base(repo_root, base_ref) or base_ref
273+
try:
274+
proc = subprocess.run(
275+
["git", "diff", "--numstat", merge_base, "--", "skills/**/SKILL.md"],
276+
cwd=repo_root,
277+
capture_output=True,
278+
text=True,
279+
check=False,
280+
)
281+
except OSError:
282+
return []
283+
if proc.returncode != 0:
284+
return []
285+
out: List[Dict[str, Any]] = []
286+
for line in proc.stdout.splitlines():
287+
parts = line.split("\t")
288+
if len(parts) != 3:
289+
continue
290+
added_s, removed_s, path = parts
291+
if added_s == "-" or removed_s == "-":
292+
continue # binary — not applicable to a SKILL.md, skip defensively
293+
try:
294+
added, removed = int(added_s), int(removed_s)
295+
except ValueError:
296+
continue
297+
out.append(
298+
{
299+
"path": path,
300+
"added": added,
301+
"removed": removed,
302+
"before_lines": _git_show_line_count(repo_root, merge_base, path),
303+
}
304+
)
305+
return out
306+
307+
308+
def lint_skill_edit_budget(
309+
repo_root: Optional[Path] = None, base_ref: str = "origin/main"
310+
) -> List[Dict[str, str]]:
311+
"""Lint the real repo's pending diff: does any changed ``SKILL.md`` blow its
312+
per-cycle edit budget vs ``base_ref``? Meant to run pre-PR in the
313+
evolution-implementation stage (see skills/evolution/evolution-implementation),
314+
mirroring how ``evolution_merge_gate.py``'s diff cap is checked locally
315+
before committing."""
316+
root = Path(repo_root) if repo_root else Path(__file__).resolve().parents[1]
317+
try:
318+
ratio = float(os.environ.get("EVOLUTION_SKILL_EDIT_BUDGET_RATIO", DEFAULT_EDIT_BUDGET_RATIO))
319+
except ValueError:
320+
ratio = DEFAULT_EDIT_BUDGET_RATIO
321+
diffs = _git_skill_md_diffs(root, base_ref)
322+
return find_edit_budget_violations(diffs, ratio=ratio)
323+
324+
147325
def main(argv: List[str]) -> int:
326+
args = argv[1:]
327+
if "--skill-edit-budget" in args:
328+
base_ref = args[args.index("--base") + 1] if "--base" in args else "origin/main"
329+
violations = lint_skill_edit_budget(base_ref=base_ref)
330+
if not violations:
331+
print("[evolution-skill-lint] OK — no skill exceeds its per-cycle edit budget")
332+
return 0
333+
print(f"[evolution-skill-lint] {len(violations)} edit-budget violation(s):", file=sys.stderr)
334+
for v in violations:
335+
print(f" - [{v['kind']}] {v['path']}: {v['detail']}", file=sys.stderr)
336+
return 1
337+
148338
violations = lint_repo()
149339
if not violations:
150340
print("[evolution-skill-lint] OK — no dead skill→script wiring")

skills/evolution/evolution-implementation/SKILL.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,22 @@ python -m pytest tests/ -x -q
223223
```
224224
- Only when local checks are green do you continue to commit + push + PR.
225225

226+
**Step 2b: If the PR touches any `skills/**/SKILL.md` — bounded skill edits
227+
(#907, SkillOpt).** SkillOpt treats a skill's text as an optimized parameter
228+
and argues unbounded full-rewrites are unstable; `evolution_skill_lint.py`
229+
enforces a per-cycle edit budget (default 50% of the skill's line count
230+
changed) so a skill is nudged incrementally, not rewritten wholesale:
231+
232+
```bash
233+
python scripts/evolution_skill_lint.py --skill-edit-budget
234+
```
235+
236+
- **Clean** → proceed to Step 3.
237+
- **Violation** → split the change into a smaller increment that stays inside
238+
the budget, or (if the file genuinely needs a full rewrite this cycle)
239+
document why in the PR body and accept it will need human review — do not
240+
bump `EVOLUTION_SKILL_EDIT_BUDGET_RATIO` just to silence the gate.
241+
226242
**Step 3: Landability gate — fit the autonomous self-merge cap.** Integration
227243
merges unattended ONLY when the PR's total changed lines (additions +
228244
deletions, summed over ALL files — tests and docs count) is ≤ 200

tests/scripts/test_evolution_skill_integrity.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44
stage to run a script that doesn't exist, or a command in a stage without the
55
`terminal` toolset, CI fails — turning the lesson of #101/#188 from "the agent
66
should notice" into "merge is blocked".
7+
8+
``TestCheckSkillEditBudget`` / ``TestFindEditBudgetViolations`` cover the #907
9+
(SkillOpt) bounded-edit gate: the pure per-cycle churn-ratio check, exercised
10+
with synthetic diff stats (the git-diff IO boundary is untested here by design
11+
— same convention as ``evolution_merge_gate.py``'s atomic-merge IO, which is
12+
"exercised only via the pure policy" in its own test file).
713
"""
814

915
import sys
@@ -12,7 +18,9 @@
1218
sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "scripts"))
1319

1420
from evolution_skill_lint import ( # noqa: E402
21+
check_skill_edit_budget,
1522
extract_run_commands,
23+
find_edit_budget_violations,
1624
find_violations,
1725
lint_repo,
1826
skill_ref_to_name,
@@ -92,3 +100,65 @@ def test_missing_skill_md_flagged(self):
92100
def test_skill_ref_to_name():
93101
assert skill_ref_to_name("evolution/research") == "evolution-research"
94102
assert skill_ref_to_name("evolution/upstream-sync") == "evolution-upstream-sync"
103+
104+
105+
class TestCheckSkillEditBudget:
106+
"""#907 (SkillOpt) — the per-cycle churn-ratio cap on one SKILL.md."""
107+
108+
def test_small_edit_within_budget_is_clean(self):
109+
# 20 changed lines of 100 = 20%, well under the 50% default budget.
110+
assert check_skill_edit_budget("skills/x/SKILL.md", 100, 10, 10) is None
111+
112+
def test_edit_exceeding_budget_is_flagged(self):
113+
v = check_skill_edit_budget("skills/x/SKILL.md", 100, 40, 20) # 60%
114+
assert v is not None
115+
assert v["kind"] == "edit_budget_exceeded"
116+
assert v["path"] == "skills/x/SKILL.md"
117+
assert "60%" in v["detail"]
118+
119+
def test_edit_at_exact_ratio_is_ok(self):
120+
# Exactly at the cap (50/100 = 50%) — inclusive, not a violation.
121+
assert check_skill_edit_budget("skills/x/SKILL.md", 100, 50, 0) is None
122+
123+
def test_edit_just_over_ratio_is_flagged(self):
124+
v = check_skill_edit_budget("skills/x/SKILL.md", 100, 51, 0)
125+
assert v is not None and v["kind"] == "edit_budget_exceeded"
126+
127+
def test_brand_new_skill_is_exempt(self):
128+
# before_lines == 0 -> creation, not a rewrite.
129+
assert check_skill_edit_budget("skills/x/SKILL.md", 0, 500, 0) is None
130+
131+
def test_tiny_existing_skill_is_exempt(self):
132+
# Below MIN_SKILL_LINES_FOR_BUDGET (20) the ratio isn't meaningful yet.
133+
assert check_skill_edit_budget("skills/x/SKILL.md", 10, 10, 0) is None
134+
135+
def test_custom_ratio_override(self):
136+
# 30% churn passes the 50% default but fails a stricter 20% budget.
137+
assert check_skill_edit_budget("skills/x/SKILL.md", 100, 30, 0) is None
138+
v = check_skill_edit_budget("skills/x/SKILL.md", 100, 30, 0, ratio=0.2)
139+
assert v is not None
140+
141+
def test_custom_min_lines_override(self):
142+
# A 15-line skill is exempt by default (< 20) but covered once the
143+
# floor is lowered.
144+
assert check_skill_edit_budget("skills/x/SKILL.md", 15, 15, 0) is None
145+
v = check_skill_edit_budget("skills/x/SKILL.md", 15, 15, 0, min_lines=10)
146+
assert v is not None
147+
148+
149+
class TestFindEditBudgetViolations:
150+
def test_mixed_diffs_only_flags_the_oversized_one(self):
151+
diffs = [
152+
{"path": "skills/a/SKILL.md", "before_lines": 100, "added": 10, "removed": 5},
153+
{"path": "skills/b/SKILL.md", "before_lines": 100, "added": 60, "removed": 10},
154+
]
155+
v = find_edit_budget_violations(diffs)
156+
assert [x["path"] for x in v] == ["skills/b/SKILL.md"]
157+
158+
def test_no_diffs_is_clean(self):
159+
assert find_edit_budget_violations([]) == []
160+
161+
def test_missing_keys_default_safely(self):
162+
# A malformed entry (missing counts) defaults to 0 everywhere rather
163+
# than raising — before_lines=0 makes it exempt (looks "new").
164+
assert find_edit_budget_violations([{"path": "skills/x/SKILL.md"}]) == []

0 commit comments

Comments
 (0)