From e4aa5d873932eb2586ed7a851d72cdc7e4f60760 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 11 Sep 2026 21:55:06 -0700 Subject: [PATCH] Read an Empty prose_lint --diff as a Failed Base, Not as No Diff (#1566) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1537, which registers on the `develop -> main` promotion rather than here. ## The defect `prose_lint.py` decided whether a run was diff-scoped by the truthiness of `--diff` rather than by its presence, so an empty value read as no `--diff` at all. The run then widened to the whole tree and reported the existing backlog as though the change had introduced it, which is the outcome the refusal on the very next line exists to prevent. An unresolvable ref took that refusal while an empty string walked past it. The exposed path is a local run whose caller substitutes a command's output, `--diff "$(git merge-base origin/develop HEAD)"`. Where `origin/develop` does not exist, as in a single-branch clone of `main`, `git merge-base` writes only to stderr and the script receives `--diff ""`. It was found while documenting that invocation for `ptr727/aiopurpleair`. ## The fix Both the scope computation and the refusal below it test `a.diff is not None`, so an empty value reaches `changed_lines`, where `git diff ''` fails with `fatal: bad revision ''`, and the existing refusal fires naming the value it could not resolve. The issue's own reproduction, against this branch: ``` $ python3 prose_lint.py --diff "" -- . error: cannot diff against '', so the run cannot be scoped to changed lines. Refusing to scan the whole tree instead, since that reports the existing backlog as though this change introduced it. Check the ref exists and that the checkout carries its history. exit=2 ``` It previously exited 1 on `a.md:1: semicolon`, a line the change never touched. ## Tests Two, because one of them alone would not hold the fix: - A unit test asserts the empty value reaches the resolver and takes the refusal, rather than asserting the exit code alone, which the whole-tree path could also produce for its own reasons. - A second test runs the same case against a real repository, since the first mocks the resolver and would stay green if git ever resolved an empty revision. Resolving one to an empty scope reads as a clean tree, which is the other half of what the routing closes. Each was proved by reverting the two production lines, where both fail with `2 != 1`, the whole-tree widening itself. ## Reach No caller in the tree can pass an empty-but-present value today, so nothing that works now starts refusing. `.github/actions/prose-gate/action.yml` rejects an unresolvable `$BASE` before invoking the script, `.github/workflows/validate-task.yml` gates that step on a pull request event, and the Husky, pre-commit and `hub-fetch-run.py` callers all pass a literal `HEAD`. `scope_note` already tested `base is None`, so no truthiness site was left disagreeing with the new one, and `gate_provenance` keeps truthiness deliberately, where an empty value falling through to the next source is what it wants. ## Verification Full local gate set green on this head: 270 cases in `scripts/tests/test_prose_lint.py` and 1405 across the suite, `ruff format --check`, `ruff check`, `mypy`, `prose_lint.py` over the gating rule set, `repo_gate.py`, `spec/validate.py`, `build_dist.py --check`, `canonical_review.py check`, and `docker_lint.py`'s seven linters. No canonical unit changed, so no carried-content pass was owed. Three local strict-review passes ran at the strongest available tier. The first raised three findings and the second six, all answered, which is the two-round edit budget spent. The third found no behavioral defect and four prose findings, open and listed here rather than fixed, per that budget: a missing comma after a fronted participle at `test_prose_lint.py:1906`, a relative pronoun binding to the nearer noun at `:2697`, an unexplained positive control at `:2705`, and both new tests asserting the exit code where the sibling refusal test also asserts the message text. One earlier finding was declined with evidence: that the refusal's closing sentence, "Check the ref exists and that the checkout carries its history", names a remedy that does not fit an empty value. It is pre-existing text this change did not write, the message prints the value with `!r` so an empty base displays as `''`, and growing a per-input remedy enumeration has repeatedly cost this repository more than it returned. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- .github/actions/prose-gate/prose_lint.py | 6 +++-- scripts/tests/test_prose_lint.py | 33 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/.github/actions/prose-gate/prose_lint.py b/.github/actions/prose-gate/prose_lint.py index e84dd271..72a24a54 100755 --- a/.github/actions/prose-gate/prose_lint.py +++ b/.github/actions/prose-gate/prose_lint.py @@ -1846,8 +1846,10 @@ def main(argv: list[str] | None = None) -> int: return 0 discovered = len(files) - scope = changed_lines(a.diff, scan_root) if a.diff else None - if a.diff and scope is None: + # Presence rather than truthiness, since an empty value is a base that failed to compute. + # Reading it as no diff at all is the silent widening the refusal below exists to prevent. + scope = changed_lines(a.diff, scan_root) if a.diff is not None else None + if a.diff is not None and scope is None: # Widening to the whole tree answers a different question, and answers it silently. # A caller scoping to a change gets the backlog reported as though the change made it. # A CI adoption hits this first, where an unresolvable base walls off the first run. diff --git a/scripts/tests/test_prose_lint.py b/scripts/tests/test_prose_lint.py index 4fecba9f..79509863 100755 --- a/scripts/tests/test_prose_lint.py +++ b/scripts/tests/test_prose_lint.py @@ -1899,6 +1899,24 @@ def test_a_failed_diff_is_an_error_rather_than_a_wider_or_narrower_scan(self) -> ): self.assertEqual(2, prose_lint.main(["--check", "dupword", "--diff", "HEAD"])) + def test_an_empty_diff_value_takes_the_refusal_rather_than_widening(self) -> None: + """An empty value is a base that failed to compute, never a request for a whole tree. + + A caller substituting a command's output, `--diff "$(git merge-base origin/develop HEAD)"`, + passes an empty string whenever that command fails. Read as truthiness the option + vanishes, the refusal `main` holds for a base it cannot resolve is never reached, and the + run reports the whole tree's backlog as though the change introduced it. + """ + bait = self.tmp / "bait.md" + bait.write_text(f"{DUP} thing\n", encoding="utf-8") + with ( + mock.patch.object(prose_lint, "discover", return_value=[bait]), + mock.patch.object(prose_lint, "changed_lines", return_value=None) as changed, + ): + self.assertEqual(2, prose_lint.main(["--check", "dupword", "--diff", ""])) + # Pins that the resolver received the empty value itself, rather than a coerced stand-in. + self.assertEqual("", changed.call_args.args[0]) + def test_list_files_prints_the_scope_and_reports_nothing(self) -> None: """The audit path for the sweep scope exits 0 even on a tree full of findings.""" bait = self.tmp / "bait.md" @@ -2672,6 +2690,21 @@ def test_scanning_one_repository_while_standing_in_another_diffs_the_one_scanned self.git(there, "commit", "-qam", "change") self.assertEqual(1, self.run_in(here, str(there), "--diff", "HEAD~1")) + def test_an_empty_base_is_refused_by_a_real_git_rather_than_by_a_mock(self) -> None: + """The precondition the empty-value refusal rests on, asserted against git itself. + + `main` routes an empty `--diff` into the resolver rather than treating it as no diff at + all, which is only a refusal because git rejects an empty revision. + `TestCli.test_an_empty_diff_value_takes_the_refusal_rather_than_widening` mocks the + resolver, so it would stay green if git ever resolved one. Resolving one to an empty scope + reads as a clean tree, which is the false pass the routing exists to close. + """ + root = self.repo() + (root / "DOC.md").write_text(self.BAIT, encoding="utf-8") + self.git(root, "commit", "-qam", "change") + self.assertEqual(1, self.run_in(root, ".", "--diff", "HEAD~1")) + self.assertEqual(2, self.run_in(root, ".", "--diff", "")) + def test_a_subtree_argument_reads_the_untracked_files_inside_it(self) -> None: """`git ls-files` prints names relative to its `-C` directory, for `--others` as well.