diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..1b61066dc --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,55 @@ +# Review rules for all PRs + +## 0. What not to flag + +CI already runs deterministic gates on every PR. If a gate would catch it, the gate reports it, +and a duplicate review comment is noise: + +| Gate | Covers | +| --- | --- | +| `precommit` (ruff, ruff-format, uv-lock, rstcheck) | Lint rules, formatting, import order, line length, `uv.lock` freshness, RST syntax | +| `typecheck` (pyright, strict) | Type errors and unused `type: ignore` in packages listed under `[tool.pyright] include` | +| `spellcheck` (codespell) | Spelling in code, docs, and comments | +| `shellcheck` | Shell script correctness in `scripts/*.sh` | +| `readme` | `README.rst` renders on PyPI | +| `docs` | Docs build | +| `generate` | The generated instrumentation README table being up to date | +| `lint-license-header-check` | Missing license headers | +| `oldest-deps-check` | Redundant pins in `tests/requirements.oldest.txt`, packages missing an `oldest` requirements file | +| `changelog` workflow | Direct edits to `CHANGELOG.md`, changelog fragment presence | +| `test` matrix | Test failures across the `oldest`/`latest`/`conformance` envs | + +Review the things a gate cannot decide: whether the change is correct, whether it matches the +GenAI semantic conventions, whether it actually achieves what the PR claims, and whether it +weakens or bypasses a gate. Style opinions that ruff does not enforce are not review comments. + +## 1. Shared configuration + +Repo-wide config is `tox.ini`, `pytest.ini`, root `pyproject.toml`, `.codespellrc`, +`.pre-commit-config.yaml`, and `scripts/`. A change there affects every package, so: + +- **No developer-local paths.** Personal venv or scratch directory names must not be added to + shared configs. `.gitignore` already covers `venv*/`, `.venv*/`, and `.tox`; anything else + belongs in a local ignore or gets renamed to match those patterns. Flag entries that only make + sense on one contributor's machine. +- **Skip lists must not weaken a gate.** Excluding a bare directory name from a lint or license + check also excludes any real source directory that happens to share that name. Prefer + path-anchored patterns (`*/.tox/*`) over bare components (`.tox`). +- **Shell must be portable.** Commands in `tox.ini` and `scripts/*.sh` run on both Linux and + macOS. Flag GNU-only flags (e.g. `xargs --no-run-if-empty`, `sed -i` without a backup suffix, + `readlink -f`). + +## 2. Config that packages can shadow + +Root config does not automatically reach every package. Before approving a change to a root +setting, check whether packages override the same section in their own `pyproject.toml`: + +- `pytest.ini` is only read when pytest's config lookup does not stop at a package + `pyproject.toml` first. These packages define `[tool.pytest.ini_options]` and therefore ignore + root `pytest.ini` entirely: `anthropic`, `agno`, `crewai`, `smolagents`, `llama-index`, + `claude-agent-sdk`. A new root pytest setting must either be added to each of them, or those + blocks removed. +- The same applies to `[tool.ruff]` and `[tool.pyright]` overrides. + +Flag a root-config change whose stated goal ("fixes the warning everywhere", "applies to all +tests") is not actually achieved for the shadowing packages. diff --git a/.github/instructions/build-config.instructions.md b/.github/instructions/build-config.instructions.md new file mode 100644 index 000000000..90e2d7fef --- /dev/null +++ b/.github/instructions/build-config.instructions.md @@ -0,0 +1,58 @@ +--- +applyTo: "tox.ini,pytest.ini,pyproject.toml,uv.lock,.codespellrc,.pre-commit-config.yaml,scripts/**,.github/workflows/**,.github/actions/**" +--- + +Review rules for PRs touching build, test-matrix, and CI configuration. Flag violations with a +link to the rule. See [copilot-instructions.md](../copilot-instructions.md) for the repo-wide +rules on shared config, portability, and shadowed settings, and for the gates whose findings must +not be repeated as review comments. + +These rules cover what CI cannot check: a test env that is never run, a bound that is never +exercised, a gate that silently stops covering ground. All of them pass CI while being wrong. + +## 1. Test matrix completeness + +A new package under `instrumentation//` must be wired into `tox.ini` in full. Check each of +these, not just the first: + +- `envlist`: `py3{…}-test-instrumentation-genai--{oldest,latest}`, the matching + `-conformance` entry, and `lint-instrumentation-genai-`. +- `[testenv] deps`: the factor-conditional `-r …/tests/requirements..txt` lines plus + `{[testenv]test_deps}` / `{[testenv]pytest_deps}`. Requirements install here, **not** in + `commands_pre`. +- `[testenv] commands`: the pytest line (which must `--ignore` `tests/test_conformance.py`), the + separate `-conformance` pytest line, and the `lint-…` ruff line. +- `[testenv:typecheck] deps`: `{toxinidir}/instrumentation/[instruments]`. + +The uv workspace picks up new packages via the `instrumentation/*` glob in root `pyproject.toml`, +so no edit is needed there. + +## 2. Lower bounds + +The `oldest` factor sets `UV_RESOLUTION=lowest-direct`, so lower bounds come from +`pyproject.toml` and `oldest-deps-check` already enforces that invariant. Do not re-report what +it catches. + +What is left to review is whether a changed bound is justified: a raised floor needs a reason in +the PR description (a feature or fix the code now depends on), and a lowered floor needs evidence +that the older version actually works, not just that the env resolves. + +## 3. Pyright scope + +`[tool.pyright] include` is opt-in and grows one package at a time. Typing more code is the goal, +so review changes here by direction: + +- Adding a package to `include` is welcome. `src/**` is never excluded. +- Excluding a package's `tests/**` and `examples/**` is the current convention when it joins + `include`, but only as a stopgap (see the comment above `exclude`). A PR that types its tests + and drops the exclusion is an improvement. +- Flag the reverse: a package removed from `include`, or a `src/**` path added to `exclude`, to + make a type error go away. + +## 4. Lint and check scripts + +A change that narrows what a check scans (new skip entry, new exclude path, a `|| true`) must say +why in the PR description. Silent scope reduction on a gate reads as "still passing" when it no +longer checks the same ground. + +See also [AGENTS.md](../../AGENTS.md) for general repo rules.