Run Pyright in CI for every package, in its own locked environment

.github/workflows/pr.yml

@@ -16,7 +16,7 @@ jobs:
     # every job passed. See docs/engineering/skills/github-prs.md.
     name: CI complete
     if: ${{ always() }}
-    needs: [test, lint, quality-guards, docker-build, test-integration]
+    needs: [test, lint, pyright, quality-guards, docker-build, test-integration]
     runs-on: ubuntu-latest
     permissions:
       contents: read
@@ -119,6 +119,9 @@ jobs:
       with:
         python-version: '3.13'
 
+    - name: Install uv (the Pyright check's tests run the pinned Pyright)
+      uses: astral-sh/setup-uv@v8.1.0
+
     - name: Test the rules registry tool
       run: python -m unittest discover -s scripts/rules/tests -t .
 
@@ -128,6 +131,53 @@ jobs:
     - name: Run quality guards
       run: python scripts/run_quality_guards.py
 
+  pyright:
+    name: Pyright
+    # Full CI runs only on the lowest unmerged pull request in a stack.
+    if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }}
+    runs-on: ubuntu-latest
+    strategy:
+      # Each package passes or fails on its own; report every one.
+      fail-fast: false
+      matrix:
+        # Each entry is a uv package, type-checked in its own locked
+        # environment with the Pyright version pinned in
+        # scripts/rules/pyright.py. The Pyright rules in rules/registry.toml
+        # list the same packages, and the rules guard fails when one is
+        # missing here.
+        package:
+          - libs/policyengine-simulation-contract
+          - libs/policyengine-simulation-observability
+          - libs/policyengine-stage12-persistence
+          - libs/policyengine-fastapi
+          - projects/policyengine-simulation-entry
+          - projects/policyengine-simulation-gateway
+          - projects/policyengine-simulation-executor
+          - projects/policyengine-apis-integ
+
+    steps:
+    - uses: actions/checkout@v6
+
+    - name: Set up Python
+      uses: actions/setup-python@v6
+      with:
+        python-version: '3.13'
+
+    - name: Install uv
+      uses: astral-sh/setup-uv@v8.1.0
+      with:
+        enable-cache: true
+
+    - name: Generate API clients
+      # apis-integ installs the generated client from its output directory.
+      if: matrix.package == 'projects/policyengine-apis-integ'
+      run: ./scripts/generate-clients.sh
+
+    - name: Run Pyright in the package's locked environment
+      env:
+        PACKAGE: ${{ matrix.package }}
+      run: python -m scripts.rules pyright --sync "$PACKAGE"
+
   docker-build:
     name: Docker build
     needs: [test, lint]  # Only run if tests and linting pass

Makefile

@@ -1,5 +1,5 @@
 # Simplified Makefile using docker-compose
-.PHONY: help dev up down build test clean logs format check push-pr-branch
+.PHONY: help dev up down build test clean logs format check require-ruff-version push-pr-branch
 
 # Load environment variables if .env exists
 ifneq (,$(wildcard deployment/.env))
@@ -30,13 +30,19 @@ help:
 	@echo "Maintenance:"
 	@echo "  make build            - Build Docker images"
 	@echo "  make format           - Format code with ruff"
-	@echo "  make check            - ruff check + pyright"
+	@echo "  make check            - CI's ruff checks + Pyright in each package"
 	@echo "  make update           - uv lock --upgrade across packages"
 	@echo "  make clean            - Remove caches and stop containers"
 	@echo "  make push-pr-branch   - Push current branch to origin for a PR"
 	@echo ""
 	@echo "Deployment is automated via GitHub Actions to Cloud Run and Modal; there is no manual deploy target."
 
+# CI's ruff, read from the lint job's `uv tool install ruff==<version>` step in
+# .github/workflows/pr.yml so the two can't drift. A ruff found on PATH may be
+# another version, which selects different rules under select = ["ALL"].
+RUFF_VERSION := $(shell sed -n 's/.*uv tool install ruff==\([0-9][0-9.]*\).*/\1/p' .github/workflows/pr.yml | head -n 1)
+RUFF := uv tool run ruff@$(RUFF_VERSION)
+
 # Development commands
 dev:
 	docker-compose -f deployment/docker-compose.yml up --build
@@ -138,24 +144,33 @@ update:
 	@echo "✅ All dependencies updated"
 
 # Code quality
-format:
-	@echo "Formatting code with ruff..."
+format: require-ruff-version
+	@echo "Formatting code with ruff $(RUFF_VERSION)..."
 	@for dir in projects/*/src libs/*/src; do \
 		if [ -d "$$dir" ]; then \
 			echo "Formatting $$dir..."; \
-			uv run ruff format $$dir; \
+			$(RUFF) format $$dir; \
 		fi \
 	done
 
