Skip to content

Fix the executor's remaining Pyright errors - #873

Draft
anth-volk wants to merge 15 commits into
pyright-executor-runtimefrom
pyright-executor-remainder
Draft

anth-volk wants to merge 15 commits into
pyright-executor-runtimefrom
pyright-executor-remainder

Conversation

@anth-volk

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

Copy link
Copy Markdown
Contributor

Fixes #872

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

Fifth in the Pyright stack, on top of #871. Fixes the executor's remaining 72 Pyright errors, so it has none: rules/baselines/pyright-executor.txt goes from 72 entries to 0. The stack's top PR deletes it and makes sim.quality.pyright-executor enforced.

Commit Errors fixed Baseline after
Add a typed operation scope next to the span scope (helper) 72
Open the executor's remaining spans and operations with typed scopes 17 55
Type the version-registry utility's routing state as it is stored 8 47
Check the shape of a simulation's output data before reading it 16 31
Narrow the output helpers' column sums and aggregate results 7 24
Type the baseline artifact outcome and the precompute manifest 6 18
Build the Stage 12 simulation's two types in separate branches 7 11
Narrow the Stage 12 parity check's report values and columns 7 4
Fix the executor's last four Pyright errors 4 0
Fail the batch, as before, when a child's call handle is missing (review) 0
Raise TypeError wherever a simulation's output data is missing (review) 0
Test the output-frame and model-version protocols on real US types (review) 0
Point the observability and type-checking guides at scopes.operation (review) 0
Open tracked_stage's scopes through the typed scopes helpers (review) 0
Test the UK weight-matrix check's TypeError for a dataset with no entity data (review) 0

What changed

  • scopes.operation (observability library): runtime.operation(...) typed like scopes.span, taking the attributes and remote_context the Modal functions pass. The Modal functions' operations (app.py, v2_app.py) and the remaining spans (budget-window batch and scheduler, segmented national reduce, Stage 12 fan-out and comparison, output builder) use the typed scopes. stage12_runtime.failure_causes.tracked_stage, which records a Stage 12 stage for failure causes and times it, also opens its span or operation with scopes.span / scopes.operation instead of calling runtime.span / runtime.operation directly. No other code in the executor's src calls those runtime methods directly: the remaining .span(...) calls in stage12_runtime/coordination.py are Stage12Coordination.span, a method already typed AbstractContextManager[object] that calls tracked_stage.

  • Version-registry utility (update_version_registry.py, run by the deploy workflows):

    • LatestVersions is non-total: a kind has no latest entry until one is routed, and readers already used .get. The few direct reads now use .get too. They are the plan's advances_latest flag and the post-publish and post-restore reports, which run after validate_routing_state has required those entries, plus a merge that copied a latest only when present.
    • RoutingState.bundles holds Mapping[str, Any], because stored manifests are read back unvalidated.
    • _json_copy is generic.
    • The finalize paths check for a missing Dict where they already reached the same outcome.
  • Output data reads. output_frames adds two runtime-checkable protocols, EntityFrames and HouseholdFrame. Each read of <simulation>.output_dataset.data.<frames> first checks isinstance(<simulation>.output_dataset.data, ...) and raises TypeError if the frames are missing (before, AttributeError on None). Pyright narrows that same expression, so the reads are unchanged, and that matters for the weighted-microdata checks:

    • they recognise weighted frames by these expressions;
    • their stripping baseline lists pd.DataFrame(simulation.output_dataset.data.household) verbatim;
    • a helper returning the frames would hide them from the checks.

    The protocol checks need only the attribute, so the tests' stand-in datasets pass. Frames are typed as pandas DataFrames (a MicroDataFrame is one), because microdf types a column read as MicroSeries | MicroDataFrame.

  • Output helpers' sums. _sum_output_variable and _change_output_variable require the selected column to be a Series before summing. A MicroSeries is one, so sums stay weighted (checked: a weighted column sums to its weighted total). They raise TypeError for a repeated column name, where float() raised TypeError before. Aggregate and ChangeAggregate results are checked for None. _output_model_dump reads model_dump with getattr.

  • Precompute and artifact outcome:

    • The manifest is constructed with its alias schema=. Pydantic accepts both names (populate_by_name), but Pyright's Pydantic support knows only the alias.
    • ArtifactBaselineSimulation records an ArtifactOutcome Literal instead of str.
    • The determinism check reads the outcome with getattr.
    • _missing_output_columns checks the model version against a runtime-checkable EntityVariableResolver protocol, since policyengine defines resolve_entity_variables only on MicrosimulationModelVersion; the tests' stand-in model versions pass.
  • Stage 12 simulation builder. It builds Simulation and Stage12CachedSimulation in separate branches instead of unpacking {"id": ...}, which Pyright matched against every keyword.

  • Stage 12 parity:

    • _is_report_list and _is_report_number return TypeGuards. TypeGuard, unlike TypeIs, doesn't narrow on False, which matters because a str is a Sequence the list check rejects.
    • Frame columns must be Series; a repeated column name raises TypeError, where the comparison failed with AttributeError on .dtype before.
  • Last four:

    • the budget-window totals are passed by name;
    • the scheduler checks the call handle that ChildSimulationHandle types as optional;
    • release_sentinel reads __doc__ or "" (__doc__ is None under -OO);
    • the SPM capability uses SPMCapability.model_validate.

