Review: return false when dotenv contains only valueless declarations

[srijan2607]

Commit-attribution correction: author and committer now link to https://github.com/srijan2607. This is a metadata-only history rewrite; every file tree is identical to the tested version. Recorded local test results remain evidence for the same code; CI must be checked at the new head.

Problem and scope

load_dotenv(stream=StringIO("EXAMPLE_KEY")) returns True even though a bare declaration has no value and is ignored. This patch returns False when every parsed declaration has value None, matching the existing empty-configuration result. Empty-string assignments still count as values.

Only src/dotenv/main.py and the new tests/test_load_result.py change. The implementation predicate matches the existing if v is not None assignment guard. Parsing, interpolation, environment contents, file discovery and override behavior are unchanged. In particular, an ordinary valued assignment skipped because override=False still returns True; this is not a general redesign of the return contract.

Review boundary

This is a public draft in my own fork, not an upstream submission. Code, tests and this description were prepared with AI assistance. A separate AI agent performed adversarial review; that is not human review. My personal review and explicit approval of both this head and the separate upstream-text preview remain pending. Do not merge or submit upstream from this draft.

• Reviewed base: 0b2880591780a426b0551436f47a17e7a76d954f
• Head to review: 0f7609f116b19ac7355daa6b5d9147a8c5817907
• Please inspect the all-None versus empty-string boundary, existing-ambient-variable behavior, and file/stream regression cases.

Portable clean-checkout setup and checks

Requires Git, uv and Python 3.13. These dependency versions match the executed local checks; this is a focused test environment, not the complete optional development-tool installation.

git clone --branch fix/valueless-load-result https://github.com/srijan2607/python-dotenv.git
cd python-dotenv
git checkout 0f7609f116b19ac7355daa6b5d9147a8c5817907
uv venv --python 3.13 .venv
uv pip install --python .venv/bin/python 'pytest==9.1.1' 'click==8.5.0' 'ruff==0.12.0'
uv pip install --python .venv/bin/python --no-deps -e .
export PATH="$PWD/.venv/bin:$PATH"
export PYTHONPATH="$PWD/src"
.venv/bin/python -m pytest tests/test_load_result.py -q
.venv/bin/python -m pytest tests -q -rs --basetemp .test-tmp
.venv/bin/ruff check .
.venv/bin/ruff format --check .
git diff --check HEAD^

Expected targeted result: 22 passed. Full suite observed: 306 passed, 1 skipped (IPython unavailable), with existing Click deprecation warnings. If an ancestor directory contains an unrelated .env, file-discovery tests can pick it up; use an isolated fresh checkout/test directory without an ancestor .env.

Minimal visible behavior:

.venv/bin/python -c 'from io import StringIO; from dotenv import load_dotenv; print(load_dotenv(stream=StringIO("EXAMPLE_KEY"))); print(load_dotenv(stream=StringIO("EXAMPLE_KEY=")))'

Expected after: False, then True. Before: True, then True.

Causal before-control with the same regression tests

Run from the patched checkout after setup:

git worktree add --detach ../python-dotenv-before 0b2880591780a426b0551436f47a17e7a76d954f
cp tests/test_load_result.py ../python-dotenv-before/tests/test_load_result.py
PYTHONPATH="$PWD/../python-dotenv-before/src" .venv/bin/python -m pytest ../python-dotenv-before/tests/test_load_result.py -q --basetemp .before-test-tmp

Expected nonzero exit: 16 failed, 6 passed, with false-return assertions failing on the original implementation. Do not treat that expected negative control as a setup failure.

Actual verification and limits

