ci: add GitHub Actions workflow and fix packaging metadata - #6
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change adds GitHub Actions checks for linting, testing, and packaging; introduces distribution-content validation; updates package metadata and project status documentation; and applies formatting-only edits to source and test files. ChangesCI and package readiness
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant uv
participant QualityTools
participant Pytest
participant PackageArtifacts
GitHubActions->>uv: install and sync locked dependencies
uv->>QualityTools: run ruff and mypy
uv->>Pytest: run tests across Python versions
uv->>PackageArtifacts: build distributions
GitHubActions->>PackageArtifacts: verify and upload artifacts
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/ci.yml:
- Around line 3-25: Restrict the workflow’s default token permissions by adding
workflow-level contents read access, and update every actions/checkout@v4
invocation to disable persisted credentials with persist-credentials: false.
Apply these changes to the workflow configuration without altering job behavior.
In `@AGENTS.md`:
- Line 155: Update the CI documentation entry in AGENTS.md to state that the
workflow enforces dependency locking with UV_LOCKED=1, replacing the inaccurate
UV_FROZEN=1 reference. Preserve the existing description of CI jobs and
triggers, and accurately describe the resulting stale-lock behavior without
adding unrelated changes.
In `@scripts/check_dist.py`:
- Around line 93-98: Extend the required-member validation in the check_dist
logic to inspect the sdist member list as well as wheel_members. Reuse
REQUIRED_WHEEL_MEMBERS (or the appropriate shared required-resource set) and
append clear problems for missing py.typed and codes/_data/*.json resources in
the sdist, while preserving the existing wheel checks.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c6f212d8-5851-4138-a353-5f6aaff558ff
📒 Files selected for processing (8)
.github/workflows/ci.ymlAGENTS.mdpyproject.tomlscripts/check_dist.pysrc/myinvois/_async_client.pysrc/myinvois/services/async_document_types.pysrc/myinvois/services/models.pytests/unit/test_async_client.py
Phase 6b, part 1. Adds CI and closes the packaging issues it surfaced. CI (.github/workflows/ci.yml), three jobs: - lint: ruff check, ruff format --check, mypy src/myinvois - test: pytest across Python 3.11/3.12/3.13, -m "not live" - package: uv build + scripts/check_dist.py, uploads dists as an artifact UV_LOCKED=1 so a stale uv.lock fails the build instead of silently resolving something other than what developers run locally. Negative-tested by adding a dependency without re-locking; the gate fires as intended. (UV_LOCKED, not UV_FROZEN -- --frozen skips re-locking without checking.) Packaging metadata: - [project.urls] shipped placeholder https://github.com/your-org/myinvois into the wheel METADATA. Corrected to danieyal/myinvois-python. This would have gone out with the first PyPI release. - Development Status classifier 3 - Alpha -> 4 - Beta, matching the README. scripts/check_dist.py guards two unrecoverable release mistakes, since a PyPI version can never be reused: - py.typed and codes/_data/*.json only ship because of the data-includes entry. If that regresses the build still succeeds, but myinvois.codes raises at runtime on a wheel install and type-checkers ignore the package. - The test signing key/cert must never ship. They are force-tracked in git for the byte-parity tests, which makes an accidental include plausible. Negative-tested: tampering a wheel (drop py.typed, add a .pem) produces both failures with actionable messages. CERTIFY_BEFORE_PUBLIC from the Phase 4 notes is now resolved rather than merely observed -- verified that neither the wheel nor the sdist ships tests/ or tests/fixtures/cert/*, and check_dist.py enforces it in CI. Formatting drift, fixed here because CI would otherwise be red on arrival: ruff format --check failed on 4 pre-existing files on master (_async_client.py, services/async_document_types.py, services/models.py, tests/unit/test_async_client.py). All four were wrapped at ruff's default 88 columns instead of this project's configured line-length = 100, i.e. Phase 6a was committed without ruff format ever being run against the project config. The fix is pure line-rejoining; no semantic change. That also means AGENTS.md had been asserting "ruff check, ruff format --check, mypy src all clean" while the format gate was red. AGENTS.md now records the drift and warns against copying a green-gates claim forward without re-running the commands. All four gates verified green locally: ruff check, ruff format --check, mypy src/myinvois, and 284 tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
danieyal
force-pushed
the
chore/ci-and-packaging
branch
from
July 22, 2026 02:27
63ddaf1 to
d0bf781
Compare
Contributor
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 @.github/workflows/ci.yml:
- Around line 13-18: Set UV_FROZEN to "1" specifically in the package job’s
environment, while retaining the global UV_LOCKED setting for lint and test
jobs. Override or scope the package job environment so its distribution
validation uses frozen mode instead of inheriting UV_LOCKED.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c79a275-c5b8-4847-88ca-5a623ee49b98
📒 Files selected for processing (8)
.github/workflows/ci.ymlAGENTS.mdpyproject.tomlscripts/check_dist.pysrc/myinvois/_async_client.pysrc/myinvois/services/async_document_types.pysrc/myinvois/services/models.pytests/unit/test_async_client.py
🚧 Files skipped from review as they are similar to previous changes (7)
- src/myinvois/services/async_document_types.py
- pyproject.toml
- src/myinvois/_async_client.py
- src/myinvois/services/models.py
- tests/unit/test_async_client.py
- scripts/check_dist.py
- AGENTS.md
All three CodeRabbit findings were valid. 1. Workflow permissions (.github/workflows/ci.yml) Added workflow-level `permissions: contents: read` and `persist-credentials: false` on all three checkout steps. Nothing in CI writes to the repo, and the workflow-level declaration also overrides a permissive repo/org default rather than inheriting it. 2. Stale AGENTS.md claim The GITHUB section still documented `UV_FROZEN=1`. That was written before I corrected the workflow to `UV_LOCKED=1` and never updated, so AGENTS.md described a guarantee the workflow does not make -- ironically the exact stale-doc failure the FORMATTING DRIFT note warns about. Now states UV_LOCKED and why --frozen is the wrong flag. 3. sdist required-member check (scripts/check_dist.py) Required data members were only validated in the wheel. `pip install --no-binary` builds from the sdist, so a data-includes regression there breaks installs just as badly and would have gone unnoticed. Renamed REQUIRED_WHEEL_MEMBERS -> REQUIRED_DATA_MEMBERS and checked both distributions, accounting for the sdist keeping the repo's `src/` prefix. Negative-tested: stripping py.typed and msic.json from the sdist alone now fails with both paths named, where previously it passed silently. Not addressed: the "Docstring Coverage 0.00%" pre-merge warning. The module has a full docstring explaining what it guards and why; the four remaining symbols are private helpers whose names say what they do, which matches the convention in scripts/extract_codes.py. Adding docstrings to satisfy a threshold would be noise. Gates green: ruff check, ruff format --check, mypy src/myinvois, 284 tests, and the dist check against a fresh build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 6b, part 1. Stacked on #5 — merge that first, and this PR's base retargets to
masterautomatically. The diff shown here is only the CI/packaging work.CI
.github/workflows/ci.yml, three jobs:ruff check,ruff format --check,mypy src/myinvoispytest -m "not live"across Python 3.11 / 3.12 / 3.13uv build+scripts/check_dist.py, uploads the dists as an artifactUV_LOCKED=1so a staleuv.lockfails the build rather than silently resolving something other than what we run locally. NoteUV_LOCKED, notUV_FROZEN—--frozenskips re-locking without checking, which is not the guarantee we want.Packaging bugs this surfaced
[project.urls]shipped placeholders.https://github.com/your-org/myinvoiswas being baked into the wheel METADATA and would have gone out with the first PyPI release. Corrected todanieyal/myinvois-python.Development Statusclassifier3 - Alpha→4 - Beta, matching the README.scripts/check_dist.py
Guards two mistakes that are unrecoverable once published, since a PyPI version can never be reused:
py.typedandcodes/_data/*.jsonship only because of thedata-includesentry. If that regresses the build still succeeds, butmyinvois.codesraises at runtime on a wheel install and type-checkers silently ignore the package.This also closes
CERTIFY_BEFORE_PUBLICfrom the Phase 4 notes — previously an open worry, now verified (neither wheel nor sdist shipstests/ortests/fixtures/cert/*) and enforced in CI.Formatting drift — please read
ruff format --checkwas already failing on 4 files onmasterbefore this PR:_async_client.py,services/async_document_types.py,services/models.py,tests/unit/test_async_client.py. All four were wrapped at ruff's default 88 columns instead of this project's configuredline-length = 100— i.e. Phase 6a (#4) was merged withoutruff formatever being run against the project config. CI would have been red on arrival, so the fix is included here. It is pure line-rejoining, no semantic change.Consequence worth noting: AGENTS.md had been asserting "
ruff check,ruff format --check,mypy srcall clean" while the format gate was red, and I repeated that claim in #5. AGENTS.md now records the drift and warns against copying a green-gates claim forward without re-running the commands.Verification
Both new guards were negative-tested rather than assumed to work:
py.typed, add a.pem) → bothcheck_dist.pyfailure classes fire with actionable messages.UV_LOCKEDgate fires.All four gates green locally:
ruff check,ruff format --check,mypy src/myinvois, 284 tests.Still TODO in Phase 6b
PyPI Trusted Publishing (gated on the
packagejob — outward-facing, so worth confirming before the first publish), the remaining 7 document types, live sandbox verification, and the async test coverage gap noted in #5.🤖 Generated with Claude Code
Summary by CodeRabbit