Behaviour

Behaviour is unchanged on every path production reaches. The changes that differ do so only in paths that already failed, and they now fail with a clearer error:

  • a simulation with no output frames, or no output data after ensure(): TypeError instead of AttributeError on None (or, in _entity_data, RuntimeError in this PR's first version);
  • a repeated column name: TypeError with the column's name, instead of TypeError or AttributeError from pandas;
  • an Aggregate with no result: RuntimeError instead of TypeError from float(None);
  • a missing child call handle (unreachable): the batch fails, as before, with a RuntimeError in place of the AttributeError.

The last two commits change no behaviour. scopes.span(runtime, name, attributes=...) and scopes.operation(runtime, name, attributes=...) call runtime.span(name, attributes=...) and runtime.operation(name, attributes=..., remote_context=None) and return the runtime's scope unchanged, so tracked_stage opens the same scopes as before.

The microdata weight checks (sim.microdata.*) and their baseline are unchanged and pass.

Commits (red, then green)

Each type-fix commit's red is the Pyright check run on the code before the fix, with that commit's entries removed from the baseline. The check reported exactly those entries as new errors (17, 8, 16, 7, 6, 7, 7, 4). Green is the check passing with the shrunk baseline, plus the executor tests. Two commits add tests first:

  • scopes.operation: red module ... has no attribute 'operation'; green, 38 library tests.
  • The output-frame protocols: red No module named 'policyengine_simulation_executor.output_frames'; green, 3 tests covering real UK year data, stand-ins and None.

The last two commits answer a later review pass:

  • tracked_stage with the typed scopes: a refactor, so there is no red. test_tracked_stage_scopes_and_records_the_stage_with_its_role records the scopes opened and passes before and after; its stand-in runtime's operation now accepts remote_context, as ObservabilityRuntime.operation does, and asserts it is None. Green: 56 failure-cause tests; Pyright 0 errors.
  • The UK weight-matrix check's TypeError: test_guard_rejects_a_dataset_with_no_entity_data passes data=None and a stand-in with no entity_data. Red, with the isinstance(dataset.data, EntityFrames) check removed: both cases failed with AttributeError ('NoneType' object has no attribute 'entity_data', 'types.SimpleNamespace' object has no attribute 'entity_data'). Green: 11 weight-matrix tests pass.

Suppressions added

None.

Real bugs found

None. precompute.build_manifest's manifest_schema= looked like a wrong keyword, but ArtifactManifest sets populate_by_name=True, so it worked.

Self-review

review-code on this PR's diff, run by a separate agent, found no bugs. It reproduced each commit's baseline, confirmed that weighted sums stay weighted and that the microdata checks still see every read, and checked the Pydantic and deployment-utility changes. Code-quality findings:

  1. The guides still prescribed runtime.operation(..., remote_context=...) and only scopes.span. Fixed: both guides name scopes.operation, and the type-checking guide says how to narrow output data without hiding the read from the microdata checks.
  2. Missing output data was handled unevenly. _entity_data raised RuntimeError or AttributeError where the new reads raise TypeError, and the UK household count used its own getattr check. Fixed: _entity_data raises TypeError in both cases (red: a two-case test got RuntimeError and AttributeError), and the household count uses EntityFrames.
  3. The scheduler's missing-call check sat before the poll's try, so it would have left the batch running instead of failed. Fixed: the check moved into _child_call, called inside the try, which restores the earlier handling. Red: a test with a handle lacking its call got the RuntimeError out of the scheduler. Green: the batch is marked failed.
  4. The protocol tests covered UK year data only. Added: US year data, and MicrosimulationModelVersion against EntityVariableResolver. Both pass on the existing code.
  5. The determinism check reads the outcome with getattr where the compute path uses isinstance(..., ArtifactBaselineSimulation). Declined: the determinism test passes a stand-in baseline that isn't an ArtifactBaselineSimulation. The error message still reports the outcome it read.

A later review-code pass over the whole stack found two more, both fixed:

  1. The UK weight-matrix check's new TypeError for a dataset with no entity data had no test. Added (the last commit).
  2. tracked_stage still called runtime.span / runtime.operation directly. Converted to the typed scopes, with no behaviour change; it was the only direct call left in src.

A review-code pass over the last two commits found no bugs. It confirmed that both helpers pass the same arguments as the direct calls and return the runtime's own scope unwrapped. It also confirmed that the new weight-matrix test fails with AttributeError without the check.

Environment variables

update_version_registry.py and release_sentinel.py are deployment utilities that the deploy workflows run. Their environment-variable reads are unchanged: GITHUB_ACTIONS, GITHUB_REPOSITORY and GITHUB_WORKFLOW_REF, which GitHub Actions provides, are read in _require_serialized_routing_workflow. No workflow, image or Modal app definition changed. app.py and v2_app.py now import policyengine_simulation_observability.scopes, part of the observability library the images already install, and the Modal image smoke covers them when this PR reaches the bottom of the stack.

Testing

  • python -m scripts.rules pyright projects/policyengine-simulation-executor: passes with 0 errors.
  • Executor unit tests: 1567 passed, 22 skipped. Observability library tests: 38 passed.
  • Quality guards and rendered rules views: pass. Ruff 0.15.22 format and check: clean.

This PR is above the bottom of the stack, so its CI runs only CI complete, which fails by design until the PRs below merge.

🤖 Generated with Claude Code

anth-volk and others added 15 commits October 22, 2026 00:33
scopes.operation(runtime, name, attributes=..., remote_context=...)
returns runtime.operation(...) unchanged, typed like scopes.span as a
context manager that doesn't suppress exceptions, so Pyright doesn't
take a Modal function's operation scope to swallow the exception raised
in its body. It takes the two keyword arguments the executor's Modal
functions pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Modal functions' operation scopes (app.py, v2_app.py) now use
scopes.operation, and the remaining spans in the budget-window batch and
scheduler, the segmented national reduce, the Stage 12 fan-out and
comparison, and the output builder use scopes.span. Pyright no longer
reports the 13 variables assigned inside them as possibly unbound, or the
4 functions that return from inside one as missing a return.

The Stage 12 failure-cause scope already declared its type
(AbstractContextManager[object]) and is unchanged. The executor's
baseline drops from 72 entries to 55.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
update_version_registry's 8 Pyright errors, without changing what it
writes or prints:

- LatestVersions is no longer total. The empty routing state, and any
  state before a release of a kind is routed, has no latest entry for
  it; readers already used .get. The few direct reads now use .get
  too: the plan's advances_latest flag and the post-publish and
  post-restore reports, which run after validate_routing_state has
  required those entries, and the legacy-seed merge, which copied a
  kind's latest only when present.
- RoutingState.bundles holds Mapping[str, Any]. A stored state is read
  back unvalidated, and the coercion and legacy seeding copy any
  mapping in; validate_routing_state checks the manifests against
  their routes.
- _json_copy is generic: a JSON round trip of JSON-safe data keeps its
  shape, so the planned state stays a RoutingState.
- finalize_routing_publication returns "no-pending" as soon as there is
  no routing-state Dict, the outcome it reached anyway; the finalize
  command treats a missing Dict like no pending publication, as it did.

The executor's baseline drops from 55 entries to 47.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
policyengine types Dataset.data as an optional BaseModel, so Pyright
rejected every read of a simulation's output frames: entity_data in the
microdata export and the precompute determinism check, household in the
UK local-authority and constituency breakdowns, and the output dataset
after ensure() in _entity_data (16 errors).

output_frames adds two runtime-checkable protocols, EntityFrames and
HouseholdFrame. Each read site now checks
`isinstance(<simulation>.output_dataset.data, ...)` first and raises
TypeError if the frames are missing (before, AttributeError on None).
Pyright narrows that same expression, so each read is unchanged. That
matters for the microdata weight checks, which recognise weighted frames
by expressions such as <simulation>.output_dataset.data.household, and
whose stripping baseline lists one of these reads verbatim. The checks
need only the attribute, so the tests' stand-in datasets still pass. The
protocols type frames as pandas DataFrames, which a MicroDataFrame is,
since microdf types a column read as MicroSeries | MicroDataFrame.

The executor's baseline drops from 47 entries to 31.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
simulation_output_common's 7 Pyright errors, without changing a total:

- _sum_output_variable and _change_output_variable sum a column chosen
  by name, which pandas types as a Series or, for a repeated name, a
  DataFrame. They now require a Series (a MicroSeries is one, so the
  sum stays weighted) and raise TypeError for a repeated name, where
  float() of a frame's per-column sums raised TypeError before.
- After Aggregate.run() and ChangeAggregate.run(), a result that is
  still None raises RuntimeError, where float(None) raised TypeError.
- _output_model_dump reads model_dump with getattr instead of testing
  hasattr, which Pyright doesn't narrow on.

The executor's baseline drops from 31 entries to 24.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The 6 Pyright errors in precompute.py and baseline_artifacts.py,
without changing behaviour:

- build_manifest passes the manifest schema by its alias, `schema`.
  ArtifactManifest also accepts `manifest_schema` (populate_by_name),
  but Pyright's Pydantic support knows only the alias as a constructor
  keyword.
- ArtifactBaselineSimulation records its outcome as ArtifactOutcome,
  the Literal the precompute result model declares (and a test already
  ties to the OUTCOME_* constants), instead of str.
- The determinism check reads the outcome with getattr, as the module
  already reads storage_id: the cohort baseline is declared a
  Simulation, which has no artifact_outcome.
- _missing_output_columns checks the model version against a
  runtime-checkable EntityVariableResolver protocol before calling
  resolve_entity_variables, which policyengine defines on
  MicrosimulationModelVersion rather than the declared
  TaxBenefitModelVersion; the tests' stand-in model versions pass.

The executor's baseline drops from 24 entries to 18.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
build_stage12_simulation chose its class, then passed the cache id by
unpacking `{"id": ...}`, a dict[str, str], which Pyright matches against
every keyword parameter (7 errors: created_at, updated_at, dynamic and
the rest). It now constructs a Simulation when there is no cache
identity and a Stage12CachedSimulation with id= and its cache
configuration when there is one, as the annual runtime's
_build_simulation does. The optional spm keyword is still passed only
when there is a selection.

The executor's baseline drops from 18 entries to 11.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
stage12_parity's 7 Pyright errors:

- _is_report_list and _is_report_number now return TypeGuards
  (Sequence[object], and int | float), so a value that passed one is
  typed as that in the comparison that follows. TypeGuard, unlike
  TypeIs, says nothing about a value that fails, which matters because
  a str is a Sequence the list check rejects.
- A frame column selected by name is a Series, or a DataFrame when the
  name repeats. _compare_entity_frames now requires a Series on both
  sides and raises TypeError naming the column for a repeated one; the
  column comparison would have failed with AttributeError on .dtype.

The executor's baseline drops from 11 entries to 4.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The budget-window totals are passed to BudgetWindowTotals by name
  instead of by unpacking a dict[str, float], which Pyright matched
  against the model's year field too.
- The scheduler's poll reads the child call handle, which
  ChildSimulationHandle types as optional, and raises if it is missing;
  resolve_child_handle always returns a handle with its call.
- release_sentinel's argument parser description reads __doc__, which
  is None when Python strips docstrings (-OO), through `or ""`.
- The US SPM capability validates the bundle's configured defaults with
  SPMCapability.model_validate, Pydantic's entry point for untyped
  input, instead of passing an object to the constructor; the
  validation is the same.

The executor now has no Pyright errors, and its baseline is empty.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The previous fix narrowed the child's optional call handle with a check
before the poll's try, so a missing handle would have raised out of the
scheduler and left the stored batch running. Before that fix the same
case failed inside the try (None.get) and the handler failed the batch.
The check now lives in _child_call, called inside the try, so a missing
handle fails the batch again. resolve_child_handle always returns a
handle with its call, so this guards an unreachable case. A new test
covers it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The output-frame reads raised TypeError for missing frames, but
_entity_data raised RuntimeError when ensure() left no output dataset
and AttributeError when it left no data. It now raises TypeError in
both cases, keeping its getattr(<simulation>.output_dataset.data, ...)
read for the microdata checks. The UK weight-matrix check in the
simulation runtime now narrows the input dataset with the same
EntityFrames protocol instead of its own getattr and dict check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The protocol tests covered UK year data only. They now also check that
policyengine's US year data passes EntityFrames and HouseholdFrame, and
that MicrosimulationModelVersion, which the US and UK model versions
derive from, satisfies EntityVariableResolver. Both pass on the
existing code; they guard the narrowing against a policyengine change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both guides prescribed scopes.span only, and the observability guide
still said to pass a received context to runtime.operation, which brings
back the Pyright errors the executor is held to zero on. They now name
scopes.operation for operations, and the type-checking guide says how to
narrow a simulation's output data without hiding the read from the
microdata checks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tracked_stage called runtime.operation and runtime.span directly; it now
calls scopes.operation and scopes.span, which call the same runtime
methods with the same name and attributes and no remote trace context.
The test stand-in's operation accepts remote_context, as the real
ObservabilityRuntime.operation does, and asserts it is None.

No behaviour change: test_tracked_stage_scopes_and_records_the_stage_with_its_role
passes before and after.

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

_require_certified_uk_weight_matrix raises TypeError("the '<name>' dataset
has no entity data loaded") when dataset.data is None or has no
entity_data mapping. Nothing tested that branch. The new test covers both
inputs; without the isinstance check they fail with AttributeError.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@anth-volk
anth-volk force-pushed the pyright-executor-remainder branch from b59e1dd to 642dc81 Compare October 21, 2026 20:33

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix the executor's remaining 72 Pyright errors

1 participant