• CPython 3.13.13, pytest 9.1.1, Click 8.5.0, Ruff 0.12.0.
• Final tests failed on the exact original base: 16 failed, 6 passed; patched regressions: 22 passed.
• Full patched suite independently rerun: 306 passed, 1 skipped; pristine baseline suite: 284 passed, 1 skipped.
• Ruff lint/format and whitespace check passed.
• Independent agent review approved this exact complete patch for fork review. Its additional differential checks compared parsed mappings and resulting environments, not only return values.
• The final full-suite rerun used a real editable installation and its actual dotenv console script. An earlier reviewer run used a temporary launcher for the same CLI because that environment initially lacked the script; it is not an installation test. One later setup attempt lacked the requested temporary parent directory; it was corrected before the successful rerun. A default temporary-directory run encountered the unrelated ancestor-.env fixture interference described above.
• IPython, other Python versions, Windows and upstream CI were not verified. No issue, comment or PR was posted upstream. No claim of maintainer acceptance or human-authored work is made.

Captured failing/passing output is provided in a separate evidence comment. Proposed upstream title/body are in another separate comment and require approval before submission.

[srijan2607]

Preview only; not submitted upstream.

Approval requested for head 0f7609f116b19ac7355daa6b5d9147a8c5817907 and the exact proposed text below. This comment is on the own-fork draft only. No upstream write is authorized by this preview.

Exact proposed upstream title

Return false when dotenv contains only valueless declarations

Exact proposed upstream body

load_dotenv() returns True for content containing only bare declarations such as EXAMPLE_KEY, although those declarations are ignored and cannot set environment variables.

Return False when all parsed values are None, as for an empty configuration. Preserve empty-string assignments and the existing result for valued assignments skipped by override=False. Parsing, interpolation and environment contents are unchanged.

Tests cover file and stream inputs, both override modes, existing environment values, bare-only content, empty assignments and mixed content.

Testing on Python 3.13:

• New regressions against the original implementation: 16 failed, 6 passed.
• New regressions after the change: 22 passed.
• Full suite: 306 passed, 1 skipped because optional IPython is unavailable.
• Ruff lint, formatting and git diff --check passed.

Code, tests and this description were prepared with AI assistance and reviewed by a separate AI agent. Personal human review and the other Python/OS combinations are not claimed by this draft.

Prerequisite issue text

None proposed: the inspected contribution instructions welcome an issue or a pull request and do not require an issue first. This is an original bounded discovery, not a claim that an existing issue is assigned or resolved. No issue has been opened.

Commit identity was corrected without changing any file tree; original execution logs retain their original source hashes as provenance. The current approval target is the metadata-equivalent head above.

[srijan2607]

Captured regression evidence and independent agent review

This evidence belongs to head 0f7609f116b19ac7355daa6b5d9147a8c5817907, against base 0b2880591780a426b0551436f47a17e7a76d954f. Code/tests and this summary are AI-assisted; the independent review was by a separate AI agent, not a person. Full reproduction commands and environment versions are in the PR body. Paths in captured output below are sanitized.

Exact-base failing control (output excerpt)

The final regression tests were copied to a pristine base worktree; expected exit status: 1.

se, source = 'stream', existing = True
content = '# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n'

    @pytest.mark.parametrize("override", [False, True])
    @pytest.mark.parametrize("source", ["file", "stream"])
    @pytest.mark.parametrize("existing", [False, True])
    @pytest.mark.parametrize(
        "content", ["DOTENV_TEST_KEY", "# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n"]
    )
    def test_load_dotenv_without_values_returns_false(
        tmp_path, override, source, existing, content
    ):
        environ = {"DOTENV_TEST_KEY": "existing"} if existing else {}
        with mock.patch.dict(os.environ, environ, clear=True):
            if source == "file":
                path = tmp_path / ".env"
                path.write_text(content)
                result = load_dotenv(path, override=override)
            else:
                result = load_dotenv(stream=io.StringIO(content), override=override)
    
>           assert result is False
E           assert True is False

