microsoft/hve-core
Publicmirrored from https://github.com/microsoft/hve-coreAvailable
docs/agents/code-review/README.md
245lines · modecode
| 1 | --- |
| 2 | title: Code Review |
| 3 | description: Pre-PR code review agents that catch functional defects and enforce project-defined coding standards through dynamic skill loading |
| 4 | sidebar_position: 1 |
| 5 | sidebar_label: Overview |
| 6 | keywords: |
| 7 | - code review |
| 8 | - pre-PR review |
| 9 | - standards review |
| 10 | - functional review |
| 11 | - coding standards |
| 12 | - skills |
| 13 | tags: |
| 14 | - agents |
| 15 | - code-review |
| 16 | - coding-standards |
| 17 | author: Microsoft |
| 18 | ms.date: 2026-03-28 |
| 19 | ms.topic: concept |
| 20 | estimated_reading_time: 8 |
| 21 | --- |
| 22 | |
| 23 | The code review system provides two complementary review passes that run before you open a pull request. A functional review catches logic errors, edge cases, and error handling gaps. A standards review enforces project-defined coding conventions through dynamically loaded skills. An orchestrator agent combines both into a single merged report. |
| 24 | |
| 25 | > Most review feedback arrives after a PR is already open, when context switching and rework costs are highest. Running these agents on a local branch before pushing catches issues while the code is still fresh. |
| 26 | |
| 27 | ## Why Pre-PR Code Review? |
| 28 | |
| 29 | | Benefit | Description | |
| 30 | |-------------------------------|--------------------------------------------------------------------------------------------| |
| 31 | | Earlier defect detection | Catches functional bugs on the branch, before reviewers spend time on a PR | |
| 32 | | Consistent standards coverage | Every diff gets the same skill-based analysis regardless of which reviewer picks up the PR | |
| 33 | | Extensible language support | Teams add their own skills without modifying the review agents | |
| 34 | | Actionable output | Every finding includes file paths, line numbers, current code, and a suggested fix | |
| 35 | |
| 36 | > [!TIP] |
| 37 | > New to hve-core code review? Start with the [functional review prompt](#functional-review) on your current branch to see the output format, then move to the [full orchestrated review](#full-orchestrated-review) once you are comfortable with the workflow. |
| 38 | |
| 39 | ## Architecture |
| 40 | |
| 41 | ```mermaid |
| 42 | flowchart TD |
| 43 | subgraph Prompts |
| 44 | P1["code-review-functional<br/>.prompt.md"] |
| 45 | P2["code-review-full<br/>.prompt.md"] |
| 46 | end |
| 47 | |
| 48 | subgraph Agents |
| 49 | AF["Code Review<br/>Functional"] |
| 50 | AS["Code Review<br/>Standards"] |
| 51 | AO["Code Review Full<br/>(Orchestrator)"] |
| 52 | end |
| 53 | |
| 54 | subgraph Instructions |
| 55 | D["Diff Computation<br/>Protocol"] |
| 56 | R["Review Artifacts<br/>Protocol"] |
| 57 | end |
| 58 | |
| 59 | subgraph Skills |
| 60 | S1["python-foundational"] |
| 61 | S2["(future language<br/>skills)"] |
| 62 | S3["Enterprise<br/>custom skills"] |
| 63 | end |
| 64 | |
| 65 | subgraph Templates |
| 66 | T1["Standards Output<br/>Format"] |
| 67 | T2["Engineering<br/>Fundamentals"] |
| 68 | end |
| 69 | |
| 70 | P1 -->|"invokes"| AF |
| 71 | P2 -->|"invokes"| AO |
| 72 | AO -->|"Step 2"| AF |
| 73 | AO -->|"Step 3"| AS |
| 74 | AF -->|"follows"| D |
| 75 | AS -->|"follows"| D |
| 76 | AF -->|"follows"| R |
| 77 | AS -->|"follows"| R |
| 78 | AS -->|"loads at runtime"| S1 & S2 & S3 |
| 79 | AS -->|"formats with"| T1 |
| 80 | AS -->|"applies"| T2 |
| 81 | ``` |
| 82 | |
| 83 | The orchestrator computes the diff once and passes it to both subagents. Each subagent produces findings independently, and the orchestrator merges them into a single deduplicated report. |
| 84 | |
| 85 | ## The Three Agents |
| 86 | |
| 87 | ### Code Review Functional |
| 88 | |
| 89 | Analyzes branch diffs for functional correctness across five focus areas: |
| 90 | |
| 91 | | Focus Area | What It Catches | |
| 92 | |----------------|-----------------------------------------------------------------------------------| |
| 93 | | Logic | Incorrect control flow, wrong boolean conditions, off-by-one errors | |
| 94 | | Edge Cases | Unhandled boundaries, missing null checks, empty collection handling | |
| 95 | | Error Handling | Uncaught exceptions, swallowed errors, resource cleanup gaps | |
| 96 | | Concurrency | Race conditions, deadlock potential, shared mutable state without synchronization | |
| 97 | | Contract | API misuse, type mismatches at boundaries, violated preconditions | |
| 98 | |
| 99 | Findings are severity-ordered (Critical, High, Medium, Low) with concrete code fixes. The agent includes false positive mitigation filters to keep noise low. |
| 100 | |
| 101 | ### Code Review Standards |
| 102 | |
| 103 | Enforces project-defined coding standards through dynamically loaded skills. The agent is language-agnostic: it scans the workspace for `**/SKILL.md` files, matches them against the languages in the diff, and loads up to 8 relevant skills per review. |
| 104 | |
| 105 | Skills provide the domain-specific checklists. The standards agent provides the review protocol, output format, and verdict logic. See [Language Skills](language-skills.md) for details on the built-in skills and how to create your own. |
| 106 | |
| 107 | ### Code Review Full (Orchestrator) |
| 108 | |
| 109 | Runs both agents in sequence and produces a merged report: |
| 110 | |
| 111 | 1. Computes the diff once using the shared [diff computation protocol](../../contributing/instructions.md) |
| 112 | 2. Invokes Code Review Functional with the diff |
| 113 | 3. Invokes Code Review Standards with the diff (and optional story reference) |
| 114 | 4. Merges findings, deduplicates overlaps, and applies the stricter verdict |
| 115 | |
| 116 | The merged report includes severity-tagged findings from both sources, a unified changed files table, combined testing recommendations, and acceptance criteria coverage when a story reference is provided. |
| 117 | |
| 118 | ## Usage Flow |
| 119 | |
| 120 | ```mermaid |
| 121 | flowchart LR |
| 122 | A["Make changes<br/>on branch"] --> B{"Which review<br/>do you need?"} |
| 123 | B -->|"Functional only"| C["Run<br/>code-review-functional"] |
| 124 | B -->|"Standards only"| D["Invoke Code Review<br/>Standards agent"] |
| 125 | B -->|"Both"| E["Run<br/>code-review-full"] |
| 126 | C --> F["Review findings"] |
| 127 | D --> F |
| 128 | E --> F |
| 129 | F --> G{"Issues found?"} |
| 130 | G -->|"Yes"| H["Fix issues on branch"] |
| 131 | H --> A |
| 132 | G -->|"No"| I["Open PR"] |
| 133 | ``` |
| 134 | |
| 135 | ### Functional Review |
| 136 | |
| 137 | Run the functional review prompt from the Copilot Chat panel: |
| 138 | |
| 139 | ```text |
| 140 | /code-review-functional |
| 141 | ``` |
| 142 | |
| 143 | Optionally specify a base branch: |
| 144 | |
| 145 | ```text |
| 146 | /code-review-functional baseBranch=origin/develop |
| 147 | ``` |
| 148 | |
| 149 | Defaults to `origin/main` when no base branch is specified. |
| 150 | |
| 151 | ### Standards Review |
| 152 | |
| 153 | The standards review does not have a standalone prompt. Invoke the Code Review Standards agent directly from the Copilot Chat panel and describe what you want reviewed. The agent detects the diff automatically using the diff computation protocol. |
| 154 | |
| 155 | ### Full Orchestrated Review |
| 156 | |
| 157 | Run both reviews in a single pass: |
| 158 | |
| 159 | ```text |
| 160 | /code-review-full |
| 161 | ``` |
| 162 | |
| 163 | Pass a work item reference to enable acceptance criteria coverage: |
| 164 | |
| 165 | ```text |
| 166 | /code-review-full story=AB#456 |
| 167 | ``` |
| 168 | |
| 169 | The orchestrator passes the story reference to the standards subagent, which includes an Acceptance Criteria Coverage table in its report. |
| 170 | |
| 171 | ## Review Output |
| 172 | |
| 173 | Both agents produce severity-ordered findings. Each finding includes: |
| 174 | |
| 175 | * A descriptive title and severity level (Critical, High, Medium, Low) |
| 176 | * The file path and line range where the issue appears |
| 177 | * The current code from the diff that has the issue |
| 178 | * A suggested fix with replacement code |
| 179 | * The category and (for standards findings) the skill that surfaced the finding |
| 180 | |
| 181 | ### Verdict Scale |
| 182 | |
| 183 | | Condition | Verdict | |
| 184 | |-------------------------------|-----------------------| |
| 185 | | Any Critical or High findings | Request changes | |
| 186 | | Only Medium or Low findings | Approve with comments | |
| 187 | | No findings | Approve | |
| 188 | |
| 189 | The orchestrator uses the stricter verdict when merging: if either subagent would request changes, the merged report requests changes. |
| 190 | |
| 191 | ### Artifact Persistence |
| 192 | |
| 193 | Review artifacts are saved to `.copilot-tracking/reviews/code-reviews/{branch-slug}/` with two files: |
| 194 | |
| 195 | * `review.md`: the full review report in the standards output format |
| 196 | * `metadata.json`: a machine-readable summary for automation |
| 197 | |
| 198 | The `metadata.json` file contains fields that CI pipelines, pre-commit hooks, and custom scripts can consume: |
| 199 | |
| 200 | ```json |
| 201 | { |
| 202 | "schema_version": "1", |
| 203 | "branch": "feat/my-feature", |
| 204 | "head_commit": "abc123...", |
| 205 | "reviewed_at": "2026-03-28T15:30:00Z", |
| 206 | "verdict": "request_changes", |
| 207 | "files_changed": ["src/main.py", "src/utils.py"], |
| 208 | "findings_count": { |
| 209 | "critical": 0, |
| 210 | "high": 2, |
| 211 | "medium": 1, |
| 212 | "low": 0 |
| 213 | }, |
| 214 | "reviewer": "code-review-full" |
| 215 | } |
| 216 | ``` |
| 217 | |
| 218 | The `verdict` field holds one of three values: `approve`, `approve_with_comments`, or `request_changes`. A pre-commit hook can read this file and block commits when the verdict is `request_changes`, ensuring review findings are addressed before code leaves the local branch. For example: |
| 219 | |
| 220 | ```bash |
| 221 | verdict=$(jq -r '.verdict' .copilot-tracking/reviews/code-reviews/*/metadata.json 2>/dev/null) |
| 222 | if [ "$verdict" = "request_changes" ]; then |
| 223 | echo "Code review requires changes. Fix findings before committing." |
| 224 | exit 1 |
| 225 | fi |
| 226 | ``` |
| 227 | |
| 228 | ## What You Need |
| 229 | |
| 230 | | Requirement | Details | |
| 231 | |---------------------|---------------------------------------------------------------| |
| 232 | | VS Code + Copilot | GitHub Copilot Chat with agent mode enabled | |
| 233 | | Git branch | A local branch with commits ahead of the base branch | |
| 234 | | hve-core collection | The `coding-standards` or `hve-core-all` collection installed | |
| 235 | |
| 236 | The agents work with any programming language. Standards enforcement requires skills that match the languages in your diff. If no matching skills are found, the standards agent notes the gap and restricts its verdict. |
| 237 | |
| 238 | ## Extending with Custom Skills |
| 239 | |
| 240 | The standards agent discovers skills dynamically at review time. You extend coverage by adding `SKILL.md` files to your repository without modifying the agent itself. See [Language Skills](language-skills.md) for the full guide on built-in skills, skill stacking, and authoring enterprise-specific standards. |
| 241 | |
| 242 | <!-- markdownlint-disable MD036 --> |
| 243 | *🤖 Crafted with precision by ✨Copilot following brilliant human instruction, |
| 244 | then carefully refined by our team of discerning human reviewers.* |
| 245 | <!-- markdownlint-enable MD036 --> |