: Evaluation lifecycle (grounded generation → review → run → analyze)

[Apurva Banka (apurvabanka)]

Summary

Adds an end-to-end evaluation lifecycle to the ESS Maker Kit: grounded test-set
generation, a maker/reviewer review-and-promotion workflow, running evaluation
test sets against a Dataverse-backed agent, and analyzing/exporting results.
Everything is additive to the existing kit — no changes to the auth app
registration or credentials.

What's included

New capabilities

• Grounded generation — generate evaluation sets grounded in the agent's
  own topics, with an ESS catalogue reference for domain coverage.
• Review & promotion lifecycle — maker/reviewer choice prompts, a
  feedback-only reviewer workflow, automatic refresh of review sets, and
  promotion of approved sets.
• Run & analyze — run evaluation test sets and analyze results
  (per-case pass/fail, failure-pattern classification, suggested actions).
• Quality exports — CSV exports of evaluation results.

New scripts (solutions/ess-maker-skills/scripts/)

• evaluation_runs.py — run test sets, list runs, fetch results/analysis.
• evaluation_review.py — reviewer routing and feedback workflow.
• evaluation_promotion.py — promote approved test sets.
• evaluation_csv.py — results/quality CSV export.
• Updates to evaluate_evals.py, push.py, setup.py, fetch_and_setup.py,
  and the Power Platform clients (powerplatform_client.py, pp_admin_client.py).

New / updated skills & prompts

• Skills: evaluations/create, generate (+ ess-catalogue.md), review,
  run, update, validate, and a dispatcher.
• Prompts: create, evaluate, menu, review, run, update.

Tests

New test suites covering the lifecycle:
test_evaluation_runs.py, test_evaluation_review.py,
test_evaluation_review_routing.py, test_evaluation_promotion.py,
test_evaluation_csv.py, test_evaluate_evals.py,
test_setup_evaluations.py, test_fetch_and_setup.py, test_push_scope.py.

Scope / non-goals

• Targets the Dataverse-backed environment path (the standard /setup
  flow). The MinimalBot / Dataverse-free POC was explored on this branch and
  reverted (12ec360); it is not part of this PR.
• No changes to the auth app ID or credential handling.

[Dawn Jeong (daeunJe0ng)]

[copilot-review] Automated review pass. Verdict: Approve with nits — no blocking issues; one Should-fix (prompt-injection hardening) and one Question.

Design: Correct layer. Dataverse-backed maker-eval lifecycle (sets stored as Copilot Studio type-19 bot-components via /setup); does not conflict with runtime Dataverse removal, and the MinimalBot POC revert (12ec360) keeps it on the standard path.

Security (deep read):

• Should-fix: the topic-grounded create flow feeds untrusted customer topic fields (modelDescription, triggerQueries, SendActivity) into generation with no "data, not instructions" guard, unlike generate (inline comment on create/SKILL.md).
• Clear: generate grounds only in the kit-shipped catalogue; durable writes gated (run POST, description PATCH under errors == 0, POST excluded from retry); auth change is in-memory username only and preserves the CWE-209 no-echo pattern; path traversal blocked in evaluation_promotion._paths; CSV formula-injection guarded.

Correctness: push.py publishes review descriptions only after full success (no review_completed over stale YAML); promotion/cleanup payload-equality guards; connection selection fails closed on ambiguous MCS profiles. One Question on _case_passed (inline).

Docs: all 4 new maker-evaluation Learn URLs resolve 200; catalogue URLs spot-checked 200; no fabricated links.

Tests: 9 PR suites 101 passed. Full scripts+flightcheck 1474 passed, 10 failed — the 10 (test_installer_launch.py, test_installer_maker_profile.py) fail identically on base 28af0c3, so pre-existing and unrelated. Caveat: all offline/mocked; the 4 API shapes and run result parsing are unverified against a live tenant — recommend capturing one real get-maker-evaluation-test-run payload before GA.

[Dawn Jeong (daeunJe0ng)]

[copilot-review] Follow-up review of the full PR at bfed56e, including the latest fixes and existing discussions. This updates my earlier “Approve with nits” assessment: I recommend changes before merge based on the additional reproductions below.

Thanks for addressing both earlier comments. I confirmed the topic-content guard is present and empty/missing metrics no longer count as PASS. I am not reopening those findings.

Purpose and approach
Connecting evaluation generation, human review, execution, and analysis is useful, and keeping evaluation storage on the existing Dataverse-backed path fits this change. I would retain the architecture. The remaining issues concern keeping review state and agent identity consistent across the workflow.

Blocking: use current remote review state when deciding whether a set can run.
In solutions/ess-maker-skills/scripts/evaluation_runs.py:458–484, remote sets are fetched, but run eligibility comes from cached local/baseline review metadata rather than the fetched description. Offline reproductions show that an untagged local copy remains runnable when the remote set is review_requested, while a locally requested review remains blocked after remote completion. Please reconcile the live marker with pending local changes before allowing execution.

Should-fix: preserve the selected agent through refresh and push.
evaluation_review.py:200–206 discovers sets across configured agents, but the update instructions invoke a push command that uses the active agent. Completing Beta’s review with Alpha active reproduced “Nothing to push in the selected scope.” Please carry the owning agent explicitly or restrict selection to the active agent.

Should-fix: publish a new set’s review marker only after its cases upload successfully.
push.py:1767–1774 includes review_requested in parent creation, before child creation finishes. A failed child upload leaves an incomplete remote set tagged for review. Please defer the marker until content succeeds. The existing-parent completion path already handles this correctly; partial parent creation itself also predates this PR.

