Skip to content

Record a redacted Stage 12 failure cause instead of only the exception class - #791

Merged
anth-volk merged 5 commits into
mainfrom
fix/stage12-preserve-error-cause
Oct 14, 2026
Merged

anth-volk merged 5 commits into
mainfrom
fix/stage12-preserve-error-cause

Conversation

@anth-volk

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

Copy link
Copy Markdown
Contributor

Fixes #790

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

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 RuntimeError or ValidationError behind Stage 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:

  • The original error is reported to telemetry first.
  • A failed failure-record write is logged and never replaces the original error.
  • A child failure is reported once.
  • The summary those writes store is the redacted cause described below, not the class name.

#789's connection-retry wiring (runtime_store(runtime)) is unchanged.

  • Bounded cause: the new stage12_runtime/failure_causes.py builds <type> in|after <stage> (<role>): <message>, capped at the 512-character error_summary limit. The limit is read from the contract's ErrorSummary type.

    • "in" names the stage that failed.
    • "after" names the last stage that finished, for a failure between stages.
    • Stages come from the existing registered span names.
  • Redaction before storage: applies to the stored summary and to the cause recorded on the operation.

    • URL credentials, including passwords that contain @, and URL query strings.
    • Query strings without a URL scheme, such as signed object paths, and signature, credential and token parameters.
    • Secret-looking key/value pairs, including quoted JSON and dictionary keys.
    • Authorization header values for every scheme, including a bare value, and cookie headers.
    • Bearer and other known token formats.
    • This process's secret environment values, such as STAGE12_DATABASE_URL and HUGGING_FACE_TOKEN.
    • Pydantic input values and SQL statement parameters.
    • Row values that PostgreSQL constraint violations repeat in Key (…)=(…) and Failing 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_exception hides the whole message, so it couldn't be reused here.

  • Shared stage helper: tracked_stage opens a stage's span or operation and records that stage for the failure summary. It replaces the coordinator's stage_scope and stage_span closures and the worker's _simulation_stage. The coordinator claim span also uses it.

  • Handlers:

    • The worker and the coordinator's dispatch, wait and outer handlers report the original error with report_original_failure first, then write the redacted summary through persist_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_record now returns the write's result, or None when the write failed. When the child record can't be read, the report falls back to the coordinator's own redacted cause.
    • The redacted cause, with the original stack, is recorded on the active operation before the failure is saved.
    • The exceptions returned to Modal are unchanged.
  • No schema change: stage and role are part of the summary text.

Example summaries:

OperationalError in stage12_child_state_create: OperationalError: connection failed: connection to server at "…", port 5432 failed: FATAL: (EMAXCONNSESSION) max clients reached in session mode - max clients are limited to pool_size: 15
ValidationError in stage12_aggregate_artifact_write: 1 validation error for SPMComparisonProvenance: Input should be a valid dictionary or instance of SPMComparisonProvenance [model_type]

Not included

  • Unredacted telemetry: two things still carry the original message, filtered only by the observability package's own redaction:

    This PR doesn't change either, and the failure_causes docstring 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 on main. A comparison failure never fails the report.

  • Dispatch failures in the entry service: policyengine-simulation-entry/stage12_backend.py still stores only the class name.

  • Redaction limits: redaction is pattern-based.

    • Numbers in a model error message can still be stored, up to 512 characters.
    • Only the two constraint-violation detail forms above are treated as row values. Other database messages that quote a rejected input are stored as written.

Rebased onto main after #752, #757 and #787

Testing

  • New tests/test_stage12_failure_causes.py (53 tests):
    • The summary names type, stage, role and message, and names the last finished stage for a failure between stages.
    • Credentials and stored values are redacted in each format listed above. Of the 22 format cases added in review, 20 fail against the earlier patterns.
    • A secret cut by the 4,096-character limit isn't exposed. Cases cover a URL, a quoted value and SQL parameters.
    • Long crafted messages are summarised in under one second, both with the length limit and with it raised to 20,000 characters.
    • Validation input values and SQL parameters are not stored, and the null-SPM-provenance validation failure produces a specific summary.
    • Summaries are bounded and validate against the contract's ErrorSummary, and unprintable errors still produce one.
    • The recorded cause keeps the original stack, and recording failures never escape.
  • New tests in tests/test_stage12_runtime.py (6):
    • The worker saves and records a redacted cause, and names a failing artifact-write stage.
    • The coordinator keeps a failed worker's cause and names the stage that rejected the aggregate.
    • A rejected dispatch stores a redacted simulation_dispatch_failed summary on the child and the report, and records the same cause. It reuses Release Stage 12 database sessions and keep original failures visible #789's RefusingDispatchInvoker.
    • A failed production-result read stores a redacted comparison_error_summary.
  • New test in tests/test_stage12_failure_recording.py: persist_failure_record returns what the write returned.
  • Updated assertions:
    • Four pre-existing assertions now expect summaries instead of class names. Their fixture failure messages no longer describe themselves as sensitive.
    • Four of Release Stage 12 database sessions and keep original failures visible #789's tests now expect the redacted summary instead of the class name. They still check that the original error is reported first, that a failed write is logged without replacing it, and that a child failure is reported once:
      • test_single_worker_reports_failure_before_an_unwritable_failure_record
      • test_coordinator_reports_failure_before_an_unwritable_failure_record
      • test_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_once
  • Suites, on Python 3.13 after uv sync --extra test, as CI runs them:
    • projects/policyengine-simulation-executor: 1117 passed, 22 skipped, 3 deselected.
    • The Stage 12 failure-cause, runtime, failure-recording and database-retry files alone: 110 passed. Each of the four rebased commits passes them on its own.
    • libs/policyengine-stage12-persistence: 17 passed.
  • Formatting and lint: CI's pinned ruff 0.15.22, with ruff.toml's rule selection, passes:
    • ruff format --check and ruff check on the changed files;
    • ruff format --check on every projects/*/src and libs/*/src directory.

🤖 Generated with Claude Code

anth-volk and others added 5 commits October 15, 2026 00:17
…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
anth-volk force-pushed the fix/stage12-preserve-error-cause branch from f467e5b to 160e16b Compare October 14, 2026 20:24
@anth-volk
anth-volk marked this pull request as ready for review October 14, 2026 22:27
@anth-volk
anth-volk merged commit 4a3a8fe into main Oct 14, 2026
14 checks passed
@anth-volk
anth-volk deleted the fix/stage12-preserve-error-cause branch 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>
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.

Stage 12 failure records keep only the exception class name

1 participant