Repository navigation
Stage 12A: Check Auth0 permissions on Simulation Entrypoint routes - #758
Merged
Merged
Conversation
Protected handlers now receive the validated CallerIdentity through a require_permission dependency instead of discarding it. Submission routes require simulation:submit, polling routes require simulation:read, and the temporary Stage 12 routes require stage12:operate. A permission may come from the permissions claim or the space-delimited scope claim. SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS accepts exactly 0 or 1 and defaults to 0. In log-only mode a missing permission emits simulation_entry_permission_missing with the permission and the token's azp, then allows the request. With 1 the request is rejected with 403. Disabled authentication has no identity to check and stays allowed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The deployment workflow passes each environment's SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS variable to the entrypoint. An unset or empty value deploys 0, and any value other than 0 or 1 fails before Cloud Run changes. Every authenticated deployment test run receives the same value. When it is 1, a new check confirms that the deployed test client's valid token is rejected on a stage12:operate route; otherwise the check skips. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Describe the route permissions, the log-only and enforced modes, the Auth0 RBAC settings and client grants required before enforcement, and the rollout and rollback steps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 12, 2026
Merged
Merged
Closed
A route permission now comes from the RBAC permissions claim for every token, but from the space-delimited scope claim only when the token was issued to a machine client (gty is client-credentials or the subject ends in @Clients). Without RBAC on the Auth0 API, a user token's scope carries any API scope the user's application requested, so it is not proof of a grant. This matters once a public sign-in application exists in the same tenant. CallerIdentity now parses gty, and the verified-token tests use a realistic client-credentials token. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CallerIdentity typed gty as a string, so a token whose gty was a list failed validation and was rejected with 403 even in log-only mode, where main had ignored the claim. It is now read as a tuple of grant types: a string or a list of strings, with any other shape ignored. The claim only decides whether scope counts, so ignoring a malformed value can only narrow what a token grants. New tests send signed tokens through JWTDecoder: user tokens with scope but no permissions are rejected under enforcement whatever their grant type, list-valued and malformed gty claims still authenticate, and a listed client-credentials grant still counts scope. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Beta never promotes its candidate revision, so a beta enforcement value applies only to the candidate that the deployment tests call, and log-only events from beta cannot reveal a missing API v1 grant. The rollout steps now watch prod for those events, note that no deployment test sends API v1's own token, and name the production-only workflow that can redeploy the entrypoint for a faster rollback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CallerIdentity allowed population by field name, so a verified token with no gty but a top-level grant_types claim counted as a client-credentials token and had its scope trusted, and a subject claim could stand in for sub (an @Clients subject marks a machine client). Nothing builds CallerIdentity by field name, so drop populate_by_name: sub, iss, aud, exp, iat, azp and gty are now read only by those claim names. Tests sign tokens that carry grant_types (with a user sub) or subject in place of the registered claims, and check that neither grants scope permissions, while a registered @Clients sub still does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oint-permissions # Conflicts: # projects/policyengine-simulation-entry/tests/test_deployment_assets.py
Main's rules registry flags guard tests no rule describes. Register the three Simulation Entrypoint permission-enforcement deployment tests as sim.deploy.permission-enforcement-wired. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oint-permissions # Conflicts: # docs/generated/rules.md
…oint-permissions # Conflicts: # docs/generated/rules.md
Brings in #765 (ruff check in CI with the first group of rule families), #809 (policyengine 6.4.1) and #791 (redacted Stage 12 failure cause). Conflicts: - auth.py: kept this branch's Callable and Literal imports and sorted the stdlib import block the way main's newly enabled I rule expects. - docs/generated/rules.md: regenerated with python -m scripts.rules render from the auto-merged registry, which holds main's enforced sim.quality.ruff-lint and this branch's sim.deploy.permission-enforcement-wired (34 of 48 rules blocking). ruff check . passes under main's ruff.toml with ruff 0.15.22, so this branch's own code needed no further changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 16, 2026
* Annotate policyengine-fastapi for ruff ANN Add the missing parameter and return annotations. AppExit forwards its callback arguments as object rather than Any, the JWT decoder's private validator returns the same dict type its caller declares, and the metrics middleware types call_next as Starlette's RequestResponseEndpoint. Ping endpoints already pass response_model explicitly, so their new return annotations do not change the generated responses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Annotate the simulation contract library for ruff ANN Type the SPM resolution and validation entry points, the gateway request pre-validators (object in, object out) and the Modal Dict store helpers. The three wrap model serializers in spm.py stay unannotated with a reasoned noqa: Pydantic turns a serializer's return annotation into the model's serialization JSON schema, which would rewrite the OpenAPI spec. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Annotate the observability and Stage 12 persistence libraries for ruff ANN Replace Any with object for values that are only inspected or passed through, and type the persistence row helpers with SQLAlchemy's RowMapping, Table and ColumnElement (imported for type checking only). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Annotate the simulation gateway for ruff ANN Every annotated route passes response_model explicitly, so FastAPI keeps using it and the OpenAPI golden file is unchanged. _bundle_payload keeps Any for **dump_kwargs, forwarded unchanged to BaseModel.model_dump, with a reasoned noqa. Test fixtures outside tests/ are annotated too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Annotate the simulation entry service for ruff ANN Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Annotate the simulation executor for ruff ANN Type the runtime, output builders, Modal entrypoints, scripts and the test fixtures that live outside tests/. PolicyEngine, Google Cloud Storage and observability types are imported for type checking only, country modules are ModuleType, and duck-typed analysis values that tests replace with SimpleNamespace are object. _try_compute_output uses a ParamSpec so its arguments are checked against the wrapped function. The in-repo src/modal package shadows the Modal library here, so Modal types stay quoted or behind TYPE_CHECKING. _image_setup.py may not import anything at module level, so it uses a local TYPE_CHECKING flag. The RemoteFunction protocol keeps Any (with a reasoned noqa) because it mirrors modal.Function.remote and spawn, whose payloads vary per function. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Enable ruff group 3 (annotations) Select ANN. Tests are exempt, alongside the docstring, magic-number and assert rules a later group will enable. scripts/rules findings are left for the next sync from policyengine-api. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Annotate the package-update script and the Stage 12 proof fixtures _json_from_stdin returns object (every caller narrows it), and the proof fixtures annotate their SQLAlchemy connection and table parameters. The schema-authority proof reads the fixtures as text, so it is unaffected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Sync the annotated rules tool from policyengine-api Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register ANN and the test-file ruff exemptions sim.quality.ruff-lint now includes ANN, and the **/tests/** per-file ignore list (S101, D1, ANN, PLR2004) is registered as sim.quality.tests-ruff-exemptions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Annotate the routing and bundle helpers #743 and #744 added #743 and #744 landed on main after this stack branched. ANN, which this PR enables, wants annotations on #744's routing-store, publication-journal and active-state helpers and on #743's bundle dataset-digest helper. The Modal store annotations are quoted, as in the gateway, because the executor tests replace the modal module with a stub. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Annotate the shared object store fake #787 added #787 landed on main after this stack branched. ANN, which this PR enables, wants annotations on the in-memory object store it moved into the executor's fixtures. They match the byte interface of the real ArtifactStore it stands in for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Annotate the segmented national child calls #795 added The respawn helpers take and return the Modal FunctionCall that Function.spawn returns, rather than Any. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Annotate the permission checks #758 added #758 landed on main after this stack branched. ANN, which this PR enables, flags three of its additions: - CallerAuthorizer.__init__ returns None; - the gty before-validator takes the raw claim as object, as the project's other before-validators do, since it narrows the value with isinstance checks; - the deployed permission-enforcement test returns None. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Annotate the job progress code #781 added #781 landed on main after this stack branched. ANN, which this PR enables, flags six of its additions: - the contract's job progress store returns a modal.Dict, as the budget-window stores do; - the Modal app's reporter factory returns a JobProgressReporter, and the stage-duration recorder takes the runtime and that reporter; - StoredJobProgressReporter.__init__ returns None; - the calculation-start hook takes the StageReportingSimulation whose on_first_ensure it calls. The new annotation-only imports sit under TYPE_CHECKING. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 16, 2026
* Document the simulation contract library for ruff D Validators, state helpers and URI functions get docstrings. Every Pydantic model and enum whose class docstring would become a JSON-schema description carries a D101 noqa instead, so the published contract schemas and the OpenAPI documents built from them stay byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the FastAPI support library for ruff D The ping and health models and the /ping/alive route are mounted in the executor app, so they keep their existing (OpenAPI-visible) docstrings under noqa. The no-op observability methods keep their explicit return None under RET501 noqa, and two stray string statements in the GCP module become the docstrings they were meant to be. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the simulation observability library for ruff D Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the Stage 12 persistence library for ruff D Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the simulation entrypoint for ruff D The entry routes are nested in create_app, so their docstrings (the OpenAPI descriptions) are untouched and the generated spec is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the simulation gateway for ruff D The /ping router endpoint keeps its OpenAPI description under a D205 noqa; the golden OpenAPI test still passes. Two placeholder pass statements in the test fixtures' mock exception classes go now that the classes have docstrings (PIE790). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the simulation executor for ruff D Covers the executor package, the Modal apps, fixtures, scripts and test docstrings. Docstrings are added to several Modal functions; images copy src/modal into a layer, so this changes those layers like any source edit. Protocol stubs keep their ... bodies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the integration tests, repo scripts and package markers for ruff D Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the Stage 12 schema-DDL proof fixtures for ruff D The schema-authority proof reads the fixtures as text; the new docstrings avoid DDL phrases, so the bad fixture is still rejected and the good one accepted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Enable ruff group 4 (docstrings) Select pydocstyle (D) and ignore D203 and D213, the incompatible halves of the D203/D211 and D212/D213 pairs. Tests stay exempt from D1 only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Drop redundant return None from documented no-op methods With a docstring as the body, the bare return None lines are redundant; remove them instead of suppressing RET501. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document published contract models and routes Replace the 61 D101 suppressions on contract, ping and health models with class docstrings written for API consumers, and fix the /ping/alive and gateway /ping route docstrings instead of suppressing D205. Pydantic and FastAPI copy these into schema and operation descriptions, so the entry, gateway and executor OpenAPI documents gain descriptions; paths, operations, properties, types and required lists are unchanged. The gateway golden spec is regenerated with generate_openapi. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Use the Public API's docstrings in the shared rules tool Both repos documented scripts/rules independently for ruff D, so the copies drifted apart. Take the Public API's wording so the tool stays byte-identical in both repos; only docstrings differ. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register D in the ruff-lint rule sim.quality.ruff-lint now lists D, and the rendered rules page and AGENTS.md Rules block are regenerated for the new tool hash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #767 added in the imperative mood #767 landed on main after this stack branched. D401, which this PR enables, wants each docstring's summary line in the imperative mood. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #743 and #744 added in the D style #743 and #744 landed on main after this stack branched. D, which this PR enables, flags their new code: blank lines after docstrings (D202), a two-line summary (D205, D209), summaries that are not in the imperative mood (D401), an escaped backslash in the sentinel's module docstring (D301), and three public functions without docstrings (D103). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #789 added in the D style #789 landed on main after this stack branched. D, which this PR enables, flags blank lines after its new docstrings (D202) and the undocumented __call__ of its ConnectionOpener protocol (D102). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the shared object store fake #787 added #787 landed on main after this stack branched. D, which this PR enables, flags the undocumented __init__ (D107), upload_bytes and read_bytes (D102) of the in-memory object store it moved into the executor's fixtures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #795 added in the D style D, which this PR enables, flags the undocumented group_label property (D102), the budget check's run-on summary (D205, D209) and two test helpers' summaries that are not in the imperative mood (D401). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #791 added in the D style #791 landed on main after this stack branched. D, which this PR enables, flags blank lines after five of its new docstrings (D202) and the undocumented __init__ (D107) and stage method (D102) of its StageProgress tracker. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #758 added in the D style #758 landed on main after this stack branched. D, which this PR enables, flags its permission checks and deployed permission test: - CallerAuthorizer's initializer and authorize now say what enforcement does with a caller that lacks a permission, and that the missing permission is always recorded; - the permission_enforcement fixture says when it skips and when it raises, and the enforced-permission deployment test says what it checks; - the blank lines after the docstrings of is_client_credentials, granted_permissions, the gty validator and the Cloud Run deployment-environment test helper go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #781 added in the D style #781 landed on main after this stack branched. D, which this PR enables, flags its job progress code: - the JobProgressReporter protocol and each of its methods say which stage boundary the runtime reports; the null reporter's methods say what they discard, with the docstring as the body; - StoredJobProgressReporter's initializer, record, start, set_execution, enter, start_segments, segment_completed and attempts say how the record changes, including when a call changes nothing; - the contract's job progress store functions say what they read and write, and StageReportingSimulation and its ensure say when the callback runs; - the floor and maximum helpers, the Modal reporter factory, the published-record helper and the gateway's stage-field helper are in the imperative mood, and blank lines after docstrings go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 16, 2026
* Fix ruff TRY, EM and BLE findings in the FastAPI library Exception messages move into a msg variable before the raise (EM), with the text unchanged. The JWT decoder re-raises with a bare raise (TRY201). Its two JWKS lookup error logs stay traceback-free under a TRY400 noqa, because callers control the token and a traceback per bad token would let them flood the logs. The token decode and the GCP project-id probe keep except Exception under a BLE001 noqa: any decode failure must become a 403, and any probe failure falls back to GOOGLE_CLOUD_PROJECT. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff TRY and EM findings in the simulation contract library Exception messages move into a msg variable before the raise (EM), with the text unchanged; Pydantic validators keep raising ValueError, so callers still get the same ValidationError. resolve_spm_selection and validate_spm_result move their try bodies into private helpers so no raise sits inside the try (TRY301, TRY300); the same ValueErrors still become the same SPMInputError, chained from the original. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff TRY, EM and BLE findings in the simulation observability library Exception messages move into a msg variable before the raise (EM), with the text unchanged. log_and_redact_exception's fallback log calls logger.exception with the same explicit exc_info, so its output is unchanged (TRY400). Its telemetry catch-all keeps except Exception under a BLE001 noqa, because the helper must never raise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff EM findings in the Stage 12 persistence library Exception messages move into a msg variable before the raise, with the text unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff TRY, EM and BLE findings in the simulation entrypoint Exception messages move into a msg variable before the raise (EM), with the text unchanged. forward() returns the proxied response from an else block (TRY300). The four Stage 12 diagnostic error logs stay traceback-free under a TRY400 noqa: three record only the error type, so a traceback could carry request or calculation data, and the dispatch timeout is fully described by its timeout. The Stage 12 context capture keeps its telemetry catch-all under a BLE001 noqa. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff TRY, EM and BLE findings in the simulation gateway Exception messages move into a msg variable before the raise (EM), with the text unchanged. The auth misconfiguration log keeps its message and gains a traceback through exc_info=True (TRY400). _runtime keeps RuntimeError and _resolve_policyengine_route keeps ValueError under a TRY004 noqa, because handlers and callers depend on those types. The job-status and budget-window handlers keep their catch-alls under a BLE001 noqa: call.get re-raises whatever the worker raised, and every failure must become a failed-job response or fall back to the seed state. The auth-bypass event keeps its catch-all too. The golden OpenAPI spec is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff TRY, EM and BLE findings in the simulation executor Exception messages move into a msg variable before the raise (EM), with the text unchanged. Raises inside try bodies move into private helpers (_parse_policy_period_dates, _certified_us_spm_capability, _require_artifact_digest, _require_local_authority_column), so the same exceptions reach the same handlers (TRY301, TRY300); dump_package_version narrows its try to the file read. qualify_stage12_parity, a local operator CLI that nothing calls or catches, now raises TypeError for a JSON value that is not an object. The other TRY004 sites keep their types under a noqa: they cross a Modal boundary, are documented or asserted by tests, or are recorded by type name in telemetry and Stage 12 error summaries. The Stage 12 comparison failure log stays traceback-free under a TRY400 noqa. Worker, cleanup and telemetry catch-alls keep except Exception under a BLE001 noqa. The Modal images copy src/modal and policyengine_simulation_executor, so their layers change like any source edit; src/modal/app.py gains no future import. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff EM findings in the integration tests and repo scripts Exception messages move into a msg variable before the raise, with the text unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Enable ruff group 5 (exceptions and messages) Select flake8-blind-except (BLE), flake8-errmsg (EM) and tryceratops (TRY). The preceding commits fix every finding; deliberate catch-alls, kept exception types and traceback-free error logs carry an inline noqa with the reason. Tests get no exemption. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Give every BLE001 suppression an inline reason Enabling BLE activated suppressions that were written before the rule was on. Seven had no reason, or kept it only in a comment above; they now carry it inline, and the "- reason" ones use the " # reason" form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Name hoisted SPM error codes error_code, not msg The EM fix moves an exception's first literal argument into a variable called msg. SPMInputError's first argument is an error code, followed by the message, so those 20 variables are renamed error_code. The gateway file also moves its BLE001 suppression reason to the " # reason" form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register TRY, EM and BLE in the ruff-lint rule sim.quality.ruff-lint now lists the group 5 families, and the rendered rules page and AGENTS.md Rules block are regenerated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Name hoisted parity mismatch codes error_code, not msg ParityMismatch takes a code and a schema path, not a message, so the 26 variables the EM fix named msg are renamed error_code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the group 5 changelog line's YAML and wording The colon after "(TRY, EM, BLE)" made YAML read the item as a mapping rather than a string. The line also said error types were unchanged, but two type checks in qualify_stage12_parity.py now raise TypeError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #743 and #744 added before raising #743 and #744 landed on main after this stack branched. EM and TRY003, which this PR enables, flag 34 raises in their sentinel, routing and package-update code that pass a literal message. Each message now moves into a msg variable before the raise, with the text unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #789's tests raise before raising PR enables, flag five raises in its new Stage 12 tests that pass a literal message; each message now moves into a msg variable first. TRY301 flags the raise inside the pooling test's transaction, which moves into a small helper so the same RuntimeError still ends that transaction. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #795 added before raising #795 landed on main after this stack branched. EM and TRY003, which this PR enables, flag its two new RuntimeError raises (a child that timed out and a spent parent budget) and the cancel failure its test fake raises; each message now moves into a msg variable first, with the text unchanged. BLE001 flags _cancel_call, which now holds the best-effort cancel loop this PR suppressed; it keeps that suppression with its reason inline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #791's tests raise before raising #791 landed on main after this stack branched. EM and TRY003, which this PR enables, flag ten raises in its new Stage 12 failure-cause and runtime tests that pass a literal message; each message now moves into a msg variable first, with the text unchanged. TRY301 flags the raise inside the try of the _raised helper, which now captures the raised error, with its traceback, through pytest.raises; that also drops the helper's blind except and its BLE001 suppression. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Write the BLE001 suppressions #791 added with an inline reason #791 landed on main after this stack branched. Its two new blind-except suppressions in the Stage 12 failure-cause module gave their reasons in the "- reason" form; like every other BLE001 suppression in this PR, they now use the " # reason" form, with the reasons unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #758 added before raising #758 landed on main after this stack branched. EM and TRY003, which this PR enables, flag four raises with a literal message: the deployed permission_enforcement fixture's RuntimeError and three stand-ins in its permission tests. Each message now moves into a msg variable first, with the text unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Write the BLE001 suppression #758 added with an inline reason #758 landed on main after this stack branched. The blind except around its simulation_entry_permission_missing event gave its reason in the "- reason" form; like every other BLE001 suppression in this PR, it now uses the " # reason" form, with the reason unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #781 added before raising #781 landed on main after this stack branched. EM and TRY003, which this PR enables, flag nine raises with a literal message: the contract's segment-count validator and eight stand-ins in its executor and gateway job progress tests. Each message now moves into a msg variable first, with the text unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Write the BLE001 suppressions #781 added with an inline reason #781 landed on main after this stack branched. Its five new blind-except suppressions, in the Modal app's reporter and stage-duration helpers, the stored reporter's read and write, and the gateway's stage-field read, gave their reasons in the "- reason" form; like every other BLE001 suppression in this PR, they now use the " # reason" form, with the reasons unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 16, 2026
…Z, ERA, TD, FIX, EXE) (#785) * Fix ruff LOG and G findings Pass logging arguments instead of f-strings (G004); each message renders to the same text. Name the economy-comparison router's logger after its module rather than its file path (LOG002). Log the gateway's auth misconfiguration with logger.exception and a fixed message (G201): the traceback already ends with the exception and its message, which the old call also repeated in the message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep naive policy dates explicit for ruff DTZ policyengine.py parses policy period keys with strptime and no zone, so its ParameterValue dates are naive, and the executor's period parsing and the tests that compare against those dates must stay naive too. Mark each kept naive datetime with a reasoned DTZ suppression, and route the policy tests' expected dates through one helper. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Replace asserts outside tests for ruff S101 The deployed-service test suite in authenticated_tests is a pytest suite that the tests glob misses, so give it the same test exemptions. Elsewhere, drop the asserts that only narrowed types: the SPM selection check now returns the hash and scenario it has already proved present, and the qualification runner closes over a non-optional runtime. The gateway image smoke raises RuntimeError when its report is incomplete, as the executor's static-file smoke does, and the fake model output raises AssertionError explicitly when the adapter asks for a mode other than JSON. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep command-line output for ruff T20 Every print the rule finds belongs to a command-line script or a Modal local entrypoint, whose stdout is its output, so keep each print. Files with one to three prints mark each with a reasoned suppression; the routing-state publisher and the policyengine package updater send their output through one small helper that holds the only print. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Route test subprocess calls through helpers for ruff S603 and S607 Each test module that runs repository scripts now does so through one helper that holds the only subprocess call, with reasoned suppressions: the scripts are repository-owned, their arguments are built by the tests, and bash and uv come from PATH as they do in the workflows. Resolving the executables in Python instead could pick a different binary, because subprocess searches the child's PATH, and some tests run scripts with an empty environment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the remaining ruff S findings Build the GCP metadata request inline from its constant http URL, with the same Metadata-Flavor header, so S310 can see the scheme. Keep every other finding with a reasoned suppression: - S105/S106: fake tokens in tests (one module constant per test module where a value repeats), and names or fingerprints that only look like secrets. - S108: the executor's fixed data directories inside the Modal container, and a lookup path the tests' fakes only pass along. - S110: add S110 to the existing reasoned BLE001 suppressions. - S102: the image tests execute repository-owned app source as Modal's /root/app.py. - S301: the SPM tests round-trip errors they built or raised. - S310: the Hub API request URL is built from the HF_ENDPOINT base and quoted path parts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Enable ruff group 6 (security, logging and debugging leftovers) Select DTZ, ERA, EXE, FIX, G, LOG, S, T20 and TD. The earlier commits on this branch fixed or suppressed, with a reason, every finding outside the shared scripts/rules tool. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register group 6 and the deployed-test ruff exemption sim.quality.ruff-lint now lists DTZ, ERA, EXE, FIX, G, LOG, S, T20 and TD. The authenticated_tests exemption gets its own rule, since a ruff per-file-ignore rule names one pattern. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Sync the rules tool from the Public API's group 6 The shared scripts/rules tool takes the Public API's group 6 fixes: explicit checks instead of asserts, and reasoned suppressions for its git and shellcheck subprocess calls, prints and calendar dates. The rendered rules page picks up the tool's new hash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 6 findings in the code #743 and #744 added #743 and #744 landed on main after this stack branched. Group 6, which this PR enables, flags their new code; each fix follows this PR's conventions: - T201: the routing publisher's new commands report through its _report helper, the package updater's dispatch check through _write_result, and the sentinel CLI gets one _report helper that holds its only print. - S603/S607: the deployment-asset tests route #744's new bash calls through _run_bash, which now takes cwd; the rollback tests and the package-update helper tests get one helper each that holds the only subprocess call, with reasoned suppressions. - S310: the sentinel's HTTP helper opens the deployed entrypoint or the test auth issuer URL that the workflow passes; reasoned suppressions. - S106: a fake bearer token in the sentinel tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add S110 to the cancel suppression #795 added #795 landed on main after this stack branched. S110, which this PR enables, flags the try-except-pass in its _cancel_call, which now holds the best-effort cancel loop this PR suppressed; like the other reasoned BLE001 suppressions, it gains S110. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 6 findings in the code #791 added #791 landed on main after this stack branched. S105, which this PR enables, flags two values that only look like secrets, and each keeps a reasoned suppression: the redaction pattern for secret-looking key names in the Stage 12 failure-cause module, and the fake database password its tests check redaction removes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 6 findings in the code #758 added #758 landed on main after this stack branched. S603, which this PR enables, flags the two new deployment tests that run the Cloud Run deploy script directly to check permission enforcement defaults and rejections. They carry the same reasoned suppression as the existing direct run of that script. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 6 findings in the code #781 added #781 landed on main after this stack branched. Group 6, which this PR enables, flags two of its additions: - S110: the Modal app's stage-duration telemetry swallows a failed set_context, so its suppression adds S110 beside BLE001, as the call attributes helper above it does; - DTZ001: the contract test that the job record rejects a naive time builds that naive time on purpose, so it carries a reasoned suppression. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Allow file-level ruff exceptions only in tests, CI/CD and command-line scripts A ruff per-file-ignores entry switches rules off for whole files. That is now acceptable where it's the norm: tests, CI/CD scripts under .github, and command-line scripts (under a scripts directory, or modules run through if __name__ == "__main__"). Everywhere else a kept finding still needs an inline suppression with its reason. The rules guard already requires a per-file-ignore rule for every entry. It now also expands each entry's pattern the way ruff does (globset syntax, and a slash-less pattern also matches file names) and fails when the pattern covers any other file, is negated, or matches nothing. scripts/rules/file_exceptions.py holds the definitions, and the registry doc describes the policy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move group 6's test, CI and script suppressions to file-level exceptions Tests, CI/CD scripts and command-line scripts now take their deliberate group 6 exceptions as registered per-file-ignores, each code chosen from the suppressions those files already carried: - tests and the deployed-service tests: S102, S105, S106, S108, S301, S603, S607, DTZ001 and DTZ007, added to their existing exemptions; - .github: T201; - scripts directories: DTZ011, S603, S607 and T201; - programs under src (the Modal deployment utilities, and the OpenAPI and Stage 12 infrastructure commands): S310 and T201. That removes 75 inline codes from 44 files. Application code keeps its inline suppressions and reasons. Comments only: no code changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 16, 2026
…#799) * Exempt tests from SLF001, INP001 and PLC0415 Tests deliberately reach into private members, pytest test directories aren't packages, and tests often import inside a test after patching the environment. Application code still gets every rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Drop response_model arguments that repeat the return type (ruff FAST001) FastAPI takes the response model from the return annotation when response_model is omitted, so the generated OpenAPI documents are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff naming and builtin-shadowing findings (N, A, PLC0414, SLF001) - Alias the shared exit registry as app_exit where it is imported, and keep the policyengine_fastapi.exit.exit name that projects import. - Use snake_case for the OpenTelemetry provider and log-handler locals. - Add the Error suffix to exceptions that never leave their project: BackendUnavailableError, BackendTimeoutError, TemporaryStage12UnsupportedRequestError and ParityMismatchError. TemporaryStage12DispatchFailed keeps its name, which the entry app logs as error_type. - Keep the camelCase budget-window fields, which are published JSON, the explicit re-exports in stage12_execution, and the two deliberate private accesses, each with a reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff INP001 per directory Declare the script directories (.github/scripts, scripts and the executor's scripts) and the rules proof fixtures as namespace packages in ruff.toml. Ruff's full output, with every rule selected, is otherwise unchanged by the setting. The executor's src/modal/utils scripts keep an inline suppression instead: declaring that directory, or adding an __init__.py, makes ruff treat it as part of a first-party "modal" package and regroup its Modal SDK imports. An __init__.py would also change the src.modal source baked into the Modal images. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep deliberate deferred imports in the shared library and CI script (ruff PLC0415) Each deferred import keeps an inline reason: the GCP exporters, logging instrumentation and metadata probe load only when used, and only the bundle-versions command runs where policyengine is installed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Import the Stage 12 backend at the top of the entry app (ruff PLC0415) app.py already imports stage12_backend at module level for its exception types, so binding Stage12ComparisonBackend there too loads nothing new. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff PLC0415 in the gateway Bind SPMSelection in endpoints.py's existing policyengine_simulation_contract.spm import. Keep the Modal function body's imports, which build the web app in its container, and the smoke app's in-image imports, each with a reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff PLC0415 in the executor Move only imports that load nothing new and that no test patches: pathlib and importlib, the policyengine_simulation_contract helpers, names from a module the file already imports at the top, and RegionGroupStrategy, which policyengine.core already loads. Every other deferred import keeps an inline reason: Modal function bodies and the image-build and smoke functions, lazily loaded policyengine models and heavy libraries, call-time resolution that the tests' patch seams rely on, and import cycles in the runtime. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap long lines in the shared libraries (ruff E501) Split long messages and the GCP log format with implicit string concatenation, so every string is unchanged, and rewrap two docstrings that no published schema uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap long generated-client calls in the integration tests (ruff E501) Bind the long generated endpoint functions to shorter local names before calling them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap long lines in the entry project (ruff E501) Split long strings with implicit concatenation, so every value is unchanged. The fake curl and gcloud scripts keep their single-line printf output with a suppression on the string. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap long lines in the executor (ruff E501) Split long messages and test strings with implicit concatenation, so every value is unchanged, and shorten docstrings that no published schema uses. The fake gh script keeps its single-line JSON output with a suppression on the string. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap long lines in the gateway (ruff E501) Split the Modal app's FastAPI description with implicit concatenation, so the published text is unchanged, and shorten four test names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap the Stage 12 DDL proof fixtures' docstrings (ruff E501) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Enable ruff group 7 (structure and imports) Select A, E, FAST, INP, N, PLC and SLF. Every finding outside the shared scripts/rules tool is fixed or carries an inline reason; the rules tool's findings are fixed upstream and synced separately. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register group 7 in the ruff-lint rule sim.quality.ruff-lint now lists A, E, FAST, INP, N, PLC and SLF. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Sync the rules tool from the Public API's group 7 The shared scripts/rules tool takes the Public API's group 7 fixes: wrapped long lines, BaseUnavailableError, and assert_violation in its tests. The rendered rules page picks up the tool's new hash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 7 findings in the code #743 and #744 added #743 and #744 landed on main after this stack branched. Group 7, which this PR enables, flags their new code; each fix follows this PR's conventions: - E501: wrap the long strings in their routing, sentinel, gateway and test code; every message and expected value reads the same. - N818: RoutingConflict never leaves the executor, so it becomes RoutingConflictError, as this PR renames the other in-project errors. - INP001: the sentinel script in src/modal/utils keeps the inline reason the other scripts there carry. - PLC0415: the sentinel's deferred contract and runtime imports and the package updater's optional receipt import keep inline reasons. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep the long Stage 12 test name #789 added under E501 #789 landed on main after this stack branched. E501, which this PR enables, flags the signature line of its lost-commit coordinator test, whose name cannot be wrapped; the line keeps a reasoned suppression rather than a shorter, vaguer name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 7 findings in the code #791 added #791 landed on main after this stack branched. E501, which this PR enables, flags three long string literals in its Stage 12 failure-cause tests; each is wrapped by implicit concatenation, with its value unchanged. N818 flags Stage12FailureCause, whose name telemetry records as the error type of every recorded cause, so it keeps its name under a reasoned suppression, like TemporaryStage12DispatchFailed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 7 findings in the code #758 added #758 landed on main after this stack branched. FAST002, which this PR enables, flags the six entry routes that take the caller's identity as a "caller: CallerIdentity | None = Depends(...)" parameter. FastAPI's Annotated form can't be used: this module postpones annotations, and FastAPI resolves them against the module's globals, where the per-app permission dependency doesn't exist. No handler reads the parameter. Each route now declares its permission dependency in the decorator's dependencies, as the gateway's routes declare require_auth and as these routes declared authentication before #758. FastAPI still runs the same dependency before the body is read, so each route checks the same permission, logs or rejects a missing one the same way, and the OpenAPI document is unchanged. The CallerIdentity import is no longer used. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 7 findings in the code #781 added #781 landed on main after this stack branched. PLC0415, which this PR enables, flags four of its deferred imports. As for the executor's other deferred imports, only an import that loads nothing new and that no test patches moves to the top: - the Modal app's NULL_JOB_PROGRESS and StoredJobProgressReporter, whose module the app's top-level imports already load, move to the top; - the job_progress_state store functions stay deferred, resolved per call so the tests can patch job_progress_state; - StageReportingSimulation stays deferred in _build_simulation, resolved per call like its neighbours so the tests can patch it; - the bundle-metadata builder's JOB_PROGRESS_CAPABILITY_VERSION stays deferred: the contract package, and pydantic with it, loads only when bundle metadata is built. Each kept import carries its reason inline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * List the lazily loaded packages for ruff instead of suppressing each import Fourteen heavy or optional packages and modules are only ever imported inside the function that needs them: the policyengine bundle, countries, provenance and tax-benefit-model modules, policyengine_us, spm_calculator, h5py, google.cloud, three OpenTelemetry exporters and instrumentations, pythonjsonlogger, and the executor's spm and baseline_artifacts modules. No application module imports any of them at module level. ruff.toml now lists them in banned-module-level-imports and selects TID253, so importing one at module level fails (TYPE_CHECKING imports are still fine), and PLC0415 accepts their in-function imports without a suppression. That removes 51 PLC0415 suppressions. Tests may still import them at module level (TID253 joins the test exemptions). Packages that some application module does import at module level (pandas, microdf, huggingface_hub, policyengine.core and others) aren't listed, so their deferred imports keep their reasons. The nine Modal utility programs, run with python -m from a directory that isn't a package, take INP001 in their existing file-level entry instead of one suppression each. Comments and configuration only: no code changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2 tasks done
anth-volk
added a commit
that referenced
this pull request
Oct 19, 2026
#758 landed on main after this stack branched. Group 8a, which this PR enables, flags its new code; each fix follows this PR's conventions: - B008: CallerAuthorizer.require_permission builds the authenticate dependency marker once and reads it from the nested dependency's default. The Annotated form can't be used: the module postpones annotations, and FastAPI resolves them against the module's globals, which don't hold the authorizer's dependency. FastAPI sees the same dependency, so each route's permission check is unchanged. - SIM105: the permission-missing event keeps its try/except, whose except line carries the BLE001/S110 reason, like the module's other telemetry. - ARG: the permission tests' fake decoders explain their unused parameters and the _decoder replacements prefix theirs; each table-driven test underscores the PROTECTED_ROUTES column it doesn't read, so the test ids stay the same. - ARG001: the deployed enforcement test requests permission_enforcement through pytest.mark.usefixtures; pytest still sets it up first, so the test skips before fetching a token. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 19, 2026
#758 landed on main after this stack branched. PLR0913, which this PR enables, flags its six-argument log-only permission test. As this PR's other table-driven tests do, each protected route is now one case: the PROTECTED_ROUTES table holds a ProtectedRoute per route, and the four tests over it take one route argument, which also drops the underscored columns two of them didn't read. The parametrized test ids are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 20, 2026
#758 landed on main after this stack branched. PLR0913, which this PR enables, flags its six-argument log-only permission test. As this PR's other table-driven tests do, each protected route is now one case: the PROTECTED_ROUTES table holds a ProtectedRoute per route, and the four tests over it take one route argument, which also drops the underscored columns two of them didn't read. The parametrized test ids are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 20, 2026
…PERF, PLW, PT, PTH, RUF, SIM and others) (#802) * Exempt the rules tool's unittest tests from pytest-style rules The shared rules tool needs only the standard library, so its tests use unittest on purpose. Registered as sim.quality.unittest-ruff-exemptions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff group 8a findings in policyengine-fastapi - JWTDecoder takes auto_error as keyword-only (FBT001/FBT002); the only caller, the gateway, already passes it by keyword. - JWTDecoder.__call__ declares its bearer dependency with Annotated (B008). - The 403s raised while validating a token chain the caught error (B904). - Iterate dicts directly instead of over .keys() (SIM118). - Name the ignored error code on the trace-filter type: ignore (PGH003). - Explain NoOpObservability.span's unused arguments: it implements the Observability protocol (ARG002). - Check expected exceptions with pytest.raises(match=...) (PT017). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff group 8a findings in the simulation contract - Merge the two branches that both fall back to the package version (SIM114), and collapse the nested as_of check (SIM102). - The test modal.Dict stand-in takes create_if_missing as keyword-only, matching Modal's own from_name (FBT001/FBT002). - Request the mock_modal fixture with usefixtures, and prefix the fake HF fetchers' unused positional parameters with an underscore (ARG). - Use tuples for parametrize names (PT006), a dict literal (C408), and match the expected validation messages (PT011). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff group 8a findings in the entry project - CallerAuthenticator reads its bearer dependency marker from a module-level default (B008). The Annotated form isn't usable there: the module postpones annotations and FastAPI resolves them against the callable's __globals__, which a callable instance lacks. The OpenAPI document is unchanged. - Keep the three telemetry try/except blocks, whose except lines carry the BLE001/S110 reasons, and suppress SIM105 on their try lines. - Mark the failing test backends' request() with @OverRide, prefix the positional-only parameters of the _decoder replacements with an underscore, and explain the fake decoders' unused parameters (ARG). - Drop the unused tmp_path requests (ARG001) and use tuples for parametrize names (PT006). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff group 8a findings in the gateway project - require_auth declares its bearer dependency through a module-level Annotated alias (B008); the module postpones annotations, and FastAPI resolves the alias through the function's globals. The OpenAPI documents are unchanged. - Chain the caught error into the 400, 404 and 503 HTTPExceptions and the route-resolution ValueErrors (B904). A BLE001 suppression that the chained raise made unused keeps its reason as a comment. - Keep the telemetry try/except blocks, whose except lines carry the BLE001/S110 reasons, and suppress SIM105 on their try lines. - Use a conditional expression for the resolved version (SIM108) and Path.open in the OpenAPI generator (PTH123). - Test stand-ins: keyword-only create_if_missing and use_bundle_default (FBT), ClassVar registries (RUF012), and explained unused parameters of the FunctionCall.get stand-in and the side-effect fixture (ARG). - Tests: usefixtures for mock_modal and the import-isolation fixture, tuple parametrize names, bare @pytest.fixture, a split assertion, an explicit check=False, dict() over a comprehension, and the decoder class passed directly instead of through a lambda. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff group 8a findings in the executor sources and fixtures - Assign spm directly instead of through setattr (B010), use dict.fromkeys (C420), list unpacking (RUF005), one combined with-statement in the same enter/exit order (SIM117), a membership test (SIM109), merged upper-casing branches (SIM114), extend over an append loop (PERF401), and Path.open/Path.unlink (PTH). - Keep, with reasons: the per-child and per-call try/except loops (PERF203), the credentials temp file that stays open across the yield (SIM115), HDFStore.keys(), which isn't a dict view (SIM118), the FastAPI lifespan hook's app parameter (ARG001), and the precompute local entrypoint's positional force flag, whose signature Modal turns into the CLI (FBT001/FBT002). - Keep the telemetry and best-effort-cancel try/except blocks, whose except lines carry the BLE001/S110 reasons, and suppress SIM105 on their try lines. - Fixtures: prefix the identity stubs' positional-only parameters and the fake local_entrypoint's **kwargs with an underscore (ARG). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve unused-argument findings in the executor tests - Request side-effect fixtures with @pytest.mark.usefixtures, and drop an unused monkeypatch request (ARG001/ARG002). - Prefix unused parameters of monkeypatched replacements with an underscore, after checking that every caller passes them positionally, and rename unused *args/**kwargs to *_args/**_kwargs (ARG001/ARG005). - Keep, with reasons, parameters that callers pass by keyword: the _load_dataset and _reduce replacements, the fake _build_simulation and economic_impact_analysis keyword-only parameters, FunctionCall.get stand-ins' timeout, and the Stage 12 writer stand-ins' prefix. Fake model versions that duck-type .py's MicrosimulationModelVersion keep their parameters too. - Mark the failing child store's create_or_resolve_simulation with @OverRide. - Use tuples for parametrize names (PT006). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve the remaining ruff group 8a findings in the executor tests - Pass booleans by keyword: DatasetSelection's is_default (FBT003), and keyword-only routed/accepted test parameters, the routed helper flag, the output-builder helper's options and the modal.Dict stand-ins' environment_name/create_if_missing, matching Modal (FBT001/FBT002). - Match the real messages in pytest.raises (PT011); one native-only check keeps its broad ValueError, with a reason, because the rejecting layer's message varies. Make regex patterns raw or escaped (RUF043), give pytest.fail a message (PT016), and move the GCP credential cleanup check into a helper so pytest.raises wraps one call (PT012/SIM117). - Use Path methods (PTH), literals and dict() (C408/C416), a list literal (SIM905), unpacking (RUF005), extend (PERF401), ClassVar annotations (RUF012), an underscore for an unused unpacked value (RUF059), explicit check=False (PLW1510) and an ASCII operator in a comment (RUF003). - Pass regions.get, checks.append and nullcontext directly instead of through lambdas; nothing patches them (PLW0108). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff group 8a findings in the CI script and integration tests - The package update script stops overwriting its loop variable (PLW2901) and replaces the pyproject through Path.replace (PTH105). - The integration conftest uses bare @pytest.fixture (PT001). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Enable ruff group 8a (arguments, idioms and pytest style) Adds ARG, ASYNC, B, C4, FBT, PD, PERF, PGH, PLW, PT, PTH, RUF and SIM to the selected rule families. Every finding outside the shared rules tool is fixed or carries an inline reason. Complexity (C90, PLR) is left for group 8b. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register group 8a in the ruff-lint rule sim.quality.ruff-lint now lists ARG, ASYNC, B, C4, FBT, PD, PERF, PGH, PLW, PT, PTH, RUF and SIM. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Sync the rules tool from the Public API's group 8a The shared scripts/rules tool takes the Public API's group 8a fixes: comprehensions, explicit check=False, bound loop variables and the removed unused workflow-parser parameter. The rendered rules page picks up the tool's new hash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 8a findings in the code #743 and #744 added #743 and #744 landed on main after this stack branched. Group 8a, which this PR enables, flags their new code; each fix follows this PR's conventions: - F821: this PR's CI-script commit dropped the package updater's os import, but #744's verify-dispatch reads os.environ, so restore it. - PTH123: the routing and sentinel CLIs open their files through Path. - RUF100: the sentinel's report helper prints with an explicit file, so its T201 suppression is unused. - ARG: tests request serialized_workflow through usefixtures (and the patched_modal fixture keeps it with a reason), drop unused monkeypatch requests, and prefix unused parameters of positional stand-ins. - FBT002: the routing-plan test helper takes force by keyword. - PT012/PT018/B905/RUF043: one call per pytest.raises, one condition per assert, an explicit non-strict zip over call chains of different lengths, and a raw match pattern. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 8a findings in the code #789 added #789 landed on main after this stack branched. Group 8a, which this PR enables, flags its new code; each fix follows this PR's conventions: - ARG001/ARG002: open_connection_once and the telemetry-recording test runtime keep keyword parameters their protocols pass, with reasons; the test's random.uniform stand-in prefixes its unused positional bound. - PERF203: the connection retry loop catches each refused attempt by design, with a reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 8a findings in the code #787 added #787 landed on main after this stack branched. Group 8a, which this PR enables, flags its new code; each fix follows this PR's conventions: - ARG002: the shared in-memory object store keeps upload_bytes's content_type, which Stage12ArtifactStore passes by keyword, with the reason this PR gave the per-file fake it replaced. - PT011: the invalid SPM provenance test expects pydantic's ValidationError, which every rejected value raises, not any ValueError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 8a findings in the code #795 added #795 landed on main after this stack branched. Group 8a, which this PR enables, flags its new code; each fix follows this PR's conventions: - ARG002: the child-deadline tests take the bare_country fixture through pytest.mark.usefixtures, like the runner tests. - ARG005: their _reduce stand-ins keep country_module, which run() passes by keyword, with the same reason. - ARG001: _crashing_call prefixes the clock it takes only to match _stuck_call's signature. - PT006: the child-timeout telemetry test names its parameters in a tuple. - SIM105: _cancel_call keeps its try/except, whose except line carries the BLE001/S110 reason, like the best-effort cancel it replaced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 8a findings in the code #791 added enables, flags its new tests; each fix follows this PR's conventions: - ARG002: its fake Modal calls, recording runtimes and rejecting aggregate writer keep the timeout, status_code and prefix keywords of the interfaces they stand in for, with the reasons this PR gave the other fakes. - PT011: the stage-and-role summary test matches the message it raises rather than accepting any ValueError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 8a findings in the code #758 added #758 landed on main after this stack branched. Group 8a, which this PR enables, flags its new code; each fix follows this PR's conventions: - B008: CallerAuthorizer.require_permission builds the authenticate dependency marker once and reads it from the nested dependency's default. The Annotated form can't be used: the module postpones annotations, and FastAPI resolves them against the module's globals, which don't hold the authorizer's dependency. FastAPI sees the same dependency, so each route's permission check is unchanged. - SIM105: the permission-missing event keeps its try/except, whose except line carries the BLE001/S110 reason, like the module's other telemetry. - ARG: the permission tests' fake decoders explain their unused parameters and the _decoder replacements prefix theirs; each table-driven test underscores the PROTECTED_ROUTES column it doesn't read, so the test ids stay the same. - ARG001: the deployed enforcement test requests permission_enforcement through pytest.mark.usefixtures; pytest still sets it up first, so the test skips before fetching a token. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 8a findings in the code #781 added #781 landed on main after this stack branched. Group 8a, which this PR enables, flags its new code; each fix follows this PR's conventions: - ARG: stand-ins whose callers pass a parameter positionally prefix it with an underscore (the progress store's reads and writes, the patched Simulation.ensure, _country_module, get_region, Function.from_name, the preempted _set_modal_call_attributes, time.sleep and the on_first_ensure stand-ins), and unused *args/**kwargs become *_args/**_kwargs. Parameters callers pass by keyword keep a reason: the impact-analysis, _load_dataset, dispatch, run_simulation_impl and _configure_modal_observability stand-ins, the spawn stand-in's observability_context, the _reduce stand-in's country_module, the fake FunctionCall.get's timeout and the generated client's sync_detailed. - ARG001/ARG002: the contract's missing-record test and the bundle manifest test request their fixtures through pytest.mark.usefixtures. - B018: the failed-analysis test assigns the property it reads inside pytest.raises. - SIM105: the Modal app's stage-duration telemetry keeps its try/except, whose except line carries the BLE001/S110 reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Exempt tests from the unused-argument rules file-wide Test fakes and stubs mirror real signatures, because callers pass those arguments by keyword: run() passes country_module=, the runtime passes keyword-only options, and stand-ins replace modal.FunctionCall.get and JWTDecoder.__call__. Fixtures also request other fixtures only for their side effects. Tests are among the files allowed file-level exceptions, so ARG001, ARG002 and ARG005 join the tests' and the deployed-service tests' exemptions. That removes 72 inline suppressions from 16 test files. Application code keeps its six unused-argument suppressions and their reasons. Comments and configuration only: no code changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Swallow best-effort errors with contextlib.suppress Nine blocks ran a best-effort call and swallowed any error with try/except Exception: pass: - telemetry context and events in the entry app, its auth, the Modal app and the gateway; - a Modal call cancel during segmented national cleanup. Each block carried three suppressions: SIM105 on the try, and BLE001 and S110 on the except. Each is now with contextlib.suppress(Exception), with the old reason as a comment above it. It suppresses the same exceptions and runs the same calls, so behaviour is unchanged. That removes 18 suppression lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 8a findings in the code #712 added #712's segmentation and cache code raised 16 group 8a findings. Each is fixed in the code, with no new suppression: - PERF203: the policy-period parser and the segment-cancellation loop move their try/except into a helper called once per item. The parser's error helper is now typed NoReturn, since it always raises. The cancellation helper keeps #712's BLE001 reason. - RUF022: stage12_runtime's __all__ is sorted. - Tests: subprocess.run states check=False (PLW1510), match= patterns are raw or escape a literal dot (RUF043), compound asserts are split (PT018), the import guard extends its list from a generator (PERF401), boolean helper and parametrized arguments are keyword-only (FBT001/FBT002), and the segment-function stand-in is an instance whose spawn method takes Modal's observability_context by keyword (ARG004), as modal.Function.from_name returns an object. The rebase onto #712 also dropped this branch's PERF203 suppression in coordination.py, since #712 replaced the loop it was on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Lint the executor as Python 3.13, its required version Ruff reads no project's requires-python here, so it linted the executor with its default target version. PERF203 applies only below Python 3.11, so the retry loop in database_retry.py needed a suppression that the executor's own Python (3.13) doesn't call for. Add the executor to per-file-target-version as py313, remove that suppression, and fix the eight findings the new target version reports: tomllib is now sorted with the standard library (I001, five files), and datetime.timezone.utc becomes datetime.UTC (UP017, three files). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Use or underscore the arguments behind five ARG suppressions - NoOpObservability now subclasses the Observability protocol it already implemented, and span() is marked @OverRide, which ruff exempts from ARG002. - The executor's in-memory object store records each object's content type, and the gateway's mock FunctionCall records the timeout each get() was called with, so both fakes use the arguments they mirror. - The gateway's import_gateway_models fixture only requested the module isolation fixture for its side effect. The test that used it now requests isolation with usefixtures and imports the models module itself, named in GatewayImportModuleNames.models. - The executor's FastAPI lifespan takes _app: Starlette and FastAPI pass the app positionally. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Make the precompute entrypoint's force option keyword-only Modal 1.4.2 builds a command-line option for every non-variadic parameter of a local entrypoint and calls it with keyword arguments, so a keyword-only force still gives --force/--no-force. The test already calls main(force=True). This removes the FBT001/FBT002 suppression. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Replace the SIM115, SIM118 and PT011 suppressions with code - setup_gcp_credentials enters the credentials temp file on an ExitStack held by a with statement around the yield. The file still stays open across the yield; the finally block restores GOOGLE_APPLICATION_CREDENTIALS first, and the stack then closes, and so deletes, the file, the same order as before. - The UK local-authority check calls HDFStore.keys(include="pandas"), the default spelled out, and keeps a comment on why `in store` differs. - The native SPM test passes check= to pytest.raises (pytest 8.4), so the typed error code is part of the raises check instead of a later assert. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Restore the lower-case coverage pragma comment in gateway auth The contextlib.suppress change rewrote "# pragma: no cover" as the prose "# Pragma: no cover". Put back the original lower-case spelling. The comment sits on its own line above the with statement, so coverage.py excludes no statement because of it either way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 20, 2026
… (ALL) (#804) * Import test modules with from-imports instead of aliases Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Group test helper parameters into composite fixtures and option objects Split the long deployment-workflow test into focused tests and hoist closure-free test doubles to module level. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin contract and persistence validation behaviour before refactoring Characterize every branch of the comparison report record's state validation, the live column type checks, Hugging Face URI parsing, the SPM non-US and historical-wrapper rules, and the v2 application name limits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the comparison report record's state validation into report and comparison phases Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Name the contract's URI, version and name-length limits Parse SPM wrapper and model release versions through one helper, and keep the keyword signatures of the public contract builders that the gateway and executor call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the live column type check into enum and plain type checks Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep the observability initializer's keyword-only identity arguments Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the order and messages of entry settings validation Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split entry settings validation into caller auth, old gateway and Stage 12 checks Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the Stage 12 adapter's complete report input and check order Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the Stage 12 annual-comparison adapter into eligibility checks and a report builder Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the Stage 12 dispatch calls, acknowledged parent and skip event Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Group Stage 12 dispatch inputs into a prepared report, origin and worker version Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin entry app telemetry, Stage 12 route failures and backend readiness Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the entry app factory into a shared runtime and route registration steps The routes keep their names, decorators, parameters and registration order, so the published OpenAPI document is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Compare the old gateway's health status with HTTPStatus.OK Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin gateway route resolution from the active routing state Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split gateway active-state route resolution into pinned, latest and requested-version steps Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the resolved region to _build_simulation instead of its code and scoping strategy Every caller already held a RegionResolution, so the builder now takes it whole. Also name the simulation runtime's preview, date-range and state-code limits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass artifact key inputs to the dataset and baseline key builders as dataclasses The payloads are built from the same fields in the same way; the golden digest tests still pass unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass baseline identity facts as one request and split out the artifact scope check Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the precompute's Modal function handles to run_precompute as one group The precompute local entrypoint keeps its --force CLI signature. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin every input the national reduce passes to the output builder Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the national reduce's run facts to build_national_output as one group Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass fan-out poll timing to the batch and segmented runners as one backoff Also name the segmented runner's consecutive poll-error limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the Stage 12 bundle schema version check Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Name the supported Stage 12 bundle schema version Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin every Stage 12 parity mismatch code, path and comparison order Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split Stage 12 parity comparisons into entity, column and report-value checks Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin parity qualification receipts, check order and default runner lifecycle Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split parity qualification into existing-run, provenance, calculation and aggregate steps Group the injectable runners and the reviewed tolerances into two option objects, and update the qualification script and tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin Stage 12 coordination validation, claims, failures, stages and aggregate output Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass each role's retained frames and descriptor to the aggregate builder together Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split Stage 12 report coordination into claim, plan, dispatch, wait and aggregate phases Group the coordinator's Modal identity and its injectable seams, and split submitted-parent validation into invocation, claim-state and production checks. The Modal coordinate_report function keeps its signature. Rebased onto #712, the phases carry #712's coordinator: one shared child-wait window opened before any setup, nonblocking probes of both children, cancellation of the unfinished children on any failure, and failure details with correlation IDs. CoordinatorDependencies gains #712's sleep, monotonic and wait_timeout_seconds seams. The report-level failure handling moves into _ReportCoordination.fail, and the run's child plans, calls and descriptors live on _ReportCoordination, so a failure in any phase can stop the started children. The coordination pins expect #712's single child-wait span and correlation-ID suffixes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin how the Modal child invoker looks up, spawns and restores children Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the child invoker one spawn request instead of six keywords Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin result comparison logs, spans and the failed state-update path Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the result comparison its coordinator services as one group Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the Stage 12 worker's reuse path and calculator timing Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the single-simulation worker its seams as one dependencies group The Modal run_single_simulation_us and _uk functions keep their signatures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin Stage 12 worker validation results, order and failures Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass Stage 12 worker validation its environment and checks as one group Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin routing state validation messages and check order Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Group a deployed app's versions in the routing publisher and split its validation The update_version_registry command-line options are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the artifact fetch layer's receipt, credential and download edge cases Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the image artifact fetch into freshness, credential and download steps The helpers stay in the self-contained image setup module with lazy imports. Changing fetch_artifacts's source changes its Modal build-layer definition, so the next simulation image build re-runs the artifact fetch layer once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the release sentinel's polling, retry statuses and problem order Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the release sentinel's deployment check into submit, poll and compare steps Group the job timeout, poll interval, sleep and clock as one JobPolling, and name the HTTP statuses the sentinel compares with. The check command's options are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the routing publisher CLI's mode order, outputs and argument errors Also pin how a publication id names its owning run, and the restore refusals for no plans and a missing routing store. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the routing publisher CLI into one command per mode The modes keep their precedence and output, and the help text is unchanged. Also split the plan-restore loop out of restore_routing_plans and name the parts of a workflow run's publication id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the package update helper's bundle-versions and verify-dispatch output Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the package update helper's bundle and dispatch checks into helpers The helper's subcommands, output lines and messages are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the Stage 12 connection retry's delay sequence and budget cut Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the Stage 12 connection retry its clock, sleep and jitter as one timing The persistence store still calls the opener with only the connect step and the operation name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the workflow expression evaluator and routing command discovery Pin what the GitHub expression evaluator returns for each operator and status function, and the argv and refusal messages routing command discovery produces, before splitting both test helpers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the workflow expression evaluator into a tokenizer and a parser Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split routing command discovery into script, array and per-command checks Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Group the routing and package-update test parameters into cases Give the fake git its reported state as one FakeGitState, pass a planned release's versions to _plan together, and parametrize each multi-value test case as one argument. The parametrized test ids are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Take the Public API's group 8b rules tool refactor Copy the Public API's scripts/rules, whose group 8b commits split the command line, the registry reader, validation, the workflow job parser and the mermaid graph builder, and pin their behaviour with new tests. The two copies were identical apart from that refactor and its tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Select every ruff rule (ALL) Every rule family now passes, so ruff.toml selects ALL instead of the per-group list. It ignores COM812 and ISC001, which conflict with ruff format (ruff's documentation says to disable them with the formatter), and D203 and D213, which are mutually exclusive with the kept D211 and D212. The per-file target version, namespace packages and per-file ignores are unchanged. The sim.quality.ruff-lint rule's enforcer codes become ["ALL"], and its statement names the four ignores and why. The rules-registry skill says every rule applies apart from the listed and registered per-file ignores, that a ruff upgrade PR fixes any new rule's violations, and that an exception is an inline noqa with a reason. The rendered rules page and the AGENTS.md rules block are regenerated, and the changelog fragment describes the complexity refactors and the switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the poll, deadline and budget timing the segmented runner #795 extended #795 landed on main after this stack branched and gave the segmented national runner a child deadline, a parent budget and a clock beside its poll backoff. PLR0913, which this PR enables, flags the eight-argument initializer; these tests pin the deployed defaults and how given values are taken before the timing is passed as one group. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the segmented runner the poll and deadline timing #795 added as one group enables, flags the segmented national runner's initializer once #795's child deadline, parent budget and clock sit beside the poll backoff. The four now travel as one SegmentedRunTiming with the same defaults, which the previous commit pins; run_segmented_national keeps its signature. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Set the fake child call's cancel failures #795 added after construction #795 landed on main after this stack branched. PLR0913 flags the six-argument initializer of the segmented national tests' fake child call once #795 added before_each_poll and cancel_failures. No test passes cancel_failures to the initializer; the two that use it set it on the call, so it now starts at zero and stays an attribute. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin how the tracked_stage #791 added scopes and records each stage #791 landed on main after this stack branched. PLR0913, which this PR enables, flags its six-argument tracked_stage, which other tests cover only through the coordinator and worker. These tests pin it directly before its stage plan, progress and runtime are passed as one group: the span or operation it opens, the attributes it passes, the role it records and the failing stage it keeps without a runtime. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the tracked_stage #791 added its stage plan, progress and runtime as one group enables, flags its six-argument tracked_stage. Each process's stage plan, progress and runtime now travel as one StageTracking: the worker builds one per run, and the coordinator derives its own from the report it coordinates. The previous commit's pins keep their assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Compare the word count in the redaction helper #791 added without a magic 2 #791 landed on main after this stack branched. PLR2004, which this PR enables, flags the len(words) == 2 test in its _before_last_word. Splitting once from the right yields at most two parts, so len(words) > 1 is the same test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Parametrize the permission tests #758 added by one route case #758 landed on main after this stack branched. PLR0913, which this PR enables, flags its six-argument log-only permission test. As this PR's other table-driven tests do, each protected route is now one case: the PROTECTED_ROUTES table holds a ProtectedRoute per route, and the four tests over it take one route argument, which also drops the underscored columns two of them didn't read. The parametrized test ids are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the runtime and job progress the segmented runner #781 reports to #781 landed on main after this stack branched and gave the segmented national runner a job progress reporter beside its runtime. PLR0913, which this PR enables, flags the now six-argument initializer; these tests pin that a runner built without one reports no job progress, and that run_segmented_national_impl gives its runner the runtime and job progress it was called with, before the two are passed as one group. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the segmented runner its runtime and the job progress #781 added as one group #781 landed on main after this stack branched. PLR0913, which this PR enables, flags the segmented national runner's initializer once #781's job progress reporter sits beside its runtime. The two now travel as one SegmentedRunReporting, whose progress defaults to reporting nothing as before; run_segmented_national_impl and dispatch_run_simulation keep their signatures, and the previous commit's pins keep their assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Name the numbers #712's segmentation and cache code compares against #712 landed on main after this stack branched. PLR2004, which this PR enables, flags five magic values in its code; each is now a named constant, with no behaviour change: - the date-range period's two parts (_DATE_RANGE_PART_COUNT, the name simulation_runtime already uses); - the segment poll's consecutive-error limit (CONSECUTIVE_POLL_ERROR_LIMIT, as in segmented_national); - the national partition's 20 region groups (NATIONAL_PARTITION_GROUP_COUNT, next to US_REGION_GROUPS); - worker validation's cache years, artifact count and probe year, from the cache precompute's PRECOMPUTE_YEARS: one dataset per year and one baseline per US region group per year make the 63 artifacts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Observe the children the #712 coordinator waits for as a coordination method #712 landed on main after this stack branched. PLR0913, which this PR enables, flags its eleven-argument _observe_simulations. It becomes _ReportCoordination._observe_children, which takes the calls and descriptors and reads the child plans, store, runtime, progress, stage plan, clock, sleep and wait window from the coordination it belongs to. The polling, failure and timeout logic is unchanged, and #712's runtime tests cover every path through it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the failure reports #712 added where each failure was caught as one site #712 landed on main after this stack branched. PLR0913, which this PR enables, flags its seven-argument report_failure. The telemetry scope, default error code, attributes and segment index that say where a failure was caught now travel as one FailureSite; the error, runtime and progress stay separate. The coordinator's three calls, the worker's and the segment runner's pass the same values as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the segment runner #712 added its lookup and polling seams as one group #712 landed on main after this stack branched. PLR0913, which this PR enables, flags Stage12SegmentRunner's nine-argument constructor. The Modal module, region groups, sleep and the two poll intervals that tests replace now travel as one SegmentRunnerSeams, whose defaults are the deployed ones; the simulation, app name, runtime and cancellation check stay separate. run_segmented_simulation passes nothing new. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the Stage 12 simulation builder #712 added its resolved inputs as one group #712 landed on main after this stack branched. PLR0913, which this PR enables, flags its eight-argument build_stage12_simulation. The country module, region, dataset selection and loaded dataset the worker resolves in its own stages now travel as one ResolvedSimulationInputs. The builder reads the scoping strategy from the region, as _build_simulation already does, so it no longer takes the region code it only deleted. The SPM parameters, normalised policy and execution input stay separate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the cache precompute #712 added its Modal function handles as one group #712 landed on main after this stack branched. PLR0913, which this PR enables, flags the cache precompute's seven-argument run_precompute. Its five Modal function handles now travel as one CachePrecomputeFunctions, as the baseline precompute's PrecomputeFunctions already do; the bucket and echo stay separate. The Modal app's entrypoint passes the same five functions, and its secrets and environment are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass the scripted child call #712's tests added its outcome as one script #712 landed on main after this stack branched. PLR0913, which this PR enables, flags the six-argument ScriptedChildCall its runtime tests use. The result, wait error, cancel error and during-wait hook that script a call's outcome now travel as one ChildCallScript; the call ID and clock stay separate. Both constructions script the same outcomes as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin which simulations the Stage 12 cache identity #712 added qualifies #712 landed on main after this stack branched. Before its eight-return qualifying_cache_identity is split, pin the ineligible cases its tests did not reach: no or a non-region-group scoping strategy, a missing, empty or non-list region group, a non-state or non-text region code, and a scope without a cache key. Each gets no cache identity. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the Stage 12 cache eligibility #712 added into baseline, region and scope checks PLR0911 flags the eight returns of qualifying_cache_identity. It now asks three questions: is this a current-law baseline on the default dataset, what is its group of state regions, and what is the region-group scope's cache key. Each check is pure, so asking them in this order gives the same result; the previous commit's pins and #712's identity tests keep their assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the frame comparison and probe checks of the determinism check #712 added #712 landed on main after this stack branched. Before its C901 verify_determinism_impl is split, pin what its tests did not reach: every kind of frame difference (an entity missing on one side, a column on only one side, a dtype change and a value change) in its sorted order, the rejection of a probe that is not a planned baseline, and that the stored baseline is downloaded only when it is not already in the data folder. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Compare the determinism check's frames #712 added in their own function C901 flags verify_determinism_impl. The comparison of cached and fresh frames moves into _frame_differences, which lists each difference in the same sorted order; the probe check, download, cache-hit check and pass record stay in verify_determinism_impl. The previous commit's pins keep their assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin every check of the segment merge #712 added #712 landed on main after this stack branched. Before its C901 merge_segment_results is split, pin the checks its tests did not reach: a result set that omits a segment, a result naming another region group, segments with different entities or column dtypes, and an entity without its identifier. The serializer refuses frames without an identifier, so that pin decodes every segment to one directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the segment merge #712 added into indexing, decoding and entity merge steps C901 and PLR0912 flag merge_segment_results. It now indexes the results (rejecting an incomplete set or a duplicate index), decodes each planned segment in partition order (rejecting an omitted segment or one naming another role or region group), and merges each entity's frames (rejecting different entities, dtypes, a missing identifier or duplicate IDs), each in its own function. The checks and their order are unchanged, and the previous commit's pins keep their assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the claim and cancellation-check paths of the worker #712 extended #712 landed on main after this stack branched and extended run_single_simulation, which C901 and PLR0915 now flag. Before it is split, pin the paths its tests did not reach: a simulation that is no longer active is refused, a claim another writer won stops the run as cancelled, and a failed status read in the cancellation check is logged by error type only and treated as not cancelled, so the run completes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the Stage 12 worker #712 extended into claim, calculate and record steps C901 and PLR0915 flag run_single_simulation, which #712 extended with segmented calculation and cancellation. It now reads as its phases: claim the simulation (refusing one that is no longer active or that another writer claimed), build the cancellation check, write the input, calculate (segmented, standard or injected, always as a SimulationCalculation), write the artifact, and record success or failure, each phase but the writes in its own function. The order of reads, writes and reports is unchanged, and the previous commit's pins keep their assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin every check of the image-build cache fetch #712 added #712 landed on main after this stack branched. Before its C901, PLR0912 and PLR0915 fetch_stage12_cache is split, pin what its two tests did not reach: each manifest rejection (no bucket or manifest, schema, bundle, year set, artifact list, artifact count and deployment digest), each artifact rejection (a non-object entry, a filename with a directory, a duplicate filename, a non-integer size and a same-size file with another digest, which leaves no partial file), and the storage client built with no credentials, a credentials object or credentials encoded as a string. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Split the image-build cache fetch #712 added into manifest, client and file steps C901, PLR0912 and PLR0915 flag fetch_stage12_cache. It now validates the whole manifest first (schema, installed bundle and years; the artifact list and its counts; then the deployment digest), builds the storage client from the deployment's credentials only when none is given, and fetches each artifact through a partial file. The checks, their order and the environment variables it reads are unchanged; the module stays self-contained for the image build. The previous commit's pins keep their assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Drop init_simulation_observability's never-passed service_version None of the four callers (the entry app, the gateway app and its test factory, and the executor's FastAPI app) or the tests passes service_version, and build_runtime falls back to the package version when it is absent. Without it the function takes five arguments, so its PLR0913 suppression goes. Modal workers still pass service_version through init_process_observability. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass create_initial_batch_state its route as one BatchRoute The batch's reported version and its Modal app name always travel together: the gateway takes both from its route resolution, and the batch scheduler from its batch context. create_initial_batch_state now takes them as one frozen BatchRoute(version, app_name), so it has five arguments and needs no PLR0913 suppression. The stored batch state is unchanged. The gateway, the scheduler and the tests pass a BatchRoute. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pass resolve_spm_selection the route's versions as one SPMRouteVersions The policyengine.py version, the model version and the route provenance are read only together, to decide whether a route with no SPM capability predates canonical SPM. resolve_spm_selection now takes them as one frozen SPMRouteVersions, so it has four arguments and needs no PLR0913 suppression. Its decisions are unchanged. The gateway, the executor and the tests pass an SPMRouteVersions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Call Stage 12 runtime functions through v2_app's module-level import v2_app.py already imports policyengine_simulation_executor.stage12_runtime at module level, and that package's __init__ imports its submodules eagerly, so the four in-function imports of the same package loaded nothing new and each needed a PLC0415 suppression. The Modal function bodies now call stage12_runtime.<name> instead. Attribute lookups happen at call time, so tests that patch stage12_runtime.run_single_simulation or stage12_runtime.coordinate_report still take effect. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Import the image setup module's standard-library modules at module level Splitting fetch_artifacts into six functions gave each one its own in-function imports of json, logging, os and pathlib.Path, raising the module's PLC0415 suppressions from 7 to 14. Those modules are always importable in the image-build step: Modal imports this module from the function's mounted source package (the function is not serialized), and stage12_cache_image.py, which also runs before add_local_python_source, already imports json, os and pathlib at module level. The module now imports them, and typing.TYPE_CHECKING, at module level, which removes twelve suppressions. test_image_setup_module_stays_self_contained now allows module-level imports of standard-library modules only (sys.stdlib_module_names), and still rejects any import of the executor package. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * List google.oauth2 and policyengine_uk as lazily loaded packages Add both to banned-module-level-imports, so ruff keeps them out of module-level imports (TID253) and accepts their in-function imports without a PLC0415 suppression. That removes the two remaining suppressions in _image_setup.py. stage12_cache_image.py was the only application module that imported google.oauth2 at module level; the import moves into _storage_client, the one function that uses it. The storage-client test now patches google.oauth2.service_account.Credentials directly, as the image setup tests already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Update the changelog entry for the removed PLR0913 suppressions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #759
Same-Repository Draft PR Check
PolicyEngine/policyengine-sim-api, not a personal fork.Summary
CallerIdentity, which used to be parsed and discarded.simulation:submit:POST /simulate/economy/comparisonandPOST /simulate/economy/budget-window.simulation:read:GET /jobs/{job_id}andGET /budget-window-jobs/{batch_job_id}.stage12:operate: both/internal/stage12/*routes.permissionsclaim on any token. The space-delimitedscopeclaim also counts, but only on client-credentials (machine) tokens, identified bygty(a string or a list) or an@clientssubject; agtyof any other shape is ignored rather than rejecting the token. Claims are read only by their registered JWT names, so agrant_typesorsubjectclaim cannot stand in forgtyorsub. Without RBAC, Auth0 copies requested API scopes into a user token'sscope, so a user token'sscopeis ignored. Public routes are unchanged.SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS, strictly0or1, defaulting to0:0(log-only): emitssimulation_entry_permission_missingwith the permission andazpclient ID, never tokens, and lets the request through.1: answers 403.SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS; unset means0. The deployed auth tests receive it too.1. It confirms the deployed test client, which must not holdstage12:operate, gets 403 from a Stage 12 route.Before setting enforcement to
1: on the shared entrypoint API in Auth0, enable RBAC and "Add Permissions in the Access Token". Then grant:simulation:submitandsimulation:readto the API v1 client and to the deployed test client (but notstage12:operateto the test client);stage12:operateto the Stage 12 operator client.Wait one access-token lifetime with no log-only events before flipping, because cached tokens don't carry new grants. The API v1 client uses a single
GATEWAY_AUTH_AUDIENCEfor both the old gateway and the entrypoint.Testing
uv run pytest tests/(entry project): 233 passed. Before this change there were 158. The new tests cover:permissionsvsscopeclaims, including near misses, andscopebeing ignored on user tokens, including signed user tokens with string, list and malformedgty, and tokens whosegrant_typesorsubjectclaims try to stand in forgtyorsub;ruff format --check src tests authenticated_testsis clean.ruff check .passes on the whole repository withmain'sruff.toml, which now selects the group 1 rule families from Run ruff check in CI with the first group of rule families #765, using CI's pinned ruff 0.15.22.main.main(up to Run ruff check in CI with the first group of rule families #765): the entry project'stests/has 519 passed (withmain's new tests), and the quality guards andpython -m scripts.rules guard --base-ref origin/mainpass. The three deployment checks for permission enforcement are registered inmain's rules registry assim.deploy.permission-enforcement-wired.Related:
🤖 Generated with Claude Code