../portable-before/tests/test_load_result.py:28: AssertionError
_ test_load_dotenv_without_values_returns_false[# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n-True-stream-True] _

tmp_path = PosixPath(' <evidence>/test-artifacts/portable-before/test_load_dotenv_without_value15')
override = True, source = 'stream', existing = True
content = '# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n'

    @pytest.mark.parametrize("override", [False, True])
    @pytest.mark.parametrize("source", ["file", "stream"])
    @pytest.mark.parametrize("existing", [False, True])
    @pytest.mark.parametrize(
        "content", ["DOTENV_TEST_KEY", "# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n"]
    )
    def test_load_dotenv_without_values_returns_false(
        tmp_path, override, source, existing, content
    ):
        environ = {"DOTENV_TEST_KEY": "existing"} if existing else {}
        with mock.patch.dict(os.environ, environ, clear=True):
            if source == "file":
                path = tmp_path / ".env"
                path.write_text(content)
                result = load_dotenv(path, override=override)
            else:
                result = load_dotenv(stream=io.StringIO(content), override=override)
    
>           assert result is False
E           assert True is False

../portable-before/tests/test_load_result.py:28: AssertionError
=========================== short test summary info ============================
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[DOTENV_TEST_KEY-False-file-False]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[DOTENV_TEST_KEY-False-file-True]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[DOTENV_TEST_KEY-False-stream-False]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[DOTENV_TEST_KEY-False-stream-True]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[DOTENV_TEST_KEY-True-file-False]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[DOTENV_TEST_KEY-True-file-True]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[DOTENV_TEST_KEY-True-stream-False]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[DOTENV_TEST_KEY-True-stream-True]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n-False-file-False]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n-False-file-True]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n-False-stream-False]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n-False-stream-True]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n-True-file-False]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n-True-file-True]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n-True-stream-False]
FAILED ../portable-before/tests/test_load_result.py::test_load_dotenv_without_values_returns_false[# comment\nDOTENV_TEST_KEY\nOTHER_KEY\n-True-stream-True]
16 failed, 6 passed in 0.18s

EXIT=1

Patched complete-suite rerun (captured output)

$ <venv>/bin/python -m pytest tests -q -rs -p no:cacheprovider --basetemp  <evidence>/test-artifacts/parent-run
........................................................................ [ 23%]
........................................................................ [ 47%]
........................................................................ [ 70%]
........................................................................ [ 94%]
..................                                                       [100%]
=============================== warnings summary ===============================
tests/test_cli.py: 32 warnings
  <checkout>/tests/conftest.py:8: DeprecationWarning: 'isolated_filesystem' is deprecated and will be removed in Click 9.0. Use 'tempfile.TemporaryDirectory' or pytest's 'tmp_path' fixture with absolute paths instead.
    with runner.isolated_filesystem():

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
SKIPPED [1] tests/test_ipython.py:7: could not import 'IPython': No module named 'IPython'
306 passed, 1 skipped, 32 warnings in 6.66s

EXIT=0

Independent adversarial review

Verdict: APPROVE_FOR_FORK_REVIEW, bound to complete patch SHA-256 f3535fea6aebbb02cbf7f5a463ec6b0005668e04331d6a4d4700b532fe417c7c. No blocking code findings remained. This does not authorize upstream publication.

The reviewer separately ran the 22 regressions, full suite, lint/format and exact-base negative control. A differential probe exercised 4,680 cases and observed only the intended return-value differences for all-None mappings; parsed mappings and final environments matched the original implementation. Existing no-override success behavior was preserved. Historical return-value discussions were checked separately from unrelated parent-directory searching.

Limits: bounded local execution only; no upstream CI or additional OS/Python matrix is represented as completed. Personal human review and approval of the separate exact upstream-text preview are still pending.

Commit identity was corrected without changing any file tree; original execution logs retain their original source hashes as provenance. The current approval target is the metadata-equivalent head above.

[srijan2607]

Superseded by #2. The replacement draft targets this fork’s main and retains the same corrected code head 0f7609f116b19ac7355daa6b5d9147a8c5817907. Please use the replacement and its separate official-submission preview; closing this older draft without merging.