Skip to content

Report only policyengine-us's own commit as build metadata git_sha - #10014

Merged
MaxGhenis merged 2 commits into
mainfrom
fix-build-metadata-git-sha
Oct 9, 2026
Merged

MaxGhenis merged 2 commits into
mainfrom
fix-build-metadata-git-sha

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #10013

Ports PolicyEngine/policyengine-uk#2192 (issue PolicyEngine/policyengine-uk#2191), which fixed the same bug in policyengine-uk.

What was wrong

build_metadata._get_git_sha() walked from the package directory up through every parent and returned git rev-parse HEAD for the first directory with a .git entry. When policyengine-us is installed into a virtualenv inside another checkout, as in a data build (.venv/lib/python3.x/site-packages/policyengine_us under a repository such as PolicyEngine/microcosm), that directory is the outer repository. get_runtime_metadata()["git_sha"], and get_data_build_metadata(), which returns the same dict, then name the outer repository's commit.

An empty or unusable .git directory caused the same failure. It passes .exists(), and git's own discovery then continues to the enclosing repository.

What it does now

_get_git_sha() returns a sha from one of two sources and otherwise returns None:

  1. policyengine-us's own checkout. The package's parent directory must hold .git, its pyproject.toml must name policyengine-us, and git rev-parse --show-toplevel must resolve to that same directory. HEAD of that repository is returned. This covers editable and development installs, including git worktrees, where .git is a file. Behaviour there is unchanged.
  2. The installer's PEP 610 record. For a git install of this copy, the vcs_info.commit_id is read from direct_url.json in the dist-info directory next to the package. A record belonging to another install elsewhere on sys.path is ignored. policyengine-core already gets its own git_sha from this file (policyengine_core/build_metadata.py::_get_direct_url_git_sha).

Git runs with the repository-redirecting variables from git rev-parse --local-env-vars removed (GIT_DIR, GIT_WORK_TREE, ...). Without that, a caller's environment, such as a git hook, could substitute another repository's HEAD. The returned value must be a 40- or 64-character hex sha. Any failure gives None rather than an exception. That covers a missing git executable and a pyproject.toml that tomllib cannot read: non-UTF-8 bytes raise UnicodeDecodeError and very deep nesting raises RecursionError. The whole lookup is also guarded, because policyengine.py calls get_data_build_metadata() unguarded while constructing the US model (tax_benefit_models/us/model.py:129-136).

Wheel and sdist installs return None, as before.

Invariants

  • For every package location, git_sha is either None or the commit of policyengine-us itself. That means HEAD of a repository whose top level is the package's parent and whose pyproject names policyengine-us, or the installer-recorded git commit for that installed copy.
  • A git repository that only contains the install is never consulted, however deeply the package sits inside it.
  • A policyengine-us checkout nested inside another repository reports its own HEAD, not the outer one.
  • The installer-record path never invents a value: it returns commit_id only when vcs == "git" and the id is a hex sha, else None.
  • The lookup never raises.

Tests

policyengine_us/tests/test_build_metadata.py adds:

  • Example tests:

    • a microcosm-style layout (.venv/lib/python3.13/site-packages inside a data repository) returns None;
    • a non-editable install in a .venv inside a policyengine-us checkout returns None;
    • a repository that vendors the package at its root (pip install --target .) returns None;
    • an empty .git directory and a malformed pyproject return None;
    • a pyproject with invalid UTF-8, UTF-16 with a BOM, or 100,000-deep nesting returns None without raising, and so does an unexpected exception inside the lookup;
    • an own checkout, a git worktree, and an own checkout nested in another repo return their own HEAD;
    • GIT_DIR/GIT_WORK_TREE pointing elsewhere is ignored;
    • an unborn HEAD and a missing git return None;
    • this repository's checkout returns its HEAD (skipped when not run from a git checkout);
    • a PEP 610 git record returns its commit, including inside an enclosing repo;
    • nine non-git or malformed records return None;
    • another copy's record is ignored, and two dist-info directories (a stale copy beside the current one) give None rather than either copy's commit;
    • get_runtime_metadata()["git_sha"] for the running install is None or a full commit id.
  • Hypothesis properties:

    • over directory layouts: 0–4 intermediate segments (such as .venv, site-packages, dist-packages or random names) combined with four repository identities;
    • over arbitrary JSON direct_url.json payloads;
    • over arbitrary pyproject.toml bytes.

    Together they encode the invariants above.

