Repository navigation
Conversation
|
Adversarial re-gate (dual review, sol + fable) — the port needs a hardening round before the schedule goes live. Blockers, most severe first:
Non-blockers: scope the Modal secrets to the Modal steps; pin |
Addresses the six blockers from Max's adversarial re-gate (sol + fable) on #53; the direct-SQL scope and overall trigger/harvest design were verified sound, so this is the safety layer only. 1. Input injection: the dispatch release_id is bound to an env var (never spliced into the run body) and allow-listed against a strict populace release-id pattern before any use; the sha8 helper reads the id from env instead of interpolating it into generated Python. 2. Duplicate-spawn / transient errors: spawn_or_wait.check consults the recorded Modal call id BEFORE the started marker (the app writes the marker minutes after the spawner writes call_id, so the old order double-spawned live runs). Only terminal Modal errors re-spawn; a transport/indeterminate error leans on the 24h marker-age backstop rather than respawning live/completed work. 3. Stranded / looping releases: the tick drains any release whose artifact is published on the Volume but has no raw/<id>.json — regardless of what `latest` is now — one per tick, oldest-first, so a run that finished after latest advanced is not stranded. An existing reform-validation/<id> branch short-circuits the drain so an open PR is never force-recreated each tick. 4. Mixed-revision checkpoints: spawn_or_wait pins the producer ref on the first spawn and reuses it on every re-spawn, so a resume can't cross a populace bump; backfill.merge additionally refuses partials that span more than one producer revision. 5. Silent scoring-mode default: backfill stamps an explicit obbba_scoring_mode and the ingest resolves+validates it against a closed vocabulary, failing loudly on a missing/unknown mode instead of defaulting to jcx_stacked. 6. Attestation + non-RV invariant: the artifact carries an _attestation block (producer ref/commit, engine pins, h5 sha, Modal app/call/image); the ingest fingerprints every non-RV row before and after the rebuild transaction and aborts if the reform-validation rebuild changed anything outside its own slice. Non-blockers: Modal secrets scoped to the Modal-only steps; `pip install modal` pinned; the "allow Actions to create PRs" repo setting documented. Tested: ingest #5/#6 covered by tests/test_reform_validation_automation.py (29 RV tests) and verified against a copy of the committed DB (non-RV rows byte-identical before/after, 675 results x 5 releases). Full suite 118 pass. The Modal/workflow runtime paths (spawn_or_wait, backfill.merge, the app, the workflow bash) are reasoned-through but not executable in CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MaxGhenis
left a comment
There was a problem hiding this comment.
Re-gate verdict (dual review, sol + fable): not merge-safe — another hardening round needed.
Same two-family review as the 8/16 re-gate. Each finding below was checked against the source at 282335a, not the commit message. B5 is fully fixed and the injection RCE core is genuinely closed; the other five blockers are partial or still open, and three fail concrete offline attacks that both reviewers reproduced. Not merging.
Before it can run at all: producer imports are stale (populace → microcosm)
backfill.py still imports from populace.build.us_runtime.reform_validation import ... (lines 151, 165, 181, 187, 248) and modal_backfill_app.py builds PYTHONPATH from /opt/populace/packages/*/src. The producer was renamed: at microcosm main that module is packages/microcosm-build/src/microcosm/build/us_runtime/reform_validation.py, namespace microcosm.*. spawn always pins producer_ref to current main, so every real spawn hits ModuleNotFoundError: populace on first import — the pipeline can't produce an artifact end-to-end today. Loud, not silent, and it predates this commit, but the workflow comment that the rename "resolve[s] through GitHub's rename redirect" is only true for git clone/API, not the Python namespace. Fixing the imports to microcosm.* also clears the producer-side halves of B4 and B6.
Fixed
B5 — silent scoring-mode default: fixed. backfill.merge stamps an explicit obbba_scoring_mode; the ingest resolves table-first then artifact against a closed frozenset and SystemExits pre-transaction on missing/unknown/empty/null (ingest_reform_validation.py:116-140, 762-769). The five legacy pins hold, l0 override still isolated at 33 rows / 1.729.0. Residual (non-blocking): the mode is asserted by the driver, not derived from the producer payload, so a valid-but-wrong mode is accepted — worth a producer-stamped mode later, but no silent default remains.
B1 RCE core: closed. The two ${{ }} splices are gone — the dispatch input is bound via RELEASE_INPUT env and the sha8 helper reads os.environ["RID_"] from single-quoted Python. Both reviewers pushed a maximally hostile RID through every downstream sink ([ -f … ], grep -F, argv) with no command execution.
Blockers still open
B2 — duplicate-spawn: not fixed. _read() (spawn_or_wait.py:58-62) collapses every exception to None, so a live run whose call_id file exists but hits a transport blip on read is indistinguishable from "no run recorded": check() skips the poll block, the started marker isn't written yet (the app writes it minutes later, after clone + install), and lines 100-101 return SPAWN "no run recorded" — a second 64GB container on the same workdir. Sol reproduced this with a Modal stub (ConnectionError on the call-id read → SPAWN). Separately, a healthy-completion TOCTOU: the drain/grep listing sees no artifact, the app then publishes and returns, and a successful get(timeout=0) returns SPAWN "finished but no artifact" (lines 93-97) → respawn of just-completed work. Fix: fail closed — only FileNotFoundError → None; and re-check the Volume for the artifact before returning SPAWN on a completed call.
B3 — stranded releases: partial. The oldest-first drain of published artifacts and the existing-branch skip both work on the designed path. But list_artifacts() (spawn_or_wait.py:163-174) enumerates only final reform_validation_<id>.json, never pending rv_*/call_id, while the spawn path checks only the current $RID — so a release that becomes non-latest and then dies before publishing is never revisited by the schedule. And the "open PR" guard is a branch-existence check (git ls-remote … heads/reform-validation/$id), not an open-PR check: a pushed branch whose gh pr create failed, a closed-unmerged branch, or a name collision strands the release forever; a transient ls-remote failure reads as branch-absence and re-pushes. Fix: enumerate pending call_ids too, gate on an actual open PR (gh pr list --head), and fail closed on the probe.
B4 — mixed-revision checkpoints: not fixed. The happy path works (pin recorded on first spawn, reused on respawn — verified through the exact A-dies / main-advances / respawn schedule). But the guard is fail-open twice over: (1) the pin read is the same _read() that swallows all errors, so a transient blip at respawn re-resolves fresh main and force-overwrites the pin (spawn_or_wait.py:134-147) — the blocker-2 mechanism reappearing inside the blocker-4 fix; (2) real_commits = {c … not in (None, "unknown")} (backfill.py:273) exempts unstamped partials, so an unknown-stamped-X + stamped-Y workdir merges and is labeled Y — the literal mislabel. Only the populace ref is pinned; backfill.py itself is re-baked from the current scorecard checkout on every modal deploy, so two respawns can run different driver code under identical populace stamps. And a mismatch is rejected but never purged or surfaced, so a mixed workdir wedges into a silent respawn/refuse loop (all ticks green). Fix: fail-closed pin read; refuse on any missing stamp; stamp the driver revision; purge-and-recompute (or fail the tick loudly) on mismatch.
B6 — attestation + non-RV invariant: not fixed. Two parts.
- Attestation is write-only. Nothing reads
_attestation— no consumer inscorecard_db/orregister_release.py, and the automation's own test fixture omits it and ingests green. It also omits the scorecard commit Max named first (the workflow records noGITHUB_SHAor artifact sha256 — thebackfill.py:297comment claiming it does is wrong), andmodal_image_idreadsMODAL_IMAGE_ID, an env var the pinned client never sets (always''). So missing/forged/inconsistent provenance ingests unnoticed. - The invariant fires after commit.
pre_fingerprintis taken, thewith db.conn:block commits, thenpost_fingerprintis compared and raises (ingest_reform_validation.py:858-891). Both reviewers reproduced: a cross-slice write raisesSystemExitbut the rogue row is already committed to the DB file — detection without rollback, contradicting the "leaves the DB exactly as it was" docstring right above it. Fix: consume the attestation (validate the block, rejectunknown/absent, add the scorecard commit + artifact sha to the PR), and computepost_fingerprintinside thewithblock so the abort rolls back.
Partial
B1 — strict validation: partial (major, not a blocker). printf '%s' "$RID" | grep -qxE (line 92) succeeds when any one line matches, so a multiline release_id whose first line is valid passes; echo "release_id=$RID" >> $GITHUB_OUTPUT (line 96) then performs GitHub Actions multiline-output (heredoc) injection, making the downstream release_id arbitrary bytes outside the charset. RCE stays blocked (downstream is env/argv/quoted, and the git/PR sinks re-validate TARGET), so the realized harm is a non-conforming id reaching the Modal spawn, not code execution — but it defeats the strict validation as worded. Fix: reject multiline (assert single line before the regex) and/or re-validate the consumed job output.
Non-blockers
- Actions-create-PR setting: documented.
- Secrets scoping: improved (step-level on the two Modal steps), but both tokens are still present in the
pip install modalstep, which runs third-party sdist build code (grpclib builds from source) — move the emptiness check out of that step. pip install modalpin:modal>=0.73,<1.0is a range, not a pin — it resolves to 0.77.0, the last release of a line Modal abandoned at 1.0 (May 2025). Pin exactly and run one real Modal check, since none of the Modal paths execute in CI.
Tests
The numbers are honest: 122 = 114 base + 5 port + 3 hardening; "118" is CI with 4 Urban tests env-skipped (absolute ~/populace-sotsn-takeup path); "29 RV tests" is 8 new automation + 21 pre-existing ingest. No tests were weakened or deleted. But coverage is thin where it matters: B1–B4 have zero tests, and mutation testing shows the B6 abort and the rebuild's delete-scope can both be removed with all 122 still green (no fixture holds a non-RV pe_results row). backfill.merge's revision refusal is pure stdlib and was unit-testable all along. A clean re-gate should pin at least the spawn decision table (stubbed modal), merge()'s revision refusal, and a seeded non-RV row driven through the fingerprint abort.
Bottom line
B5 and the injection RCE core are real progress, and the drain design is sound. But B2/B4/B6 still fail concrete attacks, B1/B3 are partial, and the producer imports are stale so nothing runs end-to-end yet. Requesting changes rather than rewriting the branch — the fixes above are each a small, local change.
Addresses Max's 8/19 dual re-gate (sol + fable). B5 and the B1 RCE core were accepted; this closes the rest. Critical — producer imports (populace -> microcosm rename): backfill.py imported `microcosm.build.us_runtime.reform_validation` was still `populace.*`, and modal_backfill_app cloned PolicyEngine/populace with PYTHONPATH over /opt/populace/packages/populace-*/src — every spawn would ModuleNotFoundError before producing anything. Now clones microcosm, imports microcosm.*, and uses microcosm-* packages. (Clears the producer-side halves of B4/B6.) B2 duplicate-spawn: `_read` failed OPEN (every error -> None), so a transport blip on the call_id read looked like "no run recorded" and double-spawned a live 64GB container. `_read` now returns None only on FileNotFoundError and propagates otherwise; check() fails closed to WAIT on an unreadable marker, and a *completed* call re-checks the Volume for its artifact before respawning finished work (the healthy-completion TOCTOU). B3 stranded releases: enumerate pending workdir call_ids (spawn now records the release_id, since sha8 is one-way) so a release that became non-latest and died before publishing is revisited; the "already covered" guard is now an actual open-PR check (`gh pr list --head`) that fails closed, not a branch-existence probe; the PR push is `--force-with-lease` so a stale branch can't strand the release. B4 mixed-revision: the pin read fails closed (an unreadable producer_ref aborts rather than re-resolving fresh main); merge() refuses partials with any missing/unknown OR mixed stamp (not just a clean 2-way mix), now stamps and checks the DRIVER (scorecard) revision as well as the producer, and PURGES the workdir on refusal so the next respawn recomputes clean instead of a silent refuse loop. Extracted `_refuse_if_mixed` (pure stdlib) for tests. B6 attestation + invariant: the ingest now CONSUMES the attestation — `_validate_attestation` rejects a missing/`unknown` block for any non-historical release; the workflow records the scorecard commit + artifact sha256 on the PR; the always-empty `modal_image_id` is dropped for real container ids. The non-RV fingerprint check moved INSIDE the transaction so a cross-slice write rolls back instead of committing then raising. B1 (partial): reject a multiline release_id before the `grep -qx` regex, which otherwise passes on a valid first line and injects via the $GITHUB_OUTPUT heredoc. Non-blockers: `modal` pinned exactly (==0.77.0); the Modal-token emptiness check moved out of the `pip install` step (no tokens in scope while third-party sdists build). Tests (B1-B4 previously had none): stubbed-modal spawn decision table (tests/test_reform_validation_spawn.py, 10 cases incl. the fail-closed paths), `_refuse_if_mixed` refusal + purge, attestation rejection, and a seeded non-RV pe_results row driven through the rebuild (pins the delete-scope + fingerprint abort). Full suite 131 pass; real ingest on a copy of the committed DB holds the invariant (675 results x 5 releases). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Round-2 re-gate addressed — pushed Critical — producer imports (populace → microcosm rename)
B2 — duplicate-spawn / TOCTOU (was: not fixed)
B3 — stranded releases (was: partial)
B4 — mixed-revision (was: not fixed)
B6 — attestation + invariant (was: not fixed)
B1 — strict validation (was: partial)
Non-blockers
Tests (B1–B4 previously had none)
Honest caveat unchanged: the Modal-runtime paths ( Requesting another review when you have a sec, @MaxGhenis. |
vahid-ahmadi
left a comment
There was a problem hiding this comment.
Review — reform-validation backfill re-home (post re-gate 2)
Ran the branch locally (131 passed / 4 skipped, ruff clean incl. tools/, CI green) and checked the pieces the two prior gates didn't cover — mainly how this lands against the moving ingest_reform_validation.py.
Verified:
- Pin precedence is right:
_base_engine(ingest_reform_validation.py:173) consults the hardcodedENGINE_VERSIONSfirst, so an automation artifact can never overwrite a historical release's authoritative pin (the l0-refit override subtlety survives viaENGINE_OVERRIDES); artifact-stamped pins only serve genuinely-new releases, and a release with neither dies onSystemExit. - OBBBA mode + attestation are fail-loud:
_obbba_scoring_modevalidates against the closed set instead of defaulting, and_validate_attestationrejects missing/unknownblocks for automation-produced releases while exempting exactly the five historical ids. - Workflow guards: Modal secrets are checked with an actionable error before spawn; harvest/register/ingest run on the 6h tick with the volume artifact removed after landing.
- Trial-merged against current
main(this branch is ~24 commits behind): merges clean — no conflict with the merged #48/#71 state.
One concrete sequencing item — collision with open #65:
I trial-merged this branch against uk/reform-validation-uk (PR #65, rebased onto main yesterday): content conflicts in scorecard_db/ingest_reform_validation.py and scorecard_db/README.md. #65 relocates the release pins into COUNTRIES["US"]["engine_versions"] (keeping an ENGINE_VERSIONS = COUNTRIES["US"][...] back-compat alias) in the same region this PR annotates and extends with _obbba_scoring_mode / VALID_OBBBA_SCORING_MODES / attestation.
The reconciliation is shallow whichever lands second — the alias means every release_id in ENGINE_VERSIONS / .get(release_id) read in this PR keeps working unchanged under #65's shape, so it's a textual conflict, not a semantic one. But whoever merges second should also decide where the automation's country lives: backfill.py and register_release.py are US-specific by construction, so under #65's config they should either pin country="US" explicitly at their ingest(...) call or grow a country argument — silent reliance on the default would be the one way the merge could rot later.
Good to land from my side; just coordinate the order with #65 (same one-line-ish resolution dance #48/#65 already did for GBP).
Re-gate round 3 — big classes cleared; three findings, two of them criticalGood news first: the injection surfaces now pass (env-bound release ids, newline rejection, allow-list re-checks on Volume reads, argv arrays throughout), secrets handling passes (tokens absent during installs, scoped to runtime, never printed), and the durable architecture is right — Three findings:
Also worth folding in: rebase onto current main — this head predates the baseline_key columns (the three-way integration is clean and preserves the current-law/OBBBA stamps, so it's mechanical) and picks up the CI that now runs your suite against a built DB. Spawn decision table 10/10, actionlint and diff-check clean. 🤖 Generated with Claude Code |
…orecard Ports the Modal-backed per-release producer (cal-diag #112, Pavel's review carries over) here per issue #15 call 3, so the reform-validation population stays current without hand-running the producer. tools/reform_validation/: - backfill.py, modal_backfill_app.py, spawn_or_wait.py: the #112 machinery (producer + 64GB Modal app + liveness-checked spawner), copied with the Modal app/volume renamed to scorecard-* and the commit target retargeted. backfill.py now also stamps a structured `engine` block (the release manifest's pe-us/-core versions) into the artifact. - register_release.py (new): lands a harvested artifact under sources/populace-reform-validation/raw/ and registers it in source.json, ordered oldest-first by release timestamp. .github/workflows/reform-validation-backfill.yml: the trigger/commit layer. A tick spawns the Modal sim if the latest release lacks a raw/<id>.json; a later tick harvests the artifact, runs register_release + the ingest to rebuild data/scorecard.db, and opens a PR for review (not auto-merged: a new release brings a new engine pin and shifts the exact-count ingest tests). ingest_reform_validation.py: a new release ingests without a code edit — _base_engine resolves the pin from the artifact's `engine` block when the release isn't in ENGINE_VERSIONS, and OBBBA scoring defaults to jcx_stacked (the current producer's mode). The five backfilled releases are unchanged. Verified end-to-end against a copy of the committed DB: a synthetic 6th release ingests to 916 results, its 36 OBBBA rows attach to the harvest claims (0 fallbacks), all stamped at the artifact's pin. Suite 115 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses the six blockers from Max's adversarial re-gate (sol + fable) on #53; the direct-SQL scope and overall trigger/harvest design were verified sound, so this is the safety layer only. 1. Input injection: the dispatch release_id is bound to an env var (never spliced into the run body) and allow-listed against a strict populace release-id pattern before any use; the sha8 helper reads the id from env instead of interpolating it into generated Python. 2. Duplicate-spawn / transient errors: spawn_or_wait.check consults the recorded Modal call id BEFORE the started marker (the app writes the marker minutes after the spawner writes call_id, so the old order double-spawned live runs). Only terminal Modal errors re-spawn; a transport/indeterminate error leans on the 24h marker-age backstop rather than respawning live/completed work. 3. Stranded / looping releases: the tick drains any release whose artifact is published on the Volume but has no raw/<id>.json — regardless of what `latest` is now — one per tick, oldest-first, so a run that finished after latest advanced is not stranded. An existing reform-validation/<id> branch short-circuits the drain so an open PR is never force-recreated each tick. 4. Mixed-revision checkpoints: spawn_or_wait pins the producer ref on the first spawn and reuses it on every re-spawn, so a resume can't cross a populace bump; backfill.merge additionally refuses partials that span more than one producer revision. 5. Silent scoring-mode default: backfill stamps an explicit obbba_scoring_mode and the ingest resolves+validates it against a closed vocabulary, failing loudly on a missing/unknown mode instead of defaulting to jcx_stacked. 6. Attestation + non-RV invariant: the artifact carries an _attestation block (producer ref/commit, engine pins, h5 sha, Modal app/call/image); the ingest fingerprints every non-RV row before and after the rebuild transaction and aborts if the reform-validation rebuild changed anything outside its own slice. Non-blockers: Modal secrets scoped to the Modal-only steps; `pip install modal` pinned; the "allow Actions to create PRs" repo setting documented. Tested: ingest #5/#6 covered by tests/test_reform_validation_automation.py (29 RV tests) and verified against a copy of the committed DB (non-RV rows byte-identical before/after, 675 results x 5 releases). Full suite 118 pass. The Modal/workflow runtime paths (spawn_or_wait, backfill.merge, the app, the workflow bash) are reasoned-through but not executable in CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses Max's 8/19 dual re-gate (sol + fable). B5 and the B1 RCE core were accepted; this closes the rest. Critical — producer imports (populace -> microcosm rename): backfill.py imported `microcosm.build.us_runtime.reform_validation` was still `populace.*`, and modal_backfill_app cloned PolicyEngine/populace with PYTHONPATH over /opt/populace/packages/populace-*/src — every spawn would ModuleNotFoundError before producing anything. Now clones microcosm, imports microcosm.*, and uses microcosm-* packages. (Clears the producer-side halves of B4/B6.) B2 duplicate-spawn: `_read` failed OPEN (every error -> None), so a transport blip on the call_id read looked like "no run recorded" and double-spawned a live 64GB container. `_read` now returns None only on FileNotFoundError and propagates otherwise; check() fails closed to WAIT on an unreadable marker, and a *completed* call re-checks the Volume for its artifact before respawning finished work (the healthy-completion TOCTOU). B3 stranded releases: enumerate pending workdir call_ids (spawn now records the release_id, since sha8 is one-way) so a release that became non-latest and died before publishing is revisited; the "already covered" guard is now an actual open-PR check (`gh pr list --head`) that fails closed, not a branch-existence probe; the PR push is `--force-with-lease` so a stale branch can't strand the release. B4 mixed-revision: the pin read fails closed (an unreadable producer_ref aborts rather than re-resolving fresh main); merge() refuses partials with any missing/unknown OR mixed stamp (not just a clean 2-way mix), now stamps and checks the DRIVER (scorecard) revision as well as the producer, and PURGES the workdir on refusal so the next respawn recomputes clean instead of a silent refuse loop. Extracted `_refuse_if_mixed` (pure stdlib) for tests. B6 attestation + invariant: the ingest now CONSUMES the attestation — `_validate_attestation` rejects a missing/`unknown` block for any non-historical release; the workflow records the scorecard commit + artifact sha256 on the PR; the always-empty `modal_image_id` is dropped for real container ids. The non-RV fingerprint check moved INSIDE the transaction so a cross-slice write rolls back instead of committing then raising. B1 (partial): reject a multiline release_id before the `grep -qx` regex, which otherwise passes on a valid first line and injects via the $GITHUB_OUTPUT heredoc. Non-blockers: `modal` pinned exactly (==0.77.0); the Modal-token emptiness check moved out of the `pip install` step (no tokens in scope while third-party sdists build). Tests (B1-B4 previously had none): stubbed-modal spawn decision table (tests/test_reform_validation_spawn.py, 10 cases incl. the fail-closed paths), `_refuse_if_mixed` refusal + purge, attestation rejection, and a seeded non-RV pe_results row driven through the rebuild (pins the delete-scope + fingerprint abort). Full suite 131 pass; real ingest on a copy of the committed DB holds the invariant (675 results x 5 releases). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rebased onto current main (picks up #74's DB-leaves-git model + #71's baseline_key columns; clean three-way). Addresses Max's 8/20 re-gate — the injection/secrets/architecture classes passed; three findings remained. Finding 1 (critical — producer couldn't start): the Modal image put the cloned microcosm shards on PYTHONPATH without installing them, so microcosm.fit's eager quantile-forest import failed. Now pip-installs the shards (constraints file keeps the release-exact engines pinned), reads the installed versions back and asserts they equal the manifest, and runs a `backfill.py --plan` smoke before the multi-hour run so a missing dep fails fast. Finding 2 (critical — attestation declarative, not verified): - Ingest now VERIFIES the block: h5_sha256 is 64-hex, commit ids are hex, and both attested engine versions must equal the artifact's own `engine` block — the forged `engine=9.9.9` / `h5_sha256="x"` now fail. - Producer rehashes the ACTUAL H5 (cached or downloaded) and stamps the observed hash, verifying it against the manifest; merge() requires the partials' single surviving revision to EQUAL the attested producer/driver (all-old partials can't be relabeled under a new driver); the workflow cross-checks the harvested artifact's Modal call id against the spawner's recorded id. Finding 3 (high — post-#74 the workflow aborted): it `git add`ed the now- gitignored `data/scorecard.db`. Now commits only the raw artifact + source.json, and verifies by building the whole DB to a throwaway path (`scorecard_db.build_db`) — the DB is never staged. Tests: attestation verification (bad sha / non-hex commit / engine mismatch, plus the clean fixture still ingests). Full RV suite 45 pass; real ingest on a copy of the committed DB holds the non-RV invariant (675 results x 5 releases). Modal-runtime paths (shard install, H5 rehash, call-id cross-check) reasoned + unit-tested where stdlib-reachable; a live dry-run before enabling the schedule remains the last mile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
770926f to
50b4260
Compare
|
Re-gate round 3 addressed — pushed Rebased onto current main as suggested — picks up #74's DB-leaves-git model and #71's Finding 1 (producer couldn't start): the Modal image now pip-installs the checked-out microcosm shards (constraints file keeps the release-exact engines pinned), reads installed versions back and asserts they equal the manifest, and runs a Finding 2 (attestation declarative → verified): ingest now checks format ( Finding 3 (post-#74 workflow): stops staging the now-gitignored Tests: attestation-verification suite (bad sha / non-hex commit / engine mismatch; clean fixture still ingests); 45 RV tests pass, non-RV invariant holds (675×5). Modal-runtime paths (shard install, H5 rehash, call-id cross-check) reasoned + unit-tested where stdlib-reachable — a live dry-run before enabling the schedule remains the last mile. @vahid-ahmadi — noted the #65 collision; it's textual (the |
|
@DTrim99 — answering your sequencing question, and correcting part of what I told you on 08-20. Land #65 first, then rebase this. #65 is approved and mergeable now; this branch is 10 behind main and still waiting on Max's re-review of round 3, so it's going to rebase regardless. Doing it in that order means the conflict is resolved once, by the person holding the smaller diff. I re-ran the trial merge on today's tips to make sure the picture hasn't moved. It hasn't — same two files, The Better still, the thing I was worried about can't happen. Country is settled by which directory the artifact lands in: One thing worth confirming when you rebase, since it's the only part that isn't mechanical: this head predates the changes in that same region, so check that |
|
@DTrim99 — status check. 158 behind That's the collision I flagged on 09-02: #65 landed on 09-06, relocating the release pins into It should still be a textual resolution rather than a semantic one — the alias means every And the Max's round-3 findings have been addressed since 08-21 and this has been waiting on his re-review a while — might be worth a nudge. |
|
@DTrim99 Please rebase and resolve the README and ingest_reform_validation.py conflicts identified September 15. Rerun CI and summarize how the new head addresses my round-three findings, so I can complete the re-review. |
Brings the branch up to date with main (322 commits, incl. #65's country-generalised reform-validation ingest and the spm-20260915 release). Two files conflicted, as flagged on 2026-09-15: scorecard_db/README.md: both sides appended to the reform-validation section. Kept main's "Country dimension (#62)" paragraph and every later population section, with the PR's "Per-release automation" paragraph placed after the country paragraph, before the OBR policy-effects section. scorecard_db/ingest_reform_validation.py: - Kept main's COUNTRIES map and ENGINE_VERSIONS alias; the PR's _obbba_scoring_mode, _validate_attestation and _base_engine now sit after the alias. - _base_engine and _validate_attestation take the country's engine_versions map. The self-describing engine.policyengine_us fallback is US-only, and attestation exempts the country's hand-pinned releases, so main's UK lane (pins injected per country) is unaffected. - _non_rv_fingerprint takes the country's run prefix, registry mark and lane, so the in-transaction non-RV invariant scopes to the slice that the ingest actually replaces. With the US-only constants, every UK ingest would have rolled back. - _obbba_results keeps main's US-only guard and _pe_block. The caller-resolved mode becomes an optional trailing keyword that falls back to the validated pinned mode, so main's positional callers keep working. The OBBBA mode is resolved only for US artifacts. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqCGPgxDukji8Bhs57c5rH
Two gaps remained in re-gate round 3, finding 2 ("attestation is
declarative, not verified"):
- backfill.py stamped the attestation's engine versions from its own
provenance-time manifest fetch. The Modal app's read-back only compared
the installed engines with the app's earlier install-time fetch.
_observed_engine now reads the installed policyengine-us/-core back
from the environment the batches ran in and stamps those values. It
refuses if they differ from the manifest being stamped. orchestrate()
runs the same check before any compute, so a manifest that moved
between the two fetches fails fast.
- The ingest accepted an attestation with no release_id, because the
cross-check passed vacuously when the field was absent. The attested id
is now required and must equal the artifact's release_id.
Tests: _observed_engine accepts matching versions and refuses drift (via
an injected version lookup). An absent or foreign attested release_id is
rejected. The six committed releases are hand-pinned and exempt, so the
built DB is unchanged (content hash 174ab3b2..., the same as main's).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LqCGPgxDukji8Bhs57c5rH
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Org-side re-push of #23 (closed 8/8 as a fork PR — fork PRs don't run CI on this private repo). Now that I have write access, this is the same branch on an org branch so it gets a real CI run; happy to rebase onto latest
mainif the gate flags conflicts. Ports the Modal-backed per-release backfill from calibration-diagnostics #112 into the Scorecard, per issue #15 call 3 (Pavel's review of #112 carries over). Once this lands, cal-diag #112 closes in favor of it.What it does
The reform-validation population currently has to be hand-filled per release. This re-homes the automation so a new certified populace/microcosm release produces its
raw/<id>.jsonand lands indata/scorecard.dbon its own.tools/reform_validation/:backfill.py,modal_backfill_app.py,spawn_or_wait.py— the Intern claim provenance: publications + reforms side tables (~20 MB of inline JSON duplication) #112 machinery verbatim modulo the changes below: the producer (reform_validation_payloaddriven in memory-bounded subprocess batches), the 64GB Modal app that runs it at the release's exact engine pins (the sim no longer fits a GitHub runner), and the liveness-checked spawner that records the Modal call id on the Volume and re-spawns dead runs from checkpointed partials.register_release.py(new) — lands a harvested artifact undersources/populace-reform-validation/raw/and registers it insource.json, ordered oldest-first by release timestamp (the order the ingest globs)..github/workflows/reform-validation-backfill.yml— the trigger/commit layer. Tick N spawns the Modal run if the latest release has noraw/<id>.json; tick N+1 harvests the artifact, runsregister_release+ingest_reform_validationto rebuild the DB slice, and opens a PR.Changes from the cal-diag original
data/scorecard.db. Modal app/volume renamedcd-*→scorecard-*.ENGINE_VERSIONS— which would be every new release the automation targets.backfill.pynow stamps a structuredengineblock (the release manifest's pe-us/-core versions) into the artifact, and_base_enginereads it when the release isn't pinned. OBBBA scoring defaults tojcx_stacked. The five backfilled releases are unchanged — their pins stay authoritative, including the l0-refit override subtlety.Requires
MODAL_TOKEN_ID/MODAL_TOKEN_SECRETrepo secrets. Supersedes #23.