Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/mattpocock-skills-reviewer.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 14 additions & 3 deletions .github/workflows/mattpocock-skills-reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,20 +182,31 @@ Focus areas by skill:

### Step 5: Post Inline Review Comments

For each issue found, create a review comment using `create-pull-request-review-comment`. Apply **progressive disclosure**: lead with a brief visible statement, then collapse verbose analysis and code examples in a `<details>` block:
For each issue found, create a review comment using `create-pull-request-review-comment`. Apply **progressive disclosure**: lead with a brief visible statement, then use a `suggestion` block when a concrete replacement exists, or a `<details>` block for complex issues.

Example with suggestion (preferred when the fix maps to specific lines):
```json
{
"path": "path/to/file.ts",
"line": 42,
"body": "**[/tdd]** Missing edge case: `value` is `null` — add a test to prevent this regression.\n\n<details>\n<summary>💡 Suggested test</summary>\n\n```ts\nit('returns default when value is null', () => {\n expect(fn(null)).toBe(defaultValue);\n});\n```\n\nMissing edge case tests are a common source of regressions.\n\n</details>\n\n@copilot please address this."
"body": "**[/tdd]** Missing edge case: `value` is `null` — add a test to prevent this regression.\n\n```suggestion\nit('returns default when value is null', () => {\n expect(fn(null)).toBe(defaultValue);\n});\n```\n\nMissing edge case tests are a common source of regressions.\n\n@copilot please address this."
}
```

Example without suggestion (use when the fix requires broader changes):
```json
{
"path": "path/to/file.ts",
"line": 42,
"body": "**[/tdd]** Missing edge case: `value` is `null` — add a test to prevent this regression.\n\n<details>\n<summary>💡 Suggested test</summary>\n\nAdd a test case that passes `null` as `value` and asserts the expected default output.\n\nMissing edge case tests are a common source of regressions.\n\n</details>\n\n@copilot please address this."
}
```

Guidelines:
- Prefix each comment with the skill name in brackets: `**[/diagnosing-bugs]**`, `**[/tdd]**`, etc.
- Keep the **immediately visible text brief** (1–2 sentences): state the issue and its impact
- Wrap code examples, detailed explanations, and multi-step suggestions in `<details><summary>💡 …</summary>` blocks
- Use a `` ```suggestion `` block (preferred) when the fix is a concrete replacement for the commented lines; set `start_line` when spanning multiple lines
- Use a `<details><summary>💡 …</summary>` block when the fix requires broader structural changes not expressible as a direct replacement
- Be specific: file path, line number, exact issue
- Limit to the **10 most impactful** issues
- End each inline comment with `@copilot please address this.` to prompt follow-up action
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pr-code-quality-reviewer.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 22 additions & 17 deletions .github/workflows/pr-code-quality-reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,27 +130,32 @@ You may use compact pseudo-language/encoding during private reasoning (examples:

### Step 4: Write Review Comments

For each significant issue, create a `create-pull-request-review-comment` with:
- **File path and line number** of the issue
- **Immediately visible text**: one brief sentence stating the issue and its impact
- **`<details>` block**: detailed explanation, code snippet fix, and rationale — collapsed by default

Example:
```markdown
**Potential nil dereference**: `user.Profile` is accessed without a nil check and will panic if the user has no profile.

<details>
<summary>💡 Suggested fix</summary>
For each significant issue, call `create-pull-request-review-comment` (safe output) with:
- `path` — repository-relative file path
- `line` — line number in the diff (use `start_line` when the replacement spans multiple lines)
- `body` — review comment body

```go
if user.Profile == nil {
return ErrNoProfile
Body format:
- **Immediately visible text**: one brief sentence stating the issue and its impact
- **GitHub suggestion** (preferred when the fix maps to the commented lines): use a ` ```suggestion ` block so the author can apply it with one click
- **`<details>` block** (when the fix requires broader structural changes): collapsed explanation and rationale

Example with suggestion (preferred):
```json
{
"path": "pkg/user/user.go",
"line": 42,
"body": "**Potential nil dereference**: `user.Profile` is accessed without a nil check and will panic if the user has no profile.\n\n```suggestion\nif user.Profile == nil {\n return ErrNoProfile\n}\nprofile := user.Profile\n```\n\nCallers that pass users without profiles will hit this panic silently."
}
```

Callers that pass users without profiles (e.g., in tests) will hit this panic silently.

</details>
Example without suggestion (use when the fix requires broader changes):
```json
{
"path": "pkg/user/user.go",
"line": 42,
"body": "**Potential nil dereference**: `user.Profile` is accessed without a nil check and will panic if the user has no profile.\n\n<details>\n<summary>💡 Suggested fix</summary>\n\nValidate `user.Profile` before accessing it; the exact change depends on the caller contract.\n\nCallers that pass users without profiles will hit this panic silently.\n\n</details>"
}
```

**Prioritization** (use your 10-comment budget aggressively):
Expand Down