Repository navigation
Hold the package to the promises it already makes - #4
Conversation
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe changes strengthen wheel runtime and type-hint validation, parameterize Trusted Publisher verification for PyPI and TestPyPI, add release and contribution documentation, expose changelog metadata, and tighten pytest warning handling and test typing. ChangesPackage validation
Trusted Publisher verification
Release documentation and metadata
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant WorkflowInput
participant GitHubActions
participant PackageIndex
participant GitHubOIDC
WorkflowInput->>GitHubActions: select publishing environment
GitHubActions->>PackageIndex: fetch environment-specific OIDC audience
GitHubActions->>GitHubOIDC: request token with audience
GitHubOIDC-->>GitHubActions: return claims
GitHubActions-->>WorkflowInput: print selected-index claims
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
be8fa11 to
3061bcb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@RELEASING.md`:
- Around line 36-47: Update the “Verify publish setup” instructions in
RELEASING.md to require running the workflow for both targets: retain the `pypi`
verification and add the equivalent verification with `target=testpypi` before
the rehearsal. Ensure the steps clearly indicate each index’s publisher
configuration must be checked independently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9840936f-6528-4e3f-9831-b36408d7d6a4
📒 Files selected for processing (6)
.github/workflows/ci.yml.github/workflows/verify-publish.ymlREADME.mdRELEASING.mdpyproject.tomltests/test_core.py
The package ships py.typed, and that file is the whole of the promise: it is what tells a downstream mypy the annotations are real. twine validates metadata, not contents, and passes a wheel missing it without a word. Losing it does not fail loudly either -- mypy stops analyzing compactref, falls back to Any, and a project with ignore_missing_imports set, which is most of them, reports "Success: no issues found" on code calling generate_reference with a list. The hints are gone and nothing says so. So CI installs the built wheel where there is no source tree to fall back on, makes it produce a reference, and makes mypy reject a call the annotations forbid. That check is run with --ignore-missing-imports and greps for the arg-type diagnostic by name: a non-zero exit does not distinguish "rejected the bad call" from "could not read the module at all", and written the obvious way the check passed against a wheel with py.typed stripped out -- the exact wheel it exists to reject. verify-publish could only ever check PyPI. The environment was hardcoded and the OIDC token requested with audience=pypi, so the workflow written to debug Trusted Publishing was structurally unable to debug the TestPyPI publisher -- which is the one that failed. It takes the index as a dispatch input now, runs in the matching environment, and reads each index's audience from its own /_/oidc/audience endpoint rather than assuming. RELEASING.md writes down what the workflows enforce but nobody could read: the version lives in pyproject.toml and __version__ and both must match the tag; publishing is triggered by a GitHub Release, not by a tag; rehearse on TestPyPI first because PyPI versions are immutable; and the two indexes are separate services whose publisher configs and environment claims do not carry over. Verify publish setup is therefore run twice, once per index -- checking only PyPI leaves the rehearsal to fail on an unconfigured TestPyPI publisher, and checking only TestPyPI leaves the same trap waiting on the release, where it costs far more. The two workflows name that input differently, `environment` and `target`, which the document now says out loud rather than leaving to be discovered. Warnings are errors under pytest. The package has no runtime dependencies, so a warning during a test run is either this code or the standard library deprecating something out from under it. The README gains a changelog section, stating that 0.1.0 references still resolve identically under 0.2.0, and a Changelog project URL so PyPI links it in the sidebar. The 0.1.0 compatibility test now types its source as SourceIdentifier rather than object, dropping a type: ignore that only existed because the annotation was wrong.
3061bcb to
b7e0643
Compare
1 participant
Three guards, all for claims compactref makes and nothing checked.
The package ships py.typed, and that file is the whole of the promise: it is what tells a downstream mypy the annotations are real. twine validates metadata, not contents, and passes a wheel missing it without a word. Losing it does not fail loudly either -- mypy stops analyzing compactref, falls back to Any, and a project with ignore_missing_imports set, which is most of them, reports "Success: no issues found" on code calling generate_reference with a list. The hints are gone and nothing says so.
So CI installs the built wheel where there is no source tree to fall back on, makes it produce a reference, and makes mypy reject a call the annotations forbid. That check is run with --ignore-missing-imports and greps for the arg-type diagnostic by name: a non-zero exit does not distinguish "rejected the bad call" from "could not read the module at all", and written the obvious way the check passed against a wheel with py.typed stripped out -- the exact wheel it exists to reject. Both directions are tested now.
Warnings are errors under pytest. The package has no runtime dependencies, so a warning during a test run is either this code or the standard library deprecating something out from under it. random-address was one importlib.abc import away from a broken 3.14 job with the DeprecationWarning sitting unread in the pytest output; the same trap was open here.
The parametrized source in the 0.1.0 compatibility test was typed object, which is not what generate_reference accepts, so the call needed a type: ignore to get past mypy. SourceIdentifier is exported for exactly this. The test now checks the signature it exercises rather than opting out of it. The one remaining suppression stays: test_rejects_unsupported_source_type passes a list on purpose to prove the TypeError.
Summary by CodeRabbit
Documentation
Quality Improvements