Repository navigation
fix(evals): require rpi-agent baseline run when its named skills change - #3233
Jamie Kim (jkim323) merged 4 commits into
Conversation
- check changed paths against the skill dirs the equivalence harness copies - share one surface resolver between the harness and the change gate - document when the plan requires the baseline equivalence run Fixes microsoft#3080 🧪 - Generated with Claude Code Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3233 +/- ##
==========================================
+ Coverage 86.61% 86.70% +0.09%
==========================================
Files 87 103 +16
Lines 11554 11862 +308
Branches 0 63 +63
==========================================
+ Hits 10007 10285 +278
- Misses 1547 1566 +19
- Partials 0 11 +11
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Bill Berry (WilliamBerryiii)
left a comment
There was a problem hiding this comment.
Thank you for fixing the primary RPI skill-change gap and sharing one current-surface resolver between the gate and materializer. I left four inline comments covering destructive path states, subject resolution, PowerShell naming, and Pester isolation. Please also resolve or explicitly accept the two remaining Required Local Checks: the local validation aggregate and link validation, whose baseline-parity exceptions are documented in the PR.
…rface - require each subject to match exactly one agent file - check deleted and renamed-away skill paths against the comparison base - use PascalCase locals and a TestDrive fixture 🧪 - Generated with Claude Code Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks Bill Berry (@WilliamBerryiii)! All four are addressed in 56c447e, with details on each thread. On the local checks: with actionlint and the uv envs set up, |
Pull Request
Description
A PR that only edits one of the RPI skills (
rpi-research,rpi-plan,rpi-implement,rpi-review) skips therpi-agentbaseline-equivalence run, even though the equivalence harness copies those skill directories into the surface it measures.Root cause, as described in the issue:
Get-AgentDependencyMap.ps1records only skills referenced by path.rpi-agent.agent.mdnames its skills in backticks, so the map has no skills for it, andGet-ChangedAIArtifact.ps1emitsaffectedAgents: []. The harness resolves the backtick form throughGet-AgentSkillReference, so the gate and the harness disagree.Three places decide on equivalence from
affectedAgents, and all of them saw the empty list:.github/workflows/eval-validation.yml:339sets theequivalence-requiredoutput.scripts/evals/New-AgentEvalPlan.ps1:291setsbaseline.requiredand the baseline producers in the plan.scripts/evals/Invoke-VallyEvals.ps1:1081onward promotes affected agents into the equivalence dispatch set.Fix
The fix goes into the manifest itself, so all three consumers stay consistent:
EquivalenceEnvironment.psm1gets a newGet-AgentSurfaceSkillfunction, extracted fromNew-CustomizedEnvironment. It returns the skill directories the harness materializes for an agent: the skills the agent references plus, transitively, those of its declared subagents.New-CustomizedEnvironmentnow calls it, so its behavior is unchanged (same set, same sort order).Get-ChangedAIArtifact.ps1checks every changed path, including the previous path of renames, against those directories for each equivalence subject. A subject with a changed path inside its surface is added toaffectedAgents.SKILL.md, because the harness copies the whole directory. That is also why the check lives inGet-ChangedAIArtifact.ps1: the manifest'sartifactslist records onlySKILL.mdfor skills, while this script sees every changed path.<slug>.agent.mdunder.github/agents. A missing or duplicate subject throws, so manifest generation fails closed: the entry script exits 2 and writes no manifest.comparisonBase, because a deleted or renamed-away skill no longer exists at HEAD. The base surface is resolved from that commit's.github/agentsand.github/skills, exported withgit archive. It is only exported when such a path falls under.github/skills. Eval Validation checks out withfetch-depth: 0, so the comparison base is available there.Scope
The issue notes that applying the harness resolver to the whole dependency map is broad (181 skill references across 51 of 64 agents). This change applies it only to the equivalence subjects:
-EquivalenceSubjectparameter that defaults torpi-agent. That matchesInvoke-VallyEvals.ps1 -EquivalenceSubject(line 138) and theValidateSet('rpi-agent')onInvoke-BaselineEquivalence.ps1 -Agent(line 105).Get-AgentDependencyMap.ps1andGet-AffectedAgentSlugsare unchanged.Documentation
docs/contributing/evals-ci.mdsaid that baseline equivalence runs when the plan marksbaseline.required: true, but not when that happens. It now explains both routes intoaffectedAgents.Related Issue(s)
Fixes #3080
Type of Change
Select all that apply:
Code & Documentation:
Infrastructure & Configuration:
Other:
.ps1,.sh,.py)Testing
Issue reproduction, before and after
I followed the issue's steps: a change set containing only
.github/skills/rpi/rpi-research/SKILL.md, passed throughGet-ChangedAIArtifact.ps1and thenNew-AgentEvalPlan.ps1.affectedAgentsbaselinein the planmain(d085729)[]required: false,reason: agent-not-affected:rpi-agent[rpi-agent]required: true,reason: affected-agent:rpi-agent, producersbaseline:gpt-6-lunaandbaseline:claude-sonnet-5With single-file change sets against this branch:
rpi-agentaffected:SKILL.mdof each ofrpi-research,rpi-plan,rpi-implementandrpi-review;rpi-research/references/research.md.data-science-engineering/evaluation-design/SKILL.md;README.md.Pester
There are 14 new tests.
EquivalenceEnvironment.Tests.ps1,Get-AgentSurfaceSkill:New-CustomizedEnvironmentactually materializes.rpi-agentsurface contains the four RPI skills.Get-ChangedAIArtifact.Tests.ps1,Equivalence surface gate, using a fixture whoserpi-agentnames its skill in backticks:SKILL.mdchange marksrpi-agentaffected.rpi-agentaffected.-EquivalenceSubjectdisables the check.Get-ChangedAIArtifact.Tests.ps1,Equivalence surface gate at the comparison base, using real two-commit git fixtures andNew-EvalChangeSet:rpi-agentaffected.rpi-agentaffected.Fixtures that contain no
rpi-agentnow pass-EquivalenceSubject @()explicitly. Those are two existing scan calls inGet-ChangedAIArtifact.Tests.ps1and one inChangedSpecStimulus.Tests.ps1. The entry-script fixture gains anrpi-agentin its base commit.I also ran two mutation checks:
affectedAgentsdisabled, the two positive gate tests fail and the two negative ones still pass.The full
scripts/tests/evals/suite on this branch: 1601 passed, 7 failed. The 7 failures are the Rust unit-test network-trace tests, which needdocker, and they fail the same way on unmodifiedmain(1587 passed, 7 failed) in the same environment.Repository checks
I ran each
npm run validate:localstep individually, plusnpm run spell-check, with the repository-pinnedactionlint1.7.10 installed and every Python project synced withuv sync --locked --only-dev, which provides the lockedruff. 30 of 31 pass, includinglint:yaml,lint:py,lint:psandspell-check.The remaining failure is
lint:md-linksin full-repository mode. It reports 5 external links in 4 files this PR does not touch, and the same links fail on unmodifiedmain:eur-lex.europa.eu: 202, three times.medium.com: 403.owasp.org/www-project-ai-security-and-privacy-guide/: 404, which looks genuinely dead.Markdown-Link-Check.ps1 -ChangedFilesOnly, the modemarkdown-link-check.ymluses in CI, completes successfully. That mode validates internal links repository-wide and external links in the changed files.npm run validate:docspasses:docs:lint,docs:lint:labels,docs:typecheckanddocs:test.Checklist
Required Checks
Required Local Checks
npm run validate:local. 30 of 31 steps pass; the remaining one is the full-repository external link check below.npm run validate:docsnpm run spell-checknpm run lint:md-links. The CI-equivalent-ChangedFilesOnlyrun passes. The full-repository run fails only on 5 external links in 4 untouched files, which also fail onmain(see Testing); I'd ask that these be accepted for this PR.Security Considerations
Additional Notes
rpi-agent.agent.md, which already marks the agent affected.🤖 Generated with Claude Code