[codex] Add Python feature mapping#2
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: 0cc9ed57a9
ℹ️ 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".
xiamx
force-pushed
the
codex/python-mapper-support
branch
from
0cc9ed5 to
99d8316
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65c5339720
ℹ️ 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 (await pathExists(join(root, "pdm.lock"))) { | ||
| return "pdm"; | ||
| } | ||
| return null; |
There was a problem hiding this comment.
detectPackageManagers classifies repos with hatch.toml as using the hatch package manager, but pythonRunner only recognizes uv, poetry, and pdm, so defaults like pytest/ruff are emitted without a runner prefix. In Hatch-managed projects where tools are only available inside Hatch environments, these suggested commands will fail even though manager detection succeeded; this makes command detection internally inconsistent for the same repo.
Useful? React with 👍 / 👎.
| const toolMatch = /^\s*\[(mypy|pyright|ruff)\]/mu.exec(source); | ||
| if (toolMatch?.[1] !== undefined) { |
There was a problem hiding this comment.
This reads only the first [mypy|pyright|ruff] section in setup.cfg because exec is used once on the whole file. If [mypy] appears before [ruff] (or vice versa), later tool sections are ignored, which can suppress expected defaults (for example, missing Ruff lint/format commands despite an explicit [ruff] section). Iterating all matches or checking each tool independently avoids this false negative.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43b2147fee
ℹ️ 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 (await pathExists(join(root, "pdm.lock"))) { | ||
| return "pdm run pytest"; | ||
| } | ||
| return "pytest"; |
There was a problem hiding this comment.
pythonTestCommand only special-cases uv.lock, poetry.lock, and pdm.lock, then falls back to plain pytest. In Hatch projects (hatch.toml present), detectProject already reports hatch run pytest, but Python feature seeds (CLI/source/test-suite tests fields) will still carry bare pytest, which can fail when pytest is only available inside Hatch environments. Add a Hatch branch here so mapper test commands stay executable and consistent with detected defaults.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ebc27108e
ℹ️ 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".
| for (const file of ["requirements.txt", "setup.cfg"]) { | ||
| if (await pathExists(join(root, file))) { | ||
| const source = await readFile(join(root, file), "utf8"); | ||
| for (const dependency of pythonRequirementNames(source)) { | ||
| info.dependencies.add(dependency); |
There was a problem hiding this comment.
The setup.cfg path currently runs every line through pythonRequirementNames, so extras group keys (for example [options.extras_require] entries like ruff = ... or pytest = ...) are treated as installed dependencies even when those packages are not declared. In that case pythonDefaultCommands will incorrectly enable ruff/pytest defaults for projects that only define similarly named extras groups, which violates the intended conservative command detection.
Useful? React with 👍 / 👎.
| async function dependencyFileHas(root: string, dependency: string): Promise<boolean> { | ||
| for (const file of ["requirements.txt", "setup.cfg"]) { | ||
| if (!(await pathExists(join(root, file)))) { | ||
| continue; | ||
| } | ||
| const source = await readFile(join(root, file), "utf8"); | ||
| if (requirementNames(source).has(dependency)) { |
There was a problem hiding this comment.
The mapper’s dependencyFileHas checks setup.cfg with requirementNames, which also interprets extras group keys as dependencies. A project with [options.extras_require] and a group named pytest will get pytest test commands in mapped features even if pytest is not installed or configured, causing non-executable suggested test commands.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6b42d397b
ℹ️ 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".
| [...dirs].some((dir) => pathMatchesPrefix(test, dir)) || | ||
| (fileStems.has(testStem) && /^(tests?|__tests__)\//u.test(test)) |
There was a problem hiding this comment.
associatedTests misses root-level pytest files for root modules because dirname("app.py") is ".", and pathMatchesPrefix(test, ".") never matches paths like test_app.py; the fallback stem check also requires tests to live under tests?/. In projects with flat layouts and console scripts targeting root modules, this drops obvious colocated tests from the CLI/source feature context.
Useful? React with 👍 / 👎.
| for (const dependency of pythonRequirementNames(source)) { | ||
| info.dependencies.add(dependency); | ||
| } | ||
| if (/^\s*\[tool:pytest\]|\[pytest\]/mu.test(source)) { |
There was a problem hiding this comment.
The pytest-config check uses ^\s*\[tool:pytest\]|\[pytest\], where only the first alternative is start-anchored. Any incidental "[pytest]" text (for example in comments or sample snippets) sets hasPytestConfig = true, which can incorrectly enable default pytest commands in projects that are not actually configured for pytest.
Useful? React with 👍 / 👎.
2 participants
Summary
Adds first-pass Python support to clawpatch feature mapping.
This includes:
pyproject.toml, setup files, requirements, and common Python source layoutssrcroots, and colocated testsNotes
Framework-specific Python route mapping for FastAPI, Flask, and Django remains out of scope for this first pass.
Validation
corepack pnpm typecheckcorepack pnpm lintcorepack pnpm format:checkcorepack pnpm test