vsadov/satori/code-review

dotnet/runtime Code Review

Review code changes against conventions and patterns established by dotnet/runtime maintainers. These rules were extracted from 43,000+ maintainer review comments across 6,600+ PRs and represent the actual standards enforced in practice.

Reviewer mindset: Be polite but very skeptical. Your job is to help speed the review process for maintainers, which includes not only finding problems the PR author may have missed but also questioning the value of the PR in its entirety. Treat the PR description and linked issues as claims to verify, not facts to accept. Question the stated direction, probe edge cases, and don't hesitate to flag concerns even when unsure.

When to Use This Skill

Use this skill when:

• Reviewing a PR or code change in dotnet/runtime
• Checking code for correctness, performance, style, or consistency issues before submitting a PR
• Asked to review, critique, or provide feedback on code changes
• Validating that a change follows dotnet/runtime conventions

Review Process

Step 0: Load Relevant Instructions

Before analyzing anything, load .github/skills/code-review/pr-assessment.md -- the holistic criteria you will assess against -- plus any and all instructions under .github/instructions that are relevant to the code changes, as indicated by the frontmatter. If conflict arises between said custom instructions and the instructions in this skill, the custom instructions supersede instructions in this skill.

Step 1: Gather Code Context (No PR Narrative Yet)

Before analyzing anything, collect as much relevant code context as you can. Critically, do NOT read the PR description, linked issues, or existing review comments yet. You must form your own independent assessment of what the code does, why it might be needed, what problems it has, and whether the approach is sound — before being exposed to the author's framing. Reading the author's narrative first anchors your judgment and makes you less likely to find real problems.

1. Diff and file list: Fetch the full diff and the list of changed files.
2. Full source files: For every changed file, read well beyond the diff hunks — diff-only review is the #1 cause of false positives and missed issues. You need the surrounding code to understand invariants, locking protocols, call patterns, and data flow. Read the whole file when it is a reasonable size; for the very large ones this repo has (morph.cpp alone is ~137k tokens) read the enclosing functions, their callers, and the types involved instead of paging through the entire file.
3. Consumers and callers: If the change modifies a public/internal API, a type that others depend on, or a virtual/interface method, search for how consumers use the functionality. Grep for callers, usages, and test sites. Understanding how the code is consumed reveals whether the change could break existing behavior or violate caller assumptions.
4. Sibling types and related code: If the change fixes a bug or adds a pattern in one type, check whether sibling types (e.g., other abstraction implementations, other collection types, platform-specific variants) have the same issue or need the same fix. Fetch and read those files too.
5. Key utility/helper files: If the diff calls into shared utilities, read those to understand the contracts (thread-safety, idempotency, etc.).
6. Git history: Check recent commits to the changed files (git log --oneline -20 -- <file>). Look for related recent changes, reverts, or prior attempts to fix the same problem. This reveals whether the area is actively churning, whether a similar fix was tried and reverted, or whether the current change conflicts with recent work.
7. Detect new public API surface: Check whether the PR introduces new public API surface. Look for:

• Changes to ref/ assembly source files (the strongest signal — these define the public API contract)
• New public members (methods, properties, types, enum values) in src/ files
• Note whether new public API was detected. If it was, you MUST load and execute the API approval verification procedure during Step 4. Read the file .github/skills/code-review/api-approval-check.md (relative to the repository root) and follow its instructions. Do not skip this step — it is blocking.

Step 2: Discover Area-Specific Agents

• If the environment supports sub-agent or task invocation, study the review agents that

actually exist in .github/agents. Their yaml frontmatter descriptions tell when they apply.

• Invoke only existing, matching area-specific review agents as subtasks during the subsequent

steps, integrating their results. Do not infer or invent an agent from an instruction file.

• If the environment lacks sub-agent tooling or no matching agent exists, continue the review

yourself. Area agents are additions to, not replacements for, the regular review.

• Some specialist skills are triggered by diff content, not path. In particular, if the diff

contains Vector64/Vector128/Vector256/Vector512, Vector<T>, or any System.Runtime.Intrinsics.* namespace usage, also apply the vectorization skill's review checklist (correctness vs. the scalar contract, remainder handling, memory safety, cross-platform consistency, and BoundedMemory test coverage). This holds regardless of which folder the change lives in.

Step 3: Form an Independent Assessment

Based only on the code context gathered above (without the PR description or issue), answer these questions:

1. What does this change actually do? Describe the behavioral change in your own words by reading the diff and surrounding code. What was the old behavior? What is the new behavior?
2. Why might this change be needed? Infer the motivation from the code itself. What bug, gap, or improvement does it appear to address?
3. Is this the right approach? Would a simpler alternative be more consistent with the codebase? Could the goal be achieved with existing functionality? Are there correctness, performance, or safety concerns?
4. What problems do you see? Identify bugs, edge cases, missing validation, thread-safety issues, performance regressions, API design problems, test gaps, and anything else that concerns you.

Write down your independent assessment before proceeding. You must produce a holistic assessment (using the criteria in .github/skills/code-review/pr-assessment.md) at this stage.

Step 4: Incorporate PR Narrative and Reconcile

Now read the PR description, labels, linked issues (in full), author information, existing review comments, and any related open issues in the same area. Treat all of this as claims to verify, not facts to accept.

1. PR metadata: Fetch the PR description, labels, linked issues, and author. Read linked issues in full — they often contain the repro, root cause analysis, and constraints the fix must satisfy.
2. Related issues: Search for other open issues in the same area (same labels, same component). This can reveal known problems the PR should also address, or constraints the author may not be aware of.
3. Existing review comments: Check if there are already review comments on the PR to avoid duplicating feedback.
4. Reconcile your assessment with the author's claims. Where your independent reading of the code disagrees with the PR description or issue, investigate further — but do not simply defer to the author's framing. If the PR claims a bug fix, a performance improvement, or a behavioral correction, verify those claims against the code and any provided evidence. If your independent assessment found problems the PR narrative doesn't acknowledge, those problems are more likely to be real, not less.
5. Update your holistic assessment if the additional context reveals information that genuinely changes your evaluation (e.g., a linked issue proves the bug is real, or an existing review comment already identified the same concern). But do not soften findings just because the PR description sounds reasonable.
6. API Approval Verification. If Step 1 detected new public API surface, you MUST now load the file .github/skills/code-review/api-approval-check.md (relative to the repository root) and execute the full procedure described there. Use the view tool, cat, or equivalent to read the file contents into your context, then follow every step. This is a blocking gate — if any check in that procedure fails, the review verdict must be ❌ Reject or ❌ Needs Changes regardless of other findings. Do not proceed without completing this step when new public API is detected. If the file cannot be loaded for any reason, report ❌ error — "Unable to load API approval verification procedure; cannot verify new public API surface" — and set the verdict to ❌ Needs Changes.

Step 5: Detailed Analysis