Repository navigation
Conversation
…meters (failing) A report or simulation whose policy sets parameters the running catalog version lacks should answer 422 with code missing_parameters, naming them, before the simulation service is asked. A catalog that cannot be queried should answer 503, as the database does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
POST /v2/reports and POST /v2/simulations now check, before the simulation service is asked, that the running catalog version has every parameter each named policy sets. A report checks its baseline's policy, then its reform's; every policy is read first, so a policy that is not stored still answers 404 before another's missing parameters answer 422. The 422 has code missing_parameters and names the parameters, as POST /v2/household-calculations answers. require_runnable_policies holds the check, for re-runs to call too. The submission checks now answer 503 when the catalog query fails, as they do when the database cannot be read, rather than 500. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…test policies The report and simulation create protocols now list PolicyParametersMissingError and SubmissionStorageError among the errors a create raises. The submission tests' stand-in policy source returns CheckedPolicyRead values, as the protocol it stands in for does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each create protocol now names the error raised when the record the simulation service created cannot be read back, and the simulation create no longer points at the report create's read-back error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2plan/c1-8-simulations-and-reports #4286 +/- ##
=======================================================================
+ Coverage 92.13% 93.12% +0.99%
=======================================================================
Files 357 357
Lines 20468 20480 +12
Branches 3298 3302 +4
=======================================================================
+ Hits 18858 19072 +214
+ Misses 1036 838 -198
+ Partials 574 570 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…rts' into v2plan/c1-11-refuse-missing-parameters
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4285
Stacked on #4281 (
v2plan/c1-8-simulations-and-reports), which adds/v2/reportsand/v2/simulations. This PR's base is that branch; it is retargeted tomasteronce #4281 merges.Summary
POST /v2/reportsandPOST /v2/simulationsrefuse a policy that sets parameters the running catalog version no longer has, before the simulation service is asked, so no record is created and no run starts. They answer 422 in the shapePOST /v2/household-calculationsuses:{"status": "error", "message": …, "code": "missing_parameters", "missing_parameters": [...]}. A policy whose parameters all exist, or that the running catalog could not check (no catalog is published for the running PolicyEngine.py version), is sent as before.require_runnable_policiesin the simulation service's submission module, whichSimulationSubmissionServicecalls after the country check. It reads every named policy first, so a policy the country does not store still answers 404 before another's missing parameters answer 422. A report checks its baseline's policy, then its reform's; when both set missing parameters, the 422 names the baseline's. It reusesrequire_policy_parameters_present, the validator behind the household calculation refusal. Re-runs are to call the same function.simulation_resource_operation, which the report and simulation routes share, answersPolicyParametersMissingErrorwith that 422 and does not log it, since the caller can act on it. A future route on the same helper, such as a re-run, gets the same answer.MetadataCatalogUnavailableError), as they already did for a database error and as household calculations do. Before, that failure answered 500.V2MissingParametersErrorResponse, and their descriptions, the 503 text and the create protocols' docstrings name the new check.User-visible contract changes
missing_parametersonPOST /v2/reportsandPOST /v2/simulations.Not covered
A parameter that keeps its name but changes its structure, unit or meaning passes a check by name, as on the household route. The check uses the catalog version of the Public API's installed PolicyEngine.py; the simulation service runs its default worker's version, as for household calculations.
Testing
Red-green:
7f4d129edadded the tests and failed for the expected reasons (14 cases: the simulation service was asked for an outdated policy, the route answered 500, the catalog error escaped unwrapped, the OpenAPI 422 had noanyOf);0e6100d65made them pass. Each of these ran by node ID and passes:tests/unit/v2/test_simulation_submissions.py: the refusal for a report's baseline or reform and for a simulation; the baseline named first when both are outdated; 404 before 422; an unchecked policy is sent; a catalog or database read failure becomesSubmissionStorageError; and the file's existing tests, whose policy stand-in now returnsCheckedPolicyReadvalues.tests/unit/v2/test_simulation_submission_database_routes.py(new): the routes with the real country, policy and submission services over SQLite, so the real catalog query runs. Outdated report and simulation policies answer 422 and reach no simulation service, policies whose parameters all exist are sent, and a catalog that cannot be queried answers 503.tests/unit/v2/test_report_create_routes.py: the 422 body and no log record for both routes, and the OpenAPI 422 entries.Checks:
alembic -c alembic-v2.ini checkon a fresh PostgreSQL 17 database reports no new operations (no revision added);lint-imports(5 contracts kept);python -m scripts.rules validate;python scripts/run_quality_guards.py(all passed); ruff, ruff format and mypy clean.python scripts/export_migration_contracts.pyregenerated the contract documents with no change. Nothing underpolicyengine_api/data/v2/changed.Review: two read-only review rounds. Round 1 found no bugs; its code-quality findings (a create protocol docstring that missed the new errors, and a test stand-in that no longer matched the policy source protocol) are fixed in
795116de8, which round 2 reviewed and found no bugs in. Round 2's remaining docstring finding (the create protocols did not name the read-back errors) is fixed ind544556ac, which changes only docstrings. Round 1 also noted that this wrapper of storage errors repeats the household calculation service's; every v2 service wraps storage errors in its own type, and no module both can import may name SQLAlchemy's errors (v2 validators and types may not import SQLAlchemy, and service modules load only when their factory runs), so they stay separate.CI: as a PR stacked on #4281, this may get only part of CI until #4281 merges and this PR is retargeted to
master.No environment variables, secrets or deployment files change.
🤖 Generated with Claude Code