Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ jobs:
runs-on: ubuntu-latest
outputs:
docs_only: ${{ steps.changes.outputs.docs_only }}
metadata_only: ${{ steps.changes.outputs.metadata_only }}
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
Expand All @@ -37,6 +38,7 @@ jobs:
if ! git show "$BASE_SHA:scripts/classify_ci_changes.py" > "$classifier"; then
echo "::notice::Trusted classifier is not present on the base branch; running full CI."
echo "docs_only=false" >> "$GITHUB_OUTPUT"
echo "metadata_only=false" >> "$GITHUB_OUTPUT"
exit 0
fi
python3 "$classifier" --base "$BASE_SHA" --head "$HEAD_SHA"
Expand Down Expand Up @@ -204,7 +206,7 @@ jobs:
tier3-macos:
name: Tier 3 macOS contract and progress
needs: classify-changes
if: ${{ !cancelled() && needs.classify-changes.outputs.docs_only != 'true' }}
if: ${{ !cancelled() && needs.classify-changes.outputs.docs_only != 'true' && needs.classify-changes.outputs.metadata_only != 'true' }}
runs-on: macos-latest
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/security.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ jobs:
runs-on: ubuntu-latest
outputs:
docs_only: ${{ steps.changes.outputs.docs_only }}
metadata_only: ${{ steps.changes.outputs.metadata_only }}
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
Expand All @@ -42,6 +43,7 @@ jobs:
if ! git show "$BASE_SHA:scripts/classify_ci_changes.py" > "$classifier"; then
echo "::notice::Trusted classifier is not present on the base branch; running full CI."
echo "docs_only=false" >> "$GITHUB_OUTPUT"
echo "metadata_only=false" >> "$GITHUB_OUTPUT"
exit 0
fi
python3 "$classifier" --base "$BASE_SHA" --head "$HEAD_SHA"
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ All notable changes to SkillEvaluator are documented in this file.

## Unreleased

### Added

- `skillevaluator validate --tier3 --previous-skill <path>` now skips a fresh
Tier 3 run when only the `metadata` field in `SKILL.md` changed and the prior
skill has a generated `skill-card.md` or `BENCHMARK.md`. The decision fails
closed for any behavioral change, invalid frontmatter, or missing evidence.

### Fixed

- Tier 3 paired pass@k evidence now respects Python's active integer-string
Expand Down
151 changes: 146 additions & 5 deletions scripts/classify_ci_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,149 @@

DOC_PREFIXES = (b"docs/", b"fern/")
KNOWN_STATUSES = frozenset(b"ACDMRTUXB")
SKILL_FILENAME = b"SKILL.md"
TIER3_EVIDENCE_FILENAMES = (b"skill-card.md", b"BENCHMARK.md")


def is_docs_only(paths: Sequence[bytes]) -> bool:
"""Return whether every changed path belongs to published documentation."""
return bool(paths) and all(path.startswith(DOC_PREFIXES) for path in paths)


def _is_skill_file(path: bytes) -> bool:
return path == SKILL_FILENAME or path.endswith(b"/" + SKILL_FILENAME)


def _split_frontmatter(content: bytes) -> tuple[bytes, bytes] | None:
"""Split a Markdown file into YAML frontmatter and body, if present.

This intentionally validates only the delimiter shape. The Tier 1 schema
validator remains responsible for validating the YAML itself; CI routing
must stay dependency-free because it runs before the project is installed.
"""
lines = content.splitlines(keepends=True)
if not lines or lines[0].rstrip(b"\r\n") != b"---":
return None

for index, line in enumerate(lines[1:], start=1):
if line.rstrip(b"\r\n") in {b"---", b"..."}:
return b"".join(lines[: index + 1]), b"".join(lines[index + 1 :])
return None


