Skip to content

Say How Each Measured Requirement Actually Fails - #648

Merged
ptr727 merged 3 commits into
developfrom
fix/floor-failure-modes
Aug 9, 2026
Merged

Say How Each Measured Requirement Actually Fails#648
ptr727 merged 3 commits into
developfrom
fix/floor-failure-modes

Conversation

@ptr727

@ptr727 ptr727 commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Answers the suppressed finding on the promotion pull request #643, which cannot take a fix of its own since its head is develop.

The python3 tool's why text says older interpreters fail with an AttributeError "at the call rather than at startup", but spec/audit.py and scripts/pr_review.py now import UTC from datetime, which raises ImportError at import time on Python < 3.11.

Accepted. The sentence collapsed two requirements into one failure mode and was wrong for one of them. str.removeprefix is a method call and fails as the text described. datetime.UTC arrives through a module-level import:

$ grep -n "from datetime import" spec/audit.py scripts/pr_review.py scripts/test_pr_review.py
spec/audit.py:33:from datetime import UTC, datetime
scripts/pr_review.py:74:from datetime import UTC, datetime
scripts/test_pr_review.py:20:from datetime import UTC, datetime, timedelta

So on an older interpreter that module raises before any of it runs.

Constructed both failures rather than describing them, since the whole point of the sentence is to name the symptom a reader is looking at:

import-time  : ImportError -> cannot import name '...' from 'datetime'
call-time    : AttributeError -> 'str' object has no attribute 'removeprefix'

The entry now names each requirement with its own mode, and says the import failure is the one a host meets first, because the script does not start at all.

This matters more than a wording nit because of where the text is read. why is what scripts/host_gate.py prints to a host that just failed the floor. Telling that reader to expect an AttributeError partway through a run, when what they will actually see is an ImportError before anything runs, sends them looking for a different problem.

Verification

spec/validate.py exits 0, scripts/host_gate.py exits 0 reporting python3 3.13.5 meets the 3.13 floor, 552 tests pass, and prose_lint.py with the CI check list exits 0.

🤖 Generated with Claude Code

The rationale collapsed two requirements into one failure mode and got it wrong for
one of them. It said an older interpreter fails "as an AttributeError at the call
rather than at startup", which is true of str.removeprefix and false of
datetime.UTC: that arrives through a module-level `from datetime import UTC` in
spec/audit.py, scripts/pr_review.py and its tests, so an older interpreter raises
ImportError before any of the module runs.

Both failures were constructed rather than described, an ImportError from a
module-level import of a name that does not exist, and an AttributeError from a
call on a str without the method. The entry now names each requirement with its own
mode and says the import failure is the one a host meets first, since the script
does not start.

This text is what host_gate.py prints to a host that fails the floor, so the mode
it names is the symptom that reader is looking at.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the python3 entry in spec/host-tools.json to more accurately describe the concrete failure modes a host will see when running repository scripts on Python versions below specific standard-library feature floors.

Changes:

  • Refines the python3.why text to distinguish import-time ImportError (for datetime.UTC on <3.11) from call-time AttributeError (for str.removeprefix/str.removesuffix on <3.9).
  • Aligns the prose with the actual import sites in spec/audit.py, scripts/pr_review.py, and tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread spec/host-tools.json Outdated
"The second is the one a host meets first" was a blanket claim over a per-module
fact. scripts/prose_lint.py calls removeprefix and imports no UTC, so a host running
that script sees the AttributeError path and never reaches an import failure.

The three modules divide cleanly, checked against the tree rather than recalled:
spec/audit.py carries both requirements and fails at import, scripts/pr_review.py
carries only the import and fails the same way, and scripts/prose_lint.py carries
only the call and so starts, runs and fails partway through. The entry now says the
mode is decided by the script a host runs rather than by the interpreter alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 9, 2026 20:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

