Repository navigation
Conversation
anth-volk
added this pull request to stack #867
October 21, 2026 15:40
This was referenced Oct 21, 2026
anth-volk
force-pushed
the
pyright-executor-remainder
branch
2 times, most recently
from
October 21, 2026 16:30
51ee385 to
f02e486
Compare
This was referenced Oct 21, 2026
anth-volk
force-pushed
the
pyright-executor-remainder
branch
from
October 21, 2026 16:59
f02e486 to
b59e1dd
Compare
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
force-pushed
the
pyright-executor-remainder
branch
from
October 21, 2026 20:33
b59e1dd to
642dc81
Compare
This branch has not been deployed
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 #872
Same-Repository Draft PR Check
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.txtgoes from 72 entries to 0. The stack's top PR deletes it and makessim.quality.pyright-executorenforced.What changed
scopes.operation(observability library):runtime.operation(...)typed likescopes.span, taking theattributesandremote_contextthe 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 withscopes.span/scopes.operationinstead of callingruntime.span/runtime.operationdirectly. No other code in the executor'ssrccalls those runtime methods directly: the remaining.span(...)calls instage12_runtime/coordination.pyareStage12Coordination.span, a method already typedAbstractContextManager[object]that callstracked_stage.Version-registry utility (
update_version_registry.py, run by the deploy workflows):LatestVersionsis non-total: a kind has no latest entry until one is routed, and readers already used.get. The few direct reads now use.gettoo. They are the plan'sadvances_latestflag and the post-publish and post-restore reports, which run aftervalidate_routing_statehas required those entries, plus a merge that copied a latest only when present.RoutingState.bundlesholdsMapping[str, Any], because stored manifests are read back unvalidated._json_copyis generic.Output data reads.
output_framesadds two runtime-checkable protocols,EntityFramesandHouseholdFrame. Each read of<simulation>.output_dataset.data.<frames>first checksisinstance(<simulation>.output_dataset.data, ...)and raisesTypeErrorif the frames are missing (before,AttributeErroronNone). Pyright narrows that same expression, so the reads are unchanged, and that matters for the weighted-microdata checks:pd.DataFrame(simulation.output_dataset.data.household)verbatim;The protocol checks need only the attribute, so the tests' stand-in datasets pass. Frames are typed as pandas DataFrames (a
MicroDataFrameis one), because microdf types a column read asMicroSeries | MicroDataFrame.Output helpers' sums.
_sum_output_variableand_change_output_variablerequire the selected column to be aSeriesbefore summing. AMicroSeriesis one, so sums stay weighted (checked: a weighted column sums to its weighted total). They raiseTypeErrorfor a repeated column name, wherefloat()raisedTypeErrorbefore.AggregateandChangeAggregateresults are checked forNone._output_model_dumpreadsmodel_dumpwithgetattr.Precompute and artifact outcome:
schema=. Pydantic accepts both names (populate_by_name), but Pyright's Pydantic support knows only the alias.ArtifactBaselineSimulationrecords anArtifactOutcomeLiteral instead ofstr.getattr._missing_output_columnschecks the model version against a runtime-checkableEntityVariableResolverprotocol, since policyengine definesresolve_entity_variablesonly onMicrosimulationModelVersion; the tests' stand-in model versions pass.Stage 12 simulation builder. It builds
SimulationandStage12CachedSimulationin separate branches instead of unpacking{"id": ...}, which Pyright matched against every keyword.Stage 12 parity:
_is_report_listand_is_report_numberreturnTypeGuards.TypeGuard, unlikeTypeIs, doesn't narrow onFalse, which matters because astris aSequencethe list check rejects.Series; a repeated column name raisesTypeError, where the comparison failed withAttributeErroron.dtypebefore.Last four:
ChildSimulationHandletypes as optional;release_sentinelreads__doc__ or ""(__doc__isNoneunder-OO);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:
ensure():TypeErrorinstead ofAttributeErroronNone(or, in_entity_data,RuntimeErrorin this PR's first version);TypeErrorwith the column's name, instead ofTypeErrororAttributeErrorfrom pandas;Aggregatewith no result:RuntimeErrorinstead ofTypeErrorfromfloat(None);RuntimeErrorin place of theAttributeError.The last two commits change no behaviour.
scopes.span(runtime, name, attributes=...)andscopes.operation(runtime, name, attributes=...)callruntime.span(name, attributes=...)andruntime.operation(name, attributes=..., remote_context=None)and return the runtime's scope unchanged, sotracked_stageopens 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: redmodule ... has no attribute 'operation'; green, 38 library tests.No module named 'policyengine_simulation_executor.output_frames'; green, 3 tests covering real UK year data, stand-ins andNone.The last two commits answer a later review pass:
tracked_stagewith the typed scopes: a refactor, so there is no red.test_tracked_stage_scopes_and_records_the_stage_with_its_rolerecords the scopes opened and passes before and after; its stand-in runtime'soperationnow acceptsremote_context, asObservabilityRuntime.operationdoes, and asserts it isNone. Green: 56 failure-cause tests; Pyright 0 errors.TypeError:test_guard_rejects_a_dataset_with_no_entity_datapassesdata=Noneand a stand-in with noentity_data. Red, with theisinstance(dataset.data, EntityFrames)check removed: both cases failed withAttributeError('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'smanifest_schema=looked like a wrong keyword, butArtifactManifestsetspopulate_by_name=True, so it worked.Self-review
review-codeon 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:runtime.operation(..., remote_context=...)and onlyscopes.span. Fixed: both guides namescopes.operation, and the type-checking guide says how to narrow output data without hiding the read from the microdata checks._entity_dataraisedRuntimeErrororAttributeErrorwhere the new reads raiseTypeError, and the UK household count used its owngetattrcheck. Fixed:_entity_dataraisesTypeErrorin both cases (red: a two-case test gotRuntimeErrorandAttributeError), and the household count usesEntityFrames.try, so it would have left the batch running instead of failed. Fixed: the check moved into_child_call, called inside thetry, which restores the earlier handling. Red: a test with a handle lacking its call got theRuntimeErrorout of the scheduler. Green: the batch is marked failed.MicrosimulationModelVersionagainstEntityVariableResolver. Both pass on the existing code.getattrwhere the compute path usesisinstance(..., ArtifactBaselineSimulation). Declined: the determinism test passes a stand-in baseline that isn't anArtifactBaselineSimulation. The error message still reports the outcome it read.A later
review-codepass over the whole stack found two more, both fixed:TypeErrorfor a dataset with no entity data had no test. Added (the last commit).tracked_stagestill calledruntime.span/runtime.operationdirectly. Converted to the typed scopes, with no behaviour change; it was the only direct call left insrc.A
review-codepass 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 withAttributeErrorwithout the check.Environment variables
update_version_registry.pyandrelease_sentinel.pyare deployment utilities that the deploy workflows run. Their environment-variable reads are unchanged:GITHUB_ACTIONS,GITHUB_REPOSITORYandGITHUB_WORKFLOW_REF, which GitHub Actions provides, are read in_require_serialized_routing_workflow. No workflow, image or Modal app definition changed.app.pyandv2_app.pynow importpolicyengine_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.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