Instruction file imported from Onemind-Services-LLC/netbox-secrets (.github/instructions/*.instructions.md). Copyright stays with the author.

Copilot Code Review Instructions

Purpose

Guide GitHub Copilot to perform comprehensive pull request reviews by comparing the PR branch against the base branch and generating actionable feedback on code quality, functionality, and maintainability.

Review Scope

When reviewing pull requests, analyze:

• Code Changes: Modified, added, and removed files and functions
• API Surface: Public interfaces, endpoints, and type definitions
• Behavioral Changes: Logic modifications, algorithm updates, default value changes
• Test Coverage: New, modified, and removed tests; coverage deltas
• Documentation: README updates, inline comments, changelog entries
• Dependencies: Package additions, upgrades, downgrades, and license changes
• CI/CD: Workflow modifications, pipeline changes
• Configuration: Environment variables, feature flags, runtime config
• Performance: Algorithmic or I/O changes affecting performance
• Security: Input validation, authentication changes, secrets handling, CVE risks
• Deprecations: Removed APIs, deprecated features, migration paths

Review Process

1. Compare PR branch against base branch at file and semantic levels
2. Run configured static analysis and linters
3. Execute test suites and record failures, flaky tests, coverage changes
4. Inspect dependency and CI workflow modifications
5. Generate structured review using the format below

Review Output Format

Structure your review comment with these sections:

Summary

• PR Title: One-line summary of changes
• Scope: High-level areas affected (e.g., authentication, payments, UI)
• Verdict: Overall assessment - "Approved with suggestions", "Changes requested", or "Approved"

Key Changes

Enhancements

List improvements with:

• File(s) affected
• Description of enhancement
• Benefit/impact
• Suggested follow-up (if any)

Example: auth/token.go refactored to use caching; reduces DB calls and improves login latency by ~30ms. Consider adding cache invalidation strategy.

Bugs and Regressions

Document issues with:

• Reproduction steps or failing test name
• Affected files/lines
• Severity (Critical/High/Medium/Low)
• Recommended fix

Example: payments/charge_test.go failing due to outdated mock. Update mock to match new gateway API signature.

New Features

Describe additions with:

• Public API changes
• Usage examples
• Migration notes for consumers

Breaking Changes and Deprecations

List with:

• What changed or was deprecated
• Replacement/migration path
• Timeline (if specified)

Example: api/v1/user endpoint removed email field. Clients should use api/v2/user which includes email in nested profile object. Deprecation effective Q2 2026.

Maintainability and Code Health

Note:

• Code complexity concerns
• Duplication or unclear abstractions
• Missing test coverage
• Technical debt added or reduced
• Suggested refactorings with priority

Example: utils/helpers.go has grown to 400 lines. Recommend splitting into domain-specific modules (auth-helpers, data-helpers) in follow-up PR.

Tests and Coverage

Summarize:

• Test results: pass/fail counts, flaky tests
• Coverage delta (% change)
• Action items for acceptable coverage

Security and Compliance

Flag security-sensitive changes:

• Authentication/authorization modifications
• Input validation gaps
• Secrets or credentials handling
• Dependency CVEs
• License changes

Recommend actions like dependency pinning, vulnerability patches, or security team review.

CI and Deployment

List:

• CI/CD pipeline changes
• Build or deployment impacts
• Required pipeline updates or rollback plans

Documentation

Identify missing or outdated docs:

• README updates needed
• API documentation gaps
• Changelog entries required
• Migration guides for breaking changes

Suggested Labels

Recommend labels: enhancement, bug, breaking-change, security, docs, tests, dependencies

Suggested Reviewers

List appropriate reviewers by expertise: backend team, frontend team, security team, DevOps

Merge Conditions

Must Fix (Blockers):

• Critical issues preventing merge

Should Fix:

• Recommended before merge

Optional:

• Nice-to-have improvements for future PRs

Severity Definitions

• Critical: Production-breaking, security vulnerability, data loss risk
• High: Major regression, failing core tests, breaking API changes
• Medium: Functional bug with workaround, missing tests, performance degradation
• Low: Cosmetic issues, minor performance, documentation-only

Review Best Practices

• Be Specific: Include file paths, line numbers, and code snippets
• Be Actionable: Provide concrete fix suggestions or next steps
• Prioritize: Address blockers first, then high/medium/low severity
• Link Context: Reference tests, docs, or CI logs when relevant
• Stay Constructive: Maintain neutral, helpful tone

Comment Templates

Minor Changes (Approval)

**Summary**: Minor enhancements and documentation updates. All tests pass.
**Verdict**: Approved
**Action**: Safe to merge after squash

Changes Requested

**Summary**: Contains breaking API change and failing tests
**Verdict**: Changes requested

**Must Fix**:
- Restore backward-compatible API or add comprehensive migration guide
- Fix failing test: `payments/charge_test.go`

**Should Fix**:
- Add deprecation warnings to legacy endpoints
- Update API documentation with migration examples

**Suggested Reviewers**: @backend-team @api-platform-team

Security Block

**Summary**: Introduces unvalidated user input in authentication flow
**Verdict**: Blocked

**Critical Issue**: Potential SQL injection in `auth/login.go:45`
**Action Required**:
1. Add parameterized queries or ORM
2. Implement input sanitization
3. Re-run security scan
4. Request security team review

**Blocker**: Cannot merge until security issues resolved

Automation Integration

When available:

• Attach linter output, test logs, and coverage diffs
• Include code snippets for suggested fixes
• Provide exact replacement code for deprecations
• Link to relevant documentation or architectural decisions

Note: Always end your review with a clear next steps list and overall merge readiness assessment.