Add vally framework and graders for azure-project-plan - #1683
Add vally framework and graders for azure-project-plan#1683Megan Mott (motm32) wants to merge 9 commits into
Conversation
| on: | ||
| # Run per-agent evals on PRs that touch agent instructions | ||
| pull_request: | ||
| branches: [main] |
There was a problem hiding this comment.
we should make this run on the feat/CoR branch too
| continue-on-error: true | ||
|
|
||
| - name: Compare (fail on regression) | ||
| if: steps.download-baseline.outcome == 'success' |
There was a problem hiding this comment.
steps.download-baseline doesn't resolve to anything — the download step above has continue-on-error: true but no id:, and there's no id: anywhere in this file. The expression is always empty, so vally compare --fail-on-regression never executes and the regression gate is off.
Same theme elsewhere in the PR: eval:cor:graders:certify and eval:cor:thresholds:validate are defined as root scripts (package.json:1027-1028) but no job calls them, so grader certification and threshold validation don't run either.
id: download-baseline on the step above fixes this one; the other two want a step in the contracts job.
| } | ||
| const exitCode = grader?.metadata?.exit_code; | ||
| const stderr = grader?.metadata?.stderr ?? ''; | ||
| if (exitCode !== undefined && exitCode !== 0 && exitCode !== 1) { |
There was a problem hiding this comment.
Exit code 1 is excluded here, but that's what Node returns on an uncaught exception — and it's also what the graders deliberately return when the product fails (validate-requirements.mjs:166). So a TypeError inside a grader is scored as a product defect.
The stderr check below catches SyntaxError, which is a parse-time failure, but not TypeError or ReferenceError — the ones you'd actually hit at runtime.
A top-level try/catch in each grader exiting a reserved code (3, say) and classifying that as harness here is probably the cheapest fix.
| fs.writeFileSync(reportPath, report.join('\n')); | ||
|
|
||
| // Machine-readable counterpart so CI can assert on the same diagnosis. | ||
| const diagnostics = { |
There was a problem hiding this comment.
run-diagnostics.json has no schemaVersion, which will bite once we're aggregating across runs — graderCertification.ts and release-thresholds.v1.json in this same PR both version their artifacts.
The bigger risk is that the reader degrades silently rather than loudly. Line 35 is results.filter(r => r.gradeResult?.passed) and the scores are ?? 0 throughout. If Vally renames or nests gradeResult, every stimulus becomes a silent 0% and the report will state with confidence that the product regressed to nothing.
We hit exactly this on the other branch when gate explanations moved from evidence to reason — 30 results were misclassified before anyone caught it; there's a note about it at evals/src/gateHealth.ts:108-117. Stamping a version here and throwing on a missing gradeResult instead of coercing to false would make the failure loud. Worth noting line 527 reads grader.evidence, which is the older of the two shapes.
| file-not-matches: 0.04 | ||
| transcript-not-contains: 0.03 | ||
| output-not-contains: 0.03 | ||
| threshold: 0.9 |
There was a problem hiding this comment.
Weighting by grader type lets a low-weight grader fail without failing the stimulus. plan-approval-scrapbook is the clearest case — its three graders are file-matches (0.05), tool-calls (0.3) and output-not-contains (0.03), totalling 0.38. If no-inline-scaffolding fails, the score is 0.35/0.38 = 0.921, which clears this threshold. The agent inlines scaffolding, a contract the PR description says we enforce is violated, and the stimulus is green.
photo-app-requirements has the same shape: no-chat-questions failing gives 0.95/0.98 = 0.969 and still passes.
It also gets worse as gates land — at 17 gates a single failure is roughly 6% of total weight and can never cross 0.9. Treating contract graders as required (any failure fails the stimulus) and keeping the weighted score for genuinely graded dimensions would hold up as this grows.
There was a problem hiding this comment.
Copilot is super long winded here, but I think that using a weighted grader for a stimulus is a bad idea, at least for now.
I think for now it should just be pass/fail, and note which step in the process it failed at. E.g. if stimulus A. failed to build (early step) vs. stim B. failed to deploy (late step), then stim B was more successful, but still not a pass.
We can figure out the exact details of how to measure it later on once we have more graders
| const validations: Array<readonly [string, ArtifactValidationResult]> = await Promise.all([ | ||
| Promise.resolve(['requirements', validateRequirementsArtifact(requirements, { requireConfirmed: true })] as const), | ||
| Promise.resolve(['project-plan', validateProjectPlanArtifact(projectPlan, { expectedStatus: 'Integrated' })] as const), | ||
| Promise.resolve(['plan-gate', validatePlanEvaluationContract(true, true, { |
There was a problem hiding this comment.
plan-gate can't fail. Every argument here is a literal, and walking the three checks in planEvaluation.ts against them:
expectedFrontend !== generatedFrontend→true !== true→ never firesexpectedFrontend && !previewManifestPresentAtCall→true && !true→ never firesexpectedFrontend && previewHtmlFilesAtCall?.length→true && 0→ never fires
So it returns valid: true with zero issues regardless of the fixture. It reads nothing from the workspace, which means no mutation to any file could falsify it. It's listed in manifest.json:7-12 with the other three validators, and it's the one with no mutation — consistent with there being no mutation that could work.
The bigger issue is what certification covers at all. These validators are evals/src/artifacts/*.ts, but the suite runs evals/graders/*.mjs, which import only node:fs and node:path — they're independent reimplementations, not wrappers. So the certified code isn't the executed code, and the two are already out of sync:
requirements-schema-versionasserts codeschemaVersion(requirements.ts:33-34);validate-requirements.mjsnever mentionsschemaVersion.project-plan-numberingassertsnonSequentialHeading(projectPlan.ts);validate-project-plan.mjsnever mentions it.
Both things certification proves are things the running grader can't do. Downgrade schemaVersion to "1" in a real run and the suite passes it.
We prefer TS anyway, so I'd collapse this rather than wrap it: drop the .mjs graders and make them .ts. Node 22.18+ runs .ts directly, so eval.yaml keeps program: node and only the filenames change — no tsx, no build step. Worth adding "node": ">=22.18" to engines since there's no floor declared today. Each grader then becomes a thin adapter: parse argv, call the validator in src/artifacts, map returned issues to an exit code. One implementation, and certification finally covers the path that runs.
While you're in there, have the adapter exit 3 when the grader itself throws, so it's distinguishable from 1 (product failed) — that's the split the report can't currently make.
The Vally suite exercises the azure-project-plan agent through the Copilot SDK with an MCP stand-in for the extension's tools, so it can verify the agent's contracts but never the code that ships. This adds a parallel vscbench config that runs the same contracts inside real VS Code with the extension installed from a VSIX. The extension registers an in-process MCP server exposing open_requirements_view and friends, which are the exact tool names the Vally tool-calls graders assert on, so the graders port across unchanged while being satisfied by the shipping code path rather than a test double. Scoped to one stimulus (photo-app-requirements) and SQLite-only assertions so nothing needs installing in the container. The requirements-schema-valid program grader is approximated with a json_valid shape check until the repo checkout needed to run the real validator is wired up. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 617f531f-88d8-4179-8fb5-a0c6ec52ff92
The eval spec never set a model, so the Copilot SDK fell back to whatever the host CLI defaults to. Locally that resolves from a developer's ~/.copilot/settings.json; in CI, with a clean HOME and the Actions token, it resolves to a different default. The same graders then produced different results in the two environments, which read as flaky graders rather than an unpinned model. Pin `defaults.model` to claude-sonnet-4.6 — one of the three models azure-project-plan.agent.md declares, and the cheapest of them, which keeps a per-PR run inside the workflow timeout. The supported set is read from the agent's own `model:` frontmatter rather than restated here, so the harness can't grade a model the product doesn't ship the agent on, and dropping a model from the product drops it from the evals too. An unsupported `--model` now fails fast with the supported list. check-agent-drift gains a matching contract so an unpinned or unsupported spec fails in a second instead of mid-run. Trials also report the model they actually ran as, instead of the hardcoded "unknown" that made every report say "Models: not reported". Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79eec597-7ca8-4de4-9cc2-9c10f3409071
2 participants
Add a Vally eval harness for the
azure-project-planagentAdds an automated evaluation suite that runs the shipped agent instructions against a set of realistic prompts and grades the artifacts it produces, so regressions in
resources/agents/**are caught in CI instead of by hand.Main changes
evals/) — a Vally suite (evals/project-plan/eval.yaml) with 7 stimuli covering requirements gathering (photo-app-requirements,api-only-inventory,multi-service-order-processing,no-datastore-converter) and plan generation/approval/feedback (plan-generation-task-app,plan-approval-scrapbook,plan-feedback-recipe-app).evals/executor/agent-assets.mjsgenerates the eval skill directly fromazure-project-plan.agent.md(carryingname/descriptionverbatim so activation is tested for real) and recursively copies the whole agent folder plusshared-references/into the workspace. Nothing is hand-restated, so the evals can't drift from what users receive or "teach to the test."evals/mcp/) — a stdio server exposing the 12workflow-tools-*webview/gate tools so tool-call contracts are observable outside VS Code. The tool list lives inworkflow-tools.mjsso it can be imported without starting a server.evals/check-agent-drift.mjs) — asserts 12 agent contracts plus cross-file consistency, and pins a SHA-256 hash ofresources/agents/**inagent-assets.lock.json. Editing agent instructions fails CI until the evals are re-run and the lock is updated with--update..github/workflows/vally-evals.yml) — runs the suite and the drift check; the Vally CLI is pinned vianpm ciinevals/instead of an unpinned global install.evals/owns its own lockfile); root only gainstypescript-eslintand aneval:driftscript.evals/is excluded from the VSIX.Graders
Built-in graders cover artifact presence/absence and tool calls (
file-exists,file-not-exists,file-matches,tool-calls,transcript-not-contains), plus three custom program graders:validate-requirements.mjs.azure/requirements.jsonparses and matches the schema; datastore rules (e.g. no datastore invented for a stateless converter, Blob Storage handled correctly)validate-project-plan.mjs.azure/project-plan.mdhas the required skeleton —Status/Createdmetadata rows, mandated sections and orderingvalidate-webview-parseable.mjsparseScaffoldPlanMarkdownlogic, so the plan webview can render itThese also enforce negative contracts: no premature
project-plan.mdduring requirements, no dotfile.requirements.json, no chat questions where the UI should be used, no inline scaffolding during planning.Report changes
evals/generate-report.cjsemits arun-diagnostics.md+run-diagnostics.jsonper run containing:agent_failure,product_failure,harness_failure, orinfrastructure_failure(upstream 5xx/429/auth/network errors are marked inconclusive, excluded from blame, and never allowed to headline a run that also contains a real defect)