Re-home the reform-validation backfill from cal-diag #112 (org-side re-push of #23)

[DTrim99]

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 main if 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>.json and lands in data/scorecard.db on its own.

tools/reform_validation/:

• backfill.py, modal_backfill_app.py, spawn_or_wait.py — the #112 machinery verbatim modulo the changes below: the producer (reform_validation_payload driven 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 under sources/populace-reform-validation/raw/ and registers it in source.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 no raw/<id>.json; tick N+1 harvests the artifact, runs register_release + ingest_reform_validation to rebuild the DB slice, and opens a PR.

Changes from the cal-diag original

• Commit target: cal-diag committed a dashboard override JSON + regenerated a TS index; here the artifact becomes a raw source and the ingest rebuilds data/scorecard.db. Modal app/volume renamed cd-* → scorecard-*.
• New releases ingest without a code edit. The old ingest hard-failed on any release not in the hardcoded ENGINE_VERSIONS — which would be every new release the automation targets. backfill.py now stamps a structured engine block (the release manifest's pe-us/-core versions) into the artifact, and _base_engine reads it when the release isn't pinned. OBBBA scoring defaults to jcx_stacked. The five backfilled releases are unchanged — their pins stay authoritative, including the l0-refit override subtlety.
• PR for review, not auto-merge (the one deliberate deviation from #112). A new release brings a new engine pin and shifts the exact-count ingest tests, so a human confirms the DB delta before merge.

Requires MODAL_TOKEN_ID / MODAL_TOKEN_SECRET repo secrets. Supersedes #23.

[MaxGhenis]

Adversarial re-gate (dual review, sol + fable) — the port needs a hardening round before the schedule goes live. Blockers, most severe first:

1. Credential-context input injection: release_id from workflow_dispatch is substituted directly into shell (workflow lines ~68-75) and interpolated into generated Python (~116) while the job holds Modal + GH tokens. Validate against a strict release-id pattern and pass via argv/env, never generated source.
2. Duplicate-spawn window: spawn_or_wait.py returns SPAWN when started is absent without first consulting the recorded call id — the remote app only writes started after clone/install, so sequential ticks can double-spawn. Also any polling exception ⇒ treated as dead run ⇒ respawn, and any modal volume get failure ⇒ treated as artifact absence; transient transport failures respawn live/completed work.
3. Stranded/looping releases: ticks only consider the current latest.json — a release completed after latest advances is never harvested; and while an automated PR is open, every tick recreates and force-pushes its branch.
4. Mixed-revision checkpoints: retries resolve a fresh populace main SHA but the Modal workdir is keyed by release only and partials are accepted by filename — a retry spanning a populace update can merge two producer revisions while labeling one.
5. Silent vocabulary default: unknown releases fall through to jcx_stacked scoring-mode; the artifact should carry a closed, validated mode field that fails loudly.
6. Generation-in-CI norm: CI deploys producer code, Modal generates data, and CI commits an opaque SQLite rebuild with only a some-rows-exist postcondition. Wanted: an attestation binding scorecard commit + producer ref + artifacts + Modal image, and a logical before/after assertion that non-RV tables are unchanged.

Non-blockers: scope the Modal secrets to the Modal steps; pin pip install modal; document that repo settings must allow Actions-created PRs. Notes: MODAL_TOKEN_ID/MODAL_TOKEN_SECRET are now set on this repo, and register/ingest ordering keys were verified consistent. Happy to pair on the fixes or take a round myself if you'd rather — the direct-SQL scope and the overall trigger/harvest design were verified sound.

[MaxGhenis]

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 in scorecard_db/ or register_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 no GITHUB_SHA or artifact sha256 — the backfill.py:297 comment claiming it does is wrong), and modal_image_id reads MODAL_IMAGE_ID, an env var the pinned client never sets (always ''). So missing/forged/inconsistent provenance ingests unnoticed.
• The invariant fires after commit. pre_fingerprint is taken, the with db.conn: block commits, then post_fingerprint is compared and raises (ingest_reform_validation.py:858-891). Both reviewers reproduced: a cross-slice write raises SystemExit but 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, reject unknown/absent, add the scorecard commit + artifact sha to the PR), and compute post_fingerprint inside the with block 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 modal step, which runs third-party sdist build code (grpclib builds from source) — move the emptiness check out of that step.
• pip install modal pin: modal>=0.73,<1.0 is 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.

