From 0b4cc5ff5bc6dcfaee125fba32ddbd9bfd32b011 Mon Sep 17 00:00:00 2001 From: Jirka Borovec <6035284+Borda@users.noreply.github.com> Date: Fri, 13 Feb 2026 23:14:02 +0900 Subject: [PATCH] Refine Copilot and CONTRIBUTING guidelines for clarity and conciseness (#2150) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Refactor Copilot and CONTRIBUTING guidelines for clarity and conciseness * Streamline and condense PR review guidelines in CONTRIBUTING.md for improved clarity and usability. * Apply suggestions from code review * fix(pre_commit): 🎨 auto format pre-commit hooks --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/CONTRIBUTING.md | 94 ++++++++ .github/copilot-instructions.md | 387 ++++++++------------------------ 2 files changed, 186 insertions(+), 295 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 8e24558f..78a7e0e8 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -286,6 +286,100 @@ To run tests with coverage: uv run pytest --cov=supervision ``` +## πŸ” PR Review Guidelines + +These guidelines help reviewers provide consistent, actionable feedback efficiently. Your goals: validate completeness, identify risks, provide actionable feedback, and highlight quality gaps. + +### Overall Recommendation + +Start with a clear recommendation using these levels: + +- 🟒 **Approve** β€” Ready to merge +- 🟑 **Minor Suggestions** β€” Improvements recommended but not blocking +- 🟠 **Request Changes** β€” Must address issues before merge +- πŸ”΄ **Block** β€” Critical issues require major rework + +Example: `🟠 Request Changes β€” Missing unit tests for PolygonMerger and no mkdocs entry.` + +### PR Completeness + +Verify requirements are met (βœ… Complete / ⚠️ Incomplete / ❌ Missing / πŸ”΅ N/A): + +- [ ] Clear description of what changed and why +- [ ] Tests added/updated for new functionality or bug fixes +- [ ] Docstrings follow [Google-style](https://google.github.io/styleguide/pyguide.html#383-functions-and-methods) +- [ ] Docs entry added to mkdocs (new functions/classes only) +- [ ] Google Colab provided (if demonstrating feature/fix) +- [ ] Screenshots/videos included (visual changes only) + +Call out missing items explicitly in your review. + +### Quality Scores + +Use **n/5 scoring** with inline code comments for specifics: + +**Code Quality (n/5):** + +- 5/5 🟒 Excellent β€” 4/5 🟒 Good β€” 3/5 🟑 Acceptable β€” 2/5 🟠 Needs Work β€” 1/5 πŸ”΄ Poor +- Check: correctness (edge cases, None checks, bounds), Python best practices (idiomatic patterns, error handling, type hints), project conventions (docstrings, linting, import order, PEP 8 naming) + +**Testing (n/5):** + +- 5/5 🟒 Comprehensive β€” 4/5 🟒 Good β€” 3/5 🟑 Adequate β€” 2/5 🟠 Insufficient β€” 1/5 πŸ”΄ Missing +- Verify: unit tests for new code, edge cases covered, specific assertions, realistic scenarios, clear test names + +**Documentation (n/5):** + +- 5/5 🟒 Excellent β€” 4/5 🟒 Good β€” 3/5 🟑 Adequate β€” 2/5 🟠 Insufficient β€” 1/5 πŸ”΄ Missing +- Confirm: docstrings for public functions/classes, parameters/returns/exceptions documented, usage examples, mkdocs integration, changelog entry for user-facing changes + +### Risk Assessment + +Flag risks with severity (5/5 πŸ”΄ Critical β€” 4/5 🟠 High β€” 3/5 🟑 Medium β€” 2/5 🟒 Low β€” 1/5 🟒 Negligible): + +**Common risk categories:** + +1. **Breaking changes** β€” API changes, removed features, behavior modifications (must include migration guide) +2. **Performance** β€” Inefficient algorithms, memory-intensive operations, bottlenecks +3. **Compatibility** β€” New Python/dependency requirements, platform-specific code +4. **Security** β€” Unvalidated input, code execution risks, data exposure + +### Review Summary Template + +```markdown +## Review Summary + +**Recommendation:** [emoji] [Status] β€” [justification] + +**PR Completeness:** +- βœ… Complete: [items] +- ❌ Missing: [gaps] + +**Quality Scores:** +- Code: n/5 [emoji] β€” [reason] +- Testing: n/5 [emoji] β€” [reason] +- Documentation: n/5 [emoji] β€” [reason] + +**Risk Level:** n/5 [emoji] β€” [description] + +**Critical Issues (Must Fix):** +1. [Issue] β€” See comment on `file.py` + +**Suggestions (Optional):** +1. [Improvement] β€” See suggestion on `file.py` + +**Next Steps:** +1. [Action item] +``` + +### Review Best Practices + +**DO:** Use inline GitHub comments with suggestions, explain *why* (not just *what*), distinguish blocking vs. nice-to-have, acknowledge good work, run linter if needed (`uv run pre-commit run --all-files`) + +**DON'T:** Mention line numbers in summary (use inline comments), give vague feedback, nitpick style (defer to tools), assume knowledge of conventions, block on minor issues + +**Tone:** Be respectful, specific, pragmatic, and consistent. Focus on actionable feedback that moves PRs toward merge. + ## πŸ“„ License By contributing, you agree that your contributions will be licensed under an [MIT license](../LICENSE.md). diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 148d292c..8e24765e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,349 +1,146 @@ -# 🚨 Priority Instruction (System-Level) +# GitHub Copilot Instructions for Supervision -When performing PR reviews, ALWAYS follow the structured format defined in this file. -This instruction overrides all default Copilot review behavior, including the GitHub UI β€œReview this PR” action. +This file provides context-aware guidance for GitHub Copilot when working in the Supervision repository. --- -# GitHub Copilot PR Review Guidelines +## πŸ“š Repository Overview -These guidelines ensure consistent, actionable, and maintainer-focused reviews for the Supervision project. +**Supervision** is a Python library providing reusable computer vision utilities for working with object detection models (YOLO, SAM, etc.). It offers tools for detections processing, tracking, annotation, and dataset management. + +- **Languages**: Python 3.9+ +- **Key Dependencies**: NumPy, OpenCV, SciPy +- **License**: MIT --- -## 🎯 Review Objectives - -Your primary goal is to help maintainers make informed merge decisions quickly by: - -1. **Validating PR completeness** against project requirements -2. **Identifying risks** that could impact users or maintainability -3. **Providing actionable feedback** the author can immediately act upon -4. **Highlighting quality gaps** in code, tests, or documentation - ---- - -## 🟒 1. Overall Recommendation - -**Start with a clear, actionable recommendation:** - -Choose one and provide a **specific** justification: - -- 🟒 **Approve** β€” Ready to merge as-is -- 🟑 **Minor Suggestions** β€” Minor improvements recommended but not blocking -- 🟠 **Request Changes** β€” Significant issues must be addressed before merge -- πŸ”΄ **Block** β€” Critical issues require major rework - -**Example:** +## πŸ—οΈ Project Structure ``` -🟠 Request Changes β€” Missing unit tests for new `PolygonMerger` class and no documentation entry added for autogeneration. +supervision/ +β”œβ”€β”€ src/ +β”‚ └── supervision/ # Main library code +β”‚ β”œβ”€β”€ detection/ # Detection utilities +β”‚ β”œβ”€β”€ draw/ # Annotation and visualization +β”‚ β”œβ”€β”€ tracker/ # Object tracking +β”‚ β”œβ”€β”€ dataset/ # Dataset management +β”‚ └── utils/ # Shared utilities +β”œβ”€β”€ tests/ # Test suite (mirrors src/supervision/) +β”œβ”€β”€ docs/ # MkDocs documentation +└── examples/ # Usage examples ``` --- -## πŸ“‹ 2. PR Completeness Check +## πŸ”§ Development Commands -Verify the PR meets project requirements. Mark each item: +**Setup:** -- βœ… **Complete** β€” Properly addressed -- ⚠️ **Incomplete** β€” Partially done, needs improvement -- ❌ **Missing** β€” Not provided -- πŸ”΅ **N/A** β€” Not applicable to this PR - -### Required Items - -- [ ] **Clear description** β€” What changed and why -- [ ] **Type of change** β€” Bug fix, feature, docs, etc. -- [ ] **Motivation/context** β€” Problem being solved (links to issue if relevant) -- [ ] **Changes list** β€” Summary of modifications -- [ ] **Tests** β€” Unit tests added/updated -- [ ] **Documentation** β€” Docstrings follow [Google-style](https://google.github.io/styleguide/pyguide.html#383-functions-and-methods) -- [ ] **Docs entry** β€” Added to mkdocs for autogeneration (new functions/classes only) -- [ ] **Google Colab** β€” Provided for demonstrating feature/fix (if applicable) -- [ ] **Screenshots/videos** β€” Included for visual changes (if applicable) - -**Call out missing items explicitly:** +```bash +# Install dependencies +uv sync --group dev --group docs --extra metrics +# Install pre-commit hooks +uv run pre-commit install ``` -❌ Missing: -- Documentation entry not added to mkdocs navigation -- No unit tests provided for `merge_polygons()` function + +**Quality Checks:** + +```bash +# Run all pre-commit hooks (formatting, linting, type checking) +uv run pre-commit run --all-files + +# Run tests with coverage +uv run pytest --cov=supervision +``` + +**Documentation:** + +```bash +# Serve docs locally at http://127.0.0.1:8000 +uv run mkdocs serve ``` --- -## πŸ“Š 3. Quality Assessment +## πŸ’» Code Conventions -### 3.1 Code Quality +### General Guidelines -Provide **specific feedback using inline comments** on the changed code. Use **n/5** scoring for quick assessment: +- Follow **[AGENTS.md](../AGENTS.md)** for task-based development workflows +- Reference **[CONTRIBUTING.md](CONTRIBUTING.md)** for detailed contribution guidelines +- All code must pass `pre-commit` hooks before committing -- **5/5** 🟒 Excellent β€” Well-structured, idiomatic, no issues -- **4/5** 🟒 Good β€” Minor improvements possible -- **3/5** 🟑 Acceptable β€” Some issues to address -- **2/5** 🟠 Needs Work β€” Multiple problems -- **1/5** πŸ”΄ Poor β€” Significant refactoring required +### Code Style -**Score: n/5** β€” [Brief justification] +- **Formatting**: Enforced by `ruff-format`, `prettier` (pre-commit) +- **Linting**: Enforced by `ruff-check` (pre-commit) +- **Type Hints**: Required on all new code +- **Docstrings**: Required using [Google Python style](https://google.github.io/styleguide/pyguide.html#383-functions-and-methods) + - Must include usage examples with primitive values + - Serve as runnable documentation -#### Check for: +### Performance -1. **Correctness** +- Avoid unnecessary NumPy array copies +- Prefer vectorized operations over Python loops +- Use OpenCV operations efficiently - - Logic errors or edge cases not handled - - Potential bugs (None checks, array bounds, division by zero) - - Incorrect assumptions +### API Design -2. **Python Best Practices** - - - Non-idiomatic patterns - - Improper exception handling - - Inefficient implementations - - Missing or incorrect type hints - -3. **Project Conventions** - - - **Docstrings:** Must follow [Google-style](https://google.github.io/styleguide/pyguide.html#383-functions-and-methods) - - **Code style:** Must pass linting (run `pre-commit run --all-files`) - - **Imports:** Standard library β†’ third-party β†’ local - - **Naming:** Clear, descriptive, follows PEP 8 - -**Place inline comments directly on problematic code**, then reference them in your summary. Example: - -```markdown -See inline comments in `detection/core.py` for: -- Null safety issue in mask processing (see inline comment on mask handling) -- Performance concern with nested loops (see inline comment on nested loops) -``` - -### 3.2 Testing Quality - -Use **n/5** scoring for test coverage and quality: - -- **5/5** 🟒 Comprehensive β€” All cases covered, high-quality assertions -- **4/5** 🟒 Good β€” Most cases covered -- **3/5** 🟑 Adequate β€” Basic coverage, some gaps -- **2/5** 🟠 Insufficient β€” Major gaps -- **1/5** πŸ”΄ Missing β€” No tests or tests don't validate functionality - -**Score: n/5** β€” [Brief justification] - -#### For New Features or Bug Fixes: - -1. **Coverage Requirements** - - - [ ] Unit tests added for new functions/classes - - [ ] Edge cases covered (empty inputs, None, large arrays, boundary conditions) - - [ ] Regression tests for bug fixes - -2. **Test Quality** - - - [ ] Assertions are specific (not just "no exception raised") - - [ ] Tests use realistic scenarios - - [ ] Test names clearly describe what they validate - -**If tests are inadequate, comment on test files and reference:** - -``` -2/5 🟠 Insufficient Testing - See comments in `test/detection/test_zone.py` -``` - -### 3.3 Documentation Quality - -Use **n/5** scoring for documentation completeness: - -- **5/5** 🟒 Excellent β€” Complete, clear, with good examples -- **4/5** 🟒 Good β€” Minor improvements possible -- **3/5** 🟑 Adequate β€” Basic docs present -- **2/5** 🟠 Insufficient β€” Incomplete or unclear -- **1/5** πŸ”΄ Missing β€” No documentation - -**Score: n/5** β€” [Brief justification] - -#### For New Features: - -1. **Docstring Requirements** - - - [ ] Docstrings for all public functions/classes - - [ ] Parameters, return values, and exceptions documented - - [ ] Usage examples in docstrings - -2. **Documentation Integration** - - - [ ] Entry added to appropriate docs page (e.g., `docs/detection/tools/*.md`) - - [ ] Added to mkdocs navigation (`mkdocs.yml`) - - [ ] Changelog entry (`docs/changelog.md`) for user-facing changes - -#### For Changes to Existing Features: - -1. **Update Requirements** - - [ ] Docstrings updated to reflect changes - - [ ] Deprecated features marked with warnings - - [ ] Migration guide for breaking changes - -**Comment on docstrings directly in code, then reference in summary.** +- Follow existing naming patterns for consistency +- Maintain backward compatibility unless explicitly breaking +- Prefer functional utilities over complex classes --- -## ⚠️ 4. Risk Assessment +## πŸ§ͺ Testing Requirements -**Explicitly flag any risks with severity:** +All new features must include: -- **5/5** πŸ”΄ Critical β€” Blocks release, must fix -- **4/5** 🟠 High β€” Serious concern, should fix -- **3/5** 🟑 Medium β€” Notable risk, consider fixing -- **2/5** 🟒 Low β€” Minor concern -- **1/5** 🟒 Negligible β€” No real risk - -### Risk Categories: - -1. **Breaking Changes** - - - Changes to public APIs (function signatures, return types) - - Removal of deprecated features - - Changed behavior in existing functionality - - **If breaking:** Must include migration instructions - -2. **Performance Impact** - - - Inefficient algorithms ($O(n^2)$ where $O(n)$ possible) - - Memory-intensive operations on large arrays - - Potential bottlenecks in hot paths - -3. **Compatibility Issues** - - - New Python version requirements - - New dependencies - - Platform-specific code - -4. **Security Concerns** - - - Unvalidated user input - - Potential code execution risks - - Sensitive data exposure - -**Example:** - -``` -Risk Level: 4/5 🟠 High Performance Risk - -Nested loop detected - see inline comment in `zone.py` for vectorization suggestion. -``` +- Unit tests covering happy path and edge cases +- Tests for `None`, empty inputs, large arrays, boundary conditions +- Clear test names describing what they validate +- Proper assertions (not just "no exception raised") --- -## πŸ’‘ 5. Constructive Suggestions +## πŸ“ Documentation Requirements -**Add inline comments to the code using GitHub's review interface**, then provide **suggested changes** using GitHub suggestion format: +For new public functions/classes: -````markdown -```suggestion -if detections is None or detections.mask is None: - return None -return process(detections.mask) -``` -```` - -### Suggestion Categories: - -1. **Code Improvements** - - - Logic simplifications - - Better error handling - - More readable implementations - -2. **Performance Optimizations** - - - NumPy vectorization opportunities - - Caching expensive computations - - Batch processing - -3. **Architecture Improvements** - - - Code reuse opportunities - - Better abstractions - - More maintainable designs - -**Best Practice:** Place suggestions as inline comments on specific code blocks, then summarize in your review. +- Google-style docstrings with parameters, returns, exceptions +- Usage examples in docstrings +- Entry in appropriate `docs/*.md` file +- Reference in `mkdocs.yml` navigation --- -## πŸ“Š 6. Review Summary Template +## πŸ” Pull Request Reviews -Use this structure for your final review comment: +**When reviewing PRs, follow the comprehensive [PR Review Guidelines](CONTRIBUTING.md#pr-review-guidelines).** -```markdown -## Review Summary +Quick checklist: -### Recommendation -[emoji] [Status] β€” [One-sentence justification] - -### PR Completeness -- βœ… Complete: [list key items] -- ❌ Missing: [list critical gaps] - -### Quality Scores -- **Code Quality:** n/5 [emoji] β€” [brief reason] -- **Testing:** n/5 [emoji] β€” [brief reason] -- **Documentation:** n/5 [emoji] β€” [brief reason] - -### Risk Level: n/5 [emoji] -[Brief risk description with reference to inline comments if applicable] - -### Critical Issues (Must Fix) -1. [Issue description] β€” See comment on `file.py` -2. [Another blocking issue] β€” See comment on `test_file.py` - -### Suggestions (Nice to Have) -1. [Improvement idea] β€” See suggestion on `file.py` -2. [Another optional enhancement] - -### Next Steps for Author -1. [Clear action item] -2. [Another clear action item] -``` +- Tests included and passing +- Docstrings follow Google style with examples +- Pre-commit hooks pass +- Breaking changes documented +- Score code quality, testing, docs (n/5 scale) +- Use inline comments + GitHub suggestion format --- -## 🎯 Best Practices for Effective Reviews +## 🌿 Branching & Commits -### DO: - -1. βœ… **Use n/5 scoring** for quick assessment of quality dimensions -2. βœ… **Place comments directly on code** using GitHub's inline comment feature -3. βœ… **Use GitHub suggestion format** for code changes when possible -4. βœ… **Reference inline comments** in your summary (e.g., "See comment on `file.py:function()`") -5. βœ… **Explain *why*** something is a problem, not just *what* is wrong -6. βœ… **Distinguish** between blocking issues and nice-to-haves -7. βœ… **Acknowledge** good work and clever solutions -8. βœ… **Run linter** locally if needed: `uv run pre-commit run --all-files` - -### DON'T: - -1. ❌ **Don't mention line numbers** in summary β€” place comments inline instead -2. ❌ **Don't give vague feedback** like "improve code quality" -3. ❌ **Don't nitpick** on personal style preferences (defer to automated tools) -4. ❌ **Don't assume** the author knows project conventions -5. ❌ **Don't focus only on problems** β€” recognize what's good -6. ❌ **Don't let perfect** be the enemy of good (minor issues shouldn't block useful PRs) +- Branch from `develop` using prefixes: `feat/`, `fix/`, `docs/`, `refactor/`, `perf/`, `test/`, `chore/` +- Use **conventional commits**: `feat:`, `fix:`, `docs:`, `refactor:`, `perf:`, `test:`, `chore:` +- All PRs target `develop` branch --- -## πŸ“ Workflow +## 🎯 Context-Aware Behavior -1. **Review files** in the PR, placing inline comments on specific issues -2. **Use GitHub suggestions** for concrete code improvements -3. **Draft your summary** using the template above -4. **Reference inline comments** instead of mentioning specific line numbers -5. **Submit review** with clear recommendation and next steps - ---- - -## πŸ—£οΈ Tone and Communication - -- **Be respectful and constructive** β€” Contributors are volunteers -- **Be specific and technical** β€” Help them learn -- **Be pragmatic** β€” Balance ideal vs. practical -- **Be consistent** β€” Follow these guidelines every time - -**Remember:** Your goal is to help maintainers efficiently assess PRs and help contributors improve their work. Focus on **actionable feedback** that moves the PR toward merge. +**For general development tasks**: Follow [AGENTS.md](../AGENTS.md) +**For pull request reviews**: Follow [PR Review Guidelines](CONTRIBUTING.md#pr-review-guidelines) +**For detailed processes**: Consult [CONTRIBUTING.md](CONTRIBUTING.md)