Skip to content

ci(skills): show before→after eval scores on skill PRs #36008

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PureWeen merged 3 commits into from
Jul 20, 2026
Merged

ci(skills): show before→after eval scores on skill PRs #36008

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a26151d
Show before→after eval scores on skill PRs
PureWeen Jun 18, 2026
22db45c
Fix skill evaluation baseline comparison
Copilot Jul 15, 2026
75f1b41
Harden skill evaluation baselines
Copilot Jul 15, 2026
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
250 changes: 246 additions & 4 deletions .github/workflows/skill-validation.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ jobs:
pull-requests: read
outputs:
head_sha: ${{ github.event.pull_request.head.sha }}
base_sha: ${{ github.event.pull_request.base.sha }}
head_repo: ${{ github.event.pull_request.head.repo.full_name }}
pr_number: ${{ github.event.pull_request.number }}
is_contributor: ${{ steps.perms.outputs.is_contributor }}
Expand Down Expand Up @@ -160,6 +161,7 @@ jobs:
runs-on: ubuntu-latest
outputs:
head_sha: ${{ steps.pr.outputs.head_sha }}
base_sha: ${{ steps.pr.outputs.base_sha }}
head_repo: ${{ steps.pr.outputs.head_repo }}
pr_number: ${{ steps.pr.outputs.pr_number }}
steps:
Expand All @@ -186,8 +188,10 @@ jobs:
PR_DATA=$(gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}")
HEAD_SHA=$(echo "$PR_DATA" | jq -r '.head.sha')
HEAD_REPO=$(echo "$PR_DATA" | jq -r '.head.repo.full_name')
BASE_SHA=$(echo "$PR_DATA" | jq -r '.base.sha')
echo "head_sha=${HEAD_SHA}" >> $GITHUB_OUTPUT
echo "head_repo=${HEAD_REPO}" >> $GITHUB_OUTPUT
echo "base_sha=${BASE_SHA}" >> $GITHUB_OUTPUT
echo "pr_number=${PR_NUMBER}" >> $GITHUB_OUTPUT

- name: Add reaction to comment
Expand Down Expand Up @@ -548,6 +552,173 @@ jobs:
echo "token=${TOKENS[$IDX]}" >> $GITHUB_OUTPUT

# ── Run LLM evaluation (Vally) ───────────────────────────────
# ── Baseline (pre-PR) evaluation ─────────────────────────────
# Run the SAME specs against the skill's instruction files as they exist
# on the PR base, so the results comment can show before → after and prove
# a reviewer change actually moved the needle. Only reviewer instructions
# (SKILL.md / *.md, NOT tests/) are reverted to base; the PR's eval specs
# and frozen fixtures are kept. Informational only — it never gates (the
# gate uses the authoritative "after" run below). Skipped when there is no
# base (manual dispatch), the skill is new on this branch, or its
# instructions are unchanged versus base, or non-Markdown reviewer assets
# changed (either case cannot produce a useful comparison).
- name: Run Vally evaluation (baseline)
id: eval-baseline
continue-on-error: true
env:
COPILOT_GITHUB_TOKEN: ${{ steps.select-token.outputs.token }}
RESULTS_PATH: eval-results-baseline/${{ matrix.entry.name }}
TESTS_PATH: ${{ matrix.entry.tests_path }}
RUNS: ${{ github.event.inputs.runs }}
BASE_SHA: ${{ needs.pr-gate.outputs.base_sha || needs.slash-gate.outputs.base_sha }}
run: |
set -e
if [ -z "${BASE_SHA:-}" ]; then
echo "No base SHA (manual dispatch?) — skipping baseline run."
echo "baseline_ran=false" >> "$GITHUB_OUTPUT"
exit 0
fi
SKILL_DIR="${TESTS_PATH%/tests}"

