From 38143f3686078b86dff53a080e746f21cf0a2ee4 Mon Sep 17 00:00:00 2001 From: Kane610 Date: Fri, 1 May 2026 18:03:31 +0200 Subject: [PATCH] Improve Copilot contributor guidance and templates --- .github/ISSUE_TEMPLATE/copilot-ask.md | 43 +++ .github/agents/axis-review-verify.agent.md | 22 ++ .github/agents/axis-review.agent.md | 20 ++ .github/copilot-instructions.md | 325 ++++++++++++++++++--- .github/pull_request_template.md | 36 +++ COPILOT_QUICK_START.md | 193 ++++++++++++ 6 files changed, 601 insertions(+), 38 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/copilot-ask.md create mode 100644 .github/pull_request_template.md create mode 100644 COPILOT_QUICK_START.md diff --git a/.github/ISSUE_TEMPLATE/copilot-ask.md b/.github/ISSUE_TEMPLATE/copilot-ask.md new file mode 100644 index 00000000..eb87d833 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/copilot-ask.md @@ -0,0 +1,43 @@ +--- +name: Copilot Ask +about: Request a feature, fix, or refactor using Copilot +title: "[COPILOT] " +labels: copilot +--- + +## What should Copilot do? + +[Clear description: implement, fix, refactor, review, etc.] + +## Constraints + +- [ ] Preserve backward compatibility +- [ ] No changes to public API +- [ ] Keep to one file/module +- [ ] Minimal change (preserve existing behavior) +- [ ] Reuse existing fixtures and patterns + +## Acceptance Criteria + +- [ ] All checks pass (`ruff check`, `ruff format`, `mypy`, `pytest`) +- [ ] Coverage maintained ≥95% (or specify affected files) +- [ ] Tests added/updated for changed code +- [ ] Follows conventions: enum `_missing_`, input normalization, XML parsing guards + +## Context + +[Paste relevant code, architecture notes, error traces, or API documentation] + +## Verification Command + +```bash +# Run these after Copilot work: +uv run ruff check . +uv run ruff format --check . +uv run mypy axis +uv run pytest tests/test_.py -v --cov=axis. --cov-report=term-missing +``` + +## Related Issues + +Relates to #[issue number] (if applicable) diff --git a/.github/agents/axis-review-verify.agent.md b/.github/agents/axis-review-verify.agent.md index d2293d38..1ea3c589 100644 --- a/.github/agents/axis-review-verify.agent.md +++ b/.github/agents/axis-review-verify.agent.md @@ -20,6 +20,28 @@ - For code review, use the `Axis Review` agent. **See also:** `.github/copilot-instructions.md`, `CONTRIBUTING.md`. + +## When to Use This Agent + +**Invoke with these keywords:** +- "verify my PR" +- "validate my changes" +- "run checks" +- "is my PR ready for merge?" +- "test and review" +- "@axis-review-verify" + +**Best for:** +- Before pushing to GitHub or opening a PR +- Full validation of code + tests + lint + type checking +- Confirmation that all checks will pass +- Finding failures and explaining what went wrong + +**Do NOT use for:** +- Quick code review only (use Axis Review instead) +- Sweeping repo-wide tests (specify affected test files: `tests/test_.py`) +- General questions or debugging (use default agent) +- Exploratory questions about the codebase --- description: "Use when reviewing Axis changes and you want findings plus command-backed validation. Keywords: review with tests, verify PR, run targeted checks, regression verification, lint/type/test confirmation." name: "Axis Review Verify" diff --git a/.github/agents/axis-review.agent.md b/.github/agents/axis-review.agent.md index 12478d74..867b44bb 100644 --- a/.github/agents/axis-review.agent.md +++ b/.github/agents/axis-review.agent.md @@ -32,6 +32,26 @@ user-invocable: true **See also:** `.github/copilot-instructions.md`, `CONTRIBUTING.md`. +## When to Use This Agent + +**Invoke with these keywords:** +- "review my code" +- "review for regressions" +- "check for bugs" +- "code review" +- "@axis-review" + +**Best for:** +- After writing code but before running full test suite +- Checking enum fallbacks, XML parsing, handler phases +- Identifying test gaps or missing edge cases +- Risk analysis on PR changes + +**Do NOT use for:** +- Running tests or lint checks (use Axis Review Verify for that) +- General questions about the codebase (use default agent) +- "Run pytest" or "verify CI" (specify Axis Review Verify instead) + ## Scope - Review code changes and nearby context for defects and unintended behavior changes. - Prioritize runtime correctness, API/contract compatibility, and edge-case handling. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index cd5be6d9..0e56bba9 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -46,51 +46,300 @@ - If work is currently on `master`, create or switch to a feature branch before committing. - If asked to commit or push from `master`, explain that the change must go through a feature branch and pull request. - For any requested git operation, verify branch state first and summarize what will happen before committing or pushing. -# Project Guidelines -## Build And Test +## How to Ask Copilot for High-Quality Work -- Use the project environment managed by `uv` (see `README.md`). -- Preferred bootstrap: `./setup.sh`. -- Python requirement is `>=3.14.0` (see `pyproject.toml`). -- Standard full checks: - - `uv run ruff check .` - - `uv run ruff format --check .` - - `uv run mypy axis` - - `uv run pytest` -- After code changes, run targeted tests for touched files first; run broader validation when shared behavior is affected. +### Request Template -## Architecture +Use this template when asking Copilot to implement, fix, or refactor. Fill in each section to ensure clear scope and expectations: -- Keep boundaries clear: - - `axis/interfaces/` contains API handlers and transport-facing logic. - - `axis/models/` contains request/response models, enums, and parsing helpers. - - `axis/device.py` and `axis/interfaces/vapix.py` orchestrate device and handler lifecycle. -- Follow the phase-based handler initialization model documented in `README.md` (`API_DISCOVERY`, `PARAM_CGI_FALLBACK`, `APPLICATION`). -- Prefer boundary normalization for incoming values (for example, enum coercion and defaults in model constructors/post-init). +``` +[CONTEXT] +I'm working on [area: new handler / bug fix / test / refactor]. +This touches [files/modules affected]. +Related issue: [link or #number]. -## Conventions +[CONSTRAINTS] +- Preserve existing behavior unless explicitly asked to change +- Follow the phase-based handler initialization model (see README.md) +- New code must have 100% test coverage +- Overall coverage must stay ≥95% +- Reuse fixtures from tests/conftest.py +- If enums are involved, include _missing_ fallback +- Keep changes minimal and targeted (no unrelated refactoring) -- Prefer minimal, targeted changes that preserve existing behavior unless the task explicitly requires a behavior change. -- Do not modify unrelated code, formatting, or tests. -- Never revert user changes unless explicitly asked. -- Before changing patterns or APIs, inspect nearby code and follow existing local style. -- Prefer root-cause fixes over workarounds. -- For enums and external inputs, preserve existing defensive normalization patterns (for example `_missing_` fallbacks and constructor normalization). -- For event/XML handling, preserve namespace-aware parsing and root-shape guards instead of assuming a fixed payload shape. +[TASK] +Please [implement / fix / refactor]: +1. [Specific step 1 with context] +2. [Specific step 2 with context] +3. [Specific step 3 with context] -## Testing Conventions +[ACCEPTANCE CRITERIA] +- ✓ All checks pass: `uv run ruff check .`, `uv run ruff format --check .`, `uv run mypy axis`, `uv run pytest` +- ✓ Tests added in `tests/test_.py` covering new code (100% coverage) +- ✓ No unrelated changes +- ✓ Follows architectural layer boundaries (models vs. interfaces) -- Add or update focused tests in the nearest relevant `tests/` module when behavior changes. -- Reuse existing async fixtures and HTTP mocking patterns from `tests/conftest.py`. -- If tests, typing, or linting fail for unrelated pre-existing reasons, report that clearly instead of fixing unrelated code. -- Expect commit hooks to run Ruff, Ruff format, and mypy; if hooks modify files, re-stage and re-run checks. +[VERIFICATION COMMAND] +uv run ruff check . +uv run ruff format --check . +uv run mypy axis +uv run pytest tests/test_.py -v --cov=axis. --cov-report=term-missing +``` -## Git Workflow +### Tips -- Never create commits on the `master` branch. -- Never push commits directly to the `master` branch. -- Before any commit or push, check the current branch and confirm it is not `master`. -- If work is currently on `master`, create or switch to a feature branch before committing. -- If asked to commit or push from `master`, explain that the change must go through a feature branch and pull request. -- For any requested git operation, verify branch state first and summarize what will happen before committing or pushing. \ No newline at end of file +- **For new handlers:** Mention the API endpoint and expected response shape. +- **For bug fixes:** Paste the error trace and describe expected behavior. +- **For tests:** Link to the code under test and describe missing scenarios. +- **For reviews:** Use the Axis Review or Axis Review Verify agents instead (see below). + +--- + +## How to Request Copilot Code Review + +### Code Review Template + +Use this when asking Copilot to review code for correctness and regressions. Choose the agent that matches your need: + +#### Code Review Only (Axis Review Agent) + +``` +@axis-review: Please review my changes for correctness and regressions. + +[CHANGED FILES] +- axis/interfaces/my_handler.py (new handler logic) +- tests/test_my_handler.py (new tests) + +[FOCUS AREAS] +- Handler initialization phase behavior +- Enum normalization and _missing_ fallback handling +- XML parsing safety (namespace-aware, root-shape guards) +- Test coverage (100% for new code) +- Boundary respect (models vs. interfaces) + +[KNOWN CONCERNS] +- Refactored the query builder (may have subtle regressions) +- Added new enum value (verify _missing_ handling) + +[PASS CRITERIA] +- No behavior regressions +- All enums have _missing_ fallback +- Input boundary normalization present +- Tests cover changed code and error paths +``` + +#### Full Validation (Axis Review Verify Agent) + +``` +@axis-review-verify: Please validate my PR and run checks. + +[CHANGED FILES] +- axis/interfaces/my_handler.py +- tests/test_my_handler.py + +[RUN THESE CHECKS] +- uv run pytest tests/test_my_handler.py -v +- uv run ruff check axis/interfaces/my_handler.py +- uv run mypy axis + +[REPORT EXPECTED] +- Code review findings (severity-ranked) +- Check results (pass/fail per command) +- Coverage impact +- Residual risks or gaps +``` + +#### When to Use Each Agent + +| Task | Agent | Use When | +|---|---|---| +| Code review + risk analysis | Axis Review | After code is ready, before running tests | +| Code review + full test/lint/type validation | Axis Review Verify | Want comprehensive feedback and passing CI confidence | +| Both | Use Axis Review first, then Axis Review Verify | For thorough feedback with validation | + +--- + +## Common Failure Modes & Anti-Patterns + +### Enum Crashes on Unknown Device Values + +**Anti-pattern:** Assuming enum always exists. + +```python +# WRONG: Will crash if value is unknown +status = Status(user_input) + +# RIGHT: Use _missing_ fallback +class Status(Enum): + OK = "ok" + ERROR = "error" + UNKNOWN = "unknown" + + @classmethod + def _missing_(cls, value: object) -> Status: + LOGGER.debug("Unknown status: %s", value) + return cls.UNKNOWN +``` + +**Why it matters:** Devices return unexpected values; unguarded enums crash at runtime. + +### XML/Event Parsing Crashes on Unexpected Structure + +**Anti-pattern:** Assuming fixed XML structure. + +```python +# WRONG: Assumes 'event' root always exists +event_data = xmltodict.parse(payload)['event'] + +# RIGHT: Check root shape and use traverse helper +root = xmltodict.parse(payload, process_namespaces=True) +if root and 'event' in root: + event_data = root['event'] +else: + LOGGER.warning("Unexpected XML structure") + event_data = {} +``` + +**Why it matters:** Event streams vary by device firmware; fixed assumptions miss edge cases. + +### Handler Initialization Phase Misuse + +**Anti-pattern:** Handler initializes in wrong phase. + +```python +# WRONG: Declares API_DISCOVERY but initializes in APPLICATION +class LightHandler(ApiHandler): + handler_groups = {HandlerGroup.API_DISCOVERY} + + def should_initialize_in_group(self, group: HandlerGroup) -> bool: + return group == HandlerGroup.APPLICATION # Contradicts declaration +``` + +**Why it matters:** Phase-based ordering ensures dependencies are met; mismatches cause failures. + +### Input Boundary Not Normalized + +**Anti-pattern:** Not coercing enums at entry points. + +```python +# WRONG: Device sends "http" but code expects HttpProtocol enum +class Config: + def __init__(self, protocol: str): + self.protocol = protocol + +# RIGHT: Coerce at boundary +class Config: + def __init__(self, protocol: str | HttpProtocol): + self.protocol = HttpProtocol(protocol) +``` + +**Why it matters:** External inputs use strings; internal code expects enums. Normalizing at the boundary prevents type confusion. + +### Incomplete Test Coverage + +**Anti-pattern:** Testing only happy path. + +```python +# WRONG: Only tests success +async def test_get_status(): + mock.respond(json={"status": "ok"}) + assert await handler.get_status() == "ok" + +# RIGHT: Test success, error, and edge cases +async def test_get_status_success(): + mock.respond(json={"status": "ok"}) + assert await handler.get_status() == "ok" + +async def test_get_status_unknown_value(): + mock.respond(json={"status": "unknown"}) + assert await handler.get_status() == StatusEnum.UNKNOWN + +async def test_get_status_malformed(): + mock.respond(json={"data": []}) # Missing "status" + with pytest.raises(KeyError): + await handler.get_status() +``` + +**Why it matters:** Untested error paths ship bugs; coverage threshold catches them. + +--- + +## Troubleshooting + +### Coverage Dropped Below 95% + +**Symptom:** CI fails with coverage check. + +**Fix:** +1. Run `uv run pytest --cov=axis --cov-report=term-missing | grep axis/` to find gaps. +2. Add tests in `tests/test_.py` for uncovered lines. +3. Ensure new code is 100% covered. +4. Rerun: `uv run pytest --cov=axis`. + +--- + +### mypy Disallow Untyped Defs Error + +**Symptom:** `error: Function is missing a return type annotation`. + +**Fix:** +```python +# WRONG +async def get_status(): + return await self.device.api_call() + +# RIGHT +async def get_status(self) -> str: + return await self.device.api_call() +``` + +--- + +### Ruff Lint Errors + +**Symptom:** `uv run ruff check .` reports errors. + +**Fix:** +1. Run `uv run ruff check . --show-source` for details. +2. Use `uv run ruff check . --fix` to auto-fix simple issues. +3. For complex issues, read the rule documentation. + +--- + +### Enum Crashes on Unknown Device Value + +**Symptom:** `ValueError: 'unexpected_value' is not a valid `. + +**Fix:** Implement `_missing_` (see Common Failure Modes above). + +--- + +### XML Parsing KeyError + +**Symptom:** `KeyError: 'event'` when parsing device response. + +**Fix:** Check root shape before accessing (see Common Failure Modes above). + +--- + +### Test Fixture Mismatch + +**Symptom:** `TypeError: unsupported operand type` or fixture errors. + +**Fix:** +1. Check [CONTRIBUTING.md](../../CONTRIBUTING.md) fixture table (aiohttp_mock_server vs. http_route_mock). +2. Ensure `async def test_*` (no decorator). +3. If overriding a fixture, document why in the fixture docstring. + +--- + +### Pre-commit Hook Modifies Files + +**Symptom:** After commit, files are modified by hooks. + +**Fix:** +```bash +git add -u # Stage the hook-modified files +git commit # Re-commit +``` \ No newline at end of file diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..9ae41dfa --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,36 @@ +## What does this PR do? + +[One-sentence summary, e.g., "Adds new PTZ handler with PARAM_CGI_FALLBACK support"] + +## Architecture Layer(s) + +- [ ] Models (`axis/models/`) +- [ ] Interfaces (`axis/interfaces/`) +- [ ] Orchestration (`axis/device.py`, `axis/interfaces/vapix.py`) +- [ ] Tests + +## Type of Change + +- [ ] New feature (handler, interface, model) +- [ ] Bug fix (minimal, targeted) +- [ ] Refactor (no behavior change) +- [ ] Documentation + +## Related Issues + +Closes #[issue number] (if applicable) + +## Verification Checklist + +- [ ] `uv run ruff check .` passes +- [ ] `uv run ruff format --check .` passes +- [ ] `uv run mypy axis` passes +- [ ] `uv run pytest` passes (coverage ≥95%) +- [ ] New tests added for new code (100% coverage for new code) +- [ ] Commit hooks modified files? If so, re-staged and re-ran checks +- [ ] No unrelated code/formatting changes +- [ ] Follows conventions: enum `_missing_` fallback, input normalization, XML parsing guards + +## Notes for Reviewers + +[Context for reviewers: what to focus on, known limitations, decisions made, or why this approach was chosen] diff --git a/COPILOT_QUICK_START.md b/COPILOT_QUICK_START.md new file mode 100644 index 00000000..de50454b --- /dev/null +++ b/COPILOT_QUICK_START.md @@ -0,0 +1,193 @@ +# Using Copilot with Axis + +This guide helps you get the most out of Copilot when contributing to the Axis library. + +## Quick Links + +- **Architecture:** [README.md](README.md) — Phase-based handler initialization +- **Conventions:** [CONTRIBUTING.md](CONTRIBUTING.md) — Coding patterns, tests, fixtures +- **Project rules:** [.github/copilot-instructions.md](.github/copilot-instructions.md) — Build, test, git workflow +- **Request templates:** [.github/copilot-instructions.md](.github/copilot-instructions.md#how-to-ask-copilot-for-high-quality-work) — High-quality ask structure +- **Review agents:** [.github/agents/](.github/agents/) — Code review and validation +- **PR template:** [.github/pull_request_template.md](.github/pull_request_template.md) + +## The Axes of Work + +### 1. Adding a New Handler (Feature) + +**What:** Implement a new Axis device API. + +**Steps:** +1. Create a model in `axis/models/.py` with dataclasses and enums (include `_missing_` fallback). +2. Create a handler in `axis/interfaces/.py` extending `ApiHandler`. +3. Register it on `Vapix` in `axis/interfaces/vapix.py`. +4. Add tests in `tests/test_.py` using fixtures from `tests/conftest.py`. + +**Copilot request:** +``` +I need to add a new handler for [feature]. + +[CONTEXT] +The VAPIX API is at [endpoint]. The response looks like [example JSON]. +This handler should participate in the [API_DISCOVERY / PARAM_CGI_FALLBACK / APPLICATION] phase. + +[TASK] +1. Add model in axis/models/[name].py with request/response dataclasses +2. Add handler in axis/interfaces/[name].py extending ApiHandler +3. Register handler on Vapix in axis/interfaces/vapix.py +4. Add tests in tests/test_[name].py using aiohttp_mock_server or http_route_mock + +[CONSTRAINTS] +- 100% test coverage for new code +- All enums have _missing_ fallback +- Input normalization in __post_init__ +- Preserve existing behavior + +[VERIFICATION] +uv run ruff check . +uv run ruff format --check . +uv run mypy axis +uv run pytest tests/test_[name].py -v --cov=axis.[name] --cov-report=term-missing +``` + +**Then:** +``` +@axis-review: Please use Axis Review to check the handler for regressions and test gaps. +``` + +### 2. Fixing a Bug + +**What:** Fix a specific issue in an existing handler or parsing logic. + +**Copilot request:** +``` +I found a bug in [area]: + +[PROBLEM] +[Describe the issue, error trace, or unexpected behavior] + +[ROOT CAUSE (if known)] +[What's causing it] + +[TASK] +Fix [area] to [desired behavior]. + +[CONSTRAINTS] +- Minimal, targeted fix (preserve existing behavior) +- Update tests if behavior changes +- No unrelated changes + +[VERIFICATION] +uv run pytest tests/test_[area].py -v +``` + +### 3. Adding Tests + +**What:** Improve test coverage for existing code. + +**Copilot request:** +``` +Add tests for [area] to improve coverage. + +[CONTEXT] +Current coverage: [%]. Missing coverage: [specific function/branch]. +Tests live in tests/test_[area].py. +Use fixtures: aiohttp_mock_server or http_route_mock (see CONTRIBUTING.md for examples). + +[TASK] +Add tests for: +1. [Happy path] +2. [Error case] +3. [Edge case] + +[CONSTRAINTS] +- 100% coverage for new/modified code +- Reuse existing fixtures +- Follow existing test patterns + +[VERIFICATION] +uv run pytest tests/test_[area].py -v --cov=axis.[area] --cov-report=term-missing +``` + +### 4. Code Review + +**What:** Have Copilot review your code for issues and regressions. + +**Copilot request (use Axis Review agent):** +``` +@axis-review: Please review my changes in [files]: + +[FOCUS AREAS] +- Handler initialization phase behavior +- Enum handling and _missing_ fallback +- XML parsing safety +- Test coverage gaps + +[PASS/FAIL CRITERIA] +- PASS: No regressions, behavior matches spec, tests cover changes +- FAIL: Breaks initialization order, missing enum fallback, or untested error paths +``` + +**Copilot request (use Axis Review Verify agent):** +``` +@axis-review-verify: Please validate my PR: + +[CHANGED FILES] +[list files] + +[RUN] +- uv run pytest tests/test_.py +- uv run ruff check axis/ +- uv run mypy axis +``` + +## Common Pitfalls + +| Pitfall | Fix | +|---|---| +| "Coverage is low" | Ensure new code has 100% coverage. Use `--cov=axis. --cov-report=term-missing` to see gaps. | +| "Enum crashes on unknown value" | Add `_missing_` classmethod that returns `.UNKNOWN` and logs a debug warning. | +| "XML parsing assumes fixed structure" | Use `xmltodict.parse(..., process_namespaces=True)` and check root shape before accessing. | +| "Handler doesn't initialize" | Check `handler_groups` and `should_initialize_in_group()` match the initialization phase. | +| "Ruff/mypy/pytest fail" | Run locally before requesting review. Fix issues in your request context. | +| "Pre-commit hook modifies files" | Re-stage and re-commit: `git add -u && git commit`. | + +## Useful Commands + +```bash +# Setup +./setup.sh + +# Full validation +uv run ruff check . +uv run ruff format --check . +uv run mypy axis +uv run pytest + +# Targeted validation +uv run pytest tests/test_.py -v +uv run ruff check axis/.py +uv run mypy axis. + +# Coverage report +uv run pytest --cov=axis --cov-report=html +# Open htmlcov/index.html + +# Pre-commit hooks +uv run pre-commit install +``` + +## Next Steps + +1. Read [CONTRIBUTING.md](CONTRIBUTING.md) for detailed conventions and architecture. +2. Explore [.github/copilot-instructions.md](.github/copilot-instructions.md) for project-wide rules. +3. Check [.github/agents/](.github/agents/) for review agent details. +4. Review the [request template](.github/copilot-instructions.md#request-template) before opening an issue. + +## Questions? + +If you run into issues, check: +1. [CONTRIBUTING.md](CONTRIBUTING.md) — conventions and patterns +2. [tests/conftest.py](tests/conftest.py) — fixture details +3. Existing test modules in `tests/` — see how similar work is tested +4. [.github/copilot-instructions.md](.github/copilot-instructions.md#troubleshooting) — troubleshooting guide