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
anth-volk
added a commit
that referenced
this pull request
Oct 15, 2026
* Enable ruff group 1 (safe auto-fixes) and fix everything it finds Select FLY, FURB, I, PIE, PYI, RET, RSE, UP and W, and fix every finding: ruff's safe fixes (import sorting, pyupgrade), then by hand: - policyengine-fastapi re-exports JWTDecoder through __all__ and drops two unused imports (the 9 F401s noted in Phase 0). - Two tests build their JSON receipts with json.dumps, not % formatting. - smoke_app uses min() rather than sorted()[0] (ruff 0.16 FURB192). scripts/** is linted as Python 3.11, the shared rules tool's minimum, so import sorting matches policyengine-api byte for byte. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Run ruff check in CI and enforce the lint rule The lint job now runs ruff check . on the whole repository, and sim.quality.ruff-lint moves from advisory to enforced with the codes selected in ruff.toml. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add a changelog entry for ruff group 1 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Sort the imports in the budget output test from #767 #767 landed on main after this stack branched, and its new test imports are unsorted under the I rule this PR enables. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Sort the imports #743 and #744 added #743 and #744 landed on main after this stack branched. A dataset-digest test from #743 and the tomllib import #744 added to the deployment assets test are unsorted under the I rule this PR enables. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Sort the imports #787 added #787 landed on main after this stack branched. It renamed the macro result fixture module to fixtures.macro_output and added fixtures.fake_object_store, and those imports in the output builder, Stage 12 artifact and Stage 12 runtime tests are unsorted under the I rule this PR enables. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Sort the imports #791 added #791 landed on main after this stack branched. I, which this PR enables, flags the import block of its new Stage 12 failure-cause tests; ruff's safe fix sorts it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 15, 2026
Brings in #765 (ruff check in CI with the first group of rule families), #809 (policyengine 6.4.1) and #791 (redacted Stage 12 failure cause). Conflicts: - auth.py: kept this branch's Callable and Literal imports and sorted the stdlib import block the way main's newly enabled I rule expects. - docs/generated/rules.md: regenerated with python -m scripts.rules render from the auto-merged registry, which holds main's enforced sim.quality.ruff-lint and this branch's sim.deploy.permission-enforcement-wired (34 of 48 rules blocking). ruff check . passes under main's ruff.toml with ruff 0.15.22, so this branch's own code needed no further changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 16, 2026
* Enable ruff group 2 (typing-only imports) and fix everything it finds Select TC and move imports used only in annotations under TYPE_CHECKING. Pydantic evaluates model annotations and FastAPI evaluates endpoint annotations at runtime, so ruff.toml lists the runtime-evaluated model base classes (every project base, since ruff only checks direct bases) and the FastAPI/APIRouter route decorators. scripts/rules matches policyengine-api. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register TC in the enforced ruff lint rule Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move the typing-only imports #789 added under TYPE_CHECKING #789 landed on main after this stack branched. TC, which this PR enables, flags the annotation-only imports in its new database-retry and failure-recording modules; they now load only under TYPE_CHECKING. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move the typing-only Callable import #795 added under TYPE_CHECKING Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move the typing-only imports #791 added under TYPE_CHECKING #791 landed on main after this stack branched. TC, which this PR enables, flags the imports its new Stage 12 failure-cause module uses only in annotations (Iterator, Mapping, ObservabilityRuntime, Stage and StagePlan); they move under TYPE_CHECKING. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * List the Pydantic bases ruff reads through imports as runtime-evaluated Ruff treats a class as runtime-evaluated when a base, read through the import statements of the class's own module, is listed in runtime-evaluated-base-classes, or is a class in that same module that is. It never follows a re-export, and only follows inheritance within a module. Four models had bases ruff reads at unlisted paths, so ruff would have moved imports their annotations need under TYPE_CHECKING if they gained any: - ArtifactBaselineSimulation and PrecomputedSimulation subclass policyengine.py's policyengine.core.Simulation; - AppSettings subclasses pydantic_settings.BaseSettings; - EconomyComparison subclassed SingleYearMacroOutput through the executor's simulation_macro_output re-export. The first two paths are listed as their subclasses import them. For EconomyComparison, compat_models now imports SingleYearMacroOutput from policyengine_simulation_contract.macro_output, the module that defines it, and that path is listed: every project entry then names its defining module, and the entry also covers subclasses elsewhere that import the class from the contract. The ruff.toml comment now describes how ruff resolves bases instead of "direct bases only". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Check that ruff treats every Pydantic model as runtime-evaluated The executor's test imports every module of the executor package and of the project libraries installed with it; the entrypoint's imports every module of its package. For every Pydantic model a module-level class statement defines, they check that: - ruff treats it as runtime-evaluated, resolving each base the way ruff does: through the model module's import statements as they stand at the class statement, reading Base[Item] as Base, following bases defined in the same module, and never following a re-export or an assignment. A failure names the model and the base path to list; - its annotations resolve, with model_rebuild(raise_errors=True). Unit tests pin the resolution: a listed base, a subscripted one, a relative import, a same-module chain through Generic, an unlisted re-export, an unlisted third-party base, an assigned alias, and a base in a package's __init__.py, which ruff names with __init__ in its path. The proof fixtures are compat_models.py before and after the previous commit. The executor's src/modal Modal entry files are not imported: they are not part of the installed package, and importing them builds Modal images and functions, which their tests do only with a fake Modal SDK and a patched environment. A static test checks that every class there derives only from builtins and the standard library, so none can be a model. The gateway defines no Pydantic models. The entrypoint shares no package with the executor that could hold test helpers, so it keeps a copy of the resolution helpers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register the runtime-evaluated model rule and document typing-only imports sim.quality.runtime-evaluated-models-resolve is enforced by the two new test files, which run in the executor and entrypoint jobs of the PR test matrix. Its nodes span both projects, so they are listed from the repository root without a project. The test files join the guard-test discovery globs, so any test added to them must be registered. docs/engineering/skills/typing-only-imports.md explains when ruff moves an import, how it decides a model is runtime-evaluated, and how to list model bases; AGENTS.md and the skills README point to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move the typing-only imports #781 added under TYPE_CHECKING #781 landed on main after this stack branched. TC, which this PR enables, flags the imports its contract job_progress module uses only in annotations (Mapping and datetime), which move under TYPE_CHECKING, and the cast in its new integration poll test, whose Client type is quoted. The module's Pydantic models annotate their timestamps with AwareDatetime, which stays a runtime import. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * List the StageReportingSimulation base #781 added as runtime-evaluated #781 landed on main after this stack branched. It added StageReportingSimulation, a policyengine.core.Simulation subclass, and made ArtifactBaselineSimulation subclass it from another module. Ruff reads that base through baseline_artifacts' import, which is not listed, so it would no longer treat ArtifactBaselineSimulation as runtime-evaluated. The sim.quality.runtime-evaluated-models-resolve guard names the path to list; it is added beside the other project bases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 16, 2026
* Document the simulation contract library for ruff D Validators, state helpers and URI functions get docstrings. Every Pydantic model and enum whose class docstring would become a JSON-schema description carries a D101 noqa instead, so the published contract schemas and the OpenAPI documents built from them stay byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the FastAPI support library for ruff D The ping and health models and the /ping/alive route are mounted in the executor app, so they keep their existing (OpenAPI-visible) docstrings under noqa. The no-op observability methods keep their explicit return None under RET501 noqa, and two stray string statements in the GCP module become the docstrings they were meant to be. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the simulation observability library for ruff D Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the Stage 12 persistence library for ruff D Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the simulation entrypoint for ruff D The entry routes are nested in create_app, so their docstrings (the OpenAPI descriptions) are untouched and the generated spec is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the simulation gateway for ruff D The /ping router endpoint keeps its OpenAPI description under a D205 noqa; the golden OpenAPI test still passes. Two placeholder pass statements in the test fixtures' mock exception classes go now that the classes have docstrings (PIE790). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the simulation executor for ruff D Covers the executor package, the Modal apps, fixtures, scripts and test docstrings. Docstrings are added to several Modal functions; images copy src/modal into a layer, so this changes those layers like any source edit. Protocol stubs keep their ... bodies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the integration tests, repo scripts and package markers for ruff D Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the Stage 12 schema-DDL proof fixtures for ruff D The schema-authority proof reads the fixtures as text; the new docstrings avoid DDL phrases, so the bad fixture is still rejected and the good one accepted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Enable ruff group 4 (docstrings) Select pydocstyle (D) and ignore D203 and D213, the incompatible halves of the D203/D211 and D212/D213 pairs. Tests stay exempt from D1 only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Drop redundant return None from documented no-op methods With a docstring as the body, the bare return None lines are redundant; remove them instead of suppressing RET501. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document published contract models and routes Replace the 61 D101 suppressions on contract, ping and health models with class docstrings written for API consumers, and fix the /ping/alive and gateway /ping route docstrings instead of suppressing D205. Pydantic and FastAPI copy these into schema and operation descriptions, so the entry, gateway and executor OpenAPI documents gain descriptions; paths, operations, properties, types and required lists are unchanged. The gateway golden spec is regenerated with generate_openapi. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Use the Public API's docstrings in the shared rules tool Both repos documented scripts/rules independently for ruff D, so the copies drifted apart. Take the Public API's wording so the tool stays byte-identical in both repos; only docstrings differ. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register D in the ruff-lint rule sim.quality.ruff-lint now lists D, and the rendered rules page and AGENTS.md Rules block are regenerated for the new tool hash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #767 added in the imperative mood #767 landed on main after this stack branched. D401, which this PR enables, wants each docstring's summary line in the imperative mood. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #743 and #744 added in the D style #743 and #744 landed on main after this stack branched. D, which this PR enables, flags their new code: blank lines after docstrings (D202), a two-line summary (D205, D209), summaries that are not in the imperative mood (D401), an escaped backslash in the sentinel's module docstring (D301), and three public functions without docstrings (D103). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #789 added in the D style #789 landed on main after this stack branched. D, which this PR enables, flags blank lines after its new docstrings (D202) and the undocumented __call__ of its ConnectionOpener protocol (D102). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the shared object store fake #787 added #787 landed on main after this stack branched. D, which this PR enables, flags the undocumented __init__ (D107), upload_bytes and read_bytes (D102) of the in-memory object store it moved into the executor's fixtures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #795 added in the D style D, which this PR enables, flags the undocumented group_label property (D102), the budget check's run-on summary (D205, D209) and two test helpers' summaries that are not in the imperative mood (D401). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #791 added in the D style #791 landed on main after this stack branched. D, which this PR enables, flags blank lines after five of its new docstrings (D202) and the undocumented __init__ (D107) and stage method (D102) of its StageProgress tracker. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #758 added in the D style #758 landed on main after this stack branched. D, which this PR enables, flags its permission checks and deployed permission test: - CallerAuthorizer's initializer and authorize now say what enforcement does with a caller that lacks a permission, and that the missing permission is always recorded; - the permission_enforcement fixture says when it skips and when it raises, and the enforced-permission deployment test says what it checks; - the blank lines after the docstrings of is_client_credentials, granted_permissions, the gty validator and the Cloud Run deployment-environment test helper go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the docstrings #781 added in the D style #781 landed on main after this stack branched. D, which this PR enables, flags its job progress code: - the JobProgressReporter protocol and each of its methods say which stage boundary the runtime reports; the null reporter's methods say what they discard, with the docstring as the body; - StoredJobProgressReporter's initializer, record, start, set_execution, enter, start_segments, segment_completed and attempts say how the record changes, including when a call changes nothing; - the contract's job progress store functions say what they read and write, and StageReportingSimulation and its ensure say when the callback runs; - the floor and maximum helpers, the Modal reporter factory, the published-record helper and the gateway's stage-field helper are in the imperative mood, and blank lines after docstrings go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 16, 2026
* Fix ruff TRY, EM and BLE findings in the FastAPI library Exception messages move into a msg variable before the raise (EM), with the text unchanged. The JWT decoder re-raises with a bare raise (TRY201). Its two JWKS lookup error logs stay traceback-free under a TRY400 noqa, because callers control the token and a traceback per bad token would let them flood the logs. The token decode and the GCP project-id probe keep except Exception under a BLE001 noqa: any decode failure must become a 403, and any probe failure falls back to GOOGLE_CLOUD_PROJECT. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff TRY and EM findings in the simulation contract library Exception messages move into a msg variable before the raise (EM), with the text unchanged; Pydantic validators keep raising ValueError, so callers still get the same ValidationError. resolve_spm_selection and validate_spm_result move their try bodies into private helpers so no raise sits inside the try (TRY301, TRY300); the same ValueErrors still become the same SPMInputError, chained from the original. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff TRY, EM and BLE findings in the simulation observability library Exception messages move into a msg variable before the raise (EM), with the text unchanged. log_and_redact_exception's fallback log calls logger.exception with the same explicit exc_info, so its output is unchanged (TRY400). Its telemetry catch-all keeps except Exception under a BLE001 noqa, because the helper must never raise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff EM findings in the Stage 12 persistence library Exception messages move into a msg variable before the raise, with the text unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff TRY, EM and BLE findings in the simulation entrypoint Exception messages move into a msg variable before the raise (EM), with the text unchanged. forward() returns the proxied response from an else block (TRY300). The four Stage 12 diagnostic error logs stay traceback-free under a TRY400 noqa: three record only the error type, so a traceback could carry request or calculation data, and the dispatch timeout is fully described by its timeout. The Stage 12 context capture keeps its telemetry catch-all under a BLE001 noqa. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff TRY, EM and BLE findings in the simulation gateway Exception messages move into a msg variable before the raise (EM), with the text unchanged. The auth misconfiguration log keeps its message and gains a traceback through exc_info=True (TRY400). _runtime keeps RuntimeError and _resolve_policyengine_route keeps ValueError under a TRY004 noqa, because handlers and callers depend on those types. The job-status and budget-window handlers keep their catch-alls under a BLE001 noqa: call.get re-raises whatever the worker raised, and every failure must become a failed-job response or fall back to the seed state. The auth-bypass event keeps its catch-all too. The golden OpenAPI spec is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff TRY, EM and BLE findings in the simulation executor Exception messages move into a msg variable before the raise (EM), with the text unchanged. Raises inside try bodies move into private helpers (_parse_policy_period_dates, _certified_us_spm_capability, _require_artifact_digest, _require_local_authority_column), so the same exceptions reach the same handlers (TRY301, TRY300); dump_package_version narrows its try to the file read. qualify_stage12_parity, a local operator CLI that nothing calls or catches, now raises TypeError for a JSON value that is not an object. The other TRY004 sites keep their types under a noqa: they cross a Modal boundary, are documented or asserted by tests, or are recorded by type name in telemetry and Stage 12 error summaries. The Stage 12 comparison failure log stays traceback-free under a TRY400 noqa. Worker, cleanup and telemetry catch-alls keep except Exception under a BLE001 noqa. The Modal images copy src/modal and policyengine_simulation_executor, so their layers change like any source edit; src/modal/app.py gains no future import. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff EM findings in the integration tests and repo scripts Exception messages move into a msg variable before the raise, with the text unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Enable ruff group 5 (exceptions and messages) Select flake8-blind-except (BLE), flake8-errmsg (EM) and tryceratops (TRY). The preceding commits fix every finding; deliberate catch-alls, kept exception types and traceback-free error logs carry an inline noqa with the reason. Tests get no exemption. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Give every BLE001 suppression an inline reason Enabling BLE activated suppressions that were written before the rule was on. Seven had no reason, or kept it only in a comment above; they now carry it inline, and the "- reason" ones use the " # reason" form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Name hoisted SPM error codes error_code, not msg The EM fix moves an exception's first literal argument into a variable called msg. SPMInputError's first argument is an error code, followed by the message, so those 20 variables are renamed error_code. The gateway file also moves its BLE001 suppression reason to the " # reason" form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register TRY, EM and BLE in the ruff-lint rule sim.quality.ruff-lint now lists the group 5 families, and the rendered rules page and AGENTS.md Rules block are regenerated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Name hoisted parity mismatch codes error_code, not msg ParityMismatch takes a code and a schema path, not a message, so the 26 variables the EM fix named msg are renamed error_code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the group 5 changelog line's YAML and wording The colon after "(TRY, EM, BLE)" made YAML read the item as a mapping rather than a string. The line also said error types were unchanged, but two type checks in qualify_stage12_parity.py now raise TypeError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #743 and #744 added before raising #743 and #744 landed on main after this stack branched. EM and TRY003, which this PR enables, flag 34 raises in their sentinel, routing and package-update code that pass a literal message. Each message now moves into a msg variable before the raise, with the text unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #789's tests raise before raising PR enables, flag five raises in its new Stage 12 tests that pass a literal message; each message now moves into a msg variable first. TRY301 flags the raise inside the pooling test's transaction, which moves into a small helper so the same RuntimeError still ends that transaction. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #795 added before raising #795 landed on main after this stack branched. EM and TRY003, which this PR enables, flag its two new RuntimeError raises (a child that timed out and a spent parent budget) and the cancel failure its test fake raises; each message now moves into a msg variable first, with the text unchanged. BLE001 flags _cancel_call, which now holds the best-effort cancel loop this PR suppressed; it keeps that suppression with its reason inline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #791's tests raise before raising #791 landed on main after this stack branched. EM and TRY003, which this PR enables, flag ten raises in its new Stage 12 failure-cause and runtime tests that pass a literal message; each message now moves into a msg variable first, with the text unchanged. TRY301 flags the raise inside the try of the _raised helper, which now captures the raised error, with its traceback, through pytest.raises; that also drops the helper's blind except and its BLE001 suppression. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Write the BLE001 suppressions #791 added with an inline reason #791 landed on main after this stack branched. Its two new blind-except suppressions in the Stage 12 failure-cause module gave their reasons in the "- reason" form; like every other BLE001 suppression in this PR, they now use the " # reason" form, with the reasons unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #758 added before raising #758 landed on main after this stack branched. EM and TRY003, which this PR enables, flag four raises with a literal message: the deployed permission_enforcement fixture's RuntimeError and three stand-ins in its permission tests. Each message now moves into a msg variable first, with the text unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Write the BLE001 suppression #758 added with an inline reason #758 landed on main after this stack branched. The blind except around its simulation_entry_permission_missing event gave its reason in the "- reason" form; like every other BLE001 suppression in this PR, it now uses the " # reason" form, with the reason unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Build the exception messages #781 added before raising #781 landed on main after this stack branched. EM and TRY003, which this PR enables, flag nine raises with a literal message: the contract's segment-count validator and eight stand-ins in its executor and gateway job progress tests. Each message now moves into a msg variable first, with the text unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Write the BLE001 suppressions #781 added with an inline reason #781 landed on main after this stack branched. Its five new blind-except suppressions, in the Modal app's reporter and stage-duration helpers, the stored reporter's read and write, and the gateway's stage-field read, gave their reasons in the "- reason" form; like every other BLE001 suppression in this PR, they now use the " # reason" form, with the reasons unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 16, 2026
…Z, ERA, TD, FIX, EXE) (#785) * Fix ruff LOG and G findings Pass logging arguments instead of f-strings (G004); each message renders to the same text. Name the economy-comparison router's logger after its module rather than its file path (LOG002). Log the gateway's auth misconfiguration with logger.exception and a fixed message (G201): the traceback already ends with the exception and its message, which the old call also repeated in the message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep naive policy dates explicit for ruff DTZ policyengine.py parses policy period keys with strptime and no zone, so its ParameterValue dates are naive, and the executor's period parsing and the tests that compare against those dates must stay naive too. Mark each kept naive datetime with a reasoned DTZ suppression, and route the policy tests' expected dates through one helper. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Replace asserts outside tests for ruff S101 The deployed-service test suite in authenticated_tests is a pytest suite that the tests glob misses, so give it the same test exemptions. Elsewhere, drop the asserts that only narrowed types: the SPM selection check now returns the hash and scenario it has already proved present, and the qualification runner closes over a non-optional runtime. The gateway image smoke raises RuntimeError when its report is incomplete, as the executor's static-file smoke does, and the fake model output raises AssertionError explicitly when the adapter asks for a mode other than JSON. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep command-line output for ruff T20 Every print the rule finds belongs to a command-line script or a Modal local entrypoint, whose stdout is its output, so keep each print. Files with one to three prints mark each with a reasoned suppression; the routing-state publisher and the policyengine package updater send their output through one small helper that holds the only print. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Route test subprocess calls through helpers for ruff S603 and S607 Each test module that runs repository scripts now does so through one helper that holds the only subprocess call, with reasoned suppressions: the scripts are repository-owned, their arguments are built by the tests, and bash and uv come from PATH as they do in the workflows. Resolving the executables in Python instead could pick a different binary, because subprocess searches the child's PATH, and some tests run scripts with an empty environment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the remaining ruff S findings Build the GCP metadata request inline from its constant http URL, with the same Metadata-Flavor header, so S310 can see the scheme. Keep every other finding with a reasoned suppression: - S105/S106: fake tokens in tests (one module constant per test module where a value repeats), and names or fingerprints that only look like secrets. - S108: the executor's fixed data directories inside the Modal container, and a lookup path the tests' fakes only pass along. - S110: add S110 to the existing reasoned BLE001 suppressions. - S102: the image tests execute repository-owned app source as Modal's /root/app.py. - S301: the SPM tests round-trip errors they built or raised. - S310: the Hub API request URL is built from the HF_ENDPOINT base and quoted path parts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Enable ruff group 6 (security, logging and debugging leftovers) Select DTZ, ERA, EXE, FIX, G, LOG, S, T20 and TD. The earlier commits on this branch fixed or suppressed, with a reason, every finding outside the shared scripts/rules tool. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register group 6 and the deployed-test ruff exemption sim.quality.ruff-lint now lists DTZ, ERA, EXE, FIX, G, LOG, S, T20 and TD. The authenticated_tests exemption gets its own rule, since a ruff per-file-ignore rule names one pattern. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Sync the rules tool from the Public API's group 6 The shared scripts/rules tool takes the Public API's group 6 fixes: explicit checks instead of asserts, and reasoned suppressions for its git and shellcheck subprocess calls, prints and calendar dates. The rendered rules page picks up the tool's new hash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 6 findings in the code #743 and #744 added #743 and #744 landed on main after this stack branched. Group 6, which this PR enables, flags their new code; each fix follows this PR's conventions: - T201: the routing publisher's new commands report through its _report helper, the package updater's dispatch check through _write_result, and the sentinel CLI gets one _report helper that holds its only print. - S603/S607: the deployment-asset tests route #744's new bash calls through _run_bash, which now takes cwd; the rollback tests and the package-update helper tests get one helper each that holds the only subprocess call, with reasoned suppressions. - S310: the sentinel's HTTP helper opens the deployed entrypoint or the test auth issuer URL that the workflow passes; reasoned suppressions. - S106: a fake bearer token in the sentinel tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add S110 to the cancel suppression #795 added #795 landed on main after this stack branched. S110, which this PR enables, flags the try-except-pass in its _cancel_call, which now holds the best-effort cancel loop this PR suppressed; like the other reasoned BLE001 suppressions, it gains S110. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 6 findings in the code #791 added #791 landed on main after this stack branched. S105, which this PR enables, flags two values that only look like secrets, and each keeps a reasoned suppression: the redaction pattern for secret-looking key names in the Stage 12 failure-cause module, and the fake database password its tests check redaction removes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 6 findings in the code #758 added #758 landed on main after this stack branched. S603, which this PR enables, flags the two new deployment tests that run the Cloud Run deploy script directly to check permission enforcement defaults and rejections. They carry the same reasoned suppression as the existing direct run of that script. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 6 findings in the code #781 added #781 landed on main after this stack branched. Group 6, which this PR enables, flags two of its additions: - S110: the Modal app's stage-duration telemetry swallows a failed set_context, so its suppression adds S110 beside BLE001, as the call attributes helper above it does; - DTZ001: the contract test that the job record rejects a naive time builds that naive time on purpose, so it carries a reasoned suppression. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Allow file-level ruff exceptions only in tests, CI/CD and command-line scripts A ruff per-file-ignores entry switches rules off for whole files. That is now acceptable where it's the norm: tests, CI/CD scripts under .github, and command-line scripts (under a scripts directory, or modules run through if __name__ == "__main__"). Everywhere else a kept finding still needs an inline suppression with its reason. The rules guard already requires a per-file-ignore rule for every entry. It now also expands each entry's pattern the way ruff does (globset syntax, and a slash-less pattern also matches file names) and fails when the pattern covers any other file, is negated, or matches nothing. scripts/rules/file_exceptions.py holds the definitions, and the registry doc describes the policy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move group 6's test, CI and script suppressions to file-level exceptions Tests, CI/CD scripts and command-line scripts now take their deliberate group 6 exceptions as registered per-file-ignores, each code chosen from the suppressions those files already carried: - tests and the deployed-service tests: S102, S105, S106, S108, S301, S603, S607, DTZ001 and DTZ007, added to their existing exemptions; - .github: T201; - scripts directories: DTZ011, S603, S607 and T201; - programs under src (the Modal deployment utilities, and the OpenAPI and Stage 12 infrastructure commands): S310 and T201. That removes 75 inline codes from 44 files. Application code keeps its inline suppressions and reasons. Comments only: no code changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 16, 2026
…#799) * Exempt tests from SLF001, INP001 and PLC0415 Tests deliberately reach into private members, pytest test directories aren't packages, and tests often import inside a test after patching the environment. Application code still gets every rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Drop response_model arguments that repeat the return type (ruff FAST001) FastAPI takes the response model from the return annotation when response_model is omitted, so the generated OpenAPI documents are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix ruff naming and builtin-shadowing findings (N, A, PLC0414, SLF001) - Alias the shared exit registry as app_exit where it is imported, and keep the policyengine_fastapi.exit.exit name that projects import. - Use snake_case for the OpenTelemetry provider and log-handler locals. - Add the Error suffix to exceptions that never leave their project: BackendUnavailableError, BackendTimeoutError, TemporaryStage12UnsupportedRequestError and ParityMismatchError. TemporaryStage12DispatchFailed keeps its name, which the entry app logs as error_type. - Keep the camelCase budget-window fields, which are published JSON, the explicit re-exports in stage12_execution, and the two deliberate private accesses, each with a reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff INP001 per directory Declare the script directories (.github/scripts, scripts and the executor's scripts) and the rules proof fixtures as namespace packages in ruff.toml. Ruff's full output, with every rule selected, is otherwise unchanged by the setting. The executor's src/modal/utils scripts keep an inline suppression instead: declaring that directory, or adding an __init__.py, makes ruff treat it as part of a first-party "modal" package and regroup its Modal SDK imports. An __init__.py would also change the src.modal source baked into the Modal images. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep deliberate deferred imports in the shared library and CI script (ruff PLC0415) Each deferred import keeps an inline reason: the GCP exporters, logging instrumentation and metadata probe load only when used, and only the bundle-versions command runs where policyengine is installed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Import the Stage 12 backend at the top of the entry app (ruff PLC0415) app.py already imports stage12_backend at module level for its exception types, so binding Stage12ComparisonBackend there too loads nothing new. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff PLC0415 in the gateway Bind SPMSelection in endpoints.py's existing policyengine_simulation_contract.spm import. Keep the Modal function body's imports, which build the web app in its container, and the smoke app's in-image imports, each with a reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Resolve ruff PLC0415 in the executor Move only imports that load nothing new and that no test patches: pathlib and importlib, the policyengine_simulation_contract helpers, names from a module the file already imports at the top, and RegionGroupStrategy, which policyengine.core already loads. Every other deferred import keeps an inline reason: Modal function bodies and the image-build and smoke functions, lazily loaded policyengine models and heavy libraries, call-time resolution that the tests' patch seams rely on, and import cycles in the runtime. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap long lines in the shared libraries (ruff E501) Split long messages and the GCP log format with implicit string concatenation, so every string is unchanged, and rewrap two docstrings that no published schema uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap long generated-client calls in the integration tests (ruff E501) Bind the long generated endpoint functions to shorter local names before calling them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap long lines in the entry project (ruff E501) Split long strings with implicit concatenation, so every value is unchanged. The fake curl and gcloud scripts keep their single-line printf output with a suppression on the string. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap long lines in the executor (ruff E501) Split long messages and test strings with implicit concatenation, so every value is unchanged, and shorten docstrings that no published schema uses. The fake gh script keeps its single-line JSON output with a suppression on the string. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap long lines in the gateway (ruff E501) Split the Modal app's FastAPI description with implicit concatenation, so the published text is unchanged, and shorten four test names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wrap the Stage 12 DDL proof fixtures' docstrings (ruff E501) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Enable ruff group 7 (structure and imports) Select A, E, FAST, INP, N, PLC and SLF. Every finding outside the shared scripts/rules tool is fixed or carries an inline reason; the rules tool's findings are fixed upstream and synced separately. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Register group 7 in the ruff-lint rule sim.quality.ruff-lint now lists A, E, FAST, INP, N, PLC and SLF. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Sync the rules tool from the Public API's group 7 The shared scripts/rules tool takes the Public API's group 7 fixes: wrapped long lines, BaseUnavailableError, and assert_violation in its tests. The rendered rules page picks up the tool's new hash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 7 findings in the code #743 and #744 added #743 and #744 landed on main after this stack branched. Group 7, which this PR enables, flags their new code; each fix follows this PR's conventions: - E501: wrap the long strings in their routing, sentinel, gateway and test code; every message and expected value reads the same. - N818: RoutingConflict never leaves the executor, so it becomes RoutingConflictError, as this PR renames the other in-project errors. - INP001: the sentinel script in src/modal/utils keeps the inline reason the other scripts there carry. - PLC0415: the sentinel's deferred contract and runtime imports and the package updater's optional receipt import keep inline reasons. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep the long Stage 12 test name #789 added under E501 #789 landed on main after this stack branched. E501, which this PR enables, flags the signature line of its lost-commit coordinator test, whose name cannot be wrapped; the line keeps a reasoned suppression rather than a shorter, vaguer name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 7 findings in the code #791 added #791 landed on main after this stack branched. E501, which this PR enables, flags three long string literals in its Stage 12 failure-cause tests; each is wrapped by implicit concatenation, with its value unchanged. N818 flags Stage12FailureCause, whose name telemetry records as the error type of every recorded cause, so it keeps its name under a reasoned suppression, like TemporaryStage12DispatchFailed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 7 findings in the code #758 added #758 landed on main after this stack branched. FAST002, which this PR enables, flags the six entry routes that take the caller's identity as a "caller: CallerIdentity | None = Depends(...)" parameter. FastAPI's Annotated form can't be used: this module postpones annotations, and FastAPI resolves them against the module's globals, where the per-app permission dependency doesn't exist. No handler reads the parameter. Each route now declares its permission dependency in the decorator's dependencies, as the gateway's routes declare require_auth and as these routes declared authentication before #758. FastAPI still runs the same dependency before the body is read, so each route checks the same permission, logs or rejects a missing one the same way, and the OpenAPI document is unchanged. The CallerIdentity import is no longer used. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the ruff group 7 findings in the code #781 added #781 landed on main after this stack branched. PLC0415, which this PR enables, flags four of its deferred imports. As for the executor's other deferred imports, only an import that loads nothing new and that no test patches moves to the top: - the Modal app's NULL_JOB_PROGRESS and StoredJobProgressReporter, whose module the app's top-level imports already load, move to the top; - the job_progress_state store functions stay deferred, resolved per call so the tests can patch job_progress_state; - StageReportingSimulation stays deferred in _build_simulation, resolved per call like its neighbours so the tests can patch it; - the bundle-metadata builder's JOB_PROGRESS_CAPABILITY_VERSION stays deferred: the contract package, and pydantic with it, loads only when bundle metadata is built. Each kept import carries its reason inline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * List the lazily loaded packages for ruff instead of suppressing each import Fourteen heavy or optional packages and modules are only ever imported inside the function that needs them: the policyengine bundle, countries, provenance and tax-benefit-model modules, policyengine_us, spm_calculator, h5py, google.cloud, three OpenTelemetry exporters and instrumentations, pythonjsonlogger, and the executor's spm and baseline_artifacts modules. No application module imports any of them at module level. ruff.toml now lists them in banned-module-level-imports and selects TID253, so importing one at module level fails (TYPE_CHECKING imports are still fine), and PLC0415 accepts their in-function imports without a suppression. That removes 51 PLC0415 suppressions. Tests may still import them at module level (TID253 joins the test exemptions). Packages that some application module does import at module level (pandas, microdf, huggingface_hub, policyengine.core and others) aren't listed, so their deferred imports keep their reasons. The nine Modal utility programs, run with python -m from a directory that isn't a package, take INP001 in their existing file-level entry instead of one suppression each. Comments and configuration only: no code changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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