ci(skills): migrate skill-eval suite from skill-validator to Vally

[PureWeen]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Summary

Migrates the skill-validation.yml workflow from the legacy dotnet/skills binary (skill-validator check + skill-validator run) to @microsoft/vally-cli@0.6.0. All 5 skill-eval suites are ported and validated green in CI.

What Changed

Workflow (skill-validation.yml)

• Replaced skill-validator install/check/run steps with vally lint --strict and vally run --eval-spec
• Added hermeticity negative-control gate that verifies the sandbox doesn't have unauthorized network access (rate-limit auth detection)
• Added workflow_dispatch manual trigger with skills and runs inputs for on-demand evaluation
• Changed fixture fetch depth from --depth=1 to --depth=2 so git diff HEAD^ HEAD works in worktree environments
• Removed all skill-validator references and legacy eval.yaml files

Eval Specs Ported (all in .github/skills/<skill>/tests/)

Skill	File	Stimuli	Status
code-review	eval.capability.vally.yaml	9 capability scenarios	✅ 9/9
code-review	eval.vally.yaml	2 regression scenarios	✅ 2/2
code-review	hermeticity.vally.yaml	1 negative control	✅
agentic-labeler	eval.vally.yaml	3 labeling scenarios	✅
try-fix	eval.vally.yaml	2 fix scenarios	✅
verify-tests-fail-without-fix	eval.vally.yaml	2 verification scenarios	✅
evaluate-pr-tests	eval.vally.yaml	2 evaluation scenarios	✅

Key Design Decisions

• Structural graders (output-matches, output-not-contains) verify hard behavioral requirements (e.g., never approve via API, verdict markers present)
• LLM prompt graders (scale_1_5) assess quality and depth with calibrated thresholds
• Regression stimuli use frozen git worktrees pinned to known-regressing commits — no network calls, fully reproducible
• Capability stimuli target real merged PRs to test against actual code review scenarios

Validation

Final CI run #27635665319: 11/11 stimuli pass, 0 failures.

Iteratively validated across 9 CI runs, fixing:

• Fixture fetch depth (parent commit needed for git diff)
• Structural floor regex (added 🔴 marker, broadened to accept Verdict/Finding)
• Merged-PR prompt engineering (agents short-circuit reviews on merged PRs)
• Environment constraints (Vally sandbox lacks GH_TOKEN — adjusted rubrics accordingly)
• LLM judge threshold calibration (structural graders verify correctness; LLM judges assess quality)

Removed

• All eval.yaml files (legacy skill-validator format)
• skill-validator binary installation steps
• --allow-repo-traversal flag usage (Vally worktrees provide full repo access natively)

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35942

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35942"

[github-actions]

Skill Validation Results

@PureWeen — new skill validation results are available based on this last commit: 32bbbba.
To request a fresh validation after new comments or commits, comment /evaluate-skills.

✅ Skill Validation Results — 32bbbba · ci(skills): migrate skill-eval suite from skill-validator to Vally · 2026-06-16T20:46:29Z

✅ Static Checks Passed

Skills checked: 19 | Agents checked: 4

Full validator output

