Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Summary
WalkthroughPin ChangesTy version pinning
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
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.
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
67f61f4 to
fad8d44
Compare
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.
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.
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 Have the following now been resolved? tests/helpers/pyproject_contracts.py (1)
♻️ 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"
+ )🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline tests/helpers/pyproject_contracts.py --items all --type function
printf '\n=== development-dependency assertions ===\n'
rg -n -C 12 'expected_dev_dependencies|dev_dependencies|ty==0\.0\.56' \
tests/helpers/pyproject_contracts.pyLength of output: 2164
An unpinned |
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 `@tests/test_helpers.py`:
- Around line 576-583: Update the subprocess invocation in the test helper to
resolve make through shutil.which before calling subprocess.run. Assert that the
resolved executable is available, then pass the absolute path with the existing
typecheck argument while preserving the current process options.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 9a9bcaf5-eadd-401b-9a8e-0496d47041b0
📒 Files selected for processing (7)
Makefiledocs/adr-004-pin-ty-version.mddocs/developers-guide.mdtemplate/docs/developers-guide.mdtemplate/pyproject.toml.jinjatests/helpers/pyproject_contracts.pytests/test_helpers.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)leynos/cuprum(auto-detected)leynos/mdtablefix(auto-detected)
The parent-Makefile typecheck boundary test spawned `make` by bare name, leaving executable resolution to the subprocess layer at spawn time. Resolve it through `shutil.which` first, assert it is present, and pass the absolute path, so the test controls which binary runs rather than relying on implicit PATH lookup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
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