From 01771e11609928a960e874e917c22f3de8da1e6c Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Wed, 11 Mar 2026 13:55:29 -0700 Subject: [PATCH 1/2] Added skills --- .github/copilot-instructions.md | 37 ++++-- .github/skills/ci-failure-debugging/SKILL.md | 84 +++++++++++++ .github/skills/samples/SKILL.md | 95 ++++++++++++++ .github/skills/sync-async-pattern/SKILL.md | 123 +++++++++++++++++++ .github/workflows/validate.yml | 2 +- 5 files changed, 327 insertions(+), 14 deletions(-) create mode 100644 .github/skills/ci-failure-debugging/SKILL.md create mode 100644 .github/skills/samples/SKILL.md create mode 100644 .github/skills/sync-async-pattern/SKILL.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d9cc428..30233b7 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -54,6 +54,21 @@ This project requires **Python 3.10 or newer**. --- +## CODE FORMATTING + +### RUNNING BLACK + +**COMMAND:** +```bash +black featuremanagement +``` + +Line length is configured to 120 in `pyproject.toml`. + +**Always run black before pylint and mypy**, as formatting fixes can resolve issues those tools detect. + +--- + ## PYLINT OPERATIONS ### RUNNING PYLINT @@ -94,19 +109,6 @@ The project uses `strict = True` in `mypy.ini`. --- -## CODE FORMATTING - -### RUNNING BLACK - -**COMMAND:** -```bash -black featuremanagement -``` - -Line length is configured to 120 in `pyproject.toml`. - ---- - ## TESTING ### RUNNING TESTS @@ -119,3 +121,12 @@ pytest tests - Sync tests are in `tests/test_*.py` - Async tests use `pytest-asyncio` and are in files ending with `_async.py` - Run tests with: `pytest tests` + +--- + +## NEW FEATURES + +When adding a new user-facing feature or capability: + +- Create a sample in `samples/` demonstrating the feature. +- Add corresponding unit tests (sync and async where applicable). diff --git a/.github/skills/ci-failure-debugging/SKILL.md b/.github/skills/ci-failure-debugging/SKILL.md new file mode 100644 index 0000000..4286af4 --- /dev/null +++ b/.github/skills/ci-failure-debugging/SKILL.md @@ -0,0 +1,84 @@ +--- +name: ci-failure-debugging +description: > + Debug and fix failing CI validation checks for this Python project. + Use when asked to fix CI failures, debug failing PR checks, fix pylint/mypy/black/cspell/pytest errors, + or when a PR validation workflow fails. +--- + +# CI Failure Debugging + +This project's PR validation runs the following checks on Python 3.10–3.14. To debug failures, identify which step failed and follow the corresponding section below. + +## Step 1: Identify the failing check + +The validation workflow runs these steps in order: + +1. **black** — code formatting +2. **pylint** — static analysis +3. **mypy** — type checking (strict mode) +4. **cspell** — spell checking +5. **pytest** — unit tests with coverage +6. **pylint (samples/tests)** — lint samples and tests with relaxed rules + +## Step 2: Reproduce locally + +Set up the environment first: + +```bash +python -m pip install -e ".[dev,test]" +``` + +Then run the specific failing check: + +| Check | Command | Notes | +|-------|---------|-------| +| pylint | `pylint featuremanagement` | | +| black | `black --check featuremanagement` | Use `black featuremanagement` to auto-fix | +| mypy | `mypy featuremanagement` | Uses `strict = True` from `mypy.ini` | +| cspell | `npx cspell "**"` | Config in `cspell.config.yaml`, custom words in `project-words.txt` | +| pytest | `pytest tests --doctest-modules --cov-report=xml --cov-report=html` | | +| pylint (samples) | `pylint --disable=missing-function-docstring,missing-class-docstring samples tests` | Requires `python -m pip install -r samples/requirements.txt` | + +## Step 3: Fix the issue + +### pylint failures + +- Run `pylint featuremanagement` and fix reported issues. +- Do NOT add `# pylint: disable` comments unless absolutely necessary. +- Do NOT add new imports or dependencies to fix warnings. +- The project disables `duplicate-code` in `pyproject.toml`. +- Max line length is 120. Min public methods is 1. Max branches is 20. Max returns is 7. + +### black failures + +- Run `black featuremanagement` to auto-format. Line length is 120 (configured in `pyproject.toml`). +- If CI uses `black --check`, it means files need reformatting — run `black` locally to fix. + +### mypy failures + +- Run `mypy featuremanagement`. The project uses `strict = True` with Python 3.10 target. +- All functions must have type annotations. +- Use `Optional[X]` or `X | None` for nullable types. +- Check `mypy.ini` for the full configuration. + +### cspell failures + +- Misspelled words: fix the typo in your code. +- Legitimate technical terms: add the word to `project-words.txt` (one word per line, alphabetically sorted). +- Do NOT modify `cspell.config.yaml` unless adding a new ignore path. + +### pytest failures + +- Run `pytest tests` to reproduce. +- Sync tests: `tests/test_*.py` +- Async tests: `tests/test_*_async.py` (use `pytest-asyncio`) +- Time window filter tests: `tests/time_window_filter/` +- Telemetry tests: `tests/test_send_telemetry_appinsights.py` +- If adding new code, ensure both sync and async tests exist where applicable. + +### pylint (samples/tests) failures + +- This step runs with `--disable=missing-function-docstring,missing-class-docstring`. +- Requires sample dependencies: `python -m pip install -r samples/requirements.txt`. +- Fix any remaining pylint issues in `samples/` and `tests/` directories. diff --git a/.github/skills/samples/SKILL.md b/.github/skills/samples/SKILL.md new file mode 100644 index 0000000..e6bd0b9 --- /dev/null +++ b/.github/skills/samples/SKILL.md @@ -0,0 +1,95 @@ +--- +name: samples +description: > + Guide for creating or updating sample applications in this project. + Use when adding a new sample, modifying an existing sample, or when asked to demonstrate + a feature management capability with example code. +--- + +# Sample Applications + +Samples live in `samples/` and demonstrate feature management capabilities to users. + +## File conventions + +- Every sample must have the Microsoft copyright header: + ```python + # ------------------------------------------------------------------------ + # Copyright (c) Microsoft Corporation. All rights reserved. + # Licensed under the MIT License. See License.txt in the project root for + # license information. + # ------------------------------------------------------------------------- + ``` +- Every sample must have a module-level docstring (one-liner describing what it demonstrates). +- Filename should end with `_sample.py` and describe what is being demonstrated (e.g., `feature_flag_sample.py`, `feature_variant_sample_with_telemetry.py`). + +## Structure of a sample + +Samples follow this general pattern: + +```python +# (copyright header) +"""Sample demonstrating .""" + +import json +import os +import sys +from featuremanagement import FeatureManager, TargetingContext + +# Load feature flags from the local JSON file +file_path = os.path.dirname(os.path.abspath(sys.argv[0])) +with open(os.path.join(file_path, "formatted_feature_flags.json"), encoding="utf-8") as f: + feature_flags = json.load(f) + +# Create FeatureManager +feature_manager = FeatureManager(feature_flags) + +# Demonstrate the feature +result = feature_manager.is_enabled("FlagName") +print(f"FlagName is {'enabled' if result else 'disabled'}") +``` + +## Feature flag definitions + +Sample feature flags go in `formatted_feature_flags.json` under `feature_management.feature_flags`. Each flag needs at minimum `id` and `enabled`. Add filters, variants, allocation, or telemetry as needed for the sample. + +## Custom filters + +Custom filters used by samples are defined in their own file (e.g., `random_filter.py`) and imported by the samples that need them. + +## Async samples + +Async samples import from `featuremanagement.aio` instead of `featuremanagement`. See `quarty_sample.py` for the async pattern. + +## Azure-connected samples + +Samples that connect to Azure App Configuration: +- Use `azure.appconfiguration.provider.load()` to get configuration +- Authenticate using `DefaultAzureCredential` from `azure-identity`, never connection strings +- List Azure dependencies in `samples/requirements.txt` + +## Telemetry samples + +Two patterns exist: +1. **Callback-based**: Pass `on_feature_evaluated=publish_telemetry` to `FeatureManager` and use `track_event()` from `featuremanagement.azuremonitor`. +2. **Web app span processor**: Use `TargetingSpanProcessor` from `featuremanagement.azuremonitor` with `configure_azure_monitor(span_processors=[...])`. + +## Dependencies + +Any new package a sample needs must be added to `samples/requirements.txt`. CI installs these before linting samples. + +## Linting + +Samples are linted with relaxed rules: +```bash +pylint --disable=missing-function-docstring,missing-class-docstring samples tests +``` + +Function and class docstrings are NOT required in samples, but module-level docstrings ARE. + +## Checklist for adding a new sample + +1. [ ] Create `samples/feature__sample.py` with copyright header and module docstring. +2. [ ] Add any new feature flags to `formatted_feature_flags.json`. +3. [ ] Add any new dependencies to `samples/requirements.txt`. +4. [ ] Verify lint passes: `pylint --disable=missing-function-docstring,missing-class-docstring samples` diff --git a/.github/skills/sync-async-pattern/SKILL.md b/.github/skills/sync-async-pattern/SKILL.md new file mode 100644 index 0000000..4df1064 --- /dev/null +++ b/.github/skills/sync-async-pattern/SKILL.md @@ -0,0 +1,123 @@ +--- +name: sync-async-pattern +description: > + Guide for implementing sync/async mirrored code in this project. + Use when adding new classes, methods, or feature filters that need both sync and async versions, + or when modifying existing sync code that has an async counterpart in featuremanagement/aio/. +--- + +# Sync/Async Mirroring Pattern + +This project maintains parallel sync and async implementations. Every change to sync code in `featuremanagement/` must be mirrored in `featuremanagement/aio/`, and vice versa. + +## Directory mapping + +| Sync | Async | +|------|-------| +| `featuremanagement/_featuremanager.py` | `featuremanagement/aio/_featuremanager.py` | +| `featuremanagement/_featurefilters.py` | `featuremanagement/aio/_featurefilters.py` | +| `featuremanagement/_defaultfilters.py` | `featuremanagement/aio/_defaultfilters.py` | +| `featuremanagement/__init__.py` | `featuremanagement/aio/__init__.py` | + +Shared code that does NOT have an async counterpart: +- `featuremanagement/_featuremanagerbase.py` — base class used by both sync and async `FeatureManager` +- `featuremanagement/_models/` — data models imported by both +- `featuremanagement/_time_window_filter/` — time window logic (no I/O, used as-is) +- `featuremanagement/azuremonitor/` — telemetry (no async version) + +## Copyright header + +Every source file MUST start with: + +```python +# ------------------------------------------------------------------------ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for +# license information. +# ------------------------------------------------------------------------- +``` + +Followed by a module-level docstring. + +## How to convert sync to async + +### Classes + +- Keep the **same class name** (e.g., both are `FeatureManager`). Users disambiguate by import path. +- Both sync and async `FeatureManager` inherit from `FeatureManagerBase`. + +### Methods + +- Add `async` to method definitions: `def evaluate(...)` → `async def evaluate(...)` +- Add `await` to calls that invoke filters, callbacks, or accessors. + +### Default filters (composition pattern) + +Async default filters do NOT duplicate logic. They wrap the sync implementation: + +```python +from .._defaultfilters import TimeWindowFilter as SyncTimeWindowFilter + +class TimeWindowFilter(FeatureFilter): + def __init__(self): + self._filter = SyncTimeWindowFilter() + + @FeatureFilter.alias("Microsoft.TimeWindow") + async def evaluate(self, context, **kwargs): + return self._filter.evaluate(context, **kwargs) +``` + +Use this pattern for any new filter whose `evaluate` does not perform I/O. + +### Callbacks and accessors + +The async `FeatureManager` supports BOTH sync and async callbacks. Use `inspect.iscoroutinefunction` to detect and handle both: + +```python +import inspect + +if inspect.iscoroutinefunction(self._on_feature_evaluated): + await self._on_feature_evaluated(result) +else: + self._on_feature_evaluated(result) +``` + +### Imports + +- Sync files import from `._models`, `._featurefilters`, etc. +- Async files import from `.._models`, `.._featurefilters`, etc. (one level up from `aio/`). + +## `__init__.py` exports + +### Sync (`featuremanagement/__init__.py`) + +Exports everything: `FeatureManager`, filters, all models, `__version__`, and defines `__all__`. + +### Async (`featuremanagement/aio/__init__.py`) + +Exports ONLY async-specific classes: `FeatureManager`, `FeatureFilter`, `TimeWindowFilter`, `TargetingFilter`. Does NOT re-export models or `__version__` — users import those from the sync package. + +When adding a new public class: +1. Add to sync `__init__.py` with `__all__` entry. +2. If it has an async version, add to async `__init__.py` with `__all__` entry. + +## Test file naming + +| Sync test | Async counterpart | +|-----------|-------------------| +| `tests/test_feature_manager.py` | `tests/test_feature_manager_async.py` | +| `tests/test_feature_variants.py` | `tests/test_feature_variants_async.py` | +| `tests/test_default_feature_flags.py` | `tests/test_default_feature_flags_async.py` | + +- Async test files append `_async` to the sync filename. +- Async tests use `pytest-asyncio` with `@pytest.mark.asyncio` on test functions. +- Not every sync test needs an async counterpart (e.g., refresh and telemetry tests are sync-only). + +## Checklist for adding new code + +1. [ ] Write the sync implementation in `featuremanagement/`. +2. [ ] Write the async mirror in `featuremanagement/aio/` following the patterns above. +3. [ ] Export from both `__init__.py` files if public. +4. [ ] Write sync tests in `tests/test_*.py`. +5. [ ] Write async tests in `tests/test_*_async.py`. +6. [ ] Run all validation: `pylint featuremanagement`, `black featuremanagement`, `mypy featuremanagement`, `pytest tests`. diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 49d6d81..50031b0 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -18,10 +18,10 @@ jobs: run: | python -m pip install --upgrade pip python -m pip install ".[dev]" + - uses: psf/black@26.3.0 - name: Analysing the code with pylint run: | pylint featuremanagement - - uses: psf/black@26.3.0 - name: Run mypy run: | mypy featuremanagement From 3608c1de902c2af5bd35bbbfa89579a5edde384b Mon Sep 17 00:00:00 2001 From: Matthew Metcalf Date: Wed, 11 Mar 2026 14:10:50 -0700 Subject: [PATCH 2/2] Update .github/workflows/validate.yml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/workflows/validate.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 50031b0..433d0c5 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -18,7 +18,9 @@ jobs: run: | python -m pip install --upgrade pip python -m pip install ".[dev]" - - uses: psf/black@26.3.0 + - name: Check code formatting with black + run: | + python -m black --check featuremanagement - name: Analysing the code with pylint run: | pylint featuremanagement