Found 5 skill(s)
[agentic-labeler] 📊 agentic-labeler: 2,839 BPE tokens [chars/4: 2,788] (standard ~), 8 sections, 0 code blocks
[agentic-labeler]    ⚠  Skill is 2,839 BPE tokens (chars/4 estimate: 2,788) — approaching "comprehensive" range where gains diminish.
[agentic-labeler]    ⚠  No code blocks — agents perform better with concrete snippets and commands.
[code-review] 📊 code-review: 5,074 BPE tokens [chars/4: 5,262] (comprehensive ✗), 38 sections, 9 code blocks
[code-review]    ⚠  Skill is 5,074 BPE tokens (chars/4 estimate: 5,262) — "comprehensive" skills hurt performance by 2.9pp on average. Consider splitting into 2–3 focused skills.
[evaluate-pr-tests] 📊 evaluate-pr-tests: 2,955 BPE tokens [chars/4: 2,949] (standard ~), 35 sections, 6 code blocks
[evaluate-pr-tests]    ⚠  Skill is 2,955 BPE tokens (chars/4 estimate: 2,949) — approaching "comprehensive" range where gains diminish.
[try-fix] 📊 try-fix: 6,916 BPE tokens [chars/4: 7,049] (comprehensive ✗), 45 sections, 17 code blocks
[try-fix]    ⚠  Skill is 6,916 BPE tokens (chars/4 estimate: 7,049) — "comprehensive" skills hurt performance by 2.9pp on average. Consider splitting into 2–3 focused skills.
[verify-tests-fail-without-fix] 📊 verify-tests-fail-without-fix: 2,271 BPE tokens [chars/4: 2,189] (detailed ✓), 26 sections, 7 code blocks
✅ All checks passed (5 skill(s))
Found 4 agent(s)
Validated 4 agent(s)
✅ All checks passed (4 agent(s))

⏭️ LLM Evaluation: Skipped

No changed skills with eval tests found.

🔍 Full results and investigation steps

[kubaflo]

🤖 Multi-model code review — Vally migration

Three models reviewed this independently (Claude Opus 4.8, GPT-5.5, Gemini 3.1 Pro), then cross-pollinated. I also pulled and read the published @microsoft/vally(-cli)@0.6.0 source to verify the tool assumptions directly.

Verdict: NEEDS_DISCUSSION — the migration is correct and well-engineered; the findings all sharpen the eval's discriminating power rather than break anything. The eval matrix is advisory (warn-only), so none of these gate merge.

Verified correct ✅ (two models + I checked the actual vally 0.6.0 source)

The schema-mismatch risk is not real — I confirmed against the published package:

• Every CLI flag (-e/--eval-spec, --skill-dir, --model, --judge-model, --output-dir, --output jsonl, --junit, --runs, --workers; lint --eval-spec --strict) exists.
• The JUnit contract the comment-job parses is exactly what vally emits: root <testsuites failures= errors=>, per-suite <property name="evalName|overallScore|threshold|passed">, <failure>/<error message=>; filenames eval-results.junit.xml / eval-results.md under the timestamped run dir; the missing-summary path is fail-closed.
• Spec vocabulary (type: capability, stimuli, environment.git.ref, grader types) is the real format; vally lint --strict is the schema gate.

Migration fidelity is good — the ports are larger than the originals (e.g. all 9 code-review scenarios preserved + made hermetic), not lossy. The hermeticity intent (model-auth-only env, no GITHUB_TOKEN/GH_TOKEN) is a genuine improvement.

Findings (see inline)

1. ⚠️ --runs 3 overrides the spec's defaults.runs: 5 (skill-validation.yml:578→593). The workflow always passes --runs, and vally's --runs overrides defaults.runs, so the regression suite's deliberate runs: 5 (high-variance, with a comment) is silently forced to 3 — below vally's own significance floor. One-line fix: only pass --runs when inputs.runs is set. (Opus-found; confirmed.)
2. ⚠️ Judge inert for multi-floor stimuli at threshold: 0.6 (agentic-labeler/.../eval.vally.yaml:730). vally scores a trial as the unweighted mean of grader scores (weights ignored), so a stimulus with 3 passing floors + 1 judge sits at ≥0.75 no matter the judge — blind to extra/out-of-scope labels and negated mentions that still satisfy output-contains. The scoring comment's "fails both floor and judge" only covers missing/wrong required labels. Fix: ≤1 floor per stimulus, or raise the threshold above n_floors/(n_floors+1). (All three models; I traced it in vally/pipeline/grading.js + scoring/scorer.js. Note: the code-review regression specs use 1 floor + judge and are fine — this bites agentic-labeler.)
3. ⚠️ Hermeticity negative control can't distinguish "hermetic" from "probe failed" (skill-validation.yml:733). Any <failure> reads ✅ Hermetic, so a network block / flake / hallucination passes the gate; it would essentially never fail if promoted to blocking. Make it a positive assertion of the anonymous CORE_LIMIT: 60. (Opus + Gemini + GPT.)
4. 💡 --output jsonl drops results.jsonl from the artifact (skill-validation.yml:590, prompt at 1069). With --output jsonl, vally streams JSONL to stdout — the file the investigate prompt references is never uploaded. Drop the flag or fix the prompt. (GPT-found; confirmed in source.)

