Skip to content

Accept null SPM provenance in Stage 12 aggregate artifacts - #787

Merged
anth-volk merged 3 commits into
mainfrom
fix/stage12-null-spm-provenance
Oct 14, 2026
Merged

anth-volk merged 3 commits into
mainfrom
fix/stage12-null-spm-provenance

Conversation

@anth-volk

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

Copy link
Copy Markdown
Contributor

Fixes #786

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

Stage 12 rejected every aggregate report without an SPM calculation, which includes every UK report. Between 6 and 13 October 2026 this caused 42 of the 153 production Stage 12 failures. All 42 were UK nationwide reports whose baseline and reform simulations had already succeeded.

  • Cause: SingleYearMacroOutput.spm_provenance is optional and defaults to None, so a report with no SPM calculation serializes "spm_provenance": null. AggregateReportArtifactPayload.validate_spm_provenance checked only that the key was present. It then validated null as an SPMComparisonProvenance object, which failed inside Stage12ArtifactStore.write_aggregate. The coordinator recorded the failure as the generic Stage 12 report coordination failed.
  • Fix: the validator now validates spm_provenance only when a value is supplied. An absent or null field is accepted. Every non-null value is still validated, so malformed or mismatched provenance records are still rejected.
  • Why the tests missed it: the coordinator tests' artifact double, FakeArtifacts.write_aggregate, saved any payload without validating it. It now writes through the real Stage12ArtifactStore, so the coordinator tests enforce the same aggregate contract as production.
  • Deployment: the change is in the contract library used by the Stage 12 coordinator. It takes effect with a stage12-deploy.yml run and needs no change to the selected worker version. The other changes are test-only.

Testing

  • Rebased on main after Release Stage 12 database sessions and keep original failures visible #789 and Add a rules registry that makes every enforced and unenforced rule visible #752. The three coordinator tests Release Stage 12 database sessions and keep original failures visible #789 added now use _aggregate_payload too, because one of them aggregates successfully and the real store rejects {"result": "complete"}.
  • New contract tests in libs/policyengine-simulation-contract/tests/test_stage12_execution.py:
    • An absent spm_provenance and a null one are both accepted.
    • Present but invalid values ({}, a string, a list, 0) are rejected.
  • New executor test in projects/policyengine-simulation-executor/tests/test_stage12_runtime.py:
    • It builds a UK report aggregate with build_aggregate_report, using the real SingleYearMacroOutput serialization.
    • It writes that aggregate with FakeArtifacts.write_aggregate, which now calls the real Stage12ArtifactStore.write_aggregate over an in-memory object store.
    • It checks that the stored artifact keeps spm_provenance: null and matches the returned digest.
  • The coordinator test double now validates aggregates:
    • FakeArtifacts.write_aggregate delegates to a real Stage12ArtifactStore, so it runs the same AggregateReportArtifactPayload validation and encoding as production. The stored aggregate path now follows the production layout under the run's artifact prefix, and one coordinator test checks that path.
    • With validation on, 10 coordinator tests failed because their stand-in aggregators returned payloads that never met the contract, such as {"result": "complete"}. They now return a valid payload from a _aggregate_payload helper, with the same result as before.
    • The stand-in aggregators that must not run also return a valid payload. If one ran, its aggregate would be saved and the existing aggregate_writes == [] checks would fail, rather than a validation error hiding the run.
  • Shared test support, following docs/engineering/skills/testing.md:
    • fixtures/fake_object_store.py holds InMemoryObjectStore. It replaces the two identical copies in test_stage12_artifacts.py and test_stage12_runtime.py.
    • fixtures/macro_output.py, renamed from fixtures/test_simulation_api_contracts.py, holds the shared macro result and its key sets. The Stage 12 runtime, output builder and standalone app tests no longer import the API contract tests' fixture module.
  • The new tests reproduce the defect. With the previous validator, the null-provenance contract case and the UK aggregate test both fail with ValidationError. In a one-off check with the previous validator, a coordinator run whose aggregate carries spm_provenance: null also failed through the validating double, recording ValidationError. The absent-field and invalid-value cases pass both before and after, as guards should.
  • Suites, on Python 3.13 with uv run --frozen:
    • libs/policyengine-simulation-contract: 153 passed.
    • projects/policyengine-simulation-executor, all of tests/: 1058 passed, 22 skipped, 3 deselected (the integration marker).
    • Of those, tests/test_stage12_runtime.py and tests/test_stage12_artifacts.py: 54 passed.
  • Formatting and lint: with CI's pinned ruff 0.15.22, ruff format --check, ruff check and git diff --check pass on every changed file.
  • Replay of a failed report: the coordinator's aggregate for one failed production UK report was rebuilt from its saved simulation outputs. It failed validation before this change and validates after it.

🤖 Generated with Claude Code

@anth-volk
anth-volk force-pushed the fix/stage12-null-spm-provenance branch from f7525ee to 8278e12 Compare October 14, 2026 17:17
anth-volk and others added 3 commits October 14, 2026 23:49
Reports without an SPM calculation, including every UK report, serialize
the optional spm_provenance field as null. The aggregate validator checked
only that the key existed and then validated null as an
SPMComparisonProvenance object, so every such aggregate was rejected
before it was saved.