def _metadata_section(frontmatter: bytes) -> tuple[bytes, bytes, bytes] | None:
"""Return the immutable prefix/suffix around a top-level ``metadata`` key.

The PR classifier must run before dependencies are installed, so it keeps a
deliberately narrow YAML shape instead of parsing arbitrary YAML. Anything
outside this conventional top-level metadata block is treated as behavioral
and therefore falls back to a full Tier 3 run.
"""
lines = frontmatter.splitlines(keepends=True)
for index, line in enumerate(lines[1:-1], start=1):
if not line.startswith(b"metadata:"):
continue
value = line[len(b"metadata:") :].strip()
end = index + 1
if not value or value.startswith(b"#"):
while end < len(lines) - 1:
candidate = lines[end]
if candidate.startswith((b" ", b"\t", b"\r", b"\n", b"#")):
end += 1
continue
break
return b"".join(lines[:index]), b"".join(lines[index:end]), b"".join(lines[end:])
return None


def _is_metadata_only_change(previous: bytes, current: bytes) -> bool:
previous_parts = _split_frontmatter(previous)
current_parts = _split_frontmatter(current)
if previous_parts is None or current_parts is None:
return False
previous_frontmatter, previous_body = previous_parts
current_frontmatter, current_body = current_parts
if previous_body != current_body:
return False

previous_metadata = _metadata_section(previous_frontmatter)
current_metadata = _metadata_section(current_frontmatter)
if previous_metadata is None and current_metadata is None:
return False
if previous_metadata is None:
current_prefix, current_block, current_suffix = current_metadata
return current_prefix + current_suffix == previous_frontmatter and bool(current_block)
if current_metadata is None:
previous_prefix, previous_block, previous_suffix = previous_metadata
return previous_prefix + previous_suffix == current_frontmatter and bool(previous_block)
previous_prefix, previous_block, previous_suffix = previous_metadata
current_prefix, current_block, current_suffix = current_metadata
return (
previous_prefix == current_prefix
and previous_suffix == current_suffix
and previous_block != current_block
)


def _merge_base(repo: Path, base: str, head: str) -> str:
result = subprocess.run(
["git", "-C", str(repo), "merge-base", base, head],
check=True,
capture_output=True,
)
merge_base = result.stdout.strip().decode("ascii")
return _validate_revision(merge_base)


def _revision_file(repo: Path, revision: str, path: bytes) -> bytes:
"""Read ``path`` from a Git revision without touching the worktree."""
path_text = os.fsdecode(path)
result = subprocess.run(
["git", "-C", str(repo), "show", f"{revision}:{path_text}"],
check=True,
capture_output=True,
)
return result.stdout


def _has_tier3_evidence(repo: Path, revision: str, skill_path: bytes) -> bool:
skill_parent = skill_path.rsplit(b"/", 1)[0] if b"/" in skill_path else b""
for filename in TIER3_EVIDENCE_FILENAMES:
evidence_path = filename if not skill_parent else skill_parent + b"/" + filename
try:
_revision_file(repo, revision, evidence_path)
except subprocess.CalledProcessError:
continue
return True
return False


def is_metadata_only(repo: Path, base: str, head: str, paths: Sequence[bytes]) -> bool:
"""Return whether a diff changes only existing skills' frontmatter.

Tier 3 is expensive and need not run after metadata-only edits, but this is
safe only when the affected skill already has a generated card or benchmark
from an earlier evaluation. Evidence is read from the merge-base revision
so a pull request cannot qualify itself by adding a new artifact.
"""
if not paths or not all(_is_skill_file(path) for path in paths):
return False

merge_base = _merge_base(repo, base, head)
for path in paths:
if not _has_tier3_evidence(repo, merge_base, path):
return False

try:
previous = _revision_file(repo, merge_base, path)
current = _revision_file(repo, head, path)
except subprocess.CalledProcessError:
return False
if not _is_metadata_only_change(previous, current):
return False
return True


def parse_name_status_z(payload: bytes) -> list[bytes]:
"""Parse ``git diff --name-status -z`` without losing rename sources."""
if not payload:
Expand Down Expand Up @@ -89,13 +225,16 @@ def changed_paths(repo: Path, base: str, head: str) -> list[bytes]:
return parse_name_status_z(result.stdout)


