Skip to content

Let the v2 report coordinator return a typed result to its caller - #896

Draft
anth-volk wants to merge 16 commits into
mainfrom
v2plan/1c-109-typed-coordinator-result
Draft

anth-volk wants to merge 16 commits into
mainfrom
v2plan/1c-109-typed-coordinator-result

Conversation

@anth-volk

Copy link
Copy Markdown
Contributor

Fixes #895

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

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) or return_to_caller. A return_to_caller context cannot name a production job to compare with, because that comparison is recorded on the Stage 12 report row.
  • ReportExecutionResult (new report_execution module): the report's totals inline as a SingleYearMacroOutput, 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.
  • ReportExecutionFailure and ReportExecutionError: 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 ReportExecutionError in the caller as the same type? Yes.

  • A Modal function container pickles the exception its function raised with modal._serialization.pickle_exception, and FunctionCall.get in the caller unpickles and raises it in modal._utils.function_utils._process_result. test_report_execution_modal_transport runs both of Modal's own functions on a ReportExecutionError and gets back the same type, failure and message. Modal's error hints (exc_with_hints) return the same object.
  • test_stage12_modal_app checks that the Modal coordinate_report function raises the returned report's error unchanged, as the same object, so Modal pickles exactly that error.
  • Across Modal client versions: the v2 worker image locks Modal 1.4.2 and the Simulation Entrypoint locks 1.5.5. A one-off check pickled the error with the executor environment's Modal 1.4.2 and raised it with the entrypoint environment's Modal 1.5.5: same type, same failure, same message.
  • Limits: no test runs a real Modal container. The caller must be able to import policyengine_simulation_contract.report_execution; the Simulation Entrypoint depends on the contract library.
  • A successful result crosses Modal as the result's JSON-mode dictionary, as the Stage 12 coordinator's descriptor does, so the container's and the caller's pydantic versions never need to agree on a pickled model. The caller validates it with ReportExecutionResult.model_validate.

The returned mode (stage12_runtime.coordinate_report_for_caller, and the Modal coordinate_report with result_delivery = return_to_caller)

  • Reads and writes no Stage 12 rows and never builds the database store. It claims nothing, so a repeated dispatch runs the report again; the caller deduplicates and uses a new evaluation_id per attempt, because artifact paths are write-once.
  • Takes no parent record (passing one raises ValueError) and checks the application, worker version and bundle digest as the Stage 12 path does.
  • Writes the simulation artifacts under the Stage 12 layout, stage-12-runs/<environment>/<YYYY>/<MM>/<evaluation_id>, built from the context; a prefix the caller sends is replaced.
  • Plans the output, starts both workers with the same delivery, waits for both within the shared window, and reads and checks their artifacts. These steps are the code the Stage 12 coordinator runs, moved unchanged into a shared base class, ReportSimulationCoordination, whose subclasses decide what a failed or stopped worker records.
  • Aggregates, applies the checks a Stage 12 aggregate artifact write applies, requires the aggregate to describe this report (report ID, bundle, requested aggregates), and returns a ReportExecutionResult. No aggregate artifact is written and no production comparison runs.
  • A failure is reported to telemetry 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. Failures before any worker starts (invalid payloads, the dispatch checks, an unset artifact bucket) raise ValueError or ValidationError, as the Stage 12 coordinator does at the same points before it writes any row.
  • A worker that must stop (its peer failed, the wait window closed, or the coordinator failed after starting it) is cancelled through Modal; a cancellation error is logged and never replaces the failure. If Modal cancels the coordinator's own input, it cancels its unfinished workers the same way and re-raises the cancellation without reporting a failure.
  • The coordinator's and both workers' telemetry operations carry 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, Modal run_single_simulation_us and _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 raises ReportExecutionError, including a segment's failure and typed input failures such as invalid_policy_period.

Segment calls of a cancelled segmented US worker

Modal stops a cancelled synchronous input by raising InputCancellation (a BaseException) in it, and does not cancel the calls that input spawned. The segment runner caught only Exception, 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 a BaseException that is not an Exception, 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

  • The Stage 12 coordinator leaves result_delivery out of the context it sends its workers, so their payload has exactly the fields it had before (tested).
  • The Modal coordinate_report function's parent argument now defaults to None; 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).
  • The Stage 12 coordinator refuses a return_to_caller context before claiming anything.
  • The shared dispatch step now keeps a started worker's call before its dispatch span closes, so a stop in that moment still finds it. On the Stage 12 path this only matters if closing the span fails: the worker that had just started is then stopped instead of being left running. Stage 12 records are otherwise unchanged.

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 variable STAGE12_ARTIFACT_BUCKET, present in beta and prod. Wiring: simulation-deploy.yml and stage12-deploy.yml call simulation-deploy.reusable.yml with deploy_stage12_v2: true and secrets: inherit; its deploy_stage12_v2 job runs in the beta or prod environment, 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 secret stage12-evaluation-runtime in the staging or main Modal environment. v2_app.py attaches 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 by setup_gcp_credentials when 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 variable STAGE12_GCP_CREDENTIALS_SECRET_NAME, in the project named by SIMULATION_ENTRYPOINT_GCP_PROJECT_ID (both present in beta and prod). Wiring: the same step authenticates to Google Cloud with the environment secrets SIMULATION_ENTRYPOINT_GCP_WIF_PROVIDER and SIMULATION_ENTRYPOINT_GCP_DEPLOY_SERVICE_ACCOUNT, reads the secret with gcloud, and writes it into the Modal secret stage12-evaluation-gcp-credentials, which worker_secrets attaches 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 by STAGE12_DATABASE_URL_SECRET_NAME (present in beta and prod), through the same step into stage12-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 secret PE_UK_PRIVATE_HF_READ_TOKEN, written by the deploy workflow into the Modal secret v2_app.py attaches as hf_secret. Unchanged.
  • MODAL_ENVIRONMENT (the Modal coordinate_report function, which this PR changes, reads it only to label its telemetry, falling back to local). 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_ID and MODAL_TOKEN_SECRET (repository and organization secrets, through secrets: 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 main runs the complete staging and then production deployment. The redeployed v2 Modal application gains the return mode, but nothing calls it. The Simulation Entrypoint sends no result_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.

  • Contract (libs/policyengine-simulation-contract): test_report_result_delivery.py (5 tests: the default and return_to_caller delivery, no production comparison for a returned report, an unknown delivery rejected); test_report_execution.py (24 tests: JSON round trip, totals as SingleYearMacroOutput, 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.
  • Executor (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.
  • Simulation Entrypoint 654 passed, gateway 261 passed, Stage 12 persistence 40 passed (the contract library is shared).
  • ruff format --check projects/*/src libs/*/src and ruff check . with the pinned Ruff 0.15.22 pass, and python -m scripts.rules pyright --sync passes for the contract library (no errors) and the executor (within its recorded baseline), at every commit. scripts/run_quality_guards.py passes.
  • Generated clients are unchanged: no Simulation Entrypoint or gateway API schema changed.
  • Not run locally: the Modal image smoke and the integration tests, which need Modal and the CI services; CI runs them.

🤖 Generated with Claude Code

anth-volk and others added 16 commits October 22, 2026 18:14
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

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.

Let the v2 report coordinator return a typed result to its caller

1 participant