Azure / Azure/azure-dev

[evals] Follow up on remaining Vally eval PR feedback from #9282, primarily around quality of evals

Open
#9,340 2 comments 0 reactions 1 assignee Claimed by @richardpark-msft View on GitHub
area/evals engineering item
Dominant language
Go
Stars
569
Forks
364
Avg merge
2d 19h
Merged PRs (30d)
136

Description

Follow-up from [PR #9282](https://github.com/Azure/azure-dev/pull/9282): remaining unresolved review feedback should be handled after the initial Vally eval port lands.

**Category 1: CI gating and eval pass/fail signal**

- [ ] Make `.github/actions/vally-eval/action.yml` fail when the JUnit output has failures, or when any stimulus verdict is 0/N. Today the Vally process can exit 0 because the eval-wide average passes even when a stimulus fails every trial.
Comment: https://github.com/Azure/azure-dev/pull/9282#discussion_r3662151591

- [x] Decide whether `environment-delete-env` should require the skill to teach `azd env remove`, or whether the eval should intentionally expose that base-model gap. The current stimulus failed 0/3 on this HEAD.
Comment: https://github.com/Azure/azure-dev/pull/9282#discussion_r3662151594

- [x] If the skill is expected to teach cleanup, add `azd env remove ` guidance to `cli/azd/test/evals/skills/azd/SKILL.md`, including the `az group delete` mental-model section.
Comment: https://github.com/Azure/azure-dev/pull/9282#discussion_r3662151598

**Category 2: Q&A grader quality**

- [ ] Revisit `eval-azd-qna.yaml` after prompt graders were removed. With only two regex graders and threshold 0.50, incomplete answers can pass, such as asking for `azure.yaml` rather than troubleshooting the error. Consider keeping one judge per stimulus and raising the threshold.
Comment: https://github.com/Azure/azure-dev/pull/9282#discussion_r3662151609

**Category 3: Report correctness and coverage**

- [x] Fix single-run report headings in `vally_report.go`; Vally 0.10 omits `trialIndex` and `totalTrials` for `runs: 1`, causing `trial 1/0` output.
Comment: https://github.com/Azure/azure-dev/pull/9282#discussion_r3662151607

- [x] Add fixture/test coverage for missing `trialIndex` and `totalTrials` so the single-run report heading behavior is pinned.
Comment: https://github.com/Azure/azure-dev/pull/9282#discussion_r3663825299

**Category 4: Experiment methodology**

- [ ] Fix `eval-azd.experiments.yaml` so the subject model cannot also be its own judge. The matrix changes `/defaults/model`, but `judge_model` remains pinned to `gpt-5.5`.
Comment: https://github.com/Azure/azure-dev/pull/9282#discussion_r3662910654

**Category 5: Dependency and diff hygiene**

- [ ] Restore or avoid unintended `package-lock.json` churn that drops `libc` from the `@github/copilot-*` Linux packages. Regenerate with npm 11.11.0+ or drop those hunks.
Comment: https://github.com/Azure/azure-dev/pull/9282#discussion_r3663825292

- [ ] Remove the stray whitespace-only line in `eval-azd-deploy.yaml`.
Comment: https://github.com/Azure/azure-dev/pull/9282#discussion_r3662151611

## Additional follow-ups from the #9378 review

Raised by @jongio during review of #9378 and not covered by the boxes above.

- [ ] Gate the `Reject PRs from forks` step in `.github/actions/vally-eval/action.yml` on the guard step succeeding (`steps.copilot.outcome == 'success'`). The workflow sets `cancel-in-progress: true`, and `always()` still runs on cancellation, so if a run is cancelled before `Check Copilot availability` finishes, `enabled` is empty, `!= 'true'` is satisfied, and a same-repo PR gets a misleading "cannot run for fork pull requests" error.
- [ ] Use `assert` + `continue` instead of `require` in the stimulus loop in `cli/azd/test/evals/vally_report_test.go`. `require` calls `t.FailNow()`, so the first missing stimulus aborts the loop; the previous `t.Errorf` + `continue` reported every missing entry in a single run.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.