Repository navigation
Conversation
Stage12InvocationContext gains result_delivery: stage12_database (the default, today's behavior) or return_to_caller, for a report coordinator that returns its result instead of recording it in the Stage 12 tables. A context that leaves the field out, as the Simulation Entrypoint's does, keeps the Stage 12 behavior. A returned report cannot name a production job to compare with, because that comparison is recorded on the Stage 12 report row. Nothing reads the field yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Stage 12 coordinator now leaves result_delivery out of the context it sends to its baseline and reform workers, so their payload has exactly the fields it had before the field existed. A coordinator whose report returns to its caller will send the field, so its workers record nothing in the Stage 12 tables either. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
New policyengine_simulation_contract.report_execution: - ReportExecutionResult: the report's aggregate inline as a SingleYearMacroOutput, its SHA-256 (report_aggregate_sha256, over compact key-sorted JSON, rejecting NaN and infinity), the output plan, the baseline and reform artifact descriptors, and provenance (bundle, worker version, Modal environment, application, simulation function, version manifest digest and coordinator call ID). It rejects artifacts that do not form the report, a digest that does not match, and requested aggregates that differ from the ones the output plan was built for, so the two cannot disagree undetected. - ReportExecutionFailure and ReportExecutionError: the redacted error code and summary a Stage 12 row would record, under a fixed message. A test sends the error through Modal's own exception transport (pickle_exception in the container, _process_result in the caller) and gets the same type with the same failure back. Nothing produces these types yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A simulation worker given a context whose result_delivery is return_to_caller reads and writes no Stage 12 rows and never builds the database store. It checks the policy periods, writes the input and the Parquet artifact exactly as a Stage 12 worker does, and returns the artifact descriptor. A failure is reported once, recorded as the operation's cause, and raised as ReportExecutionError with the redacted code and summary a Stage 12 row would record, including a segment's failure and typed input failures such as invalid_policy_period. No record can tell a returned simulation to stop, so a segmented US run is never told to stop through the database; its coordinator cancels it through Modal instead. The Stage 12 worker is unchanged: the policy check, input write, calculation and artifact write move into _SimulationRun, which both modes use, and the failure detail into _simulation_failure_detail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
New stage12_runtime.coordinate_report_for_caller runs a report whose context asks for return_to_caller and returns a ReportExecutionResult payload with the aggregate inline. It reads and writes no Stage 12 rows and never uses the database store: - It claims nothing, so a repeated dispatch runs the report again; the caller deduplicates. - It writes artifacts under the Stage 12 layout built from the context (environment, creation time, report), replacing any prefix the caller sent, and starts both workers with the same delivery. - It plans the output, starts both workers, waits for them within the shared window, and reads and checks their artifacts with the same code the Stage 12 coordinator runs. It then aggregates, applies the checks a Stage 12 aggregate artifact write applies, requires the aggregate to describe this report, and validates its result as SingleYearMacroOutput. No aggregate artifact is written and no production comparison runs. - A failure is reported once, recorded as the operation's cause, and raised as ReportExecutionError with the redacted code and summary a Stage 12 row would record. A worker's own ReportExecutionError takes precedence over its call error, as a Stage 12 worker's recorded failure does. A worker that must stop is cancelled through Modal, and a cancellation error is logged without replacing the failure. The shared steps move unchanged into ReportSimulationCoordination, whose subclasses decide what a failed or stopped child records. The Stage 12 coordinator keeps its behavior, and now refuses a context that asks for return_to_caller before claiming anything. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The v2 Modal application's coordinate_report function now takes its parent record as an optional argument. A context whose result_delivery is return_to_caller runs coordinate_report_for_caller, refuses a parent record, and labels its operation with result_delivery=return_to_caller so telemetry tells it apart from Stage 12 runs. Every other context runs the Stage 12 coordinator exactly as before, with the same operation attributes; the Simulation Entrypoint still passes all three arguments, and a Stage 12 report without a parent record is still rejected before anything is claimed or started. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Modal stops a cancelled synchronous input by raising InputCancellation in it. That is a BaseException, so the segment runner's except Exception handlers never ran, and the up to 20 region-group segment calls the worker had started kept running until they finished or reached their timeout. The segment runner now cancels every started segment call when dispatch or the wait stops on a BaseException that is not an Exception, then re-raises it. Failures that are Exceptions keep their existing handling, so a segment failure or a cancellation read from the Stage 12 record still cancels each segment exactly once. A returned report's coordinator stops its workers through Modal, so this is how a returned segmented run stops its segments. On the Stage 12 path the coordinator cancels a segmented worker through Modal only when it could not record the cancellation; that fallback now stops the segment calls too. Stage 12 records and results are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Modal stops a cancelled coordinator input by raising InputCancellation, a BaseException, in it, and does not cancel the calls that input spawned. The returned coordinator caught only Exceptions, so its baseline and reform workers kept running after a caller cancelled the report. coordinate_report_for_caller now cancels every started, unfinished worker through Modal when it stops on a BaseException that is not an Exception, then re-raises it. A cancellation is not a report failure, so nothing is reported or raised as ReportExecutionError. With the segment runner change, a cancelled segmented worker then cancels its segment calls. The Stage 12 coordinator is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test that sends ReportExecutionError through Modal's exception transport imports two private Modal client functions. It moves into its own module, so a Modal upgrade that moves them fails only that module instead of every report execution test. A new test checks that the Modal coordinate_report function raises a returned report's ReportExecutionError unchanged, as the same object, and still shuts its telemetry runtime down. Modal pickles exactly what the function raises, so with the transport test this covers both halves of the error's path to the caller; neither runs a real Modal container. Both tests cover existing behavior and passed on their first run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ings The module docstring described Stage 12 coordination only, and ReportSimulationCoordination._failure_detail still described Stage 12 steps although the returned coordinator shares it. Both now say what each coordinator records. Documentation only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two short windows remained in which Modal's InputCancellation left segment calls running: while a dispatch span was closing after its call had started, because the call was added to the list only after the span closed; and between the end of the dispatch phase and the start of the wait. The segment runner now adds each call to its list inside the dispatch span, and one handler covering both the dispatch and the wait cancels every started call on a BaseException that is not an Exception. Failures that are Exceptions keep their existing handling. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two comments still said that cancelling a segmented worker through Modal leaves its region-group segment calls running. Since the segment runner answers Modal's InputCancellation by cancelling them, both now describe that. The Stage 12 coordinator still writes the cancellation to the worker's record first and uses Modal only when that write fails. Documentation only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The returned coordinator rejects an aggregate whose report ID, bundle or requested aggregates differ from the report, but only the report ID was tested. The test now covers all three. It covers existing behavior and passed on its first run; with the bundle and requested-aggregates comparisons removed from the coordinator, those two cases fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ReportSimulationCoordination._spawn_child returned a worker's call only after the dispatch span had closed, and its callers stored it after that. If Modal cancelled the returned coordinator in that short window, the coordinator could not see the call, and the started worker (with a segmented US worker's 20 segment calls) ran to the end for a report no caller would receive. _spawn_child now keeps the call in calls inside the dispatch span, and both coordinators rely on it instead of storing the call afterwards. On the Stage 12 path this also means that a failure while the span closes stops the worker that had just started, instead of leaving it running; Stage 12 records are otherwise unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The coordinator operation of a returned report carries result_delivery=return_to_caller, but its baseline and reform workers' operations did not, so their timings and failures were counted as Stage 12 worker data unless a query joined them through the trace. The US and UK worker functions now add the same attribute when their context asks for the report to return to its caller. Stage 12 workers keep exactly their current attributes (tested). The region-group segment function receives no dispatch context and is unchanged; its spans join the worker's trace. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The returned result crosses Modal as JSON values so that the container's and the caller's pydantic versions need not agree, but ReportExecutionError pickled its ReportExecutionFailure model itself, with pydantic's internal state. It now pickles the failure's JSON-mode values and rebuilds the model when it is unpickled. Modal's exception transport still raises it in the caller as the same type with the same failure (tested), including from Modal 1.4.2 in a worker to Modal 1.5.5 in the Simulation Entrypoint's environment (checked once by hand). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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 #895
Same-Repository Draft PR Check
PolicyEngine/policyengine-sim-api, not a personal fork.Summary
The v2 Modal report coordinator can now hand a report's result straight back to its caller, instead of only recording it in the Stage 12 tables and the Stage 12 artifact bucket. A new dispatch-context field,
result_delivery, selects the mode. Leaving it out keeps today's Stage 12 behavior, and the Simulation Entrypoint does not send it, so every Stage 12 comparison run behaves as before. Nothing calls the new mode yet: the Simulation Entrypoint's v2 economy path, a later change, will.This replaces the closed #836 and #837 as one PR, keeping what still fits, with the fixes from their review and from three review rounds on this branch.
The contract (
policyengine_simulation_contract)Stage12InvocationContext.result_delivery:stage12_database(the default) orreturn_to_caller. Areturn_to_callercontext cannot name a production job to compare with, because that comparison is recorded on the Stage 12 report row.ReportExecutionResult(newreport_executionmodule): the report's totals inline as aSingleYearMacroOutput, their SHA-256 (report_aggregate_sha256, over compact key-sorted JSON, rejecting NaN and infinity), the output plan, the baseline and reform artifact descriptors, and provenance (bundle, worker version, Modal environment, application and simulation function, version manifest digest, coordinator call ID). It rejects artifacts that do not form the report, a digest that does not match, and requested aggregates that differ from the ones the output plan was built for, in content or order, so the two cannot disagree undetected.ReportExecutionFailureandReportExecutionError: the redacted error code and summary a Stage 12 row would record for the same failure, under a fixed message. The error pickles its failure as plain JSON values and rebuilds the model when it is unpickled, so the two sides' pydantic versions need not agree.Does Modal raise
ReportExecutionErrorin the caller as the same type? Yes.modal._serialization.pickle_exception, andFunctionCall.getin the caller unpickles and raises it inmodal._utils.function_utils._process_result.test_report_execution_modal_transportruns both of Modal's own functions on aReportExecutionErrorand gets back the same type, failure and message. Modal's error hints (exc_with_hints) return the same object.test_stage12_modal_appchecks that the Modalcoordinate_reportfunction raises the returned report's error unchanged, as the same object, so Modal pickles exactly that error.policyengine_simulation_contract.report_execution; the Simulation Entrypoint depends on the contract library.ReportExecutionResult.model_validate.The returned mode (
stage12_runtime.coordinate_report_for_caller, and the Modalcoordinate_reportwithresult_delivery = return_to_caller)evaluation_idper attempt, because artifact paths are write-once.ValueError) and checks the application, worker version and bundle digest as the Stage 12 path does.stage-12-runs/<environment>/<YYYY>/<MM>/<evaluation_id>, built from the context; a prefix the caller sends is replaced.ReportSimulationCoordination, whose subclasses decide what a failed or stopped worker records.ReportExecutionResult. No aggregate artifact is written and no production comparison runs.ReportExecutionErrorwith the redacted code and summary a Stage 12 row would record. A worker's ownReportExecutionErrortakes precedence over its call error, as a Stage 12 worker's recorded failure does. Failures before any worker starts (invalid payloads, the dispatch checks, an unset artifact bucket) raiseValueErrororValidationError, as the Stage 12 coordinator does at the same points before it writes any row.result_delivery=return_to_caller, so they can be told apart from Stage 12 runs. The region-group segment function receives no dispatch context and is unchanged; its spans join the worker's trace.Workers (
run_single_simulation, Modalrun_single_simulation_usand_uk)With
return_to_caller, a worker checks the policy periods, writes the input and Parquet artifact exactly as a Stage 12 worker does, and returns the artifact descriptor, touching no rows. A failure raisesReportExecutionError, including a segment's failure and typed input failures such asinvalid_policy_period.Segment calls of a cancelled segmented US worker
Modal stops a cancelled synchronous input by raising
InputCancellation(aBaseException) in it, and does not cancel the calls that input spawned. The segment runner caught onlyException, so up to 20 region-group segment calls kept running after their worker was cancelled through Modal. It now keeps each call as soon as it starts, inside its dispatch span, and cancels every started segment call when its dispatch or wait stops on aBaseExceptionthat is not anException, then re-raises it; failures that are Exceptions keep their existing handling, and each call is cancelled once. This is how a returned segmented run stops its segments. On the Stage 12 path, the coordinator cancels a segmented worker through Modal only when it could not write the cancellation to the worker's record; that fallback now stops the segment calls too. No Stage 12 record or result changes.The Stage 12 path is unchanged
result_deliveryout of the context it sends its workers, so their payload has exactly the fields it had before (tested).coordinate_reportfunction'sparentargument now defaults toNone; the Simulation Entrypoint still passes all three arguments, and a Stage 12 report without a parent record is still rejected before anything is claimed or started (tested). Stage 12 operation attributes are unchanged (tested).return_to_callercontext before claiming anything.Environment variables
This PR adds, removes or rewires no environment variable, workflow input, GitHub secret, Secret Manager secret or Modal secret. Variables the changed runtime code reads:
STAGE12_ARTIFACT_BUCKET(both modes;artifact_store()in the coordinator and workers). Provider: the GitHub environment variableSTAGE12_ARTIFACT_BUCKET, present inbetaandprod. Wiring:simulation-deploy.ymlandstage12-deploy.ymlcallsimulation-deploy.reusable.ymlwithdeploy_stage12_v2: trueandsecrets: inherit; itsdeploy_stage12_v2job runs in thebetaorprodenvironment, and its step "Synchronize the separate Stage 12 runtime secret" passes the variable to.github/scripts/stage12-sync-modal-secrets.sh, which writes it into the Modal secretstage12-evaluation-runtimein thestagingormainModal environment.v2_app.pyattaches that secret to the coordinator, both country workers and the segment worker (worker_secrets). The variable's value was not inspected.GOOGLE_APPLICATION_CREDENTIALS_JSON(both modes; read bysetup_gcp_credentialswhen the artifact store builds its Cloud Storage client, so the returned mode needs it for every artifact read and write). Provider: the Secret Manager secret named by the GitHub environment variableSTAGE12_GCP_CREDENTIALS_SECRET_NAME, in the project named bySIMULATION_ENTRYPOINT_GCP_PROJECT_ID(both present inbetaandprod). Wiring: the same step authenticates to Google Cloud with the environment secretsSIMULATION_ENTRYPOINT_GCP_WIF_PROVIDERandSIMULATION_ENTRYPOINT_GCP_DEPLOY_SERVICE_ACCOUNT, reads the secret withgcloud, and writes it into the Modal secretstage12-evaluation-gcp-credentials, whichworker_secretsattaches to the same functions. Not verified: the secret's content.STAGE12_DATABASE_URL(Stage 12 mode only; the returned mode never reads it). Provider: the Secret Manager secret named bySTAGE12_DATABASE_URL_SECRET_NAME(present inbetaandprod), through the same step intostage12-evaluation-runtime. Unchanged.HUGGING_FACE_TOKEN(both modes; the workers' dataset loading, and the failure redaction, which reads it only to remove its value from summaries). Provider: the organization secretPE_UK_PRIVATE_HF_READ_TOKEN, written by the deploy workflow into the Modal secretv2_app.pyattaches ashf_secret. Unchanged.MODAL_ENVIRONMENT(the Modalcoordinate_reportfunction, which this PR changes, reads it only to label its telemetry, falling back tolocal). Provider: Modal, which documents setting it in every function container to the name of the Modal environment the container runs in. No workflow, script or secret in this repository provides it. Not verified in the deployed containers. Unchanged.The deploy job also uses
MODAL_TOKEN_IDandMODAL_TOKEN_SECRET(repository and organization secrets, throughsecrets: inherit); the changed runtime code does not read them, because a coordinator starts its workers with Modal's own container credentials.What a merge deploys, and what it waits for
Merging to
mainruns the complete staging and then production deployment. The redeployed v2 Modal application gains the return mode, but nothing calls it. The Simulation Entrypoint sends noresult_delivery, so every Stage 12 report keeps its behavior. No image, function resource setting, timeout, secret attachment or workflow changes.Before it may merge, this PR waits only for CI (
CI complete) and a maintainer's review. It depends on no other PR and no operator step, and it adds no database migration.Testing
Each behavior change has tests that fail without it, checked commit by commit against the parent commit's code. The tests that cover existing behavior (the Modal coordinator passing the error through unchanged, the aggregate mismatch cases, a Stage 12 report still needing its parent record) passed on their first run; the aggregate mismatch cases fail when the coordinator's comparisons are removed.
libs/policyengine-simulation-contract):test_report_result_delivery.py(5 tests: the default andreturn_to_callerdelivery, no production comparison for a returned report, an unknown delivery rejected);test_report_execution.py(24 tests: JSON round trip, totals asSingleYearMacroOutput, digest checks and stability, each artifact consistency check, requested aggregates against the output plan in content and order, timezone, unique aggregates, unknown fields, failure bounds, pickling, and a pickle holding only JSON values);test_report_execution_modal_transport.py(1 test, Modal's exception transport). 279 passed.projects/policyengine-simulation-executor):test_stage12_returned_report.py(32 tests: returned workers without the database, their failures and segmented runs; the returned coordinator's typed result, stages, repeated dispatch, dispatch checks, each failure and cancellation path, Modal cancelling the coordinator during the wait or while a dispatch span closes, aggregate and artifact checks);test_stage12_segment_fanout.py(4 new tests: Modal cancelling a segmented worker during the wait, during dispatch, while a dispatch span closes, and before the wait);test_stage12_modal_app.py(8 new tests: routing by delivery, refusing a parent record, unchanged Stage 12 attributes, the error passed through unchanged, and both workers' operation attributes in each mode);test_stage12_runtime.py(1 new test: Stage 12 workers receive the context without the delivery field). Full suite: 1,693 passed, 22 skipped.ruff format --check projects/*/src libs/*/srcandruff check .with the pinned Ruff 0.15.22 pass, andpython -m scripts.rules pyright --syncpasses for the contract library (no errors) and the executor (within its recorded baseline), at every commit.scripts/run_quality_guards.pypasses.🤖 Generated with Claude Code