Repository navigation
Record a redacted Stage 12 failure cause instead of only the exception class - #791
Merged
Merged
Conversation
This was referenced Oct 13, 2026
anth-volk
force-pushed
the
fix/stage12-preserve-error-cause
branch
3 times, most recently
from
October 14, 2026 19:56
7e90ee6 to
f467e5b
Compare
…n class Stage 12 coordinator, worker and result-comparison failures stored only type(error).__name__, so a database session limit, a UK aggregate validation defect and an invalid reform path all appeared as RuntimeError or ValidationError behind "Stage 12 report coordination failed". Failure records now store a bounded summary of the form "<type> in|after <stage> (<role>): <message>", capped at the 512-character error_summary limit. Messages are redacted first: URL credentials, signed URL query strings, secret-looking assignments, bearer and known token formats, this process's secret environment values, Pydantic input values and SQL statement parameters are removed. Stage tracking follows the existing registered span names, so the summary names the stage that failed or the last stage that finished before a failure between spans. The coordinator no longer overwrites a cause that the failed worker already persisted, and copies that cause onto the report record. The redacted cause, with the original stack, is also recorded on the active operation before the failure is persisted. The exceptions returned to Modal are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Redact quoted JSON and dictionary keys, non-Bearer Authorization schemes and cookie headers, signed query strings without a URL scheme, and the row values that PostgreSQL constraint violations echo in their detail, from both the stored summary and the exported cause. Patterns that could start inside a run of word characters now start only where that run starts, so redaction is linear in the message length. Redaction also reads at most 4,096 characters: the word that limit cuts is dropped, and quoted values, SQL parameters and validation inputs whose closing delimiter was cut are redacted to the end. Take the summary length from the contract's ErrorSummary, detect SQLAlchemy errors by type, and state in the module docstring that span exception events are outside this redaction. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the coordinator's stage_scope and stage_span closures and the worker's _simulation_stage with tracked_stage, which opens a stage's span or operation and records it as the process's progress. The coordinator claim span uses it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Assert the redacted summary stored on a child and its report when Modal rejects a dispatch, and the comparison_error_summary stored when the production result cannot be read. Reword fixture failure messages that described themselves as sensitive, since the tests now assert those messages are stored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#787 made the coordinator tests' artifact double write aggregates through the real Stage12ArtifactStore, which rejects anything that is not a valid AggregateReportArtifactPayload. Use _aggregate_payload for this PR's aggregator stubs, as #787 did for main's. The redacted comparison-failure test needs a valid aggregate for the report to succeed. An aggregator that must not run now shows up as a written aggregate if it does run, instead of as a rejection hidden by the failure the test expects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
force-pushed
the
fix/stage12-preserve-error-cause
branch
from
October 14, 2026 20:24
f467e5b to
160e16b
Compare
anth-volk
marked this pull request as ready for review
October 14, 2026 22:27
anth-volk
added a commit
that referenced
this pull request
Oct 14, 2026
#791 records a redacted Stage 12 failure cause (type, stage, simulation role and message) where this branch persisted a generic message with a correlation ID for every unexpected error. Keep this branch's failure model (typed failures, preserved worker causes, cancellation of unfinished simulations, exactly-once reports) and use #791's cause in it: - redacted_failure_detail summarises an unexpected error with failure_summary, followed by its correlation ID. The cause is shortened, never the ID, to fit the 512-character summary. Typed, propagated and cancellation failures are unchanged. - The coordinator and workers open their spans and operations through tracked_stage, so a failure names its stage. The coordinator's concurrent wait records each child's role while probing it. - The coordinator and workers attach the redacted cause, with the original stack, to the failing operation before writing the failure record. The original error is still reported once. - Tests expect the new summaries, and #791's cause tests are ported with this branch's error codes. The README and the Stage 12 migration notes describe the new summary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 tasks done
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 #790
Same-Repository Draft PR Check
PolicyEngine/policyengine-sim-api, not a personal fork.Summary
Stage 12 failure records stored only the exception class name. Database session limits, the UK aggregate validation defect (#786) and invalid reform paths therefore all appeared as
RuntimeErrororValidationErrorbehindStage 12 report coordination failed.This branch is rebased on #789. It now writes its redacted cause through #789's failure-recording helpers, so both behaviours hold together:
#789's connection-retry wiring (
runtime_store(runtime)) is unchanged.Bounded cause: the new
stage12_runtime/failure_causes.pybuilds<type> in|after <stage> (<role>): <message>, capped at the 512-charactererror_summarylimit. The limit is read from the contract'sErrorSummarytype.Redaction before storage: applies to the stored summary and to the cause recorded on the operation.
@, and URL query strings.Authorizationheader values for every scheme, including a bare value, and cookie headers.STAGE12_DATABASE_URLandHUGGING_FACE_TOKEN.Key (…)=(…)andFailing row contains (…)details. Column names and the constraint message are kept.Redaction reads at most 4,096 characters of a message, and its patterns run in linear time. The word cut by that limit is dropped. Quoted values, SQL parameters and validation inputs whose closing delimiter was cut are redacted to the end.
log_and_redact_exceptionhides the whole message, so it couldn't be reused here.Shared stage helper:
tracked_stageopens a stage's span or operation and records that stage for the failure summary. It replaces the coordinator'sstage_scopeandstage_spanclosures and the worker's_simulation_stage. The coordinator claim span also uses it.Handlers:
report_original_failurefirst, then write the redacted summary throughpersist_failure_record. The outer handler skips the report for a child failure that was already reported. The result-comparison handler also stores the redacted summary._mark_child_failed, now shared by the dispatch and wait handlers, keeps a cause that a failed worker already saved and returns the cause the child record holds. The coordinator copies that cause onto the report record.persist_failure_recordnow returns the write's result, orNonewhen the write failed. When the child record can't be read, the report falls back to the coordinator's own redacted cause.No schema change: stage and role are part of the summary text.
Example summaries:
Not included
Unredacted telemetry: two things still carry the original message, filtered only by the observability package's own redaction:
runtime.spanorruntime.operationscope;report_original_failurelogs before the write.This PR doesn't change either, and the
failure_causesdocstring says so.Initial claim failures: these are raised outside the failure handler, so no record is written for them. Since Release Stage 12 database sessions and keep original failures visible #789, a refused connection on the claim is retried.
Result-comparison write path: the comparison handler keeps its own guarded write and doesn't use
persist_failure_record, as onmain. A comparison failure never fails the report.Dispatch failures in the entry service:
policyengine-simulation-entry/stage12_backend.pystill stores only the class name.Redaction limits: redaction is pattern-based.
Rebased onto main after #752, #757 and #787
changelog_entry.yamlkeeps Accept null SPM provenance in Stage 12 aggregate artifacts #787's line and this PR's line.Stage12ArtifactStore. That broketest_comparison_failure_stores_a_redacted_cause, whose aggregator returned{"result": {"budget": 100}}, which is not a valid aggregate. This PR's aggregator stubs now return_aggregate_payload(...), as Accept null SPM provenance in Stage 12 aggregate artifacts #787 did for main's, including the two that must not run. The double validates an aggregate before recording the write, so with an invalid stub an unexpected run would have been hidden by the failure the test expects.rules-registry,ci-scripts) pass.Testing
tests/test_stage12_failure_causes.py(53 tests):ErrorSummary, and unprintable errors still produce one.tests/test_stage12_runtime.py(6):simulation_dispatch_failedsummary on the child and the report, and records the same cause. It reuses Release Stage 12 database sessions and keep original failures visible #789'sRefusingDispatchInvoker.comparison_error_summary.tests/test_stage12_failure_recording.py:persist_failure_recordreturns what the write returned.test_single_worker_reports_failure_before_an_unwritable_failure_recordtest_coordinator_reports_failure_before_an_unwritable_failure_recordtest_coordinator_reports_a_child_failure_once_when_its_record_is_unwritable. The child record can't be read here, so the report stores the coordinator's own cause.test_coordinator_reports_a_child_dispatch_failure_onceuv sync --extra test, as CI runs them:projects/policyengine-simulation-executor: 1117 passed, 22 skipped, 3 deselected.libs/policyengine-stage12-persistence: 17 passed.ruff.toml's rule selection, passes:ruff format --checkandruff checkon the changed files;ruff format --checkon everyprojects/*/srcandlibs/*/srcdirectory.🤖 Generated with Claude Code