History-trained agentic files + expert reviewer (proposal)#35062
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35062Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35062" |
There was a problem hiding this comment.
Pull request overview
Adds a proposal set of GitHub Copilot “agentic artifacts” for .NET MAUI: a MAUI expert reviewer agent, a routing skill to activate review dimensions based on changed paths/platforms, and scoped instruction files for common MAUI hotspots (handlers/layout/perf/public API/threading + CollectionView per platform).
Changes:
- Added
maui-expert-revieweragent with a dimension-based review workflow and CHECK items. - Added
expert-reviewrouting skill with a folder→dimension table and platform detection rules. - Added 8 folder-scoped instruction files for CollectionView (Android/iOS/Windows) and general MAUI review areas.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/agents/maui-expert-reviewer.md | New “expert reviewer” agent definition and workflow across 21 MAUI review dimensions. |
| .github/skills/expert-review/SKILL.md | New dimension router skill (path routing table + platform detection + selection algorithm). |
| .github/instructions/collectionview-android.instructions.md | Android CollectionView (RecyclerView) review guidance. |
| .github/instructions/collectionview-ios.instructions.md | iOS/MacCatalyst CollectionView (UICollectionView/Items2) review guidance + deprecation notes. |
| .github/instructions/collectionview-windows.instructions.md | Windows CollectionView (WinUI) review guidance. |
| .github/instructions/handler-patterns.instructions.md | Handler mapper/lifecycle/null-safety/native-defaults guidance. |
| .github/instructions/layout-system.instructions.md | Layout measure/arrange contract + constraint propagation + perf notes. |
| .github/instructions/performance-hotpaths.instructions.md | Hot-path allocation/perf guidance for layout/scroll/binding paths. |
| .github/instructions/public-api.instructions.md | Public API guidance + PublicAPI.Unshipped workflow pointer. |
| .github/instructions/threading-async.instructions.md | Threading/async/UI-thread dispatch and race-condition guidance. |
Code Review — PR #35062 (Updated)Scope: Docs/agent/skill/instruction changes only — no Overall AssessmentThe data in this PR is genuinely valuable — ~57 of the 97 CHECK items are novel content not found anywhere in the existing review infrastructure. The extraction surfaced institutional knowledge that was previously only in PR comment threads (native defaults preservation, CollectionView per-platform specifics, trimming/AOT patterns, type choice guidance). The routing table concept — mapping file paths to review focus areas — is the single most innovative idea in the PR. However, the architecture creates a parallel review system alongside the existing Recommendation: Restructure, Don't Discard✅ KEEP all 8 instruction files (with fixes)These follow the repo's existing pattern exactly ( The 3 CollectionView per-platform files are especially valuable and should be kept as separate
These are scoped to their respective handler directories and will grow as native platform guidance is added over time. The remaining 5 general instruction files:
Fix these issues before merging:
✅ KEEP the routing table — merge into
|
| Novel content | Where it goes in review-rules.md |
|---|---|
| Native Platform Defaults (Dim 14) — capture before override, restore on clear | New §: "Native Defaults Preservation" |
| CollectionView Android specifics — range notifications, ViewHolder recycling | Expand existing §10 or add §10a |
| CollectionView iOS specifics — cell measurement scoping, static callbacks | Expand existing §10 or add §10b |
Trimming/AOT (Dim 18) — Type.GetType() restrictions, DynamicallyAccessedMembers |
New §: "Trimming & AOT" |
| Type Choice (Dim 17) — struct boxing, record vs struct | New §: "Type Choice" |
| Complexity Reduction (Dim 16) — don't reimplement existing infra | New §: "Complexity" |
| Logic Verification — "verify against original repro, not just new test" | Add to existing §21 |
| Architectural Layer — compatibility shim isolation, cross-cutting via IView | Add to existing §5 or new § |
| "Concrete scenario" requirement — discard findings without one | Add to code-review workflow |
⚠️ Recommend not adding the agent (maui-expert-reviewer)
The 334-line agent definition substantially overlaps with review-rules.md. Its 4-wave workflow restructures code-review's existing workflow without being fundamentally different. Having it in .github/agents/ creates a parallel invocation path that bypasses the pr-review → code-review pipeline. The agent also contradicts itself internally — "Wave 3" instructs posting ≤15 inline PR comments, while a later "Operational Notes" line says "terminal only, do NOT post PR comments."
The novel content survives in review-rules.md. The routing survives in code-review/references/routing-table.md. The instruction files provide platform-scoped context automatically.
⚠️ Recommend not adding the routing skill (expert-review/)
Its routing table moves into code-review/references/routing-table.md. The skill name mismatch (expert-reviewing in frontmatter vs expert-review/ folder) is one symptom — the deeper issue is that this functionality belongs inside code-review, not alongside it.
Summary
| Component | Action | Rationale |
|---|---|---|
| 3 CollectionView per-platform files | ✅ Keep (with applyTo fixes) |
Per-platform handler guidance, will grow over time |
| 5 general instruction files | ✅ Keep (with applyTo fixes) |
Follows existing repo pattern, auto-scoped |
| Routing table | ✅ Keep → move to code-review/references/ |
Best idea in the PR, wrong container |
| ~57 novel CHECK items | ✅ Keep → merge into review-rules.md |
Valuable institutional knowledge |
| "Concrete scenario" rule | ✅ Keep → add to code-review workflow |
High-signal improvement |
maui-expert-reviewer agent |
Overlaps existing system, internal contradictions, bypasses pipeline | |
expert-review/ skill |
Routing table moves to code-review |
The PR's data is excellent work — the novel content is a strong hit rate from the extraction pipeline. The restructuring above preserves 100% of the new knowledge while avoiding a parallel review system.
🔍 Skill Validation Results❌ Static Checks FailedSkills checked: 15 | Agents checked: 0 Full validator output⏭️ LLM Evaluation: Skipped
|
484b9bc to
4349b52
Compare
4349b52 to
59b75b9
Compare
59b75b9 to
3fb1f02
Compare
…indings Adds a MAUI expert review agent trained on 28,180 review comments from 5 core maintainers, plus 8 folder-scoped instruction files and inline review posting. ## What's added - .github/agents/maui-expert-reviewer.md (665 lines) 30 review dimensions, 200+ CHECK items, per-dimension sub-agent workflow. Writes inline-findings.json for file:line PR review comments. Dual mode: posts directly when local, writes JSON when in CI. - .github/instructions/ (8 new files, 261 lines total) CollectionView Android/iOS/Windows (platform-isolated), handler patterns, layout system, performance hot paths, public API, threading/async. - .github/scripts/post-inline-review.ps1 (185 lines) Posts inline-findings.json as GitHub PR review with file:line comments. ## What's changed - .github/skills/code-review/SKILL.md — delegates to expert reviewer agent - .github/skills/code-review/references/review-rules.md — deleted (absorbed into agent) - .github/skills/code-review/tests/eval.yaml — updated assertions - .github/scripts/Review-PR.ps1 — wired inline findings posting in Step 3 - eng/pipelines/ci-copilot.yml — added COMMENTS_VIA_FILE env var ## Architecture Dual posting mode controlled by COMMENTS_VIA_FILE env var: - CI (set by ci-copilot.yml): agent writes JSON, post-inline-review.ps1 posts - Local (@maui-expert-reviewer): agent posts directly via gh api Two PATs in CI: COPILOT_TOKEN for LLM, GH_COMMENT_TOKEN for GitHub API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
53fb77f to
f1e1bf7
Compare
try-fix skill now self-reviews its changes against expert reviewer dimensions and fixes violations before reporting back. Applied in try-fix/SKILL.md so it works regardless of who invokes try-fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
f1e1bf7 to
baa4792
Compare
|
Closed in favour of #35198 |
> [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Description Replaces `review-rules.md` (flat 345-line checklist) with a dimensional expert review agent. Single source of truth for all review rules, organized into 30 dimensions for per-dimension sub-agent evaluation. Adds inline file:line PR comments alongside the existing wall-of-text summary. Extracted from 28k review comments across 5 maintainers via [extraction-pipeline](https://github.com/dotnet/fsharp/blob/main/.github/agents/extraction-pipeline.md). No functional code changes. Recreated from #35062 on a dotnet/maui branch (originally opened from a fork). ## What changed **Before:** `review-rules.md` had 345 lines of flat rules. `code-review` skill loaded them all into one context. Output was a single wall-of-text PR comment. **After:** Rules absorbed into `maui-expert-reviewer.md` as 30 dimensions with 200+ CHECK items. Each dimension runs as an independent sub-agent with focused context. Output is inline file:line PR comments via `inline-findings.json`. ## CI Flow ``` Review-PR.ps1 prompt: 1. code-review → maui-expert-reviewer agent → inline-findings.json 2. pr-review → Pre-Flight → Try-Fix → Report (sees findings, no duplication) Posting: post-inline-review.ps1 → .json → GitHub file:line comments (NEW) post-ai-summary-comment.ps1 → {phase}/content.md → wall-of-text (existing) CI: COMMENTS_VIA_FILE=true → agent writes .json, script posts Local: agent writes .json, code-review posts directly via gh api ``` ## Files | Action | File | What | |--------|------|------| | **Add** | `agents/maui-expert-reviewer.md` | 30 dimensions, 200+ CHECKs, routing table | | **Add** | `instructions/collectionview-{android,ios,windows}` | Platform-isolated CV rules | | **Add** | `instructions/{handler-patterns,layout-system,performance-hotpaths,public-api,threading-async}` | Domain-specific ambient guidance | | **Add** | `scripts/post-inline-review.ps1` | Posts .json as GitHub PR review | | **Del** | `skills/code-review/references/review-rules.md` | Absorbed into agent | | **Mod** | `skills/code-review/SKILL.md` | Delegates to agent | | **Mod** | `scripts/Review-PR.ps1` | Prompt + inline posting wiring | | **Mod** | `eng/pipelines/ci-copilot.yml` | `COMMENTS_VIA_FILE` env var | --------- Co-authored-by: kubaflo <kubaflo@users.noreply.github.com> Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
> [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Description Replaces `review-rules.md` (flat 345-line checklist) with a dimensional expert review agent. Single source of truth for all review rules, organized into 30 dimensions for per-dimension sub-agent evaluation. Adds inline file:line PR comments alongside the existing wall-of-text summary. Extracted from 28k review comments across 5 maintainers via [extraction-pipeline](https://github.com/dotnet/fsharp/blob/main/.github/agents/extraction-pipeline.md). No functional code changes. Recreated from dotnet#35062 on a dotnet/maui branch (originally opened from a fork). ## What changed **Before:** `review-rules.md` had 345 lines of flat rules. `code-review` skill loaded them all into one context. Output was a single wall-of-text PR comment. **After:** Rules absorbed into `maui-expert-reviewer.md` as 30 dimensions with 200+ CHECK items. Each dimension runs as an independent sub-agent with focused context. Output is inline file:line PR comments via `inline-findings.json`. ## CI Flow ``` Review-PR.ps1 prompt: 1. code-review → maui-expert-reviewer agent → inline-findings.json 2. pr-review → Pre-Flight → Try-Fix → Report (sees findings, no duplication) Posting: post-inline-review.ps1 → .json → GitHub file:line comments (NEW) post-ai-summary-comment.ps1 → {phase}/content.md → wall-of-text (existing) CI: COMMENTS_VIA_FILE=true → agent writes .json, script posts Local: agent writes .json, code-review posts directly via gh api ``` ## Files | Action | File | What | |--------|------|------| | **Add** | `agents/maui-expert-reviewer.md` | 30 dimensions, 200+ CHECKs, routing table | | **Add** | `instructions/collectionview-{android,ios,windows}` | Platform-isolated CV rules | | **Add** | `instructions/{handler-patterns,layout-system,performance-hotpaths,public-api,threading-async}` | Domain-specific ambient guidance | | **Add** | `scripts/post-inline-review.ps1` | Posts .json as GitHub PR review | | **Del** | `skills/code-review/references/review-rules.md` | Absorbed into agent | | **Mod** | `skills/code-review/SKILL.md` | Delegates to agent | | **Mod** | `scripts/Review-PR.ps1` | Prompt + inline posting wiring | | **Mod** | `eng/pipelines/ci-copilot.yml` | `COMMENTS_VIA_FILE` env var | --------- Co-authored-by: kubaflo <kubaflo@users.noreply.github.com> Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
Replaces
review-rules.md(flat 345-line checklist) with a dimensional expert review agent. Single source of truth for all review rules, organized into 30 dimensions for per-dimension sub-agent evaluation. Adds inline file:line PR comments alongside the existing wall-of-text summary.Extracted from 28k review comments across 5 maintainers via extraction-pipeline. No functional code changes.
What changed
Before:
review-rules.mdhad 345 lines of flat rules.code-reviewskill loaded them all into one context. Output was a single wall-of-text PR comment.After: Rules absorbed into
maui-expert-reviewer.mdas 30 dimensions with 200+ CHECK items. Each dimension runs as an independent sub-agent with focused context. Output is inline file:line PR comments viainline-findings.json.CI Flow
Files
agents/maui-expert-reviewer.mdinstructions/collectionview-{android,ios,windows}instructions/{handler-patterns,layout-system,performance-hotpaths,public-api,threading-async}scripts/post-inline-review.ps1skills/code-review/references/review-rules.mdskills/code-review/SKILL.mdscripts/Review-PR.ps1eng/pipelines/ci-copilot.ymlCOMMENTS_VIA_FILEenv var