def _write_result(docs_only: bool) -> None:
line = f"docs_only={'true' if docs_only else 'false'}"
def _write_result(docs_only: bool, metadata_only: bool) -> None:
lines = (
f"docs_only={'true' if docs_only else 'false'}",
f"metadata_only={'true' if metadata_only else 'false'}",
)
output_path = os.environ.get("GITHUB_OUTPUT")
if output_path:
with Path(output_path).open("a", encoding="utf-8") as output:
output.write(f"{line}\n")
print(line)
output.writelines(f"{line}\n" for line in lines)
print(*lines, sep="\n")


def _parser() -> argparse.ArgumentParser:
Expand All @@ -114,11 +253,13 @@ def main(argv: Sequence[str] | None = None) -> int:
if not paths:
raise ValueError("no changed paths found")
docs_only = is_docs_only(paths)
metadata_only = is_metadata_only(args.repo, args.base, args.head, paths)
except (OSError, subprocess.CalledProcessError, ValueError) as error:
print(f"change classification failed; falling back to full CI: {error}", file=sys.stderr)
docs_only = False
metadata_only = False

_write_result(docs_only)
_write_result(docs_only, metadata_only)
return 0


Expand Down
71 changes: 70 additions & 1 deletion src/skillevaluator/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ def _run_agent_eval_or_skip(
harbor_keep_jobs: bool = False,
block_on_agent_eval: bool = False,
validate_source: bool = True,
previous_skill: Path | None = None,
progress_reporter=None,
) -> ValidationResult:
"""Run Tier 3 live agent evaluation and fold the result into the combined report.
Expand All @@ -462,6 +463,36 @@ def _run_agent_eval_or_skip(
describing why Tier 3 could not run. Tier 3 remains advisory by default,
and callers can opt into blocking behavior.
"""
if previous_skill is not None:
from skillevaluator.evaluation.tier3_report import advisory_skip_result
from skillevaluator.tier3.change_detection import tier3_run_decision

decision = tier3_run_decision(target_path, previous_skill)
if decision.should_skip:
change_summary = (
"only SKILL.md metadata changed"
if decision.reason_code == "metadata_only_change"
else "SKILL.md content is unchanged"
)
result = advisory_skip_result(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Carry validated reuse through reports and gates

advisory_skip_result() sets passed=False and carries no prior Tier 3 payload, while this branch labels it not_required. I reproduced default validation reporting overall success in JSON while BENCHMARK.md said INCOMPLETE; with --block-on-agent-eval, the same accepted decision failed the gate and exited 1. This makes the optimization unusable for strict or publication CI. Please emit a dedicated validated-reuse result carrying the prior provenance/evidence, or update every gate and reporter to consume the reuse state consistently.

f"Tier 3 live evaluation skipped: {change_summary} "
f"since the prior evaluation ({decision.evidence_file}).",
skill_name=target_path.name,
)
result.metadata["tier3_change_decision"] = decision.to_dict()
result.metadata["tier3_applicability"] = {
"applicability": "not_required",
"reason_code": decision.reason_code,
"source_kind": "skill",
}
payload = result.metadata.get("agent_eval")
if isinstance(payload, dict):
payload["reason_code"] = decision.reason_code
summary = payload.get("summary")
if isinstance(summary, dict):
summary["reason_code"] = decision.reason_code
return result