Results

Run locally on macOS with git 2.55 and the worktree's uv sync --extra dev environment (Python 3.14).

  • uv run ruff format and uv run ruff check on the changed files pass.

  • uv run pytest policyengine_us/tests/test_build_metadata.py -rs: 38 passed, 1 skipped. The skip is the policyengine_bundles contract test, because that package is not installed locally. CI runs it, and it passes.

  • Mutation checks. Each mutant below is killed by the suite:

    • (A) the original except (OSError, TOMLDecodeError) with no outer guard;
    • (B) taking the first dist-info instead of requiring exactly one;
    • (C) the original parent walk, which fails 13 tests.
  • End to end. Real uv pip installs into virtualenvs inside a scratch git repository laid out like microcosm. The probe loads the installed build_metadata.py, with policyengine_core stubbed because the sha lookup does not use it:

    install git_sha
    main (99b9b05), non-editable the scratch repo's HEAD (bug reproduced)
    this branch, non-editable null
    this branch via uv pip install "policyengine-us @ git+file://…@b96f342f" b96f342f… (from uv's direct_url.json)
    this branch, editable worktree the worktree's HEAD, b96f342f…

Independent review. The first round (Opus 5.5) found that a non-UTF-8 or deeply nested pyproject.toml made the lookup raise, which broke the never-raises invariant. It also found that the duplicate dist-info rule was untested. I reproduced both errors on e84a725; b96f342 fixes them and adds the tests listed above. The second round approved b96f342 after executing the suite (38 passed). It also ran 10 mutants, all killed; 13 layout probes, including a submodule, a --separate-git-dir clone, a shallow clone, a sha256 repository and broken .git symlinks; and a fuzz of _declares_package, which never raised.

Downstream compatibility

From reading the code at policyengine.py main (07bae750):

  • The fix changes no certification or validation outcome. policyengine.py reads get_data_build_metadata() in one place (tax_benefit_models/us/model.py:129-136, used in common/model_version.py:144-148), and it uses only data_build_fingerprint. The fingerprint hashes surface files and does not include git_sha, so this PR cannot move it.
  • _validate_runtime_policyengine_us_match never reads this module's git_sha. It compares a long-term sidecar's commit with the runtime commit that it reads itself from importlib.metadata.distribution("policyengine-us") and that distribution's direct_url.json (tax_benefit_models/us/datasets.py:678-705, :741-774). For git installs, it and this module now read the same PEP 610 commit_id.
  • built_with_model_package.git_sha is only passed through, into built_with_model_git_sha and the TRO's pe:builtWithModelGitSha. Every schema field holding it is Optional[str], and the writers skip None (provenance/certification.py:672).
  • The key set is unchanged: name, version, git_sha, data_build_fingerprint, core. That matters because the policyengine-bundles contract that CI pins uses extra="forbid" and types git_sha as str | None.
  • Effect on microcosm. microcosm installs policyengine-us from the PyPI registry (uv.lock). Its US release manifests record only {name, version} for built_with_model_package (tools/build_us_fiscal_refresh_release.py:10649, tools/build_us_acs_local_release.py:3419), and nothing in it imports this module. Installed in its .venv, the current wheel's _get_git_sha() returns microcosm's HEAD, but no manifest records that value. Registry installs will report git_sha: null after this fix: honest, but empty.

Published US manifests checked (read-only; nothing was changed)

One US manifest carries the wrong sha. It is on the Hugging Face policyengine/policyengine-us-data branch mp-ecps-2024-2cdd45d-20260605, uploaded 2026-06-05 (HF commit a091769a): releases/mp-ecps-2024-2cdd45d-20260605/release_manifest.json (sha256 4961f6ee…).

  • It records built_with_model_package = {policyengine-us 1.715.3, git_sha f7458313c86fa580fb1e43a2f18252d67cf76e4a}.
  • That sha equals the manifest's own build.metadata.data_package_git_sha. It is a policyengine-us-data commit ("Update publication candidate", 2026-05-30), and GitHub returns 422 for it in this repository.
  • It was copied downstream into policyengine.py 4.14.1 (data/release_manifests/us.json and us.trace.tro.jsonld; on PyPI since 2026-06-05, not yanked) and into the archived policyengine-bundles (bundles/4.14.0/countries/us.json:17).
  • policyengine.py 4.14.2 reverted it the next day (b98ebc8b).
  • It is provenance only: nothing compares it.

Every other US manifest checked has no wrong sha:

  • The release manifests on HF policyengine-us-data main have git_sha null or absent.
  • All 31 policyengine/populace-us release and parent manifests, the line bundled in current policyengine.py, have no git_sha.
  • The public long-term sidecars record no commit.

Not checked: the private repos (policyengine-us-data-private, populace-us-private), which were not readable with the available token.

Same pattern elsewhere. The archived policyengine-us-data has its own copy of the parent walk in utils/policyengine.py::_find_git_root, which feeds long-term sidecar commit_id and git_dirty. The repository is archived, so this is noted, not fixed. The policyengine-uk counterpart is PolicyEngine/policyengine-uk#2192. The never-raises defect above also applies there and is noted on that PR.

axiom: n/a: build provenance metadata only, no policy change

🤖 Generated with Claude Code

MaxGhenis and others added 2 commits October 8, 2026 10:56
_get_git_sha walked from the package directory up through every parent
and returned HEAD of the first directory with a .git entry. Installed
into a virtualenv inside another checkout (a data repository such as
microcosm), that was the outer repository's commit. An empty .git
directory led to the same answer through git's own upward discovery.

The sha now comes only from policyengine-us's own checkout (package
parent holds .git, its pyproject names policyengine-us, and git's
top level is that directory) or from the installer's PEP 610
direct_url.json git record for this copy; otherwise None. Git runs
without the repository-redirecting environment variables, and the
lookup never raises. Ports PolicyEngine/policyengine-uk#2192.

