Conversation
The ty 0.0.56 release broke estate repositories whose CI installed ty unpinned (falcon-correlate and polythene mains were red for days). Pin every ty installation site to 0.0.56 so upgrades become deliberate: - the parent Makefile's typecheck target now runs uvx ty@0.0.56; - the templated dev dependency group now declares ty==0.0.56, which pins the generated Makefile's `uv run ty` invocations; - the contract tests assert the pinned forms. Both rendered toggle states resolve ty 0.0.56 and pass `make typecheck`, and the template's own gates pass with the pin in place.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 36 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 (1)
WalkthroughPin ChangesPin ty dependency version
Compact metadata: 7 files changed, +56/-2 lines. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Address review feedback on the ty pin: - document the ty==0.0.56 pin in the parent developers' guide and the generated developers' guide, including why it exists (unpinned installations broke falcon-correlate and polythene mains when ty 0.0.56 landed) and the policy that bumps are deliberate (update the pin and fix any new diagnostics in one pull request); - record the decision as ADR-003, following the repository's existing ADR convention; - add an integration test that runs the parent `make typecheck` with a fake uvx shim on PATH and asserts the captured invocation includes the pinned ty@0.0.56 token (POSIX-only, skipped on Windows), keeping the existing string-contract assertions.
|
Both review findings addressed in the latest push ( Finding 1 (documentation) — valid, fixed. The ty==0.0.56 pin is now documented in Finding 2 (behavioural coverage) — valid, fixed. Added One nearby observation assessed as out of scope: Validation: new test passes in isolation; full contract suite 34 passed, 2 skipped (act-gated); |
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (2 warnings)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== ADR files ==="
fd -i 'adr-00' docs
echo "=== developers-guide.md ty mentions ==="
rg -n 'ty' docs/developers-guide.md | rg -i 'ty(@|==|check|0\.0\.56|pin)'
echo "=== template developers-guide.md ty mentions ==="
rg -n 'ty' template/docs/developers-guide.md | rg -i 'ty(@|==|check|0\.0\.56|pin)'
echo "=== ADR-003 content ==="
fd -i 'adr-003' docs -x cat {}
echo "=== test for pinned ty invocation ==="
rg -n 'test_parent_makefile_typecheck_invokes_pinned_ty' tests/test_helpers.py -A 40
echo "=== git log for commit 3575221 ==="
git log --oneline -5Length of output: 4709 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '480,545p' tests/test_helpers.pyLength of output: 2480
Developer Documentation: Verified. Testing (Unit and Behavioural): Verified. No further work is required for these two warnings. No follow-up issue needed. The MD013 over-length line you flagged as out-of-scope in ᕮ(•ᴥ•)ᕮ |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/helpers/pyproject_contracts.py (1)
35-47: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the dev dependency assertion exact.
dependency in dev_dependenciesstill passes if an unpinnedtystays alongsidety==0.0.56, so this contract can drift without failing.♻️ Tighten the assertion
- for dependency in [ - "pytest", - "interrogate", - "pip-audit", - "ruff", - "pyright", - "ty==0.0.56", - "pytest-timeout", - "pytest-xdist", - ]: - assert dependency in dev_dependencies, ( - f"expected generated dev dependencies to include {dependency}" - ) + expected_dev_dependencies = { + "pytest", + "interrogate", + "pip-audit", + "ruff", + "pyright", + "ty==0.0.56", + "pytest-timeout", + "pytest-xdist", + } + assert set(dev_dependencies) == expected_dev_dependencies, ( + "expected generated dev dependencies to match the pinned contract" + ) + assert len(dev_dependencies) == len(expected_dev_dependencies), ( + "expected generated dev dependencies to contain no duplicates" + )🤖 Prompt for 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. In `@tests/helpers/pyproject_contracts.py` around lines 35 - 47, The dev dependency contract in the helper assertion is too loose because `dependency in dev_dependencies` allows extra or duplicate entries like an unpinned `ty` to slip through. Update the check in `tests/helpers/pyproject_contracts.py` to use an exact match against the full expected set for `dev_dependencies`, and add a duplicate/length guard so the generated list must match the pinned contract precisely when validating the `dev_dependencies` collection.
🤖 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.
Outside diff comments:
In `@tests/helpers/pyproject_contracts.py`:
- Around line 35-47: The dev dependency contract in the helper assertion is too
loose because `dependency in dev_dependencies` allows extra or duplicate entries
like an unpinned `ty` to slip through. Update the check in
`tests/helpers/pyproject_contracts.py` to use an exact match against the full
expected set for `dev_dependencies`, and add a duplicate/length guard so the
generated list must match the pinned contract precisely when validating the
`dev_dependencies` collection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ca2b7544-dd43-4973-8a4d-3a0b4a680d5d
📒 Files selected for processing (7)
Makefiledocs/adr-003-pin-ty-version.mddocs/developers-guide.mdtemplate/docs/developers-guide.mdtemplate/pyproject.toml.jinjatests/helpers/pyproject_contracts.pytests/test_helpers.py
Replace the loose membership check on `dev_dependencies` with an exact set equality assertion plus a duplicate-count guard. The old `dependency in dev_dependencies` loop would silently pass if an extra unpinned `ty` entry co-existed alongside `ty==0.0.56`. The new assertion requires the rendered list to match the pinned contract precisely — no extra entries, no duplicates.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
2 participants
Rationale
ty 0.0.56 broke estate repositories whose CI installed ty unpinned (
uv tool install ty,uvx ty, unpinned dev-dependencies): falcon-correlate and polythene mains were red for days. This pins every ty site in the template repository — both the template tree that generated repositories inherit and the repository's own gates — to0.0.56, so future upgrades are deliberate rather than incidental.Sites changed
Makefile(repository's own typecheck gate):uvx … ty check tests/→uvx … ty@0.0.56 check tests/.template/pyproject.toml.jinja(inherited by generated repositories): dev group"ty"→"ty==0.0.56". This also pins the generated Makefile'suv run ty --version/uv run ty checkinvocations, which resolve from the dev group; no change totemplate/Makefile.jinjais needed. The templated CI (ci.yml.jinja) typechecks viamake typecheck, so it is covered by the same pin; its "Install CLI tools" step does not install ty.tests/test_helpers.pyandtests/helpers/pyproject_contracts.py: contract assertions updated to expect the pinned forms.Validation
make check-fmt,make lint,make typecheck(now running ty 0.0.56: "All checks passed!"), and the full contract suitemake test(33 passed, 2 skipped, 3 snapshots passed).use_rust=falseanduse_rust=true): each resolves ty 0.0.56 and passesmake typecheck(ty check my_project tests) cleanly, so the pin surfaces no ty 0.0.56 failures in generated projects.Note for maintainers
Future ty bumps should be deliberate: update the pin in the parent
Makefile, the templated dev group, and the contract assertions together, and re-render both toggle states before merging.🤖 Generated with Claude Code