Also worth a look (non-blocking)

• fetch-depth: 0 (line 470) on dotnet/maui is a full-history clone, but the very next step already git fetch --depth=2es each fixture SHA — the deep clone looks redundant. (Opus + Gemini.)
• Cross-PR conflict: #34884 and #35925 add scenarios to the old code-review/tests/eval.yaml this PR deletes; whichever merges second must re-port into .vally.yaml. (Opus + Gemini.)
• Reporting: a matrix leg that errors before writing JUnit is silently dropped from the comment's aggregate (other legs can still show ✅). Since eval is advisory this is cosmetic, but a per-leg verdict artifact would make it honest. (GPT.)

_{Independent verdicts: Opus 4.8 — NEEDS_DISCUSSION (high) · GPT-5.5 — NEEDS_CHANGES · Gemini 3.1 Pro — NEEDS_CHANGES. All three (and a direct source check) agree the migration/tool-wiring is correct; the asks are eval-quality, not safety.}

[kubaflo]

🤖 Multi-model re-review — round 2 (head aee98bb)

Fast turnaround 👍 — re-reviewed your 3 follow-up commits.

Addressed since round 1 ✅

• JUnit robustness (the round-1 "matrix leg can false-green" note): the new <testsuites>-missing guard and the EVAL_RC≠0 but 0 failures → treat as failure guard (in both evaluate and the hermeticity gate) close the swallowed-partial-output path. 👍
• Floor precision: 'LGTM' → 'Verdict: LGTM' stops the capability floor false-failing on prose like "not LGTM material".
• Dead .github/agents/** outputs/trigger removed — good cleanup.

Still open (carried from round 1)

1. ⚠️ --runs 3 overrides defaults.runs: 5 — skill-validation.yml:570→585. The one clean one-liner: only pass --runs when inputs.runs is set, else the regression suite silently drops from 5 trials to 3. (inline)
2. ⚠️ Judge inert for ≥2-floor stimuli at threshold: 0.6 — agentic-labeler/.../eval.vally.yaml:730. The floor-precision fixes help, but the unweighted-mean math still pins 3-floor stimuli ≥0.75 regardless of the judge. ≤1 floor, or raise the threshold above n_floors/(n_floors+1). (inline)
3. ⚠️ Hermeticity inversion still can't tell "hermetic" from "probe failed" — skill-validation.yml:742. The new guard handles a missing <testsuites>, but a content <failure> from a flake/network-block/hallucination still reads ✅ Hermetic. Make it a positive assertion of the anonymous CORE_LIMIT: 60.
4. 💡 --output jsonl drops results.jsonl — skill-validation.yml:582; the investigate prompt (line ~1061) still references a file that isn't uploaded.

Tool-wiring remains verified-correct against the published @microsoft/vally@0.6.0 source. None of these gate merge (eval is advisory) — they sharpen the suite.

@PureWeen — #1 and #3 are the highest-value remaining; the rest are minor. Thanks for the quick iterations! 🙏

_{3-model panel: Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro.}

[PureWeen]

🤖 Multi-model Adversarial Review — Round 4

Methodology: 3 independent reviewers (different model families) with adversarial consensus.

Findings Actioned

Three verified issues in the hermeticity gate, all fixed in 32bbbba:

Finding	Consensus	Severity	Status
Vally exit code not captured (echo "$?" consumed it) — no exit-code guard possible	Verified independently (1/3 Gemini flagged underlying symptom)	⚠️	✅ Fixed
find aborts under set -e + pipefail if output dir missing — fails non-blocking job	1/3 (Gemini) — verified valid	⚠️	✅ Fixed
--output jsonl still in hermeticity gate — inconsistent with evaluate job fix	Verified independently	💡	✅ Fixed

Findings Discarded

Finding	Source	Reason
COPILOT_GITHUB_TOKEN exposed to PR-controlled specs	1/3 (GPT)	Pre-existing workflow design, not introduced by this PR. Token is model-auth only (not a GitHub API token); gh CLI doesn't read it.
JUnit <failure> regex misses text-content messages	1/3 (Gemini)	Doesn't affect pass/fail verdicts — only cosmetic in PR comment details
JUnit <testsuites> multi-line formatting	1/3 (Gemini)	Theoretical — standard JUnit format puts attributes on same line. The exit-code guard now catches this class of issue anyway.

Verdict: No blocking issues remain. All actionable findings fixed and pushed.

[PureWeen]

🤖 Multi-model Adversarial Review — Round 5 (Final)

Methodology: 3 independent reviewers (Opus 4.8 / GPT-5.5 / Gemini 3.1 Pro) with adversarial consensus.

Result: ✅ No actionable findings

All 6 findings were 1/3 consensus (single reviewer only). Each was independently verified and discarded:

Finding	Source	Reason for Discard
.npmrc in PR checkout could redirect npx	GPT (❌)	Pre-existing workflow pattern, not introduced by this PR. COPILOT_GITHUB_TOKEN is model-auth only.
Capability tests hit anonymous rate limit	Gemini (❌)	By design — spec comments (lines 12-21) document why these are intentionally live. Empirically passing across 9+ CI runs.
JUnit multiline XML parsing	Gemini (⚠️)	Repeat from R4, already mitigated by exit-code guard added in R4.
Comment shows failing trials under passed suite	Opus (💡)	Display polish — doesn't affect gating.
Dead eval_passed output diverges from suite gating	Opus (💡)	Output is unused by downstream jobs; cosmetic only.
Low-floor stimuli judge-fragile at 0.85 threshold	Opus (💡)	Documented tradeoff (kubaflo review). Warn-only gate, won't block PRs.

Verdict: Clean. No blocking issues, no warnings, no fixes needed. PR is ready for merge.

5 rounds of adversarial review complete. All actionable findings from rounds 1-4 have been fixed and pushed.

[kubaflo]

Multi-model code review — PR #35942 Round 3

3-model consensus: LGTM

What changed since Round 2

Shane addressed ALL 4 round-2 findings:

1. ✅ --runs override fixed — Now conditional RUNS_ARGS (only set when inputs.runs provided) → spec defaults.runs: 5 honored
2. ✅ Judge dilution fixed — scoring.threshold: 0.6 → 0.85 (judge retains veto power: max 4 floors min 0.80 < 0.85)
3. ✅ Hermeticity inversion fixed — Now POSITIVE control asserting CORE_LIMIT: 60 with threshold: 1.0 and FAILS=0 && ERRS=0 = hermetic, plus exit-code capture
4. ✅ --output jsonl removed — File reference removed from investigate prompt

Both fixes verified bug-free at Vally source level.

Findings

No new issues. Non-blocking suggestion for fetch-depth: 0 remains acceptable (already discussed).

Verdict

LGTM (unanimous)

Confidence: High

Reasoning: All actionable findings from rounds 1-2 have been addressed. The skill-validation integration is correct and the eval suite is properly configured.

---

Review conducted by: @kubaflo's autonomous multi-model review loop
Models: Claude Opus 4.8, GPT-5.5, Gemini 3.1 Pro
Timestamp: 2026-06-16T21:10Z