spec/host-tools.json:53

  • The why text says both str.removeprefix and str.removesuffix are called in scripts/prose_lint.py, but the repository only uses removeprefix there. removesuffix appears in spec/audit.py (e.g. s.removesuffix("|")), so the call-site list is currently inaccurate and could mislead someone debugging an older-interpreter failure.
            "why": "Every script here is standard library only, so a bare interpreter is enough and no package floor exists. The floor is the toolchain target rather than a measured breakage one version below it, which is the one entry here that reads that way and says so rather than implying a defect nobody found. pyproject.toml sets ruff target-version to py313 and mypy python_version to 3.13, so what those tools report describes 3.13 and describes no other interpreter, and a run below the floor is unverified rather than known broken. Neither tool runs in CI, which gates the standard-library script tests and the prose and repo gates and nothing else, so this floor is a configuration choice rather than an enforced result, and a host failing it has no CI failure to point at. Two hard requirements are measured, both sit lower, and they fail differently. str.removeprefix and str.removesuffix need 3.9 and are called in spec/audit.py and scripts/prose_lint.py, so an older interpreter starts, runs, and raises AttributeError when it reaches the call. datetime.UTC needs 3.11 and arrives through a module-level from datetime import UTC in spec/audit.py, scripts/pr_review.py and its tests, so an older interpreter raises ImportError before any of those modules run at all. Which mode a host sees is decided by the script it runs rather than by the interpreter alone: spec/audit.py carries both and fails at import, scripts/pr_review.py carries only the import and fails the same way, and scripts/prose_lint.py carries only the call and therefore starts, runs, and fails partway through. The name rather than the version is what differs per platform, which the second probe covers.",

Two corrections, one from the review and one from auditing the rest of the field
after it, which is the pass that should have happened four findings ago.

The named one: the entry said both removeprefix and removesuffix are called in
spec/audit.py and scripts/prose_lint.py. Enumerating call sites per method rather
than with one combined grep gives removeprefix at prose_lint.py:91 and audit.py:714,
and removesuffix at audit.py:74 and audit.py:715 only. The combined grep is what
produced the combined claim.

The one the audit found: the entry said CI gates "the standard-library script tests
and the prose and repo gates and nothing else". It also runs markdownlint, cspell,
actionlint, editorconfig-checker and spec/validate.py. The point being made, that no
Python linter or type checker runs there, survives, and the "nothing else" did not.

Every remaining claim was checked against the tree in the same pass. Standard
library only holds, verified by walking every import in every tracked .py against
sys.stdlib_module_names and the local modules, with no third-party import found. The
pyproject settings, the UTC import sites and the per-module failure modes each read
as stated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 9, 2026 20:06
@ptr727

ptr727 commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Answering the suppressed finding from the round on d2e51c9a. Fixed in 2abc6f4, along with one more that finding prompted me to go looking for.

The why text says both str.removeprefix and str.removesuffix are called in scripts/prose_lint.py, but the repository only uses removeprefix there.

Accepted. Enumerating per method rather than with one combined grep, which is exactly what produced the combined claim:

$ grep -n "\.removeprefix(" $(git ls-files '*.py')
scripts/prose_lint.py:91:    return path.as_posix().removeprefix('./')
spec/audit.py:714:    s = s.removeprefix("|")

$ grep -n "\.removesuffix(" $(git ls-files '*.py')
spec/audit.py:74:        return r.stdout.strip().rstrip("/").removesuffix(".git").split("/")[-1], True
spec/audit.py:715:    s = s.removesuffix("|")

The entry now names each method against the files that call it.

The audit that finding should have triggered four findings ago

This is the fifth correction to this one field, so instead of fixing the named sentence and pushing, I read every remaining claim in it against the tree. That found a sixth, which no review had reached:

Neither tool runs in CI, which gates the standard-library script tests and the prose and repo gates and nothing else

validate-task.yml also runs markdownlint, cspell, actionlint, editorconfig-checker and spec/validate.py. The point being made survives, that no Python linter or type checker runs in CI, and the "nothing else" did not. It now lists what CI actually runs.

The rest checked out. "Standard library only" was verified by walking every import in every tracked .py against sys.stdlib_module_names plus the local module names, with no third-party import found, rather than by trusting the sentence. The pyproject.toml settings, the three UTC import sites and the per-module failure modes each read as stated.

What kept going wrong

Six inaccuracies in one field, and every one has the same shape: a sentence generalising past what I had actually measured. Each measurement was one grep away, and the reason five of them reached review is that I corrected the sentence I was shown and never re-read its neighbours. The field's stated purpose, written above it in this same file, is to keep a floor from becoming folklore, and an unverified generalisation is how folklore gets written down.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 25e01e3 into develop Aug 9, 2026
7 checks passed
@ptr727
ptr727 deleted the fix/floor-failure-modes branch August 9, 2026 20:11
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.

2 participants