Skip to content

Enable ruff structure and import rules (INP, PLC, SLF, N, A, FAST, E) - #799

Merged
anth-volk merged 23 commits into
mainfrom
ruff-structure-imports
Oct 16, 2026
Merged

anth-volk merged 23 commits into
mainfrom
ruff-structure-imports

Conversation

@anth-volk

@anth-volk anth-volk commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #798

Same-Repository Draft PR Check

  • This PR is a draft.
  • This PR is from a branch in PolicyEngine/policyengine-sim-api, not a personal fork.

Summary

Phase 1 step 1.8, group 7: structure and imports. Bottom of the stack since #785 merged.

  • Test exemption (decided for this group): **/tests/** and **/authenticated_tests/** now also ignore SLF001, INP001 and PLC0415, alongside S101, D1, ANN and PLR2004. 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. Both test-exemption rules in the registry are updated to match.
  • Rules: ruff.toml gains A, E, FAST, INP, N, PLC and SLF. All 339 findings left after the exemption are fixed or kept with an inline reason.
  • Imports inside functions (PLC0415, 176):
    • Moved (16): only imports that load nothing new, create no cycle and aren't patched by tests. Each was checked by importing every affected module in a fresh interpreter.
    • Listed as lazily loaded (51 suppressions removed):
      • What's listed: 14 heavy or optional packages and modules that no application module imports at module level:
        • the policyengine bundle, countries, provenance and tax_benefit_models modules, and policyengine_us;
        • spm_calculator, h5py, google.cloud and pythonjsonlogger;
        • three OpenTelemetry exporters and instrumentations;
        • the executor's spm and baseline_artifacts modules.
      • Mechanism: they're in ruff's banned-module-level-imports, with TID253 selected. Importing one at module level now fails, though TYPE_CHECKING imports are fine. PLC0415 accepts their in-function imports without a suppression.
      • Tests may still import them at module level, because TID253 joins the test exemptions.
    • Kept (116), each with a reason: Modal and image code, test seams, packages that some application module does import at module level (pandas, huggingface_hub, policyengine.core and others), and import cycles. Roadmap items cover the Modal imports (image.imports()) and the test seams.
  • Namespace packages (INP001, 13): .github/scripts, scripts, the executor's scripts and rules/fixtures aren't packages, so they're declared in ruff's top-level namespace-packages setting. Compared with the full ruff check output before the change, nothing else differs. The nine executor src/modal/utils programs take INP001 in their existing file-level entry (a program under src) instead of inline. Declaring that directory as a namespace package made ruff treat import modal (the Modal SDK) as first-party, and an __init__.py would change the source copied into the Modal images. A per-file ignore changes neither.
  • Names (N, A, PLC0414, SLF001):
    • Renamed exceptions: the entry app's BackendUnavailableError, BackendTimeoutError and TemporaryStage12UnsupportedRequestError, and the executor's ParityMismatchError. Each is caught by name and never logged or stored by class name.
    • Kept: TemporaryStage12DispatchFailed, because the entry app logs its class name as error_type.
    • Published fields: the 14 camelCase budget-window fields in gateway_models.py keep their names.
    • Other renames: three OpenTelemetry locals, the exit import aliased as app_exit, and four gateway test names shortened for line length.
    • Kept with a reason: the contract's explicit re-exports, the exit object projects import, and two private accesses with no public equivalent.
  • FastAPI (FAST001, 13): response_model arguments that repeat the return type are dropped. The gateway golden OpenAPI is unchanged, and the entry, executor and gateway OpenAPI dumps are identical before and after.
  • Line length (E501): wrapped, with every split string's text identical, which a syntax-tree comparison confirms. Four multi-line fake shell scripts in tests keep a suppression, because their one-line printf JSON can't be split without changing the script.
  • Modal images: many executor and gateway image files change, mostly in import placement and line wrapping; src/modal/app.py gains no __future__ import. CI's image smoke will run when this PR reaches the bottom of the stack.
  • Registry: sim.quality.ruff-lint now lists the seven families and TID253. The rules tool is synced from the Public API's group 7 (Enable ruff structure and import rules (INP, PLC, SLF, N, A, FAST, E) policyengine-api#3920).

Testing

  • Suites: at this branch's tip after today's restack onto main, all seven passed:

    Project Passed
    executor 1,190 (22 skipped)
    entry 529
    gateway 243
    contract 192
    observability 31
    stage12 17
    fastapi 14

    The lazy-import list and the INP001 change touch project code only in noqa comments and configuration. The suites weren't re-run for them.

  • At each tip from Enable ruff structure and import rules (INP, PLC, SLF, N, A, FAST, E) #799 to Register weighted-microdata rules, weight the builder tests, and check change weights and household counts #807:

  • The lazy-import mechanism was checked with ruff 0.15.22 in a small probe:

    • listing a package exempts in-function imports of it and its submodules from PLC0415;
    • TID253 flags a module-level import of it;
    • an import under TYPE_CHECKING is still allowed.
  • Golden OpenAPI: the gateway golden OpenAPI test passes, and the golden file is unchanged.

  • Integration tests: the conftest change in the integration-test project was checked by parsing only, because its tests need live services.

  • CI/CD: no workflow changed, and no environment variable was added or rewired.

  • CI: this is now the lowest unmerged PR in the stack, so its full CI runs.

🤖 Generated with Claude Code

@anth-volk
anth-volk added this pull request to stack #755 October 14, 2026 13:54
@anth-volk anth-volk changed the title ruff structure imports Enable ruff structure and import rules (INP, PLC, SLF, N, A, FAST, E) Oct 14, 2026
@anth-volk
anth-volk force-pushed the ruff-structure-imports branch 2 times, most recently from 2a7ddcc to 30b23b7 Compare October 14, 2026 14:47
@anth-volk
anth-volk force-pushed the ruff-structure-imports branch 4 times, most recently from b013982 to 548762d Compare October 14, 2026 20:04
@anth-volk
anth-volk force-pushed the ruff-structure-imports branch 4 times, most recently from 51e611a to fddf990 Compare October 15, 2026 14:06
@anth-volk
anth-volk force-pushed the ruff-structure-imports branch 2 times, most recently from 4bd1b75 to f523429 Compare October 15, 2026 16:06
@anth-volk
anth-volk force-pushed the ruff-structure-imports branch from f523429 to 5039fe2 Compare October 15, 2026 21:56
@anth-volk
anth-volk force-pushed the ruff-structure-imports branch 5 times, most recently from f773fd9 to 87a863d Compare October 16, 2026 05:58
Base automatically changed from ruff-security-logging to main October 16, 2026 06:06
anth-volk and others added 2 commits October 16, 2026 10:06
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>
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>
anth-volk and others added 20 commits October 16, 2026 10:06
- 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>
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>
…(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>
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>
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>
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>
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>
Bind the long generated endpoint functions to shorter local names before
calling them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
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>
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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
sim.quality.ruff-lint now lists A, E, FAST, INP, N, PLC and SLF.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
#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>
#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>
#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>
#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>
#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>
@anth-volk
anth-volk force-pushed the ruff-structure-imports branch from 87a863d to 19b613a Compare October 16, 2026 06:06
…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>
@anth-volk
anth-volk marked this pull request as ready for review October 16, 2026 06:39
@anth-volk
anth-volk requested a review from a team as a code owner October 16, 2026 06:39
@anth-volk
anth-volk merged commit 5980a90 into main Oct 16, 2026
14 checks passed
@anth-volk
anth-volk deleted the ruff-structure-imports branch October 16, 2026 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable ruff structure and import rules (INP, PLC, SLF, N, A, FAST, E)

1 participant