[DTrim99]

Round-2 re-gate addressed — pushed 770926f, CI green (pytest·ruff + typecheck·build). Mapping each finding to its fix:

Critical — producer imports (populace → microcosm rename)

backfill.py now imports microcosm.build.us_runtime.reform_validation and modal_backfill_app.py clones PolicyEngine/microcosm with PYTHONPATH over /opt/microcosm/packages/microcosm-*/src (package names confirmed against microcosm main). End-to-end ModuleNotFoundError gone.

B2 — duplicate-spawn / TOCTOU (was: not fixed)

• _read fails closed: returns None only on FileNotFoundError, propagates every other error. check() returns WAIT on an unreadable call_id/started marker instead of "no run recorded" → SPAWN.
• On a completed call, re-check the Volume for the artifact before returning SPAWN (_artifact_present) — the healthy-completion TOCTOU respawn is closed.

B3 — stranded releases (was: partial)

• spawn now records rv_<sha8>/release_id; new list_pending() enumerates pending workdirs, and the workflow revisits the current release plus any stranded pending run (not just $RID).
• The "already covered" guard is a real open-PR check (gh pr list --head … --state open) that fails closed, replacing the branch-existence probe; the PR push is --force-with-lease.

B4 — mixed-revision (was: not fixed)

• Pin read fails closed (unreadable producer_ref aborts rather than re-resolving fresh main).
• merge() refuses on any missing/unknown OR mixed stamp, now stamps + checks the driver (scorecard) revision alongside the producer, and purges the workdir on refusal so the next respawn recomputes clean (no silent refuse loop). Refusal extracted to _refuse_if_mixed (pure stdlib) and unit-tested.

B6 — attestation + invariant (was: not fixed)

• The ingest now consumes the attestation: _validate_attestation rejects a missing/unknown block for any non-historical release.
• The non-RV fingerprint check moved inside the with db.conn transaction, so a cross-slice write rolls back instead of committing-then-raising.
• The "Open PR" step records the scorecard commit + artifact sha256; the always-empty modal_image_id is dropped for real container ids (modal_call_id/modal_task_id).

B1 — strict validation (was: partial)

resolve rejects a multiline release_id (wc -l guard) before the grep -qx regex, closing the $GITHUB_OUTPUT heredoc-injection path.

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). Actions-create-PR setting stays documented.

Tests (B1–B4 previously had none)

• tests/test_reform_validation_spawn.py — stubbed-modal spawn decision table, 10 cases including the fail-closed call_id-unreadable and completion-TOCTOU paths.
• _refuse_if_mixed refusal + workdir purge; attestation rejection (missing + unknown); a seeded non-RV pe_results row driven through the rebuild (pins the delete-scope and the fingerprint abort).
• Full suite 131 pass; real ingest on a copy of the committed DB holds the invariant (675 results × 5 releases).

Honest caveat unchanged: the Modal-runtime paths (spawn_or_wait, merge, the app, the workflow bash) are reasoned + unit-tested where stdlib-reachable, but don't execute in CI — a live dry-run before enabling the schedule is still worth doing, happy to pair on it.

Requesting another review when you have a sec, @MaxGhenis.

[vahid-ahmadi]

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 hardcoded ENGINE_VERSIONS first, so an automation artifact can never overwrite a historical release's authoritative pin (the l0-refit override subtlety survives via ENGINE_OVERRIDES); artifact-stamped pins only serve genuinely-new releases, and a release with neither dies on SystemExit.
• OBBBA mode + attestation are fail-loud: _obbba_scoring_mode validates against the closed set instead of defaulting, and _validate_attestation rejects missing/unknown blocks 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).