-check:
-	@echo "Running code quality checks..."
-	@for dir in projects/*/src libs/*/src; do \
-		if [ -d "$$dir" ]; then \
-			echo "Checking $$dir..."; \
-			uv run ruff check $$dir; \
-			uv run pyright $$dir; \
-		fi \
-	done
+# Pyright runs in each package's own locked environment, with the version CI
+# pins; apis-integ needs the generated client. See
+# docs/engineering/skills/type-checking.md.
+# The ruff commands are the lint job's, so a passing make check means the job
+# passes too.
+check: require-ruff-version generate-clients
+	@echo "Running code quality checks (ruff $(RUFF_VERSION))..."
+	@status=0; \
+	$(RUFF) format --check projects/*/src libs/*/src || status=1; \
+	$(RUFF) check . || status=1; \
+	python3 -m scripts.rules pyright --sync || status=1; \
+	exit $$status
+
+require-ruff-version:
+	@if [ -z "$(RUFF_VERSION)" ]; then \
+		echo "Could not read the ruff version from .github/workflows/pr.yml (uv tool install ruff==<version>)"; \
+		exit 1; \
+	fi
 
 push-pr-branch:
 	@BRANCH=$$(git branch --show-current); \

docs/engineering/skills/observability.md

@@ -79,6 +79,11 @@ All span names for supported run configurations are defined in
 appropriate `StagePlan` and call `plan.name(Stage.VALUE)`. Do not add span name
 string literals in services or workers.
 
+Open a span with `scopes.span(runtime, plan.name(Stage.VALUE))` from
+`policyengine_simulation_observability.scopes` rather than `runtime.span(...)`.
+It returns the same scope, typed so that Pyright knows an exception raised in
+the block propagates; see `docs/engineering/skills/type-checking.md`.
+
 When adding a run configuration or runtime stage:
 
 1. add it to `RunConfiguration` or `Stage`;

docs/engineering/skills/rules-registry.md

@@ -58,7 +58,7 @@ gap is visible, rather than leaving it out.
 | `[rule.proof]` | Optional; see "Proof". |
 | `[rule.baseline]` | Only for `ratcheted` rules: `file` listing tolerated violations, and their `count`. |
 | `[[rule.exception]]` | Temporary carve-out: `path`, `reason`, `expires`. |
-| `[rule.downgrade]` | Required `reason` when a rule moves from enforced/ratcheted to advisory/doc-only. |
+| `[rule.downgrade]` | Required `reason` when a rule moves from enforced/ratcheted to advisory/doc-only, or from enforced to ratcheted. |
 
 `blocking` is derived, never declared: a rule blocks merges when its status is
 `enforced` or `ratcheted` and its tier is not `judgment`.
