feat(environment): built-in manifest registry shipped in the wheel - #961
Merged
Conversation
BENCHFLOW_ENV_REGISTRY required a local directory of name@version.toml pins, and the documented recipe assumed a repo checkout — a bare pip install had to hand-download env0@prod.toml from raw.githubusercontent and reconstruct the layout by guesswork. Move the canonical pins from benchmarks/_environments/ into the package (src/benchflow/environment/_registry/) so hatchling ships them in the wheel as package data, and resolve them as the built-in default registry when BENCHFLOW_ENV_REGISTRY is unset. The env var, when set, wins entirely (no fallback into the built-ins); an explicit registry= argument still beats both. Lookup goes through importlib.resources with a one-shot temp-dir materialization so zipped installs work too. - benchmarks/_environments/ keeps only a README pointing at the new single source of truth (no divergent copies) - content-addressed sha256 logging of resolved manifests unchanged - unknown names now error listing the available specs - docs: environment-plane Registry section, cli.md flag row, pin headers, and the _environments README updated; CLI help mentions the built-in default Acceptance: fresh venv + pip install of the built wheel resolves --environment-manifest env0@prod (and env0@outage) with no env vars set, logging the sha256 provenance from site-packages.
- Drop the zipped-install mkdtemp materialization branch: unreachable (the repo's package-data idiom is Path(__file__)-relative everywhere — compose files, default rubric, demo task — so zip imports are unsupported package-wide), untested, and it carried latent lifecycle defects (uncleaned mkdtemp, unserialized concurrent first call). _builtin_registry_dir is now a Path(__file__)-relative lookup like its siblings; the importlib.resources packaging test still pins that the pins ship as package data. - Not-a-directory errors now name the source of the bad path ($BENCHFLOW_ENV_REGISTRY vs the registry= argument), with tests for both — the path had no test before. - Pin the empty-env-var semantic: BENCHFLOW_ENV_REGISTRY="" counts as unset (docstring line + test). - Add the missing resolution-order test: an explicit registry= argument beats a SET env var. - Fix an accidental capture group in the no-builtin-fallback test regex.
xdotli
temporarily deployed
to
pypi-internal-preview
August 9, 2026 17:45 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5150749246
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+97
to
+98
| """The acceptance contract: a bare pip install resolves ``env0@prod`` with | ||
| no ``$BENCHFLOW_ENV_REGISTRY`` set — the pins ship inside the package.""" |
There was a problem hiding this comment.
This acceptance test specifically guards the commit's fix for bare-install registry resolution, but its docstring does not identify the PR or commit; several adjacent regression tests added for the same behavior also omit that traceability. Name the guarded PR/commit in these docstrings as required by the repository convention.
AGENTS.md reference: AGENTS.md:L16-L17
Useful? React with 👍 / 👎.
Merged
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
1 participant
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.
A pip-installed user could not use
--environment-manifest name@versionat all:BENCHFLOW_ENV_REGISTRYhad to point at a local directory of manifest files, and the documented recipe assumed a repo checkout. A fresh-user dogfood had to hand-downloadenv0@prod.tomlfrom raw.githubusercontent.com and reconstruct the layout by guesswork.What changed
benchmarks/_environments/*.toml→src/benchflow/environment/_registry/(git mv, byte-faithful apart from header wording;benchmarks/_environments/keeps a pointer README — one source of truth, no divergent copies). Verified through the actual artifacts: sdist and sdist-built wheel both carry the TOMLs.registry=argument →$BENCHFLOW_ENV_REGISTRY(wins entirely when set — no silent fallback; a missing/non-directory path errors loudly and the message names its source, env var vs argument; empty value counts as unset, documented and tested) → built-in registry. sha256 content-addressed resolution logging unchanged.env0@prodresolves from site-packages (8 services, sha256 logged),env0@outage(6 services, perturbation intact), unknown names error listing available specs.Review
Structural review before opening (REQUEST-CHANGES → fixed): the initial zipped-install temp-dir materialization branch was deleted on four grounds — unreachable (the repo's universal package-data idiom is
Path(__file__)-relative, so zip imports are already unsupported package-wide), untested, leaky/racy lifecycle, and a hand-rolledimportlib.resources.as_file. The shipped lookup is three lines matching repo precedent; packaging is still pinned by an importlib.resources test. Remaining review minors (error provenance, empty-means-unset, argument-beats-env-var ordering) all landed with tests.Note for external consumers: the pins' sha256 content addresses changed with the header edits (no in-repo hash records them; disclosed here for anyone comparing
env_hashacross versions).Gates: ruff format/check, ty; registry/environment/docs-drift 104 passed; manifest/registry sweep 483 passed.