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 5d2b83cf6..7f8b72ad0 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,8 +71,28 @@ the errors the new version reports. ## Baselines -A ratcheted package's 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 :: :: @@ -92,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 @@ -121,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. diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 975c6288c..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`: 4 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/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 deleted file mode 100644 index d220f2a32..000000000 --- a/rules/baselines/pyright-apis-integ.txt +++ /dev/null @@ -1,13 +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. -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/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/rules/registry.toml b/rules/registry.toml index 6d54e4876..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 = 4 [rule.proof] bad_fixture = "rules/fixtures/sim.quality.pyright-apis-integ/bad" good_fixture = "rules/fixtures/sim.quality.pyright-apis-integ/good" 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)