fix: doctor reads every pin form the org actually uses - #140
Conversation
![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: 4ecd0073ab
ℹ️ 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".
| _PIN_PATTERNS = ( | ||
| re.compile(r"^\s*commoner[-_]probe\s*==\s*([0-9][^\s;#]*)", re.I | re.MULTILINE), | ||
| re.compile(r"commoner-probe(?:\.git)?@v?([0-9][^\s;#\"']*)", re.I), | ||
| re.compile(rf"commoner[-_]probe{_EXTRAS}\s*==\s*([0-9][^\s;#,\"']*)", re.I), |
There was a problem hiding this comment.
Because this pattern is now unanchored and has no check for a comment prefix, a file containing # commoner-probe==0.14.3 before an active commoner-probe==0.15.0 is reported as pinned to 0.14.3: search() selects the commented occurrence first and the loop stops. This can make doctor report a nonexistent mismatch and exit 1, so commented occurrences must be excluded while retaining support for quoted TOML dependency entries.
Useful? React with 👍 / 👎.
| #: exact pin, so this is a finding rather than an absence — reporting nothing | ||
| #: filed it beside the files that never mention the package at all. | ||
| _UNPINNED = re.compile( | ||
| rf"^\s*[\"']?commoner[-_]probe{_EXTRAS}\s*(?:[<>~!=]=|@|$)", re.I | re.MULTILINE) |
There was a problem hiding this comment.
When a consumer uses the valid compact form dependencies = ["commoner-probe>=0.14"], this start-of-line pattern cannot reach the requirement and declared_pins() returns {} rather than unpinned. Consequently doctor can exit successfully even though the newly enforced exact-pin policy is violated; unpinned detection needs the same quoted inline-TOML context support added for exact pins.
Useful? React with 👍 / 👎.
|
Both fixed in the head commit, each reproduced first. Commented pin wins — real, and it was a regression from unanchoring the pattern in the first place. Comment tails are stripped before any pattern runs. A Inline TOML range — real. The unpinned test kept its start-of-line anchor while the pin patterns lost theirs, so the compact form was unreachable and a violated pin policy exited successfully. Now unanchored too, which is safe because comments are gone by then. Re-ran against the seven live consumer files: still seven pins, unchanged. 1731 passed, ruff clean. @codex review |
💡 Codex Reviewcommoner-probe/commoner_probe/doctor.py Line 50 in 920760d Recognize semicolons after bare requirements
When a valid conditional dependency omits whitespace before its marker, such as commoner-probe/commoner_probe/doctor.py Line 40 in 920760d Require a package-name boundary before exact pins
Because this now-unanchored pattern has no left boundary, a requirements file containing only another valid distribution such as ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review — the head is now |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 920760d3a8
ℹ️ 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".
| _PIN_PATTERNS = ( | ||
| re.compile(r"^\s*commoner[-_]probe\s*==\s*([0-9][^\s;#]*)", re.I | re.MULTILINE), | ||
| re.compile(r"commoner-probe(?:\.git)?@v?([0-9][^\s;#\"']*)", re.I), | ||
| re.compile(rf"commoner[-_]probe{_EXTRAS}\s*==\s*([0-9][^\s;#,\"']*)", re.I), |
There was a problem hiding this comment.
For a requirements file containing a different package such as my-commoner-probe==9.9.9, this unanchored regex starts matching at the inner commoner-probe substring and reports 9.9.9 as this package's pin. doctor can consequently report a false version mismatch and exit 1; require a package-name boundary before commoner while retaining support for quoted TOML entries.
Useful? React with 👍 / 👎.
| #: like the pin patterns, because the compact TOML form | ||
| #: `dependencies = ["commoner-probe>=0.14"]` is valid and a start-of-line test | ||
| #: could not reach it: a violated pin policy then exited successfully. | ||
| _UNPINNED = re.compile(rf"commoner[-_]probe{_EXTRAS}\s*(?:[<>~!]=|<|>|@|[\"',\]\s]|$)", re.I) |
There was a problem hiding this comment.
When a pyproject.toml passed via --requirements merely mentions the name in metadata, for example description = "built on commoner-probe", the closing quote satisfies this pattern and the file is reported as unpinned. That creates a mismatch and exit 1 even though the project never declares this package as a dependency, so the search needs to distinguish dependency entries from arbitrary TOML string values.
Useful? React with 👍 / 👎.
|
Both correct, both reproduced first, fixed in
Verified against the live consumer files, not only fixtures: all seven still read (six on 0.14.3, one on 0.14.9). 1734 passed, ruff clean. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 761c483cd8
ℹ️ 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".
| _PIN_PATTERNS = ( | ||
| re.compile(r"^\s*commoner[-_]probe\s*==\s*([0-9][^\s;#]*)", re.I | re.MULTILINE), | ||
| re.compile(r"commoner-probe(?:\.git)?@v?([0-9][^\s;#\"']*)", re.I), | ||
| re.compile(rf"{_NAME}\s*==\s*([0-9][^\s;#,\"']*)", re.I), |
There was a problem hiding this comment.
When TOML metadata contains version-like prose, such as description = "built for commoner-probe==9.9.9", this unanchored pattern makes declared_pins() report 9.9.9 even if the project only depends on unrelated packages, causing doctor to report a nonexistent mismatch. Fresh evidence beyond the prior prose comment is that only the unpinned patterns were narrowed; reproducing against the current tree shows exact-pin prose is still accepted, so exact matches also need to be limited to actual requirement tokens.
Useful? React with 👍 / 👎.
|
Correct, and fixed in
Running the reader over the eight live pin files found a second case of the same defect:
Positive control, the eight live files: the seven real consumer pins still read (0.14.3 six times, 0.14.9 once), and the two self-referential files now report nothing. 1739 passed, 2 skipped, ruff clean. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31f2381510
ℹ️ 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".
| #: scalar assignment — `name = "commoner-probe"` and the console-script key — | ||
| #: so an opening quote alone is not enough. Only `[` and `,` open a dependency | ||
| #: list. | ||
| _TOKEN = r"(?:^|[\[,]\s*[\"'])\s*" |
There was a problem hiding this comment.
When a supplied pyproject.toml contains non-dependency list metadata such as keywords = ["commoner-probe==9.9.9"], this token accepts the opening [ and declared_pins() reports 9.9.9, producing a false mismatch for a project that does not depend on the package. Fresh evidence beyond the prior prose report is that the current narrowing excludes scalar prose but still treats every quoted TOML array element as a dependency; the TOML form needs to be tied to dependency or optional-dependency declarations.
Useful? React with 👍 / 👎.
| re.compile(rf"{_TOKEN}{_NAME}\s*(?:[<>~!]=|[<>@])", re.I | re.MULTILINE), | ||
| re.compile(rf"^\s*{_NAME}\s*$", re.I | re.MULTILINE), | ||
| re.compile(rf"[\[,]\s*[\"']\s*{_NAME}\s*[\"']", re.I | re.MULTILINE), |
There was a problem hiding this comment.
When the dependency has an environment marker but no version, for example commoner-probe; python_version < "3.12" or the equivalent quoted TOML entry, none of these patterns matches because there is neither a range operator nor an immediate line end/closing quote after the name. declared_pins() therefore returns {} and doctor can succeed even though this dependency violates the newly enforced exact-pin policy; accept a semicolon and marker after the name as an unpinned declaration.
Useful? React with 👍 / 👎.
|
Both correct, both reproduced first, fixed in
Positive control, the eight live pin files plus this repo's own: the seven real consumer pins still read (0.14.3 six times, 0.14.9 once), and the two self-referential files report nothing. 1745 passed, 2 skipped, ruff clean. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17a6854e07
ℹ️ 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".
| if match: | ||
| found[str(p)] = match.group(1) | ||
| break | ||
| else: | ||
| if any(pat.search(h) for h in haystacks for pat in _UNPINNED_PATTERNS): |
There was a problem hiding this comment.
When one file declares the package more than once, such as exact-pinning it in one optional-dependency group while using a range in another, the first exact match breaks this loop and the else that checks _UNPINNED_PATTERNS never runs. If that exact version matches the environment, doctor exits successfully even though another dependency declaration still violates the exact-pin policy; all relevant declarations need to be checked before choosing the file's result.
Useful? React with 👍 / 👎.
| if text[i] == "[": | ||
| depth += 1 | ||
| elif text[i] == "]": | ||
| depth -= 1 |
There was a problem hiding this comment.
When an earlier dependency contains a valid marker value with a bracket, for example "other; platform_version == ']'", this counter treats that quoted ] as the end of the TOML array. A later commoner-probe>=0.14 entry is then excluded from the returned haystack, so declared_pins() returns nothing and doctor silently misses the unpinned dependency; bracket counting must distinguish TOML string contents from structural brackets.
Useful? React with 👍 / 👎.
| if pin == "unpinned": | ||
| out.append( | ||
| f"{where} names this package with no exact version. The org pins " | ||
| "with == or @vX.Y.Z, because a range moves under the consumer " | ||
| "without anyone deciding to move it.") |
There was a problem hiding this comment.
When --requirements contains a bare or ranged dependency, this new branch makes doctor exit 1, but both commoner_probe/cli.py and docs/CLI.md still tell users that it exits 1 only when two known version numbers disagree. An unpinned dependency supplies no known version number, so the documented CLI contract now contradicts the intentional exact-pin validation and should be updated alongside this behavior.
Useful? React with 👍 / 👎.
|
All three correct, fixed in
Positive control, the nine live pin files: seven real consumer pins read (0.14.3 ×5, 0.14.9 ×2 — theright2read moved to 0.14.9 during this session), and the two self-referential files report nothing. 1748 passed, 2 skipped, ruff clean. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e63b1fda4
ℹ️ 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".
| if quote: | ||
| if char == quote: | ||
| quote = "" |
There was a problem hiding this comment.
When a valid TOML marker uses escaped double quotes and contains a bracket, such as "other; platform_version == \"\]\"" before a later commoner-probe>=0.14 entry, this closes quote at the escaped quote and then treats the marker's ] as the structural end of the array. The later declaration is omitted and doctor can exit 0 over an unpinned dependency. Fresh evidence beyond the prior bracket comment is that the new scanner skips brackets inside ordinary quoted strings but does not account for TOML escape sequences.
Useful? React with 👍 / 👎.
| _DEP_KEYS = frozenset({"dependencies", "requires", "requires-dist"}) | ||
| _DEP_TABLES = ("optional-dependencies", "dependency-groups") | ||
| _TABLE_HEADER = re.compile(r"^\s*\[([^\]]+)\]\s*$", re.MULTILINE) | ||
| _ARRAY_KEY = re.compile(r"^\s*([A-Za-z_][\w.-]*)\s*=\s*\[", re.MULTILINE) |
There was a problem hiding this comment.
For valid TOML such as [project.optional-dependencies] followed by "test" = ["commoner-probe>=0.14"], this key regex does not match the quoted group name, so the array is never inspected and the unpinned declaration is silently missed. Quoting does not change the TOML key's value, and test remains a valid optional-dependency group name, so the array reader needs to support quoted keys rather than only bare keys.
Useful? React with 👍 / 👎.
| declares = (match.group(1) in _DEP_KEYS | ||
| or any(table.endswith(t) for t in _DEP_TABLES)) |
There was a problem hiding this comment.
When a supplied pyproject contains arbitrary tool metadata such as [tool.example] dependencies = ["commoner-probe==9.9.9"], this global key-name check treats it as a project dependency and can produce a false mismatch even though [tool.example] does not declare package installation metadata. Fresh evidence beyond the prior non-dependency-array comment is that the current narrowing recognizes dependencies and requires in every table; these keys should count only in the applicable project/build tables, while optional-dependency and dependency-group tables retain their group-key behavior.
Useful? React with 👍 / 👎.
|
All three correct, fixed in
One observation worth stating plainly, since this is round 6 on one function: what I am maintaining here is a small TOML parser, and each round has been another thing real TOML does. Positive control, the nine live pin files: seven consumer pins read, the two self-referential files report nothing. 1752 passed, 2 skipped, ruff clean. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c07bc7a350
ℹ️ 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".
| key = next(g for g in match.groups() if g is not None) | ||
| declares = ( | ||
| (key in _DEP_KEYS and table.strip('"\'') in _DEP_KEY_TABLES) | ||
| or any(table.endswith(t) for t in _DEP_TABLES)) |
There was a problem hiding this comment.
When a tool defines [tool.example.optional-dependencies] with x = ["commoner-probe==9.9.9"], this suffix check treats the array as project packaging metadata and doctor reports a false mismatch. Only [project.optional-dependencies] and the root [dependency-groups] table have the relevant packaging semantics, so compare the resolved table path rather than accepting every table whose name ends with these components.
Useful? React with 👍 / 👎.
| declares = ( | ||
| (key in _DEP_KEYS and table.strip('"\'') in _DEP_KEY_TABLES) | ||
| or any(table.endswith(t) for t in _DEP_TABLES)) |
There was a problem hiding this comment.
For valid TOML using dotted keys, such as project.dependencies = ["commoner-probe>=0.14"] at the root, _ARRAY_KEY returns the raw key project.dependencies, which fails both branches here. The resulting project data is equivalent to a dependencies key in [project], but doctor returns no declaration and can exit successfully over an unpinned dependency; the table/key path needs to be resolved before applying these allowlists.
Useful? React with 👍 / 👎.
| A `#` counts as a comment when it opens a line or follows whitespace, which | ||
| is how both requirements files and TOML write one. | ||
| """ | ||
| return re.sub(r"(?m)(?:^|(?<=\s))#.*$", "", text) |
There was a problem hiding this comment.
In TOML, # starts a comment outside a string even when it immediately follows punctuation. With dependencies = [#"commoner-probe==9.9.9"] followed by a real unrelated entry on the next line, this substitution leaves the comment intact, so the array scanner reads the commented string and reports a nonexistent 9.9.9 pin; a comment containing ] can likewise terminate scanning before a real dependency. Comment removal must recognize these starts while still preserving hashes inside strings.
Useful? React with 👍 / 👎.
|
All three correct. I have taken option 2 on all seven findings at once: the scanner is deleted, and The three you filed are each a rule the scanner did not have. Why the deletion rather than a seventh patch. Rounds 5, 6 and 7 produced seven findings and every one was the same defect: a hand-written parser meeting another thing real TOML does. The next round would have found an eighth. Reading a format needs a reader for that format. The cost, stated plainly.
Positive control, the nine live pin files: seven consumer pins read (0.14.3 ×5, 0.14.9 ×2), and the two self-referential files report nothing. 1755 passed, 2 skipped, ruff clean. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 628221b7e4
ℹ️ 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".
| #: prose; each was read as this package's pin, and `doctor` then failed a | ||
| #: consumer that never depended on it. | ||
| _EXTRAS = r"(?:\[[^\]]*\])?" | ||
| _NAME = rf"commoner[-_]probe{_EXTRAS}" |
There was a problem hiding this comment.
When a dependency uses the normalized-equivalent spelling commoner.probe, such as project.dependencies = ["commoner.probe>=0.14"], _NAME matches neither the exact nor unpinned patterns because it permits only - and _. declared_pins() therefore returns {} and doctor can exit successfully despite the newly enforced exact-pin policy; Python distribution-name normalization treats dots, hyphens, and underscores equivalently.
Useful? React with 👍 / 👎.
| if match: | ||
| found[str(p)] = match.group(1) | ||
| break | ||
| pin = pin if pin is not None else match.group(1) |
There was a problem hiding this comment.
When a newly supported TOML file contains two exact declarations with different versions—for example 0.15.0 in project.dependencies and 9.9.9 in an optional-dependency group—this assignment permanently retains the first match. Since neither declaration is unpinned, the file is reported only as 0.15.0; if that matches the environment, doctor exits 0 while silently ignoring the conflicting exact pin, despite the surrounding logic's intent to classify every declaration.
Useful? React with 👍 / 👎.
Found by running it against the seven live consumer files during a /pm recompute. It reported ONE pin where seven exist. Two assumptions were wrong. The requirement was anchored to the start of a line, so a `pyproject.toml` writing it quoted inside a dependency list read as no pin at all. And extras were not allowed, so `commoner-probe[http,pdf]==0.14.3` — the form four of the seven use — matched nothing. A file that names the package with no exact version now reports `unpinned` rather than nothing. The org pins with == or @vX.Y.Z, because a range moves under the consumer without anyone deciding to move it, and filing that beside the files that never mention the package hid it. The census this now produces: six files on 0.14.3 across five repos, one on v0.14.9, none on 0.15.0. Five tests, each written from a form measured in a real consumer file.
Codex P2s on this PR, both caused by unanchoring the pattern. `search()` returns the first occurrence, so a commented old pin above an active one won. `doctor` then reported a mismatch that does not exist and exited 1. Comment tails are now removed before any pattern runs, treating `#` as a comment where it opens a line or follows whitespace — the form both requirements files and TOML use. The unpinned test kept its start-of-line anchor, so the valid compact form `dependencies = ["commoner-probe>=0.14"]` was unreachable and a violated exact-pin policy exited successfully. It is now unanchored like the pin patterns, which is safe because comments are already gone by then. Re-run against the seven live consumer files: still seven pins, unchanged. 1731 passed, ruff clean. Four tests, three of which failed first.
my-commoner-probe==9.9.9 matched at the inner substring and was read as this package's pin. The patterns now require a name boundary. The unpinned pattern accepted any closing quote, so description = "built on commoner-probe" reported a file as unpinned. It now needs a range operator after the name, or the name as the whole requirement token. All seven live consumer pins still read.
The exact-pin pattern was left unanchored when the unpinned ones were narrowed, so `description = "built for commoner-probe==9.9.9"` reported 9.9.9 for a project that depends on nothing of ours. A reader sent to fix that pin finds no pin to fix. Both pin forms now have to open a line or a quoted element of a list. Running the reader over the eight live pin files found the second half. This package's own `pyproject.toml` read as unpinned: `name =` names the package, and so does the console-script key. Neither declares a dependency, so a bare name now counts only as a whole line or a whole quoted list element. The seven live consumer pins still read: 0.14.3 six times and 0.14.9 once.
…ot a pin Two findings from the review of the token narrowing. A quoted list element was read as a requirement wherever it sat, so `keywords = ["commoner-probe==9.9.9"]` reported a pin for a project that depends on nothing of ours. In a `.toml` file the search now runs inside dependency arrays only: `dependencies`, `requires`, and every key of an optional-dependency or dependency-group table. A requirements file is a list of requirements, so all of it still counts. `commoner-probe; python_version < "3.12"` matched nothing, so a file that violates the exact-pin policy passed the check. A marker now ends a bare requirement the way a line end does. The seven live consumer pins still read.
Three findings from the review of the dependency-array narrowing. A file can name the package twice — an exact pin in `dependencies` and a range in an optional group. The reader stopped at the first pin, so the violation was never looked for, and a pin that matched the environment let `doctor` exit 0 over a file that breaks the policy it enforces. Classification is now per declaration: one line of a requirements file, one quoted element of a dependency array. A file-wide rule cannot work, because a git pin holds both shapes in one string — `@` opens an unpinned URL requirement and the tag closes an exact one. Bracket counting read a `]` inside a quoted marker as the end of the array, so every later entry fell outside the search. Strings are skipped. `doctor --help` and `docs/CLI.md` still said exit 1 meant two known numbers disagreeing. An unpinned dependency supplies no number and now exits 1 too.
Three findings from the review of the per-declaration classifier. A basic TOML string processes escapes, so `\"` is a quote character. The scanner read it as the end of the string, went back into structure mode inside a marker, and let the marker's `]` close the array. Every later dependency then fell outside the search. A literal string has no escapes, so the rule is scoped to double quotes. A TOML key may be quoted, and quoting does not change its value. `"test" = [...]` names the same optional-dependency group as `test`. `dependencies` and `requires` now count only in the tables that give them meaning: the root, `[project]` and `[build-system]`. A tool's own `[tool.example] dependencies` array configures that tool and installs nothing, so a version in it was inventing a pin. The seven live consumer pins still read.
Three more findings, and all three are the same defect as the four before them: a hand-written scanner meeting another thing real TOML does. A tool's own `[tool.x.optional-dependencies]` matched by suffix. A dotted `project.dependencies` key matched nothing. A `#` after punctuation was not read as a comment. So the scanner is deleted. `tomllib` reads the file, and it arrived in 3.11, so `requires-python` moves from `>=3.10` to `>=3.11`. Python 3.10 reaches end of life in October 2026 and SPEC 0 dropped it in 2024. Every consumer repo in the org runs 3.14; none is affected. `doctor.py` loses 97 lines and gains 64. `source_version()` uses the parser too: its regex took the first `version = "..."` in the file, which is the project's version only while no table above declares one. CI now runs 3.11, 3.12 and 3.13. The README, CONTRIBUTING and the classifiers say 3.11.
Two findings from the review of the tomllib rewrite. PEP 503 normalises `-`, `_` and `.` to one distribution name, so `commoner.probe>=0.14` names this package and pip installs it. The reader saw neither an exact pin nor an unpinned one, and the file passed the check it breaks. One file can also carry two exact pins at different versions. Keeping the first hid the second, and a first that matched the environment exited 0. A file in that state is reported as a conflict, because which version installs depends on the resolver.
628221b to
bd5993e
Compare
|
Both correct, both reproduced first, fixed in
Positive control, the eight live pin files: seven consumer pins read (0.14.3 ×5, 0.14.9 ×2), and this repo's own 1800 passed, 2 skipped, ruff clean on the rebased branch. This PR is the last one open, and 0.15.0 is cut from it once it goes quiet. The release carries the 3.11 floor. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd5993ec41
ℹ️ 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".
| match = next( | ||
| (m for m in (pat.search(declaration) for pat in _PIN_PATTERNS) if m), None) |
There was a problem hiding this comment.
Fresh evidence beyond the earlier two-pin report is that one valid PEP 508 declaration can contain both pins, for example commoner-probe==0.15.0,==9.9.9. Because this takes only the first search() result from the declaration, declared_pins() reports 0.15.0 rather than a conflict; an environment running 0.15.0 therefore makes doctor exit successfully even though the requirement is contradictory. Collect all exact-version matches within each declaration before performing the conflict check.
Useful? React with 👍 / 👎.
`commoner-probe==0.15.0,==9.9.9` is one valid PEP 508 requirement and nothing satisfies it. The reader took the first specifier and reported a clean pin, so an environment on 0.15.0 exited 0 over a contradiction. Every `==` in the specifier set is collected. The marker after `;` is cut first, because a version inside it is a condition and not a pin.
|
Correct, fixed in the head commit, reproduced first.
The marker is cut before the scan. A version inside Positive control, the eight live pin files: seven consumer pins read unchanged, and this repo's own 1803 passed, 2 skipped, ruff clean. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04c4443355
ℹ️ 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".
| found_here = _exact_versions(declaration, match.start()) or [match.group(1)] | ||
| pins.extend(v for v in found_here if v not in pins) |
There was a problem hiding this comment.
When equivalent PEP 440 spellings occur together, such as commoner-probe==0.15,==0.15.0, both specifiers select the same normalized version, but this raw-string deduplication retains two entries and reports conflict: 0.15, 0.15.0. doctor therefore exits 1 for a valid, satisfiable requirement whose exact pins agree; compare normalized versions before deciding that multiple collected values conflict.
Useful? React with 👍 / 👎.
1 participant
Found by running
commoner-probe doctoragainst the seven live consumer pin files during a/pmrecompute. It reported one pin where seven exist.Two assumptions were wrong:
pyproject.tomlwriting it quoted inside a dependency list read as no pin at all;commoner-probe[http,pdf]==0.14.3— the form four of the seven use — matched nothing.A file that names the package with no exact version now reports
unpinnedrather than nothing. The org pins with==or@vX.Y.Z, because a range moves under the consumer without anyone deciding to move it, and returning nothing filed that beside the files that never mention the package at all.The census it now produces
None on 0.15.0, which is unreleased.
Verification
1727 passed, ruff clean. Five new tests, each written from a form measured in a real consumer file rather than imagined.