Repository navigation
Report job stage and progress on annual economy job status - #781
Merged
Merged
Conversation
The worker records which stage an annual job is in (loading_data,
baseline, reform or simulating, outputs) in a new Modal Dict keyed by
job_id. GET /jobs/{job_id} adds optional stage, progress, timestamps,
segment counts and per-stage durations. Clients that read only status
see no change.
The gateway reports a stage only for jobs routed to a worker whose
bundle manifest declares job_progress, so jobs on older versioned apps
are never shown as queued for their whole run. Store failures and
malformed records drop the stage fields and never change the status,
result or HTTP status code.
Fixes #780
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One week of production telemetry (6-13 October 2026) shows that the hand-chosen floors were far from the measured stage shares. Queue time is about 39% of a UK national run, outputs take almost no time, and the segmented calculation is about 96% of a US national run. A single table cannot fit both paths. - The worker records its execution path (monolithic or segmented), and the gateway picks the matching floor table. - Before the path is known, a stage uses the lowest floor that any path gives it, so progress never moves backwards. - run_simulation closes the last stage inside its operation and adds job_stage_seconds_<stage> attributes, so telemetry can split the baseline from the reform calculation. Refs #780 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The regional children of a segmented national run finish at about the same time, and California finishes last. A completed-segment count therefore jumps late and then waits, and a backtest on the 6-13 October runtime figures put it at 16-27 points of mean error. A time-based estimate held inside the stage range stays at about 7 points. - progress is the floor of the current stage on every path; segments stay in the response for captions. - progress_max is the highest percent a running job reaches before its next stage. While the path is unknown it is the highest limit of any path, so a client estimate is not held back. - The schema and README tell clients to accept stage values they do not know, because later versions may add stages. Refs #780 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 13, 2026
anth-volk
reviewed
Oct 14, 2026
anth-volk
left a comment
Contributor
There was a problem hiding this comment.
Two comments from a review of the stage reporting: a possible progress regression when Modal runs the same call again, and a mismatch between the description and what budget-window children do.
Modal can run the same call again after a preemption, under the same call id. run_simulation built a new reporter each time, so start() wrote a fresh loading_data record and progress dropped from reform (67) or outputs (97) back to 3 and 25. started_at and the stage durations also covered only the last attempt. start() now reads the stored record and continues it: started_at and the ended-stage durations carry over, attempts counts up, and the published stage stays at the earlier attempt's stage until this attempt reaches a stage that starts at least as far along. A finished earlier record is continued too, because a preempted attempt can run its finally block. run_simulation adds job_attempts to its telemetry. The docs now say that budget-window children, which are ordinary run_simulation calls, write their own records that nothing reads yet. Refs #780 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
main renamed fixtures/test_simulation_api_contracts.py to fixtures/macro_output.py, so the job progress tests failed to collect in CI against the merged branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nci-b36fbc # Conflicts: # projects/policyengine-simulation-executor/src/modal/segmented_national.py
The beta deployment's integration tests submit real US national, US state and UK jobs and poll them to completion. They now record every running and complete status body and check the stage fields: a known stage and progress <= progress_max <= 99 while running, progress that never moves backwards, and progress 100, started_at, finished_at and stage_durations on completion. A Stage 12-only deployment runs these tests against the existing gateway, which may predate stage reporting. The check applies only when the gateway reports submitted_at, which every gateway with stage reporting does. Refs #780 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 15, 2026
…nci-b36fbc # Conflicts: # libs/policyengine-simulation-contract/src/policyengine_simulation_contract/gateway_models.py # libs/policyengine-simulation-contract/tests/conftest.py # projects/policyengine-simulation-executor/src/modal/segmented_national.py # projects/policyengine-simulation-executor/src/policyengine_simulation_executor/baseline_artifacts.py # projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_builder.py # projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_runtime.py # projects/policyengine-simulation-executor/tests/test_baseline_artifacts.py # projects/policyengine-simulation-executor/tests/test_canonical_spm.py # projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py # projects/policyengine-simulation-gateway/fixtures/gateway_endpoints.py
Modal preempts a call with SIGINT, which run_simulation receives as KeyboardInterrupt, and then runs the call again under the same call id. The finally block called finish(), which marked the record finished, dropped the stage an earlier attempt had reached and published this attempt's own stage. A second preemption therefore moved progress backwards: an attempt preempted while loading data after an earlier one reached the reform published 25 instead of 67, and the next attempt held loading_data. run_simulation now calls the reporter's new interrupt() on KeyboardInterrupt, both during the calculation and before it starts. It counts the current stage's time but leaves the record running with any held stage, and finish() does nothing afterwards. A failed job still reports the stage it stopped in. The beta integration check now takes the stage list from the generated client's JobStage instead of a hand-written copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
marked this pull request as ready for review
October 15, 2026 18:31
anth-volk
added a commit
that referenced
this pull request
Oct 16, 2026
* Enable ruff group 2 (typing-only imports) and fix everything it finds Select TC and move imports used only in annotations under TYPE_CHECKING. Pydantic evaluates model annotations and FastAPI evaluates endpoint annotations at runtime, so ruff.toml lists the runtime-evaluated model base classes (every project base, since ruff only checks direct bases) and the FastAPI/APIRouter route decorators. scripts/rules matches policyengine-api. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register TC in the enforced ruff lint rule Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move the typing-only imports #789 added under TYPE_CHECKING #789 landed on main after this stack branched. TC, which this PR enables, flags the annotation-only imports in its new database-retry and failure-recording modules; they now load only under TYPE_CHECKING. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move the typing-only Callable import #795 added under TYPE_CHECKING Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move the typing-only imports #791 added under TYPE_CHECKING #791 landed on main after this stack branched. TC, which this PR enables, flags the imports its new Stage 12 failure-cause module uses only in annotations (Iterator, Mapping, ObservabilityRuntime, Stage and StagePlan); they move under TYPE_CHECKING. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * List the Pydantic bases ruff reads through imports as runtime-evaluated Ruff treats a class as runtime-evaluated when a base, read through the import statements of the class's own module, is listed in runtime-evaluated-base-classes, or is a class in that same module that is. It never follows a re-export, and only follows inheritance within a module. Four models had bases ruff reads at unlisted paths, so ruff would have moved imports their annotations need under TYPE_CHECKING if they gained any: - ArtifactBaselineSimulation and PrecomputedSimulation subclass policyengine.py's policyengine.core.Simulation; - AppSettings subclasses pydantic_settings.BaseSettings; - EconomyComparison subclassed SingleYearMacroOutput through the executor's simulation_macro_output re-export. The first two paths are listed as their subclasses import them. For EconomyComparison, compat_models now imports SingleYearMacroOutput from policyengine_simulation_contract.macro_output, the module that defines it, and that path is listed: every project entry then names its defining module, and the entry also covers subclasses elsewhere that import the class from the contract. The ruff.toml comment now describes how ruff resolves bases instead of "direct bases only". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Check that ruff treats every Pydantic model as runtime-evaluated The executor's test imports every module of the executor package and of the project libraries installed with it; the entrypoint's imports every module of its package. For every Pydantic model a module-level class statement defines, they check that: - ruff treats it as runtime-evaluated, resolving each base the way ruff does: through the model module's import statements as they stand at the class statement, reading Base[Item] as Base, following bases defined in the same module, and never following a re-export or an assignment. A failure names the model and the base path to list; - its annotations resolve, with model_rebuild(raise_errors=True). Unit tests pin the resolution: a listed base, a subscripted one, a relative import, a same-module chain through Generic, an unlisted re-export, an unlisted third-party base, an assigned alias, and a base in a package's __init__.py, which ruff names with __init__ in its path. The proof fixtures are compat_models.py before and after the previous commit. The executor's src/modal Modal entry files are not imported: they are not part of the installed package, and importing them builds Modal images and functions, which their tests do only with a fake Modal SDK and a patched environment. A static test checks that every class there derives only from builtins and the standard library, so none can be a model. The gateway defines no Pydantic models. The entrypoint shares no package with the executor that could hold test helpers, so it keeps a copy of the resolution helpers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register the runtime-evaluated model rule and document typing-only imports sim.quality.runtime-evaluated-models-resolve is enforced by the two new test files, which run in the executor and entrypoint jobs of the PR test matrix. Its nodes span both projects, so they are listed from the repository root without a project. The test files join the guard-test discovery globs, so any test added to them must be registered. docs/engineering/skills/typing-only-imports.md explains when ruff moves an import, how it decides a model is runtime-evaluated, and how to list model bases; AGENTS.md and the skills README point to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move the typing-only imports #781 added under TYPE_CHECKING #781 landed on main after this stack branched. TC, which this PR enables, flags the imports its contract job_progress module uses only in annotations (Mapping and datetime), which move under TYPE_CHECKING, and the cast in its new integration poll test, whose Client type is quoted. The module's Pydantic models annotate their timestamps with AwareDatetime, which stays a runtime import. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * List the StageReportingSimulation base #781 added as runtime-evaluated #781 landed on main after this stack branched. It added StageReportingSimulation, a policyengine.core.Simulation subclass, and made ArtifactBaselineSimulation subclass it from another module. Ruff reads that base through baseline_artifacts' import, which is not listed, so it would no longer treat ArtifactBaselineSimulation as runtime-evaluated. The sim.quality.runtime-evaluated-models-resolve guard names the path to list; it is added beside the other project bases. 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
* 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>
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 #780
Same-repository draft PR check
PolicyEngine/policyengine-sim-api, not a personal fork.Summary
GET /jobs/{job_id}reports onlyrunning,completeorfailed, so clients estimate progress from elapsed time. This PR makes the worker record which stage an annual economy job is in and adds optional stage, progress and timing fields to the status response. Clients that read onlystatussee no change.Phase 2 of #780 (runtime statistics: p50/p90 by country, region type and execution path) is not in this PR.
Response shape
Running, monolithic (for example a state run):
{ "status": "running", "result": null, "error": null, "resolved_app_name": "policyengine-simulation-py6-2-7", "policyengine_bundle": { "...": "..." }, "submitted_at": "2026-10-13T12:00:00Z", "started_at": "2026-10-13T12:00:30Z", "stage": "reform", "stage_started_at": "2026-10-13T12:01:00Z", "progress": 67, "progress_max": 96, "stage_durations": { "loading_data": 12.5, "baseline": 17.5 } }Running, segmented US national:
"stage": "simulating","segments": {"completed": 10, "total": 20},"progress": 5,"progress_max": 96.Complete:
"progress": 100,started_at,finished_atand the fullstage_durations; nostage. Failed:stageis the stage the job stopped in; noprogress.Stages and where each starts
queuedloading_datarun_simulationentry (country model, region, dataset, simulation build)baselineensure()of the baseline simulation, insideeconomic_impact_analysisreformensure()of the reform simulationsimulatingsegments.completedrises as each one returnsoutputseconomic_impact_analysisreturns (monolithic) or before the national reduce (segmented)economic_impact_analysiscallsbaseline.ensure()and thenreform.ensure()internally, so the runtime cannot observe the boundary from outside.StageReportingSimulation, a smallSimulationsubclass, calls back once when its firstensure()starts.ArtifactBaselineSimulationnow extends it, and_build_simulationreturns it on the plain path.Progress values
progressis the percent at which the current stage starts in a typical successful production run. The values depend on the execution path, which the worker records asmonolithicorsegmented. They come from the production runtime analysis of 6–13 October 2026 (submission-to-completion time):queuedloading_databaselinereformsimulatingoutputseconomic_impact_analysistake almost no time (US state: 0.25 s), sooutputsstarts at 97.progress. Anthony confirmed that the 20 regional children finish at about the same time and California finishes last. A segment count then jumps late and waits; a backtest on the October figures gave it 16–27 points of mean error.segmentsstays in the response for captions such as "19 of 20 regions done".progress_maxis the highest percent a running job reaches before its next stage (one below the next floor, or 99 for the last stage). For a smooth bar, a client moves its own time-based estimate betweenprogressandprogress_maxand never moves the bar backwards. In the same backtest, this hybrid stayed at 3–15 points of mean error for normal runs, against 20–32 for the current fixed-duration bar on UK and US state runs. While a job isqueuedits path is unknown, soprogress_maxis the highest limit of any path (24).Later versions may add stages. The schema and README tell clients to accept a
stagevalue they do not know. Clients generated with strict enums from an older schema would reject a new value; API v1 reads plain dict keys and is not affected.The values stay provisional until phase 2 of #780 measures them per country and region type.
Telemetry
run_simulationnow closes its last stage inside its operation and setsjob_stage_seconds_<stage>attributes, for examplejob_stage_seconds_baselineandjob_stage_seconds_reform. These are the first main-path measurements that separate the baseline from the reform calculation. The attribute set is bounded byJobStage;docs/engineering/skills/observability.mddocuments it.Compatibility and failure handling
JobStatusResponsefields are optional and omitted when empty. New submissions addsubmitted_at; jobs submitted before deployment keep a byte-identical body.queuedfor the whole run.build_bundle_manifest_metadatanow declaresjob_progress: 1; the gateway stores the routed app's capability in the job metadata and reports a stage only when it is present.started_atand the ended-stage durations carry over,attemptscounts up, and the published stage stays at the earlier attempt's stage until the new attempt reaches a stage at least as far along.progresstherefore never moves backwards.run_simulationaddsjob_attemptsto its telemetry.try, so result-validation failures still produce the redactedfailedbody.data.get(...), so the extra keys do not affect it.Changes
job_progress.py(JobStage,JobProgressRecord,JobSegmentProgress, the provisionalSTAGE_PROGRESS_FLOOR,job_progress_fields) andjob_progress_state.py(Modal Dictsimulation-api-job-progress).JobStatusResponsegains the optional fields.modal_job_progress_readin the annual-impact plan; the gateway measures the read with it.StoredJobProgressReporter(created only inrun_simulation, keyed bycurrent_function_call_id()), theStageReportingSimulationhook, the execution path indispatch_run_simulation, theoutputstransition inSimulationOutputBuilder.analysis, segment counting inSegmentedNationalRunner, the stage-duration attributes, and thejob_progressmanifest key. Segment children, budget-window parents and Stage 12 use the no-op reporter. Budget-window children are ordinaryrun_simulationcalls, so each writes a record under its own call id;/budget-window-jobsdoes not read those records yet.submitted_atand the capability at submit, and adds the fields to the 200, 202 and 500 status responses.README.mddocuments the fields.Testing
progressnever decreases over all written records.ruff format --checkpasses for everysrcfolder.ruff checkon the changed source files reports no new findings compared withmain(one existing import-order finding is fixed).with runtime.span(...)blocks, andmodal.current_function_call_id, whichapp.pyalready calls.Existing tests changed only at their seams: the plain-simulation test double now replaces
StageReportingSimulationinstead ofpolicyengine.core.Simulation, two dispatch fakes acceptprogress, and the simulation doubles gained a no-opon_first_ensure.Deployed verification: the beta deployment's integration tests (
projects/policyengine-apis-integ/tests/simulation/test_calculate.py) submit real US national, US state and UK jobs. They now record every running and complete status body and check:stageandprogress ≤ progress_max ≤ 99;progressnever moves backwards;progress100,started_at,finished_atandstage_durations, and nostage.A Stage 12-only deployment runs these tests against the existing gateway, which may predate stage reporting, so the check applies only when the gateway reports
submitted_at(every gateway with this PR does). Six unit tests cover the check itself and run in PR CI.Not run locally: no Modal deployment and no live economy job; the beta deployment runs the checks above.
Notes for review
job_stage_seconds_*attributes will show which one.simulatingbetter than separatebaselineandreformstages.libs/policyengine-simulation-contract/uv.lockonmainstill pinspolicyengine-observability3.0.1, while feat: measure Stage 12 coordinator preparation #708 raised the floor to 3.0.2.uv syncrewrites it locally. This PR leaves the lockfile unchanged.Downstream
stage,progress,progress_max, the timestamps andsegmentson the economycomputingresponse. It can merge before or after this PR.progressinSocietyWideCalcStrategyand name the stage in theRunProgresscaption. Keep the time-based estimate as the fallback.🤖 Generated with Claude Code