Validate the field only when a value is supplied. Malformed provenance
records are still rejected.

Fixes #786

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Stage 12 artifact and runtime tests each defined the same in-memory
object store, and the runtime tests imported the macro result from the
API contract tests' fixture module. Move the object store to
fixtures/fake_object_store.py and the macro result and its key sets to
fixtures/macro_output.py, and import both from there in every test that
uses them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FakeArtifacts.write_aggregate encoded whatever it was given, so no
coordinator test enforced AggregateReportArtifactPayload, and the null
SPM provenance rejection reached production without a failing coordinator
test. Write aggregates through the real Stage12ArtifactStore over the
shared in-memory object store instead, so the coordinator tests run the
same validation and encoding as production.

The coordinator tests' stand-in aggregators returned payloads such as
{"result": "complete"} that never satisfied the contract. They now return
a contract-valid payload with the same result. Aggregators that must not
run return one too, so an unexpected run is still caught as a written
aggregate rather than hidden by a validation failure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@anth-volk
anth-volk force-pushed the fix/stage12-null-spm-provenance branch from 25c1ee0 to 0951722 Compare October 14, 2026 19:50
@anth-volk anth-volk mentioned this pull request Oct 14, 2026
2 tasks done
@anth-volk
anth-volk force-pushed the fix/stage12-null-spm-provenance branch from 0951722 to d29ce9d Compare October 14, 2026 19:56
@anth-volk
anth-volk marked this pull request as ready for review October 14, 2026 20:13
@anth-volk
anth-volk merged commit fc1ddca into main Oct 14, 2026
14 checks passed
@anth-volk
anth-volk deleted the fix/stage12-null-spm-provenance branch October 14, 2026 20:13
anth-volk added a commit that referenced this pull request Oct 14, 2026
#787 made the coordinator tests' artifact double write aggregates through
the real Stage12ArtifactStore, which rejects anything that is not a valid
AggregateReportArtifactPayload. Use _aggregate_payload for this PR's
aggregator stubs, as #787 did for main's. The redacted comparison-failure
test needs a valid aggregate for the report to succeed. An aggregator that
must not run now shows up as a written aggregate if it does run, instead of
as a rejection hidden by the failure the test expects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk added a commit that referenced this pull request Oct 14, 2026
#787 made the coordinator tests' artifact double write aggregates through
the real Stage12ArtifactStore, which rejects anything that is not a valid
AggregateReportArtifactPayload. Use _aggregate_payload for this PR's
aggregator stubs that must not run, as #787 did for main's, so an
unexpected run shows up as a written aggregate instead of as a rejection
hidden by the failure the test expects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk added a commit that referenced this pull request Oct 14, 2026
#787 made the coordinator tests' artifact double write aggregates through
the real Stage12ArtifactStore, which rejects anything that is not a valid
AggregateReportArtifactPayload. Use _aggregate_payload for this PR's
aggregator stubs that must not run, as #787 did for main's, so an
unexpected run shows up as a written aggregate instead of as a rejection
hidden by the failure the test expects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk added a commit that referenced this pull request Oct 14, 2026
#787 landed on main after this stack branched. It renamed the macro result
fixture module to fixtures.macro_output and added fixtures.fake_object_store,
and those imports in the output builder, Stage 12 artifact and Stage 12
runtime tests are unsorted under the I rule this PR enables.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk added a commit that referenced this pull request Oct 14, 2026
…n class (#791)

* Record a redacted Stage 12 failure cause instead of only the exception class

Stage 12 coordinator, worker and result-comparison failures stored only
type(error).__name__, so a database session limit, a UK aggregate
validation defect and an invalid reform path all appeared as RuntimeError
or ValidationError behind "Stage 12 report coordination failed".

Failure records now store a bounded summary of the form
"<type> in|after <stage> (<role>): <message>", capped at the 512-character
error_summary limit. Messages are redacted first: URL credentials, signed
URL query strings, secret-looking assignments, bearer and known token
formats, this process's secret environment values, Pydantic input values
and SQL statement parameters are removed. Stage tracking follows the
existing registered span names, so the summary names the stage that failed
or the last stage that finished before a failure between spans.

The coordinator no longer overwrites a cause that the failed worker already
persisted, and copies that cause onto the report record. The redacted
cause, with the original stack, is also recorded on the active operation
before the failure is persisted. The exceptions returned to Modal are
unchanged.

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

* Redact more credential formats in Stage 12 failure causes

Redact quoted JSON and dictionary keys, non-Bearer Authorization schemes
and cookie headers, signed query strings without a URL scheme, and the
row values that PostgreSQL constraint violations echo in their detail,
from both the stored summary and the exported cause.

Patterns that could start inside a run of word characters now start
only where that run starts, so redaction is linear in the message
length. Redaction also reads at most 4,096 characters: the word that
limit cuts is dropped, and quoted values, SQL parameters and validation
inputs whose closing delimiter was cut are redacted to the end.

Take the summary length from the contract's ErrorSummary, detect
SQLAlchemy errors by type, and state in the module docstring that span
exception events are outside this redaction.

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

* Share one stage helper for Stage 12 spans and failure progress

Replace the coordinator's stage_scope and stage_span closures and the
worker's _simulation_stage with tracked_stage, which opens a stage's
span or operation and records it as the process's progress. The
coordinator claim span uses it too.

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

* Test dispatch and comparison failure summaries in Stage 12

Assert the redacted summary stored on a child and its report when Modal
rejects a dispatch, and the comparison_error_summary stored when the
production result cannot be read. Reword fixture failure messages that
described themselves as sensitive, since the tests now assert those
messages are stored.

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

* Return valid aggregates from the Stage 12 test aggregators

#787 made the coordinator tests' artifact double write aggregates through
the real Stage12ArtifactStore, which rejects anything that is not a valid
AggregateReportArtifactPayload. Use _aggregate_payload for this PR's
aggregator stubs, as #787 did for main's. The redacted comparison-failure
test needs a valid aggregate for the report to succeed. An aggregator that
must not run now shows up as a written aggregate if it does run, instead of
as a rejection hidden by the failure the test expects.

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 15, 2026
* Enable ruff group 1 (safe auto-fixes) and fix everything it finds

Select FLY, FURB, I, PIE, PYI, RET, RSE, UP and W, and fix every finding:
ruff's safe fixes (import sorting, pyupgrade), then by hand:
- policyengine-fastapi re-exports JWTDecoder through __all__ and drops two
  unused imports (the 9 F401s noted in Phase 0).
- Two tests build their JSON receipts with json.dumps, not % formatting.
- smoke_app uses min() rather than sorted()[0] (ruff 0.16 FURB192).
scripts/** is linted as Python 3.11, the shared rules tool's minimum, so
import sorting matches policyengine-api byte for byte.

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

* Run ruff check in CI and enforce the lint rule

The lint job now runs ruff check . on the whole repository, and
sim.quality.ruff-lint moves from advisory to enforced with the codes
selected in ruff.toml.

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

* Add a changelog entry for ruff group 1

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

* Sort the imports in the budget output test from #767

#767 landed on main after this stack branched, and its new test imports
are unsorted under the I rule this PR enables.

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

* Sort the imports #743 and #744 added

#743 and #744 landed on main after this stack branched. A dataset-digest
test from #743 and the tomllib import #744 added to the deployment assets
test are unsorted under the I rule this PR enables.

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

* Sort the imports #787 added

#787 landed on main after this stack branched. It renamed the macro result
fixture module to fixtures.macro_output and added fixtures.fake_object_store,
and those imports in the output builder, Stage 12 artifact and Stage 12
runtime tests are unsorted under the I rule this PR enables.

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

* Sort the imports #791 added

#791 landed on main after this stack branched. I, which this PR enables,
flags the import block of its new Stage 12 failure-cause tests; ruff's
safe fix sorts it.

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 19, 2026
* Add Stage 12 segmentation foundations

* Add Stage 12 segment calculation and merge

* Add Stage 12 cache identity and loading

* Add Stage 12 cache precompute app

* Bake Stage 12 caches into US workers

* Fan out Stage 12 US simulations

* Qualify and document Stage 12 execution

* Fix Stage 12 precompute image sources

* Validate Stage 12 cache deploy access

* Keep Stage 12 deployment validation independent

* Validate Stage 12 reform intervals

* Preserve Stage 12 failure details

* Observe Stage 12 simulations concurrently

* Document Stage 12 failure handling

* Reconcile segmented Stage 12 execution with current typed provenance

* Identify the report and child in Stage 12 cancellation logs

Cancellation state-update and cancel-request failures previously logged
only the simulation role and error type. They now also log the report's
evaluation_id and the child's simulation_execution_id, in the message and
as record attributes, and still omit the error's own message.

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

* Stop started Stage 12 workers when coordinator startup fails

Only a failed spawn used to cancel already-started children. Any other
coordinator failure after the first spawn, such as a failed dispatch
placeholder write, a failed invocation attach, a failed restore, or a
retained-artifact error, reached the report failure handler, which marked
the report failed but left every started worker running until its own
timeout.

The handler now stops every started, unfinished child with the existing
cancellation path: it records the child as incomplete with
cancelled_after_coordinator_failure, cancels an unsegmented call, and
lets a segmented call stop itself once it reads that record. Children
that already finished are left unchanged. Child, dispatch and wait-timeout
failures keep their own peer cancellation, and cancellation or
state-update errors are logged without replacing the original failure.

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

* Open the Stage 12 child wait window at coordinator entry

The child-wait deadline used to start when observation began, after
claiming, output planning (including the country model load),
retained-artifact reads and dispatch. Setup time therefore came out of
the coordinator's remaining Modal time rather than the children's
window.

The 3,000 s window now opens when coordinate_report starts, so setup
shortens the children's window and the coordinator always keeps 2,100 s
of its 5,100 s Modal timeout after the window: up to 900 s for the
production-result wait and 1,200 s to read results, aggregate, write
artifacts and record the outcome. The timeout summary now says the
window is measured from the coordinator's start.

A new test runs the coordinator with 0, 400 and 2,000 s of simulated
setup, checks that the window closes 3,000 s after entry, and checks the
declared Modal timeouts still leave the production wait plus at least
600 s.

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

* Size the Stage 12 wait window and coordinator timeout from worker timeouts

The coordinator stopped waiting 3,000 s after it started, although a US
simulation worker may run for 3,900 s and each of its region-group calls
for 3,600 s, so US simulations could never use the time Modal allowed
them. The coordinator's 5,100 s timeout was not derived from either.

Following the rule from #793, a new dependency-free stage12_timeouts
module defines the worker timeouts, sets the shared child-wait window to
the longest of them (3,900 s), and derives the coordinator's Modal
timeout as that window plus the 900 s production-result wait plus 600 s
to read results, aggregate, write artifacts and record the outcome:
5,400 s. A complete testing-environment US report took 288 s, so 600 s
of finishing time is ample. The Modal app, coordinator and comparison
step all read these constants.

The last poll no longer sleeps past the deadline, so the window closes
on time instead of up to 1 s late and the finishing time is not eroded.

Tests read the declared Modal timeouts and check that the window covers
every simulation worker's timeout and that the coordinator's timeout
leaves the production wait plus at least 600 s. They also check that
setup time shortens the window and that an exhausted window gives the
children no extra time. Only the Modal timeout values change: no image,
secret or environment variable attachment changes.

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

* Record cancelled Stage 12 simulations as failed and always stop them

Adopt #793's cancellation practice in the concurrent coordinator:

- Every simulation the coordinator stops, after a peer failure, a wait
  timeout or a coordinator failure, is recorded as failed with
  simulation_cancelled, and its error_summary gives the reason, for
  example "Cancelled after the baseline simulation failed.". The
  cancelled_after_peer_failure and cancelled_after_coordinator_failure
  codes and the incomplete status for cancellations are gone; the wait
  timeout stays the report's error code.
- Only a running record changes, so a simulation that already succeeded
  or failed keeps its outcome. Workers already treat any status other
  than running as a cancellation, so a segmented worker still sees the
  failed record and cancels its own region-group calls.
- An unsegmented simulation is always cancelled through Modal, even when
  its record could not be written. A segmented simulation is cancelled
  through Modal only when its record could not be written, because Modal
  does not cancel spawned calls recursively. Cancellation and
  state-update errors are still logged without replacing the original
  failure.

Carry over #793's tests for a cancelled and recorded reform, a finished
baseline left alone, a restored call cancelled on retry, finished
siblings left unchanged, and both unfinished children cancelled on
timeout. Add tests for the Modal fallback after a failed state write,
for segmented and unsegmented simulations. ImmediateCall now counts
cancellations, and the entry and persistence fixtures use the new
cancellation record. Add a changelog line, as #793 did.

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

* Test that a closed Stage 12 wait window is reported first

The rebase onto #789 reports a closed child-wait window to telemetry
before cancellation and report failure records are written, as every
other Stage 12 failure is. This test makes those writes fail and checks
that the timeout still reached telemetry first.

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

* Keep a failed Stage 12 child's failure when its own record errors

The coordinator read a failed child's record, to choose its failure detail,
outside any guard. One database error on that read reached the report
handler: the report was recorded as report_coordination_failed, and the
failed child and its peer were both recorded as cancelled "after the
coordinator failed".

Reading and writing the failed child's own record, in both the child-wait
and the dispatch-failure paths, is now non-fatal in the same way as the
cancellation writes. The error is logged with the report and simulation
identifiers and its type, but not its message. The report keeps the child's
failure as its code and cause, the failed child is recorded as failed with
its own cause where the record can still be written and is never recorded
as cancelled, and its peers are cancelled "after the <role> simulation
failed".

Each failure is now also reported once. The coordinator, the simulation
worker and the segment fan-out reported an unexpected error both through
the original-failure report and again through the correlation-ID log. One
report now carries the correlation ID that the redacted summary persists,
with the report and simulation identifiers. A segment failure is reported
where it occurs, and the worker does not report it again.

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

* Mount only the read-only Hugging Face secret in the Stage 12 precompute app

The Stage 12 cache precompute app, which runs on every Stage 12 deployment,
mounted the retiring policyengine-data-credentials and huggingface-token
(HF_TOKEN) Modal secrets on its image build and all five functions. The
credential runbook removes both from simulation code.

Its image build and functions now mount pe-uk-private-hf-read-token, which
supplies HUGGING_FACE_TOKEN, declared through the same constants and
required key as the executor and v2 apps. The US data the app reads comes
only from the public policyengine/populace-us dataset repository, which the
v2 US worker image already downloads with this token. The guard test that
keeps the retired names out of deployment sources now covers this app.

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

* Register the Stage 12-only integration test in the rules registry

The rules registry guard that landed on main with #752 requires every
deployment-asset guard test to belong to a rule. Add the test that keeps
the Stage 12-only deploy from running the existing stack's complete
integration suite to sim.stage12.deployment-scope, widen the statement to
say so, and re-render the rules page and the AGENTS.md block.

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

* Return valid aggregates from the Stage 12 test aggregators

#787 made the coordinator tests' artifact double write aggregates through
the real Stage12ArtifactStore, which rejects anything that is not a valid
AggregateReportArtifactPayload. Use _aggregate_payload for this PR's
aggregator stubs that must not run, as #787 did for main's, so an
unexpected run shows up as a written aggregate instead of as a rejection
hidden by the failure the test expects.

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

* Keep the v2 manifest readable by readers released before the cache

The manifest models forbid unknown fields, so readers already deployed
reject a manifest carrying the new cache release fields, and a cold
reader then has no valid manifest. The manifest is published before
the reading revision is promoted, staging deploys never promote, and a
rollback or revert would read the same document.

The publisher now writes the manifest under the active-with-cache key
and the same manifest without cache releases under the existing active
key. Current readers read active-with-cache, falling back to active
only before the first publish that writes both. A test pins the fields
earlier readers accept.

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

* Fix Stage 12 cache file names, forced rebuilds and the determinism gate

Name each baseline file as .py's Simulation.storage_id names the file
it saves and loads. A US simulation always resolves an SPM selection,
so .py appends its digest; the cache used the bare simulation ID, so
precompute never found the file it had calculated and no worker could
load a baked one. The identity now holds the resolved selection the
worker builds the simulation with, and a test compares the planned
name with the real worker model's.

Remove force from the Stage 12 cache. Stored files are immutable and
create-only, and HDF5 baselines are not byte-reproducible, so v1's
force_recompute input made every forced run fail.

Record a passing determinism check in the cache bucket and publish the
manifest only when that record exists. Previously a run that computed
no baselines skipped the check, so running again after a failure
published baselines that had failed it. The check now also requires
the stored baseline to load as a cache hit.

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

* Remove the Stage 12 ruff suppressions the main merge added

Fix the findings instead of suppressing them; only deliberate catch-all
handlers keep a suppression, each with its reason.

- Import at module level everywhere the deferred imports had no real
  need: the precompute app, the image-build download, the cache build,
  worker validation, qualification, segment fan-out and the v2 app.
  Modules that tests patch are imported whole and called through.
- Break the two import cycles. Stage12CachedSimulation moves from
  stage12_cache.runtime to stage12_runtime.cached_simulation, and
  should_segment_simulation moves into the partition module, so the
  simulation module imports both at module level. The stage12_cache
  package no longer imports .py.
- When a stored baseline fails its output plan, clear .py's in-memory
  cache with its public clear() instead of replacing one private entry.
  The next ensure loads the saved file.
- Annotate the precompute's Modal handles as modal.Function and drop the
  duplicated RemoteFunction protocol, and type the fan-out's Modal module.
- Move two raises out of try blocks into helpers, and raise TypeError
  for the image build's two manifest type checks.
- Give the six catch-all suppressions this PR already had their reasons.

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

* Install only US data in the Stage 12 cache precompute image

The precompute image ran `policyengine bundle install` with --venv, which
asks .py to pip-install the bundle's packages into the uv environment.
That environment has no pip, so with .py 6.7.5 the image build failed
with "No module named pip" in the first testing deploy. The locked
modal-simulation-image group already installs .py and the country
packages, so the image now installs only the US data through
bundle_data_install_command, as the v2 worker images do.

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 19, 2026
#787 landed on main after this stack branched. Group 8a, which this PR
enables, flags its new code; each fix follows this PR's conventions:

- ARG002: the shared in-memory object store keeps upload_bytes's
  content_type, which Stage12ArtifactStore passes by keyword, with the
  reason this PR gave the per-file fake it replaced.
- PT011: the invalid SPM provenance test expects pydantic's
  ValidationError, which every rejected value raises, not any ValueError.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk added a commit that referenced this pull request Oct 20, 2026
…PERF, PLW, PT, PTH, RUF, SIM and others) (#802)

* Exempt the rules tool's unittest tests from pytest-style rules

The shared rules tool needs only the standard library, so its tests use
unittest on purpose. Registered as sim.quality.unittest-ruff-exemptions.

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

* Resolve ruff group 8a findings in policyengine-fastapi

- JWTDecoder takes auto_error as keyword-only (FBT001/FBT002); the only
  caller, the gateway, already passes it by keyword.
- JWTDecoder.__call__ declares its bearer dependency with Annotated (B008).
- The 403s raised while validating a token chain the caught error (B904).
- Iterate dicts directly instead of over .keys() (SIM118).
- Name the ignored error code on the trace-filter type: ignore (PGH003).
- Explain NoOpObservability.span's unused arguments: it implements the
  Observability protocol (ARG002).
- Check expected exceptions with pytest.raises(match=...) (PT017).

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

* Resolve ruff group 8a findings in the simulation contract

- Merge the two branches that both fall back to the package version
  (SIM114), and collapse the nested as_of check (SIM102).
- The test modal.Dict stand-in takes create_if_missing as keyword-only,
  matching Modal's own from_name (FBT001/FBT002).
- Request the mock_modal fixture with usefixtures, and prefix the fake
  HF fetchers' unused positional parameters with an underscore (ARG).
- Use tuples for parametrize names (PT006), a dict literal (C408), and
  match the expected validation messages (PT011).

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

* Resolve ruff group 8a findings in the entry project

- CallerAuthenticator reads its bearer dependency marker from a
  module-level default (B008). The Annotated form isn't usable there:
  the module postpones annotations and FastAPI resolves them against the
  callable's __globals__, which a callable instance lacks. The OpenAPI
  document is unchanged.
- Keep the three telemetry try/except blocks, whose except lines carry
  the BLE001/S110 reasons, and suppress SIM105 on their try lines.
- Mark the failing test backends' request() with @OverRide, prefix the
  positional-only parameters of the _decoder replacements with an
  underscore, and explain the fake decoders' unused parameters (ARG).
- Drop the unused tmp_path requests (ARG001) and use tuples for
  parametrize names (PT006).

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

* Resolve ruff group 8a findings in the gateway project

- require_auth declares its bearer dependency through a module-level
  Annotated alias (B008); the module postpones annotations, and FastAPI
  resolves the alias through the function's globals. The OpenAPI
  documents are unchanged.
- Chain the caught error into the 400, 404 and 503 HTTPExceptions and the
  route-resolution ValueErrors (B904). A BLE001 suppression that the
  chained raise made unused keeps its reason as a comment.
- Keep the telemetry try/except blocks, whose except lines carry the
  BLE001/S110 reasons, and suppress SIM105 on their try lines.
- Use a conditional expression for the resolved version (SIM108) and
  Path.open in the OpenAPI generator (PTH123).
- Test stand-ins: keyword-only create_if_missing and use_bundle_default
  (FBT), ClassVar registries (RUF012), and explained unused parameters
  of the FunctionCall.get stand-in and the side-effect fixture (ARG).
- Tests: usefixtures for mock_modal and the import-isolation fixture,
  tuple parametrize names, bare @pytest.fixture, a split assertion, an
  explicit check=False, dict() over a comprehension, and the decoder
  class passed directly instead of through a lambda.

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

* Resolve ruff group 8a findings in the executor sources and fixtures

- Assign spm directly instead of through setattr (B010), use
  dict.fromkeys (C420), list unpacking (RUF005), one combined
  with-statement in the same enter/exit order (SIM117), a membership
  test (SIM109), merged upper-casing branches (SIM114), extend over an
  append loop (PERF401), and Path.open/Path.unlink (PTH).
- Keep, with reasons: the per-child and per-call try/except loops
  (PERF203), the credentials temp file that stays open across the
  yield (SIM115), HDFStore.keys(), which isn't a dict view (SIM118),
  the FastAPI lifespan hook's app parameter (ARG001), and the precompute
  local entrypoint's positional force flag, whose signature Modal turns
  into the CLI (FBT001/FBT002).
- Keep the telemetry and best-effort-cancel try/except blocks, whose
  except lines carry the BLE001/S110 reasons, and suppress SIM105 on
  their try lines.
- Fixtures: prefix the identity stubs' positional-only parameters and
  the fake local_entrypoint's **kwargs with an underscore (ARG).

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

* Resolve unused-argument findings in the executor tests

- Request side-effect fixtures with @pytest.mark.usefixtures, and drop
  an unused monkeypatch request (ARG001/ARG002).
- Prefix unused parameters of monkeypatched replacements with an
  underscore, after checking that every caller passes them positionally,
  and rename unused *args/**kwargs to *_args/**_kwargs (ARG001/ARG005).
- Keep, with reasons, parameters that callers pass by keyword: the
  _load_dataset and _reduce replacements, the fake _build_simulation and
  economic_impact_analysis keyword-only parameters, FunctionCall.get
  stand-ins' timeout, and the Stage 12 writer stand-ins' prefix. Fake
  model versions that duck-type .py's MicrosimulationModelVersion keep
  their parameters too.
- Mark the failing child store's create_or_resolve_simulation with
  @OverRide.
- Use tuples for parametrize names (PT006).

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

* Resolve the remaining ruff group 8a findings in the executor tests

- Pass booleans by keyword: DatasetSelection's is_default (FBT003), and
  keyword-only routed/accepted test parameters, the routed helper flag,
  the output-builder helper's options and the modal.Dict stand-ins'
  environment_name/create_if_missing, matching Modal (FBT001/FBT002).
- Match the real messages in pytest.raises (PT011); one native-only
  check keeps its broad ValueError, with a reason, because the
  rejecting layer's message varies. Make regex patterns raw or escaped
  (RUF043), give pytest.fail a message (PT016), and move the GCP
  credential cleanup check into a helper so pytest.raises wraps one call
  (PT012/SIM117).
- Use Path methods (PTH), literals and dict() (C408/C416), a list
  literal (SIM905), unpacking (RUF005), extend (PERF401), ClassVar
  annotations (RUF012), an underscore for an unused unpacked value
  (RUF059), explicit check=False (PLW1510) and an ASCII operator in a
  comment (RUF003).
- Pass regions.get, checks.append and nullcontext directly instead of
  through lambdas; nothing patches them (PLW0108).

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

* Resolve ruff group 8a findings in the CI script and integration tests

- The package update script stops overwriting its loop variable
  (PLW2901) and replaces the pyproject through Path.replace (PTH105).
- The integration conftest uses bare @pytest.fixture (PT001).

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

* Enable ruff group 8a (arguments, idioms and pytest style)

Adds ARG, ASYNC, B, C4, FBT, PD, PERF, PGH, PLW, PT, PTH, RUF and SIM to
the selected rule families. Every finding outside the shared rules tool
is fixed or carries an inline reason. Complexity (C90, PLR) is left for
group 8b.

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

* Register group 8a in the ruff-lint rule

sim.quality.ruff-lint now lists ARG, ASYNC, B, C4, FBT, PD, PERF, PGH,
PLW, PT, PTH, RUF and SIM.

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

* Sync the rules tool from the Public API's group 8a

The shared scripts/rules tool takes the Public API's group 8a fixes:
comprehensions, explicit check=False, bound loop variables and the
removed unused workflow-parser parameter. The rendered rules page picks
up the tool's new hash.

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

* Fix the ruff group 8a findings in the code #743 and #744 added

#743 and #744 landed on main after this stack branched. Group 8a, which
this PR enables, flags their new code; each fix follows this PR's
conventions:

- F821: this PR's CI-script commit dropped the package updater's os
  import, but #744's verify-dispatch reads os.environ, so restore it.
- PTH123: the routing and sentinel CLIs open their files through Path.
- RUF100: the sentinel's report helper prints with an explicit file, so
  its T201 suppression is unused.
- ARG: tests request serialized_workflow through usefixtures (and the
  patched_modal fixture keeps it with a reason), drop unused monkeypatch
  requests, and prefix unused parameters of positional stand-ins.
- FBT002: the routing-plan test helper takes force by keyword.
- PT012/PT018/B905/RUF043: one call per pytest.raises, one condition per
  assert, an explicit non-strict zip over call chains of different
  lengths, and a raw match pattern.

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

* Fix the ruff group 8a findings in the code #789 added

#789 landed on main after this stack branched. Group 8a, which this PR
enables, flags its new code; each fix follows this PR's conventions:

- ARG001/ARG002: open_connection_once and the telemetry-recording test
  runtime keep keyword parameters their protocols pass, with reasons; the
  test's random.uniform stand-in prefixes its unused positional bound.
- PERF203: the connection retry loop catches each refused attempt by
  design, with a reason.

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

* Fix the ruff group 8a findings in the code #787 added

#787 landed on main after this stack branched. Group 8a, which this PR
enables, flags its new code; each fix follows this PR's conventions:

- ARG002: the shared in-memory object store keeps upload_bytes's
  content_type, which Stage12ArtifactStore passes by keyword, with the
  reason this PR gave the per-file fake it replaced.
- PT011: the invalid SPM provenance test expects pydantic's
  ValidationError, which every rejected value raises, not any ValueError.

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

* Fix the ruff group 8a findings in the code #795 added

#795 landed on main after this stack branched. Group 8a, which this PR
enables, flags its new code; each fix follows this PR's conventions:

- ARG002: the child-deadline tests take the bare_country fixture through
  pytest.mark.usefixtures, like the runner tests.
- ARG005: their _reduce stand-ins keep country_module, which run() passes
  by keyword, with the same reason.
- ARG001: _crashing_call prefixes the clock it takes only to match
  _stuck_call's signature.
- PT006: the child-timeout telemetry test names its parameters in a tuple.
- SIM105: _cancel_call keeps its try/except, whose except line carries the
  BLE001/S110 reason, like the best-effort cancel it replaced.

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

* Fix the ruff group 8a findings in the code #791 added

enables, flags its new tests; each fix follows this PR's conventions:

- ARG002: its fake Modal calls, recording runtimes and rejecting
  aggregate writer keep the timeout, status_code and prefix keywords of
  the interfaces they stand in for, with the reasons this PR gave the
  other fakes.
- PT011: the stage-and-role summary test matches the message it raises
  rather than accepting any ValueError.

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

* Fix the ruff group 8a findings in the code #758 added

#758 landed on main after this stack branched. Group 8a, which this PR
enables, flags its new code; each fix follows this PR's conventions:

- B008: CallerAuthorizer.require_permission builds the authenticate
  dependency marker once and reads it from the nested dependency's
  default. The Annotated form can't be used: the module postpones
  annotations, and FastAPI resolves them against the module's globals,
  which don't hold the authorizer's dependency. FastAPI sees the same
  dependency, so each route's permission check is unchanged.
- SIM105: the permission-missing event keeps its try/except, whose
  except line carries the BLE001/S110 reason, like the module's other
  telemetry.
- ARG: the permission tests' fake decoders explain their unused
  parameters and the _decoder replacements prefix theirs; each
  table-driven test underscores the PROTECTED_ROUTES column it doesn't
  read, so the test ids stay the same.
- ARG001: the deployed enforcement test requests permission_enforcement
  through pytest.mark.usefixtures; pytest still sets it up first, so the
  test skips before fetching a token.

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

* Fix the ruff group 8a findings in the code #781 added

#781 landed on main after this stack branched. Group 8a, which this PR
enables, flags its new code; each fix follows this PR's conventions:

- ARG: stand-ins whose callers pass a parameter positionally prefix it
  with an underscore (the progress store's reads and writes, the patched
  Simulation.ensure, _country_module, get_region, Function.from_name, the
  preempted _set_modal_call_attributes, time.sleep and the
  on_first_ensure stand-ins), and unused *args/**kwargs become
  *_args/**_kwargs. Parameters callers pass by keyword keep a reason: the
  impact-analysis, _load_dataset, dispatch, run_simulation_impl and
  _configure_modal_observability stand-ins, the spawn stand-in's
  observability_context, the _reduce stand-in's country_module, the fake
  FunctionCall.get's timeout and the generated client's sync_detailed.
- ARG001/ARG002: the contract's missing-record test and the bundle
  manifest test request their fixtures through pytest.mark.usefixtures.
- B018: the failed-analysis test assigns the property it reads inside
  pytest.raises.
- SIM105: the Modal app's stage-duration telemetry keeps its try/except,
  whose except line carries the BLE001/S110 reason.

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

* Exempt tests from the unused-argument rules file-wide

Test fakes and stubs mirror real signatures, because callers pass those
arguments by keyword: run() passes country_module=, the runtime passes
keyword-only options, and stand-ins replace modal.FunctionCall.get and
JWTDecoder.__call__. Fixtures also request other fixtures only for their
side effects.

Tests are among the files allowed file-level exceptions, so ARG001,
ARG002 and ARG005 join the tests' and the deployed-service tests'
exemptions. That removes 72 inline suppressions from 16 test files.
Application code keeps its six unused-argument suppressions and their
reasons.

Comments and configuration only: no code changes.

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

* Swallow best-effort errors with contextlib.suppress

Nine blocks ran a best-effort call and swallowed any error with
try/except Exception: pass:
- telemetry context and events in the entry app, its auth, the Modal
  app and the gateway;
- a Modal call cancel during segmented national cleanup.

Each block carried three suppressions: SIM105 on the try, and BLE001
and S110 on the except.

Each is now with contextlib.suppress(Exception), with the old reason as
a comment above it. It suppresses the same exceptions and runs the same
calls, so behaviour is unchanged. That removes 18 suppression lines.

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

* Fix the ruff group 8a findings in the code #712 added

#712's segmentation and cache code raised 16 group 8a findings. Each is
fixed in the code, with no new suppression:

- PERF203: the policy-period parser and the segment-cancellation loop
  move their try/except into a helper called once per item. The parser's
  error helper is now typed NoReturn, since it always raises. The
  cancellation helper keeps #712's BLE001 reason.
- RUF022: stage12_runtime's __all__ is sorted.
- Tests: subprocess.run states check=False (PLW1510), match= patterns
  are raw or escape a literal dot (RUF043), compound asserts are split
  (PT018), the import guard extends its list from a generator (PERF401),
  boolean helper and parametrized arguments are keyword-only
  (FBT001/FBT002), and the segment-function stand-in is an instance
  whose spawn method takes Modal's observability_context by keyword
  (ARG004), as modal.Function.from_name returns an object.

The rebase onto #712 also dropped this branch's PERF203 suppression in
coordination.py, since #712 replaced the loop it was on.

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

* Lint the executor as Python 3.13, its required version

Ruff reads no project's requires-python here, so it linted the executor
with its default target version. PERF203 applies only below Python 3.11,
so the retry loop in database_retry.py needed a suppression that the
executor's own Python (3.13) doesn't call for.

Add the executor to per-file-target-version as py313, remove that
suppression, and fix the eight findings the new target version reports:
tomllib is now sorted with the standard library (I001, five files), and
datetime.timezone.utc becomes datetime.UTC (UP017, three files).

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

* Use or underscore the arguments behind five ARG suppressions

- NoOpObservability now subclasses the Observability protocol it already
  implemented, and span() is marked @OverRide, which ruff exempts from
  ARG002.
- The executor's in-memory object store records each object's content
  type, and the gateway's mock FunctionCall records the timeout each get()
  was called with, so both fakes use the arguments they mirror.
- The gateway's import_gateway_models fixture only requested the module
  isolation fixture for its side effect. The test that used it now requests
  isolation with usefixtures and imports the models module itself, named
  in GatewayImportModuleNames.models.
- The executor's FastAPI lifespan takes _app: Starlette and FastAPI pass
  the app positionally.

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

* Make the precompute entrypoint's force option keyword-only

Modal 1.4.2 builds a command-line option for every non-variadic parameter
of a local entrypoint and calls it with keyword arguments, so a
keyword-only force still gives --force/--no-force. The test already calls
main(force=True). This removes the FBT001/FBT002 suppression.

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

* Replace the SIM115, SIM118 and PT011 suppressions with code

- setup_gcp_credentials enters the credentials temp file on an ExitStack
  held by a with statement around the yield. The file still stays open
  across the yield; the finally block restores GOOGLE_APPLICATION_CREDENTIALS
  first, and the stack then closes, and so deletes, the file, the same order
  as before.
- The UK local-authority check calls HDFStore.keys(include="pandas"), the
  default spelled out, and keeps a comment on why `in store` differs.
- The native SPM test passes check= to pytest.raises (pytest 8.4), so the
  typed error code is part of the raises check instead of a later assert.

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

* Restore the lower-case coverage pragma comment in gateway auth

The contextlib.suppress change rewrote "# pragma: no cover" as the prose
"# Pragma: no cover". Put back the original lower-case spelling. The
comment sits on its own line above the with statement, so coverage.py
excludes no statement because of it either way.

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

---------

Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

Stage 12 rejects aggregates whose optional SPM provenance is null

1 participant