Repository navigation
Conversation
d5d1f0e to
c75c444
Compare
c75c444 to
6c58a36
Compare
|
Review at Verdict: a safe first step. It adds no tables and no data, the migration reverses cleanly, and only
Checked and fine: one transaction covers the schema creation and all revisions; What I ran: fresh clone, 🤖 Generated with Claude Code |
6c58a36 to
433f44e
Compare
433f44e to
874e15d
Compare
Add libs/policyengine-simulation-persistence, the package through which the simulation service (the Simulation Entrypoint) connects to the Public API's Postgres database with its own role. This commit holds only the connection: the runtime engine, built from the canonical postgresql:// URL the deployment stores, with the psycopg 3 driver, a small pool, pre-ping, a connect timeout and no server-side prepared statements, so the same URL works through Supabase's poolers. A malformed URL raises ValueError without repeating the URL or the parser's message, since the URL holds a password. The package is unit-tested in the PR test matrix and by make test, type-checked by the Pyright job under sim.quality.pyright-libraries-and-entry, and scanned by the database access guard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…its database
The Simulation Entrypoint reads SIMULATION_DATABASE_URL, its own role's URL
for the database that holds the Public API's tables. When it is set, the app
builds a pooled engine (no connection opens until first use), disposes of it
at shutdown, and GET /ready reports whether a pooled connection answered a
trivial query:
{"status": "ready", "simulation_database": {"reachable": true}}
The database does not change the readiness status, because no old-gateway or
Stage 12 route uses it yet: an unreachable or slow database (bounded at three
seconds) is reported as unreachable, logged with its error type only, and
/ready keeps following the old gateway. Without the setting, /ready keeps its
old shape. A URL that does not use the canonical postgresql:// scheme fails
validation without repeating the URL.
The entrypoint depends on the persistence library, so its image's build
context now includes it; a new test checks that every local library the
entrypoint installs is in the build context.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The deploy script built its Cloud Run secret list in array assignments with one command substitution per secret. Bash gives such an assignment the exit status of its last substitution only, so in the Stage 12 block a failure to resolve either Modal token secret (an invalid name, a failed version listing, or no enabled version) did not stop the script, and it deployed with an empty secret reference. Each secret is now resolved in its own assignment, so set -e stops the script at the first failure, before gcloud run deploy runs: the main path's secrets through add_runtime_secret, and the Stage 12 comparison path's through add_stage12_runtime_secret, its own copy with the same behaviour, so it shares no deploy helper with the main path and is removed with it. A test covers every runtime secret (both Modal token secrets deployed before this change), and another checks that each block uses only its own helper. Both are registered under sim.deploy.workflow-shell-safety. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch tightens both rules, which had no proof: - sim.deploy.workflow-shell-safety now also requires every runtime secret to be resolved in its own assignment. combined_secret_resolutions finds a Bash array assignment that resolves more than one secret, where an earlier failure would pass unnoticed. The deploy script passes it (a new test, registered under the rule); main's script before this branch fails it, and so does the proof's bad fixture. - sim.deploy.container-context-minimal now also requires every local library the entrypoint installs to be in its build context. libraries_missing_from_context compares a project's path sources with its .dockerignore; the existing test uses it, and the proof's bad fixture leaves one library out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The deploy script now requires SIMULATION_DATABASE_URL_SECRET_NAME (a GitHub environment variable in beta and prod, passed in the deploy step's env) and mounts that Secret Manager secret's newest enabled version as SIMULATION_DATABASE_URL on every deploy, whatever the Stage 12 setting. The secret holds the simulation service role's URL for the database that holds the Public API's tables. It is resolved like the other runtime secrets, in its own assignment, so a missing or invalid name, or a secret with no enabled version, stops the deploy before gcloud run deploy runs. The deployment document describes the wiring. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…readiness check
The persistence library now lists the table privileges the simulation
service's role needs on the Public API's tables (SERVICE_TABLE_PRIVILEGES):
SELECT and INSERT on the tables it creates rows in (simulations, reports,
simulation_runs, report_runs, report_run_simulation_runs and
household_jobs), and SELECT on the policy, household and catalog tables a
run reads. missing_table_privileges asks PostgreSQL, in one query, which of
them the connected role lacks; a table missing from the public schema lacks
all of its privileges, and a table of the same name in another schema on the
search path doesn't count.
GET /ready reports the result next to whether the database was reached:
{"status": "ready",
"simulation_database": {"reachable": true, "missing_privileges": []}}
missing_privileges is null when the database was not reached. Neither field
changes the readiness status.
Those tests need PostgreSQL, which the Test job doesn't provide, so a new
Simulation database job runs the library's postgres-marked tests on
postgres:17 and fails rather than skips without the server. CI complete
needs it. The new rule sim.persistence.missing-privileges-reported, in a new
persistence category, records the check, with a proof.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…role needs The candidate smoke check now sets REQUIRE_SIMULATION_DATABASE_ACCESS=true and then requires /ready to report the simulation service's database reached with no table privilege missing, which shows that the role's URL reached the revision and that the role holds its grants. A failure names what /ready reported, such as "INSERT on simulation_runs". /ready waits at most three seconds for the database, while a new connection may take up to five, so a cold candidate whose first connection is slow is asked again while it reports the database unreachable, up to SIMULATION_DATABASE_READY_MAX_ATTEMPTS requests in all (six by default), with the usual retry delay. A reached database with a missing privilege fails at once. The stable and rollback checks don't set the requirement, because an older image may not use the database. The fake curl in the deployment tests can now serve a sequence of /ready bodies, and the new tests are registered under sim.deploy.smoke-tests-strict. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ruff's formatter, at the pinned version, rewraps two calls in the test modules this branch added. No behaviour changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s database The persistence library now describes, with SQLAlchemy Core in service_metadata, the tables the simulation service creates rows in: simulations, reports, simulation_runs, report_runs, report_run_simulation_runs and household_jobs. Each model matches the table the Public API's v2 migration chain builds, column for column (type, nullability, server default), with every constraint and index under the chain's name. Foreign keys stay between these tables, because another service writes the policy, household and catalog rows they name. Status, trigger, role and type columns use StrEnum classes and store each member's value. The models only describe the tables: runtime code never creates or changes them. Tests marked public_api_schema run as the service's role (policyengine_v2_simulation_service) in a database the Public API's chain built with its declared grants, named by PUBLIC_API_SCHEMA_DATABASE_URL: the role lacks none of the privileges the service needs, and creates and reads back a row in every modelled table, in a transaction rolled back afterwards. Without the database they skip. The database-access skill now says that the Public API's migrations own every table this repository uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tion Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ing the database unreachable missing_table_privileges found each table with to_regclass, which raises "permission denied for schema" for a role without USAGE on public. The readiness check caught that as a failure to reach the database, so a reached database with a grant missing was reported as unreachable, and the candidate check asked again six times before failing with a message about connectivity. Tables are now found through PostgreSQL's catalog (pg_class and pg_namespace), which any role may read, and a missing USAGE privilege on the schema is named first, as "USAGE on schema public". The rule's statement says so, and the new test is one of its enforcers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…wn check Settings._validate_simulation_database checked only the URL's scheme, while the engine also refuses a URL without a user name, host or database, or with surrounding spaces. The settings now ask normalized_database_url, so they accept exactly the URLs the engine does, and say why one is invalid without repeating it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…upport module The database deployment tests imported cloud_run_deploy_environment from another test module. It now lives in test_deployment_assets_support with the other shared deployment helpers, and both modules import it from there. No test changes what it checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The OpenAPI test's comment still spoke of the database's revision, the persistence tests' conftest named a script this change does not add, and AGENTS.md's summary of PR CI did not mention the PostgreSQL tests. The deployment document now says what each environment needs, and in which order, before the first deploy that carries the connection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Public API's revision that stores the economy output fields with no column adds report_runs.spm_provenance (JSON) and data_version (varchar(128)), both nullable. The models now have them, so they again match the tables the Public API's chain builds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ops waiting /ready waits at most three seconds for the database, but asyncio.wait_for can't stop the thread the query runs on, so a database that accepted the connection and then stalled held a worker of the default thread pool for as long as it stalled, once per request to the public route. The query now runs in a transaction whose statement_timeout, set with set_config local to that transaction, is two seconds, so the database ends it first; it works through Supabase's transaction pooler and never outlives the check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The privilege check looks only at the public schema, but the models left their schema unnamed, so a write through them followed the role's search_path and would reach a table of the same name in a schema earlier on it. service_metadata now names public, as the Public API's tables live there, and the foreign keys name it too. The privilege check reuses the same constant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
create_app built the engine from SIMULATION_DATABASE_URL before the lifespan ran Settings.validate(), so a malformed URL stopped the import with the library's ValueError and the settings' own check only ever ran in tests. create_app now runs that check first, so a malformed URL is a ConfigurationError naming SIMULATION_DATABASE_URL, without repeating it, at startup like every other setting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The deploy script's comment said the Stage 12 block's wrapper shares no deploy helper with the main path, but both wrappers call resolve_secret_reference. The comment and the rule's statement now say what is separate: the wrapper itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…on in the library The database's statement_timeout bounds only statements, not the wait for a pooled connection, connecting or a dead socket, so a stalled database still let each request to the public /ready route hold a thread of the default executor, which the Stage 12 backend's calls and the old-gateway client's name lookups also use. SimulationDatabase now runs the check on a ThreadPoolExecutor of one thread of its own, so a stalled database holds at most that thread and never one of the default executor; checks that time out while it is busy are cancelled before they start. dispose stops it. The bounded transaction moves to the persistence library as read_missing_table_privileges, where PostgreSQL tests show that it reads the privileges, that the database ends a statement that runs too long, and that no statement_timeout stays on the pooled connection either way. The readiness documents and the smoke message say that "reachable": false also covers a database that was reached but didn't answer in time, and the models' docstring no longer claims a check the service doesn't make yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SQLAlchemy 2.1, which the persistence library's own lock resolves, gives an enum the metadata's schema, but 2.0.54, which the entrypoint's image installs, doesn't, so there the six enum types followed the role's search_path. Each now names public itself. The test fails under 2.0.54 without the change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t line combined_secret_resolutions took a ) anywhere on an array assignment's first line as its end, so an assignment whose first element resolved a secret on that line, with a second on the next, was read as one line and passed. The assignment now ends on its first line only when that line's parentheses balance. The new test is one of sim.deploy.workflow-shell-safety's enforcers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eport_run_comparisons The Public API's revision dce0caa329cc moves labor_supply_result, cliff_impact_result, spm_provenance and data_version from report_runs to a new report_run_comparisons table, one row per report run and compared pair, which the Modal workers write and the simulation service doesn't. The report_runs model drops them, so it again matches the Public API's table. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… check fully The enum test now also checks the type the helper builds before a table adopts it, so it fails under the persistence library's own SQLAlchemy 2.1 when the helper stops naming public, not only under the image's 2.0. The stalled-database test checks that only the first of three checks ever ran, and the dispose test that a started worker exits. "reachable": false is described as any failed check (not reached, login refused, no answer in time), the smoke script's retry message says so, the persistence readme says statement_timeout ends each statement of the check, and SimulationDatabase.dispose says the database is single-use afterwards. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… readiness wording A test now shows that /ready reports a disposed database's check as failed, and it fails if dispose leaves the check's thread running. The readiness docstrings, the smoke script's comment and the deployment document no longer speak of a database that was not reached where any failed check is meant, and the time limit is described per statement. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
874e15d to
21760a9
Compare
Fixes #897
Same-Repository Draft PR Check
PolicyEngine/policyengine-sim-api, not a personal fork.Changelog
changelog.d/<name>.<type>.md, whose type is breaking, added, changed, fixed or removed:changelog.d/897.added.md.Summary
The Simulation Entrypoint (the simulation service) connects to the Postgres database that holds the Public API's v2 tables, with its own login role, which can create simulations, reports, their runs and household jobs there. This repository gets no schema and no migration chain of its own: the Public API's migration chain creates those tables, and its privilege manifest grants the role. This replaces this PR's earlier design, which gave the entrypoint its own
simulationschema, an Alembic chain and a migration job before each deploy; all of that is removed, with the rules that only served it.libs/policyengine-simulation-persistence):SELECTandINSERTon the tables it creates rows in,SELECTon the policy, household and catalog tables a run reads) and a one-query check of which the connected role lacks;simulations,reports,simulation_runs,report_runs,report_run_simulation_runsandhousehold_jobs, matching the Public API's tables column for column, with their constraints and indexes. Their foreign keys stay between these tables.SIMULATION_DATABASE_URLas a pinned Secret Manager version on every deploy.GET /readyreportssimulation_database {reachable, missing_privileges}without changing its status. The check runs on a thread of its own, in a transaction whosestatement_timeoutends any statement that runs past two seconds. The candidate check requires the database reached with no privilege missing, asking again while the check fails (as it does for a cold candidate); the stable and rollback checks don't.sim.persistence.missing-privileges-reported, with a proof, in a newpersistencecategory;sim.deploy.workflow-shell-safety,sim.deploy.container-context-minimalandsim.deploy.smoke-tests-strictare tightened, the first two with proofs.Simulation databasejob runs the persistence tests that need PostgreSQL onpostgres:17, and fails rather than skips without the server;CI completeneeds it. The tests that run as the service's role against the Public API's own tables need a database the Public API's chain built; they skip here, and the stacked pull request'sPublic API schemajob runs them.Environment variables
SIMULATION_DATABASE_URLpolicyengine-simulation-entry(the project named bySIMULATION_ENTRYPOINT_GCP_PROJECT_ID)SIMULATION_DATABASE_URL_SECRET_NAMEto its newest enabled version and mounts it withgcloud run deploy --set-secrets, on every deploySIMULATION_DATABASE_URL_SECRET_NAMEbeta,prod)env:of the "Deploy Cloud Run entrypoint candidate" step indeploy_entrypoint, whose job declaresenvironment: ${{ inputs.release_environment }}REQUIRE_SIMULATION_DATABASE_ACCESStrueenv:of the "Verify the complete deployed request path" stepSIMULATION_DATABASE_READY_MAX_ATTEMPTSSIMULATION_TEST_DATABASE_URL,SIMULATION_REQUIRE_DATABASE_TESTSenv:ofSimulation database, pointing at itspostgres:17servicePUBLIC_API_SCHEMA_DATABASE_URL,SIMULATION_REQUIRE_PUBLIC_API_SCHEMApublic_api_schemaChecked on GitHub on 2026-11-04 (read only):
SIMULATION_ENTRYPOINT_GCP_PROJECT_IDispolicyengine-simulation-entryandSIMULATION_ENTRYPOINT_RUNTIME_SERVICE_ACCOUNTissim-entry-beta-runtime@…/sim-entry-prod-runtime@…inbeta/prod. Not verified:SIMULATION_DATABASE_URL_SECRET_NAMEis not set inbetaorprod, and the secrets, the role and its grants don't exist yet; the operator steps below create them. Each missing piece stops the deploy rather than being skipped.What this waits for before merging
In this order:
simulation_runsandreport_run_simulation_runsand reshapes the other four tables, merged and deployed to staging and production, with the simulation service role's grants in its privilege manifest.policyengine_v2_simulation_servicein the Supabase database that holds the Public API's v2 tables (databasepostgresof the staging and of the production v2 project), with its own generated password. The Public API's next deploy grants it the privileges its manifest declares (scripts/reconcile_v2_privileges.py apply).postgresql://policyengine_v2_simulation_service:<password>@<the same pooler host and port as the stage12-*-database-url secrets>/postgres, as a new Secret Manager secret inpolicyengine-simulation-entry:simulation-service-staging-database-urlandsimulation-service-production-database-url.sim-entry-beta-runtime@policyengine-simulation-entry.iam.gserviceaccount.comthe Secret Accessor role on the staging secret, andsim-entry-prod-runtime@…on the production one; give the deploy service account permission to list each secret's versions, as for thestage12-*-database-urlsecrets.SIMULATION_DATABASE_URL_SECRET_NAMEtosimulation-service-staging-database-urlinbetaandsimulation-service-production-database-urlinprod.gcloud secrets versions list <name> --project policyengine-simulation-entryshows an enabled version; after the first deploy,curl <candidate URL>/ready | jq .simulation_databaseshows{"reachable": true, "missing_privileges": []}(the candidate check requires it).Merging before steps 1 and 2 makes every deploy stop: without the variable at the deploy step, and without the role, its grants or the new tables at the candidate check.
main's branch rules require from someone other than the author.The pull request stacked on this one adds the CI check that compares the models with the Public API's tables.
Testing
Rebuilt on
mainat 04bf1fe on 2026-11-04 (this replaces the earlier history). Every commit was written test first: the new or changed tests were run by node ID and seen to fail for the expected reason, then to pass. Before each commit, Ruff (the CI-pinned version), the quality guards with the rules registry, the generated rules document,uv lock --checkfor the persistence library and the entrypoint, and Pyright for both passed; after review, every commit was checked again for all but Pyright.public_api_schemaran aspolicyengine_v2_simulation_serviceagainst a database built with the Public API's v2 migration chain and privilege manifest at the head of its open pull request for these tables: the role lacks no privilege it needs, and creates and reads back a row in each of the six tables. The models also matched those tables exactly (columns, defaults, constraints, indexes and enums).Simulation databaseincluded, had passed.🤖 Generated with Claude Code