From 4c515bc685a99a37cc0c204c87ffbb4fd9035af7 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 19:47:43 +0400 Subject: [PATCH 1/5] Call the authenticated endpoints with either client in the integ tests The generated sync_detailed of each endpoint that declares bearer authentication accepts only an AuthenticatedClient, but the integration tests pass a plain Client against local services, which run without authentication (4 Pyright errors). endpoint_calls has one function per such endpoint (submit and poll, for annual and budget-window runs). Each takes either client and sends exactly what sync_detailed sends: the endpoint module's _get_kwargs, through the client's own httpx client, parsed by its _build_response. The headers the tests send don't change. A new test records the requests through an httpx mock transport and checks it: with an AuthenticatedClient each function sends sync_detailed's exact request, and with a plain Client the same request without Authorization. The two polling tests that stubbed sync_detailed now stub endpoint_calls. apis-integ now has no Pyright errors, and its baseline is empty. Co-Authored-By: Claude Opus 5.5 --- docs/generated/rules.md | 2 +- .../tests/simulation/conftest.py | 16 +- .../tests/simulation/endpoint_calls.py | 57 ++++++++ .../tests/simulation/test_calculate.py | 26 +--- .../tests/simulation/test_endpoint_calls.py | 138 ++++++++++++++++++ rules/baselines/pyright-apis-integ.txt | 4 - rules/registry.toml | 2 +- 7 files changed, 206 insertions(+), 39 deletions(-) create mode 100644 projects/policyengine-apis-integ/tests/simulation/endpoint_calls.py create mode 100644 projects/policyengine-apis-integ/tests/simulation/test_endpoint_calls.py diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 975c6288c..a43a7eef9 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -66,7 +66,7 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf - `sim.ci.multiline-run-is-script`: 13 remaining in `rules/baselines/ci-multiline-run.txt` - `sim.microdata.weight-stripping-confined`: 0 remaining in `rules/baselines/sim-microdata-weight-stripping.txt` -- `sim.quality.pyright-apis-integ`: 4 remaining in `rules/baselines/pyright-apis-integ.txt` +- `sim.quality.pyright-apis-integ`: 0 remaining in `rules/baselines/pyright-apis-integ.txt` ### Exceptions and review dates diff --git a/projects/policyengine-apis-integ/tests/simulation/conftest.py b/projects/policyengine-apis-integ/tests/simulation/conftest.py index 4903df629..eb29b2991 100644 --- a/projects/policyengine-apis-integ/tests/simulation/conftest.py +++ b/projects/policyengine-apis-integ/tests/simulation/conftest.py @@ -6,16 +6,13 @@ import httpx import pytest from policyengine_api_simulation_client import AuthenticatedClient, Client -from policyengine_api_simulation_client.api.default import ( - get_budget_window_job_status_budget_window_jobs_batch_job_id_get, - submit_budget_window_batch_simulate_economy_budget_window_post, -) from policyengine_api_simulation_client.models import ( BudgetWindowBatchRequest, BudgetWindowBatchStatusResponse, ) from pydantic_settings import BaseSettings, SettingsConfigDict +from . import endpoint_calls from .polling import poll_retry_delay_seconds BUDGET_WINDOW_YEARS = ["2026", "2027"] @@ -97,11 +94,8 @@ def _poll_budget_window_batch( last_status_code: HTTPStatus | None = None last_content = b"" - status_endpoint = get_budget_window_job_status_budget_window_jobs_batch_job_id_get while time.monotonic() < deadline: - response = status_endpoint.sync_detailed( - batch_job_id=batch_job_id, client=client - ) + response = endpoint_calls.budget_window_status(client, batch_job_id) last_status_code = response.status_code last_content = response.content @@ -202,13 +196,9 @@ def decode_response_content(): @pytest.fixture def submit_budget_window_batch(client: Client | AuthenticatedClient): """Submit a budget-window batch through the generated client.""" - submit_endpoint = submit_budget_window_batch_simulate_economy_budget_window_post def submit(request: BudgetWindowBatchRequest): - return submit_endpoint.sync_detailed( - client=client, - body=request, - ) + return endpoint_calls.submit_budget_window(client, request) return submit diff --git a/projects/policyengine-apis-integ/tests/simulation/endpoint_calls.py b/projects/policyengine-apis-integ/tests/simulation/endpoint_calls.py new file mode 100644 index 000000000..b140cf675 --- /dev/null +++ b/projects/policyengine-apis-integ/tests/simulation/endpoint_calls.py @@ -0,0 +1,57 @@ +"""Call the generated client's authenticated endpoints with either client. + +The generated ``sync_detailed`` of an endpoint that declares bearer +authentication accepts only an ``AuthenticatedClient``. Against local services, +which run without authentication, these tests use a plain ``Client`` and send no +``Authorization`` header. Each function here sends exactly what the endpoint's +``sync_detailed`` sends: the endpoint module's request keywords +(``_get_kwargs``), through the given client's own httpx client, with the +response parsed by the module's ``_build_response``. Only the client type it +accepts differs. +""" + +from policyengine_api_simulation_client import AuthenticatedClient, Client +from policyengine_api_simulation_client.api.default import ( + get_budget_window_job_status_budget_window_jobs_batch_job_id_get, + get_job_status_jobs_job_id_get, + submit_budget_window_batch_simulate_economy_budget_window_post, + submit_simulation_simulate_economy_comparison_post, +) +from policyengine_api_simulation_client.models import ( + BudgetWindowBatchRequest, + SimulationRequest, +) + + +def submit_simulation(client: Client | AuthenticatedClient, body: SimulationRequest): + """POST /simulate/economy/comparison.""" + endpoint = submit_simulation_simulate_economy_comparison_post + request = endpoint._get_kwargs(body=body) + response = client.get_httpx_client().request(**request) + return endpoint._build_response(client=client, response=response) + + +def job_status(client: Client | AuthenticatedClient, job_id: str): + """GET /jobs/{job_id}.""" + endpoint = get_job_status_jobs_job_id_get + request = endpoint._get_kwargs(job_id=job_id) + response = client.get_httpx_client().request(**request) + return endpoint._build_response(client=client, response=response) + + +def submit_budget_window( + client: Client | AuthenticatedClient, body: BudgetWindowBatchRequest +): + """POST /simulate/economy/budget-window.""" + endpoint = submit_budget_window_batch_simulate_economy_budget_window_post + request = endpoint._get_kwargs(body=body) + response = client.get_httpx_client().request(**request) + return endpoint._build_response(client=client, response=response) + + +def budget_window_status(client: Client | AuthenticatedClient, batch_job_id: str): + """GET /budget-window-jobs/{batch_job_id}.""" + endpoint = get_budget_window_job_status_budget_window_jobs_batch_job_id_get + request = endpoint._get_kwargs(batch_job_id=batch_job_id) + response = client.get_httpx_client().request(**request) + return endpoint._build_response(client=client, response=response) diff --git a/projects/policyengine-apis-integ/tests/simulation/test_calculate.py b/projects/policyengine-apis-integ/tests/simulation/test_calculate.py index 60de0c923..105d7bcb6 100644 --- a/projects/policyengine-apis-integ/tests/simulation/test_calculate.py +++ b/projects/policyengine-apis-integ/tests/simulation/test_calculate.py @@ -13,10 +13,6 @@ import pytest from policyengine_api_simulation_client import AuthenticatedClient, Client -from policyengine_api_simulation_client.api.default import ( - get_job_status_jobs_job_id_get, - submit_simulation_simulate_economy_comparison_post, -) from policyengine_api_simulation_client.models import ( JobStage, JobStatusResponse, @@ -25,6 +21,7 @@ SimulationRequest, ) +from . import endpoint_calls from .polling import poll_retry_delay_seconds _REQUIRED_ECONOMY_RESULT_SECTIONS = {"budget", "poverty", "inequality"} @@ -96,9 +93,7 @@ def poll_for_completion( deadline = time.monotonic() + max_wait_seconds while time.monotonic() < deadline: - response = get_job_status_jobs_job_id_get.sync_detailed( - job_id=job_id, client=client - ) + response = endpoint_calls.job_status(client, job_id) if bodies is not None and response.status_code in { HTTPStatus.OK, @@ -177,10 +172,7 @@ def submit_simulation_request( client: Client | AuthenticatedClient, request: SimulationRequest, ) -> JobSubmitResponse: - response = submit_simulation_simulate_economy_comparison_post.sync_detailed( - client=client, - body=request, - ) + response = endpoint_calls.submit_simulation(client, request) assert response.status_code == HTTPStatus.OK, ( f"Simulation submit failed with status {response.status_code}: " f"{response.content!r}" @@ -433,7 +425,7 @@ def _poll_with_responses( sleep_seconds: list[float] = [] def get_status( - *, job_id: str, client: Client | AuthenticatedClient + client: Client | AuthenticatedClient, job_id: str ) -> _StubJobStatusHttpResponse: del job_id, client return next(response_iterator) @@ -441,11 +433,7 @@ def get_status( def record_sleep(seconds: float) -> None: sleep_seconds.append(seconds) - monkeypatch.setattr( - get_job_status_jobs_job_id_get, - "sync_detailed", - get_status, - ) + monkeypatch.setattr(endpoint_calls, "job_status", get_status) monkeypatch.setattr(time, "sleep", record_sleep) result = poll_for_completion( cast("Client", object()), @@ -554,9 +542,7 @@ def test_poll_for_completion_records_running_and_complete_bodies( ] ) monkeypatch.setattr( - get_job_status_jobs_job_id_get, - "sync_detailed", - lambda *, job_id, client: next(response_iterator), + endpoint_calls, "job_status", lambda client, job_id: next(response_iterator) ) monkeypatch.setattr(time, "sleep", lambda _seconds: None) diff --git a/projects/policyengine-apis-integ/tests/simulation/test_endpoint_calls.py b/projects/policyengine-apis-integ/tests/simulation/test_endpoint_calls.py new file mode 100644 index 000000000..b57aee6b3 --- /dev/null +++ b/projects/policyengine-apis-integ/tests/simulation/test_endpoint_calls.py @@ -0,0 +1,138 @@ +"""endpoint_calls sends what the generated sync_detailed sends. + +Each request is captured by an httpx mock transport. With an +AuthenticatedClient, the endpoint_calls function and the endpoint's own +sync_detailed must send the same request. With a plain Client, the request is +the same except that it carries no Authorization header. +""" + +from collections.abc import Callable + +import httpx +import pytest +from policyengine_api_simulation_client import AuthenticatedClient, Client +from policyengine_api_simulation_client.api.default import ( + get_budget_window_job_status_budget_window_jobs_batch_job_id_get, + get_job_status_jobs_job_id_get, + submit_budget_window_batch_simulate_economy_budget_window_post, + submit_simulation_simulate_economy_comparison_post, +) +from policyengine_api_simulation_client.models import ( + BudgetWindowBatchRequest, + SimulationRequest, +) + +from . import endpoint_calls + +BASE_URL = "http://simulation.test" +TOKEN = "integration-token" + + +def _recording_transport(requests: list[httpx.Request]) -> httpx.MockTransport: + def respond(request: httpx.Request) -> httpx.Response: + requests.append(request) + return httpx.Response(500, json={"detail": "recorded"}) + + return httpx.MockTransport(respond) + + +def _sent(request: httpx.Request) -> tuple[str, str, dict[str, str], bytes]: + """Return the method, URL, headers and body a request puts on the wire.""" + return (request.method, str(request.url), dict(request.headers), request.content) + + +def _authenticated_client(requests: list[httpx.Request]) -> AuthenticatedClient: + return AuthenticatedClient( + base_url=BASE_URL, + token=TOKEN, + httpx_args={"transport": _recording_transport(requests)}, + ) + + +def _plain_client(requests: list[httpx.Request]) -> Client: + return Client( + base_url=BASE_URL, httpx_args={"transport": _recording_transport(requests)} + ) + + +SIMULATION_REQUEST = SimulationRequest(country="us", scope="macro") +BUDGET_WINDOW_REQUEST = BudgetWindowBatchRequest( + country="us", + region="us", + start_year="2026", + window_size=2, + max_parallel=1, + scope="macro", +) + +# Each endpoint: its endpoint_calls function and its sync_detailed, called with +# the same arguments. +ENDPOINT_CALLS: dict[ + str, + tuple[ + Callable[[Client | AuthenticatedClient], object], + Callable[[AuthenticatedClient], object], + ], +] = { + "submit_simulation": ( + lambda client: endpoint_calls.submit_simulation(client, SIMULATION_REQUEST), + lambda client: submit_simulation_simulate_economy_comparison_post.sync_detailed( + client=client, body=SIMULATION_REQUEST + ), + ), + "job_status": ( + lambda client: endpoint_calls.job_status(client, "fc-job"), + lambda client: get_job_status_jobs_job_id_get.sync_detailed( + job_id="fc-job", client=client + ), + ), + "submit_budget_window": ( + lambda client: endpoint_calls.submit_budget_window( + client, BUDGET_WINDOW_REQUEST + ), + lambda client: ( + submit_budget_window_batch_simulate_economy_budget_window_post.sync_detailed( + client=client, body=BUDGET_WINDOW_REQUEST + ) + ), + ), + "budget_window_status": ( + lambda client: endpoint_calls.budget_window_status(client, "fc-batch"), + lambda client: ( + get_budget_window_job_status_budget_window_jobs_batch_job_id_get.sync_detailed( + batch_job_id="fc-batch", client=client + ) + ), + ), +} + + +@pytest.mark.parametrize("endpoint", sorted(ENDPOINT_CALLS)) +def test_an_authenticated_call_sends_what_sync_detailed_sends(endpoint: str): + support_call, generated_call = ENDPOINT_CALLS[endpoint] + support_requests: list[httpx.Request] = [] + generated_requests: list[httpx.Request] = [] + + support_call(_authenticated_client(support_requests)) + generated_call(_authenticated_client(generated_requests)) + + assert [_sent(request) for request in support_requests] == [ + _sent(request) for request in generated_requests + ] + assert support_requests[0].headers["authorization"] == f"Bearer {TOKEN}" + + +@pytest.mark.parametrize("endpoint", sorted(ENDPOINT_CALLS)) +def test_a_plain_call_sends_the_same_request_without_authorization(endpoint: str): + support_call, generated_call = ENDPOINT_CALLS[endpoint] + plain_requests: list[httpx.Request] = [] + generated_requests: list[httpx.Request] = [] + + support_call(_plain_client(plain_requests)) + generated_call(_authenticated_client(generated_requests)) + + method, url, headers, body = _sent(generated_requests[0]) + del headers["authorization"] + assert [_sent(request) for request in plain_requests] == [ + (method, url, headers, body) + ] diff --git a/rules/baselines/pyright-apis-integ.txt b/rules/baselines/pyright-apis-integ.txt index d220f2a32..6554848fe 100644 --- a/rules/baselines/pyright-apis-integ.txt +++ b/rules/baselines/pyright-apis-integ.txt @@ -7,7 +7,3 @@ # occur with # python -m scripts.rules pyright projects/policyengine-apis-integ --write-baseline # then set the rule's [rule.baseline] count in rules/registry.toml to match. -projects/policyengine-apis-integ/tests/simulation/conftest.py :: reportArgumentType :: Argument of type "Client | AuthenticatedClient" cannot be assigned to parameter "client" of type "AuthenticatedClient" in function "sync_detailed" -projects/policyengine-apis-integ/tests/simulation/conftest.py :: reportArgumentType :: Argument of type "Client | AuthenticatedClient" cannot be assigned to parameter "client" of type "AuthenticatedClient" in function "sync_detailed" -projects/policyengine-apis-integ/tests/simulation/test_calculate.py :: reportArgumentType :: Argument of type "Client | AuthenticatedClient" cannot be assigned to parameter "client" of type "AuthenticatedClient" in function "sync_detailed" -projects/policyengine-apis-integ/tests/simulation/test_calculate.py :: reportArgumentType :: Argument of type "Client | AuthenticatedClient" cannot be assigned to parameter "client" of type "AuthenticatedClient" in function "sync_detailed" diff --git a/rules/registry.toml b/rules/registry.toml index 6d54e4876..8c9253f65 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -1164,7 +1164,7 @@ job = "pyright" invocation = "python -m scripts.rules pyright" [rule.baseline] file = "rules/baselines/pyright-apis-integ.txt" -count = 4 +count = 0 [rule.proof] bad_fixture = "rules/fixtures/sim.quality.pyright-apis-integ/bad" good_fixture = "rules/fixtures/sim.quality.pyright-apis-integ/good" From e089bb6b410763633e9c4b31a9fc34cdfb087c86 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 19:48:02 +0400 Subject: [PATCH 2/5] Prove the apis-integ Pyright rule as an enforced rule As for the gateway and executor: the bad fixture, the integration tests' real report from main at ceb30e17, now fails with every error new and no baseline, and the good fixture is a clean report recorded from the fixed tests. Co-Authored-By: Claude Opus 5.5 --- .../bad/baseline.txt | 4 -- .../good/baseline.txt | 3 -- .../good/pyright.json | 39 ++----------------- scripts/rules/tests/test_pyright.py | 20 ++++------ 4 files changed, 10 insertions(+), 56 deletions(-) delete mode 100644 rules/fixtures/sim.quality.pyright-apis-integ/bad/baseline.txt delete mode 100644 rules/fixtures/sim.quality.pyright-apis-integ/good/baseline.txt diff --git a/rules/fixtures/sim.quality.pyright-apis-integ/bad/baseline.txt b/rules/fixtures/sim.quality.pyright-apis-integ/bad/baseline.txt deleted file mode 100644 index 11a2c9887..000000000 --- a/rules/fixtures/sim.quality.pyright-apis-integ/bad/baseline.txt +++ /dev/null @@ -1,4 +0,0 @@ -# Proof fixture: the baseline omits the test_calculate.py error in -# pyright.json, and lists the conftest.py error twice although it occurs once. -projects/policyengine-apis-integ/tests/simulation/conftest.py :: reportArgumentType :: Argument of type "Client | AuthenticatedClient" cannot be assigned to parameter "client" of type "AuthenticatedClient" in function "sync_detailed" -projects/policyengine-apis-integ/tests/simulation/conftest.py :: reportArgumentType :: Argument of type "Client | AuthenticatedClient" cannot be assigned to parameter "client" of type "AuthenticatedClient" in function "sync_detailed" diff --git a/rules/fixtures/sim.quality.pyright-apis-integ/good/baseline.txt b/rules/fixtures/sim.quality.pyright-apis-integ/good/baseline.txt deleted file mode 100644 index 94c3a30d2..000000000 --- a/rules/fixtures/sim.quality.pyright-apis-integ/good/baseline.txt +++ /dev/null @@ -1,3 +0,0 @@ -# Proof fixture: the baseline lists exactly the errors in pyright.json. -projects/policyengine-apis-integ/tests/simulation/conftest.py :: reportArgumentType :: Argument of type "Client | AuthenticatedClient" cannot be assigned to parameter "client" of type "AuthenticatedClient" in function "sync_detailed" -projects/policyengine-apis-integ/tests/simulation/test_calculate.py :: reportArgumentType :: Argument of type "Client | AuthenticatedClient" cannot be assigned to parameter "client" of type "AuthenticatedClient" in function "sync_detailed" diff --git a/rules/fixtures/sim.quality.pyright-apis-integ/good/pyright.json b/rules/fixtures/sim.quality.pyright-apis-integ/good/pyright.json index e1f570448..661446e21 100644 --- a/rules/fixtures/sim.quality.pyright-apis-integ/good/pyright.json +++ b/rules/fixtures/sim.quality.pyright-apis-integ/good/pyright.json @@ -1,42 +1,9 @@ { "version": "1.1.414", - "generalDiagnostics": [ - { - "file": "/repository/projects/policyengine-apis-integ/tests/simulation/conftest.py", - "severity": "error", - "message": "Argument of type \"Client | AuthenticatedClient\" cannot be assigned to parameter \"client\" of type \"AuthenticatedClient\" in function \"sync_detailed\"\n  Type \"Client | AuthenticatedClient\" is not assignable to type \"AuthenticatedClient\"\n    \"Client\" is not assignable to \"AuthenticatedClient\"", - "range": { - "start": { - "line": 102, - "character": 46 - }, - "end": { - "line": 102, - "character": 52 - } - }, - "rule": "reportArgumentType" - }, - { - "file": "/repository/projects/policyengine-apis-integ/tests/simulation/test_calculate.py", - "severity": "error", - "message": "Argument of type \"Client | AuthenticatedClient\" cannot be assigned to parameter \"client\" of type \"AuthenticatedClient\" in function \"sync_detailed\"\n  Type \"Client | AuthenticatedClient\" is not assignable to type \"AuthenticatedClient\"\n    \"Client\" is not assignable to \"AuthenticatedClient\"", - "range": { - "start": { - "line": 98, - "character": 34 - }, - "end": { - "line": 98, - "character": 40 - } - }, - "rule": "reportArgumentType" - } - ], + "generalDiagnostics": [], "summary": { - "filesAnalyzed": 10, - "errorCount": 2, + "filesAnalyzed": 12, + "errorCount": 0, "warningCount": 0, "informationCount": 0 } diff --git a/scripts/rules/tests/test_pyright.py b/scripts/rules/tests/test_pyright.py index 7fa74dc8c..f45f19dbf 100644 --- a/scripts/rules/tests/test_pyright.py +++ b/scripts/rules/tests/test_pyright.py @@ -225,24 +225,18 @@ def test_rule_proof_pyright_executor(self) -> None: def test_rule_proof_pyright_apis_integ(self) -> None: rule_id = "sim.quality.pyright-apis-integ" - client_mismatch = ( - 'Argument of type "Client | AuthenticatedClient" cannot be assigned to ' - 'parameter "client" of type "AuthenticatedClient" in function ' - '"sync_detailed"' - ) + # The integration tests' real errors before their fix: a plain Client + # passed to endpoints typed as needing an AuthenticatedClient. bad = proof_comparison(rule_id, "bad") self.assertEqual( [(error.path, error.line) for error in bad.new_errors], - [(f"{APIS_INTEG_TESTS}/test_calculate.py", 99)], - ) - self.assertEqual( - bad.stale_entries, - ( - f"{APIS_INTEG_TESTS}/conftest.py :: reportArgumentType :: " - f"{client_mismatch}", - ), + [ + (f"{APIS_INTEG_TESTS}/conftest.py", 103), + (f"{APIS_INTEG_TESTS}/test_calculate.py", 99), + ], ) + self.assertEqual(bad.stale_entries, ()) self.assertTrue(proof_comparison(rule_id, "good").passed) From 06e4ea871cdf07d2c6215b0259dcfebb9526baba Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 19:48:41 +0400 Subject: [PATCH 3/5] Enforce Pyright for apis-integ, and drop the last Pyright baseline The integration tests have no Pyright errors, so sim.quality.pyright-apis-integ moves from ratcheted to enforced and its empty baseline goes. Every package's Pyright rule is now enforced; the type-checking skill keeps the baseline mechanism for a future upgrade or package that needs one. Co-Authored-By: Claude Opus 5.5 --- docs/engineering/skills/type-checking.md | 15 ++++++++------- docs/generated/rules.md | 7 +++---- rules/baselines/pyright-apis-integ.txt | 9 --------- rules/registry.toml | 9 +++------ 4 files changed, 14 insertions(+), 26 deletions(-) delete mode 100644 rules/baselines/pyright-apis-integ.txt diff --git a/docs/engineering/skills/type-checking.md b/docs/engineering/skills/type-checking.md index 5d2b83cf6..ff43f656b 100644 --- a/docs/engineering/skills/type-checking.md +++ b/docs/engineering/skills/type-checking.md @@ -9,12 +9,11 @@ package. 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`, `sim.quality.pyright-gateway` and - `sim.quality.pyright-executor`: the contract, observability, Stage 12 - persistence and fastapi libraries, the simulation entry, the gateway and the - executor have no Pyright errors. -- `sim.quality.pyright-apis-integ`: the integration tests may have only the - errors their baseline lists (see [Baselines](#baselines)). +- `sim.quality.pyright-libraries-and-entry`, `sim.quality.pyright-gateway`, + `sim.quality.pyright-executor` and `sim.quality.pyright-apis-integ`: the + contract, observability, Stage 12 persistence and fastapi libraries, the + simulation entry, the gateway, the executor and the generated-client + integration tests have no Pyright errors. - `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 @@ -72,7 +71,9 @@ the errors the new version reports. ## Baselines -A ratcheted package's baseline, `rules/baselines/pyright-.txt`, lists +No package has a Pyright baseline now. If a Pyright upgrade or a new package +brings errors that can't be fixed in the same pull request, a package's rule +can be ratcheted: its baseline, `rules/baselines/pyright-.txt`, lists the errors it may still have, one line per error, sorted: ```text diff --git a/docs/generated/rules.md b/docs/generated/rules.md index a43a7eef9..081a8b87e 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,7 +8,7 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 52 | 3 | 0 | 14 | 69 | +| mechanical | 53 | 2 | 0 | 14 | 69 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 0 | 0 | @@ -66,7 +66,6 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf - `sim.ci.multiline-run-is-script`: 13 remaining in `rules/baselines/ci-multiline-run.txt` - `sim.microdata.weight-stripping-confined`: 0 remaining in `rules/baselines/sim-microdata-weight-stripping.txt` -- `sim.quality.pyright-apis-integ`: 0 remaining in `rules/baselines/pyright-apis-integ.txt` ### Exceptions and review dates @@ -288,7 +287,7 @@ flowchart LR | `sim.quality.deployed-tests-ruff-exemptions` | The deployed-service test suite (projects/*/authenticated_tests, pytest run against a live candidate) gets the same ruff exemptions as unit tests (sim.quality.tests-ruff-exemptions); every other ruff rule applies to it. | mechanical | enforced | yes | ruff per-file ignore of ANN, ARG001, ARG002, ARG005, D1, DTZ001, DTZ007, INP001, PLC0415, PLR2004, S101, S102, S105, S106, S108, S301, S603, S607, SLF001, TID253 for `**/authenticated_tests/**` | `pr.yml:lint` | — | | `sim.quality.every-package-type-checked` | Every uv package under libs/ and projects/ (a directory with a pyproject.toml) is listed by a Pyright rule, so the pyright CI job type-checks it. | mechanical | enforced | yes | quality guard `pyright-coverage` | `pr.yml:quality-guards` | yes | | `sim.quality.every-project-tested` | Every project and library with unit tests runs them in the PR test matrix. | mechanical | doc-only | no | nothing | — | — | -| `sim.quality.pyright-apis-integ` | The generated-client integration-test project, apis-integ, has no Pyright errors beyond those its baseline lists, and the baseline lists only errors that still occur, so it only shrinks; checked with the pinned Pyright version in the project's own locked environment. | mechanical | ratcheted | yes | pyright in `projects/policyengine-apis-integ` | `pr.yml:pyright` | yes | +| `sim.quality.pyright-apis-integ` | The generated-client integration-test project, apis-integ, has no Pyright errors, checked with the pinned Pyright version in the project's own locked environment. | mechanical | enforced | yes | pyright in `projects/policyengine-apis-integ` | `pr.yml:pyright` | yes | | `sim.quality.pyright-executor` | The simulation executor has no Pyright errors, checked with the pinned Pyright version in the executor's own locked environment. | mechanical | enforced | yes | pyright in `projects/policyengine-simulation-executor` | `pr.yml:pyright` | yes | | `sim.quality.pyright-gateway` | The simulation gateway has no Pyright errors, checked with the pinned Pyright version in the gateway's own locked environment. | mechanical | enforced | yes | pyright in `projects/policyengine-simulation-gateway` | `pr.yml:pyright` | yes | | `sim.quality.pyright-libraries-and-entry` | The contract, observability, Stage 12 persistence and fastapi libraries and the simulation entry have no Pyright errors, checked with the pinned Pyright version in each package's own locked environment. | mechanical | enforced | yes | pyright in `libs/policyengine-simulation-contract`, `libs/policyengine-simulation-observability`, `libs/policyengine-stage12-persistence`, `libs/policyengine-fastapi`, `projects/policyengine-simulation-entry` | `pr.yml:pyright` | yes | @@ -315,7 +314,7 @@ flowchart LR j_CI_pr_yml_quality_guards["CI pr.yml:quality-guards"] r_sim_quality_every_project_tested("sim.quality.every-project-tested (doc-only)") gap_unenforced>"not enforced"] - r_sim_quality_pyright_apis_integ("sim.quality.pyright-apis-integ (ratcheted)") + r_sim_quality_pyright_apis_integ("sim.quality.pyright-apis-integ (enforced)") e_pyright_in_projects_policyengine_apis_integ["pyright in projects/policyengine-apis-integ"] j_CI_pr_yml_pyright["CI pr.yml:pyright"] r_sim_quality_pyright_executor("sim.quality.pyright-executor (enforced)") diff --git a/rules/baselines/pyright-apis-integ.txt b/rules/baselines/pyright-apis-integ.txt deleted file mode 100644 index 6554848fe..000000000 --- a/rules/baselines/pyright-apis-integ.txt +++ /dev/null @@ -1,9 +0,0 @@ -# Tolerated Pyright errors in projects/policyengine-apis-integ. -# This list may only shrink. -# -# Rule sim.quality.pyright-apis-integ. Each entry is -# :: :: , one line per -# error, sorted. After fixing errors, remove the entries that no longer -# occur with -# python -m scripts.rules pyright projects/policyengine-apis-integ --write-baseline -# then set the rule's [rule.baseline] count in rules/registry.toml to match. diff --git a/rules/registry.toml b/rules/registry.toml index 8c9253f65..be8a3155d 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -1149,11 +1149,11 @@ invocation = "unittest discover -s scripts/rules/tests" [[rule]] id = "sim.quality.pyright-apis-integ" category = "quality" -statement = "The generated-client integration-test project, apis-integ, has no Pyright errors beyond those its baseline lists, and the baseline lists only errors that still occur, so it only shrinks; checked with the pinned Pyright version in the project's own locked environment." -rationale = "docs/engineering/skills/type-checking.md#baselines" +statement = "The generated-client integration-test project, apis-integ, has no Pyright errors, checked with the pinned Pyright version in the project's own locked environment." +rationale = "docs/engineering/skills/type-checking.md#rules" scope = ["projects/policyengine-apis-integ/tests/**/*.py", "projects/policyengine-apis-integ/policyengine_api_full_integ/*.py"] tier = "mechanical" -status = "ratcheted" +status = "enforced" autofix = false [rule.enforcer] kind = "pyright" @@ -1162,9 +1162,6 @@ projects = ["projects/policyengine-apis-integ"] workflow = ".github/workflows/pr.yml" job = "pyright" invocation = "python -m scripts.rules pyright" -[rule.baseline] -file = "rules/baselines/pyright-apis-integ.txt" -count = 0 [rule.proof] bad_fixture = "rules/fixtures/sim.quality.pyright-apis-integ/bad" good_fixture = "rules/fixtures/sim.quality.pyright-apis-integ/good" From 8aca648fbea698e8ea4d61c9211e51ac103eec72 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 21 Oct 2026 20:28:28 +0400 Subject: [PATCH 4/5] Say that ratcheting a Pyright rule again needs a downgrade reason With every Pyright rule enforced, the type-checking skill describes ratcheting one again for a future upgrade or package. It now says that needs a [rule.downgrade] reason, which the guard requires for a move from enforced to ratcheted. Co-Authored-By: Claude Opus 5.5 --- docs/engineering/skills/type-checking.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/engineering/skills/type-checking.md b/docs/engineering/skills/type-checking.md index ff43f656b..69180372a 100644 --- a/docs/engineering/skills/type-checking.md +++ b/docs/engineering/skills/type-checking.md @@ -73,7 +73,8 @@ the errors the new version reports. No package has a Pyright baseline now. If a Pyright upgrade or a new package brings errors that can't be fixed in the same pull request, a package's rule -can be ratcheted: its baseline, `rules/baselines/pyright-.txt`, lists +can be ratcheted, which for an enforced rule needs a `[rule.downgrade] reason` +in the registry. Its baseline, `rules/baselines/pyright-.txt`, lists the errors it may still have, one line per error, sorted: ```text From 12592808d808c13cfc93d7c90b7727dba1225e65 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:31:42 +0400 Subject: [PATCH 5/5] Say that weakening a rule is a person's decision, not an agent's The type-checking guide described ratcheting a Pyright rule as an option whenever errors couldn't be fixed in the same pull request, which an agent could read as permission to do it. The guide, the rules-registry guide and AGENTS.md now tell agents to stop and report errors they can't fix instead of downgrading a rule, creating or growing a baseline, or moving a package out of a rule. The type-checking guide also said changing a baseline needs code-owner approval. sim.process.rule-machinery-code-owned is doc-only and merging doesn't require that approval, so the guide now says what the rules guard doesn't check and that review has to catch it. It also drops the HouseholdFrame protocol, which #873 removed. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 5 ++ docs/engineering/skills/rules-registry.md | 9 +++- docs/engineering/skills/type-checking.md | 66 ++++++++++++++++------- 3 files changed, 59 insertions(+), 21 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index aa0bf0098..956710160 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,6 +32,11 @@ When adding, changing, or removing a rule, a guard test, a checker configuration, or a CI job that enforces one, read `docs/engineering/skills/rules-registry.md`. +Never weaken a rule yourself: don't downgrade a rule's status, add a +`[rule.downgrade]` section, create or grow a baseline, or move a package or +path out of a rule's scope. When a rule reports violations you can't fix, stop +and report them to a person, who decides whether to relax the rule. + Before adding a Pydantic model or a class that other models inherit from, or moving an import under `if TYPE_CHECKING:`, read `docs/engineering/skills/typing-only-imports.md`. diff --git a/docs/engineering/skills/rules-registry.md b/docs/engineering/skills/rules-registry.md index b81892a91..d174eb842 100644 --- a/docs/engineering/skills/rules-registry.md +++ b/docs/engineering/skills/rules-registry.md @@ -127,7 +127,14 @@ fail the guard yet. `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, or from enforced - to ratcheted, needs an explicit `[rule.downgrade] reason`. + to ratcheted, needs an explicit `[rule.downgrade] reason`. The guard accepts + any non-empty reason; the reason is for the reviewer. +- **Weakening a rule is a decision for a person, not an agent.** Agents don't + downgrade a rule, add a `[rule.downgrade]` section, create a baseline or add + entries to one, or move a package or path out of a rule's scope. When a rule + reports violations an agent can't fix in its pull request, the agent stops + and reports them, with what it tried. Fixing violations and shrinking a + baseline is fine. ## File-level exceptions diff --git a/docs/engineering/skills/type-checking.md b/docs/engineering/skills/type-checking.md index 69180372a..7f8b72ad0 100644 --- a/docs/engineering/skills/type-checking.md +++ b/docs/engineering/skills/type-checking.md @@ -71,11 +71,28 @@ the errors the new version reports. ## Baselines -No package has a Pyright baseline now. If a Pyright upgrade or a new package -brings errors that can't be fixed in the same pull request, a package's rule -can be ratcheted, which for an enforced rule needs a `[rule.downgrade] reason` -in the registry. Its baseline, `rules/baselines/pyright-.txt`, lists -the errors it may still have, one line per error, sorted: +No package has a Pyright baseline now: every Pyright rule is enforced. + +### When you can't fix Pyright errors + +Weakening a Pyright rule is a decision for a person, not an agent. If Pyright +reports errors you can't fix in your pull request, including after a Pyright +upgrade or in a new package, stop and report them: list the errors and say +what you tried. Don't: + +- change a Pyright rule's `status` from `enforced`, or add a + `[rule.downgrade]` section; +- create a file under `rules/baselines/`, or add entries to one; +- move a package out of an enforced rule's `enforcer.projects`, or into a new + rule; +- weaken a package's `[tool.pyright]` settings (see above). + +Fixing a ratcheted package's errors and shrinking its baseline is fine. + +### How a baseline works + +A ratcheted rule's baseline, `rules/baselines/pyright-.txt`, lists the +errors the package may still have, one line per error, sorted: ```text :: :: @@ -94,19 +111,29 @@ After fixing errors: 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. +What stops a baseline growing is the rules guard, which compares each existing +rule's status and baseline count with the base branch. 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 check: -Once a baseline is empty, delete it and make the rule `enforced`. +- what the reason says: any non-empty reason passes; +- which entries changed: a change that fixes one error and adds another keeps + the count and passes; +- which packages a rule lists: moving a package from an enforced rule into a + new ratcheted rule passes without a reason, because a new rule has no status + on the base branch to compare with. + +Review is what catches these. `.github/CODEOWNERS` assigns `rules/` to the core +developers, but `sim.process.rule-machinery-code-owned` is doc-only: merging +doesn't require their approval. Whoever reviews a pull request that changes a +baseline, a rule's status or a rule's `enforcer.projects` checks it. -Baselines are under `rules/`, so changing one needs code-owner approval -(`sim.process.rule-machinery-code-owned`). +When a person decides to ratchet a package's rule, `--write-baseline` creates +its baseline: it refuses an error an existing baseline doesn't list, but +records every current error when the file doesn't exist yet. + +Once a baseline is empty, delete it and make the rule `enforced`. ## Fixing errors @@ -123,7 +150,6 @@ Baselines are under `rules/`, so changing one needs code-owner approval managers as possibly suppressing exceptions, so after `with runtime.span(...)` Pyright reports each variable the block assigned as possibly unbound. - Before reading a simulation's output frames, narrow - `.output_dataset.data` with the `EntityFrames` or - `HouseholdFrame` protocols in `policyengine_simulation_executor.output_frames`, - and keep reading through that expression: the microdata weight checks - recognise weighted frames by it. + `.output_dataset.data` with the `EntityFrames` protocol in + `policyengine_simulation_executor.output_frames`, and keep reading through + that expression: the microdata weight checks recognise weighted frames by it.