@@ -72,7 +72,8 @@ gap is visible, rather than leaving it out.
 | `ruff` | `mode` = `format`, `lint` (with `codes`) or `per-file-ignore` (with `pattern`, `codes`) | Codes are selected / the ignore entry exists and covers only [files allowed file-level exceptions](#file-level-exceptions) |
 | `mypy` | `paths` | Each path is in `[tool.mypy].files` |
 | `workflow-job` | optional `step` | The CI job exists (the job itself is the check) |
-| `pyright`, `import-linter` | `config` (+ `contract` for Import Linter) | The config file exists |
+| `pyright` | `projects`: the package directories it checks | Each package exists, is listed by no other Pyright rule, has no `pyrightconfig.json`, lists what Pyright checks in its `pyproject.toml` `[tool.pyright] include`, and is in the CI job's matrix |
+| `import-linter` | `config`, `contract` | The config file exists |
 | `docs-guard` | `doc`, `marker` | The doc exists |
 | `llm-review` | `prompt`, `model` | The prompt exists; judgment tier only |
 | `none` | | Nothing; only for `doc-only` and `advisory` rules |
@@ -125,8 +126,8 @@ fail the guard yet.
 - Other enforcers (mypy, Pyright, Import Linter, custom guards) may start
   `ratcheted` with a baseline file of tolerated violations. Baselines may only
   shrink: the guard compares counts against the base branch.
-- Moving a rule from enforced/ratcheted to advisory/doc-only needs an explicit
-  `[rule.downgrade] reason`.
+- Moving a rule from enforced/ratcheted to advisory/doc-only, or from enforced
+  to ratcheted, needs an explicit `[rule.downgrade] reason`.
 
 ## File-level exceptions
 

docs/engineering/skills/type-checking.md

@@ -0,0 +1,121 @@
+# Type Checking
+
+Read this before changing a package's Pyright settings, its Pyright baseline or
+the pinned Pyright version, when the Pyright check fails, or when adding a
+package.
+
+## Rules
+
+Every uv package is type-checked with Pyright on every pull request, by the
+`pyright` job in `.github/workflows/pr.yml`:
+
+- `sim.quality.pyright-libraries-and-entry`: the contract, observability,
+  Stage 12 persistence and fastapi libraries and the simulation entry have no
+  Pyright errors.
+- `sim.quality.pyright-gateway`, `sim.quality.pyright-executor` and
+  `sim.quality.pyright-apis-integ`: these packages may have only the errors
+  their baselines list (see [Baselines](#baselines)).
+- `sim.quality.every-package-type-checked`: a Pyright rule lists every uv
+  package under `libs/` and `projects/`. When you add a package, give it a
+  `[tool.pyright] include`, add it to a Pyright rule's `enforcer.projects`, and
+  add it to the `pyright` job's matrix.
+
+## Running the check
+
+From the repository root:
+
+```bash
+make check                                   # CI's ruff, then Pyright in every package
+python -m scripts.rules pyright --sync projects/policyengine-simulation-gateway
+python -m scripts.rules pyright --list-packages
+```
+
+`--sync` first runs `uv sync --frozen --all-extras` in the package; leave it
+out to check the environment as it is. apis-integ installs the generated API
+client, so run `./scripts/generate-clients.sh` before checking it (`make check`
+does).
+
+The check:
+
+- runs one pinned Pyright version, `PYRIGHT_VERSION` in
+  `scripts/rules/pyright.py`, through `uv tool run`;
+- points it at the interpreter of the package's locked environment
+  (`<package>/.venv/bin/python`), so imports resolve against the package's own
+  dependencies;
+- checks the paths in the package's `[tool.pyright] include`. Without an
+  `include`, Pyright would check the whole directory, tests included. The rules
+  guard fails when a package a Pyright rule lists has none, has a
+  `pyrightconfig.json` (which Pyright would read instead), is listed by another
+  Pyright rule, or is missing from the `pyright` job's matrix.
+
+Other `[tool.pyright]` settings can weaken the check without failing anything,
+for example `ignore`, a lower `typeCheckingMode` or a rule set to `"none"`.
+`pyproject.toml` files aren't code-owned, so review a change to them as you
+would a rule change.
+
+### Why one pinned version
+
+Each package's `build` extra also installs Pyright, for editors, and its lock
+file pin can differ from package to package (1.1.409 to 1.1.414 when the check
+was added). CI ignores those pins:
+
+- Pyright's messages change between releases. For the same executor error,
+  1.1.409 prints `Series | Unknown | DataFrame` and 1.1.414 prints
+  `Series | Unknown | Self@DataFrame`, and baseline entries quote messages.
+- The daily dependency update relocks every package with `--upgrade`, so the
+  locked versions move whenever Pyright releases. A dependency update shouldn't
+  change type-check results, just as a ruff release can't (ruff is pinned in
+  `pr.yml`).
+
+To upgrade Pyright, change `PYRIGHT_VERSION` in its own pull request, and fix
+the errors the new version reports.
+
+## Baselines
+
+A ratcheted package's baseline, `rules/baselines/pyright-<package>.txt`, lists
+the errors it may still have, one line per error, sorted:
+
+```text
+<path> :: <Pyright rule> :: <first line of the message>
+```
+
+Entries carry no line numbers, so editing elsewhere in a file doesn't change
+them. The check fails on an error the baseline doesn't list, and on an entry
+for an error that no longer occurs, so each fix removes its entry in the same
+commit and the baseline only shrinks.
+
+After fixing errors:
+
+1. run `python -m scripts.rules pyright <package> --write-baseline` (without a
+   package, it does every ratcheted package). It rewrites the baseline as the
+   errors that occur now, and refuses when one of them isn't already listed;
+2. set the rule's `[rule.baseline] count` in `rules/registry.toml` to the
+   number of entries left.
+
+What stops a baseline growing is the rules guard, which compares counts. It
+fails when the count doesn't match the file, when the count is higher than on
+the base branch, and when an enforced rule becomes ratcheted without a
+`[rule.downgrade] reason`. It doesn't compare entries: a change that fixes one
+error and adds another keeps the count and passes the guard, so code-owner
+review of `rules/` is what catches that. `--write-baseline` refuses an error
+an existing baseline doesn't list, but records every current error when the
+file doesn't exist yet, which is how a package is first ratcheted.
+
+Once a baseline is empty, delete it and make the rule `enforced`.
+
+Baselines are under `rules/`, so changing one needs code-owner approval
+(`sim.process.rule-machinery-code-owned`).
+
+## Fixing errors
+
+- Prefer narrowing a type (an `isinstance` check, an early return) or
+  correcting an annotation to `cast`.
+- Suppress an error only as a last resort, with the rule and the reason on the
+  same line: `# pyright: ignore[reportAttributeAccessIssue]  # <reason>`.
+- If an error shows a real bug, first write a test that demonstrates it, then
+  fix it.
+- Open observability spans with `scopes.span(runtime, ...)` from
+  `policyengine_simulation_observability.scopes`, not `runtime.span(...)`.
+  policyengine-observability types its scope managers as possibly suppressing
+  exceptions, so after `with runtime.span(...)` Pyright reports each variable
+  the block assigned as possibly unbound.