[MaxGhenis]

Re-gate round 3 — big classes cleared; three findings, two of them critical

Good 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 — register_release.py's committed raw artifact is exactly what the new deterministic builder ingests, so backfilled results survive DB rebuilds. That last point matters because the ground moved under this PR: since #74, data/scorecard.db is no longer committed at all (CI builds it from scratch each run).

Three findings:

1. Critical — the renamed producer still can't start. The populace → microcosm namespace fixes are correct, but the Modal image installs scikit-learn while current microcosm.fit declares and eagerly imports quantile-forest>=1.4.2 (packages/microcosm-fit/pyproject.toml / src/microcosm/fit/qrf.py) — and the image adds the cloned shards to PYTHONPATH without installing their declared dependencies. A Modal-equivalent dependency probe fails with ModuleNotFoundError: quantile_forest; adding it lets the real 30-batch plan load. Suggest installing the checked-out shards properly (so their dependency declarations resolve) and adding a real backfill.py --plan smoke test to CI so this class can't recur silently.

2. Critical — attestation is declarative, not verified. The ingest checks only that producer_commit, driver_commit, and h5_sha256 are non-empty strings; a probe with a wrong bundle, conflicting engines, junk commits, and h5_sha256="x" was accepted and produced engine=9.9.9 in pe_results. On the producer side: the floating manifest is fetched twice (install-time and provenance-time) with installed versions never read back; a cached H5 is reused without rehashing yet stamped with the current manifest's expected hash; the partial-revision check rejects mixed revisions but doesn't require the singleton revision to equal the one placed in the final attestation (all-old partials can be relabeled under a new driver); and the harvested artifact's Modal call id is never compared to the spawner's recorded call id. The fix shape: verify each attested field against an independent observation (rehash the H5, read back installed versions, cross-check call ids), and make the ingest validate format and cross-consistency, not presence.

3. High — post-The database leaves git: derived artifact, built in CI, published to Supabase storage #74 the workflow aborts before opening its PR. It still ingests directly into data/scorecard.db, row-count-checks it, and git add data/scorecard.db — which now fails under set -e because the path is gitignored and absent, so no commit or PR is ever created. The post-The database leaves git: derived artifact, built in CI, published to Supabase storage #74 shape: commit the raw artifact + source.json (your durable outputs), run PYTHONPATH=. python -m scorecard_db.build_db for verification, and let the no-drift gates prove the ingest reproduces — the DB itself is never staged.

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

[DTrim99]

Re-gate round 3 addressed — pushed 50b4260, CI green (now against a built DB per #74).

Rebased onto current main as suggested — picks up #74's DB-leaves-git model and #71's baseline_key columns; clean three-way, baseline_key/OBBBA stamps preserved.

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 backfill.py --plan smoke before the multi-hour run so a missing dep (quantile-forest) fails fast.

Finding 2 (attestation declarative → verified): ingest now checks format (h5_sha256 64-hex, commit ids hex) and cross-consistency — both attested engine versions must equal the artifact's engine block, so engine=9.9.9 / h5_sha256="x" are rejected. Producer-side: rehash the actual H5 (cached or downloaded) and stamp the observed hash; merge() requires the partials' single revision to equal the attested producer/driver (no relabeling all-old partials under a new driver); the workflow cross-checks the harvested artifact's Modal call id against the spawner's recorded id.

Finding 3 (post-#74 workflow): stops staging the now-gitignored data/scorecard.db — commits only the raw artifact + source.json, and verifies by building the whole DB to a throwaway path (scorecard_db.build_db).

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 ENGINE_VERSIONS alias keeps reads working), happy to land in whatever order you prefer and pin the automation to country="US" explicitly at merge. @MaxGhenis — re-review when you have a sec.