Repository navigation
Conversation
…ns differ The persistence tests gain schema_comparison, the comparison the next commit runs between sim's table models and the tables the Public API's migration chain creates. describe_tables reads, for each table of the public schema, its columns (type, with an enum named as "enum <type>"; whether they allow null; their default), its constraints (primary key, unique, foreign key, check and exclusion) by name with PostgreSQL's own definition, and its other indexes by name with their definition, and the values of each enum its columns use. schema_differences lists every way the models' tables differ from the real ones, in both directions; tables the models leave out are not compared. Both sides are read through the same catalog queries, so a type, default or definition is compared the way PostgreSQL itself writes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ommit in CI A new PR job, Public API schema, checks out PolicyEngine/policyengine-api (public, so without a secret or persisted credentials) at the commit pinned in libs/policyengine-simulation-persistence/public-api-commit, builds its tables with its own v2 migration chain into the job's PostgreSQL and grants the privileges its manifest declares (public-api-schema-reference.sh), and runs the persistence tests marked public_api_schema, which fail rather than skip there. test_public_api_schema builds the models' tables from service_metadata in a disposable database and compares both through the catalog: every column's type, nullability and default, every constraint and index by name and definition, and every enum's values. The role-write tests run in the same job. Last, the job requires the pinned commit to be on the Public API's master branch, so the models only describe a merged chain. CI complete needs the job. A sim pull request that changes the models moves the pin with them. The new Public API schema drift workflow runs the same comparison every Monday and on demand against the head of the Public API's master, so a merged change there to a modelled table shows up without waiting for a sim change. The new rule sim.persistence.models-match-public-api records this, with a proof whose bad models copy simulation_runs.status as a varchar and leave out modal_call_id. Every script the jobs call is tested with fake commands. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
service_metadata keys its tables by schema and name since the models name the public schema, while the catalog descriptions are keyed by table name alone, so the comparison described no table of the models. It now passes the tables' names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The rules registry ties sim.persistence.models-match-public-api's proof to the jobs in its [[rule.ci]] entries, the Public API schema job and the drift run, but both select -m public_api_schema and the proof had no marker, so neither collected it. It is marked now, and a test keeps it so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… documentation gaps Both jobs run the Public API's code, from a pinned and perhaps unmerged commit, after checking out sim, whose checkout kept the job's token; now neither checkout keeps it. The tests now also check that the reference script installs the Public API's lock on Python 3.12, that the weekly run builds and installs as the PR job does, and that an index only the models have is reported. The persistence readme says what each marked test needs and what to pin after a squash or rebase merge, the weekly run's comment and readme say what else can fail it, the database-access skill names the one shell script that reaches a database, and AGENTS.md lists the comparison among PR CI's checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e pin to the Public API's latest head Both schema jobs select tests by the public_api_schema marker, and the rules guard trusts their pytest command without reading markers, so a test that lost the marker would leave the jobs while every job stayed green. A test now checks that each test the rule names, its proof included, carries the marker and not postgres. The comparison test is no longer marked postgres: the Simulation database job selects that marker but has no Public API database, so the test could only skip there. The marker's descriptions say what it now means. The pin moves to the Public API branch's new head, e7c362b16, whose two newer revisions change result-table keys and grant SELECT on dynamics; the models and the service role's required privileges still match it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The marker test covered one rule and one marker pair. It now reads, for every pytest rule of the persistence library, the markers its CI jobs select, and requires each test the rule names, its proof included, to carry them, to carry no marker that would send a schema test to the Simulation database job, and to carry no skip, skipif or xfail mark, which would let it pass its job unrun. The marker descriptions, the Simulation database job's comment and its test's docstring say which job runs which tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… their own table The Public API's pull request now ends at c9c6eb6d3, whose revision dce0caa329cc moves four report_runs columns to report_run_comparisons; the models below follow it, and the comparison passes there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The marker check skipped a rule's CI entry whose invocation had no -m, so a second job running the rule's tests unfiltered, where they skip without a database, went unchecked while the test passed. Each entry must now select exactly one marker, read with shell quoting, and an invocation without one, or with a marker expression, fails the test. The module docstring and the Simulation database job's comment say which job runs which tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Public API's pull request for these tables was moved onto its master (528cef1f4), so its old head, c9c6eb6d3, can never reach master and the job's last step would never pass. The pin moves to the new head, d674032a2, where the tables the simulation service writes, their keys and its role's grants are as before: the comparison and the role-write tests pass against a database built there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been 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 #995
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/b2-public-api-schema-check.added.md.Summary
sim's CI now fails when its models of the Public API's tables (from the pull request below this one) differ from the tables the Public API's own v2 migration chain creates.
pr.yml): checks outPolicyEngine/policyengine-api, which is public, at the commit pinned inlibs/policyengine-simulation-persistence/public-api-commit, without persisting any credential (neither checkout keeps the job's token, since the job runs the Public API's code).public-api-schema-reference.shbuilds the Public API's tables with its chain into the job's PostgreSQL 17 and grants the privileges its manifest declares. The job then runs the persistence tests markedpublic_api_schema, which fail rather than skip there:service_metadatain a disposable database and reads both through PostgreSQL's catalog (tests/schema_comparison.py): every column's type, nullability and default, every primary key, unique, foreign key, check and exclusion constraint and every other index by name and PostgreSQL's own definition, and every enum's values, in both directions;master, so the models only ever describe a chain the Public API has merged.CI completeneeds the job.Public API schema driftworkflow runs the same tests every Monday and on demand against the head of the Public API'smaster, so a merged change there shows up without waiting for a sim change.sim.persistence.models-match-public-api, with a proof whose bad models copysimulation_runs.statusas a varchar and leave outmodal_call_id. A test keeps every test the rule names in the jobs that run it. Every script the jobs call is tested with fake commands.Environment variables
None is a secret, and the jobs read no secret.
SIMULATION_TEST_DATABASE_URL,PUBLIC_API_SCHEMA_DATABASE_URL,SIMULATION_REQUIRE_DATABASE_TESTS,SIMULATION_REQUIRE_PUBLIC_API_SCHEMApostgres:17serviceenv:ofpublic-api-schemaand ofpublic-api-schema-driftcommit(step output)refpublic-api-schema-commit.sh, from the pin fileGITHUB_OUTPUTPUBLIC_API_SCHEMA_COMMITpublic-api-schema-reference.shGITHUB_ENVV2_MIGRATION_DATABASE_URL,V2_ALEMBIC_DISPOSABLE_TEST,DEPLOYMENT_ENVIRONMENT,UV_PYTHONpublic-api-schema-reference.shfromPUBLIC_API_SCHEMA_DATABASE_URLand literals (1,ci,3.12)What this waits for before merging
master. The pin is that pull request's head,d674032a2, which sits on the Public API'smasterat528cef1f4. If the pull request is moved again, or lands by squash or rebase, the pin moves with it.Testing
Built on the pull request below (its head 21760a9). The pin moved from
c9c6eb6d3tod674032a2when the Public API's pull request was moved onto itsmaster; the comparison and the role-write tests pass against a database built atd674032a2. 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 Pyright passed. Against a PostgreSQL 17 database built bypublic-api-schema-reference.shfrom the Public API at the pinned commit: the comparison found no difference, a deliberate model difference (simulation_runs.output_sha256modelled asNOT NULL) made it fail, and the role-write tests passed. During review, the default-branch check was tried against scratch clones of the Public API: a merged commit passes, and the open pull request's head fails with the expected message. Only the tests this pull request adds or changes were run locally; this push's GitHub Actions runs report the rest, and this job fails at its last step until the Public API's pull request merges.🤖 Generated with Claude Code