if validate_source:
from skillevaluator.evaluation.tier3_report import dataset_required_result
from skillevaluator.tier3.evals_spec import validate_tier3_source
Expand Down Expand Up @@ -973,6 +1004,14 @@ def _print_run_banner(target_path: Path, content_type: str, profile: str | None)
help_group=_TIER3_GROUP,
help="Also run Tier 3 live agent evaluation (requires a valid eval dataset or native Harbor source).",
)
@click.option(
"--previous-skill",
type=click.Path(exists=True, path_type=Path),
default=None,
cls=GroupedOption,
help_group=_TIER3_GROUP,
help="Previous evaluated copy of this skill. Metadata-only SKILL.md changes skip a fresh Tier 3 run when it has a skill card or benchmark.",
)
@click.option(
"--block-on-agent-eval/--no-block-on-agent-eval",
default=None,
Expand Down Expand Up @@ -1131,6 +1170,7 @@ def validate(
dedup: bool,
block_on_dedup: bool | None,
agent_eval: bool,
previous_skill: Path | None,
block_on_agent_eval: bool | None,
autopilot: bool,
agents: str,
Expand Down Expand Up @@ -1233,6 +1273,16 @@ def validate(
)
return

tier3_change_decision = None
if previous_skill is not None:
if not agent_eval:
raise click.ClickException("--previous-skill requires --tier3 or --agent-eval.")
if not preflight_tier3_source:
raise click.ClickException("--previous-skill applies only to a single skill.")
from skillevaluator.tier3.change_detection import tier3_run_decision

tier3_change_decision = tier3_run_decision(target_path, previous_skill)

# Quiet (default) drives the compact pipeline view; --verbose keeps the
# historical full-detail stream, as does DEBUG logging via the group -v.
quiet = not verbose and not logging.getLogger().isEnabledFor(logging.DEBUG)
Expand Down Expand Up @@ -1351,7 +1401,7 @@ def _on_check(name: str) -> None:
# Tier 3 is advisory, so a dataset-generation failure must not abort
# validate after Tier 1/2 already ran -- Tier 3 skips with the reason.
autopilot_error: str | None = None
if autopilot:
if autopilot and (tier3_change_decision is None or tier3_change_decision.should_run):
try:
dataset_note = _ensure_autopilot_dataset(target_path, quiet=quiet)
except (Exception, SystemExit) as exc:
Expand Down Expand Up @@ -1391,6 +1441,7 @@ def _on_engine_tail(lines: list[str]) -> None:
harbor_keep_jobs=harbor_keep_jobs,
block_on_agent_eval=block_on_agent_eval_effective,
validate_source=preflight_tier3_source,
previous_skill=previous_skill,
progress_reporter=reporter,
)
results.append(tier3_result)
Expand Down Expand Up @@ -1739,6 +1790,12 @@ def dedup_scan(
show_default=True,
help="Comma-separated Harbor agents (claude is an alias for claude-code).",
)
@click.option(
"--previous-skill",
type=click.Path(exists=True, path_type=Path),
default=None,
help="Previous evaluated copy of this skill. Metadata-only SKILL.md changes skip the live evaluation when it has a skill card or benchmark.",
)
@click.option("--env-mode", default="docker", show_default=True, type=ENV_MODE_CHOICE)
@click.option(
"--autopilot",
Expand Down Expand Up @@ -1783,6 +1840,7 @@ def dedup_scan(
def evaluate(
skill_path: Path,
agents: str,
previous_skill: Path | None,
env_mode: str,
autopilot: bool,
skip_baseline: bool,
Expand All @@ -1808,6 +1866,17 @@ def evaluate(
progress: str,
) -> None:
"""Run Tier 3 live agent evaluation."""
if previous_skill is not None:
from skillevaluator.tier3.change_detection import tier3_run_decision

decision = tier3_run_decision(skill_path, previous_skill)
if decision.should_skip:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Include the evaluation configuration in the reuse key

This early return occurs before EvaluationOptions is built, and the previous artifact is never parsed for its run identity. With otherwise identical manifests, I changed --model, --n-attempts, and --env-mode; the command still printed skill_unchanged and exited 0. Model and agent selection, attempts, environment, grading/evaluator versions, and related options all affect evidence freshness, so please compare the complete prior and current evaluation identities before allowing reuse.

click.echo(
"Tier 3 live evaluation skipped: "
f"{decision.reason_code} confirmed by prior {decision.evidence_file}."
)
return

from skillevaluator.evaluation import EvaluationOptions, EvaluationService
from skillevaluator.tier3.harbor.progress import create_progress_reporter

Expand Down
Loading
Loading