Fixes #10013

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review finding: tomllib raises UnicodeDecodeError for non-UTF-8 bytes
(e.g. a UTF-16 pyproject written by Windows PowerShell) and
RecursionError for very deep nesting, and _declares_package caught
neither, so get_runtime_metadata() could raise. Catch ValueError and
RecursionError there, and guard _get_git_sha as a whole so provenance
lookup can never stop the model loading.

Tests: unparseable pyproject examples, a Hypothesis property over
arbitrary pyproject bytes, the outer guard, and duplicate dist-info
directories (which must give None, not either copy's commit).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Merge audit for head b96f342f3386a0b837590cd2b9e87f0789489ca5:

  • CI: gh pr checks exits 0, with 26/26 passing on this head (run "Pull request", conclusion success). mergeable: MERGEABLE, not a draft, no CHANGES_REQUESTED review.
  • Independent review, round 1 (Opus 5.5, Subfleet 20261008-105931-pe10014-review, on e84a725): REQUEST_CHANGES.
    • Finding: a non-UTF-8 or deeply nested pyproject.toml made the lookup raise.
    • Finding: the duplicate dist-info rule was untested.
    • Both errors were reproduced, then fixed and tested in b96f342.
  • Independent review, round 2 (Opus 5.5, Subfleet 20261008-175717-pe10014-review-r2, on b96f342): APPROVE.
    • It executed the test file: 38 passed, 1 skipped.
    • All 10 mutants it ran were killed.
    • All 13 install-layout probes and a _declares_package fuzz behaved as expected.
    • It re-checked the five stated invariants.
  • Why admin merge: the only remaining block is the required approving review, which no one else can give on this PR. It is merged with --admin --match-head-commit pinned to the reviewed head.

@MaxGhenis
MaxGhenis merged commit ddce93a into main Oct 9, 2026
26 checks passed
@MaxGhenis
MaxGhenis deleted the fix-build-metadata-git-sha branch October 9, 2026 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build_metadata git_sha reports the enclosing repository's commit when installed inside another checkout

1 participant