Should-fix: preserve conversational cases in previews and counts.
evaluation_csv.py:99–105 and the review/run counting helpers omit MultiTurnEvaluationCase, although the existing creation flow supports it. One valid conversational case reproduced a header-only CSV and a zero-case count. Please support this case type or explicitly report unsupported export instead of silently omitting it.

Test coverage and security
Please include the new lifecycle suites in continuous integration; the current test job runs tests/flightcheck, not these tests/scripts suites. No high-confidence exploitable vulnerability was established in the changed application code. The stale review gate is a workflow defect, not a demonstrated authorization bypass.

Verification: nine focused suites passed all 102 tests. Running python -m pytest tests/scripts tests/flightcheck produced 1,475 passes and 10 failures on head, versus 1,395 passes and the same 10 failures on base. Those installer failures are pre-existing. Eight offline reproduction/control probes confirmed the reported behavior. Ruff, compilation, and diff checks passed. No live tenant execution or end-to-end Copilot validation was performed.

[Apurva Banka (apurvabanka)]

Thanks for the thorough follow-up review, Dawn Jeong (@daeunJe0ng). I've addressed all five findings. Summary below.

Blocking — use current remote review state when deciding whether a set can run ( evaluation_runs.py )
Run eligibility now reconciles the live remote review marker (parsed from the fetched set description) against unpushed local changes, instead of trusting cached local/baseline metadata. If there's a pending local change, local intent governs ( NOT_PUSHED  /  PENDING ); otherwise the deployed/remote marker governs. This fixes both reproductions: an untagged local copy is now blocked when the remote is  review_requested , and a locally-requested review clears once the remote completes. Added two regression tests.

Should-fix — preserve the selected agent through refresh and push ( evaluation_review.py )
Discovery now scopes to the active agent ( config["agent"] ) — the same agent  push.py  operates on — rather than iterating every entry in  config["agents"] . This stops surfacing sets under a non-active agent that the push handoff can't reach, so completing Beta's review while Alpha is active no longer produces "Nothing to push in the selected scope." Added a two-agent regression test.

Should-fix — publish a new set's review marker only after its cases upload successfully ( push.py )
New parents no longer receive the  [ADK-REVIEW]  description inline at create time. The marker is now staged into the existing  pending_descriptions  deferral and PATCHed only when  errors == 0  (after child cases succeed); the publish loop resolves a new parent's  botcomponentid  from  pending_creates  as a fallback. A failed child upload no longer leaves an incomplete remote set tagged for review. This aligns the new-parent path with the already-correct existing-parent path.

Note: I did not add an end-to-end push test here — the push suite is helper-level only, with no  push()  network harness. The fix reuses the already-contracted deferral path and the covered  _review_description_for_create  helper. Happy to add a full harness if you'd prefer.

Should-fix — preserve conversational cases in previews and counts ( evaluation_csv.py ,  evaluation_review.py ,  evaluation_runs.py )
 MultiTurnEvaluationCase  is now counted in review/run previews and exported in Copilot Studio conversation-import format ( conversationNumber, question, response ) instead of being silently omitted (previously a header-only CSV and a zero count). CSV formula-injection guarding is preserved. Added a multi-turn export test.

Test coverage — include the lifecycle suites in CI ( .github/workflows/ci.yml )
Added a  script-tests  job running  tests/scripts , which now enforces the evaluation create/review/run/CSV/push-scope suites on every PR. It ignores only  test_installer_launch.py  and  test_installer_maker_profile.py  — the pre-existing red-by-design installer TDD tests for the unimplemented  MAKER_PROFILE_INSTALLED  feature — with a comment to remove the ignores when that feature lands.

Verification

•  tests/scripts  (minus the two ignored installer files): 360 passed
• Focused push/review/runs/csv suites: 153 passed
• Ruff clean,  push.py  compiles,  ci.yml  validates (4 jobs)

No live-tenant execution was performed; the API shapes and run-result parsing remain offline/mocked, consistent with the earlier caveat.

[Dawn Jeong (daeunJe0ng)]

[copilot-review] Thanks, Apurva. I reviewed the full PR at d5dea9f, the latest fixes, and all comments. Approving the merge with the follow-ups below. This supersedes my earlier recommendation to hold the merge.

Confirmed fixed: active-agent targeting, deferring new-parent review markers until content succeeds, conversational case counts, lifecycle test coverage in continuous integration, and both originally reported remote/local review-state cases. The topic-content guard and no-metrics fixes remain intact.

Non-blocking follow-ups

• Push retry safety, highest priority: In push.py:2131–2154, if parent/child creation succeeds but the deferred description PATCH fails, the successful component IDs are not saved. Retrying then creates duplicate records. Please preserve recoverable IDs without incorrectly advancing the entire baseline, and add a description-only-failure/retry test. Fix before broad customer rollout.
• Conversational CSV content: In evaluation_csv.py:154–161, text is read from the outer item, but the existing creation instructions put it under activity.text. That documented shape exports blank question/response cells. Please support the documented nesting and use it in the regression fixture before relying on conversational CSV export/import. The YAML itself remains intact.
• Review-state deletion edge case: In evaluation_runs.py:496–510, absent local review.json plus a completed baseline can still allow a run while the remote marker is review_requested. Please keep the remote request blocking through that transition and cover the run command as well as discovery. I am downgrading this from merge-blocking: it is a workflow edge case, not a demonstrated authorization bypass.

Verification: 106 focused tests passed. Broader scripts/FlightCheck tests produced 1,479 passes and the same 10 installer failures reproduced on the previous head and base. Separate offline probes reproduced the three follow-ups above. No high-confidence exploitable security vulnerability was established; no live tenant or end-to-end Copilot validation was performed.

These follow-ups do not hold this merge, but the approval does not mean the affected paths have been validated for customer release.