Prescribe the uv --script shebang and test it - #38
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 57 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fecccd8706
ℹ️ 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".
| f"{CORRECT_SHEBANG}\n" | ||
| "# /// script\n" | ||
| '# requires-python = ">=3.13"\n' | ||
| '# dependencies = ["packaging"]\n' |
There was a problem hiding this comment.
Avoid resolving PyPI during this unit test
In network-restricted CI or local runs where uv is installed but PyPI is unavailable, this test fails before it can validate the shebang: the temp script declares packaging, so uv run --script has to resolve the package from the default PyPI index at execution time. I reproduced python -m pytest tests/test_scripting_shebang.py -q failing with Failed to fetch: https://pypi.org/simple/packaging/; use a local path dependency or skip when dependency resolution cannot run rather than making the parent test suite depend on live PyPI.
Useful? React with 👍 / 👎.
The scripting standards document and the two generate_typos_config.py scripts (root and template copies) shebanged their PEP 723 script blocks with `#!/usr/bin/env -S uv run python`. That form executes the interpreter directly and silently ignores the inline metadata block, so a directly invoked script fails at import time because its declared dependencies were never installed. `#!/usr/bin/env -S uv run --script` reads the metadata block and installs the dependencies first, which is what every PEP 723 script actually needs. Fix all three occurrences and add a documented rationale to the scripting standards guide. Add tests/test_scripting_shebang.py, which asserts no file in the repository or template tree ships the broken shebang above a PEP 723 block, and behaviourally proves the correct shebang installs and imports a declared dependency when the script is executed directly.
fecccd8 to
8fd8bfb
Compare
Summary
#!/usr/bin/env -S uv run pythonexecutes the interpreter directly andignores the PEP 723 inline metadata block, so a directly invoked script
fails at import time because its declared dependencies were never
installed. Verified empirically: a script with
dependencies = ["cyclopts>=2.9"]fails atimport cycloptsunder the old form andsucceeds under
-S uv run --script.block:
scripts/generate_typos_config.py, its template copytemplate/scripts/generate_typos_config.py, and theworked example in
template/docs/scripting-standards.md.Added a short rationale to the standards doc explaining why
--scriptisrequired.
tests/test_scripting_shebang.pywith two tests:a static guard asserting no file in the repository or Copier template tree
ships the broken shebang above a PEP 723 block, and a behavioural test that
writes a temporary script with the correct shebang and a one-dependency
PEP 723 block, executes it directly, and asserts the dependency imports and
the process exits
0. The behavioural test skips gracefully whenuvisunavailable.
Test plan
make check-fmt— passedmake lint— passed (Ruff and Interrogate)make typecheck— passedmake spelling— passedmake test— 46 passed, 2 skipped, 1 known pre-existing failure(
tests/test_typos_rollout.py::test_committed_config_matches_current_shared_dictionary,caused by upstream shared-dictionary drift unrelated to this change)