setup-maf-evals: report generation, three evaluator tiers, MSTest project shape - #2
Closed
leslierichardson95 wants to merge 18 commits into
Closed
setup-maf-evals: report generation, three evaluator tiers, MSTest project shape#2leslierichardson95 wants to merge 18 commits into
leslierichardson95 wants to merge 18 commits into
Conversation
…detection) Rewrites SKILL.md and references to specify the v2 overhaul: - Replaces hand-rolled markdown runner with a Microsoft.Extensions.AI.Evaluation.Reporting + `aieval` HTML report pipeline (GA 10.7.0). - Switches the scaffolded project from a console runner to an MSTest `<App>.Evals.Tests` project matching the canonical Learn-docs pattern. - Categorizes evaluators into three independent tiers (NLP / Quality / Safety) with separate env knobs (EVAL_USE_REAL_AGENT, EVAL_USE_REAL_JUDGE, EVAL_USE_FOUNDRY_SAFETY). - Adds IChatClient auto-detection so generated AgentChatClientFactory wires to the app's existing chat client registration. - Adds opt-in Safety tier (ContentHarmEvaluator via Azure AI Foundry) and opt-in GitHub Actions workflow. - New eval.yaml has 8 scenarios covering scaffold, update-mode preservation, IChatClient detection, reporting wiring, schema v2, CI workflow, safety opt-in. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Skill Coverage Report
Uncovered:
|
Three gaps surfaced when scaffolding ELI5Agent.Evals.Tests against an Aspire 13.2 + Foundry app: 1. **Detection table missing the Aspire Inference pattern.** `builder.AddAzureChatCompletionsClient(\"chat\").AddChatClient(\"chat\")` is the standard Aspire 13.2 way of wiring an IChatClient against a Foundry chat deployment, and was not in the v2 detection table. Added to ichatclient-detection.md with a note that the argument is the connection-string name. 2. **Connection-string setup not surfaced for standalone runs.** When the app uses Aspire orchestration, `ConnectionStrings:<alias>` is populated by the AppHost — but `dotnet test` runs outside the host and gets a silent missing-config NRE on first real-agent run. Added a `Connection-string setup for standalone test runs` section to ichatclient-detection.md that surfaces both user-secrets and env-var setup paths and points to `azd env get-values`. 3. **WordCountEvaluator implementation not pinned.** The catalog called the evaluator out as `always scaffolded (custom)` but never gave a verbatim template. Added the Learn-doc canonical implementation to evaluators-catalog.md so every scaffold gets the same (correct) IEvaluator skeleton. Validated end-to-end against ELI5Agent: scaffolded ELI5Agent.Evals.Tests, `dotnet test` exits 0 in stub tier with 4 scenarios x 4 metrics (Words/BLEU/GLEU/F1) producing a 670 KB report.html. Real-judge tier deferred (no Azure creds set up locally for ELI5Agent — itself the trigger for finding 2). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…gnostic 1. New references/metrics-glossary.md authored as the source of truth for metric definitions/scales/thresholds across NLP, Quality, and Safety tiers. Includes the canonical Reporting/MetricsGlossary.cs template that the skill emits into <App>.Evals.Tests/Reporting/. 2. Factory template in references/ichatclient-detection.md now wraps DI resolution in try/catch and throws a friendly InvalidOperationException naming the connection-string key + the exact 'dotnet user-secrets set' command + the env-var alternative + 'azd env get-values' pointer. Replaces the silent NRE that ELI5Agent dogfooding hit when EVAL_USE_REAL_AGENT=1 with no creds set up. 3. Two new eval.yaml scenarios bind the new behavior: - scaffold-emits-metrics-glossary: asserts MetricsGlossary.cs exists, references metrics-glossary.md, has [AssemblyCleanup] - factory-emits-friendly-secrets-diagnostic: asserts AgentChatClientFactory mentions 'dotnet user-secrets' + 'ConnectionStrings' 4. Captured two MSTest constraints that turned up while implementing the glossary writer: - quality-modes.md ReportingConfig: ExecutionName must be cached at class load, NOT re-evaluated per call (otherwise AievalReport and MetricsGlossary land in different timestamped folders 5 s apart) - metrics-glossary.md template now declares MetricsGlossary as a plain static class; MSTest forbids more than one [AssemblyCleanup] per assembly (UTA014). Glossary write is chained from AievalReport's single AssemblyCleanup, wrapped in try/catch so a glossary-write failure doesn't mask the report. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Spec was creeping toward the validator's 'comprehensive' threshold (3,574 BPE tokens / 14,238 chars / 265 lines). Pulled prose-heavy sections into references and kept SKILL.md focused on decision-relevant content. - Step 3 (Scaffold): replaced 27-line file tree with a one-sentence summary + link to project-template.md. Kept the post-write powershell as a fenced code block so the agent has concrete commands. - Steps 4-9 (telemetry/quality/compare/safety/panel/CI): collapsed bulleted prose into 3-4 line stubs that retain the decision facts (default ON/OFF, opt-in semantics, env knob) and link the corresponding reference. - Step 11 (Surface in chat): added the metrics-glossary.md path to the 'Paths' bullet and to the trailing 'see also' line. - ## Common pitfalls: extracted to references/common-pitfalls.md (also adds two new entries from this work: the multi-AssemblyCleanup MSTest constraint and the AgentChatClientFactory friendly-NRE pattern). SKILL.md keeps a one-line link. - ## References: trimmed each bullet to one short line; consolidated the three external links onto a single line. Net effect: SKILL.md 14,238 -> 10,449 chars, 265 -> 189 lines, 3,574 -> 2,672 BPE tokens (-25%). Validator still passes; eval.yaml scenarios are unaffected (assertions are file-based, not prose-based). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Owner
Author
|
Two new commits on this branch since the dogfood-findings push ( \8255ef8\ — per-run metrics glossary + friendly user-secrets diagnostic
\�12a9a6\ — slim SKILL.md by moving prose into references/
Validator and dogfood:
User-secrets check scopeLightweight runtime diagnostic only (10 lines in the factory template). No host-system probing, no calls to \dotnet user-secrets list, no Azure cred validation. Those remain DefaultAzureCredential / user-secrets / \�zd env's job. |
Dogfood findings against ELI5Agent surfaced 3 real failure modes when promoting from stub to judge tier: 1. user-secrets silently not loading: dotnet test runs under testhost.exe as the entry assembly, so Host.CreateApplicationBuilder() does NOT pick up the secrets store keyed off the test project's UserSecretsId. Fix: factory template now calls builder.Configuration.AddUserSecrets(typeof(...).Assembly, optional: true). 2. services.ai.azure.com hostname strips dashes from the resource name (foundry-abc -> foundryabc.services.ai.azure.com). The legacy properties.endpoint value points at cognitiveservices.azure.com which 404s for the /models route. 3. Foundry resources provisioned by Aspire/azd usually have disableLocalAuth=true. Key-based auth returns 403; drop the Key= segment and rely on DefaultAzureCredential. Updates: - references/ichatclient-detection.md: factory template adds AddUserSecrets; diagnostic message now lists Entra and Key options + endpoint gotchas. - references/project-template.md: GlobalUsings.cs adds Microsoft.Extensions .Configuration / .DependencyInjection / .Hosting (needed by the factory). - references/common-pitfalls.md: 3 new entries covering the above. - SKILL.md: step 11 now surfaces the exact 2-command judge-promotion path. - tests/dotnet-ai/setup-maf-evals/eval.yaml: new factory-loads-user-secrets-explicitly scenario. Verified end-to-end: ELI5Agent judge tier 4/4 passing in 2m 24s, report.html shows 9 metric families (Words/BLEU/GLEU/F1 + Relevance/ Coherence/Fluency/Completeness/Equivalence). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Dogfooding the Judge tier against ELI5Agent's gpt-5-mini judge surfaced a silent failure: reasoning models (gpt-5*, o-series) reject the max_tokens parameter that Azure.AI.Inference still sends, returning HTTP 400 unsupported_parameter. The MEAI Quality evaluators swallow the 400 and record it as a per-metric error row -- tests exit 0 but every Quality column is an error. Pitfalls doc now covers: - which model families are affected (gpt-5*, o1/o3/o-series) - how the failure manifests (test pass + errors in report.html, not a hard test failure) - the workaround (pick a non-reasoning judge: gpt-4o / gpt-4o-mini / gpt-4-turbo; if the agent uses reasoning, set EVAL_JUDGE_DEPLOYMENT_NAME=<non-reasoning-alias> to split agent and judge deployments) - az CLI snippet to list deployment model families SKILL.md step 11's judge-promotion block now flags this alongside the existing dash/key-auth gotchas. eval.yaml gets a smoke assertion that common-pitfalls.md mentions both max_tokens and the env-var workaround. Verified end-to-end: re-pointed ELI5Agent at a gpt-4o-mini Foundry deployment, cleared _store/, re-ran judge tier: 4/4 passing in 44s, clean report (no max_tokens / unsupported_parameter strings anywhere), all 9 metric families populate (4 NLP + Relevance / Coherence / Fluency / Completeness / Equivalence). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…+ compare opt-in Three changes from a dogfood-driven review: 1. Compare mode -> opt-in (step 2 dotnet#4 + step 6). Largest scaffold for least-common surface; users opt in if they actually need matrix.json side-by-side. Scaffold step now only emits Compare/* on opt-in. 2. Step 11 chat output restructured to lead with Quality as the headline evaluation. NLP framed as the zero-config sanity check (still on by default so first \dotnet test\ produces a real report). Safety + Compare listed as additional categories below. Step 2 tier table re-ordered with Quality on top and a 'Default' column for clarity. 3. New common-pitfalls section 'Tuning Quality for stylistic agents' covering the case where ELI5 / summarizer / strict-format / persona agents get punished by generic rubrics in CompletenessEvaluator and EquivalenceEvaluator. Includes three remediation patterns: - drop offending evaluators per app - rewrite goldens in agent voice - custom rubric-driven evaluator evaluators-catalog.md now has the full RubricEvaluator template (reads Quality/rubric.md, judges via the active IChatClient, emits a RubricFit numeric metric with rationale). Step 11 surfaces this caveat on first scaffold so users don't see bad Quality scores against a bad-fit rubric and conclude their agent is broken. eval.yaml adds three smoke assertions: - compare-mode-is-opt-in-not-default - pitfalls-doc-warns-stylistic-agents-fail-completeness Validator green (9 skills + 1 agent + 1 plugin). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n (CS9006)
Found by dogfooding the v2 template against ELI5Agent: the prompt block
used single-dollar interpolated raw string with {{ }} attempting to
escape literal JSON braces. C# raw strings reject {{ as literal in
single-dollar mode (CS9006: 'does not start with enough $ characters
to allow this many consecutive opening braces as content').
Fix: switch to double-dollar raw string $$"""...""" where literal { is
fine and {{var}} is interpolation. Added a comment in the template
explaining the choice so users don't "simplify" it back.
End-to-end validated against ELI5Agent:
- Compiles clean.
- 4/4 judge-tier tests pass in 58s with the new evaluator wired.
- RubricFit column appears in report.html with substantive rationales.
- Score comparison (same agent, same responses):
generic Relevance 3.25, Coherence 3.00, Fluency 3.50
vs RubricFit (ELI5-tuned rubric) 5.00 / 5
-- exactly the remediation outcome documented in common-pitfalls.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document that 'Cache Miss' on every call is expected in the real-agent + real-judge workflow because the judge cache key includes the agent's response, which varies run-to-run. Also note that executionName scopes the cache. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three changes that make the MEAI response cache actually pay off across runs (verified on ELI5Agent: cold 39s/0 hits -> warm 15s/20 hits with the same scenarios): 1. QualityTests uses run.ChatConfiguration!.ChatClient (the cached wrapper) for the agent call by default, only falling back to the uncached factory when EVAL_JUDGE_DEPLOYMENT_NAME splits judge from agent. Previously every run created a fresh uncached agent client and the agent's varying output guaranteed judge cache misses too. 2. Drop the executionName argument from DiskBasedReportingConfiguration.Create. executionName is part of the cache scope -- passing a fresh per-run timestamp guaranteed misses regardless of input. Report folder timestamping moves to the new EvalEnv.ReportFolder (EVAL_REPORT_FOLDER env var). 3. SKILL.md step 11 + common-pitfalls.md rewritten to explain the cache payoff: first run ~60s populates _store/cache/, every subsequent run against unchanged inputs ~5s with zero LLM cost. Compare mode keeps a stable per-entry executionName for cache reuse across compare runs. ichatclient-detection.md documents the override-splits-cache trade-off. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Owner
Author
|
Cache hits now actually work. Three changes (commit 2959158) — verified on ELI5Agent:
Test phase alone dropped 30s → 6s. Same 4 scenarios, 5 evaluators each, no LLM tokens spent on run 2. What changed:
When SKILL.md step 11 + |
…skills Polish pass after dogfooding 3 perf skills against ELI5Agent (1-agent MAF/Aspire/Foundry app): - scan-agentic-app-perf: new check-id-glossary.md (24 check codes, severity, per-category file links, cross-skill route table) written to .copilot/perf-reports/ alongside scan reports. Mirrors the metrics-glossary.md pattern from setup-maf-evals v2. - scan / configure / select: new references/common-pitfalls.md capturing dogfooded false-positive patterns (MA4 in AppHost is expected, MA1 false-fires on 1-agent apps, O3 in MEAI source is normal, sentinel parsing must fail-closed, role classification edge cases, etc). - agentic-perf-reviewer.agent.md: inline Common pitfalls section (agents have no references/ folder by convention). - tests/dotnet-ai/agentic-perf-reviewer/eval.yaml: 11 routing- discrimination scenarios (4 should-invoke, 4 should-defer-to- optimizing-dotnet-performance, 3 should-route-to-child-skill). Validator: ✅ 9 skills + 1 agent + 1 plugin pass. Markdownlint: ✅ 0 errors on changed files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Owner
Author
|
Polish pass — dogfooded 3 sibling perf skills against ELI5Agent + added routing tests for the agent. What landed (commit
|
| Item | Result |
|---|---|
scan-agentic-app-perf on ELI5Agent |
1 info finding (MA4 in AppHost — correctly surfaced as no action required), zero noise |
configure-agentic-perf-rules on ELI5Agent |
Fresh install of v0.1.0 managed block; second run is byte-identical (SHA256 verified) |
select-agent-models on ELI5Agent |
Correctly aborts on 1-agent app per spec line 23-24 (intentional, not a defect) |
New check-id-glossary.md in scan skill |
24 check codes table, severity, category-doc links, cross-skill route table. Pattern mirrors setup-maf-evals' metrics-glossary.md. Written per-run to .copilot/perf-reports/. |
New common-pitfalls.md in scan / configure / select |
False-positive patterns + per-check sharpening guidance |
Inline pitfalls in agentic-perf-reviewer.agent.md |
Agents have no references/ folder by convention |
tests/dotnet-ai/agentic-perf-reviewer/eval.yaml |
11 routing scenarios: 4 should-invoke / 4 should-defer-to-optimizing-dotnet-performance / 3 should-route-to-child-skill |
Validation
markdownlint-cli2on all changed files: 0 errorsskill-validator check --plugin plugins/dotnet-ai: ✅ 9 skills + 1 agent + 1 plugin pass- Agent description trigger phrases verified present:
agentic,Microsoft Agent Framework,Aspire,Foundry,perf,cost,topology,model selection,slow,Do NOT use
Known follow-ups (out of scope for this commit)
- 18 pre-existing
MD033/no-inline-htmlerrors in scan skillreferences/*-checks.md(literal<agent>/<tool-name>placeholders) — separate cleanup - Single-agent apps don't stress MA1/T1/T2/MH1 — dogfooding
interview-coach-v2would exercise the multi-agent surface area - Routing
eval.yamlis authored but the eval harness for agents doesn't exist ineng/skill-validatoryet (evaluatecommand only supports skills today)
…rences Wrap <agent>, <tool-name>, <A>, <B>, <leaf>, <downstream>, <names>, <tool-A>, <tool-B>, <tool>, <N> placeholder identifiers in backticks across the 5 *-checks.md files. These had been parsed as raw inline HTML elements by markdownlint (MD033/no-inline-html), producing 17 pre-existing errors. The intent was always literal placeholders in the 'Next:' action templates, so backticks are the correct fix. Verified: markdownlint-cli2 on all 5 changed files now reports 0 errors. Skill validator still passes (9 skills + 1 agent + 1 plugin). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Owner
Author
|
Follow-up polish on PR #2. Item 1 — Dogfooded
|
| Check | Severity | Fired? | Notes |
|---|---|---|---|
T2 LLM-routed handoff fan-out from triage (4-way) |
critical | ✅ | Triage routes among 4 specialists per turn — exactly the pattern T2 is designed to catch. Two-LLM-hop pattern surfaced cleanly. |
T3 Handoff cycles (every specialist ↔ triage) |
critical | ✅ | 4 directed cycles detected; report cites lines 281-287 with full evidence snippet. |
| T1 Agent count = 5 (threshold 3) | warn | ✅ | Cited all 5 declaration lines (26/77/136/199/255). |
TI2 Duplicate tool surface (interviewDataTools on 5/5 agents) |
warn | ✅ | Tool-list comparison correctly identified copy-paste. |
| MH1 Full history flows through every agent | warn | ✅ | Absence-of-X check correctly listed search patterns + 0-hit result, per evidence-gate rule. |
| MH2 No history cap | warn | ✅ | Same absence-of-X pattern. |
| MH3 History sent to deterministic-style router | warn | ✅ | Correctly identified triage as a router given its rule-based instruction set. |
| PW2 Large per-agent system prompts (~3.9K tokens total) | warn | ✅ | Per-agent line + token-estimate breakdown rendered. |
| MA1 All agents on same model | info (downgraded from critical) | ⚠ | This is the interesting case: the common-pitfalls guidance ("don't promote MA1 to critical without consulting select-agent-models' role-matrix") fired correctly. The report explains why MA1 is defensible here — every role's matrix-primary is gpt-4o-mini anyway. |
| MA4 Model literal in AppHost | info | ✅ | Correctly framed as "no action — AppHost is the right place". Matches pitfalls file. |
Report: Q:\source\interview-coach-v2\.copilot\perf-reports\scan-20260622-213000.md (14,153 bytes; mirrored to latest-scan.md, SHA256 verified equal).
Glossary parity: also wrote check-id-glossary.md per the v2 spec — pattern validated on a real repo.
Net signal: 2 critical, 6 warn, 2 info — the report is substantive without being noisy, and the MA1 framing nuance from common-pitfalls landed correctly. No false positives.
Item 2 — Fixed 17 pre-existing MD033 errors (commit 6769639)
Wrapped placeholder tokens (<agent>, <tool-name>, <A>, <B>, <leaf>, <downstream>, <names>, <tool-A>, <tool-B>, <tool>, <N>) in backticks across 5 *-checks.md files. These were always intended as literal placeholders in the Next: action templates.
- markdownlint-cli2: ✅ 0 errors on the 5 changed files (down from 17)
- skill-validator: ✅ still passes (9 skills + 1 agent + 1 plugin)
Remaining items from prior next-steps list
- ✅ Item 1 — dogfood scan against interview-coach-v2
- ✅ Item 2 — fix 18 MD033 errors (was actually 17; all fixed)
- ⏳ Item 3 — agent-routing eval harness in skill-validator (not started; agent-routing eval.yaml already authored in commit
ee31991) - ⏳ Item 4 — CI smoke step wiring skill-validator (not started)
Surfaced as a skill-gap during the code-review-buddy dogfood
(leslierichardson95/code-review-buddy Phase 2.5): users designing a
new agentic app had no skill-driven way to consult the role-model
matrix before writing any code — they had to read
references/role-model-matrix.md directly.
Plan mode is source-free:
- Triggers on 'design my topology', 'planning a new app',
'what models should I use, haven't written code yet'.
- Asks for { agents, quality_priority, provider_constraint }.
- Single-agent plans are valid (recommend mode's <2 abort is bypassed).
- Output adds a deployment-shape section (alias -> model id, who uses it)
+ a verify-checklist driving a later recommend-mode pass for
prediction-held confirmation.
Files written to a separate model-plan-design-<ts>.md series so the
later recommend run can sit next to it.
Skill description / WHEN / NOT-WHEN updated. references/plan-template.md
documents the new layout + relaxed empty-plan contract.
Validated against the existing dogfood:
code-review-buddy/.copilot/perf-reports/model-plan-design.md was hand-
written following exactly this layout in Phase 2.5; verify-mode
(model-plan-verify.md) confirmed every prediction held.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resets scope after dogfood feedback. Original motivation for select-agent-models was 'Copilot always defaults to gpt-4o when scaffolding agents.' The skill grew well past that — recommend/plan/ apply modes, role-model matrix doc, registry-currency questions, greenfield plan-mode follow-on, etc. — and Foundry's model-router covers the per-request piece better than the skill ever could. Net change: minimum viable fix lives where it always should have — in rule dotnet#3 of the configure-agentic-perf-rules managed block that Copilot already reads on every turn. configure-agentic-perf-rules (bumped to v0.2.0): - Rule dotnet#3 in managed-block-template.md is now self-contained and prescriptive: a role-pick table covering router/validator/formatter/ worker -> small-fast; planner -> reasoning-class; creative -> frontier. Calls out Foundry model-router as the recommended worker pick when prompt length varies. Says 'stop and ask if unsure; do not silently default to gpt-4o.' - rule-rationales.md rule dotnet#3 mirrors the new table with why-it-matters + when-frontier-is-justified + 'state the why in code' guidance. - Removed all references to select-agent-models from NOT-WHEN / companion-skills sections. scan-agentic-app-perf: - MA1/MA2/MA3 'Next' actions now point at rule dotnet#3 of the managed block instead of select-agent-models. 'Ref:' fields updated. - check-id-glossary.md, common-pitfalls.md, report-template.md, SKILL.md description + follow-up offer updated to drop the skill reference. setup-maf-evals: - Description NOT-WHEN no longer mentions select-agent-models. - Follow-up recommendation after telemetry/quality runs now references 'a model swap per rule dotnet#3' instead of the deleted skill. - aspire-dashboard-panel.md drops the cross-reference. agentic-perf-reviewer.agent.md: - Description + Skills-used drop select-agent-models. - Pass 2 model-finding route now points at configure-agentic-perf-rules. - Pass 3 follow-up letters condensed from 4 to 3 (A=rules, B=evals, C=stop). - Single-agent-app pitfall reworded: rule dotnet#3 covers single agents too, no abort-to-skip needed. DELETED: plugins/dotnet-ai/skills/select-agent-models/ (6 files). The audit case (scan-agentic-app-perf MA1-MA4) still covers existing apps. The greenfield case is the rule itself, sitting in copilot- instructions.md where Copilot reads it on every scaffold turn — which is the actual problem we set out to solve. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Same simplification rationale as the select-agent-models retirement. After moving role-aware model selection into rule dotnet#3 of configure-agentic-perf-rules, the agent's orchestration logic collapsed to: - run scan-agentic-app-perf (Pass 2) - route MA* findings to configure-agentic-perf-rules - route O* findings to setup-maf-evals That's logic Copilot already does organically from the skills' descriptions and the scan's existing 'ref:' fields + step-6 routing offer. The agent was adding ~70% ceremony for ~30% real value. Net change: - DELETED agents/agentic-perf-reviewer.agent.md (~180 lines) - DELETED agents/ directory (now empty) - Removed 'agents' key from plugin.json and .codex-plugin/plugin.json - The one unique guard worth keeping ('don't recommend a model downgrade without an evals quality follow-up') migrated into scan-agentic-app-perf SKILL.md common pitfalls. - The other 'don't pre-confirm apply-mode writes on user's behalf' guard already lives in scan-agentic-app-perf step 6.4. Users now invoke the skills directly: - 'scan my agentic app' -> scan-agentic-app-perf (which offers its own A/B/C follow-up menu) - 'install perf rules' -> configure-agentic-perf-rules - 'wire up evals' -> setup-maf-evals Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…slugs; prune arbitrary thresholds
Two related cleanups based on dogfood UX feedback.
1. Cryptic codes -> human-readable slugs.
Findings used to be tagged with MA1, TI4, PW3, etc. — codes that
were stable + greppable but required readers to flip to a glossary
file every time. The prefix-routing case (e.g. 'MA* -> select-
agent-models') that justified the convention is gone now that
select-agent-models and the orchestrator agent are retired.
Schema:
check_id: T1 | T2 | TI1 | ... | MA4
becomes:
check: <category>.<descriptor>
where category is one of topology / tools / history / prompt /
parallel / otel / model and the descriptor names the actual issue.
Examples:
MA1 -> model.same-default
MH1 -> history.full-share
TI4 -> tools.description-too-long
O1 -> otel.missing-sdk
No glossary lookup needed — the slug self-describes.
check-id-glossary.md -> check-glossary.md with a new
slug -> description table.
2. Pruned arbitrary-threshold checks.
Five checks were taste calls dressed up as audit findings; they
fired on legitimate designs as often as on real bloat:
- T1 'agent count > 3' (DELETED)
- T2 'LLM handoff edges per turn > 2' (DELETED)
- TI1 'tools per agent > 8' (DELETED)
- PW2 'few-shot examples > 3' (DELETED)
- P1 'sequential awaits over independent inputs' (DELETED;
general .NET concurrency anti-pattern, belongs to
optimizing-dotnet-performance)
Each deprecated check leaves a 'What used to live here' breadcrumb
in its category file so users grep-finding the old name see why
it was removed and where the underlying concern is now addressed.
Net: 24 checks -> 19 checks, no schema gymnastics, no glossary
round-trips for the reader.
Files touched:
- references/{topology,tool-inventory,message-history,prompt-weight,parallelism,otel-coverage,model-assignment}-checks.md rewritten
- references/check-id-glossary.md -> check-glossary.md (rename + rewrite)
- references/report-template.md schema update
- references/common-pitfalls.md per-check sharpening updated for new slugs
- SKILL.md finding schema + sort key + references list updated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ten scan.md Dogfood feedback: three generated files per run (scan-<ts>.md + latest-scan.md + check-glossary.md) plus a new timestamped file every scan made even a clean repo's .copilot/perf-reports/ look like a hoarder's basement after a week. The slug rename made the glossary file redundant anyway — 'history.full-share' is self-describing in a way that 'MH1' never was. Changes: - Single output file: .copilot/perf-reports/scan.md, overwritten on every run. No timestamped copies, no latest-* mirrors, no per-run glossary file. - Category legend is inlined into the report (the blockquote at the top of '## Findings') so readers see what topology/history/model/ etc. mean without leaving the file. - references/check-glossary.md remains as a dev-facing catalog (for skill maintainers adding new checks); it is explicitly NOT copied into user repos. - references/report-template.md updated with the new filename + the inlined legend block. - Validation contract simplified: one file exists, has a '## Findings' section, counts match chat. - Multi-AppHost case: per-host filename is now scan-<host>.md (still overwritten each run, no timestamp suffix). Git is the history mechanism if anyone wants one — the skill no longer reinvents version control by hand. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…m rules #1/#2; bump to 0.3.0 Rules #1 (agent count) and #2 (handoff edges) shipped with numeric ceilings (agent_count_max: 3, llm_routed_edges_max_per_turn: 2) that tripped legitimate designs (specialist-handoff workflows, multi-hop deterministic chains with one LLM-routed intent decision) as often as they caught real bloat. The justify-the-add gates were always the actual mechanism; the numbers were illusory precision. Mirrors the same prune applied to scan-agentic-app-perf (a38f3f8) where the topology.agent-count and topology.handoff-fanout checks were removed for the same reason. Changes: - managed-block-template.md: drop both keys from the thresholds YAML block; reword rules #1/#2 to explicitly say 'no hard ceiling — justify the add'. - rule-rationales.md: drop the 'when you must add a 4th+' subsection; add a 'on thresholds' note explaining why the numeric ceilings were removed (so consumers reading rationales understand the v0.3.0 change). - threshold-defaults.md: drop the first 2 rows of the threshold table; add a deprecation note pointing to the rules. - SKILL.md: bump version 0.2.0 -> 0.3.0; update the threshold-preservation example from agent_count_max to per_turn_input_token_warn. - common-pitfalls.md: update strict-parse example from agent_count_max to per_turn_input_token_warn. - eval.yaml: drop agent_count_max literal assertion from fresh-install test; bump idempotency-test fixture from v0.1.0 -> v0.3.0; bump threshold-preservation fixture from v0.0.1 to drop deprecated keys from the input YAML; add new scenario validating v0.2.0 -> v0.3.0 upgrade drops deprecated keys with a chat warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Owner
Author
|
Closing — this PR was opened against the wrong base branch ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Overhauls the
setup-maf-evalsskill so it produces the canonical Microsoft.Extensions.AI.Evaluation HTML report (per the Learn doc tutorial) instead of a hand-rolled markdown approximation.Dogfooding the v1 skill on a real MAF + Aspire + Foundry app revealed it never wired
Microsoft.Extensions.AI.Evaluation.Reporting— users got a markdown table where the docs promised a rich HTML report. Hence this rewrite.What changes
<App>.Evals.Tests(matches Learn doc +dotnet/ai-samplescanonical pattern;dotnet test+ Test Explorer + CI integration are native).DiskBasedReportingConfiguration+ScenarioRun.EvaluateAsync+[AssemblyCleanup]invokes theaievaldotnet tool (pinned viadotnet-tools.json).BLEUEvaluator/GLEUEvaluator/F1Evaluator/WordCountEvaluator): always on, no API key.RelevanceEvaluatoretc.): on whenEVAL_USE_REAL_JUDGE=1.ContentHarmEvaluatorbundle + others): opt-in viaEVAL_USE_FOUNDRY_SAFETY=1.Agent vs judge are also independently switchable (
EVAL_USE_REAL_AGENT).AddAzureOpenAIChatClient/AddOllamaChatClient/AddAIInference/ explicitservices.AddSingleton<IChatClient>and generatesWire/AgentChatClientFactory.csautomatically. Three cases handled (single hit, multiple hits, none).schema_version,reference_response(for BLEU/GLEU/F1/Equivalence/Completeness),context(for Groundedness),expected_tool_calls(for ToolCallAccuracy). Migration is additive.ContentHarmEvaluatorover 4 separate evaluators: 1 Foundry call vs 4 for the same metric bundle (Hate/SelfHarm/Violence/Sexual)..github/workflows/evals.ymlruns evals on PRs, tier-detects from repo secrets, uploadsreport.htmlas a build artifact.Assert.Inconclusivewhen Foundry creds missing — never fails the build for missing opt-in capability.Files
Updated:
SKILL.md(full rewrite — 11 numbered workflow steps),references/project-template.md,references/quality-modes.md,references/telemetry-capture.md,references/compare-mode.md,tests/dotnet-ai/setup-maf-evals/eval.yaml(8 new scenarios).New:
references/ichatclient-detection.md,references/evaluators-catalog.md,references/safety-mode.md,references/ci-workflow.md,references/dotnet-tools-manifest.md.Validation
eng/skill-validator/srcdotnet run -- check --plugin plugins/dotnet-ai-> ✅ all checks pass (9 skills, 1 agent, 1 plugin).eval.yamlcovers 8 scenarios binding the spec:scaffold-evals-tests-project-fresh,scaffold-with-safety-tier,scaffold-with-ci-workflow,ichatclient-detection-azure-openai,ichatclient-detection-missing-emits-stub,skip-when-no-app-host,update-mode-preserves-user-data,tier-banner-surfaces-in-chat-output.Pre-merge checklist
interview-coach-v2clone end-to-end.report.htmlwith ≥4 metric columns (Words/BLEU/GLEU/F1).EVAL_USE_REAL_JUDGE=1tier adds ≥3 Quality metrics (Relevance/Coherence/Fluency).dotnet testexits 0 in stub tier with no Azure creds.eval.yamlfixtures still pass (no regression).Background
This is the second skill PR in the
dev-integrationseries — see alsoskills/setup-maf-evals(v1, already merged intodev-integration). The v1 commit history is preserved; v2 sits on top.Upstream
dotnet/skillsPR deferred until validation finishes.