# Ensure the base commit is available locally (fork PRs may need it
# fetched from the base repo, added as `upstream` by the fixture step).
if ! git cat-file -e "${BASE_SHA}^{commit}" 2>/dev/null; then
git fetch --no-tags --depth=1 upstream "$BASE_SHA" 2>/dev/null || true
fi
if ! git cat-file -e "${BASE_SHA}^{commit}" 2>/dev/null; then
echo "::warning::Base commit ${BASE_SHA} unavailable — skipping baseline run."
echo "baseline_ran=false" >> "$GITHUB_OUTPUT"
exit 0
fi

# Skill is new on this branch ⇒ no pre-change reviewer to compare to.
if ! git cat-file -e "${BASE_SHA}:${SKILL_DIR}/SKILL.md" 2>/dev/null; then
echo "No base SKILL.md under ${SKILL_DIR} — new skill; skipping baseline."
echo "baseline_ran=false" >> "$GITHUB_OUTPUT"
exit 0
fi

# Compare the union of base and PR Markdown instructions. This catches
# deleted/renamed files while keeping the PR's eval specs and scripts.
mapfile -t INSTR < <(
{
git ls-tree -r --name-only "$BASE_SHA" -- "$SKILL_DIR"
git ls-tree -r --name-only HEAD -- "$SKILL_DIR"
} | awk -v prefix="${SKILL_DIR}/" '
index($0, prefix) == 1 &&
index($0, prefix "tests/") != 1 &&
/\.md$/ { print }
' | sort -u
)
if [ ${#INSTR[@]} -eq 0 ]; then
echo "No instruction files under ${SKILL_DIR} — skipping baseline."
echo "baseline_ran=false" >> "$GITHUB_OUTPUT"
exit 0
fi

# If reviewer instructions are identical to base, before == after.
if git diff --quiet "$BASE_SHA" HEAD -- "${INSTR[@]}"; then
echo "Skill instructions unchanged vs base — before == after; skipping baseline."
echo "baseline_ran=false" >> "$GITHUB_OUTPUT"
exit 0
fi

# An invoked script or other non-Markdown asset changed with the
# instructions would make this a hybrid base/PR reviewer. Skip instead.
mapfile -t NON_MARKDOWN_ASSETS < <(
{
git ls-tree -r --name-only "$BASE_SHA" -- "$SKILL_DIR"
git ls-tree -r --name-only HEAD -- "$SKILL_DIR"
} | awk -v prefix="${SKILL_DIR}/" '
index($0, prefix) == 1 &&
index($0, prefix "tests/") != 1 &&
!/\.md$/ { print }
' | sort -u
)
if [ ${#NON_MARKDOWN_ASSETS[@]} -gt 0 ] &&
! git diff --quiet "$BASE_SHA" HEAD -- "${NON_MARKDOWN_ASSETS[@]}"; then
echo "Non-Markdown reviewer assets changed — skipping partial baseline."
echo "baseline_ran=false" >> "$GITHUB_OUTPUT"
exit 0
fi

echo "Reverting ${#INSTR[@]} instruction file(s) under ${SKILL_DIR} to base ${BASE_SHA} for the baseline run"
# Always restore PR content when this step exits, even on error, so the
# authoritative "after" run below never sees base content.
restore() { git restore --staged --worktree --source=HEAD -- "$SKILL_DIR"; }
trap restore EXIT
for f in "${INSTR[@]}"; do
if git cat-file -e "${BASE_SHA}:${f}" 2>/dev/null; then
git checkout "$BASE_SHA" -- "$f"
else
# File was added by the PR; it did not exist pre-change.
rm -f "$f"
fi
done

SPECS=()
for f in "$TESTS_PATH"/eval*.vally.yaml; do
[ -e "$f" ] || continue
SPECS+=("-e" "$f")
done
if [ ${#SPECS[@]} -eq 0 ]; then
echo "No eval*.vally.yaml specs — skipping baseline."
echo "baseline_ran=false" >> "$GITHUB_OUTPUT"
exit 0
fi

RUNS_ARGS=()
if [ -n "${RUNS:-}" ]; then
RUNS_N=$(printf '%s' "$RUNS" | tr -cd '0-9')
[ -n "$RUNS_N" ] && RUNS_ARGS=(--runs "$RUNS_N")
fi

# Advisory exit — the baseline number is informational; do not fail.
set +e
npx -y "@microsoft/vally-cli@${VALLY_VERSION}" eval \
"${SPECS[@]}" \
--skill-dir .github/skills \
--output-dir "$RESULTS_PATH" \
--junit \
--model claude-opus-4.6 \
--judge-model claude-opus-4.6 \
"${RUNS_ARGS[@]}" \
--workers 4 \
--verbose
BASELINE_RC=$?
set -e
echo "baseline vally exit: $BASELINE_RC (advisory)"

# A threshold miss is a valid before score. Execution errors or
# incomplete output are not comparable and must not produce a trend.
JUNIT=$(find "$RESULTS_PATH" -name 'eval-results.junit.xml' -type f 2>/dev/null | head -1 || true)
if [ -z "$JUNIT" ]; then
echo "::warning::No baseline JUnit report under $RESULTS_PATH — skipping baseline."
echo "baseline_ran=false" >> "$GITHUB_OUTPUT"
else
ROOT=$(grep -m1 '<testsuites' "$JUNIT" || true)
if [ -z "$ROOT" ]; then
echo "::warning::Baseline JUnit report contains no <testsuites> element — skipping baseline."
echo "baseline_ran=false" >> "$GITHUB_OUTPUT"
else
FAILS=$(printf '%s' "$ROOT" | sed -nE 's/.*failures="([0-9]+)".*/\1/p'); FAILS=${FAILS:-0}
ERRS=$(printf '%s' "$ROOT" | sed -nE 's/.*errors="([0-9]+)".*/\1/p'); ERRS=${ERRS:-0}
echo "Baseline JUnit aggregate: failures=$FAILS errors=$ERRS"
if [ "$ERRS" -ne 0 ] || { [ "$BASELINE_RC" -ne 0 ] && [ "$FAILS" -eq 0 ]; }; then
echo "::warning::Baseline evaluation had execution errors or incomplete output — skipping baseline."
echo "baseline_ran=false" >> "$GITHUB_OUTPUT"
else
echo "baseline_ran=true" >> "$GITHUB_OUTPUT"
fi
fi
fi

# Belt-and-suspenders: guarantee the PR's skill content is restored before
# the authoritative run, regardless of how the baseline step exited.
- name: Restore PR skill content
if: always()
run: git restore --staged --worktree --source=HEAD -- .github/skills

- name: Run Vally evaluation
id: eval-run
env:
Expand Down Expand Up @@ -651,6 +822,15 @@ jobs:
include-hidden-files: true
retention-days: 14

- name: Upload baseline results
if: always() && steps.eval-baseline.outputs.baseline_ran == 'true'
uses: actions/upload-artifact@v4
with:
name: skill-eval-baseline-${{ matrix.entry.name }}
path: eval-results-baseline/${{ matrix.entry.name }}/
include-hidden-files: true
retention-days: 14

# ==========================================================================
# HERMETICITY GATE (positive assertion)
# Runs hermeticity.vally.yaml — a single stimulus that passes only when
Expand Down Expand Up @@ -834,6 +1014,15 @@ jobs:
merge-multiple: false
continue-on-error: true

- name: Download baseline eval result artifacts
if: needs.evaluate.result == 'success' || needs.evaluate.result == 'failure'
uses: actions/download-artifact@v4
with:
pattern: skill-eval-baseline-*
path: eval-results-baseline/
merge-multiple: false
continue-on-error: true

- name: Download hermeticity results
if: always()
uses: actions/download-artifact@v4
Expand Down Expand Up @@ -1000,13 +1189,23 @@ jobs:
const m = (tag || '').match(new RegExp(key + '="([^"]*)"'));
return m ? xmlDecode(m[1]) : null;
}
function artifactScope(file, root, artifactPrefix) {
const [artifact] = path.relative(root, file).split(path.sep);
return artifact && artifact.startsWith(artifactPrefix)
? artifact.slice(artifactPrefix.length)
: '';
}
function suiteKey(scope, label) {
return `${scope}\u0000${label}`;
}

const suites = [];
let evalPassed = true;
let hasResults = false;
if (fs.existsSync('eval-results')) {
const junitFiles = findFilesByName('eval-results', 'eval-results.junit.xml');
for (const jf of junitFiles) {
const scope = artifactScope(jf, 'eval-results', 'skill-eval-results-');
let xml = '';
try { xml = fs.readFileSync(jf, 'utf8'); } catch { continue; }
const blocks = xml.match(/<testsuite\b[\s\S]*?<\/testsuite>/g) || [];
Expand Down Expand Up @@ -1034,7 +1233,7 @@ jobs:
if (!failures.has(tcName)) failures.set(tcName, { kind, msg });
}
}
suites.push({ label, score, threshold, passed, failures: [...failures.entries()] });
suites.push({ scope, label, score, threshold, passed, failures: [...failures.entries()] });
}
}
}
Expand All @@ -1061,16 +1260,59 @@ jobs:
lines.push('');

// ── Per-suite results table ─────────────────────────
lines.push('| Suite | Score | Threshold | Verdict |');
lines.push('|-------|-------|-----------|---------|');
// Baseline scores are keyed by skill artifact and eval name, so
// equal suite names in different skills remain distinct.
const baselineScores = new Map();
if (fs.existsSync('eval-results-baseline')) {
const bFiles = findFilesByName('eval-results-baseline', 'eval-results.junit.xml');
for (const jf of bFiles) {
const bScope = artifactScope(jf, 'eval-results-baseline', 'skill-eval-baseline-');
let bxml = '';
try { bxml = fs.readFileSync(jf, 'utf8'); } catch { continue; }
const bblocks = bxml.match(/<testsuite\b[\s\S]*?<\/testsuite>/g) || [];
for (const block of bblocks) {
const bOpen = (block.match(/<testsuite\b[^>]*>/) || [''])[0];
const bLabel = suiteProp(block, 'evalName') || tagAttr(bOpen, 'name') || '(unnamed)';
baselineScores.set(suiteKey(bScope, bLabel), {
score: suiteProp(block, 'overallScore'),
passed: suiteProp(block, 'passed') === 'true',
});
}
}
}
const hasBaseline = baselineScores.size > 0;

if (hasBaseline) {
lines.push('| Suite | Before | After | Threshold | Verdict |');
lines.push('|-------|--------|-------|-----------|---------|');
} else {
lines.push('| Suite | Score | Threshold | Verdict |');
lines.push('|-------|-------|-----------|---------|');
}
for (const s of suites) {
const sc = s.score != null && s.score !== '' ? Number(s.score).toFixed(2) : '—';
const th = s.threshold != null && s.threshold !== '' ? Number(s.threshold).toFixed(2) : '—';
const v = s.passed ? '✅' : '❌';
const label = (s.label || '').replace(/\|/g, '\\|');
lines.push(`| ${label} | ${sc} | ${th} | ${v} |`);
if (hasBaseline) {
const b = baselineScores.get(suiteKey(s.scope, s.label));
const bsc = b && b.score != null && b.score !== '' ? Number(b.score).toFixed(2) : '—';
let trend = '';
if (b && b.score != null && b.score !== '' && s.score != null && s.score !== '') {
const d = Number(s.score) - Number(b.score);
if (d > 0.001) trend = ' 📈';
else if (d < -0.001) trend = ' 📉';
}
lines.push(`| ${label} | ${bsc} | ${sc}${trend} | ${th} | ${v} |`);
} else {
lines.push(`| ${label} | ${sc} | ${th} | ${v} |`);
}
}
lines.push('');
if (hasBaseline) {
lines.push('_**Before** = these specs run against the skill on the PR base (the pre-change reviewer); **After** = with this PR. A rise (📈) means the change made the reviewer catch a regression it previously missed. The Before run is informational and never gates._');
lines.push('');
}
Comment on lines +1312 to +1315

// ── Failing stimuli detail ──────────────────────────
for (const s of suites.filter(x => x.failures.length > 0)) {
Expand Down
Loading