Skip to content

Refactor within ruff's complexity limits, then select every ruff rule (ALL) - #804

Draft
anth-volk wants to merge 69 commits into
ruff-idioms-argumentsfrom
ruff-complexity
Draft

anth-volk wants to merge 69 commits into
ruff-idioms-argumentsfrom
ruff-complexity

Conversation

@anth-volk

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

Copy link
Copy Markdown
Contributor

Fixes #803

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 8b, the last group: complexity refactors, then select = ["ALL"]. Stacked on #802. This completes step 1.8.

Every function over ruff's complexity limits is refactored, not suppressed. Each refactor is preceded by a commit that adds characterization tests pinning the current behaviour. Every commit passes ruff and the quality guards.

  • Contract and persistence libraries:

    • ComparisonReportRecord.validate_state keeps its Pydantic signature and is split into report and comparison checks.
    • The live column-type check is split into enum and plain-type checks.
    • URI, version and name-length limits are named constants.
  • Entry:

    • create_app (complexity 52, 144 statements) keeps every route's name, decorators, parameters and registration order. The shared behaviour moves into _EntryRuntime, and Stage 12 error logs still record only the error type.
    • Settings validation is split into caller-auth, old-gateway and Stage 12 checks.
    • The Stage 12 adapter is split into eligibility checks and a report builder.
    • Stage 12 dispatch takes a prepared report, an origin and a worker version.
  • Gateway: active-state route resolution is split into pinned, latest and requested-version steps.

  • Executor: long argument lists become small frozen groups, including:

    • artifact key inputs;
    • baseline identity requests;
    • precompute function handles;
    • national-reduce inputs;
    • fan-out poll backoff;
    • qualification runners and tolerances;
    • Stage 12 coordinator invocation and dependencies;
    • child spawn requests;
    • worker seams.

    Stage 12 coordination (complexity over 10, 11 arguments, 109 statements) becomes a _ReportCoordination class with claim, plan, dispatch, wait and aggregate phases. The Stage 12 parity checks are split by entity, column and report value. The image artifact fetch is split into freshness, credential and download steps.

  • Code from Expose the certified dataset SHA-256 and assert the served identity at deploy #743, Gate releases on a numerical sentinel and make them reversible #744 and Release Stage 12 database sessions and keep original failures visible #789, which merged while this was in progress, is refactored the same way:

    • release_sentinel.check_deployment (JobPolling, a _Deployment with per-case and job-wait steps, HTTPStatus.OK).
    • The routing publisher's main: one command function per mode, with the same precedence. Its --help output is byte-identical.
    • restore_routing_plans.
    • The package updater's bundle_versions and verify_dispatch.
    • open_connection_with_retry (RetryTiming).
    • The workflow-expression and routing-command test helpers.

    Release Stage 12 database sessions and keep original failures visible #789's original-failure reporting and session release are kept inside the new coordination phases.

  • Kept with a reason (3): # noqa: PLR0913 on three library functions other projects import: create_initial_batch_state, resolve_spm_selection and init_simulation_observability. Modal function signatures, local entrypoints, FastAPI routes and Pydantic validators keep their signatures.

  • Rules tool: takes the Public API's group 8b refactor (Refactor tests, scripts and the rules tool, then select every ruff rule (ALL) policyengine-api#3934). Each repository owns its copy; this one is currently identical. Its tests went from 74 to 109.

  • ALL:

    • ruff.toml selects every rule, and ignores only the two that conflict with ruff format (COM812, ISC001) and one of each mutually exclusive docstring pair (D203, D213).
    • The per-file exemptions are unchanged.
    • ALL found nothing that wasn't already fixed.
    • sim.quality.ruff-lint lists ALL.
    • The rules-registry skill doc says every rule applies, and that a ruff upgrade PR fixes any new rule's violations.

Noticed, not changed: in release_sentinel, a job that fails but still returns an object result has its values compared anyway. The new tests pin that existing behaviour. It may be worth confirming it's intended.

Testing

Every suite passes:

Project Base This PR
fastapi 14 14
contract 147 201
observability 30 30
stage12 17 38
entry 444 528
gateway 230 247
executor 1,060 1,265

The additions are the characterization tests: 381 in all. The gateway has 1 test deselected and the executor 22 skipped and 3 deselected, as on the base. The grouping renamed 7 parametrized test IDs in the entry's deploy-script tests, and 34 in the gateway's SPM route tests.

  • OpenAPI: the gateway golden OpenAPI test passes, with tests/golden unchanged. The entry, executor and gateway OpenAPI dumps are byte-identical to the base.
  • Lint: ruff check . with ALL gives 0 findings, and ruff format --check . passes.
  • Rules tool and guards: the rules tool's 109 tests pass on Python 3.11. The quality guards pass at every commit, and the gate test passes at the tip.
  • Modal images: files in the executor and gateway images change, all inside function bodies or module helpers. src/modal/app.py is untouched.
  • CI/CD: no environment-variable read was added or changed; some reads only moved within changed files.
    • RELEASE_PAYLOAD is set from the check-policyengine-updates.yml step env:.
    • The sentinel's SIMULATION_TEST_AUTH_* values come from the SIMULATION_ENTRYPOINT_* secrets through secrets: inherit.
    • The image fetch's GOOGLE_APPLICATION_CREDENTIALS_JSON comes from the gcp_secret Modal secret.
    • I haven't checked that those secrets exist in the GitHub and Modal settings.
  • CI: above the bottom of its stack, so only CI complete runs until the PRs below merge.

🤖 Generated with Claude Code

@anth-volk
anth-volk added this pull request to stack #755 October 14, 2026 18:50
@anth-volk anth-volk changed the title ruff complexity Refactor within ruff's complexity limits, then select every ruff rule (ALL) Oct 14, 2026
@anth-volk
anth-volk force-pushed the ruff-complexity branch 3 times, most recently from 96b676c to 358d65a Compare October 14, 2026 21:06
@anth-volk
anth-volk force-pushed the ruff-complexity branch 3 times, most recently from 257e32d to c0bea9c Compare October 15, 2026 14:06
anth-volk and others added 16 commits October 16, 2026 01:28
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
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>
… comparison phases

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ge 12 checks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and a report builder

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ker version

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ion 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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk and others added 28 commits October 16, 2026 01:28
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s validation

The update_version_registry command-line options are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…cases

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d 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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ompare 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>
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>
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>
…utput

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pers

The helper's subcommands, output lines and messages are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… 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 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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ecks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
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>
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>
…tended

#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>
…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>
#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>
#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>
…e as one group

#791 landed on main after this stack branched. PLR0913, which this PR
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>
…agic 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>
#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>
#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>
… 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>

This branch has not been deployed

No deployments
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.

Refactor within ruff's complexity limits, then select every ruff rule (ALL)

1 participant