Repository navigation
docs: add eval review guidance - #333
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Co-authored-by: Rodriguespn <44656907+Rodriguespn@users.noreply.github.com>
|
|
||
| Start with [Eval criteria](#eval-criteria) and [Writing prompts](#writing-prompts): confirm the `motivation:` cites a real user problem and the prompt has one clear, observable goal without leaking commands, schema details, or rubric language. | ||
|
|
||
| Then review [Writing scorers](#writing-scorers) assertion by assertion. Each check should map to a user requirement, avoid known false passes and false fails, measure behavior instead of one implementation path, and keep diagnostics as metrics rather than pass/fail gates. Ask for a deterministic or fixture-based check whenever it can replace `judge()`. |
There was a problem hiding this comment.
I couldn't find a way to record a non-gating metric in apps/framework. As far as I can tell, a check either passes or fails, so nobody can actually follow "keep diagnostics as metrics" yet. Could you link to the mechanism, or reword it to something like "don't turn incidental diagnostics into pass/fail checks"?
There was a problem hiding this comment.
I interpret it to mean that we keep the check but not have it gate the scenario's overall pass/fail, since there is a per-check pass/fail separate from per-scenario pass/fail. It isn't that useful in its current form though, eventually I think AI-1114 will offer a better strategy for diagnostic checks, so maybe the particular guidance here can wait on that.
There was a problem hiding this comment.
Good catch. I’ll replace the unsupported “metrics” wording with “don’t turn incidental diagnostics into pass/fail requirements.” We can revisit richer diagnostic checks under AI-1114.
|
|
||
| Use the [Submitting evals for review](#submitting-evals-for-review) workflow to require refreshed CI results before approval. The refreshed eval CI job should be green, which means the requested evals ran and results were recorded; it does not mean every agent passed the eval. After CI completes, inspect at least one passing result and each distinct failure shape, and confirm the experiment matches the intended suite, skills, runtime, hosted or local state, and any CLI or Docker assumptions. | ||
|
|
||
| Classify failing eval results before requesting changes: agent gap, product gap, scorer bug, or harness/runtime failure. A failing eval can be valid signal, but at least one known-bad solution or counterexample should fail to prove the scorer catches the intended failure mode. |
There was a problem hiding this comment.
This reads as a new approval requirement, but the guide doesn't say how to produce or show a counterexample. Only one eval has scorer tests today (evals/cli/build-database-002-stack-lifecycle/scoring.test.ts). Could you say how to show it, for example with a scorer unit test like that one or a failing run described in the PR? If not, maybe soften it to "ideally".
There was a problem hiding this comment.
Would love to revisit AI-934 at some point, maybe a smaller version of this could add the new vitest scorers in parallel w/ existing smoke stuff so we can start migrating new stuff over to the newer system w/o legacy concerns.
There was a problem hiding this comment.
Agreed. I’ll soften this to “ideally” and clarify that evidence can be a scorer unit test or a failing run linked or described in the PR. We can handle broader Vitest scorer migration separately under AI-934.
|
Thanks both. I’ve updated the checklist so incidental diagnostics are not treated as pass/fail checks, and softened the counterexample guidance with a scorer unit test or linked failing run as examples. |
a17a4d4
into
columferry/cli-2398-add-a-docker-less-local-stack-e2e-eval-agent-cli-lifecycle
Summary
CONTRIBUTING.md.Source