Improve android-reviewer skill accuracy and update gh-aw#11425
Improve android-reviewer skill accuracy and update gh-aw#11425jonathanpeppers wants to merge 1 commit into
Conversation
- Update gh-aw from v0.68.3 to v0.74.8 and run upgrade codemods - Scope IsNullOrEmpty() extension rule to netstandard2.0 projects only, since modern .NET already has [NotNullWhen] on string.IsNullOrEmpty - Add constraint to verify project context (TargetFramework, references) before applying rules to prevent false positives - Require suggestions to be posted as inline comments, not just summarized in the review body Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the agentic workflow tooling (gh-aw) and refines the android-reviewer skill guidance to reduce false positives by scoping certain C# rules to the appropriate project context and requiring inline (diff-attached) comments.
Changes:
- Upgrade gh-aw tooling and regenerate compiled workflow lock files (plus add the generated maintenance workflow).
- Update android-reviewer guidance to verify project configuration (TargetFramework/references) before applying rules and to post suggestions as inline comments.
- Scope the
IsNullOrEmpty()extension-method rule tonetstandard2.0projects only.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/nightly-fix-finder.md | Reformats/updates gh-aw frontmatter and safe-outputs config for the fix-finder workflow. |
| .github/workflows/nightly-fix-finder.lock.yml | Regenerated lock workflow with gh-aw v0.74.8 and updated runtime/tooling scaffolding. |
| .github/workflows/copilot-setup-steps.yml | Updates setup-cli action reference for gh-aw tooling bootstrap workflow. |
| .github/workflows/android-reviewer.md | Tightens reviewer constraints around project-context verification and inline comments. |
| .github/workflows/android-reviewer.lock.yml | Regenerated lock workflow with gh-aw v0.74.8 and updated runtime/tooling scaffolding. |
| .github/workflows/agentics-maintenance.yml | Adds the gh-aw generated maintenance workflow for expiring entities/cache cleanup/etc. |
| .github/skills/android-reviewer/SKILL.md | Updates reviewer methodology to emphasize inline comments and project-context verification. |
| .github/skills/android-reviewer/references/csharp-rules.md | Scopes IsNullOrEmpty() extension recommendation to netstandard2.0 and adds TargetFramework verification guidance. |
| .github/aw/actions-lock.json | Updates pinned action entries for gh-aw compilation (github-script v9.0.0, setup-cli v0.74.8, setup v0.74.8). |
| .github/agents/agentic-workflows.agent.md | Updates agent dispatcher docs and references to gh-aw v0.74.8 resources/guides. |
| - name: Install gh-aw extension | ||
| uses: github/gh-aw-actions/setup-cli@ba90f2186d7ad780ec640f364005fa24e797b360 # v0.68.3 | ||
| uses: github/gh-aw-actions/setup-cli@efa55847f72aadb03490d955263ff911bf758700 # v0.74.8 | ||
| with: | ||
| version: v0.68.3 | ||
|
|
There was a problem hiding this comment.
The setup-cli action version in actions-lock.json was updated automatically by gh aw upgrade. copilot-setup-steps.yml is the bootstrap workflow and its version is managed independently -- these are not expected to match.
| - 💡 **suggestion** — Consider changing. Style, readability, optional improvements. | ||
|
|
||
| **Every review should produce at least one inline comment.** Even clean PRs have opportunities for improvement — code consolidation, missing edge-case tests, perf micro-optimizations, or documentation gaps. Use 💡 suggestions for these. A review with zero comments appears superficial and misses the chance to share knowledge. Only omit inline comments if the PR is truly trivial (e.g., a 1-line typo fix or dependency bump). | ||
| **Every review should produce at least one inline comment.** Even clean PRs have opportunities for improvement — code consolidation, missing edge-case tests, perf micro-optimizations, or documentation gaps. Use 💡 suggestions for these. A review with zero inline comments appears superficial and misses the chance to share knowledge. Only omit inline comments if the PR is truly trivial (e.g., a 1-line typo fix or dependency bump). **Do NOT summarize suggestions only in the review body — post them as inline comments on the relevant line.** If a suggestion cannot be posted inline (e.g., it's about pre-existing code or missing code), either find the closest relevant changed line to attach it to, or omit it entirely rather than burying it in the summary. |
There was a problem hiding this comment.
This is a pre-existing instruction that this PR did not introduce. Our change actually improves the situation: "if a suggestion cannot be posted inline, omit it entirely rather than burying it in the summary" -- the net effect is fewer bogus comments, not more.
| @@ -1,156 +1,159 @@ | |||
| --- | |||
There was a problem hiding this comment.
gh aw upgrade (v0.68.3 -> v0.74.8) re-serializes YAML frontmatter into a canonical format: alphabetical keys, block-style arrays, stripped unnecessary quotes. This is intentional normalization from the upgrade codemods, not a manual change.
The AI reviewer on PR #11415 produced two false positives: it suggested using the
IsNullOrEmpty()extension method in a .NET 11 console app that doesn't reference the assembly defining it, and it summarized suggestions only in the review body instead of posting inline comments.This PR fixes the root causes and updates the gh-aw tooling.
Changes
Update gh-aw from v0.68.3 to v0.74.8, run
gh aw upgradecodemodsScope
IsNullOrEmpty()rule to netstandard2.0 projects -- theNullableExtensionsclass exists as a workaround for netstandard2.0's BCL lacking[NotNullWhen]onstring.IsNullOrEmpty. In modern .NET (.NET 10+), the static method is already properly annotated, so the extension is unnecessary. The rule now says to verifyTargetFrameworkbefore flagging.Add "verify project context" constraint -- reviewer must check
TargetFramework, references, and available APIs before applying rules. A rule about an extension method is irrelevant if the project doesn't reference the assembly that defines it.Require inline comments, not body-only summaries -- suggestions must be posted as inline comments on changed lines. If a suggestion can't be attached to a changed line, omit it rather than burying it in the summary.
Useful description of why the change is necessary.
Links to issues fixed
Unit tests