test: keep .env out of the suite; report the real interpreter in notebooks - #12
Merged
Merged
Conversation
…books The suite was documented as hermetic in three places, but core/config.py reads .env for every field and builds its settings singleton at import, so a local .env silently changed what the suite asserted. Following the README's own advice to set NOTEBOOK_VERIFICATION_ENABLED=false turned the suite red, which is how this was found. conftest.py now sets DATA_CONCIERGE_ENV_FILE="" before anything imports config, and config.py honours it (empty means no env file). Three tests asserted shipped defaults or a security guard's default posture by reading the live singleton, which also reflects the process environment; they now assert Settings.model_fields[...] or pin the value with monkeypatch, so an exported variable cannot reach them either. Separately, generated notebooks hardcoded language_info.version = "3.11" in two places — impossible since requires-python became >=3.12. The notebook is the deliverable and its provenance has to be true, so both now report platform.python_version(). Verified: 802 passed with a hostile .env (both flags flipped) AND the same two variables exported in the shell; 802 passed clean; ruff clean. Generated notebook metadata confirmed to carry the running interpreter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both README sections still described the bug the previous commit fixed. They claimed core/config.py hardcodes env_file=".env" and that a .env entry fails test_ships_enabled — neither is true now: env_file is conditional on DATA_CONCIERGE_ENV_FILE, which tests/conftest.py sets to "", and test_ships_enabled asserts the class default rather than the live singleton. Confirmed by appending the flag to .env and running the file: 33 passed. Setting it in .env is now a fine way to keep a local default, so both sections say so. The behaviour-flags section at :264 carried the same advice plus a cross-reference to the reason being removed, so it is updated too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Two related bits of drift, both found the hard way while setting up a local instance.
1. The suite was not actually hermetic
CONTRIBUTING.md, the CI comment and thesuggested_commandsmemory all statedthe suite needs no
.env. True — butcore/config.pyreads one for everyfield and builds its
settingssingleton at import time, so a developer's local.envsilently changed what the suite asserted.Concretely: following the README's own troubleshooting advice and setting
NOTEBOOK_VERIFICATION_ENABLED=falseturned the suite red. That is how this wasfound.
tests/conftest.pysetsDATA_CONCIERGE_ENV_FILE=""before anything importsconfig;config.pyhonours it, empty meaning no env file.by reading the live singleton, which also reflects the process environment.
They now assert
Settings.model_fields[...].default, or pin the value withmonkeypatch, so an exported variable cannot reach them either:test_ships_enabled,test_ships_locked_down,test_model_validator_rejects_on_update_construction.conftest guard does not cover.
2. Generated notebooks claimed an impossible interpreter
notebook_generator.pyhardcodedlanguage_info.version = "3.11"in two places.That has been wrong since
requires-pythonbecame>=3.12. The notebook is thedeliverable — reproducible, citable, independently verifiable — so its provenance
has to be true. Both sites now report
platform.python_version().Verified
802 passedwith a hostile.env(both flags flipped) and the same twovariables exported in the shell
802 passedclean;ruff checkclean(
language_info.version == platform.python_version())Note
MCP_ALLOW_PRIVATE_URLSwas the second latent instance of this bug — thevariable exists precisely so local development can point at localhost MCP
servers, which makes the developer most likely to run the suite the one most
likely to have it set.
🤖 Generated with Claude Code